Skip to content

[DT-4378][risk=low] Add admin endpoints to resync study asset records - #3226

Open
kevinmarete wants to merge 4 commits into
developfrom
km-dt-4378-sync-study-endpoint
Open

kevinmarete wants to merge 4 commits into
developfrom
km-dt-4378-sync-study-endpoint

Conversation

@kevinmarete

Copy link
Copy Markdown
Contributor

Addresses

https://broadworkbench.atlassian.net/browse/DT-4378

Security risk: low — two new admin-only endpoints that rerun an existing, reviewed sync; no change to who can read or write studies.

Summary

StudyAssetRecordSync.syncStudy (#3213) had no caller. This adds the trigger the plan's Rollback steps need, and DT-4331 will use the bulk call to convert existing studies instead of a SQL changeset.

  • POST /api/dataset/study/{studyId}/assets/sync: runs syncStudy for one study in its own transaction, then reindexes the study's datasets. syncStudy's 404 (no study) and 400 (a biospecimen in row-id form with no sampleId) pass through.
  • POST /api/dataset/study/assets/sync?since=<ISO-8601 instant>: every study, or each study with create_date or update_date at or after since (new studies have a null update_date). One transaction per study in id order. Streams one result per line as application/x-ndjson, then a summary line; a failing study is reported and the run continues, and the run also continues if the caller disconnects.
  • The plan's Rollback section names both calls.

Differences from the design in the ticket comment:

  • The single-study call lives in StudyAssetResource. That class owns api/dataset/study/{studyId}/assets, and Jersey picks the root class with the longer literal prefix and does not fall back, so the route on StudyResource answered 404.
  • The reindex covers only datasets already in the index (indexed_date IS NOT NULL), not indexStudy, which indexes every dataset of the study and would re-add one an admin removed. The result reports index as UPDATED, NOT_INDEXED or FAILED; a bulk request that answers 200 with "errors": true counts as FAILED.

Testing

  • StudyResourceTest, StudyAssetResourceTest, DatasetServiceTest, StudyDAOTest, StudyAssetRecordSyncTest: admin-only on both calls, error pass-through, a malformed since, the loop continuing past a failed study and a dropped client, reindex outcomes, the since query (inclusive, new and edited studies), and a sync that rolls back when a later asset type fails.
  • mvn validate passes the OpenAPI spec.

Have you read CONTRIBUTING.md lately? If not, do that first.

  • Label PR with a Jira ticket number and include a link to the ticket
  • Label PR with a security risk modifier [no, low, medium, high]
  • PR describes scope of changes
  • Get a minimum of one thumbs worth of review, preferably two if enough team members are available
  • Get PO sign-off for all non-trivial UI or workflow changes
  • Verify all tests go green
  • Test this change deployed correctly and works on dev environment after deployment

🤖 Generated with Claude Code

POST /api/dataset/study/{studyId}/assets/sync runs syncStudy for one study;
POST /api/dataset/study/assets/sync?since= runs it for every study, or those
changed since an instant, one transaction each, streaming per-study results.
Each reindexes only the study's datasets already in the search index.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kevinmarete kevinmarete self-assigned this Oct 30, 2026
@kevinmarete
kevinmarete requested a balanced review from Copilot October 30, 2026 20:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

kevinmarete and others added 2 commits October 30, 2026 16:43
…s 500

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@kevinmarete
kevinmarete marked this pull request as ready for review October 30, 2026 22:11
@kevinmarete
kevinmarete requested a review from a team as a code owner October 30, 2026 22:11
@kevinmarete
kevinmarete requested review from fboulnois, otchet-broad and rushtong and removed request for a team October 30, 2026 22:11

@rushtong rushtong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, only low findings 👍🏽

  • Low: The bulk resync runs on one request thread. A run over all studies can take longer than the ingress timeout. The summary then goes only to the server log.
  • Low: If a study is deleted during the run, the summary counts it as failed with a 404.
  • Low: The since filter finds only the edits that change study.update_date.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants