Merge SQLite-maintained bot pagination counts from PR #337

app/controllers/messages/by_bots_controller.rb

@@ -37,7 +37,7 @@ class Messages::ByBotsController < MessagesController
     end
 
     def set_pagination_headers
-      headers["X-Total-Count"] = @room.messages.count.to_s
+      headers["X-Total-Count"] = @room.messages_count.to_s
 
       if next_page = next_page_params
         headers["Link"] = %(<#{room_bot_messages_url(@room, params[:bot_key], **next_page)}>; rel="next")

app/models/room/messages_count.rb

@@ -0,0 +1,96 @@
+# Keeps rooms.messages_count correct for every SQLite writer — Rails, bulk SQL,
+# and foreign connections — without ActiveRecord counter_cache callbacks that
+# those paths skip (and that would double-count if combined with triggers).
+class Room::MessagesCount
+  INSERT_TRIGGER = "messages_ai_rooms_messages_count"
+  DELETE_TRIGGER = "messages_ad_rooms_messages_count"
+  UPDATE_TRIGGER = "messages_au_rooms_messages_count"
+  TRIGGERS = [ INSERT_TRIGGER, DELETE_TRIGGER, UPDATE_TRIGGER ].freeze
+
+  class << self
+    def install!(connection = ActiveRecord::Base.connection)
+      with_immediate_write(connection) do
+        replace_triggers!(connection)
+      end
+    end
+
+    def uninstall!(connection = ActiveRecord::Base.connection)
+      TRIGGERS.each do |name|
+        connection.execute("DROP TRIGGER IF EXISTS #{name}")
+      end
+    end
+
+    def backfill!(connection = ActiveRecord::Base.connection)
+      connection.execute <<~SQL
+        UPDATE rooms SET messages_count = (
+          SELECT COUNT(*) FROM messages WHERE messages.room_id = rooms.id
+        )
+      SQL
+    end
+
+    # schema.rb does not dump SQLite triggers; reinstall after schema:load.
+    # Repair rechecks, rebuilds counts, and installs triggers in one write txn
+    # so concurrent writers and concurrent boot repairs cannot observe a gap.
+    def ensure!(connection = ActiveRecord::Base.connection)
+      return unless connection.adapter_name.match?(/sqlite/i)
+      return unless connection.data_source_exists?(:rooms)
+      return unless connection.column_exists?(:rooms, :messages_count)
+      return if triggers_installed?(connection)
+
+      with_immediate_write(connection) do
+        next if triggers_installed?(connection)
+
+        uninstall!(connection)
+        backfill!(connection)
+        create_triggers!(connection)
+      end
+    end
+
+    def trigger_installed?(connection, name)
+      connection.select_value(
+        "SELECT 1 FROM sqlite_master WHERE type = 'trigger' AND name = #{connection.quote(name)}"
+      ).present?
+    end
+
+    def triggers_installed?(connection = ActiveRecord::Base.connection)
+      TRIGGERS.all? { |name| trigger_installed?(connection, name) }
+    end
+
+    private
+      def with_immediate_write(connection)
+        if connection.transaction_open?
+          yield
+        else
+          connection.raw_connection.transaction(:immediate) { yield }
+        end
+      end
+
+      def replace_triggers!(connection)
+        uninstall!(connection)
+        create_triggers!(connection)
+      end
+
+      def create_triggers!(connection)
+        connection.execute <<~SQL
+          CREATE TRIGGER #{INSERT_TRIGGER} AFTER INSERT ON messages
+          BEGIN
+            UPDATE rooms SET messages_count = messages_count + 1 WHERE id = NEW.room_id;
+          END
+        SQL
+        connection.execute <<~SQL
+          CREATE TRIGGER #{DELETE_TRIGGER} AFTER DELETE ON messages
+          BEGIN
+            UPDATE rooms SET messages_count = messages_count - 1 WHERE id = OLD.room_id;
+          END
+        SQL
+        connection.execute <<~SQL
+          CREATE TRIGGER #{UPDATE_TRIGGER} AFTER UPDATE OF room_id ON messages
+          WHEN OLD.room_id IS NOT NEW.room_id
+          BEGIN
+            UPDATE rooms SET messages_count = messages_count - 1 WHERE id = OLD.room_id;
+            UPDATE rooms SET messages_count = messages_count + 1 WHERE id = NEW.room_id;
+          END
+        SQL
+      end
+  end
+end

config/initializers/room_messages_count.rb

@@ -0,0 +1,5 @@
+# schema.rb cannot dump SQLite triggers. Cover existing DBs that lost them.
+Rails.application.config.after_initialize do
+  Room::MessagesCount.ensure!
+rescue ActiveRecord::NoDatabaseError, ActiveRecord::ConnectionNotEstablished
+end

db/migrate/20261007190000_add_messages_count_to_rooms.rb

@@ -0,0 +1,15 @@
+class AddMessagesCountToRooms < ActiveRecord::Migration[8.2]
+  def up
+    add_column :rooms, :messages_count, :integer, null: false, default: 0
+
+    # Backfill before installing triggers so the COUNT rewrite does not race with
+    # concurrent inserts, and so we never rely on Rails callbacks for the tally.
+    Room::MessagesCount.backfill!(connection)
+    Room::MessagesCount.install!(connection)
+  end
+
+  def down
+    Room::MessagesCount.uninstall!(connection)
+    remove_column :rooms, :messages_count
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[8.2].define(version: 2026_10_05_022000) do
+ActiveRecord::Schema[8.2].define(version: 2026_10_07_190000) do
   create_table "accounts", force: :cascade do |t|
     t.datetime "created_at", null: false
     t.text "custom_styles"
@@ -124,6 +124,7 @@ ActiveRecord::Schema[8.2].define(version: 2026_10_05_022000) do
     t.string "name"
     t.string "type", null: false
     t.datetime "updated_at", null: false
+    t.integer "messages_count", default: 0, null: false
   end
 
   create_table "searches", force: :cascade do |t|
@@ -183,3 +184,4 @@ ActiveRecord::Schema[8.2].define(version: 2026_10_05_022000) do
   # Note that virtual tables may not work with other database engines. Be careful if changing database.
   create_virtual_table "message_search_index", "fts5", ["body", "tokenize=porter"]
 end
+

lib/tasks/room_messages_count.rake

@@ -0,0 +1,16 @@
+# schema.rb cannot dump SQLite triggers — reinstall after schema loads.
+namespace :room_messages_count do
+  task ensure: :environment do
+    ActiveRecord::Base.connection_pool.with_connection do |connection|
+      Room::MessagesCount.ensure!(connection)
+    end
+  end
+end
+
+%w[db:schema:load db:test:load_schema].each do |task_name|
+  next unless Rake::Task.task_defined?(task_name)
+
+  Rake::Task[task_name].enhance do
+    Rake::Task["room_messages_count:ensure"].invoke
+  end
+end

test/controllers/messages/by_bots_controller_test.rb

@@ -1,6 +1,9 @@
 require "test_helper"
+require "active_record/testing/query_assertions"
 
 class Messages::ByBotsControllerTest < ActionDispatch::IntegrationTest
+  include ActiveRecord::Assertions::QueryAssertions
+
   setup do
     @room = rooms(:watercooler)
   end
@@ -93,7 +96,9 @@ class Messages::ByBotsControllerTest < ActionDispatch::IntegrationTest
       @room.messages.create!(body: "Filler #{i}", creator: users(:jason), client_message_id: "filler-#{i}")
     end
 
-    get room_bot_messages_url(@room, users(:bender).bot_key)
+    assert_no_queries_match(/SELECT COUNT\(\*\) FROM "messages"/i) do
+      get room_bot_messages_url(@room, users(:bender).bot_key)
+    end
     assert_response :success
 
     json = JSON.parse(response.body)
@@ -119,7 +124,10 @@ class Messages::ByBotsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "index in a room with no messages" do
-    get room_bot_messages_url(rooms(:bender_and_kevin), users(:bender).bot_key)
+    room = rooms(:bender_and_kevin)
+    assert_equal 0, room.messages_count
+
+    get room_bot_messages_url(room, users(:bender).bot_key)
     assert_response :success
 
     assert_equal [], JSON.parse(response.body)

test/models/room/messages_count_lifecycle_test.rb

@@ -0,0 +1,237 @@
+require "test_helper"
+require "sqlite3"
+
+# Destructive trigger DDL and foreign connections need their own file so parallel
+# workers (and transactional tests in messages_count_test) keep a stable trigger set.
+class Room::MessagesCountLifecycleTest < ActiveSupport::TestCase
+  self.use_transactional_tests = false
+
+  setup do
+    Room::MessagesCount.ensure!
+    @room = rooms(:designers)
+    @other_room = rooms(:pets)
+    Room::MessagesCount.backfill!
+    @room.reload
+    @other_room.reload
+  end
+
+  teardown do
+    Message.where("client_message_id LIKE ?", "count-%").delete_all
+    Room::MessagesCount.ensure!
+    Room::MessagesCount.backfill!
+  end
+
+  test "ensure! installs missing triggers and keeps accurate counts" do
+    before = @room.reload.messages_count
+    Room::MessagesCount.uninstall!
+
+    assert_not Room::MessagesCount.triggers_installed?
+
+    Room::MessagesCount.ensure!
+
+    assert Room::MessagesCount.triggers_installed?
+    assert_equal before, @room.reload.messages_count
+    assert_equal @room.messages.count, @room.messages_count
+
+    assert_difference -> { @room.reload.messages_count }, +1 do
+      @room.messages.create!(creator: users(:jason), body: "After ensure", client_message_id: "count-ensure")
+    end
+  end
+
+  test "ensure! repairs a partial trigger install and backfills drifted counts" do
+    Room::MessagesCount.uninstall!
+    ActiveRecord::Base.connection.execute <<~SQL
+      CREATE TRIGGER #{Room::MessagesCount::INSERT_TRIGGER} AFTER INSERT ON messages
+      BEGIN
+        UPDATE rooms SET messages_count = messages_count + 1 WHERE id = NEW.room_id;
+      END
+    SQL
+
+    assert Room::MessagesCount.trigger_installed?(ActiveRecord::Base.connection, Room::MessagesCount::INSERT_TRIGGER)
+    assert_not Room::MessagesCount.trigger_installed?(ActiveRecord::Base.connection, Room::MessagesCount::DELETE_TRIGGER)
+
+    # Delete fires with no delete trigger → counter drifts high.
+    drifted = @room.messages.create!(creator: users(:jason), body: "Drift", client_message_id: "count-partial-drift")
+    ActiveRecord::Base.connection.execute("DELETE FROM messages WHERE id = #{drifted.id}")
+    assert_operator @room.reload.messages_count, :>, @room.messages.count
+
+    Room::MessagesCount.ensure!
+
+    assert Room::MessagesCount.triggers_installed?
+    assert_equal @room.messages.count, @room.reload.messages_count
+  end
+
+  test "ensure! backfills writes that landed while the insert trigger was missing" do
+    Room::MessagesCount.uninstall!
+    before_count = @room.messages.count
+    before_cached = @room.reload.messages_count
+    assert_equal before_count, before_cached
+
+    path = File.expand_path(ActiveRecord::Base.connection_db_config.database)
+    now = Time.current.utc.strftime("%Y-%m-%d %H:%M:%S.%6N")
+    ActiveRecord::Base.connection_pool.release_connection
+
+    SQLite3::Database.new(path) do |db|
+      db.busy_timeout = 5_000
+      db.execute(
+        "INSERT INTO messages (room_id, creator_id, client_message_id, created_at, updated_at) VALUES (?, ?, ?, ?, ?)",
+        [ @room.id, users(:david).id, "count-missing-insert", now, now ]
+      )
+    end
+
+    assert_equal before_count + 1, @room.messages.count
+    assert_equal before_cached, @room.reload.messages_count
+
+    Room::MessagesCount.ensure!
+
+    assert Room::MessagesCount.triggers_installed?
+    assert_equal @room.messages.count, @room.reload.messages_count
+    assert_equal before_cached + 1, @room.messages_count
+  end
+
+  test "concurrent ensure! repairs leave triggers installed and counts accurate" do
+    Room::MessagesCount.uninstall!
+    path = File.expand_path(ActiveRecord::Base.connection_db_config.database)
+    now = Time.current.utc.strftime("%Y-%m-%d %H:%M:%S.%6N")
+    ActiveRecord::Base.connection_pool.release_connection
+
+    SQLite3::Database.new(path) do |db|
+      db.busy_timeout = 5_000
+      db.execute(
+        "INSERT INTO messages (room_id, creator_id, client_message_id, created_at, updated_at) VALUES (?, ?, ?, ?, ?)",
+        [ @room.id, users(:david).id, "count-concurrent-drift", now, now ]
+      )
+    end
+
+    assert_not_equal @room.messages.count, @room.reload.messages_count
+
+    errors = []
+    errors_mutex = Mutex.new
+
+    workers = 2.times.map do
+      Thread.new do
+        ActiveRecord::Base.connection_pool.with_connection do |connection|
+          Room::MessagesCount.ensure!(connection)
+        end
+      rescue => error
+        errors_mutex.synchronize { errors << error }
+      end
+    end
+
+    workers.each(&:join)
+
+    assert_empty errors, -> { errors.map(&:full_message).join("\n") }
+    assert Room::MessagesCount.triggers_installed?
+    assert_equal @room.messages.count, @room.reload.messages_count
+  end
+
+  test "writes concurrent with ensure! repair leave accurate counts" do
+    Room::MessagesCount.uninstall!
+    path = File.expand_path(ActiveRecord::Base.connection_db_config.database)
+    now = Time.current.utc.strftime("%Y-%m-%d %H:%M:%S.%6N")
+    ActiveRecord::Base.connection_pool.release_connection
+
+    errors = []
+    errors_mutex = Mutex.new
+
+    ensure_thread = Thread.new do
+      ActiveRecord::Base.connection_pool.with_connection do |connection|
+        Room::MessagesCount.ensure!(connection)
+      end
+    rescue => error
+      errors_mutex.synchronize { errors << error }
+    end
+
+    insert_thread = Thread.new do
+      SQLite3::Database.new(path) do |db|
+        db.busy_timeout = 10_000
+        db.execute(
+          "INSERT INTO messages (room_id, creator_id, client_message_id, created_at, updated_at) VALUES (?, ?, ?, ?, ?)",
+          [ @room.id, users(:david).id, "count-during-repair", now, now ]
+        )
+      end
+    rescue => error
+      errors_mutex.synchronize { errors << error }
+    end
+
+    [ ensure_thread, insert_thread ].each(&:join)
+
+    assert_empty errors, -> { errors.map(&:full_message).join("\n") }
+    assert Room::MessagesCount.triggers_installed?
+    assert_equal @room.messages.count, @room.reload.messages_count
+    assert Message.exists?(client_message_id: "count-during-repair")
+  end
+
+  test "install! never exposes a partial trigger set to other connections" do
+    Room::MessagesCount.uninstall!
+    path = File.expand_path(ActiveRecord::Base.connection_db_config.database)
+    ActiveRecord::Base.connection_pool.release_connection
+
+    stop = false
+    partial_snapshots = []
+    snapshots_mutex = Mutex.new
+
+    watcher = Thread.new do
+      SQLite3::Database.new(path) do |db|
+        db.busy_timeout = 5_000
+        until stop
+          names = db.execute(
+            "SELECT name FROM sqlite_master WHERE type = 'trigger' AND name IN (#{Room::MessagesCount::TRIGGERS.map { "'#{it}'" }.join(", ")})"
+          ).flatten
+          if names.any? && names.sort != Room::MessagesCount::TRIGGERS.sort
+            snapshots_mutex.synchronize { partial_snapshots << names.sort }
+          end
+        end
+      end
+    end
+
+    25.times { Room::MessagesCount.install! }
+    stop = true
+    watcher.join
+
+    assert_empty partial_snapshots
+    assert Room::MessagesCount.triggers_installed?
+  end
+
+  test "foreign SQLite connections keep the counter in step" do
+    path = File.expand_path(ActiveRecord::Base.connection_db_config.database)
+    now = Time.current.utc.strftime("%Y-%m-%d %H:%M:%S.%6N")
+    before = @room.reload.messages_count
+    other_before = @other_room.reload.messages_count
+
+    ActiveRecord::Base.connection_pool.release_connection
+
+    SQLite3::Database.new(path) do |db|
+      db.busy_timeout = 5_000
+      db.execute(
+        "INSERT INTO messages (room_id, creator_id, client_message_id, created_at, updated_at) VALUES (?, ?, ?, ?, ?)",
+        [ @room.id, users(:david).id, "count-foreign-insert", now, now ]
+      )
+    end
+
+    assert_equal before + 1, @room.reload.messages_count
+    assert_equal @room.messages.count, @room.messages_count
+
+    foreign_id = Message.find_by!(client_message_id: "count-foreign-insert").id
+
+    ActiveRecord::Base.connection_pool.release_connection
+
+    SQLite3::Database.new(path) do |db|
+      db.busy_timeout = 5_000
+      db.execute("UPDATE messages SET room_id = ? WHERE id = ?", [ @other_room.id, foreign_id ])
+    end
+
+    assert_equal before, @room.reload.messages_count
+    assert_equal other_before + 1, @other_room.reload.messages_count
+
+    ActiveRecord::Base.connection_pool.release_connection
+
+    SQLite3::Database.new(path) do |db|
+      db.busy_timeout = 5_000
+      db.execute("DELETE FROM messages WHERE id = ?", [ foreign_id ])
+    end
+
+    assert_equal before, @room.reload.messages_count
+    assert_equal other_before, @other_room.reload.messages_count
+  end
+end

test/models/room/messages_count_test.rb

@@ -0,0 +1,86 @@
+require "test_helper"
+
+class Room::MessagesCountTest < ActiveSupport::TestCase
+  setup do
+    @room = rooms(:designers)
+    @other_room = rooms(:pets)
+  end
+
+  test "fixture rooms start with an accurate messages_count" do
+    [ rooms(:watercooler), rooms(:designers), rooms(:bender_and_kevin) ].each do |room|
+      assert_equal room.messages.count, room.messages_count, "#{room.name || room.id} fixture count"
+    end
+  end
+
+  test "ActiveRecord create and destroy adjust the counter once" do
+    assert_difference -> { @room.reload.messages_count }, +1 do
+      assert_difference -> { @room.messages.count }, +1 do
+        @room.messages.create!(creator: users(:jason), body: "Hello", client_message_id: "count-ar-create")
+      end
+    end
+
+    assert_equal @room.messages.count, @room.reload.messages_count
+
+    assert_difference -> { @room.reload.messages_count }, -1 do
+      assert_difference -> { @room.messages.count }, -1 do
+        @room.messages.order(:id).last.destroy
+      end
+    end
+
+    assert_equal @room.messages.count, @room.reload.messages_count
+  end
+
+  test "bulk insert_all and delete_all keep the counter in step" do
+    rows = Array.new(3) do |i|
+      {
+        room_id: @room.id,
+        creator_id: users(:david).id,
+        client_message_id: "count-bulk-#{i}",
+        created_at: Time.current,
+        updated_at: Time.current
+      }
+    end
+
+    assert_difference -> { @room.reload.messages_count }, +3 do
+      Message.insert_all!(rows)
+    end
+
+    assert_equal @room.messages.count, @room.reload.messages_count
+
+    assert_difference -> { @room.reload.messages_count }, -3 do
+      @room.messages.where(client_message_id: rows.map { it[:client_message_id] }).delete_all
+    end
+
+    assert_equal @room.messages.count, @room.reload.messages_count
+  end
+
+  test "rolled back writes leave the counter unchanged" do
+    before = @room.reload.messages_count
+
+    Message.transaction do
+      @room.messages.create!(creator: users(:jason), body: "Nope", client_message_id: "count-rollback")
+      raise ActiveRecord::Rollback
+    end
+
+    assert_equal before, @room.reload.messages_count
+    assert_nil Message.find_by(client_message_id: "count-rollback")
+  end
+
+  test "moving a message between rooms moves the counter" do
+    message = @room.messages.create!(creator: users(:jason), body: "Move me", client_message_id: "count-move")
+
+    assert_difference -> { @room.reload.messages_count }, -1 do
+      assert_difference -> { @other_room.reload.messages_count }, +1 do
+        message.update!(room: @other_room)
+      end
+    end
+
+    assert_equal @room.messages.count, @room.reload.messages_count
+    assert_equal @other_room.messages.count, @other_room.reload.messages_count
+  end
+
+  test "Message does not declare an ActiveRecord counter_cache" do
+    reflection = Message.reflect_on_association(:room)
+    assert_not reflection.counter_cache_column
+  end
+end

test/test_helper.rb

@@ -33,4 +33,13 @@ class ActiveSupport::TestCase
   teardown do
     WebMock.reset!
   end
+
+  # fixtures :all inserts messages before rooms, so INSERT triggers cannot count
+  # yet. Backfill once after load; ensure! covers schema.rb (triggers not dumped).
+  def load_fixtures(config)
+    fixtures = super
+    Room::MessagesCount.ensure!
+    Room::MessagesCount.backfill!
+    fixtures
+  end
 end