Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug in resolve_relationship_condition with sources with virtual columns #115

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 0 additions & 5 deletions Changes
Expand Up @@ -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
Expand Down
20 changes: 18 additions & 2 deletions lib/DBIx/Class/Relationship/Base.pm
Expand Up @@ -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) ) {

Expand All @@ -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 };
}
),

Expand Down
20 changes: 18 additions & 2 deletions lib/DBIx/Class/ResultSet.pm
Expand Up @@ -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) ) {

Expand All @@ -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 };
}
),

Expand Down
17 changes: 16 additions & 1 deletion lib/DBIx/Class/ResultSource.pm
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions maint/gen_pod_inherit
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion t/cdbi/06-hasa.t
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion t/cdbi/18-has_a.t
Expand Up @@ -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({
Expand Down