Thanks for the reply. I haven't yet figured out whether I completely agree with your reasoning, but it is really useful to see the intention behind these functions written down in black and white.
Keith and I are going to be writing more tests for Delivery quite soon, so I'd like to defer the full resolution of these questions (including JNIDelivery state/memory management issues) until then. For now, I will just document this discrepancy in a TODO in the Delivery.isSettled JavaDoc. Also, would you be able to add a comment to pn_delivery_settled to hint that it is not simply the property accessor counterpart to pn_delivery_settle? Phil On 7 March 2013 22:13, Rafael Schloming <r...@alum.mit.edu> wrote: > On Wed, Mar 6, 2013 at 10:27 PM, Phil Harvey <p...@philharveyonline.com > >wrote: > > > Ok, I hadn't realised that settling a delivery in proton-c had such a > > significant effect on its state. > > > > Does this imply that it's illegal for an application to use a delivery in > > any way once it's called pn_delivery_settle? If so, we should document > this > > in in the header file. > > > > It does. > > Also, any views on my function name suggestions? > > > > I'm not super keen on what you're proposing for the C API. I'll try to > describe my reservations, but I'm doing a bit of thinking out loud here, so > bear with me. > > Right now pn_delivery_readable, pn_delivery_writable, pn_delivery_updated, > and pn_delivery_settled are all flags indicating a situation that has > arisen asynchronously due to remote activity, and hence very related to the > notion of the work queue. If anyone of those flags becomes true then the > delivery will appear in the work queue. These flags, however, are also > influenced by local actions, and in that respect they aren't the same as > the remote/local state fields that appear elsewhere in the API. > > For example local/remote source/target, local/remote container, > local/remote hostname, etc, directly correspond to state values held by > both the local and remote endpoint. In each of these cases they are only > ever set based on what the remote peer reports, and short of getting the > remote peer to report something else, there is no way to effect them > through the API. The above flags, however, are also influenced by local > actions, e.g. a delivery might become readable when the remote peer > supplies message data, but it stops being readable when you read whatever > is available, likewise the delivery's remote state might be updated > asynchronously, however the local app can access the updated value and then > call pn_delivery_clear() to clear the updated flag. > > In the case of settlement there is another difference from the > local/remote_foo pattern as used elsewhere. In all cases of the > local/remote_foo pattern remote_foo is a fact currently being remembered by > the remote endpoint, whereas in the case of settlement, the remote endpoint > has actually destroyed all knowledge of the delivery, and so the flag is > actually indicating the absence of any facts at all at the remote endpoint > rather than, e.g., indicating that the remote endpoint is holding onto a > delivery object with a settled flag whose value is now true. In other words > settled is actually a verb rather than a noun as remote_settled might > suggest. > > I suspect something of this intuition was present on the Java side when the > current names were chosen from the choice of remote*ly*Settled() rather > than remoteSettled(). > > One possibility for naming would be to adopt the remotely_settled > terminology on the C side, however I'm not mad on this either as it breaks > what is currently a common pattern between pn_delivery_update/updated, and > pn_delivery_settle/settled, the former being the local verb, and the latter > being indication that the remote verb has occurred. Another possibility > would be to simply eliminate the isLocallySettled() notion from the Java > API and make isSettled() a synonym for isRemotelySettled(). > > Probably a very relevant question though before trying to sort out naming > consistency is whether the semantics are actually compatible. It occurs to > me that if you're backing the Java Delivery interface with a JNIDelivery > you may well get some very strange behaviour if you keep around references > to locally settled deliveries. I'm guessing the settled deliveries will > probably get reused by the C impl and their tags/state/etc will randomly > start changing based on their new usage. I think a correctly wrapped JNI > layer would need to discard any pointer to the C delivery and therefore > need to throw some kind of exception when any of the methods on Delivery > were actually called post settlement. > > I'm also wondering what Delivery.free() actually does and given that there > isn't a Delivery.isFreed() if perhaps Delivery.free() is more analogous to > what pn_delivery_settle() is actually doing. > > Apologies for the long and rambling reply. It's been a long and rambling > kind of day. ;-) > > --Rafael >
