From fb81d363545489ca613ea860e8f6e7146f9baf5e Mon Sep 17 00:00:00 2001 From: Sascha Karnatz Date: Tue, 8 Oct 2024 14:32:58 +0200 Subject: [PATCH] Remove search module Remove the Search module im lib and moved the function into the main module. It is now less confusing. Also add a small test for the page index job. --- README.md | 2 +- .../alchemy/pg_search/page_extension.rb | 2 +- app/jobs/alchemy/pg_search/index_page_job.rb | 2 +- lib/alchemy-pg_search.rb | 46 +++++++++++++--- lib/alchemy/pg_search/search.rb | 55 ------------------- spec/features/fulltext_search_feature_spec.rb | 2 +- spec/jobs/index_page_job_spec.rb | 14 +++++ ...arch_spec.rb => alchemy-pg_search_spec.rb} | 26 ++++----- 8 files changed, 70 insertions(+), 79 deletions(-) delete mode 100644 lib/alchemy/pg_search/search.rb create mode 100644 spec/jobs/index_page_job_spec.rb rename spec/lib/{search_spec.rb => alchemy-pg_search_spec.rb} (86%) diff --git a/README.md b/README.md index 59184ae..07e3393 100644 --- a/README.md +++ b/README.md @@ -235,7 +235,7 @@ and reindex your database in your Rails console ```rb # rails console -$ Alchemy::PgSearch::Search.rebuild +$ Alchemy::PgSearch.rebuild ``` ## Contributing diff --git a/app/extensions/alchemy/pg_search/page_extension.rb b/app/extensions/alchemy/pg_search/page_extension.rb index 09009c9..48bc985 100644 --- a/app/extensions/alchemy/pg_search/page_extension.rb +++ b/app/extensions/alchemy/pg_search/page_extension.rb @@ -23,7 +23,7 @@ def searchable? private def remove_unpublished_page - Alchemy::PgSearch::Search.remove_page(self) unless searchable? + Alchemy::PgSearch.remove_page(self) unless searchable? end end diff --git a/app/jobs/alchemy/pg_search/index_page_job.rb b/app/jobs/alchemy/pg_search/index_page_job.rb index e1a5760..525bc44 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) - Search.index_page(page) + PgSearch.index_page(page) end end end diff --git a/lib/alchemy-pg_search.rb b/lib/alchemy-pg_search.rb index 40ffac9..c7424bc 100644 --- a/lib/alchemy-pg_search.rb +++ b/lib/alchemy-pg_search.rb @@ -1,6 +1,5 @@ require "alchemy/pg_search/engine" require "alchemy/pg_search/config" -require "alchemy/pg_search/search" module Alchemy module PgSearch @@ -16,6 +15,37 @@ def self.is_searchable?(ingredient_type) SEARCHABLE_INGREDIENTS.include?(ingredient_type.gsub(/Alchemy::Ingredients::/, "")) end + ## + # index all supported Alchemy models + def self.rebuild + [Alchemy::Page, Alchemy::Ingredient].each do |model| + ::PgSearch::Multisearch.rebuild(model) + 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) + remove_page page + + page.update_pg_search_document + page.all_elements.includes(:ingredients).find_each do |element| + element.ingredients.select { |i| Alchemy::PgSearch.is_searchable?(i.type) }.each do |ingredient| + ingredient.update_pg_search_document + end + end + end + ## # search for page results # @@ -23,13 +53,15 @@ def self.is_searchable?(ingredient_type) # @param ability [nil|CanCan::Ability] # @return [ActiveRecord::Relation] def self.search(query, ability: nil) - Search.search(query, ability: ability) - end + query = ::PgSearch.multisearch(query) + .select("JSON_AGG(content) as content", :page_id) + .reorder("") + .group(:page_id) + .joins(:page) - ## - # index all supported Alchemy models - def self.rebuild - Search.rebuild + query = query.merge(Alchemy::Page.accessible_by(ability, :read)) if ability + + query end end end diff --git a/lib/alchemy/pg_search/search.rb b/lib/alchemy/pg_search/search.rb deleted file mode 100644 index 97b401d..0000000 --- a/lib/alchemy/pg_search/search.rb +++ /dev/null @@ -1,55 +0,0 @@ -module Alchemy - module PgSearch - module Search - - ## - # index all supported Alchemy models - def self.rebuild - [Alchemy::Page, Alchemy::Ingredient].each do |model| - ::PgSearch::Multisearch.rebuild(model) - 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) - remove_page page - - page.update_pg_search_document - page.all_elements.includes(:ingredients).find_each do |element| - element.ingredients.select { |i| Alchemy::PgSearch.is_searchable?(i.type) }.each do |ingredient| - ingredient.update_pg_search_document - end - end - end - - ## - # search for page results - # - # @param query [string] - # @param ability [nil|CanCan::Ability] - # @return [ActiveRecord::Relation] - def self.search(query, ability: nil) - query = ::PgSearch.multisearch(query) - .select("JSON_AGG(content) as content", :page_id) - .reorder("") - .group(:page_id) - .joins(:page) - - query = query.merge(Alchemy::Page.accessible_by(ability, :read)) if ability - - query - end - end - end -end diff --git a/spec/features/fulltext_search_feature_spec.rb b/spec/features/fulltext_search_feature_spec.rb index c7f6df5..43e48c4 100644 --- a/spec/features/fulltext_search_feature_spec.rb +++ b/spec/features/fulltext_search_feature_spec.rb @@ -59,7 +59,7 @@ it "does not display results placed on global pages" do # A layout page is configured and the page is indexed after publish public_page.update!(layoutpage: true) - Alchemy::PgSearch::Search.index_page public_page + Alchemy::PgSearch.index_page public_page visit("/suche?query=search") expect(page).to have_css("h2.no_search_results") diff --git a/spec/jobs/index_page_job_spec.rb b/spec/jobs/index_page_job_spec.rb new file mode 100644 index 0000000..082f7e1 --- /dev/null +++ b/spec/jobs/index_page_job_spec.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Alchemy::PgSearch::IndexPageJob, type: :job do + include ActiveJob::TestHelper + + let(:page) { create(:alchemy_page, :public) } + + it "calls the index_page - method" do + expect(Alchemy::PgSearch).to receive(:index_page).with(page) + described_class.perform_now(page) + end +end diff --git a/spec/lib/search_spec.rb b/spec/lib/alchemy-pg_search_spec.rb similarity index 86% rename from spec/lib/search_spec.rb rename to spec/lib/alchemy-pg_search_spec.rb index ab7c38e..1ae1c1d 100644 --- a/spec/lib/search_spec.rb +++ b/spec/lib/alchemy-pg_search_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Alchemy::PgSearch::Search do +describe Alchemy::PgSearch do let(:page_version) { create(:alchemy_page_version, :published) } let(:ingredient_element) { create(:alchemy_element, :with_ingredients, name: "ingredient_test", public: true, page_version: page_version) } @@ -22,7 +22,7 @@ context 'after rebuild' do before do prepared_ingredients - Alchemy::PgSearch::Search.rebuild + Alchemy::PgSearch.rebuild end it 'should have entries (2 Pages + 3 Ingredients)' do @@ -39,11 +39,11 @@ context 'remove_page' do before do prepared_ingredients - Alchemy::PgSearch::Search.rebuild + Alchemy::PgSearch.rebuild end context 'remove first page' do - before { Alchemy::PgSearch::Search.remove_page first_page } + before { Alchemy::PgSearch.remove_page first_page } it 'should have only one page and relative ingredients (1 Page + 3 Ingredients)' do expect(PgSearch::Document.count).to eq(4) @@ -55,7 +55,7 @@ end context 'remove second page' do - before { Alchemy::PgSearch::Search.remove_page second_page } + before { Alchemy::PgSearch.remove_page second_page } it 'should have only one page (1 Page)' do expect(PgSearch::Document.count).to eq(1) @@ -80,7 +80,7 @@ context 'first_page' do before do - Alchemy::PgSearch::Search.index_page first_page + Alchemy::PgSearch.index_page first_page end it 'should have only one entry' do @@ -94,7 +94,7 @@ context 'second_page' do before do - Alchemy::PgSearch::Search.index_page second_page + Alchemy::PgSearch.index_page second_page end it 'should have four entries (1 Page + 3 Ingredients)' do @@ -112,7 +112,7 @@ let!(:nested_element) { create(:alchemy_element, :with_ingredients, name: "article", public: true, page_version: page_version, parent_element: ingredient_element) } before do - Alchemy::PgSearch::Search.index_page second_page + Alchemy::PgSearch.index_page second_page end it 'should have 6 documents' do @@ -130,10 +130,10 @@ context 'page searchable' do let(:searchable) { true } let!(:page) { create(:alchemy_page, :public, name: "Searchable Page", searchable: searchable) } - let(:result) { Alchemy::PgSearch::Search.search "searchable" } + let(:result) { Alchemy::PgSearch.search "searchable" } before do - Alchemy::PgSearch::Search.rebuild + Alchemy::PgSearch.rebuild end it 'should find one page' do @@ -151,12 +151,12 @@ end context 'search' do - let(:result) { Alchemy::PgSearch::Search.search "foo" } + let(:result) { Alchemy::PgSearch.search "foo" } before do create(:alchemy_page, :restricted, :public, name: "foo") prepared_ingredients - Alchemy::PgSearch::Search.rebuild + Alchemy::PgSearch.rebuild end it 'should find two pages' do @@ -165,7 +165,7 @@ context 'ability' do let(:user) { User.create(alchemy_roles: ["member"]) } - let(:result) { Alchemy::PgSearch::Search.search "foo", ability: Alchemy::Permissions.new(user) } + let(:result) { Alchemy::PgSearch.search "foo", ability: Alchemy::Permissions.new(user) } context 'with a logged in user' do it 'should find two pages' do