From: Peter Rabbitson <ribasushi@cpan.org>
Date: Thu, 8 Apr 2010 10:36:05 +0000 (+0000)
Subject: Refactor the version handling
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=6d766626a066b757058d023830f93cf39bb9c302;p=dbsrgits%2FDBIx-Class-Historic.git

Refactor the version handling
Clean up normalization wrt non-numeric version parts (i.e. mysql)
---

diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index a823f22..655f506 100644
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -19,7 +19,7 @@ use Sub::Name ();
 __PACKAGE__->mk_group_accessors('simple' =>
   qw/_connect_info _dbi_connect_info _dbh _sql_maker _sql_maker_opts _conn_pid
      _conn_tid transaction_depth _dbh_autocommit _driver_determined savepoints
-     __server_info/
+     _server_info_hash/
 );
 
 # the values for these accessors are picked out (and deleted) from
@@ -908,6 +908,7 @@ sub _populate_dbh {
 
   my @info = @{$self->_dbi_connect_info || []};
   $self->_dbh(undef); # in case ->connected failed we might get sent here
+  $self->_server_info_hash (undef);
   $self->_dbh($self->_connect(@info));
 
   $self->_conn_pid($$);
@@ -920,8 +921,6 @@ sub _populate_dbh {
   $self->{transaction_depth} = $self->_dbh_autocommit ? 0 : 1;
 
   $self->_run_connection_actions unless $self->{_in_determine_driver};
-
-  $self->_populate_server_info;
 }
 
 sub _run_connection_actions {
@@ -934,35 +933,39 @@ sub _run_connection_actions {
   $self->_do_connection_actions(connect_call_ => $_) for @actions;
 }
 
-sub _populate_server_info {
+sub _server_info {
   my $self = shift;
-  my %info;
 
-  my $dbms_ver = eval {
-      local $@;
-      $self->_get_dbh->get_info(18)
-  };
+  unless ($self->_server_info_hash) {
 
-  if (defined $dbms_ver) {
-    $info{dbms_ver} = $dbms_ver;
+    my %info;
 
-    ($dbms_ver) = $dbms_ver =~ /^(\S+)/;
+    my $server_version = $self->_get_server_version;
 
-    my @verparts = split /\./, $dbms_ver;
-    $info{dbms_ver_normalized} = sprintf "%d.%03d%03d", @verparts;
-  }
-
-  $self->__server_info(\%info);
+    if (defined $server_version) {
+      $info{dbms_version} = $server_version;
 
-  return \%info;
-}
+      my ($numeric_version) = $server_version =~ /^([\d\.]+)/;
+      my @verparts = split (/\./, $numeric_version);
+      if (
+        @verparts
+          &&
+        @verparts <= 3
+          &&
+        ! grep { $_ > 999 } (@verparts)
+      ) {
+        $info{normalized_dbms_version} = sprintf "%d.%03d%03d", @verparts;
+      }
+    }
 
-sub _server_info {
-  my $self = shift;
+    $self->_server_info_hash(\%info);
+  }
 
-  $self->_get_dbh;
+  return $self->_server_info_hash
+}
 
-  return $self->__server_info(@_);
+sub _get_server_version {
+  eval { shift->_get_dbh->get_info(18) };
 }
 
 sub _determine_driver {
diff --git a/lib/DBIx/Class/Storage/DBI/InterBase.pm b/lib/DBIx/Class/Storage/DBI/InterBase.pm
index e6a9143..a0f934a 100644
--- a/lib/DBIx/Class/Storage/DBI/InterBase.pm
+++ b/lib/DBIx/Class/Storage/DBI/InterBase.pm
@@ -155,7 +155,7 @@ sub _set_sql_dialect {
   }
 }
 
-sub _populate_server_info {
+sub _get_server_version {
   my $self = shift;
 
   return $self->next::method(@_) if ref $self ne __PACKAGE__;
diff --git a/lib/DBIx/Class/Storage/DBI/MSSQL.pm b/lib/DBIx/Class/Storage/DBI/MSSQL.pm
index 5940868..d9c8ee0 100644
--- a/lib/DBIx/Class/Storage/DBI/MSSQL.pm
+++ b/lib/DBIx/Class/Storage/DBI/MSSQL.pm
@@ -206,8 +206,7 @@ sub sql_maker {
   unless ($self->_sql_maker) {
     unless ($self->{_sql_maker_opts}{limit_dialect}) {
 
-      my ($version) = $self->_server_info->{dbms_ver} =~ /^(\d+)/;
-      $version ||= 0;
+      my $version = $self->_server_info->{normalized_dbms_version} || 0;
 
       $self->{_sql_maker_opts} = {
         limit_dialect => ($version >= 9 ? 'RowNumberOver' : 'Top'),
diff --git a/lib/DBIx/Class/Storage/DBI/Pg.pm b/lib/DBIx/Class/Storage/DBI/Pg.pm
index 8f5ac8d..4428b1f 100644
--- a/lib/DBIx/Class/Storage/DBI/Pg.pm
+++ b/lib/DBIx/Class/Storage/DBI/Pg.pm
@@ -20,7 +20,7 @@ sub _supports_insert_returning {
   my $self = shift;
 
   return 1
-    if $self->_server_info->{dbms_ver_normalized} >= 8.002;
+    if $self->_server_info->{normalized_dbms_version} >= 8.002;
 
   return 0;
 }
diff --git a/lib/DBIx/Class/Storage/DBI/Replicated.pm b/lib/DBIx/Class/Storage/DBI/Replicated.pm
index 6d226d2..afd8d5c 100644
--- a/lib/DBIx/Class/Storage/DBI/Replicated.pm
+++ b/lib/DBIx/Class/Storage/DBI/Replicated.pm
@@ -367,9 +367,17 @@ has 'write_handler' => (
     _dbh_sth
     _dbh_execute
     _prefetch_insert_auto_nextvals
+  /,
+
+  # TODO these need to be spread out to ALL servers not just the master
+  qw/
+    _get_server_version
+    _server_info
+    _server_info_hash
   /],
 );
 
+
 has _master_connect_info_opts =>
   (is => 'rw', isa => HashRef, default => sub { {} });
 
diff --git a/lib/DBIx/Class/Storage/DBI/SQLite.pm b/lib/DBIx/Class/Storage/DBI/SQLite.pm
index 30e7b2b..186cb60 100644
--- a/lib/DBIx/Class/Storage/DBI/SQLite.pm
+++ b/lib/DBIx/Class/Storage/DBI/SQLite.pm
@@ -50,13 +50,13 @@ sub deployment_statements {
 
   $sqltargs ||= {};
 
-  my $sqlite_version = eval { $self->_server_info->{dbms_ver} };
-  $sqlite_version ||= '';
+  # it'd be cool to use the normalized perl-style version but this needs sqlt hacking as well
+  if (my $sqlite_version = $self->_server_info->{dbms_version}) {
+    # numify, SQLT does a numeric comparison
+    $sqlite_version =~ s/^(\d+) \. (\d+) (?: \. (\d+))? .*/${1}.${2}/x;
 
-  # numify, SQLT does a numeric comparison
-  $sqlite_version =~ s/^(\d+) \. (\d+) (?: \. (\d+))? .*/${1}.${2}/x;
-
-  $sqltargs->{producer_args}{sqlite_version} = $sqlite_version if $sqlite_version;
+    $sqltargs->{producer_args}{sqlite_version} = $sqlite_version if $sqlite_version;
+  }
 
   $self->next::method($schema, $type, $version, $dir, $sqltargs, @rest);
 }
diff --git a/lib/DBIx/Class/Storage/DBI/Sybase/Microsoft_SQL_Server.pm b/lib/DBIx/Class/Storage/DBI/Sybase/Microsoft_SQL_Server.pm
index 7983473..49cd9e6 100644
--- a/lib/DBIx/Class/Storage/DBI/Sybase/Microsoft_SQL_Server.pm
+++ b/lib/DBIx/Class/Storage/DBI/Sybase/Microsoft_SQL_Server.pm
@@ -55,23 +55,19 @@ sub _dbh_rollback {
   $dbh->do('ROLLBACK');
 }
 
-sub _populate_server_info {
+sub _get_server_version {
   my $self = shift;
 
-  my $info = $self->next::method(@_);
-
   my $product_version = $self->_get_dbh->selectrow_hashref('xp_msver ProductVersion');
 
   if ((my $version = $data->{Character_Value}) =~ /^(\d+)\./) {
-    $info->{dbms_ver} = $version;
-  } else {
-    $self->throw_exception(q{
-MSSQL Version Retrieval Failed, Your ProductVersion's Character_Value is missing
-or malformed!
+    return $version;
+  }
+  else {
+    $self->throw_exception(
+      "MSSQL Version Retrieval Failed, Your ProductVersion's Character_Value is missing or malformed!"
     });
   }
-
-  return $info;
 }
 
 1;
diff --git a/t/72pg.t b/t/72pg.t
index 4065b26..542b915 100644
--- a/t/72pg.t
+++ b/t/72pg.t
@@ -23,16 +23,20 @@ EOM
 our @test_classes; #< array that will be pushed into by test classes defined in this file
 DBICTest::Schema->load_classes( map {s/.+:://;$_} @test_classes ) if @test_classes;
 
-my $schema;
-
-require DBIx::Class::Storage::DBI::Pg;
+my $test_server_supports_insert_returning = do {
+  my $s = DBICTest::Schema->connect($dsn, $user, $pass);
+  $s->storage->_determine_driver;
+  $s->storage->_supports_insert_returning;
+};
 
-my $can_insert_returning =
-  DBIx::Class::Storage::DBI::Pg->can('can_insert_returning');
+my $schema;
 
-for my $use_insert_returning (0..1) {
+for my $use_insert_returning ($test_server_supports_insert_returning
+  ? (0,1)
+  : (0)
+) {
   no warnings qw/redefine once/;
-  local *DBIx::Class::Storage::DBI::Pg::can_insert_returning = sub {
+  local *DBIx::Class::Storage::DBI::Pg::_supports_insert_returning = sub {
     $use_insert_returning
   };
 
@@ -69,13 +73,6 @@ for my $use_insert_returning (0..1) {
   $schema = DBICTest::Schema->connect($dsn, $user, $pass);
   $schema->storage->ensure_connected;
 
-  if ($use_insert_returning && (not $can_insert_returning->($schema->storage)))
-  {
-    diag "Your version of PostgreSQL does not support INSERT ... RETURNING.";
-    diag "*** SKIPPING FURTHER TESTS";
-    last;
-  }
-
   drop_test_schema($schema);
   create_test_schema($schema);
 
@@ -281,7 +278,7 @@ for my $use_insert_returning (0..1) {
 
 ######## test non-serial auto-pk
 
-  if ($schema->storage->can_insert_returning) {
+  if ($schema->storage->_supports_insert_returning) {
     $schema->source('TimestampPrimaryKey')->name('dbic_t_schema.timestamp_primary_key_test');
     my $row = $schema->resultset('TimestampPrimaryKey')->create({});
     ok $row->id;