Created
May 14, 2011 02:18
-
-
Save AquaGeek/971648 to your computer and use it in GitHub Desktop.
Rails Lighthouse ticket #3486
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| From f259b38eee1b2a14507868e835f2ac187263a616 Mon Sep 17 00:00:00 2001 | |
| From: Jordan Brough <[email protected]> | |
| 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 | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| From 466ce19b2989324be1fca05b5b553700997b6803 Mon Sep 17 00:00:00 2001 | |
| From: Jordan Brough <[email protected]> | |
| 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 | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| From 5573e913b8651ea53ae6d94686b4b63f2cca2e83 Mon Sep 17 00:00:00 2001 | |
| From: Michael Schuerig <[email protected]> | |
| 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 <[email protected]> | |
| --- | |
| 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 <[email protected]> | |
| 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 <[email protected]> | |
| --- | |
| 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 <[email protected]> | |
| 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 <[email protected]> | |
| 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 | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| From 48fcff95ae1b5e55837e81a4c132a3c208a235f8 Mon Sep 17 00:00:00 2001 | |
| From: Michael Schuerig <[email protected]> | |
| 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 <[email protected]> | |
| --- | |
| 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 <[email protected]> | |
| 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 <[email protected]> | |
| --- | |
| 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 <[email protected]> | |
| 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 <[email protected]> | |
| 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 <[email protected]> | |
| 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 | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| From 5055887efe3f3d560003d14599e8c048059fa830 Mon Sep 17 00:00:00 2001 | |
| From: Jordan Brough <[email protected]> | |
| 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 | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| From 9e300598ee86d318d5312478d398f2ebdd1db2c0 Mon Sep 17 00:00:00 2001 | |
| From: Jordan Brough <[email protected]> | |
| 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 <[email protected]> | |
| 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