diff --git a/lib/UR/Context.pm b/lib/UR/Context.pm index 9f40a33e..c063a749 100644 --- a/lib/UR/Context.pm +++ b/lib/UR/Context.pm @@ -2624,6 +2624,14 @@ sub commit { } $self->__signal_change__('commit',1); + $self->_after_commit(); + + return 1; +} + +sub _after_commit { + my $self = shift; + $_->delete foreach UR::Change->get(); foreach ( $self->all_objects_loaded('UR::Object') ) { @@ -2750,6 +2758,42 @@ sub clear_cache { 1; } +sub _get_changed_objects_for_sync_databases { + my $self = shift; + + return ( + $self->all_objects_loaded('UR::Object::Ghost'), + grep { $_->__changes__ } $self->all_objects_loaded('UR::Object') + #UR::Util->mapreduce_grep(sub { $_[0]->__changes__ },$self->all_objects_loaded('UR::Object')) + ); +} + +sub _change_summary_for_saving_object { + my($self, $object_to_save) = @_; + + # This object may have uncommitted changes already saved. + # If so, work from the last saved data. + # Normally, we go with the last committed data. + my $compare_version = ($object_to_save->{'db_saved_uncommitted'} ? 'db_saved_uncommitted' : 'db_committed'); + + my ($action,$change_summary); + if ($object_to_save->isa('UR::Object::Ghost')) + { + $action = 'delete'; + } + elsif ($object_to_save->{$compare_version}) + { + $action = 'update'; + $change_summary = $object_to_save->property_diff($object_to_save->{$compare_version}); + } + else + { + $action = 'insert'; + } + + return($action, $change_summary); +} + sub _order_data_sources_for_saving { my @data_sources = @_; @@ -2768,7 +2812,6 @@ sub _order_data_sources_for_saving { @data_sources; } - our $IS_SYNCING_DATABASE = 0; sub _sync_databases { my $self = shift; @@ -2797,11 +2840,7 @@ sub _sync_databases { } # Determine what has changed. - my @changed_objects = ( - $self->all_objects_loaded('UR::Object::Ghost'), - grep { $_->__changes__ } $self->all_objects_loaded('UR::Object') - #UR::Util->mapreduce_grep(sub { $_[0]->__changes__ },$self->all_objects_loaded('UR::Object')) - ); + my @changed_objects = $self->_get_changed_objects_for_sync_databases(); return 1 unless (@changed_objects); diff --git a/lib/UR/Context/SyncableTransaction.pm b/lib/UR/Context/SyncableTransaction.pm new file mode 100644 index 00000000..7cace25a --- /dev/null +++ b/lib/UR/Context/SyncableTransaction.pm @@ -0,0 +1,119 @@ +package UR::Context::SyncableTransaction; + +use strict; +use warnings; + +require UR; +our $VERSION = "0.43"; # UR $VERSION + +use Carp; + +UR::Object::Type->define( + class_name => __PACKAGE__, + is => 'UR::Context::Transaction', + has_constant_calculated => [ + _change_summary_data => { is => 'HASH', + doc => 'mapping of class+id to a hash of property name + new value', + calculate => '$self->_build_change_summary_data', + }, + ], +); + +sub _build_change_summary_data { + my $self = shift; + + my @changes = $self->get_changes(); + + my $data = {}; + foreach my $change ( @changes ) { + my($class_name, $id, $aspect) = map { $change->$_ } qw(changed_class_name changed_id changed_aspect); + if ($aspect eq 'create') { + $data->{'++created++'}->{$class_name}->{$id} = undef; + delete $data->{'++deleted++'}->{$id}; + } elsif ($aspect eq 'delete') { + $data->{'++deleted++'}->{$class_name}->{$id} = undef; + delete $data->{'++created++'}->{$id}; + } + + next unless $class_name->__meta__->property_meta_for_name($aspect); + + my $obj = $class_name->get($id); + $data->{$class_name}->{$id}->{$aspect} = $obj->$aspect; + } + return $data; +} + +sub _change_summary_data_for_saving_object { + my($self, $object) = @_; + my $change_data = $self->_change_summary_data(); + + my($class, $id) = ($object->class, $object->id); + + if ($change_data->{'++created++'}->{$class}->{$id}) { + return ('insert', undef); + + } elsif ($change_data->{'++deleted++'}->{$class}->{$id}) { + return ('delete', undef); + + } else { + return ('update', $change_data->{$class}->{$id}); + } +} + +sub commit { + my $self = shift; + + $self->UR::Context::commit(); + $self->SUPER::commit(); + $self->__invalidate_change_summary_data__(); +} + +sub _get_changed_objects_for_sync_databases { + my $self = shift; + my $change_data = $self->_change_summary_data; + + my @objects; + foreach my $class ( keys %$change_data ) { + next if $class =~ m/\+\+/; # skip ++created++ and ++deleted++ + my @ids = keys %{ $change_data->{$class} }; + push @objects, $class->get(\@ids); + } + + my $created = $change_data->{'++created++'}; + foreach my $class ( keys %$created ) { + my @ids = keys %{ $created->{$class} }; + push @objects, $class->get(\@ids); + } + + my $deleted = $change_data->{'++deleted++'}; + foreach my $class ( keys %$deleted ) { + my $ghost_class = $class->ghost_class; + my @ids = keys %{ $deleted->{$class} }; + push @objects, $ghost_class->get(\@ids); + } + + return @objects; +} + +sub _after_commit { + # clean up change_counts for objects + my $self = shift; + + my $change_data = $self->_change_summary_data; + my($created, $deleted) = @$change_data{'++created++', '++deleted++'}; + + foreach my $obj ( $self->_get_changed_objects_for_sync_databases ) { + my($class, $id) = ($obj->class, $obj->id); + if ($created->{$class}->{$id} + or + $deleted->{$class}->{$id} + ) { + delete $obj->{_change_count}; + + } else { + $obj->{_change_count} -= values %{$change_data->{$class}->{$id}}; + } + } +} + +1; diff --git a/lib/UR/Context/Transaction.pm b/lib/UR/Context/Transaction.pm index afba3e2c..89288ff5 100644 --- a/lib/UR/Context/Transaction.pm +++ b/lib/UR/Context/Transaction.pm @@ -62,7 +62,7 @@ sub begin { ); unless ($self) { - Carp::confess("Failed to being transaction!"); + Carp::confess("Failed to begin transaction!"); } push @open_transaction_stack, $self; diff --git a/lib/UR/DataSource/RDBMS.pm b/lib/UR/DataSource/RDBMS.pm index b1243547..e86d1c04 100644 --- a/lib/UR/DataSource/RDBMS.pm +++ b/lib/UR/DataSource/RDBMS.pm @@ -3017,7 +3017,7 @@ sub _lookup_class_for_table_name { sub _default_save_sql_for_object { - my $self = shift; + my $self = shift; my $object_to_save = shift; my %params = @_; @@ -3025,29 +3025,9 @@ sub _default_save_sql_for_object { my $class_object = $object_to_save->__meta__; - # This object may have uncommitted changes already saved. - # If so, work from the last saved data. - # Normally, we go with the last committed data. - - my $compare_version = ($object_to_save->{'db_saved_uncommitted'} ? 'db_saved_uncommitted' : 'db_committed'); - # Determine what the overall save action for the object is, # and get a specific change summary if we're doing an update. - - my ($action,$change_summary); - if ($object_to_save->isa('UR::Object::Ghost')) - { - $action = 'delete'; - } - elsif ($object_to_save->{$compare_version}) - { - $action = 'update'; - $change_summary = $object_to_save->property_diff($object_to_save->{$compare_version}); - } - else - { - $action = 'insert'; - } + my ($action,$change_summary) = UR::Context->current->_change_summary_for_saving_object($object_to_save); # Handle each table. There is usually only one, unless, # there is inheritance within the schema. diff --git a/t/URT/t/99_sync-transaction.t b/t/URT/t/99_sync-transaction.t new file mode 100644 index 00000000..fce32baa --- /dev/null +++ b/t/URT/t/99_sync-transaction.t @@ -0,0 +1,107 @@ +use strict; +use warnings; + +use File::Basename; +use lib File::Basename::dirname(__FILE__)."/../../../lib"; +use lib File::Basename::dirname(__FILE__)."/../.."; +use URT; + +use Test::More tests => 6; +use URT::DataSource::SomeSQLite; + +my $dbh = URT::DataSource::SomeSQLite->get_default_handle; +ok($dbh, 'Got DB handle'); + +my $existing_obj_id = 99; +&setup_classes_and_db(); + +my($created_obj_id, $created_obj_name); +subtest create => sub { + plan tests => 6; + + my $external_obj = URT::NamedThing->create(name => 'external'); + ok($external_obj, 'Create object outside transaction'); + + my $trans = UR::Context::SyncableTransaction->begin(); + ok($trans, 'begin syncable transaction'); + + my $internal_obj = URT::NamedThing->create(name => 'created'); + ok($internal_obj, 'Create object in transaction'); + ($created_obj_id, $created_obj_name) = ($internal_obj->id, $internal_obj->name); + + ok($trans->commit(), 'commit() transaction'); + + my $row = get_row_from_db_with_id($internal_obj->id); + is_deeply($row, + [ $internal_obj->id, $internal_obj->name], + 'Object was saved to DB'); + + $row = get_row_from_db_with_id($external_obj->id); + ok(! $row, 'Object external to transaction was not saved'); +}; + +subtest delete => sub { + plan tests => 5; + + ok(get_row_from_db_with_id($existing_obj_id), 'Object exists in db'); + + my $trans = UR::Context::SyncableTransaction->begin(); + ok($trans, 'begin syncable transaction'); + + my $obj = URT::NamedThing->get($existing_obj_id); + ok($obj->delete, 'delete object'); + + ok($trans->commit(), 'commit transaction'); + ok(! get_row_from_db_with_id($existing_obj_id), 'Object is deleted'); +}; + +subtest change => sub { + plan tests => 6; + + is_deeply(get_row_from_db_with_id($created_obj_id), + [ $created_obj_id, $created_obj_name ], + 'Object previously created and saved is still in DB'); + ok(my $obj = URT::NamedThing->get($created_obj_id), 'Got object previously created and saved'); + + my $trans = UR::Context::SyncableTransaction->begin(); + ok($trans, 'begin syncable transaction'); + + my $altered_name = $created_obj_name . '_foo'; + ok($obj->name($altered_name), 'Change name'); + + ok($trans->commit(), 'Commit transaction'); + + is_deeply(get_row_from_db_with_id($created_obj_id), + [ $created_obj_id, $altered_name ], + 'Name is changed in DB'); +}; + + +sub get_row_from_db_with_id { + my $id = shift; + my $sth = $dbh->prepare('select * from named_thing where named_thing_id = ?'); + $sth->execute($id); + my $row = $sth->fetchrow_arrayref(); + return $row; +} + + +sub setup_classes_and_db { + ok( $dbh->do("create table named_thing (named_thing_id integer PRIMARY KEY, name varchar NOT NULL)"), + 'Created named_thing table'); + + $dbh->do("insert into named_thing values($existing_obj_id, 'bob')"); + ok($dbh->commit(), 'DB commit'); + + UR::Object::Type->define( + class_name => 'URT::NamedThing', + id_by => [ + named_thing_id => { is => 'Integer' }, + ], + has => [ + name => { is => 'String' }, + ], + data_source => 'URT::DataSource::SomeSQLite', + table_name => 'named_thing', + ); +}