From: Tomas Doran <bobtfish@bobtfish.net>
Date: Sun, 21 Dec 2008 14:30:19 +0000 (+0000)
Subject: Authentication back compat fail due to ::ClassData not behaving like C::D::I, not... 
X-Git-Tag: 5.8000_05~86
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=commitdiff_plain;h=85781ad7ed72409d08127c0c23610ecb2de5663a

Authentication back compat fail due to ::ClassData not behaving like C::D::I, not CAF compat or MRO as previously suspected
---

diff --git a/TODO b/TODO
index 9178843..78bee7c 100644
--- a/TODO
+++ b/TODO
@@ -6,9 +6,9 @@ Pending patches:
 
 Known issues:  
 
-  - Fix t/caf_backcompat_plugin_accessor_override.t - Test doesn't properly
-    demonstrate the issue as it fails against 5.70. Only passes in real auth
-    plugin due to method ordering?
+  - Fix t/cdi_backcompat_plugin_accessor_override.t - Catalyst::ClassData
+    doesn't emulate Class::Data::Inheritable by crapping on the symbol table
+    of the top level MyApp class, which causes back-compat fail.
 
 Cleanups:
 
diff --git a/t/caf_backcompat_plugin_accessor_override.t b/t/caf_backcompat_plugin_accessor_override.t
deleted file mode 100644
index e612fdc..0000000
--- a/t/caf_backcompat_plugin_accessor_override.t
+++ /dev/null
@@ -1,24 +0,0 @@
-use strict;
-use warnings;
-use lib 't/lib';
-
-use Test::More tests => 1;
-use Test::Exception;
-
-# Force a stack trace.
-use Carp;
-$SIG{__DIE__} = \&Carp::confess;
-
-{
-    package CAFCompatTestApp;
-    use Catalyst qw/
-	    +CAFCompatTestPlugin
-    /;
-}
-
-TODO: {
-    local $TODO = 'The overridden setup in CAFCompatTestApp + the overridden accessor causes destruction';
-    lives_ok {
-        CAFCompatTestApp->setup;
-    } 'Setup app with plugins which says use base qw/Class::Accessor::Fast/';
-}
diff --git a/t/cdi_backcompat_plugin_accessor_override.t b/t/cdi_backcompat_plugin_accessor_override.t
new file mode 100644
index 0000000..8a91781
--- /dev/null
+++ b/t/cdi_backcompat_plugin_accessor_override.t
@@ -0,0 +1,32 @@
+use strict;
+use warnings;
+use lib 't/lib';
+
+use Test::More tests => 1;
+use Test::Exception;
+
+# Force a stack trace.
+use Carp;
+$SIG{__DIE__} = \&Carp::confess;
+
+{
+    package CDICompatTestApp;
+    use Catalyst qw/
+	    +CDICompatTestPlugin
+    /;
+    # Calling ->config here (before we call setup). With CDI/Cat 5.70 this
+    # causes *CDICompatTestApp::_config to have a class data accessor created.
+    
+    # If this doesn't happen, then later when we've added CDICompatTestPlugin
+    # to @ISA, we fail in the overridden ->setup method when we call ->config
+    # again, as we get the CAF accessor from CDICompatTestPlugin, not the one
+    # created in this package as a side-effect of this call. :-(
+    __PACKAGE__->config;
+}
+
+TODO: {
+    local $TODO = 'The overridden setup in CDICompatTestApp + the overridden accessor causes destruction';
+    lives_ok {
+        CDICompatTestApp->setup;
+    } 'Setup app with plugins which says use base qw/Class::Accessor::Fast/';
+}
diff --git a/t/lib/CAFCompatTestPlugin.pm b/t/lib/CAFCompatTestPlugin.pm
deleted file mode 100644
index b663ae5..0000000
--- a/t/lib/CAFCompatTestPlugin.pm
+++ /dev/null
@@ -1,30 +0,0 @@
-package CAFCompatTestPlugin;
-
-# This plugin specificially tests an edge case of CAF compat,
-# where you load a plugin which uses base CAF, and then override
-# a core catalyst accessor (_config in this case)..
-
-# This is what happens if you use the authentication back-compat
-# stuff, as C::A::Plugin::Credential::Password is added to the plugin
-# list, and the base C::A::C::P class, does the mk_accessors, and
-# then the C::P::A class calls the config method before setup finishes...
-
-use strict;
-use warnings;
-
-# Note that we don't actually _really_ use CAF here, as MX::Adopt::CAF
-# is in place...
-use base qw/Class::Accessor::Fast/;
-
-BEGIN {
-    __PACKAGE__->mk_accessors(qw/_config/);
-}
-
-sub setup {
-    my $app = shift;
-
-    $app->config;
-    $app->NEXT::setup(@_);
-}
-
-1;
diff --git a/t/lib/CDICompatTestPlugin.pm b/t/lib/CDICompatTestPlugin.pm
new file mode 100644
index 0000000..43b4270
--- /dev/null
+++ b/t/lib/CDICompatTestPlugin.pm
@@ -0,0 +1,27 @@
+package CDICompatTestPlugin;
+
+# This plugin specificially tests an edge case of C::D::I compat,
+# where you load a plugin which creates an accessor with the same
+# name as a class data accessor (_config in this case)..
+
+# This is what happens if you use the authentication back-compat
+# stuff, as C::A::Plugin::Credential::Password is added to the plugin
+# list, and that uses base C::A::C::P class, does the mk_accessors.
+
+# If a class data method called _config hasn't been created in 
+# MyApp ($app below), then our call to ->config gets our accessor
+# (rather than the class data one), and we fail..
+
+use strict;
+use warnings;
+use base qw/Class::Accessor::Fast/;
+__PACKAGE__->mk_accessors(qw/_config/);
+
+sub setup {
+    my $app = shift;
+
+    $app->config;
+    $app->NEXT::setup(@_);
+}
+
+1;