Fix and test for issues when components import or define a meta method
Tomas Doran [Sun, 7 Dec 2008 21:05:30 +0000 (21:05 +0000)]
Changes
TODO
lib/Catalyst.pm
lib/Catalyst/ClassData.pm
lib/Catalyst/Component.pm
lib/Catalyst/Controller.pm
lib/Catalyst/Log.pm
t/meta_method_unneeded.t [new file with mode: 0644]

diff --git a/Changes b/Changes
index 6ec0bc2..15432c4 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,10 @@
 # This file documents the revision history for Perl extension Catalyst.
 
+        - Fix issues in Catalyst::Controller::WrapCGI, 
+          Catalyst::Plugin::Cache::Curried and any other component which
+          import (or define) their own meta method by always explicitly
+          calling Class::MOP::Object->meta inside Catalyst (t0m)
+          - Add test for this (t0m)
         - Add test case for the bug which is causing the 
           Catalyst::Plugin::Authentication tests to fail (t0m)
         - Fix a bug in uri_for which could cause it to generate paths
diff --git a/TODO b/TODO
index d22b09f..47a8013 100644 (file)
--- a/TODO
+++ b/TODO
 
   - CatalystX-Imports, Class::MOP doesn't consider anon subs in the symbol table as methods, tests + fix?
 
-  - Catalyst::Plugin::Cache::Curried
-  - Catalyst::Controller::WrapCGI
-    - Both import 'meta' into controller package - mst thinks this can be fixed?.
-      but @marcus> and the generated accessors even call $_[0]->meta, so I don't think we can remove it. 
-      . Add a test for this in core.
-      . Fix core to use CMOP to look up metaclass, rather than ->meta method, and
-        namespace::clean the meta method out, fix generated accessors to do the same?
-
   - MooseX::Emulate::Class::Accessor::Fast
     - Tests for uses of C::A::F from Catalyst repos. - t0m
     - New release once these are done.
index 63c2fb7..7fdb797 100644 (file)
@@ -5,6 +5,7 @@ package Catalyst;
 use Class::C3;
 
 use Moose;
+use Class::MOP::Object ();
 extends 'Catalyst::Component';
 use bytes;
 use Catalyst::Exception;
@@ -2082,9 +2083,10 @@ sub setup_engine {
     }
 
     if ( $ENV{MOD_PERL} ) {
-
+        my $meta = $class->Class::MOP::Object::meta();
+        
         # create the apache method
-        $class->meta->add_method('apache' => sub { shift->engine->apache });
+        $meta->add_method('apache' => sub { shift->engine->apache });
 
         my ( $software, $version ) =
           $ENV{MOD_PERL} =~ /^(\S+)\/(\d+(?:[\.\_]\d+)+)/;
@@ -2216,7 +2218,7 @@ sub setup_log {
 
     my $env_debug = Catalyst::Utils::env_value( $class, 'DEBUG' );
     if ( defined($env_debug) ? $env_debug : $debug ) {
-        $class->meta->add_method('debug' => sub { 1 });
+        $class->Class::MOP::Object::meta()->add_method('debug' => sub { 1 });
         $class->log->debug('Debug messages enabled');
     }
 }
@@ -2240,7 +2242,7 @@ sub setup_stats {
 
     my $env = Catalyst::Utils::env_value( $class, 'STATS' );
     if ( defined($env) ? $env : ($stats || $class->debug ) ) {
-        $class->meta->add_method('use_stats' => sub { 1 });
+        $class->Class::MOP::Object::meta()->add_method('use_stats' => sub { 1 });
         $class->log->debug('Statistics enabled');
     }
 }
@@ -2283,9 +2285,9 @@ the plugin name does not begin with C<Catalyst::Plugin::>.
         $proto->_plugins->{$plugin} = 1;
         unless ($instant) {
             no strict 'refs';
-            if( $class->can('meta') ){
-              my @superclasses = ($plugin, $class->meta->superclasses );
-              $class->meta->superclasses(@superclasses);
+            if ( my $meta = $class->Class::MOP::Object::meta() ) {
+              my @superclasses = ($plugin, $meta->superclasses );
+              $meta->superclasses(@superclasses);
             } else {
               unshift @{"$class\::ISA"}, $plugin;
             }
index 2b00412..3882ccd 100644 (file)
@@ -2,6 +2,7 @@ package Catalyst::ClassData;
 
 use Moose::Role;
 use Class::MOP;
+use Class::MOP::Object;
 use Scalar::Util 'blessed';
 
 sub mk_classdata {
@@ -11,9 +12,9 @@ sub mk_classdata {
 
   my $slot = '$'.$attribute;
   my $accessor =  sub {
-    my $meta = $_[0]->meta;
     my $pkg = ref $_[0] || $_[0];
-    if(@_ > 1){
+    my $meta = $pkg->Class::MOP::Object::meta();
+    if (@_ > 1){
       $meta->namespace->{$attribute} = \$_[1];
       return $_[1];
     }
@@ -42,7 +43,7 @@ sub mk_classdata {
   confess("Failed to create accessor: $@ ")
     unless ref $accessor eq 'CODE';
 
-  my $meta = $class->meta;
+  my $meta = $class->Class::MOP::Object::meta();
   my $immutable_options;
   if( $meta->is_immutable ){
     $immutable_options = $meta->get_immutable_options;
index 0b92b4d..36b0523 100644 (file)
@@ -2,6 +2,7 @@ package Catalyst::Component;
 
 use Moose;
 use Class::MOP;
+use Class::MOP::Object;
 use MooseX::Adopt::Class::Accessor::Fast;
 use Catalyst::Utils;
 use Class::C3::Adopt::NEXT;
@@ -97,8 +98,8 @@ sub config {
         # work in a subclass. If we don't have the package symbol in the
         # current class we know we need to copy up to ours, which calling
         # the setter will do for us.
-
-        unless ($self->meta->has_package_symbol('$_config')) {
+        my $meta = $self->Class::MOP::Object::meta();
+        unless ($meta->has_package_symbol('$_config')) {
 
             $config = $self->merge_config_hashes( $config, {} );
             $self->_config( $config );
index 9ea7e7d..c5e6249 100644 (file)
@@ -4,6 +4,7 @@ package Catalyst::Controller;
 use base qw/Catalyst::Component Catalyst::AttrContainer/;
 use Moose;
 
+use Class::MOP::Object ();
 use Scalar::Util qw/blessed/;
 use Catalyst::Exception;
 use Catalyst::Utils;
@@ -189,7 +190,7 @@ sub register_actions {
     my $class = ref $self || $self;
     #this is still not correct for some reason.
     my $namespace = $self->action_namespace($c);
-    my $meta = $self->meta;
+    my $meta = $self->Class::MOP::Object::meta();
     my %methods = map { $_->body => $_->name }
         grep { $_->package_name ne 'Moose::Object' } #ignore Moose::Object methods
             $meta->get_all_methods;
index fad0f0e..a74f614 100644 (file)
@@ -4,6 +4,7 @@ use Moose;
 with 'MooseX::Emulate::Class::Accessor::Fast';
 
 use Data::Dump;
+use Class::MOP::Object ();
 
 our %LEVELS = ();
 
@@ -14,7 +15,7 @@ has abort => (is => 'rw');
 {
     my @levels = qw[ debug info warn error fatal ];
 
-    my $meta = __PACKAGE__->meta;
+    my $meta = __PACKAGE__->Class::MOP::Object::meta();
     for ( my $i = 0 ; $i < @levels ; $i++ ) {
 
         my $name  = $levels[$i];
diff --git a/t/meta_method_unneeded.t b/t/meta_method_unneeded.t
new file mode 100644 (file)
index 0000000..8d871d1
--- /dev/null
@@ -0,0 +1,22 @@
+use strict;
+use warnings;
+use Test::More tests => 1;
+use Test::Exception;
+use Carp ();
+$SIG{__DIE__} = \&Carp::confess; # Stacktrace please.
+
+# Doing various silly things, like for example
+# use CGI qw/:stanard/ in your conrtoller / app
+# will overwrite your meta method, therefore Catalyst
+# can't depend on it being there correctly.
+
+# This is/was demonstrated by Catalyst::Controller::WrapCGI
+# and Catalyst::Plugin::Cache::Curried
+
+{    
+    package TestAppWithMeta;
+    use Catalyst;
+    sub meta {}
+}
+
+lives_ok { TestAppWithMeta->setup } 'Can setup an app which defines its own meta method';