From 34363ad1a391f0635f884f4fec19ede594a0f231 Mon Sep 17 00:00:00 2001 From: David Garcia Date: Wed, 18 Jan 2017 10:05:46 +0100 Subject: [PATCH 1/2] Add a failing test for validation on update --- .../test/unit/model_store_test.rb | 33 ++++++++++++++----- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/elasticsearch-persistence/test/unit/model_store_test.rb b/elasticsearch-persistence/test/unit/model_store_test.rb index 442ab56ee..68e434cc6 100644 --- a/elasticsearch-persistence/test/unit/model_store_test.rb +++ b/elasticsearch-persistence/test/unit/model_store_test.rb @@ -31,6 +31,8 @@ class DummyStoreModel attribute :count, Integer, default: 0 attribute :created_at, DateTime, default: lambda { |o,a| Time.now.utc } attribute :updated_at, DateTime, default: lambda { |o,a| Time.now.utc } + + validates :title, presence: true end setup do @@ -65,7 +67,7 @@ class DummyStoreModel $stderr.expects(:puts).with('CREATED') $stderr.expects(:puts).with('SAVED') - DummyStoreModelWithCallback.create name: 'test' + DummyStoreModelWithCallback.create title: 'test' end end @@ -163,7 +165,7 @@ def valid?; false; end; DummyStoreModelWithCallback.after_save { $stderr.puts "SAVED" } $stderr.expects(:puts).with('SAVED') - d = DummyStoreModelWithCallback.new name: 'Test' + d = DummyStoreModelWithCallback.new title: 'Test' d.save end @@ -176,7 +178,7 @@ def valid?; false; end; end .returns({'_id' => 'abc'}) - d = DummyStoreModel.new name: 'Test' + d = DummyStoreModel.new title: 'Test' d.instance_variable_set(:@_index, 'my_custom_index') d.instance_variable_set(:@_type, 'my_custom_type') d.save @@ -191,7 +193,7 @@ def valid?; false; end; end .returns({'_id' => 'abc', '_index' => 'foo', '_type' => 'bar', '_version' => '100'}) - d = DummyStoreModel.new name: 'Test' + d = DummyStoreModel.new title: 'Test' d.instance_variable_set(:@_index, 'my_custom_index') d.instance_variable_set(:@_type, 'my_custom_type') d.save @@ -311,7 +313,7 @@ def valid?; false; end; assert subject.update( {}, { script: 'EXEC' } ) end - should "not update an invalid model" do + should "not update an already invalid model" do @gateway .expects(:update) .never @@ -323,6 +325,19 @@ def valid?; false; end; assert ! subject.update(title: 'INVALID') end + should "not update an invalid model" do + + subject.instance_eval do + def persisted?; true; end; + end + + @gateway + .expects(:update) + .never + + assert ! subject.update(title: nil) + end + should "skip the validation with the :validate option" do subject.expects(:persisted?).returns(true).at_least_once subject.expects(:id).returns('abc123').at_least_once @@ -376,7 +391,7 @@ def valid?; false; end; DummyStoreModelWithCallback.after_update { $stderr.puts "UPDATED" } $stderr.expects(:puts).with('UPDATED') - d = DummyStoreModelWithCallback.new name: 'Test' + d = DummyStoreModelWithCallback.new title: 'Test' d.expects(:persisted?).returns(true) d.update name: 'Update' end @@ -390,7 +405,7 @@ def valid?; false; end; end .returns({'_id' => 'abc'}) - d = DummyStoreModel.new name: 'Test' + d = DummyStoreModel.new title: 'Test' d.instance_variable_set(:@_index, 'my_custom_index') d.instance_variable_set(:@_type, 'my_custom_type') d.expects(:persisted?).returns(true) @@ -407,12 +422,12 @@ def valid?; false; end; end .returns({'_id' => 'abc', '_index' => 'foo', '_type' => 'bar', '_version' => '100'}) - d = DummyStoreModel.new name: 'Test' + d = DummyStoreModel.new title: 'Test' d.instance_variable_set(:@_index, 'my_custom_index') d.instance_variable_set(:@_type, 'my_custom_type') d.expects(:persisted?).returns(true) - d.update name: 'Update' + d.update title: 'Update' assert_equal 'foo', d._index assert_equal 'bar', d._type From c54a0234402d848504e771afadceccac66b9cee9 Mon Sep 17 00:00:00 2001 From: David Garcia Date: Wed, 18 Jan 2017 10:54:48 +0100 Subject: [PATCH 2/2] Update the attributes on the model --- .../lib/elasticsearch/persistence/model/store.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/elasticsearch-persistence/lib/elasticsearch/persistence/model/store.rb b/elasticsearch-persistence/lib/elasticsearch/persistence/model/store.rb index a6c44c998..6a56aef87 100644 --- a/elasticsearch-persistence/lib/elasticsearch/persistence/model/store.rb +++ b/elasticsearch-persistence/lib/elasticsearch/persistence/model/store.rb @@ -112,6 +112,7 @@ def destroy(options={}) # @return [Hash] The Elasticsearch response as a Hash # def update(attributes={}, options={}) + attributes = self.attributes.merge(attributes) unless options.delete(:validate) == false return false unless valid? end