fk: use random digest names

The name of the foreign key is not relevant from a users perspective.
Using random names resolves the urge to rename the foreign key when the
respective table or column is renamed.
This commit is contained in:
Yves Senn 2014-06-20 11:54:17 +02:00
parent 8550ba307d
commit 8768305f20
8 changed files with 59 additions and 33 deletions

View File

@ -30,7 +30,7 @@ FOREIGN KEY (#{quote_column_name(o.column)})
end
def visit_DropForeignKey(name)
"DROP CONSTRAINT #{name}"
"DROP CONSTRAINT #{quote_column_name(name)}"
end
private

View File

@ -35,7 +35,7 @@ module ActiveRecord
end
def primary_key
options[:primary_key]
options[:primary_key] || default_primary_key
end
def on_delete
@ -45,6 +45,15 @@ module ActiveRecord
def on_update
options[:on_update]
end
def custom_primary_key?
options[:primary_key] != default_primary_key
end
private
def default_primary_key
"id"
end
end
# Represents the schema of an SQL table in an abstract way. This class

View File

@ -650,11 +650,10 @@ module ActiveRecord
return unless supports_foreign_keys?
options[:column] ||= foreign_key_column_for(to_table)
primary_key = options.fetch(:primary_key, "id")
options = {
column: options[:column],
primary_key: primary_key,
primary_key: options[:primary_key],
name: foreign_key_name(from_table, options),
on_delete: options[:on_delete],
on_update: options[:on_update]
@ -674,8 +673,17 @@ module ActiveRecord
options = { column: foreign_key_column_for(options_or_to_table) }
end
fk_name_to_delete = options.fetch(:name) do
fk_to_delete = foreign_keys(from_table).detect {|fk| fk.column == options[:column] }
if fk_to_delete
fk_to_delete.name
else
raise ArgumentError, "Table '#{from_table}' has no foreign key on column '#{options[:column]}'"
end
end
at = create_alter_table from_table
at.drop_foreign_key foreign_key_name(from_table, options)
at.drop_foreign_key fk_name_to_delete
execute schema_creation.accept at
end
@ -686,11 +694,7 @@ module ActiveRecord
def foreign_key_name(table_name, options) # :nodoc:
options.fetch(:name) do
identifier = "#{table_name}_#{options.fetch(:column)}_fk"
if identifier.length > allowed_index_name_length
raise ArgumentError, "Foreign key name '#{identifier}' is too long; the limit is #{allowed_index_name_length} characters"
end
identifier
"fk_rails_#{SecureRandom.hex(5)}"
end
end

View File

@ -226,10 +226,20 @@ HEADER
parts = [
'add_foreign_key ' + remove_prefix_and_suffix(foreign_key.from_table).inspect,
remove_prefix_and_suffix(foreign_key.to_table).inspect,
'column: ' + foreign_key.column.inspect,
'primary_key: ' + foreign_key.primary_key.inspect,
'name: ' + foreign_key.name.inspect
]
if foreign_key.column != @connection.foreign_key_column_for(foreign_key.to_table)
parts << ('column: ' + foreign_key.column.inspect)
end
if foreign_key.custom_primary_key?
parts << ('primary_key: ' + foreign_key.primary_key.inspect)
end
if foreign_key.name !~ /^fk_rails_[0-9a-f]{10}$/
parts << ('name: ' + foreign_key.name.inspect)
end
parts << ('on_update: ' + foreign_key.on_update.inspect) if foreign_key.on_update
parts << ('on_delete: ' + foreign_key.on_delete.inspect) if foreign_key.on_delete

View File

@ -42,7 +42,7 @@ module ActiveRecord
assert_equal "fk_test_has_fk", fk.from_table
assert_equal "fk_test_has_pk", fk.to_table
assert_equal "fk_id", fk.column
assert_equal "id", fk.primary_key
assert_equal "pk_id", fk.primary_key
assert_equal "fk_name", fk.name
end
@ -57,7 +57,7 @@ module ActiveRecord
assert_equal "rockets", fk.to_table
assert_equal "rocket_id", fk.column
assert_equal "id", fk.primary_key
assert_equal "astronauts_rocket_id_fk", fk.name
assert_match(/^fk_rails_.{10}$/, fk.name)
end
def test_add_foreign_key_with_column
@ -71,7 +71,7 @@ module ActiveRecord
assert_equal "rockets", fk.to_table
assert_equal "rocket_id", fk.column
assert_equal "id", fk.primary_key
assert_equal "astronauts_rocket_id_fk", fk.name
assert_match(/^fk_rails_.{10}$/, fk.name)
end
def test_add_foreign_key_with_non_standard_primary_key
@ -146,15 +146,6 @@ module ActiveRecord
assert_equal :nullify, fk.on_update
end
def test_add_foreign_key_with_too_long_identifier
with_example_table @connection, "long_table_name_will_result_in_a_long_foreign_key_name", "rocket_id integer" do
e = assert_raises(ArgumentError) do
@connection.add_foreign_key "long_table_name_will_result_in_a_long_foreign_key_name", "rockets"
end
assert_match(/^Foreign key name 'long_table_name_will_result_in_a_long_foreign_key_name_rocket_id_fk' is too long;/, e.message)
end
end
def test_remove_foreign_key_inferes_column
@connection.add_foreign_key :astronauts, :rockets
@ -179,9 +170,21 @@ module ActiveRecord
assert_equal [], @connection.foreign_keys("astronauts")
end
def test_remove_foreign_non_existing_foreign_key_raises
assert_raises ArgumentError do
@connection.remove_foreign_key :astronauts, :rockets
end
end
def test_schema_dumping
@connection.add_foreign_key :astronauts, :rockets
output = dump_table_schema "astronauts"
assert_match %r{\s+add_foreign_key "astronauts", "rockets"$}, output
end
def test_schema_dumping_with_options
output = dump_table_schema "fk_test_has_fk"
assert_match %r{\s+add_foreign_key "fk_test_has_fk", "fk_test_has_pk", column: "fk_id", primary_key: "id", name: "fk_name"$}, output
assert_match %r{\s+add_foreign_key "fk_test_has_fk", "fk_test_has_pk", column: "fk_id", primary_key: "pk_id", name: "fk_name"$}, output
end
def test_schema_dumping_on_delete_and_on_update_options
@ -205,7 +208,7 @@ module ActiveRecord
def test_add_foreign_key_is_reversible
migration = CreateCitiesAndHousesMigration.new
silence_stream($stdout) { migration.migrate(:up) }
assert_equal ["houses_city_id_fk"], @connection.foreign_keys("houses").map(&:name)
assert_equal 1, @connection.foreign_keys("houses").size
ensure
silence_stream($stdout) { migration.migrate(:down) }
end

View File

@ -1,2 +1,2 @@
first:
id: 1
pk_id: 1

View File

@ -855,10 +855,10 @@ ActiveRecord::Schema.define do
t.integer :fk_id, null: false
end
create_table :fk_test_has_pk, force: true do |t|
create_table :fk_test_has_pk, force: true, primary_key: "pk_id" do |t|
end
execute "ALTER TABLE fk_test_has_fk ADD CONSTRAINT fk_name FOREIGN KEY (#{quote_column_name 'fk_id'}) REFERENCES #{quote_table_name 'fk_test_has_pk'} (#{quote_column_name 'id'})"
execute "ALTER TABLE fk_test_has_fk ADD CONSTRAINT fk_name FOREIGN KEY (#{quote_column_name 'fk_id'}) REFERENCES #{quote_table_name 'fk_test_has_pk'} (#{quote_column_name 'pk_id'})"
execute "ALTER TABLE lessons_students ADD CONSTRAINT student_id_fk FOREIGN KEY (#{quote_column_name 'student_id'}) REFERENCES #{quote_table_name 'students'} (#{quote_column_name 'id'})"
end

View File

@ -7,7 +7,7 @@ ActiveRecord::Schema.define do
execute "DROP TABLE fk_test_has_pk" rescue nil
execute <<_SQL
CREATE TABLE 'fk_test_has_pk' (
'id' INTEGER NOT NULL PRIMARY KEY
'pk_id' INTEGER NOT NULL PRIMARY KEY
);
_SQL
@ -16,7 +16,7 @@ _SQL
'id' INTEGER NOT NULL PRIMARY KEY,
'fk_id' INTEGER NOT NULL,
FOREIGN KEY ('fk_id') REFERENCES 'fk_test_has_pk'('id')
FOREIGN KEY ('fk_id') REFERENCES 'fk_test_has_pk'('pk_id')
);
_SQL
end
end