From d3fd4e4ed9bae3775969c5c913b15bbd927e9ad9 Mon Sep 17 00:00:00 2001 From: David Stosik Date: Mon, 19 Mar 2018 17:42:23 +0000 Subject: Expose foreign key name ignore pattern in configuration When dumping the database schema, Rails will dump foreign key names only if those names were not generate by Rails. Currently this is determined by checking if the foreign key name is `fk_rails_` followed by a 10-character hash. At [Cookpad](https://github.com/cookpad), we use [Departure](https://github.com/departurerb/departure) (Percona's pt-online-schema-change runner for ActiveRecord migrations) to run migrations. Often, `pt-osc` will make a copy of a table in order to run a long migration without blocking it. In this copy process, foreign keys are copied too, but [their name is prefixed with an underscore to prevent name collision ](https://www.percona.com/doc/percona-toolkit/LATEST/pt-online-schema-change.html#cmdoption-pt-online-schema-change-alter-foreign-keys-method). In the process described above, we often end up with a development database that contains foreign keys which name starts with `_fk_rails_`. That name does not match the ignore pattern, so next time Rails dumps the database schema (eg. when running `rake db:migrate`), our `db/schema.rb` file ends up containing those unwanted foreign key names. This also produces an unwanted git diff that we'd prefer not to commit. In this PR, I'd like to suggest a way to expose the foreign key name ignore pattern to the Rails configuration, so that individual projects can decide on a different pattern of foreign keys that will not get their names dumped in `schema.rb`. --- activerecord/lib/active_record.rb | 1 + .../connection_adapters/abstract/schema_definitions.rb | 4 ++++ .../connection_adapters/abstract/schema_statements.rb | 2 +- activerecord/lib/active_record/core.rb | 2 ++ activerecord/lib/active_record/foreign_keys.rb | 12 ++++++++++++ activerecord/lib/active_record/railtie.rb | 1 + activerecord/lib/active_record/schema_dumper.rb | 2 +- 7 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 activerecord/lib/active_record/foreign_keys.rb (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index d43378c64f..66084e9b1c 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -42,6 +42,7 @@ module ActiveRecord autoload :CounterCache autoload :DynamicMatchers autoload :Enum + autoload :ForeignKeys autoload :InternalMetadata autoload :Explain autoload :Inheritance 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 584a86da21..b91f591e1a 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -101,6 +101,10 @@ module ActiveRecord end alias validated? validate? + def export_name_on_schema_dump? + name !~ ActiveRecord::Base.fk_ignore_pattern + end + def defined_for?(to_table_ord = nil, to_table: nil, **options) if to_table_ord self.to_table == to_table_ord.to_s 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 e2147b7fcf..ef45fff9d2 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -1324,7 +1324,7 @@ module ActiveRecord identifier = "#{table_name}_#{options.fetch(:column)}_fk" hashed_identifier = Digest::SHA256.hexdigest(identifier).first(10) - "fk_rails_#{hashed_identifier}" + "#{ActiveRecord::ForeignKeys::PREFIX}_#{hashed_identifier}" end end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index e1a0b2ecf8..cb0be9787f 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -125,6 +125,8 @@ module ActiveRecord mattr_accessor :belongs_to_required_by_default, instance_accessor: false + mattr_accessor :fk_ignore_pattern, instance_accessor: false + class_attribute :default_connection_handler, instance_writer: false def self.connection_handler diff --git a/activerecord/lib/active_record/foreign_keys.rb b/activerecord/lib/active_record/foreign_keys.rb new file mode 100644 index 0000000000..87ce3ace20 --- /dev/null +++ b/activerecord/lib/active_record/foreign_keys.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module ActiveRecord + module ForeignKeys + # The prefix used by Rails to name unnamed foreign keys. + PREFIX = "fk_rails" + + # Default regular expression used by Rails to determine if a foreign key + # name was generated. + DEFAULT_IGNORE_PATTERN = /^#{PREFIX}_[0-9a-f]{10}$/ + end +end diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 6ab80a654d..7f58780fdf 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -27,6 +27,7 @@ module ActiveRecord config.active_record.use_schema_cache_dump = true config.active_record.maintain_test_schema = true + config.active_record.fk_ignore_pattern = ActiveRecord::ForeignKeys::DEFAULT_IGNORE_PATTERN config.active_record.sqlite3 = ActiveSupport::OrderedOptions.new config.active_record.sqlite3.represent_boolean_as_integer = nil diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index b8d848b999..72b7460342 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -210,7 +210,7 @@ HEADER parts << "primary_key: #{foreign_key.primary_key.inspect}" end - if foreign_key.name !~ /^fk_rails_[0-9a-f]{10}$/ + if foreign_key.export_name_on_schema_dump? parts << "name: #{foreign_key.name.inspect}" end -- cgit v1.2.3 From 864e500817e7bcda4753f001ba862f7adbdb1c15 Mon Sep 17 00:00:00 2001 From: David Stosik Date: Tue, 20 Mar 2018 18:08:11 +0000 Subject: Document config.active_record.fk_ignore_pattern --- activerecord/lib/active_record/core.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index cb0be9787f..4e63d1a273 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -125,6 +125,10 @@ module ActiveRecord mattr_accessor :belongs_to_required_by_default, instance_accessor: false + ## + # :singleton-method: + # Specify a custom regular expression matching foreign keys which name + # should not be dumped to db/schema.rb. mattr_accessor :fk_ignore_pattern, instance_accessor: false class_attribute :default_connection_handler, instance_writer: false -- cgit v1.2.3 From f6e612b2723ca7ae730ba3e98267c19356b386b3 Mon Sep 17 00:00:00 2001 From: David Stosik Date: Thu, 22 Mar 2018 10:12:58 +0000 Subject: Move fk_ignore_pattern from config.active_record to SchemaDumper This makes more sense, as the foreign key ignore pattern is only used by the schema dumper. --- .../connection_adapters/abstract/schema_definitions.rb | 2 +- activerecord/lib/active_record/core.rb | 6 ------ activerecord/lib/active_record/railtie.rb | 1 - activerecord/lib/active_record/schema_dumper.rb | 6 ++++++ 4 files changed, 7 insertions(+), 8 deletions(-) (limited to 'activerecord/lib') 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 b91f591e1a..6a498b353c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -102,7 +102,7 @@ module ActiveRecord alias validated? validate? def export_name_on_schema_dump? - name !~ ActiveRecord::Base.fk_ignore_pattern + name !~ ActiveRecord::SchemaDumper.fk_ignore_pattern end def defined_for?(to_table_ord = nil, to_table: nil, **options) diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 4e63d1a273..e1a0b2ecf8 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -125,12 +125,6 @@ module ActiveRecord mattr_accessor :belongs_to_required_by_default, instance_accessor: false - ## - # :singleton-method: - # Specify a custom regular expression matching foreign keys which name - # should not be dumped to db/schema.rb. - mattr_accessor :fk_ignore_pattern, instance_accessor: false - class_attribute :default_connection_handler, instance_writer: false def self.connection_handler diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 7f58780fdf..6ab80a654d 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -27,7 +27,6 @@ module ActiveRecord config.active_record.use_schema_cache_dump = true config.active_record.maintain_test_schema = true - config.active_record.fk_ignore_pattern = ActiveRecord::ForeignKeys::DEFAULT_IGNORE_PATTERN config.active_record.sqlite3 = ActiveSupport::OrderedOptions.new config.active_record.sqlite3.represent_boolean_as_integer = nil diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 72b7460342..8fc2752f0c 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -17,6 +17,12 @@ module ActiveRecord # Only strings are accepted if ActiveRecord::Base.schema_format == :sql. cattr_accessor :ignore_tables, default: [] + ## + # :singleton-method: + # Specify a custom regular expression matching foreign keys which name + # should not be dumped to db/schema.rb. + cattr_accessor :fk_ignore_pattern, default: ActiveRecord::ForeignKeys::DEFAULT_IGNORE_PATTERN + class << self def dump(connection = ActiveRecord::Base.connection, stream = STDOUT, config = ActiveRecord::Base) connection.create_schema_dumper(generate_options(config)).dump(stream) -- cgit v1.2.3