From: Dylan William Hardison <dylan@hardison.net>
Date: Sat, 1 Feb 2020 15:11:19 +0000 (+0100)
Subject: [Bug 1592129] remove subclass loading and driver delegation from Schema->new.
X-Git-Tag: bugzilla-5.2~61
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4d4a65b3bf867b768bd1e22cbd12476a6cbd1f7a;p=thirdparty%2Fbugzilla.git

[Bug 1592129] remove subclass loading and driver delegation from Schema->new.

Bugzilla::DB::Schema->new() was both a normal constructor and also a
class-loading factory method. It is simpler to just do the class loading at the
call-site (in Bugzilla::DB::_bz_schema).

It's not very likely extensions relied on this behavior so this should be a good
change.
---

diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm
index 0fa9bf5038..7262cf888c 100644
--- a/Bugzilla/DB.pm
+++ b/Bugzilla/DB.pm
@@ -26,6 +26,7 @@ use Bugzilla::Error;
 use Bugzilla::DB::Schema;
 use Bugzilla::Version;
 
+use Scalar::Util qw(blessed);
 use List::Util qw(max);
 use Storable qw(dclone);
 
@@ -1155,12 +1156,21 @@ sub bz_set_next_serial_value {
 # Schema Information Methods
 #####################################################################
 
+sub _bz_schema_class {
+  my ($self) = @_;
+  my $class = blessed($self) // $self;
+  my @class_parts = split(/::/, $class);
+  splice(@class_parts, -1, 0, 'Schema');
+
+  return join('::', @class_parts);
+}
+
 sub _bz_schema {
   my ($self) = @_;
   return $self->{private_bz_schema} if exists $self->{private_bz_schema};
-  my @module_parts = split('::', ref $self);
-  my $module_name  = pop @module_parts;
-  $self->{private_bz_schema} = Bugzilla::DB::Schema->new($module_name);
+  my $schema_class = $self->_bz_schema_class;
+  eval "require $schema_class";
+  $self->{private_bz_schema} = $schema_class->new();
   return $self->{private_bz_schema};
 }
 
diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm
index 2ab8a983a3..6e15ef1db7 100644
--- a/Bugzilla/DB/Schema.pm
+++ b/Bugzilla/DB/Schema.pm
@@ -1758,13 +1758,8 @@ sub new {
 =item C<new>
 
  Description: Public constructor method used to instantiate objects of this
-              class. However, it also can be used as a factory method to
-              instantiate database-specific subclasses when an optional
-              driver argument is supplied.
- Parameters:  $driver (optional) - Used to specify the type of database.
-              This routine C<die>s if no subclass is found for the specified
-              driver.
-              $schema (optional) - A reference to a hash. Callers external
+              class.
+ Parameters:  $schema (optional) - A reference to a hash. Callers external
                   to this package should never use this parameter.
  Returns:     new instance of the Schema class or a database-specific subclass
 
@@ -1772,15 +1767,7 @@ sub new {
 
   my $this   = shift;
   my $class  = ref($this) || $this;
-  my $driver = shift;
-
-  if ($driver) {
-    (my $subclass = $driver) =~ s/^(\S)/\U$1/;
-    $class .= '::' . $subclass;
-    eval "require $class;";
-    die "The $class class could not be found ($subclass " . "not supported?): $@"
-      if ($@);
-  }
+
   die "$class is an abstract base class. Instantiate a subclass instead."
     if ($class eq __PACKAGE__);
 
@@ -2983,7 +2970,7 @@ sub deserialize_abstract {
     }
   }
 
-  return $class->new(undef, $thawed_hash);
+  return $class->new($thawed_hash);
 }
 
 #####################################################################
diff --git a/t/013dbschema.t b/t/013dbschema.t
index 062a22992e..c2d827e4d3 100644
--- a/t/013dbschema.t
+++ b/t/013dbschema.t
@@ -19,6 +19,7 @@ use warnings;
 use lib qw(. t lib);
 use Bugzilla;
 use Bugzilla::DB::Schema;
+use Bugzilla::DB::Schema::Mysql;
 
 
 # SQL reserved words
@@ -57,7 +58,7 @@ our $schema;
 our @tables;
 
 BEGIN {
-  $schema = Bugzilla::DB::Schema->new("Mysql");
+  $schema = Bugzilla::DB::Schema::Mysql->new;
   @tables = $schema->get_table_list();
 }