diff --git a/lib/dynamoid/fields.rb b/lib/dynamoid/fields.rb index e06ffda..3c73334 100644 --- a/lib/dynamoid/fields.rb +++ b/lib/dynamoid/fields.rb @@ -10,8 +10,10 @@ module Fields included do class_attribute :attributes class_attribute :range_key + class_attribute :lock_field self.attributes = {} + self.lock_field = :lock_version field :created_at, :datetime field :updated_at, :datetime @@ -42,6 +44,11 @@ def range(name, type = :string) self.range_key = name end + def optimistic_lock_field(name) + field(name, :integer) + self.lock_field = name + end + def table(options) #a default 'id' column is created when Dynamoid::Document is included unless(attributes.has_key? hash_key) @@ -135,4 +142,4 @@ def set_type end -end \ No newline at end of file +end diff --git a/lib/dynamoid/persistence.rb b/lib/dynamoid/persistence.rb index 69bc1e2..412fee4 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,17 +169,18 @@ 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 field 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]) + lock_field = self.class.lock_field + if(self.class.attributes[lock_field]) raise "Optimistic locking cannot be used with Partitioning" if(Dynamoid::Config.partitioning) - t.add(lock_version: 1) + t.add(lock_field => 1) end yield t @@ -257,7 +258,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,20 +266,25 @@ 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 ||= {} (conditions[:unless_exists] ||= []) << self.class.hash_key end - # Add an optimistic locking check if the lock_version column exists - if(self.class.attributes[:lock_version]) + # Add an optimistic locking check if the lock field column exists + lock_field = self.class.lock_field + if(self.class.attributes[lock_field]) conditions ||= {} raise "Optimistic locking cannot be used with Partitioning" if(Dynamoid::Config.partitioning) - 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]) + lock_field_value = self.send(lock_field) + self.send("#{lock_field}=", (lock_field_value || 0) + 1) + # Uses the original lock field value from ActiveModel::Dirty in case + # user changed lock field manually + if(changes[lock_field][0]) + (conditions[:if] ||= {})[lock_field] = changes[lock_field][0] + end end Dynamoid::Adapter.write(self.class.table_name, self.dump, conditions) diff --git a/spec/app/models/team.rb b/spec/app/models/team.rb new file mode 100644 index 0000000..7e1bbda --- /dev/null +++ b/spec/app/models/team.rb @@ -0,0 +1,8 @@ +class Team + include Dynamoid::Document + + field :captain + field :players, :serialized + + optimistic_lock_field :commit_version +end diff --git a/spec/dynamoid/persistence_spec.rb b/spec/dynamoid/persistence_spec.rb index f103a4f..a6f8610 100644 --- a/spec/dynamoid/persistence_spec.rb +++ b/spec/dynamoid/persistence_spec.rb @@ -33,25 +33,37 @@ 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 + + it 'prevents concurrent writes to tables with a custom lock field' do + team = Team.create + a1 = team + a2 = Team.find(team.id) + + a1.captain = 'Cristiano Ronaldo' + a2.captain = 'Lionel Messi' + 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 @@ -256,18 +268,29 @@ 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! end.to raise_error(Dynamoid::Errors::ConditionalCheckFailedException) end + it 'prevents concurrent writes to tables with a custom lock field' do + t1 = Team.create + t2 = Team.find(t1.id) + t2.update! { |t| t.set(:captain => "Cristiano Ronaldo") } + + expect do + t1.captain = "Lionel Messi" + t1.save! + end.to raise_error(Dynamoid::Errors::ConditionalCheckFailedException) + end + end context 'delete' do