register sources on schema class, never instance

Change _invoke_loader in Loader.pm to always invoke the loader on the
Schema class, never the instance, even if an instance is passed in, and
then merge the state of the class into the instance if necessary.

Fix up a couple of tests to work with this new logic.

Also change make_schema_at to use ->connect instead of ->connection
because of people relying on instance state in ->connection (RT#74175),
being careful to copy the storage back to the Schema class.

diff --git a/Changes b/Changes
index e69b5d9..5cadd30 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,10 @@
 Revision history for Perl extension DBIx::Class::Schema::Loader
 
+        - use ::Schema::connect instead of ::Schema::connection in
+          make_schema_at (RT#74175)
+        - register sources on the schema class, never the instance, regardless
+          of how the connection is made for dynamic schemas
+
 0.07020  2012-03-31 21:34:06
         - fix some mro issues under perl 5.8
 

diff --git a/Makefile.PL b/Makefile.PL
index 9c6bca7..24d0b48 100644 (file)
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -58,6 +58,7 @@ requires 'Exporter'                    => '5.63';
 requires 'Try::Tiny'                   => 0;
 requires 'String::ToIdentifier::EN'    => '0.05';
 requires 'String::CamelCase'           => '0.02';
+requires 'Hash::Merge'                 => 0;
 
 if ($Module::Install::AUTHOR && ! $args->{skip_author_deps}) {
     eval { require Module::Install::ReadmeFromPod }

diff --git a/lib/DBIx/Class/Schema/Loader.pm b/lib/DBIx/Class/Schema/Loader.pm
index cf7f7e0..ecf89d5 100644 (file)
--- a/lib/DBIx/Class/Schema/Loader.pm
+++ b/lib/DBIx/Class/Schema/Loader.pm
@@ -9,6 +9,8 @@ use Carp::Clan qw/^DBIx::Class/;
 use Scalar::Util 'weaken';
 use Sub::Name 'subname';
 use DBIx::Class::Schema::Loader::Utils 'array_eq';
+use Try::Tiny;
+use Hash::Merge 'merge';
 use namespace::clean;
 
 # Always remember to do all digits for the version even if they're 0
@@ -170,10 +172,15 @@ sub _invoke_loader {
 
     my $args = $self->_loader_args;
 
-    # set up the schema/schema_class arguments
-    $args->{schema} = $self;
+    # temporarily copy $self's storage to class
+    my $class_storage = $class->storage;
+    if (ref $self) {
+        $class->storage($self->storage);
+        $class->storage->set_schema($class);
+    }
+
+    $args->{schema} = $class;
     $args->{schema_class} = $class;
-    weaken($args->{schema}) if ref $self;
     $args->{dump_directory} ||= $self->dump_to_dir;
     $args->{naming} = $self->naming if $self->naming;
     $args->{use_namespaces} = $self->use_namespaces if defined $self->use_namespaces;
@@ -186,14 +193,76 @@ sub _invoke_loader {
     };
 
     my $impl = $loader_class || "DBIx::Class::Schema::Loader" . $self->storage_type;
-    eval { $self->ensure_class_loaded($impl) };
-    croak qq/Could not load loader_class "$impl": "$@"/ if $@;
+    try {
+        $self->ensure_class_loaded($impl)
+    }
+    catch {
+        croak qq/Could not load loader_class "$impl": "$_"/;
+    };
 
-    $self->loader($impl->new(%$args));
-    $self->loader->load;
-    $self->_loader_invoked(1);
+    $class->loader($impl->new(%$args));
+    $class->loader->load;
+    $class->_loader_invoked(1);
 
-    $self;
+    # copy to $self
+    if (ref $self) {
+        $self->loader($class->loader);
+        $self->_loader_invoked(1);
+
+        $self->_merge_state_from($class);
+    }
+
+    # restore $class's storage
+    $class->storage($class_storage);
+
+    return $self;
+}
+
+# FIXME This needs to be moved into DBIC at some point, otherwise we are
+# maintaining things to do with DBIC guts, which we have no business of
+# maintaining. But at the moment it would be just dead code in DBIC, so we'll
+# maintain it here.
+sub _merge_state_from {
+    my ($self, $from) = @_;
+
+    my $orig_class_mappings       = $self->class_mappings;
+    my $orig_source_registrations = $self->source_registrations;
+
+    $self->_copy_state_from($from);
+
+    $self->class_mappings(merge($orig_class_mappings, $self->class_mappings))
+        if $orig_class_mappings;
+
+    $self->source_registrations(merge($orig_source_registrations, $self->source_registrations))
+        if $orig_source_registrations;
+}
+
+sub _copy_state_from {
+    my $self = shift;
+    my ($from) = @_;
+
+    # older DBIC's do not have this method
+    if (try { DBIx::Class->VERSION('0.08197'); 1 }) {
+        return $self->next::method(@_);
+    }
+    else {
+        # this is a copy from DBIC git master pre 0.08197
+        $self->class_mappings({ %{$from->class_mappings} });
+        $self->source_registrations({ %{$from->source_registrations} });
+
+        foreach my $moniker ($from->sources) {
+            my $source = $from->source($moniker);
+            my $new = $source->new($source);
+            # we use extra here as we want to leave the class_mappings as they are
+            # but overwrite the source_registrations entry with the new source
+            $self->register_extra_source($moniker => $new);
+        }
+
+        if ($from->storage) {
+            $self->storage($from->storage);
+            $self->storage->set_schema($self);
+        }
+    }
 }
 
 =head2 connection
@@ -457,10 +526,16 @@ sub make_schema_at {
         @{$target . '::ISA'} = qw/DBIx::Class::Schema::Loader/;
     }
 
-    eval { $target->_loader_invoked(0) };
+    $target->_loader_invoked(0);
 
     $target->loader_options($opts);
-    $target->connection(@$connect_info);
+
+    my $temp_schema = $target->connect(@$connect_info);
+
+    $target->storage($temp_schema->storage);
+    $target->storage->set_schema($target);
+
+    return $target;
 }
 
 =head2 rescan

diff --git a/t/24loader_subclass.t b/t/24loader_subclass.t
index 5d54a2f..2d3b595 100644 (file)
--- a/t/24loader_subclass.t
+++ b/t/24loader_subclass.t
@@ -14,6 +14,7 @@ my %invocations = (
     loader_class => sub {
         package DBICTest::Schema::1;
         use base qw/ DBIx::Class::Schema::Loader /;
+        __PACKAGE__->_loader_invoked(0);
         __PACKAGE__->naming('current');
         __PACKAGE__->loader_class(shift);
         __PACKAGE__->connect($make_dbictest_db::dsn);
@@ -21,6 +22,7 @@ my %invocations = (
     connect_info => sub {
         package DBICTeset::Schema::2;
         use base qw/ DBIx::Class::Schema::Loader /;
+        __PACKAGE__->_loader_invoked(0);
         __PACKAGE__->naming('current');
         __PACKAGE__->connect($make_dbictest_db::dsn, { loader_class => shift });
     },

diff --git a/t/backcompat/0.04006/23dumpmore.t b/t/backcompat/0.04006/23dumpmore.t
index ba39b6d..8a48d0d 100644 (file)
--- a/t/backcompat/0.04006/23dumpmore.t
+++ b/t/backcompat/0.04006/23dumpmore.t
@@ -4,6 +4,7 @@ use lib qw(t/backcompat/0.04006/lib);
 use File::Path;
 use make_dbictest_db;
 use dbixcsl_test_dir qw/$tdir/;
+use Class::Unload ();
 
 require DBIx::Class::Schema::Loader;
 
@@ -30,8 +31,8 @@ sub do_dump_test {
         $schema_class->connect($make_dbictest_db::dsn);
     };
     my $err = $@;
-    $schema_class->storage->disconnect if !$err && $schema_class->storage;
-    undef *{$schema_class};
+
+    Class::Unload->unload($schema_class);
 
     is($err, $tdata{error});
 

diff --git a/t/lib/dbixcsl_dumper_tests.pm b/t/lib/dbixcsl_dumper_tests.pm
index 9e71ff8..d37900c 100644 (file)
--- a/t/lib/dbixcsl_dumper_tests.pm
+++ b/t/lib/dbixcsl_dumper_tests.pm
@@ -8,6 +8,7 @@ use IO::Handle;
 use List::MoreUtils 'any';
 use DBIx::Class::Schema::Loader::Utils 'dumper_squashed';
 use DBIx::Class::Schema::Loader ();
+use Class::Unload ();
 use namespace::clean;
 
 use dbixcsl_test_dir '$tdir';
@@ -74,8 +75,7 @@ sub _dump_directly {
     };
     my $err = $@;
 
-    $schema_class->storage->disconnect if !$err && $schema_class->storage;
-    undef *{$schema_class};
+    Class::Unload->unload($schema_class);
 
     _check_error($err, $tdata{error});