From 964dda7b993aac91f73e97dd86e0993583642432 Mon Sep 17 00:00:00 2001 From: Jorge Date: Thu, 28 May 2015 13:29:45 -0700 Subject: [PATCH] run :save callbacks on update and update! --- lib/dynamoid/persistence.rb | 32 ++++++++++++++++--------------- spec/app/models/camel_case.rb | 18 +++++++++++++---- spec/dynamoid/persistence_spec.rb | 32 ++++++++++++++++++++++++------- 3 files changed, 56 insertions(+), 26 deletions(-) diff --git a/lib/dynamoid/persistence.rb b/lib/dynamoid/persistence.rb index 69bc1e2..765884c 100644 --- a/lib/dynamoid/persistence.rb +++ b/lib/dynamoid/persistence.rb @@ -155,7 +155,7 @@ def persisted? # @since 0.2.0 def save(options = {}) self.class.create_table - + if new_record? conditions = { :unless_exists => [self.class.hash_key]} conditions[:unless_exists] << range_key if(range_key) @@ -169,22 +169,24 @@ def save(options = {}) end # - # update!() will increment the lock_version if the table has the column, but will not check it. Thus, a concurrent save will - # never cause an update! to fail, but an update! may cause a concurrent save to fail. + # update!() will increment the lock_version if the table has the column, but will not check it. Thus, a concurrent save will + # never cause an update! to fail, but an update! may cause a concurrent save to fail. # # def update!(conditions = {}, &block) - run_callbacks(:update) do - options = range_key ? {:range_key => dump_field(self.read_attribute(range_key), self.class.attributes[range_key])} : {} - new_attrs = Dynamoid::Adapter.update_item(self.class.table_name, self.hash_key, options.merge(:conditions => conditions)) do |t| - if(self.class.attributes[:lock_version]) - raise "Optimistic locking cannot be used with Partitioning" if(Dynamoid::Config.partitioning) - t.add(lock_version: 1) + run_callbacks(:save) do + run_callbacks(:update) do + options = range_key ? {:range_key => dump_field(self.read_attribute(range_key), self.class.attributes[range_key])} : {} + new_attrs = Dynamoid::Adapter.update_item(self.class.table_name, self.hash_key, options.merge(:conditions => conditions)) do |t| + if(self.class.attributes[:lock_version]) + raise "Optimistic locking cannot be used with Partitioning" if(Dynamoid::Config.partitioning) + t.add(lock_version: 1) + end + + yield t end - - yield t + load(new_attrs) end - load(new_attrs) end end @@ -257,7 +259,7 @@ def dump_field(value, options) raise ArgumentError, "Unknown type #{options[:type]}" end end - + # Persist the object into the datastore. Assign it an id first if it doesn't have one; then afterwards, # save its indexes. # @@ -265,7 +267,7 @@ def dump_field(value, options) def persist(conditions = nil) run_callbacks(:save) do self.hash_key = SecureRandom.uuid if self.hash_key.nil? || self.hash_key.blank? - + # Add an exists check to prevent overwriting existing records with new ones if(new_record?) conditions ||= {} @@ -276,7 +278,7 @@ def persist(conditions = nil) if(self.class.attributes[:lock_version]) conditions ||= {} raise "Optimistic locking cannot be used with Partitioning" if(Dynamoid::Config.partitioning) - self.lock_version = (lock_version || 0) + 1 + self.lock_version = (lock_version || 0) + 1 #Uses the original lock_version value from ActiveModel::Dirty in case user changed lock_version manually (conditions[:if] ||= {})[:lock_version] = changes[:lock_version][0] if(changes[:lock_version][0]) end diff --git a/spec/app/models/camel_case.rb b/spec/app/models/camel_case.rb index 45e6f21..2fa59d9 100644 --- a/spec/app/models/camel_case.rb +++ b/spec/app/models/camel_case.rb @@ -7,18 +7,20 @@ class CamelCase has_many :users has_one :sponsor has_and_belongs_to_many :subscriptions - + before_create :doing_before_create after_create :doing_after_create before_update :doing_before_update after_update :doing_after_update - + before_save :doing_before_save + after_save :doing_after_save + private - + def doing_before_create true end - + def doing_after_create true end @@ -31,4 +33,12 @@ def doing_after_update true end + def doing_before_save + true + end + + def doing_after_save + true + end + end diff --git a/spec/dynamoid/persistence_spec.rb b/spec/dynamoid/persistence_spec.rb index f103a4f..d6fe45f 100644 --- a/spec/dynamoid/persistence_spec.rb +++ b/spec/dynamoid/persistence_spec.rb @@ -33,25 +33,25 @@ Dynamoid::Adapter.read("dynamoid_tests_addresses", @address.id)[:id].should == @address.id end - + it 'prevents concurrent writes to tables with a lock_version' do @address.save! a1 = @address a2 = Address.find(@address.id) - + a1.city = 'Seattle' a2.city = 'San Francisco' - + a1.save! expect { a2.save! }.to raise_exception(Dynamoid::Errors::ConditionalCheckFailedException) end - + configured_with 'partitioning' do it 'raises an error when attempting to use optimistic locking' do expect { address.save! }.to raise_exception end end - + it 'assigns itself an id on save only if it does not have one' do @address.id = 'test123' @address.save @@ -227,6 +227,24 @@ end end + it 'runs before_save callbacks when doing #update' do + instance = CamelCase.create(:color => 'blue') + instance.expects(:doing_before_save).once.returns(true) + + instance.update do |t| + t.set(:color => 'red') + end + end + + it 'runs after_save callbacks when doing #update' do + instance = CamelCase.create(:color => 'blue') + instance.expects(:doing_after_save).once.returns(true) + + instance.update do |t| + t.set(:color => 'red') + end + end + it 'support add/delete operation on a field' do @tweet.update do |t| t.add(:count => 3) @@ -256,12 +274,12 @@ end }.to raise_error(Dynamoid::Errors::ConditionalCheckFailedException) end - + it 'prevents concurrent saves to tables with a lock_version' do @address.save! a2 = Address.find(@address.id) a2.update! { |a| a.set(:city => "Chicago") } - + expect do @address.city = "Seattle" @address.save!