From 613c3e883f7bdc1b2b1eef45091b2483d89659f9 Mon Sep 17 00:00:00 2001 From: "Marcos G. Zimmermann" Date: Wed, 10 Jul 2024 18:25:48 -0300 Subject: [PATCH] feat: index_callback on update with update action instead of index (#8) * feat: index_callback on update with update action instead of index * chore: remove debug code --- Gemfile.lock | 2 +- ci/Gemfile.rails-5.2.lock | 2 +- ci/Gemfile.rails-6.0.lock | 2 +- ci/Gemfile.rails-6.1.lock | 2 +- ci/Gemfile.rails-7.0.lock | 2 +- ci/Gemfile.rails-7.1.lock | 2 +- .../callbacks/indexing_on_update.rb | 23 +++- lib/esse/active_record/model.rb | 14 ++- lib/esse/active_record/version.rb | 2 +- .../callbacks/indexing_on_update_spec.rb | 100 ++++++++++++++++++ .../callbacks/update_lazy_attribute_spec.rb | 1 + .../active_record/model/esse_callback_spec.rb | 2 +- .../model/index_callback_spec.rb | 2 +- .../update_lazy_attribute_callback_spec.rb | 3 +- spec/support/config_helpers.rb | 4 + 15 files changed, 149 insertions(+), 14 deletions(-) create mode 100644 spec/esse/active_record/callbacks/indexing_on_update_spec.rb diff --git a/Gemfile.lock b/Gemfile.lock index 70b867d..fcd2f30 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - esse-active_record (0.3.0) + esse-active_record (0.3.1) activerecord (>= 4.2, < 8) esse (>= 0.3.0) diff --git a/ci/Gemfile.rails-5.2.lock b/ci/Gemfile.rails-5.2.lock index d407f29..5963372 100644 --- a/ci/Gemfile.rails-5.2.lock +++ b/ci/Gemfile.rails-5.2.lock @@ -1,7 +1,7 @@ PATH remote: .. specs: - esse-active_record (0.3.0) + esse-active_record (0.3.1) activerecord (>= 4.2, < 8) esse (>= 0.3.0) diff --git a/ci/Gemfile.rails-6.0.lock b/ci/Gemfile.rails-6.0.lock index ec3274b..b303506 100644 --- a/ci/Gemfile.rails-6.0.lock +++ b/ci/Gemfile.rails-6.0.lock @@ -1,7 +1,7 @@ PATH remote: .. specs: - esse-active_record (0.3.0) + esse-active_record (0.3.1) activerecord (>= 4.2, < 8) esse (>= 0.3.0) diff --git a/ci/Gemfile.rails-6.1.lock b/ci/Gemfile.rails-6.1.lock index 0eb93bc..2cd9e1c 100644 --- a/ci/Gemfile.rails-6.1.lock +++ b/ci/Gemfile.rails-6.1.lock @@ -1,7 +1,7 @@ PATH remote: .. specs: - esse-active_record (0.3.0) + esse-active_record (0.3.1) activerecord (>= 4.2, < 8) esse (>= 0.3.0) diff --git a/ci/Gemfile.rails-7.0.lock b/ci/Gemfile.rails-7.0.lock index ec3274b..b303506 100644 --- a/ci/Gemfile.rails-7.0.lock +++ b/ci/Gemfile.rails-7.0.lock @@ -1,7 +1,7 @@ PATH remote: .. specs: - esse-active_record (0.3.0) + esse-active_record (0.3.1) activerecord (>= 4.2, < 8) esse (>= 0.3.0) diff --git a/ci/Gemfile.rails-7.1.lock b/ci/Gemfile.rails-7.1.lock index 269a7c4..a2ebe35 100644 --- a/ci/Gemfile.rails-7.1.lock +++ b/ci/Gemfile.rails-7.1.lock @@ -1,7 +1,7 @@ PATH remote: .. specs: - esse-active_record (0.3.0) + esse-active_record (0.3.1) activerecord (>= 4.2, < 8) esse (>= 0.3.0) diff --git a/lib/esse/active_record/callbacks/indexing_on_update.rb b/lib/esse/active_record/callbacks/indexing_on_update.rb index ea280eb..1da6c2a 100644 --- a/lib/esse/active_record/callbacks/indexing_on_update.rb +++ b/lib/esse/active_record/callbacks/indexing_on_update.rb @@ -3,13 +3,20 @@ module Esse::ActiveRecord module Callbacks class IndexingOnUpdate < Callback + attr_reader :update_with + + def initialize(with: :index, **kwargs, &block) + @update_with = with + super(**kwargs, &block) + end + def call(model) record = block_result || model document = repo.serialize(record) return true unless document - repo.index.index(document, **options) + update_document(document) return true unless document.routing prev_record = model.class.new(model.attributes.merge(model.previous_changes.transform_values(&:first))).tap(&:readonly!) @@ -27,6 +34,20 @@ def call(model) true end + + protected + + def update_document(document) + if update_with == :update + begin + repo.index.update(document, **options) + rescue Esse::Transport::NotFoundError + repo.index.index(document, **options) + end + else + repo.index.index(document, **options) + end + end end register_callback(:indexing, :update, IndexingOnUpdate) diff --git a/lib/esse/active_record/model.rb b/lib/esse/active_record/model.rb index b484f7c..e04cfa8 100644 --- a/lib/esse/active_record/model.rb +++ b/lib/esse/active_record/model.rb @@ -48,8 +48,18 @@ def esse_callback(index_repo_name, operation_name, on: %i[create update destroy] # For namespace, use `/` as the separator. # @raise [ArgumentError] when the repo and events are already registered # @raise [ArgumentError] when the specified index have multiple repos - def index_callback(index_repo_name, on: %i[create update destroy], **options, &block) - esse_callback(index_repo_name, :indexing, on: on, **options, &block) + def index_callback(index_repo_name, on: %i[create update destroy], with: nil, **options, &block) + if with + Array(on).each do |event| + if on == :update + esse_callback(index_repo_name, :indexing, on: event, with: with, **options, &block) + else + esse_callback(index_repo_name, :indexing, on: event, **options, &block) + end + end + else + esse_callback(index_repo_name, :indexing, on: on, **options, &block) + end end def update_lazy_attribute_callback(index_repo_name, attribute_name, on: %i[create update destroy], **options, &block) diff --git a/lib/esse/active_record/version.rb b/lib/esse/active_record/version.rb index d05e0ab..2daf1ef 100644 --- a/lib/esse/active_record/version.rb +++ b/lib/esse/active_record/version.rb @@ -2,6 +2,6 @@ module Esse module ActiveRecord - VERSION = '0.3.0' + VERSION = '0.3.1' end end diff --git a/spec/esse/active_record/callbacks/indexing_on_update_spec.rb b/spec/esse/active_record/callbacks/indexing_on_update_spec.rb new file mode 100644 index 0000000..44619d3 --- /dev/null +++ b/spec/esse/active_record/callbacks/indexing_on_update_spec.rb @@ -0,0 +1,100 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Esse::ActiveRecord::Callbacks::IndexingOnUpdate do + describe '.initialize' do + let(:repo) { instance_double(Esse::Repository) } + + it 'sets update_with' do + callback = described_class.new(repo: repo, with: :update) + expect(callback.update_with).to eq(:update) + end + + it 'sets options' do + callback = described_class.new(repo: repo, foo: :bar) + expect(callback.options).to eq(foo: :bar) + end + end + + describe '.call' do + let(:ok_response) { { 'result' => 'indexed' } } + let(:state_class) do + Class.new(State) do + include Esse::ActiveRecord::Model + index_callback 'states:state', on: :update + end + end + let!(:state) { create_record(state_class, name: 'Illinois') } + + before do + clear_active_record_hooks + stub_cluster_info + stub_esse_index(:states) do + repository :state, const: true do + document do |state, **| + { + _id: state.id, + name: state.name, + } + end + end + end + end + + after do + clean_db + end + + context 'when update_with is :index' do + it 'indexes the update record' do + expect(StatesIndex).to receive(:index).and_call_original + expect(StatesIndex).to esse_receive_request(:index).with( + id: state.id, + index: StatesIndex.index_name, + body: {name: 'IL'}, + ).and_return(ok_response) + + state.update!(name: 'IL') + end + end + + context 'when update_with is :update' do + let(:state_class) do + Class.new(State) do + include Esse::ActiveRecord::Model + index_callback 'states:state', on: :update, with: :update + end + end + + it 'updates the update record when it exist' do + expect(StatesIndex).to receive(:update).and_call_original + expect(StatesIndex).to esse_receive_request(:update).with( + id: state.id, + index: StatesIndex.index_name, + body: {doc: { name: 'IL' } }, + ).and_return(ok_response) + + state.update!(name: 'IL') + end + + it 'indexes the update record when it does not exist' do + expect(StatesIndex).to receive(:update).and_call_original + expect(StatesIndex).to esse_receive_request(:update).with( + id: state.id, + index: StatesIndex.index_name, + body: {doc: { name: 'IL' } }, + ).and_raise_http_status(404, { 'error' => { 'type' => 'not_found' } }) + + expect(StatesIndex).to receive(:index).and_call_original + expect(StatesIndex).to esse_receive_request(:index).with( + id: state.id, + index: StatesIndex.index_name, + body: {name: 'IL'}, + ).and_return(ok_response) + + state.update!(name: 'IL') + end + end + end +end diff --git a/spec/esse/active_record/callbacks/update_lazy_attribute_spec.rb b/spec/esse/active_record/callbacks/update_lazy_attribute_spec.rb index de7a872..5472ffc 100644 --- a/spec/esse/active_record/callbacks/update_lazy_attribute_spec.rb +++ b/spec/esse/active_record/callbacks/update_lazy_attribute_spec.rb @@ -30,6 +30,7 @@ before do stub_cluster_info + clear_active_record_hooks stub_esse_index(:states) do repository :state, const: true do document do |state, **| diff --git a/spec/esse/active_record/model/esse_callback_spec.rb b/spec/esse/active_record/model/esse_callback_spec.rb index 57ed9ac..a516699 100644 --- a/spec/esse/active_record/model/esse_callback_spec.rb +++ b/spec/esse/active_record/model/esse_callback_spec.rb @@ -36,7 +36,7 @@ class DumpTempCallbackOnDestroy < DumpTempCallback; end before do DummyCallbackRepo.clear - Thread.current[Esse::ActiveRecord::Hooks::STORE_STATE_KEY] = nil + clear_active_record_hooks @__callbacks = Esse::ActiveRecord::Callbacks.instance_variable_get(:@callbacks) Esse::ActiveRecord::Callbacks.register_callback(:temp, :create, DumpTempCallbackOnCreate) Esse::ActiveRecord::Callbacks.register_callback(:temp, :update, DumpTempCallbackOnUpdate) diff --git a/spec/esse/active_record/model/index_callback_spec.rb b/spec/esse/active_record/model/index_callback_spec.rb index 8a58324..06792e3 100644 --- a/spec/esse/active_record/model/index_callback_spec.rb +++ b/spec/esse/active_record/model/index_callback_spec.rb @@ -4,7 +4,7 @@ let(:backend_proxy) { double } before do - Thread.current[Esse::ActiveRecord::Hooks::STORE_STATE_KEY] = nil + clear_active_record_hooks @models_value_backup = Esse::ActiveRecord::Hooks.models.dup Esse::ActiveRecord::Hooks.models.clear stub_cluster_info diff --git a/spec/esse/active_record/model/update_lazy_attribute_callback_spec.rb b/spec/esse/active_record/model/update_lazy_attribute_callback_spec.rb index 8a5dc9d..2564745 100644 --- a/spec/esse/active_record/model/update_lazy_attribute_callback_spec.rb +++ b/spec/esse/active_record/model/update_lazy_attribute_callback_spec.rb @@ -8,8 +8,7 @@ end before do - Thread.current[Esse::ActiveRecord::Hooks::STORE_STATE_KEY] = nil - + clear_active_record_hooks @__hooks_models = Esse::ActiveRecord::Hooks.models.dup Esse::ActiveRecord::Hooks.models.clear stub_cluster_info diff --git a/spec/support/config_helpers.rb b/spec/support/config_helpers.rb index 78680d1..6f99460 100644 --- a/spec/support/config_helpers.rb +++ b/spec/support/config_helpers.rb @@ -41,4 +41,8 @@ def stub_cluster_info(distribution: 'elasticsearch', version: '7.11.0') def with_cluster_config(id: :default, **opts) with_config { |c| c.cluster(id).assign(opts) } end + + def clear_active_record_hooks + Thread.current[Esse::ActiveRecord::Hooks::STORE_STATE_KEY] = nil + end end