From 318ee32cdd0b5907366fd79e2b8b87338c1fd16e Mon Sep 17 00:00:00 2001 From: Molly Struve Date: Fri, 21 Feb 2020 08:13:36 -0500 Subject: [PATCH] Add worker for removing Elasticsearch docs (#6213) [deploy] --- app/models/chat_channel_membership.rb | 3 ++- app/models/classified_listing.rb | 3 ++- app/models/concerns/searchable.rb | 4 ++++ app/models/tag.rb | 4 +++- app/services/search/base.rb | 4 ++++ .../remove_from_elasticsearch_index_worker.rb | 11 ++++++++++ spec/models/chat_channel_membership_spec.rb | 13 +++++++++--- spec/models/classified_listing_spec.rb | 13 +++++++++--- spec/models/concerns/searchable_spec.rb | 21 +++++++++++++++++++ spec/models/tag_spec.rb | 13 +++++++++--- .../search/chat_channel_membership_spec.rb | 10 +++++++++ .../search/classified_listing_spec.rb | 10 +++++++++ spec/services/search/tag_spec.rb | 10 +++++++++ ...ve_from_elasticsearch_index_worker_spec.rb | 14 +++++++++++++ 14 files changed, 121 insertions(+), 12 deletions(-) create mode 100644 app/workers/search/remove_from_elasticsearch_index_worker.rb create mode 100644 spec/models/concerns/searchable_spec.rb create mode 100644 spec/workers/search/remove_from_elasticsearch_index_worker_spec.rb diff --git a/app/models/chat_channel_membership.rb b/app/models/chat_channel_membership.rb index b00ced440..11c0967c0 100644 --- a/app/models/chat_channel_membership.rb +++ b/app/models/chat_channel_membership.rb @@ -15,7 +15,8 @@ class ChatChannelMembership < ApplicationRecord validates :role, inclusion: { in: %w[member mod] } validate :permission - after_commit :index_to_elasticsearch + after_commit :index_to_elasticsearch, on: %i[create update] + after_commit :remove_from_elasticsearch, on: [:destroy] algoliasearch index_name: "SecuredChatChannelMembership_#{Rails.env}", auto_index: false do attribute :id, :status, :viewable_by, :chat_channel_id, :last_opened_at, diff --git a/app/models/classified_listing.rb b/app/models/classified_listing.rb index 54af4c6c0..e8f0306e2 100644 --- a/app/models/classified_listing.rb +++ b/app/models/classified_listing.rb @@ -27,7 +27,8 @@ class ClassifiedListing < ApplicationRecord before_save :evaluate_markdown before_create :create_slug before_validation :modify_inputs - after_commit :index_to_elasticsearch + after_commit :index_to_elasticsearch, on: %i[create update] + after_commit :remove_from_elasticsearch, on: [:destroy] acts_as_taggable_on :tags has_many :credits, as: :purchase, inverse_of: :purchase, dependent: :nullify diff --git a/app/models/concerns/searchable.rb b/app/models/concerns/searchable.rb index b4ca40958..4d0e07305 100644 --- a/app/models/concerns/searchable.rb +++ b/app/models/concerns/searchable.rb @@ -7,6 +7,10 @@ module Searchable self.class::SEARCH_CLASS.index(id, serialized_search_hash) end + def remove_from_elasticsearch + Search::RemoveFromElasticsearchIndexWorker.perform_async(self.class::SEARCH_CLASS.to_s, id) + end + def serialized_search_hash self.class::SEARCH_SERIALIZER.new(self).serializable_hash.dig(:data, :attributes) end diff --git a/app/models/tag.rb b/app/models/tag.rb index 6c231f284..06b23d1f5 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -25,7 +25,9 @@ class Tag < ActsAsTaggableOn::Tag before_validation :evaluate_markdown before_validation :pound_it before_save :calculate_hotness_score - after_commit :bust_cache, :index_to_elasticsearch + after_commit :bust_cache + after_commit :index_to_elasticsearch, on: %i[create update] + after_commit :remove_from_elasticsearch, on: [:destroy] before_save :mark_as_updated include Searchable diff --git a/app/services/search/base.rb b/app/services/search/base.rb index 74e5227b9..4fea555a6 100644 --- a/app/services/search/base.rb +++ b/app/services/search/base.rb @@ -13,6 +13,10 @@ module Search SearchClient.get(id: doc_id, index: self::INDEX_ALIAS) end + def delete_document(doc_id) + SearchClient.delete(id: doc_id, index: self::INDEX_ALIAS) + end + def create_index(index_name: self::INDEX_NAME) SearchClient.indices.create(index: index_name, body: settings) end diff --git a/app/workers/search/remove_from_elasticsearch_index_worker.rb b/app/workers/search/remove_from_elasticsearch_index_worker.rb new file mode 100644 index 000000000..e74353a7d --- /dev/null +++ b/app/workers/search/remove_from_elasticsearch_index_worker.rb @@ -0,0 +1,11 @@ +module Search + class RemoveFromElasticsearchIndexWorker + include Sidekiq::Worker + + sidekiq_options queue: :medium_priority + + def perform(search_class, id) + search_class.safe_constantize.delete_document(id) + end + end +end diff --git a/spec/models/chat_channel_membership_spec.rb b/spec/models/chat_channel_membership_spec.rb index f5cccf64f..edca39fbc 100644 --- a/spec/models/chat_channel_membership_spec.rb +++ b/spec/models/chat_channel_membership_spec.rb @@ -5,7 +5,7 @@ RSpec.describe ChatChannelMembership, type: :model do describe "#index_to_elasticsearch" do it "enqueues job to index tag to elasticsearch" do - sidekiq_assert_enqueued_with(job: Search::ChatChannelMembershipEsIndexWorker, args: [chat_channel_membership.id]) do + sidekiq_assert_enqueued_with(job: described_class::SEARCH_INDEX_WORKER, args: [chat_channel_membership.id]) do chat_channel_membership.index_to_elasticsearch end end @@ -20,12 +20,19 @@ RSpec.describe ChatChannelMembership, type: :model do end describe "#after_commit" do - it "enqueues job to index chat_channel_membership to elasticsearch" do + it "on update enqueues job to index chat_channel_membership to elasticsearch" do chat_channel_membership.save - sidekiq_assert_enqueued_with(job: Search::ChatChannelMembershipEsIndexWorker, args: [chat_channel_membership.id]) do + sidekiq_assert_enqueued_with(job: described_class::SEARCH_INDEX_WORKER, args: [chat_channel_membership.id]) do chat_channel_membership.save end end + + it "on destroy enqueues job to delete chat_channel_membership from elasticsearch" do + chat_channel_membership.save + sidekiq_assert_enqueued_with(job: Search::RemoveFromElasticsearchIndexWorker, args: [described_class::SEARCH_CLASS.to_s, chat_channel_membership.id]) do + chat_channel_membership.destroy + end + end end describe "#serialized_search_hash" do diff --git a/spec/models/classified_listing_spec.rb b/spec/models/classified_listing_spec.rb index 093ac5853..b7d706a61 100644 --- a/spec/models/classified_listing_spec.rb +++ b/spec/models/classified_listing_spec.rb @@ -74,7 +74,7 @@ RSpec.describe ClassifiedListing, type: :model do describe "#index_to_elasticsearch" do it "enqueues job to index classified_listing to elasticsearch" do - sidekiq_assert_enqueued_with(job: Search::ClassifiedListingEsIndexWorker, args: [classified_listing.id]) do + sidekiq_assert_enqueued_with(job: described_class::SEARCH_INDEX_WORKER, args: [classified_listing.id]) do classified_listing.index_to_elasticsearch end end @@ -89,13 +89,20 @@ RSpec.describe ClassifiedListing, type: :model do end describe "#after_commit" do - it "enqueues worker to index tag to elasticsearch" do + it "on update enqueues worker to index tag to elasticsearch" do classified_listing.save - sidekiq_assert_enqueued_with(job: Search::ClassifiedListingEsIndexWorker, args: [classified_listing.id]) do + sidekiq_assert_enqueued_with(job: described_class::SEARCH_INDEX_WORKER, args: [classified_listing.id]) do classified_listing.save end end + + it "on destroy enqueues job to delete classified_listing from elasticsearch" do + classified_listing.save + sidekiq_assert_enqueued_with(job: Search::RemoveFromElasticsearchIndexWorker, args: [described_class::SEARCH_CLASS.to_s, classified_listing.id]) do + classified_listing.destroy + end + end end describe "#serialized_search_hash" do diff --git a/spec/models/concerns/searchable_spec.rb b/spec/models/concerns/searchable_spec.rb new file mode 100644 index 000000000..ce081d071 --- /dev/null +++ b/spec/models/concerns/searchable_spec.rb @@ -0,0 +1,21 @@ +require "rails_helper" + +class SearchableModel + include Searchable + SEARCH_CLASS = Search::Tag + + def id + 1 + end +end + +RSpec.describe Searchable do + describe "#remove_from_elasticsearch" do + it "enqueues job to delete model document from elasticsearch" do + model = SearchableModel.new + sidekiq_assert_enqueued_with(job: Search::RemoveFromElasticsearchIndexWorker, args: [SearchableModel::SEARCH_CLASS.to_s, model.id]) do + model.remove_from_elasticsearch + end + end + end +end diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index 17ca4ce6c..2840839e1 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -78,7 +78,7 @@ RSpec.describe Tag, type: :model do describe "#index_to_elasticsearch" do it "enqueues job to index tag to elasticsearch" do - sidekiq_assert_enqueued_with(job: Search::TagEsIndexWorker, args: [tag.id]) do + sidekiq_assert_enqueued_with(job: described_class::SEARCH_INDEX_WORKER, args: [tag.id]) do tag.index_to_elasticsearch end end @@ -93,12 +93,19 @@ RSpec.describe Tag, type: :model do end describe "#after_commit" do - it "enqueues job to index tag to elasticsearch" do + it "on update enqueues job to index tag to elasticsearch" do tag.save - sidekiq_assert_enqueued_with(job: Search::TagEsIndexWorker, args: [tag.id]) do + sidekiq_assert_enqueued_with(job: described_class::SEARCH_INDEX_WORKER, args: [tag.id]) do tag.save end end + + it "on destroy enqueues job to delete tag from elasticsearch" do + tag.save + sidekiq_assert_enqueued_with(job: Search::RemoveFromElasticsearchIndexWorker, args: [described_class::SEARCH_CLASS.to_s, tag.id]) do + tag.destroy + end + end end describe "#serialized_search_hash" do diff --git a/spec/services/search/chat_channel_membership_spec.rb b/spec/services/search/chat_channel_membership_spec.rb index 58d4cac4d..cef4d6be6 100644 --- a/spec/services/search/chat_channel_membership_spec.rb +++ b/spec/services/search/chat_channel_membership_spec.rb @@ -18,6 +18,16 @@ RSpec.describe Search::ChatChannelMembership, type: :service, elasticsearch: tru end end + describe "::delete_document" do + it "deletes a document for a given ID from elasticsearch" do + chat_channel_membership = FactoryBot.create(:chat_channel_membership) + chat_channel_membership.index_to_elasticsearch_inline + expect { described_class.find_document(chat_channel_membership.id) }.not_to raise_error + described_class.delete_document(chat_channel_membership.id) + expect { described_class.find_document(chat_channel_membership.id) }.to raise_error(Elasticsearch::Transport::Transport::Errors::NotFound) + end + end + describe "::create_index" do it "creates an elasticsearch index with INDEX_NAME" do described_class.delete_index diff --git a/spec/services/search/classified_listing_spec.rb b/spec/services/search/classified_listing_spec.rb index a9af0471b..013920268 100644 --- a/spec/services/search/classified_listing_spec.rb +++ b/spec/services/search/classified_listing_spec.rb @@ -18,6 +18,16 @@ RSpec.describe Search::ClassifiedListing, type: :service, elasticsearch: true do end end + describe "::delete_document" do + it "deletes a document for a given ID from elasticsearch" do + classified_listing = FactoryBot.create(:classified_listing) + classified_listing.index_to_elasticsearch_inline + expect { described_class.find_document(classified_listing.id) }.not_to raise_error + described_class.delete_document(classified_listing.id) + expect { described_class.find_document(classified_listing.id) }.to raise_error(Elasticsearch::Transport::Transport::Errors::NotFound) + end + end + describe "::create_index" do it "creates an elasticsearch index with INDEX_NAME" do described_class.delete_index diff --git a/spec/services/search/tag_spec.rb b/spec/services/search/tag_spec.rb index 8f3cd023a..f22dc1f05 100644 --- a/spec/services/search/tag_spec.rb +++ b/spec/services/search/tag_spec.rb @@ -67,6 +67,16 @@ RSpec.describe Search::Tag, type: :service, elasticsearch: true do end end + describe "::delete_document" do + it "deletes a document for a given ID from elasticsearch" do + tag = FactoryBot.create(:tag) + tag.index_to_elasticsearch_inline + expect { described_class.find_document(tag.id) }.not_to raise_error + described_class.delete_document(tag.id) + expect { described_class.find_document(tag.id) }.to raise_error(Elasticsearch::Transport::Transport::Errors::NotFound) + end + end + describe "::create_index" do it "creates an elasticsearch index with INDEX_NAME" do described_class.delete_index diff --git a/spec/workers/search/remove_from_elasticsearch_index_worker_spec.rb b/spec/workers/search/remove_from_elasticsearch_index_worker_spec.rb new file mode 100644 index 000000000..e6df2e3d2 --- /dev/null +++ b/spec/workers/search/remove_from_elasticsearch_index_worker_spec.rb @@ -0,0 +1,14 @@ +require "rails_helper" + +RSpec.describe Search::RemoveFromElasticsearchIndexWorker, type: :worker do + let(:worker) { subject } + + include_examples "#enqueues_on_correct_queue", "medium_priority", ["SearchClass", 1] + + it "deletes document for given search class" do + search_class = Search::Tag + allow(search_class).to receive(:delete_document) + described_class.new.perform(search_class.to_s, 1) + expect(search_class).to have_received(:delete_document).with(1) + end +end