[jira] [Issue Comment Edited] (SHIRO-307) DomainPermission does not fully support domain, actions and targets properties

["Phil Steitz (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SHIRO-307?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13054227#comment-13054227 ] 

Phil Steitz edited comment on SHIRO-307 at 6/24/11 5:08 AM:
------------------------------------------------------------

Thanks, Wes.  Works4me other than missing the constructor that I added in the second version of the patch:

{code}
public DomainPermission(String domain, Set<String> actions, Set<String> targets) {
        this.domain = domain;
        this.actions = actions;
        this.targets = targets;
        setParts(domain, actions, targets);
    }
{code}

I don't actually use or depend on the properties of this class and I wonder if in fact we need them, since (at least in the use I am making of it) it is essentially just a facade for WildcardPermission.  I don't know what the general Shiro philosophy is on mutability and in particular mutable permissions, but it might be simpler to not introduce it in the facade.  I guess that's why you made the setters protected?  But then how exactly are they expected to be used?   So maybe a) the setters should be dropped... or b) the fields themselves should be dropped...  or even c) drop the class entirely and just add a constructor as above to WildcardPermission

Sorry if my newbie-ness is leading to confusion here.

      was (Author: psteitz):
    Thanks, Wes.  Works4me other than missing the constructor that I added in the second version of the patch:
{code}
public DomainPermission(String domain, Set<String> actions, Set<String> targets) {
        this.domain = domain;
        this.actions = actions;
        this.targets = targets;
        setParts(domain, actions, targets);
    }
{code}

I don't actually use or depend on the properties of this class and I wonder if in fact we need them, since (at least in the use I am making of it) it is essentially just a facade for WildcardPermission.  I don't know what the general Shiro philosophy is on mutability and in particular mutable permissions, but it might be simpler to not introduce it in the facade.  I guess that's why you made the setters protected?  But then how exactly are they expected to be used?   So maybe a) the setters should be dropped... or b) the fields themselves should be dropped...  or even c) drop the class entirely and just add a constructor as above to WildcardPermission

Sorry if my newbie-ness is leading to confusion here.
  
> 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