Summary: | Improve acknowledging and nacking IQs | ||
---|---|---|---|
Product: | Wocky | Reporter: | Will Thompson <will> |
Component: | General | 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/wocky.git;a=shortlog;h=refs/heads/ack-and-nak | ||
Whiteboard: | review+ | ||
i915 platform: | i915 features: |
Description
Will Thompson
2010-01-27 07:03:17 UTC
No, we shouldn't: Rob's pointed out that some servers send roster pushes with no id='', and we shouldn't drop those. We should, however, make it easier for code to cope with such stanzas, by making build_iq_reply and friends not assert in this case. Branch coming up to improve acking/naking in general… Here's a WIP branch. It is on top of <http://git.collabora.co.uk/?p=user/wjt/wocky.git;a=shortlog;h=refs/heads/misc> which is reviewable plz! OOI, why didn't you use wocky_strdiff everywhere in your misc branch instead of any strcmps? Your misc branch looks fine. Your ack-and-nak branch also looks fine apart from: * the two TODOs in the tests * I got a bit confused by 7882c658b7d5. If you could highlight that get/set have exactly one child elements, result have zero or one child elements then that'd make things clearer. The way the RFC says how an error SHOULD have one and only one child element from the original IQ is confusing as I wondered how it could ever be >1. Seems fine otherwise. (In reply to comment #4) > Your misc branch looks fine. Merged, cheers. > Your ack-and-nak branch also looks fine apart from: > > * the two TODOs in the tests Fixed and squashed. > * I got a bit confused by 7882c658b7d5. If you could highlight that get/set > have exactly one child elements, result have zero or one child elements then > that'd make things clearer. The way the RFC says how an error SHOULD have one > and only one child element from the original IQ is confusing as I wondered how > it could ever be >1. Fixed. I've also added a patch adding wocky_porter_send_iq_gerror() for application-specific error conditions. + /* Send an illegal IQ with two child elements. We expect that + * wocky_porter_acknowledge_iq() should cope. */ + iq = wocky_stanza_build (WOCKY_STANZA_TYPE_IQ, + WOCKY_STANZA_SUB_TYPE_GET, "juliet@example.com", "romeo@example.net", + '(', "sup-dawg", ')', + '(', "i-heard-you-like-stanzas", ')', + NULL); + expected_reply = wocky_stanza_build ( + WOCKY_STANZA_TYPE_IQ, WOCKY_STANZA_SUB_TYPE_RESULT, + "romeo@example.net", "juliet@example.com", + '(', "sup-dawg", + '@', "lions", "tigers", + ')', + NULL); Isn't this a bit of a poor test? If you reply with something other than a 'sup-dawg' child element, then the result IQ has two child elements, right? It'd be cooler if this was more explicit. Oh, no, I'm getting confused with error. Ignore that then. LOOKS HOT. Merged, thanks! |
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.