John Siracusa wrote on 10/26/07 10:45 PM:
> Here are my thoughts for revisions:
> 
> * I'm not sure ping()ing belongs here at all, at least as an auto-called
> method.  But if we do add it, we should use DBI's ping() method rather than
> trying to roll our own.  (As an example, "select 1" doesn't work in every
> database, and sometimes even when it does it's not the best choice.)
> 
> * Fewer methods relating to caching.  Initially, it should just be a ping()
> object method (maybe) and new_or_cached() and clear_db_cache() class
> methods.
> 
> * The cache key should be type and domain and that's it.
> 
> * Modifying a registry entry should invalidate the cached instance of the db
> for that type/domain.
> 
> What I'm trying to do here is start small.  If we eventually want to get
> into TTL and stuff like that, I'm inclined to go to a mix-in class that
> delegates all caching to a "real" caching module like Cache::Cache.  But for
> a first try at this, I'd like to avoid painting ourselves into a corner.
> 
> So, keep it simple :)  I can do the revisions if you want, or you can have a
> shot at it.  Let me know.
> 

Thanks for these comments and to Perrin for the suggestion to use DBI's
connect_cached(). Another pass is attached. This one is much simpler, and
leverages DBI's caching feature instead of inventing a new one. It passes the
same test the other patch did. I'm not sure if there are nuances to
connect_cached() that would make it behave differently. In particular, I don't
see a way to clear the DBI cache (could be there but I didn't notice it in the
docs).


-- 
Peter Karman  .  http://peknet.com/  .  [EMAIL PROTECTED]
Index: t/dbh_cache.t
===================================================================
--- t/dbh_cache.t       (revision 0)
+++ t/dbh_cache.t       (revision 0)
@@ -0,0 +1,40 @@
+#!/usr/bin/perl -w
+
+use strict;
+
+BEGIN {
+    require Test::More;
+    eval { require DBD::SQLite };
+
+    if ( $@ || $DBD::SQLite::VERSION < 1.08 || $ENV{'RDBO_NO_SQLITE'} ) {
+        Test::More->import(
+            skip_all => $ENV{'RDBO_NO_SQLITE'}
+            ? 'SQLite tests disabled'
+            : 'Missing DBD::SQLite 1.08+'
+        );
+    }
+    elsif ( $DBD::SQLite::VERSION == 1.13 )
+    {
+        Test::More->import( skip_all => 'DBD::SQLite 1.13 is broken' );
+    }
+    else {
+        Test::More->import( tests => 5 );
+    }
+}
+
+BEGIN {
+    require 't/test-lib.pl';
+    use_ok('Rose::DB');
+}
+
+Rose::DB->default_domain('test');
+Rose::DB->default_type('sqlite_admin');
+
+ok( my $db = Rose::DB->new_or_cached(), "new_or_cached db" );
+
+ok( ref $db && $db->isa('Rose::DB'), 'new()' );
+
+ok( my $db2 = Rose::DB->new_or_cached(), "new_or_cached db2" );
+
+is( $db->dbh, $db2->dbh, "same DBI handle used" );
+
Index: lib/Rose/DB.pm
===================================================================
--- lib/Rose/DB.pm      (revision 1470)
+++ lib/Rose/DB.pm      (working copy)
@@ -366,6 +366,14 @@
   return $self;
 }
 
+sub new_or_cached 
+{
+  my $self = shift;
+  my $db = $self->new(@_);
+  $db->{'cached_dbh'} = 1;
+  return $db;
+}
+
 sub class 
 {
   my($self) = shift;
@@ -820,13 +828,20 @@
   my $options = $self->connect_options;
 
   my $dsn = $self->dsn;
+  
+  my $method = $self->{'cached_dbh'} ? 'connect_cached' : 'connect';
 
-  $Debug && warn "DBI->connect('$dsn', '", $self->username, "', ...)\n";
+  $Debug && warn "DBI->$method('$dsn', '", $self->username, "', ...)\n";
 
   $self->{'error'} = undef;
   $self->{'database_version'} = undef;
 
-  my $dbh = DBI->connect($dsn, $self->username, $self->password, $options);
+  my $dbh = DBI->$method($dsn, $self->username, $self->password, 
+                         { %$options, 
+                           _rdb_type => $self->type, 
+                           _rdb_domain => $self->domain 
+                         }
+                        );
 
   unless($dbh)
   {
@@ -2759,6 +2774,11 @@
 
 You can change this mapping with the L<driver_class|/driver_class> class 
method.
 
+=item <new_or_cached PARAMS>
+
+Acts just like new() except that the DBI connect_cached() method will be used 
instead
+of connect().
+
 =back
 
 =head1 OBJECT METHODS
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
Rose-db-object mailing list
Rose-db-object@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/rose-db-object

Reply via email to