Sangeetha-Grantha

Metadata Value
Status Completed
Version 2.2.0
Last Updated 2026-07-18
Author Sangeetha Grantha Team
Priority P1
Epic TRACK-109 (W2 / W8)
Decisions D9 (decision log)
Depends on TRACK-110, TRACK-111
Supersedes TRACK-014 (Bulk Import Testing & QA)

TRACK-112: Money-Path Service & API Scenarios (Step 5)

Goal

Cover the business-critical flows with real-DB integration tests, prioritized by risk. Estimated 1–2 weeks, incremental. Supersedes the deferred TRACK-014. Completed with findings — see §Findings.

Implementation Plan

Phase 1 — Service scenarios (S1–S7) ✅ MoneyPathServiceTest (22 tests)

Phase 2 — API scenarios (A1–A5) ✅ MoneyPathApiTest (18 tests)

Phase 3 — Fixtures ✅ MoneyPathFixtures

Findings

Writing the coverage surfaced five defects. F1 is fixed; the rest are pinned by characterisation tests whose comments state the condition that should invert them, so a fix surfaces visibly.

# Finding Status
F1 Approving an import whose payload has no COMPLETED extraction derived a source document with no extraction run, violating ksr_doc_requires_extraction_ck (V44) and aborting the approval after the krithi was committed — orphaned krithi, import left PENDING, retry could duplicate it. Fixed — the document node is only derived when an extraction is resolvable; otherwise the revision is still written, curator-attributed. Guarded by S6c.
F2 saveKrithiSections mutates exactly the state revisions capture but wrote no revision, so section edits left no recovery point. Fixed — both curator section-edit paths (saveKrithiSections, saveLyricVariantSections) now snapshot the resulting state as a CURATOR_EDIT revision attributed to the JWT user, inside one transaction with the save and the audit row. An unattributed call still saves but records no revision (ADR-014 requires attribution) rather than failing the edit. Guarded by three S6 tests. Still open: KrithiRevisionDto carries no krithi-level metadata, so a title/raga change is not recoverable from history — revisions are section-scoped by design (ADR-014). Extending them to metadata is a TRACK-117 decision.
F3 No route checks the roles claim; authenticate("admin-auth") validates only signature, audience and a userId claim. Any validly-signed token — including one with no roles — had full admin access. Compounded by POST /v1/auth/token minting caller-supplied roles behind the shared ADMIN_TOKEN. Fixed — see below. Guarded by five A1 tests.
F4 reviewImport’s broad catch (e: Exception) re-wraps NoSuchElementException as RuntimeException("Failed to create krithi: …"), so a missing import returns 500 instead of 404 and leaks internal phrasing to the caller. Fixed — the catch now lets NoSuchElementException (404) and IllegalArgumentException (400) propagate with their own identity; only genuinely unexpected failures are wrapped. Guarded by two A4 tests.
F6 DatabaseFactory.dbQuery did not join an enclosing transaction — every nested call opened its own and committed independently. A service could not make a multi-repo operation atomic by wrapping it: the wrap compiled, read as a transaction boundary, and did nothing. reviewImport relied on exactly that and could leave a committed krithi behind when a later step failed. Fixed — see below. Guarded by DbQueryNestingTest (4 tests) and S6d.
F5 AutoApprovalService treats an unparseable duplicateCandidates payload as “no duplicates” and auto-approves — fail-open on a deduplication guard. Open — pinned by S2 GAP test.

The authorisation fix (F3) — what changed

Three parts, all in this track’s scope; the shared-ADMIN_TOKEN login itself remains TRACK-119’s to retire, since replacing it needs OAuth/OTP.

  1. Roles come from storage. POST /v1/auth/token reads the user’s role_assignments instead of copying the request’s roles list into the JWT. The roles field is removed from AuthTokenRequest; ignoreUnknownKeys is on, so a client still sending it is ignored rather than rejected. This closes the escalation: the shared token no longer mints arbitrary roles.
  2. A 403 tier exists. Route.requireRole (a route-scoped plugin on Ktor’s AuthenticationChecked hook) gates every admin route on grp_sangita_admin. It deliberately does nothing when there is no principal — otherwise an anonymous request would get 403 instead of the auth plugin’s 401, which both pre-empts the challenge and misreports the problem. 401 and 403 are now distinct and tested as such.
  3. Refresh re-reads roles. /v1/auth/refresh no longer carries the old token’s claim forward, so a revoked role cannot be renewed indefinitely. Refresh stays outside requireRole so a caller whose role was revoked can still reach it.

Role taxonomy is unchanged and remains a TRACK-119 decision. R__seed_01_reference.sql defines exactly one role, so authorisation is a single admin tier — the viewer/curator/admin matrix A1 originally imagined still has nothing to bind to. The role code now lives in one place (support/Roles.kt), previously duplicated in BootstrapAdmin.

Operational note. Any user without grp_sangita_admin now gets 403 on admin routes where they previously had full access. bootstrap-admin assigns the role and no users are seeded, so a correctly bootstrapped environment is unaffected; users created through the user-management API need an explicit role assignment. Because enforcement reads the token claim, a role revoked mid-session stays effective until the token expires (24h default) unless the client refreshes — closing that window means a per-request storage check or a shorter TTL, which is a TRACK-119 call.

The transaction fix (F6) — what it took

Wrapping reviewImport in dbQuery was not sufficient, and would have been a silent no-op. An empirical probe against Exposed 1.0 established that a nested newSuspendedTransaction opens its own transaction and commits independently, while the ambient transaction is visible to nested suspend calls. So the fix had to go in DatabaseFactory.dbQuery itself: if TransactionManager.currentOrNull() returns a transaction, join it instead of opening a new one.

Blast radius was checked before changing shared infrastructure: no dbQuery block anywhere in the api module currently nests a dal.* repo call, so joining is a no-op for all existing code and only affects code that deliberately nests.

Making the promotion atomic then exposed a second problem. With one transaction per approval, two concurrent approvals of the same import no longer see each other’s uncommitted krithi under READ COMMITTED, so both created one — S7 started failing with 2 krithis. The old code had only avoided this by accident: incremental commits narrowed the window, they did not close it. The fix is a proper one — ImportRepository.findByIdForUpdate takes a row lock on the import for the duration of the promotion, so the second approval blocks, then observes the import already APPROVED and adopts the krithi the first one produced.

Test-quality note. The first atomicity test written for this was vacuous: it used a composer-less import as the failure trigger, which throws before anything is written, so it passed with the fix disabled. It was replaced with a trigger that fails late — an unresolvable reviewer id, which fails the ADR-014 revision write after the krithi, sections, junction rows and source evidence are all written. Both S6d and DbQueryNestingTest were verified to fail with the fix reverted.

Acceptance Criteria

References