Merge lp:~rainct/zeitgeist/insert-event-fixes into lp:~zeitgeist/zeitgeist/bluebird

[Siegfried Gevatter]

- Pre-process events before they are send to the extensions (LP: #628804).
- Fix Storage Monitor to take into account that events may be NULL.
- Fix error messages (related to MOVE_EVENT) that were interchanged.

[Michal Hruby (mhr3)]

Looks good to me.

review: Approve

=== modified file 'extensions/fts++/test/test-indexer.cpp'
--- extensions/fts++/test/test-indexer.cpp	2012-02-10 12:07:27 +0000
+++ extensions/fts++/test/test-indexer.cpp	2012-02-10 13:40:28 +0000
@@ -169,13 +169,22 @@
 static guint
 index_event (Fixture *fix, ZeitgeistEvent *event)
 {
+  GPtrArray *events;
   guint event_id = 0;
+  guint *event_ids;
+  int num_events_inserted;
 
   // add event to DBs
-  event_id = zeitgeist_engine_insert_event (ZEITGEIST_ENGINE (fix->db),
-                                            event, NULL, NULL);
+  events = g_ptr_array_new ();
+  g_ptr_array_add (events, event);
+  event_ids = zeitgeist_engine_insert_events (ZEITGEIST_ENGINE (fix->db),
+                                              events, NULL,
+                                              &num_events_inserted, NULL);
+  g_assert_cmpint (1, ==, num_events_inserted);
+  event_id = *event_ids;
+  g_ptr_array_unref (events);
 
-  GPtrArray *events = g_ptr_array_new_with_free_func (g_object_unref);
+  events = g_ptr_array_new_with_free_func (g_object_unref);
   g_ptr_array_add (events, event); // steal event ref
   zeitgeist_indexer_index_events (fix->indexer, events);
   g_ptr_array_unref (events);

=== modified file 'extensions/storage-monitor.vala'
--- extensions/storage-monitor.vala	2012-02-02 18:57:35 +0000
+++ extensions/storage-monitor.vala	2012-02-10 13:40:28 +0000
@@ -269,10 +269,11 @@
         {
             for (int i = 0; i < events.length; ++i)
             {
+                if (events[i] == null) continue;
                 for (int j = 0; j < events[i].subjects.length; ++j)
                 {
                     Subject subject = events[i].subjects[j];
-                    if (subject.storage == "")
+                    if (Utils.is_empty_string (subject.storage))
                         subject.storage = find_storage_for_uri (subject.uri);
                 }
             }

=== modified file 'src/engine.vala'
--- src/engine.vala	2012-02-07 17:02:05 +0000
+++ src/engine.vala	2012-02-10 13:40:28 +0000
@@ -27,6 +27,7 @@
 
 using Zeitgeist;
 using Zeitgeist.SQLite;
+using Zeitgeist.Utils;
 
 namespace Zeitgeist
 {
@@ -62,21 +63,74 @@
     public uint32[] insert_events (GenericArray<Event> events,
         BusName? sender=null) throws EngineError
     {
+        // Any changes to events need to be done here so they'll
+        // be taken into consideration by the extensions (LP: #928804).
+        for (int i = 0; i < events.length; ++i)
+        {
+            preprocess_event (events[i]);
+        }
+
         extension_collection.call_pre_insert_events (events, sender);
         uint32[] event_ids = new uint32[events.length];
         database.begin_transaction ();
         for (int i = 0; i < events.length; ++i)
         {
             if (events[i] != null)
-                event_ids[i] = insert_event (events[i], sender);
+                event_ids[i] = insert_event (events[i]);
         }
         database.end_transaction ();
         extension_collection.call_post_insert_events (events, sender);
         return event_ids;
     }
 
-    public uint32 insert_event (Event event,
-        BusName? sender=null) throws EngineError
+    private void preprocess_event (Event event) throws EngineError
+    {
+        // Iterate through subjects and check for validity
+        for (int i = 0; i < event.num_subjects(); ++i)
+        {
+            unowned Subject subject = event.subjects[i];
+
+            // If current_uri is unset, give it the same value as URI
+            if (is_empty_string (subject.current_uri))
+                subject.current_uri = subject.uri;
+
+            if (event.interpretation == ZG.MOVE_EVENT
+                && subject.uri == subject.current_uri)
+            {
+                throw new EngineError.INVALID_ARGUMENT (
+                    "Redundant event: event.interpretation indicates " +
+                    "the uri has been moved yet the subject.uri and " +
+                    "subject.current_uri are identical");
+            }
+            else if (event.interpretation != ZG.MOVE_EVENT
+                && subject.uri != subject.current_uri)
+            {
+                throw new EngineError.INVALID_ARGUMENT (
+                    "Illegal event: unless event.interpretation is " +
+                    "'MOVE_EVENT' then subject.uri and " +
+                    "subject.current_uri have to be the same");
+            }
+
+            // If subject manifestation and interpretation are not set,
+            // we try to automatically determine them from the other data.
+            if (is_empty_string (subject.manifestation))
+            {
+                unowned string? manifestation = manifestation_for_uri (
+                    subject.uri);
+                if (manifestation != null)
+                    subject.manifestation = manifestation;
+            }
+            if (is_empty_string (subject.interpretation))
+            {
+                unowned string? interpretation = interpretation_for_mimetype (
+                    subject.mimetype);
+                if (interpretation != null)
+                    subject.interpretation = interpretation;
+            }
+        }
+    }
+
+    private uint32 insert_event (Event event) throws EngineError
         requires (event.id == 0)
         requires (event.num_subjects () > 0)
     {
@@ -89,7 +143,7 @@
             var storages = new GenericArray<string> ();
             var subj_uris = new SList<string> ();
 
-            if (event.origin != "")
+            if (!is_empty_string (event.origin))
                 uris.add (event.origin);
 
             // Iterate through subjects and check for validity
@@ -105,34 +159,14 @@
                 subj_uris.append (subject.uri);
 
                 uris.add (subject.uri);
-
-                if (subject.current_uri == "" || subject.current_uri == null)
-                    subject.current_uri = subject.uri;
-
-                if (event.interpretation == ZG.MOVE_EVENT
-                    && subject.uri == subject.current_uri)
-                {
-                    throw new EngineError.INVALID_ARGUMENT (
-                        "Illegal event: unless event.interpretation is " +
-                        "'MOVE_EVENT' then subject.uri and " +
-                        "subject.current_uri have to be the same");
-                }
-                else if (event.interpretation != ZG.MOVE_EVENT
-                    && subject.uri != subject.current_uri)
-                {
-                    throw new EngineError.INVALID_ARGUMENT (
-                        "Redundant event: event.interpretation indicates " +
-                        "the uri has been moved yet the subject.uri and " +
-                        "subject.current_uri are identical");
-                }
-
-                uris.add (subject.current_uri);
-
-                if (subject.origin != "")
+                if (subject.uri != subject.current_uri)
+                    uris.add (subject.current_uri);
+
+                if (!is_empty_string (subject.origin))
                     uris.add (subject.origin);
-                if (subject.text != "")
+                if (!is_empty_string (subject.text))
                     texts.add (subject.text);
-                if (subject.storage != "")
+                if (!is_empty_string(subject.storage))
                     storages.add (subject.storage);
             }
 
@@ -180,25 +214,6 @@
 
             unowned Subject subject = event.subjects[i];
 
-            // If subject manifestation and interpretation are not set,
-            // we try to automatically determine them from the other data.
-
-            if (subject.manifestation == "")
-            {
-                unowned string? manifestation = manifestation_for_uri (
-                    subject.uri);
-                if (manifestation != null)
-                    subject.manifestation = manifestation;
-            }
-
-            if (subject.interpretation == "")
-            {
-                unowned string? interpretation = interpretation_for_mimetype (
-                    subject.mimetype);
-                if (interpretation != null)
-                    subject.interpretation = interpretation;
-            }
-
             insert_stmt.bind_text (8, subject.uri);
             insert_stmt.bind_text (9, subject.current_uri);
             insert_stmt.bind_int64 (10,

=== modified file 'test/dbus/blacklist-test.py'
--- test/dbus/blacklist-test.py	2012-02-08 12:49:38 +0000
+++ test/dbus/blacklist-test.py	2012-02-10 13:40:28 +0000
@@ -203,7 +203,9 @@
 		self._assert_template_count(1)
 
 		# Blocking the current_uri works
-		self._assert_insert_blocked(Event.new_for_values(subject_current_uri="t"))
+		self._assert_insert_blocked(Event.new_for_values(
+			interpretation=Interpretation.MOVE_EVENT,
+			subject_current_uri="t"))
 
 		# But if we only set uri (and leave it up to Zeitgeist to set current_uri
 		# to the same value?