Permalink
Browse files

Uniqueness validation uses a proc to specify the `:conditions` option.

This is a follow up to #5321 and follows the general direction in
AR to make things lazy evaluated.
  • Loading branch information...
senny committed Mar 9, 2013
1 parent 15970ef commit ad1a24f0e04ccff17df604c7781f4805ace2ec0f
@@ -1,5 +1,14 @@
## Rails 4.0.0 (unreleased) ##
+* Uniqueness validation allows you to pass `:conditions` to limit
+ the constraint lookup.
+
+ Example:
+
+ validates_uniqueness_of :title, conditions: -> { where('approved = ?', true) }
@dre-hh

dre-hh Aug 28, 2013

This is great stuff, but by using this you can't deal with Concurrency and integrity Problem easily anymore.

The doc below says
"Using this validation method in conjunction with ActiveRecord::Validations#save does not guarantee the absence of duplicate record insertions, because uniqueness checks on the application level are inherently prone to race conditions"
It suggests a workaround by simply adding a unique index.

In case of such :conditions option, this is not that simple any more. You could solve this by adding stored_procedure for checking integrity or using a partial unique index in postgres. The point is, that you have to write this logic twice - in application and in the backend, making the validator kind of useless, since it's not actually garanteeing uniqueness. The same also applies for the :allow_nil option. It's always true for the unique indices in mysql for example and if you do not set it, you would actually need a stored porcedure again for the right integrity check.

Is there any way, we can generate this stored procedures for the unique validators? It must not happen automaticly.
Just a simple generate method

+
+ *Mattias Pfeiffer + Yves Senn*
+
* `connection` is deprecated as an instance method.
This allows end-users to have a `connection` method on their models
without clashing with ActiveRecord internals.
@@ -2,6 +2,10 @@ module ActiveRecord
module Validations
class UniquenessValidator < ActiveModel::EachValidator # :nodoc:
def initialize(options)
+ if options[:conditions] && !options[:conditions].respond_to?(:call)
+ raise ArgumentError, "#{options[:conditions]} was passed as :conditions but is not callable. " \
+ "Pass a callable instead: `conditions: -> { where('approved = ?', true) }`"
+ end
super({ case_sensitive: true }.merge!(options))
end
@@ -19,7 +23,7 @@ def validate_each(record, attribute, value)
relation = relation.and(table[finder_class.primary_key.to_sym].not_eq(record.id)) if record.persisted?
relation = scope_relation(record, table, relation)
relation = finder_class.unscoped.where(relation)
- relation.merge!(options[:conditions]) if options[:conditions]
+ relation = relation.merge(options[:conditions]) if options[:conditions]
if relation.exists?
error_options = options.except(:case_sensitive, :scope, :conditions)
@@ -116,7 +120,7 @@ module ClassMethods
# of the title attribute:
#
# class Article < ActiveRecord::Base
- # validates_uniqueness_of :title, conditions: where('status != ?', 'archived')
+ # validates_uniqueness_of :title, conditions: -> { where('status != ?', 'archived') }
# end
#
# When the record is created, a check is performed to make sure that no
@@ -132,7 +136,7 @@ module ClassMethods
# the uniqueness constraint.
# * <tt>:conditions</tt> - Specify the conditions to be included as a
# <tt>WHERE</tt> SQL fragment to limit the uniqueness constraint lookup
- # (e.g. <tt>conditions: where('status = ?', 'active')</tt>).
+ # (e.g. <tt>conditions: -> { where('status = ?', 'active') }</tt>).
# * <tt>:case_sensitive</tt> - Looks for an exact match. Ignored by
# non-text columns (+true+ by default).
# * <tt>:allow_nil</tt> - If set to +true+, skips this validation if the
@@ -348,7 +348,7 @@ def test_validate_straight_inheritance_uniqueness
end
def test_validate_uniqueness_with_conditions
- Topic.validates_uniqueness_of(:title, :conditions => Topic.where('approved = ?', true))
+ Topic.validates_uniqueness_of :title, conditions: -> { where('approved = ?', true) }
Topic.create("title" => "I'm a topic", "approved" => true)
Topic.create("title" => "I'm an unapproved topic", "approved" => false)
@@ -359,6 +359,12 @@ def test_validate_uniqueness_with_conditions
assert t4.valid?, "t4 should be valid"
end
+ def test_validate_uniqueness_with_non_callable_conditions_is_not_supported
+ assert_raises(ArgumentError) {
+ Topic.validates_uniqueness_of :title, conditions: Topic.where('approved = ?', true)
+ }
+ end
+
def test_validate_uniqueness_with_array_column
return skip "Uniqueness on arrays has only been tested in PostgreSQL so far." if !current_adapter? :PostgreSQLAdapter

0 comments on commit ad1a24f

Please sign in to comment.