Fix `create_or_find_by` scope pollution on the non-transactional retry

Rails 8.1で create_or_find_by 系メソッドが、トランザクション外で再試行した際にスコープが二重に適用されレコードが取得できなくなるバグを修正しました。これにより、トップレベルの呼び出しでも期待どおりに既存レコードを返すようになります。

背景

create_or_find_by がスコープ汚染を起こすケースは、ActiveRecord::RecordNotUnique が発生した後のリトライ処理に起因します。 #57192 でトランザクション内の if connection.transaction_open? ブランチは rewhere(attributes).lock.take! に置き換えられましたが、else ブランチは依然として find_by!(attributes) を呼び出していました。

else ブランチのコードはスコープを二度適用し、条件が矛盾して RecordNotFound を引き起こします。 例えば Subscriber.where(nick: "alice").create_or_find_by(nick: "bob") は WHERE "nick" = 'alice' AND "nick" = 'bob' という不可能な条件を生成し、既に存在するレコードを返せませんでした。

テストがこのバグを捕捉できなかったのは、テストスイートが常にトランザクション内で実行されるため connection.transaction_open? が常に真になるからです。その結果 else ブランチは実行されず、実運用でのみ顕在化していました。

技術的な変更

activerecord/lib/active_record/relation.rb の実装を二箇所修正し、else ブランチでも rewhere(attributes).take! を呼び出すように統一しました。 変更前後の差分は以下の通りです。

@@ -283,7 +283,7 @@ def create_or_find_by(attributes, &block)
         if connection.transaction_open?
           rewhere(attributes).lock.take!
         else
-          find_by!(attributes)
+          rewhere(attributes).take!
         end
       end
     end
@@ -303,7 +303,7 @@ def create_or_find_by!(attributes, &block)
         if connection.transaction_open?
           rewhere(attributes).lock.take!
         else
-          find_by!(attributes)
+          rewhere(attributes).take!
         end
       end
     end

.lock の付与はトランザクション内でのみ意味があるため、else ブランチでは除去しました。 rewhere(attributes).take! はスコープを再適用しつつロックなしで単一レコードを取得するので、汚染が防がれます。

テストコード activerecord/test/cases/create_or_find_by_outside_transaction_test.rb を新規追加し、トランザクション外での挙動を検証しました。 テストは self.use_transactional_tests = false に設定し、実際のトップレベル呼び出しと同様の環境を再現しています。

# frozen_string_literal: true

require "cases/helper"
require "models/subscriber"

class CreateOrFindByOutsideTransactionTest < ActiveRecord::TestCase
  self.use_transactional_tests = false

  def setup
    Subscriber.create!(nick: "cofb_bob", name: "Bob")
  end

  def teardown
    Subscriber.where(nick: "cofb_bob").delete_all
  end

  def test_create_or_find_by_with_polluted_scope_outside_a_transaction
    existing = Subscriber.find_by!(nick: "cofb_bob")
    found = Subscriber.where(nick: "cofb_alice").create_or_find_by(nick: "cofb_bob")
    assert_equal existing, found
  end

  def test_create_or_find_by_bang_with_polluted_scope_outside_a_transaction
    existing = Subscriber.find_by!(nick: "cofb_bob")
    found = Subscriber.where(nick: "cofb_alice").create_or_find_by!(nick: "cofb_bob")
    assert_equal existing, found
  end
end

設計判断

トランザクション有無に関係なく同一ロジックを適用する方針で実装を統一しました。 既存の if connection.transaction_open? 分岐は残しつつ、else 側でも同等の rewhere(...).take! を用いることで、コードパスの差異を最小化しました。

ロック取得 (.lock) を除去したのは、トランザクション外ではロックが作用せず無駄になるためです。 この選択は機能的な影響を与えず、パフォーマンスとシンプルさのトレードオフとして妥当です。

まとめ

今回の修正は、create_or_find_by 系メソッドがトランザクション外で再試行した際にスコープ汚染を起こすバグを除去し、トップレベル呼び出しでも正しいレコード取得を保証します。実装は既存ロジックを踏襲しつつ最小限の変更で済ませており、後方互換性を保ったまま信頼性を向上させました。