v0.8.1 — readable one-time token in transactional tests, strong_migrations-safe generators

Why

An app on the fleet that upgraded from 0.5 to 0.8.0 found that every one of its transactional tests of the token reveal page got nil since 0.6, and that the upgrade generators produced migrations its strong_migrations setup refused. Both are fixed here; no migration for existing installs.

Two fixes found when an app on the fleet upgraded from 0.5 to 0.8.0. The second one now covers every upgrade generator.

Fixed

• The one-time token is readable in transactional tests again, once, as in production. Since 0.6.0 a retrieval inside any open transaction was refused, and Rails' transactional tests run each test inside one, so a test that created a key and read its token got nil (:inside_transaction), and the dashboard's token page redirected. When Rails.env is test and the only open transaction is an outer, non-joinable one (what Rails' transactional tests, rspec-rails and DatabaseCleaner's :transaction strategy open, and what a system test's server thread shares), the claim of token_shown_at runs in it: the first retrieval returns the token, and a second one, or one from a copy of the session, does not. A transaction opened on top of it is still refused. Outside the test environment nothing changes. Unlike config.idempotency_in_test_transactions, this needs no switch: apps whose tests broke on 0.6.0 must get them back without new configuration, and the exemption covers only the test's own transaction, not every depth. The check reads the shape of the transaction, so in a test without a transaction of its own, an outer transaction(joinable: false) that the app opens is exempt too; the README lists the cases. Apps can remove their test-only workarounds.
• Every upgrade generator now writes a migration that strong_migrations accepts on PostgreSQL. Before, it refused three of them. Those three now run with disable_ddl_transaction!, and each step checks what exists, so a run that stopped half way can run again. An invalid index that a failed concurrent build left on PostgreSQL counts as absent: the next run removes it and builds it again (add_created_by, add_key_types, add_authentication_index). On MySQL and SQLite they run as before, except where this list says otherwise. A migration that an earlier version generated and that already ran needs nothing.
  • add_created_by (0.7.0) built the polymorphic index inside the migration's transaction, which blocks writes to api_keys. It now adds the columns, then builds index_api_keys_on_created_by with algorithm: :concurrently on PostgreSQL (and PostGIS).
  • add_key_types (0.3.0) built its three indexes inside the transaction. They are now built concurrently on PostgreSQL. strong_migrations calls the four-column index a best-practice problem on every database. The install migration has the same index, so the migration keeps it inside safety_assured when the app uses strong_migrations.
  • add_restrictions (0.5.0) added its check constraint and checked every row under a lock that blocks reads and writes. On PostgreSQL it now adds the constraint without that check (validate: false), then validates it, which does not block reads and writes. On MySQL, which refuses a literal default on a JSON column, the column now defaults to the expression JSON_OBJECT() (MySQL 8.0.13 or later), so the migration runs and existing keys get an empty object. On MySQL strong_migrations still refuses this migration twice: at add_column, for the callable default, and at the check constraint, which MySQL adds by copying the table. There, check the size of api_keys, then wrap the whole body of up in safety_assured, and run it when the table can be locked for the copy.
  • add_authentication_index (0.4.0) already built its index concurrently. It now also does so on PostGIS.
  • add_key_activity, add_device_authorizations and add_idempotency_keys already passed strong_migrations.
• Every generated migration, the install included, passes rubocop-rails-omakase, the Rails default style. Array literals of symbols are %i[], the key types are two methods instead of an array, and each case has its own line under expression =. Before, four files had 16 offenses.

Changed

• ApiKeys.test_environment? says whether Rails.env is test. The token handoff and config.idempotency_in_test_transactions both use it.

Tests

• The database check (test/databases/check.rb, in CI on PostgreSQL, MySQL and SQLite) now runs all seven upgrade migrations as rails db:migrate runs them, with strong_migrations, from an install made before 0.3.0 that has a key. It runs them down and up again, and again after a run that stopped half way. On PostgreSQL it also leaves an invalid index from a failed concurrent build and shows that the next run builds it again.
• A generator test generates all eight migrations and runs RuboCop with the omakase config on them.
• The token handoff is tested in transactional tests (the helper and the dashboard) and with real commits.

Review

Two adversarial passes before merge: the first found one major (four generator templates still failed rubocop-rails-omakase, the likely source of the bracket failures the app hit) and five minors (documenting the test-transaction exemption's three edge cases, one shared test-environment predicate, invalid-index reruns after a failed concurrent build, MySQL guidance, the changelog date); the second pass confirmed them and tightened the MySQL advice. 790 tests on Rails 7.2, 8.0 and 8.1; the generated migrations pass a strong_migrations check on PostgreSQL, MySQL and SQLite.

Commits