Skip to content

Commit

Permalink
A number of equivalent-logic ::FC refactors
Browse files Browse the repository at this point in the history
Stop invoking a comparison on the unfiltered values - delegate it properly
to set_column()
  • Loading branch information
ribasushi committed Jun 4, 2014
1 parent b342451 commit dc6dada
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 15 deletions.
40 changes: 26 additions & 14 deletions lib/DBIx/Class/FilterColumn.pm
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ sub get_filtered_column {

sub get_column {
my ($self, $col) = @_;

if (exists $self->{_filtered_column}{$col}) {
return $self->{_column_data}{$col} ||= $self->_column_to_storage (
$col, $self->{_filtered_column}{$col}
Expand All @@ -86,11 +87,12 @@ sub get_column {
sub get_columns {
my $self = shift;

foreach my $col (keys %{$self->{_filtered_column}||{}}) {
$self->{_column_data}{$col} ||= $self->_column_to_storage (
$col, $self->{_filtered_column}{$col}
) if exists $self->{_filtered_column}{$col};
}
$self->{_column_data}{$_} = $self->_column_to_storage (
$_, $self->{_filtered_column}{$_}
) for grep
{ ! exists $self->{_column_data}{$_} }
keys %{$self->{_filtered_column}||{}}
;

$self->next::method (@_);
}
Expand All @@ -104,19 +106,29 @@ sub store_column {
$self->next::method(@_);
}

sub has_column_loaded {
my ($self, $col) = @_;
return 1 if exists $self->{_filtered_column}{$col};
return $self->next::method($col);
}

sub set_filtered_column {
my ($self, $col, $filtered) = @_;

# do not blow up the cache via set_column unless necessary
# (filtering may be expensive!)
if (exists $self->{_filtered_column}{$col}) {
return $filtered
if ($self->_eq_column_values ($col, $filtered, $self->{_filtered_column}{$col} ) );

$self->make_column_dirty ($col); # so the comparison won't run again
# unlike IC, FC does not need to deal with the 'filter' abomination
# thus we can short-curcuit filtering entirely and never call set_column
# in case this is already a dirty change OR the row never touched storage
if (
! $self->in_storage
or
$self->is_column_changed($col)
) {
$self->make_column_dirty($col);
delete $self->{_column_data}{$col};
}

$self->set_column($col, $self->_column_to_storage($col, $filtered));
else {
$self->set_column($col, $self->_column_to_storage($col, $filtered));
};

return $self->{_filtered_column}{$col} = $filtered;
}
Expand Down
22 changes: 21 additions & 1 deletion t/row/filter_column.t
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ CACHE_TEST: {
ok (! $artist->is_column_changed ('rank'), 'Column not marked as dirty on same accessor-set value');
is ($artist->rank, '6', 'Column set properly');
is $from_storage_ran, $expected_from, 'from did not run';
is $to_storage_ran, $expected_to, 'to did not run';
is $to_storage_ran, ++$expected_to, 'to did run once (call in to set_column)';

$artist->store_column(rank => 4);
ok (! $artist->is_column_changed ('rank'), 'Column not marked as dirty on differing store_column value');
Expand All @@ -120,6 +120,26 @@ CACHE_TEST: {
is $to_storage_ran, $expected_to, 'to did not run';
}

# test in-memory operations
for my $artist_maker (
sub { $schema->resultset('Artist')->new({ rank => 42 }) },
sub { my $art = $schema->resultset('Artist')->new({}); $art->rank(42); $art },
) {

my $expected_from = $from_storage_ran;
my $expected_to = $to_storage_ran;

my $artist = $artist_maker->();

is $from_storage_ran, $expected_from, 'from has not run yet';
is $to_storage_ran, $expected_to, 'to has not run yet';

ok( ! $artist->has_column_loaded('artistid'), 'pk not loaded' );
ok( $artist->has_column_loaded('rank'), 'Filtered column marked as loaded under new' );
is( $artist->rank, 42, 'Proper unfiltered value' );
is( $artist->get_column('rank'), 21, 'Proper filtered value' );
}

IC_DIE: {
throws_ok {
DBICTest::Schema::Artist->inflate_column(rank =>
Expand Down
1 change: 1 addition & 0 deletions xt/podcoverage.t
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ my $exceptions = {
store_column
get_column
get_columns
has_column_loaded
/],
},
'DBIx::Class::ResultSource' => {
Expand Down

0 comments on commit dc6dada

Please sign in to comment.