Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate mismatched collation comparison for uniquness validator #35350

Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -1,3 +1,11 @@
* Deprecate mismatched collation comparison for uniquness validator.

Uniqueness validator will no longer enforce case sensitive comparison in Rails 6.1.
To continue case sensitive comparison on the case insensitive column,
pass `case_sensitive: true` option explicitly to the uniqueness validator.

*Ryuta Kamizono*

* Add `reselect` method. This is a short-hand for `unscope(:select).select(fields)`.

Fixes #27340.
@@ -506,7 +506,7 @@ def raw_connection
@connection
end

def default_uniqueness_comparison(attribute, value) # :nodoc:
def default_uniqueness_comparison(attribute, value, klass) # :nodoc:
case_sensitive_comparison(attribute, value)
end

@@ -453,6 +453,20 @@ def primary_keys(table_name) # :nodoc:
SQL
end

def default_uniqueness_comparison(attribute, value, klass) # :nodoc:
column = column_for_attribute(attribute)

if column.collation && !column.case_sensitive?
ActiveSupport::Deprecation.warn(<<~MSG.squish)
Uniqueness validator will no longer enforce case sensitive comparison in Rails 6.1.
To continue case sensitive comparison on the :#{attribute.name} attribute in #{klass} model,
pass `case_sensitive: true` option explicitly to the uniqueness validator.
MSG
end

super
end

def case_sensitive_comparison(attribute, value) # :nodoc:
column = column_for_attribute(attribute)

@@ -63,7 +63,7 @@ def build_relation(klass, attribute, value)
if bind.nil?
attr.eq(bind)
elsif !options.key?(:case_sensitive)
klass.connection.default_uniqueness_comparison(attr, bind)
klass.connection.default_uniqueness_comparison(attr, bind, klass)
elsif options[:case_sensitive]
klass.connection.case_sensitive_comparison(attr, bind)
else
@@ -314,6 +314,51 @@ def test_validate_case_insensitive_uniqueness_with_special_sql_like_chars
assert t3.save, "Should save t3 as unique"
end

if current_adapter?(:Mysql2Adapter)
def test_deprecate_validate_uniqueness_mismatched_collation
Topic.validates_uniqueness_of(:author_email_address)

topic1 = Topic.new(author_email_address: "david@loudthinking.com")
topic2 = Topic.new(author_email_address: "David@loudthinking.com")

assert_equal 1, Topic.where(author_email_address: "david@loudthinking.com").count

assert_deprecated do
assert_not topic1.valid?
assert_not topic1.save
assert topic2.valid?
assert topic2.save
end

assert_equal 2, Topic.where(author_email_address: "david@loudthinking.com").count
assert_equal 2, Topic.where(author_email_address: "David@loudthinking.com").count
end
end

def test_validate_case_sensitive_uniqueness_by_default
Topic.validates_uniqueness_of(:author_email_address)

topic1 = Topic.new(author_email_address: "david@loudthinking.com")
topic2 = Topic.new(author_email_address: "David@loudthinking.com")

assert_equal 1, Topic.where(author_email_address: "david@loudthinking.com").count

ActiveSupport::Deprecation.silence do
assert_not topic1.valid?
assert_not topic1.save
assert topic2.valid?
assert topic2.save
end

if current_adapter?(:Mysql2Adapter)
assert_equal 2, Topic.where(author_email_address: "david@loudthinking.com").count
assert_equal 2, Topic.where(author_email_address: "David@loudthinking.com").count

This comment has been minimized.

Copy link
@kamipo

kamipo Mar 4, 2019

Author Member

That happening this (break the uniqueness contract) easily without race condition is what I want to prevent, which frequently experienced to me as MySQL DBA.

else
assert_equal 1, Topic.where(author_email_address: "david@loudthinking.com").count
assert_equal 1, Topic.where(author_email_address: "David@loudthinking.com").count
end
end

def test_validate_case_sensitive_uniqueness
Topic.validates_uniqueness_of(:title, case_sensitive: true, allow_nil: true)

@@ -8,6 +8,13 @@
# #
# ------------------------------------------------------------------- #

case_sensitive_options =
if current_adapter?(:Mysql2Adapter)
{ collation: "utf8mb4_bin" }
else
{}
end

create_table :accounts, force: true do |t|
t.references :firm, index: false
t.string :firm_name
@@ -266,7 +273,7 @@
end

create_table :dashboards, force: true, id: false do |t|
t.string :dashboard_id
t.string :dashboard_id, **case_sensitive_options
t.string :name
end

@@ -330,15 +337,15 @@
end

create_table :essays, force: true do |t|
t.string :name
t.string :name, **case_sensitive_options
t.string :writer_id
t.string :writer_type
t.string :category_id
t.string :author_id
end

create_table :events, force: true do |t|
t.string :title, limit: 5
t.string :title, limit: 5, **case_sensitive_options
end

create_table :eyes, force: true do |t|
@@ -380,16 +387,16 @@
end

create_table :guids, force: true do |t|
t.column :key, :string
t.column :key, :string, **case_sensitive_options
end

create_table :guitars, force: true do |t|
t.string :color
end

create_table :inept_wizards, force: true do |t|
t.column :name, :string, null: false
t.column :city, :string, null: false
t.column :name, :string, null: false, **case_sensitive_options
t.column :city, :string, null: false, **case_sensitive_options
t.column :type, :string
end

@@ -876,8 +883,8 @@
end

create_table :topics, force: true do |t|
t.string :title, limit: 250
t.string :author_name
t.string :title, limit: 250, **case_sensitive_options
t.string :author_name, **case_sensitive_options
t.string :author_email_address
if subsecond_precision_supported?
t.datetime :written_on, precision: 6
@@ -889,10 +896,10 @@
# use VARCHAR2(4000) instead of CLOB datatype as CLOB data type has many limitations in
# Oracle SELECT WHERE clause which causes many unit test failures
if current_adapter?(:OracleAdapter)
t.string :content, limit: 4000
t.string :content, limit: 4000, **case_sensitive_options
t.string :important, limit: 4000
else
t.text :content
t.text :content, **case_sensitive_options
t.text :important
end
t.boolean :approved, default: true
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.