public

Rails Lighthouse ticket #3486

  • Download Gist
2-3-stable-unique-constraints.patch
Diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299
From f259b38eee1b2a14507868e835f2ac187263a616 Mon Sep 17 00:00:00 2001
From: Jordan Brough <jordan@animoto.com>
Date: Wed, 11 Nov 2009 13:31:34 -0700
Subject: [PATCH] Friendly ActiveRecord error handling for db-enforced unique constraints
 
---
activerecord/lib/active_record.rb | 1 +
activerecord/lib/active_record/base.rb | 4 ++
.../connection_adapters/abstract_adapter.rb | 7 +++
.../connection_adapters/mysql_adapter.rb | 12 ++++++
.../connection_adapters/postgresql_adapter.rb | 7 +++
.../connection_adapters/sqlite_adapter.rb | 7 +++
activerecord/lib/active_record/locale/en.yml | 2 +
.../lib/active_record/unique_constraints.rb | 24 ++++++++++++
activerecord/test/cases/adapter_test.rb | 35 +++++++++++++++++
activerecord/test/cases/unique_constraints_test.rb | 40 ++++++++++++++++++++
activerecord/test/models/unique_item.rb | 2 +
activerecord/test/schema/schema.rb | 15 +++++++
12 files changed, 156 insertions(+), 0 deletions(-)
create mode 100644 activerecord/lib/active_record/unique_constraints.rb
create mode 100644 activerecord/test/cases/unique_constraints_test.rb
create mode 100644 activerecord/test/models/unique_item.rb
 
diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb
index 2f8c5c7..8469255 100644
--- a/activerecord/lib/active_record.rb
+++ b/activerecord/lib/active_record.rb
@@ -68,6 +68,7 @@ module ActiveRecord
autoload :TestCase, 'active_record/test_case'
autoload :Timestamp, 'active_record/timestamp'
autoload :Transactions, 'active_record/transactions'
+ autoload :UniqueConstraints, 'active_record/unique_constraints'
autoload :Validations, 'active_record/validations'
module Locking
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index d6b264c..1fe6bf8 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -3177,6 +3177,10 @@ module ActiveRecord #:nodoc:
include AutosaveAssociation, NestedAttributes
include Aggregations, Transactions, Reflection, Batches, Calculations, Serialization
+
+ # UniqueConstraints needs to be included after Transactions, because we need #save_with_unique_constraints
+ # to rescue after the transaction has been resolved for postgres.
+ include UniqueConstraints
end
end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 6b7b363..6adea19 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -197,6 +197,13 @@ module ActiveRecord
end
end
+ # given an ActiveRecord::RecordNotUnique exception and a table name this returns the index whose
+ # unique constraint was violated, or nil if it can't determine the index
+ def index_for_record_not_unique(not_unique_exception, table_name) #:nodoc:
+ # override in derived class
+ nil
+ end
+
protected
def log(sql, name)
if block_given?
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 5414a1d..635a8fc 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -571,6 +571,18 @@ module ActiveRecord
where_sql
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ case exception.message
+ when /Duplicate entry.*for key (\d+)/
+ index_position = $1.to_i
+ # minus two b/c mysql message is one-based + rails excludes primary key index from indexes list
+ indexes(table_name)[index_position - 2]
+ when /Duplicate entry.*for key '(\w+)'/
+ index_name = $1
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
def translate_exception(exception, message)
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index b1cac88..c313082 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -941,6 +941,13 @@ module ActiveRecord
sql.replace "SELECT * FROM (#{sql}) AS id_list ORDER BY #{order}"
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/unique constraint "(\w+)"/)
+ index_name = match[1]
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
# Returns the version of the connected PostgreSQL version.
def postgresql_version
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index 79eea1d..31519f1 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -315,6 +315,13 @@ module ActiveRecord
"INSERT INTO #{table_name} VALUES(NULL)"
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/column(?:s)? (.*) (?:is|are) not unique/)
+ index_columns = match[1].split(', ')
+ indexes(table_name).detect { |i| i.columns == index_columns }
+ end
+ end
+
protected
def select(sql, name = nil) #:nodoc:
execute(sql, name).map do |row|
diff --git a/activerecord/lib/active_record/locale/en.yml b/activerecord/lib/active_record/locale/en.yml
index 2813524..a7dec95 100644
--- a/activerecord/lib/active_record/locale/en.yml
+++ b/activerecord/lib/active_record/locale/en.yml
@@ -15,6 +15,8 @@ en:
too_short: "is too short (minimum is {{count}} characters)"
wrong_length: "is the wrong length (should be {{count}} characters)"
taken: "has already been taken"
+ taken_multiple: "has already been taken for {{context}}"
+ taken_generic: "Unique requirement not met"
not_a_number: "is not a number"
greater_than: "must be greater than {{count}}"
greater_than_or_equal_to: "must be greater than or equal to {{count}}"
diff --git a/activerecord/lib/active_record/unique_constraints.rb b/activerecord/lib/active_record/unique_constraints.rb
new file mode 100644
index 0000000..99e6353
--- /dev/null
+++ b/activerecord/lib/active_record/unique_constraints.rb
@@ -0,0 +1,24 @@
+module ActiveRecord
+ # See ActiveRecord::Transactions::ClassMethods for documentation.
+ module UniqueConstraints
+ def self.included(base)
+ base.class_eval do
+ alias_method_chain :save, :unique_constraints
+ end
+ end
+
+ def save_with_unique_constraints(perform_validation = true) #:nodoc:
+ save_without_unique_constraints(perform_validation)
+ rescue ActiveRecord::RecordNotUnique => e
+ index = connection.index_for_record_not_unique(e.original_exception, self.class.table_name)
+ if !index
+ errors.add_to_base(I18n.translate(:'activerecord.errors.messages.taken_generic'))
+ elsif index.columns.size == 1
+ errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
+ else
+ errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
+ end
+ false
+ end
+ end
+end
\ No newline at end of file
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 4797112..0883ea0 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -157,4 +157,39 @@ class AdapterTest < ActiveRecord::TestCase
end
end
end
+
+ # tests for db-enforced unique constraints
+ unique_scenarios = {
+ :single => {:sql => "INSERT INTO unique_items(uniq) VALUES('abc')",
+ :expected => ['uniq']},
+ :single_named => {:sql => "INSERT INTO unique_items(uniq_named) VALUES('abc')",
+ :expected => ['uniq_named']},
+ :multiple => {:sql => "INSERT INTO unique_items(uniq_1, uniq_2, uniq_3) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1', 'uniq_2', 'uniq_3']},
+ :multiple_named => {:sql => "INSERT INTO unique_items(uniq_1_named, uniq_2_named, uniq_3_named) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1_named', 'uniq_2_named', 'uniq_3_named']}
+ }
+ unique_scenarios.each do |name, data|
+ test "finds index for unique constraint #{name}" do
+ @connection.execute data[:sql]
+ exception = get_exception(ActiveRecord::RecordNotUnique) do
+ @connection.execute data[:sql]
+ end
+ # postgres can't query indexes until transaction is rolled back
+ @connection.rollback_db_transaction if current_adapter?(:PostgreSQLAdapter)
+ assert_equal data[:expected], @connection.index_for_record_not_unique(exception, 'unique_items').columns
+ end
+ end
+
+ protected
+
+ def get_exception(expected_exception)
+ begin
+ yield
+ rescue expected_exception => e
+ e
+ else
+ flunk "Expected #{expected_exception} exception"
+ end
+ end
end
diff --git a/activerecord/test/cases/unique_constraints_test.rb b/activerecord/test/cases/unique_constraints_test.rb
new file mode 100644
index 0000000..2b5d0ee
--- /dev/null
+++ b/activerecord/test/cases/unique_constraints_test.rb
@@ -0,0 +1,40 @@
+require "cases/helper"
+require 'models/unique_item'
+
+class UniquenessValidationTest < ActiveRecord::TestCase
+
+ def test_validate_uniqueness_on_create
+ valid_item = UniqueItem.create!(:uniq => 'abc')
+ invalid_item = UniqueItem.create(:uniq => 'abc')
+ assert_equal "has already been taken", invalid_item.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_save
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.uniq = item_1.uniq
+ item_2.save
+ assert_equal "has already been taken", item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_update
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.update_attributes(:uniq => item_1.uniq)
+ assert_equal "has already been taken", item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_multiple
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal "has already been taken for uniq_2/uniq_3", invalid_item.errors[:uniq_1]
+ end
+
+ def test_validate_uniqueness_generic
+ ActiveRecord::Base.connection.expects(:index_for_record_not_unique).returns(nil)
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal "Unique requirement not met", invalid_item.errors[:base]
+ end
+
+end
\ No newline at end of file
diff --git a/activerecord/test/models/unique_item.rb b/activerecord/test/models/unique_item.rb
new file mode 100644
index 0000000..3c6f6f0
--- /dev/null
+++ b/activerecord/test/models/unique_item.rb
@@ -0,0 +1,2 @@
+class UniqueItem < ActiveRecord::Base
+end
\ No newline at end of file
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 984c5ba..8d64e51 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -479,6 +479,21 @@ ActiveRecord::Schema.define do
end
end
+ create_table :unique_items, :force => true do |t|
+ t.string :uniq
+ t.string :uniq_1
+ t.string :uniq_2
+ t.string :uniq_3
+ t.string :uniq_named
+ t.string :uniq_1_named
+ t.string :uniq_2_named
+ t.string :uniq_3_named
+ end
+ add_index :unique_items, [:uniq], :unique => true
+ add_index :unique_items, [:uniq_named], :unique => true, :name => 'unique_named_single_index'
+ add_index :unique_items, [:uniq_1, :uniq_2, :uniq_3], :unique => true
+ add_index :unique_items, [:uniq_1_named, :uniq_2_named, :uniq_3_named], :unique => true, :name => 'unique_named_multiple_index'
+
except 'SQLite' do
# fk_test_has_fk should be before fk_test_has_pk
create_table :fk_test_has_fk, :force => true do |t|
--
1.6.1.2
master-unique-constraints.patch
Diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299
From 466ce19b2989324be1fca05b5b553700997b6803 Mon Sep 17 00:00:00 2001
From: Jordan Brough <jordan@animoto.com>
Date: Wed, 11 Nov 2009 11:34:27 -0700
Subject: [PATCH] Friendly ActiveRecord error handling for db-enforced unique constraints
 
---
activerecord/lib/active_record.rb | 1 +
activerecord/lib/active_record/base.rb | 4 ++
.../connection_adapters/abstract_adapter.rb | 7 +++
.../connection_adapters/mysql_adapter.rb | 12 ++++++
.../connection_adapters/postgresql_adapter.rb | 7 +++
.../connection_adapters/sqlite_adapter.rb | 7 +++
activerecord/lib/active_record/locale/en.yml | 2 +
.../lib/active_record/unique_constraints.rb | 24 ++++++++++++
activerecord/test/cases/adapter_test.rb | 35 +++++++++++++++++
activerecord/test/cases/unique_constraints_test.rb | 40 ++++++++++++++++++++
activerecord/test/models/unique_item.rb | 2 +
activerecord/test/schema/schema.rb | 15 +++++++
12 files changed, 156 insertions(+), 0 deletions(-)
create mode 100644 activerecord/lib/active_record/unique_constraints.rb
create mode 100644 activerecord/test/cases/unique_constraints_test.rb
create mode 100644 activerecord/test/models/unique_item.rb
 
diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb
index 8195e78..0aac5be 100644
--- a/activerecord/lib/active_record.rb
+++ b/activerecord/lib/active_record.rb
@@ -70,6 +70,7 @@ module ActiveRecord
autoload :TestCase, 'active_record/test_case'
autoload :Timestamp, 'active_record/timestamp'
autoload :Transactions, 'active_record/transactions'
+ autoload :UniqueConstraints, 'active_record/unique_constraints'
autoload :Types, 'active_record/types'
autoload :Validations, 'active_record/validations'
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 056f29f..e2efb2a 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -3094,6 +3094,10 @@ module ActiveRecord #:nodoc:
include Aggregations, Transactions, Reflection, Batches, Calculations, Serialization
+ # UniqueConstraints needs to be included after Transactions, because we need #save_with_unique_constraints
+ # to rescue after the transaction has been resolved for postgres.
+ include UniqueConstraints
+
end
end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 8fae26b..832b962 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -198,6 +198,13 @@ module ActiveRecord
end
end
+ # given an ActiveRecord::RecordNotUnique exception and a table name this returns the index whose
+ # unique constraint was violated, or nil if it can't determine the index
+ def index_for_record_not_unique(not_unique_exception, table_name) #:nodoc:
+ # override in derived class
+ nil
+ end
+
protected
def log(sql, name, &block)
ActiveSupport::Notifications.instrument(:sql, :sql => sql, :name => name, &block)
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index ad36ff2..3ca7608 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -561,6 +561,18 @@ module ActiveRecord
where_sql
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ case exception.message
+ when /Duplicate entry.*for key (\d+)/
+ index_position = $1.to_i
+ # minus two b/c mysql message is one-based + rails excludes primary key index from indexes list
+ indexes(table_name)[index_position - 2]
+ when /Duplicate entry.*for key '(\w+)'/
+ index_name = $1
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
def translate_exception(exception, message)
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 1d52c5e..70bbbd8 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -929,6 +929,13 @@ module ActiveRecord
sql << order_columns * ', '
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/unique constraint "(\w+)"/)
+ index_name = match[1]
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
# Returns the version of the connected PostgreSQL version.
def postgresql_version
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index c9c2892..72dc049 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -294,6 +294,13 @@ module ActiveRecord
"VALUES(NULL)"
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/column(?:s)? (.*) (?:is|are) not unique/)
+ index_columns = match[1].split(', ')
+ indexes(table_name).detect { |i| i.columns == index_columns }
+ end
+ end
+
protected
def select(sql, name = nil) #:nodoc:
execute(sql, name).map do |row|
diff --git a/activerecord/lib/active_record/locale/en.yml b/activerecord/lib/active_record/locale/en.yml
index 092f5f0..8144735 100644
--- a/activerecord/lib/active_record/locale/en.yml
+++ b/activerecord/lib/active_record/locale/en.yml
@@ -15,6 +15,8 @@ en:
too_short: "is too short (minimum is {{count}} characters)"
wrong_length: "is the wrong length (should be {{count}} characters)"
taken: "has already been taken"
+ taken_multiple: "has already been taken for {{context}}"
+ taken_generic: "Unique requirement not met"
not_a_number: "is not a number"
greater_than: "must be greater than {{count}}"
greater_than_or_equal_to: "must be greater than or equal to {{count}}"
diff --git a/activerecord/lib/active_record/unique_constraints.rb b/activerecord/lib/active_record/unique_constraints.rb
new file mode 100644
index 0000000..7a17b6a
--- /dev/null
+++ b/activerecord/lib/active_record/unique_constraints.rb
@@ -0,0 +1,24 @@
+module ActiveRecord
+ # See ActiveRecord::Transactions::ClassMethods for documentation.
+ module UniqueConstraints
+ def self.included(base)
+ base.class_eval do
+ alias_method_chain :save, :unique_constraints
+ end
+ end
+
+ def save_with_unique_constraints(perform_validation = true) #:nodoc:
+ save_without_unique_constraints(perform_validation)
+ rescue ActiveRecord::RecordNotUnique => e
+ index = connection.index_for_record_not_unique(e, self.class.table_name)
+ if !index
+ errors[:base] << I18n.translate(:'activerecord.errors.messages.taken_generic')
+ elsif index.columns.size == 1
+ errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
+ else
+ errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
+ end
+ false
+ end
+ end
+end
\ No newline at end of file
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index c59be26..4c8f58b 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -142,4 +142,39 @@ class AdapterTest < ActiveRecord::TestCase
end
end
end
+
+ # tests for db-enforced unique constraints
+ unique_scenarios = {
+ :single => {:sql => "INSERT INTO unique_items(uniq) VALUES('abc')",
+ :expected => ['uniq']},
+ :single_named => {:sql => "INSERT INTO unique_items(uniq_named) VALUES('abc')",
+ :expected => ['uniq_named']},
+ :multiple => {:sql => "INSERT INTO unique_items(uniq_1, uniq_2, uniq_3) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1', 'uniq_2', 'uniq_3']},
+ :multiple_named => {:sql => "INSERT INTO unique_items(uniq_1_named, uniq_2_named, uniq_3_named) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1_named', 'uniq_2_named', 'uniq_3_named']}
+ }
+ unique_scenarios.each do |name, data|
+ test "finds index for unique constraint #{name}" do
+ @connection.execute data[:sql]
+ exception = get_exception(ActiveRecord::RecordNotUnique) do
+ @connection.execute data[:sql]
+ end
+ # postgres can't query indexes until transaction is rolled back
+ @connection.rollback_db_transaction if current_adapter?(:PostgreSQLAdapter)
+ assert_equal data[:expected], @connection.index_for_record_not_unique(exception, 'unique_items').columns
+ end
+ end
+
+ protected
+
+ def get_exception(expected_exception)
+ begin
+ yield
+ rescue expected_exception => e
+ e
+ else
+ flunk "Expected #{expected_exception} exception"
+ end
+ end
end
diff --git a/activerecord/test/cases/unique_constraints_test.rb b/activerecord/test/cases/unique_constraints_test.rb
new file mode 100644
index 0000000..92ba6d8
--- /dev/null
+++ b/activerecord/test/cases/unique_constraints_test.rb
@@ -0,0 +1,40 @@
+require "cases/helper"
+require 'models/unique_item'
+
+class UniquenessValidationTest < ActiveRecord::TestCase
+
+ def test_validate_uniqueness_on_create
+ valid_item = UniqueItem.create!(:uniq => 'abc')
+ invalid_item = UniqueItem.create(:uniq => 'abc')
+ assert_equal ["has already been taken"], invalid_item.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_save
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.uniq = item_1.uniq
+ item_2.save
+ assert_equal ["has already been taken"], item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_update
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.update_attributes(:uniq => item_1.uniq)
+ assert_equal ["has already been taken"], item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_multiple
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal ["has already been taken for uniq_2/uniq_3"], invalid_item.errors[:uniq_1]
+ end
+
+ def test_validate_uniqueness_generic
+ ActiveRecord::Base.connection.expects(:index_for_record_not_unique).returns(nil)
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal ["Unique requirement not met"], invalid_item.errors[:base]
+ end
+
+end
\ No newline at end of file
diff --git a/activerecord/test/models/unique_item.rb b/activerecord/test/models/unique_item.rb
new file mode 100644
index 0000000..3c6f6f0
--- /dev/null
+++ b/activerecord/test/models/unique_item.rb
@@ -0,0 +1,2 @@
+class UniqueItem < ActiveRecord::Base
+end
\ No newline at end of file
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 15e5e12..a2b3da4 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -532,6 +532,21 @@ ActiveRecord::Schema.define do
t.string :title
end
+ create_table :unique_items, :force => true do |t|
+ t.string :uniq
+ t.string :uniq_1
+ t.string :uniq_2
+ t.string :uniq_3
+ t.string :uniq_named
+ t.string :uniq_1_named
+ t.string :uniq_2_named
+ t.string :uniq_3_named
+ end
+ add_index :unique_items, [:uniq], :unique => true
+ add_index :unique_items, [:uniq_named], :unique => true, :name => 'unique_named_single_index'
+ add_index :unique_items, [:uniq_1, :uniq_2, :uniq_3], :unique => true
+ add_index :unique_items, [:uniq_1_named, :uniq_2_named, :uniq_3_named], :unique => true, :name => 'unique_named_multiple_index'
+
except 'SQLite' do
# fk_test_has_fk should be before fk_test_has_pk
create_table :fk_test_has_fk, :force => true do |t|
--
1.6.1.2
unique-patch-2-3-stable.diff
Diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 342 343 344 345 346 347 348 349 350 351 352 353 354 355 356 357 358 359 360 361 362 363 364 365 366 367 368 369 370 371 372 373 374 375 376 377 378 379 380 381 382 383 384 385 386 387 388 389 390 391 392 393 394 395 396 397 398 399 400 401 402 403 404 405 406 407 408 409 410 411 412 413 414 415 416 417 418 419 420 421 422 423 424 425 426 427 428 429 430 431 432 433 434 435 436 437 438 439 440 441 442 443 444 445 446 447 448 449 450 451 452 453 454 455 456 457 458 459 460 461 462 463 464 465 466 467 468 469 470 471 472 473 474 475 476 477 478 479 480 481 482 483 484 485 486 487 488 489 490 491 492 493 494 495 496 497 498 499 500 501 502 503 504 505 506 507 508 509 510 511 512 513 514 515 516 517 518 519 520 521 522 523 524 525 526 527 528 529 530 531 532 533 534 535 536 537 538 539 540 541 542 543 544 545 546 547 548 549 550 551 552 553 554 555 556 557 558 559 560 561 562 563 564 565 566 567 568 569 570 571 572 573 574 575 576 577 578 579 580 581 582 583 584 585 586 587 588 589 590 591 592 593 594 595 596 597 598 599
From 5573e913b8651ea53ae6d94686b4b63f2cca2e83 Mon Sep 17 00:00:00 2001
From: Michael Schuerig <michael@schuerig.de>
Date: Sun, 5 Apr 2009 01:19:29 +0200
Subject: [PATCH 1/4] Translate adapter errors that indicate a violated uniqueness constraint to ActiveRecord::RecordNotUnique exception derived from ActiveReecord::StatementInvalid.
 
Signed-off-by: Michael Koziarski <michael@koziarski.com>
---
activerecord/lib/active_record/base.rb | 4 ++++
.../connection_adapters/abstract_adapter.rb | 7 ++++++-
.../connection_adapters/mysql_adapter.rb | 11 +++++++++++
.../connection_adapters/postgresql_adapter.rb | 9 +++++++++
.../connection_adapters/sqlite_adapter.rb | 10 ++++++++++
activerecord/test/cases/adapter_test.rb | 7 +++++++
6 files changed, 47 insertions(+), 1 deletions(-)
 
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 4929969..27b645d 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -55,6 +55,10 @@ module ActiveRecord #:nodoc:
class StatementInvalid < ActiveRecordError
end
+ # Raised when a record cannot be inserted because it would violate a uniqueness constraint.
+ class RecordNotUnique < StatementInvalid
+ end
+
# Raised when number of bind variables in statement given to <tt>:condition</tt> key (for example, when using +find+ method)
# does not match number of expected variables.
#
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 22871f2..6b7b363 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -216,9 +216,14 @@ module ActiveRecord
@last_verification = 0
message = "#{e.class.name}: #{e.message}: #{sql}"
log_info(message, name, 0)
- raise ActiveRecord::StatementInvalid, message
+ raise translate_exception(e, message)
end
+ def translate_exception(e, message)
+ # override in derived class
+ ActiveRecord::StatementInvalid.new(message)
+ end
+
def format_log_entry(message, dump = nil)
if ActiveRecord::Base.colorize_logging
if @@row_even
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index a8aa16e..8625395 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -586,6 +586,17 @@ module ActiveRecord
where_sql
end
+ protected
+
+ def translate_exception(exception, message)
+ case exception.errno
+ when 1062
+ RecordNotUnique.new(message)
+ else
+ super
+ end
+ end
+
private
def connect
encoding = @config[:encoding]
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index bc289ff..4bd7e4a 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -958,6 +958,15 @@ module ActiveRecord
end
end
+ def translate_exception(exception, message)
+ case exception.message
+ when /duplicate key value violates unique constraint/
+ RecordNotUnique.new(message)
+ else
+ super
+ end
+ end
+
private
# The internal PostgreSQL identifier of the money data type.
MONEY_COLUMN_TYPE_OID = 790 #:nodoc:
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index 0bf97a9..88831ee 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -435,6 +435,16 @@ module ActiveRecord
'INTEGER PRIMARY KEY NOT NULL'.freeze
end
end
+
+ def translate_exception(exception, message)
+ case exception.message
+ when /column(s)? .* (is|are) not unique/
+ RecordNotUnique.new(message)
+ else
+ super
+ end
+ end
+
end
class SQLite2Adapter < SQLiteAdapter # :nodoc:
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 3dd3dd8..27d1625 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -142,4 +142,11 @@ class AdapterTest < ActiveRecord::TestCase
assert_equal " LIMIT 1,7 OFFSET 7", @connection.add_limit_offset!("", :limit=>sql_inject, :offset=>7)
end
end
+
+ def test_uniqueness_violations_are_translated_to_specific_exception
+ @connection.execute "INSERT INTO subscribers(nick) VALUES('me')"
+ assert_raises(ActiveRecord::RecordNotUnique) do
+ @connection.execute "INSERT INTO subscribers(nick) VALUES('me')"
+ end
+ end
end
--
1.6.1.2
 
 
From 1b69f6d39bbb35be22de771261426bfdb2b0459e Mon Sep 17 00:00:00 2001
From: Michael Schuerig <michael@schuerig.de>
Date: Sun, 5 Apr 2009 01:42:21 +0200
Subject: [PATCH 2/4] Translate foreign key violations to ActiveRecord::InvalidForeignKey exceptions.
 
Signed-off-by: Michael Koziarski <michael@koziarski.com>
---
activerecord/lib/active_record/base.rb | 4 ++++
.../connection_adapters/mysql_adapter.rb | 2 ++
.../connection_adapters/postgresql_adapter.rb | 2 ++
activerecord/test/cases/adapter_test.rb | 8 ++++++++
4 files changed, 16 insertions(+), 0 deletions(-)
 
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 27b645d..57c1d6d 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -59,6 +59,10 @@ module ActiveRecord #:nodoc:
class RecordNotUnique < StatementInvalid
end
+ # Raised when a record cannot be inserted or updated because it references a non-existent record.
+ class InvalidForeignKey < StatementInvalid
+ end
+
# Raised when number of bind variables in statement given to <tt>:condition</tt> key (for example, when using +find+ method)
# does not match number of expected variables.
#
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 8625395..ac3dfda 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -592,6 +592,8 @@ module ActiveRecord
case exception.errno
when 1062
RecordNotUnique.new(message)
+ when 1452
+ InvalidForeignKey.new(message)
else
super
end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 4bd7e4a..4ab06e8 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -962,6 +962,8 @@ module ActiveRecord
case exception.message
when /duplicate key value violates unique constraint/
RecordNotUnique.new(message)
+ when /violates foreign key constraint/
+ InvalidForeignKey.new(message)
else
super
end
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 27d1625..4797112 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -149,4 +149,12 @@ class AdapterTest < ActiveRecord::TestCase
@connection.execute "INSERT INTO subscribers(nick) VALUES('me')"
end
end
+
+ def test_foreign_key_violations_are_translated_to_specific_exception
+ unless @connection.adapter_name == 'SQLite'
+ assert_raises(ActiveRecord::InvalidForeignKey) do
+ @connection.execute "INSERT INTO fk_test_has_fk (fk_id) VALUES (0)"
+ end
+ end
+ end
end
--
1.6.1.2
 
 
From edffa1bda17cca21e84cc953ebbabae4d295d0db Mon Sep 17 00:00:00 2001
From: Michael Koziarski <michael@koziarski.com>
Date: Fri, 26 Jun 2009 16:59:27 +1200
Subject: [PATCH 3/4] Make sure the wrapped exceptions also have the original exception available.
 
[#2419 state:committed]
---
activerecord/lib/active_record/base.rb | 15 +++++++++++++--
.../connection_adapters/mysql_adapter.rb | 4 ++--
.../connection_adapters/postgresql_adapter.rb | 4 ++--
.../connection_adapters/sqlite_adapter.rb | 2 +-
4 files changed, 18 insertions(+), 7 deletions(-)
 
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 57c1d6d..7c8c866 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -55,12 +55,23 @@ module ActiveRecord #:nodoc:
class StatementInvalid < ActiveRecordError
end
+ # Parent class for all specific exceptions which wrap database driver exceptions
+ # provides access to the original exception also.
+ class WrappedDatabaseException < StatementInvalid
+ attr_reader :original_exception
+
+ def initialize(message, original_exception)
+ super(message)
+ @original_exception, = original_exception
+ end
+ end
+
# Raised when a record cannot be inserted because it would violate a uniqueness constraint.
- class RecordNotUnique < StatementInvalid
+ class RecordNotUnique < WrappedDatabaseException
end
# Raised when a record cannot be inserted or updated because it references a non-existent record.
- class InvalidForeignKey < StatementInvalid
+ class InvalidForeignKey < WrappedDatabaseException
end
# Raised when number of bind variables in statement given to <tt>:condition</tt> key (for example, when using +find+ method)
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index ac3dfda..1fffec8 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -591,9 +591,9 @@ module ActiveRecord
def translate_exception(exception, message)
case exception.errno
when 1062
- RecordNotUnique.new(message)
+ RecordNotUnique.new(message, exception)
when 1452
- InvalidForeignKey.new(message)
+ InvalidForeignKey.new(message, exception)
else
super
end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 4ab06e8..b1cac88 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -961,9 +961,9 @@ module ActiveRecord
def translate_exception(exception, message)
case exception.message
when /duplicate key value violates unique constraint/
- RecordNotUnique.new(message)
+ RecordNotUnique.new(message, exception)
when /violates foreign key constraint/
- InvalidForeignKey.new(message)
+ InvalidForeignKey.new(message, exception)
else
super
end
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index 88831ee..79eea1d 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -439,7 +439,7 @@ module ActiveRecord
def translate_exception(exception, message)
case exception.message
when /column(s)? .* (is|are) not unique/
- RecordNotUnique.new(message)
+ RecordNotUnique.new(message, exception)
else
super
end
--
1.6.1.2
 
 
From 5dc2eb5d07f8e999f0c9d0d3932a5a7d5bce2dae Mon Sep 17 00:00:00 2001
From: Jordan Brough <jordan@animoto.com>
Date: Wed, 11 Nov 2009 13:31:34 -0700
Subject: [PATCH 4/4] Friendly ActiveRecord error handling for db-enforced unique constraints
 
---
activerecord/lib/active_record.rb | 1 +
activerecord/lib/active_record/base.rb | 4 ++
.../connection_adapters/abstract_adapter.rb | 7 +++
.../connection_adapters/mysql_adapter.rb | 12 ++++++
.../connection_adapters/postgresql_adapter.rb | 7 +++
.../connection_adapters/sqlite_adapter.rb | 7 +++
activerecord/lib/active_record/locale/en.yml | 2 +
.../lib/active_record/unique_constraints.rb | 24 ++++++++++++
activerecord/test/cases/adapter_test.rb | 35 +++++++++++++++++
activerecord/test/cases/unique_constraints_test.rb | 40 ++++++++++++++++++++
activerecord/test/models/unique_item.rb | 2 +
activerecord/test/schema/schema.rb | 15 +++++++
12 files changed, 156 insertions(+), 0 deletions(-)
create mode 100644 activerecord/lib/active_record/unique_constraints.rb
create mode 100644 activerecord/test/cases/unique_constraints_test.rb
create mode 100644 activerecord/test/models/unique_item.rb
 
diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb
index 2f8c5c7..8469255 100644
--- a/activerecord/lib/active_record.rb
+++ b/activerecord/lib/active_record.rb
@@ -68,6 +68,7 @@ module ActiveRecord
autoload :TestCase, 'active_record/test_case'
autoload :Timestamp, 'active_record/timestamp'
autoload :Transactions, 'active_record/transactions'
+ autoload :UniqueConstraints, 'active_record/unique_constraints'
autoload :Validations, 'active_record/validations'
module Locking
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 7c8c866..da9c110 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -3200,6 +3200,10 @@ module ActiveRecord #:nodoc:
include AutosaveAssociation, NestedAttributes
include Aggregations, Transactions, Reflection, Batches, Calculations, Serialization
+
+ # UniqueConstraints needs to be included after Transactions, because we need #save_with_unique_constraints
+ # to rescue after the transaction has been resolved for postgres.
+ include UniqueConstraints
end
end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 6b7b363..6adea19 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -197,6 +197,13 @@ module ActiveRecord
end
end
+ # given an ActiveRecord::RecordNotUnique exception and a table name this returns the index whose
+ # unique constraint was violated, or nil if it can't determine the index
+ def index_for_record_not_unique(not_unique_exception, table_name) #:nodoc:
+ # override in derived class
+ nil
+ end
+
protected
def log(sql, name)
if block_given?
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 1fffec8..658df1f 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -586,6 +586,18 @@ module ActiveRecord
where_sql
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ case exception.message
+ when /Duplicate entry.*for key (\d+)/
+ index_position = $1.to_i
+ # minus two b/c mysql message is one-based + rails excludes primary key index from indexes list
+ indexes(table_name)[index_position - 2]
+ when /Duplicate entry.*for key '(\w+)'/
+ index_name = $1
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
def translate_exception(exception, message)
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index b1cac88..c313082 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -941,6 +941,13 @@ module ActiveRecord
sql.replace "SELECT * FROM (#{sql}) AS id_list ORDER BY #{order}"
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/unique constraint "(\w+)"/)
+ index_name = match[1]
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
# Returns the version of the connected PostgreSQL version.
def postgresql_version
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index 79eea1d..31519f1 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -315,6 +315,13 @@ module ActiveRecord
"INSERT INTO #{table_name} VALUES(NULL)"
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/column(?:s)? (.*) (?:is|are) not unique/)
+ index_columns = match[1].split(', ')
+ indexes(table_name).detect { |i| i.columns == index_columns }
+ end
+ end
+
protected
def select(sql, name = nil) #:nodoc:
execute(sql, name).map do |row|
diff --git a/activerecord/lib/active_record/locale/en.yml b/activerecord/lib/active_record/locale/en.yml
index 2813524..a7dec95 100644
--- a/activerecord/lib/active_record/locale/en.yml
+++ b/activerecord/lib/active_record/locale/en.yml
@@ -15,6 +15,8 @@ en:
too_short: "is too short (minimum is {{count}} characters)"
wrong_length: "is the wrong length (should be {{count}} characters)"
taken: "has already been taken"
+ taken_multiple: "has already been taken for {{context}}"
+ taken_generic: "Unique requirement not met"
not_a_number: "is not a number"
greater_than: "must be greater than {{count}}"
greater_than_or_equal_to: "must be greater than or equal to {{count}}"
diff --git a/activerecord/lib/active_record/unique_constraints.rb b/activerecord/lib/active_record/unique_constraints.rb
new file mode 100644
index 0000000..99e6353
--- /dev/null
+++ b/activerecord/lib/active_record/unique_constraints.rb
@@ -0,0 +1,24 @@
+module ActiveRecord
+ # See ActiveRecord::Transactions::ClassMethods for documentation.
+ module UniqueConstraints
+ def self.included(base)
+ base.class_eval do
+ alias_method_chain :save, :unique_constraints
+ end
+ end
+
+ def save_with_unique_constraints(perform_validation = true) #:nodoc:
+ save_without_unique_constraints(perform_validation)
+ rescue ActiveRecord::RecordNotUnique => e
+ index = connection.index_for_record_not_unique(e.original_exception, self.class.table_name)
+ if !index
+ errors.add_to_base(I18n.translate(:'activerecord.errors.messages.taken_generic'))
+ elsif index.columns.size == 1
+ errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
+ else
+ errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
+ end
+ false
+ end
+ end
+end
\ No newline at end of file
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 4797112..0883ea0 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -157,4 +157,39 @@ class AdapterTest < ActiveRecord::TestCase
end
end
end
+
+ # tests for db-enforced unique constraints
+ unique_scenarios = {
+ :single => {:sql => "INSERT INTO unique_items(uniq) VALUES('abc')",
+ :expected => ['uniq']},
+ :single_named => {:sql => "INSERT INTO unique_items(uniq_named) VALUES('abc')",
+ :expected => ['uniq_named']},
+ :multiple => {:sql => "INSERT INTO unique_items(uniq_1, uniq_2, uniq_3) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1', 'uniq_2', 'uniq_3']},
+ :multiple_named => {:sql => "INSERT INTO unique_items(uniq_1_named, uniq_2_named, uniq_3_named) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1_named', 'uniq_2_named', 'uniq_3_named']}
+ }
+ unique_scenarios.each do |name, data|
+ test "finds index for unique constraint #{name}" do
+ @connection.execute data[:sql]
+ exception = get_exception(ActiveRecord::RecordNotUnique) do
+ @connection.execute data[:sql]
+ end
+ # postgres can't query indexes until transaction is rolled back
+ @connection.rollback_db_transaction if current_adapter?(:PostgreSQLAdapter)
+ assert_equal data[:expected], @connection.index_for_record_not_unique(exception, 'unique_items').columns
+ end
+ end
+
+ protected
+
+ def get_exception(expected_exception)
+ begin
+ yield
+ rescue expected_exception => e
+ e
+ else
+ flunk "Expected #{expected_exception} exception"
+ end
+ end
end
diff --git a/activerecord/test/cases/unique_constraints_test.rb b/activerecord/test/cases/unique_constraints_test.rb
new file mode 100644
index 0000000..2b5d0ee
--- /dev/null
+++ b/activerecord/test/cases/unique_constraints_test.rb
@@ -0,0 +1,40 @@
+require "cases/helper"
+require 'models/unique_item'
+
+class UniquenessValidationTest < ActiveRecord::TestCase
+
+ def test_validate_uniqueness_on_create
+ valid_item = UniqueItem.create!(:uniq => 'abc')
+ invalid_item = UniqueItem.create(:uniq => 'abc')
+ assert_equal "has already been taken", invalid_item.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_save
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.uniq = item_1.uniq
+ item_2.save
+ assert_equal "has already been taken", item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_update
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.update_attributes(:uniq => item_1.uniq)
+ assert_equal "has already been taken", item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_multiple
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal "has already been taken for uniq_2/uniq_3", invalid_item.errors[:uniq_1]
+ end
+
+ def test_validate_uniqueness_generic
+ ActiveRecord::Base.connection.expects(:index_for_record_not_unique).returns(nil)
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal "Unique requirement not met", invalid_item.errors[:base]
+ end
+
+end
\ No newline at end of file
diff --git a/activerecord/test/models/unique_item.rb b/activerecord/test/models/unique_item.rb
new file mode 100644
index 0000000..3c6f6f0
--- /dev/null
+++ b/activerecord/test/models/unique_item.rb
@@ -0,0 +1,2 @@
+class UniqueItem < ActiveRecord::Base
+end
\ No newline at end of file
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index b046104..17a6fc4 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -499,6 +499,21 @@ ActiveRecord::Schema.define do
t.string :title
end
+ create_table :unique_items, :force => true do |t|
+ t.string :uniq
+ t.string :uniq_1
+ t.string :uniq_2
+ t.string :uniq_3
+ t.string :uniq_named
+ t.string :uniq_1_named
+ t.string :uniq_2_named
+ t.string :uniq_3_named
+ end
+ add_index :unique_items, [:uniq], :unique => true
+ add_index :unique_items, [:uniq_named], :unique => true, :name => 'unique_named_single_index'
+ add_index :unique_items, [:uniq_1, :uniq_2, :uniq_3], :unique => true
+ add_index :unique_items, [:uniq_1_named, :uniq_2_named, :uniq_3_named], :unique => true, :name => 'unique_named_multiple_index'
+
except 'SQLite' do
# fk_test_has_fk should be before fk_test_has_pk
create_table :fk_test_has_fk, :force => true do |t|
--
1.6.1.2
unique-patch-2-3-stable.patch
Diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 342 343 344 345 346 347 348 349 350 351 352 353 354 355 356 357 358 359 360 361 362 363 364 365 366 367 368 369 370 371 372 373 374 375 376 377 378 379 380 381 382 383 384 385 386 387 388 389 390 391 392 393 394 395 396 397 398 399 400 401 402 403 404 405 406 407 408 409 410 411 412 413 414 415 416 417 418 419 420 421 422 423 424 425 426 427 428 429 430 431 432 433 434 435 436 437 438 439 440 441 442 443 444 445 446 447 448 449 450 451 452 453 454 455 456 457 458 459 460 461 462 463 464 465 466 467 468 469 470 471 472 473 474 475 476 477 478 479 480 481 482 483 484 485 486 487 488 489 490 491 492 493 494 495 496 497 498 499 500 501 502 503 504 505 506 507 508 509 510 511 512 513 514 515 516 517 518 519 520 521 522 523 524 525 526 527 528 529 530 531 532 533 534 535 536 537 538 539 540 541 542 543 544 545 546 547 548 549 550 551 552 553 554 555 556 557 558 559 560 561 562 563 564 565 566 567 568 569 570 571 572 573 574 575 576 577 578 579 580 581 582 583 584 585 586 587 588 589 590 591 592 593 594 595 596 597 598 599 600 601 602 603 604 605 606 607 608 609 610 611 612 613 614 615 616 617 618 619 620 621 622 623 624 625 626 627 628 629 630 631 632 633 634 635 636 637 638 639 640 641 642 643 644 645 646 647 648 649 650 651 652 653 654 655 656 657 658 659 660 661 662 663 664 665 666 667 668 669 670 671 672 673 674 675 676 677 678 679 680 681 682 683 684 685 686 687 688 689 690 691 692 693 694 695 696 697 698 699 700 701 702 703 704 705 706 707 708 709 710 711 712 713 714 715 716 717 718 719 720 721 722 723 724 725 726 727 728 729 730 731 732 733 734 735 736 737 738
From 48fcff95ae1b5e55837e81a4c132a3c208a235f8 Mon Sep 17 00:00:00 2001
From: Michael Schuerig <michael@schuerig.de>
Date: Sun, 5 Apr 2009 01:19:29 +0200
Subject: [PATCH 1/5] Translate adapter errors that indicate a violated uniqueness constraint to ActiveRecord::RecordNotUnique exception derived from ActiveReecord::StatementInvalid.
 
Signed-off-by: Michael Koziarski <michael@koziarski.com>
---
activerecord/lib/active_record/base.rb | 4 ++++
.../connection_adapters/abstract_adapter.rb | 7 ++++++-
.../connection_adapters/mysql_adapter.rb | 9 +++++++++
.../connection_adapters/postgresql_adapter.rb | 9 +++++++++
.../connection_adapters/sqlite_adapter.rb | 10 ++++++++++
activerecord/test/cases/adapter_test.rb | 7 +++++++
6 files changed, 45 insertions(+), 1 deletions(-)
 
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index ac82cc1..94468a4 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -56,6 +56,10 @@ module ActiveRecord #:nodoc:
class StatementInvalid < ActiveRecordError
end
+ # Raised when a record cannot be inserted because it would violate a uniqueness constraint.
+ class RecordNotUnique < StatementInvalid
+ end
+
# Raised when number of bind variables in statement given to <tt>:condition</tt> key (for example, when using +find+ method)
# does not match number of expected variables.
#
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index b8a6a71..6d21960 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -224,9 +224,14 @@ module ActiveRecord
@last_verification = 0
message = "#{e.class.name}: #{e.message}: #{sql}"
log_info(message, name, 0)
- raise ActiveRecord::StatementInvalid, message
+ raise translate_exception(e, message)
end
+ def translate_exception(e, message)
+ # override in derived class
+ ActiveRecord::StatementInvalid.new(message)
+ end
+
def format_log_entry(message, dump = nil)
if ActiveRecord::Base.colorize_logging
if @@row_even
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 6f41f84..62d9417 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -602,6 +602,15 @@ module ActiveRecord
end
end
+ def translate_exception(exception, message)
+ case exception.errno
+ when 1062
+ RecordNotUnique.new(message)
+ else
+ super
+ end
+ end
+
private
def connect
encoding = @config[:encoding]
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index a348318..66afc3c 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -931,6 +931,15 @@ module ActiveRecord
end
end
+ def translate_exception(exception, message)
+ case exception.message
+ when /duplicate key value violates unique constraint/
+ RecordNotUnique.new(message)
+ else
+ super
+ end
+ end
+
private
# The internal PostgreSQL identifier of the money data type.
MONEY_COLUMN_TYPE_OID = 790 #:nodoc:
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index 1616698..e0d04e6 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -436,6 +436,16 @@ module ActiveRecord
'INTEGER PRIMARY KEY NOT NULL'.freeze
end
end
+
+ def translate_exception(exception, message)
+ case exception.message
+ when /column(s)? .* (is|are) not unique/
+ RecordNotUnique.new(message)
+ else
+ super
+ end
+ end
+
end
class SQLite2Adapter < SQLiteAdapter # :nodoc:
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index b7fa6df..2118c10 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -141,4 +141,11 @@ class AdapterTest < ActiveRecord::TestCase
assert_equal " LIMIT 1,7 OFFSET 7", @connection.add_limit_offset!("", :limit=>sql_inject, :offset=>7)
end
end
+
+ def test_uniqueness_violations_are_translated_to_specific_exception
+ @connection.execute "INSERT INTO subscribers(nick) VALUES('me')"
+ assert_raises(ActiveRecord::RecordNotUnique) do
+ @connection.execute "INSERT INTO subscribers(nick) VALUES('me')"
+ end
+ end
end
--
1.7.2.1
 
 
From 9efed026a931dc4203b4332b0951b8b09c669476 Mon Sep 17 00:00:00 2001
From: Michael Schuerig <michael@schuerig.de>
Date: Sun, 5 Apr 2009 01:42:21 +0200
Subject: [PATCH 2/5] Translate foreign key violations to ActiveRecord::InvalidForeignKey exceptions.
 
Signed-off-by: Michael Koziarski <michael@koziarski.com>
---
activerecord/lib/active_record/base.rb | 4 ++++
.../connection_adapters/mysql_adapter.rb | 2 ++
.../connection_adapters/postgresql_adapter.rb | 2 ++
activerecord/test/cases/adapter_test.rb | 8 ++++++++
4 files changed, 16 insertions(+), 0 deletions(-)
 
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 94468a4..c42b1ab 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -60,6 +60,10 @@ module ActiveRecord #:nodoc:
class RecordNotUnique < StatementInvalid
end
+ # Raised when a record cannot be inserted or updated because it references a non-existent record.
+ class InvalidForeignKey < StatementInvalid
+ end
+
# Raised when number of bind variables in statement given to <tt>:condition</tt> key (for example, when using +find+ method)
# does not match number of expected variables.
#
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 62d9417..e49908b 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -606,6 +606,8 @@ module ActiveRecord
case exception.errno
when 1062
RecordNotUnique.new(message)
+ when 1452
+ InvalidForeignKey.new(message)
else
super
end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 66afc3c..d79aea5 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -935,6 +935,8 @@ module ActiveRecord
case exception.message
when /duplicate key value violates unique constraint/
RecordNotUnique.new(message)
+ when /violates foreign key constraint/
+ InvalidForeignKey.new(message)
else
super
end
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 2118c10..7a9a289 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -148,4 +148,12 @@ class AdapterTest < ActiveRecord::TestCase
@connection.execute "INSERT INTO subscribers(nick) VALUES('me')"
end
end
+
+ def test_foreign_key_violations_are_translated_to_specific_exception
+ unless @connection.adapter_name == 'SQLite'
+ assert_raises(ActiveRecord::InvalidForeignKey) do
+ @connection.execute "INSERT INTO fk_test_has_fk (fk_id) VALUES (0)"
+ end
+ end
+ end
end
--
1.7.2.1
 
 
From 3d2796f98d7f2c33bb63f6c2f2026588b99fb2c9 Mon Sep 17 00:00:00 2001
From: Michael Koziarski <michael@koziarski.com>
Date: Fri, 26 Jun 2009 16:59:27 +1200
Subject: [PATCH 3/5] Make sure the wrapped exceptions also have the original exception available.
 
[#2419 state:committed]
---
activerecord/lib/active_record/base.rb | 15 +++++++++++++--
.../connection_adapters/mysql_adapter.rb | 4 ++--
.../connection_adapters/postgresql_adapter.rb | 4 ++--
.../connection_adapters/sqlite_adapter.rb | 2 +-
4 files changed, 18 insertions(+), 7 deletions(-)
 
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index c42b1ab..9fc1ea5 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -56,12 +56,23 @@ module ActiveRecord #:nodoc:
class StatementInvalid < ActiveRecordError
end
+ # Parent class for all specific exceptions which wrap database driver exceptions
+ # provides access to the original exception also.
+ class WrappedDatabaseException < StatementInvalid
+ attr_reader :original_exception
+
+ def initialize(message, original_exception)
+ super(message)
+ @original_exception, = original_exception
+ end
+ end
+
# Raised when a record cannot be inserted because it would violate a uniqueness constraint.
- class RecordNotUnique < StatementInvalid
+ class RecordNotUnique < WrappedDatabaseException
end
# Raised when a record cannot be inserted or updated because it references a non-existent record.
- class InvalidForeignKey < StatementInvalid
+ class InvalidForeignKey < WrappedDatabaseException
end
# Raised when number of bind variables in statement given to <tt>:condition</tt> key (for example, when using +find+ method)
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index e49908b..0ecab7b 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -605,9 +605,9 @@ module ActiveRecord
def translate_exception(exception, message)
case exception.errno
when 1062
- RecordNotUnique.new(message)
+ RecordNotUnique.new(message, exception)
when 1452
- InvalidForeignKey.new(message)
+ InvalidForeignKey.new(message, exception)
else
super
end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index d79aea5..590a4f1 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -934,9 +934,9 @@ module ActiveRecord
def translate_exception(exception, message)
case exception.message
when /duplicate key value violates unique constraint/
- RecordNotUnique.new(message)
+ RecordNotUnique.new(message, exception)
when /violates foreign key constraint/
- InvalidForeignKey.new(message)
+ InvalidForeignKey.new(message, exception)
else
super
end
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index e0d04e6..ae1ffa0 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -440,7 +440,7 @@ module ActiveRecord
def translate_exception(exception, message)
case exception.message
when /column(s)? .* (is|are) not unique/
- RecordNotUnique.new(message)
+ RecordNotUnique.new(message, exception)
else
super
end
--
1.7.2.1
 
 
From 11a16a9838ecabfb25f5fc89e93bdc3d7b25c4bf Mon Sep 17 00:00:00 2001
From: Jordan Brough <jordan@animoto.com>
Date: Wed, 11 Nov 2009 13:31:34 -0700
Subject: [PATCH 4/5] Friendly ActiveRecord error handling for db-enforced unique constraints
 
---
activerecord/lib/active_record.rb | 1 +
activerecord/lib/active_record/base.rb | 1 +
.../connection_adapters/abstract_adapter.rb | 7 +++
.../connection_adapters/mysql_adapter.rb | 12 ++++++
.../connection_adapters/postgresql_adapter.rb | 13 ++++++
.../connection_adapters/sqlite_adapter.rb | 7 +++
activerecord/lib/active_record/locale/en.yml | 2 +
.../lib/active_record/unique_constraints.rb | 30 +++++++++++++++
activerecord/test/cases/adapter_test.rb | 35 +++++++++++++++++
activerecord/test/cases/unique_constraints_test.rb | 40 ++++++++++++++++++++
activerecord/test/models/unique_item.rb | 2 +
activerecord/test/schema/schema.rb | 15 +++++++
12 files changed, 165 insertions(+), 0 deletions(-)
create mode 100644 activerecord/lib/active_record/unique_constraints.rb
create mode 100644 activerecord/test/cases/unique_constraints_test.rb
create mode 100644 activerecord/test/models/unique_item.rb
 
diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb
index 0d0ada9..3404c16 100644
--- a/activerecord/lib/active_record.rb
+++ b/activerecord/lib/active_record.rb
@@ -68,6 +68,7 @@ module ActiveRecord
autoload :TestCase, 'active_record/test_case'
autoload :Timestamp, 'active_record/timestamp'
autoload :Transactions, 'active_record/transactions'
+ autoload :UniqueConstraints, 'active_record/unique_constraints'
autoload :Validations, 'active_record/validations'
module Locking
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 9fc1ea5..2e79954 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -3230,6 +3230,7 @@ module ActiveRecord #:nodoc:
include AutosaveAssociation, NestedAttributes
include Aggregations, Transactions, Reflection, Batches, Calculations, Serialization
+ include UniqueConstraints
end
end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 6d21960..c7e8269 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -199,6 +199,13 @@ module ActiveRecord
end
end
+ # given an ActiveRecord::RecordNotUnique exception and a table name this returns the index whose
+ # unique constraint was violated, or nil if it can't determine the index
+ def index_for_record_not_unique(not_unique_exception, table_name) #:nodoc:
+ # override in derived class
+ nil
+ end
+
protected
def log(sql, name)
if block_given?
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 0ecab7b..64f8f87 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -588,6 +588,18 @@ module ActiveRecord
where_sql
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ case exception.message
+ when /Duplicate entry.*for key (\d+)/
+ index_position = $1.to_i
+ # minus two b/c mysql message is one-based + rails excludes primary key index from indexes list
+ indexes(table_name)[index_position - 2]
+ when /Duplicate entry.*for key '(\w+)'/
+ index_name = $1
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
def quoted_columns_for_index(column_names, options = {})
length = options[:length] if options.is_a?(Hash)
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 590a4f1..2eb3fce 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -669,6 +669,12 @@ module ActiveRecord
indexes
end
+ # postgres doesn't let us query this in the middle of an aborted transaction so caching can come in handy
+ def cached_indexes(table_name)
+ @cached_indexes ||= {}
+ @cached_indexes[table_name] ||= indexes(table_name)
+ end
+
# Returns the list of all column definitions for a table.
def columns(table_name, name = nil)
# Limit, precision, and scale are all handled by the superclass.
@@ -914,6 +920,13 @@ module ActiveRecord
sql.replace "SELECT * FROM (#{sql}) AS id_list ORDER BY #{order}"
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/unique constraint "(\w+)"/)
+ index_name = match[1]
+ cached_indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
# Returns the version of the connected PostgreSQL version.
def postgresql_version
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index ae1ffa0..a43f6d4 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -316,6 +316,13 @@ module ActiveRecord
"INSERT INTO #{table_name} VALUES(NULL)"
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/column(?:s)? (.*) (?:is|are) not unique/)
+ index_columns = match[1].split(', ')
+ indexes(table_name).detect { |i| i.columns == index_columns }
+ end
+ end
+
protected
def select(sql, name = nil) #:nodoc:
execute(sql, name).map do |row|
diff --git a/activerecord/lib/active_record/locale/en.yml b/activerecord/lib/active_record/locale/en.yml
index 6dab5e2..353fbda 100644
--- a/activerecord/lib/active_record/locale/en.yml
+++ b/activerecord/lib/active_record/locale/en.yml
@@ -15,6 +15,8 @@ en:
too_short: "is too short (minimum is %{count} characters)"
wrong_length: "is the wrong length (should be %{count} characters)"
taken: "has already been taken"
+ taken_multiple: "has already been taken for %{context}"
+ taken_generic: "Unique requirement not met"
not_a_number: "is not a number"
greater_than: "must be greater than %{count}"
greater_than_or_equal_to: "must be greater than or equal to %{count}"
diff --git a/activerecord/lib/active_record/unique_constraints.rb b/activerecord/lib/active_record/unique_constraints.rb
new file mode 100644
index 0000000..6702e37
--- /dev/null
+++ b/activerecord/lib/active_record/unique_constraints.rb
@@ -0,0 +1,30 @@
+module ActiveRecord
+ # See ActiveRecord::Transactions::ClassMethods for documentation.
+ module UniqueConstraints
+ def self.included(base)
+ base.class_eval do
+ alias_method_chain :save, :unique_constraints
+ end
+ end
+
+ # work around postgres not allowing us to query indexes in the middle of an aborted transaction
+ def cache_indexes
+ connection.cached_indexes(self.class.table_name) if connection.respond_to?(:cached_indexes)
+ end
+
+ def save_with_unique_constraints(perform_validation = true) #:nodoc:
+ cache_indexes
+ save_without_unique_constraints(perform_validation)
+ rescue ActiveRecord::RecordNotUnique => e
+ index = connection.index_for_record_not_unique(e.original_exception, self.class.table_name)
+ if !index
+ errors.add_to_base(I18n.translate(:'activerecord.errors.messages.taken_generic'))
+ elsif index.columns.size == 1
+ errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
+ else
+ errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
+ end
+ false
+ end
+ end
+end
\ No newline at end of file
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 7a9a289..f99e8d6 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -156,4 +156,39 @@ class AdapterTest < ActiveRecord::TestCase
end
end
end
+
+ # tests for db-enforced unique constraints
+ unique_scenarios = {
+ :single => {:sql => "INSERT INTO unique_items(uniq) VALUES('abc')",
+ :expected => ['uniq']},
+ :single_named => {:sql => "INSERT INTO unique_items(uniq_named) VALUES('abc')",
+ :expected => ['uniq_named']},
+ :multiple => {:sql => "INSERT INTO unique_items(uniq_1, uniq_2, uniq_3) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1', 'uniq_2', 'uniq_3']},
+ :multiple_named => {:sql => "INSERT INTO unique_items(uniq_1_named, uniq_2_named, uniq_3_named) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1_named', 'uniq_2_named', 'uniq_3_named']}
+ }
+ unique_scenarios.each do |name, data|
+ test "finds index for unique constraint #{name}" do
+ @connection.execute data[:sql]
+ exception = get_exception(ActiveRecord::RecordNotUnique) do
+ @connection.execute data[:sql]
+ end
+ # postgres can't query indexes until transaction is rolled back
+ @connection.rollback_db_transaction if current_adapter?(:PostgreSQLAdapter)
+ assert_equal data[:expected], @connection.index_for_record_not_unique(exception, 'unique_items').columns
+ end
+ end
+
+ protected
+
+ def get_exception(expected_exception)
+ begin
+ yield
+ rescue expected_exception => e
+ e
+ else
+ flunk "Expected #{expected_exception} exception"
+ end
+ end
end
diff --git a/activerecord/test/cases/unique_constraints_test.rb b/activerecord/test/cases/unique_constraints_test.rb
new file mode 100644
index 0000000..2b5d0ee
--- /dev/null
+++ b/activerecord/test/cases/unique_constraints_test.rb
@@ -0,0 +1,40 @@
+require "cases/helper"
+require 'models/unique_item'
+
+class UniquenessValidationTest < ActiveRecord::TestCase
+
+ def test_validate_uniqueness_on_create
+ valid_item = UniqueItem.create!(:uniq => 'abc')
+ invalid_item = UniqueItem.create(:uniq => 'abc')
+ assert_equal "has already been taken", invalid_item.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_save
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.uniq = item_1.uniq
+ item_2.save
+ assert_equal "has already been taken", item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_update
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.update_attributes(:uniq => item_1.uniq)
+ assert_equal "has already been taken", item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_multiple
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal "has already been taken for uniq_2/uniq_3", invalid_item.errors[:uniq_1]
+ end
+
+ def test_validate_uniqueness_generic
+ ActiveRecord::Base.connection.expects(:index_for_record_not_unique).returns(nil)
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal "Unique requirement not met", invalid_item.errors[:base]
+ end
+
+end
\ No newline at end of file
diff --git a/activerecord/test/models/unique_item.rb b/activerecord/test/models/unique_item.rb
new file mode 100644
index 0000000..3c6f6f0
--- /dev/null
+++ b/activerecord/test/models/unique_item.rb
@@ -0,0 +1,2 @@
+class UniqueItem < ActiveRecord::Base
+end
\ No newline at end of file
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 2e5299c..acd1f02 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -536,6 +536,21 @@ ActiveRecord::Schema.define do
t.string :title
end
+ create_table :unique_items, :force => true do |t|
+ t.string :uniq
+ t.string :uniq_1
+ t.string :uniq_2
+ t.string :uniq_3
+ t.string :uniq_named
+ t.string :uniq_1_named
+ t.string :uniq_2_named
+ t.string :uniq_3_named
+ end
+ add_index :unique_items, [:uniq], :unique => true
+ add_index :unique_items, [:uniq_named], :unique => true, :name => 'unique_named_single_index'
+ add_index :unique_items, [:uniq_1, :uniq_2, :uniq_3], :unique => true
+ add_index :unique_items, [:uniq_1_named, :uniq_2_named, :uniq_3_named], :unique => true, :name => 'unique_named_multiple_index'
+
except 'SQLite' do
# fk_test_has_fk should be before fk_test_has_pk
create_table :fk_test_has_fk, :force => true do |t|
--
1.7.2.1
 
 
From 14feddfdda9a7fd3709c099a458bd4f8e81aea59 Mon Sep 17 00:00:00 2001
From: Jordan Brough <jordan@animoto.com>
Date: Fri, 15 Jan 2010 15:10:55 -0700
Subject: [PATCH 5/5] Add bang method handling to db-enforced unique constraints
 
---
.../lib/active_record/unique_constraints.rb | 30 ++++++++++++++-----
activerecord/test/cases/unique_constraints_test.rb | 30 ++++++++++++++++++++
2 files changed, 52 insertions(+), 8 deletions(-)
 
diff --git a/activerecord/lib/active_record/unique_constraints.rb b/activerecord/lib/active_record/unique_constraints.rb
index 6702e37..b3c1234 100644
--- a/activerecord/lib/active_record/unique_constraints.rb
+++ b/activerecord/lib/active_record/unique_constraints.rb
@@ -4,6 +4,7 @@ module ActiveRecord
def self.included(base)
base.class_eval do
alias_method_chain :save, :unique_constraints
+ alias_method_chain :save!, :unique_constraints
end
end
@@ -16,15 +17,28 @@ module ActiveRecord
cache_indexes
save_without_unique_constraints(perform_validation)
rescue ActiveRecord::RecordNotUnique => e
- index = connection.index_for_record_not_unique(e.original_exception, self.class.table_name)
- if !index
- errors.add_to_base(I18n.translate(:'activerecord.errors.messages.taken_generic'))
- elsif index.columns.size == 1
- errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
- else
- errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
- end
+ parse_unique_exception(e)
false
end
+
+ def save_with_unique_constraints! #:nodoc:
+ cache_indexes
+ save_without_unique_constraints!
+ rescue ActiveRecord::RecordNotUnique => e
+ parse_unique_exception(e)
+ raise RecordInvalid.new(self)
+ end
+
+ protected
+ def parse_unique_exception(e)
+ index = connection.index_for_record_not_unique(e.original_exception, self.class.table_name)
+ if !index
+ errors.add_to_base(I18n.translate(:'activerecord.errors.messages.taken_generic'))
+ elsif index.columns.size == 1
+ errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
+ else
+ errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
+ end
+ end
end
end
\ No newline at end of file
diff --git a/activerecord/test/cases/unique_constraints_test.rb b/activerecord/test/cases/unique_constraints_test.rb
index 2b5d0ee..8adae04 100644
--- a/activerecord/test/cases/unique_constraints_test.rb
+++ b/activerecord/test/cases/unique_constraints_test.rb
@@ -9,6 +9,12 @@ class UniquenessValidationTest < ActiveRecord::TestCase
assert_equal "has already been taken", invalid_item.errors[:uniq]
end
+ def test_validate_uniqueness_on_create!
+ valid_item = UniqueItem.create!(:uniq => 'abc')
+ e = get_unique_exception { UniqueItem.create!(:uniq => 'abc') }
+ assert_equal "has already been taken", e.record.errors[:uniq]
+ end
+
def test_validate_uniqueness_on_save
item_1 = UniqueItem.create!(:uniq => 'abc')
item_2 = UniqueItem.create!(:uniq => 'xyz')
@@ -17,6 +23,14 @@ class UniquenessValidationTest < ActiveRecord::TestCase
assert_equal "has already been taken", item_2.errors[:uniq]
end
+ def test_validate_uniqueness_on_save!
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.uniq = item_1.uniq
+ e = get_unique_exception { item_2.save! }
+ assert_equal("has already been taken", e.record.errors[:uniq])
+ end
+
def test_validate_uniqueness_on_update
item_1 = UniqueItem.create!(:uniq => 'abc')
item_2 = UniqueItem.create!(:uniq => 'xyz')
@@ -24,6 +38,13 @@ class UniquenessValidationTest < ActiveRecord::TestCase
assert_equal "has already been taken", item_2.errors[:uniq]
end
+ def test_validate_uniqueness_on_update!
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ e = get_unique_exception { item_2.update_attributes!(:uniq => item_1.uniq) }
+ assert_equal("has already been taken", e.record.errors[:uniq])
+ end
+
def test_validate_uniqueness_multiple
valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
@@ -37,4 +58,13 @@ class UniquenessValidationTest < ActiveRecord::TestCase
assert_equal "Unique requirement not met", invalid_item.errors[:base]
end
+ protected
+ def get_unique_exception
+ yield
+ rescue ActiveRecord::RecordInvalid => e
+ e
+ else
+ flunk "Expected ActiveRecord::RecordInvalid exception"
+ end
+
end
\ No newline at end of file
--
1.7.2.1
unique-patch-master.diff
Diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299
From 5055887efe3f3d560003d14599e8c048059fa830 Mon Sep 17 00:00:00 2001
From: Jordan Brough <jordan@animoto.com>
Date: Tue, 22 Dec 2009 07:42:48 -0700
Subject: [PATCH] Friendly ActiveRecord error handling for db-enforced unique constraints
 
---
activerecord/lib/active_record.rb | 1 +
activerecord/lib/active_record/base.rb | 4 ++
.../connection_adapters/abstract_adapter.rb | 7 +++
.../connection_adapters/mysql_adapter.rb | 12 ++++++
.../connection_adapters/postgresql_adapter.rb | 7 +++
.../connection_adapters/sqlite_adapter.rb | 7 +++
activerecord/lib/active_record/locale/en.yml | 2 +
.../lib/active_record/unique_constraints.rb | 24 ++++++++++++
activerecord/test/cases/adapter_test.rb | 35 +++++++++++++++++
activerecord/test/cases/unique_constraints_test.rb | 40 ++++++++++++++++++++
activerecord/test/models/unique_item.rb | 2 +
activerecord/test/schema/schema.rb | 15 +++++++
12 files changed, 156 insertions(+), 0 deletions(-)
create mode 100644 activerecord/lib/active_record/unique_constraints.rb
create mode 100644 activerecord/test/cases/unique_constraints_test.rb
create mode 100644 activerecord/test/models/unique_item.rb
 
diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb
index 2376bbd..f3cff35 100644
--- a/activerecord/lib/active_record.rb
+++ b/activerecord/lib/active_record.rb
@@ -68,6 +68,7 @@ module ActiveRecord
autoload :Timestamp
autoload :Transactions
autoload :Types
+ autoload :UniqueConstraints
autoload :Validations
module AttributeMethods
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 321bba4..da52ec1 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -3117,6 +3117,10 @@ module ActiveRecord #:nodoc:
include Aggregations, Transactions, Reflection, Batches, Calculations, Serialization
+ # UniqueConstraints needs to be included after Transactions, because we need #save_with_unique_constraints
+ # to rescue after the transaction has been resolved for postgres.
+ include UniqueConstraints
+
end
end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 8fae26b..832b962 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -198,6 +198,13 @@ module ActiveRecord
end
end
+ # given an ActiveRecord::RecordNotUnique exception and a table name this returns the index whose
+ # unique constraint was violated, or nil if it can't determine the index
+ def index_for_record_not_unique(not_unique_exception, table_name) #:nodoc:
+ # override in derived class
+ nil
+ end
+
protected
def log(sql, name, &block)
ActiveSupport::Notifications.instrument(:sql, :sql => sql, :name => name, &block)
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index fa28bc6..d83e3d0 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -576,6 +576,18 @@ module ActiveRecord
where_sql
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ case exception.message
+ when /Duplicate entry.*for key (\d+)/
+ index_position = $1.to_i
+ # minus two b/c mysql message is one-based + rails excludes primary key index from indexes list
+ indexes(table_name)[index_position - 2]
+ when /Duplicate entry.*for key '(\w+)'/
+ index_name = $1
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
def translate_exception(exception, message)
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 1d52c5e..70bbbd8 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -929,6 +929,13 @@ module ActiveRecord
sql << order_columns * ', '
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/unique constraint "(\w+)"/)
+ index_name = match[1]
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
# Returns the version of the connected PostgreSQL version.
def postgresql_version
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index c9c2892..72dc049 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -294,6 +294,13 @@ module ActiveRecord
"VALUES(NULL)"
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/column(?:s)? (.*) (?:is|are) not unique/)
+ index_columns = match[1].split(', ')
+ indexes(table_name).detect { |i| i.columns == index_columns }
+ end
+ end
+
protected
def select(sql, name = nil) #:nodoc:
execute(sql, name).map do |row|
diff --git a/activerecord/lib/active_record/locale/en.yml b/activerecord/lib/active_record/locale/en.yml
index 092f5f0..8144735 100644
--- a/activerecord/lib/active_record/locale/en.yml
+++ b/activerecord/lib/active_record/locale/en.yml
@@ -15,6 +15,8 @@ en:
too_short: "is too short (minimum is {{count}} characters)"
wrong_length: "is the wrong length (should be {{count}} characters)"
taken: "has already been taken"
+ taken_multiple: "has already been taken for {{context}}"
+ taken_generic: "Unique requirement not met"
not_a_number: "is not a number"
greater_than: "must be greater than {{count}}"
greater_than_or_equal_to: "must be greater than or equal to {{count}}"
diff --git a/activerecord/lib/active_record/unique_constraints.rb b/activerecord/lib/active_record/unique_constraints.rb
new file mode 100644
index 0000000..7a17b6a
--- /dev/null
+++ b/activerecord/lib/active_record/unique_constraints.rb
@@ -0,0 +1,24 @@
+module ActiveRecord
+ # See ActiveRecord::Transactions::ClassMethods for documentation.
+ module UniqueConstraints
+ def self.included(base)
+ base.class_eval do
+ alias_method_chain :save, :unique_constraints
+ end
+ end
+
+ def save_with_unique_constraints(perform_validation = true) #:nodoc:
+ save_without_unique_constraints(perform_validation)
+ rescue ActiveRecord::RecordNotUnique => e
+ index = connection.index_for_record_not_unique(e, self.class.table_name)
+ if !index
+ errors[:base] << I18n.translate(:'activerecord.errors.messages.taken_generic')
+ elsif index.columns.size == 1
+ errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
+ else
+ errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
+ end
+ false
+ end
+ end
+end
\ No newline at end of file
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index c59be26..4c8f58b 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -142,4 +142,39 @@ class AdapterTest < ActiveRecord::TestCase
end
end
end
+
+ # tests for db-enforced unique constraints
+ unique_scenarios = {
+ :single => {:sql => "INSERT INTO unique_items(uniq) VALUES('abc')",
+ :expected => ['uniq']},
+ :single_named => {:sql => "INSERT INTO unique_items(uniq_named) VALUES('abc')",
+ :expected => ['uniq_named']},
+ :multiple => {:sql => "INSERT INTO unique_items(uniq_1, uniq_2, uniq_3) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1', 'uniq_2', 'uniq_3']},
+ :multiple_named => {:sql => "INSERT INTO unique_items(uniq_1_named, uniq_2_named, uniq_3_named) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1_named', 'uniq_2_named', 'uniq_3_named']}
+ }
+ unique_scenarios.each do |name, data|
+ test "finds index for unique constraint #{name}" do
+ @connection.execute data[:sql]
+ exception = get_exception(ActiveRecord::RecordNotUnique) do
+ @connection.execute data[:sql]
+ end
+ # postgres can't query indexes until transaction is rolled back
+ @connection.rollback_db_transaction if current_adapter?(:PostgreSQLAdapter)
+ assert_equal data[:expected], @connection.index_for_record_not_unique(exception, 'unique_items').columns
+ end
+ end
+
+ protected
+
+ def get_exception(expected_exception)
+ begin
+ yield
+ rescue expected_exception => e
+ e
+ else
+ flunk "Expected #{expected_exception} exception"
+ end
+ end
end
diff --git a/activerecord/test/cases/unique_constraints_test.rb b/activerecord/test/cases/unique_constraints_test.rb
new file mode 100644
index 0000000..92ba6d8
--- /dev/null
+++ b/activerecord/test/cases/unique_constraints_test.rb
@@ -0,0 +1,40 @@
+require "cases/helper"
+require 'models/unique_item'
+
+class UniquenessValidationTest < ActiveRecord::TestCase
+
+ def test_validate_uniqueness_on_create
+ valid_item = UniqueItem.create!(:uniq => 'abc')
+ invalid_item = UniqueItem.create(:uniq => 'abc')
+ assert_equal ["has already been taken"], invalid_item.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_save
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.uniq = item_1.uniq
+ item_2.save
+ assert_equal ["has already been taken"], item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_update
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.update_attributes(:uniq => item_1.uniq)
+ assert_equal ["has already been taken"], item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_multiple
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal ["has already been taken for uniq_2/uniq_3"], invalid_item.errors[:uniq_1]
+ end
+
+ def test_validate_uniqueness_generic
+ ActiveRecord::Base.connection.expects(:index_for_record_not_unique).returns(nil)
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal ["Unique requirement not met"], invalid_item.errors[:base]
+ end
+
+end
\ No newline at end of file
diff --git a/activerecord/test/models/unique_item.rb b/activerecord/test/models/unique_item.rb
new file mode 100644
index 0000000..3c6f6f0
--- /dev/null
+++ b/activerecord/test/models/unique_item.rb
@@ -0,0 +1,2 @@
+class UniqueItem < ActiveRecord::Base
+end
\ No newline at end of file
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 0dd9da4..1aea762 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -532,6 +532,21 @@ ActiveRecord::Schema.define do
t.string :title
end
+ create_table :unique_items, :force => true do |t|
+ t.string :uniq
+ t.string :uniq_1
+ t.string :uniq_2
+ t.string :uniq_3
+ t.string :uniq_named
+ t.string :uniq_1_named
+ t.string :uniq_2_named
+ t.string :uniq_3_named
+ end
+ add_index :unique_items, [:uniq], :unique => true
+ add_index :unique_items, [:uniq_named], :unique => true, :name => 'unique_named_single_index'
+ add_index :unique_items, [:uniq_1, :uniq_2, :uniq_3], :unique => true
+ add_index :unique_items, [:uniq_1_named, :uniq_2_named, :uniq_3_named], :unique => true, :name => 'unique_named_multiple_index'
+
except 'SQLite' do
# fk_test_has_fk should be before fk_test_has_pk
create_table :fk_test_has_fk, :force => true do |t|
--
1.6.1.2
unique-patch-master.patch
Diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 342 343 344 345 346 347 348 349 350 351 352 353 354 355 356 357 358 359 360 361 362 363 364 365 366 367 368 369 370 371 372 373 374 375 376 377 378 379 380 381 382 383 384 385 386 387 388 389 390 391 392 393 394 395 396 397 398 399 400 401 402 403 404 405 406 407 408 409 410 411 412 413 414 415 416 417 418 419 420 421 422 423 424 425 426 427 428 429 430 431 432 433 434 435 436 437 438 439 440 441 442 443 444 445 446 447 448 449 450 451 452 453 454 455 456 457 458 459 460 461
From 9e300598ee86d318d5312478d398f2ebdd1db2c0 Mon Sep 17 00:00:00 2001
From: Jordan Brough <jordan@animoto.com>
Date: Tue, 9 Feb 2010 13:13:01 -0700
Subject: [PATCH 1/2] Friendly ActiveRecord error handling for db-enforced unique constraints
 
---
activerecord/lib/active_record.rb | 1 +
activerecord/lib/active_record/base.rb | 1 +
.../connection_adapters/abstract_adapter.rb | 7 +++
.../connection_adapters/mysql2_adapter.rb | 12 ++++++
.../connection_adapters/mysql_adapter.rb | 12 ++++++
.../connection_adapters/postgresql_adapter.rb | 13 ++++++
.../connection_adapters/sqlite_adapter.rb | 7 +++
activerecord/lib/active_record/locale/en.yml | 2 +
.../lib/active_record/unique_constraints.rb | 29 ++++++++++++++
activerecord/test/cases/adapter_test.rb | 35 +++++++++++++++++
activerecord/test/cases/unique_constraints_test.rb | 40 ++++++++++++++++++++
activerecord/test/models/unique_item.rb | 2 +
activerecord/test/schema/schema.rb | 15 +++++++
13 files changed, 176 insertions(+), 0 deletions(-)
create mode 100644 activerecord/lib/active_record/unique_constraints.rb
create mode 100644 activerecord/test/cases/unique_constraints_test.rb
create mode 100644 activerecord/test/models/unique_item.rb
 
diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb
index 5afb978..869c90b 100644
--- a/activerecord/lib/active_record.rb
+++ b/activerecord/lib/active_record.rb
@@ -78,6 +78,7 @@ module ActiveRecord
autoload :SessionStore
autoload :Timestamp
autoload :Transactions
+ autoload :UniqueConstraints
autoload :Validations
end
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index effb17b..9909663 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -1940,6 +1940,7 @@ MSG
# #save_with_autosave_associations to be wrapped inside a transaction.
include AutosaveAssociation, NestedAttributes
include Aggregations, Transactions, Reflection, Serialization
+ include UniqueConstraints
NilClass.add_whiner(self) if NilClass.respond_to?(:add_whiner)
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 3a3a73f..43ffa46 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -207,6 +207,13 @@ module ActiveRecord
"active_record_#{open_transactions}"
end
+ # given an ActiveRecord::RecordNotUnique exception and a table name this returns the index whose
+ # unique constraint was violated, or nil if it can't determine the index
+ def index_for_record_not_unique(not_unique_exception, table_name) #:nodoc:
+ # override in derived class
+ nil
+ end
+
protected
def log(sql, name = "SQL")
diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
index acf1832..47da0da 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
@@ -536,6 +536,18 @@ module ActiveRecord
where_sql
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ case exception.message
+ when /Duplicate entry.*for key (\d+)/
+ index_position = $1.to_i
+ # minus two b/c mysql message is one-based + rails excludes primary key index from indexes list
+ indexes(table_name)[index_position - 2]
+ when /Duplicate entry.*for key '(\w+)'/
+ index_name = $1
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
def quoted_columns_for_index(column_names, options = {})
length = options[:length] if options.is_a?(Hash)
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index cdf1ebf..7c89aef 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -646,6 +646,18 @@ module ActiveRecord
where_sql
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ case exception.message
+ when /Duplicate entry.*for key (\d+)/
+ index_position = $1.to_i
+ # minus two b/c mysql message is one-based + rails excludes primary key index from indexes list
+ indexes(table_name)[index_position - 2]
+ when /Duplicate entry.*for key '(\w+)'/
+ index_name = $1
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
def quoted_columns_for_index(column_names, options = {})
length = options[:length] if options.is_a?(Hash)
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 46c0f3f..bea174b 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -708,6 +708,12 @@ module ActiveRecord
end.compact
end
+ # postgres doesn't let us query this in the middle of an aborted transaction so we sometimes need to cache beforehand
+ def cached_indexes(table_name) #:nodoc:
+ @cached_indexes ||= {}
+ @cached_indexes[table_name] ||= indexes(table_name)
+ end
+
# Returns the list of all column definitions for a table.
def columns(table_name, name = nil)
# Limit, precision, and scale are all handled by the superclass.
@@ -944,6 +950,13 @@ module ActiveRecord
sql << order_columns * ', '
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/unique constraint "(\w+)"/)
+ index_name = match[1]
+ cached_indexes(table_name).detect { |i| i.name == index_name }
+ end
+ end
+
protected
# Returns the version of the connected PostgreSQL version.
def postgresql_version
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index f650a1b..02eebd8 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -340,6 +340,13 @@ module ActiveRecord
"VALUES(NULL)"
end
+ def index_for_record_not_unique(exception, table_name) #:nodoc:
+ if match = exception.message.match(/column(?:s)? (.*) (?:is|are) not unique/)
+ index_columns = match[1].split(', ')
+ indexes(table_name).detect { |i| i.columns == index_columns }
+ end
+ end
+
protected
def select(sql, name = nil, binds = []) #:nodoc:
exec_query(sql, name, binds).to_a
diff --git a/activerecord/lib/active_record/locale/en.yml b/activerecord/lib/active_record/locale/en.yml
index 44328f6..e310fba 100644
--- a/activerecord/lib/active_record/locale/en.yml
+++ b/activerecord/lib/active_record/locale/en.yml
@@ -9,6 +9,8 @@ en:
errors:
messages:
taken: "has already been taken"
+ taken_multiple: "has already been taken for %{context}"
+ taken_generic: "Unique requirement not met"
record_invalid: "Validation failed: %{errors}"
# Append your own errors here or at the model/attributes scope.
diff --git a/activerecord/lib/active_record/unique_constraints.rb b/activerecord/lib/active_record/unique_constraints.rb
new file mode 100644
index 0000000..4a6d748
--- /dev/null
+++ b/activerecord/lib/active_record/unique_constraints.rb
@@ -0,0 +1,29 @@
+module ActiveRecord
+ module UniqueConstraints
+ extend ActiveSupport::Concern
+
+ included do
+ alias_method_chain :save, :unique_constraints
+ end
+
+ # work around postgres not allowing us to query indexes in the middle of an aborted transaction
+ def cache_indexes #:nodoc:
+ connection.cached_indexes(self.class.table_name) if connection.respond_to?(:cached_indexes)
+ end
+
+ def save_with_unique_constraints(*args) #:nodoc:
+ cache_indexes
+ save_without_unique_constraints(*args)
+ rescue ActiveRecord::RecordNotUnique => e
+ index = connection.index_for_record_not_unique(e, self.class.table_name)
+ if !index
+ errors[:base] << I18n.t(:'activerecord.errors.messages.taken_generic')
+ elsif index.columns.size == 1
+ errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
+ else
+ errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
+ end
+ false
+ end
+ end
+end
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 49b2e94..1cb8b0a 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -141,4 +141,39 @@ class AdapterTest < ActiveRecord::TestCase
end
end
end
+
+ # tests for db-enforced unique constraints
+ unique_scenarios = {
+ :single => {:sql => "INSERT INTO unique_items(uniq) VALUES('abc')",
+ :expected => ['uniq']},
+ :single_named => {:sql => "INSERT INTO unique_items(uniq_named) VALUES('abc')",
+ :expected => ['uniq_named']},
+ :multiple => {:sql => "INSERT INTO unique_items(uniq_1, uniq_2, uniq_3) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1', 'uniq_2', 'uniq_3']},
+ :multiple_named => {:sql => "INSERT INTO unique_items(uniq_1_named, uniq_2_named, uniq_3_named) VALUES('a', 'b', 'c')",
+ :expected => ['uniq_1_named', 'uniq_2_named', 'uniq_3_named']}
+ }
+ unique_scenarios.each do |name, data|
+ test "finds index for unique constraint #{name}" do
+ @connection.execute data[:sql]
+ exception = get_exception(ActiveRecord::RecordNotUnique) do
+ @connection.execute data[:sql]
+ end
+ # postgres can't query indexes until transaction is rolled back
+ @connection.rollback_db_transaction if current_adapter?(:PostgreSQLAdapter)
+ assert_equal data[:expected], @connection.index_for_record_not_unique(exception, 'unique_items').columns
+ end
+ end
+
+ protected
+
+ def get_exception(expected_exception)
+ begin
+ yield
+ rescue expected_exception => e
+ e
+ else
+ flunk "Expected #{expected_exception} exception"
+ end
+ end
end
diff --git a/activerecord/test/cases/unique_constraints_test.rb b/activerecord/test/cases/unique_constraints_test.rb
new file mode 100644
index 0000000..18d1890
--- /dev/null
+++ b/activerecord/test/cases/unique_constraints_test.rb
@@ -0,0 +1,40 @@
+require "cases/helper"
+require 'models/unique_item'
+
+class UniquenessValidationTest < ActiveRecord::TestCase
+
+ def test_validate_uniqueness_on_create
+ valid_item = UniqueItem.create!(:uniq => 'abc')
+ invalid_item = UniqueItem.create(:uniq => 'abc')
+ assert_equal ["has already been taken"], invalid_item.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_save
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.uniq = item_1.uniq
+ item_2.save
+ assert_equal ["has already been taken"], item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_on_update
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.update_attributes(:uniq => item_1.uniq)
+ assert_equal ["has already been taken"], item_2.errors[:uniq]
+ end
+
+ def test_validate_uniqueness_multiple
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal ["has already been taken for uniq_2/uniq_3"], invalid_item.errors[:uniq_1]
+ end
+
+ def test_validate_uniqueness_generic
+ ActiveRecord::Base.connection.expects(:index_for_record_not_unique).returns(nil)
+ valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
+ assert_equal ["Unique requirement not met"], invalid_item.errors[:base]
+ end
+
+end
diff --git a/activerecord/test/models/unique_item.rb b/activerecord/test/models/unique_item.rb
new file mode 100644
index 0000000..e805cb8
--- /dev/null
+++ b/activerecord/test/models/unique_item.rb
@@ -0,0 +1,2 @@
+class UniqueItem < ActiveRecord::Base
+end
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 326c336..d408154 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -668,6 +668,21 @@ ActiveRecord::Schema.define do
t.string :name
end
+ create_table :unique_items, :force => true do |t|
+ t.string :uniq
+ t.string :uniq_1
+ t.string :uniq_2
+ t.string :uniq_3
+ t.string :uniq_named
+ t.string :uniq_1_named
+ t.string :uniq_2_named
+ t.string :uniq_3_named
+ end
+ add_index :unique_items, [:uniq], :unique => true
+ add_index :unique_items, [:uniq_named], :unique => true, :name => 'unique_named_single_index'
+ add_index :unique_items, [:uniq_1, :uniq_2, :uniq_3], :unique => true
+ add_index :unique_items, [:uniq_1_named, :uniq_2_named, :uniq_3_named], :unique => true, :name => 'unique_named_multiple_index'
+
except 'SQLite' do
# fk_test_has_fk should be before fk_test_has_pk
create_table :fk_test_has_fk, :force => true do |t|
--
1.7.2.1
 
 
From fb06e862fc2cb5d48717d6af948466fb40cb002d Mon Sep 17 00:00:00 2001
From: Jordan Brough <jordan@animoto.com>
Date: Tue, 9 Feb 2010 13:50:35 -0700
Subject: [PATCH 2/2] Handling for db-enforced unique constraints in bang methods
 
---
.../lib/active_record/unique_constraints.rb | 33 ++++++++++++++-----
activerecord/test/cases/unique_constraints_test.rb | 30 ++++++++++++++++++
2 files changed, 54 insertions(+), 9 deletions(-)
 
diff --git a/activerecord/lib/active_record/unique_constraints.rb b/activerecord/lib/active_record/unique_constraints.rb
index 4a6d748..cb7a665 100644
--- a/activerecord/lib/active_record/unique_constraints.rb
+++ b/activerecord/lib/active_record/unique_constraints.rb
@@ -3,7 +3,9 @@ module ActiveRecord
extend ActiveSupport::Concern
included do
- alias_method_chain :save, :unique_constraints
+ [:save, :save!].each do |method|
+ alias_method_chain method, :unique_constraints
+ end
end
# work around postgres not allowing us to query indexes in the middle of an aborted transaction
@@ -15,15 +17,28 @@ module ActiveRecord
cache_indexes
save_without_unique_constraints(*args)
rescue ActiveRecord::RecordNotUnique => e
- index = connection.index_for_record_not_unique(e, self.class.table_name)
- if !index
- errors[:base] << I18n.t(:'activerecord.errors.messages.taken_generic')
- elsif index.columns.size == 1
- errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
- else
- errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
- end
+ parse_unique_exception(e)
false
end
+
+ def save_with_unique_constraints! #:nodoc:
+ cache_indexes
+ save_without_unique_constraints!
+ rescue ActiveRecord::RecordNotUnique => e
+ parse_unique_exception(e)
+ raise RecordInvalid.new(self)
+ end
+
+ protected
+ def parse_unique_exception(e)
+ index = connection.index_for_record_not_unique(e, self.class.table_name)
+ if !index
+ errors[:base] << I18n.translate(:'activerecord.errors.messages.taken_generic')
+ elsif index.columns.size == 1
+ errors.add(index.columns.first, :taken, :value => attributes[index.columns.first])
+ else
+ errors.add(index.columns.first, :taken_multiple, :context => index.columns.slice(1..-1).join('/'), :value => attributes[index.columns.first])
+ end
+ end
end
end
diff --git a/activerecord/test/cases/unique_constraints_test.rb b/activerecord/test/cases/unique_constraints_test.rb
index 18d1890..d4489fc 100644
--- a/activerecord/test/cases/unique_constraints_test.rb
+++ b/activerecord/test/cases/unique_constraints_test.rb
@@ -9,6 +9,12 @@ class UniquenessValidationTest < ActiveRecord::TestCase
assert_equal ["has already been taken"], invalid_item.errors[:uniq]
end
+ def test_validate_uniqueness_on_create!
+ valid_item = UniqueItem.create!(:uniq => 'abc')
+ e = get_unique_exception { UniqueItem.create!(:uniq => 'abc') }
+ assert_equal ["has already been taken"], e.record.errors[:uniq]
+ end
+
def test_validate_uniqueness_on_save
item_1 = UniqueItem.create!(:uniq => 'abc')
item_2 = UniqueItem.create!(:uniq => 'xyz')
@@ -17,6 +23,14 @@ class UniquenessValidationTest < ActiveRecord::TestCase
assert_equal ["has already been taken"], item_2.errors[:uniq]
end
+ def test_validate_uniqueness_on_save!
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ item_2.uniq = item_1.uniq
+ e = get_unique_exception { item_2.save! }
+ assert_equal(["has already been taken"], e.record.errors[:uniq])
+ end
+
def test_validate_uniqueness_on_update
item_1 = UniqueItem.create!(:uniq => 'abc')
item_2 = UniqueItem.create!(:uniq => 'xyz')
@@ -24,6 +38,13 @@ class UniquenessValidationTest < ActiveRecord::TestCase
assert_equal ["has already been taken"], item_2.errors[:uniq]
end
+ def test_validate_uniqueness_on_update!
+ item_1 = UniqueItem.create!(:uniq => 'abc')
+ item_2 = UniqueItem.create!(:uniq => 'xyz')
+ e = get_unique_exception { item_2.update_attributes!(:uniq => item_1.uniq) }
+ assert_equal(["has already been taken"], e.record.errors[:uniq])
+ end
+
def test_validate_uniqueness_multiple
valid_item = UniqueItem.create!(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
invalid_item = UniqueItem.create(:uniq_1 => 'a', :uniq_2 => 'b', :uniq_3 => 'c')
@@ -37,4 +58,13 @@ class UniquenessValidationTest < ActiveRecord::TestCase
assert_equal ["Unique requirement not met"], invalid_item.errors[:base]
end
+ protected
+ def get_unique_exception
+ yield
+ rescue ActiveRecord::RecordInvalid => e
+ e
+ else
+ flunk "Expected ActiveRecord::RecordInvalid exception"
+ end
+
end
--
1.7.2.1

Please sign in to comment on this gist.

Something went wrong with that request. Please try again.