Bug 34716

Summary:	Split wocky_porter_register_handler up to make sender matching more explicit.
Product:	Telepathy	Reporter:	Will Thompson <will>
Component:	gabble	Assignee:	Telepathy bugs list <telepathy-bugs>
Status:	RESOLVED FIXED	QA Contact:	Telepathy bugs list <telepathy-bugs>
Severity:	normal
Priority:	medium	Keywords:	patch
Version:	unspecified
Hardware:	Other
OS:	All
URL:	http://git.collabora.co.uk/?p=user/wjt/telepathy-gabble-wjt.git;a=shortlog;h=refs/heads/register-handler-from
Whiteboard:
i915 platform:		i915 features:
Bug Depends on:
Bug Blocks:	33911

Description Will Thompson 2011-02-25 05:24:41 UTC

Currently we have:

+guint wocky_porter_register_handler (WockyPorter *self,
+    WockyStanzaType type,
+    WockyStanzaSubType sub_type,
+    const gchar *from,
+    guint priority,
+    WockyPorterHandlerFunc callback,
+    gpointer user_data,
+    ...) G_GNUC_NULL_TERMINATED;

If 'from' is NULL, stanzas from anyone are matched. There is no way to match stanza from the server.

The attached branch splits this function into three:

• wocky_porter_register_handler_from(), which accepts a non-NULL 'const gchar *from' argument;
• wocky_porter_register_handler_from_anyone(), which matches stanzas from any sender;
• wocky_porter_register_handler_from_server(), which matches stanzas from the server, using the same logic as matching IQ replies from the server.

It also fixes a bug where matching JIDs without a node part, like "example.com", would actually match stanzas from anyone.

Comment 1 Jonny Lamb 2011-02-25 07:23:12 UTC

If you'd put your register_handler_va implementation AFTER the old register_handler implementation it would have been a lot easier to review. :-(

Anyway, it looks fine. I like the way there are now three functions for the three cases, it makes things really explicit what's going on.

I didn't really review all the test changes in depth, but yay go go go. Who's going to update gabble? :-P

Comment 2 Will Thompson 2011-02-25 07:26:34 UTC

Merged. I'm moving the bug to Gabble, which needs updating.

Comment 3 Will Thompson 2011-02-28 13:05:51 UTC

hi, here's a gabble branch!

Comment 4 Jonny Lamb 2011-03-03 06:53:45 UTC

32ddfbaf559dbb1 confuses me. I read the commit message and the comment in the test and it appears to want to check that gabble doesn't crash when a roster push with no id attribute comes in. That's great.

But it also appears to say that Will has added code to discard such stanzas, except in the test you check that the roster push has succeeded? What am I missing?

fyi I opened bug #34975 about the unknown IQ handler in GabbleConnection. I didn't realise the porter didn't do this already!

POW, REVIEW DONE!

Comment 5 Will Thompson 2011-03-10 02:49:46 UTC

Jonny approved an amended version of that patch with a clearer commit message: <http://cgit.freedesktop.org/telepathy/telepathy-gabble/commit/?id=99b609ad16d0345394661cc10a3e4202c79b7e42>

So, I've merged the branch. Hooray! <http://cgit.freedesktop.org/telepathy/telepathy-gabble/commit/?id=b2f292263f188d88b83c610d8a7f560bddf85e38>

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.