Join conditions supplied to add_relationship are now field-validated
Matt S Trout [Tue, 2 Aug 2005 05:20:46 +0000 (05:20 +0000)]
lib/DBIx/Class/CDBICompat/ImaDBI.pm
lib/DBIx/Class/Relationship.pm
t/06relationship.t

index 22b7eff..9a451a2 100644 (file)
@@ -17,20 +17,20 @@ __PACKAGE__->mk_classdata('_transform_sql_handlers' =>
         return $class->_table_name unless $data;
         my ($f_class, $alias) = split(/=/, $data);
         $f_class ||= $class;
-        $self->{_aliases}{$alias} = $f_class;
+        $self->{_classes}{$alias} = $f_class;
         return $f_class->_table_name." ${alias}";
       },
     'ESSENTIAL' =>
       sub {
         my ($self, $class, $data) = @_;
         return join(' ', $class->columns('Essential')) unless $data;
-        return join(' ', $self->{_aliases}{$data}->columns('Essential'));
+        return join(' ', $self->{_classes}{$data}->columns('Essential'));
       },
     'JOIN' =>
       sub {
         my ($self, $class, $data) = @_;
         my ($from, $to) = split(/ /, $data);
-        my ($from_class, $to_class) = @{$self->{_aliases}}{$from, $to};
+        my ($from_class, $to_class) = @{$self->{_classes}}{$from, $to};
         my ($rel_obj) = grep { $_->{class} && $_->{class} eq $to_class }
                           values %{ $from_class->_relationships };
         unless ($rel_obj) {
@@ -42,6 +42,7 @@ __PACKAGE__->mk_classdata('_transform_sql_handlers' =>
         $self->throw( "No relationship to JOIN from ${from_class} to ${to_class}" )
           unless $rel_obj;
         my $attrs = {
+          %$self,
           _aliases => { self => $from, foreign => $to },
           _action => 'join',
         };
index a13c0e3..9b405e9 100644 (file)
@@ -28,11 +28,20 @@ on searches.
 sub add_relationship {
   my ($class, $rel, $f_class, $cond, $attrs) = @_;
   die "Can't create relationship without join condition" unless $cond;
+  $attrs ||= {};
+  eval "use $f_class;";
   my %rels = %{ $class->_relationships };
   $rels{$rel} = { class => $f_class,
                   cond  => $cond,
                   attrs => $attrs };
   $class->_relationships(\%rels);
+  #warn %{$f_class->_columns};
+  return unless eval { %{$f_class->_columns}; }; # Foreign class not loaded
+  my %join = (%$attrs, _action => 'join',
+    _aliases => { 'self' => 'me', 'foreign' => $rel },
+    _classes => { 'me' => $class, $rel => $f_class });
+  eval { $class->_cond_resolve($cond, \%join) };
+  $class->throw("Error creating relationship $rel: $@") if $@;
 }
 
 sub _cond_key {
@@ -45,11 +54,14 @@ sub _cond_key {
     return $key;
   } elsif ($action eq 'join') {
     my ($type, $field) = split(/\./, $key);
-    if ($attrs->{_aliases}{$type}) {
-      return join('.', $attrs->{_aliases}{$type}, $field);
+    if (my $alias = $attrs->{_aliases}{$type}) {
+      my $class = $attrs->{_classes}{$alias};
+      $self->throw("Unknown column $field on $class as $alias")
+        unless exists $class->_columns->{$field};
+      return join('.', $alias, $field);
     } else {
       $self->throw( "Unable to resolve type ${type}: only have aliases for ".
-            join(', ', keys %{$attrs->{_aliases}{$type} || {}}) );
+            join(', ', keys %{$attrs->{_aliases} || {}}) );
     }
   }
   return $self->NEXT::ACTUAL::_cond_key($attrs, $key);
@@ -69,11 +81,14 @@ sub _cond_value {
     return '?';
   } elsif ($action eq 'join') {
     my ($type, $field) = split(/\./, $value);
-    if ($attrs->{_aliases}{$type}) {
-      return join('.', $attrs->{_aliases}{$type}, $field);
+    if (my $alias = $attrs->{_aliases}{$type}) {
+      my $class = $attrs->{_classes}{$alias};
+      $self->throw("Unknown column $field on $class as $alias")
+        unless exists $class->_columns->{$field};
+      return join('.', $alias, $field);
     } else {
       $self->throw( "Unable to resolve type ${type}: only have aliases for ".
-            join(', ', keys %{$attrs->{_aliases}{$type} || {}}) );
+            join(', ', keys %{$attrs->{_aliases} || {}}) );
     }
   }
       
index bf3830f..ed19fb5 100644 (file)
@@ -28,7 +28,7 @@ is( ($artist->search_related('cds'))[3]->title, 'Big Flop', 'create_related ok'
 
 SKIP: {
 
-  skip "Relationship with invalid cols not yet checked", 1;
+  #skip "Relationship with invalid cols not yet checked", 1;
 
 # try to add a bogus relationship using the wrong cols
 eval {
@@ -37,7 +37,7 @@ eval {
         { 'foreign.cd' => 'self.cdid' }
     );
 };
-like($@, qr/no such accessor/, 'failed when creating a rel with invalid key, ok');
+like($@, qr/Unknown column/, 'failed when creating a rel with invalid key, ok');
 
 } # End SKIP block