You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@shiro.apache.org by Emond Papegaaij <em...@topicus.nl> on 2012/12/27 15:14:13 UTC
Private methods in AuthorizingRealm
Hi all,
AuthorizingRealm is a good starting point for most custom realms, however the
large number of private methods prevent real customization. Over a year ago,
someone already asked to change isPermitted to protected in SHIRO-332 and now
we are facing similar issues.
In our application it is quite expensive to resolve the permissions for a
role, so I want to cache the permissions for an AuthorizationInfo, just as the
AuthorizationInfo itself is cached. For this to work, I have to change the
implementation of getPermissions to first check the cache and then perform the
default behavior. If getPermissions were protected, I could simply override
that method, check the cache and call super if nothing was cached, but now I
have to copy-paste all these methods (over 100 lines of code) to change only a
few lines:
private Collection<Permission> getPermissions(AuthorizationInfo)
to change the code
private Collection<Permission> resolvePermissions(Collection<String>)
private Collection<Permission> resolveRolePermissions(Collection<String>)
because they are used by getPermissions and private
private boolean isPermitted(Permission, AuthorizationInfo)
because it uses getPermissions
protected void checkPermission(Permission, AuthorizationInfo)
protected boolean[] isPermitted(List<Permission>, AuthorizationInfo)
public boolean isPermitted(PrincipalCollection, Permission)
protected boolean isPermittedAll(Collection<Permission>, AuthorizationInfo)
because they use isPermitted
Can this issue please be fixed for 1.2.2? The changes are binary compatible
and small, but would help is a lot.
Best regards,
Emond Papegaaij
Re: Private methods in AuthorizingRealm
Posted by Emond Papegaaij <em...@topicus.nl>.
Hi Les,
Good to hear that this will be changed. I didn't know about your strict
compatibility guidelines. For Wicket we only guarantee binary
compatibility, also on patch releases. If 1.3 is the version this will be
fixed in, that's fine. For now, we can live with the code duplication.
Best regards,
Emond
On Friday 28 December 2012 09:55:51 Les Hazlewood wrote:
> Hi Edmond,
>
> I'll fix this ASAP in trunk for 1.3, but it can't be fixed for 1.2.2:
> Per our versioning guidelines (http://apr.apache.org/versioning.html):
>
> Patch Versions:
>
> "To retain perfect source and binary compatibility, a patch release
> can only change function implementations. Changes to the API, to the
> signatures of public functions, or to the interpretation of function
> parameters is not allowed. Effectively, these releases are pure bug
> fix releases."
>
> In other words, if you use 1.2.4 for example, you should be able to
> downgrade to 1.2.1 with _no_ impact to your code. This wouldn't be
> possible in the scenario you describe.
>
> HTH,
>
> Les
>
>
> On Thu, Dec 27, 2012 at 6:14 AM, Emond Papegaaij
>
> <em...@topicus.nl> wrote:
> > Hi all,
> >
> >
> >
> > AuthorizingRealm is a good starting point for most custom realms, however
> > the large number of private methods prevent real customization. Over a
> > year ago, someone already asked to change isPermitted to protected in
> > SHIRO-332 and now we are facing similar issues.
> >
> >
> >
> > In our application it is quite expensive to resolve the permissions for a
> > role, so I want to cache the permissions for an AuthorizationInfo, just
> > as the AuthorizationInfo itself is cached. For this to work, I have to
> > change the implementation of getPermissions to first check the cache and
> > then perform the default behavior. If getPermissions were protected, I
> > could simply override that method, check the cache and call super if
> > nothing was cached, but now I have to copy-paste all these methods (over
> > 100 lines of code) to change only a few lines:
> >
> >
> >
> > private Collection<Permission> getPermissions(AuthorizationInfo)
> >
> > to change the code
> >
> > private Collection<Permission> resolvePermissions(Collection<String>)
> >
> > private Collection<Permission> resolveRolePermissions(Collection<String>)
> >
> > because they are used by getPermissions and private
> >
> > private boolean isPermitted(Permission, AuthorizationInfo)
> >
> > because it uses getPermissions
> >
> > protected void checkPermission(Permission, AuthorizationInfo)
> >
> > protected boolean[] isPermitted(List<Permission>, AuthorizationInfo)
> >
> > public boolean isPermitted(PrincipalCollection, Permission)
> >
> > protected boolean isPermittedAll(Collection<Permission>,
> > AuthorizationInfo)
> >
> > because they use isPermitted
> >
> >
> >
> > Can this issue please be fixed for 1.2.2? The changes are binary
> > compatible and small, but would help is a lot.
> >
> >
> >
> > Best regards,
> >
> > Emond Papegaaij
Re: Private methods in AuthorizingRealm
Posted by Les Hazlewood <lh...@apache.org>.
Hi Edmond,
I'll fix this ASAP in trunk for 1.3, but it can't be fixed for 1.2.2:
Per our versioning guidelines (http://apr.apache.org/versioning.html):
Patch Versions:
"To retain perfect source and binary compatibility, a patch release
can only change function implementations. Changes to the API, to the
signatures of public functions, or to the interpretation of function
parameters is not allowed. Effectively, these releases are pure bug
fix releases."
In other words, if you use 1.2.4 for example, you should be able to
downgrade to 1.2.1 with _no_ impact to your code. This wouldn't be
possible in the scenario you describe.
HTH,
Les
On Thu, Dec 27, 2012 at 6:14 AM, Emond Papegaaij
<em...@topicus.nl> wrote:
>
> Hi all,
>
>
>
> AuthorizingRealm is a good starting point for most custom realms, however the large number of private methods prevent real customization. Over a year ago, someone already asked to change isPermitted to protected in SHIRO-332 and now we are facing similar issues.
>
>
>
> In our application it is quite expensive to resolve the permissions for a role, so I want to cache the permissions for an AuthorizationInfo, just as the AuthorizationInfo itself is cached. For this to work, I have to change the implementation of getPermissions to first check the cache and then perform the default behavior. If getPermissions were protected, I could simply override that method, check the cache and call super if nothing was cached, but now I have to copy-paste all these methods (over 100 lines of code) to change only a few lines:
>
>
>
> private Collection<Permission> getPermissions(AuthorizationInfo)
>
> to change the code
>
> private Collection<Permission> resolvePermissions(Collection<String>)
>
> private Collection<Permission> resolveRolePermissions(Collection<String>)
>
> because they are used by getPermissions and private
>
> private boolean isPermitted(Permission, AuthorizationInfo)
>
> because it uses getPermissions
>
> protected void checkPermission(Permission, AuthorizationInfo)
>
> protected boolean[] isPermitted(List<Permission>, AuthorizationInfo)
>
> public boolean isPermitted(PrincipalCollection, Permission)
>
> protected boolean isPermittedAll(Collection<Permission>, AuthorizationInfo)
>
> because they use isPermitted
>
>
>
> Can this issue please be fixed for 1.2.2? The changes are binary compatible and small, but would help is a lot.
>
>
>
> Best regards,
>
> Emond Papegaaij