A number of equivalent-logic ::FC refactors

Stop invoking a comparison on the unfiltered values - delegate it properly
to set_column()

diff --git a/lib/DBIx/Class/FilterColumn.pm b/lib/DBIx/Class/FilterColumn.pm
index c48b6c1..6d7e48c 100644 (file)
--- a/lib/DBIx/Class/FilterColumn.pm
+++ b/lib/DBIx/Class/FilterColumn.pm
@@ -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}
@@ -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 (@_);
 }
@@ -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;
 }

diff --git a/t/row/filter_column.t b/t/row/filter_column.t
index 0ed8edf..fb8dd00 100644 (file)
--- a/t/row/filter_column.t
+++ b/t/row/filter_column.t
@@ -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');
@@ -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 =>

diff --git a/xt/podcoverage.t b/xt/podcoverage.t
index da48580..a16a365 100644 (file)
--- a/xt/podcoverage.t
+++ b/xt/podcoverage.t
@@ -57,6 +57,7 @@ my $exceptions = {
             store_column
             get_column
             get_columns
+            has_column_loaded
         /],
     },
     'DBIx::Class::ResultSource' => {