Skip to content

Commit

Permalink
Avoid ResultSourceProxy calls whenever possible
Browse files Browse the repository at this point in the history
Along with efficiency gains this commit makes a very subtle but crucially
important change: From here now on when we operate on an instance, we are
guaranteed to query this instance's result source. The previous codepaths
would nearly randomly switch between the current rsrc instance and the one
registered with the corresponding result class.

This will allow for proper synthetic result instance construction further on
  • Loading branch information
ribasushi committed Jul 22, 2014
1 parent 12b348d commit 4006691
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 43 deletions.
2 changes: 2 additions & 0 deletions Changes
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ Revision history for DBIx::Class
savepoints on DBD::SQLite < 1.39

* Misc
- Ensure source metadata calls always take place on the result source
instance registered with the caller
- IFF DBIC_TRACE output defaults to STDERR we now silence the possible
wide-char warnings if the trace happens to contain unicode
- Remove ::ResultSource::resolve_condition - the underlying machinery
Expand Down
22 changes: 11 additions & 11 deletions lib/DBIx/Class/FilterColumn.pm
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ sub _column_from_storage {

return $value if is_literal_value($value);

my $info = $self->column_info($col)
my $info = $self->result_source->column_info($col)
or $self->throw_exception("No column info for $col");

return $value unless exists $info->{_filter_info};
Expand All @@ -49,7 +49,7 @@ sub _column_to_storage {

return $value if is_literal_value($value);

my $info = $self->column_info($col) or
my $info = $self->result_source->column_info($col) or
$self->throw_exception("No column info for $col");

return $value unless exists $info->{_filter_info};
Expand All @@ -63,7 +63,7 @@ sub get_filtered_column {
my ($self, $col) = @_;

$self->throw_exception("$col is not a filtered column")
unless exists $self->column_info($col)->{_filter_info};
unless exists $self->result_source->column_info($col)->{_filter_info};

return $self->{_filtered_column}{$col}
if exists $self->{_filtered_column}{$col};
Expand Down Expand Up @@ -140,12 +140,10 @@ sub set_filtered_column {
sub update {
my ($self, $data, @rest) = @_;

my $colinfos = $self->result_source->columns_info;

foreach my $col (keys %{$data||{}}) {
if (
$self->has_column($col)
&&
exists $self->column_info($col)->{_filter_info}
) {
if ( exists $colinfos->{$col}{_filter_info} ) {
$self->set_filtered_column($col, delete $data->{$col});

# FIXME update() reaches directly into the object-hash
Expand All @@ -160,14 +158,16 @@ sub update {

sub new {
my ($class, $data, @rest) = @_;
my $source = $data->{-result_source}

my $rsrc = $data->{-result_source}
or $class->throw_exception('Sourceless rows are not supported with DBIx::Class::FilterColumn');

my $obj = $class->next::method($data, @rest);

my $colinfos = $rsrc->columns_info;

foreach my $col (keys %{$data||{}}) {
if ($obj->has_column($col) &&
exists $obj->column_info($col)->{_filter_info} ) {
if (exists $colinfos->{$col}{_filter_info} ) {
$obj->set_filtered_column($col, $data->{$col});
}
}
Expand Down
6 changes: 3 additions & 3 deletions lib/DBIx/Class/InflateColumn.pm
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ sub _inflated_column {
is_literal_value($value) #that would be a not-yet-reloaded literal update
);

my $info = $self->column_info($col)
my $info = $self->result_source->column_info($col)
or $self->throw_exception("No column info for $col");

return $value unless exists $info->{_inflate_info};
Expand All @@ -133,7 +133,7 @@ sub _deflated_column {
is_literal_value($value)
);

my $info = $self->column_info($col) or
my $info = $self->result_source->column_info($col) or
$self->throw_exception("No column info for $col");

return $value unless exists $info->{_inflate_info};
Expand All @@ -160,7 +160,7 @@ sub get_inflated_column {
my ($self, $col) = @_;

$self->throw_exception("$col is not an inflated column")
unless exists $self->column_info($col)->{_inflate_info};
unless exists $self->result_source->column_info($col)->{_inflate_info};

# we take care of keeping things in sync
return $self->{_inflated_column}{$col}
Expand Down
14 changes: 9 additions & 5 deletions lib/DBIx/Class/InflateColumn/File.pm
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ sub register_column {
sub _file_column_file {
my ($self, $column, $filename) = @_;

my $column_info = $self->column_info($column);
my $column_info = $self->result_source->column_info($column);

return unless $column_info->{is_file_column};

Expand All @@ -60,8 +60,10 @@ sub _file_column_file {
sub delete {
my ( $self, @rest ) = @_;

for ( $self->columns ) {
if ( $self->column_info($_)->{is_file_column} ) {
my $colinfos = $self->result_source->columns_info;

for ( keys %$colinfos ) {
if ( $colinfos->{$_}{is_file_column} ) {
rmtree( [$self->_file_column_file($_)->dir], 0, 0 );
last; # if we've deleted one, we've deleted them all
}
Expand All @@ -75,9 +77,11 @@ sub insert {

# cache our file columns so we can write them to the fs
# -after- we have a PK
my $colinfos = $self->result_source->columns_info;

my %file_column;
for ( $self->columns ) {
if ( $self->column_info($_)->{is_file_column} ) {
for ( keys %$colinfos ) {
if ( $colinfos->{$_}{is_file_column} ) {
$file_column{$_} = $self->$_;
$self->store_column($_ => $self->$_->{filename});
}
Expand Down
6 changes: 3 additions & 3 deletions lib/DBIx/Class/Relationship/Base.pm
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,9 @@ sub related_resultset {

return $self->{related_resultsets}{$rel} = do {

my $rel_info = $self->relationship_info($rel)
my $rsrc = $self->result_source;

my $rel_info = $rsrc->relationship_info($rel)
or $self->throw_exception( "No such relationship '$rel'" );

my $attrs = (@_ > 1 && ref $_[$#_] eq 'HASH' ? pop(@_) : {});
Expand All @@ -485,8 +487,6 @@ sub related_resultset {
if (@_ > 1 && (@_ % 2 == 1));
my $query = ((@_ > 1) ? {@_} : shift);

my $rsrc = $self->result_source;

# condition resolution may fail if an incomplete master-object prefetch
# is encountered - that is ok during prefetch construction (not yet in_storage)
my ($cond, $is_crosstable) = try {
Expand Down
9 changes: 5 additions & 4 deletions lib/DBIx/Class/ResultSourceProxy.pm
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,11 @@ for my $method_to_proxy (qw/
relationship_info
has_relationship
/) {
quote_sub
__PACKAGE__."::$method_to_proxy"
=> "shift->result_source_instance->$method_to_proxy (\@_);"
;
quote_sub __PACKAGE__."::$method_to_proxy", sprintf( <<'EOC', $method_to_proxy );
DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and DBIx::Class::_Util::fail_on_internal_call;
shift->result_source_instance->%s (@_);
EOC

}

1;
46 changes: 29 additions & 17 deletions lib/DBIx/Class/Row.pm
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,12 @@ sub new {
}
$inflated->{$key} = $rel_obj;
next;
} elsif ($class->has_column($key)
&& $class->column_info($key)->{_inflate_info}) {
}
elsif (
$rsrc->has_column($key)
and
$rsrc->column_info($key)->{_inflate_info}
) {
$inflated->{$key} = $attrs->{$key};
next;
}
Expand Down Expand Up @@ -672,7 +676,7 @@ sub get_column {
}

$self->throw_exception( "No such column '${column}' on " . ref $self )
unless $self->has_column($column);
unless $self->result_source->has_column($column);

return undef;
}
Expand Down Expand Up @@ -800,7 +804,7 @@ sub make_column_dirty {
my ($self, $column) = @_;

$self->throw_exception( "No such column '${column}' on " . ref $self )
unless exists $self->{_column_data}{$column} || $self->has_column($column);
unless exists $self->{_column_data}{$column} || $self->result_source->has_column($column);

# the entire clean/dirty code relies on exists, not on true/false
return 1 if exists $self->{_dirty_columns}{$column};
Expand Down Expand Up @@ -842,9 +846,9 @@ See L<DBIx::Class::InflateColumn> for how to setup inflation.
sub get_inflated_columns {
my $self = shift;

my $loaded_colinfo = $self->columns_info ([
grep { $self->has_column_loaded($_) } $self->columns
]);
my $loaded_colinfo = $self->result_source->columns_info;
$self->has_column_loaded($_) or delete $loaded_colinfo->{$_}
for keys %$loaded_colinfo;

my %cols_to_return = ( %{$self->{_column_data}}, %$loaded_colinfo );

Expand Down Expand Up @@ -887,7 +891,7 @@ sub get_inflated_columns {

sub _is_column_numeric {
my ($self, $column) = @_;
my $colinfo = $self->column_info ($column);
my $colinfo = $self->result_source->column_info ($column);

# cache for speed (the object may *not* have a resultsource instance)
if (
Expand Down Expand Up @@ -1018,7 +1022,7 @@ sub _eq_column_values {
# value tracked between column changes and commitment to storage
sub _track_storage_value {
my ($self, $col) = @_;
return defined first { $col eq $_ } ($self->primary_columns);
return defined first { $col eq $_ } ($self->result_source->primary_columns);
}

=head2 set_columns
Expand Down Expand Up @@ -1080,9 +1084,11 @@ See also L<DBIx::Class::Relationship::Base/set_from_related>.

sub set_inflated_columns {
my ( $self, $upd ) = @_;
my $rsrc;
foreach my $key (keys %$upd) {
if (ref $upd->{$key}) {
my $info = $self->relationship_info($key);
$rsrc ||= $self->result_source;
my $info = $rsrc->relationship_info($key);
my $acc_type = $info->{attrs}{accessor} || '';

if ($acc_type eq 'single') {
Expand All @@ -1095,7 +1101,11 @@ sub set_inflated_columns {
"Recursive update is not supported over relationships of type '$acc_type' ($key)"
);
}
elsif ($self->has_column($key) && exists $self->column_info($key)->{_inflate_info}) {
elsif (
$rsrc->has_column($key)
and
exists $rsrc->column_info($key)->{_inflate_info}
) {
$self->set_inflated_column($key, delete $upd->{$key});
}
}
Expand Down Expand Up @@ -1135,7 +1145,9 @@ sub copy {
$changes ||= {};
my $col_data = { %{$self->{_column_data}} };

my $colinfo = $self->columns_info([ keys %$col_data ]);
my $rsrc = $self->result_source;

my $colinfo = $rsrc->columns_info([ keys %$col_data ]);
foreach my $col (keys %$col_data) {
delete $col_data->{$col}
if $colinfo->{$col}{is_auto_increment};
Expand All @@ -1144,7 +1156,7 @@ sub copy {
my $new = { _column_data => $col_data };
bless $new, ref $self;

$new->result_source($self->result_source);
$new->result_source($rsrc);
$new->set_inflated_columns($changes);
$new->insert;

Expand All @@ -1153,12 +1165,12 @@ sub copy {
# constraints
my $rel_names_copied = {};

foreach my $rel_name ($self->result_source->relationships) {
my $rel_info = $self->result_source->relationship_info($rel_name);
foreach my $rel_name ($rsrc->relationships) {
my $rel_info = $rsrc->relationship_info($rel_name);

next unless $rel_info->{attrs}{cascade_copy};

my $resolved = $self->result_source->_resolve_condition(
my $resolved = $rsrc->_resolve_condition(
$rel_info->{cond}, $rel_name, $new, $rel_name
);

Expand Down Expand Up @@ -1198,7 +1210,7 @@ extend this method to catch all data setting methods.
sub store_column {
my ($self, $column, $value) = @_;
$self->throw_exception( "No such column '${column}' on " . ref $self )
unless exists $self->{_column_data}{$column} || $self->has_column($column);
unless exists $self->{_column_data}{$column} || $self->result_source->has_column($column);
$self->throw_exception( "set_column called for ${column} without value" )
if @_ < 3;
return $self->{_column_data}{$column} = $value;
Expand Down

0 comments on commit 4006691

Please sign in to comment.