smarter inflect_plural in RelBuilder
Rafael Kitover [Sun, 7 Mar 2010 22:56:19 +0000 (17:56 -0500)]
lib/DBIx/Class/Schema/Loader/RelBuilder.pm
lib/DBIx/Class/Schema/Loader/RelBuilder/Compat/v0_040.pm
t/25backcompat_v4.t
t/backcompat/0.04006/lib/dbixcsl_common_tests.pm
t/lib/dbixcsl_common_tests.pm
t/lib/make_dbictest_db_with_unique.pm [moved from t/lib/make_dbictest_db2.pm with 97% similarity]

index 2fe5824..0a86d60 100644 (file)
@@ -98,7 +98,7 @@ sub new {
 
 # pluralize a relationship name
 sub _inflect_plural {
-    my ($self, $relname) = @_;
+    my ($self, $relname, $method) = @_;
 
     return '' if !defined $relname || $relname eq '';
 
@@ -111,7 +111,9 @@ sub _inflect_plural {
         return $inflected if $inflected;
     }
 
-    return Lingua::EN::Inflect::Number::to_PL($relname);
+    $method ||= '_to_PL';
+
+    return $self->$method($relname);
 }
 
 # Singularize a relationship name
@@ -129,7 +131,29 @@ sub _inflect_singular {
         return $inflected if $inflected;
     }
 
-    return Lingua::EN::Inflect::Number::to_S($relname);
+    return $self->_to_S($relname);
+}
+
+sub _to_PL {
+    my ($self, $name) = @_;
+
+    $name =~ s/_/ /g;
+    my $plural = Lingua::EN::Inflect::Number::to_PL($name);
+    $plural =~ s/ /_/g;
+
+    return $plural;
+}
+
+sub _old_to_PL {
+    my ($self, $name) = @_;
+
+    return Lingua::EN::Inflect::Number::to_PL($name);
+}
+
+sub _to_S {
+    my ($self, $name) = @_;
+
+    return Lingua::EN::Inflect::Number::to_S($name);
 }
 
 # accessor for options to be passed to each generated relationship
@@ -161,19 +185,17 @@ sub _array_eq {
 }
 
 sub _remote_attrs {
-       my ($self, $local_moniker, $local_cols) = @_;
+    my ($self, $local_moniker, $local_cols) = @_;
 
-       # get our base set of attrs from _relationship_attrs, if present
-       my $attrs = $self->_relationship_attrs('belongs_to') || {};
+    # get our base set of attrs from _relationship_attrs, if present
+    my $attrs = $self->_relationship_attrs('belongs_to') || {};
 
-       # If the referring column is nullable, make 'belongs_to' an
-       # outer join, unless explicitly set by relationship_attrs
-       my $nullable = grep { $self->{schema}->source($local_moniker)->column_info($_)->{is_nullable} }
-               @$local_cols;
-       $attrs->{join_type} = 'LEFT'
-           if $nullable && !defined $attrs->{join_type};
+    # If the referring column is nullable, make 'belongs_to' an
+    # outer join, unless explicitly set by relationship_attrs
+    my $nullable = grep { $self->{schema}->source($local_moniker)->column_info($_)->{is_nullable} } @$local_cols;
+    $attrs->{join_type} = 'LEFT' if $nullable && !defined $attrs->{join_type};
 
-       return $attrs;
+    return $attrs;
 }
 
 sub _remote_relname {
@@ -274,20 +296,26 @@ sub _relnames_and_method {
 
     # If more than one rel between this pair of tables, use the local
     # col names to distinguish
-    my $local_relname;
-    my $old_multirel_name; #< TODO: remove me
+    my ($local_relname, $old_local_relname, $local_relname_uninflected, $old_local_relname_uninflected);
     if ( $counters->{$remote_moniker} > 1) {
         my $colnames = lc(q{_} . join(q{_}, @$local_cols));
         $remote_relname .= $colnames if keys %$cond > 1;
 
         $local_relname = lc($local_table) . $colnames;
-        $local_relname =~ s/_id$//
-            #< TODO: remove me
-            and $old_multirel_name = $self->_inflect_plural( lc($local_table) . $colnames );
+        $local_relname =~ s/_id$//;
+
+        $local_relname_uninflected = $local_relname;
         $local_relname = $self->_inflect_plural( $local_relname );
 
+        $old_local_relname_uninflected = lc($local_table) . $colnames;
+        $old_local_relname = $self->_inflect_plural( lc($local_table) . $colnames, '_old_to_PL' );
+
     } else {
+        $local_relname_uninflected = lc $local_table;
         $local_relname = $self->_inflect_plural(lc $local_table);
+
+        $old_local_relname_uninflected = lc $local_table;
+        $old_local_relname = $self->_inflect_plural(lc $local_table, '_old_to_PL');
     }
 
     my $remote_method = 'has_many';
@@ -297,14 +325,11 @@ sub _relnames_and_method {
     if (_array_eq([ $local_source->primary_columns ], $local_cols) ||
             grep { _array_eq($_->[1], $local_cols) } @$uniqs) {
         $remote_method = 'might_have';
-        $local_relname = $self->_inflect_singular($local_relname);
-        #< TODO: remove me
-        $old_multirel_name = $self->_inflect_singular($old_multirel_name);
+        $local_relname = $self->_inflect_singular($local_relname_uninflected);
+        $old_local_relname = $self->_inflect_singular($old_local_relname_uninflected);
     }
 
-    # TODO: remove me after 0.05003 release
-    $old_multirel_name
-        and warn __PACKAGE__." $VERSION: warning, stripping trailing _id from ${remote_class} relation '$old_multirel_name', renaming to '$local_relname'.  This behavior is new as of 0.05003.\n";
+    warn __PACKAGE__." $VERSION: renaming ${remote_class} relation '$old_local_relname' to '$local_relname'.  This behavior is new as of 0.05003.\n" if $old_local_relname && $local_relname ne $old_local_relname;
 
     return ( $local_relname, $remote_relname, $remote_method );
 }
index d329e63..adb5f30 100644 (file)
@@ -6,6 +6,12 @@ use Class::C3;
 
 use base 'DBIx::Class::Schema::Loader::RelBuilder';
 
+sub _to_PL {
+    my ($self, $name) = @_;
+
+    return Lingua::EN::Inflect::Number::to_PL($name);
+}
+
 sub _relnames_and_method {
     my ( $self, $local_moniker, $rel, $cond, $uniqs, $counters ) = @_;
 
index 56ca9ce..bb9d54d 100644 (file)
@@ -10,7 +10,7 @@ use File::Slurp 'slurp';
 use DBIx::Class::Schema::Loader ();
 use Lingua::EN::Inflect::Number ();
 use lib qw(t/lib);
-use make_dbictest_db2;
+use make_dbictest_db_with_unique;
 
 my $DUMP_DIR = './t/_common_dump';
 rmtree $DUMP_DIR;
@@ -792,7 +792,7 @@ sub run_loader {
     };
     undef $@;
 
-    my @connect_info = $make_dbictest_db2::dsn;
+    my @connect_info = $make_dbictest_db_with_unique::dsn;
     my @loader_warnings;
     local $SIG{__WARN__} = sub { push(@loader_warnings, $_[0]); };
     eval qq{
index 66b4d5a..827d5a8 100644 (file)
@@ -43,7 +43,7 @@ sub _monikerize {
 sub run_tests {
     my $self = shift;
 
-    plan tests => 89;
+    plan tests => 91;
 
     $self->create();
 
@@ -255,7 +255,7 @@ sub run_tests {
     is( $obj2->id, 2 );
 
     SKIP: {
-        skip $self->{skip_rels}, 50 if $self->{skip_rels};
+        skip $self->{skip_rels}, 52 if $self->{skip_rels};
 
         my $moniker3 = $monikers->{loader_test3};
         my $class3   = $classes->{loader_test3};
@@ -346,6 +346,14 @@ sub run_tests {
         my $rs_rel4 = $obj3->search_related('loader_test4zes');
         isa_ok( $rs_rel4->first, $class4);
 
+        # test that _id is not stripped and prepositions in rel names are
+        # ignored
+        ok ($class4->has_relationship('loader_test5_to_ids'),
+            "rel with preposition 'to' and _id pluralized backward-compatibly");
+
+        ok ($class4->has_relationship('loader_test5_from_ids'),
+            "rel with preposition 'from' and _id pluralized backward-compatibly");
+
         # find on multi-col pk
         my $obj5 = $rsobj5->find({id1 => 1, id2 => 1});
         is( $obj5->id2, 1 );
@@ -624,7 +632,11 @@ sub create {
                 id1 INTEGER NOT NULL,
                 iD2 INTEGER NOT NULL,
                 dat VARCHAR(8),
+                from_id INTEGER,
+                to_id INTEGER,
                 PRIMARY KEY (id1,id2)
+                FOREIGN KEY (from_id) REFERENCES loader_test4 (id),
+                FOREIGN KEY (to_id) REFERENCES loader_test4 (id)
             ) $self->{innodb}
         },
 
index 7e25122..75158fa 100644 (file)
@@ -85,7 +85,7 @@ sub _custom_column_info {
 sub run_tests {
     my $self = shift;
 
-    plan tests => 157 + ($self->{extra}->{count} || 0);
+    plan tests => 159 + ($self->{extra}->{count} || 0);
 
     $self->create();
 
@@ -173,7 +173,7 @@ sub setup_schema {
 
        $warn_count++ for grep /^Bad table or view/, @loader_warnings;
 
-       $warn_count++ for grep /stripping trailing _id/, @loader_warnings;
+       $warn_count++ for grep /renaming \S+ relation/, @loader_warnings;
 
        my $vendor = $self->{vendor};
        $warn_count++ for grep /${vendor}_\S+ has no primary key/,
@@ -377,7 +377,7 @@ sub test_schema {
     );
 
     SKIP: {
-        skip $self->{skip_rels}, 99 if $self->{skip_rels};
+        skip $self->{skip_rels}, 101 if $self->{skip_rels};
 
         my $moniker3 = $monikers->{loader_test3};
         my $class3   = $classes->{loader_test3};
@@ -510,6 +510,13 @@ sub test_schema {
         my $rs_rel4 = $obj3->search_related('loader_test4zes');
         isa_ok( $rs_rel4->first, $class4);
 
+        # check rel naming with prepositions
+        ok ($class4->has_relationship('loader_test5s_to'),
+            "rel with preposition 'to' pluralized correctly");
+
+        ok ($class4->has_relationship('loader_test5s_from'),
+            "rel with preposition 'from' pluralized correctly");
+
         # find on multi-col pk
         my $obj5 = 
            eval { $rsobj5->find({id1 => 1, iD2 => 1}) } ||
@@ -959,7 +966,11 @@ sub create {
                 id1 INTEGER NOT NULL,
                 iD2 INTEGER NOT NULL,
                 dat VARCHAR(8),
-                PRIMARY KEY (id1,iD2)
+                from_id INTEGER,
+                to_id INTEGER,
+                PRIMARY KEY (id1,iD2),
+                FOREIGN KEY (from_id) REFERENCES loader_test4 (id),
+                FOREIGN KEY (to_id) REFERENCES loader_test4 (id)
             ) $self->{innodb}
         },
 
similarity index 97%
rename from t/lib/make_dbictest_db2.pm
rename to t/lib/make_dbictest_db_with_unique.pm
index f927da6..4eb8516 100644 (file)
@@ -1,4 +1,4 @@
-package make_dbictest_db2;
+package make_dbictest_db_with_unique;
 
 use strict;
 use warnings;