[DT-3863] Reconcile legacy Data Use and recompute affected matches

[kevinmarete]

Addresses

DT-3863. Its only dependency, DT-3864, is already merged. Scoped by the DT-3861 audit.

Security risk: low.

Summary

Recomputes the persisted matches affected by the legacy noncanonical Data Use population. One dataset additionally needs its Data Use corrected; that needs a domain decision and is not done here.

What this PR does

Two admin-only endpoints and the driver behind them:

• GET /api/datause/legacy/noncanonical — the datasets whose shape the canonical validator would reject, by id and classification label.
• POST /api/datause/legacy/recomputeMatches — recomputes stored matches under V5, and returns the classification counts either side of the run so a recompute-only run can be seen to have changed no stored Data Use. Rewrites match rows and rationales only, never elections, final votes, or historical automation votes.

Why the recompute is safe to merge now: DataUsePrimaryValidator accepts any single primary, Other included, so the 566 SINGLE(OTHER) and 509 open-access NONE datasets are valid — DataUseMatcherV5 merely abstains on them. Their stored matches predate that policy and need recomputing; their data does not need changing, so no approved disposition is involved.

Restartable: the run writes no stored Data Use, so a rerun simply recomputes the same matches, and the response lists the datasets that failed so a rerun can be scoped to them.

What needs approval

Exactly one dataset is invalid — MULTIPLE(HMB+OTHER) under controlled access, the only one of 5,811 the audit found. DT-3861's review labelled it Mixed and recorded that mixed cases must not be normalized without an explicit domain-approved disposition.

@jlaw-codes @ncalvanese1 — as Data Use domain owners, could one of you approve on DT-3863 either:

1. the recommendation below, or
2. a different DataUse value, with where the approval is recorded.

Recommendation: keep hmbResearch as the single primary and move the Other text to secondaryOther, giving SINGLE(HMB). The reviewer labelled it Mixed because the text reads as a use restriction rather than a primary category, and HMB is already asserted on the record — so this invents no category the reviewer did not choose. This is a proposal, not a decision taken.

Merging before that approval is intentional: until a disposition exists V5 abstains on the record, so the DAC sees ABSTAIN with a manual-review rationale rather than an automated decision. Nothing in this PR is blocked on it.

After approval

No code change. Four calls:

1. GET /api/datause/legacy/noncanonical — the dataset id.
2. PUT /api/dataset/{id}/datause — applies the approved value; validates, translates, audits, syncs the index.
3. POST /api/datause/legacy/recomputeMatches — the response carries its own before/after reconciliation.
4. GET /api/datause/legacy/noncanonical — empty.

Testing

Full suite: 4626 tests, 0 failures, 1 pre-existing skip. Spotless clean, no lenient() stubbing, synthetic data only. PersistedDataUseDAOTest runs against the real schema. New classes are at 100% instruction and branch coverage.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[otchet-broad]

Has this been tested against the production database? It would be good to understand the number of changes that are going to happen in that environment. I know there's a dry run feature, but if we've got branches in code for dev-only cases, it would be good to remove them and minimize what we need to do here.

[kevinmarete]

Yes — DT-3861 ran read-only against the production database (all 5,811 datasets, no active/inactive filter), and those are the numbers in the description. The production change volume is bounded and small:

• No stored Data Use is written. The recompute rewrites only match_entity rows and their rationales — never elections, final votes, or historical automation votes.
• 19 datasets are both on an ABSTAIN shape and reachable through a DAR, which is the only population the recompute touches. The audit counted 52 mapped persisted-match rows across them (39 involving Other, 13 involving NONE), out of 1,085 persisted matches in total.
• The response carries its own before/after classification counts, so a run that had changed any stored Data Use would show up in its own output.

On dev-only branches: there are none — no dry-run flag and no environment conditional anywhere in the diff. The read-only GET /noncanonical is the only preview, and it is part of the production sequence rather than a dev affordance.

There was a path we will never exercise, though, and cda4b1a removes it (−299 lines). LegacyDataUseDisposition's normalize/defer machinery was never constructed by any endpoint, since the one dataset needing correction goes through the existing PUT /api/dataset/{id}/datause. That took with it the skipped counter (structurally always 0), the failuresByReason map (only one reason could ever occur), the validation and not-found failure branches, and the DatasetService dependency. What remains is a single loop: recompute per DAR, retry once, report what failed.

[fboulnois]

Findings:

Correctness

1. findDarReferenceIdsByDatasetId is unfiltered by DAR state in PersistedDataUseDAO.java:59

The recompute creates match rows for unsubmitted draft DARs, and wipes historical V1/V4 match rows on archived DARs with closed elections. The dar_usage CTE (PersistedDataUseDAO.java:42) counts drafts too, which inflates darCount and needsMatchRecompute.

2. apply() commits the Data Use write before the match recompute in LegacyDataUseService.java:187

A recompute failure reports the record as failed ("left unchanged") even though the Data Use write already committed. The retry then writes the Data Use a second time, producing a duplicate audit entry and a duplicate ES sync.

3. isAlreadyApplied skips the match recompute in LegacyDataUseService.java:153

An already-normalized record is marked skipped including its recompute. This contradicts the javadoc at line 150 ("a recompute always runs") and permanently strands the bad match rows from finding 1 on rerun.

4. access_management CTE diverges from Dataset.getAccessManagement in PersistedDataUseDAO.java:30

The CTE prefers any non-null canonical value — including blank or non-enum values — and never falls back to the legacy property. This produces false noncanonical reports whose proposed "fix" DataUsePrimaryValidator then rejects.

Consistency and efficiency

5. Recompute is per-dataset rather than per-DAR in LegacyDataUseService.java:196

A DAR shared across datasets is deleted and rebuilt once per dataset, and matchesRecomputed double-counts instead of counting distinct DARs.

6. findAllPersistedDataUse() runs three times per invocation — LegacyDataUseService.java:81

Called for the pre-report, the candidate list, and the post-report. This contradicts the DAO's "read once per run" note and makes reconcilesWith / leftClassificationsUnchanged false under concurrent dataset writes.

7. Normalize.approvalReference is never persisted or logged in LegacyDataUseDisposition.java:16

The traceability its javadoc promises does not exist.

8. accessManagementLabel() can emit values outside the OpenAPI enum in PersistedDataUseRow.java:16

It also uses default-locale toLowerCase() where the rest of the codebase uses Locale.ROOT.

[kevinmarete]

Thanks — findings 1 and 4 were both real, and 1 was worse than written up. All eight are now addressed across three commits.

Already gone in cda4b1a, which landed just before you submitted: findings 2, 3, and 7 all lived in the disposition path. Normalize and Defer were never constructed by any endpoint, so that commit deleted the type, apply(), isAlreadyApplied(), the skipped counter, and the failuresByReason map. The write-then-recompute ordering, the skip-swallows-recompute contradiction, and the unpersisted approvalReference went with them.

f01f7a4 — finding 1. Correct, and the archived case is data loss rather than churn: reprocessMatchesForPurpose deletes unconditionally but rebuilds through findByReferenceId, which filters archived out — so the delete lands and the insert does not. Both queries now filter to submitted, unarchived requests using the same predicate findByReferenceId applies, which keeps reachability and rebuildability in lockstep rather than merely agreeing today.

f01f7a4 — finding 4. Correct. Dataset#getAccessManagement falls back to the legacy property when the canonical value does not parse as an AccessManagement; the CTE accepted any non-null value, so a blank or non-enum one blocked the fallback. It now aggregates only values in the enum, which as a side effect confines accessManagementLabel() to the four values its schema declares — so finding 8's enum concern is closed too, and the toLowerCase is now Locale.ROOT.

d056a4c — finding 5. A run-scoped set of reprocessed reference ids: each DAR is rebuilt once and matchesRecomputed counts distinct DARs. Each is recorded only after it succeeds, so a retry stays scoped to what is still outstanding.

d056a4c — finding 6. Three reads down to two; the candidate list reuses the rows the before report is built from. Two is the floor, since the after read has to be fresh to reconcile.

Worth flagging on the production numbers I posted above: excluding drafts and archived requests can only shrink that 19-dataset population, so the bound still holds.

The four new PersistedDataUseDAOTest cases — draft exclusion, archived exclusion, and the two unusable-value paths — are verifying in CI rather than locally, as I had no Docker daemon up.

[otchet-broad]

Feedback from Claude:

1. A failed rebuild leaves a DAR with zero match rows

src/main/java/org/broadinstitute/consent/http/service/LegacyDataUseService.java:113 · correctness

reprocessMatchesForPurpose deletes before it inserts, each MatchDAO call auto-commits, and the failure is swallowed — yet the run reports the dataset as "left unchanged".

Dataset 16 has DAR ref-a. recomputeMatches calls matchService.reprocessMatchesForPurpose("ref-a"), which runs matchDAO.deleteRationalesByPurposeIds + deleteMatchesByPurposeId (each its own auto-commit statement on jdbi.onDemand), then insertMatches throws (deadlock, connection reset, constraint error). recomputeWithOneRetry catches, calls reprocessMatchesForPurpose("ref-a") again — deleting a second time — and it throws again. Outcome(true, 0, true) is returned; the response reports failed=1, failedDatasetIds=[16]. ref-a now has no match_entity and no match rationale rows at all.

LegacyDataUseRunReport.java:12 documents failed as "datasets left unchanged because every attempt failed" and legacyDataUseRecomputeMatches.yaml says "Restartable" and "rewrites only match rows" — both are false for this path. MatchDAO already extends Transactional<MatchDAO>, so wrapping the delete+insert in matchDAO.useTransaction was available and unused.

---

2. Exception messages are echoed to the client, leaking Other free text

src/main/java/org/broadinstitute/consent/http/resources/LegacyDataUseResource.java:41 · data leak

Both endpoints funnel exceptions into Resource#createExceptionResponse, which logs and returns e.getMessage() — the exact leak every other class in this PR is written to prevent.

Resource.java:249-268: createExceptionResponse calls logWarn("Returning error response to client: " + e.getMessage()) and builds new Error(e.getMessage(), 500) as the response body. The PR's stated invariant is the opposite:

• PersistedDataUseClassifier.java:32 — "Deliberately no cause or payload: the value can hold Other free text"
• LegacyDataUseService.java:105 — "No message or cause: a failure raised while matching can quote the Other free text"
• LegacyDataUseRunReport.java:8 — "failures carry a dataset id, never an exception message"

Any exception that escapes the service — a JDBI exception whose message quotes a row, or an NPE/IllegalArgumentException from PersistedDataUseClassification's EnumSet.copyOf on a malformed category list — is echoed verbatim to the admin client and into the application log, bypassing the redaction the rest of the diff enforces. The classes are careful; the boundary is not.

---

3. matchesRecomputed undercounts on partial success

LegacyDataUseService.java:126 · correctness

The recomputed set suppresses the already-done DARs from the retry's return value.

Dataset X's findDarReferenceIdsByDatasetId returns [a, b, c]. reprocessMatchesForPurpose succeeds for a and b (both added to recomputed), then throws for c. recomputeWithOneRetry retries: recomputeMatches skips a and b via the recomputed.contains guard, calls c, which succeeds → returns count=1. The report credits 1 DAR when 3 were actually rewritten.

If the retry of c throws too, Outcome(true, 0, true) credits 0 and lists X in failedDatasetIds, so the response asserts nothing changed while a's and b's match rows were replaced. LegacyDataUseRunReport.yaml documents matchesRecomputed as "Distinct Data Access Requests whose matches were recomputed", which the value does not equal.

---

4. MAX() picks the wrong access-management value on duplicate rows

src/main/java/org/broadinstitute/consent/http/db/PersistedDataUseDAO.java:34 · correctness

MAX(LOWER(TRIM(property_value))) picks the lexicographically greatest access-management value, whereas Dataset#getAccessManagement picks the first in property order — so a dataset with duplicate rows for one schema_property is graded against the wrong canonical shape.

A dataset has two dataset_property rows with schema_property='accessManagement', values 'controlled' and 'open' (PersistedDataUseDAOTest.canonicalPropertyWinsOverTheLegacyOne proves duplicate schema_property rows on one dataset are insertable — it inserts two properties both with property_key=1, so nothing prevents two rows for the same schema_property). The CTE returns MAX = 'open'; Dataset#getAccessManagement's parseAccessManagementProperty uses .findFirst() over the properties list and returns CONTROLLED.

With data_use = {"generalUse":true}: PersistedDataUseRow.isOpenAccess() is true → isCanonicalFor(true) requires State.NONE → the dataset is reported noncanonical over /noncanonical, while DataUsePrimaryValidator.validate(dataUse, CONTROLLED) accepts it. The class javadoc at lines 19-22 claims the fallback behaves "as Dataset#getAccessManagement does", so this is a contract violation, not just an edge case.

---

5. The access-management key match is case-sensitive

PersistedDataUseDAO.java:40 · correctness

dp.schema_property IN ('accessManagement', 'consentGroup.accessManagement') is case-sensitive, but Dataset#parseAccessManagementProperty matches with equalsIgnoreCase — a differently-cased stored key is invisible to the report.

A dataset_property row stored with schema_property='AccessManagement' (or 'consentGroup.AccessManagement') and property_value='open': Dataset.parseAccessManagementProperty uses schemaProperty.equalsIgnoreCase(property.getSchemaProperty()), so the running application resolves AccessManagement.OPEN and DataUsePrimaryValidator requires Shape.NONE. The DAO's exact-match IN clause skips the row, so access_management is NULL → accessManagementLabel() = "missing" and isOpenAccess() = false → isCanonicalFor(false) requires State.SINGLE.

A data_use of {} is then reported as noncanonical ("NONE under missing") when the app considers it canonical, and a data_use of {"generalUse":true} is reported canonical when the app rejects it. The cross-tab in countsByAccessManagement also drops the dataset into the wrong "missing" bucket.

---

6. processed counts datasets that rebuilt nothing

LegacyDataUseService.java:92 · correctness

processed counts datasets that raised no exception, not datasets whose matches were recomputed.

The PR's own test datasetWithNoDarsChangesNoMatches asserts processed=1 / matchesRecomputed=0 for dataset 19, whose findDarReferenceIdsByDatasetId returns []. The same happens in production two ways:

1. A candidate whose only DAR was already rebuilt by an earlier candidate in the same run (all reference ids in recomputed).
2. A TOCTOU window — findAllPersistedDataUse reports darCount=1 so needsMatchRecompute() is true, then the DAR is archived before findDarReferenceIdsByDatasetId runs, which now excludes it.

Both increment processed with zero DARs touched. LegacyDataUseRunReport.java:11 and LegacyDataUseRunReport.yaml both document processed as "Datasets whose matches were recomputed", so an operator reading processed=200 cannot tell how many datasets were actually rebuilt.

---

7. An unbounded full-population write runs synchronously on the request thread

LegacyDataUseResource.java:49 · robustness

A client or gateway timeout abandons the run mid-flight and discards the only record of what it did.

POST /api/datause/legacy/recomputeMatches does, in one request: findAllPersistedDataUse over every dataset row, then per abstaining candidate a findDarReferenceIdsByDatasetId query, then per reachable DAR a reprocessMatchesForPurpose (delete rationales + delete matches + findByReferenceId + a findDatasetById and a V5 match per dataset on the DAR + N inserts), then a second full findAllPersistedDataUse. With a few thousand noncanonical datasets this is tens of thousands of statements.

The load balancer returns 504 at its idle timeout; the thread keeps deleting and reinserting match rows, and the LegacyDataUseRunResult — including failedDatasetIds, the only thing that lets a rerun be scoped — is never delivered, so the operator cannot tell which datasets completed. This codebase already injects ExecutorService for bulk work (DatasetRegistrationService, DACAutomationRuleService, DacDashboardService); nothing here uses it, and no batching or limit parameter is offered.

---

8. isComplete(candidateCount) is a tautology

src/main/java/org/broadinstitute/consent/http/models/datause/LegacyDataUseRunReport.java:25 · correctness

It can never be false for a report produced by run(), so the "rerun decision" its javadoc says rests on it rests on nothing.

LegacyDataUseService.run (lines 86-97) increments exactly one of processed or failedDatasetIds per candidate, and constructs the report with failed = failedDatasetIds.size(). So processed + failed == candidates.size() by construction, for every input including a run where every dataset failed. The only way isComplete returns false is a caller passing a count that is not candidates.size() — which is what the tests do (emptyCandidateListIsComplete asserts isComplete(1) is false on an empty run). No production code calls it.

The javadoc "Every record accounted for, which is the check a rerun decision rests on" advertises a safety property the method cannot provide; an operator trusting it gets a green light regardless of outcome.

---

9. Six new public methods have no production caller

src/main/java/org/broadinstitute/consent/http/models/datause/LegacyDataUseRunResult.java:11 · dead code

Called only by their own tests — including the two the javadoc and OpenAPI present as the run's reconciliation guarantee:

• leftClassificationsUnchanged() — LegacyDataUseRunResult.java:11
• reconcilesWith() — PersistedDataUseReport.java:78
• percentage() — PersistedDataUseReport.java:70
• isComplete() — LegacyDataUseRunReport.java:25
• findNoncanonicalRows() — LegacyDataUseService.java:44
• findRowsNeedingMatchRecompute() — LegacyDataUseService.java:56

Nothing in recomputeAbstainingMatches ever evaluates leftClassificationsUnchanged() or reconcilesWith() before returning, so the invariant legacyDataUseRecomputeMatches.yaml states ("a recompute-only run can be seen to have left them unchanged") is only checkable by a human eyeballing the JSON — the code that could enforce it exists and is bypassed. findRowsNeedingMatchRecompute is dead in a stricter sense: it is the exact filter recomputeAbstainingMatches inlines at line 69. findNoncanonicalRows is public and returns rows carrying the raw data_use (Other free text) even though only the redacted view is meant to leave the service.

---

10. data_use JSON is re-parsed roughly 8× per dataset per run

src/main/java/org/broadinstitute/consent/http/models/datause/PersistedDataUseRow.java:27 · efficiency

classification() re-runs a Gson parse of the raw data_use on every call; the codebase already has a memoizing parser (DataUseParser) and a shared Gson (GsonUtil.getInstance()).

One recomputeAbstainingMatches call over N datasets: PersistedDataUseReport.from parses each row 3× (countByLabelDescending at line 37, the groupingBy's countByLabelDescending at line 45, and !row.isCanonical() at line 33), and from() runs twice (before at line 65, report() at line 75) = 6 parses/row; plus row.classification().abstainsWhenMatched() at line 67 and needsMatchRecompute() at line 69 = 8 parses/row. findNoncanonicalViews adds 3 more per row (isCanonical, classification().label(), needsMatchRecompute()).

DataUseParser exists for exactly this column and memoizes via a ConcurrentMap keyed on the raw string; PersistedDataUseClassifier.java:16 instead builds a fresh, uncached GsonUtil.gsonBuilderWithAdapters().create() when GsonUtil.getInstance() already returns a shared instance. Caching the classification on the row (or reusing DataUseParser's cache) removes 7 of the 8 parses.

---

11. The second full-table read buys no real reconciliation evidence

LegacyDataUseService.java:75 · efficiency

The "after" report is built from a second full-table read even though the run provably writes no data_use, so it can only ever detect an unrelated concurrent edit — at the price of doubling the most expensive query in the endpoint.

recomputeAbstainingMatches derives before from the rows it already holds (line 65) and after from report() (line 75), which re-issues findAllPersistedDataUse over every dataset and re-parses every data_use 3×. Nothing between the two reads writes dataset.data_use — the only mutation is match_entity/match rationale rows. So before and after are identical unless a separate admin happens to PUT /api/dataset/{id}/datause during the window, in which case leftClassificationsUnchanged() reports false and blames the recompute for a change it did not make.

The PR's own test asserts verify(persistedDataUseDAO, times(2)).findAllPersistedDataUse(), locking the double read in. Reusing before for after (or checksumming instead of re-reading) gives the same guarantee at half the cost and without the false positive.

---

12. Selection filters are duplicated in the service and on the row

LegacyDataUseService.java:66 · simplification

Lines 66-70 build abstaining = rows.filter(row.classification().abstainsWhenMatched()) then candidates = abstaining.filter(needsMatchRecompute), while findRowsNeedingMatchRecompute (line 56) is already exactly the second filter and needsMatchRecompute (PersistedDataUseRow.java:37) is already abstainsWhenMatched() && darCount > 0.

The only thing the local abstaining list buys is the log line's abstaining.size() - candidates.size() count, obtainable as a single partitioningBy or by counting darCount==0 among candidates' complement. If the reachability rule in needsMatchRecompute changes (say, to exclude datasets with no DAC), the inlined pair here silently keeps the old two-step meaning and the log's "left alone" figure stops matching. Likewise findNoncanonicalRows (line 45) duplicates PersistedDataUseReport.from's own !row.isCanonical() filter (PersistedDataUseReport.java:33).

---

13. A raw DAO is passed as a service constructor parameter

LegacyDataUseService.java:34 · convention

docs/ai/CLAUDE.md ("DAO instantiation") states: "All Jdbi SQL object DAOs (interfaces in org.broadinstitute.consent.http.db) must be instantiated inside the service constructor via jdbi.onDemand(XxxDAO.class). Never pass a raw DAO as a constructor parameter to a service."

Line 34 is LegacyDataUseService(PersistedDataUseDAO persistedDataUseDAO, MatchService matchService) — PersistedDataUseDAO is an interface in org.broadinstitute.consent.http.db, passed as a constructor parameter. The root CLAUDE.md pulls this file into scope ("For guidelines on specific components, review the documents in the docs/ai/ directory"). Only one other service in the package does anything similar (ElasticSearchCapabilityService); the repo's normal escape hatch for testability is @VisibleForTesting (used in DACAutomationRuleService, DaaService, DarCollectionService) or mocking Jdbi.onDemand.

---

14. PersistedDataUseReport stores its maps without a defensive copy

src/main/java/org/broadinstitute/consent/http/models/datause/PersistedDataUseReport.java:22 · correctness

Unlike its sibling LegacyDataUseRunReport, which copies its list, a directly-constructed report is mutable through the caller's reference.

LegacyDataUseRunReport.java:20 adds a compact constructor doing failedDatasetIds = List.copyOf(failedDatasetIds); PersistedDataUseReport has no compact constructor, so new PersistedDataUseReport(1, mutableMap, mutableNestedMap, 0, 0, 0) — the shape LegacyDataUseResourceTest.runResult already uses — keeps the caller's map live. Any future caller (or a caller mutating the map after construction) changes a record that leftClassificationsUnchanged() compares by equals(), so the pre/post comparison can silently agree or disagree based on post-construction mutation.

Note the copy must not be Map.copyOf for the values produced by from(), which are intentionally insertion-ordered (see the comment at line 65); an unmodifiable LinkedHashMap/TreeMap copy is needed.

[kevinmarete]

Thanks — ten of the fourteen are fixed across three commits. Four are answered below rather than changed, and two of the write-ups do not hold as described.

#	Outcome
1	Fixed in fdaa7f0f. The rebuild is computed before anything is deleted, and the delete and insert share one matchDAO.useTransaction. This is the path DataAccessRequestResource and MatchResource take as well, so the hole closes for them too.
3	Fixed in 5c358431. The count comes from the growth of the run's rebuilt set rather than from one attempt's return value, and is credited whether or not the dataset went on to fail.
5	Fixed in 3dbe71f7. LOWER(dp.schema_property), matching the equalsIgnoreCase in Dataset#parseAccessManagementProperty.
6	Fixed in 5c358431. processed now says it counts datasets that completed without a failure, and a new unchanged reports the subset that rebuilt nothing.
8	Deleted in 5c358431.
9	Fixed in 5c358431, except as noted below. percentage, reconcilesWith, isComplete and findRowsNeedingMatchRecompute are gone; findNoncanonicalRows is inlined so rows carrying the raw value no longer leave the service; and leftClassificationsUnchanged is now evaluated by the run, which warns when a recompute-only run moved a record.
10, 12	Fixed in 5c358431. One parse per row per pass instead of roughly eight, and the duplicated two-step filter collapses into one partition over that parse.
13	5c358431 marks the constructor @VisibleForTesting, the escape hatch DACAutomationRuleService and DaaService use. The parameter itself stays, so this is the convention's spirit rather than its letter.
14	Fixed in 5c358431, with order-preserving copies rather than Map.copyOf, as you noted.

Two corrections.

findNoncanonicalRows did have a production caller — findNoncanonicalViews at LegacyDataUseService.java:52. Five of the six were genuinely uncalled outside tests; that one was not. It is inlined now regardless, which is the better answer to the point underneath it: the method returned rows carrying the raw value.

The mechanism named in #2 cannot fire. PersistedDataUseClassification's compact constructor guards categories.isEmpty() before reaching EnumSet.copyOf, so the malformed-category-list throw is not available. The classifier also catches every parse failure and returns UNPARSEABLE, so no exception message from this code can carry the raw value.

#2, the leak boundary. The observation itself stands: createExceptionResponse does return e.getMessage(), and that is every resource in the repo, not these two endpoints. What can still reach it here is a database-layer message from findAllPersistedDataUse or findDarReferenceIdsByDatasetId — those quote SQL and bind values, not data_use — since anything raised inside the run is caught per candidate and reported as an id. Redacting one resource while the shared error path stays as it is would leave the codebase less consistent, not safer; the fix worth making is to Resource#createExceptionResponse for every caller, and it is not this ticket.

#4, MAX() versus findFirst(). The premise holds — I found no unique constraint on dataset_property over (dataset_id, schema_property), so two rows for one key are insertable, and the two would then disagree. They agree for every dataset with at most one row per key, which is what the registration path writes. Making the CTE order-sensitive to match findFirst means picking by dataset_property_id, which encodes an assumption about insertion order that Dataset only satisfies by accident of how the properties list is loaded. I would rather confirm from the DT-3861 audit whether any dataset actually has duplicates before trading a deterministic aggregate for a positional one; if any do, the honest fix is a constraint, not a matching ORDER BY.

#7, the synchronous run. True as written, and the numbers bound it: the audit found 19 datasets both on an abstaining shape and reachable through a DAR, across 52 mapped persisted matches. That is seconds of work, not the tens of thousands of statements the finding sizes for. The part of it that survives is real though — if the request does time out, the failedDatasetIds are lost even though the writes are consistent. What makes that recoverable rather than dangerous is that a rerun is safe by construction, and after fdaa7f0f an abandoned run leaves every DAR either fully rebuilt or untouched. An executor and a batch limit are the right answer if this population ever grows; at 19 datasets they would be scaffolding around a query that finishes before the load balancer notices.

#11, the second read. Keeping it. Your false-positive point is the good half of the argument, and it is now handled: the run evaluates leftClassificationsUnchanged() and logs a warning, so a concurrent PUT /datause shows up as a warning to investigate instead of silently sitting in the response for an operator to misread. The cost half is much smaller than it was — the second read now parses each row once rather than three times — and a checksum over the same full-table read saves nothing.

recomputeWithOneRetry catches Exception, which is broad enough to treat a programming error as infrastructure worth retrying — it swallowed a Mockito strict-stubbing error of mine and reported the dataset as a transient failure. d39d21df makes both attempts log the exception's class, not its message, which is what the redaction protects.

Tests run: 100 green across the six suites this touches, including PersistedDataUseDAOTest against a real Postgres container. The two accounting tests fail against the previous code with expected: <1> but was: <0> and expected: <2> but was: <1>.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud