diff --git a/app/models/card/searchable.rb b/app/models/card/searchable.rb index caf9b4fea..8cc108b23 100644 --- a/app/models/card/searchable.rb +++ b/app/models/card/searchable.rb @@ -4,10 +4,9 @@ module Card::Searchable included do include ::Searchable - has_many :card_search_records, class_name: "Search::Record" - scope :mentioning, ->(query, user:) do - joins(:card_search_records).merge(Search::Record.for_query(query, user: user)) + search_record_class = Search::Record.for(user.account_id) + joins(search_record_class.card_join).merge(search_record_class.for_query(query, user: user)) end end diff --git a/app/models/concerns/searchable.rb b/app/models/concerns/searchable.rb index 894c23b94..8b39504ef 100644 --- a/app/models/concerns/searchable.rb +++ b/app/models/concerns/searchable.rb @@ -2,9 +2,6 @@ module Searchable extend ActiveSupport::Concern included do - has_one :search_record, class_name: "Search::Record", as: :searchable - - before_save :ensure_current_account after_create_commit :create_in_search_index after_update_commit :update_in_search_index after_destroy_commit :remove_from_search_index @@ -15,22 +12,16 @@ module Searchable end private - def ensure_current_account - unless Current.account - raise "Current.account must be set to save #{self.class.name}" - end - end - def create_in_search_index - create_search_record!(search_record_attributes) + search_record_class.create!(search_record_attributes) end def update_in_search_index - search_record&.update!(search_record_attributes) || create_in_search_index + search_record_class.upsert!(search_record_attributes) end def remove_from_search_index - search_record&.destroy + search_record_class.find_by(searchable_type: self.class.name, searchable_id: id)&.destroy end def search_record_attributes @@ -46,6 +37,10 @@ module Searchable } end + def search_record_class + Search::Record.for(account_id) + end + # Models must implement these methods: # - account_id: returns the account id # - search_title: returns title string or nil diff --git a/app/models/search/record.rb b/app/models/search/record.rb index 424a40363..2aaff7c24 100644 --- a/app/models/search/record.rb +++ b/app/models/search/record.rb @@ -6,6 +6,22 @@ class Search::Record < ApplicationRecord validates :account_id, :searchable_type, :searchable_id, :card_id, :board_id, :created_at, presence: true + class << self + def upsert!(attributes) + record = find_by(searchable_type: attributes[:searchable_type], searchable_id: attributes[:searchable_id]) + if record + record.update!(attributes) + record + else + create!(attributes) + end + end + + def card_join + "INNER JOIN #{table_name} ON #{table_name}.card_id = cards.id" + end + end + scope :for_query, ->(query, user:) do query = Search::Query.wrap(query) diff --git a/app/models/search/record/sqlite.rb b/app/models/search/record/sqlite.rb index 85aeb59f2..c3da9131a 100644 --- a/app/models/search/record/sqlite.rb +++ b/app/models/search/record/sqlite.rb @@ -25,6 +25,10 @@ module Search::Record::SQLite "snippet(search_records_fts, 1, #{opening_mark}, #{closing_mark}, #{ellipsis}, 20) AS result_content", "#{connection.quote(query.terms)} AS query" ] end + + def for(account_id) + self + end end def card_title diff --git a/app/models/search/record/trilogy.rb b/app/models/search/record/trilogy.rb index dd10334c8..7705f3b71 100644 --- a/app/models/search/record/trilogy.rb +++ b/app/models/search/record/trilogy.rb @@ -4,23 +4,26 @@ module Search::Record::Trilogy SHARD_COUNT = 16 included do + self.abstract_class = true before_save :set_account_key, :stem_content scope :matching, ->(query, account_id) do full_query = "+account#{account_id} +(#{Search::Stemmer.stem(query)})" where("MATCH(#{table_name}.account_key, #{table_name}.content, #{table_name}.title) AGAINST(? IN BOOLEAN MODE)", full_query) end + + SHARD_CLASSES = SHARD_COUNT.times.map do |shard_id| + Class.new(self) do + self.table_name = "search_records_#{shard_id}" + + def self.name + "Search::Record" + end + end + end.freeze end class_methods do - def compute_table_name - if Current.account - "search_records_#{shard_id_for_account(Current.account.id)}" - else - "search_records_0" - end - end - def shard_id_for_account(account_id) Zlib.crc32(account_id.to_s) % SHARD_COUNT end @@ -28,6 +31,10 @@ module Search::Record::Trilogy def search_fields(query) "#{connection.quote(query.terms)} AS query" end + + def for(account_id) + SHARD_CLASSES[shard_id_for_account(account_id)] + end end def card_title diff --git a/app/models/user/searcher.rb b/app/models/user/searcher.rb index 60a901ee8..090adf4b8 100644 --- a/app/models/user/searcher.rb +++ b/app/models/user/searcher.rb @@ -6,7 +6,7 @@ module User::Searcher end def search(terms) - Search::Record.search(terms, user: self) + Search::Record.for(account_id).search(terms, user: self) end def remember_search(terms) diff --git a/lib/tasks/search.rake b/lib/tasks/search.rake index df39c3a70..6b47dbde3 100644 --- a/lib/tasks/search.rake +++ b/lib/tasks/search.rake @@ -12,16 +12,10 @@ namespace :search do end puts "Reindexing cards..." - Card.find_each do |card| - Current.account = card.account - card.reindex - end + Card.includes(:rich_text_description).find_each(&:reindex) puts "Reindexing comments..." - Comment.find_each do |comment| - Current.account = comment.account - comment.reindex - end + Comment.includes(:rich_text_body, :card).find_each(&:reindex) puts "Done! Reindexed #{Card.count} cards and #{Comment.count} comments." end diff --git a/test/models/card/searchable_test.rb b/test/models/card/searchable_test.rb index c0a99ab23..d6dcdf60c 100644 --- a/test/models/card/searchable_test.rb +++ b/test/models/card/searchable_test.rb @@ -32,25 +32,16 @@ class Card::SearchableTest < ActiveSupport::TestCase assert_not_includes results, card_in_other_board end - test "card requires Current.account to be set" do - Current.account = nil - - error = assert_raises(RuntimeError) do - @board.cards.create!(title: "Test card", creator: @user) - end - - assert_match(/Current.account must be set to save Card/, error.message) - end - test "deleting card removes search record and FTS entry" do + search_record_class = Search::Record.for(@user.account_id) card = @board.cards.create!(title: "Card to delete", creator: @user) # Verify search record exists - search_record = Search::Record.find_by(searchable_type: "Card", searchable_id: card.id) + search_record = search_record_class.find_by(searchable_type: "Card", searchable_id: card.id) assert_not_nil search_record, "Search record should exist after card creation" # For SQLite, verify FTS entry exists - if Search::Record.connection.adapter_name == "SQLite" + if search_record_class.connection.adapter_name == "SQLite" fts_entry = search_record.search_records_fts assert_not_nil fts_entry, "FTS entry should exist" assert_equal card.title, fts_entry.title @@ -60,11 +51,11 @@ class Card::SearchableTest < ActiveSupport::TestCase card.destroy # Verify search record is deleted - search_record = Search::Record.find_by(searchable_type: "Card", searchable_id: card.id) + search_record = search_record_class.find_by(searchable_type: "Card", searchable_id: card.id) assert_nil search_record, "Search record should be deleted after card deletion" # For SQLite, verify FTS entry is deleted - if Search::Record.connection.adapter_name == "SQLite" + if search_record_class.connection.adapter_name == "SQLite" fts_count = Search::Record::SQLite::Fts.where(rowid: card.id).count assert_equal 0, fts_count, "FTS entry should be deleted" end diff --git a/test/models/comment/searchable_test.rb b/test/models/comment/searchable_test.rb index b8b77a8b0..7ba73847c 100644 --- a/test/models/comment/searchable_test.rb +++ b/test/models/comment/searchable_test.rb @@ -8,14 +8,15 @@ class Comment::SearchableTest < ActiveSupport::TestCase end test "comment search" do + search_record_class = Search::Record.for(@user.account_id) # Comment is indexed on create comment = @card.comments.create!(body: "searchable comment text", creator: @user) - record = Search::Record.find_by(searchable_type: "Comment", searchable_id: comment.id) + record = search_record_class.find_by(searchable_type: "Comment", searchable_id: comment.id) assert_not_nil record # Comment is updated in index comment.update!(body: "updated text") - record = Search::Record.find_by(searchable_type: "Comment", searchable_id: comment.id) + record = search_record_class.find_by(searchable_type: "Comment", searchable_id: comment.id) assert_match /updat/, record.content # Comment is removed from index on destroy @@ -23,17 +24,17 @@ class Comment::SearchableTest < ActiveSupport::TestCase search_record_id = record.id # For SQLite, verify FTS entry exists before deletion - if Search::Record.connection.adapter_name == "SQLite" + if search_record_class.connection.adapter_name == "SQLite" fts_entry = record.search_records_fts assert_not_nil fts_entry, "FTS entry should exist before comment deletion" end comment.destroy - record = Search::Record.find_by(searchable_type: "Comment", searchable_id: comment_id) + record = search_record_class.find_by(searchable_type: "Comment", searchable_id: comment_id) assert_nil record # For SQLite, verify FTS entry is also deleted - if Search::Record.connection.adapter_name == "SQLite" + if search_record_class.connection.adapter_name == "SQLite" fts_count = Search::Record::SQLite::Fts.where(rowid: search_record_id).count assert_equal 0, fts_count, "FTS entry should be deleted after comment deletion" end @@ -48,18 +49,8 @@ class Comment::SearchableTest < ActiveSupport::TestCase # Comment stores parent card_id and board_id new_comment = @card.comments.create!(body: "test comment", creator: @user) - record = Search::Record.find_by(searchable_type: "Comment", searchable_id: new_comment.id) + record = search_record_class.find_by(searchable_type: "Comment", searchable_id: new_comment.id) assert_equal @card.id, record.card_id assert_equal @board.id, record.board_id end - - test "comment requires Current.account to be set" do - Current.account = nil - - error = assert_raises(RuntimeError) do - @card.comments.create!(body: "Test comment", creator: @user) - end - - assert_match(/Current.account must be set to save Comment/, error.message) - end end diff --git a/test/models/search_test.rb b/test/models/search_test.rb index be4174ff8..1f4981090 100644 --- a/test/models/search_test.rb +++ b/test/models/search_test.rb @@ -9,10 +9,10 @@ class SearchTest < ActiveSupport::TestCase comment_card = @board.cards.create!(title: "Some card", creator: @user) comment_card.comments.create!(body: "overflowing text", creator: @user) - results = Search::Record.search("layout", user: @user) + results = Search::Record.for(@user.account_id).search("layout", user: @user) assert results.find { |it| it.card_id == card.id } - results = Search::Record.search("overflowing", user: @user) + results = Search::Record.for(@user.account_id).search("overflowing", user: @user) assert results.find { |it| it.card_id == comment_card.id && it.searchable_type == "Comment" } # Don't include inaccessible boards @@ -21,13 +21,13 @@ class SearchTest < ActiveSupport::TestCase accessible_card = @board.cards.create!(title: "searchable content", creator: @user) inaccessible_card = inaccessible_board.cards.create!(title: "searchable content", creator: other_user) - results = Search::Record.search("searchable", user: @user) + results = Search::Record.for(@user.account_id).search("searchable", user: @user) assert results.find { |it| it.card_id == accessible_card.id } assert_not results.find { |it| it.card_id == inaccessible_card.id } # Empty board_ids returns no results user_without_access = User.create!(name: "No Access User", account: @account) - results = Search::Record.search("anything", user: user_without_access) + results = Search::Record.for(user_without_access.account_id).search("anything", user: user_without_access) assert_empty results end end