From: Peter Rabbitson <ribasushi@cpan.org>
Date: Sun, 14 Sep 2014 18:59:58 +0000 (+0200)
Subject: Ensure the custom rel cond resolver does not trigger forgotten compat shim
X-Git-Tag: v0.082800~42
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=c200d94979bde5ac74070d3e898927433b0e667c;p=dbsrgits%2FDBIx-Class.git

Ensure the custom rel cond resolver does not trigger forgotten compat shim

During the rush to get custom rels out the door (this is why rushing
fucking sucks), a697fa31 introduced a shortsighted workaround into
::SQLMaker::_from_chunk_to_sql(). This code slipped consequent review
and made its way into the codebase... 4 FUCKING YEARS AGO!!! >:(

Since it is not known how much stuff relies on the insanity being there
(moreover we have tests that rely on it) leave things as is for the time
being. The only change is  making the cond resolver *completely* oblivious to
the "single-element hash" workaround (albeit via a silly hack).

In the process exposed that ora-joins module is entirely incapable
of understanding non-equality conds... fml

See next commits for added warnings, etc.
---

diff --git a/lib/DBIx/Class/ResultSource.pm b/lib/DBIx/Class/ResultSource.pm
index 34f26e9..4669926 100644
--- a/lib/DBIx/Class/ResultSource.pm
+++ b/lib/DBIx/Class/ResultSource.pm
@@ -2163,10 +2163,28 @@ sub _resolve_relationship_condition {
           ;
         }
       }
+      elsif (
+        $col_eqs->{$lhs} =~ /^ ( \Q$args->{self_alias}\E \. .+ ) /x
+          and
+        ($colinfos->{$1}||{})->{-result_source} == $rel_rsrc
+      ) {
+        my ($lcol, $rcol) = map
+          { $colinfos->{$_}{-colname} }
+          ( $lhs, $1 )
+        ;
+        carp_unique(
+          "The $exception_rel_id specifies equality of column '$lcol' and the "
+        . "*VALUE* '$rcol' (you did not use the { -ident => ... } operator)"
+        );
+      }
     }
   }
 
-  $ret
+  # FIXME - temporary, to fool the idiotic check in SQLMaker::_join_condition
+  $ret->{condition} = { -and => [ $ret->{condition} ] }
+    unless $ret->{condition} eq UNRESOLVABLE_CONDITION;
+
+  $ret;
 }
 
 =head2 related_source
diff --git a/lib/DBIx/Class/SQLMaker/OracleJoins.pm b/lib/DBIx/Class/SQLMaker/OracleJoins.pm
index b95c56e..44f4b08 100644
--- a/lib/DBIx/Class/SQLMaker/OracleJoins.pm
+++ b/lib/DBIx/Class/SQLMaker/OracleJoins.pm
@@ -80,6 +80,19 @@ sub _recurse_oracle_joins {
         && $jt !~ /inner/i;
     }
 
+    # FIXME - the code below *UTTERLY* doesn't work with custom conds... sigh
+    # for the time being do not do any processing with the likes of _collapse_cond
+    # instead only unroll the -and hack if present
+    $on = $on->{-and}[0] if (
+      ref $on eq 'HASH'
+        and
+      keys %$on == 1
+        and
+      ref $on->{-and} eq 'ARRAY'
+        and
+      @{$on->{-and}} == 1
+    );
+
     # sadly SQLA treats where($scalar) as literal, so we need to jump some hoops
     push @where, map { \sprintf ('%s%s = %s%s',
       ref $_ ? $self->_recurse_where($_) : $self->_quote($_),
diff --git a/t/lib/DBICTest/Schema/Track.pm b/t/lib/DBICTest/Schema/Track.pm
index c43591a..10d49f7 100644
--- a/t/lib/DBICTest/Schema/Track.pm
+++ b/t/lib/DBICTest/Schema/Track.pm
@@ -116,6 +116,20 @@ __PACKAGE__->has_many (
   }
 );
 
+__PACKAGE__->has_many (
+  deliberately_broken_all_cd_tracks => __PACKAGE__,
+  sub {
+    # This is for test purposes only. A regular user does not
+    # need to sanity check the passed-in arguments, this is what
+    # the tests are for :)
+    my $args = &check_customcond_args;
+
+    return {
+      "$args->{foreign_alias}.cd" => "$args->{self_alias}.cd"
+    };
+  }
+);
+
 our $hook_cb;
 
 sub sqlt_deploy_hook {
diff --git a/t/relationship/custom.t b/t/relationship/custom.t
index fb48597..f79b605 100644
--- a/t/relationship/custom.t
+++ b/t/relationship/custom.t
@@ -3,6 +3,7 @@ use warnings;
 
 use Test::More;
 use Test::Exception;
+use Test::Warn;
 use lib qw(t/lib);
 use DBICTest ':DiffSQL';
 
@@ -297,6 +298,28 @@ is_deeply
   'Proper find with related via coderef cond',
 ;
 
+warnings_exist {
+  is_same_sql_bind(
+    $single_track->deliberately_broken_all_cd_tracks->as_query,
+    '(
+      SELECT me.trackid, me.cd, me.position, me.title, me.last_updated_on, me.last_updated_at
+        FROM track track__row
+        JOIN track me
+          ON me.cd = ?
+      WHERE track__row.trackid = ?
+    )',
+    [
+      [{ dbic_colname => "me.cd", sqlt_datatype => "integer" }
+        => "track__row.cd" ],
+      [{ dbic_colname => "track__row.trackid", sqlt_datatype => "integer" }
+        => 19 ],
+    ],
+    'Expected nonsensical JOIN cond',
+  ),
+} qr/\Qrelationship 'deliberately_broken_all_cd_tracks' on source 'Track' specifies equality of column 'cd' and the *VALUE* 'cd' (you did not use the { -ident => ... } operator)/,
+  'Warning on 99.9999% malformed custom cond'
+;
+
 $single_track->set_from_related( cd_cref_cond => undef );
 ok $single_track->is_column_changed('cd');
 is $single_track->get_column('cd'), undef, 'UNset from related via coderef cond';