Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsafe foreign keys with create_table #187

Closed
markbrown4 opened this issue Jun 27, 2022 · 1 comment
Closed

Unsafe foreign keys with create_table #187

markbrown4 opened this issue Jun 27, 2022 · 1 comment

Comments

@markbrown4
Copy link

markbrown4 commented Jun 27, 2022

It doesn't appear like adding foreign keys within create_table are detected as unsafe. e.g.

class Test < ActiveRecord::Migration[6.1]
  def change
    create_table :test do |t|
      t.references :user, foreign_key: true
    end
  end
end

As I understand it this would hold a lock on the users table.

I couldn't find a way to detect this with a custom check. This monkey patch seems to work but I'm wondering if this should make it's way into Strong Migrations itself or if there's a better way to achieve it.

# Prevent foreign keys being defined with t.references e.g.
# t.references :x, foreign_key: true
# t.references :x, foreign_key: {}

class ActiveRecord::ConnectionAdapters::ReferenceDefinition
  alias_method :__add_to__, :add_to
  def add_to(table)
    if foreign_key
      raise ArgumentError
    end
    __add_to__(table)
  end
end

# Prevent foreign keys being defined within column definitions e.g.
# t.bigint :x_id, foreign_key: true
# t.bigint :x_id, foreign_key: {}

class ActiveRecord::ConnectionAdapters::TableDefinition
  alias_method :__column__, :column
  def column(name, type, index: nil, **options)
    if options[:foreign_key]
      raise ArgumentError
    end
    __column__(name, type, index: index, **options)
  end
end

Thanks! 🙏

@ankane
Copy link
Owner

ankane commented Jun 27, 2022

Hey @markbrown4, check out #170 for previous discussion about adding foreign keys to new tables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants