From e5385b6802fea659c6181c7bc49c6ea2b181dff8 Mon Sep 17 00:00:00 2001 From: Henry Van Styn Date: Mon, 16 Oct 2017 10:00:54 +0200 Subject: [PATCH 1/3] Fix changelog - text no longer applies after 7648acb5 --- Changes | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Changes b/Changes index fabe41dc0..822f340fc 100644 --- a/Changes +++ b/Changes @@ -5,11 +5,6 @@ Current Known Issues / Regressions Revision history for DBIx::Class * Notable Changes and Deprecations - - The entire class hierarchy now explicitly sets the 'c3' mro, even - in cases where load_components was not used. Extensive testing led - the maintainer believe this is safe, but this is a very complex - area and reality may turn out to be different. If **ANYHTING** at - all seems out of place, please file a report at once - The unique constraint info (including the primary key columns) is no longer shared between related (class and schema-level) ResultSource instances. If your app stops working with no obvious pointers, set From 1044aa13fd307a30f953b125336af9d1c8f06d18 Mon Sep 17 00:00:00 2001 From: Henry Van Styn Date: Mon, 16 Oct 2017 10:22:24 +0200 Subject: [PATCH 2/3] Bit forgotten in 5f48fa56 generates a ton of spurios .pod's --- maint/gen_pod_inherit | 1 + 1 file changed, 1 insertion(+) diff --git a/maint/gen_pod_inherit b/maint/gen_pod_inherit index 4164da6c6..a46fe5f9a 100755 --- a/maint/gen_pod_inherit +++ b/maint/gen_pod_inherit @@ -65,6 +65,7 @@ Pod::Inherit->new({ DBIx::Class::Componentised Class::C3::Componentised DBIx::Class::AccessorGroup + DBIx::Class::MethodAttributes Class::Accessor::Grouped Moose::Object Exporter From 942c5be898df155496959a8c2e26257cfaee41e5 Mon Sep 17 00:00:00 2001 From: Henry Van Styn Date: Mon, 16 Oct 2017 10:23:42 +0200 Subject: [PATCH 3/3] Fix fallout from 786c1cdd Before the interface of resolve_relationship_condition was simplified, it contained a codepath exempting result objects from any checks of their get_columns() results. Keep the stringent check ( so that new code using this function will not assume questionable defaults going forward ), and instead fix all callsites where this condition might arise -- This commit fixes the bug revealed by downstream RapidApp test case in this commit: * https://github.com/vanstyn/RapidApp/commit/c319a41904 Huge thanks to @ribasushi for identifing the needed code changes to address this once I was able to provide the failing test case in RapidApp --- lib/DBIx/Class/Relationship/Base.pm | 20 ++++++++++++++++++-- lib/DBIx/Class/ResultSet.pm | 20 ++++++++++++++++++-- lib/DBIx/Class/ResultSource.pm | 17 ++++++++++++++++- t/cdbi/06-hasa.t | 2 +- t/cdbi/18-has_a.t | 2 +- 5 files changed, 54 insertions(+), 7 deletions(-) diff --git a/lib/DBIx/Class/Relationship/Base.pm b/lib/DBIx/Class/Relationship/Base.pm index b7e74eb58..49d830439 100644 --- a/lib/DBIx/Class/Relationship/Base.pm +++ b/lib/DBIx/Class/Relationship/Base.pm @@ -957,7 +957,8 @@ sub set_from_related { : ( ! defined blessed $f_obj ) ? $f_obj : do { - my $f_result_class = $self->result_source->related_source($rel)->result_class; + my $f_result_source = $self->result_source->related_source($rel); + my $f_result_class = $f_result_source->result_class; unless( $f_obj->isa($f_result_class) ) { @@ -975,7 +976,22 @@ sub set_from_related { ); } - +{ $f_obj->get_columns }; + my $fvals = { $f_obj->get_columns }; + + # The very low level resolve_relationship_condition() deliberately contains + # extra logic to ensure that it isn't passed garbage. Unfortunately we can + # get into a situation where an object *has* extra columns on it, which + # the interface of ->get_columns is obligated to return. In order not to + # compromise the sanity checks within r_r_c, simply do a cleanup pass here, + # and in 2 other spots within the codebase to keep things consistent + # + # FIXME - perhaps this should warn, but that's a battle for another day + # + +{ map { + exists $fvals->{$_} + ? ( $_ => $fvals->{$_} ) + : () + } $f_result_source->columns }; } ), diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm index 030f2924b..e66a74044 100644 --- a/lib/DBIx/Class/ResultSet.pm +++ b/lib/DBIx/Class/ResultSet.pm @@ -841,7 +841,8 @@ sub find { foreign_values => ( (! defined blessed $foreign_val) ? $foreign_val : do { - my $f_result_class = $rsrc->related_source($key)->result_class; + my $f_rsrc = $rsrc->related_source($key); + my $f_result_class = $f_rsrc->result_class; unless( $foreign_val->isa($f_result_class) ) { @@ -859,7 +860,22 @@ sub find { ); } - +{ $foreign_val->get_columns }; + my $fvals = { $foreign_val->get_columns }; + + # The very low level resolve_relationship_condition() deliberately contains + # extra logic to ensure that it isn't passed garbage. Unfortunately we can + # get into a situation where an object *has* extra columns on it, which + # the interface of ->get_columns is obligated to return. In order not to + # compromise the sanity checks within r_r_c, simply do a cleanup pass here, + # and in 2 other spots within the codebase to keep things consistent + # + # FIXME - perhaps this should warn, but that's a battle for another day + # + +{ map { + exists $fvals->{$_} + ? ( $_ => $fvals->{$_} ) + : () + } $f_rsrc->columns }; } ), diff --git a/lib/DBIx/Class/ResultSource.pm b/lib/DBIx/Class/ResultSource.pm index ddef5449c..2af12f6f0 100644 --- a/lib/DBIx/Class/ResultSource.pm +++ b/lib/DBIx/Class/ResultSource.pm @@ -2218,7 +2218,22 @@ sub _resolve_condition :DBIC_method_is_indirect_sugar { } # more compat elsif( $_ == 0 and $res_args[0]->isa( $__expected_result_class_isa ) ) { - $res_args[0] = { $res_args[0]->get_columns }; + my $fvals = { $res_args[0]->get_columns }; + + # The very low level resolve_relationship_condition() deliberately contains + # extra logic to ensure that it isn't passed garbage. Unfortunately we can + # get into a situation where an object *has* extra columns on it, which + # the interface of ->get_columns is obligated to return. In order not to + # compromise the sanity checks within r_r_c, simply do a cleanup pass here, + # and in 2 other spots within the codebase to keep things consistent + # + # FIXME - perhaps this should warn, but that's a battle for another day + # + $res_args[0] = { map { + exists $fvals->{$_} + ? ( $_ => $fvals->{$_} ) + : () + } $res_args[0]->result_source->columns }; } } else { diff --git a/t/cdbi/06-hasa.t b/t/cdbi/06-hasa.t index abad17023..6d47c1232 100644 --- a/t/cdbi/06-hasa.t +++ b/t/cdbi/06-hasa.t @@ -118,7 +118,7 @@ sub fail_with_bad_object { NumExplodingSheep => 23 } ); - } qr/is not a column on related source 'Director'/; + } qr/isn't a Director/; } package Foo; diff --git a/t/cdbi/18-has_a.t b/t/cdbi/18-has_a.t index 6304b2c93..a7b069c06 100644 --- a/t/cdbi/18-has_a.t +++ b/t/cdbi/18-has_a.t @@ -110,7 +110,7 @@ is( Rating => 'R', NumExplodingSheep => 23 }); - } qr/is not a column on related source 'Director'/, "Can't have film as codirector"; + } qr/isn't a Director/, "Can't have film as codirector"; is $fail, undef, "We didn't get anything"; my $tastes_bad = YA::Film->create({