fix(persistence): make testcontainers watchdog feature non-Windows only#79
Merged
Merged
Conversation
The `watchdog` feature on `testcontainers` 0.27 pulls in `signal_hook::consts::SIGQUIT` and `signal_hook::iterator`, both of which are gated to non-Windows. As a result, the Windows clippy job fails with E0432 "unresolved imports" when checking the persistence crate's tests. Move the `watchdog`-enabled testcontainers dependency under `[target.'cfg(not(windows))'.dev-dependencies]` and add a plain testcontainers entry for Windows. Watchdog cleanup is only useful for Unix SIGTERM handling, so dropping the feature on Windows preserves the intended behavior on Linux/macOS without breaking the Windows build.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
smunini
added a commit
that referenced
this pull request
Apr 27, 2026
…ows only Extends the fix from #79 to two crates that were missed. The `watchdog` feature on `testcontainers` 0.27 pulls in `signal_hook::consts::SIGQUIT` and `signal_hook::iterator`, both of which are gated to non-Windows. Under `cargo clippy --all-features` on Windows, feature unification across the workspace re-activated `watchdog` on `testcontainers` because `helios-hts` and `helios-subscriptions` still requested it unconditionally, causing E0432 "unresolved imports" on the Windows linting job. Move the `watchdog`-enabled testcontainers dependency under `[target.'cfg(not(windows))'.dev-dependencies]` in both crates and add a plain testcontainers entry for Windows, mirroring the existing split in `helios-persistence`.
smunini
added a commit
that referenced
this pull request
Apr 27, 2026
* refactor(search): make search-param type resolution registry-driven
The REST extractor's hardcoded "well-known param name → SearchParamType"
match silently misclassified anything outside the list (e.g. Goal.target-date
fell through to a value-shape heuristic, which mongodb's strict typed query
builder then rejected with HTTP 400). Three other call sites carried similar
duplicate tables.
Replace the lot with a single deterministic resolver:
resolve_param_type(registry, resource_type, name, values)
1. registry.get_param(resource_type, name)
2. registry.get_param("Resource", name) ← global params
3. value-shape heuristic (unregistered custom only)
Plumbed via a new SearchProvider::search_param_registry() trait method that
every backend already had as a concrete accessor; REST handlers acquire it
from state.storage(). Hardcoded match tables in mongodb's build_search_params
and sqlite's chain_builder are gone; sqlite's dead-code infer_target_type
also removed.
Effect on the failing inferno mongodb test: Goal?patient&target-date now
resolves target-date as Date deterministically, build_date_filter parses the
value correctly, returns 200. The two DiagnosticReport patient&status 400s
were never caused by type inference (status was already Token) and need
separate triage.
Net: −150 lines of redundant hardcoded mappings, +30 lines of resolver,
1 trait method, 8 new unit tests covering registry hit / Resource-base
fallback / value heuristic / target lookup. All 642 persistence and 184
rest lib tests pass; clippy clean with project CI flags.
* feat(persistence/composite): registry-driven extract_references for _include (#82)
Composite's extract_references for _include / _revinclude was doing hardcoded
JSON field-name matching (with a small `patient/subject/encounter/performer`
alias map). Custom search parameters and standard ones whose JSON field name
doesn't match the param name silently returned zero references — the same
"in a real implementation, FHIRPath would..." stub pattern PR #80 set out
to find.
Replace the body with a registry-first lookup:
1. registry.get_param(resource_type, name)
2. registry.get_param("Resource", name)
3. SearchParameterExtractor::extract_for_param — runs the canonical
FHIRPath expression and filters to IndexValue::Reference
The hardcoded alias map is retained but only fires for params that aren't
in the registry at all, so unregistered custom params keep working.
Verification:
- New test composite::storage::tests::test_extract_references_uses_registry_expression
proves the registry's FHIRPath expression is what gets evaluated
- 643 helios-persistence lib tests pass (was 642)
- Clippy clean
* fix(hts,subscriptions): make testcontainers watchdog feature non-Windows only
Extends the fix from #79 to two crates that were missed. The
`watchdog` feature on `testcontainers` 0.27 pulls in
`signal_hook::consts::SIGQUIT` and `signal_hook::iterator`, both of
which are gated to non-Windows. Under `cargo clippy --all-features`
on Windows, feature unification across the workspace re-activated
`watchdog` on `testcontainers` because `helios-hts` and
`helios-subscriptions` still requested it unconditionally, causing
E0432 "unresolved imports" on the Windows linting job.
Move the `watchdog`-enabled testcontainers dependency under
`[target.'cfg(not(windows))'.dev-dependencies]` in both crates and
add a plain testcontainers entry for Windows, mirroring the existing
split in `helios-persistence`.
* fix(rest): restore SearchParamType import dropped in merge
The merge of `main` into `feat/registry-driven-search-param-types`
combined two changes that touched adjacent code: #80 (registry-driven
type resolution, removed `SearchParamType` from imports) and #f9196dc1
(fix prefix-only-for-date/number/quantity, used `SearchParamType` in
the body). The merge resolved by keeping main's import list but the
branch's body, leaving the type referenced in `parse_search_parameter`
without a declaration. CI hit E0433 on both Linux and Windows.
Re-add `SearchParamType` to the `helios_persistence::types` use list.
smunini
added a commit
that referenced
this pull request
Apr 27, 2026
* refactor(search): make search-param type resolution registry-driven
The REST extractor's hardcoded "well-known param name → SearchParamType"
match silently misclassified anything outside the list (e.g. Goal.target-date
fell through to a value-shape heuristic, which mongodb's strict typed query
builder then rejected with HTTP 400). Three other call sites carried similar
duplicate tables.
Replace the lot with a single deterministic resolver:
resolve_param_type(registry, resource_type, name, values)
1. registry.get_param(resource_type, name)
2. registry.get_param("Resource", name) ← global params
3. value-shape heuristic (unregistered custom only)
Plumbed via a new SearchProvider::search_param_registry() trait method that
every backend already had as a concrete accessor; REST handlers acquire it
from state.storage(). Hardcoded match tables in mongodb's build_search_params
and sqlite's chain_builder are gone; sqlite's dead-code infer_target_type
also removed.
Effect on the failing inferno mongodb test: Goal?patient&target-date now
resolves target-date as Date deterministically, build_date_filter parses the
value correctly, returns 200. The two DiagnosticReport patient&status 400s
were never caused by type inference (status was already Token) and need
separate triage.
Net: −150 lines of redundant hardcoded mappings, +30 lines of resolver,
1 trait method, 8 new unit tests covering registry hit / Resource-base
fallback / value heuristic / target lookup. All 642 persistence and 184
rest lib tests pass; clippy clean with project CI flags.
* feat(persistence/composite): registry-driven extract_references for _include (#82)
Composite's extract_references for _include / _revinclude was doing hardcoded
JSON field-name matching (with a small `patient/subject/encounter/performer`
alias map). Custom search parameters and standard ones whose JSON field name
doesn't match the param name silently returned zero references — the same
"in a real implementation, FHIRPath would..." stub pattern PR #80 set out
to find.
Replace the body with a registry-first lookup:
1. registry.get_param(resource_type, name)
2. registry.get_param("Resource", name)
3. SearchParameterExtractor::extract_for_param — runs the canonical
FHIRPath expression and filters to IndexValue::Reference
The hardcoded alias map is retained but only fires for params that aren't
in the registry at all, so unregistered custom params keep working.
Verification:
- New test composite::storage::tests::test_extract_references_uses_registry_expression
proves the registry's FHIRPath expression is what gets evaluated
- 643 helios-persistence lib tests pass (was 642)
- Clippy clean
* fix(hts,subscriptions): make testcontainers watchdog feature non-Windows only
Extends the fix from #79 to two crates that were missed. The
`watchdog` feature on `testcontainers` 0.27 pulls in
`signal_hook::consts::SIGQUIT` and `signal_hook::iterator`, both of
which are gated to non-Windows. Under `cargo clippy --all-features`
on Windows, feature unification across the workspace re-activated
`watchdog` on `testcontainers` because `helios-hts` and
`helios-subscriptions` still requested it unconditionally, causing
E0432 "unresolved imports" on the Windows linting job.
Move the `watchdog`-enabled testcontainers dependency under
`[target.'cfg(not(windows))'.dev-dependencies]` in both crates and
add a plain testcontainers entry for Windows, mirroring the existing
split in `helios-persistence`.
* fix(rest): restore SearchParamType import dropped in merge
The merge of `main` into `feat/registry-driven-search-param-types`
combined two changes that touched adjacent code: #80 (registry-driven
type resolution, removed `SearchParamType` from imports) and #f9196dc1
(fix prefix-only-for-date/number/quantity, used `SearchParamType` in
the body). The merge resolved by keeping main's import list but the
branch's body, leaving the type referenced in `parse_search_parameter`
without a declaration. CI hit E0433 on both Linux and Windows.
Re-add `SearchParamType` to the `helios_persistence::types` use list.
* feat(persistence/postgres): support multi-step forward & reverse chains (#81)
Postgres' ChainedSearchProvider was returning Ok(Vec::new()) for any forward
chain longer than 2 segments and for nested _has reverse chains, silently
producing empty result sets where SQLite returned correct matches. Backend-
behavior divergence with no error signal — the kind of "simplified for now"
stub PR #80 set out to find.
Port the proven SQLite ChainQueryBuilder pattern to Postgres: nested SELECTs
over search_index, registry-driven target-type resolution via
resolve_param_targets(), the same Patient-default for ambiguous multi-target
references SQLite uses. Postgres-specific syntax: $N placeholders, ILIKE for
case-insensitive match, SUBSTRING(... FROM POSITION('/' IN ...) + 1) in
place of SUBSTR + INSTR for the reverse-chain reference-id extraction.
Verification:
- 6 new unit tests in chain_builder covering parsing, SQL shape, explicit
type modifier, ambiguous-target inference, empty-chain error, and
reverse-chain SUBSTRING/POSITION emission
- 2 new testcontainers-backed integration tests porting the SQLite three-
level chain and reverse-chain terminal cases — both pass against a real
Postgres instance
- All 609 helios-persistence lib tests still green; clippy clean
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
mainsince Feature/subscriptions phase 3 Email Channel #75 enabled thewatchdogfeature ontestcontainers = \"0.27\". That feature pulls insignal_hook::consts::SIGQUITandsignal_hook::iterator, which are gated to non-Windows in thesignal-hookcrate, so checkingcrates/persistenceon Windows fails withE0432: unresolved imports.testcontainersentry under[target.'cfg(not(windows))'.dev-dependencies]and add a plaintestcontainers = \"0.27\"for Windows. Cleanup on Actions cancellations (SIGTERM) is the watchdog's reason for existing, so dropping the feature on Windows preserves Linux/macOS behavior without breaking the Windows build.Cargo.lockchanges — the same crate version is selected on all targets.Test plan
cargo check -p helios-persistence --features mongodb,R4 --tests(macOS)