ActiveModel::Validatorのインスタンス変数にオブジェクトをメモ化してはならない

Rails Ruby Webサービス開発

タイトルの通り。

実は以前にも踏んだことがあるのだけれど、しばらく期間が開いてまた踏んでしまったので書き残しておくことにする。
ActiveModel::Validatorを継承するValidatorなんて日常的に書くわけではないのでたまに踏んでしまうのは仕方ないという話もあるが…

さて、ActiveRecordにはバリデーションを表現する方法が複数ある。

  • validatesでカラムにActiveModel::EachValidatorのサブクラスを対応付ける方法
  • validate_withでクラスにActiveModel::Validatorのサブクラスを対応付ける方法
  • validateでクラスに定義されたprivatre methodを呼び出させる方法

ある日、仕事でActiveModel::Validatorを継承したValidatorを作っていた。
そのValidatorではとある深遠な理由でバリデーション対象のレコ―ドから辿るのが少々面倒なレコードを複数回参照していたので、思わずValidator内で

def hoge_record
  @hoge_record ||= record.fuga.piyo.hoge
end

というようにインスタンス変数にメモ化してしまった。
すると、request specで一つのテストケースのみを指定した場合はテストがpassするが、2つ以上のテストケースをまとめて実行するとfailするという事象に見舞われる。

これはActiveModel::Validationsが実行すべきValidatorをModelに登録する仕組みから来るもの。
具体的に言うと、Validatorがインスタンス化されるのはvalidate_withによってValidatorがModelに対応付けられるタイミングであって、そのインスタンスが何度も使いまわされるから。

そのため||=を使ってインスタンス変数にメモ化してしまうと、初回のvalidateでメモ化したオブジェクトが入りっぱなしになってしまうため意図した動作にならない。

validate_withによってValidatorがModelに対応付けられるタイミングでValidatorがインスタンス化される、当該のコードはここ。

def validates_with(*args, &block)
  options = args.extract_options!
  options[:class] = self

  args.each do |klass|
    validator = klass.new(options, &block)

    if validator.respond_to?(:attributes) && !validator.attributes.empty?
      validator.attributes.each do |attribute|
        _validators[attribute.to_sym] << validator
      end
    else
      _validators[nil] << validator
    end

    validate(validator, options)
  end
end

バリデーション対象のレコードから辿れるのであれば、バリデーション対象のレコード側でメモ化してしまうのも手だが、バリデーション対象のレコードのパブリックメソッドにてメモ化してしまうのがドメインモデル間の関係性として本当に適切かなのかは疑問が残るので少し悩ましい。