From 0315c5cec5e14e6c6640ace781de1655b9e2ed7c Mon Sep 17 00:00:00 2001 From: Dong Wook Koo Date: Mon, 28 Jul 2014 14:58:20 -0400 Subject: [PATCH 1/5] update document should update with hash from as_indexed_json which could also contain derived attributes. --- .../lib/elasticsearch/model/indexing.rb | 4 +- .../test/unit/indexing_test.rb | 49 ++++++++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/elasticsearch-model/lib/elasticsearch/model/indexing.rb b/elasticsearch-model/lib/elasticsearch/model/indexing.rb index ac507b079..032e532e9 100644 --- a/elasticsearch-model/lib/elasticsearch/model/indexing.rb +++ b/elasticsearch-model/lib/elasticsearch/model/indexing.rb @@ -393,7 +393,9 @@ def delete_document(options={}) def update_document(options={}) if changed_attributes = self.instance_variable_get(:@__changed_attributes) attributes = if respond_to?(:as_indexed_json) - self.as_indexed_json.select { |k,v| changed_attributes.keys.map(&:to_s).include? k.to_s } + indexed_json = self.as_indexed_json + (self.attributes.keys - changed_attributes.keys).map(&:to_s).each{ |k| indexed_json.delete(k) } + indexed_json else changed_attributes end diff --git a/elasticsearch-model/test/unit/indexing_test.rb b/elasticsearch-model/test/unit/indexing_test.rb index b78651340..e77bcc459 100644 --- a/elasticsearch-model/test/unit/indexing_test.rb +++ b/elasticsearch-model/test/unit/indexing_test.rb @@ -186,6 +186,8 @@ def self.before_save(&block) (@callbacks ||= {})[block.hash] = block end + def attributes; { :_id => 1, :foo => 'B', :bar => 'D', :baz => 'F' }; end + def changed_attributes; [:foo, :bar]; end def changes @@ -193,7 +195,32 @@ def changes end def as_indexed_json(options={}) - { :foo => 'B' } + { :foo => 'B', :bar => 'D' } + end + end + + class ::DummyIndexingModelWithCallbacksAndCustomAsIndexedJsonWithDerivedAttribute + extend Elasticsearch::Model::Indexing::ClassMethods + include Elasticsearch::Model::Indexing::InstanceMethods + + def self.before_save(&block) + (@callbacks ||= {})[block.hash] = block + end + + def attributes; { :_id => 1, :foo => 'B', :bar => 'D', :baz => 'F' }; end + + def changed_attributes; [:foo, :bar]; end + + def changes + {:foo => ['A', 'B'], :bar => ['C', 'D']} + end + + def as_indexed_json(options={}) + { :foo => 'B', :bar => 'D', :qux => derived_value } + end + + def derived_value + 'E' end end @@ -367,6 +394,26 @@ def as_indexed_json(options={}) instance.update_document end + should "include derived attributes from custom as_indexed_json during partial update" do + client = mock('client') + instance = ::DummyIndexingModelWithCallbacksAndCustomAsIndexedJsonWithDerivedAttribute.new + + # Set the fake `changes` hash + instance.instance_variable_set(:@__changed_attributes, {foo: 'B'}) + + client.expects(:update).with do |payload| + assert_equal({foo: 'B', qux: 'E'}, payload[:body][:doc]) + true + end + + instance.expects(:client).returns(client) + instance.expects(:index_name).returns('foo') + instance.expects(:document_type).returns('bar') + instance.expects(:id).returns('1') + + instance.update_document + end + should "update only the specific attributes" do client = mock('client') instance = ::DummyIndexingModelWithCallbacks.new From 1c9ceb5e19ad79969a0d9d1c7b9eb2191d8a8d6f Mon Sep 17 00:00:00 2001 From: Dong Wook Koo Date: Wed, 13 Aug 2014 11:41:55 -0400 Subject: [PATCH 2/5] fixed update_document to handle all keys as strings. --- .../lib/elasticsearch/model/indexing.rb | 4 ++-- .../test/unit/indexing_test.rb | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/elasticsearch-model/lib/elasticsearch/model/indexing.rb b/elasticsearch-model/lib/elasticsearch/model/indexing.rb index 032e532e9..5e5c70689 100644 --- a/elasticsearch-model/lib/elasticsearch/model/indexing.rb +++ b/elasticsearch-model/lib/elasticsearch/model/indexing.rb @@ -393,8 +393,8 @@ def delete_document(options={}) def update_document(options={}) if changed_attributes = self.instance_variable_get(:@__changed_attributes) attributes = if respond_to?(:as_indexed_json) - indexed_json = self.as_indexed_json - (self.attributes.keys - changed_attributes.keys).map(&:to_s).each{ |k| indexed_json.delete(k) } + indexed_json = self.as_indexed_json.with_indifferent_access + (self.attributes.keys.map(&:to_s) - changed_attributes.keys.map(&:to_s)).each{ |k| indexed_json.delete(k) } indexed_json else changed_attributes diff --git a/elasticsearch-model/test/unit/indexing_test.rb b/elasticsearch-model/test/unit/indexing_test.rb index e77bcc459..844b5ff02 100644 --- a/elasticsearch-model/test/unit/indexing_test.rb +++ b/elasticsearch-model/test/unit/indexing_test.rb @@ -194,6 +194,27 @@ def changes {:foo => ['A', 'B'], :bar => ['C', 'D']} end + def as_indexed_json(options={}) + { :foo => 'B' } + end + end + + class ::DummyIndexingModelWithCallbacksAndCustomAsIndexedJsonMultiValue + extend Elasticsearch::Model::Indexing::ClassMethods + include Elasticsearch::Model::Indexing::InstanceMethods + + def self.before_save(&block) + (@callbacks ||= {})[block.hash] = block + end + + def attributes; { :_id => 1, :foo => 'B', :bar => 'D', :baz => 'F' }; end + + def changed_attributes; [:foo, :bar]; end + + def changes + {:foo => ['A', 'B'], :bar => ['C', 'D']} + end + def as_indexed_json(options={}) { :foo => 'B', :bar => 'D' } end From 6b04a8ba0dfa8cd85beb93b85c387d3249c8a89a Mon Sep 17 00:00:00 2001 From: Dong Wook Koo Date: Wed, 13 Aug 2014 11:50:27 -0400 Subject: [PATCH 3/5] added case where only valid custom as_indexed_json key value pair is selected for specific changed attribute. --- .../test/unit/indexing_test.rb | 43 ++++++++++--------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/elasticsearch-model/test/unit/indexing_test.rb b/elasticsearch-model/test/unit/indexing_test.rb index 844b5ff02..0e71c0a92 100644 --- a/elasticsearch-model/test/unit/indexing_test.rb +++ b/elasticsearch-model/test/unit/indexing_test.rb @@ -199,27 +199,6 @@ def as_indexed_json(options={}) end end - class ::DummyIndexingModelWithCallbacksAndCustomAsIndexedJsonMultiValue - extend Elasticsearch::Model::Indexing::ClassMethods - include Elasticsearch::Model::Indexing::InstanceMethods - - def self.before_save(&block) - (@callbacks ||= {})[block.hash] = block - end - - def attributes; { :_id => 1, :foo => 'B', :bar => 'D', :baz => 'F' }; end - - def changed_attributes; [:foo, :bar]; end - - def changes - {:foo => ['A', 'B'], :bar => ['C', 'D']} - end - - def as_indexed_json(options={}) - { :foo => 'B', :bar => 'D' } - end - end - class ::DummyIndexingModelWithCallbacksAndCustomAsIndexedJsonWithDerivedAttribute extend Elasticsearch::Model::Indexing::ClassMethods include Elasticsearch::Model::Indexing::InstanceMethods @@ -415,6 +394,28 @@ def derived_value instance.update_document end + should "exclude attributes not contained in changed_attributes from custom as_indexed_json during partial update" do + client = mock('client') + instance = ::DummyIndexingModelWithCallbacksAndCustomAsIndexedJson.new + + # Set the fake `changes` hash + instance.instance_variable_set(:@__changed_attributes, {'foo' => 'B' }) + + # Overload as_indexed_json + instance.expects(:as_indexed_json).returns({ 'foo' => 'BAR', 'bar' => 'BAZ' }) + + client.expects(:update).with do |payload| + assert_equal({'foo' => 'BAR'}, payload[:body][:doc]) + end + + instance.expects(:client).returns(client) + instance.expects(:index_name).returns('foo') + instance.expects(:document_type).returns('bar') + instance.expects(:id).returns('1') + + instance.update_document + end + should "include derived attributes from custom as_indexed_json during partial update" do client = mock('client') instance = ::DummyIndexingModelWithCallbacksAndCustomAsIndexedJsonWithDerivedAttribute.new From 9c176eed8080f58a796f6a410e75fe6dd780d0c8 Mon Sep 17 00:00:00 2001 From: Dong Wook Koo Date: Fri, 25 Aug 2017 14:46:49 -0400 Subject: [PATCH 4/5] fixed to use hash method only --- elasticsearch-model/lib/elasticsearch/model/indexing.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/elasticsearch-model/lib/elasticsearch/model/indexing.rb b/elasticsearch-model/lib/elasticsearch/model/indexing.rb index 5e5c70689..624bc626b 100644 --- a/elasticsearch-model/lib/elasticsearch/model/indexing.rb +++ b/elasticsearch-model/lib/elasticsearch/model/indexing.rb @@ -393,8 +393,8 @@ def delete_document(options={}) def update_document(options={}) if changed_attributes = self.instance_variable_get(:@__changed_attributes) attributes = if respond_to?(:as_indexed_json) - indexed_json = self.as_indexed_json.with_indifferent_access - (self.attributes.keys.map(&:to_s) - changed_attributes.keys.map(&:to_s)).each{ |k| indexed_json.delete(k) } + indexed_json = self.as_indexed_json + (self.attributes.keys.map(&:to_s) - changed_attributes.keys.map(&:to_s)).each{ |k| indexed_json.delete_if{ |a,b| a.to_s == k } } indexed_json else changed_attributes From 5cc68b52fbd7653469a086ffa01278425b8c6cd2 Mon Sep 17 00:00:00 2001 From: Dong Wook Koo Date: Fri, 25 Aug 2017 14:47:00 -0400 Subject: [PATCH 5/5] fix test --- elasticsearch-model/test/unit/indexing_test.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/elasticsearch-model/test/unit/indexing_test.rb b/elasticsearch-model/test/unit/indexing_test.rb index 0e71c0a92..7a7c1b643 100644 --- a/elasticsearch-model/test/unit/indexing_test.rb +++ b/elasticsearch-model/test/unit/indexing_test.rb @@ -406,6 +406,7 @@ def derived_value client.expects(:update).with do |payload| assert_equal({'foo' => 'BAR'}, payload[:body][:doc]) + true end instance.expects(:client).returns(client)