LoginSignup
TrendQuestionOfficial EventOfficial ColumnOpportunitiesOrganization
8
3

More than 1 year has passed since last update.

vis-its

VISITS Advent Calendar 2020 Day 7

@kshibata101(VISITS Technologies株式会社)

posted at

updated at

Organization

[ruby] shoulda-matcherでハマって仕事が進まなかった話

Ruby,RSpec

こちらはVISITS advent calendar 2020 7日目の記事です。
7日目と言いつつ、参加者少数につき土日お休みにさせてもらったので、7つ目ではありませんが、、

RSpecでmodel specを書くときによくお世話になるshouldla-matcherで最近少しハマったので、その話を共有しておこうと思います。
仕事が進まないことはよくあるので、ここでは特に触れません。むしろ触れないでください。

TL;DR

shoulda-matcherのvalidate_presence_ofなどは、テストを行う際対象カラムにnilや空文字に設定して挙動を確認する仕様になっているようです。

このため、validate_presence_ofに指定したカラムを特定の値に固定したり、今回自分がハマったように親の外部キーのような存在する前提のカラムにしてしまうと、予期せぬ挙動が発生するので注意が必要になります。

このため、もしvalidate_presence_ofのようなvalidatorを使ってテストする際は、nilが来ないはずでもnilが来るケースがあると思って対応しておくと良いと思います。

背景

今回親子関係にあるモデルで、子供側に親の値を条件にvalidationを入れる必要が出てきました。
テストとコードを実装後、既存のテストも同時に実行してみたところ、なぜかshoudla-matcherのテストで一部だけ落ちることに。

テストのログを見る限り、追加した親モデルがnilになってるせいで落ちてるんですが、きちんと設定しているしそもそも他のカラムの同じvalidateは通ってるし、なんなんこのバグ、、みたいな感じになりました。

具体例

今回はRailsにRSpecを導入して実験してみます。

今回は親子関係のモデルで発生したので、具体例としてParentとChildという２つのモデルで説明してみようと思います。

テーブル構成は以下のようなイメージです。

class CreateParents < ActiveRecord::Migration[6.0]
  def change
    create_table :parents do |t|
      t.string :first_name, null: false
      t.string :last_name, null: false
      t.timestamps
    end
  end
end

class CreateChildren < ActiveRecord::Migration[6.0]
  def change
    create_table :children do |t|
      t.references :parent, index: false
      t.integer :number, null: false
      t.string :first_name, null: false
      t.string :last_name, null: false
      t.timestamps

      t.index [:parent_id, :number], unique: true
    end
  end
end

ParentとChildは1:Nな関係で、Childには必ずparent_idが存在する想定です。(optionalではない)

numberは第何子に相当するかを表すカラムだと思ってください。
unique関連でもテストが落ちたので、[parent_id, number]というカラムでuniqueになるような想定にしました。

また、Childのvalidationの条件にParentの値を用いて今回の事象に遭遇したため、last_nameカラムが一致するかどうかをカスタムのvalidationとして追加してみます。

parent.rb
class Parent < ApplicationRecord
  has_many :children, dependent: :destroy
end
child.rb
class Child < ApplicationRecord
  belongs_to :parent

  validates :parent_id, presence: true
  validates :parent_id, uniqueness: { scope: [:number] }
  validates :number, presence: true
  validates :number, uniqueness: { scope: [:parent_id] } # parent_id側でチェックしているが確認のため
  validate :last_name_should_be_the_same_as_parent

  def last_name_should_be_the_same_as_parent
    errors.add(:last_name, :should_be_the_same_as_parent) unless last_name == parent.last_name
  end
end

実行するテストは以下の通りです。

child_spec.rb
require 'rails_helper'

RSpec.describe Child, type: :model do
  let(:parent) { create :parent, last_name: 'Tanaka' }
  subject(:child) { build :child, parent: parent, last_name: 'Tanaka' }

  describe 'associations' do
    it { should belong_to(:parent) }
  end

  describe 'validations' do
    it { should validate_presence_of(:parent_id) }
    it { should validate_presence_of(:number) }
    it { should validate_uniqueness_of(:parent_id).scoped_to(:number) }
    it { should validate_uniqueness_of(:number).scoped_to(:parent_id) }
  end
end

通常はsubjectは設定しなくてもいいのですが、今回last_nameでの比較が必要なため、factory_botでmodelを生成してvalidationを実行します。

この状態でテストを実行すると、

$ rspec -fd
Child
  associations
    is expected to belong to parent required: true (FAILED - 1)
validations
    is expected to validate that :parent_id cannot be empty/falsy (FAILED - 2)
    is expected to validate that :number cannot be empty/falsy
    is expected to validate that :parent_id is case-sensitively unique within the scope of :number
    is expected to validate that :number is case-sensitively unique within the scope of :parent_id (FAILED - 3)

:

Failed examples:

rspec ./spec/models/child_spec.rb:8 # Child associations is expected to belong to parent required: true
rspec ./spec/models/child_spec.rb:12 # Child validations is expected to validate that :parent_id cannot be empty/falsy
rspec ./spec/models/child_spec.rb:15 # Child validations is expected to validate that :number is case-sensitively unique within the scope of :parent_id

ということで、3つほど落ちました。
エラーはいずれも

NoMethodError:
  undefined method `last_name' for nil:NilClass
# ./app/models/child.rb:13:in `last_name_should_be_the_same_as_parent'

ということで、parentがnilになってるようです。
factory_botでもきちんと設定してるし、presenceもuniqueの方も全部落ちるならまだ分かるのに特定のものだけ落ちてるし、、で色々と混乱しました。

試しに、デバッグログを追加してspec実行直前とエラー発生直前のchildを調べてみると...

child_spec.rb#validate_presence_of(
it {
    Rails.logger.debug('== before validate_presence_of(:parent_id) ==')
    Rails.logger.debug(child.attributes)
    should validate_presence_of(:parent_id)
}
child.rb#last_name_should_be_the_same_as_parent
def last_name_should_be_the_same_as_parent
  logger.debug('== in last_name_should_be_the_same_as_parent ==')
  logger.debug(attributes)
  errors.add(:last_name, :should_be_the_same_as_parent) unless last_name == parent.last_name
end
log/test.log
== before validate_presence_of(:parent_id) ==
{"id"=>nil, "parent_id"=>50, "number"=>nil, "first_name"=>nil, "last_name"=>"Tanaka", "created_at"=>nil, "updated_at"=>nil}

:

== in last_name_should_be_the_same_as_parent ==
{"id"=>nil, "parent_id"=>nil, "number"=>nil, "first_name"=>nil, "last_name"=>"Tanaka", "created_at"=>nil, "updated_at"=>nil}

spec実行前はparent_idが設定されていますが、validationの直前ではparent_idがnilになっていました。

validateの流れ

結論から言うと、shoulda-matcherのvalidate_presence_ofは内部的に、対象カラムに実際にnilや空文字を設定して、エラーが発生するかを確認している模様です。
このため、途中でエラーではなく例外が発生した場合、エラーが発生しなかったという扱いで落ちてしまうようでした。

validate_presence_ofの場合

validate_presence_ofのケースでは、matches?の中で、disallowedなvaluesに対して想定通りエラーが変えるかを以下のような感じで検証していました。

lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb#L142
def matches?(subject)
  super(subject)

  possibly_ignore_interference_by_writer

  if secure_password_being_validated?
    ignore_interference_by_writer.default_to(when: :blank?)

    disallowed_values.all? do |value|
      disallows_and_double_checks_value_of!(value)
    end
  else
    (!expects_to_allow_nil? || allows_value_of(nil)) &&
      disallowed_values.all? do |value|
        disallows_original_or_typecast_value?(value) # ここが実行される
      end
  end
end

disallows_original_or_typecast_value?の内部では、disallow_value_matcherという失敗ケースのmatcherを作って、テストがコケるかをチェックしていました。
以下のmessageの引数には :blank が渡ってくるため、matcherが:blankを返すようであれば想定通り、といった挙動のようです。

lib/shoulda/matchers/active_model/validation_matcher.rb
def disallows_value_of(value, message = nil, &block)
  matcher = disallow_value_matcher(value, message, &block)
  run_allow_or_disallow_matcher(matcher)
end

この過程で、modelのerrorsに対して値がセットされるようであれば問題ないのですが、先程のように例外が発生してしまうと途中終了してしまいます。

validate_uniqueness_of

validate_uniqueness_ofはmatchesの条件が多いですが、今回はmatches_uniqueness_with_scopes?で引っかかりました。

lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb#L336
def matches?(given_record)
  @given_record = given_record
  @all_records = model.all

  matches_presence_of_attribute? &&
    matches_presence_of_scopes? &&
    matches_scopes_configuration? &&
    matches_uniqueness_without_scopes? &&
    matches_uniqueness_with_case_sensitivity_strategy? &&
    matches_uniqueness_with_scopes? &&
    matches_allow_nil? &&
    matches_allow_blank?
  ensure
    Uniqueness::TestModels.remove_all
  end
end

内部では setting_next_value_for というメソッドでscope(ここではparent_id)に対して今の設定値の次の値(100なら101など)を設定します。
おそらく重複のテストのためにかぶらないはずの次の値を設定することで、uniqueのチェックを行う模様です。

最終的に追えなかったので憶測ではありますが、発行されたクエリを見る限り次の値の親を取得しようとしていた(が当然数値的に生成しただけでDBには存在しない)ため、parentがnilとなって同じ箇所で例外が発生したようです。

lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb#L775
def setting_next_value_for(scope)
  previous_value = @all_records.map(&scope).compact.max

  next_value =
    if previous_value.blank?
      dummy_value_for(scope)
    else
      next_value_for(scope, previous_value)
    end

  set_attribute_on_new_record!(scope, next_value)

  yield
ensure
  set_attribute_on_new_record!(scope, previous_value)
end

belong_to

今回はvalidationではないbelong_toのassocication側でも例外となってコケていました。
belong_toの方では以下のようなmatchを当てる模様です。

引っかかっていたのは最後のsubmatchers_match?のところです。

lib/shoulda/matchers/active_record/association_matcher.rb#L1148
def matches?(subject)
  @subject = subject
  association_exists? &&
    macro_correct? &&
    validate_inverse_of_through_association &&
    (polymorphic? || class_exists?) &&
    foreign_key_exists? &&
    primary_key_exists? &&
    class_name_correct? &&
    join_table_correct? &&
    utosave_correct? &&
    index_errors_correct? &&
    conditions_correct? &&
    validate_correct? &&
    touch_correct? &&
    submatchers_match?
end

belong_toのshoulda-matcherを用いた場合、optionalをつけないとさきほどのsubmatcherの方でrequired_matcherが指定されますが、これによってparentに対してnilがdisallowされるかチェックされるような形になります。
この場合もmatcherの途中で例外が発生するため落ちる模様です。

lib/shoulda/matchers/active_record/association_matchers/required_matcher.rb#L6

def initialize(attribute_name, required)
  @attribute_name = attribute_name
  @required = required
  @submatcher = ActiveModel::DisallowValueMatcher.new(nil).
    for(attribute_name).
    with_message(validation_message_key)
  @missing_option = ''
end

じゃあどうするか？

単純に例外を防げばいいので、実装上問題なければnilガードしておくのが良さそうです。

  def last_name_should_be_the_same_as_parent
    errors.add(:last_name, :should_be_the_same_as_parent) unless last_name == parent&.last_name
  end

parent_idにnilがセットされると当然このテストは通らないのですが、あくまでテスト対象となっているカラムでエラーが出るのか確認しているため、例外にならなければ大丈夫のようです。

ただテストに合わせて実装を変えることにはなるので、実装方針次第ではテストを省略することや別の方法でテストすることを考えたり、さらには設計自体が悪い可能性もあるのでそこを考えてもよいかもしれません。

もし別の手段で回避する方法をご存知の方いらっしゃったらご教示いただきたいですm(__)m

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
What you can do with signing up
Sign upLogin
8
3