> On June 20, 2013, 5:12 p.m., Andrew Stitcher wrote: > > /trunk/qpid/cpp/src/qpid/broker/ConnectionIdentity.h, line 45 > > <https://reviews.apache.org/r/11997/diff/1/?file=309166#file309166line45> > > > > This line makes ConnectionIdentity amqp 0_10 specific - surely that's > > not intended? > > > > Also it doesn't seem to be an identity type of functionality/property. > > > > (I also can't see where it is actually used.)
Yes, I've changed that to use qpid::types::Variant::Map. Its used in HA code at present to idnetify particular categories of connection. > On June 20, 2013, 5:12 p.m., Andrew Stitcher wrote: > > /trunk/qpid/cpp/src/qpid/ha/ConnectionObserver.cpp, line 86 > > <https://reviews.apache.org/r/11997/diff/1/?file=309179#file309179line86> > > > > Was this a deliberate deletion? > > > > It doesn't seem to be explained as part of the overall change? > > > > If this is a bug fix then I thin kit should be called out specifically. Oops! Well spotted, that is unintended. It was an artefact of an initial attempt to avoid the abort() call here by throwing an exception (as suggested in the current doc for the ConnectionObserver interface), but that doesn't actually work as the HA plugin holds a pointer to the object passed in for aborting at a later stage in some cases. I'll fix that. - Gordon ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11997/#review22168 ----------------------------------------------------------- On June 20, 2013, 2:51 p.m., Gordon Sim wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11997/ > ----------------------------------------------------------- > > (Updated June 20, 2013, 2:51 p.m.) > > > Review request for qpid, Andrew Stitcher, Alan Conway, Kenneth Giusti, Ted > Ross, and Chug Rolke. > > > Description > ------- > > In preparation for authorisation in AMQP 1.0, I needed to decouple > ConnectionObserver from the 0-10 specific Connection class: > > * combined Connection::getMgmtId() and Connection::getUrl() as it turns out > they are the same thing (though stored twice); I kept the getMgmtId() as the > name as I think its more correct and informative as to purpose > * add a couple of methods - isLink() and getClientProperties() - to > ConnectionIdentity (which seem reasonably in keeping with the current scope > of that interface) > * added new ConnectionControl interface extending ConnectionIdentity and use > this in place of broker::Connection itself in ConnectionObserver (at present > this just adds one method, abort()) > > Most of the rest of the changes merely update the implementations of > ConnectionObserver accordingly. Other changes: > > * have ConnectionIdentity of publisher returned by pointer from Message since > it may not be set > * remove Message::getPublisherOwnership(), add > Message::isLocalTo(OwnershipToken*) instead > * remove Message::getPublisherUserId(), Message::getPublisherObjectId(), > Message::getPublisherUrl(); these essentially duplicate the same methods on > ConnectionIdentity, are used only by ManagementAgent and (particularly the > latter two) really only make sense in that context anyway and I'm keen to > keep Message as clean as possible > > > This addresses bug QPID-4712. > https://issues.apache.org/jira/browse/QPID-4712 > > > Diffs > ----- > > /trunk/qpid/cpp/src/Makefile.am 1494646 > /trunk/qpid/cpp/src/qpid/acl/AclConnectionCounter.h 1494646 > /trunk/qpid/cpp/src/qpid/acl/AclConnectionCounter.cpp 1494646 > /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1494646 > /trunk/qpid/cpp/src/qpid/broker/Connection.h 1494646 > /trunk/qpid/cpp/src/qpid/broker/Connection.cpp 1494646 > /trunk/qpid/cpp/src/qpid/broker/ConnectionControl.h PRE-CREATION > /trunk/qpid/cpp/src/qpid/broker/ConnectionIdentity.h 1494646 > /trunk/qpid/cpp/src/qpid/broker/ConnectionObserver.h 1494646 > /trunk/qpid/cpp/src/qpid/broker/ConnectionObservers.h 1494646 > /trunk/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1494646 > /trunk/qpid/cpp/src/qpid/broker/Message.h 1494646 > /trunk/qpid/cpp/src/qpid/broker/Message.cpp 1494646 > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1494646 > /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1494646 > /trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1494646 > /trunk/qpid/cpp/src/qpid/ha/BackupConnectionExcluder.h 1494646 > /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.h 1494646 > /trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1494646 > /trunk/qpid/cpp/src/qpid/ha/ConnectionObserver.h 1494646 > /trunk/qpid/cpp/src/qpid/ha/ConnectionObserver.cpp 1494646 > /trunk/qpid/cpp/src/qpid/ha/Primary.h 1494646 > /trunk/qpid/cpp/src/qpid/ha/Primary.cpp 1494646 > /trunk/qpid/cpp/src/qpid/ha/RemoteBackup.h 1494646 > /trunk/qpid/cpp/src/qpid/ha/RemoteBackup.cpp 1494646 > /trunk/qpid/cpp/src/qpid/management/ManagementAgent.cpp 1494646 > > Diff: https://reviews.apache.org/r/11997/diff/ > > > Testing > ------- > > all tests pass > > > Thanks, > > Gordon Sim > >
