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

Show reason for foreign key error when loading fixtures #47990

Merged
merged 1 commit into from
Apr 20, 2023

Conversation

ghiculescu
Copy link
Member

@ghiculescu ghiculescu commented Apr 19, 2023

#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 #44943 and #47780 for attempts to fix this.

This PR rolls up some of the ideas from those PRs into one that's hopefully mergable.

  • all_foreign_keys_valid? is deprecated in favour of check_all_foreign_keys_valid!.
  • Postgres and Sqlite adapters raise an error with detail about the foreign key error.
  • Authors of other PRs added as co-authors here to get credit.
  • Deprecations updated to work with new deprecation APIs.
  • 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 #47780
Closes #44943

cc @s-mage @danini-the-panini

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>
@byroot
Copy link
Member

byroot commented Apr 20, 2023

If there is no more concerns I can merge.

@ghiculescu
Copy link
Member Author

ghiculescu commented Apr 20, 2023

@byroot I think we should merge it. I've made a PR for https://github.com/jruby/activerecord-jdbc-adapter, see jruby/activerecord-jdbc-adapter#1129.

It doesn't seem like https://github.com/rsim/oracle-enhanced has implemented the relevant method, nor has https://github.com/rails-sqlserver/activerecord-sqlserver-adapter. I don't know of any other popular adapters, but happy to reach out to (or make PRs for) any others worth checking.

@yahonda
Copy link
Member

yahonda commented Apr 20, 2023

Thanks for the work.

yahonda added a commit to yahonda/rails that referenced this pull request Apr 24, 2023
This commit addresses the `DEPRECATION WARNING: all_foreign_keys_valid? is deprecated` warning
introduced via rails#47990 at test_all_foreign_keys_valid_having_foreign_keys_in_multiple_schemas

```
$ ARCONN=postgresql bin/test test/cases/adapters/postgresql/referential_integrity_test.rb -n test_all_foreign_keys_valid_having_foreign_keys_in_multiple_schemas
Using postgresql
Run options: -n test_all_foreign_keys_valid_having_foreign_keys_in_multiple_schemas --seed 38041

DEPRECATION WARNING: all_foreign_keys_valid? is deprecated and will be removed from Rails 7.2 (called from block (3 levels) in run at /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest/test.rb:102)
/home/yahonda/src/github.com/rails/rails/activerecord/test/cases/adapters/postgresql/referential_integrity_test.rb:129:in `test_all_foreign_keys_valid_having_foreign_keys_in_multiple_schemas'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest/test.rb:102:in `block (3 levels) in run'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest/test.rb:199:in `capture_exceptions'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest/test.rb:97:in `block (2 levels) in run'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest.rb:296:in `time_it'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest/test.rb:96:in `block in run'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest.rb:391:in `on_signal'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest/test.rb:247:in `with_info_handler'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest/test.rb:95:in `run'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest.rb:1051:in `run_one_method'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest.rb:365:in `run_one_method'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest.rb:352:in `block (2 levels) in run'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest.rb:351:in `each'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest.rb:351:in `block in run'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest.rb:391:in `on_signal'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest.rb:378:in `with_info_handler'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest.rb:350:in `run'
  /home/yahonda/src/github.com/rails/rails/railties/lib/rails/test_unit/line_filtering.rb:10:in `run'
  /home/yahonda/src/github.com/rails/rails/activerecord/test/cases/test_case.rb:252:in `run'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest.rb:182:in `block in __run'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest.rb:182:in `map'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest.rb:182:in `__run'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest.rb:159:in `run'
  /home/yahonda/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/minitest-5.17.0/lib/minitest.rb:83:in `block in autorun'
.

Finished in 0.025025s, 39.9607 runs/s, 79.9214 assertions/s.
1 runs, 2 assertions, 0 failures, 0 errors, 0 skips
$
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants