こちらは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として追加してみます。
class Parent < ApplicationRecord
has_many :children, dependent: :destroy
end
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
実行するテストは以下の通りです。
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を調べてみると...
it {
Rails.logger.debug('== before validate_presence_of(:parent_id) ==')
Rails.logger.debug(child.attributes)
should validate_presence_of(:parent_id)
}
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
== 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に対して想定通りエラーが変えるかを以下のような感じで検証していました。
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を返すようであれば想定通り、といった挙動のようです。
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?で引っかかりました。
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となって同じ箇所で例外が発生したようです。
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?のところです。
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の途中で例外が発生するため落ちる模様です。
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