From e10809e40b239f7802eb5d60a9cbfcc3645afffa Mon Sep 17 00:00:00 2001 From: Ross Kaffenberger Date: Sat, 7 Dec 2024 20:05:36 -0500 Subject: [PATCH] Make page batch jobs nested classes of their counterparts --- app/jobs/pages/analyze_topics_job.rb | 10 +++++++ app/jobs/pages/batch_analyze_topics_job.rb | 11 -------- app/jobs/pages/batch_embedding_job.rb | 11 -------- app/jobs/pages/batch_upsert_pages_job.rb | 6 ++--- app/jobs/pages/embedding_job.rb | 10 +++++++ app/jobs/pages/refresh_search_index_job.rb | 16 +++++++++-- spec/jobs/pages/analyze_topics_job_spec.rb | 26 ++++++++++++++++++ .../pages/batch_analyze_topics_job_spec.rb | 27 ------------------- spec/jobs/pages/batch_embedding_job_spec.rb | 19 ------------- .../jobs/pages/batch_upsert_pages_job_spec.rb | 10 ++++--- spec/jobs/pages/embedding_job_spec.rb | 18 +++++++++++++ .../pages/refresh_search_index_job_spec.rb | 26 ++++++++++-------- spec/models/page/searchable_spec.rb | 11 ++++---- spec/requests/search_spec.rb | 18 +++++++------ spec/system/searches_spec.rb | 4 +-- 15 files changed, 120 insertions(+), 103 deletions(-) delete mode 100644 app/jobs/pages/batch_analyze_topics_job.rb delete mode 100644 app/jobs/pages/batch_embedding_job.rb delete mode 100644 spec/jobs/pages/batch_analyze_topics_job_spec.rb delete mode 100644 spec/jobs/pages/batch_embedding_job_spec.rb diff --git a/app/jobs/pages/analyze_topics_job.rb b/app/jobs/pages/analyze_topics_job.rb index 696ee76e..9c611c6a 100644 --- a/app/jobs/pages/analyze_topics_job.rb +++ b/app/jobs/pages/analyze_topics_job.rb @@ -3,6 +3,16 @@ module Pages class AnalyzeTopicsJob < ApplicationJob + class Batch < ApplicationJob + queue_as :default + + def perform + Page.published.where.missing(:topics).find_each do |page| + Pages::AnalyzeTopicsJob.perform_later(page) + end + end + end + queue_as :default def perform(page) diff --git a/app/jobs/pages/batch_analyze_topics_job.rb b/app/jobs/pages/batch_analyze_topics_job.rb deleted file mode 100644 index cfe3ce4a..00000000 --- a/app/jobs/pages/batch_analyze_topics_job.rb +++ /dev/null @@ -1,11 +0,0 @@ -module Pages - class BatchAnalyzeTopicsJob < ApplicationJob - queue_as :default - - def perform - Page.published.where.missing(:topics).find_each do |page| - Pages::AnalyzeTopicsJob.perform_later(page) - end - end - end -end diff --git a/app/jobs/pages/batch_embedding_job.rb b/app/jobs/pages/batch_embedding_job.rb deleted file mode 100644 index 978db7fe..00000000 --- a/app/jobs/pages/batch_embedding_job.rb +++ /dev/null @@ -1,11 +0,0 @@ -module Pages - class BatchEmbeddingJob < ApplicationJob - queue_as :default - - def perform - Page.published.where.missing(:page_embedding).find_each do |page| - Pages::EmbeddingJob.perform_later(page) - end - end - end -end diff --git a/app/jobs/pages/batch_upsert_pages_job.rb b/app/jobs/pages/batch_upsert_pages_job.rb index 9bae012b..a4da79ec 100644 --- a/app/jobs/pages/batch_upsert_pages_job.rb +++ b/app/jobs/pages/batch_upsert_pages_job.rb @@ -5,9 +5,9 @@ class BatchUpsertPagesJob < ApplicationJob def perform(limit: nil) Page.upsert_collection_from_sitepress!(limit: limit) - Pages::RefreshSearchIndexJob.perform_later - Pages::BatchAnalyzeTopicsJob.perform_later - Pages::BatchEmbeddingJob.perform_later + Pages::RefreshSearchIndexJob::Batch.perform_later + Pages::AnalyzeTopicsJob::Batch.perform_later + Pages::EmbeddingJob::Batch.perform_later end end end diff --git a/app/jobs/pages/embedding_job.rb b/app/jobs/pages/embedding_job.rb index 2278ff2c..006a3ae9 100644 --- a/app/jobs/pages/embedding_job.rb +++ b/app/jobs/pages/embedding_job.rb @@ -1,5 +1,15 @@ module Pages class EmbeddingJob < ApplicationJob + class Batch < ApplicationJob + queue_as :default + + def perform + Page.published.where.missing(:page_embedding).find_each do |page| + Pages::EmbeddingJob.perform_later(page) + end + end + end + queue_as :default def perform(page) diff --git a/app/jobs/pages/refresh_search_index_job.rb b/app/jobs/pages/refresh_search_index_job.rb index 833dfba3..155a23fa 100644 --- a/app/jobs/pages/refresh_search_index_job.rb +++ b/app/jobs/pages/refresh_search_index_job.rb @@ -1,7 +1,19 @@ module Pages class RefreshSearchIndexJob < ApplicationJob - def perform - Page.published.find_each(&:update_in_search_index) + class Batch < ApplicationJob + queue_as :default + + def perform + Page.published.find_each do |page| + Pages::RefreshSearchIndexJob.perform_later(page) + end + end + end + + queue_as :default + + def perform(page) + page.update_in_search_index end end end diff --git a/spec/jobs/pages/analyze_topics_job_spec.rb b/spec/jobs/pages/analyze_topics_job_spec.rb index ffa80332..83f9fa68 100644 --- a/spec/jobs/pages/analyze_topics_job_spec.rb +++ b/spec/jobs/pages/analyze_topics_job_spec.rb @@ -28,4 +28,30 @@ expect(page.reload.topics.pluck(:name)).to include("Ruby on Rails") end end + + describe Pages::AnalyzeTopicsJob::Batch do + it "doesn’t blow up" do + allow(SitepressPage).to receive(:render_html).and_return(Faker::HTML.random) + + topics = FactoryBot.create_list(:topic, 2) + + page_1, page_2, page_3 = Page.upsert_collection_from_sitepress!(limit: 3).each do |page| + page.touch(:published_at) + end + + # articles without topics + page_1.update!(topics: []) + page_2.update!(topics: []) + + # articles with topics + page_3.update!(topics: topics) + + # not articles + FactoryBot.create_list(:page, 3) + + expect(Pages::AnalyzeTopicsJob).to receive(:perform_later).exactly(2).times + + described_class.perform_now + end + end end diff --git a/spec/jobs/pages/batch_analyze_topics_job_spec.rb b/spec/jobs/pages/batch_analyze_topics_job_spec.rb deleted file mode 100644 index cdae6bed..00000000 --- a/spec/jobs/pages/batch_analyze_topics_job_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -require "rails_helper" - -RSpec.describe Pages::BatchAnalyzeTopicsJob, type: :job do - it "doesn’t blow up" do - allow(SitepressPage).to receive(:render_html).and_return(Faker::HTML.random) - - topics = FactoryBot.create_list(:topic, 2) - - page_1, page_2, page_3 = Page.upsert_collection_from_sitepress!(limit: 3).each do |page| - page.touch(:published_at) - end - - # articles without topics - page_1.update!(topics: []) - page_2.update!(topics: []) - - # articles with topics - page_3.update!(topics: topics) - - # not articles - FactoryBot.create_list(:page, 3) - - expect(Pages::AnalyzeTopicsJob).to receive(:perform_later).exactly(2).times - - described_class.perform_now - end -end diff --git a/spec/jobs/pages/batch_embedding_job_spec.rb b/spec/jobs/pages/batch_embedding_job_spec.rb deleted file mode 100644 index 8b822c27..00000000 --- a/spec/jobs/pages/batch_embedding_job_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -require "rails_helper" - -RSpec.describe Pages::BatchEmbeddingJob, type: :job do - it "doesn’t blow up" do - FactoryBot.create(:page, :published) - expect(Pages::EmbeddingJob).to receive(:perform_later).once - - described_class.perform_now - end - - it "doesn’t blow up when page has embedding" do - page = FactoryBot.create(:page, :published) - FactoryBot.create(:page_embedding, id: page.id) - - expect(Pages::EmbeddingJob).not_to receive(:perform_later) - - described_class.perform_now - end -end diff --git a/spec/jobs/pages/batch_upsert_pages_job_spec.rb b/spec/jobs/pages/batch_upsert_pages_job_spec.rb index 24e315ef..3e0e9194 100644 --- a/spec/jobs/pages/batch_upsert_pages_job_spec.rb +++ b/spec/jobs/pages/batch_upsert_pages_job_spec.rb @@ -28,12 +28,14 @@ end it "enqueues refresh search index job and analyze topics job" do - allow(Pages::RefreshSearchIndexJob).to receive(:perform_later) - allow(Pages::BatchAnalyzeTopicsJob).to receive(:perform_later) + allow(Pages::RefreshSearchIndexJob::Batch).to receive(:perform_later) + allow(Pages::AnalyzeTopicsJob::Batch).to receive(:perform_later) + allow(Pages::EmbeddingJob::Batch).to receive(:perform_later) described_class.perform_now - expect(Pages::RefreshSearchIndexJob).to have_received(:perform_later) - expect(Pages::BatchAnalyzeTopicsJob).to have_received(:perform_later) + expect(Pages::EmbeddingJob::Batch).to have_received(:perform_later) + expect(Pages::RefreshSearchIndexJob::Batch).to have_received(:perform_later) + expect(Pages::AnalyzeTopicsJob::Batch).to have_received(:perform_later) end end diff --git a/spec/jobs/pages/embedding_job_spec.rb b/spec/jobs/pages/embedding_job_spec.rb index 7055787c..ef54fa91 100644 --- a/spec/jobs/pages/embedding_job_spec.rb +++ b/spec/jobs/pages/embedding_job_spec.rb @@ -32,4 +32,22 @@ expect(page_embedding.embedding).to be_a(Array) expect(page_embedding.embedding.length).to eq(PageEmbedding::OPENAI_EMBEDDING_LENGTH) end + + describe Pages::EmbeddingJob::Batch do + it "doesn’t blow up" do + FactoryBot.create(:page, :published) + expect(Pages::EmbeddingJob).to receive(:perform_later).once + + described_class.perform_now + end + + it "doesn’t blow up when page has embedding" do + page = FactoryBot.create(:page, :published) + FactoryBot.create(:page_embedding, id: page.id) + + expect(Pages::EmbeddingJob).not_to receive(:perform_later) + + described_class.perform_now + end + end end diff --git a/spec/jobs/pages/refresh_search_index_job_spec.rb b/spec/jobs/pages/refresh_search_index_job_spec.rb index 6ac2eae2..8e163695 100644 --- a/spec/jobs/pages/refresh_search_index_job_spec.rb +++ b/spec/jobs/pages/refresh_search_index_job_spec.rb @@ -1,20 +1,24 @@ require "rails_helper" RSpec.describe Pages::RefreshSearchIndexJob, type: :job do - it "doesn’t blow up when indexing the current pages" do - Page.upsert_collection_from_sitepress! + describe Pages::RefreshSearchIndexJob::Batch do + it "doesn’t blow up when indexing the current pages" do + Page.upsert_collection_from_sitepress! + perform_enqueued_jobs - expect { described_class.perform_now }.not_to raise_error - end + expect { Pages::RefreshSearchIndexJob::Batch.perform_now }.not_to raise_error + end - it "indexes the right stuff" do - page_1 = FactoryBot.create(:page, :published, request_path: "/articles/introducing-joy-of-rails") - page_2 = FactoryBot.create(:page, :published, request_path: "/articles/custom-color-schemes-with-ruby-on-rails") - page_3 = FactoryBot.create(:page, :unpublished, request_path: "/articles/joy-of-rails-2") + it "indexes the right stuff" do + page_1 = FactoryBot.create(:page, :published, request_path: "/articles/introducing-joy-of-rails") + page_2 = FactoryBot.create(:page, :published, request_path: "/articles/custom-color-schemes-with-ruby-on-rails") + page_3 = FactoryBot.create(:page, :unpublished, request_path: "/articles/joy-of-rails-2") - described_class.perform_now + Pages::RefreshSearchIndexJob::Batch.perform_now + perform_enqueued_jobs - expect(Page.join_search_index).to include(page_1, page_2) - expect(Page.join_search_index).not_to include(page_3) + expect(Page.join_search_index).to include(page_1, page_2) + expect(Page.join_search_index).not_to include(page_3) + end end end diff --git a/spec/models/page/searchable_spec.rb b/spec/models/page/searchable_spec.rb index f5b9188a..ec742c66 100644 --- a/spec/models/page/searchable_spec.rb +++ b/spec/models/page/searchable_spec.rb @@ -4,14 +4,14 @@ describe ".search" do it "allows searching for pages" do page = FactoryBot.create(:page, :published, request_path: "/") - Pages::RefreshSearchIndexJob.perform_now + page.update_in_search_index expect(Page.search("Joy of Rails")).to include page end it "works after rebuilding index" do page = FactoryBot.create(:page, :published, request_path: "/") - Pages::RefreshSearchIndexJob.perform_now + page.update_in_search_index Page.find_each(&:update_in_search_index) @@ -23,7 +23,8 @@ it "orders search results by rank" do page_1 = FactoryBot.create(:page, :published, request_path: "/articles/custom-color-schemes-with-ruby-on-rails") page_2 = FactoryBot.create(:page, :published, request_path: "/articles/mastering-custom-configuration-in-rails") - Pages::RefreshSearchIndexJob.perform_now + page_1.update_in_search_index + page_2.update_in_search_index search = Page.search("custom con*") @@ -33,8 +34,8 @@ describe ".with_snippets" do it "orders search results by rank" do - FactoryBot.create(:page, :published, request_path: "/articles/custom-color-schemes-with-ruby-on-rails") - Pages::RefreshSearchIndexJob.perform_now + page = FactoryBot.create(:page, :published, request_path: "/articles/custom-color-schemes-with-ruby-on-rails") + page.update_in_search_index page = Page.search("Color*").with_snippets.first diff --git a/spec/requests/search_spec.rb b/spec/requests/search_spec.rb index 36031560..902d0149 100644 --- a/spec/requests/search_spec.rb +++ b/spec/requests/search_spec.rb @@ -35,8 +35,8 @@ end it "renders the search results without query" do - FactoryBot.create(:page, :published, request_path: "/pwa-showcase") - Pages::RefreshSearchIndexJob.perform_now + article = FactoryBot.create(:page, :published, request_path: "/pwa-showcase") + article.update_in_search_index get search_path @@ -46,9 +46,10 @@ end it "renders the search results with query as turbo stream" do - FactoryBot.create(:page, :published, request_path: "/pwa-showcase") - FactoryBot.create(:page, :published, request_path: "/articles/introducing-joy-of-rails") - Pages::RefreshSearchIndexJob.perform_now + article1 = FactoryBot.create(:page, :published, request_path: "/pwa-showcase") + article2 = FactoryBot.create(:page, :published, request_path: "/articles/introducing-joy-of-rails") + article1.update_in_search_index + article2.update_in_search_index get search_path(format: :turbo_stream), params: {query: "Progressive Web Apps"} @@ -63,9 +64,10 @@ end it "renders the search results with query" do - FactoryBot.create(:page, :published, request_path: "/pwa-showcase") - FactoryBot.create(:page, :published, request_path: "/articles/introducing-joy-of-rails") - Pages::RefreshSearchIndexJob.perform_now + article1 = FactoryBot.create(:page, :published, request_path: "/pwa-showcase") + article2 = FactoryBot.create(:page, :published, request_path: "/articles/introducing-joy-of-rails") + article1.update_in_search_index + article2.update_in_search_index post search_path, params: {query: "Progressive Web Apps"} diff --git a/spec/system/searches_spec.rb b/spec/system/searches_spec.rb index 53d975d6..d9d0bd5f 100644 --- a/spec/system/searches_spec.rb +++ b/spec/system/searches_spec.rb @@ -2,8 +2,8 @@ RSpec.describe "Searches", type: :system do it "show search results" do - FactoryBot.create(:page, :published, request_path: "/pwa-showcase") - Pages::RefreshSearchIndexJob.perform_now + article = FactoryBot.create(:page, :published, request_path: "/pwa-showcase") + article.update_in_search_index visit root_path