Merge pull request #42674 from ghiculescu/verify-fk-after-fixture-insert

Verify foreign keys after loading fixtures

activerecord/lib/active_record.rb

@@ -313,6 +313,15 @@ module ActiveRecord
   singleton_class.attr_accessor :suppress_multiple_database_warning
   self.suppress_multiple_database_warning = false
 
+  ##
+  # :singleton-method:
+  # If true, Rails will verify all foreign keys in the database after loading fixtures.
+  # An error will be raised if there are any foreign key violations, indicating incorrectly
+  # written fixtures.
+  # Supported by PostgreSQL and SQLite.
+  singleton_class.attr_accessor :verify_foreign_keys_for_fixtures
+  self.verify_foreign_keys_for_fixtures = false
+
   def self.eager_load!
     super
     ActiveRecord::Locking.eager_load!

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

@@ -489,6 +489,11 @@ module ActiveRecord
         yield
       end
 
+      # Override to check all foreign key constraints in a database.
+      def all_foreign_keys_valid?
+        true
+      end
+
       # CONNECTION MANAGEMENT ====================================
 
       # Checks whether the connection to the database is still active. This includes

activerecord/lib/active_record/connection_adapters/postgresql/referential_integrity.rb

@@ -37,6 +37,38 @@ Rails needs superuser privileges to disable referential integrity.
           rescue ActiveRecord::ActiveRecordError
           end
         end
+
+        def all_foreign_keys_valid? # :nodoc:
+          sql = <<~SQL
+            do $$
+              declare r record;
+            BEGIN
+            FOR r IN (
+              SELECT FORMAT(
+                'UPDATE pg_constraint SET convalidated=false WHERE conname = ''%I''; ALTER TABLE %I VALIDATE CONSTRAINT %I;',
+                constraint_name,
+                table_name,
+                constraint_name
+              ) AS constraint_check
+              FROM information_schema.table_constraints WHERE constraint_type = 'FOREIGN KEY'
+            )
+              LOOP
+                EXECUTE (r.constraint_check);
+              END LOOP;
+            END;
+            $$;
+          SQL
+
+          begin
+            transaction(requires_new: true) do
+              execute(sql)
+            end
+
+            true
+          rescue ActiveRecord::StatementInvalid
+            false
+          end
+        end
       end
     end
   end

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

@@ -211,6 +211,10 @@ module ActiveRecord
         end
       end
 
+      def all_foreign_keys_valid? # :nodoc:
+        execute("PRAGMA foreign_key_check").blank?
+      end
+
       # SCHEMA STATEMENTS ========================================
 
       def primary_keys(table_name) # :nodoc:

activerecord/lib/active_record/fixtures.rb

@@ -637,6 +637,10 @@ module ActiveRecord
 
             conn.insert_fixtures_set(table_rows_for_connection, table_rows_for_connection.keys)
 
+            if ActiveRecord.verify_foreign_keys_for_fixtures && !conn.all_foreign_keys_valid?
+              raise "Foreign key violations found in your fixture data. Ensure you aren't referring to labels that don't exist on associations."
+            end
+
             # Cap primary key sequences to max(pk).
             if conn.respond_to?(:reset_pk_sequence!)
               set.each { |fs| conn.reset_pk_sequence!(fs.table_name) }

activerecord/test/cases/fixtures_test.rb

@@ -790,6 +790,76 @@ class ForeignKeyFixturesTest < ActiveRecord::TestCase
   end
 end
 
+class FkObjectToPointTo < ActiveRecord::Base
+  has_many :fk_pointing_to_non_existent_objects
+end
+class FkPointingToNonExistentObject < ActiveRecord::Base
+  belongs_to :fk_object_to_point_to
+end
+
+class FixturesWithForeignKeyViolationsTest < ActiveRecord::TestCase
+  fixtures :fk_object_to_point_to
+
+  def setup
+    # other tests in this file load the parrots fixture but not the treasure one (see `test_create_fixtures`).
+    # this creates FK violations since Parrot and ParrotTreasure records are created.
+    # those violations can cause false positives in these tests. since they aren't related to these tests we
+    # delete the irrelevant records here (this test is transactional so it's fine).
+    Parrot.all.each(&:destroy)
+  end
+
+  def test_raises_fk_violations
+    fk_pointing_to_non_existent_object = <<~FIXTURE
+    first:
+      fk_object_to_point_to: one
+    FIXTURE
+    File.write(FIXTURES_ROOT + "/fk_pointing_to_non_existent_object.yml", fk_pointing_to_non_existent_object)
+
+    with_verify_foreign_keys_for_fixtures do
+      if current_adapter?(:SQLite3Adapter, :PostgreSQLAdapter)
+        assert_raise RuntimeError do
+          ActiveRecord::FixtureSet.create_fixtures(FIXTURES_ROOT, ["fk_pointing_to_non_existent_object"])
+        end
+      else
+        assert_nothing_raised do
+          ActiveRecord::FixtureSet.create_fixtures(FIXTURES_ROOT, ["fk_pointing_to_non_existent_object"])
+        end
+      end
+    end
+
+  ensure
+    File.delete(FIXTURES_ROOT + "/fk_pointing_to_non_existent_object.yml")
+    ActiveRecord::FixtureSet.reset_cache
+  end
+
+  def test_does_not_raise_if_no_fk_violations
+    fk_pointing_to_valid_object = <<~FIXTURE
+    first:
+      fk_object_to_point_to_id: 1
+    FIXTURE
+    File.write(FIXTURES_ROOT + "/fk_pointing_to_non_existent_object.yml", fk_pointing_to_valid_object)
+
+    with_verify_foreign_keys_for_fixtures do
+      assert_nothing_raised do
+        ActiveRecord::FixtureSet.create_fixtures(FIXTURES_ROOT, ["fk_pointing_to_non_existent_object"])
+      end
+    end
+
+  ensure
+    File.delete(FIXTURES_ROOT + "/fk_pointing_to_non_existent_object.yml")
+    ActiveRecord::FixtureSet.reset_cache
+  end
+
+  private
+    def with_verify_foreign_keys_for_fixtures
+      setting_was = ActiveRecord.verify_foreign_keys_for_fixtures
+      ActiveRecord.verify_foreign_keys_for_fixtures = true
+      yield
+    ensure
+      ActiveRecord.verify_foreign_keys_for_fixtures = setting_was
+    end
+end
+
 class OverRideFixtureMethodTest < ActiveRecord::TestCase
   fixtures :topics
 

activerecord/test/fixtures/fk_object_to_point_to.yml

@@ -0,0 +1,2 @@
+first:
+  id: 1

activerecord/test/schema/schema.rb

@@ -1241,6 +1241,16 @@ ActiveRecord::Schema.define do
     end
   end
 
+  disable_referential_integrity do
+    create_table :fk_object_to_point_tos, force: :cascade do |t|
+    end
+
+    create_table :fk_pointing_to_non_existent_objects, force: true do |t|
+      t.references :fk_object_to_point_to, null: false, index: false
+      t.foreign_key :fk_object_to_point_tos, column: "fk_object_to_point_to_id", name: "fk_that_will_be_broken"
+    end
+  end
+
   create_table :overloaded_types, force: true do |t|
     t.float :overloaded_float, default: 500
     t.float :unoverloaded_float

guides/source/configuring.md

@@ -481,6 +481,13 @@ in controllers and views. This defaults to `false`.
 
 * `config.active_record.enumerate_columns_in_select_statements` when true, will always include column names in `SELECT` statements, and avoid wildcard `SELECT * FROM ...` queries. This avoids prepared statement cache errors when adding columns to a PostgreSQL database for example. Defaults to `false`.
 
+* `config.active_record.destroy_all_in_batches` ensures
+  ActiveRecord::Relation#destroy_all to perform the record's deletion in batches.
+  ActiveRecord::Relation#destroy_all won't longer return the collection of the deleted
+  records after enabling this option.
+
+* `config.active_record.verify_foreign_keys_for_fixtures` ensures all foreign key constraints are valid after fixtures are loaded in tests. Supported by PostgreSQL and SQLite only. Defaults to `false`.
+
 The MySQL adapter adds one additional configuration option:
 
 * `ActiveRecord::ConnectionAdapters::Mysql2Adapter.emulate_booleans` controls whether Active Record will consider all `tinyint(1)` columns as booleans. Defaults to `true`.
@@ -511,11 +518,6 @@ The schema dumper adds two additional configuration options:
   `fk_rails_` are not exported to the database schema dump.
   Defaults to `/^fk_rails_[0-9a-f]{10}$/`.
 
-* `config.active_record.destroy_all_in_batches` ensures
-  ActiveRecord::Relation#destroy_all to perform the record's deletion in batches.
-  ActiveRecord::Relation#destroy_all won't longer return the collection of the deleted
-  records after enabling this option.
-
 ### Configuring Action Controller
 
 `config.action_controller` includes a number of configuration settings:
@@ -1103,6 +1105,7 @@ text/javascript image/svg+xml application/postscript application/x-shockwave-fla
 - `config.action_controller.silence_disabled_session_errors` : `false`
 - `config.action_mailer.smtp_timeout`: `5`
 - `config.active_storage.video_preview_arguments`: `"-vf 'select=eq(n\\,0)+eq(key\\,1)+gt(scene\\,0.015),loop=loop=-1:size=2,trim=start_frame=1' -frames:v 1 -f image2"`
+- `config.active_record.verify_foreign_keys_for_fixtures`: `true`
 
 #### For '6.1', defaults from previous versions below and:
 

railties/lib/rails/application/configuration.rb

@@ -226,6 +226,10 @@ module Rails
               "-vf 'select=eq(n\\,0)+eq(key\\,1)+gt(scene\\,0.015),loop=loop=-1:size=2,trim=start_frame=1'" \
               " -frames:v 1 -f image2"
           end
+
+          if respond_to?(:active_record)
+            active_record.verify_foreign_keys_for_fixtures = true
+          end
         else
           raise "Unknown version #{target_version.to_s.inspect}"
         end

railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_0.rb.tt

@@ -55,3 +55,6 @@
 
 # Enforce destroy in batches when calling ActiveRecord::Relation#destroy_all
 # Rails.application.config.active_record.destroy_all_in_batches = true
+
+# Raise when running tests if fixtures contained foreign key violations
+# Rails.application.config.active_record.verify_foreign_keys_for_fixtures = true

railties/test/application/configuration_test.rb

@@ -2305,6 +2305,32 @@ module ApplicationTests
       assert_equal true, ActiveRecord::Base.has_many_inversing
     end
 
+    test "ActiveRecord.verify_foreign_keys_for_fixtures is true by default for new apps" do
+      app "development"
+
+      assert_equal true, ActiveRecord.verify_foreign_keys_for_fixtures
+    end
+
+    test "ActiveRecord.verify_foreign_keys_for_fixtures is false by default for upgraded apps" do
+      remove_from_config '.*config\.load_defaults.*\n'
+
+      app "development"
+
+      assert_equal false, ActiveRecord.verify_foreign_keys_for_fixtures
+    end
+
+    test "ActiveRecord.verify_foreign_keys_for_fixtures can be configured via config.active_record.verify_foreign_keys_for_fixtures" do
+      remove_from_config '.*config\.load_defaults.*\n'
+
+      app_file "config/initializers/new_framework_defaults_7_0.rb", <<-RUBY
+        Rails.application.config.active_record.verify_foreign_keys_for_fixtures = true
+      RUBY
+
+      app "development"
+
+      assert_equal true, ActiveRecord.verify_foreign_keys_for_fixtures
+    end
+
     test "ActiveSupport::MessageEncryptor.use_authenticated_message_encryption is true by default for new apps" do
       app "development"