Let t.foreign_key use the same to_table twice

[george-carlin]

I just discovered this issue with migrations in Rails 5 beta 2. Say I want to create a table with multiple foreign keys pointing to the same table:

def change
  create_table :flights do |t|
    t.integer :from_id, index: true, null: false
    t.integer :to_id,   index: true, null: false

    t.foreign_key :airports, column: :from_id
    t.foreign_key :airports, column: :to_id
  end
end

The problem is that this only creates the last foreign key (in this case the one on to_id) and ignores any previous foreign keys. From the source it's easy to see why:

def foreign_key(table_name, options = {}) # :nodoc:
  foreign_keys[table_name] = options
end

The hash keys are being re-used, so later definitions will overwrite previous ones.

This PR contains a simple fix, by turning foreign_keys[table_name] into an Array and allowing multiple definitions to be made for the same to_table.

[rails-bot]

r? @schneems

(@rails-bot has picked a reviewer for you, use r? to override)

[george-carlin]

CI has failed, but the error doesn't seem to have anything to do with the changes I made. Here's the failing job and the error is ClientTest#test_many_clients: ThreadError: queue empty in actioncable/test/client_test.rb. Can someone explain to me what's going on?

[kamipo]

I think @foreign_keys should be an array rather than a hash because foreign keys can be added to the same table.

--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -212,7 +212,7 @@ module ActiveRecord
       def initialize(name, temporary, options, as = nil)
         @columns_hash = {}
         @indexes = {}
-        @foreign_keys = {}
+        @foreign_keys = []
         @primary_keys = nil
         @temporary = temporary
         @options = options
@@ -330,7 +330,7 @@ module ActiveRecord
       end

       def foreign_key(table_name, options = {}) # :nodoc:
-        foreign_keys[table_name] = options
+        foreign_keys << [table_name, options]
       end

[senny]

Suggestion by @kamipo to turn @foreign_keys into an array seems reasonable. @georgemillo would you mind to update the PR accordingly?

[george-carlin]

Updated and rebased.

[kamipo]

to_table, options -> fk[0], fk[1] is reasonable?

[george-carlin]

You're right; the original syntax works. I didn't realise that you could pass two parameters to the block like that when iterating over a nested array.

Reverted and rebased.

[senny]

@georgemillo thanks. I merged a slightly modified version with a single assertion in the test-case. This combines all assertions and will give much more context when something goes wrong. 84c1824#diff-110b6f2d17622b19682ec73cce8b015fR160