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.

8 files changed, 59 insertions, 33 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb
index e495304376..5d13ee3633 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb
@@ -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
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
index 2e47e68754..785cfd9dbc 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -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
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
index 4da42717c1..e203767992 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -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
 
diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb
index 84ce725409..64bc68eefd 100644
--- a/activerecord/lib/active_record/schema_dumper.rb
+++ b/activerecord/lib/active_record/schema_dumper.rb
@@ -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
 
diff --git a/activerecord/test/cases/migration/foreign_key_test.rb b/activerecord/test/cases/migration/foreign_key_test.rb
index 6a24df076d..c985092b4c 100644
--- a/activerecord/test/cases/migration/foreign_key_test.rb
+++ b/activerecord/test/cases/migration/foreign_key_test.rb
@@ -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
diff --git a/activerecord/test/fixtures/fk_test_has_pk.yml b/activerecord/test/fixtures/fk_test_has_pk.yml
index c93952180b..73882bac41 100644
--- a/activerecord/test/fixtures/fk_test_has_pk.yml
+++ b/activerecord/test/fixtures/fk_test_has_pk.yml
@@ -1,2 +1,2 @@
 first:
-  id: 1
\ No newline at end of file
+  pk_id: 1
\ No newline at end of file
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 6ce82c71a8..03d33c151a 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -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
diff --git a/activerecord/test/schema/sqlite_specific_schema.rb b/activerecord/test/schema/sqlite_specific_schema.rb
index b7aff4f47d..b5552c2755 100644
--- a/activerecord/test/schema/sqlite_specific_schema.rb
+++ b/activerecord/test/schema/sqlite_specific_schema.rb
@@ -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
\ No newline at end of file
+end