Skip to content

Commit

Permalink
don't try to generate foreign keys for fake primary keys, fixes #19
Browse files Browse the repository at this point in the history
if a :primary_key isn't actually unique (or is insufficiently unique),
don't try to create a key, since it will fail.
  • Loading branch information
jenseng committed May 18, 2015
1 parent 4e4518c commit e85a9dc
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 2 deletions.
5 changes: 5 additions & 0 deletions lib/immigrant/key_validator.rb
Expand Up @@ -6,11 +6,16 @@ def valid?(key)
return false unless tables.include?(key.to_table)
return false unless column_exists?(key.from_table, key.options[:column])
return false unless column_exists?(key.to_table, key.options[:primary_key])
return false unless key.options[:primary_key] == "id" || has_unique_index?(key.to_table, key.options[:primary_key])
true
end

private

def has_unique_index?(table_name, column_name)
connection.indexes(table_name).any? { |index| index.columns == [column_name] && index.unique && index.where.nil? }
end

def column_exists?(table_name, column_name)
columns_for(table_name).any? { |column| column.name == column_name }
end
Expand Down
29 changes: 27 additions & 2 deletions test/immigrant_test.rb
Expand Up @@ -41,16 +41,21 @@ def tables
ActiveSupport::DescendantsTracker.direct_descendants(ActiveRecord::Base).map(&:table_name)
end
def columns(table_name)
AnyColumn.new
AnyCollection.new
end
def indexes(table_name)
AnyCollection.new
end
end

class AnyColumn
class AnyCollection
def any?
true
end
end

Index = ActiveRecord::ConnectionAdapters::IndexDefinition

def teardown
subclasses = ActiveSupport::DescendantsTracker.direct_descendants(ActiveRecord::Base)
subclasses.each do |subclass|
Expand Down Expand Up @@ -455,6 +460,26 @@ class Widget < ActiveRecord::Base
end
end

test 'insufficiently unique "primary" keys should not generate a foreign key' do
given <<-CODE
class Setting < ActiveRecord::Base; end
class SettingValue < ActiveRecord::Base
belongs_to :setting,
:conditions => { :deleted_at => nil },
foreign_key: :name, primary_key: :name
end
CODE

# emulate `"index_settings_on_name" UNIQUE, btree (name) WHERE deleted_at IS NULL`
# it's unique, but not enough for a foreign key just on name
indexes = [Index.new("settings", ["name"], true, "deleted_at IS NULL")]

MockConnection.stub_any_instance(:indexes, indexes) do
assert_equal([], infer_keys)
end
end

test 'ignore_keys should be respected' do
given <<-CODE
class User < ActiveRecord::Base; end
Expand Down

0 comments on commit e85a9dc

Please sign in to comment.