Merge lp:~rainct/zeitgeist/matches-template into lp:~zeitgeist/zeitgeist/bluebird

[Siegfried Gevatter]

[Michal Hruby (mhr3)]

115 + // FIXME: change this to va_list once Vala bug #647097 is fixed
116 + public Event.full (string? interpretation=null,
117 + string? manifestation=null, string? actor=null,
118 + string? origin=null, GenericArray<Subject>? subjects=null)

This is very close to public API, I'd prefer this didn't change, so if we want a variant with a PtrArray, let's call it something else (for example full_from_ptrarray)

328 + // FIXME: figure out how we want to treat multiple subjects in the template

Same way engine does, ie AND them within one event.

review: Needs Fixing

[Siegfried Gevatter (rainct)]

2012/2/17 Michal Hruby <email address hidden>:
> This is very close to public API, I'd prefer this didn't change, so if we want a variant with a PtrArray, let's call it something else (for example full_from_ptrarray)

I don't want PtrArray, it's just a workaround since Vala is br0ken.

> 328     +    // FIXME: figure out how we want to treat multiple subjects in the template
> Same way engine does, ie AND them within one event.

Yeah, that's not what I mean, but rather: if there's two empty
subjects, does that mean anything (ie. I want events with exactly two
subject) or are they just ignored? But I retract the question, the
former would just be confusing.

[Michal Hruby (mhr3)]

> I don't want PtrArray, it's just a workaround since Vala is br0ken.

Then don't add that method yet.

[lp:~rainct/zeitgeist/matches-template]

406. By Siegfried Gevatter on 2012-02-18

Leave va_list there but don't use it.

[Michal Hruby (mhr3)]

review: Approve

=== modified file 'src/datamodel.vala'
--- src/datamodel.vala	2012-02-15 18:27:21 +0000
+++ src/datamodel.vala	2012-02-18 21:37:18 +0000
@@ -4,6 +4,8 @@
  *             By Seif Lotfy <[e-mail]>
  *             By Siegfried-Angel Gevatter Pujals <[e-mail]>
  * Copyright © 2011 Manish Sinha <[e-mail]>
+ * Copyright © 2012 Canonical Ltd.
+ *             By Siegfried-A. Gevatter <[e-mail]>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU Lesser General Public License as published by
@@ -262,15 +264,18 @@
         ANY             = 2  // The event subjects may or may not be available
     }
 
-    private bool check_field_match (string property,
-            string template_property, bool is_symbol = false,
+    private bool check_field_match (string? property,
+            string? template_property, bool is_symbol = false,
             bool can_wildcard = false)
     {
         var matches = false;
+        var is_negated = false;
         var parsed = template_property;
-        var is_negated = Utils.parse_negation (ref parsed);
-
-        if (parsed == "")
+
+        if (parsed != null)
+            is_negated = Utils.parse_negation (ref parsed);
+
+        if (Utils.is_empty_string (parsed))
         {
             return true;
         }
@@ -278,14 +283,15 @@
         {
             matches = true;
         }
-        else if (is_symbol &&
+        else if (is_symbol && property != null &&
             Symbol.get_all_parents (property).find_custom (parsed, strcmp) != null)
         {
             matches = true;
         }
         else if (can_wildcard && Utils.parse_wildcard (ref parsed))
         {
-            if (property.has_prefix (parsed)) matches = true;
+            if (property != null && property.has_prefix (parsed))
+                matches = true;
         }
 
         return (is_negated) ? !matches : matches;
@@ -297,27 +303,27 @@
 
         public uint32    id { get; set; }
         public int64     timestamp { get; set; }
-        public string    origin { get; set; }
+        public string?   origin { get; set; }
 
-        public string actor
+        public string? actor
         {
             get { return _actor; }
-            set { _actor = url_store.insert_const (value); }
+            set { _actor = (value != null) ? url_store.insert_const (value) : null; }
         }
-        public string interpretation
+        public string? interpretation
         {
             get { return _interpretation; }
-            set { _interpretation = url_store.insert_const (value); }
+            set { _interpretation = (value != null) ? url_store.insert_const (value) : null; }
         }
-        public string manifestation
+        public string? manifestation
         {
             get { return _manifestation; }
-            set { _manifestation = url_store.insert_const (value); }
+            set { _manifestation = (value != null) ? url_store.insert_const (value) : null; }
         }
 
-        private unowned string _actor;
-        private unowned string _interpretation;
-        private unowned string _manifestation;
+        private unowned string? _actor;
+        private unowned string? _interpretation;
+        private unowned string? _manifestation;
 
         public GenericArray<Subject> subjects { get; set; }
         public ByteArray? payload { get; set; }
@@ -342,6 +348,24 @@
             subjects.add (subject);
         }
 
+        public Event.full (string? interpretation=null,
+            string? manifestation=null, string? actor=null,
+            string? origin=null, ...)
+        {
+            this.interpretation = interpretation;
+            this.manifestation = manifestation;
+            this.actor = actor;
+            this.origin = origin;
+
+            // FIXME: We can't use this until Vala bug #647097 is fixed
+            /*
+            var subjects = va_list ();
+            unowned Subject subject;
+            while ((subject = subjects.arg ()) != null)
+                add_subject (subject);
+            */
+        }
+
         public Event.from_variant (Variant event_variant) throws EngineError {
             assert_sig (event_variant.get_type_string () == "(" +
                 Utils.SIG_EVENT + ")", "Invalid D-Bus signature.");
@@ -560,39 +584,53 @@
     {
         private static StringChunk url_store;
 
-        public string uri { get; set; }
-        public string origin { get; set; }
-        public string text { get; set; }
-        public string storage { get; set; }
+        public string? uri { get; set; }
+        public string? origin { get; set; }
+        public string? text { get; set; }
+        public string? storage { get; set; }
         // FIXME: current_uri is often the same as uri, we don't need to waste
         // memory for it
-        public string current_uri { get; set; }
+        public string? current_uri { get; set; }
 
-        public string mimetype
+        public string? mimetype
         {
             get { return _mimetype; }
-            set { _mimetype = url_store.insert_const (value); }
+            set { _mimetype = (value != null) ? url_store.insert_const (value) : null; }
         }
-        public string interpretation
+        public string? interpretation
         {
             get { return _interpretation; }
-            set { _interpretation = url_store.insert_const (value); }
+            set { _interpretation = (value != null) ? url_store.insert_const (value) : null; }
         }
-        public string manifestation
+        public string? manifestation
         {
             get { return _manifestation; }
-            set { _manifestation = url_store.insert_const (value); }
+            set { _manifestation = (value != null) ? url_store.insert_const (value) : null; }
         }
 
-        private unowned string _mimetype;
-        private unowned string _interpretation;
-        private unowned string _manifestation;
+        private unowned string? _mimetype;
+        private unowned string? _interpretation;
+        private unowned string? _manifestation;
 
         static construct
         {
             url_store = new StringChunk (4096);
         }
 
+        public Subject.full (string? uri=null,
+            string? interpretation=null, string? manifestation=null,
+            string? mimetype=null, string? origin=null, string? text=null,
+            string? storage=null, string? current_uri=null)
+        {
+            this.interpretation = interpretation;
+            this.manifestation = manifestation;
+            this.mimetype = mimetype;
+            this.origin = origin;
+            this.text = text;
+            this.storage = storage;
+            this.current_uri = current_uri;
+        }
+
         public Subject.from_variant (Variant subject_variant)
             throws EngineError
         {

=== modified file 'test/direct/Makefile.am'
--- test/direct/Makefile.am	2012-02-13 19:43:15 +0000
+++ test/direct/Makefile.am	2012-02-18 21:37:18 +0000
@@ -6,9 +6,11 @@
 	--pkg gmodule-2.0 \
 	$(srcdir)/assertions.vapi \
 	--Xcc=-w \
+	-g \
 	$(NULL)
 
 TESTS = \
+	datamodel-test \
 	marshalling-test \
 	mimetype-test \
 	query-operators-test \
@@ -36,6 +38,9 @@
 	$(top_srcdir)/src/mimetype.vala \
 	$(NULL)
 
+datamodel-test: datamodel-test.vala $(SRC_FILES)
+	$(VALA_V)$(VALAC) $(VALAFLAGS) -o $@ $^
+
 marshalling-test: marshalling-test.vala $(SRC_FILES)
 	$(VALA_V)$(VALAC) $(VALAFLAGS) -o $@ $^
 
@@ -54,7 +59,8 @@
 clean-local:
 	rm -f *.~[0-9]~
 
-DISTCLEANFILES = \
+CLEANFILES = \
+	datamodel-test \
 	marshalling-test \
 	mimetype-test \
 	query-operators-test \
@@ -64,6 +70,7 @@
 
 EXTRA_DIST = \
 	assertions.vapi \
+	datamodel-test.vala \
 	marshalling-test.vala \
 	mimetype-test.vala \
 	query-operators-test.vala \

=== added file 'test/direct/datamodel-test.vala'
--- test/direct/datamodel-test.vala	1970-01-01 00:00:00 +0000
+++ test/direct/datamodel-test.vala	2012-02-18 21:37:18 +0000
@@ -0,0 +1,70 @@
+/* datamodel-test.vala
+ *
+ * Copyright © 2012 Canonical Ltd.
+ *             By Siegfried-A. Gevatter <[e-mail]>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+using Zeitgeist;
+
+int main (string[] argv)
+{
+    Test.init (ref argv);
+
+    Test.add_func ("/Datamodel/MatchesTemplate/anything", matches_template_anything_test);
+
+    return Test.run ();
+}
+
+void matches_template_anything_test ()
+{
+    // Let's get a template with everything null
+    var templ = new Event.full ();
+    var event = new Event.full ("interp", "manif", "actor", "origin");
+
+    // Test with zero subjects
+    assert (templ.matches_template (templ));
+    assert (event.matches_template (templ));
+
+    var subject = new Subject.full ();
+    event.add_subject (subject);
+
+    // Test with one subject
+    assert (event.matches_template (templ));
+
+    var subject2 = new Subject.full ("uri", "interp", "manif", "mimetype",
+        "origin", "text", "storage", "current_uri");
+    event.add_subject (subject2);
+
+    // Test with two subjects
+    assert (event.matches_template (templ));
+
+    // Let's ensure that empty strings are also working...
+    templ.interpretation = "";
+    assert (event.matches_template (templ));
+
+    // As well as just a wildcard
+    templ.actor = "*";
+    assert (event.matches_template (templ));
+
+    // FIXME: figure out how we want to treat multiple subjects in the template
+
+    // Now check something that doesn't match
+    templ.manifestation = "No thanks!";
+    assert (!event.matches_template (templ));
+}
+
+// vim:expandtab:ts=4:sw=4