[Bps-public-commit] Prophet branch, master, updated. 374af585c21494ad5f4f0d2d12f2855c6eabb702

spang at bestpractical.com spang at bestpractical.com
Mon Aug 10 11:15:45 EDT 2009


The branch, master has been updated
       via  374af585c21494ad5f4f0d2d12f2855c6eabb702 (commit)
      from  2b8ee89bf4792b4ee112ab9b8eab5ca3cc5eb545 (commit)

Summary of changes:
 lib/Prophet/Replica/sqlite.pm           |   56 +++++++++++++++++++++++--------
 t/Settings/t/database-settings-editor.t |    7 ++++
 t/database-settings.t                   |    3 +-
 t/real-conflicting-merge.t              |    5 +++
 t/simple-conflicting-merge.t            |    4 ++
 5 files changed, 60 insertions(+), 15 deletions(-)

- Log -----------------------------------------------------------------
commit 374af585c21494ad5f4f0d2d12f2855c6eabb702
Author: Christine Spang <spang at bestpractical.com>
Date:   Mon Aug 10 16:00:06 2009 +0100

    Some post-Jesse-discussion refactoring of SQLite caching
    
    Go back to using a package variable, but add a level of depth
    to make sure we don't mix caches for handles for different
    DBs.

diff --git a/lib/Prophet/Replica/sqlite.pm b/lib/Prophet/Replica/sqlite.pm
index af10010..61972c3 100644
--- a/lib/Prophet/Replica/sqlite.pm
+++ b/lib/Prophet/Replica/sqlite.pm
@@ -96,11 +96,40 @@ has '+resolution_db_handle' => (
     },
 );
 
-has 'prop_cache' => (
-    is      => 'rw',
-    isa     => 'HashRef',
-    default => sub { {} },
-);
+our $PROP_CACHE = {};
+
+sub has_cached_prop {
+    my $self = shift;
+    my $prop = shift;
+
+    return exists $PROP_CACHE->{$self->db_uuid}->{$prop};
+}
+
+sub fetch_cached_prop {
+    my $self = shift;
+    my $prop = shift;
+
+    return $PROP_CACHE->{$self->db_uuid}->{$prop};
+}
+
+sub set_cached_prop {
+    my $self = shift;
+    my ($prop, $value) = @_;
+
+    $PROP_CACHE->{$self->db_uuid}->{$prop} = $value;
+}
+
+sub delete_cached_prop {
+    my $self = shift;
+    my $prop = shift;
+
+    delete $PROP_CACHE->{$self->db_uuid}->{$prop};
+}
+
+sub clear_prop_cache {
+    my $db_uuid = shift;
+    delete $PROP_CACHE->{$db_uuid};
+}
 
 use constant scheme   => 'sqlite';
 use constant userdata_dir    => 'userdata';
@@ -409,8 +438,7 @@ sub _delete_record_props_from_db {
     my %args = validate( @_, { uuid => 1 } );
 
     $self->dbh->do("DELETE FROM record_props where uuid = ?", {}, $args{uuid});
-    delete $self->prop_cache->{$args{uuid}};
-
+    $self->delete_cached_prop( $args{uuid} );
 }
 
 =head2 traverse_changesets { after => SEQUENCE_NO, UNTIL => SEQUENCE_NO, callback => sub { } } 
@@ -761,10 +789,10 @@ sub set_record_props {
 
     my $inside_edit = $self->current_edit ? 1 : 0;
     $self->begin_edit() unless ($inside_edit);
-   
+
     # clear the cache  before computing the diffs. this is probably paranoid
-    delete $self->prop_cache->{$args{uuid}};
-    
+    $self->delete_cached_prop( $args{uuid} );
+
     my $old_props = $self->get_record_props( uuid => $args{'uuid'}, type => $args{'type'});
     my %new_props = %$old_props;
 
@@ -779,7 +807,7 @@ sub set_record_props {
     $self->_write_record_to_db( type  => $args{'type'}, uuid  => $args{'uuid'}, props => \%new_props);
 
     # Clear the cache now that we've actually written out changed props
-    delete $self->prop_cache->{$args{uuid}};
+    $self->delete_cached_prop( $args{uuid} );
 
     my $change = Prophet::Change->new( {   record_type => $args{'type'}, record_uuid => $args{'uuid'}, change_type => 'update_file' });
     $change->add_prop_change( name => $_, old  => $old_props->{$_}, new  => $args{props}->{$_}) for (keys %{$args{props}});
@@ -793,13 +821,13 @@ sub get_record_props {
     my $self = shift;
     my %args = ( uuid => undef, type => undef, @_ )
         ;    # validate is slooow validate( @_, { uuid => 1, type => 1 } );
-    unless ( exists $self->prop_cache->{ $args{uuid} } ) {
+    unless ( $self->has_cached_prop( $args{uuid} ) ) {
         my $sth = $self->dbh->prepare("SELECT prop, value from record_props WHERE uuid = ?");
         $sth->execute( $args{uuid} );
         my $items = $sth->fetchall_arrayref;
-        $self->prop_cache->{ $args{uuid} } = {map {@$_} @$items};
+        $self->set_cached_prop( $args{uuid}, { map {@$_} @$items } );
     }
-    return $self->prop_cache->{ $args{uuid} };
+    return $self->fetch_cached_prop( $args{uuid} );
 }
 
 sub record_exists {
diff --git a/t/Settings/t/database-settings-editor.t b/t/Settings/t/database-settings-editor.t
index 56035df..02440f5 100644
--- a/t/Settings/t/database-settings-editor.t
+++ b/t/Settings/t/database-settings-editor.t
@@ -61,6 +61,11 @@ is($template_ok, 'ok!', "interactive template was correct");
 # check the settings with settings --show
 $valid_settings_output = Prophet::Util->slurp('t/data/settings-second.tmpl');
 
+# look up db uuid and clear the prop cache, since we've modified the
+# on-disk props via another process
+my ($db_uuid) = (run_settings_command( 'info' ) =~ /DB UUID: (.*)\n/);
+Prophet::Replica::sqlite::clear_prop_cache( $db_uuid );
+
 ($out, my $error) = run_settings_command( qw/settings show/ );
 is( $out, $valid_settings_output, "changed settings output matches" );
 warn "going to print error of settings show";
@@ -78,6 +83,8 @@ run_output_matches( 'settings', [ 'settings', 'edit' ],
     ], [], "interactive settings set with JSON error went ok",
 );
 
+Prophet::Replica::sqlite::clear_prop_cache( $db_uuid );
+
 # check the tempfile to see if the template presented to the editor was correct
 chomp($template_ok = Prophet::Util->slurp($filename));
 is($template_ok, 'ok!', "interactive template was correct");
diff --git a/t/database-settings.t b/t/database-settings.t
index 9848d35..fa2c3bb 100644
--- a/t/database-settings.t
+++ b/t/database-settings.t
@@ -78,7 +78,8 @@ exit;
   
    
     # just for good measure, create a ticket
-    run_ok( 'prophet', [qw(create --type Bug -- --status new --from alice )], "Created a record as alice" );
+    ok( run_command( qw(create --type Bug -- --status new --from alice ) ),
+        'Created a record as alice' );
     run_output_matches( 'prophet', [qw(search --type Bug --regex .)], [qr/new/], [], " Found our record" );
 
 
diff --git a/t/real-conflicting-merge.t b/t/real-conflicting-merge.t
index da2d8c5..254dbfc 100644
--- a/t/real-conflicting-merge.t
+++ b/t/real-conflicting-merge.t
@@ -69,6 +69,11 @@ as_bob {
 
     my $conflict_obj;
 
+    # XXX I am not entirely sure why we need to clear the cache here,
+    # but we do
+    my ($db_uuid) = (run_command( 'info' ) =~ /DB UUID: (.*)\n/);
+    Prophet::Replica::sqlite::clear_prop_cache( $db_uuid );
+
     throws_ok {
         $target->import_changesets( from => $source, force => 1);
     }
diff --git a/t/simple-conflicting-merge.t b/t/simple-conflicting-merge.t
index 08598c3..118a5fe 100644
--- a/t/simple-conflicting-merge.t
+++ b/t/simple-conflicting-merge.t
@@ -56,6 +56,10 @@ as_alice { $alice_app = Prophet::CLI->new()->app_handle; $alice = $alice_app->ha
 
 
 as_alice {
+    # XXX I am not entirely sure why we need to clear the cache here,
+    # but we do
+    my ($db_uuid) = (run_command( 'info' ) =~ /DB UUID: (.*)\n/);
+    Prophet::Replica::sqlite::clear_prop_cache( $db_uuid );
     ok( run_command(
             'update', '--type', 'Bug', '--uuid',
             $record_id, '--', '--status' => 'stalled',

-----------------------------------------------------------------------



More information about the Bps-public-commit mailing list