[jifty-devel] [PATCH] Call isa() and can() as Methods

Jesse Vincent jesse at bestpractical.com
Tue Mar 14 17:59:14 EST 2006




On Tue, Mar 14, 2006 at 02:20:59PM -0800, chromatic wrote:
> While doing some research on Jifty::DBI, I read the Jifty code and wrote this 
> patch to replace UNIVERSAL::isa() and UNIVERSAL::can() calls with their 
> method equivalents (or alternate tests, as appropriate).  All tests pass.

Thanks!

Sadly, it looks like we're missing Jifty core coverage for RightsFrom.
From our internal app:

t/19-task-collection-acls.......Can't call method "isa" without a
package or object reference at /usr/local/share/perl/5.8.7/Jifty/RightsFrom.pm line 113.



> 
> -- c
> 

> === lib/Jifty/Action/Record/Update.pm
> ==================================================================
> --- lib/Jifty/Action/Record/Update.pm	(revision 14160)
> +++ lib/Jifty/Action/Record/Update.pm	(local)
> @@ -91,7 +91,7 @@
>  
>          # Skip fields that have not changed
>          my $old = $self->record->$field;
> -        $old = $old->id if UNIVERSAL::isa( $old, "Jifty::Record" );
> +        $old = $old->id if $old->isa( 'Jifty::Record' );
>      
>          # if both the new and old values are defined and equal, we don't want to change em
>          # XXX TODO "$old" is a cheap hack to scalarize datetime objects
> === lib/Jifty/Action/Record.pm
> ==================================================================
> --- lib/Jifty/Action/Record.pm	(revision 14160)
> +++ lib/Jifty/Action/Record.pm	(local)
> @@ -69,7 +69,7 @@
>      if (ref $record_class) {
>          $self->record($record_class);
>          $self->argument_value($_, $self->record->$_) for @{ $self->record->_primary_keys };
> -    } elsif (UNIVERSAL::isa($args{record}, $record_class)) {
> +    } elsif ($args{record}->isa($record_class)) {
>          $self->record($args{record});
>          $self->argument_value($_, $self->record->$_) for @{ $self->record->_primary_keys };
>      } else {
> @@ -116,7 +116,7 @@
>  
>              # If the current value is actually a pointer to another object, dereference it
>              $current_value = $current_value->id
> -              if UNIVERSAL::isa( $current_value, 'Jifty::Record' );
> +              if $current_value->isa( 'Jifty::Record' );
>              $info->{default_value} = $current_value if $self->record->id;
>          }
>  
> @@ -143,7 +143,7 @@
>  
>          elsif ( defined $column->refers_to ) {
>              my $ref = $column->refers_to;
> -            if ( UNIVERSAL::isa( $ref, 'Jifty::Record' ) ) {
> +            if ( $ref->isa( 'Jifty::Record' ) ) {
>  
>                  my $collection = Jifty::Collection->new(
>                      record_class => $ref,
> === lib/Jifty/Action.pm
> ==================================================================
> --- lib/Jifty/Action.pm	(revision 14160)
> +++ lib/Jifty/Action.pm	(local)
> @@ -566,7 +566,7 @@
>      my $default_method = 'canonicalize_' . $field;
>  
>      if ( $field_info->{canonicalizer}
> -        and UNIVERSAL::isa( $field_info->{canonicalizer}, 'CODE' ) )
> +        and defined &{ $field_info->{canonicalizer} } )
>      {
>          $value = $field_info->{canonicalizer}->( $self, $value );
>      }
> @@ -651,7 +651,7 @@
>  
>      # Finally, fall back to running a validator sub
>      if ( $field_info->{validator}
> -        and UNIVERSAL::isa( $field_info->{validator}, 'CODE' ) )
> +        and defined &{ $field_info->{validator} } )
>      {
>          return $field_info->{validator}->( $self, $value );
>      }
> === lib/Jifty/Dispatcher.pm
> ==================================================================
> --- lib/Jifty/Dispatcher.pm	(revision 14160)
> +++ lib/Jifty/Dispatcher.pm	(local)
> @@ -683,7 +683,7 @@
>      eval { Jifty->handler->mason->handle_comp(request->path); };
>      my $err = $@;
>      # Handle parse errors
> -    if ( $err and not UNIVERSAL::isa $err, 'HTML::Mason::Exception::Abort' ) {
> +    if ( $err and not eval { $err->isa( 'HTML::Mason::Exception::Abort' ) } ) {
>          # XXX TODO: get this into the browser somehow
>          warn "Mason error: $err";
>          Jifty->web->redirect("/__jifty/error/mason_internal_error");
> === lib/Jifty/Mason/Halo.pm
> ==================================================================
> --- lib/Jifty/Mason/Halo.pm	(revision 14160)
> +++ lib/Jifty/Mason/Halo.pm	(local)
> @@ -44,7 +44,7 @@
>          push @$STACK,
>              {
>              id           => $halo_base,
> -            args         => [map {UNIVERSAL::isa($_,"GLOB") ? "*GLOB*" : $_} @{$context->args}],
> +            args         => [map { eval { fileno( $_ ) } ? "*GLOB*" : $_} @{$context->args}],
>              start_time   => Time::HiRes::time(),
>              path         => $context->comp->path,
>              subcomponent => (  $context->comp->is_subcomp() ? 1:0),
> === lib/Jifty/Model/Schema.pm
> ==================================================================
> --- lib/Jifty/Model/Schema.pm	(revision 14160)
> +++ lib/Jifty/Model/Schema.pm	(local)
> @@ -79,7 +79,7 @@
>    my $self = shift;
>    my $ver = shift;
>  
> -  unless (UNIVERSAL::isa($ver, "version")) {
> +  unless ( eval { $ver->isa( 'version' ) } ) {
>      $self->log->fatal("Version must be a version object");
>      return;
>    }
> === lib/Jifty/Object.pm
> ==================================================================
> --- lib/Jifty/Object.pm	(revision 14160)
> +++ lib/Jifty/Object.pm	(local)
> @@ -79,7 +79,7 @@
>              my $caller_self      = $DB::args[0];
>              next unless (ref($caller_self)); #skip class methods;
>              next if ($caller_self eq $self);
> -            next unless UNIVERSAL::can($caller_self => 'current_user');
> +            next unless caller_self->can('current_user');
>  
>              eval {
>                  if ( $caller_self->current_user and $caller_self->current_user->id) {
> === lib/Jifty/Record.pm
> ==================================================================
> --- lib/Jifty/Record.pm	(revision 14160)
> +++ lib/Jifty/Record.pm	(local)
> @@ -165,7 +165,7 @@
>          return $self->delegate_current_user_can($right, @_); 
>      }
>  
> -    unless ( UNIVERSAL::isa( $self->current_user, 'Jifty::CurrentUser' ) ) {
> +    unless ( $self->current_user->isa( 'Jifty::CurrentUser' ) ) {
>          $self->log->error(
>              "Hm. called to authenticate without a currentuser - "
>                  . $self->current_user );
> @@ -260,7 +260,7 @@
>      my $classname = $column->refers_to();
>  
>      return undef unless $classname;
> -    return unless UNIVERSAL::isa( $classname, 'Jifty::DBI::Collection' );
> +    return unless $classname->isa( 'Jifty::DBI::Collection' );
>  
>  
>      my $coll = $classname->new( current_user => $self->current_user );
> @@ -302,7 +302,7 @@
>  
>      return unless defined $value;
>      return undef unless $classname;
> -    return unless UNIVERSAL::isa( $classname, 'Jifty::Record' );
> +    return unless $classname->isa( 'Jifty::Record' );
>  
>      # XXX TODO FIXME we need to figure out the right way to call new here
>      # perhaps the handle should have an initiializer for records/collections
> === lib/Jifty/RightsFrom.pm
> ==================================================================
> --- lib/Jifty/RightsFrom.pm	(revision 14160)
> +++ lib/Jifty/RightsFrom.pm	(local)
> @@ -110,7 +110,7 @@
>      my $obj_type = $column->refers_to();
>  
>  
> -    if ( UNIVERSAL::isa( $attribs{ $column->name }, $obj_type ) ) {
> +    if ( $attribs{ $column->name }->isa( $obj_type ) ) {
>          $obj = $attribs{ $column->name };
>      } elsif ( $attribs{ $column->name }
>          || $self->__value( $column->name )
> === lib/Jifty/Script/Schema.pm
> ==================================================================
> --- lib/Jifty/Script/Schema.pm	(revision 14160)
> +++ lib/Jifty/Script/Schema.pm	(local)
> @@ -195,9 +195,9 @@
>         # TODO XXX FIXME:
>         #   This *will* try to generate SQL for abstract base classes you might
>         #   stick in $AC::Model::.
> -        next if not UNIVERSAL::isa( $model, 'Jifty::Record' );
> +        next unless $model->isa( 'Jifty::Record' );
>          do { log->info("Skipping $model"); next }
> -            if ( UNIVERSAL::can( $model, 'since' )
> +            if ( $model->can( 'since' )
>              and $appv < $model->since );
>  
>          $log->info("Using $model");
> @@ -228,7 +228,7 @@
>              Jifty::Util->require($bootstrapper);
>  
>              $bootstrapper->run()
> -                if ( UNIVERSAL::can( $bootstrapper => 'run' ) );
> +                if  $bootstrapper->can( 'run' );
>          };
>          die $@ if $@;
>  
> @@ -283,8 +283,7 @@
>      for my $model ( __PACKAGE__->models ) {
>  
>          # We don't want to get the Collections, for example.
> -        do {next}
> -            unless UNIVERSAL::isa( $model, 'Jifty::Record' );
> +        do {next} unless $model->isa( 'Jifty::Record' );
>  
>          # Set us up the table
>          $model = $model->new;
> @@ -292,7 +291,7 @@
>              ->_db_schema_table_from_model($model);
>  
>          # If this whole table is new
> -        if (    UNIVERSAL::can( $model, "since" )
> +        if (    $model->can( 'since' )
>              and $appv >= $model->since
>              and $dbv < $model->since )
>          {
> === lib/Jifty/Server.pm
> ==================================================================
> --- lib/Jifty/Server.pm	(revision 14160)
> +++ lib/Jifty/Server.pm	(local)
> @@ -124,7 +124,7 @@
>  sub recording_on {
>      my $class = shift;
>      our @ISA;
> -    unshift @ISA, "HTTP::Server::Simple::Recorder" unless UNIVERSAL::isa($class, 'HTTP::Server::Simple::Recorder');
> +    unshift @ISA, "HTTP::Server::Simple::Recorder" unless $class->isa('HTTP::Server::Simple::Recorder');
>  } 
>  
>  1;
> === share/web/templates/__jifty/admin/_elements/nav
> ==================================================================
> --- share/web/templates/__jifty/admin/_elements/nav	(revision 14160)
> +++ share/web/templates/__jifty/admin/_elements/nav	(local)
> @@ -22,7 +22,7 @@
>      # TODO XXX FIXME:
>      #   This *will* try to generate SQL for abstract base classes you might
>      #   stick in $AC::Model::.
> -    next if not UNIVERSAL::isa( $model, 'Jifty::Record' );
> +    next unless $model->isa( 'Jifty::Record' );
>      next if $model =~ /::SUPER$/;
>      push @models, $model;
>  }
> === share/web/templates/__jifty/admin/index.html
> ==================================================================
> --- share/web/templates/__jifty/admin/index.html	(revision 14160)
> +++ share/web/templates/__jifty/admin/index.html	(local)
> @@ -22,7 +22,7 @@
>      # TODO XXX FIXME:
>      #   This *will* try to generate SQL for abstract base classes you might
>      #   stick in $AC::Model::.
> -    next if not UNIVERSAL::isa( $model, 'Jifty::Record' );
> +    next unless $model->isa( 'Jifty::Record' );
>      next if $model =~ /::SUPER$/;
>      push @models, $model;
>  }
> 

> _______________________________________________
> jifty-devel mailing list
> jifty-devel at lists.jifty.org
> http://lists.jifty.org/cgi-bin/mailman/listinfo/jifty-devel


-- 


More information about the jifty-devel mailing list