Re: Proposal for change notifications

[Iago Toral Quiroga <itoral igalia com>]

Hi,

my comments below.

El mar, 18-01-2011 a las 23:55 +0100, Juan A. Suárez Romero escribió:
(...)
> Thus, we identify this 3 situations:
> 
>   * Plugins that doesn't support any kind of notification
>   * Plugins that can notify that "something" changed (new content was
> added, or content was removed)
>   * Plugins that can notify that something changed, and more precisely,
> what changed (a file has been updated) or where happened (content in a
> specific folder has happened).
> 
> 
> Based on above situations, I propose this first approach:
> 
> guint
> grl_media_source_update_notify_add(GrlSource *source,
> 				GrlMediaSourceUpdatedCb callback,
> 				gpointer user_data);
> 
> void
> grl_media_source_update_notify_remove(GrlSource *source,
> 				guint notify_id);
> 
> 
> These operations are optional, and if plugin supports them, it should be
> listed in the GrlSupportedOps (using a new flag like
> GRL_OP_UPDATE_NOTIFY).
> 
> The first one installs a callback that will be invoked when a change
> happens in the source.
> 
> Regarding the *_add() function, not sure if we should indicate which
> element we want to monitor. I didn't consider it, but maybe it's worth
> to add as parameter: if plugin can monitor a specific element, then it
> can use it, else it will ignore the parameter. Plugin's documentation
> should clearly state if it allows to monitor a specific folder or not.
> 

Can't think of many use cases for that, although we can add this if
people need it, but would not push it for it myself at the moment.

> 
> How the notifications are sent? This is the callback signature:
> 
> typedef void
> (*GrlMediaSourceUpdatedCb) (GrlMediaSource *source,
> 			GrlMedia *media,
> 			GrlMediaSourceUpdatedType type,
> 			gboolean include_children,
> 			gpointer user_data);
> 
> 
> "media" is the media that has changed. Usually, it will be a GrlBox,
> meaning that content in that box has changed. But I keep the more
> generic GrlMedia just in case a plugin has the ability to specifically
> notifiy that a media has changed.

I have some doubts here. Let's discuss this proposal over a specific
example: let's say I copy a new file and Tracker indexes it. The plugin
would invoke the notify callback with the parent folder... but what is
that parent folder? for sources that do not implement browse, this does
not even make sense! We could be interested in notify signals even if
the source is only searchable, right?

> We use a "media" and not the media "id" because it is expected that if
> it changes, user application will perform a metadata/browse operation.
> Thus it doesn't need to create the right media to do it; it already has
> the changed element to use.

That's also coherent with the rest of the library, we never expose the
ids now.

(...)

> "include_children" means that the media, or any of its descendants, have
> changed. Of course, it only means in the (standard) case of using a
> GrlBox as media.
> 
> If include_children==FALSE, it means that the box content (the direct
> children) have changed. But if include_children==True then any of the
> direct or indirect (children, grand-children, ...) have changed.
> 
> Thus, in the case that a plugin is only able to notify that a changed
> happened in the pugin, but not where, then it can use the root box as
> media, and include_children==True, meaning that "something in the root
> or below has changed".

The idea looks good to me, but I dislike the 'include_children' name.
What about 'change_location_unknown'? Also, again, we chould check how
this works with searchable-only plugins.

> "type" indicates the kind of change. Currently, I'm only thinking on
> having 3 different values:
> 
>   * GRL_UPDATE_ADDED
>   * GRL_UPDATE_REMOVED
>   * GRL_UPDATE_CHANGED
> 
> When something has been added and removed at the same time, the last
> value would be used.

I guess you mean "when various items have been added / removed", the
last value would be used.

What about the case of a signle media file that has been updated? (for
example its metadata has been changed). Would we use CHANGED for this
case too?

> I'm not very sure if we actually need this "type" value. I've proposed
> it because I was also contemplating a new kind of update: when a source
> is being updated. This happens, for instance, when Tracker is updating
> the content, or if a Podcasts plugin is downloading the latest clips. In
> this case, a GRL_UPDATED_UPDATING signal can be used to tell it is being
> updated.

Yeah, probably when a source is being updated it should wait until it
has finished before signaling anything. In that case it could emit a
bunch of ADDED / REMOVED notifications or just a single CHANGED.

Iago