Don't use Current.account for search records

It doesn't actually work, and even if we could make it work reliably
we are better off if the records always know to go to the right shard.

It does make the interface a bit more complicated as we need to select
the right shard class with `for(account_id)`.

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
 

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

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)
 

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

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

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)

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

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

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

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