Add dry run = true FIX - #206
Merged
Jagadeeshftw merged 2 commits intoJul 27, 2026
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
================================================================================
PULL REQUEST
TITLE
feat(anchors): add ?dryRun=true preflight validation to POST /api/v1/anchors/bulk
BRANCH
feat/bulk-anchors-dry-run -> main
COMMIT
8957953 feat(anchors): add ?dryRun=true preflight validation to POST /anchors/bulk
================================================================================
SUMMARY
AnchorService.registerBulk already validated an entire batch (duplicate ids,
conflicts with the existing registry) before persisting any of it. This PR
exposes that same validation as a read-only preflight check so integrators
building an onboarding UI can show inline errors before committing a batch.
Passing ?dryRun=true to POST /api/v1/anchors/bulk runs the identical
validation and reports the identical outcome, but persists nothing.
================================================================================
WHAT CHANGED
Separated the validate and persist phases
The validation phase of registerBulk was extracted into a new private,
side-effect-free method validateBulk(). Both dry-run and real calls go
through it, so the two paths cannot drift apart as validation rules evolve
in the future. This is the core of the fix: previously there was no seam
between "check the batch" and "write the batch".
Added a dryRun parameter to registerBulk
registerBulk(input: unknown, dryRun = false): Anchor[]
When dryRun is true, the method validates the batch and returns the records
that WOULD have been created, then stops. this.repo.upsert is never invoked
for any entry. The parameter is defaulted, so every existing caller is
unaffected.
Added strict flag parsing (optionalBooleanFlag)
New helper in src/utils/validation.ts. Only "true" and "false" are
accepted, in any casing and with surrounding whitespace tolerated. An
absent value defaults to false.
Anything else is a 400: "yes", "1", "on", a typo like "ture", a bare
?dryRun with no value, or a repeated ?dryRun=true&dryRun=true (which
Express turns into an array).
This is deliberate rather than defensive boilerplate. With lenient parsing
a typo would fall through to "not set" and perform a real, persisting
write at the exact moment the caller explicitly asked for a preflight.
Failing loudly is the safe direction for a flag whose whole purpose is
preventing a side effect.
Route wiring
POST /bulk reads req.query.dryRun, passes it through, and responds
201 with { anchors, dryRun }. The dryRun boolean lets a client confirm
whether the batch was actually committed. It is an additive field, so
existing clients reading response.anchors are unaffected.
Documentation
OpenAPI spec gains the dryRun parameter and a description of the preflight
behaviour. README documents the flag, its strict parsing, and the response
shape.
================================================================================
API BEHAVIOUR
POST /api/v1/anchors/bulk?dryRun=true
{ "anchors": [{ "id": "a1" }, { "id": "a2", "name": "Two" }] }
-> 201 Created
{ "anchors": [ ...would-be-registered records... ], "dryRun": true }
Registry unchanged.
POST /api/v1/anchors/bulk
(same body, a1 already registered)
-> 409 Conflict
{ "error": { "code": "CONFLICT",
"message": "anchor "a1" is already registered" } }
POST /api/v1/anchors/bulk?dryRun=true
(same body, a1 already registered)
-> 409 Conflict
{ "error": { "code": "CONFLICT",
"message": "anchor "a1" is already registered" } }
Byte-identical to the real call. Registry unchanged.
POST /api/v1/anchors/bulk?dryRun=ture
-> 400 Bad Request
{ "error": { "code": "BAD_REQUEST",
"message": ""dryRun" must be "true" or "false"" } }
Nothing registered.
================================================================================
ACCEPTANCE CRITERIA
[x] POST /api/v1/anchors/bulk?dryRun=true returns the same success/error
outcome as a real call would, but registers nothing.
[x] A batch that would fail validation fails identically in both modes.
Verified by tests that run the same batch against two fresh service
instances and assert equal status, code and message.
[x] Repository state is provably unchanged after a dry run, verified by
tests checking AnchorRepository.count() and all() before and after,
plus a jest.spyOn(repo, "upsert") asserting upsert is never called.
[x] Minimum 95% test coverage. Overall 96.3%; the three files carrying new
logic are at 100%.
[x] Clear documentation. OpenAPI spec, README, and JSDoc on both the new
and modified methods explaining the reasoning, not just the mechanics.
================================================================================
VALIDATION
Test suite
Test Suites: 40 passed, 40 total
Tests: 407 passed, 407 total (374 before this PR, 33 added)
No pre-existing test was modified or deleted. All 374 original tests
still pass unchanged, which is the evidence that the refactor of
registerBulk preserved existing behaviour.
Coverage
The only uncovered lines in anchors.ts (97-98) are a pre-existing 501
branch for an absent settlements service, untouched by this PR.
Build
npm run build (tsc) exit 0, no errors
npm run lint (eslint) exit 0, no warnings
End-to-end smoke test against a running app instance
dryRun=true -> 201, registry count 0
real -> 201, registry count 2
conflict dry -> 409, identical body to real
conflict real -> 409
outcomes identical: true
typo flag -> 400, nothing registered
final registry: [a1, a2]
================================================================================
TESTS ADDED (33)
src/services/anchorService.test.ts (14)
src/routes/anchors.test.ts (13)
src/utils/validation.test.ts (6)
src/openapi.test.ts (1)
================================================================================
FILES CHANGED (9 modified, 0 added)
src/services/anchorService.ts +47 -8 extracted validateBulk,
added dryRun parameter
src/routes/anchors.ts +13 -3 parse ?dryRun, pass through,
return flag
src/utils/validation.ts +23 -0 optionalBooleanFlag helper
src/openapi.ts +13 -2 dryRun parameter + description
src/services/anchorService.test.ts +189 -1 14 dry-run service tests
src/routes/anchors.test.ts +198 -0 13 dry-run route tests
src/utils/validation.test.ts +38 -0 6 flag-parsing tests
src/openapi.test.ts +13 -0 spec assertion
README.md +10 -3 endpoint documentation
Total: 534 insertions, 10 deletions
================================================================================
SECURITY
Read-only preflight validation. No new data is exposed: the response contains
only the records the caller just submitted, echoed back in normalised form.
The change explicitly prevents a persistence side effect when requested, and
the strict flag parsing closes the failure mode where a malformed dryRun value
could be silently downgraded into a real write.
No changes to authentication, authorisation, rate limiting, or the audit log.
The endpoint remains subject to all existing middleware.
================================================================================
BACKWARD COMPATIBILITY
No breaking changes.
compile and behave exactly as before.
reading response.anchors are unaffected.
================================================================================
NOTE FOR REVIEWERS
One deliberate detail worth a second opinion: in dry-run mode all returned
records share a single registeredAt timestamp, whereas the real path calls
new Date() once per anchor. This is cosmetic for a preflight, since the
values are discarded and the real registration generates fresh timestamps
on commit. If you would prefer the two paths be byte-identical, aligning
them is a one-line change.
Two choices were made explicitly rather than by default, and are easy to
revisit:
rather than 200. The dryRun: true field in the body disambiguates.
than lenient, for the safety reason described above.
Closes #152