Wire history read endpoints and vread#117
Merged
Merged
Conversation
The history storage layer was fully implemented (versioned resource_history table + InstanceHistoryProvider/TypeHistoryProvider/SystemHistoryProvider traits), but the REST read handlers returned 501 Not Implemented even though the CapabilityStatement advertised the interactions. Wire them up: - history.rs: instance/type/system history handlers now call the providers and build `type: history` Bundles. Each entry carries request (method+url), response (status, weak ETag, lastModified), and the resource body for non-delete versions. Supports `_count` (capped at max page size) and `_since` (RFC3339; malformed value -> 400). Instance history returns 404 only when the result is empty *and* unfiltered (an existing resource always has >= 1 version), so a `_since` that excludes everything still yields an empty Bundle. - vread.rs: calls VersionedStorage::vread, mirrors the read handler response (ETag, Content-Type with fhirVersion), and 404s an unknown version. - composite/storage.rs: CompositeStorage only delegated up to InstanceHistoryProvider. Add a DynSystemHistoryProvider field, implement TypeHistoryProvider + SystemHistoryProvider (delegating to the primary), and raise the with_full_primary bound so type/system history work on the -elasticsearch composite variants. - Raise router (fhir_routes.rs) and app-builder (lib.rs) trait bounds to require TypeHistoryProvider + SystemHistoryProvider. All four base backends already implement these; CompositeStorage now does too. Tests: add `mod history_reads` to rest_conformance.rs (8 tests) covering instance/type/system history, _count, malformed _since, vread of specific versions, and not-found cases.
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
1 participant
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
The history storage layer was fully implemented (versioned
resource_historytable +InstanceHistoryProvider/TypeHistoryProvider/SystemHistoryProvidertraits), but the REST read handlers returned501 Not Implementedeven though the CapabilityStatement advertised the interactions. This PR wires them up so advertised capability matches runtime behavior.Changes
handlers/history.rs— instance/type/system history handlers now call the providers and buildtype: historyBundles. Each entry carriesrequest(method+url),response(status, weak ETag,lastModified), and the resource body for non-delete versions. Supports_count(capped at max page size) and_since(RFC3339; malformed →400). Instance history returns404only when the result is empty and unfiltered (an existing resource always has ≥1 version), so a_sincethat excludes everything still yields an empty Bundle.handlers/vread.rs— callsVersionedStorage::vread, mirrors the read handler response (ETag,Content-TypewithfhirVersion), and404s an unknown version.composite/storage.rs—CompositeStorageonly delegated up toInstanceHistoryProvider. Added aDynSystemHistoryProviderfield, implementedTypeHistoryProvider+SystemHistoryProvider(delegating to the primary), and raised thewith_full_primarybound so type/system history work on the-elasticsearchcomposite variants.lib.rs+routing/fhir_routes.rs— raised router/app-builder trait bounds to requireTypeHistoryProvider + SystemHistoryProvider. All four base backends already implement these;CompositeStoragenow does too.Endpoints now Live
/{type}/{id}/_history/{type}/_history/_history/{type}/{id}/_history/{vid}(The R6 Trial Use
DELETEhistory/version endpoints were already Live.)Testing
mod history_readstorest_conformance.rs(8 tests): instance/type/system history,_countlimiting, malformed_since→ 400, vread of specific versions, and not-found cases.helios-restsuite green (194 unit + all integration binaries); composite tests green (--features R4).--features postgres,elasticsearch,s3,mongodb.cargo fmt+ clippy (CI flags) clean.