[
https://issues.apache.org/jira/browse/SHIRO-307?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13054792#comment-13054792
]
Les Hazlewood edited comment on SHIRO-307 at 6/25/11 1:23 AM:
--------------------------------------------------------------
Ah yes - good questions.
I removed that constructor and had the others protected because typically the
DomainPermission (at least in my experience) would always subclassed to
represent the 'domain' (or more accurately, the resource type) being protected.
For example,
DocumentPermission extends DomainPermission
Based on this use case at least, there is no need for a public constructor
taking in the 'domain' argument since it is always derived from the class name.
So, in essence, DomainPermission was intended as a way to represent resources
in a compile-time type-safe manner. It doesn't really provide any benefit
other than that (and maybe a way to codify explicit 'targets' and 'actions'
whereas those concepts are not mandated in the WildcardPermission superclass).
On another note, I forgot about the mutability issue - Permissions are intended
to be immutable, so we won't need any of the mutators (also helps w/
concurrency). I suppose someone could make a mutable Permission
implementation, but I haven't seen the need for one and no-one seems to have
problems with them being immutable (like you've pointed out).
I guess my vote would be to deprecate DomainPermission entirely since the
actions/targets representation could be stored as a String or a Set of Strings
depending on how you wish to store that data - which is application specific.
Thoughts?
was (Author: lhazlewood):
Ah yes - good questions.
I removed that constructor and had the others protected because typically the
DomainPermission (at least in my experience) would always subclassed to
represent the 'domain' (or more accurately, the resource type) being protected.
For example,
DocumentPermission extends DomainPermission
Based on this use case at least, there is no need for a public constructor
taking in the 'domain' argument since it is always derived from the class name.
So, in essence, DomainPermission was intended as a way to represent resources
in a compile-time type-safe manner. It doesn't really provide any benefit
other than that (and maybe a way to codify explicit 'targets' and 'actions'
whereas those concepts are not mandated in the WildcardPermission superclass).
On another note, I forgot about the mutability issue - Permissions are intended
to be immutable, so we won't need any of the mutators (also helps w/
concurrency). I suppose someone could make a mutable Permission
implementation, but I haven't seen the need for one and no-one seems to have
problems with them being immutable (like you've pointed out).
I guess my vote would be to deprecate DomainPermission entirely since whether
the actions/targets representation could be stored as a String or a Set of
Strings depending on how you wish to store that data - which is application
specific. Thoughts?
> DomainPermission does not fully support domain, actions and targets properties
> ------------------------------------------------------------------------------
>
> Key: SHIRO-307
> URL: https://issues.apache.org/jira/browse/SHIRO-307
> Project: Shiro
> Issue Type: Bug
> Components: Authorization (access control)
> Affects Versions: 1.0.0, 1.1.0
> Reporter: Phil Steitz
> Assignee: Les Hazlewood
> Attachments: DomainPermission.patch
>
>
> Per the class javadoc, DomainPermission is designed to be a base class for
> Permission implementations that persist permission parts as separate
> properties. It defines private fields for domain, actions and targets and
> exposes getters/setters for these, but the setParts and constructor methods
> that set Permission state do not call the property setters and the property
> setters don't call setParts. Property synchronization needs to be added to
> this class.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira