Fix #save! and raise RecordNotSaved when a callback throws :abort and terminates the process

Author: andrykonchin

lib/dynamoid/persistence.rb

@@ -519,7 +519,8 @@ def persisted?
     # If a model is new and hash key (+id+ by default) is not assigned yet
     # it was assigned implicitly with random UUID value.
     #
-    # If +lock_version+ attribute is declared it will be incremented. If it's blank then it will be initialized with 1.
+    # If +lock_version+ attribute is declared it will be incremented. If it's
+    # blank then it will be initialized with 1.
     #
     # +save+ method call raises +Dynamoid::Errors::RecordNotUnique+ exception
     # if primary key (hash key + optional range key) already exists in a
@@ -553,6 +554,80 @@ def save(options = {})
       end
     end
 
+    # Create new model or persist changes.
+    #
+    # Run the validation and callbacks. Raises
+    # +Dynamoid::Errors::DocumentNotValid+ is validation fails.
+    #
+    #   user = User.create
+    #
+    #   user.age = 26
+    #   user.save! # => user
+    #
+    # Validation can be skipped with +validate: false+ option:
+    #
+    #   user = User.new(age: -1)
+    #   user.save!(validate: false) # => user
+    #
+    # +save!+ by default sets timestamps attributes - +created_at+ and
+    # +updated_at+ when creates new model and updates +updated_at+ attribute
+    # when updates already existing one.
+    #
+    # Changing +updated_at+ attribute at updating a model can be skipped with
+    # +touch: false+ option:
+    #
+    #   user.save!(touch: false)
+    #
+    # If a model is new and hash key (+id+ by default) is not assigned yet
+    # it was assigned implicitly with random UUID value.
+    #
+    # If +lock_version+ attribute is declared it will be incremented. If it's
+    # blank then it will be initialized with 1.
+    #
+    # +save!+ method call raises +Dynamoid::Errors::RecordNotUnique+ exception
+    # if primary key (hash key + optional range key) already exists in a
+    # table.
+    #
+    # +save!+ method call raises +Dynamoid::Errors::StaleObjectError+ exception
+    # if there is +lock_version+ attribute and the document in a table was
+    # already changed concurrently and +lock_version+ was consequently
+    # increased.
+    #
+    # +save!+ method call raises +Dynamoid::Errors::RecordNotSaved+ exception
+    # if some callback aborted execution.
+    #
+    # When a table is not created yet the first +save!+ method call will create
+    # a table. It's useful in test environment to avoid explicit table
+    # creation.
+    #
+    # @param options [Hash] (optional)
+    # @option options [true|false] :validate validate a model or not - +true+ by default (optional)
+    # @option options [true|false] :touch update tiemstamps fields or not - +true+ by default (optional)
+    # @return [true|false] Whether saving successful or not
+    def save!(options = {})
+      # validation is handled in the Validation module
+
+      if Dynamoid.config.create_table_on_save
+        self.class.create_table(sync: true)
+      end
+
+      create_or_update = new_record? ? :create : :update
+      aborted = true
+
+      run_callbacks(:save) do
+        run_callbacks(create_or_update) do
+          aborted = false
+          Save.call(self, touch: options[:touch])
+        end
+      end
+
+      if aborted
+        raise Dynamoid::Errors::RecordNotSaved, self
+      end
+
+      nil
+    end
+
     # Update multiple attributes at once, saving the object once the updates
     # are complete.
     #

lib/dynamoid/transaction_write.rb

@@ -178,6 +178,18 @@ def rollback
     # When a model is not persisted - its id should have unique value.
     # Otherwise a transaction will be rolled back.
     #
+    # There are the following differences between transactional and
+    # non-transactional +#save!+:
+    # - transactional +#save!+ doesn't support the +:touch+ option
+    # - transactional +#save!+ doesn't support +lock_version+ attribute so it
+    #   will not be incremented and will not be checked to detect concurrent
+    #   modification of a model and +Dynamoid::Errors::StaleObjectError+
+    #   exception will not be raised
+    # - transactional +#save!+ doesn't raise +Dynamoid::Errors::RecordNotUnique+
+    #   at saving new model when primary key is already used. A generic
+    #   +Aws::DynamoDB::Errors::TransactionCanceledException+ is raised instead.
+    # - a table isn't created lazily if it doesn't exist yet
+    #
     # @param model [Dynamoid::Document] a model
     # @param options [Hash] (optional)
     # @option options [true|false] :validate validate a model or not - +true+ by default (optional)
@@ -216,6 +228,18 @@ def save!(model, **options)
     # When a model is not persisted - its id should have unique value.
     # Otherwise a transaction will be rolled back.
     #
+    # There are the following differences between transactional and
+    # non-transactional +#save+:
+    # - transactional +#save+ doesn't support the +:touch+ option
+    # - transactional +#save+ doesn't support +lock_version+ attribute so it
+    #   will not be incremented and will not be checked to detect concurrent
+    #   modification of a model and +Dynamoid::Errors::StaleObjectError+
+    #   exception will not be raised
+    # - transactional +#save+ doesn't raise +Dynamoid::Errors::RecordNotUnique+
+    #   at saving new model when primary key is already used. A generic
+    #   +Aws::DynamoDB::Errors::TransactionCanceledException+ is raised instead.
+    # - a table isn't created lazily if it doesn't exist yet
+    #
     # @param model [Dynamoid::Document] a model
     # @param options [Hash] (optional)
     # @option options [true|false] :validate validate a model or not - +true+ by default (optional)
@@ -248,6 +272,17 @@ def save(model, **options)
     #
     # Validates model and runs callbacks.
     #
+    # There are the following differences between transactional and
+    # non-transactional +#create+:
+    # - transactional +#create!+ doesn't support +lock_version+ attribute so it
+    #   will not be incremented and will not be checked to detect concurrent
+    #   modification of a model and +Dynamoid::Errors::StaleObjectError+
+    #   exception will not be raised
+    # - transactional +#create!+ doesn't raise +Dynamoid::Errors::RecordNotUnique+
+    #   at saving new model when primary key is already used. A generic
+    #   +Aws::DynamoDB::Errors::TransactionCanceledException+ is raised instead.
+    # - a table isn't created lazily if it doesn't exist yet
+    #
     # @param model_class [Class] a model class which should be instantiated
     # @param attributes [Hash|Array<Hash>] attributes of a model
     # @param block [Proc] a block to process a model after initialization
@@ -287,6 +322,17 @@ def create!(model_class, attributes = {}, &block)
     #
     # Validates model and runs callbacks.
     #
+    # There are the following differences between transactional and
+    # non-transactional +#create+:
+    # - transactional +#create+ doesn't support +lock_version+ attribute so it
+    #   will not be incremented and will not be checked to detect concurrent
+    #   modification of a model and +Dynamoid::Errors::StaleObjectError+
+    #   exception will not be raised
+    # - transactional +#create+ doesn't raise +Dynamoid::Errors::RecordNotUnique+
+    #   at saving new model when primary key is already used. A generic
+    #   +Aws::DynamoDB::Errors::TransactionCanceledException+ is raised instead.
+    # - a table isn't created lazily if it doesn't exist yet
+    #
     # @param model_class [Class] a model class which should be instantiated
     # @param attributes [Hash|Array<Hash>] attributes of a model
     # @param block [Proc] a block to process a model after initialization

lib/dynamoid/validations.rb

@@ -31,11 +31,12 @@ def valid?(context = nil)
     #
     # @private
     # @since 0.2.0
-    def save!
-      raise Dynamoid::Errors::DocumentNotValid, self unless valid?
+    def save!(options = {})
+      unless valid?
+        raise Dynamoid::Errors::DocumentNotValid, self
+      end
 
-      save(validate: false)
-      self
+      super
     end
 
     def update_attribute(attribute, value)

spec/dynamoid/persistence/save_spec.rb

@@ -619,6 +619,53 @@ def around_save_callback
       end
     end
 
+    context "when a callback aborts saving" do
+      it 'aborts creation if callback throws :abort' do
+        if ActiveSupport.version < Gem::Version.new('5.0')
+          skip "Rails 4.x and below don't support aborting with `throw :abort`"
+        end
+
+        klass = new_class do
+          field :name
+          before_create { throw :abort }
+        end
+        klass.create_table
+        obj = klass.new(name: 'Alex')
+
+        result = nil
+        expect {
+          result = obj.save
+        }.not_to change { klass.count }
+
+        expect(result).to eql false
+        expect(obj).not_to be_persisted
+        expect(obj).to be_changed
+      end
+
+      it 'aborts updating if callback throws :abort' do
+        if ActiveSupport.version < Gem::Version.new('5.0')
+          skip "Rails 4.x and below don't support aborting with `throw :abort`"
+        end
+
+        klass = new_class do
+          field :name
+          before_update { throw :abort }
+        end
+
+        obj = klass.create!(name: 'Alex')
+        obj.name = 'Alex [Updated]'
+
+        result = nil
+        expect {
+          result = obj.save
+        }.not_to change { klass.find(obj.id).name }
+
+        expect(result).to eql false
+        expect(obj).to be_persisted
+        expect(obj).to be_changed
+      end
+    end
+
     context 'not unique primary key' do
       context 'composite key' do
         let(:klass_with_composite_key) do
@@ -848,4 +895,53 @@ def around_save_callback
       end
     end
   end
+
+  describe "#save!" do
+    context "when a callback aborts saving" do
+      it 'aborts creation and raises RecordNotSaved if callback throws :abort' do
+        if ActiveSupport.version < Gem::Version.new('5.0')
+          skip "Rails 4.x and below don't support aborting with `throw :abort`"
+        end
+
+        klass = new_class do
+          field :name
+          before_create { throw :abort }
+        end
+        klass.create_table
+        obj = klass.new(name: 'Alex')
+
+        expect {
+          expect {
+            obj.save!
+          }.to raise_error(Dynamoid::Errors::RecordNotSaved)
+        }.not_to change { klass.count }
+
+        expect(obj).not_to be_persisted
+        expect(obj).to be_changed
+      end
+
+      it 'aborts updating and raises RecordNotSaved if callback throws :abort' do
+        if ActiveSupport.version < Gem::Version.new('5.0')
+          skip "Rails 4.x and below don't support aborting with `throw :abort`"
+        end
+
+        klass = new_class do
+          field :name
+          before_update { throw :abort }
+        end
+
+        obj = klass.create!(name: 'Alex')
+        obj.name = 'Alex [Updated]'
+
+        expect {
+          expect {
+            obj.save!
+          }.to raise_error(Dynamoid::Errors::RecordNotSaved)
+        }.not_to change { klass.count }
+
+        expect(obj).to be_persisted
+        expect(obj).to be_changed
+      end
+    end
+  end
 end