diff --git a/lib/immigrant/key_validator.rb b/lib/immigrant/key_validator.rb index fd4492f..8adad2d 100644 --- a/lib/immigrant/key_validator.rb +++ b/lib/immigrant/key_validator.rb @@ -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 diff --git a/test/immigrant_test.rb b/test/immigrant_test.rb index b7fc565..9f88704 100644 --- a/test/immigrant_test.rb +++ b/test/immigrant_test.rb @@ -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| @@ -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