feat(rest,persistence): resolve conditional interactions in Bundle entries - #860
Merged
Conversation
…lt::Deleted `ConditionalDeleteResult::Deleted` was a unit variant, so nothing downstream of a conditional delete could name the row the criteria resolved to. The HTTP `conditional_delete_handler` emitted an AuditEvent with no entity, and CompositeStorage's delegate path could not sync the delete to a secondary search backend because, as its own comment said, it did not have the id. The variant now carries the pre-delete snapshot. SQLite, PostgreSQL and MongoDB already held it at the return site; the composite dedicated-search path does too, and its delegate path now emits `SyncEvent::Delete` for the carried id. The REST handler inserts the same `AuditResponseContext` the instance delete inserts. S3's stub is unchanged. Groundwork for #511: the bundle DELETE-by-criteria arm needs this id for audit attribution. Tests: sqlite_tests asserts the carried id matches the created resource; postgres/mongodb pattern matches updated; core storage unit test updated.
…h's location MongoDB was the only backend resolving `ifNoneExist` inside a transaction, and its match path answered `BundleEntryResult::ok(match)`, whose `location` is `None`. Every `process_transaction` loop builds the bundle's fullUrl → id map from a POST entry's `location`, so a `urn:uuid:` reference to a matched entry was left unresolved and stored verbatim — R4 §3.1.0.11.2 requires it to resolve to the match. Add `bundle_if_none_exist_gate` in `core::preconditions`, next to `bundle_if_match_gate`, mapping 0/1/n matches to proceed / 200-with-location / 412 multiple-matches, plus the `multiple_matches_entry` and `not_supported_entry` builders the SQLite and PostgreSQL executors will use when they adopt the same gate (#511). MongoDB's inline match is replaced by the gate. Tests: unit tests for the gate and builders; the MongoDB transaction test now asserts the 200 entry's location equals the 201 entry's, and a new testcontainers test proves a urn:uuid subject on a sibling Observation resolves to the matched Patient.
…eating a duplicate The SQLite transaction executor ignored `BundleEntry.if_none_exist` and created unconditionally at 201, so a transaction replayed with the same `ifNoneExist` criteria produced a duplicate. The resource endpoint honours the header, and `/metadata` advertises `conditionalCreate` for every type. `SearchProvider::search` takes a fresh pooled connection, which under `BEGIN IMMEDIATE` cannot see rows earlier entries of the same bundle wrote. Lift its body into `search_with_connection` and run it on the transaction's own connection through `SqliteTransaction::with_connection`; the POST arm now resolves the criteria there and answers through the shared `bundle_if_none_exist_gate` (201 / 200-with-location / 412). `find_matching_resources` keeps its behaviour via the extracted `conditional_query`. When search is offloaded to a secondary backend the local index is empty for every row, so an in-transaction search would always find nothing; such an entry is refused with 501 not-supported and the bundle rolls back rather than duplicating silently. Part of #511. Tests: `test_bundle_conditional_create` is no longer ignored and now asserts 200 with the match's location; new tests cover urn:uuid resolution to the match, multiple matches rolling back an earlier create, the same criteria twice in one bundle, and the offloaded refusal. sqlite_tests and the lib sqlite tests pass unchanged.
…creating a duplicate Same defect and same fix as the SQLite executor: the transaction POST arm ignored `BundleEntry.if_none_exist` and created unconditionally, so a replayed transaction produced a duplicate at 201 while the resource endpoint honoured the header. Lift the body of `SearchProvider::search` into `search_with_client` and run it on the transaction's own client, after `flush()` so buffered creates from earlier entries are visible exactly as they are to `read`. The POST arm resolves the criteria there and answers through `bundle_if_none_exist_gate` (201 / 200-with-location / 412). A bundle that puts `ifNoneExist` on every entry forfeits create batching, which is the correct trade. With search offloaded to a secondary backend the entry is refused with 501 rather than duplicated against an always-empty local index. Part of #511. Tests: two testcontainers tests cover 201-then-200 across bundles and within one bundle, urn:uuid resolution to the match, and an ambiguous criteria rolling back an earlier create. The full postgres_tests suite passes unchanged.
Since #503 the batch arm refused `PUT/DELETE [type]?[criteria]` per entry with 400 and never read `request.ifNoneExist`, while `/metadata` advertised conditionalCreate/Update/Delete for every type. The machinery existed: `ConditionalStorage` is implemented by every backend and already bounds the router; only `batch_handler` declared a narrower bound. Widen the bound and add three executors mirroring the resource endpoints: - `PUT [type]?[criteria]` → `conditional_update` with upsert: one match updates (200, location `Type/id`), none creates (201), several 412. - `DELETE [type]?[criteria]` → `conditional_delete`: deleted or no match 204 (R4 §3.1.0.7.1), several 412 (`conditionalDelete: "single"`). - `POST` + `ifNoneExist` → `conditional_create`: created 201, match 200 with the match's location (agreeing with the transaction executors), several 412. The header is passed verbatim like `If-None-Exist`. URL criteria are percent-decoded once through `parse_query_pairs`, so the backend sees what axum's `Query` gives the resource endpoints, with repeated keys kept rather than collapsed. Still refused: criteria on a POST (FHIR defines none), empty criteria, and `ifMatch` on any conditional entry, which names a version of an instance the server has yet to resolve. Conditional entries are read-then-write in the backend, so `batch_concurrency` serializes a bundle carrying one. A conditional DELETE answers 204 with no body and its URL carries no id, so the arm hands the deleted resource's identity to `emit_entry_audit` through an explicit `AuditTarget`; the response stays bodiless. A backend whose `ConditionalStorage` is a stub (S3) answers 501 per entry through the existing `UnsupportedCapability` mapping. Part of #511. URL criteria inside a transaction remain declined whole. Tests: `DelayStorage` gains a scripted `ConditionalStorage` recording what reached storage; unit tests pin decoded criteria, verbatim `ifNoneExist`, every result → status mapping, the 501 funnel, the concurrency clamp, and the remaining refusals. batch_conformance runs each interaction against in-memory SQLite, including percent-encoded identifiers, several matches writing nothing, and a transaction replayed with `ifNoneExist` resolving a urn:uuid reference to the match.
Describe what #511 landed: `ifNoneExist` on batch and transaction POST entries across backends, URL criteria resolved in batch bundles with the resource-endpoint status mapping, the serial execution of bundles carrying a conditional entry, the `ifMatch` and POST-criteria refusals, the offloaded-search refusal, and that URL criteria in transactions remain declined whole (#859). Re-point the `/metadata` note at #514.
aacruzgon
force-pushed
the
fix/511-bundle-conditional-interactions
branch
from
September 2, 2026 00:39
cc21841 to
f051e32
Compare
This was referenced Sep 2, 2026
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
smunini
approved these changes
Sep 2, 2026
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
2 participants
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.
Summary
Bundle entries can now perform FHIR conditional interactions. Batch bundles resolve
PUT [type]?[criteria],DELETE [type]?[criteria]andPOST+ifNoneExistthroughConditionalStoragewith the same status mapping as the resource endpoints. SQLite and PostgreSQL honorifNoneExistinside a transaction, where they previously created a duplicate at 201 while/metadataadvertisedconditionalCreate.Closes #511. URL criteria inside a transaction remain declined whole; that is #859.
Changes
Persistence
ConditionalDeleteResult::Deletedcarries the deleted resource, so the HTTP conditional delete audits an entity and CompositeStorage syncs the delete to secondaries.bundle_if_none_exist_gateincore::preconditions(withmultiple_matches_entry/not_supported_entry), used by all three transaction executors. It setslocationon the 200 match, which fixes a latent MongoDB bug:urn:uuidreferences to a matchedifNoneExistentry were never resolved.search_with_connection/search_with_client) so the transaction POST arm can resolveifNoneExiston its own connection. Postgres flushes buffered creates first. Offloaded-search composites (SQLite/PG + Elasticsearch) refuse the entry with 501 rather than duplicating against an empty local index.REST batch path
batch_handlerand friends gain theConditionalStoragebound the router already carries.ifNoneExist: created 201, match 200 with the match's location, several 412.ifMatchon any conditional entry.ConditionalStorageanswers 501 per entry through the existing error mapping; no new capability predicate (capabilities: /metadata is a hardcoded literal claiming ~16 capabilities unconditionally, and CapabilityProvider is a stub that cannot yet replace it #514 owns/metadata).Docs:
crates/rest/README.mddescribes the new behavior.Testing
cargo fmt --all -- --checkclean; CI clippy line (--all-targets --all-features -D warnings+ the 8 allows) clean.cargo test -p helios-rest: 579 lib tests + all integration suites pass;batch_conformancehas 12 new cases (PUT/DELETE criteria, percent-encoded identifiers, several matches writing nothing,ifNoneExisttwice, a transaction replayed withifNoneExistresolving aurn:uuidreference).cargo test -p helios-persistence:transactions_suiteun-ignorestest_bundle_conditional_createand adds four cases;sqlite_testsand lib pass unchanged.postgres_tests151/151 pass with two new transaction tests;mongodb_testsconditional tests pass with a newurn:uuidresolution test.Notes
ConditionalDeleteResult::Deletedis nowDeleted(StoredResource).&or=mis-splits in the backend.processingwith the message indetails.text(the fix(rest): render batch entry failures through the single-resource error mapping #516 seam), so a 412 here does not carrymultiple-matchesyet.