From 98a7763d27f0c534583e17445a38e57d780387a9 Mon Sep 17 00:00:00 2001 From: angela-helios Date: Sat, 1 Aug 2026 02:03:34 -0400 Subject: [PATCH] fix(sqlite): apply _tag/_profile/_security/_source search filters The sqlite query builder diverted every underscore-prefixed parameter into the special-parameter handler, which only implements _id, _lastUpdated, _text, _content, and _filter and returns None for the rest -- so _tag, _profile, _security, and _source were silently dropped and the search returned the unfiltered result set, while the self link echoed the parameter as if it had been applied. Strict handling did not catch it either: the registry knows these parameters (the spec defines them on Resource with meta.* expressions), so they are not 'unknown'. The extraction side was already correct -- the writer indexes meta.tag, meta.profile, meta.security, and meta.source under their own param_name rows. Only the query side dropped them, and only on sqlite: the postgres builder special-cases just _id/_lastUpdated and routes everything else by type. Sqlite now does the same for these four, letting them take the regular token/uri path against search_index. Closes #474 --- .../backends/sqlite/search/query_builder.rs | 14 +- .../tests/search/meta_params_tests.rs | 158 ++++++++++++++++++ crates/persistence/tests/search/mod.rs | 1 + crates/rest/tests/search_integration.rs | 70 ++++++++ 4 files changed, 241 insertions(+), 2 deletions(-) create mode 100644 crates/persistence/tests/search/meta_params_tests.rs diff --git a/crates/persistence/src/backends/sqlite/search/query_builder.rs b/crates/persistence/src/backends/sqlite/search/query_builder.rs index c64b89036..a8e54e9f8 100644 --- a/crates/persistence/src/backends/sqlite/search/query_builder.rs +++ b/crates/persistence/src/backends/sqlite/search/query_builder.rs @@ -382,8 +382,18 @@ impl QueryBuilder { param: &SearchParameter, param_offset: usize, ) -> Option { - // Handle special parameters - if param.name.starts_with('_') { + // Handle special parameters. `_tag`/`_profile`/`_security`/`_source` + // are NOT special on the query side: the extractor indexes them from + // meta like any typed parameter (rows in search_index under their own + // param_name), so they take the regular token/uri path below. Routing + // them into the special handler silently dropped the condition and + // returned the unfiltered result set (#474). + if param.name.starts_with('_') + && !matches!( + param.name.as_str(), + "_tag" | "_profile" | "_security" | "_source" + ) + { return self.build_special_parameter_condition(param, param_offset); } diff --git a/crates/persistence/tests/search/meta_params_tests.rs b/crates/persistence/tests/search/meta_params_tests.rs new file mode 100644 index 000000000..0cf06fb14 --- /dev/null +++ b/crates/persistence/tests/search/meta_params_tests.rs @@ -0,0 +1,158 @@ +//! Tests for the meta search parameters `_tag`, `_profile`, `_security`, +//! and `_source` (#474). +//! +//! These are ordinary typed parameters indexed from `Resource.meta.*` by the +//! spec `SearchParameter` set; the query builder must apply them rather than +//! treating them as unimplemented specials and silently returning the +//! unfiltered result set. + +use serde_json::json; + +use helios_persistence::core::{ResourceStorage, SearchProvider}; +use helios_persistence::tenant::{TenantContext, TenantId, TenantPermissions}; +use helios_persistence::types::{SearchParamType, SearchParameter, SearchQuery, SearchValue}; + +use helios_fhir::FhirVersion; + +#[cfg(feature = "sqlite")] +use helios_persistence::backends::sqlite::SqliteBackend; + +#[cfg(feature = "sqlite")] +fn create_sqlite_backend() -> SqliteBackend { + super::make_sqlite_backend() +} + +fn create_tenant() -> TenantContext { + TenantContext::new( + TenantId::new("test-tenant"), + TenantPermissions::full_access(), + ) +} + +#[cfg(feature = "sqlite")] +async fn seed_meta_patients(backend: &SqliteBackend, tenant: &TenantContext) { + let tagged = json!({ + "resourceType": "Patient", + "meta": { + "tag": [{"system": "http://example.org/tags", "code": "test-data"}], + "profile": ["http://example.org/StructureDefinition/custom-patient"], + "security": [{ + "system": "http://terminology.hl7.org/CodeSystem/v3-ActCode", + "code": "HTEST" + }] + }, + "gender": "female" + }); + let untagged = json!({"resourceType": "Patient", "gender": "male"}); + backend + .create(tenant, "Patient", tagged, FhirVersion::default()) + .await + .unwrap(); + backend + .create(tenant, "Patient", untagged, FhirVersion::default()) + .await + .unwrap(); +} + +fn token_param(name: &str, system: Option<&str>, code: &str) -> SearchParameter { + SearchParameter { + name: name.to_string(), + param_type: SearchParamType::Token, + modifier: None, + values: vec![SearchValue::token(system, code)], + chain: vec![], + components: vec![], + } +} + +#[cfg(feature = "sqlite")] +#[tokio::test] +async fn test_tag_filters_to_tagged_resources() { + let backend = create_sqlite_backend(); + let tenant = create_tenant(); + seed_meta_patients(&backend, &tenant).await; + + let query = SearchQuery::new("Patient").with_parameter(token_param( + "_tag", + Some("http://example.org/tags"), + "test-data", + )); + let result = backend + .search(&tenant, &query.with_count(100)) + .await + .unwrap(); + + assert_eq!( + result.resources.len(), + 1, + "_tag must filter, not fall through to the unfiltered set" + ); + assert_eq!(result.resources.items[0].content()["gender"], "female"); +} + +#[cfg(feature = "sqlite")] +#[tokio::test] +async fn test_tag_with_no_match_returns_empty() { + let backend = create_sqlite_backend(); + let tenant = create_tenant(); + seed_meta_patients(&backend, &tenant).await; + + let query = SearchQuery::new("Patient").with_parameter(token_param( + "_tag", + Some("http://example.org/tags"), + "no-such-tag", + )); + let result = backend + .search(&tenant, &query.with_count(100)) + .await + .unwrap(); + + assert_eq!(result.resources.len(), 0); +} + +#[cfg(feature = "sqlite")] +#[tokio::test] +async fn test_security_filters_by_security_label() { + let backend = create_sqlite_backend(); + let tenant = create_tenant(); + seed_meta_patients(&backend, &tenant).await; + + let query = SearchQuery::new("Patient").with_parameter(token_param( + "_security", + Some("http://terminology.hl7.org/CodeSystem/v3-ActCode"), + "HTEST", + )); + let result = backend + .search(&tenant, &query.with_count(100)) + .await + .unwrap(); + + assert_eq!(result.resources.len(), 1); + assert_eq!(result.resources.items[0].content()["gender"], "female"); +} + +#[cfg(feature = "sqlite")] +#[tokio::test] +async fn test_profile_filters_by_canonical() { + let backend = create_sqlite_backend(); + let tenant = create_tenant(); + seed_meta_patients(&backend, &tenant).await; + + let query = SearchQuery::new("Patient").with_parameter(SearchParameter { + name: "_profile".to_string(), + param_type: SearchParamType::Uri, + modifier: None, + values: vec![SearchValue::string( + "http://example.org/StructureDefinition/custom-patient", + )], + chain: vec![], + components: vec![], + }); + let result = backend + .search(&tenant, &query.with_count(100)) + .await + .unwrap(); + + assert_eq!(result.resources.len(), 1); + assert_eq!(result.resources.items[0].content()["gender"], "female"); +} diff --git a/crates/persistence/tests/search/mod.rs b/crates/persistence/tests/search/mod.rs index 1dbf97dc7..0fc0c3f74 100644 --- a/crates/persistence/tests/search/mod.rs +++ b/crates/persistence/tests/search/mod.rs @@ -41,6 +41,7 @@ pub fn make_sqlite_backend() -> SqliteBackend { pub mod chained_tests; pub mod date_tests; pub mod include_tests; +pub mod meta_params_tests; pub mod modifier_tests; pub mod number_tests; pub mod pagination_tests; diff --git a/crates/rest/tests/search_integration.rs b/crates/rest/tests/search_integration.rs index c0094311a..329107839 100644 --- a/crates/rest/tests/search_integration.rs +++ b/crates/rest/tests/search_integration.rs @@ -2577,3 +2577,73 @@ mod summary_count { assert!(body["total"].is_null(), "explicit _total=none wins: {body}"); } } + +// ============================================================================ +// Meta Parameter Tests (_tag / _profile / _security, #474) +// ============================================================================ + +mod meta_params { + use super::*; + use helios_persistence::tenant::{TenantContext, TenantId, TenantPermissions}; + + async fn seed_tagged_patient(backend: &SqliteBackend) { + let tenant = TenantContext::new( + TenantId::new("test-tenant"), + TenantPermissions::full_access(), + ); + let tagged = json!({ + "resourceType": "Patient", + "meta": { + "tag": [{"system": "http://example.org/tags", "code": "test-data"}], + "profile": ["http://example.org/StructureDefinition/custom-patient"] + }, + "name": [{"family": "Tagged"}] + }); + backend + .create(&tenant, "Patient", tagged, FhirVersion::R4) + .await + .unwrap(); + } + + #[tokio::test] + async fn test_tag_filters_over_http() { + let (server, backend) = create_test_server().await; + seed_search_test_data(&backend).await; + seed_tagged_patient(&backend).await; + + let response = server + .get("/Patient?_tag=http%3A%2F%2Fexample.org%2Ftags%7Ctest-data") + .add_header(X_TENANT_ID, HeaderValue::from_static("test-tenant")) + .await; + + response.assert_status_ok(); + let body: Value = response.json(); + let entries = get_bundle_entries(&body); + assert_eq!( + entries.len(), + 1, + "_tag must filter instead of returning every patient" + ); + assert_eq!(entries[0]["resource"]["name"][0]["family"], "Tagged"); + } + + #[tokio::test] + async fn test_profile_filters_over_http() { + let (server, backend) = create_test_server().await; + seed_search_test_data(&backend).await; + seed_tagged_patient(&backend).await; + + let response = server + .get( + "/Patient?_profile=http%3A%2F%2Fexample.org%2FStructureDefinition%2Fcustom-patient", + ) + .add_header(X_TENANT_ID, HeaderValue::from_static("test-tenant")) + .await; + + response.assert_status_ok(); + let body: Value = response.json(); + let entries = get_bundle_entries(&body); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0]["resource"]["name"][0]["family"], "Tagged"); + } +}