content and doc string of RemoteInterface.EventsChanged signal are different

Bug #398379 reported by Markus Korn
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Zeitgeist Framework
Fix Released
Undecided
Markus Korn

Bug Description

From the docstring of this signal:

"""It contains a list of dictionaries containing one or more of `added`, [...]"""

Given that the signature of this signal is "a{sv}" this will never return a list of dicts. The result will always be a dict with one single item with the type of change as the key, and a list of changed items (or uris) as value.
So the docstring needs to be fixed.

And while we are on it we should think about the content of this signal again: why are we using a dict here? In my opinion a dict would only make sense if the content can be something like:

{"changed": [<item1>, <item2>], "deleted": ["uri/to/item3", "uri/to/item4"]}

With the current design of our engine such signal is impossible, and as far as I can see we will never be able to send such signal.
So I suggest changing the content of this signal to a tuple, with <type of change> as first element and a list of changed items as 2nd one:

("changed", [<item1>, <item2>])

Markus

Related branches

Revision history for this message
Siegfried Gevatter (rainct) wrote :

That signature, "a{sv}", *is* a list of dicts (the "a" stands for the list, and the "{}" for the dict), albeit with only one item, as you can see below. It is like this because D-Bus doesn't seem to accept standalone dicts.

Generally I wouldn't don't agree with your argument about the current engine, as that may change (and whatever the engine does is an implementation detail, not an API one), but given that I can't think of any use case where having a dict would be useful (other than an implementation which would buffer the signals, in which case sending out three signals instead of one wouldn't be that a big problem either) and for the principle of KISS (and making use of the API easier) I'll agree with you. Let's change this in 0.2.

(dbus.Dictionary({dbus.String(u'added'): dbus.Array([dbus.Struct((dbus.Int32(1247410102), dbus.String(u'file:///home/rainct/Desenvolupament/Python/gnome-zeitgeist/extra/zeitgeist-trayicon.desktop.in.in'), dbus.String(u'zeitgeist-trayicon.desktop.in.in'), dbus.String(u'Other'), dbus.String(u'File'), dbus.String(u'application/octet-stream'), dbus.String(u'Desenvolupament,Python,gnome-zeitgeist,extra'), dbus.String(u''), dbus.Boolean(False), dbus.String(u'http://gnome.org/zeitgeist/schema/1.0/core#ModifyEvent'), dbus.String(u''), dbus.String(u'/usr/share/applications/geany.desktop'), dbus.String(u'')), signature=None), dbus.Struct((dbus.Int32(1247410102), dbus.String(u'file:///home/rainct/Desenvolupament/Python/gnome-zeitgeist/extra/zeitgeist-trayicon.desktop.in.in'), dbus.String(u'zeitgeist-trayicon.desktop.in.in'), dbus.String(u'Other'), dbus.String(u'File'), dbus.String(u'application/octet-stream'), dbus.String(u'Desenvolupament,Python,gnome-zeitgeist,extra'), dbus.String(u''), dbus.Boolean(False), dbus.String(u'http://gnome.org/zeitgeist/schema/1.0/core#VisitEvent'), dbus.String(u''), dbus.String(u'/usr/share/applications/geany.desktop'), dbus.String(u'')), signature=None)], signature=dbus.Signature('(isssssssbssss)'), variant_level=1)}, signature=dbus.Signature('sv')),)

Changed in zeitgeist:
milestone: none → 0.2
Revision history for this message
Siegfried Gevatter (rainct) wrote :

Ah you're right, it is only one dict (I found it weird that you couldn't use it standalone...), so the documentation is indeed wrong, good catch. I'm still happy with the proposed change too.

Revision history for this message
Markus Korn (thekorn) wrote :

It turns out that my proposed change of the EventsChanged content is not easy to implement because of a restriction of dbus (or the python implementation of dbus)

The content can look like
("modified"|"added", [{<item1>}, {<item2>}])
("deleted", ["uri1, "uri2"])

which results in a signature of "(sav)" (where 'av' is either a a list of dicts or a list of strings)

But unfortunatly this raises a DBus exception.

Markus Korn (thekorn)
Changed in zeitgeist:
assignee: nobody → Markus Korn (thekorn)
status: New → In Progress
Revision history for this message
Markus Korn (thekorn) wrote :

please ignore my last comment, it is just not true, (sav) works fine

Revision history for this message
Siegfried Gevatter (rainct) wrote :

Fixed.

Changed in zeitgeist:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.