diff --git a/.github/workflows/style.yml b/.github/workflows/style.yml index 0478f11d..8e7a5fc4 100644 --- a/.github/workflows/style.yml +++ b/.github/workflows/style.yml @@ -37,5 +37,6 @@ jobs: rubygems: ${{ matrix.rubygems }} bundler: ${{ matrix.bundler }} bundler-cache: true + cache-version: v2 - name: Run Rubocop run: bundle exec rubocop -DESP diff --git a/.rubocop.yml b/.rubocop.yml index e64f8264..8c70e917 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -65,6 +65,9 @@ Style/TrailingCommaInArguments: Enabled: false Style/UnlessElse: Enabled: false +Style/OneClassPerFile: + Exclude: + - 'README.md' # We aren't so brave to tackle all these issues right now Layout/LineLength: diff --git a/.rubocop_rspec.yml b/.rubocop_rspec.yml index 3025e4a0..345f9611 100644 --- a/.rubocop_rspec.yml +++ b/.rubocop_rspec.yml @@ -48,3 +48,7 @@ RSpec/StubbedMock: RSpec/IndexedLet: Enabled: false + +# Disable while callbacks specs use printing into output +RSpec/Output: + Enabled: false diff --git a/Gemfile.lock b/Gemfile.lock index 99b88001..d0754297 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -9,9 +9,9 @@ PATH GEM remote: https://rubygems.org/ specs: - activemodel (8.1.0) - activesupport (= 8.1.0) - activesupport (8.1.0) + activemodel (8.1.2) + activesupport (= 8.1.2) + activesupport (8.1.2) base64 bigdecimal concurrent-ruby (~> 1.0, >= 1.3.1) @@ -24,14 +24,16 @@ GEM securerandom (>= 0.3) tzinfo (~> 2.0, >= 2.0.5) uri (>= 0.13.1) + addressable (2.8.9) + public_suffix (>= 2.0.2, < 8.0) appraisal (2.5.0) bundler rake thor (>= 0.14.0) ast (2.4.3) aws-eventstream (1.4.0) - aws-partitions (1.1176.0) - aws-sdk-core (3.234.0) + aws-partitions (1.1221.0) + aws-sdk-core (3.242.0) aws-eventstream (~> 1, >= 1.3.0) aws-partitions (~> 1, >= 1.992.0) aws-sigv4 (~> 1.9) @@ -39,53 +41,66 @@ GEM bigdecimal jmespath (~> 1, >= 1.6.1) logger - aws-sdk-dynamodb (1.155.0) - aws-sdk-core (~> 3, >= 3.234.0) + aws-sdk-dynamodb (1.163.0) + aws-sdk-core (~> 3, >= 3.241.4) aws-sigv4 (~> 1.5) aws-sigv4 (1.12.1) aws-eventstream (~> 1, >= 1.0.2) base64 (0.3.0) - bigdecimal (3.3.1) - byebug (12.0.0) + bigdecimal (4.0.1) + byebug (13.0.0) + reline (>= 0.6.0) childprocess (5.1.0) logger (~> 1.5) codecov (0.6.0) simplecov (>= 0.15, < 0.22) coderay (1.1.3) - concurrent-ruby (1.3.5) - connection_pool (2.5.4) + concurrent-ruby (1.3.6) + connection_pool (3.0.2) diff-lcs (1.6.2) docile (1.4.1) drb (2.2.3) - i18n (1.14.7) + i18n (1.14.8) concurrent-ruby (~> 1.0) iniparse (1.5.0) + io-console (0.8.2) jmespath (1.6.2) - json (2.15.1) + json (2.18.1) + json-schema (6.1.0) + addressable (~> 2.8) + bigdecimal (>= 3.1, < 5) language_server-protocol (3.17.0.5) lint_roller (1.1.0) logger (1.7.0) + mcp (0.8.0) + json-schema (>= 4.1) method_source (1.1.0) - minitest (5.26.0) + minitest (6.0.2) + drb (~> 2.0) + prism (~> 1.5) overcommit (0.68.0) childprocess (>= 0.6.3, < 6) iniparse (~> 1.4) rexml (>= 3.3.9) parallel (1.27.0) - parser (3.3.9.0) + parser (3.3.10.2) ast (~> 2.4.1) racc - prism (1.6.0) - pry (0.15.2) + prism (1.9.0) + pry (0.16.0) coderay (~> 1.1) method_source (~> 1.0) - pry-byebug (3.11.0) - byebug (~> 12.0) - pry (>= 0.13, < 0.16) + reline (>= 0.6.0) + pry-byebug (3.12.0) + byebug (~> 13.0) + pry (>= 0.13, < 0.17) + public_suffix (7.0.5) racc (1.8.1) rainbow (3.1.1) - rake (13.3.0) + rake (13.3.1) regexp_parser (2.11.3) + reline (0.6.3) + io-console (~> 0.5) rexml (3.4.4) rspec (3.13.2) rspec-core (~> 3.13.0) @@ -96,25 +111,26 @@ GEM rspec-expectations (3.13.5) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.13.0) - rspec-mocks (3.13.6) + rspec-mocks (3.13.8) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.13.0) - rspec-support (3.13.6) - rubocop (1.81.6) + rspec-support (3.13.7) + rubocop (1.85.1) json (~> 2.3) language_server-protocol (~> 3.17.0.2) lint_roller (~> 1.1.0) + mcp (~> 0.6) parallel (~> 1.10) parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 2.9.3, < 3.0) - rubocop-ast (>= 1.47.1, < 2.0) + rubocop-ast (>= 1.49.0, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 4.0) - rubocop-ast (1.47.1) + rubocop-ast (1.49.0) parser (>= 3.3.7.2) - prism (~> 1.4) - rubocop-md (2.0.3) + prism (~> 1.7) + rubocop-md (2.0.4) lint_roller (~> 1.1) rubocop (>= 1.72.1) rubocop-packaging (0.6.0) @@ -127,9 +143,9 @@ GEM rubocop-rake (0.7.1) lint_roller (~> 1.1) rubocop (>= 1.72.1) - rubocop-rspec (3.7.0) + rubocop-rspec (3.9.0) lint_roller (~> 1.1) - rubocop (~> 1.72, >= 1.72.1) + rubocop (~> 1.81) rubocop-thread_safety (0.7.3) lint_roller (~> 1.1) rubocop (~> 1.72, >= 1.72.1) @@ -149,18 +165,18 @@ GEM simplecov simplecov-lcov (0.9.0) simplecov_json_formatter (0.1.4) - thor (1.4.0) + thor (1.5.0) tzinfo (2.0.6) concurrent-ruby (~> 1.0) unicode-display_width (3.2.0) unicode-emoji (~> 4.1) - unicode-emoji (4.1.0) - uri (1.0.4) - yard (0.9.37) + unicode-emoji (4.2.0) + uri (1.1.1) + yard (0.9.38) PLATFORMS ruby - x86_64-linux + x86_64-darwin-24 DEPENDENCIES appraisal diff --git a/lib/dynamoid/adapter_plugin/aws_sdk_v3/item_updater.rb b/lib/dynamoid/adapter_plugin/aws_sdk_v3/item_updater.rb index 54284c74..40c746cc 100644 --- a/lib/dynamoid/adapter_plugin/aws_sdk_v3/item_updater.rb +++ b/lib/dynamoid/adapter_plugin/aws_sdk_v3/item_updater.rb @@ -58,7 +58,7 @@ def set(values) # delete explicitly attributes if assigned nil value and configured # to not store nil values values_to_update = values_sanitized.reject { |_, v| v.nil? } - values_to_delete = values_sanitized.select { |_, v| v.nil? } + values_to_delete = values_sanitized.select { |_, v| v.nil? } # rubocop:disable Style/PartitionInsteadOfDoubleSelect @updates.merge!(values_to_update) @deletions.merge!(values_to_delete) diff --git a/lib/dynamoid/fields/declare.rb b/lib/dynamoid/fields/declare.rb index e8aaae42..2eca407c 100644 --- a/lib/dynamoid/fields/declare.rb +++ b/lib/dynamoid/fields/declare.rb @@ -77,7 +77,7 @@ def generate_instance_methods_for_alias end def warn_if_method_exists(method) - if @source.instance_methods.include?(method.to_sym) + if @source.method_defined?(method.to_sym) Dynamoid.logger.warn("Method #{method} generated for the field #{@name} overrides already existing method") end end diff --git a/lib/dynamoid/transaction_write/destroy.rb b/lib/dynamoid/transaction_write/destroy.rb index 54b02b2c..28babe74 100644 --- a/lib/dynamoid/transaction_write/destroy.rb +++ b/lib/dynamoid/transaction_write/destroy.rb @@ -15,10 +15,10 @@ def initialize(model, **options) end def on_registration - validate_model! - @aborted = true @model.run_callbacks(:destroy) do + validate_primary_key! + @aborted = false true end @@ -70,7 +70,7 @@ def action_request private - def validate_model! + def validate_primary_key! raise Dynamoid::Errors::MissingHashKey if @model.hash_key.nil? raise Dynamoid::Errors::MissingRangeKey if @model_class.range_key? && @model.range_value.nil? end diff --git a/lib/dynamoid/transaction_write/save.rb b/lib/dynamoid/transaction_write/save.rb index 0039cb75..cf4e11fc 100644 --- a/lib/dynamoid/transaction_write/save.rb +++ b/lib/dynamoid/transaction_write/save.rb @@ -18,8 +18,6 @@ def initialize(model, **options) end def on_registration - validate_model! - if @options[:validate] != false && !(@valid = @model.valid?) if @options[:raise_error] raise Dynamoid::Errors::DocumentNotValid, @model @@ -35,6 +33,8 @@ def on_registration @model.run_callbacks(:save) do @model.run_callbacks(callback_name) do @model.run_callbacks(:validate) do + validate_primary_key! + @aborted = false true end @@ -88,7 +88,7 @@ def action_request private - def validate_model! + def validate_primary_key! raise Dynamoid::Errors::MissingHashKey if !@was_new_record && @model.hash_key.nil? raise Dynamoid::Errors::MissingRangeKey if @model_class.range_key? && @model.range_value.nil? end diff --git a/lib/dynamoid/transaction_write/update_fields.rb b/lib/dynamoid/transaction_write/update_fields.rb index 1af00b4a..f8cb7f30 100644 --- a/lib/dynamoid/transaction_write/update_fields.rb +++ b/lib/dynamoid/transaction_write/update_fields.rb @@ -58,7 +58,7 @@ def action_request builder.set_attributes(changes_dumped) else nil_attributes = changes_dumped.select { |_, v| v.nil? } - non_nil_attributes = changes_dumped.reject { |_, v| v.nil? } + non_nil_attributes = changes_dumped.reject { |_, v| v.nil? } # rubocop:disable Style/PartitionInsteadOfDoubleSelect builder.remove_attributes(nil_attributes.keys) builder.set_attributes(non_nil_attributes) diff --git a/spec/dynamoid/adapter_plugin/aws_sdk_v3_spec.rb b/spec/dynamoid/adapter_plugin/aws_sdk_v3_spec.rb index ddf637b9..66e31354 100644 --- a/spec/dynamoid/adapter_plugin/aws_sdk_v3_spec.rb +++ b/spec/dynamoid/adapter_plugin/aws_sdk_v3_spec.rb @@ -16,7 +16,7 @@ 4 => [:id, { range_key: { range: :number } }], 5 => [:id, { read_capacity: 10_000, write_capacity: 1000 }] }.each do |n, args| - name = "dynamoid_tests_TestTable#{n}" + name = "dynamoid_tests_TestTable#{n}" # rubocop:disable RSpec/LeakyLocalVariable let(:"test_table#{n}") do Dynamoid.adapter.create_table(name, *args) name @@ -987,14 +987,14 @@ def dynamo_request(table_name, conditions = [], options = {}) it 'performs query on a table and returns items' do Dynamoid.adapter.put_item(test_table1, id: '1', name: 'Josh') - expect(Dynamoid.adapter.query(test_table1, { id: [[:eq, '1']] }).first).to eq([[id: '1', name: 'Josh'], { last_evaluated_key: nil }]) + expect(Dynamoid.adapter.query(test_table1, { id: [[:eq, '1']] }).first).to eq([[{ id: '1', name: 'Josh' }], { last_evaluated_key: nil }]) end it 'performs query on a table and returns items if there are multiple items' do Dynamoid.adapter.put_item(test_table1, id: '1', name: 'Josh') Dynamoid.adapter.put_item(test_table1, id: '2', name: 'Justin') - expect(Dynamoid.adapter.query(test_table1, { id: [[:eq, '1']] }).first).to eq([[id: '1', name: 'Josh'], { last_evaluated_key: nil }]) + expect(Dynamoid.adapter.query(test_table1, { id: [[:eq, '1']] }).first).to eq([[{ id: '1', name: 'Josh' }], { last_evaluated_key: nil }]) end context 'backoff is specified' do @@ -1031,14 +1031,14 @@ def dynamo_request(table_name, conditions = [], options = {}) it 'performs scan on a table and returns items' do Dynamoid.adapter.put_item(test_table1, id: '1', name: 'Josh') - expect(Dynamoid.adapter.scan(test_table1, [name: { eq: 'Josh' }]).to_a).to eq [[[{ id: '1', name: 'Josh' }], { last_evaluated_key: nil }]] + expect(Dynamoid.adapter.scan(test_table1, [{ name: { eq: 'Josh' } }]).to_a).to eq [[[{ id: '1', name: 'Josh' }], { last_evaluated_key: nil }]] end it 'performs scan on a table and returns items if there are multiple items but only one match' do Dynamoid.adapter.put_item(test_table1, id: '1', name: 'Josh') Dynamoid.adapter.put_item(test_table1, id: '2', name: 'Justin') - expect(Dynamoid.adapter.scan(test_table1, [name: { eq: 'Josh' }]).to_a).to eq [[[{ id: '1', name: 'Josh' }], { last_evaluated_key: nil }]] + expect(Dynamoid.adapter.scan(test_table1, [{ name: { eq: 'Josh' } }]).to_a).to eq [[[{ id: '1', name: 'Josh' }], { last_evaluated_key: nil }]] end it 'performs scan on a table and returns multiple items if there are multiple matches' do @@ -1046,7 +1046,7 @@ def dynamo_request(table_name, conditions = [], options = {}) Dynamoid.adapter.put_item(test_table1, id: '2', name: 'Josh') expect( - Dynamoid.adapter.scan(test_table1, [name: { eq: 'Josh' }]).to_a + Dynamoid.adapter.scan(test_table1, [{ name: { eq: 'Josh' } }]).to_a ).to match( [ [ diff --git a/spec/dynamoid/adapter_spec.rb b/spec/dynamoid/adapter_spec.rb index 12c5302c..263959c1 100644 --- a/spec/dynamoid/adapter_spec.rb +++ b/spec/dynamoid/adapter_spec.rb @@ -17,7 +17,7 @@ def test_table 3 => [:id, { range_key: { range: :number } }], 4 => [:id, { range_key: { range: :number } }] }.each do |n, args| - name = "dynamoid_tests_TestTable#{n}" + name = "dynamoid_tests_TestTable#{n}" # rubocop:disable RSpec/LeakyLocalVariable let(:"test_table#{n}") do Dynamoid.adapter.create_table(name, *args) name diff --git a/spec/dynamoid/persistence/destroy_spec.rb b/spec/dynamoid/persistence/destroy_spec.rb index 6c597ecf..fceba261 100644 --- a/spec/dynamoid/persistence/destroy_spec.rb +++ b/spec/dynamoid/persistence/destroy_spec.rb @@ -308,6 +308,32 @@ def around_destroy_callback expect { obj.destroy }.to output('start around_destroyfinish around_destroy').to_stdout end + it 'runs callbacks in the proper order' do + klass_with_all_callbacks = new_class do + before_destroy { puts 'run before_destroy' } + after_destroy { puts 'run after_destroy' } + around_destroy :around_destroy_callback + + def around_destroy_callback + puts 'start around_destroy' + yield + puts 'finish around_destroy' + end + end + + # print each message on new line to force RSpec to show meaningful diff + expected_output = [ # rubocop:disable Style/StringConcatenation + 'run before_destroy', + 'start around_destroy', + 'finish around_destroy', + 'run after_destroy', + ].join("\n") + "\n" + + obj = klass_with_all_callbacks.create! + + expect { obj.destroy }.to output(expected_output).to_stdout + end + it 'aborts destroying and returns false if a before_destroy 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`" @@ -326,6 +352,37 @@ def around_destroy_callback expect(result).to eql false expect(obj.destroyed?).to eql nil end + + it 'runs before and around callbacks before validating primary key' do + klass_with_callbacks_and_composite_key = new_class do + range :name + + before_destroy { puts 'run before_destroy' } + after_destroy { puts 'run after_destroy' } + around_destroy :around_destroy_callback + + def around_destroy_callback + puts 'start around_destroy' + yield + puts 'finish around_destroy' + end + end + + obj = klass_with_callbacks_and_composite_key.create!(name: 'John') + obj.name = nil + + # print each message on new line to force RSpec to show meaningful diff + expected_output = [ # rubocop:disable Style/StringConcatenation + 'run before_destroy', + 'start around_destroy', + ].join("\n") + "\n" + + expect { + expect { + obj.destroy + }.to raise_exception(Dynamoid::Errors::MissingRangeKey) + }.to output(expected_output).to_stdout + end end context 'when table arn is specified', remove_constants: [:Payment] do diff --git a/spec/dynamoid/persistence/import_spec.rb b/spec/dynamoid/persistence/import_spec.rb index ff59e347..9f7cb37c 100644 --- a/spec/dynamoid/persistence/import_spec.rb +++ b/spec/dynamoid/persistence/import_spec.rb @@ -53,7 +53,7 @@ end it 'supports empty containers in `serialized` fields' do - users = User.import([name: 'Philip', favorite_colors: Set.new]) + users = User.import([{ name: 'Philip', favorite_colors: Set.new }]) user = User.find(users[0].id) expect(user.favorite_colors).to eq Set.new @@ -268,7 +268,7 @@ it 'works well with hash keys of any type' do a = nil expect { - a, = klass.import([hash: { 1 => :b }]) + a, = klass.import([{ hash: { 1 => :b } }]) }.not_to raise_error expect(klass.find(a.id)[:hash]).to eql('1': 'b') diff --git a/spec/dynamoid/persistence/save_spec.rb b/spec/dynamoid/persistence/save_spec.rb index 56058d1f..5ecd5747 100644 --- a/spec/dynamoid/persistence/save_spec.rb +++ b/spec/dynamoid/persistence/save_spec.rb @@ -425,6 +425,8 @@ def around_create_callback it 'runs callbacks in the proper order' do klass_with_callbacks = new_class do + field :name + before_validation { puts 'run before_validation' } after_validation { puts 'run after_validation' } @@ -432,6 +434,10 @@ def around_create_callback after_create { puts 'run after_create' } around_create :around_create_callback + before_update { puts 'run before_update' } + after_update { puts 'run after_update' } + around_update :around_update_callback + before_save { puts 'run before_save' } after_save { puts 'run after_save' } around_save :around_save_callback @@ -442,6 +448,12 @@ def around_create_callback puts 'finish around_create' end + def around_update_callback + puts 'start around_update' + yield + puts 'finish around_update' + end + def around_save_callback puts 'start around_save' yield @@ -466,6 +478,50 @@ def around_save_callback expect { obj.save }.to output(expected_output).to_stdout end + + it 'runs before and around callbacks before validating primary key' do + klass_with_callbacks_and_composite_key = new_class do + range :name + + before_validation { puts 'run before_validation' } + after_validation { puts 'run after_validation' } + + before_create { puts 'run before_create' } + after_create { puts 'run after_create' } + around_create :around_create_callback + + before_save { puts 'run before_save' } + after_save { puts 'run after_save' } + around_save :around_save_callback + + def around_create_callback + puts 'start around_create' + yield + puts 'finish around_create' + end + + def around_save_callback + puts 'start around_save' + yield + puts 'finish around_save' + end + end + obj = klass_with_callbacks_and_composite_key.new(name: nil) + + # print each message on new line to force RSpec to show meaningful diff + expected_output = [ # rubocop:disable Style/StringConcatenation + 'run before_validation', + 'run after_validation', + 'run before_save', + 'start around_save', + 'run before_create', + 'start around_create', + ].join("\n") + "\n" + + expect { + expect { obj.save }.to raise_exception(Dynamoid::Errors::MissingRangeKey) + }.to output(expected_output).to_stdout + end end context 'persisted model' do @@ -516,6 +572,10 @@ def around_update_callback before_validation { puts 'run before_validation' } after_validation { puts 'run after_validation' } + before_create { puts 'run before_create' } + after_create { puts 'run after_create' } + around_create :around_create_callback + before_update { puts 'run before_update' } after_update { puts 'run after_update' } around_update :around_update_callback @@ -524,6 +584,12 @@ def around_update_callback after_save { puts 'run after_save' } around_save :around_save_callback + def around_create_callback + puts 'start around_create' + yield + puts 'finish around_create' + end + def around_update_callback puts 'start around_update' yield @@ -558,6 +624,54 @@ def around_save_callback expect { obj.save }.to output(expected_output).to_stdout }.to output.to_stdout end + + it 'runs before and around callbacks before validating primary key' do + klass_with_callbacks_and_composite_key = new_class do + range :name + + before_validation { puts 'run before_validation' } + after_validation { puts 'run after_validation' } + + before_update { puts 'run before_update' } + after_update { puts 'run after_update' } + around_update :around_update_callback + + before_save { puts 'run before_save' } + after_save { puts 'run after_save' } + around_save :around_save_callback + + def around_update_callback + puts 'start around_update' + yield + puts 'finish around_update' + end + + def around_save_callback + puts 'start around_save' + yield + puts 'finish around_save' + end + end + + expect { # to suppress printing at model creation + obj = klass_with_callbacks_and_composite_key.create!(name: 'John') + obj.name = nil + + # print each message on new line to force RSpec to show meaningful diff + expected_output = [ # rubocop:disable Style/StringConcatenation + 'run before_validation', + 'run after_validation', + 'run before_save', + 'start around_save', + 'run before_update', + 'start around_update', + ].join("\n") + "\n" + + expect { + expect { obj.save }.to raise_exception(Dynamoid::Errors::MissingRangeKey) + }.to output(expected_output).to_stdout + }.to output.to_stdout + end end it 'runs before_save callback' do @@ -615,6 +729,49 @@ def around_save_callback obj = klass_with_callback.new(name: 'Alex') expect { obj.save }.to output('run after_validation').to_stdout end + + it 'runs only validation callbacks and skips all the other when validation fails' do + klass_with_callbacks = new_class do + field :name + + validates :name, presence: true + + before_validation { puts 'run before_validation' } + after_validation { puts 'run after_validation' } + + before_create { puts 'run before_create' } + after_create { puts 'run after_create' } + around_create :around_create_callback + + before_save { puts 'run before_save' } + after_save { puts 'run after_save' } + around_save :around_save_callback + + def around_create_callback + puts 'start around_create' + yield + puts 'finish around_create' + end + + def around_save_callback + puts 'start around_save' + yield + puts 'finish around_save' + end + end + klass_with_callbacks.create_table + obj = klass_with_callbacks.new(name: '') + + # print each message on new line to force RSpec to show meaningful diff + expected_output = [ # rubocop:disable Style/StringConcatenation + 'run before_validation', + 'run after_validation', + ].join("\n") + "\n" + + expect { + expect { obj.save }.to output(expected_output).to_stdout + }.not_to change { klass_with_callbacks.count } + end end context 'when a callback aborts saving' do diff --git a/spec/dynamoid/transaction_write/destroy_spec.rb b/spec/dynamoid/transaction_write/destroy_spec.rb index 212a4466..b3fc8969 100644 --- a/spec/dynamoid/transaction_write/destroy_spec.rb +++ b/spec/dynamoid/transaction_write/destroy_spec.rb @@ -333,6 +333,35 @@ def around_destroy_callback expect(result).to eql false end + + it 'runs before and around callbacks before validating primary key' do + klass_with_callbacks_and_composite_key = new_class do + range :name + + before_destroy { ScratchPad << 'run before_destroy' } + around_destroy :around_destroy_callback + + def around_destroy_callback + ScratchPad << 'start around_destroy' + yield + ScratchPad << 'finish around_destroy' + end + end + + obj = klass_with_callbacks_and_composite_key.create!(name: 'John') + obj.name = nil + + expect { + described_class.execute do |txn| + txn.destroy obj + end + }.to raise_exception(Dynamoid::Errors::MissingRangeKey) + + expect(ScratchPad.recorded).to eql [ + 'run before_destroy', + 'start around_destroy', + ] + end end context 'when table arn is specified', remove_constants: [:Payment] do diff --git a/spec/dynamoid/transaction_write/save_spec.rb b/spec/dynamoid/transaction_write/save_spec.rb index 7867e70a..11368071 100644 --- a/spec/dynamoid/transaction_write/save_spec.rb +++ b/spec/dynamoid/transaction_write/save_spec.rb @@ -974,6 +974,62 @@ def around_create_callback expect(ScratchPad.recorded).to eql [] end + it 'runs only validation callbacks and skips all the other when validation fails' do + klass_with_all_callbacks_and_validation = new_class do + field :name + + validates :name, presence: true + + before_validation { ScratchPad << 'run before_validation' } + after_validation { ScratchPad << 'run after_validation' } + + before_create { ScratchPad << 'run before_create' } + after_create { ScratchPad << 'run after_create' } + around_create :around_create_callback + + before_update { ScratchPad << 'run before_update' } + after_update { ScratchPad << 'run after_update' } + around_update :around_update_callback + + before_save { ScratchPad << 'run before_save' } + after_save { ScratchPad << 'run after_save' } + around_save :around_save_callback + + def around_create_callback + ScratchPad << 'start around_create' + yield + ScratchPad << 'finish around_create' + end + + def around_update_callback + ScratchPad << 'start around_update' + yield + ScratchPad << 'finish around_update' + end + + def around_save_callback + ScratchPad << 'start around_save' + yield + ScratchPad << 'finish around_save' + end + end + + klass_with_all_callbacks_and_validation.create_table + obj = klass_with_all_callbacks_and_validation.new(name: '') + ScratchPad.record [] + + expect { + described_class.execute do |txn| + txn.save obj + end + }.not_to change { klass_with_all_callbacks_and_validation.count } + + expect(ScratchPad.recorded).to eql [ + 'run before_validation', + 'run after_validation', + ] + end + it 'skips *_validation callbacks when validate: false option specified and valid model' do klass_with_callbacks = new_class do before_validation { ScratchPad << 'run before_validation' } @@ -1012,6 +1068,66 @@ def around_create_callback expect(ScratchPad.recorded).to eql [] end + + it 'runs before and around callbacks before validating primary key' do + klass_with_all_callbacks_and_composite_key = new_class do + range :name + + before_validation { ScratchPad << 'run before_validation' } + after_validation { ScratchPad << 'run after_validation' } + + before_create { ScratchPad << 'run before_create' } + after_create { ScratchPad << 'run after_create' } + around_create :around_create_callback + + before_update { ScratchPad << 'run before_update' } + after_update { ScratchPad << 'run after_update' } + around_update :around_update_callback + + before_save { ScratchPad << 'run before_save' } + after_save { ScratchPad << 'run after_save' } + around_save :around_save_callback + + def around_create_callback + ScratchPad << 'start around_create' + yield + ScratchPad << 'finish around_create' + end + + def around_update_callback + ScratchPad << 'start around_update' + yield + ScratchPad << 'finish around_update' + end + + def around_save_callback + ScratchPad << 'start around_save' + yield + ScratchPad << 'finish around_save' + end + end + + klass_with_all_callbacks_and_composite_key.create_table + obj = klass_with_all_callbacks_and_composite_key.new(name: nil) + ScratchPad.record [] + + expect { + expect { + described_class.execute do |txn| + txn.save obj + end + }.to raise_exception(Dynamoid::Errors::MissingRangeKey) + }.not_to change { klass_with_all_callbacks_and_composite_key.count } + + expect(ScratchPad.recorded).to eql [ + 'run before_validation', + 'run after_validation', + 'run before_save', + 'start around_save', + 'run before_create', + 'start around_create', + ] + end end context 'persisted model' do @@ -1179,6 +1295,62 @@ def around_update_callback ] end + it 'runs only validation callbacks and skips all the other when validation fails' do + klass_with_all_callbacks_and_validation = new_class do + field :name + + validates :name, presence: true + + before_validation { ScratchPad << 'run before_validation' } + after_validation { ScratchPad << 'run after_validation' } + + before_create { ScratchPad << 'run before_create' } + after_create { ScratchPad << 'run after_create' } + around_create :around_create_callback + + before_update { ScratchPad << 'run before_update' } + after_update { ScratchPad << 'run after_update' } + around_update :around_update_callback + + before_save { ScratchPad << 'run before_save' } + after_save { ScratchPad << 'run after_save' } + around_save :around_save_callback + + def around_create_callback + ScratchPad << 'start around_create' + yield + ScratchPad << 'finish around_create' + end + + def around_update_callback + ScratchPad << 'start around_update' + yield + ScratchPad << 'finish around_update' + end + + def around_save_callback + ScratchPad << 'start around_save' + yield + ScratchPad << 'finish around_save' + end + end + + obj = klass_with_all_callbacks_and_validation.create!(name: 'John') + obj.name = nil + ScratchPad.record [] + + expect { + described_class.execute do |txn| + txn.save obj + end + }.not_to change { klass_with_all_callbacks_and_validation.count } + + expect(ScratchPad.recorded).to eql [ + 'run before_validation', + 'run after_validation', + ] + end + it 'runs callbacks immediately' do obj = klass_with_all_callbacks.create!(name: 'John') obj.name = 'Bob' @@ -1206,6 +1378,66 @@ def around_update_callback ) expect(ScratchPad.recorded).to eql [] end + + it 'runs before and around callbacks before validating primary key' do + klass_with_all_callbacks_and_composite_key = new_class do + range :name + + before_validation { ScratchPad << 'run before_validation' } + after_validation { ScratchPad << 'run after_validation' } + + before_create { ScratchPad << 'run before_create' } + after_create { ScratchPad << 'run after_create' } + around_create :around_create_callback + + before_update { ScratchPad << 'run before_update' } + after_update { ScratchPad << 'run after_update' } + around_update :around_update_callback + + before_save { ScratchPad << 'run before_save' } + after_save { ScratchPad << 'run after_save' } + around_save :around_save_callback + + def around_create_callback + ScratchPad << 'start around_create' + yield + ScratchPad << 'finish around_create' + end + + def around_update_callback + ScratchPad << 'start around_update' + yield + ScratchPad << 'finish around_update' + end + + def around_save_callback + ScratchPad << 'start around_save' + yield + ScratchPad << 'finish around_save' + end + end + + obj = klass_with_all_callbacks_and_composite_key.create!(name: 'John') + obj.name = nil + ScratchPad.record [] + + expect { + expect { + described_class.execute do |txn| + txn.save obj + end + }.to raise_exception(Dynamoid::Errors::MissingRangeKey) + }.not_to change { klass_with_all_callbacks_and_composite_key.count } + + expect(ScratchPad.recorded).to eql [ + 'run before_validation', + 'run after_validation', + 'run before_save', + 'start around_save', + 'run before_update', + 'start around_update', + ] + end end end