docs: spec Phase 9 Wave 7 — per-medium track-cardinality domain invariant
This commit is contained in:
daniel-c-harvey
@@ -163,7 +163,7 @@ Sequenced as four waves. Wave 1 is a prerequisite for everything; within Waves 2
|
||||
|
||||
**Dependency summary:** `1 → 2 → 3 → 4`. Wave 4 (public site) can begin once Wave 2's `api/release` family is stable; both Wave 4 **build and acceptance** are independent of Wave 3 (CMS) — the body-less `POST api/release/{id}/mix/waveform` trigger (9.2.B) can seed real waveform datum for acceptance testing without any CMS in existence, and hero images seed via a script against 9.2.B likewise.
|
||||
|
||||
Waves 1–5 are landed (`COMPLETED.md §9`). Wave 6 below closes two functional gaps a post-landing smoke-test survey surfaced — surfaces the medium taxonomy did *not* reach, not regressions.
|
||||
Waves 1–5 are landed (`COMPLETED.md §9`). Wave 6 closes two functional gaps a post-landing smoke-test survey surfaced — surfaces the medium taxonomy did *not* reach, not regressions. Wave 7 hardens the single-track-per-medium rule from a CMS-form convention into a real domain invariant — the one place the medium taxonomy is *declared but not enforced* below the UI.
|
||||
|
||||
### 9.6 Wave 6 — Gap Closure
|
||||
|
||||
@@ -192,6 +192,26 @@ Two functional gaps the landed Phase 9 surface left open. Both are real (medium
|
||||
|
||||
**Dependency summary for Wave 6:** A and B are independent. B is unblocked and clear-cut (mirror the proven `BatchUpload` collapse). A is blocked only on the Daniel product decision above — once the destination is chosen, its build is trivial. Neither depends on the other.
|
||||
|
||||
### 9.7 Wave 7 — Domain Invariant Hardening: per-medium track cardinality
|
||||
|
||||
The single-track-per-release rule for Session/Mix is enforced **only in the CMS form layer today** (the `BatchUpload`/`BatchEdit` master-list collapse, §9.6.B). Below the form there is no enforcement: `UnifiedTrackService.UploadAsync` links a track to a release without checking the release's medium or its existing track count, and §9.5.A's first-upload-authoritative behaviour links a *second* track to an existing `(album, artist)` release with no cardinality gate. A multi-track Session/Mix is therefore reachable via repeated separate uploads or any non-CMS `POST api/track/upload` caller. This wave makes per-medium cardinality a **real domain invariant** rather than a UI convention. Full design — the generalised rule, the enforcement-layer trade-offs, the orphan-avoidance reordering, the relationship to the existing rules, and the back-compat reality — lives in `product-notes/phase-9-medium-cardinality-invariant.md`. One item, gated on one Daniel decision (the open question below).
|
||||
|
||||
- **What:** Promote per-medium track-count from a form convention to a domain invariant enforced at the upload-service boundary. Declare each medium's allowed cardinality as data — `Cut → 1..N`, `Session → 1..1`, `Mix → 1..1` — in a single `ReleaseMedium`-keyed lookup (`MediumRules`, in `DeepDrftModels`), extensible by one entry per future medium. `UnifiedTrackService.UploadAsync` reads the resolved release's medium + live track count and **rejects** a track-add that would exceed the medium's `Max` (only the find path — a freshly created release is always within range). The existing `CountLiveTracksByRelease` (already on `ITrackService`, backs the delete cascade) supplies the count; no new counting primitive.
|
||||
- **Why:** Daniel ruled single-track-per-Session/Mix a *hard constraint* (§9.5/§9.6, resolved). Today it is form-deep only — the upload endpoint and any scripted ApiKey caller bypass it, and the first-upload-authoritative write path adds a second track to an existing non-Cut release with no check. The data model itself does not forbid what the product forbids. Hardening it at the service layer makes every domain writer pass the rule, closes the gap, and — by declaring cardinality as one shared rule both the form and the service read — guarantees the UI and the domain cannot drift.
|
||||
- **Shape:**
|
||||
- **The rule as data.** `MediumRules.CardinalityOf(medium)` returns a `(Min, Max)` value type; no three-arm `switch` in any service. The same lookup the upload service enforces is the one the CMS form collapse reads (refactor `OnMediumChanged` from its hardcoded `medium is Session or Mix` to `MediumRules.CardinalityOf(medium).IsSingleTrack`) — one source, two consumers (form shapes the UI, service enforces the limit), so they cannot diverge. This is a consume-the-new-rule refactor of §9.6.B's landed collapse, **not** a re-litigation of it.
|
||||
- **Enforcement in the orchestrator, not `TrackManager`.** The check lives in `UnifiedTrackService` (the true boundary for a track-add-to-a-release operation), not the lower-level SQL `Create`. Express the guard generally — `if (liveCount + 1) > cardinality.Max` — so a future bounded-but-not-single medium is covered by the same line.
|
||||
- **Reorder to avoid orphaning the vault write.** Today `UploadAsync` writes the vault *before* resolving the release. A rejection at that point orphans the audio. Move the cardinality pre-check **before** `AddTrackAsync`: peek the release by `(album, artist)` (a read via the existing `GetReleaseByTitleAndArtistAsync`, not a create), read its medium + count, reject early — then vault-write only the accepted upload. This reordering is part of the wave, not an afterthought.
|
||||
- **Violation behaviour.** Return a NetBlocks `ResultContainer` failure with a clear message ("A {medium} release holds a single track; '{title}' already has one"). The controller surfaces it as a `409 Conflict` (honest — well-formed request, rule violation) if cheap, `400` otherwise. The CMS already bubbles upload-failure messages inline; no bespoke UI — the common case never reaches the API because the form collapse stops it first, so this is the backstop for the paths the form does not cover.
|
||||
- **Leave `ReleaseType`-applicability alone.** Do **not** merge the cardinality rule with the `ReleaseType`-only-for-Cut invariant — they are different kinds of rule (count constraint vs. field relevance). They may co-locate as separate named members of `MediumRules`, but no generic "medium invariant engine." Only cardinality is new this wave.
|
||||
- **Tests.** Extend `MediumWritePathTests` (the §9.5 EF in-memory fixture): Session/Mix reject a second track-add; Cut accepts the Nth; first track on a new Session/Mix succeeds; `MediumRules.CardinalityOf` returns the declared ranges.
|
||||
- **Acceptance criteria:** A second track-add to an existing Session or Mix release is rejected at `POST api/track/upload` with a clear failure message and no vault orphan; a Cut release accepts many tracks unchanged; the first track on any medium succeeds; the CMS form collapse and the service enforcement both read `MediumRules` (no duplicated cardinality logic); the existing `ReleaseType`-only-for-Cut enforcement is untouched.
|
||||
- **Back-compat (verified):** No violating data exists — Phase 9 is unmerged, every release migrated to `Cut` (many-track), zero multi-track Session/Mix releases exist. A DB backstop (if chosen, see open question) goes on clean with no data-cleanup migration; the service check has nothing to reconcile. Note honestly: **no** DB-level cardinality or medium constraint exists today (`ReleaseConfiguration` carries only the `(title, artist)` unique index and the `is_deleted` index) — closing that absence is the wave.
|
||||
- **Open question (Daniel — philosophy call, do not pre-empt):** Enforce the cardinality invariant in the **`UnifiedTrackService` domain layer only** (recommended), or *also* add a **Postgres constraint-trigger DB backstop** so a future writer that bypasses the service cannot violate it?
|
||||
- **Service-only (recommended).** Consistent with the phase's own documented stance — the `ReleaseType`-only-for-Cut invariant chose service enforcement over `HasCheckConstraint` *by choice, not necessity* (`phase-9-release-medium-types.md` §1); cardinality is the same advisory-vs-storage shape and choosing the DB here would split the phase's philosophy. `UnifiedTrackService` is the *only* track-add path today — the "non-CMS caller" still goes through it (`POST api/track/upload`). The bypass a DB backstop defends against (a writer skipping the service entirely) does not exist in the codebase. And the migration is clean either way, so the backstop is free to add *later* if a second writer ever appears.
|
||||
- **DB backstop (defer).** A partial unique index cannot express this directly (the medium lives on the `release` table, not `track`; Postgres partial predicates can't cross tables). The expressible form is a hand-written PL/pgSQL constraint-trigger EF does not model — a standing maintenance surface. Defensible only if Daniel wants storage-layer immutability over service-layer truth.
|
||||
- **Recommendation: service-only (C3), defer the DB backstop (C2) as a free-to-add-later option.** This is a decision about where the system's structural truth lives — the service layer vs. the storage layer — not an implementation detail. It is Daniel's to make. Two minor sub-questions ride along (`409` vs `400` status; `MediumRules` in `DeepDrftModels`) — both have clear recommendations and should not block.
|
||||
|
||||
---
|
||||
|
||||
## Working with this file
|
||||
|
||||
@@ -0,0 +1,398 @@
|
||||
# Phase 9 — Per-Medium Track-Cardinality Invariant (Wave 7)
|
||||
|
||||
Status: spec / design. Author: product-designer. Date: 2026-06-13.
|
||||
**Plan only — no code edits made by this doc.**
|
||||
|
||||
Cross-references: `PLAN.md §9.7` (the concise wave entry this note backs),
|
||||
`product-notes/phase-9-release-medium-types.md` (the medium taxonomy and the `ReleaseType`-only-for-Cut
|
||||
invariant this note generalises from), `COMPLETED.md §9.5.A` (the first-upload-authoritative write path
|
||||
whose gap this closes), `COMPLETED.md §9.6.B` (the form-level single-track collapse this note demotes
|
||||
to a UI convenience over a real invariant). Memory: *One source, multiple views*,
|
||||
*Design for adaptability up front*.
|
||||
|
||||
---
|
||||
|
||||
## 1. The gap, stated plainly
|
||||
|
||||
Phase 9 resolved (open question §8, `phase-9-release-medium-types.md`) that **Session and Mix are
|
||||
single-track media** — one track per release — and **Cut is many-track**. Daniel ruled this a *hard
|
||||
constraint*, not a soft preference.
|
||||
|
||||
But the constraint is enforced **only in the CMS form layer today**:
|
||||
|
||||
- `BatchUpload.OnMediumChanged` collapses the multi-track master list to one row for Session/Mix
|
||||
(landed Wave 3).
|
||||
- `BatchEdit` mirrors that collapse (landed §9.6.B).
|
||||
|
||||
Below the form there is **no enforcement**. Specifically:
|
||||
|
||||
- `UnifiedTrackService.UploadAsync` resolves a release via `FindOrCreateRelease`, then calls
|
||||
`_sqlTrackService.Create(trackDto)` with the resolved `ReleaseId`. Neither path checks how many
|
||||
tracks the release already holds, nor whether its medium permits more than one.
|
||||
- §9.5.A made the **first upload authoritative** for a release's medium: a second upload carrying the
|
||||
same `(title, artist)` links to the existing release and its medium is *not* re-evaluated. That is
|
||||
correct for the medium *value* — but it means a second track can be linked to a Session or Mix
|
||||
release **with no cardinality gate**.
|
||||
|
||||
**Two concrete ways to reach a multi-track Session/Mix today:**
|
||||
|
||||
1. **Repeated single-track uploads.** Upload track 1 as a Session (CMS sends `medium=session`,
|
||||
creates the release). Upload track 2 with the same album+artist — the CMS form would *not* offer
|
||||
this for a Session, but the upload endpoint accepts it: `FindOrCreateRelease` finds the existing
|
||||
Session release, `Create` adds a second track. The form collapse is bypassed because each upload
|
||||
is a *separate* request, not one multi-row batch.
|
||||
2. **Any non-CMS API caller.** `POST api/track/upload` is an ApiKey endpoint. A script, a future
|
||||
intake tool, or a re-run of a seeding job can `POST` a second track at an existing Session/Mix
|
||||
release directly. The form layer is not in the path at all.
|
||||
|
||||
The form collapse is a good **authoring affordance** — it stops the common case at the friendliest
|
||||
point. But it is a UI convention dressed as an invariant. The invariant Daniel asked for must live
|
||||
at the **domain model level**, where every writer passes.
|
||||
|
||||
This note specs that hardening.
|
||||
|
||||
---
|
||||
|
||||
## 2. The rule, generalised — medium → cardinality as first-class data
|
||||
|
||||
Frame per-medium track-count not as three special cases but as a **declared property of each medium**,
|
||||
in exactly the "one table per concern, no scattered switch" discipline Phase 9 already established for
|
||||
medium → card, medium → projection, medium → route.
|
||||
|
||||
### 2.1 The cardinality declaration
|
||||
|
||||
A single lookup, keyed by `ReleaseMedium`, declaring each medium's allowed track count as a range:
|
||||
|
||||
```
|
||||
Cut → 1..N (many-track: Single is 1, EP/Album are many)
|
||||
Session → 1..1 (exactly one)
|
||||
Mix → 1..1 (exactly one)
|
||||
```
|
||||
|
||||
Expressed as a small value type + a single source-of-truth lookup, e.g.:
|
||||
|
||||
```csharp
|
||||
// DeepDrftModels/Enums/ — sibling to ReleaseMedium, the medium's structural metadata
|
||||
public readonly record struct MediumCardinality(int Min, int Max) // Max = int.MaxValue for unbounded
|
||||
{
|
||||
public bool Allows(int trackCount) => trackCount >= Min && trackCount <= Max;
|
||||
public bool IsSingleTrack => Max == 1;
|
||||
}
|
||||
|
||||
public static class MediumRules
|
||||
{
|
||||
private static readonly IReadOnlyDictionary<ReleaseMedium, MediumCardinality> Cardinalities =
|
||||
new Dictionary<ReleaseMedium, MediumCardinality>
|
||||
{
|
||||
[ReleaseMedium.Cut] = new(1, int.MaxValue),
|
||||
[ReleaseMedium.Session] = new(1, 1),
|
||||
[ReleaseMedium.Mix] = new(1, 1),
|
||||
};
|
||||
|
||||
public static MediumCardinality CardinalityOf(ReleaseMedium medium) => Cardinalities[medium];
|
||||
}
|
||||
```
|
||||
|
||||
**Why a lookup, not a three-arm `switch` buried in `UnifiedTrackService`:** the same rule is consumed
|
||||
in at least three places — the upload service (reject an over-limit add), the CMS form (decide whether
|
||||
to collapse the master list), and potentially the `BatchEdit` warning path (§9.6.B's "release holds N,
|
||||
should hold 1" reconciliation). Three consumers of one rule is precisely the scattered-`switch` smell
|
||||
the phase forbids. One declaration, three readers. A future medium declares its cardinality in **one
|
||||
place** and every consumer honours it automatically — the same Open/Closed payoff the rest of Phase 9
|
||||
is built around. The CMS form collapse (`OnMediumChanged`) should *also* be refactored to read
|
||||
`MediumRules.CardinalityOf(medium).IsSingleTrack` rather than its current hardcoded
|
||||
`medium is Session or Mix` test, so the form and the domain agree by construction (see §5).
|
||||
|
||||
**Placement of `MediumRules`:** `DeepDrftModels` is the natural home — it is referenced by every
|
||||
project, it already owns `ReleaseMedium`, and the rule is a pure declaration with no dependencies.
|
||||
This keeps the rule visible to the CMS form layer (`DeepDrftManager`) and the API service layer
|
||||
(`DeepDrftAPI`) from one shared origin, with no duplication.
|
||||
|
||||
### 2.2 What "track count" means here
|
||||
|
||||
The count is **live tracks on the release** (not soft-deleted). `CountLiveTracksByRelease` already
|
||||
exists on `ITrackService` / `TrackManager` — it backs the delete-cascade's empty-release cleanup
|
||||
(`UnifiedTrackService.TrySoftDeleteEmptyReleaseAsync`). The cardinality check reuses it; no new
|
||||
counting primitive is needed. (Verified: `TrackManager.CountLiveTracksByRelease` →
|
||||
`Repository.CountLiveTracksByReleaseAsync`.)
|
||||
|
||||
---
|
||||
|
||||
## 3. Where enforcement should live — the layered options
|
||||
|
||||
Three candidate layers, not mutually exclusive. The real decision is **service-only vs.
|
||||
service + DB backstop** (§3.4, the open question).
|
||||
|
||||
### 3.1 Layer A — the upload service (the load-bearing layer)
|
||||
|
||||
`UnifiedTrackService.UploadAsync` is where the second-track-add becomes reachable, so it is the
|
||||
**primary** enforcement point. After `FindOrCreateRelease` returns an **existing** release (the find
|
||||
path, not the create path — a freshly created release has zero tracks and cannot violate), and before
|
||||
`Create`, gate the add:
|
||||
|
||||
```
|
||||
release resolved (existing) →
|
||||
medium = release.Medium
|
||||
cardinality = MediumRules.CardinalityOf(medium)
|
||||
if cardinality.Max == 1:
|
||||
liveCount = CountLiveTracksByRelease(release.Id)
|
||||
if liveCount >= 1:
|
||||
return ResultContainer<TrackDto>.CreateFailResult(
|
||||
"A {medium} release holds a single track; '{release.Title}' already has one.")
|
||||
// ⚠ and DO NOT orphan the vault write — see §3.3
|
||||
```
|
||||
|
||||
Notes:
|
||||
|
||||
- **Only the find path needs the check.** The create path (release did not exist) yields a 0-track
|
||||
release; the first track is always within `1..1`. Checking only the find branch keeps the cost off
|
||||
the common first-upload path.
|
||||
- **The check is medium-driven, not Session/Mix-hardcoded.** `cardinality.Max == 1` (or
|
||||
`IsSingleTrack`) — a future single-track medium is covered with no edit here.
|
||||
- **General form, not just `Max == 1`.** Although today only `Max == 1` can be violated by a single
|
||||
add (Cut's `Max` is unbounded), express the guard as `if (liveCount + 1) > cardinality.Max` so a
|
||||
future bounded-but-not-single medium (say a hypothetical "Split" capped at 2) is covered by the
|
||||
same line. Cheap generality, honours *Design for adaptability up front*.
|
||||
|
||||
### 3.2 Layer B — `TrackManager.Create` (rejected as the primary home, noted for completeness)
|
||||
|
||||
One could push the check into `TrackManager.Create` / `FindOrCreateRelease` so the SQL service itself
|
||||
refuses. Tempting (it is "the domain model"), but:
|
||||
|
||||
- `Create` does not today know the release's medium without an extra read, and it is called on the
|
||||
loose-track path (no release) too. Threading cardinality through it muddies a method that is
|
||||
currently medium-agnostic.
|
||||
- The orphaned-vault-write concern (§3.3) is a *`UnifiedTrackService`* concern — it owns the
|
||||
two-store ordering. The cardinality check must run **before** the vault write to avoid orphaning,
|
||||
and only `UnifiedTrackService` sees both stores. That argues for the check living in the
|
||||
orchestrator, not the SQL service.
|
||||
|
||||
**Recommendation: enforce in `UnifiedTrackService`, not `TrackManager`.** The orchestrator is the
|
||||
true domain boundary for a *track-add-to-a-release* operation; `TrackManager.Create` is a lower-level
|
||||
SQL primitive.
|
||||
|
||||
### 3.3 Ordering — check before the vault write, or you trade a bad row for an orphan
|
||||
|
||||
`UploadAsync` today does: vault write (`AddTrackAsync`) → resolve release → SQL `Create`. If the
|
||||
cardinality check sits where §3.1 places it (after release resolution), the vault write has **already
|
||||
happened** — a rejection there orphans the audio in the `tracks` vault, the exact failure mode §4.3
|
||||
(`Dual-write rollback / dead-letter log`) exists to worry about.
|
||||
|
||||
**Move the cardinality pre-check earlier.** The check needs only `(album, artist)` to resolve the
|
||||
release and read its medium + count — none of which require the audio to be in the vault. Restructure
|
||||
`UploadAsync` to:
|
||||
|
||||
1. If `album` present: resolve-or-peek the release, read its medium + live count, **reject here if the
|
||||
add would exceed cardinality** — *before* `AddTrackAsync` writes the vault.
|
||||
2. Only then process + vault-write the audio.
|
||||
3. Then `Create`.
|
||||
|
||||
This is a real change to the method's shape (today resolution happens *after* the vault write). It is
|
||||
worth it: the alternative is a guaranteed orphan on every rejected over-limit upload. Flag for the
|
||||
implementer that the resolve-before-vault-write reordering is part of this wave, not an afterthought.
|
||||
|
||||
*Subtlety:* "resolve" on the pre-check should be a **read, not a create** — a `GET`-shaped
|
||||
`GetReleaseByTitleAndArtist` peek, so the pre-check does not create a release for an upload it is
|
||||
about to reject. The actual `FindOrCreateRelease` still runs at its normal point for the accepted
|
||||
path. (`GetReleaseByTitleAndArtistAsync` already exists on the repository — verified, it backs
|
||||
`FindOrCreateRelease`'s own find.)
|
||||
|
||||
### 3.4 Layer C — DB backstop (the open question)
|
||||
|
||||
A database-level guarantee so even a writer that bypasses `UnifiedTrackService` (a raw SQL insert, a
|
||||
future second service, a botched migration) cannot create a multi-track Session/Mix. The natural shape:
|
||||
|
||||
> **A partial unique index on `track(release_id)` filtered to releases whose medium is single-track.**
|
||||
|
||||
The catch: the filter predicate references `release.medium`, which lives on a **different table**
|
||||
(`release`, not `track`). Postgres partial-index predicates cannot reference another table. So the
|
||||
clean "partial unique index" is not directly expressible. Three real options for a DB backstop:
|
||||
|
||||
- **(C1) Denormalise medium (or a `is_single_track` boolean) onto `track`.** Then
|
||||
`CREATE UNIQUE INDEX ... ON track(release_id) WHERE is_single_track`. Cost: a denormalised column
|
||||
kept in sync with the release's medium (a trigger or app-level write discipline) — the kind of
|
||||
duplication Phase 8's normalization fought. **Not recommended** — the cure reintroduces the disease.
|
||||
- **(C2) A deferrable constraint trigger** (`CREATE CONSTRAINT TRIGGER`) that counts sibling tracks
|
||||
on insert/update and raises if a single-track release would exceed one. Expressible, enforced at the
|
||||
DB, no denormalised column. Cost: a hand-written PL/pgSQL trigger in a migration — a maintenance
|
||||
surface EF does not model, and a thing the team must remember exists. Honest but heavy.
|
||||
- **(C3) No DB backstop. Service-layer enforcement only**, documented as the single chokepoint, with
|
||||
a test asserting it. Accept that a writer bypassing `UnifiedTrackService` could violate the
|
||||
invariant — and rely on `UnifiedTrackService` being the *only* track-add path (it is, today).
|
||||
|
||||
**The phase's documented stance favours domain rules over DB constraints** — the
|
||||
`ReleaseType`-only-for-Cut invariant chose service enforcement over `HasCheckConstraint` *by choice,
|
||||
not necessity* (`phase-9-release-medium-types.md` §1). That precedent points at **(C3)**.
|
||||
|
||||
**But** Daniel said "domain model level," which is genuinely ambiguous between:
|
||||
|
||||
- *"Make it a real domain invariant rather than a UI convention"* — which (C3) satisfies: the
|
||||
domain **service** is the model's gatekeeper, the rule is declared as data, every domain writer
|
||||
passes it. The DB is not the domain model; the service layer is.
|
||||
- *"Push it down to the storage layer so it cannot be violated"* — which argues for (C2).
|
||||
|
||||
**Recommendation: (C3) — service-layer enforcement, no DB backstop — but flag (C2) as a deliberate
|
||||
deferral, not an oversight.** Reasons:
|
||||
|
||||
1. **Consistency with the phase's own ruling.** `ReleaseType`-only-for-Cut is *exactly* this shape
|
||||
(an advisory-vs-storage invariant) and Phase 9 already chose the service layer, in writing, with a
|
||||
named rationale. Choosing the DB here would split the phase's philosophy down the middle.
|
||||
2. **Single writer today.** `UnifiedTrackService` is the only path that adds a track to a release.
|
||||
The "non-CMS caller" threat (§1.2) is *also* a caller of `UnifiedTrackService` (it goes through
|
||||
`POST api/track/upload` → `UploadAsync`). The bypass that (C2) defends against — a writer that
|
||||
skips `UnifiedTrackService` entirely — **does not exist in the codebase**. (C2) defends a door no
|
||||
one can currently walk through.
|
||||
3. **The migration is free either way (§6).** There is no violating data, so (C2) could be *added
|
||||
later* with zero data cleanup if a second writer ever appears. Deferring it costs nothing; adding
|
||||
it now buys protection against a hypothetical.
|
||||
4. **EF-modelled vs. hand-rolled.** (C2) is a raw PL/pgSQL trigger EF does not track — a standing
|
||||
maintenance and "why does this exist" cost. The phase has been disciplined about not adding
|
||||
storage-layer machinery the ORM cannot see.
|
||||
|
||||
**This is the one genuine Daniel decision in the wave.** It is not an implementation detail — it is a
|
||||
philosophy call about where the system's structural truth lives. Surface it explicitly (§8, Q1).
|
||||
|
||||
---
|
||||
|
||||
## 4. Behaviour on violation
|
||||
|
||||
### 4.1 API layer
|
||||
|
||||
The upload service returns a NetBlocks **failure result** — the existing pattern, no new mechanism:
|
||||
|
||||
```csharp
|
||||
return ResultContainer<TrackDto>.CreateFailResult(
|
||||
$"A {medium} release holds a single track. '{title}' already has one — " +
|
||||
"edit the existing track or choose a different release.");
|
||||
```
|
||||
|
||||
`TrackController.UploadTrack` already maps a failed `ResultContainer` to a non-200 response. The
|
||||
cardinality rejection is a **client error (the request is well-formed but violates a rule), so a
|
||||
`409 Conflict`** is the honest status — distinct from the `400` used for malformed input and the
|
||||
`500` used for processing failure. Recommend the controller distinguish the cardinality failure as
|
||||
`409`; if that is more plumbing than the wave wants, a `400` with the clear message is acceptable
|
||||
(the message carries the meaning either way). Minor — flag, do not block on it.
|
||||
|
||||
### 4.2 CMS presentation
|
||||
|
||||
The CMS already surfaces upload failures (the `ResultContainer` message bubbles to the form). For the
|
||||
cardinality rejection specifically:
|
||||
|
||||
- The **common case never reaches the API** — the form collapse (§5) means an admin authoring a
|
||||
Session/Mix through `BatchUpload`/`BatchEdit` cannot assemble a multi-track payload in the first
|
||||
place. The API rejection is the **backstop for the paths the form does not cover** (repeated
|
||||
separate single uploads, scripted callers).
|
||||
- When the rejection *does* surface (e.g. an admin uploads a second single track to an existing
|
||||
Session via the single-track `TrackNew` form), show the failure message inline on the upload form,
|
||||
same channel as any other upload error. No bespoke UI — the message is self-explanatory.
|
||||
|
||||
---
|
||||
|
||||
## 5. Relationship to the existing rules — fold the form collapse, leave `ReleaseType` alone
|
||||
|
||||
Two adjacent rules already exist. Recommendation on each:
|
||||
|
||||
### 5.1 Form-level single-track collapse (§9.6.B) — **fold into the cardinality rule**
|
||||
|
||||
`BatchUpload.OnMediumChanged` and `BatchEdit` currently test `medium is Session or Mix` (or
|
||||
equivalent) to decide whether to collapse the master list. **Refactor both to read
|
||||
`MediumRules.CardinalityOf(medium).IsSingleTrack`.** This makes the form a *consumer* of the same
|
||||
declaration the service enforces, so the two cannot drift — the day a fourth single-track medium is
|
||||
added, the form collapses for it automatically and the service rejects over-limit adds for it
|
||||
automatically, from one edited line (`MediumRules`). This is the *One source, multiple views*
|
||||
preference applied to a rule rather than a view: one cardinality declaration, read by the form (to
|
||||
shape the UI) and the service (to enforce the limit).
|
||||
|
||||
This is a **refactor of landed code**, so it is in-scope for the wave only as a *consume-the-new-rule*
|
||||
change — the behaviour is unchanged, the source of the truth moves. Frame it that way in the PLAN
|
||||
item so it is not mistaken for re-litigating §9.6.B.
|
||||
|
||||
### 5.2 `ReleaseType`-only-for-Cut — **leave separate, do not merge**
|
||||
|
||||
Tempting to declare a grand "MediumRules owns every per-medium structural invariant" — both the
|
||||
cardinality *and* the ReleaseType-applicability rule. **Recommend against merging them into one
|
||||
mechanism.** They are different *kinds* of rule:
|
||||
|
||||
- Cardinality is a **count constraint on a relationship** (tracks-per-release).
|
||||
- ReleaseType-applicability is a **field-relevance rule** (which columns are meaningful for which
|
||||
medium).
|
||||
|
||||
Forcing both into one abstraction would produce a `MediumRules` that is half "how many tracks" and
|
||||
half "which fields apply" — a grab-bag keyed by medium, not a cohesive concept. They *can* both live
|
||||
in the `MediumRules` static class as **separate, clearly-named members** (`CardinalityOf(...)` and,
|
||||
if ever wanted, `UsesReleaseType(...)`) without being a single fused rule. But do not build a generic
|
||||
"medium invariant engine." Two named rules sharing a home is fine; one rule pretending to be both is
|
||||
the over-abstraction trap. (Today `ReleaseType`-applicability is enforced at the converter + the
|
||||
controller reset and works; this wave should **not** disturb it. Only *cardinality* is new.)
|
||||
|
||||
**Net:** fold the form collapse into the cardinality rule (they are the same rule at two layers);
|
||||
keep `ReleaseType`-applicability as its own thing, co-located but not merged.
|
||||
|
||||
---
|
||||
|
||||
## 6. Migration / back-compat reality (code-verified, stated honestly)
|
||||
|
||||
**There is no violating data.** Verified this session:
|
||||
|
||||
- Phase 9 is a single unmerged feature on `dev`; the `medium` column was added with
|
||||
`HasDefaultValue(ReleaseMedium.Cut)`, so every pre-existing release migrated to `Cut`
|
||||
(`ReleaseConfiguration` line 69).
|
||||
- `Cut` is many-track, so no migrated release can violate the cardinality rule.
|
||||
- Zero `Session` / `Mix` releases exist with multiple tracks (none were authored before the rule, and
|
||||
the only authoring path — the CMS form — already collapses them).
|
||||
|
||||
**Consequences:**
|
||||
|
||||
- A DB backstop (C2), *if* chosen, can be added **with no data-cleanup migration** — no pre-flight
|
||||
query for offending rows, no remediation step. The constraint goes on clean.
|
||||
- The service-layer check (C3) likewise has no historical data to reconcile.
|
||||
- The §9.6.B open question (how `BatchEdit` should render an *existing* multi-track Session/Mix) is
|
||||
**moot in practice** today — no such release exists — but its safe-reconciliation reasoning still
|
||||
stands as defensive design and should not be removed. The cardinality enforcement here makes the
|
||||
hypothetical even less reachable: the only way to *create* a multi-track Session/Mix is the gap this
|
||||
wave closes.
|
||||
|
||||
This is the honest contrast to an earlier mistaken belief that a DB constraint already rejects this:
|
||||
**it does not.** `ReleaseConfiguration` has a unique index on `(title, artist)` and the `is_deleted`
|
||||
index — **no** cardinality or medium-correlated constraint exists today. Closing that absence is the
|
||||
whole point of the wave.
|
||||
|
||||
---
|
||||
|
||||
## 7. Test coverage this wave should carry
|
||||
|
||||
`MediumWritePathTests` (the §9.5 fixture, EF in-memory) is the natural home. The cardinality rule is
|
||||
exactly the kind of pure SQL-layer predicate that fixture already exercises. Add:
|
||||
|
||||
- A Session release with one track **rejects** a second track-add (the find-path-over-limit case).
|
||||
- A Mix release with one track **rejects** a second track-add.
|
||||
- A Cut release **accepts** a second (and Nth) track-add (the many-track path is unaffected).
|
||||
- The first track on a *new* Session/Mix release **succeeds** (create-path, 0→1 is within `1..1`).
|
||||
- `MediumRules.CardinalityOf` returns the declared ranges for each medium (guards the declaration).
|
||||
|
||||
The orphan-avoidance reordering (§3.3) is a `UnifiedTrackService` behaviour and harder to unit-test in
|
||||
isolation (it spans the vault + SQL), but at minimum the **pre-check rejects before any vault write**
|
||||
should be asserted if the test seam allows — otherwise call it out as an integration gap.
|
||||
|
||||
This sits inside the project-wide "test coverage outside FileDatabase" backlog item (`PLAN.md`
|
||||
miscellaneous) — the cardinality tests are an attached cost of this wave, not a separate initiative.
|
||||
|
||||
---
|
||||
|
||||
## 8. Open questions (need Daniel before build)
|
||||
|
||||
1. **Service-only vs. DB backstop (§3.4) — the real decision.** Enforce the cardinality invariant
|
||||
in the `UnifiedTrackService` domain layer only (C3, recommended — consistent with the phase's
|
||||
`ReleaseType` precedent and the single-writer reality), or *also* add a Postgres constraint-trigger
|
||||
backstop (C2) so a future writer that bypasses the service cannot violate it? **Recommend C3, defer
|
||||
C2 as a free-to-add-later option** — the migration is clean either way (§6), so deferring costs
|
||||
nothing and adding now defends a door no caller can currently walk through. *This is a philosophy
|
||||
call about where structural truth lives, not an implementation detail — it is Daniel's.*
|
||||
|
||||
2. **Violation HTTP status (§4.1).** `409 Conflict` (honest — well-formed request, rule violation) vs.
|
||||
`400 Bad Request` (less plumbing, message still carries the meaning). *Recommend 409 if cheap,
|
||||
accept 400 otherwise — minor, should not block the wave.*
|
||||
|
||||
3. **`MediumRules` placement.** `DeepDrftModels` (recommended — referenced everywhere, owns
|
||||
`ReleaseMedium`, pure declaration) vs. a service-layer home. *Recommend `DeepDrftModels`;* flag only
|
||||
because it determines which project the CMS form and the API service both import the rule from.
|
||||
Reference in New Issue
Block a user