From 7bc471d400d50febf8e3e1c3c03120a580c4c9e0 Mon Sep 17 00:00:00 2001 From: Anthony Brummett Date: Thu, 19 Feb 2015 16:15:37 -0600 Subject: [PATCH 1/8] typo --- lib/UR/Context/Transaction.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/UR/Context/Transaction.pm b/lib/UR/Context/Transaction.pm index 56e71ae9..0010cc58 100644 --- a/lib/UR/Context/Transaction.pm +++ b/lib/UR/Context/Transaction.pm @@ -51,7 +51,7 @@ sub begin { ); unless ($self) { - Carp::confess("Failed to being transaction!"); + Carp::confess("Failed to begin transaction!"); } push @open_transaction_stack, $self; From 7c976f18978837ea4b2595a17522ff65e261963a Mon Sep 17 00:00:00 2001 From: Anthony Brummett Date: Thu, 19 Feb 2015 16:17:44 -0600 Subject: [PATCH 2/8] Extract method to determine changes for a single object A transaction-centric commit() will override this to work off of the change log --- lib/UR/Context.pm | 25 +++++++++++++++++++++++++ lib/UR/DataSource/RDBMS.pm | 24 ++---------------------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/lib/UR/Context.pm b/lib/UR/Context.pm index 5cbe35c9..2ae54dfb 100644 --- a/lib/UR/Context.pm +++ b/lib/UR/Context.pm @@ -2681,6 +2681,31 @@ sub clear_cache { 1; } +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); +} our $IS_SYNCING_DATABASE = 0; sub _sync_databases { diff --git a/lib/UR/DataSource/RDBMS.pm b/lib/UR/DataSource/RDBMS.pm index df883a28..678123af 100644 --- a/lib/UR/DataSource/RDBMS.pm +++ b/lib/UR/DataSource/RDBMS.pm @@ -2981,7 +2981,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 = @_; @@ -2989,29 +2989,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. From 94df3477be47e61e086ab6fafe87c25730633390 Mon Sep 17 00:00:00 2001 From: Anthony Brummett Date: Thu, 19 Feb 2015 16:41:51 -0600 Subject: [PATCH 3/8] Extract method to perform post-commit cleanup syncable transaction will want to do its own thing to fix up object's change_count --- lib/UR/Context.pm | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/UR/Context.pm b/lib/UR/Context.pm index 2ae54dfb..8540d536 100644 --- a/lib/UR/Context.pm +++ b/lib/UR/Context.pm @@ -2557,6 +2557,14 @@ sub commit { } $self->__signal_change__('commit',1); + $self->_after_commit(); + + return 1; +} + +sub _after_commit { + my $self = shift; + foreach ( $self->all_objects_loaded('UR::Object') ) { delete $_->{'_change_count'}; } From af8b00212cb488bc1e3e8b3c479c2b948e711215 Mon Sep 17 00:00:00 2001 From: Anthony Brummett Date: Thu, 19 Feb 2015 17:25:10 -0600 Subject: [PATCH 4/8] extract method to determine the list of changed objects to save --- lib/UR/Context.pm | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/UR/Context.pm b/lib/UR/Context.pm index 8540d536..ef6b3062 100644 --- a/lib/UR/Context.pm +++ b/lib/UR/Context.pm @@ -2689,6 +2689,16 @@ 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) = @_; @@ -2743,11 +2753,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); From a057a61524ba6de4064cae0212c142683e420d6b Mon Sep 17 00:00:00 2001 From: Anthony Brummett Date: Thu, 19 Feb 2015 17:48:02 -0600 Subject: [PATCH 5/8] Syncing a transaction to the DB is minimally working Testing by creating an object and saving it --- lib/UR/Context/SyncableTransaction.pm | 120 ++++++++++++++++++++++++++ t/URT/t/99_sync-transaction.t | 68 +++++++++++++++ 2 files changed, 188 insertions(+) create mode 100644 lib/UR/Context/SyncableTransaction.pm create mode 100644 t/URT/t/99_sync-transaction.t diff --git a/lib/UR/Context/SyncableTransaction.pm b/lib/UR/Context/SyncableTransaction.pm new file mode 100644 index 00000000..6f6ad81d --- /dev/null +++ b/lib/UR/Context/SyncableTransaction.pm @@ -0,0 +1,120 @@ +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 ) { +$DB::single=1; + 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/t/URT/t/99_sync-transaction.t b/t/URT/t/99_sync-transaction.t new file mode 100644 index 00000000..77056d1a --- /dev/null +++ b/t/URT/t/99_sync-transaction.t @@ -0,0 +1,68 @@ +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 => 4; +use URT::DataSource::SomeSQLite; + +my $dbh = URT::DataSource::SomeSQLite->get_default_handle; +ok($dbh, 'Got DB handle'); + +&setup_classes_and_db(); + +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'); + + 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'); +}; + + +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(99, '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', + ); +} From b9281b85fb4f5dc893999fe3012a305c31e6d209 Mon Sep 17 00:00:00 2001 From: Anthony Brummett Date: Thu, 19 Feb 2015 17:54:37 -0600 Subject: [PATCH 6/8] add test to deleting --- t/URT/t/99_sync-transaction.t | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/t/URT/t/99_sync-transaction.t b/t/URT/t/99_sync-transaction.t index 77056d1a..b62419a1 100644 --- a/t/URT/t/99_sync-transaction.t +++ b/t/URT/t/99_sync-transaction.t @@ -6,12 +6,13 @@ use lib File::Basename::dirname(__FILE__)."/../../../lib"; use lib File::Basename::dirname(__FILE__)."/../.."; use URT; -use Test::More tests => 4; +use Test::More tests => 5; 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(); subtest create => sub { @@ -37,6 +38,20 @@ subtest create => sub { 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'); +}; sub get_row_from_db_with_id { my $id = shift; @@ -51,7 +66,7 @@ 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(99, 'bob')"); + $dbh->do("insert into named_thing values($existing_obj_id, 'bob')"); ok($dbh->commit(), 'DB commit'); UR::Object::Type->define( From 18f5fa4a09e6c02c006006d8d2737507feaf5a9c Mon Sep 17 00:00:00 2001 From: Anthony Brummett Date: Thu, 19 Feb 2015 17:59:45 -0600 Subject: [PATCH 7/8] test changing data --- t/URT/t/99_sync-transaction.t | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/t/URT/t/99_sync-transaction.t b/t/URT/t/99_sync-transaction.t index b62419a1..fce32baa 100644 --- a/t/URT/t/99_sync-transaction.t +++ b/t/URT/t/99_sync-transaction.t @@ -6,7 +6,7 @@ use lib File::Basename::dirname(__FILE__)."/../../../lib"; use lib File::Basename::dirname(__FILE__)."/../.."; use URT; -use Test::More tests => 5; +use Test::More tests => 6; use URT::DataSource::SomeSQLite; my $dbh = URT::DataSource::SomeSQLite->get_default_handle; @@ -15,6 +15,7 @@ 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; @@ -26,6 +27,7 @@ subtest create => sub { 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'); @@ -53,6 +55,28 @@ subtest delete => sub { 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 = ?'); From 498fee585b4c22eff3798f1b84c0934a181a498a Mon Sep 17 00:00:00 2001 From: Anthony Brummett Date: Fri, 20 Feb 2015 14:12:38 -0600 Subject: [PATCH 8/8] remove breakpoint --- lib/UR/Context/SyncableTransaction.pm | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/UR/Context/SyncableTransaction.pm b/lib/UR/Context/SyncableTransaction.pm index 6f6ad81d..7cace25a 100644 --- a/lib/UR/Context/SyncableTransaction.pm +++ b/lib/UR/Context/SyncableTransaction.pm @@ -81,7 +81,6 @@ sub _get_changed_objects_for_sync_databases { my $created = $change_data->{'++created++'}; foreach my $class ( keys %$created ) { -$DB::single=1; my @ids = keys %{ $created->{$class} }; push @objects, $class->get(\@ids); }