Deprecate the insane forest of SQLT::Parser::DBIC arguments
Peter Rabbitson [Sat, 16 Feb 2013 15:12:50 +0000 (16:12 +0100)]
It seems like things have been just piled up with no plan nor reason. Bring in
the broom - converge on an argument name that actuall makes sense - dbic_schema

Changes
lib/SQL/Translator/Parser/DBIx/Class.pm
t/99dbic_sqlt_parser.t

diff --git a/Changes b/Changes
index 6eacbb1..be07d01 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,10 @@
 Revision history for DBIx::Class
 
+    * New Features / Changes
+        - A bunch of nonsensically named arguments to the SQL::Translator
+          parser have been marked as deprecated (while still fully
+          supported)
+
     * Fixes
         - Fix duplicated selected columns when calling 'count' when a same
           aggregate function is used more than once in a 'having' clause
index dc13790..1817c1c 100644 (file)
@@ -36,11 +36,22 @@ use base qw(Exporter);
 sub parse {
     my ($tr, $data)   = @_;
     my $args          = $tr->parser_args;
-    my $dbicschema    = $args->{'DBIx::Class::Schema'} ||  $args->{"DBIx::Schema"} ||$data;
-    $dbicschema     ||= $args->{'package'};
-    my $limit_sources = $args->{'sources'};
+
+    my $dbicschema = $data || $args->{dbic_schema};
+
+    for (qw(DBIx::Class::Schema DBIx::Schema package)) {
+      if (defined (my $s = delete $args->{$_} )) {
+        carp_unique("Supplying a schema via  ... parser_args => { '$_' => \$schema } is deprecated. Please use parser_args => { dbic_schema => \$schema } instead");
+
+        # move it from the deprecated to the proper $args slot
+        unless ($dbicschema) {
+          $args->{dbic_schema} = $dbicschema = $s;
+        }
+      }
+    }
 
     DBIx::Class::Exception->throw('No DBIx::Class::Schema') unless ($dbicschema);
+
     if (!ref $dbicschema) {
       eval "require $dbicschema"
         or DBIx::Class::Exception->throw("Can't load $dbicschema: $@");
@@ -53,7 +64,7 @@ sub parse {
       unless ($schema->name);
 
     my @monikers = sort $dbicschema->sources;
-    if ($limit_sources) {
+    if (my $limit_sources = $args->{'sources'}) {
         my $ref = ref $limit_sources || '';
         $dbicschema->throw_exception ("'sources' parameter must be an array or hash ref")
           unless( $ref eq 'ARRAY' || ref eq 'HASH' );
@@ -422,7 +433,7 @@ from a DBIx::Class::Schema instance
  my $trans  = SQL::Translator->new (
       parser      => 'SQL::Translator::Parser::DBIx::Class',
       parser_args => {
-          package => $schema,
+          dbic_schema => $schema,
           add_fk_index => 0,
           sources => [qw/
             Artist
@@ -452,6 +463,27 @@ L<DBIx::Class::Schema/create_ddl_dir>.
 
 =head1 PARSER OPTIONS
 
+=head2 dbic_schema
+
+The DBIx::Class schema (either an instance or a class name) to be parsed.
+This argument is in fact optional - instead one can supply it later at
+translation time as an argument to L<SQL::Translator/translate>. In
+other words both of the following invocations are valid and will produce
+conceptually identical output:
+
+  my $yaml = SQL::Translator->new(
+    parser => 'SQL::Translator::Parser::DBIx::Class',
+    parser_args => {
+      dbic_schema => $schema,
+    },
+    producer => 'SQL::Translator::Producer::YAML',
+  )->translate;
+
+  my $yaml = SQL::Translator->new(
+    parser => 'SQL::Translator::Parser::DBIx::Class',
+    producer => 'SQL::Translator::Producer::YAML',
+  )->translate(data => $schema);
+
 =head2 add_fk_index
 
 Create an index for each foreign key.
index b98e7f2..ef08a53 100644 (file)
@@ -2,6 +2,7 @@ use strict;
 use warnings;
 
 use Test::More;
+use Test::Warn;
 use Test::Exception;
 use Scalar::Util ();
 
@@ -21,11 +22,22 @@ BEGIN {
 
   my @schemas = (
     create_schema ({ schema => $s }),
-    create_schema ({ args => { parser_args => { 'DBIx::Class::Schema' => $s } } }),
-    create_schema ({ args => { parser_args => { 'DBIx::Schema' => $s } } }),
-    create_schema ({ args => { parser_args => { package => $s } } }),
+    create_schema ({ args => { parser_args => { dbic_schema => $s } } }),
   );
 
+  for my $parser_args_key (qw(
+    DBIx::Class::Schema
+    DBIx::Schema
+    package
+  )) {
+    warnings_exist {
+      push @schemas, create_schema({
+        args => { parser_args => { $parser_args_key => $s } }
+      });
+    } qr/\Qparser_args => {\E.+?is deprecated/,
+    "deprecated crazy parser_arg '$parser_args_key' warned";
+  }
+
   Scalar::Util::weaken ($s);
 
   ok (!$s, 'Schema not leaked');
@@ -211,7 +223,6 @@ done_testing;
 sub create_schema {
   my $args = shift;
 
-  my $schema = $args->{schema};
   my $additional_sqltargs = $args->{args} || {};
 
   my $sqltargs = {
@@ -224,7 +235,9 @@ sub create_schema {
   my $sqlt = SQL::Translator->new( $sqltargs );
 
   $sqlt->parser('SQL::Translator::Parser::DBIx::Class');
-  return $sqlt->translate({ data => $schema }) || die $sqlt->error;
+  return $sqlt->translate(
+    $args->{schema} ? ( data => $args->{schema} ) : ()
+  ) || die $sqlt->error;
 }
 
 sub get_table {