Skip to content

Commit 4186d7d

Browse files
Rylan-cgiclaude
andcommitted
docs(flyway): record the Option 2 decision and correct the seed convention
Per the discussion on #280, Option 2 is selected: seed data moves to repeatable migrations, and only DDL keeps a version. This records the decision and brings the operational README in line with it. docs/decisions/flyway-test-fixture-strategy.md ---------------------------------------------- Status moves Proposed -> ACCEPTED. Rewritten to select Option 2 and to keep Option 3 as a stretch goal rather than the target. Sharpened the root cause. The original framing was "a flat, shared, sequential namespace"; the operative property is that THE COLLISION IS INVISIBLE TO GIT. Two branches claiming the same version produce two differently NAMED files, so git merges them cleanly and the failure surfaces only when Flyway loads. That is what selects the fix: an identifier derived from content rather than sequence puts the clash back on one file path, where git reports it normally. Also separates the two problems that were conflated, because they have different answers: P1 (version collision, kills the suite at boot) and P2 (one shared mutable dataset — PK collisions, cross-class leakage, order-dependence). Option 1 is now recorded as REJECTED rather than a viable interim: day-granularity timestamps were already adopted for this exact reason and collided on 2026-08-13 and again on 2026-08-19, because everyone hand-picks today's date, making a date exactly as scarce as an integer. The fifth collision (V20260819) is added to the record. The Option 2 behaviour is documented as verified rather than assumed — probed on Flyway 12.4.0 against the real Oracle container: all 46 versioned migrations at ranks 1-46, all repeatables at 47-51, and repeatables ordered lexicographically by description (05 -> 50 -> 90 -> bbb). So R__-after-V__ is a tool guarantee, and a numeric prefix is what makes FK ordering deterministic. Why Option 3 is deferred is recorded with its measurements: 74% of fixture data (912 of 1,232 INSERTs) is shared backbone, so it would relocate ~26% and leave the shared dataset in place; 115 IT classes with only 2 @transactional, so rollback is unavailable and isolation must be hand-built; Flyway is 8.57s once per JVM against a ~65s container start; and — the decisive risk — per-class fixtures can make the cross-tenant IDOR tests pass VACUOUSLY, since they assert "someone else's row -> 404" and would still pass if the foreign row were never provisioned at all. backend/src/test/resources/db/README.md --------------------------------------- Convention 1 now says seed data goes in R__ with a content-derived name and an ordering prefix (data 10-80, constraints/FKs 90+). Convention 1a keeps versions for DDL only and says explicitly to keep INSERTs out of them, since a seed in a new V__ reopens the collision. The timestamp history is kept as a note, because it is the evidence for the rule. Corrected the stale claim that ruled this out. The README asserted a fixture-inserting migration "still takes a version, because re-running it on a reused container would duplicate rows." Wrong on both halves: Flyway re-runs a repeatable only when its CHECKSUM changes, and AbstractOracleIT creates the container fresh per JVM (no withReuse), so each repeatable applies exactly once per run. R__cost_detail_bridge_culvert_fks.sql already stated this two lines from where the README contradicted it. Also repointed three now-stale "per convention 1" cross-references in the historical per-track notes, which would otherwise send a reader to a rule about R__ seeds when they are reading about a past V__ choice. Scope ----- Docs only. No migration is renamed, no test changes, no existing fixture converted — the current chain freezes as the baseline and the convention governs new work. The one follow-up the ADR names is prefixing R__cost_detail_bridge_culvert_fks.sql with 90_, which changes no behaviour until a prefixed R__ constraint exists to sort against. Separately noted in the ADR as not done here: extending FlywayMigrationVersionUniquenessTest to reject a new V__ containing INSERTs. That is what would make the convention self-enforcing — the README alone did not prevent any of the five collisions, while the guard test caught the fifth. Verified: FlywayMigrationVersionUniquenessTest green; branch merged up to origin/main first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent b33859d commit 4186d7d

2 files changed

Lines changed: 110 additions & 56 deletions

File tree

backend/src/test/resources/db/README.md

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -22,23 +22,44 @@ two such branches merge:
2222

2323
## Conventions
2424

25-
1. **Version numbers.** Take the next free integer after the highest `V` currently on `main` **plus
26-
any in-flight PRs you know about**. When you rebase/merge `main` and hit a duplicate, bump *your*
27-
(newer) migration to the next free slot — never renumber someone else's merged migration. If we
28-
keep colliding, switch to timestamp versions (`V20260728__…`), which removes the race entirely.
29-
30-
**Timestamp versions do NOT remove the race** — proven 2026-08-13, when three branches all reached
31-
for the same two days (`V20260814` → schedule 5 subpage fixtures, `V20260815` → schedule 9 write
32-
fixtures). Everyone picks today's or tomorrow's date, so a date is just as scarce as an integer.
33-
34-
1a. **Schema changes that must apply LAST: use a repeatable migration (`R__…`).** If your script adds a
35-
constraint or index over data that *other* migrations seed — rather than seeding its own fixtures —
36-
it does not want a version number at all. It wants to run after everything, which is precisely
37-
Flyway's guarantee for repeatable migrations. `R__cost_detail_bridge_culvert_fks.sql` is the worked
38-
example: it declares the delivery FKs on `ILCR_COST_REPORT_DETAIL` after every schedule's fixtures
39-
have populated their per-report column, and it cannot collide with anyone. `FlywayMigrationVersionUniquenessTest`
40-
ignores `R__` files by design. This is NOT a general escape hatch — a migration that inserts its own
41-
fixtures still takes a version, because re-running it on a reused container would duplicate rows.
25+
1. **Seed data goes in a repeatable migration (`R__`), not a versioned one.** Decided 2026-08-20 —
26+
see `docs/decisions/flyway-test-fixture-strategy.md`. Name the file for **what it seeds**, with a
27+
numeric ordering prefix: `R__<10-80>_<what_it_seeds>.sql` for data,
28+
`R__<90+>_<name>.sql` for constraints, indexes and FKs that must land after the data.
29+
30+
Why: a version number is a **shared, sequential** resource, and two branches claiming the same one
31+
produce two *differently named files* — so **git merges them cleanly and the failure only appears
32+
when Flyway loads**, taking the whole `*IT` suite down at boot. Five collisions on record. With a
33+
content-derived `R__` name, a shared prefix is harmless (both files run, ordered by the rest of the
34+
name) and an identical name is the same path, which git reports as an ordinary conflict.
35+
36+
Verified on Flyway 12.4.0: repeatables apply **after every versioned migration**, in
37+
**lexicographic order of description** — so the numeric prefix is what fixes FK ordering, and
38+
digits sort before letters.
39+
40+
1a. **Only DDL keeps a version.** `V<next>__<name>.sql` for adding or altering tables. Take the next
41+
free integer after the highest `V` on `main` plus any in-flight PR you know about; on a duplicate,
42+
bump *your* (newer) migration and never renumber someone else's merged one.
43+
`FlywayMigrationVersionUniquenessTest` catches a clash at PR time rather than at IT boot. **Keep
44+
`INSERT`s out of these files** — putting seed rows in a new `V__` reopens the collision this
45+
convention exists to close.
46+
47+
*(Historical note, kept because it is the reason for the rule above: timestamp versions were tried
48+
and did NOT remove the race. Proven 2026-08-13, when three branches reached for the same two days
49+
(`V20260814`, `V20260815`), and again on 2026-08-19 with `V20260819`. Everyone hand-picks today's
50+
date, so a date is exactly as scarce as an integer.)*
51+
52+
1b. **`R__` files are safe to re-run here, and this is not the escape hatch it once looked like.** An
53+
earlier revision of this README said a fixture-inserting migration "still takes a version, because
54+
re-running it on a reused container would duplicate rows." That was wrong on both halves. Flyway
55+
re-runs a repeatable migration **only when its checksum changes**, and `AbstractOracleIT` creates
56+
the container **fresh per JVM** (no `withReuse`), so every repeatable applies exactly once per run.
57+
The only residual case is editing an `R__` file against a container you are deliberately reusing —
58+
which needs a clean container, the same caveat every DDL fixture here already carries.
59+
`R__cost_detail_bridge_culvert_fks.sql` is the worked example of the `90+` band: it declares the
60+
delivery FKs on `ILCR_COST_REPORT_DETAIL` after every schedule's fixtures have populated their
61+
per-report column. It predates the prefix convention and still carries no number; it should gain
62+
`90_` when the first prefixed seed lands beside it.
4263
2. **Fixture ID ranges.** Namespace seed entities by track so PKs can't overlap:
4364

4465
| Track | `MILL_ID` block | Notes |
@@ -54,14 +75,14 @@ two such branches merge:
5475
| Schedule 9 | **700–706** | `V20260815` |
5576
| Schedule 10 | **710–716** | `V20260817` |
5677

57-
**Schedule 5 sub-pages (`V20260814`, Story 7.4)** — a **timestamp version**, per convention 1 and
58-
the `V20260807` precedent. Seeds the first item-62 / item-68 rows the suite has ever held, on its
78+
**Schedule 5 sub-pages (`V20260814`, Story 7.4)** — a **timestamp version**, per the historical
79+
note in convention 1a and the `V20260807` precedent. Seeds the first item-62 / item-68 rows the suite has ever held, on its
5980
own mills so no destructive test can touch Story 7.2's `670–676`: `690` the write playground
6081
(Draft 2016–2023, one destructive concern per year), `691` Submitted → the write-gate 409, `692`
6182
check-status against real sub-page rows, `693` owned solely by the authorization IT. The block was
6283
`680–683` until Schedule 7B's `V20260811` landed on `main` claiming `680–681`; both migrations
6384
`INSERT INTO THE.MILL` those ids, so the merge would have failed Flyway outright on ORA-00001.
64-
Per convention 1 the newer (unmerged) claim moved. PK ranges are
85+
Per convention 1a the newer (unmerged) claim moved. PK ranges are
6586
a **new block**, verified above every value in use (the previous high-water mark was `8438`):
6687
`CAMP_REPORT_ID` **`8700–8719`** and `ILCR_COST_REPORT_DETAIL_ID` **`8720–8799`** — both below the
6788
sequence starts. It adds NO cost item (62/68/141/142 already exist via `V34`/`V31`).
@@ -212,5 +233,6 @@ snapshot they seed into, with the `V3x` references in the `schedule6` `*IT`s and
212233
lockstep. Version numbers only; no seed-ID clash (schedule 6 owns mills `660–666`).
213234

214235
This is the third version collision on this convention (schedule 2, schedule 11, schedule 6), and
215-
each one was caught only after CI went red on a branch that was otherwise green. The
216-
timestamp-version escape hatch in convention 1 above is worth taking.
236+
each one was caught only after CI went red on a branch that was otherwise green. Under the 2026-08-20 decision this class of
237+
clash no longer arises for seed data at all: it goes in an `R__` file, which has no version to claim
238+
(convention 1 above).

0 commit comments

Comments
 (0)