Opened 12 years ago

Closed 12 years ago

#4500 closed enhancement (fixed)

[patch/ccla] Clean up cometd.js a bit, improve reporting for subscribe/unsubscribe

Reported by: guest Owned by: alex
Priority: high Milestone:
Component: Dojox Version: 0.9
Keywords: cometd Cc: brendonh@…
Blocked By: Blocking:

Description

This ticket does a couple of things:

  • Removes two places where arguments were introspected and swapped. These were for back-compat but I understand there'll be a clean API break for 1.0.
  • Removes "/meta/subscribe" handling from longPollTransport.deliver() (and thereby also callbackPollTransport). In fact this code was unreachable anyway, because cometd._deliver() handles it and then never calls the transport's deliver.
  • Adds two maps to cometd -- pendingSubscriptions and pendingUnsubscriptions. When the application calls cometd.subscribe() or cometd.unsubscribe(), a Deferred is created, stored in one of these maps, and returned to the user. Later, when the response comes in from the server, the same Deferred is looked up and fired. This means the application can be informed of the success or failure of these operations.

I originally tried to do this by changing the deferred returned by xTransport.sendMessage, then realised that the response from the server can come on /any/ connection, so the transport is the wrong place for it. It now seems to me that messages (/all/ messages) should have a messageID field which allows responses to be uniquely mapped to requests. Until we have that in the Bayeux protocol, looking up by subscription channel is the best we can do.

Attachments (2)

patch.txt (5.3 KB) - added by guest 12 years ago.
patch2.txt (10.4 KB) - added by guest 12 years ago.

Download all attachments as: .zip

Change History (7)

Changed 12 years ago by guest

Attachment: patch.txt added

comment:1 Changed 12 years ago by dante

Owner: changed from Tom Trenka to alex
Summary: Clean up cometd.js a bit, improve reporting for subscribe/unsubscribe[patch/ccal] Clean up cometd.js a bit, improve reporting for subscribe/unsubscribe

ccla via Cogini? not verified. but talked to brendon.

comment:2 Changed 12 years ago by dante

Summary: [patch/ccal] Clean up cometd.js a bit, improve reporting for subscribe/unsubscribe[patch/ccla] Clean up cometd.js a bit, improve reporting for subscribe/unsubscribe

comment:3 Changed 12 years ago by alex

Status: newassigned

this patch needs work. Dojo style is to use 4-space tabs instead of spaces, to remove spaces before and after conditions, and to make sure that expressions are space-efficient. The following line:

(pendingDef !== undefined)

and those like it should be re-written as:

(!pendingDef)

Please consult the style guide: http://dojotoolkit.org/developer/StyleGuide

Regards

comment:4 Changed 12 years ago by guest

Fair enough. Here's a second version (patch2.txt) with the following changes:

  • All 4-space blocks changed to tabs. Some of the code in the original 0.9 file didn't follow this, so there's some cruft in the patch to convert it.
  • Trailing whitespace removed from all lines. As above, this affects some original code.
  • Spaces removed around conditional expressions.
  • if(pendingDef !== undefined) changed to if(pendingDef), which I'm pretty sure is what you meant to type.

Cheers, Brendon

Changed 12 years ago by guest

Attachment: patch2.txt added

comment:5 Changed 12 years ago by alex

Resolution: fixed
Status: assignedclosed

(In [10573]) merging patch from brendonh@…. Nice work. Fixes #4500

Note: See TracTickets for help on using tickets.