From: John Napiorkowski <jjn1056@yahoo.com>
Date: Wed, 7 May 2008 22:40:30 +0000 (+0000)
Subject: changed the way args are passed to a storage, should make it easier to use existing... 
X-Git-Tag: v0.08240~402^2~53
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=dbsrgits%2FDBIx-Class.git;a=commitdiff_plain;h=106d5f3b7e03a68fb2e125772e3f06a34115f22d

changed the way args are passed to a storage, should make it easier to use existing code using this, added the master as a fallback to the the replicants, lots of small documentation updates and test improvements.  all tests passing
---

diff --git a/lib/DBIx/Class/Schema.pm b/lib/DBIx/Class/Schema.pm
index 3ce1c1c..c3ce0e6 100644
--- a/lib/DBIx/Class/Schema.pm
+++ b/lib/DBIx/Class/Schema.pm
@@ -14,7 +14,6 @@ use base qw/DBIx::Class/;
 __PACKAGE__->mk_classdata('class_mappings' => {});
 __PACKAGE__->mk_classdata('source_registrations' => {});
 __PACKAGE__->mk_classdata('storage_type' => '::DBI');
-__PACKAGE__->mk_classdata('storage_type_args' => {});
 __PACKAGE__->mk_classdata('storage');
 __PACKAGE__->mk_classdata('exception_action');
 __PACKAGE__->mk_classdata('stacktrace' => $ENV{DBIC_TRACE} || 0);
@@ -638,9 +637,9 @@ sub setup_connection_class {
 
 =over 4
 
-=item Arguments: $storage_type
+=item Arguments: $storage_type|[$storage_type, \%args]
 
-=item Return Value: $storage_type
+=item Return Value: $storage_type|[$storage_type, \%args]
 
 =back
 
@@ -654,6 +653,11 @@ in cases where the appropriate subclass is not autodetected, such as when
 dealing with MSSQL via L<DBD::Sybase>, in which case you'd set it to
 C<::DBI::Sybase::MSSQL>.
 
+If your storage type requires instantiation arguments, those are defined as a 
+second argument in the form of a hashref and the entire value needs to be
+wrapped into an arrayref.  See L<DBIx::Class::Storage::DBI::Replicated> for an
+example of this.
+
 =head2 connection
 
 =over 4
@@ -676,14 +680,17 @@ or L<DBIx::Class::Storage> in general.
 sub connection {
   my ($self, @info) = @_;
   return $self if !@info && $self->storage;
-  my $storage_class = $self->storage_type;
+  
+  my ($storage_class, $args) = ref $self->storage_type ? 
+    (@{$self->storage_type},{}) : ($self->storage_type, {});
+    
   $storage_class = 'DBIx::Class::Storage'.$storage_class
     if $storage_class =~ m/^::/;
   eval "require ${storage_class};";
   $self->throw_exception(
     "No arguments to load_classes and couldn't load ${storage_class} ($@)"
   ) if $@;
-  my $storage = $storage_class->new($self, $self->storage_type_args);
+  my $storage = $storage_class->new($self=>$args);
   $storage->connect_info(\@info);
   $self->storage($storage);
   return $self;
diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index b3ef3d6..212be1e 100644
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -1708,6 +1708,31 @@ sub build_datetime_parser {
     }
 }
 
+=head2 is_replicating
+
+A boolean that reports if a particular L<DBIx::Class::Storage::DBI> is set to
+replicate from a master database.  Default is undef, which is the result
+returned by databases that don't support replication.
+
+=cut
+
+sub is_replicating {
+    return;
+    
+}
+
+=head2 lag_behind_master
+
+Returns a number that represents a certain amount of lag behind a master db
+when a given storage is replicating.  The number is database dependent, but
+starts at zero and increases with the amount of lag. Default in undef
+
+=cut
+
+sub lag_behind_master {
+    return;
+}
+
 sub DESTROY {
   my $self = shift;
   return if !$self->_dbh;
diff --git a/lib/DBIx/Class/Storage/DBI/Replicated.pm b/lib/DBIx/Class/Storage/DBI/Replicated.pm
index 6f81945..6408cb4 100644
--- a/lib/DBIx/Class/Storage/DBI/Replicated.pm
+++ b/lib/DBIx/Class/Storage/DBI/Replicated.pm
@@ -19,7 +19,7 @@ storage type, add some replicated (readonly) databases, and perform reporting
 tasks.
 
     ## Change storage_type in your schema class
-    $schema->storage_type( '::DBI::Replicated' );
+    $schema->storage_type( ['::DBI::Replicated', {balancer=>'::Random'}] );
     
     ## Add some slaves.  Basically this is an array of arrayrefs, where each
     ## arrayref is database connect information
@@ -67,15 +67,12 @@ to: L<DBIx::Class::Storage::DBI::Replicated::Pool>.
 has 'pool_type' => (
     is=>'ro',
     isa=>'ClassName',
-    required=>1,
-    lazy=>1,
-    default=>'DBIx::Class::Storage::DBI::Replicated::Pool',
+    lazy_build=>1,
     handles=>{
     	'create_pool' => 'new',
     },
 );
 
-
 =head2 balancer_type
 
 The replication pool requires a balance class to provider the methods for
@@ -86,15 +83,12 @@ choose how to spread the query load across each replicant in the pool.
 has 'balancer_type' => (
     is=>'ro',
     isa=>'ClassName',
-    required=>1,
-    lazy=>1,
-    default=>'DBIx::Class::Storage::DBI::Replicated::Balancer',
+    lazy_build=>1,
     handles=>{
     	'create_balancer' => 'new',
     },
 );
 
-
 =head2 pool
 
 Is a <DBIx::Class::Storage::DBI::Replicated::Pool> or derived class.  This is a
@@ -115,7 +109,6 @@ has 'pool' => (
     /],
 );
 
-
 =head2 balancer
 
 Is a <DBIx::Class::Storage::DBI::Replicated::Balancer> or derived class.  This 
@@ -129,7 +122,6 @@ has 'balancer' => (
     lazy_build=>1,
 );
 
-
 =head2 master
 
 The master defines the canonical state for a pool of connected databases.  All
@@ -146,7 +138,6 @@ has 'master' => (
     lazy_build=>1,
 );
 
-
 =head1 ATTRIBUTES IMPLEMENTING THE DBIx::Storage::DBI INTERFACE
 
 The following methods are delegated all the methods required for the 
@@ -169,7 +160,6 @@ has 'read_handler' => (
     /],    
 );
 
-
 =head2 write_handler
 
 Defines an object that implements the write side of L<BIx::Class::Storage::DBI>.
@@ -207,7 +197,6 @@ has 'write_handler' => (
     /],
 );
 
-
 =head1 METHODS
 
 This class defines the following methods.
@@ -227,7 +216,14 @@ sub new {
     my $schema = shift @_;
     my $storage_type_args = shift @_;
     my $obj = $class->SUPER::new($schema, $storage_type_args, @_);
-  
+    
+    ## Hate to do it this way, but can't seem to get advice on the attribute working right
+    ## maybe we can do a type and coercion for it. 
+    if( $storage_type_args->{balancer_type} && $storage_type_args->{balancer_type}=~m/^::/) {
+    	$storage_type_args->{balancer_type} = 'DBIx::Class::Storage::DBI::Replicated::Balancer'.$storage_type_args->{balancer_type};
+    	eval "require $storage_type_args->{balancer_type}";
+    }
+    
     return $class->meta->new_object(
         __INSTANCE__ => $obj,
         %$storage_type_args,
@@ -245,6 +241,16 @@ sub _build_master {
 	DBIx::Class::Storage::DBI->new;
 }
 
+=head2 _build_pool_type
+
+Lazy builder for the L</pool_type> attribute.
+
+=cut
+
+sub _build_pool_type {
+    return 'DBIx::Class::Storage::DBI::Replicated::Pool';
+}
+
 =head2 _build_pool
 
 Lazy builder for the L</pool> attribute.
@@ -255,6 +261,16 @@ sub _build_pool {
     shift->create_pool;
 }
 
+=head2 _build_balancer_type
+
+Lazy builder for the L</balancer_type> attribute.
+
+=cut
+
+sub _build_balancer_type {
+    return 'DBIx::Class::Storage::DBI::Replicated::Balancer';
+}
+
 =head2 _build_balancer
 
 Lazy builder for the L</balancer> attribute.  This takes a Pool object so that
@@ -264,7 +280,9 @@ the balancer knows which pool it's balancing.
 
 sub _build_balancer {
     my $self = shift @_;
-    $self->create_balancer(pool=>$self->pool);
+    $self->create_balancer(
+        pool=>$self->pool, 
+        master=>$self->master);
 }
 
 =head2 _build_write_handler
diff --git a/lib/DBIx/Class/Storage/DBI/Replicated/Balancer.pm b/lib/DBIx/Class/Storage/DBI/Replicated/Balancer.pm
index bf47c07..7ca7cff 100644
--- a/lib/DBIx/Class/Storage/DBI/Replicated/Balancer.pm
+++ b/lib/DBIx/Class/Storage/DBI/Replicated/Balancer.pm
@@ -21,6 +21,20 @@ method by which query load can be spread out across each replicant in the pool.
 
 This class defines the following attributes.
 
+=head2 master
+
+The L<DBIx::Class::Storage::DBI> object that is the master database all the
+replicants are trying to follow.  The balancer needs to know it since it's the
+ultimate fallback.
+
+=cut
+
+has 'master' => (
+    is=>'ro',
+    isa=>'DBIx::Class::Storage::DBI',
+    required=>1,
+);
+
 =head2 pool
 
 The L<DBIx::Class::Storage::DBI::Replicated::Pool> object that we are trying to
@@ -70,7 +84,7 @@ Lazy builder for the L</current_replicant_storage> attribute.
 
 sub _build_current_replicant {
     my $self = shift @_;
-    $self->next_storage($self->pool);
+    $self->next_storage;
 }
 
 =head2 next_storage
@@ -80,15 +94,18 @@ default behavior is to grap the first replicant it finds but you can write
 your own subclasses of L<DBIx::Class::Storage::DBI::Replicated::Balancer> to 
 support other balance systems.
 
+This returns from the pool of active replicants.  If there are no active
+replicants, then you should have it return the master as an ultimate fallback.
+
 =cut
 
 sub next_storage {
 	my $self = shift @_;
-	return ($self->pool->active_replicants)[0]
-	  if $self->pool->active_replicants;
+	my $next = ($self->pool->active_replicants)[0];
+	return $next ? $next:$self->master;
 }
 
-=head2 after: select
+=head2 before: select
 
 Advice on the select attribute.  Each time we use a replicant
 we need to change it via the storage pool algorithm.  That way we are spreading
@@ -96,13 +113,13 @@ the load evenly (hopefully) across existing capacity.
 
 =cut
 
-after 'select' => sub {
+before 'select' => sub {
     my $self = shift @_;
     my $next_replicant = $self->next_storage;
     $self->current_replicant($next_replicant);
 };
 
-=head2 after: select_single
+=head2 before: select_single
 
 Advice on the select_single attribute.  Each time we use a replicant
 we need to change it via the storage pool algorithm.  That way we are spreading
@@ -110,13 +127,13 @@ the load evenly (hopefully) across existing capacity.
 
 =cut
 
-after 'select_single' => sub {
+before 'select_single' => sub {
     my $self = shift @_;
     my $next_replicant = $self->next_storage;
     $self->current_replicant($next_replicant);
 };
 
-=head2 after: columns_info_for
+=head2 before: columns_info_for
 
 Advice on the current_replicant_storage attribute.  Each time we use a replicant
 we need to change it via the storage pool algorithm.  That way we are spreading
@@ -124,7 +141,7 @@ the load evenly (hopefully) across existing capacity.
 
 =cut
 
-after 'columns_info_for' => sub {
+before 'columns_info_for' => sub {
     my $self = shift @_;
     my $next_replicant = $self->next_storage;
     $self->current_replicant($next_replicant);
diff --git a/lib/DBIx/Class/Storage/DBI/Replicated/Balancer/Random.pm b/lib/DBIx/Class/Storage/DBI/Replicated/Balancer/Random.pm
index 66f0827..6c66ea8 100644
--- a/lib/DBIx/Class/Storage/DBI/Replicated/Balancer/Random.pm
+++ b/lib/DBIx/Class/Storage/DBI/Replicated/Balancer/Random.pm
@@ -40,12 +40,11 @@ be requested several times in a row.
 =cut
 
 sub next_storage {
-	my $self = shift @_;
-	return (shuffle($self->pool->active_replicants))[0]
-	  if $self->pool->active_replicants;
+    my $self = shift @_;
+    my $next = (shuffle($self->pool->active_replicants))[0];
+    return $next ? $next : $self->master;
 }
 
-
 =head1 AUTHOR
 
 John Napiorkowski <john.napiorkowski@takkle.com>
diff --git a/lib/DBIx/Class/Storage/DBI/mysql.pm b/lib/DBIx/Class/Storage/DBI/mysql.pm
index ec36176..002f50b 100644
--- a/lib/DBIx/Class/Storage/DBI/mysql.pm
+++ b/lib/DBIx/Class/Storage/DBI/mysql.pm
@@ -34,6 +34,14 @@ sub _svp_rollback {
     $self->dbh->do("ROLLBACK TO SAVEPOINT $name")
 }
 
+sub is_replicating {
+    my $self = shift @_;
+}
+
+sub lag_behind_master {
+    my $self = shift @_;
+}
+
 1;
 
 =head1 NAME
diff --git a/t/93storage_replication.t b/t/93storage_replication.t
index 589e499..2d1e58b 100644
--- a/t/93storage_replication.t
+++ b/t/93storage_replication.t
@@ -13,7 +13,6 @@ BEGIN {
 
 use_ok 'DBIx::Class::Storage::DBI::Replicated::Pool';
 use_ok 'DBIx::Class::Storage::DBI::Replicated::Balancer';
-use_ok 'DBIx::Class::Storage::DBI::Replicated::Balancer::Random';
 use_ok 'DBIx::Class::Storage::DBI::Replicated::Replicant';
 use_ok 'DBIx::Class::Storage::DBI::Replicated';
 
@@ -49,10 +48,11 @@ TESTSCHEMACLASSES: {
     sub init_schema {
         my $class = shift @_;
         my $schema = DBICTest->init_schema(
-            storage_type=>'::DBI::Replicated', 
-            storage_type_args=>{
-            	balancer_type=>'DBIx::Class::Storage::DBI::Replicated::Balancer::Random',
-            });
+            storage_type=>[
+            	'::DBI::Replicated' => {
+            		balancer_type=>'::Random',
+            	}],
+            );
 
         return $schema;
     }
@@ -320,8 +320,16 @@ ok $replicated->schema->resultset('Artist')->find(1)
     
 ok $replicated->schema->resultset('Artist')->find(2)
     => 'back to replicant 2.';
-    
 
+## set all the replicants to inactive, and make sure the balancer falls back to
+## the master.
+
+$replicated->schema->storage->replicants->{"t/var/DBIxClass_slave1.db"}->active(0);
+$replicated->schema->storage->replicants->{"t/var/DBIxClass_slave2.db"}->active(0);
+    
+ok $replicated->schema->resultset('Artist')->find(2)
+    => 'Fallback to master'; 
+       
 ## Delete the old database files
 $replicated->cleanup;
 
diff --git a/t/lib/DBICTest.pm b/t/lib/DBICTest.pm
index eb8fbc2..d33f336 100755
--- a/t/lib/DBICTest.pm
+++ b/t/lib/DBICTest.pm
@@ -84,9 +84,6 @@ sub init_schema {
     } else {
       $schema = DBICTest::Schema->compose_namespace('DBICTest');
     }
-    if( $args{storage_type_args}) {
-    	$schema->storage_type_args($args{storage_type_args});
-    } 
     if( $args{storage_type}) {
     	$schema->storage_type($args{storage_type});
     }    
@@ -115,9 +112,9 @@ of tables for testing.
 
 sub deploy_schema {
     my $self = shift;
-    my $schema = shift;
+    my $schema = shift; 
 
-    if ($ENV{"DBICTEST_SQLT_DEPLOY"}) {
+    if ($ENV{"DBICTEST_SQLT_DEPLOY"}) { 
         return $schema->deploy();
     } else {
         open IN, "t/lib/sqlite.sql";