Fix uniqueness matcher when scope is a *_type attr
Secondary author: Elliot Winkler <elliot.winkler@gmail.com> This commit fixes `validate_uniqueness_of` when used with `scoped_to` so that when one of the scope attributes is a polymorphic *_type attribute and the model has another validation on the same attribute, the matcher does not fail with an error. As a part of the matching process, `validate_uniqueness_of` tries to create two records that have the same values for each of the attributes passed to `scoped_to`, except one of the attributes has a different value. This way, the second record should be valid because it shouldn't clash with the first one. It does this one attribute at a time. Let's say the attribute in question is a polymorphic *_type attribute. The value of this attribute is intended to be the name of a model as a string. Let's assume the first record has a meaningful value for this attribute already and we're trying to find a value for the second record. In order to produce such a value, `validate_uniqueness_of` generally calls #next (a common method in Ruby to generate a succeeding version of an object sequentially) on the first object's corresponding value. So in the case of a *_type attribute, since it's a string, it would call #next on that string. For instance, "User" would become "Uses". You might have noticed a problem with this, which is "what if Uses is not a valid model?" This is okay as long as there's nothing that is trying to access the polymorphic association. Because as soon as this happens, Rails will attempt to find a record using the polymorphic type -- in other words, it will try to find the model that the *_type attribute corresponds to. One of the ways this can happen is if the *_type attribute in question has a validation on it itself. Let's look at an example. Given these models: ``` ruby class User < ActiveRecord::Base end class Favorite < ActiveRecord::Base belongs_to :favoriteable, polymorphic: true validates :favoriteable, presence: true validates :favoriteable_id, uniqueness: { scope: [:favoriteable_type] } end FactoryGirl.define do factory :user factory :favorite do association :favoriteable, factory: :user end end ``` and the following test: ``` ruby require 'rails_helper' describe Favorite do context 'validations' do before { FactoryGirl.create(:favorite) } it do should validate_uniqueness_of(:favoriteable_id). scoped_to(:favoriteable_type) end end end ``` prior to this commit, the test would have failed with: ``` Failures: 1) Favorite validations should require case sensitive unique value for favoriteable_id scoped to favoriteable_type Failure/Error: should validate_uniqueness_of(:favoriteable_id). NameError: uninitialized constant Uses # ./spec/models/favorite_spec.rb:6:in `block (3 levels) in <top (required)>' ``` Here, a Favorite record is created where `favoriteable_type` is set to "Uses", and then validations are run on that record. The presence validation on `favoriteable_type` is run which tries to access a "Uses" model. But that model doesn't exist, so the test raises an error. Now `validates_uniqueness_of` will set the *_type attribute to a meaningful value. It still does this by calling #next on the first record's value, but then it makes a new model that is simply an alias for the original model. Hence, in our example, Uses would become a model that is aliased to User.
This commit is contained in:
parent
72e2b3411e
commit
4ad15205a5
|
@ -1,4 +1,5 @@
|
||||||
require 'shoulda/matchers/active_model/helpers'
|
require 'shoulda/matchers/active_model/helpers'
|
||||||
|
require 'shoulda/matchers/active_model/uniqueness'
|
||||||
require 'shoulda/matchers/active_model/validation_matcher'
|
require 'shoulda/matchers/active_model/validation_matcher'
|
||||||
require 'shoulda/matchers/active_model/validation_message_finder'
|
require 'shoulda/matchers/active_model/validation_message_finder'
|
||||||
require 'shoulda/matchers/active_model/exception_message_finder'
|
require 'shoulda/matchers/active_model/exception_message_finder'
|
||||||
|
|
|
@ -0,0 +1,14 @@
|
||||||
|
module Shoulda
|
||||||
|
module Matchers
|
||||||
|
module ActiveModel
|
||||||
|
# @private
|
||||||
|
module Uniqueness
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
require 'shoulda/matchers/active_model/uniqueness/model'
|
||||||
|
require 'shoulda/matchers/active_model/uniqueness/namespace'
|
||||||
|
require 'shoulda/matchers/active_model/uniqueness/test_model_creator'
|
||||||
|
require 'shoulda/matchers/active_model/uniqueness/test_models'
|
|
@ -0,0 +1,45 @@
|
||||||
|
module Shoulda
|
||||||
|
module Matchers
|
||||||
|
module ActiveModel
|
||||||
|
module Uniqueness
|
||||||
|
# @private
|
||||||
|
class Model
|
||||||
|
def self.next_unique_copy_of(model_name, namespace)
|
||||||
|
model = new(model_name, namespace)
|
||||||
|
|
||||||
|
while model.already_exists?
|
||||||
|
model = model.next
|
||||||
|
end
|
||||||
|
|
||||||
|
model
|
||||||
|
end
|
||||||
|
|
||||||
|
def initialize(name, namespace)
|
||||||
|
@name = name
|
||||||
|
@namespace = namespace
|
||||||
|
end
|
||||||
|
|
||||||
|
def already_exists?
|
||||||
|
namespace.has?(name)
|
||||||
|
end
|
||||||
|
|
||||||
|
def next
|
||||||
|
Model.new(name.next, namespace)
|
||||||
|
end
|
||||||
|
|
||||||
|
def symlink_to(parent)
|
||||||
|
namespace.set(name, parent.dup)
|
||||||
|
end
|
||||||
|
|
||||||
|
def to_s
|
||||||
|
[namespace, name].join('::')
|
||||||
|
end
|
||||||
|
|
||||||
|
protected
|
||||||
|
|
||||||
|
attr_reader :name, :namespace
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -0,0 +1,36 @@
|
||||||
|
module Shoulda
|
||||||
|
module Matchers
|
||||||
|
module ActiveModel
|
||||||
|
module Uniqueness
|
||||||
|
# @private
|
||||||
|
class Namespace
|
||||||
|
def initialize(constant)
|
||||||
|
@constant = constant
|
||||||
|
end
|
||||||
|
|
||||||
|
def has?(name)
|
||||||
|
constant.const_defined?(name)
|
||||||
|
end
|
||||||
|
|
||||||
|
def set(name, value)
|
||||||
|
constant.const_set(name, value)
|
||||||
|
end
|
||||||
|
|
||||||
|
def clear
|
||||||
|
constant.constants.each do |child_constant|
|
||||||
|
constant.__send__(:remove_const, child_constant)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def to_s
|
||||||
|
constant.to_s
|
||||||
|
end
|
||||||
|
|
||||||
|
protected
|
||||||
|
|
||||||
|
attr_reader :constant
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -0,0 +1,50 @@
|
||||||
|
require 'thread'
|
||||||
|
|
||||||
|
module Shoulda
|
||||||
|
module Matchers
|
||||||
|
module ActiveModel
|
||||||
|
module Uniqueness
|
||||||
|
# @private
|
||||||
|
class TestModelCreator
|
||||||
|
def self.create(model_name, namespace)
|
||||||
|
Mutex.new.synchronize do
|
||||||
|
new(model_name, namespace).create
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def initialize(model_name, namespace)
|
||||||
|
@model_name = model_name
|
||||||
|
@namespace = namespace
|
||||||
|
end
|
||||||
|
|
||||||
|
def create
|
||||||
|
new_model.tap do |new_model|
|
||||||
|
new_model.symlink_to(existing_model)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
protected
|
||||||
|
|
||||||
|
attr_reader :model_name, :namespace
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def model_name_without_namespace
|
||||||
|
model_name.demodulize
|
||||||
|
end
|
||||||
|
|
||||||
|
def new_model
|
||||||
|
@_new_model ||= Model.next_unique_copy_of(
|
||||||
|
model_name_without_namespace,
|
||||||
|
namespace
|
||||||
|
)
|
||||||
|
end
|
||||||
|
|
||||||
|
def existing_model
|
||||||
|
@_existing_model ||= model_name.constantize
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -0,0 +1,24 @@
|
||||||
|
require 'thread'
|
||||||
|
|
||||||
|
module Shoulda
|
||||||
|
module Matchers
|
||||||
|
module ActiveModel
|
||||||
|
module Uniqueness
|
||||||
|
# @private
|
||||||
|
module TestModels
|
||||||
|
def self.create(model_name)
|
||||||
|
TestModelCreator.create(model_name, root_namespace)
|
||||||
|
end
|
||||||
|
|
||||||
|
def self.remove_all
|
||||||
|
root_namespace.clear
|
||||||
|
end
|
||||||
|
|
||||||
|
def self.root_namespace
|
||||||
|
@_root_namespace ||= Namespace.new(self)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -216,10 +216,13 @@ module Shoulda
|
||||||
@original_subject = subject
|
@original_subject = subject
|
||||||
@subject = subject.class.new
|
@subject = subject.class.new
|
||||||
@expected_message ||= :taken
|
@expected_message ||= :taken
|
||||||
|
|
||||||
set_scoped_attributes &&
|
set_scoped_attributes &&
|
||||||
validate_everything_except_duplicate_nils? &&
|
validate_everything_except_duplicate_nils? &&
|
||||||
validate_after_scope_change? &&
|
validate_after_scope_change? &&
|
||||||
allows_nil?
|
allows_nil?
|
||||||
|
ensure
|
||||||
|
Uniqueness::TestModels.remove_all
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
@ -262,6 +265,7 @@ module Shoulda
|
||||||
instance.__send__("#{@attribute}=", value)
|
instance.__send__("#{@attribute}=", value)
|
||||||
ensure_secure_password_set(instance)
|
ensure_secure_password_set(instance)
|
||||||
instance.save(validate: false)
|
instance.save(validate: false)
|
||||||
|
@created_record = instance
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -305,6 +309,12 @@ module Shoulda
|
||||||
@existing_record = create_record_in_database
|
@existing_record = create_record_in_database
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def model_class?(model_name)
|
||||||
|
model_name.constantize.ancestors.include?(::ActiveRecord::Base)
|
||||||
|
rescue NameError
|
||||||
|
false
|
||||||
|
end
|
||||||
|
|
||||||
def validate_after_scope_change?
|
def validate_after_scope_change?
|
||||||
if @options[:scopes].blank?
|
if @options[:scopes].blank?
|
||||||
true
|
true
|
||||||
|
@ -322,6 +332,8 @@ module Shoulda
|
||||||
key == previous_value
|
key == previous_value
|
||||||
end
|
end
|
||||||
available_values.keys.last
|
available_values.keys.last
|
||||||
|
elsif scope.to_s =~ /_type$/ && model_class?(previous_value)
|
||||||
|
Uniqueness::TestModels.create(previous_value).to_s
|
||||||
elsif previous_value.respond_to?(:next)
|
elsif previous_value.respond_to?(:next)
|
||||||
previous_value.next
|
previous_value.next
|
||||||
elsif previous_value.respond_to?(:to_datetime)
|
elsif previous_value.respond_to?(:to_datetime)
|
||||||
|
|
|
@ -418,6 +418,56 @@ describe Shoulda::Matchers::ActiveModel::ValidateUniquenessOfMatcher do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context "when testing that a polymorphic *_type column is one of the validation scopes" do
|
||||||
|
it "sets that column to a meaningful value that works with other validations on the same column" do
|
||||||
|
user_model = define_model :user
|
||||||
|
favorite_columns = {
|
||||||
|
favoriteable_id: { type: :integer, options: { null: false } },
|
||||||
|
favoriteable_type: { type: :string, options: { null: false } }
|
||||||
|
}
|
||||||
|
favorite_model = define_model :favorite, favorite_columns do
|
||||||
|
attr_accessible :favoriteable
|
||||||
|
belongs_to :favoriteable, polymorphic: true
|
||||||
|
validates :favoriteable, presence: true
|
||||||
|
validates :favoriteable_id, uniqueness: { scope: :favoriteable_type }
|
||||||
|
end
|
||||||
|
|
||||||
|
user = user_model.create!
|
||||||
|
favorite_model.create!(favoriteable: user)
|
||||||
|
new_favorite = favorite_model.new
|
||||||
|
|
||||||
|
expect(new_favorite).
|
||||||
|
to validate_uniqueness_of(:favoriteable_id).
|
||||||
|
scoped_to(:favoriteable_type)
|
||||||
|
end
|
||||||
|
|
||||||
|
context "if the model the *_type column refers to is namespaced, and shares the last part of its name with an existing model" do
|
||||||
|
it "still works" do
|
||||||
|
define_class 'User'
|
||||||
|
define_module 'Models'
|
||||||
|
user_model = define_model 'Models::User'
|
||||||
|
favorite_columns = {
|
||||||
|
favoriteable_id: { type: :integer, options: { null: false } },
|
||||||
|
favoriteable_type: { type: :string, options: { null: false } }
|
||||||
|
}
|
||||||
|
favorite_model = define_model 'Models::Favorite', favorite_columns do
|
||||||
|
attr_accessible :favoriteable
|
||||||
|
belongs_to :favoriteable, polymorphic: true
|
||||||
|
validates :favoriteable, presence: true
|
||||||
|
validates :favoriteable_id, uniqueness: { scope: :favoriteable_type }
|
||||||
|
end
|
||||||
|
|
||||||
|
user = user_model.create!
|
||||||
|
favorite_model.create!(favoriteable: user)
|
||||||
|
new_favorite = favorite_model.new
|
||||||
|
|
||||||
|
expect(new_favorite).
|
||||||
|
to validate_uniqueness_of(:favoriteable_id).
|
||||||
|
scoped_to(:favoriteable_type)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def case_sensitive_validation_with_existing_value(attr_type)
|
def case_sensitive_validation_with_existing_value(attr_type)
|
||||||
model = define_model(:example, attr: attr_type) do
|
model = define_model(:example, attr: attr_type) do
|
||||||
attr_accessible :attr
|
attr_accessible :attr
|
||||||
|
|
|
@ -14,6 +14,30 @@ module ClassBuilder
|
||||||
[qualified_namespace, name_without_namespace]
|
[qualified_namespace, name_without_namespace]
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def define_module(module_name, &block)
|
||||||
|
module_name = module_name.to_s.camelize
|
||||||
|
|
||||||
|
namespace, name_without_namespace =
|
||||||
|
ClassBuilder.parse_constant_name(module_name)
|
||||||
|
|
||||||
|
if namespace.const_defined?(name_without_namespace, false)
|
||||||
|
namespace.__send__(:remove_const, name_without_namespace)
|
||||||
|
end
|
||||||
|
|
||||||
|
eval <<-RUBY
|
||||||
|
module #{namespace}::#{name_without_namespace}
|
||||||
|
end
|
||||||
|
RUBY
|
||||||
|
|
||||||
|
namespace.const_get(name_without_namespace).tap do |constant|
|
||||||
|
constant.unloadable
|
||||||
|
|
||||||
|
if block
|
||||||
|
constant.module_eval(&block)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def define_class(class_name, parent_class = Object, &block)
|
def define_class(class_name, parent_class = Object, &block)
|
||||||
class_name = class_name.to_s.camelize
|
class_name = class_name.to_s.camelize
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue