[Bug 24122] Move PEP code to Wocky

[bugzilla-daemon at freedesktop.org]

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


Will Thompson <will.thompson at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |will.thompson at collabora.co.u
                   |                            |k
         AssignedTo|telepathy-                  |guillaume.desmottes at collabor
                   |bugs at lists.freedesktop.org  |a.co.uk
          QAContact|                            |telepathy-
                   |                            |bugs at lists.freedesktop.org




--- Comment #1 from Will Thompson <will.thompson at collabora.co.uk>  2009-09-23 14:22:22 PST ---
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?

<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. Ditto the following patch, to a
slightly lesser extent, and make_publish_stanza().

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

+  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);
        ...
      }

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

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.

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?

Does anything check the gboolean returned by
gabble_pubsub_event_handler()?

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

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?

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


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