Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

run :save callbacks on update and update! #193

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 17 additions & 15 deletions lib/dynamoid/persistence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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

Expand Down Expand Up @@ -257,15 +259,15 @@ 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.
#
# @since 0.2.0
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 ||= {}
Expand All @@ -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
Expand Down
18 changes: 14 additions & 4 deletions spec/app/models/camel_case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -31,4 +33,12 @@ def doing_after_update
true
end

def doing_before_save
true
end

def doing_after_save
true
end

end
32 changes: 25 additions & 7 deletions spec/dynamoid/persistence_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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!
Expand Down