On Thu, May 11, 2006 at 12:31:47 +0200, Michele Beltrame wrote:
> Hello!
> 
> I'm attaching a patch for Catalyst::Plugin::Authorization::Roles.

Hi,

I missed this, sorry. If i seem to be ignoring patches please slap
me - I get a lot of mail and sometimes things to mailing lists get
lost....


The code looks good, except for the self check thing- since it's an
optimisation the user class should provide a ->check_roles_any
method, otherwise nothing is gained.

For now the entire if/else can be removed, until
Auth::Store::DBIC implements the internals to optimise it.

If you have tests you may commit this yourself- coordinate with us
on #catalyst or #catalyst-dev and we'll give you SVN access.

Thanks a lot for your work!

> +        my $have = Set::Object->new( $user->roles() );
> +        my $role_ok = 0;
> +        for my $role (@_) {
> +            my $need = Set::Object->new(@_);
> +            $role_ok = 1, last if $have->superset($need);
> +        }

This part can be simplified, simply construct a set from the @_
(please give it a name, btw, it makes reading the code easier), then
you can do:

        if ( $have->intersection( $wanted )->size > 0 ) {
                # ok    
        }

thus removing the need for the loop.

-- 
  Yuval Kogman <[EMAIL PROTECTED]>
http://nothingmuch.woobling.org  0xEBD27418

Attachment: pgpOsvqwZmz3h.pgp
Description: PGP signature

_______________________________________________
List: [email protected]
Listinfo: http://lists.rawmode.org/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/[email protected]/
Dev site: http://dev.catalyst.perl.org/

Reply via email to