fix(rest): parse bundle entry URLs without their query string

[aacruzgon]

Summary

parse_request_url split a bundle entry's request.url on / and nothing else, so every FHIR
conditional interaction was dispatched against a resource type containing its own criteria.
Patient?identifier=http://example.org|12345 parsed as the type Patient?identifier=http: with an
empty id — the // inside the value supplying the extra segment — and storage was then addressed
with that type.

This PR parses the URL correctly and refuses conditional entries rather than resolving them.
Resolving them is #511, filed alongside this work with the full implementation map.

The critical detail is that stripping the query alone is a regression, not a fix. With the
criteria gone, PUT Patient?identifier=x reaches create_or_update with an empty id, and SQLite
writes the row rather than rejecting it: create_or_update inserts "id": "" into the resource
before delegating to create, whose id fallback fires on an absent id, not an empty one
(sqlite/storage.rs:186-214 and :95-99). Every later such entry then reads that row back and
overwrites it. So today's corruption is quarantined under a bogus resource type no search can
address; a bare strip moves it into the real Patient table. The empty-id guards here are what make
the parse fix safe.

Changes

fix(rest): parse bundle entry URLs without their query string

• Split the query off before the path; drop empty segments so a leading / and the
  [type]/?[criteria] form both reduce to the type alone. Signature unchanged, so four of the five
  call sites — the concurrency scan, both transaction sites, the audit path — are repaired with no
  edits.
• The 0 => Err("Empty URL") arm was unreachable (str::split always yields ≥1 element), so an
  absent request.url parsed as the resource type "" and POST created a row under it. Now
  reachable.
• Refuse conditional entries per-entry with 400, and refuse PUT/DELETE entries that name no
  instance. GET is untouched, leaving search-style entries to batch/transaction: search-style GET entries are mis-parsed as instance reads #478.
• The concurrency scan's StructureDefinition carve-out now also matches
  StructureDefinition?url=…, where it silently did not before.

fix(rest): decline transaction entries whose URL carries a query

• Every backend's parse_url splits on / alone and takes the last two segments (sqlite, postgres
  and mongodb carry byte-equivalent copies), and the transaction PUT arm commits without validating
  either field. PUT Patient?identifier=… commits a row typed Patient?identifier=http:;
  PUT Patient/123?_format=json commits one whose id is 123?_format=json; PUT Patient?name=peter
  fails the whole bundle citing the URL format rather than the criteria.
• Decline any non-GET entry whose URL carries a query, raised before process_transaction so the
  bundle is declined intact. RestError::NotSupported (400 + not-supported) per the convention
  documented at error.rs:38-41 and R4's repeated SHOULD.
• This corrects batch: parse_request_url never strips the query string, so every conditional entry is dispatched against a resource type containing the query #503's own claim that the transaction path is unaffected because the backends resolve
  conditionals themselves. They do not — only MongoDB's ifNoneExist path does.
• This is a designed constraint, not a wiring gap. Discussion #28
  designs Transaction as CRUD-only — tenant/create/read/update/delete/commit/rollback
  — and the real trait at core/transaction.rs:147-189 is that shape plus is_active(). Building a Modern FHIR Persistence Layer - Architectural Considerations for Healthcare's AI Era #28 also
  states the isolation contract the resolution would have to satisfy: "Operations within a
  transaction see their own uncommitted changes but are isolated from concurrent transactions."
  A resolver that searches committed state would miss the bundle's own uncommitted entries. MongoDB
  is the one backend with a session-scoped resolver that meets this
  (find_matching_resources_in_bundle_transaction, .session(&mut *session)), which is exactly why
  its ifNoneExist path is carved out below rather than refused.

test(rest): assert the query-guard refusal on the field the server actually writes

• Added after review began, and it corrects this PR's own second commit.
  a_transaction_get_with_a_query_is_not_declined_by_the_query_guard cannot fail: its predicate is
  a three-way conjunction whose third clause reads body["issue"][0]["diagnostics"], but the guard it
  tests raises RestError::NotSupported, which renders through create_operation_outcome
  (error.rs:560-571) into details.text. grep -n diagnostics crates/rest/src/error.rs returns
  nothing — RestError never emits that field. So declined_by_the_guard is permanently false and
  the negated assert holds even when a GET entry is declined by the guard, which is the one thing
  the test exists to catch.
• One field path: ["diagnostics"] → ["details"]["text"]. No production code changes and the test
  count is unchanged.
• Verified non-vacuous in the only way available for a negation: with the GET exemption at
  batch.rs:418 removed so the guard does fire on a GET entry, the corrected test fails
  (GET entries must stay on #478's path, not this guard: …) while the original, against that same
  broken guard, still reports test result: ok. 1 passed.
• Found while grounding batch: every per-entry error carries OperationOutcome code processing — a 403, 404 and 405 are indistinguishable without parsing English #504. It landed here rather than there so the blame stays with the PR that
  owns the test; fix(rest): parse bundle entry methods through one shared matcher #515 and fix(rest): render batch entry failures through the single-resource error mapping #516 were restacked on top.

docs(rest): correct the bundle conditional-header claims

• The README advertised ifNoneMatch (populated by parse_bundle_entry, read by nothing) and
  ifNoneExist (MongoDB transactions only) as supported bundle-entry headers.

Testing

cargo test -p helios-rest        →  1093 passed, 0 failed
cargo fmt --all                  →  clean

12 new tests: 6 unit (parser table, criteria predicate, concurrency carve-out, refusals against a
mock whose write methods are unimplemented!()) and 6 integration against SQLite asserting nothing
is written or deleted.

Verified non-vacuous. With the empty-id guard disabled, a_type_level_put_never_writes_an_empty_id_row
fails with left: "201 Created", right: "400 Bad Request" — confirming PUT Patient writes the
empty-id row on the unguarded parse.

Corrected. This section previously said the CI-style gate "exits 101 on 12 pre-existing lints,
filed as #513", implying a caveat a reviewer had to accept on trust. That describes the bare
cargo clippy --all-targets --all-features -- -D warnings, which is not what CI runs. CI's
invocation is .github/workflows/ci.yml:497 and carries eight allow-flags — -A collapsible_if,
-A doc-lazy-continuation, -A field_reassign_with_default and five siblings. Every lint in those
five files falls inside them, so CI's actual command passes clean, exit 0. Verified on the
current tip of this stack. cargo clippy -p helios-rest --all-targets --all-features reports no
lints in either file this PR touches. #513 remains open but is lower priority than its framing
suggests, since CI allows these today.

Notes

• Interaction with fix(rest): execute search-style GET bundle entries as searches #481 (issue batch/transaction: search-style GET entries are mis-parsed as instance reads #478). fix(rest): execute search-style GET bundle entries as searches #481 already contains let path = url.split('?').next()
  inside parse_request_url but does not guard the write arms, so on its own it moves the empty-id
  write into the real Patient table. If fix(rest): execute search-style GET bundle entries as searches #481 lands first, this should follow immediately; if this
  lands first, fix(rest): execute search-style GET bundle entries as searches #481's strip becomes a no-op on rebase. Flagged on that PR.
• ifNoneExist is deliberately untouched on the transaction path. MongoDB resolves it inside the
  bundle's session (mongodb/storage.rs:2675 → find_matching_resources_in_bundle_transaction,
  .session(&mut *session)), covered by mongodb_integration_transaction_bundle_conditional_headers.
  Refusing it would have removed a working atomic feature, and CI would not have caught it — that
  test drives the backend directly and bypasses the REST layer. SQLite and Postgres silently ignore
  it and create a duplicate; that wants a backend capability predicate, tracked in batch/transaction: conditional interactions are refused rather than resolved — wire bundle entries to ConditionalStorage #511.
• Known divergence introduced here, deliberately. PUT Patient/123?_format=json is now accepted
  in a batch (the query is stripped, the instance is addressed) but declines the whole
  transaction. The batch path keys on conditional_criteria, which correctly ignores a query on an
  instance URL; the transaction guard keys on url.contains('?'). The broader transaction predicate
  is intentional — the backends re-parse the raw URL query-blind, so letting it through commits an id
  of 123?_format=json. Refusing is strictly better than committing garbage, but narrowing the guard
  properly means normalising the URL at the process_transaction handoff, which is a behaviour change
  with no transaction-side coverage. Left as-is; noted so it is a decision rather than a surprise.
• /metadata still advertises conditionalCreate/conditionalUpdate/conditionalDelete for
  every resource type (capabilities.rs:370-373). Accurate for the resource endpoints, not for
  bundle entries. The literal claims ~16 capabilities unconditionally and the trait meant to replace
  it is itself a stub — filed as capabilities: /metadata is a hardcoded literal claiming ~16 capabilities unconditionally, and CapabilityProvider is a stub that cannot yet replace it #514. Making the conditional flags true on the bundle path is batch/transaction: conditional interactions are refused rather than resolved — wire bundle entries to ConditionalStorage #511.
• Deliberately out of scope: absolute-URL entry URLs (the REST and backend parsers disagree on
  the type, a separate defect); [type]/$op entry URLs; batch PATCH (batch: PATCH entries return 405 and methods are case-sensitive — the batch and transaction method matchers disagree #502); per-entry
  OperationOutcome issue codes — the new refusals carry processing like the other 15 call sites,
  which batch: every per-entry error carries OperationOutcome code processing — a 403, 404 and 405 are indistinguishable without parsing English #504 fixes uniformly.
• Conflict surface: batch_conformance.rs gains an appended mod conditional_entries at EOF, where
  fix(rest): execute search-style GET bundle entries as searches #481 and fix(transaction): resolve conditional references before execution #467 also append. Trivial three-way keep-both.

Closes #503

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!