Skip to content

Commit

Permalink
Protect several resolve_relationship_condition() callsites
Browse files Browse the repository at this point in the history
Some external use of DBIx::Class::ParameterizedJoinHack revealed a couple
sites where the relationship resolution may unexpectedly, yet non-fatally
fail. This protects all the reasonable places (partially reverting b47fb9c),
downgrading the exceptions to once-per-callsite warnings.

I did not have time to dig to find the underlying problem, there may very well
be a real bug lurking around :/

For reproduction of the (now) warnings: see https://github.com/ctrlo/lenio
  • Loading branch information
ribasushi committed Sep 27, 2016
1 parent 1bd54f3 commit e5c6382
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 15 deletions.
42 changes: 38 additions & 4 deletions lib/DBIx/Class/Relationship/Base.pm
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,16 @@ use base qw/DBIx::Class/;
use Scalar::Util qw/weaken blessed/;
use DBIx::Class::_Util qw(
UNRESOLVABLE_CONDITION DUMMY_ALIASPAIR
dbic_internal_try fail_on_internal_call
dbic_internal_try dbic_internal_catch fail_on_internal_call
);
use DBIx::Class::SQLMaker::Util 'extract_equality_conditions';
use DBIx::Class::Carp;

# FIXME - this should go away
# instead Carp::Skip should export usable keywords or something like that
my $unique_carper;
BEGIN { $unique_carper = \&carp_unique }

use namespace::clean;

=head1 NAME
Expand Down Expand Up @@ -525,8 +531,7 @@ sub related_resultset {

my $relcond_is_freeform = ref $rel_info->{cond} eq 'CODE';

my $jfc = $rsrc->_resolve_relationship_condition(

my $rrc_args = {
rel_name => $rel,
self_result_object => $self,

Expand All @@ -545,8 +550,37 @@ sub related_resultset {
# out of an existing object, with the new source being at the head
# of the FROM chain. Having a 'me' alias is nothing but expected there
foreign_alias => 'me',
};

)->{join_free_condition};
my $jfc = (
# In certain extraordinary circumstances the relationship resolution may
# throw (e.g. when walking through elaborate custom conds)
# In case the object is "real" (i.e. in_storage) we just go ahead and
# let the exception surface. Otherwise we carp and move on.
#
# The elaborate code-duplicating ternary is there because the xsified
# ->in_storage() is orders of magnitude faster than the Try::Tiny-like
# construct below ( perl's low level tooling is truly shit :/ )
( $self->in_storage or DBIx::Class::_Util::in_internal_try )
? $rsrc->_resolve_relationship_condition($rrc_args)->{join_free_condition}
: dbic_internal_try {
$rsrc->_resolve_relationship_condition($rrc_args)->{join_free_condition}
}
dbic_internal_catch {
$unique_carper->(
"Resolution of relationship '$rel' failed unexpectedly, "
. 'please relay the following error and seek assistance via '
. DBIx::Class::_ENV_::HELP_URL . ". Encountered error: $_"
);

# FIXME - this is questionable
# force skipping re-resolution, and instead just return an UC rset
$relcond_is_freeform = 0;

# RV
undef;
}
);

my $rel_rset;

Expand Down
9 changes: 5 additions & 4 deletions lib/DBIx/Class/Relationship/ManyToMany.pm
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ use warnings;
use DBIx::Class::Carp;
use DBIx::Class::_Util qw( quote_sub perlstring );

# FIXME - this souldn't be needed
my $cu;
BEGIN { $cu = \&carp_unique }
# FIXME - this should go away
# instead Carp::Skip should export usable keywords or something like that
my $unique_carper;
BEGIN { $unique_carper = \&carp_unique }

use namespace::clean;

Expand Down Expand Up @@ -82,7 +83,7 @@ EOC
my @extra_meth_qsub_args = (
{
'$rel_attrs' => \{ alias => $f_rel, %{ $rel_attrs||{} } },
'$carp_unique' => \$cu,
'$carp_unique' => \$unique_carper,
},
{ attributes => [
'DBIC_method_is_indirect_sugar',
Expand Down
34 changes: 27 additions & 7 deletions lib/DBIx/Class/ResultSource/RowParser.pm
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,15 @@ use DBIx::Class::ResultSource::RowParser::Util qw(
assemble_simple_parser
assemble_collapsing_parser
);
use DBIx::Class::_Util 'DUMMY_ALIASPAIR';
use DBIx::Class::_Util qw( DUMMY_ALIASPAIR dbic_internal_try dbic_internal_catch );

use DBIx::Class::Carp;

# FIXME - this should go away
# instead Carp::Skip should export usable keywords or something like that
my $unique_carper;
BEGIN { $unique_carper = \&carp_unique }

use namespace::clean;

# Accepts a prefetch map (one or more relationships for the current source),
Expand Down Expand Up @@ -187,13 +192,28 @@ sub _resolve_collapse {
is_single => ( $inf->{attrs}{accessor} && $inf->{attrs}{accessor} ne 'multi' ),
is_inner => ( ( $inf->{attrs}{join_type} || '' ) !~ /^left/i),
rsrc => $self->related_source($rel),
fk_map => $self->_resolve_relationship_condition(
rel_name => $rel,
fk_map => (
dbic_internal_try {
$self->_resolve_relationship_condition(
rel_name => $rel,

# an API where these are optional would be too cumbersome,
# instead always pass in some dummy values
DUMMY_ALIASPAIR,
)->{identity_map},
}
dbic_internal_catch {

$unique_carper->(
"Resolution of relationship '$rel' failed unexpectedly, "
. 'please relay the following error and seek assistance via '
. DBIx::Class::_ENV_::HELP_URL . ". Encountered error: $_"
);

# an API where these are optional would be too cumbersome,
# instead always pass in some dummy values
DUMMY_ALIASPAIR,
)->{identity_map},
# RV
+{}
}
),
};
}

Expand Down

0 comments on commit e5c6382

Please sign in to comment.