diff --git a/.rubocop.yml b/.rubocop.yml index c8a7804..9d09794 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -31,9 +31,6 @@ RSpec/MessageSpies: RSpec/FilePath: Enabled: false -RSpec/SpecFilePathFormat: - Enabled: false - Layout/SpaceInsideHashLiteralBraces: Enabled: false @@ -49,3 +46,6 @@ Style/TrailingCommaInHashLiteral: Style/StringLiterals: Enabled: true EnforcedStyle: single_quotes + +RSpec/MultipleMemoizedHelpers: + Max: 10 diff --git a/Gemfile.lock b/Gemfile.lock index 0b5fa21..cc50584 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - esse-active_record (0.2.0) + esse-active_record (0.2.1) activerecord (>= 4.2, < 8) esse (>= 0.2.3) diff --git a/ci/Gemfile.rails-5.2.lock b/ci/Gemfile.rails-5.2.lock index c125964..1be0d7d 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.2.0) + esse-active_record (0.2.1) activerecord (>= 4.2, < 8) esse (>= 0.2.3) @@ -157,4 +157,4 @@ DEPENDENCIES yard BUNDLED WITH - 2.3.21 + 2.3.22 diff --git a/ci/Gemfile.rails-6.0.lock b/ci/Gemfile.rails-6.0.lock index f0c2ec1..c1ddce3 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.2.0) + esse-active_record (0.2.1) activerecord (>= 4.2, < 8) esse (>= 0.2.3) diff --git a/ci/Gemfile.rails-6.1.lock b/ci/Gemfile.rails-6.1.lock index 5de2a71..abb1757 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.2.0) + esse-active_record (0.2.1) activerecord (>= 4.2, < 8) esse (>= 0.2.3) diff --git a/ci/Gemfile.rails-7.0.lock b/ci/Gemfile.rails-7.0.lock index f0c2ec1..c1ddce3 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.2.0) + esse-active_record (0.2.1) activerecord (>= 4.2, < 8) esse (>= 0.2.3) diff --git a/ci/Gemfile.rails-7.1.lock b/ci/Gemfile.rails-7.1.lock index 71dee63..90103a0 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.2.0) + esse-active_record (0.2.1) activerecord (>= 4.2, < 8) esse (>= 0.2.3) diff --git a/lib/esse/active_record/model.rb b/lib/esse/active_record/model.rb index 04c6ab5..eae263c 100644 --- a/lib/esse/active_record/model.rb +++ b/lib/esse/active_record/model.rb @@ -38,7 +38,7 @@ def index_callbacks(index_repo_name, on: %i[create update destroy], **options, & Esse::ActiveRecord::Hooks.register_model(self) if_enabled = -> { Esse::ActiveRecord::Hooks.enabled?(index_repo_name) && Esse::ActiveRecord::Hooks.enabled_for_model?(self.class, index_repo_name) } - (on & %i[create update]).each do |event| + (on & %i[create]).each do |event| after_commit(on: event, if: if_enabled) do opts = self.class.esse_index_repos.fetch(index_repo_name).fetch(operation_name) record = opts.fetch(:record) @@ -49,6 +49,34 @@ def index_callbacks(index_repo_name, on: %i[create update destroy], **options, & true end end + (on & %i[update]).each do |event| + after_commit(on: event, if: if_enabled) do + opts = self.class.esse_index_repos.fetch(index_repo_name).fetch(operation_name) + record = opts.fetch(:record) + record = instance_exec(&record) if record.respond_to?(:call) + repo = Esse::ActiveRecord::Hooks.resolve_index_repository(index_repo_name) + document = repo.serialize(record) + next true unless document + + repo.index.index(document, **opts[:options]) + next true unless document.routing + + prev_record = self.class.new(attributes.merge(previous_changes.transform_values(&:first))).tap(&:readonly!) + prev_document = repo.serialize(prev_record) + + next true unless prev_document + next true if [prev_document.id, prev_document.routing].include?(nil) + next true if prev_document.routing == document.routing + next true if prev_document.id != document.id + + begin + repo.index.delete(prev_document, **opts[:options]) + rescue Esse::Transport::NotFoundError + end + + true + end + end (on & %i[destroy]).each do |event| after_commit(on: event, if: if_enabled) do opts = self.class.esse_index_repos.fetch(index_repo_name).fetch(operation_name) diff --git a/lib/esse/active_record/version.rb b/lib/esse/active_record/version.rb index 0252e1a..e07e50e 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.2.0' + VERSION = '0.2.1' end end diff --git a/spec/esse/active_record/model_spec.rb b/spec/esse/active_record/model_spec.rb index ea12758..ca69b70 100644 --- a/spec/esse/active_record/model_spec.rb +++ b/spec/esse/active_record/model_spec.rb @@ -37,6 +37,7 @@ _id: county.id, name: county.name, type: 'county', + routing: county.state_id || 1, }.tap do |doc| doc[:state] = { id: county.state.id, name: county.state.name } if county.state end @@ -237,6 +238,71 @@ end end + context 'when on :update with a the document that has a routing key' do + let(:index_ok_response) { { 'result' => 'indexed' } } + let(:model_class) do + Class.new(County) do + include Esse::ActiveRecord::Model + index_callbacks 'geographies:county', on: %i[update] + end + end + let(:il) { create_record(State, name: 'Illinois') } + let(:ny) { create_record(State, name: 'New York') } + let(:county) { create_record(model_class, name: 'Cook', state: il) } + + it 'indexes the document in new routing and deletes the document from previous routing' do + expect(GeographiesIndex).to receive(:index).and_call_original + expect(GeographiesIndex).to esse_receive_request(:index).with( + id: county.id, + index: GeographiesIndex.index_name, + routing: ny.id, + body: {name: 'Cook', type: 'county', state: { id: ny.id, name: ny.name }}, + ).and_return(index_ok_response) + + expect(GeographiesIndex).to receive(:delete).and_call_original + expect(GeographiesIndex).to esse_receive_request(:delete).with( + id: county.id, + index: GeographiesIndex.index_name, + routing: il.id, + ).and_return('result' => 'deleted') + + county.update(state: ny) + end + + it 'does not delete the document when the routing key is not changed' do + expect(GeographiesIndex).to receive(:index).and_call_original + expect(GeographiesIndex).to esse_receive_request(:index).with( + id: county.id, + index: GeographiesIndex.index_name, + routing: il.id, + body: {name: 'Cook County', type: 'county', state: { id: il.id, name: il.name }}, + ).and_return(index_ok_response) + + expect(GeographiesIndex).not_to receive(:delete) + + county.update(name: 'Cook County') + end + + it 'does not raise error when the document does not exist' do + expect(GeographiesIndex).to receive(:index).and_call_original + expect(GeographiesIndex).to esse_receive_request(:index).with( + id: county.id, + index: GeographiesIndex.index_name, + routing: ny.id, + body: {name: 'Cook', type: 'county', state: { id: ny.id, name: ny.name }}, + ).and_return(index_ok_response) + + expect(GeographiesIndex).to receive(:delete).and_call_original + expect(GeographiesIndex).to esse_receive_request(:delete).with( + id: county.id, + index: GeographiesIndex.index_name, + routing: il.id, + ).and_raise_http_status(404, { 'error' => { 'type' => 'not_found' } }) + + county.update(state: ny) + end + end + context 'when on destroy' do let(:delete_ok_response) { { 'result' => 'deleted' } } diff --git a/spec/esse/plugin/active_record/collection_definition_spec.rb b/spec/esse/plugin/active_record/collection_definition_spec.rb index ecfb02a..c601f2c 100644 --- a/spec/esse/plugin/active_record/collection_definition_spec.rb +++ b/spec/esse/plugin/active_record/collection_definition_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Esse::Plugins::ActiveRecord, '.collection' do +RSpec.describe Esse::Plugins::ActiveRecord, '.collection' do # rubocop:disable RSpec/SpecFilePathFormat describe 'index collection definition' do context 'when the type is a model class' do it 'define collection with only class name without raise an error' do