Attachment 66848 Details for Bug 53016 – fix

[patch] fix

0001-libzeitgeist-add-null-event-handling.patch (text/plain), 9.62 KB, created by Siegfried-Angel Gevatter Pujals on 2012-09-08 15:16:52 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Siegfried-Angel Gevatter Pujals

Created: 2012-09-08 15:16:52 UTC

Size: 9.62 KB

patch

obsolete

>From 2e76449bcad7a239315ad704b5b06114897336dc Mon Sep 17 00:00:00 2001
>From: Siegfried-Angel Gevatter Pujals <siegfried@gevatter.com>
>Date: Sat, 8 Sep 2012 17:14:29 +0200
>Subject: [PATCH] libzeitgeist: add "null event" handling
>
>GetEvents returns null-events when a requested ID doesn't exist. This
>commit fixes libzeitgeist to properly support this.
>
>For this to work, the ResultSet interface had to change from
>next_value() to next()+get().
>
>https://bugs.freedesktop.org/show_bug.cgi?id=53016
>---
> examples/get-events-with-id.vala    |   10 ++++++---
> libzeitgeist/API_CHANGES            |    2 ++
> libzeitgeist/datamodel.vala         |   27 +++++++++++++++++++++----
> libzeitgeist/log.vala               |    1 +
> libzeitgeist/result-set.vala        |   38 ++++++++++++++++++++++-------------
> libzeitgeist/simple-result-set.vala |   21 +++++++++----------
> test/c/test-log.c                   |    4 ++--
> 7 files changed, 69 insertions(+), 34 deletions(-)
>
>diff --git a/examples/get-events-with-id.vala b/examples/get-events-with-id.vala
>index 611933c..6f53fef 100644
>--- a/examples/get-events-with-id.vala
>+++ b/examples/get-events-with-id.vala
>@@ -1,7 +1,7 @@
> int main ()
> {
>     var ids = new Array<uint32>();
>-    uint32 id1 = 210;
>+    uint32 id1 = 2100000;
>     uint32 id2 = 222;
>     ids.append_val(id1);
>     ids.append_val(id2);
>@@ -10,10 +10,14 @@ int main ()
> 
>     Zeitgeist.Log zg = new Zeitgeist.Log ();
>     zg.get_events (ids, null, (obj, res) => {
>-        Zeitgeist.ResultSet events = zg.get_events.end (res);
>+        Zeitgeist.ResultSet events = null;
>+        events = zg.get_events.end (res);
>         foreach (Zeitgeist.Event event in events)
>         {
>-            stdout.printf ("Subject: %s\n", event.subjects[0].uri);
>+            if (event != null)
>+                stdout.printf ("First subject: %s\n", event.subjects[0].uri);
>+            else
>+                stdout.printf ("Event doesn't exist.\n");
>         }
>         loop.quit();
>     });
>diff --git a/libzeitgeist/API_CHANGES b/libzeitgeist/API_CHANGES
>index 2357895..f4181c6 100644
>--- a/libzeitgeist/API_CHANGES
>+++ b/libzeitgeist/API_CHANGES
>@@ -15,6 +15,8 @@ API CHANGES FROM LIBZEITGEIST1:
>  - introduced zeitgeist_time_range_intersect
>  - removed time_range_get_{start,end}_iso8601
>  - changed timestamp_for_now to be timestamp_now
>+ - zeitgeist_log_get_events may return NULL events (instead of critical'ing)
>+ - changed the ResultSet interface to match Vala's (next+get, and has_next as a bonu)
> 
> Vala:
>  - stuff is more valaish (eg. properties vs set/get)
>diff --git a/libzeitgeist/datamodel.vala b/libzeitgeist/datamodel.vala
>index 19d47e3..b6b5124 100644
>--- a/libzeitgeist/datamodel.vala
>+++ b/libzeitgeist/datamodel.vala
>@@ -29,6 +29,7 @@ namespace Zeitgeist
>     [DBus (name = "org.gnome.zeitgeist.DataModelError")]
>     public errordomain DataModelError {
>         INVALID_SIGNATURE,
>+        NULL_EVENT,
>         TOO_MANY_RESULTS
>     }
> 
>@@ -429,6 +430,12 @@ namespace Zeitgeist
>             Variant payload_variant = iter.next_value ();
> 
>             var event_props = event_array.n_children ();
>+
>+            if (event_props == 0)
>+            {
>+                throw new DataModelError.NULL_EVENT ("This is an empty event.");
>+            }
>+
>             assert_sig (event_props >= 5, "Missing event information.");
>             id = (uint32) uint64.parse (event_array.next_value().get_string ());
>             var str_timestamp = event_array.next_value().get_string ();
>@@ -595,14 +602,26 @@ namespace Zeitgeist
>     namespace Events
>     {
> 
>-        public static GenericArray<Event> from_variant (Variant vevents)
>+        public static GenericArray<Event?> from_variant (Variant vevents)
>             throws DataModelError
>         {
>-            GenericArray<Event> events = new GenericArray<Event> ();
>+            GenericArray<Event?> events = new GenericArray<Event> ();
> 
>             assert (vevents.get_type_string () == "a("+Utils.SIG_EVENT+")");
>-            foreach (Variant event in vevents)
>-                events.add (new Event.from_variant (event));
>+            foreach (Variant vevent in vevents)
>+            {
>+                Event? event = null;
>+                try
>+                {
>+                    event = new Event.from_variant (vevent);
>+                }
>+                catch (DataModelError err)
>+                {
>+                    if (!(err is DataModelError.NULL_EVENT))
>+                        throw err;
>+                }
>+                events.add (event);
>+            }
> 
>             return events;
>         }
>diff --git a/libzeitgeist/log.vala b/libzeitgeist/log.vala
>index 47e3311..c7903ae 100644
>--- a/libzeitgeist/log.vala
>+++ b/libzeitgeist/log.vala
>@@ -218,6 +218,7 @@ public class Log : QueuedProxyWrapper
>         {
>             DBusConnection conn = ((DBusProxy) proxy).get_connection ();
> 
>+            // FIXME: check exception
>             uint registration_id = conn.register_object<RemoteMonitor> (
>                 monitor.get_path (), monitor);
>             monitors.insert (monitor, registration_id);
>diff --git a/libzeitgeist/result-set.vala b/libzeitgeist/result-set.vala
>index 5312d8e..72b8595 100644
>--- a/libzeitgeist/result-set.vala
>+++ b/libzeitgeist/result-set.vala
>@@ -75,17 +75,17 @@ public interface ResultSet : Object
>      * Get the event at the current cursor position.
>      *
>      * To retrieve the current event and advance the cursor position call
>-     * zeitgeist_result_set_next() in stead of this method.
>+     * zeitgeist_result_set_next() instead of this method.
>      *
>      * @param self The #ZeitgeistResultSet to get an event from
>      *
>      * @return The #ZeitgeistEvent at the current cursor position
>      */
>-     public abstract Event peek ();
>+     public abstract Event? get ();
> 
>     /**
>-     * Set the cursor position. Following calls to zeitgeist_result_set_peek()
>-     * or zeitgeist_result_set_next() will read the event at position @pos.
>+     * Set the cursor position. Following calls to zeitgeist_result_set_get()
>+     * will read the event at position @pos.
>      *
>      * @param self The #ZeitgeistResultSet to seek in
>      * @param pos The position to seek to
>@@ -102,20 +102,30 @@ public interface ResultSet : Object
>     public abstract uint tell ();
> 
>     /**
>-     * Get an iterator object
>-     * @param self The #ZeitgeistResultSet to seek in
>+     * Check whether there is any remaining event.
>+     *
>+     * @param self The #ZeitgeistResultSet to check
>+     *
>+     * @return Whether there is any remaining event
>      */
>-    public ResultSet iterator () {
>-        return this;
>-    }
>-
>-    public abstract Event? next_value ();
>+    public abstract bool has_next ();
> 
>-    public abstract bool has_next();
>+    /**
>+     * Advance to the next event, if there is any remaining one.
>+     *
>+     * @param self The #ZeitgeistResultSet to advance
>+     *
>+     * @return Whether there were any events remaining
>+     */
>+    public abstract bool next ();
> 
>-    public Event? next ()
>+    /**
>+     * Do not use this method! It is only for use by Vala.
>+     */
>+    public ResultSet iterator ()
>     {
>-        return next_value ();
>+        // Damn you Vala. Why is Iterator<> in Gee?
>+        return this;
>     }
> }
> 
>diff --git a/libzeitgeist/simple-result-set.vala b/libzeitgeist/simple-result-set.vala
>index 1e241c5..68e45a7 100644
>--- a/libzeitgeist/simple-result-set.vala
>+++ b/libzeitgeist/simple-result-set.vala
>@@ -27,15 +27,15 @@ namespace Zeitgeist
> internal class SimpleResultSet : Object, ResultSet
> {
> 
>-    private GenericArray<Event> events;
>+    private GenericArray<Event?> events;
>     private uint num_estimated_matches;
>     private uint cursor;
> 
>-    internal SimpleResultSet (GenericArray<Event> events)
>+    internal SimpleResultSet (GenericArray<Event?> events)
>     {
>         this.events = events;
>         num_estimated_matches = events.length;
>-        cursor = 0;
>+        cursor = -1;
>     }
> 
>     internal SimpleResultSet.with_num_matches (
>@@ -43,7 +43,7 @@ internal class SimpleResultSet : Object, ResultSet
>     {
>         this.events = events;
>         num_estimated_matches = matches;
>-        cursor = 0;
>+        cursor = -1;
>     }
> 
>     public uint size ()
>@@ -56,8 +56,9 @@ internal class SimpleResultSet : Object, ResultSet
>         return num_estimated_matches;
>     }
> 
>-    public Event peek ()
>+    public new Event? get ()
>     {
>+        warn_if_fail (cursor < events.length);
>         return events.get (cursor);
>     }
> 
>@@ -71,16 +72,14 @@ internal class SimpleResultSet : Object, ResultSet
>         return cursor;
>     }
> 
>-    public Event? next_value ()
>+    public bool has_next ()
>     {
>-        if (cursor < events.length)
>-            return events.get (cursor++);
>-        return null;
>+        return cursor < (events.length - 1);
>     }
> 
>-    public bool has_next()
>+    public bool next ()
>     {
>-        return (cursor < events.length);
>+        return (++cursor < events.length);
>     }
> 
> }
>diff --git a/test/c/test-log.c b/test/c/test-log.c
>index 2d54c80..cf26af4 100644
>--- a/test/c/test-log.c
>+++ b/test/c/test-log.c
>@@ -97,10 +97,10 @@ _on_events_received (ZeitgeistLog *log,
>   event_ids = g_array_sized_new (FALSE, FALSE, sizeof (guint32),
>                                  zeitgeist_result_set_size(events));
>   i = 0;
>-  while (zeitgeist_result_set_has_next (events))
>+  while (zeitgeist_result_set_next (events))
>     {
>       g_assert_cmpint (i, ==, zeitgeist_result_set_tell (events));
>-      ev = zeitgeist_result_set_next (events);
>+      ev = zeitgeist_result_set_get (events);
>       _ev = ZEITGEIST_EVENT (g_ptr_array_index (expected_events, i));
>       g_assert_cmpstr (zeitgeist_event_get_interpretation (ev), ==,
>                        zeitgeist_event_get_interpretation (_ev));
>-- 
>1.7.9.5
>

Actions: View | Splinter Review

Attachments on bug 53016: 66848

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.