From: Peter Rabbitson Date: Mon, 21 Jul 2014 10:41:46 +0000 (+0200) Subject: Avoid ResultSourceProxy calls whenever possible X-Git-Tag: v0.082800~123 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=4006691d;p=dbsrgits%2FDBIx-Class.git Avoid ResultSourceProxy calls whenever possible 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 --- diff --git a/Changes b/Changes index 6cd1d01..d69a071 100644 --- a/Changes +++ b/Changes @@ -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 diff --git a/lib/DBIx/Class/FilterColumn.pm b/lib/DBIx/Class/FilterColumn.pm index 222dabd..2e8fbd5 100644 --- a/lib/DBIx/Class/FilterColumn.pm +++ b/lib/DBIx/Class/FilterColumn.pm @@ -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}; @@ -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}; @@ -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}; @@ -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 @@ -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}); } } diff --git a/lib/DBIx/Class/InflateColumn.pm b/lib/DBIx/Class/InflateColumn.pm index e10af30..b235a4d 100644 --- a/lib/DBIx/Class/InflateColumn.pm +++ b/lib/DBIx/Class/InflateColumn.pm @@ -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}; @@ -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}; @@ -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} diff --git a/lib/DBIx/Class/InflateColumn/File.pm b/lib/DBIx/Class/InflateColumn/File.pm index aa06dbc..195e6ef 100644 --- a/lib/DBIx/Class/InflateColumn/File.pm +++ b/lib/DBIx/Class/InflateColumn/File.pm @@ -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}; @@ -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 } @@ -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}); } diff --git a/lib/DBIx/Class/Relationship/Base.pm b/lib/DBIx/Class/Relationship/Base.pm index e0007ff..ab7f33c 100644 --- a/lib/DBIx/Class/Relationship/Base.pm +++ b/lib/DBIx/Class/Relationship/Base.pm @@ -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(@_) : {}); @@ -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 { diff --git a/lib/DBIx/Class/ResultSourceProxy.pm b/lib/DBIx/Class/ResultSourceProxy.pm index db4337a..1e1f307 100644 --- a/lib/DBIx/Class/ResultSourceProxy.pm +++ b/lib/DBIx/Class/ResultSourceProxy.pm @@ -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; diff --git a/lib/DBIx/Class/Row.pm b/lib/DBIx/Class/Row.pm index d856ab5..d356218 100644 --- a/lib/DBIx/Class/Row.pm +++ b/lib/DBIx/Class/Row.pm @@ -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; } @@ -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; } @@ -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}; @@ -842,9 +846,9 @@ See L 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 ); @@ -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 ( @@ -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 @@ -1080,9 +1084,11 @@ See also L. 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') { @@ -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}); } } @@ -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}; @@ -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; @@ -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 ); @@ -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;