Skip to content

Embed URL

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Rails Lighthouse ticket #3486
From f259b38eee1b2a14507868e835f2ac187263a616 Mon Sep 17 00:00:00 2001
From: Jordan Brough <jordan@animoto.com>
Date: Wed, 11 Nov 2009 13:31:34 -0700
Subject: [PATCH] Friendly ActiveRecord error handling for db-enforced unique constraints
---
activerecord/lib/active_record.rb | 1 +
activerecord/lib/active_record/base.rb | 4 ++
.../connection_adapters/abstract_adapter.rb | 7 +++
.../connection_adapters/mysql_adapter.rb | 12 ++++++
.../connection_adapters/postgresql_adapter.rb | 7 +++
.../connection_adapters/sqlite_adapter.rb | 7 +++
activerecord/lib/active_record/locale/en.yml | 2 +
.../lib/active_record/unique_constraints.rb | 24 ++++++++++++
activerecord/test/cases/adapter_test.rb | 35 +++++++++++++++++
activerecord/test/cases/unique_constraints_test.rb | 40 ++++++++++++++++++++
activerecord/test/models/unique_item.rb | 2 +
activerecord/test/schema/schema.rb | 15 +++++++
12 files changed, 156 insertions(+), 0 deletions(-)
create mode 100644 activerecord/lib/active_record/unique_constraints.rb
create mode 100644 activerecord/test/cases/unique_constraints_test.rb
create mode 100644 activerecord/test/models/unique_item.rb
diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb
index 2f8c5c7..8469255 100644
--- a/activerecord/lib/active_record.rb
+++ b/activerecord/lib/active_record.rb
@@ -68,6 +68,7 @@ module ActiveRecord
autoload :TestCase, 'active_record/test_case'
autoload :Timestamp, 'active_record/timestamp'
autoload :Transactions, 'active_record/transactions'
+ autoload :UniqueConstraints, 'active_record/unique_constraints'
autoload :Validations, 'active_record/validations'
module Locking
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index d6b264c..1fe6bf8 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -3177,6 +3177,10 @@ module ActiveRecord #:nodoc:
include AutosaveAssociation, NestedAttributes
include Aggregations, Transactions, Reflection, Batches, Calculations, Serialization
+
+ # UniqueConstraints needs to be included after Transactions, because we need #save_with_unique_constraints
+ # to rescue after the transaction has been resolved for postgres.
+ include UniqueConstraints
end
end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 6b7b363..6adea19 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -197,6 +197,13 @@ module ActiveRecord
end
end
+ # given an ActiveRecord::RecordNotUnique exception and a table name this returns the index whose
+ # unique constraint was violated, or nil if it can't determine the index
+ def index_for_record_not_unique(not_unique_exception, table_name) #:nodoc:
+ # override in derived class
+ nil
+ end
+
protected
def log(sql, name)
if block_given?
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 5414a1d..635a8fc 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -571,6 +571,18 @@ module ActiveRecord
where_sql
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ case exception.message
+ when /Duplicate entry.*for key (\d+)/
+ index_position = $1.to_i
+ # minus two b/c mysql message is one-based + rails excludes primary key index from indexes list
+ indexes(table_name)[index_position - 2]
+ when /Duplicate entry.*for key '(\w+)'/
+ index_name = $1
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
def translate_exception(exception, message)
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index b1cac88..c313082 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -941,6 +941,13 @@ module ActiveRecord
sql.replace "SELECT * FROM (#{sql}) AS id_list ORDER BY #{order}"
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/unique constraint "(\w+)"/)
+ index_name = match[1]
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
# Returns the version of the connected PostgreSQL version.
def postgresql_version
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index 79eea1d..31519f1 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -315,6 +315,13 @@ module ActiveRecord
"INSERT INTO #{table_name} VALUES(NULL)"
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/column(?:s)? (.*) (?:is|are) not unique/)
+ index_columns = match[1].split(', ')
+ indexes(table_name).detect { |i| i.columns == index_columns }
+ end
+ end
+
protected
def select(sql, name = nil) #:nodoc:
execute(sql, name).map do |row|
diff --git a/activerecord/lib/active_record/locale/en.yml b/activerecord/lib/active_record/locale/en.yml
index 2813524..a7dec95 100644
--- a/activerecord/lib/active_record/locale/en.yml
+++ b/activerecord/lib/active_record/locale/en.yml
@@ -15,6 +15,8 @@ en:
too_short: "is too short (minimum is {{count}} characters)"
wrong_length: "is the wrong length (should be {{count}} characters)"
taken: "has already been taken"
+ taken_multiple: "has already been taken for {{context}}"
+ taken_generic: "Unique requirement not met"
not_a_number: "is not a number"
greater_than: "must be greater than {{count}}"
greater_than_or_equal_to: "must be greater than or equal to {{count}}"
diff --git a/activerecord/lib/active_record/unique_constraints.rb b/activerecord/lib/active_record/unique_constraints.rb
new file mode 100644
index 0000000..99e6353
--- /dev/null
+++ b/activerecord/lib/active_record/unique_constraints.rb
@@ -0,0 +1,24 @@
+module ActiveRecord
+ # See ActiveRecord::Transactions::ClassMethods for documentation.
+ module UniqueConstraints
+ def self.included(base)
+ base.class_eval do
+ alias_method_chain :save, :unique_constraints
+ end
+ end
+
+ def save_with_unique_constraints(perform_validation = true) #:nodoc:
+ save_without_unique_constraints(perform_validation)
+ rescue ActiveRecord::RecordNotUnique => e
+ index = connection.index_for_record_not_unique(e.original_exception, self.class.table_name)
+ if !index
+ errors.add_to_base(I18n.translate(:'activerecord.errors.messages.taken_generic'))
+ elsif index.columns.size == 1
+ errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
+ else
+ errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
+ end
+ false
+ end
+ end
+end
\ No newline at end of file
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 4797112..0883ea0 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -157,4 +157,39 @@ class AdapterTest < ActiveRecord::TestCase
end
end
end
+
+ # tests for db-enforced unique constraints
+ unique_scenarios = {
+ :single => {:sql => "INSERT INTO unique_items(uniq) VALUES('abc')",
+ :expected => ['uniq']},
+ :single_named => {:sql => "INSERT INTO unique_items(uniq_named) VALUES('abc')",
+ :expected => ['uniq_named']},
+ :multiple => {:sql => "INSERT INTO unique_items(uniq_1, uniq_2, uniq_3) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1', 'uniq_2', 'uniq_3']},
+ :multiple_named => {:sql => "INSERT INTO unique_items(uniq_1_named, uniq_2_named, uniq_3_named) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1_named', 'uniq_2_named', 'uniq_3_named']}
+ }
+ unique_scenarios.each do |name, data|
+ test "finds index for unique constraint #{name}" do
+ @connection.execute data[:sql]
+ exception = get_exception(ActiveRecord::RecordNotUnique) do
+ @connection.execute data[:sql]
+ end
+ # postgres can't query indexes until transaction is rolled back
+ @connection.rollback_db_transaction if current_adapter?(:PostgreSQLAdapter)
+ assert_equal data[:expected], @connection.index_for_record_not_unique(exception, 'unique_items').columns
+ end
+ end
+
+ protected
+
+ def get_exception(expected_exception)
+ begin
+ yield
+ rescue expected_exception => e
+ e
+ else
+ flunk "Expected #{expected_exception} exception"
+ end
+ end
end
diff --git a/activerecord/test/cases/unique_constraints_test.rb b/activerecord/test/cases/unique_constraints_test.rb
new file mode 100644
index 0000000..2b5d0ee
--- /dev/null
+++ b/activerecord/test/cases/unique_constraints_test.rb
@@ -0,0 +1,40 @@
+require "cases/helper"
+require 'models/unique_item'
+
+class UniquenessValidationTest < ActiveRecord::TestCase
+
+ def test_validate_uniqueness_on_create
+ valid_item = UniqueItem.create!(:uniq => 'abc')
+ invalid_item = UniqueItem.create(:uniq => 'abc')
+ assert_equal "has already been taken", invalid_item.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_save
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.uniq = item_1.uniq
+ item_2.save
+ assert_equal "has already been taken", item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_update
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.update_attributes(:uniq => item_1.uniq)
+ assert_equal "has already been taken", item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_multiple
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal "has already been taken for uniq_2/uniq_3", invalid_item.errors[:uniq_1]
+ end
+
+ def test_validate_uniqueness_generic
+ ActiveRecord::Base.connection.expects(:index_for_record_not_unique).returns(nil)
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal "Unique requirement not met", invalid_item.errors[:base]
+ end
+
+end
\ No newline at end of file
diff --git a/activerecord/test/models/unique_item.rb b/activerecord/test/models/unique_item.rb
new file mode 100644
index 0000000..3c6f6f0
--- /dev/null
+++ b/activerecord/test/models/unique_item.rb
@@ -0,0 +1,2 @@
+class UniqueItem < ActiveRecord::Base
+end
\ No newline at end of file
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 984c5ba..8d64e51 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -479,6 +479,21 @@ ActiveRecord::Schema.define do
end
end
+ create_table :unique_items, :force => true do |t|
+ t.string :uniq
+ t.string :uniq_1
+ t.string :uniq_2
+ t.string :uniq_3
+ t.string :uniq_named
+ t.string :uniq_1_named
+ t.string :uniq_2_named
+ t.string :uniq_3_named
+ end
+ add_index :unique_items, [:uniq], :unique => true
+ add_index :unique_items, [:uniq_named], :unique => true, :name => 'unique_named_single_index'
+ add_index :unique_items, [:uniq_1, :uniq_2, :uniq_3], :unique => true
+ add_index :unique_items, [:uniq_1_named, :uniq_2_named, :uniq_3_named], :unique => true, :name => 'unique_named_multiple_index'
+
except 'SQLite' do
# fk_test_has_fk should be before fk_test_has_pk
create_table :fk_test_has_fk, :force => true do |t|
--
1.6.1.2
From 466ce19b2989324be1fca05b5b553700997b6803 Mon Sep 17 00:00:00 2001
From: Jordan Brough <jordan@animoto.com>
Date: Wed, 11 Nov 2009 11:34:27 -0700
Subject: [PATCH] Friendly ActiveRecord error handling for db-enforced unique constraints
---
activerecord/lib/active_record.rb | 1 +
activerecord/lib/active_record/base.rb | 4 ++
.../connection_adapters/abstract_adapter.rb | 7 +++
.../connection_adapters/mysql_adapter.rb | 12 ++++++
.../connection_adapters/postgresql_adapter.rb | 7 +++
.../connection_adapters/sqlite_adapter.rb | 7 +++
activerecord/lib/active_record/locale/en.yml | 2 +
.../lib/active_record/unique_constraints.rb | 24 ++++++++++++
activerecord/test/cases/adapter_test.rb | 35 +++++++++++++++++
activerecord/test/cases/unique_constraints_test.rb | 40 ++++++++++++++++++++
activerecord/test/models/unique_item.rb | 2 +
activerecord/test/schema/schema.rb | 15 +++++++
12 files changed, 156 insertions(+), 0 deletions(-)
create mode 100644 activerecord/lib/active_record/unique_constraints.rb
create mode 100644 activerecord/test/cases/unique_constraints_test.rb
create mode 100644 activerecord/test/models/unique_item.rb
diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb
index 8195e78..0aac5be 100644
--- a/activerecord/lib/active_record.rb
+++ b/activerecord/lib/active_record.rb
@@ -70,6 +70,7 @@ module ActiveRecord
autoload :TestCase, 'active_record/test_case'
autoload :Timestamp, 'active_record/timestamp'
autoload :Transactions, 'active_record/transactions'
+ autoload :UniqueConstraints, 'active_record/unique_constraints'
autoload :Types, 'active_record/types'
autoload :Validations, 'active_record/validations'
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 056f29f..e2efb2a 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -3094,6 +3094,10 @@ module ActiveRecord #:nodoc:
include Aggregations, Transactions, Reflection, Batches, Calculations, Serialization
+ # UniqueConstraints needs to be included after Transactions, because we need #save_with_unique_constraints
+ # to rescue after the transaction has been resolved for postgres.
+ include UniqueConstraints
+
end
end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 8fae26b..832b962 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -198,6 +198,13 @@ module ActiveRecord
end
end
+ # given an ActiveRecord::RecordNotUnique exception and a table name this returns the index whose
+ # unique constraint was violated, or nil if it can't determine the index
+ def index_for_record_not_unique(not_unique_exception, table_name) #:nodoc:
+ # override in derived class
+ nil
+ end
+
protected
def log(sql, name, &block)
ActiveSupport::Notifications.instrument(:sql, :sql => sql, :name => name, &block)
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index ad36ff2..3ca7608 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -561,6 +561,18 @@ module ActiveRecord
where_sql
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ case exception.message
+ when /Duplicate entry.*for key (\d+)/
+ index_position = $1.to_i
+ # minus two b/c mysql message is one-based + rails excludes primary key index from indexes list
+ indexes(table_name)[index_position - 2]
+ when /Duplicate entry.*for key '(\w+)'/
+ index_name = $1
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
def translate_exception(exception, message)
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 1d52c5e..70bbbd8 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -929,6 +929,13 @@ module ActiveRecord
sql << order_columns * ', '
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/unique constraint "(\w+)"/)
+ index_name = match[1]
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
# Returns the version of the connected PostgreSQL version.
def postgresql_version
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index c9c2892..72dc049 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -294,6 +294,13 @@ module ActiveRecord
"VALUES(NULL)"
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/column(?:s)? (.*) (?:is|are) not unique/)
+ index_columns = match[1].split(', ')
+ indexes(table_name).detect { |i| i.columns == index_columns }
+ end
+ end
+
protected
def select(sql, name = nil) #:nodoc:
execute(sql, name).map do |row|
diff --git a/activerecord/lib/active_record/locale/en.yml b/activerecord/lib/active_record/locale/en.yml
index 092f5f0..8144735 100644
--- a/activerecord/lib/active_record/locale/en.yml
+++ b/activerecord/lib/active_record/locale/en.yml
@@ -15,6 +15,8 @@ en:
too_short: "is too short (minimum is {{count}} characters)"
wrong_length: "is the wrong length (should be {{count}} characters)"
taken: "has already been taken"
+ taken_multiple: "has already been taken for {{context}}"
+ taken_generic: "Unique requirement not met"
not_a_number: "is not a number"
greater_than: "must be greater than {{count}}"
greater_than_or_equal_to: "must be greater than or equal to {{count}}"
diff --git a/activerecord/lib/active_record/unique_constraints.rb b/activerecord/lib/active_record/unique_constraints.rb
new file mode 100644
index 0000000..7a17b6a
--- /dev/null
+++ b/activerecord/lib/active_record/unique_constraints.rb
@@ -0,0 +1,24 @@
+module ActiveRecord
+ # See ActiveRecord::Transactions::ClassMethods for documentation.
+ module UniqueConstraints
+ def self.included(base)
+ base.class_eval do
+ alias_method_chain :save, :unique_constraints
+ end
+ end
+
+ def save_with_unique_constraints(perform_validation = true) #:nodoc:
+ save_without_unique_constraints(perform_validation)
+ rescue ActiveRecord::RecordNotUnique => e
+ index = connection.index_for_record_not_unique(e, self.class.table_name)
+ if !index
+ errors[:base] << I18n.translate(:'activerecord.errors.messages.taken_generic')
+ elsif index.columns.size == 1
+ errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
+ else
+ errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
+ end
+ false
+ end
+ end
+end
\ No newline at end of file
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index c59be26..4c8f58b 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -142,4 +142,39 @@ class AdapterTest < ActiveRecord::TestCase
end
end
end
+
+ # tests for db-enforced unique constraints
+ unique_scenarios = {
+ :single => {:sql => "INSERT INTO unique_items(uniq) VALUES('abc')",
+ :expected => ['uniq']},
+ :single_named => {:sql => "INSERT INTO unique_items(uniq_named) VALUES('abc')",
+ :expected => ['uniq_named']},
+ :multiple => {:sql => "INSERT INTO unique_items(uniq_1, uniq_2, uniq_3) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1', 'uniq_2', 'uniq_3']},
+ :multiple_named => {:sql => "INSERT INTO unique_items(uniq_1_named, uniq_2_named, uniq_3_named) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1_named', 'uniq_2_named', 'uniq_3_named']}
+ }
+ unique_scenarios.each do |name, data|
+ test "finds index for unique constraint #{name}" do
+ @connection.execute data[:sql]
+ exception = get_exception(ActiveRecord::RecordNotUnique) do
+ @connection.execute data[:sql]
+ end
+ # postgres can't query indexes until transaction is rolled back
+ @connection.rollback_db_transaction if current_adapter?(:PostgreSQLAdapter)
+ assert_equal data[:expected], @connection.index_for_record_not_unique(exception, 'unique_items').columns
+ end
+ end
+
+ protected
+
+ def get_exception(expected_exception)
+ begin
+ yield
+ rescue expected_exception => e
+ e
+ else
+ flunk "Expected #{expected_exception} exception"
+ end
+ end
end
diff --git a/activerecord/test/cases/unique_constraints_test.rb b/activerecord/test/cases/unique_constraints_test.rb
new file mode 100644
index 0000000..92ba6d8
--- /dev/null
+++ b/activerecord/test/cases/unique_constraints_test.rb
@@ -0,0 +1,40 @@
+require "cases/helper"
+require 'models/unique_item'
+
+class UniquenessValidationTest < ActiveRecord::TestCase
+
+ def test_validate_uniqueness_on_create
+ valid_item = UniqueItem.create!(:uniq => 'abc')
+ invalid_item = UniqueItem.create(:uniq => 'abc')
+ assert_equal ["has already been taken"], invalid_item.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_save
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.uniq = item_1.uniq
+ item_2.save
+ assert_equal ["has already been taken"], item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_update
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.update_attributes(:uniq => item_1.uniq)
+ assert_equal ["has already been taken"], item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_multiple
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal ["has already been taken for uniq_2/uniq_3"], invalid_item.errors[:uniq_1]
+ end
+
+ def test_validate_uniqueness_generic
+ ActiveRecord::Base.connection.expects(:index_for_record_not_unique).returns(nil)
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal ["Unique requirement not met"], invalid_item.errors[:base]
+ end
+
+end
\ No newline at end of file
diff --git a/activerecord/test/models/unique_item.rb b/activerecord/test/models/unique_item.rb
new file mode 100644
index 0000000..3c6f6f0
--- /dev/null
+++ b/activerecord/test/models/unique_item.rb
@@ -0,0 +1,2 @@
+class UniqueItem < ActiveRecord::Base
+end
\ No newline at end of file
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 15e5e12..a2b3da4 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -532,6 +532,21 @@ ActiveRecord::Schema.define do
t.string :title
end
+ create_table :unique_items, :force => true do |t|
+ t.string :uniq
+ t.string :uniq_1
+ t.string :uniq_2
+ t.string :uniq_3
+ t.string :uniq_named
+ t.string :uniq_1_named
+ t.string :uniq_2_named
+ t.string :uniq_3_named
+ end
+ add_index :unique_items, [:uniq], :unique => true
+ add_index :unique_items, [:uniq_named], :unique => true, :name => 'unique_named_single_index'
+ add_index :unique_items, [:uniq_1, :uniq_2, :uniq_3], :unique => true
+ add_index :unique_items, [:uniq_1_named, :uniq_2_named, :uniq_3_named], :unique => true, :name => 'unique_named_multiple_index'
+
except 'SQLite' do
# fk_test_has_fk should be before fk_test_has_pk
create_table :fk_test_has_fk, :force => true do |t|
--
1.6.1.2
From 5573e913b8651ea53ae6d94686b4b63f2cca2e83 Mon Sep 17 00:00:00 2001
From: Michael Schuerig <michael@schuerig.de>
Date: Sun, 5 Apr 2009 01:19:29 +0200
Subject: [PATCH 1/4] Translate adapter errors that indicate a violated uniqueness constraint to ActiveRecord::RecordNotUnique exception derived from ActiveReecord::StatementInvalid.
Signed-off-by: Michael Koziarski <michael@koziarski.com>
---
activerecord/lib/active_record/base.rb | 4 ++++
.../connection_adapters/abstract_adapter.rb | 7 ++++++-
.../connection_adapters/mysql_adapter.rb | 11 +++++++++++
.../connection_adapters/postgresql_adapter.rb | 9 +++++++++
.../connection_adapters/sqlite_adapter.rb | 10 ++++++++++
activerecord/test/cases/adapter_test.rb | 7 +++++++
6 files changed, 47 insertions(+), 1 deletions(-)
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 4929969..27b645d 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -55,6 +55,10 @@ module ActiveRecord #:nodoc:
class StatementInvalid < ActiveRecordError
end
+ # Raised when a record cannot be inserted because it would violate a uniqueness constraint.
+ class RecordNotUnique < StatementInvalid
+ end
+
# Raised when number of bind variables in statement given to <tt>:condition</tt> key (for example, when using +find+ method)
# does not match number of expected variables.
#
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 22871f2..6b7b363 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -216,9 +216,14 @@ module ActiveRecord
@last_verification = 0
message = "#{e.class.name}: #{e.message}: #{sql}"
log_info(message, name, 0)
- raise ActiveRecord::StatementInvalid, message
+ raise translate_exception(e, message)
end
+ def translate_exception(e, message)
+ # override in derived class
+ ActiveRecord::StatementInvalid.new(message)
+ end
+
def format_log_entry(message, dump = nil)
if ActiveRecord::Base.colorize_logging
if @@row_even
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index a8aa16e..8625395 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -586,6 +586,17 @@ module ActiveRecord
where_sql
end
+ protected
+
+ def translate_exception(exception, message)
+ case exception.errno
+ when 1062
+ RecordNotUnique.new(message)
+ else
+ super
+ end
+ end
+
private
def connect
encoding = @config[:encoding]
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index bc289ff..4bd7e4a 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -958,6 +958,15 @@ module ActiveRecord
end
end
+ def translate_exception(exception, message)
+ case exception.message
+ when /duplicate key value violates unique constraint/
+ RecordNotUnique.new(message)
+ else
+ super
+ end
+ end
+
private
# The internal PostgreSQL identifier of the money data type.
MONEY_COLUMN_TYPE_OID = 790 #:nodoc:
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index 0bf97a9..88831ee 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -435,6 +435,16 @@ module ActiveRecord
'INTEGER PRIMARY KEY NOT NULL'.freeze
end
end
+
+ def translate_exception(exception, message)
+ case exception.message
+ when /column(s)? .* (is|are) not unique/
+ RecordNotUnique.new(message)
+ else
+ super
+ end
+ end
+
end
class SQLite2Adapter < SQLiteAdapter # :nodoc:
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 3dd3dd8..27d1625 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -142,4 +142,11 @@ class AdapterTest < ActiveRecord::TestCase
assert_equal " LIMIT 1,7 OFFSET 7", @connection.add_limit_offset!("", :limit=>sql_inject, :offset=>7)
end
end
+
+ def test_uniqueness_violations_are_translated_to_specific_exception
+ @connection.execute "INSERT INTO subscribers(nick) VALUES('me')"
+ assert_raises(ActiveRecord::RecordNotUnique) do
+ @connection.execute "INSERT INTO subscribers(nick) VALUES('me')"
+ end
+ end
end
--
1.6.1.2
From 1b69f6d39bbb35be22de771261426bfdb2b0459e Mon Sep 17 00:00:00 2001
From: Michael Schuerig <michael@schuerig.de>
Date: Sun, 5 Apr 2009 01:42:21 +0200
Subject: [PATCH 2/4] Translate foreign key violations to ActiveRecord::InvalidForeignKey exceptions.
Signed-off-by: Michael Koziarski <michael@koziarski.com>
---
activerecord/lib/active_record/base.rb | 4 ++++
.../connection_adapters/mysql_adapter.rb | 2 ++
.../connection_adapters/postgresql_adapter.rb | 2 ++
activerecord/test/cases/adapter_test.rb | 8 ++++++++
4 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 27b645d..57c1d6d 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -59,6 +59,10 @@ module ActiveRecord #:nodoc:
class RecordNotUnique < StatementInvalid
end
+ # Raised when a record cannot be inserted or updated because it references a non-existent record.
+ class InvalidForeignKey < StatementInvalid
+ end
+
# Raised when number of bind variables in statement given to <tt>:condition</tt> key (for example, when using +find+ method)
# does not match number of expected variables.
#
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 8625395..ac3dfda 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -592,6 +592,8 @@ module ActiveRecord
case exception.errno
when 1062
RecordNotUnique.new(message)
+ when 1452
+ InvalidForeignKey.new(message)
else
super
end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 4bd7e4a..4ab06e8 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -962,6 +962,8 @@ module ActiveRecord
case exception.message
when /duplicate key value violates unique constraint/
RecordNotUnique.new(message)
+ when /violates foreign key constraint/
+ InvalidForeignKey.new(message)
else
super
end
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 27d1625..4797112 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -149,4 +149,12 @@ class AdapterTest < ActiveRecord::TestCase
@connection.execute "INSERT INTO subscribers(nick) VALUES('me')"
end
end
+
+ def test_foreign_key_violations_are_translated_to_specific_exception
+ unless @connection.adapter_name == 'SQLite'
+ assert_raises(ActiveRecord::InvalidForeignKey) do
+ @connection.execute "INSERT INTO fk_test_has_fk (fk_id) VALUES (0)"
+ end
+ end
+ end
end
--
1.6.1.2
From edffa1bda17cca21e84cc953ebbabae4d295d0db Mon Sep 17 00:00:00 2001
From: Michael Koziarski <michael@koziarski.com>
Date: Fri, 26 Jun 2009 16:59:27 +1200
Subject: [PATCH 3/4] Make sure the wrapped exceptions also have the original exception available.
[#2419 state:committed]
---
activerecord/lib/active_record/base.rb | 15 +++++++++++++--
.../connection_adapters/mysql_adapter.rb | 4 ++--
.../connection_adapters/postgresql_adapter.rb | 4 ++--
.../connection_adapters/sqlite_adapter.rb | 2 +-
4 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 57c1d6d..7c8c866 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -55,12 +55,23 @@ module ActiveRecord #:nodoc:
class StatementInvalid < ActiveRecordError
end
+ # Parent class for all specific exceptions which wrap database driver exceptions
+ # provides access to the original exception also.
+ class WrappedDatabaseException < StatementInvalid
+ attr_reader :original_exception
+
+ def initialize(message, original_exception)
+ super(message)
+ @original_exception, = original_exception
+ end
+ end
+
# Raised when a record cannot be inserted because it would violate a uniqueness constraint.
- class RecordNotUnique < StatementInvalid
+ class RecordNotUnique < WrappedDatabaseException
end
# Raised when a record cannot be inserted or updated because it references a non-existent record.
- class InvalidForeignKey < StatementInvalid
+ class InvalidForeignKey < WrappedDatabaseException
end
# Raised when number of bind variables in statement given to <tt>:condition</tt> key (for example, when using +find+ method)
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index ac3dfda..1fffec8 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -591,9 +591,9 @@ module ActiveRecord
def translate_exception(exception, message)
case exception.errno
when 1062
- RecordNotUnique.new(message)
+ RecordNotUnique.new(message, exception)
when 1452
- InvalidForeignKey.new(message)
+ InvalidForeignKey.new(message, exception)
else
super
end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 4ab06e8..b1cac88 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -961,9 +961,9 @@ module ActiveRecord
def translate_exception(exception, message)
case exception.message
when /duplicate key value violates unique constraint/
- RecordNotUnique.new(message)
+ RecordNotUnique.new(message, exception)
when /violates foreign key constraint/
- InvalidForeignKey.new(message)
+ InvalidForeignKey.new(message, exception)
else
super
end
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index 88831ee..79eea1d 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -439,7 +439,7 @@ module ActiveRecord
def translate_exception(exception, message)
case exception.message
when /column(s)? .* (is|are) not unique/
- RecordNotUnique.new(message)
+ RecordNotUnique.new(message, exception)
else
super
end
--
1.6.1.2
From 5dc2eb5d07f8e999f0c9d0d3932a5a7d5bce2dae Mon Sep 17 00:00:00 2001
From: Jordan Brough <jordan@animoto.com>
Date: Wed, 11 Nov 2009 13:31:34 -0700
Subject: [PATCH 4/4] Friendly ActiveRecord error handling for db-enforced unique constraints
---
activerecord/lib/active_record.rb | 1 +
activerecord/lib/active_record/base.rb | 4 ++
.../connection_adapters/abstract_adapter.rb | 7 +++
.../connection_adapters/mysql_adapter.rb | 12 ++++++
.../connection_adapters/postgresql_adapter.rb | 7 +++
.../connection_adapters/sqlite_adapter.rb | 7 +++
activerecord/lib/active_record/locale/en.yml | 2 +
.../lib/active_record/unique_constraints.rb | 24 ++++++++++++
activerecord/test/cases/adapter_test.rb | 35 +++++++++++++++++
activerecord/test/cases/unique_constraints_test.rb | 40 ++++++++++++++++++++
activerecord/test/models/unique_item.rb | 2 +
activerecord/test/schema/schema.rb | 15 +++++++
12 files changed, 156 insertions(+), 0 deletions(-)
create mode 100644 activerecord/lib/active_record/unique_constraints.rb
create mode 100644 activerecord/test/cases/unique_constraints_test.rb
create mode 100644 activerecord/test/models/unique_item.rb
diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb
index 2f8c5c7..8469255 100644
--- a/activerecord/lib/active_record.rb
+++ b/activerecord/lib/active_record.rb
@@ -68,6 +68,7 @@ module ActiveRecord
autoload :TestCase, 'active_record/test_case'
autoload :Timestamp, 'active_record/timestamp'
autoload :Transactions, 'active_record/transactions'
+ autoload :UniqueConstraints, 'active_record/unique_constraints'
autoload :Validations, 'active_record/validations'
module Locking
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 7c8c866..da9c110 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -3200,6 +3200,10 @@ module ActiveRecord #:nodoc:
include AutosaveAssociation, NestedAttributes
include Aggregations, Transactions, Reflection, Batches, Calculations, Serialization
+
+ # UniqueConstraints needs to be included after Transactions, because we need #save_with_unique_constraints
+ # to rescue after the transaction has been resolved for postgres.
+ include UniqueConstraints
end
end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 6b7b363..6adea19 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -197,6 +197,13 @@ module ActiveRecord
end
end
+ # given an ActiveRecord::RecordNotUnique exception and a table name this returns the index whose
+ # unique constraint was violated, or nil if it can't determine the index
+ def index_for_record_not_unique(not_unique_exception, table_name) #:nodoc:
+ # override in derived class
+ nil
+ end
+
protected
def log(sql, name)
if block_given?
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 1fffec8..658df1f 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -586,6 +586,18 @@ module ActiveRecord
where_sql
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ case exception.message
+ when /Duplicate entry.*for key (\d+)/
+ index_position = $1.to_i
+ # minus two b/c mysql message is one-based + rails excludes primary key index from indexes list
+ indexes(table_name)[index_position - 2]
+ when /Duplicate entry.*for key '(\w+)'/
+ index_name = $1
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
def translate_exception(exception, message)
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index b1cac88..c313082 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -941,6 +941,13 @@ module ActiveRecord
sql.replace "SELECT * FROM (#{sql}) AS id_list ORDER BY #{order}"
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/unique constraint "(\w+)"/)
+ index_name = match[1]
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
# Returns the version of the connected PostgreSQL version.
def postgresql_version
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index 79eea1d..31519f1 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -315,6 +315,13 @@ module ActiveRecord
"INSERT INTO #{table_name} VALUES(NULL)"
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/column(?:s)? (.*) (?:is|are) not unique/)
+ index_columns = match[1].split(', ')
+ indexes(table_name).detect { |i| i.columns == index_columns }
+ end
+ end
+
protected
def select(sql, name = nil) #:nodoc:
execute(sql, name).map do |row|
diff --git a/activerecord/lib/active_record/locale/en.yml b/activerecord/lib/active_record/locale/en.yml
index 2813524..a7dec95 100644
--- a/activerecord/lib/active_record/locale/en.yml
+++ b/activerecord/lib/active_record/locale/en.yml
@@ -15,6 +15,8 @@ en:
too_short: "is too short (minimum is {{count}} characters)"
wrong_length: "is the wrong length (should be {{count}} characters)"
taken: "has already been taken"
+ taken_multiple: "has already been taken for {{context}}"
+ taken_generic: "Unique requirement not met"
not_a_number: "is not a number"
greater_than: "must be greater than {{count}}"
greater_than_or_equal_to: "must be greater than or equal to {{count}}"
diff --git a/activerecord/lib/active_record/unique_constraints.rb b/activerecord/lib/active_record/unique_constraints.rb
new file mode 100644
index 0000000..99e6353
--- /dev/null
+++ b/activerecord/lib/active_record/unique_constraints.rb
@@ -0,0 +1,24 @@
+module ActiveRecord
+ # See ActiveRecord::Transactions::ClassMethods for documentation.
+ module UniqueConstraints
+ def self.included(base)
+ base.class_eval do
+ alias_method_chain :save, :unique_constraints
+ end
+ end
+
+ def save_with_unique_constraints(perform_validation = true) #:nodoc:
+ save_without_unique_constraints(perform_validation)
+ rescue ActiveRecord::RecordNotUnique => e
+ index = connection.index_for_record_not_unique(e.original_exception, self.class.table_name)
+ if !index
+ errors.add_to_base(I18n.translate(:'activerecord.errors.messages.taken_generic'))
+ elsif index.columns.size == 1
+ errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
+ else
+ errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
+ end
+ false
+ end
+ end
+end
\ No newline at end of file
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 4797112..0883ea0 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -157,4 +157,39 @@ class AdapterTest < ActiveRecord::TestCase
end
end
end
+
+ # tests for db-enforced unique constraints
+ unique_scenarios = {
+ :single => {:sql => "INSERT INTO unique_items(uniq) VALUES('abc')",
+ :expected => ['uniq']},
+ :single_named => {:sql => "INSERT INTO unique_items(uniq_named) VALUES('abc')",
+ :expected => ['uniq_named']},
+ :multiple => {:sql => "INSERT INTO unique_items(uniq_1, uniq_2, uniq_3) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1', 'uniq_2', 'uniq_3']},
+ :multiple_named => {:sql => "INSERT INTO unique_items(uniq_1_named, uniq_2_named, uniq_3_named) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1_named', 'uniq_2_named', 'uniq_3_named']}
+ }
+ unique_scenarios.each do |name, data|
+ test "finds index for unique constraint #{name}" do
+ @connection.execute data[:sql]
+ exception = get_exception(ActiveRecord::RecordNotUnique) do
+ @connection.execute data[:sql]
+ end
+ # postgres can't query indexes until transaction is rolled back
+ @connection.rollback_db_transaction if current_adapter?(:PostgreSQLAdapter)
+ assert_equal data[:expected], @connection.index_for_record_not_unique(exception, 'unique_items').columns
+ end
+ end
+
+ protected
+
+ def get_exception(expected_exception)
+ begin
+ yield
+ rescue expected_exception => e
+ e
+ else
+ flunk "Expected #{expected_exception} exception"
+ end
+ end
end
diff --git a/activerecord/test/cases/unique_constraints_test.rb b/activerecord/test/cases/unique_constraints_test.rb
new file mode 100644
index 0000000..2b5d0ee
--- /dev/null
+++ b/activerecord/test/cases/unique_constraints_test.rb
@@ -0,0 +1,40 @@
+require "cases/helper"
+require 'models/unique_item'
+
+class UniquenessValidationTest < ActiveRecord::TestCase
+
+ def test_validate_uniqueness_on_create
+ valid_item = UniqueItem.create!(:uniq => 'abc')
+ invalid_item = UniqueItem.create(:uniq => 'abc')
+ assert_equal "has already been taken", invalid_item.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_save
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.uniq = item_1.uniq
+ item_2.save
+ assert_equal "has already been taken", item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_update
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.update_attributes(:uniq => item_1.uniq)
+ assert_equal "has already been taken", item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_multiple
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal "has already been taken for uniq_2/uniq_3", invalid_item.errors[:uniq_1]
+ end
+
+ def test_validate_uniqueness_generic
+ ActiveRecord::Base.connection.expects(:index_for_record_not_unique).returns(nil)
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal "Unique requirement not met", invalid_item.errors[:base]
+ end
+
+end
\ No newline at end of file
diff --git a/activerecord/test/models/unique_item.rb b/activerecord/test/models/unique_item.rb
new file mode 100644
index 0000000..3c6f6f0
--- /dev/null
+++ b/activerecord/test/models/unique_item.rb
@@ -0,0 +1,2 @@
+class UniqueItem < ActiveRecord::Base
+end
\ No newline at end of file
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index b046104..17a6fc4 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -499,6 +499,21 @@ ActiveRecord::Schema.define do
t.string :title
end
+ create_table :unique_items, :force => true do |t|
+ t.string :uniq
+ t.string :uniq_1
+ t.string :uniq_2
+ t.string :uniq_3
+ t.string :uniq_named
+ t.string :uniq_1_named
+ t.string :uniq_2_named
+ t.string :uniq_3_named
+ end
+ add_index :unique_items, [:uniq], :unique => true
+ add_index :unique_items, [:uniq_named], :unique => true, :name => 'unique_named_single_index'
+ add_index :unique_items, [:uniq_1, :uniq_2, :uniq_3], :unique => true
+ add_index :unique_items, [:uniq_1_named, :uniq_2_named, :uniq_3_named], :unique => true, :name => 'unique_named_multiple_index'
+
except 'SQLite' do
# fk_test_has_fk should be before fk_test_has_pk
create_table :fk_test_has_fk, :force => true do |t|
--
1.6.1.2
From 48fcff95ae1b5e55837e81a4c132a3c208a235f8 Mon Sep 17 00:00:00 2001
From: Michael Schuerig <michael@schuerig.de>
Date: Sun, 5 Apr 2009 01:19:29 +0200
Subject: [PATCH 1/5] Translate adapter errors that indicate a violated uniqueness constraint to ActiveRecord::RecordNotUnique exception derived from ActiveReecord::StatementInvalid.
Signed-off-by: Michael Koziarski <michael@koziarski.com>
---
activerecord/lib/active_record/base.rb | 4 ++++
.../connection_adapters/abstract_adapter.rb | 7 ++++++-
.../connection_adapters/mysql_adapter.rb | 9 +++++++++
.../connection_adapters/postgresql_adapter.rb | 9 +++++++++
.../connection_adapters/sqlite_adapter.rb | 10 ++++++++++
activerecord/test/cases/adapter_test.rb | 7 +++++++
6 files changed, 45 insertions(+), 1 deletions(-)
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index ac82cc1..94468a4 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -56,6 +56,10 @@ module ActiveRecord #:nodoc:
class StatementInvalid < ActiveRecordError
end
+ # Raised when a record cannot be inserted because it would violate a uniqueness constraint.
+ class RecordNotUnique < StatementInvalid
+ end
+
# Raised when number of bind variables in statement given to <tt>:condition</tt> key (for example, when using +find+ method)
# does not match number of expected variables.
#
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index b8a6a71..6d21960 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -224,9 +224,14 @@ module ActiveRecord
@last_verification = 0
message = "#{e.class.name}: #{e.message}: #{sql}"
log_info(message, name, 0)
- raise ActiveRecord::StatementInvalid, message
+ raise translate_exception(e, message)
end
+ def translate_exception(e, message)
+ # override in derived class
+ ActiveRecord::StatementInvalid.new(message)
+ end
+
def format_log_entry(message, dump = nil)
if ActiveRecord::Base.colorize_logging
if @@row_even
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 6f41f84..62d9417 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -602,6 +602,15 @@ module ActiveRecord
end
end
+ def translate_exception(exception, message)
+ case exception.errno
+ when 1062
+ RecordNotUnique.new(message)
+ else
+ super
+ end
+ end
+
private
def connect
encoding = @config[:encoding]
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index a348318..66afc3c 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -931,6 +931,15 @@ module ActiveRecord
end
end
+ def translate_exception(exception, message)
+ case exception.message
+ when /duplicate key value violates unique constraint/
+ RecordNotUnique.new(message)
+ else
+ super
+ end
+ end
+
private
# The internal PostgreSQL identifier of the money data type.
MONEY_COLUMN_TYPE_OID = 790 #:nodoc:
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index 1616698..e0d04e6 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -436,6 +436,16 @@ module ActiveRecord
'INTEGER PRIMARY KEY NOT NULL'.freeze
end
end
+
+ def translate_exception(exception, message)
+ case exception.message
+ when /column(s)? .* (is|are) not unique/
+ RecordNotUnique.new(message)
+ else
+ super
+ end
+ end
+
end
class SQLite2Adapter < SQLiteAdapter # :nodoc:
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index b7fa6df..2118c10 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -141,4 +141,11 @@ class AdapterTest < ActiveRecord::TestCase
assert_equal " LIMIT 1,7 OFFSET 7", @connection.add_limit_offset!("", :limit=>sql_inject, :offset=>7)
end
end
+
+ def test_uniqueness_violations_are_translated_to_specific_exception
+ @connection.execute "INSERT INTO subscribers(nick) VALUES('me')"
+ assert_raises(ActiveRecord::RecordNotUnique) do
+ @connection.execute "INSERT INTO subscribers(nick) VALUES('me')"
+ end
+ end
end
--
1.7.2.1
From 9efed026a931dc4203b4332b0951b8b09c669476 Mon Sep 17 00:00:00 2001
From: Michael Schuerig <michael@schuerig.de>
Date: Sun, 5 Apr 2009 01:42:21 +0200
Subject: [PATCH 2/5] Translate foreign key violations to ActiveRecord::InvalidForeignKey exceptions.
Signed-off-by: Michael Koziarski <michael@koziarski.com>
---
activerecord/lib/active_record/base.rb | 4 ++++
.../connection_adapters/mysql_adapter.rb | 2 ++
.../connection_adapters/postgresql_adapter.rb | 2 ++
activerecord/test/cases/adapter_test.rb | 8 ++++++++
4 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 94468a4..c42b1ab 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -60,6 +60,10 @@ module ActiveRecord #:nodoc:
class RecordNotUnique < StatementInvalid
end
+ # Raised when a record cannot be inserted or updated because it references a non-existent record.
+ class InvalidForeignKey < StatementInvalid
+ end
+
# Raised when number of bind variables in statement given to <tt>:condition</tt> key (for example, when using +find+ method)
# does not match number of expected variables.
#
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 62d9417..e49908b 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -606,6 +606,8 @@ module ActiveRecord
case exception.errno
when 1062
RecordNotUnique.new(message)
+ when 1452
+ InvalidForeignKey.new(message)
else
super
end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 66afc3c..d79aea5 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -935,6 +935,8 @@ module ActiveRecord
case exception.message
when /duplicate key value violates unique constraint/
RecordNotUnique.new(message)
+ when /violates foreign key constraint/
+ InvalidForeignKey.new(message)
else
super
end
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 2118c10..7a9a289 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -148,4 +148,12 @@ class AdapterTest < ActiveRecord::TestCase
@connection.execute "INSERT INTO subscribers(nick) VALUES('me')"
end
end
+
+ def test_foreign_key_violations_are_translated_to_specific_exception
+ unless @connection.adapter_name == 'SQLite'
+ assert_raises(ActiveRecord::InvalidForeignKey) do
+ @connection.execute "INSERT INTO fk_test_has_fk (fk_id) VALUES (0)"
+ end
+ end
+ end
end
--
1.7.2.1
From 3d2796f98d7f2c33bb63f6c2f2026588b99fb2c9 Mon Sep 17 00:00:00 2001
From: Michael Koziarski <michael@koziarski.com>
Date: Fri, 26 Jun 2009 16:59:27 +1200
Subject: [PATCH 3/5] Make sure the wrapped exceptions also have the original exception available.
[#2419 state:committed]
---
activerecord/lib/active_record/base.rb | 15 +++++++++++++--
.../connection_adapters/mysql_adapter.rb | 4 ++--
.../connection_adapters/postgresql_adapter.rb | 4 ++--
.../connection_adapters/sqlite_adapter.rb | 2 +-
4 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index c42b1ab..9fc1ea5 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -56,12 +56,23 @@ module ActiveRecord #:nodoc:
class StatementInvalid < ActiveRecordError
end
+ # Parent class for all specific exceptions which wrap database driver exceptions
+ # provides access to the original exception also.
+ class WrappedDatabaseException < StatementInvalid
+ attr_reader :original_exception
+
+ def initialize(message, original_exception)
+ super(message)
+ @original_exception, = original_exception
+ end
+ end
+
# Raised when a record cannot be inserted because it would violate a uniqueness constraint.
- class RecordNotUnique < StatementInvalid
+ class RecordNotUnique < WrappedDatabaseException
end
# Raised when a record cannot be inserted or updated because it references a non-existent record.
- class InvalidForeignKey < StatementInvalid
+ class InvalidForeignKey < WrappedDatabaseException
end
# Raised when number of bind variables in statement given to <tt>:condition</tt> key (for example, when using +find+ method)
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index e49908b..0ecab7b 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -605,9 +605,9 @@ module ActiveRecord
def translate_exception(exception, message)
case exception.errno
when 1062
- RecordNotUnique.new(message)
+ RecordNotUnique.new(message, exception)
when 1452
- InvalidForeignKey.new(message)
+ InvalidForeignKey.new(message, exception)
else
super
end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index d79aea5..590a4f1 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -934,9 +934,9 @@ module ActiveRecord
def translate_exception(exception, message)
case exception.message
when /duplicate key value violates unique constraint/
- RecordNotUnique.new(message)
+ RecordNotUnique.new(message, exception)
when /violates foreign key constraint/
- InvalidForeignKey.new(message)
+ InvalidForeignKey.new(message, exception)
else
super
end
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index e0d04e6..ae1ffa0 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -440,7 +440,7 @@ module ActiveRecord
def translate_exception(exception, message)
case exception.message
when /column(s)? .* (is|are) not unique/
- RecordNotUnique.new(message)
+ RecordNotUnique.new(message, exception)
else
super
end
--
1.7.2.1
From 11a16a9838ecabfb25f5fc89e93bdc3d7b25c4bf Mon Sep 17 00:00:00 2001
From: Jordan Brough <jordan@animoto.com>
Date: Wed, 11 Nov 2009 13:31:34 -0700
Subject: [PATCH 4/5] Friendly ActiveRecord error handling for db-enforced unique constraints
---
activerecord/lib/active_record.rb | 1 +
activerecord/lib/active_record/base.rb | 1 +
.../connection_adapters/abstract_adapter.rb | 7 +++
.../connection_adapters/mysql_adapter.rb | 12 ++++++
.../connection_adapters/postgresql_adapter.rb | 13 ++++++
.../connection_adapters/sqlite_adapter.rb | 7 +++
activerecord/lib/active_record/locale/en.yml | 2 +
.../lib/active_record/unique_constraints.rb | 30 +++++++++++++++
activerecord/test/cases/adapter_test.rb | 35 +++++++++++++++++
activerecord/test/cases/unique_constraints_test.rb | 40 ++++++++++++++++++++
activerecord/test/models/unique_item.rb | 2 +
activerecord/test/schema/schema.rb | 15 +++++++
12 files changed, 165 insertions(+), 0 deletions(-)
create mode 100644 activerecord/lib/active_record/unique_constraints.rb
create mode 100644 activerecord/test/cases/unique_constraints_test.rb
create mode 100644 activerecord/test/models/unique_item.rb
diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb
index 0d0ada9..3404c16 100644
--- a/activerecord/lib/active_record.rb
+++ b/activerecord/lib/active_record.rb
@@ -68,6 +68,7 @@ module ActiveRecord
autoload :TestCase, 'active_record/test_case'
autoload :Timestamp, 'active_record/timestamp'
autoload :Transactions, 'active_record/transactions'
+ autoload :UniqueConstraints, 'active_record/unique_constraints'
autoload :Validations, 'active_record/validations'
module Locking
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 9fc1ea5..2e79954 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -3230,6 +3230,7 @@ module ActiveRecord #:nodoc:
include AutosaveAssociation, NestedAttributes
include Aggregations, Transactions, Reflection, Batches, Calculations, Serialization
+ include UniqueConstraints
end
end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 6d21960..c7e8269 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -199,6 +199,13 @@ module ActiveRecord
end
end
+ # given an ActiveRecord::RecordNotUnique exception and a table name this returns the index whose
+ # unique constraint was violated, or nil if it can't determine the index
+ def index_for_record_not_unique(not_unique_exception, table_name) #:nodoc:
+ # override in derived class
+ nil
+ end
+
protected
def log(sql, name)
if block_given?
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 0ecab7b..64f8f87 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -588,6 +588,18 @@ module ActiveRecord
where_sql
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ case exception.message
+ when /Duplicate entry.*for key (\d+)/
+ index_position = $1.to_i
+ # minus two b/c mysql message is one-based + rails excludes primary key index from indexes list
+ indexes(table_name)[index_position - 2]
+ when /Duplicate entry.*for key '(\w+)'/
+ index_name = $1
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
def quoted_columns_for_index(column_names, options = {})
length = options[:length] if options.is_a?(Hash)
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 590a4f1..2eb3fce 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -669,6 +669,12 @@ module ActiveRecord
indexes
end
+ # postgres doesn't let us query this in the middle of an aborted transaction so caching can come in handy
+ def cached_indexes(table_name)
+ @cached_indexes ||= {}
+ @cached_indexes[table_name] ||= indexes(table_name)
+ end
+
# Returns the list of all column definitions for a table.
def columns(table_name, name = nil)
# Limit, precision, and scale are all handled by the superclass.
@@ -914,6 +920,13 @@ module ActiveRecord
sql.replace "SELECT * FROM (#{sql}) AS id_list ORDER BY #{order}"
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/unique constraint "(\w+)"/)
+ index_name = match[1]
+ cached_indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
# Returns the version of the connected PostgreSQL version.
def postgresql_version
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index ae1ffa0..a43f6d4 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -316,6 +316,13 @@ module ActiveRecord
"INSERT INTO #{table_name} VALUES(NULL)"
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/column(?:s)? (.*) (?:is|are) not unique/)
+ index_columns = match[1].split(', ')
+ indexes(table_name).detect { |i| i.columns == index_columns }
+ end
+ end
+
protected
def select(sql, name = nil) #:nodoc:
execute(sql, name).map do |row|
diff --git a/activerecord/lib/active_record/locale/en.yml b/activerecord/lib/active_record/locale/en.yml
index 6dab5e2..353fbda 100644
--- a/activerecord/lib/active_record/locale/en.yml
+++ b/activerecord/lib/active_record/locale/en.yml
@@ -15,6 +15,8 @@ en:
too_short: "is too short (minimum is %{count} characters)"
wrong_length: "is the wrong length (should be %{count} characters)"
taken: "has already been taken"
+ taken_multiple: "has already been taken for %{context}"
+ taken_generic: "Unique requirement not met"
not_a_number: "is not a number"
greater_than: "must be greater than %{count}"
greater_than_or_equal_to: "must be greater than or equal to %{count}"
diff --git a/activerecord/lib/active_record/unique_constraints.rb b/activerecord/lib/active_record/unique_constraints.rb
new file mode 100644
index 0000000..6702e37
--- /dev/null
+++ b/activerecord/lib/active_record/unique_constraints.rb
@@ -0,0 +1,30 @@
+module ActiveRecord
+ # See ActiveRecord::Transactions::ClassMethods for documentation.
+ module UniqueConstraints
+ def self.included(base)
+ base.class_eval do
+ alias_method_chain :save, :unique_constraints
+ end
+ end
+
+ # work around postgres not allowing us to query indexes in the middle of an aborted transaction
+ def cache_indexes
+ connection.cached_indexes(self.class.table_name) if connection.respond_to?(:cached_indexes)
+ end
+
+ def save_with_unique_constraints(perform_validation = true) #:nodoc:
+ cache_indexes
+ save_without_unique_constraints(perform_validation)
+ rescue ActiveRecord::RecordNotUnique => e
+ index = connection.index_for_record_not_unique(e.original_exception, self.class.table_name)
+ if !index
+ errors.add_to_base(I18n.translate(:'activerecord.errors.messages.taken_generic'))
+ elsif index.columns.size == 1
+ errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
+ else
+ errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
+ end
+ false
+ end
+ end
+end
\ No newline at end of file
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 7a9a289..f99e8d6 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -156,4 +156,39 @@ class AdapterTest < ActiveRecord::TestCase
end
end
end
+
+ # tests for db-enforced unique constraints
+ unique_scenarios = {
+ :single => {:sql => "INSERT INTO unique_items(uniq) VALUES('abc')",
+ :expected => ['uniq']},
+ :single_named => {:sql => "INSERT INTO unique_items(uniq_named) VALUES('abc')",
+ :expected => ['uniq_named']},
+ :multiple => {:sql => "INSERT INTO unique_items(uniq_1, uniq_2, uniq_3) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1', 'uniq_2', 'uniq_3']},
+ :multiple_named => {:sql => "INSERT INTO unique_items(uniq_1_named, uniq_2_named, uniq_3_named) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1_named', 'uniq_2_named', 'uniq_3_named']}
+ }
+ unique_scenarios.each do |name, data|
+ test "finds index for unique constraint #{name}" do
+ @connection.execute data[:sql]
+ exception = get_exception(ActiveRecord::RecordNotUnique) do
+ @connection.execute data[:sql]
+ end
+ # postgres can't query indexes until transaction is rolled back
+ @connection.rollback_db_transaction if current_adapter?(:PostgreSQLAdapter)
+ assert_equal data[:expected], @connection.index_for_record_not_unique(exception, 'unique_items').columns
+ end
+ end
+
+ protected
+
+ def get_exception(expected_exception)
+ begin
+ yield
+ rescue expected_exception => e
+ e
+ else
+ flunk "Expected #{expected_exception} exception"
+ end
+ end
end
diff --git a/activerecord/test/cases/unique_constraints_test.rb b/activerecord/test/cases/unique_constraints_test.rb
new file mode 100644
index 0000000..2b5d0ee
--- /dev/null
+++ b/activerecord/test/cases/unique_constraints_test.rb
@@ -0,0 +1,40 @@
+require "cases/helper"
+require 'models/unique_item'
+
+class UniquenessValidationTest < ActiveRecord::TestCase
+
+ def test_validate_uniqueness_on_create
+ valid_item = UniqueItem.create!(:uniq => 'abc')
+ invalid_item = UniqueItem.create(:uniq => 'abc')
+ assert_equal "has already been taken", invalid_item.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_save
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.uniq = item_1.uniq
+ item_2.save
+ assert_equal "has already been taken", item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_update
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.update_attributes(:uniq => item_1.uniq)
+ assert_equal "has already been taken", item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_multiple
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal "has already been taken for uniq_2/uniq_3", invalid_item.errors[:uniq_1]
+ end
+
+ def test_validate_uniqueness_generic
+ ActiveRecord::Base.connection.expects(:index_for_record_not_unique).returns(nil)
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal "Unique requirement not met", invalid_item.errors[:base]
+ end
+
+end
\ No newline at end of file
diff --git a/activerecord/test/models/unique_item.rb b/activerecord/test/models/unique_item.rb
new file mode 100644
index 0000000..3c6f6f0
--- /dev/null
+++ b/activerecord/test/models/unique_item.rb
@@ -0,0 +1,2 @@
+class UniqueItem < ActiveRecord::Base
+end
\ No newline at end of file
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 2e5299c..acd1f02 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -536,6 +536,21 @@ ActiveRecord::Schema.define do
t.string :title
end
+ create_table :unique_items, :force => true do |t|
+ t.string :uniq
+ t.string :uniq_1
+ t.string :uniq_2
+ t.string :uniq_3
+ t.string :uniq_named
+ t.string :uniq_1_named
+ t.string :uniq_2_named
+ t.string :uniq_3_named
+ end
+ add_index :unique_items, [:uniq], :unique => true
+ add_index :unique_items, [:uniq_named], :unique => true, :name => 'unique_named_single_index'
+ add_index :unique_items, [:uniq_1, :uniq_2, :uniq_3], :unique => true
+ add_index :unique_items, [:uniq_1_named, :uniq_2_named, :uniq_3_named], :unique => true, :name => 'unique_named_multiple_index'
+
except 'SQLite' do
# fk_test_has_fk should be before fk_test_has_pk
create_table :fk_test_has_fk, :force => true do |t|
--
1.7.2.1
From 14feddfdda9a7fd3709c099a458bd4f8e81aea59 Mon Sep 17 00:00:00 2001
From: Jordan Brough <jordan@animoto.com>
Date: Fri, 15 Jan 2010 15:10:55 -0700
Subject: [PATCH 5/5] Add bang method handling to db-enforced unique constraints
---
.../lib/active_record/unique_constraints.rb | 30 ++++++++++++++-----
activerecord/test/cases/unique_constraints_test.rb | 30 ++++++++++++++++++++
2 files changed, 52 insertions(+), 8 deletions(-)
diff --git a/activerecord/lib/active_record/unique_constraints.rb b/activerecord/lib/active_record/unique_constraints.rb
index 6702e37..b3c1234 100644
--- a/activerecord/lib/active_record/unique_constraints.rb
+++ b/activerecord/lib/active_record/unique_constraints.rb
@@ -4,6 +4,7 @@ module ActiveRecord
def self.included(base)
base.class_eval do
alias_method_chain :save, :unique_constraints
+ alias_method_chain :save!, :unique_constraints
end
end
@@ -16,15 +17,28 @@ module ActiveRecord
cache_indexes
save_without_unique_constraints(perform_validation)
rescue ActiveRecord::RecordNotUnique => e
- index = connection.index_for_record_not_unique(e.original_exception, self.class.table_name)
- if !index
- errors.add_to_base(I18n.translate(:'activerecord.errors.messages.taken_generic'))
- elsif index.columns.size == 1
- errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
- else
- errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
- end
+ parse_unique_exception(e)
false
end
+
+ def save_with_unique_constraints! #:nodoc:
+ cache_indexes
+ save_without_unique_constraints!
+ rescue ActiveRecord::RecordNotUnique => e
+ parse_unique_exception(e)
+ raise RecordInvalid.new(self)
+ end
+
+ protected
+ def parse_unique_exception(e)
+ index = connection.index_for_record_not_unique(e.original_exception, self.class.table_name)
+ if !index
+ errors.add_to_base(I18n.translate(:'activerecord.errors.messages.taken_generic'))
+ elsif index.columns.size == 1
+ errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
+ else
+ errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
+ end
+ end
end
end
\ No newline at end of file
diff --git a/activerecord/test/cases/unique_constraints_test.rb b/activerecord/test/cases/unique_constraints_test.rb
index 2b5d0ee..8adae04 100644
--- a/activerecord/test/cases/unique_constraints_test.rb
+++ b/activerecord/test/cases/unique_constraints_test.rb
@@ -9,6 +9,12 @@ class UniquenessValidationTest < ActiveRecord::TestCase
assert_equal "has already been taken", invalid_item.errors[:uniq]
end
+ def test_validate_uniqueness_on_create!
+ valid_item = UniqueItem.create!(:uniq => 'abc')
+ e = get_unique_exception { UniqueItem.create!(:uniq => 'abc') }
+ assert_equal "has already been taken", e.record.errors[:uniq]
+ end
+
def test_validate_uniqueness_on_save
item_1 = UniqueItem.create!(:uniq => 'abc')
item_2 = UniqueItem.create!(:uniq => 'xyz')
@@ -17,6 +23,14 @@ class UniquenessValidationTest < ActiveRecord::TestCase
assert_equal "has already been taken", item_2.errors[:uniq]
end
+ def test_validate_uniqueness_on_save!
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.uniq = item_1.uniq
+ e = get_unique_exception { item_2.save! }
+ assert_equal("has already been taken", e.record.errors[:uniq])
+ end
+
def test_validate_uniqueness_on_update
item_1 = UniqueItem.create!(:uniq => 'abc')
item_2 = UniqueItem.create!(:uniq => 'xyz')
@@ -24,6 +38,13 @@ class UniquenessValidationTest < ActiveRecord::TestCase
assert_equal "has already been taken", item_2.errors[:uniq]
end
+ def test_validate_uniqueness_on_update!
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ e = get_unique_exception { item_2.update_attributes!(:uniq => item_1.uniq) }
+ assert_equal("has already been taken", e.record.errors[:uniq])
+ end
+
def test_validate_uniqueness_multiple
valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
@@ -37,4 +58,13 @@ class UniquenessValidationTest < ActiveRecord::TestCase
assert_equal "Unique requirement not met", invalid_item.errors[:base]
end
+ protected
+ def get_unique_exception
+ yield
+ rescue ActiveRecord::RecordInvalid => e
+ e
+ else
+ flunk "Expected ActiveRecord::RecordInvalid exception"
+ end
+
end
\ No newline at end of file
--
1.7.2.1
From 5055887efe3f3d560003d14599e8c048059fa830 Mon Sep 17 00:00:00 2001
From: Jordan Brough <jordan@animoto.com>
Date: Tue, 22 Dec 2009 07:42:48 -0700
Subject: [PATCH] Friendly ActiveRecord error handling for db-enforced unique constraints
---
activerecord/lib/active_record.rb | 1 +
activerecord/lib/active_record/base.rb | 4 ++
.../connection_adapters/abstract_adapter.rb | 7 +++
.../connection_adapters/mysql_adapter.rb | 12 ++++++
.../connection_adapters/postgresql_adapter.rb | 7 +++
.../connection_adapters/sqlite_adapter.rb | 7 +++
activerecord/lib/active_record/locale/en.yml | 2 +
.../lib/active_record/unique_constraints.rb | 24 ++++++++++++
activerecord/test/cases/adapter_test.rb | 35 +++++++++++++++++
activerecord/test/cases/unique_constraints_test.rb | 40 ++++++++++++++++++++
activerecord/test/models/unique_item.rb | 2 +
activerecord/test/schema/schema.rb | 15 +++++++
12 files changed, 156 insertions(+), 0 deletions(-)
create mode 100644 activerecord/lib/active_record/unique_constraints.rb
create mode 100644 activerecord/test/cases/unique_constraints_test.rb
create mode 100644 activerecord/test/models/unique_item.rb
diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb
index 2376bbd..f3cff35 100644
--- a/activerecord/lib/active_record.rb
+++ b/activerecord/lib/active_record.rb
@@ -68,6 +68,7 @@ module ActiveRecord
autoload :Timestamp
autoload :Transactions
autoload :Types
+ autoload :UniqueConstraints
autoload :Validations
module AttributeMethods
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 321bba4..da52ec1 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -3117,6 +3117,10 @@ module ActiveRecord #:nodoc:
include Aggregations, Transactions, Reflection, Batches, Calculations, Serialization
+ # UniqueConstraints needs to be included after Transactions, because we need #save_with_unique_constraints
+ # to rescue after the transaction has been resolved for postgres.
+ include UniqueConstraints
+
end
end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 8fae26b..832b962 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -198,6 +198,13 @@ module ActiveRecord
end
end
+ # given an ActiveRecord::RecordNotUnique exception and a table name this returns the index whose
+ # unique constraint was violated, or nil if it can't determine the index
+ def index_for_record_not_unique(not_unique_exception, table_name) #:nodoc:
+ # override in derived class
+ nil
+ end
+
protected
def log(sql, name, &block)
ActiveSupport::Notifications.instrument(:sql, :sql => sql, :name => name, &block)
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index fa28bc6..d83e3d0 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -576,6 +576,18 @@ module ActiveRecord
where_sql
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ case exception.message
+ when /Duplicate entry.*for key (\d+)/
+ index_position = $1.to_i
+ # minus two b/c mysql message is one-based + rails excludes primary key index from indexes list
+ indexes(table_name)[index_position - 2]
+ when /Duplicate entry.*for key '(\w+)'/
+ index_name = $1
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
def translate_exception(exception, message)
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 1d52c5e..70bbbd8 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -929,6 +929,13 @@ module ActiveRecord
sql << order_columns * ', '
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/unique constraint "(\w+)"/)
+ index_name = match[1]
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
# Returns the version of the connected PostgreSQL version.
def postgresql_version
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index c9c2892..72dc049 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -294,6 +294,13 @@ module ActiveRecord
"VALUES(NULL)"
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/column(?:s)? (.*) (?:is|are) not unique/)
+ index_columns = match[1].split(', ')
+ indexes(table_name).detect { |i| i.columns == index_columns }
+ end
+ end
+
protected
def select(sql, name = nil) #:nodoc:
execute(sql, name).map do |row|
diff --git a/activerecord/lib/active_record/locale/en.yml b/activerecord/lib/active_record/locale/en.yml
index 092f5f0..8144735 100644
--- a/activerecord/lib/active_record/locale/en.yml
+++ b/activerecord/lib/active_record/locale/en.yml
@@ -15,6 +15,8 @@ en:
too_short: "is too short (minimum is {{count}} characters)"
wrong_length: "is the wrong length (should be {{count}} characters)"
taken: "has already been taken"
+ taken_multiple: "has already been taken for {{context}}"
+ taken_generic: "Unique requirement not met"
not_a_number: "is not a number"
greater_than: "must be greater than {{count}}"
greater_than_or_equal_to: "must be greater than or equal to {{count}}"
diff --git a/activerecord/lib/active_record/unique_constraints.rb b/activerecord/lib/active_record/unique_constraints.rb
new file mode 100644
index 0000000..7a17b6a
--- /dev/null
+++ b/activerecord/lib/active_record/unique_constraints.rb
@@ -0,0 +1,24 @@
+module ActiveRecord
+ # See ActiveRecord::Transactions::ClassMethods for documentation.
+ module UniqueConstraints
+ def self.included(base)
+ base.class_eval do
+ alias_method_chain :save, :unique_constraints
+ end
+ end
+
+ def save_with_unique_constraints(perform_validation = true) #:nodoc:
+ save_without_unique_constraints(perform_validation)
+ rescue ActiveRecord::RecordNotUnique => e
+ index = connection.index_for_record_not_unique(e, self.class.table_name)
+ if !index
+ errors[:base] << I18n.translate(:'activerecord.errors.messages.taken_generic')
+ elsif index.columns.size == 1
+ errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
+ else
+ errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
+ end
+ false
+ end
+ end
+end
\ No newline at end of file
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index c59be26..4c8f58b 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -142,4 +142,39 @@ class AdapterTest < ActiveRecord::TestCase
end
end
end
+
+ # tests for db-enforced unique constraints
+ unique_scenarios = {
+ :single => {:sql => "INSERT INTO unique_items(uniq) VALUES('abc')",
+ :expected => ['uniq']},
+ :single_named => {:sql => "INSERT INTO unique_items(uniq_named) VALUES('abc')",
+ :expected => ['uniq_named']},
+ :multiple => {:sql => "INSERT INTO unique_items(uniq_1, uniq_2, uniq_3) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1', 'uniq_2', 'uniq_3']},
+ :multiple_named => {:sql => "INSERT INTO unique_items(uniq_1_named, uniq_2_named, uniq_3_named) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1_named', 'uniq_2_named', 'uniq_3_named']}
+ }
+ unique_scenarios.each do |name, data|
+ test "finds index for unique constraint #{name}" do
+ @connection.execute data[:sql]
+ exception = get_exception(ActiveRecord::RecordNotUnique) do
+ @connection.execute data[:sql]
+ end
+ # postgres can't query indexes until transaction is rolled back
+ @connection.rollback_db_transaction if current_adapter?(:PostgreSQLAdapter)
+ assert_equal data[:expected], @connection.index_for_record_not_unique(exception, 'unique_items').columns
+ end
+ end
+
+ protected
+
+ def get_exception(expected_exception)
+ begin
+ yield
+ rescue expected_exception => e
+ e
+ else
+ flunk "Expected #{expected_exception} exception"
+ end
+ end
end
diff --git a/activerecord/test/cases/unique_constraints_test.rb b/activerecord/test/cases/unique_constraints_test.rb
new file mode 100644
index 0000000..92ba6d8
--- /dev/null
+++ b/activerecord/test/cases/unique_constraints_test.rb
@@ -0,0 +1,40 @@
+require "cases/helper"
+require 'models/unique_item'
+
+class UniquenessValidationTest < ActiveRecord::TestCase
+
+ def test_validate_uniqueness_on_create
+ valid_item = UniqueItem.create!(:uniq => 'abc')
+ invalid_item = UniqueItem.create(:uniq => 'abc')
+ assert_equal ["has already been taken"], invalid_item.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_save
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.uniq = item_1.uniq
+ item_2.save
+ assert_equal ["has already been taken"], item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_update
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.update_attributes(:uniq => item_1.uniq)
+ assert_equal ["has already been taken"], item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_multiple
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal ["has already been taken for uniq_2/uniq_3"], invalid_item.errors[:uniq_1]
+ end
+
+ def test_validate_uniqueness_generic
+ ActiveRecord::Base.connection.expects(:index_for_record_not_unique).returns(nil)
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal ["Unique requirement not met"], invalid_item.errors[:base]
+ end
+
+end
\ No newline at end of file
diff --git a/activerecord/test/models/unique_item.rb b/activerecord/test/models/unique_item.rb
new file mode 100644
index 0000000..3c6f6f0
--- /dev/null
+++ b/activerecord/test/models/unique_item.rb
@@ -0,0 +1,2 @@
+class UniqueItem < ActiveRecord::Base
+end
\ No newline at end of file
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 0dd9da4..1aea762 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -532,6 +532,21 @@ ActiveRecord::Schema.define do
t.string :title
end
+ create_table :unique_items, :force => true do |t|
+ t.string :uniq
+ t.string :uniq_1
+ t.string :uniq_2
+ t.string :uniq_3
+ t.string :uniq_named
+ t.string :uniq_1_named
+ t.string :uniq_2_named
+ t.string :uniq_3_named
+ end
+ add_index :unique_items, [:uniq], :unique => true
+ add_index :unique_items, [:uniq_named], :unique => true, :name => 'unique_named_single_index'
+ add_index :unique_items, [:uniq_1, :uniq_2, :uniq_3], :unique => true
+ add_index :unique_items, [:uniq_1_named, :uniq_2_named, :uniq_3_named], :unique => true, :name => 'unique_named_multiple_index'
+
except 'SQLite' do
# fk_test_has_fk should be before fk_test_has_pk
create_table :fk_test_has_fk, :force => true do |t|
--
1.6.1.2
From 9e300598ee86d318d5312478d398f2ebdd1db2c0 Mon Sep 17 00:00:00 2001
From: Jordan Brough <jordan@animoto.com>
Date: Tue, 9 Feb 2010 13:13:01 -0700
Subject: [PATCH 1/2] Friendly ActiveRecord error handling for db-enforced unique constraints
---
activerecord/lib/active_record.rb | 1 +
activerecord/lib/active_record/base.rb | 1 +
.../connection_adapters/abstract_adapter.rb | 7 +++
.../connection_adapters/mysql2_adapter.rb | 12 ++++++
.../connection_adapters/mysql_adapter.rb | 12 ++++++
.../connection_adapters/postgresql_adapter.rb | 13 ++++++
.../connection_adapters/sqlite_adapter.rb | 7 +++
activerecord/lib/active_record/locale/en.yml | 2 +
.../lib/active_record/unique_constraints.rb | 29 ++++++++++++++
activerecord/test/cases/adapter_test.rb | 35 +++++++++++++++++
activerecord/test/cases/unique_constraints_test.rb | 40 ++++++++++++++++++++
activerecord/test/models/unique_item.rb | 2 +
activerecord/test/schema/schema.rb | 15 +++++++
13 files changed, 176 insertions(+), 0 deletions(-)
create mode 100644 activerecord/lib/active_record/unique_constraints.rb
create mode 100644 activerecord/test/cases/unique_constraints_test.rb
create mode 100644 activerecord/test/models/unique_item.rb
diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb
index 5afb978..869c90b 100644
--- a/activerecord/lib/active_record.rb
+++ b/activerecord/lib/active_record.rb
@@ -78,6 +78,7 @@ module ActiveRecord
autoload :SessionStore
autoload :Timestamp
autoload :Transactions
+ autoload :UniqueConstraints
autoload :Validations
end
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index effb17b..9909663 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -1940,6 +1940,7 @@ MSG
# #save_with_autosave_associations to be wrapped inside a transaction.
include AutosaveAssociation, NestedAttributes
include Aggregations, Transactions, Reflection, Serialization
+ include UniqueConstraints
NilClass.add_whiner(self) if NilClass.respond_to?(:add_whiner)
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 3a3a73f..43ffa46 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -207,6 +207,13 @@ module ActiveRecord
"active_record_#{open_transactions}"
end
+ # given an ActiveRecord::RecordNotUnique exception and a table name this returns the index whose
+ # unique constraint was violated, or nil if it can't determine the index
+ def index_for_record_not_unique(not_unique_exception, table_name) #:nodoc:
+ # override in derived class
+ nil
+ end
+
protected
def log(sql, name = "SQL")
diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
index acf1832..47da0da 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
@@ -536,6 +536,18 @@ module ActiveRecord
where_sql
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ case exception.message
+ when /Duplicate entry.*for key (\d+)/
+ index_position = $1.to_i
+ # minus two b/c mysql message is one-based + rails excludes primary key index from indexes list
+ indexes(table_name)[index_position - 2]
+ when /Duplicate entry.*for key '(\w+)'/
+ index_name = $1
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
def quoted_columns_for_index(column_names, options = {})
length = options[:length] if options.is_a?(Hash)
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index cdf1ebf..7c89aef 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -646,6 +646,18 @@ module ActiveRecord
where_sql
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ case exception.message
+ when /Duplicate entry.*for key (\d+)/
+ index_position = $1.to_i
+ # minus two b/c mysql message is one-based + rails excludes primary key index from indexes list
+ indexes(table_name)[index_position - 2]
+ when /Duplicate entry.*for key '(\w+)'/
+ index_name = $1
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
def quoted_columns_for_index(column_names, options = {})
length = options[:length] if options.is_a?(Hash)
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 46c0f3f..bea174b 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -708,6 +708,12 @@ module ActiveRecord
end.compact
end
+ # postgres doesn't let us query this in the middle of an aborted transaction so we sometimes need to cache beforehand
+ def cached_indexes(table_name) #:nodoc:
+ @cached_indexes ||= {}
+ @cached_indexes[table_name] ||= indexes(table_name)
+ end
+
# Returns the list of all column definitions for a table.
def columns(table_name, name = nil)
# Limit, precision, and scale are all handled by the superclass.
@@ -944,6 +950,13 @@ module ActiveRecord
sql << order_columns * ', '
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/unique constraint "(\w+)"/)
+ index_name = match[1]
+ cached_indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
# Returns the version of the connected PostgreSQL version.
def postgresql_version
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index f650a1b..02eebd8 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -340,6 +340,13 @@ module ActiveRecord
"VALUES(NULL)"
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/column(?:s)? (.*) (?:is|are) not unique/)
+ index_columns = match[1].split(', ')
+ indexes(table_name).detect { |i| i.columns == index_columns }
+ end
+ end
+
protected
def select(sql, name = nil, binds = []) #:nodoc:
exec_query(sql, name, binds).to_a
diff --git a/activerecord/lib/active_record/locale/en.yml b/activerecord/lib/active_record/locale/en.yml
index 44328f6..e310fba 100644
--- a/activerecord/lib/active_record/locale/en.yml
+++ b/activerecord/lib/active_record/locale/en.yml
@@ -9,6 +9,8 @@ en:
errors:
messages:
taken: "has already been taken"
+ taken_multiple: "has already been taken for %{context}"
+ taken_generic: "Unique requirement not met"
record_invalid: "Validation failed: %{errors}"
# Append your own errors here or at the model/attributes scope.
diff --git a/activerecord/lib/active_record/unique_constraints.rb b/activerecord/lib/active_record/unique_constraints.rb
new file mode 100644
index 0000000..4a6d748
--- /dev/null
+++ b/activerecord/lib/active_record/unique_constraints.rb
@@ -0,0 +1,29 @@
+module ActiveRecord
+ module UniqueConstraints
+ extend ActiveSupport::Concern
+
+ included do
+ alias_method_chain :save, :unique_constraints
+ end
+
+ # work around postgres not allowing us to query indexes in the middle of an aborted transaction
+ def cache_indexes #:nodoc:
+ connection.cached_indexes(self.class.table_name) if connection.respond_to?(:cached_indexes)
+ end
+
+ def save_with_unique_constraints(*args) #:nodoc:
+ cache_indexes
+ save_without_unique_constraints(*args)
+ rescue ActiveRecord::RecordNotUnique => e
+ index = connection.index_for_record_not_unique(e, self.class.table_name)
+ if !index
+ errors[:base] << I18n.t(:'activerecord.errors.messages.taken_generic')
+ elsif index.columns.size == 1
+ errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
+ else
+ errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
+ end
+ false
+ end
+ end
+end
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 49b2e94..1cb8b0a 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -141,4 +141,39 @@ class AdapterTest < ActiveRecord::TestCase
end
end
end
+
+ # tests for db-enforced unique constraints
+ unique_scenarios = {
+ :single => {:sql => "INSERT INTO unique_items(uniq) VALUES('abc')",
+ :expected => ['uniq']},
+ :single_named => {:sql => "INSERT INTO unique_items(uniq_named) VALUES('abc')",
+ :expected => ['uniq_named']},
+ :multiple => {:sql => "INSERT INTO unique_items(uniq_1, uniq_2, uniq_3) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1', 'uniq_2', 'uniq_3']},
+ :multiple_named => {:sql => "INSERT INTO unique_items(uniq_1_named, uniq_2_named, uniq_3_named) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1_named', 'uniq_2_named', 'uniq_3_named']}
+ }
+ unique_scenarios.each do |name, data|
+ test "finds index for unique constraint #{name}" do
+ @connection.execute data[:sql]
+ exception = get_exception(ActiveRecord::RecordNotUnique) do
+ @connection.execute data[:sql]
+ end
+ # postgres can't query indexes until transaction is rolled back
+ @connection.rollback_db_transaction if current_adapter?(:PostgreSQLAdapter)
+ assert_equal data[:expected], @connection.index_for_record_not_unique(exception, 'unique_items').columns
+ end
+ end
+
+ protected
+
+ def get_exception(expected_exception)
+ begin
+ yield
+ rescue expected_exception => e
+ e
+ else
+ flunk "Expected #{expected_exception} exception"
+ end
+ end
end
diff --git a/activerecord/test/cases/unique_constraints_test.rb b/activerecord/test/cases/unique_constraints_test.rb
new file mode 100644
index 0000000..18d1890
--- /dev/null
+++ b/activerecord/test/cases/unique_constraints_test.rb
@@ -0,0 +1,40 @@
+require "cases/helper"
+require 'models/unique_item'
+
+class UniquenessValidationTest < ActiveRecord::TestCase
+
+ def test_validate_uniqueness_on_create
+ valid_item = UniqueItem.create!(:uniq => 'abc')
+ invalid_item = UniqueItem.create(:uniq => 'abc')
+ assert_equal ["has already been taken"], invalid_item.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_save
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.uniq = item_1.uniq
+ item_2.save
+ assert_equal ["has already been taken"], item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_update
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.update_attributes(:uniq => item_1.uniq)
+ assert_equal ["has already been taken"], item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_multiple
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal ["has already been taken for uniq_2/uniq_3"], invalid_item.errors[:uniq_1]
+ end
+
+ def test_validate_uniqueness_generic
+ ActiveRecord::Base.connection.expects(:index_for_record_not_unique).returns(nil)
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal ["Unique requirement not met"], invalid_item.errors[:base]
+ end
+
+end
diff --git a/activerecord/test/models/unique_item.rb b/activerecord/test/models/unique_item.rb
new file mode 100644
index 0000000..e805cb8
--- /dev/null
+++ b/activerecord/test/models/unique_item.rb
@@ -0,0 +1,2 @@
+class UniqueItem < ActiveRecord::Base
+end
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 326c336..d408154 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -668,6 +668,21 @@ ActiveRecord::Schema.define do
t.string :name
end
+ create_table :unique_items, :force => true do |t|
+ t.string :uniq
+ t.string :uniq_1
+ t.string :uniq_2
+ t.string :uniq_3
+ t.string :uniq_named
+ t.string :uniq_1_named
+ t.string :uniq_2_named
+ t.string :uniq_3_named
+ end
+ add_index :unique_items, [:uniq], :unique => true
+ add_index :unique_items, [:uniq_named], :unique => true, :name => 'unique_named_single_index'
+ add_index :unique_items, [:uniq_1, :uniq_2, :uniq_3], :unique => true
+ add_index :unique_items, [:uniq_1_named, :uniq_2_named, :uniq_3_named], :unique => true, :name => 'unique_named_multiple_index'
+
except 'SQLite' do
# fk_test_has_fk should be before fk_test_has_pk
create_table :fk_test_has_fk, :force => true do |t|
--
1.7.2.1
From fb06e862fc2cb5d48717d6af948466fb40cb002d Mon Sep 17 00:00:00 2001
From: Jordan Brough <jordan@animoto.com>
Date: Tue, 9 Feb 2010 13:50:35 -0700
Subject: [PATCH 2/2] Handling for db-enforced unique constraints in bang methods
---
.../lib/active_record/unique_constraints.rb | 33 ++++++++++++++-----
activerecord/test/cases/unique_constraints_test.rb | 30 ++++++++++++++++++
2 files changed, 54 insertions(+), 9 deletions(-)
diff --git a/activerecord/lib/active_record/unique_constraints.rb b/activerecord/lib/active_record/unique_constraints.rb
index 4a6d748..cb7a665 100644
--- a/activerecord/lib/active_record/unique_constraints.rb
+++ b/activerecord/lib/active_record/unique_constraints.rb
@@ -3,7 +3,9 @@ module ActiveRecord
extend ActiveSupport::Concern
included do
- alias_method_chain :save, :unique_constraints
+ [:save, :save!].each do |method|
+ alias_method_chain method, :unique_constraints
+ end
end
# work around postgres not allowing us to query indexes in the middle of an aborted transaction
@@ -15,15 +17,28 @@ module ActiveRecord
cache_indexes
save_without_unique_constraints(*args)
rescue ActiveRecord::RecordNotUnique => e
- index = connection.index_for_record_not_unique(e, self.class.table_name)
- if !index
- errors[:base] << I18n.t(:'activerecord.errors.messages.taken_generic')
- elsif index.columns.size == 1
- errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
- else
- errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
- end
+ parse_unique_exception(e)
false
end
+
+ def save_with_unique_constraints! #:nodoc:
+ cache_indexes
+ save_without_unique_constraints!
+ rescue ActiveRecord::RecordNotUnique => e
+ parse_unique_exception(e)
+ raise RecordInvalid.new(self)
+ end
+
+ protected
+ def parse_unique_exception(e)
+ index = connection.index_for_record_not_unique(e, self.class.table_name)
+ if !index
+ errors[:base] << I18n.translate(:'activerecord.errors.messages.taken_generic')
+ elsif index.columns.size == 1
+ errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
+ else
+ errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
+ end
+ end
end
end
diff --git a/activerecord/test/cases/unique_constraints_test.rb b/activerecord/test/cases/unique_constraints_test.rb
index 18d1890..d4489fc 100644
--- a/activerecord/test/cases/unique_constraints_test.rb
+++ b/activerecord/test/cases/unique_constraints_test.rb
@@ -9,6 +9,12 @@ class UniquenessValidationTest < ActiveRecord::TestCase
assert_equal ["has already been taken"], invalid_item.errors[:uniq]
end
+ def test_validate_uniqueness_on_create!
+ valid_item = UniqueItem.create!(:uniq => 'abc')
+ e = get_unique_exception { UniqueItem.create!(:uniq => 'abc') }
+ assert_equal ["has already been taken"], e.record.errors[:uniq]
+ end
+
def test_validate_uniqueness_on_save
item_1 = UniqueItem.create!(:uniq => 'abc')
item_2 = UniqueItem.create!(:uniq => 'xyz')
@@ -17,6 +23,14 @@ class UniquenessValidationTest < ActiveRecord::TestCase
assert_equal ["has already been taken"], item_2.errors[:uniq]
end
+ def test_validate_uniqueness_on_save!
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.uniq = item_1.uniq
+ e = get_unique_exception { item_2.save! }
+ assert_equal(["has already been taken"], e.record.errors[:uniq])
+ end
+
def test_validate_uniqueness_on_update
item_1 = UniqueItem.create!(:uniq => 'abc')
item_2 = UniqueItem.create!(:uniq => 'xyz')
@@ -24,6 +38,13 @@ class UniquenessValidationTest < ActiveRecord::TestCase
assert_equal ["has already been taken"], item_2.errors[:uniq]
end
+ def test_validate_uniqueness_on_update!
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ e = get_unique_exception { item_2.update_attributes!(:uniq => item_1.uniq) }
+ assert_equal(["has already been taken"], e.record.errors[:uniq])
+ end
+
def test_validate_uniqueness_multiple
valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
@@ -37,4 +58,13 @@ class UniquenessValidationTest < ActiveRecord::TestCase
assert_equal ["Unique requirement not met"], invalid_item.errors[:base]
end
+ protected
+ def get_unique_exception
+ yield
+ rescue ActiveRecord::RecordInvalid => e
+ e
+ else
+ flunk "Expected ActiveRecord::RecordInvalid exception"
+ end
+
end
--
1.7.2.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.