menu isUserInRole impl

[Bob Schellink <sa...@gmail.com>]

Hi all,

If a menu doesn't define any roles, isUserInRoles will return false. I wonder if this is the correct 
default. Wouldn't it be better to return true if no roles are defined? Otherwise one will have to 
define an anonymous role and set it on these menus.

regards

bob


Proposed change:

   public boolean isUserInRoles() {
     // PROPOSED CHANGE START
     List roles = getRoles();
     if (roles == null || roles.isEmpty()) {
       return true;
     }
     // END

     if (getAccessController() == null) {
       String msg = "Menu accessController has not been defined";
       throw new IllegalStateException(msg);
     }

     HttpServletRequest request = getContext().getRequest();

     for (int i = 0, size = roles.size(); i < size; i++) {
       String rolename = (String) roles.get(i);
       if (getAccessController().hasAccess(request, rolename)) {
         return true;
       }
     }

     return false;
   }

Re: menu isUserInRole impl

[Malcolm Edgar <ma...@gmail.com>]

I think in terms of the method name this is correct behaviour. If
there are not roles defined, then I dont think it is correct to say
the user belongs to the empty set of roles.

Making this change could impact existing applications in unexpected ways.

You can always write you menu macro to test for the menu.getRoles().isEmpty()

regards Malcolm Edgar

On Sun, Nov 29, 2009 at 1:18 PM, Bob Schellink <sa...@gmail.com> wrote:
> Hi all,
>
> If a menu doesn't define any roles, isUserInRoles will return false. I
> wonder if this is the correct default. Wouldn't it be better to return true
> if no roles are defined? Otherwise one will have to define an anonymous role
> and set it on these menus.
>
> regards
>
> bob
>
>
> Proposed change:
>
>  public boolean isUserInRoles() {
>    // PROPOSED CHANGE START
>    List roles = getRoles();
>    if (roles == null || roles.isEmpty()) {
>      return true;
>    }
>    // END
>
>    if (getAccessController() == null) {
>      String msg = "Menu accessController has not been defined";
>      throw new IllegalStateException(msg);
>    }
>
>    HttpServletRequest request = getContext().getRequest();
>
>    for (int i = 0, size = roles.size(); i < size; i++) {
>      String rolename = (String) roles.get(i);
>      if (getAccessController().hasAccess(request, rolename)) {
>        return true;
>      }
>    }
>
>    return false;
>  }
>
>