Verify foreign keys after loading fixtures #42674

ghiculescu · 2021-07-01T21:18:35Z

When writing fixtures, it's possible to define associations that don't exist, even if a foreign key exists. For example:

# test/fixtures/parrots.yml
george:
  name: "Curious George"
  pirate: redbeard

# test/fixtures/pirates.yml
blackbeard:
  name: "Blackbeard"

When the fixtures are created, parrots(:george).pirate will be nil, but it's not clear why. This can make it hard to debug tests and can give false confidence in passing ones.

This happens because Rails disables referential integrity when inserting fixtures. This makes the fixtures algorithm much simpler - it can just create the fixtures in alphabetical order and assume that the other side of a foreign key constraint will eventually be added.

Ideally we would check foreign keys once all fixtures have been loaded, so that we can be sure that the foreign key constraints were met. This PR introduces that. To enable it:

config.active_record.verify_foreign_keys_for_fixtures = true

I'm proposing we enable this in 7.0 for new apps and have added it to new framework defaults. When run against our app, it found 3 fixture files with unmet FK constraints - turns out all those fixtures weren't being used and were safe to delete.

The implementation works with SQLite (which has a builtin pragma for this) and PostgreSQL (which we use, and it was easy enough to write a query for). I haven't implemented for MySQL as I wasn't able to find any good advice on how to do it, but would love to add it in if anyone knows how.

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

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

guides/source/configuring.md

activerecord/test/cases/fixtures_test.rb

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

When writing fixtures, it's currently possible to define associations that don't exist, even if a foreign key exists. For example: ```yml george: name: "Curious George" pirate: redbeard blackbeard: name: "Blackbeard" ``` When the fixtures are created, `parrots(:george).pirate` will be nil, but it's not immediately clear why. This can make it hard to debug tests and can give false confidence in passing ones. This can happen because Rails [disables referential integrity](https://github.com/rails/rails/blob/f263530bf709f611920b5f663882e5113b4f984b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb#L407) when inserting fixtures. This makes the fixtures algorithm much simpler - it can just create the fixtures in alphabetical order and assume that the other side of a foreign key constraint will *eventually* be added. Ideally we would check foreign keys once all fixtures have been loaded, so that we can be sure that the foreign key constraints were met. This PR introduces that. To enable it: ```ruby config.active_record.verify_foreign_keys_for_fixtures = true ``` I'm proposing we enable this in 7.0 for new apps and have added it to new framework defaults. When run against our app, it found 3 fixture files with unmet FK constraints - turns out all those fixtures weren't being used and were safe to delete.

ghiculescu · 2021-07-08T02:22:02Z

PSA: if anyone who knows more MySQL than me comes across this it’d be great to see it implemented for that too. I’m happy to help with the PR

zzak · 2021-07-10T23:05:59Z

@ghiculescu If it's worth adding this to thisweekinrails do you think it's worth adding a changelog? 🙇

ghiculescu · 2021-07-11T13:29:38Z

Yes. 🤦 I will tomorrow.

@zzak

Fixes rails#42674 (comment) cc @zzak

Add CHANGELOG for #42674

…o_fk_violations Managed to reproduce CI failure at https://buildkite.com/rails/rails/builds/79496#0c03f856-9be1-4ca0-88c9-e1df21ae0a07 This commitaddresses the following errors by loading :author_addresses fixture because `:authors` has a foreign key to `:author_addresses`. * sqlite3 adapter ```ruby $ bin/test test/cases/adapters/sqlite3/explain_test.rb test/cases/fixtures_test.rb -n "/^(?:SQLite3ExplainTest#(?:test_explain_with_eager_loading)|FixturesWithForeignKeyViolationsTest#(?:test_does_not_raise_if_no_fk_violations))$/" --seed 2529 Using sqlite3 Run options: -n "/^(?:SQLite3ExplainTest#(?:test_explain_with_eager_loading)|FixturesWithForeignKeyViolationsTest#(?:test_does_not_raise_if_no_fk_violations))$/" --seed 2529 .E Error: FixturesWithForeignKeyViolationsTest#test_does_not_raise_if_no_fk_violations: RuntimeError: Foreign key violations found in your fixture data. Ensure you aren't referring to labels that don't exist on associations. /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:641:in `block in insert' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:629:in `each' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:629:in `insert' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:615:in `read_and_insert' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:567:in `create_fixtures' /home/yahonda/src/github.com/rails/rails/activerecord/test/cases/fixtures_test.rb:844:in `block (2 levels) in test_does_not_raise_if_no_fk_violations' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:34:in `assert_nothing_raised' /home/yahonda/src/github.com/rails/rails/activerecord/test/cases/fixtures_test.rb:843:in `block in test_does_not_raise_if_no_fk_violations' /home/yahonda/src/github.com/rails/rails/activerecord/test/cases/fixtures_test.rb:857:in `with_verify_foreign_keys_for_fixtures' /home/yahonda/src/github.com/rails/rails/activerecord/test/cases/fixtures_test.rb:842:in `test_does_not_raise_if_no_fk_violations' bin/test test/cases/fixtures_test.rb:835 Finished in 0.045767s, 43.6991 runs/s, 174.7966 assertions/s. 2 runs, 8 assertions, 0 failures, 1 errors, 0 skips $ ``` * postgresql adapter ```ruby $ ARCONN=postgresql bin/test test/cases/adapters/postgresql/explain_test.rb test/cases/fixtures_test.rb -n "/^(?:PostgreSQLExplainTest#(?:test_explain_with_eager_loading)|FixturesWithForeignKeyViolationsTest#(?:test_does_not_raise_if_no_fk_violations))$/" --seed 16926 Using postgresql Run options: -n "/^(?:PostgreSQLExplainTest#(?:test_explain_with_eager_loading)|FixturesWithForeignKeyViolationsTest#(?:test_does_not_raise_if_no_fk_violations))$/" --seed 16926 .E Error: FixturesWithForeignKeyViolationsTest#test_does_not_raise_if_no_fk_violations: RuntimeError: Foreign key violations found in your fixture data. Ensure you aren't referring to labels that don't exist on associations. /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:641:in `block in insert' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:629:in `each' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:629:in `insert' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:615:in `read_and_insert' /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:567:in `create_fixtures' /home/yahonda/src/github.com/rails/rails/activerecord/test/cases/fixtures_test.rb:844:in `block (2 levels) in test_does_not_raise_if_no_fk_violations' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:34:in `assert_nothing_raised' /home/yahonda/src/github.com/rails/rails/activerecord/test/cases/fixtures_test.rb:843:in `block in test_does_not_raise_if_no_fk_violations' /home/yahonda/src/github.com/rails/rails/activerecord/test/cases/fixtures_test.rb:857:in `with_verify_foreign_keys_for_fixtures' /home/yahonda/src/github.com/rails/rails/activerecord/test/cases/fixtures_test.rb:842:in `test_does_not_raise_if_no_fk_violations' bin/test test/cases/fixtures_test.rb:835 Finished in 0.121193s, 16.5027 runs/s, 49.5080 assertions/s. 2 runs, 6 assertions, 0 failures, 1 errors, 0 skips $ ``` * It does not reproduce with mysql2 adapter because MySQL does not have features to validate existing foreign keys, but added :author_addresses in Mysql2ExplainTest for consistency. Follow up rails#42674

dvodvo · 2022-07-06T05:25:31Z

I'm not confident that's something we want to have in Rails.
I can see why this would tend to be very long and agree with this conclusion.
I would even add that launching a Unit test should make no assumptions about fixtures; that appears to be the wrong place for it.

However, sometimes the process of fishing out the guilty party when one absolutely needs many fixtures becomes excessively long. Geometrically long, in fact.

Instead, when this monster rears its head, I would instinctively like to do something akin to: rails test [path_to_fixtures][optionally: '/specific_fixtures.yml'
The process would run through the invoked YAML file(s) and stop upon the first error, reporting that.

ghiculescu · 2022-07-06T14:06:01Z

@mjc-gh @dvodvo you might be interested in this PR that @danini-the-panini is working on: #44943

hey-leon · 2023-04-19T16:57:44Z

@ghiculescu cool feature!

rails#42674 added the ability to have Rails verify foreign keys when creating fixtures. Feedback from users since then is it would be handy to know *which* foreign keys are being violated. See rails#44943 and rails#47780 for attempts to fix this. This PR rolls up some of the ideas from those PRs into one that's hopefully mergable. - [x] `all_foreign_keys_valid?` is deprecated in favour of `check_all_foreign_keys_valid!`. - [x] Postgres and Sqlite adapters raise an error with detail about the foreign key error. - [x] Authors of other PRs added as co-authors here to get credit. - [x] Deprecations updated to work with new deprecation APIs. - [x] Tests updated. Here's what the error messages will now look like. Postgres: ``` Foreign key violations found in your fixture data. Ensure you aren't referring to labels that don't exist on associations. Error from database: PG::ForeignKeyViolation: ERROR: insert or update on table "fk_pointing_to_non_existent_objects" violates foreign key constraint "fk_that_will_be_broken" DETAIL: Key (fk_object_to_point_to_id)=(980190962) is not present in table "fk_object_to_point_tos". CONTEXT: SQL statement "UPDATE pg_constraint SET convalidated=false WHERE conname = 'fk_that_will_be_broken' AND connamespace::regnamespace = 'public'::regnamespace; ALTER TABLE public.fk_pointing_to_non_existent_objects VALIDATE CONSTRAINT fk_that_will_be_broken;" PL/pgSQL function inline_code_block line 16 at EXECUTE ``` Sqlite: ``` Foreign key violations found in your fixture data. Ensure you aren't referring to labels that don't exist on associations. Error from database: Foreign key violations found: fk_pointing_to_non_existent_objects ``` Closes rails#47780 Closes rails#44943 Co-Authored-By: s-mage <s-mage@users.noreply.github.com> Co-Authored-By: danini-the-panini <danini-the-panini@users.noreply.github.com>

rails-bot bot added activerecord docs railties labels Jul 1, 2021

ghiculescu commented Jul 1, 2021

View reviewed changes

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb Outdated Show resolved Hide resolved

ghiculescu commented Jul 1, 2021

View reviewed changes

activerecord/lib/active_record/connection_adapters/postgresql/referential_integrity.rb Outdated Show resolved Hide resolved

ghiculescu force-pushed the verify-fk-after-fixture-insert branch from e03ae36 to fcb761c Compare July 1, 2021 21:21

ghiculescu commented Jul 1, 2021

View reviewed changes

guides/source/configuring.md Show resolved Hide resolved

ghiculescu force-pushed the verify-fk-after-fixture-insert branch 6 times, most recently from 20c2cdc to 9f295e0 Compare July 7, 2021 20:02

rafaelfranca reviewed Jul 7, 2021

View reviewed changes

ghiculescu force-pushed the verify-fk-after-fixture-insert branch from 9f295e0 to 47467fe Compare July 7, 2021 20:41

ghiculescu requested a review from rafaelfranca July 7, 2021 21:24

rafaelfranca merged commit 831031a into rails:main Jul 7, 2021

ghiculescu deleted the verify-fk-after-fixture-insert branch July 7, 2021 21:39

ghiculescu added the thisweekinrails label Jul 8, 2021

ghiculescu added a commit to ghiculescu/rails that referenced this pull request Jul 12, 2021

Add CHANGELOG for rails#42674

32ff2c2

Fixes rails#42674 (comment) cc @zzak

ghiculescu mentioned this pull request Jul 12, 2021

Add CHANGELOG for #42674 #42760

Merged

zzak added a commit that referenced this pull request Jul 13, 2021

Merge pull request #42760 from ghiculescu/patch-4

bd720c9

Add CHANGELOG for #42674

yahonda mentioned this pull request Jul 21, 2021

Address FixturesWithForeignKeyViolationsTest#test_does_not_raise_if_no_fk_violations error #42829

Merged

ghiculescu mentioned this pull request Dec 15, 2021

While loading fixtures disable triggers only for defined fixture tables #43876

Closed

ghiculescu mentioned this pull request Apr 19, 2023

Show reason for foreign key error when loading fixtures #47990

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Verify foreign keys after loading fixtures #42674

Verify foreign keys after loading fixtures #42674

ghiculescu commented Jul 1, 2021 •

edited

ghiculescu commented Jul 8, 2021

zzak commented Jul 10, 2021

ghiculescu commented Jul 11, 2021

dvodvo commented Jul 6, 2022 •

edited

ghiculescu commented Jul 6, 2022

hey-leon commented Apr 19, 2023

Verify foreign keys after loading fixtures #42674

Verify foreign keys after loading fixtures #42674

Conversation

ghiculescu commented Jul 1, 2021 • edited

ghiculescu commented Jul 8, 2021

zzak commented Jul 10, 2021

ghiculescu commented Jul 11, 2021

dvodvo commented Jul 6, 2022 • edited

ghiculescu commented Jul 6, 2022

hey-leon commented Apr 19, 2023

ghiculescu commented Jul 1, 2021 •

edited

dvodvo commented Jul 6, 2022 •

edited