From 0ac1185784a7276905b92b83c19e9d3e92717a90 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Mon, 9 Dec 2024 17:45:03 +0100 Subject: [PATCH] Use PgSearch::Document.rebuild for page reindex Adds a searchable_content method to page extension as well and configure multisearch that it uses this to build its content for pg search. That way we can remove our custom rebuild implementation. --- .../alchemy/pg_search/page_extension.rb | 10 +-- .../alchemy/search/element_extension.rb | 4 ++ .../alchemy/search/page_extension.rb | 4 ++ app/jobs/alchemy/pg_search/index_page_job.rb | 2 +- lib/alchemy-pg_search.rb | 38 ++--------- spec/jobs/index_page_job_spec.rb | 4 +- spec/lib/alchemy-pg_search_spec.rb | 66 +++---------------- spec/models/element_spec.rb | 25 +++++++ spec/models/page_spec.rb | 52 +++++++++++++-- 9 files changed, 100 insertions(+), 105 deletions(-) diff --git a/app/extensions/alchemy/pg_search/page_extension.rb b/app/extensions/alchemy/pg_search/page_extension.rb index 18c5a03..43b4a47 100644 --- a/app/extensions/alchemy/pg_search/page_extension.rb +++ b/app/extensions/alchemy/pg_search/page_extension.rb @@ -6,24 +6,18 @@ def self.prepended(base) base.after_save :remove_unpublished_page base.multisearchable( against: [ - :meta_description, - :meta_keywords, :name, + :searchable_content ], additional_attributes: ->(page) { { page_id: page.id, searchable_created_at: page.public_on } }, if: :searchable?, ) end - def searchable? - (definition.key?(:searchable) ? definition[:searchable] : true) && - searchable && public? && !layoutpage? - end - private def remove_unpublished_page - Alchemy::PgSearch.remove_page(self) unless searchable? + ::PgSearch::Document.delete_by(page_id: id) unless searchable? end end diff --git a/app/extensions/alchemy/search/element_extension.rb b/app/extensions/alchemy/search/element_extension.rb index 85ff1fc..8215efc 100644 --- a/app/extensions/alchemy/search/element_extension.rb +++ b/app/extensions/alchemy/search/element_extension.rb @@ -10,6 +10,10 @@ def searchable def searchable? searchable && public? && page.searchable? && page_version.public? end + + def searchable_content + ingredients.select(&:searchable?).map(&:searchable_content).join(" ").squish + end end Alchemy::Element.prepend(Alchemy::Search::ElementExtension) diff --git a/app/extensions/alchemy/search/page_extension.rb b/app/extensions/alchemy/search/page_extension.rb index b59dc5b..d455b49 100644 --- a/app/extensions/alchemy/search/page_extension.rb +++ b/app/extensions/alchemy/search/page_extension.rb @@ -5,6 +5,10 @@ def searchable? (definition.key?(:searchable) ? definition[:searchable] : true) && searchable && public? && !layoutpage? end + + def searchable_content + all_elements.includes(ingredients: {element: :page}).map(&:searchable_content).join(" ") + end end Alchemy::Page.prepend(Alchemy::Search::PageExtension) diff --git a/app/jobs/alchemy/pg_search/index_page_job.rb b/app/jobs/alchemy/pg_search/index_page_job.rb index 525bc44..ce5baca 100644 --- a/app/jobs/alchemy/pg_search/index_page_job.rb +++ b/app/jobs/alchemy/pg_search/index_page_job.rb @@ -2,7 +2,7 @@ module Alchemy module PgSearch class IndexPageJob < BaseJob def perform(page) - PgSearch.index_page(page) + page.update_pg_search_document end end end diff --git a/lib/alchemy-pg_search.rb b/lib/alchemy-pg_search.rb index 4b7f7e1..6899928 100644 --- a/lib/alchemy-pg_search.rb +++ b/lib/alchemy-pg_search.rb @@ -17,37 +17,13 @@ module PgSearch extend Config ## - # index all supported Alchemy pages - def self.rebuild - ActiveRecord::Base.transaction do - ::PgSearch::Document.delete_all - Alchemy::Page.all.each{ |page| index_page(page) } - end - end - - ## - # remove the whole index for the page - # - # @param page [Alchemy::Page] - def self.remove_page(page) - ::PgSearch::Document.delete_by(page_id: page.id) - end - - ## - # index a single page and indexable ingredients - # - # @param page [Alchemy::Page] - def self.index_page(page) - page.update_pg_search_document - - document = page.pg_search_document - return if document.nil? - - ingredient_content = page.all_elements.includes(ingredients: {element: :page}).map do |element| - element.ingredients.select { |i| i.searchable? }.map(&:searchable_content).join(" ") - end.join(" ") - - document.update_column(:content, "#{document.content} #{ingredient_content}".squish) + # Reindex all supported Alchemy pages + def self.rebuild(clean_up: true, transactional: true) + ::PgSearch::Multisearch.rebuild( + Alchemy::Page, + clean_up: clean_up, + transactional: transactional + ) end ## diff --git a/spec/jobs/index_page_job_spec.rb b/spec/jobs/index_page_job_spec.rb index 082f7e1..155583f 100644 --- a/spec/jobs/index_page_job_spec.rb +++ b/spec/jobs/index_page_job_spec.rb @@ -7,8 +7,8 @@ let(:page) { create(:alchemy_page, :public) } - it "calls the index_page - method" do - expect(Alchemy::PgSearch).to receive(:index_page).with(page) + it "calls the update_pg_search_document - method" do + expect(page).to receive(:update_pg_search_document) described_class.perform_now(page) end end diff --git a/spec/lib/alchemy-pg_search_spec.rb b/spec/lib/alchemy-pg_search_spec.rb index de0ae4b..2f4d2b7 100644 --- a/spec/lib/alchemy-pg_search_spec.rb +++ b/spec/lib/alchemy-pg_search_spec.rb @@ -7,69 +7,23 @@ describe '#rebuild' do subject { described_class.rebuild } - it 'should have both created pages indexed documents' do + it 'should have created pages indexed documents' do expect(PgSearch::Document.count).to be(2) end - it "has 3 Pages (Root Page + 2 created pages)" do - subject - expect(PgSearch::Document.count).to be(3) - end - end - - describe "#remove_page" do - subject { described_class.remove_page(first_page) } - - it "should remove the page from search index" do - expect { subject }.to change { PgSearch::Document.count }.by(-1) - expect(first_page.reload.pg_search_document).to be_nil - end - end - - describe "#index_page" do - let!(:first_page) { create(:alchemy_page, :public, name: "Mixed", page_layout: "mixed", autogenerate_elements: true) } - let(:content) { first_page.pg_search_document.content } - - before do - PgSearch::Document.destroy_all # clean the whole index - end - - subject { described_class.index_page(first_page.reload) } - - it "creates a new pg_search document" do + it "has an additional page (Root Page + 2 created pages)" do expect { subject }.to change { PgSearch::Document.count }.by(1) end - it "has the page title as content" do - subject - expect(content).to include("Mixed ") - end - - it "has ingredient as content" do - subject - expect(content).to include("public title") - end - - it "hasn't not searchable ingredient in content" do - subject - expect(content).to_not include("secret password") - end - - it "stores stripped content from ingredient" do - subject - expect(content).to include("public richtext") - end - - it "removes whitespace from content" do - subject - expect(content).to start_with("Mixed") - end - - context "hidden page" do - let!(:first_page) { create(:alchemy_page) } + context "with parameter" do + it "calls PgSearch::Multisearch.rebuild with clean_up and transactional enabled" do + expect(::PgSearch::Multisearch).to receive(:rebuild).with(Alchemy::Page, clean_up: true, transactional: true) + subject + end - it "creates nothing" do - expect { subject }.to change { PgSearch::Document.count }.by(0) + it "calls PgSearch::Multisearch.rebuild with clean_up and transactional disabled" do + expect(::PgSearch::Multisearch).to receive(:rebuild).with(Alchemy::Page, clean_up: false, transactional: false) + described_class.rebuild(clean_up: false, transactional: false) end end end diff --git a/spec/models/element_spec.rb b/spec/models/element_spec.rb index 84cfb49..af51fcd 100644 --- a/spec/models/element_spec.rb +++ b/spec/models/element_spec.rb @@ -72,4 +72,29 @@ end end end + + describe "searchable_content" do + let(:element) do + page_version = create(:alchemy_page_version, :published) + create(:alchemy_element, :with_ingredients, name: "ingredient_test", public: true, page_version: page_version) + end + let!(:first_ingredient) { create(:alchemy_ingredient_headline, value: "foo bar", element: element) } + + subject { element.searchable_content } + + it "should contain ingredient content" do + is_expected.to eq("foo bar") + end + + context "ignore not searchable elements" do + before do + element.public = false + element.save + end + + it "should not find the unsearchable content" do + is_expected.to_not include("foo") + end + end + end end diff --git a/spec/models/page_spec.rb b/spec/models/page_spec.rb index 5f3e814..203f5bc 100644 --- a/spec/models/page_spec.rb +++ b/spec/models/page_spec.rb @@ -40,9 +40,10 @@ PgSearch::Document.create(page_id: page.id, content: "foo") end + subject { page.save } + it "should not remove the document, if the page is searchable" do - page.save - expect(PgSearch::Document.count).to eq(1) + expect { subject }.to change { PgSearch::Document.count }.by(0) end context "but configured as not searchable" do @@ -55,19 +56,34 @@ end it "should remove the document" do - page.save - expect(PgSearch::Document.count).to eq(0) + expect { subject }.to change { PgSearch::Document.count }.by(-1) end end - describe "unpublished page" do + context "unpublished page" do let(:page) { create(:alchemy_page) } - it "should remove the document, if the page is not searchable" do - page.save + it "should not store the page, if it is not searchable" do expect(PgSearch::Document.count).to eq(0) end end + + context "stored content" do + let!(:page) { create(:alchemy_page, :public, name: "Searchable Page", page_layout: "mixed", autogenerate_elements: true) } + let(:content) { page.pg_search_document.content } + + before do + subject + end + + it "should store the page name" do + expect(content).to start_with("Searchable Page") + end + + it "should store the searchable_content" do + expect(content).to include("public title") + end + end end describe "additional_attributes" do @@ -79,4 +95,26 @@ expect(page.pg_search_document.searchable_created_at).to eq(page.public_on) end end + + describe "searchable_content" do + let!(:searchable_page) { create(:alchemy_page, :public, name: "Searchable Page", page_layout: "mixed", autogenerate_elements: true) } + + before do + PgSearch::Document.destroy_all # clean the whole index + end + + subject { searchable_page.searchable_content } + + it "has ingredients" do + expect(subject).to include("public title") + end + + it "hasn't not searchable ingredient" do + expect(subject).to_not include("secret password") + end + + it "stores stripped content from ingredient" do + expect(subject).to include("public richtext") + end + end end