----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9703/#review17258 -----------------------------------------------------------
This is a promising first cut. Thanks for posting the patch. trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp <https://reviews.apache.org/r/9703/#comment36690> Don't put a queue count here as this forces a default into the command line switch. When that happens then quotas can never be disabled by have no cli setting and no rules in the rule file. trunk/qpid/cpp/src/qpid/acl/AclResourceCounter.cpp <https://reviews.apache.org/r/9703/#comment36689> The comparison for queue resources should be "<" not "<=". For connections the same line is <= but connections have a different life cycle. trunk/qpid/cpp/src/tests/acl.py <https://reviews.apache.org/r/9703/#comment36695> I'd like to see the name of the queue to be test-globally unique. The queue declare may fail because of a quota limit or becuse the queue already exists. If you use unique names then you can isolate the creation error to a quota denial. trunk/qpid/cpp/src/tests/acl.py <https://reviews.apache.org/r/9703/#comment36691> when a queue declare fails they you get the exception but as a side effect the session is gone. In order for the test to keep going you have to recreate a new session. See test_queue_allow_mode for example that does this. trunk/qpid/cpp/src/tests/acl.py <https://reviews.apache.org/r/9703/#comment36692> probably need a try/catch here - Chug Rolke On March 1, 2013, 5:07 p.m., Ernie Allen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9703/ > ----------------------------------------------------------- > > (Updated March 1, 2013, 5:07 p.m.) > > > Review request for qpid and Chug Rolke. > > > Description > ------- > > Implements acl queue limits per user. > - Modifies existing code to parse the acl file > - Adds code to count queues per user > - Adds max queue count of 65530 > - Adds python tests > > > This addresses bug QPID-4604. > https://issues.apache.org/jira/browse/QPID-4604 > > > Diffs > ----- > > trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1450922 > trunk/qpid/cpp/src/qpid/acl/AclData.h 1450922 > trunk/qpid/cpp/src/qpid/acl/AclData.cpp 1450922 > trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp 1450922 > trunk/qpid/cpp/src/qpid/acl/AclReader.h 1450922 > trunk/qpid/cpp/src/qpid/acl/AclReader.cpp 1450922 > trunk/qpid/cpp/src/qpid/acl/AclResourceCounter.h 1450922 > trunk/qpid/cpp/src/qpid/acl/AclResourceCounter.cpp 1450922 > trunk/qpid/cpp/src/tests/acl.py 1451307 > > Diff: https://reviews.apache.org/r/9703/diff/ > > > Testing > ------- > > Passes python tests: > - limit on a named user > - limit on a user in a group > - two acl entries for same user, last entry is honored > - limit of 0 on user is enforced > - if all is specified, unnamed user gets all's limit > - if all is not specified, unnamed user is denied > > > Thanks, > > Ernie Allen > >
