Simplify messages_count trigger install and harden ensure!

Require all three SQLite triggers before treating the counter as installed,
drop the schema.rb / dump-rewrite install paths in favor of rake ensure after
schema load, isolate destructive trigger tests, and cover foreign room moves
plus fixture baseline counts.

Co-authored-by: Thomas Klemm <github@tklemm.eu>

app/models/room/messages_count.rb

@@ -5,6 +5,7 @@ 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)
@@ -32,7 +33,7 @@ class Room::MessagesCount
     end
 
     def uninstall!(connection = ActiveRecord::Base.connection)
-      [ INSERT_TRIGGER, DELETE_TRIGGER, UPDATE_TRIGGER ].each do |name|
+      TRIGGERS.each do |name|
         connection.execute("DROP TRIGGER IF EXISTS #{name}")
       end
     end
@@ -47,9 +48,10 @@ class Room::MessagesCount
 
     # schema.rb does not dump SQLite triggers; reinstall after schema:load.
     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 trigger_installed?(connection, INSERT_TRIGGER)
+      return if TRIGGERS.all? { |name| trigger_installed?(connection, name) }
 
       install!(connection)
     end
@@ -61,4 +63,3 @@ class Room::MessagesCount
     end
   end
 end
-

config/initializers/room_messages_count.rb

@@ -1,10 +1,5 @@
-# schema.rb cannot dump SQLite triggers. Reinstall after boot when an existing DB
-# is missing them (db:schema:load / test schema load also call ensure! via rake).
+# schema.rb cannot dump SQLite triggers. Cover existing DBs that lost them.
 Rails.application.config.after_initialize do
-  ActiveRecord::Base.connection_pool.with_connection do |connection|
-    next unless connection.adapter_name.match?(/sqlite/i)
-
-    Room::MessagesCount.ensure!(connection)
-  end
+  Room::MessagesCount.ensure!
 rescue ActiveRecord::NoDatabaseError, ActiveRecord::ConnectionNotEstablished
 end

db/migrate/20261007190000_add_messages_count_to_rooms.rb

@@ -1,8 +1,6 @@
 class AddMessagesCountToRooms < ActiveRecord::Migration[8.2]
   def up
-    unless column_exists?(:rooms, :messages_count)
-      add_column :rooms, :messages_count, :integer, null: false, default: 0
-    end
+    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.
@@ -12,6 +10,6 @@ class AddMessagesCountToRooms < ActiveRecord::Migration[8.2]
 
   def down
     Room::MessagesCount.uninstall!(connection)
-    remove_column :rooms, :messages_count if column_exists?(:rooms, :messages_count)
+    remove_column :rooms, :messages_count
   end
 end

db/schema.rb

@@ -183,6 +183,5 @@ ActiveRecord::Schema[8.2].define(version: 2026_10_07_190000) do
   # Virtual tables defined in this database.
   # 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"]
-  # SQLite triggers are not dumped by schema.rb; keep rooms.messages_count honest after schema:load.
-  Room::MessagesCount.install!
 end
+

lib/tasks/room_messages_count.rake

@@ -1,38 +1,16 @@
-# schema.rb cannot dump SQLite triggers. Reinstall after loads, and re-append
-# Room::MessagesCount.install! after dumps so the call is not lost.
+# 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|
-      next unless connection.adapter_name.match?(/sqlite/i)
-
       Room::MessagesCount.ensure!(connection)
     end
   end
-
-  task append_schema_install: :environment do
-    schema = Rails.root.join("db/schema.rb")
-    contents = schema.read
-    marker = "Room::MessagesCount.install!"
-    next if contents.include?(marker)
-
-    contents.sub!(/\nend\n?\z/, <<~RUBY)
-
-      # SQLite triggers are not dumped by schema.rb; keep rooms.messages_count honest after schema:load.
-      #{marker}
-    end
-    RUBY
-    schema.write(contents)
-  end
 end
 
-{
-  "db:schema:load" => "room_messages_count:ensure",
-  "db:test:load_schema" => "room_messages_count:ensure",
-  "db:schema:dump" => "room_messages_count:append_schema_install"
-}.each do |task_name, enhancement|
+%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[enhancement].invoke
+    Rake::Task["room_messages_count:ensure"].invoke
   end
 end

test/controllers/messages/by_bots_controller_test.rb

@@ -103,7 +103,6 @@ class Messages::ByBotsControllerTest < ActionDispatch::IntegrationTest
 
     json = JSON.parse(response.body)
     assert_equal Message::PAGE_SIZE, json.size
-    assert_equal @room.reload.messages_count.to_s, response.headers["X-Total-Count"]
     assert_equal "41", response.headers["X-Total-Count"]
     assert_not_includes json.map { it["id"] }, messages(:fourth).id
 

test/models/room/messages_count_test.rb

@@ -5,7 +5,12 @@ class Room::MessagesCountTest < ActiveSupport::TestCase
   setup do
     @room = rooms(:designers)
     @other_room = rooms(:pets)
-    synchronize!(@room, @other_room)
+  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
@@ -64,7 +69,6 @@ class Room::MessagesCountTest < ActiveSupport::TestCase
 
   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")
-    synchronize!(@room, @other_room)
 
     assert_difference -> { @room.reload.messages_count }, -1 do
       assert_difference -> { @other_room.reload.messages_count }, +1 do
@@ -76,6 +80,31 @@ class Room::MessagesCountTest < ActiveSupport::TestCase
     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
+
+# Destructive trigger DDL and foreign connections need a committed DB.
+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 without changing existing counts" do
     before = @room.reload.messages_count
     Room::MessagesCount.uninstall!
@@ -84,68 +113,41 @@ class Room::MessagesCountTest < ActiveSupport::TestCase
 
     Room::MessagesCount.ensure!
 
-    assert Room::MessagesCount.trigger_installed?(ActiveRecord::Base.connection, Room::MessagesCount::INSERT_TRIGGER)
+    assert Room::MessagesCount::TRIGGERS.all? { |name|
+      Room::MessagesCount.trigger_installed?(ActiveRecord::Base.connection, name)
+    }
     assert_equal before, @room.reload.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
-  ensure
-    Room::MessagesCount.ensure!
   end
 
-  test "backfill plus triggers do not double-count ActiveRecord writes" do
+  test "ensure! repairs a partial trigger install" do
     Room::MessagesCount.uninstall!
     ActiveRecord::Base.connection.execute <<~SQL
-      UPDATE rooms SET messages_count = (
-        SELECT COUNT(*) FROM messages WHERE messages.room_id = rooms.id
-      )
+      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
-    Room::MessagesCount.install!
 
-    assert_equal @room.messages.count, @room.reload.messages_count
+    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)
 
-    assert_difference -> { @room.reload.messages_count }, +1 do
-      assert_difference -> { @room.messages.count }, +1 do
-        @room.messages.create!(creator: users(:jason), body: "Once", client_message_id: "count-no-double")
-      end
-    end
-
-    assert_equal @room.messages.count, @room.reload.messages_count
-  ensure
     Room::MessagesCount.ensure!
-  end
 
-  private
-    def synchronize!(*rooms)
-      Room::MessagesCount.ensure!
-      Room::MessagesCount.backfill!
-      rooms.each(&:reload)
-    end
-end
-
-# Foreign connections cannot join the transactional fixture lock; run outside it.
-class Room::MessagesCountForeignConnectionTest < ActiveSupport::TestCase
-  self.use_transactional_tests = false
-
-  setup do
-    Room::MessagesCount.ensure!
-    @room = rooms(:designers)
-    Room::MessagesCount.backfill!
-    @room.reload
-  end
-
-  teardown do
-    Message.where(client_message_id: "count-foreign-insert").delete_all
-    Room::MessagesCount.backfill!
+    assert Room::MessagesCount::TRIGGERS.all? { |name|
+      Room::MessagesCount.trigger_installed?(ActiveRecord::Base.connection, name)
+    }
   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
 
-    # Release AR's checkout so the native connection can write.
     ActiveRecord::Base.connection_pool.release_connection
 
     SQLite3::Database.new(path) do |db|
@@ -163,12 +165,22 @@ class Room::MessagesCountForeignConnectionTest < ActiveSupport::TestCase
 
     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 @room.messages.count, @room.messages_count
+    assert_equal other_before, @other_room.reload.messages_count
   end
 end

test/test_helper.rb

@@ -13,8 +13,8 @@ Room::MessagesCount.ensure!
 WebMock.enable!
 
 module RoomMessagesCountFixtures
-  # Fixture YAML loads alphabetically, so messages are inserted before rooms.
-  # INSERT triggers then update zero room rows; reconcile once after load.
+  # Fixture YAML still inserts messages before rooms via some load paths, so
+  # INSERT triggers can update zero room rows. Reconcile once after load.
   def load_fixtures(config)
     fixtures = super
     Room::MessagesCount.backfill!
@@ -28,15 +28,13 @@ class ActiveSupport::TestCase
 
   parallelize(workers: :number_of_processors)
 
-  # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order.
-  fixtures :all
+  # Prefer rooms before messages when the loader honors declaration order.
+  fixtures :accounts, :users, :rooms, :memberships, :messages, "action_text/rich_texts",
+           :boosts, :searches, :sessions, :webhooks, "push/subscriptions"
 
   include SessionTestHelper, MentionTestHelper, TurboTestHelper, DnsTestHelper
 
   setup do
-    # DROP TRIGGER commits outside transactional fixtures; put triggers back each test.
-    Room::MessagesCount.ensure!
-
     ActionCable.server.pubsub.clear
 
     Rails.configuration.tap do |config|
@@ -52,4 +50,3 @@ class ActiveSupport::TestCase
     WebMock.reset!
   end
 end
-