From f6c2f46fbbb902bd215c6f064a14ab143e5dc182 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 23 Jan 2020 11:27:28 -0800 Subject: [PATCH] Move Search::RemoveFromIndexJob to Sidekiq (#5637) [deploy] * Create RemoveFromIndexWorker * Create spec for RemoveFromIndexWorker * Update RemoveFromIndexJob to RemoveFromIndexWorker - Update references from the job to the new worker - Update specs to use the new worker - Fix spec in comment_spec that causes linter/RuboCop violation * Remove unnecessary argument nil check * Update spec for missing data --- app/models/article.rb | 10 ++--- app/models/comment.rb | 4 +- app/models/page_view.rb | 2 +- app/models/user.rb | 2 +- .../search/remove_from_index_worker.rb | 11 +++++ spec/models/article_spec.rb | 40 ++++++++++++++----- spec/models/comment_spec.rb | 4 +- spec/models/page_view_spec.rb | 4 +- .../search/remove_from_index_worker_spec.rb | 32 +++++++++++++++ 9 files changed, 86 insertions(+), 23 deletions(-) create mode 100644 app/workers/search/remove_from_index_worker.rb create mode 100644 spec/workers/search/remove_from_index_worker_spec.rb diff --git a/app/models/article.rb b/app/models/article.rb index 19200acf6..54ef9eca4 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -267,9 +267,9 @@ class Article < ApplicationRecord if record.published && record.tag_list.exclude?("hiring") Search::IndexWorker.perform_async("Article", record.id) else - Search::RemoveFromIndexJob.perform_later(Article.algolia_index_name, record.id) - Search::RemoveFromIndexJob.perform_later("searchables_#{Rails.env}", record.index_id) - Search::RemoveFromIndexJob.perform_later("ordered_articles_#{Rails.env}", record.index_id) + Search::RemoveFromIndexWorker.perform_async(Article.algolia_index_name, record.id) + Search::RemoveFromIndexWorker.perform_async("searchables_#{Rails.env}", record.index_id) + Search::RemoveFromIndexWorker.perform_async("ordered_articles_#{Rails.env}", record.index_id) end end @@ -386,8 +386,8 @@ class Article < ApplicationRecord private def delete_related_objects - Search::RemoveFromIndexJob.perform_now("searchables_#{Rails.env}", index_id) - Search::RemoveFromIndexJob.perform_now("ordered_articles_#{Rails.env}", index_id) + Search::RemoveFromIndexWorker.new.perform("searchables_#{Rails.env}", index_id) + Search::RemoveFromIndexWorker.new.perform("ordered_articles_#{Rails.env}", index_id) end def search_score diff --git a/app/models/comment.rb b/app/models/comment.rb index c1d4f70ea..92f257f3d 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -117,14 +117,14 @@ class Comment < ApplicationRecord if record.deleted == false Search::IndexWorker.perform_async("Comment", record.id) else - Search::RemoveFromIndexJob.perform_later(Comment.algolia_index_name, record.index_id) + Search::RemoveFromIndexWorker.perform_async(Comment.algolia_index_name, record.index_id) end end # this should remain public because it's called by AlgoliaSearch::AlgoliaJob in .trigger_index def remove_algolia_index remove_from_index! - Search::RemoveFromIndexJob.perform_now("ordered_comments_#{Rails.env}", index_id) + Search::RemoveFromIndexWorker.new.perform("ordered_comments_#{Rails.env}", index_id) end def path diff --git a/app/models/page_view.rb b/app/models/page_view.rb index e565c8528..21530ee0a 100644 --- a/app/models/page_view.rb +++ b/app/models/page_view.rb @@ -48,7 +48,7 @@ class PageView < ApplicationRecord def self.trigger_index_sync(record, remove) if remove - Search::RemoveFromIndexJob.perform_later(algolia_index_name, record.id) + Search::RemoveFromIndexWorker.perform_async(algolia_index_name, record.id) else Search::IndexWorker.perform_async("PageView", record.id) end diff --git a/app/models/user.rb b/app/models/user.rb index 95fa6dd3c..1fb0c97a9 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -439,7 +439,7 @@ class User < ApplicationRecord def remove_from_algolia_index remove_from_index! - Search::RemoveFromIndexJob.perform_later("searchables_#{Rails.env}", index_id) + Search::RemoveFromIndexWorker.perform_async("searchables_#{Rails.env}", index_id) end def unsubscribe_from_newsletters diff --git a/app/workers/search/remove_from_index_worker.rb b/app/workers/search/remove_from_index_worker.rb new file mode 100644 index 000000000..8dcd3ce33 --- /dev/null +++ b/app/workers/search/remove_from_index_worker.rb @@ -0,0 +1,11 @@ +module Search + class RemoveFromIndexWorker + include Sidekiq::Worker + + sidekiq_options queue: :medium_priority, retry: 10 + + def perform(index, key) + Algolia::Index.new(index).delete_object(key) + end + end +end diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index 7b62742b4..2b54025a3 100644 --- a/spec/models/article_spec.rb +++ b/spec/models/article_spec.rb @@ -466,20 +466,40 @@ RSpec.describe Article, type: :model do end context "when indexing and deindexing" do - it "deindexes unpublished article" do - expect do + it "deindexes unpublished article from Article index" do + sidekiq_assert_enqueued_with(job: Search::RemoveFromIndexWorker, args: [described_class.algolia_index_name, article.id]) do article.update(body_markdown: "---\ntitle: Title\npublished: false\ndescription:\ntags: one\n---\n\n") - end.to have_enqueued_job(Search::RemoveFromIndexJob).with(described_class.algolia_index_name, article.id). - and have_enqueued_job(Search::RemoveFromIndexJob).with("searchables_#{Rails.env}", article.index_id). - and have_enqueued_job(Search::RemoveFromIndexJob).with("ordered_articles_#{Rails.env}", article.index_id) + end end - it "deindexes hiring article" do - expect do + it "deindexes unpublished article from searchables index" do + sidekiq_assert_enqueued_with(job: Search::RemoveFromIndexWorker, args: ["searchables_#{Rails.env}", article.index_id]) do + article.update(body_markdown: "---\ntitle: Title\npublished: false\ndescription:\ntags: one\n---\n\n") + end + end + + it "deindexes unpublished article from ordered_articles index" do + sidekiq_assert_enqueued_with(job: Search::RemoveFromIndexWorker, args: ["ordered_articles_#{Rails.env}", article.index_id]) do + article.update(body_markdown: "---\ntitle: Title\npublished: false\ndescription:\ntags: one\n---\n\n") + end + end + + it "deindexes hiring article from Article index" do + sidekiq_assert_enqueued_with(job: Search::RemoveFromIndexWorker, args: [described_class.algolia_index_name, article.id]) do article.update(body_markdown: "---\ntitle: Title\npublished: true\ndescription:\ntags: hiring\n---\n\n") - end.to have_enqueued_job(Search::RemoveFromIndexJob).with(described_class.algolia_index_name, article.id). - and have_enqueued_job(Search::RemoveFromIndexJob).with("searchables_#{Rails.env}", article.index_id). - and have_enqueued_job(Search::RemoveFromIndexJob).with("ordered_articles_#{Rails.env}", article.index_id) + end + end + + it "deindexes hiring article from searchables index" do + sidekiq_assert_enqueued_with(job: Search::RemoveFromIndexWorker, args: ["searchables_#{Rails.env}", article.index_id]) do + article.update(body_markdown: "---\ntitle: Title\npublished: true\ndescription:\ntags: hiring\n---\n\n") + end + end + + it "deindexes hiring article from ordered_articles index" do + sidekiq_assert_enqueued_with(job: Search::RemoveFromIndexWorker, args: ["ordered_articles_#{Rails.env}", article.index_id]) do + article.update(body_markdown: "---\ntitle: Title\npublished: true\ndescription:\ntags: hiring\n---\n\n") + end end it "indexes published non-hiring article" do diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index ab016a708..089cb161d 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -368,9 +368,9 @@ RSpec.describe Comment, type: :model do context "when deleted is true" do it "checks auto-deindexing" do - expect do + sidekiq_assert_enqueued_with(job: Search::RemoveFromIndexWorker, args: [described_class.algolia_index_name, comment.index_id]) do comment.update(deleted: true) - end.to have_enqueued_job(Search::RemoveFromIndexJob).with(described_class.algolia_index_name, comment.index_id) + end end end end diff --git a/spec/models/page_view_spec.rb b/spec/models/page_view_spec.rb index 5b6a56342..2141b4d69 100644 --- a/spec/models/page_view_spec.rb +++ b/spec/models/page_view_spec.rb @@ -28,9 +28,9 @@ RSpec.describe PageView, type: :model do end it "removes deleted records" do - expect do + sidekiq_assert_enqueued_with(job: Search::RemoveFromIndexWorker, args: [described_class.algolia_index_name, page_view.id]) do page_view.destroy - end.to have_enqueued_job(Search::RemoveFromIndexJob).exactly(:once).with(described_class.algolia_index_name, page_view.id) + end end end end diff --git a/spec/workers/search/remove_from_index_worker_spec.rb b/spec/workers/search/remove_from_index_worker_spec.rb new file mode 100644 index 000000000..712a046df --- /dev/null +++ b/spec/workers/search/remove_from_index_worker_spec.rb @@ -0,0 +1,32 @@ +require "rails_helper" + +RSpec.describe Search::RemoveFromIndexWorker, type: :worker do + let(:worker) { subject } + + include_examples "#enqueues_on_correct_queue", "medium_priority", ["searchables_#{Rails.env}", "users-456"] + + describe "#perform" do + let(:algolia_index) { instance_double(Algolia::Index) } + + before do + allow(Algolia::Index).to receive(:new).and_return(algolia_index) + allow(algolia_index).to receive(:delete_object) + end + + it "calls the service" do + worker.perform("searchables_#{Rails.env}", "users-456") + + expect(algolia_index).to have_received(:delete_object).with("users-456").once + end + + it "doesn't raise an error if key is missing" do + # key is nil + expect { worker.perform("searchables_#{Rails.env}", nil) }.not_to raise_error + end + + it "doesn't raise an error if index is missing" do + # index is nil + expect { worker.perform(nil, "users-456") }.not_to raise_error + end + end +end