[Bug 24122] Move PEP code to Wocky

[bugzilla-daemon at freedesktop.org]

http://bugs.freedesktop.org/show_bug.cgi?id=24122





--- Comment #3 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk>  2009-09-24 04:20:00 PST ---
(In reply to comment #1)
> Can you get more than one <item> in an event? My reading of XEP-0060
> suggests you might. We should probably handle that case by calling the
> handler callback n times?

Good point, but that shouldn't happen with PEP. Anyway, the new API doesn't
have a callback anymore.

> <http://git.collabora.co.uk/?p=user/cassidy/telepathy-gabble;a=commitdiff;h=90cf6e92879906b62db2c2d03f336180e706c073>:
> :(. I think this is much less clear than the code it replaces; I can't
> see the content for the boilerplate.

Well that's how Sjoerd implemented _build in Wocky. If you think the Gabble API
is better, feel free to open a Wocky bug to discuss improvements.


> Ditto the following patch, to a
> slightly lesser extent, and make_publish_stanza().

Humm; I don't understand. What do you mean?


> Why does wocky_pubsub_register_event_handler return an int, rather
> than the PubsubEventHandler pointer (opaque, of course).

This was inspired from the porter register API. Anyway, this function doesn't
exist in WockyPepService any more.


> +  if (priv->handler_id != 0)
> +    {
> +      wocky_porter_unregister_handler (priv->porter, priv->handler_id);
> +      priv->handler_id = 0;
> +    }
> +
> +  if (priv->porter != NULL)
> +    g_object_unref (priv->porter);
> 
> If priv->porter can be NULL, then the first block will break. But
> the id will be non-zero iff the porter is non-NULL, so that'll never
> happen. It'd be clearer as:
> 
>     if (priv->porter != NULL)
>       {
>         g_assert (priv->handler_id != 0);
>         ...
>       }

Good point; I did that.

> I guess the fact that handlers on the porter return uints rather than
> opaque pointers is why register_event_handler() does that?

yep :)

> If the WockyPubsub is disposed with outstanding requests, do the
> callbacks get called?
> • If so, is the DBusGMethodInvocation still valid if the
>   GabbleConnection it came from has died?
>   • If so, we're safe, because the callbacks so far (at least in
>     d4ad9554b0d3) don't use the conn if the request failed, but it
>     feels accidental.
>   • If not, we crash.
> • If not, we leak.

It can't. The GSimpleAsyncResult has a ref on the source object so it can't be
disposed while the operation is running.

> Asserting that priv->porter is non-NULL in
> wocky_pubsub_send_query_async seems wrong: surely client code could
> call this before the connection's connected?

Function now raises an error if there is no porter.

> Does anything check the gboolean returned by
> gabble_pubsub_event_handler()?

Yes; it tells to the stanza dispatcher if the handler actually handled the
stanza or not.

> +  /* TODO: subscribe to node if needed */

I need the Wocky capabilities component to do that. Gabble still announce the
+notify so it's fine for now.

> I wonder if it'd be worth having a wrapper around the changed cb which
> does the 'make a handle from the jid' stuff for you?

Not sure. In the Brand fully Wockified Gabble World; Gabble should use contacts
objects.

> I enjoy how this branch makes a WockyPubsub, and then towards the end
> stops directly using it. And then deletes it? What?!

Yeah, that's because after discussion we Sjoerd we decided to have a separated
API for PEP and for Pubsub. I didn't re-record the branch as most of the
changes were still useful in the new version and I didn't want to fight
conflicts of the dead. :)


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.