Skip to content

Commit

Permalink
fix: delete doc from old route when it changes
Browse files Browse the repository at this point in the history
  • Loading branch information
marcosgz committed Jul 4, 2024
1 parent 44b5018 commit cb42744
Show file tree
Hide file tree
Showing 9 changed files with 102 additions and 9 deletions.
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
@@ -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)

Expand Down
4 changes: 2 additions & 2 deletions ci/Gemfile.rails-5.2.lock
Original file line number Diff line number Diff line change
@@ -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)

Expand Down Expand Up @@ -157,4 +157,4 @@ DEPENDENCIES
yard

BUNDLED WITH
2.3.21
2.3.22
2 changes: 1 addition & 1 deletion ci/Gemfile.rails-6.0.lock
Original file line number Diff line number Diff line change
@@ -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)

Expand Down
2 changes: 1 addition & 1 deletion ci/Gemfile.rails-6.1.lock
Original file line number Diff line number Diff line change
@@ -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)

Expand Down
2 changes: 1 addition & 1 deletion ci/Gemfile.rails-7.0.lock
Original file line number Diff line number Diff line change
@@ -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)

Expand Down
2 changes: 1 addition & 1 deletion ci/Gemfile.rails-7.1.lock
Original file line number Diff line number Diff line change
@@ -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)

Expand Down
30 changes: 29 additions & 1 deletion lib/esse/active_record/model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lib/esse/active_record/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@

module Esse
module ActiveRecord
VERSION = '0.2.0'
VERSION = '0.2.1'
end
end
65 changes: 65 additions & 0 deletions spec/esse/active_record/model_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -172,6 +173,70 @@
model.touch
end

context "when the document has a routing key" do
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

it 'index the associated model using the block definition' do
model_class = Class.new(County) do
include Esse::ActiveRecord::Model
Expand Down

0 comments on commit cb42744

Please sign in to comment.