[
https://issues.apache.org/jira/browse/QPID-7318?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15399814#comment-15399814
]
Rob Godfrey commented on QPID-7318:
-----------------------------------
h4. Patch 4
{quote}
* In ACO child creation is authorised in createChildAsync. In a couple of
places (e.g., BindingImpl) it is again checked in validateOnCreate. Is this by
design?
{quote}
Yes it is intentional - BindingImpls are not created in the "proper" way...
there is much wrong in Bindings...
{quote}
* we had this elsewhere but the way "priority" in AccessControlProvider and
ACCESS_CONTROL_PROVIDER_COMPARATOR work a low "priority" means it will take
precedence over a high "priority". common but not very intuitive. But I now see
that you mention this in the UI which is good.
{quote}
Yeah - the way priority is being used is consistent with our general approach.
"Number 1 priority" means very high priority. This is just English being
illogical.
{quote}
* AccessControlSource seems to be only used in tests. Why was it split from
AccessControlProvider? Should CommonAccessControlProvider extend
AccessControlSource?
{quote}
It is only used in tests and is a way of allowing mocks to implement functions
that otherwise would require us to expose private methods as part of the public
interface of the objects. Having these interfaces is ugly, but I consider it
way less offensive than making private implementation details public simply to
allow for allowing the tests to mock objects. Our non-test classes should
never need to implement these interfaces as they inherit from
AbstractConfiguredObject and thus the actual (private/protected) methods can be
used.
{quote}
* Beside all the formating being of this instance deserves special mention: The
closing brace of CachingSecurityToken#authorise is on the wrong line.
* I have the same concern about the while-loop in
CachingSecurityToken#authorise as about the one in
CachingSecurityToken#authoriseMethod mentioned above
{quote}
Think this was actually what I fixed in the previous commit.
> [Java Broker] Refactor existing ACL plugin code
> -----------------------------------------------
>
> Key: QPID-7318
> URL: https://issues.apache.org/jira/browse/QPID-7318
> Project: Qpid
> Issue Type: Improvement
> Components: Java Broker
> Reporter: Rob Godfrey
> Assignee: Rob Godfrey
> Fix For: qpid-java-6.1
>
>
> While the aim is to redesign the ACL implementation in the v6.2 or v7.0
> timeframe, there is still utility in tidying up the existing ACL
> implementation a bit. In particular by separating out functions and
> providing a better encapsulation, we will make the job of writing automated
> upgraders to any new ACL implementation substantially easier.
> As a first step we can separate out the parsing of the ACL file, from the
> "rule based" implementation of ACLs.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]