From: Brandon Black <blblack@gmail.com>
Date: Fri, 27 Jan 2006 23:28:59 +0000 (+0000)
Subject: some shuffling/refactoring of the relationship code, and a TODO file added
X-Git-Tag: 0.03000~36
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=16f6b6ac6c89347d9f38b16b4adbb417db3d804c;p=dbsrgits%2FDBIx-Class-Schema-Loader.git

some shuffling/refactoring of the relationship code, and a TODO file added
---

diff --git a/TODO b/TODO
new file mode 100644
index 0000000..ca3138e
--- /dev/null
+++ b/TODO
@@ -0,0 +1,18 @@
+
+Reminders to myself or whoever else ever looks in here...
+
+SQLite needs some heavy refactoring, the subroutines are becoming to complex to understand easily.
+MySQL needs implicit FK support, I think.
+the base/use injection stuff needs error checking/reporting
+
+The whole things needs to be refactored so that we don't pollute Schema's namespace.
+
+  - Currently, the user's schema class ISA Schema::Loader ISA Schema::Loader::VENDOR ISA Schema::Loader::Generic ISA Schema, which means all of our methods and accessors in those classes end up in the final user "Schema" class.  The current hack to minimize that is that all the names are prefixed with "_loader" to avoid clashes.
+  - Ideally, user schema class ISA Schema::Loader, and that's it.  Schema::Loader only implements the methods that we need to export to the Schema class for later user (load_from_connection, tables, moniker).  Schema::Loader::VENDOR ISA Schema::Loader::Generic, but they have a new() routine and instantiate an object which does the Loading.  Schema::Loader passes them the its own class name (the user schema class) so that they know what namespace to target the loading operations at.
+
+After all that, consider:
+   If local column is UNIQUE or PK, use has_one() for relation?
+   Re-scan relations/tables after initial relation setup to find ->many_to_many() relations to be set up?
+   Check NULLability of columns involved in the relationship, which might suggest a more optimal non-default -join-type?
+
+...
diff --git a/lib/DBIx/Class/Schema/Loader/DB2.pm b/lib/DBIx/Class/Schema/Loader/DB2.pm
index 3c458c9..8cb4ecb 100644
--- a/lib/DBIx/Class/Schema/Loader/DB2.pm
+++ b/lib/DBIx/Class/Schema/Loader/DB2.pm
@@ -115,7 +115,7 @@ SQL
                 $cond{$other_cols[$i]} = $self_cols[$i];
             }
 
-            eval { $class->_loader_make_relations ($table, $other, \%cond); };
+            eval { $class->_loader_make_cond_rel ($table, $other, \%cond); };
             warn qq/\# belongs_to_many failed "$@"\n\n/
               if $@ && $class->_loader_debug;
         }
diff --git a/lib/DBIx/Class/Schema/Loader/Generic.pm b/lib/DBIx/Class/Schema/Loader/Generic.pm
index 52f8fc3..23b59c3 100644
--- a/lib/DBIx/Class/Schema/Loader/Generic.pm
+++ b/lib/DBIx/Class/Schema/Loader/Generic.pm
@@ -174,69 +174,82 @@ sub _loader_stringify_hash {
            . ' }';
 }
 
-# Setup has_a and has_many relationships
-sub _loader_make_relations {
-
-    my ( $class, $table, $other, $cond ) = @_;
-    my $table_class = $class->_loader_find_table_class($table);
-    my $other_class = $class->_loader_find_table_class($other);
-
-    my $table_relname = lc $table;
-    my $other_relname = lc $other;
+# Inflect a relationship name
+#   XXX (should pluralize, but currently also tends to de-pluralize plurals)
+sub _loader_inflect_relname {
+    my ($class, $relname) = @_;
 
     if(my $inflections = $class->_loader_inflect) {
-        $table_relname = $inflections->{$table_relname}
-          if exists $inflections->{$table_relname};
+        $relname = $inflections->{$relname}
+          if exists $inflections->{$relname};
     }
     else {
-        $table_relname = Lingua::EN::Inflect::PL($table_relname);
+        $relname = Lingua::EN::Inflect::PL($relname);
     }
 
-    if(ref($cond) eq 'HASH') {
-        # for single-column case, set the relname to the column name,
-        # to make filter accessors work
-        if(scalar keys %$cond == 1) {
-            my ($col) = keys %$cond;
-            $other_relname = $cond->{$col};
-        }
+    return $relname;
+}
 
-        my $rev_cond = { reverse %$cond };
+# Set up a simple relation with just a local col and foreign table
+sub _loader_make_simple_rel {
+    my ($class, $table, $other, $col) = @_;
 
-        my $cond_printable = _loader_stringify_hash($cond)
-            if $class->_loader_debug;
-        my $rev_cond_printable = _loader_stringify_hash($rev_cond)
-            if $class->_loader_debug;
+    my $table_class = $class->_loader_find_table_class($table);
+    my $other_class = $class->_loader_find_table_class($other);
+    my $table_relname = $class->_loader_inflect_relname(lc $table);
 
-        warn qq/\# Belongs_to relationship\n/ if $class->_loader_debug;
+    warn qq/\# Belongs_to relationship\n/ if $class->_loader_debug;
+    warn qq/$table_class->belongs_to( '$col' => '$other_class' );\n\n/
+      if $class->_loader_debug;
+    $table_class->belongs_to( $col => $other_class );
 
-        warn qq/$table_class->belongs_to( '$other_relname' => '$other_class',/
-          .  qq/$cond_printable);\n\n/
-          if $class->_loader_debug;
+    warn qq/\# Has_many relationship\n/ if $class->_loader_debug;
+    warn qq/$other_class->has_many( '$table_relname' => '$table_class',/
+      .  qq/$col);\n\n/
+      if $class->_loader_debug;
 
-        $table_class->belongs_to( $other_relname => $other_class, $cond);
+    $other_class->has_many( $table_relname => $table_class, $col);
+}
 
-        warn qq/\# Has_many relationship\n/ if $class->_loader_debug;
+# Set up a complex relation based on a hashref condition
+sub _loader_make_cond_rel {
+    my ( $class, $table, $other, $cond ) = @_;
 
-        warn qq/$other_class->has_many( '$table_relname' => '$table_class',/
-          .  qq/$rev_cond_printable);\n\n/
-          .  qq/);\n\n/
-          if $class->_loader_debug;
+    my $table_class = $class->_loader_find_table_class($table);
+    my $other_class = $class->_loader_find_table_class($other);
+    my $table_relname = $class->_loader_inflect_relname(lc $table);
+    my $other_relname = lc $other;
 
-        $other_class->has_many( $table_relname => $table_class, $rev_cond);
-    }
-    else { # implicit stuff, just a col name
-        warn qq/\# Belongs_to relationship\n/ if $class->_loader_debug;
-        warn qq/$table_class->belongs_to( '$cond' => '$other_class' );\n\n/
-          if $class->_loader_debug;
-        $table_class->belongs_to( $cond => $other_class );
-
-        warn qq/\# Has_many relationship\n/ if $class->_loader_debug;
-        warn qq/$other_class->has_many( '$table_relname' => '$table_class',/
-          .  qq/$cond);\n\n/
-          if $class->_loader_debug;
-
-        $other_class->has_many( $table_relname => $table_class, $cond);
+    # for single-column case, set the relname to the column name,
+    # to make filter accessors work
+    if(scalar keys %$cond == 1) {
+        my ($col) = keys %$cond;
+        $other_relname = $cond->{$col};
     }
+
+    my $rev_cond = { reverse %$cond };
+
+    my $cond_printable = _loader_stringify_hash($cond)
+        if $class->_loader_debug;
+    my $rev_cond_printable = _loader_stringify_hash($rev_cond)
+        if $class->_loader_debug;
+
+    warn qq/\# Belongs_to relationship\n/ if $class->_loader_debug;
+
+    warn qq/$table_class->belongs_to( '$other_relname' => '$other_class',/
+      .  qq/$cond_printable);\n\n/
+      if $class->_loader_debug;
+
+    $table_class->belongs_to( $other_relname => $other_class, $cond);
+
+    warn qq/\# Has_many relationship\n/ if $class->_loader_debug;
+
+    warn qq/$other_class->has_many( '$table_relname' => '$table_class',/
+      .  qq/$rev_cond_printable);\n\n/
+      .  qq/);\n\n/
+      if $class->_loader_debug;
+
+    $other_class->has_many( $table_relname => $table_class, $rev_cond);
 }
 
 # Load and setup classes
@@ -317,7 +330,7 @@ sub _loader_relationships {
         foreach my $relid (keys %$rels) {
             my $reltbl = $rels->{$relid}->{tbl};
             my $cond   = $rels->{$relid}->{cols};
-            eval { $class->_loader_make_relations( $table, $reltbl, $cond ) };
+            eval { $class->_loader_make_cond_rel( $table, $reltbl, $cond ) };
               warn qq/\# belongs_to_many failed "$@"\n\n/
                 if $@ && $class->_loader_debug;
         }
diff --git a/lib/DBIx/Class/Schema/Loader/SQLite.pm b/lib/DBIx/Class/Schema/Loader/SQLite.pm
index 7139e3f..03cab07 100644
--- a/lib/DBIx/Class/Schema/Loader/SQLite.pm
+++ b/lib/DBIx/Class/Schema/Loader/SQLite.pm
@@ -76,38 +76,37 @@ SELECT sql FROM sqlite_master WHERE tbl_name = ?
 
             # Grab reference
             chomp $col;
-            if( $col =~ /^(.*)\s+REFERENCES\s+(\w+) (?: \s* \( (.*) \) )? /ix ) {
+	    next if $col !~ /^(.*)\s+REFERENCES\s+(\w+) (?: \s* \( (.*) \) )? /ix;
 
-                my ($cols, $f_table, $f_cols) = ($1, $2, $3);
+            my ($cols, $f_table, $f_cols) = ($1, $2, $3);
 
-                if($cols =~ /^\(/) { # Table-level
-                    $cols =~ s/^\(\s*//;
-                    $cols =~ s/\s*\)$//;
-                }
-                else {               # Inline
-                    $cols =~ s/\s+.*$//;
-                }
+            if($cols =~ /^\(/) { # Table-level
+                $cols =~ s/^\(\s*//;
+                $cols =~ s/\s*\)$//;
+            }
+            else {               # Inline
+                $cols =~ s/\s+.*$//;
+            }
 
-                my $cond;
-
-                if($f_cols) {
-                    my @cols = map { s/\s*//g; $_ } split(/\s*,\s*/,$cols);
-                    my @f_cols = map { s/\s*//g; $_ } split(/\s*,\s*/,$f_cols);
-                    die "Mismatched column count in rel for $table => $f_table"
-                      if @cols != @f_cols;
-                    $cond = {};
-                    for(my $i = 0 ; $i < @cols; $i++) {
-                        $cond->{$f_cols[$i]} = $cols[$i];
-                    }
-                }
-                else {
-                    $cond = $cols;
-                }
+            my $cond;
 
-                eval { $class->_loader_make_relations( $table, $f_table, $cond ) };
-                warn qq/\# belongs_to_many failed "$@"\n\n/
-                  if $@ && $class->_loader_debug;
+            if($f_cols) {
+                my @cols = map { s/\s*//g; $_ } split(/\s*,\s*/,$cols);
+                my @f_cols = map { s/\s*//g; $_ } split(/\s*,\s*/,$f_cols);
+                die "Mismatched column count in rel for $table => $f_table"
+                  if @cols != @f_cols;
+                $cond = {};
+                for(my $i = 0 ; $i < @cols; $i++) {
+                    $cond->{$f_cols[$i]} = $cols[$i];
+                }
+                eval { $class->_loader_make_cond_rel( $table, $f_table, $cond ) };
             }
+            else {
+                eval { $class->_loader_make_simple_rel( $table, $f_table, $cols ) };
+            }
+
+            warn qq/\# belongs_to_many failed "$@"\n\n/
+              if $@ && $class->_loader_debug;
         }
     }
 }
diff --git a/lib/DBIx/Class/Schema/Loader/mysql.pm b/lib/DBIx/Class/Schema/Loader/mysql.pm
index 24c3f35..f94b159 100644
--- a/lib/DBIx/Class/Schema/Loader/mysql.pm
+++ b/lib/DBIx/Class/Schema/Loader/mysql.pm
@@ -68,7 +68,7 @@ sub _loader_relationships {
                 $cond->{$f_cols[$i]} = $cols[$i];
             }
 
-            eval { $class->_loader_make_relations( $table, $f_table, $cond) };
+            eval { $class->_loader_make_cond_rel( $table, $f_table, $cond) };
             warn qq/\# belongs_to_many failed "$@"\n\n/ if $@ && $class->_loader_debug;
         }