diff --git a/crates/rest/src/handlers/batch.rs b/crates/rest/src/handlers/batch.rs index 7f60b86d0..da26735c2 100644 --- a/crates/rest/src/handlers/batch.rs +++ b/crates/rest/src/handlers/batch.rs @@ -57,7 +57,7 @@ pub async fn batch_handler( request: Request, ) -> RestResult where - S: ResourceStorage + BundleProvider + Send + Sync, + S: ResourceStorage + BundleProvider + helios_persistence::core::SearchProvider + Send + Sync, { // Extract the Principal from request extensions (set by auth middleware). // If present, per-entry scope checks will be enforced. @@ -204,7 +204,7 @@ async fn process_transaction( principal: Option<&Principal>, ) -> RestResult where - S: ResourceStorage + BundleProvider + Send + Sync, + S: ResourceStorage + BundleProvider + helios_persistence::core::SearchProvider + Send + Sync, { debug!( tenant = %tenant.tenant_id(), @@ -254,6 +254,14 @@ where } } + // Conditional references (`Type?query`) resolve against the server's + // content before anything executes, per the transaction processing rules: + // exactly one match rewrites the reference to `Type/id`; zero or several + // fail the bundle (#459). They used to be stored verbatim — unsearchable + // and unresolvable. References to entries created by this same bundle use + // `fullUrl`s, which the storage layer resolves during processing. + resolve_conditional_references(state, &tenant, &mut indexed_entries).await?; + // Write-path validation: transactions are atomic, so any invalid write // entry rejects the whole bundle before anything executes. for (index, entry, _) in &indexed_entries { @@ -839,6 +847,149 @@ fn status_text(code: &str) -> &'static str { /// Parses a bundle entry from JSON into a BundleEntry struct. /// /// Returns the BundleEntry and optionally the fullUrl for reference resolution. +/// Resolves conditional references (`Type?query`) in the bundle's resources +/// against the server's content, per the transaction processing rules: +/// exactly one match rewrites the reference to `Type/id`, zero or several +/// fail the bundle (#459). They used to pass through into storage verbatim, +/// where nothing can search or resolve them. +async fn resolve_conditional_references( + state: &AppState, + tenant: &TenantExtractor, + indexed_entries: &mut [(usize, BundleEntry, Option)], +) -> RestResult<()> +where + S: ResourceStorage + helios_persistence::core::SearchProvider + Send + Sync, +{ + use std::collections::HashMap; + + // Collect every distinct conditional reference first: bundles repeat the + // same one heavily (every Synthea entry names its location), and each + // lookup is a search. + let mut conditionals: HashMap> = HashMap::new(); + for (_, entry, _) in indexed_entries.iter() { + if let Some(resource) = &entry.resource { + collect_conditional_references(resource, &mut conditionals); + } + } + if conditionals.is_empty() { + return Ok(()); + } + + for (reference, resolved) in conditionals.iter_mut() { + let (resource_type, query_string) = + reference.split_once('?').expect("collected with a '?'"); + let pairs: Vec<(String, String)> = url::form_urlencoded::parse(query_string.as_bytes()) + .map(|(k, v)| (k.into_owned(), v.into_owned())) + .collect(); + let registry = state.storage().search_param_registry(tenant.context()); + let mut query = { + let registry = registry.read(); + crate::extractors::build_search_query_from_pairs(resource_type, &pairs, ®istry) + .map_err(|e| RestError::BadRequest { + message: format!( + "Conditional reference '{reference}' is not a valid search: {e}" + ), + })? + }; + // Two is enough to prove the match is not unique. + query.count = Some(2); + let result = state + .storage() + .search(tenant.context(), &query) + .await + .map_err(RestError::from)?; + match result.resources.items.as_slice() { + [only] => { + *resolved = Some(format!("{}/{}", only.resource_type(), only.id())); + } + [] => { + return Err(RestError::BadRequest { + message: format!( + "Conditional reference '{reference}' matches no existing resource" + ), + }); + } + _ => { + return Err(RestError::BadRequest { + message: format!( + "Conditional reference '{reference}' matches more than one resource" + ), + }); + } + } + } + + for (_, entry, _) in indexed_entries.iter_mut() { + if let Some(resource) = &mut entry.resource { + rewrite_conditional_references(resource, &conditionals); + } + } + Ok(()) +} + +/// Whether a reference literal is a conditional reference (`Type?query`). +fn is_conditional_reference(reference: &str) -> bool { + match reference.split_once('?') { + Some((head, query)) => { + !head.is_empty() + && !query.is_empty() + && head.chars().next().is_some_and(|c| c.is_ascii_uppercase()) + && head.chars().all(|c| c.is_ascii_alphanumeric()) + } + None => false, + } +} + +/// Walks a resource collecting conditional `reference` literals. +fn collect_conditional_references( + value: &Value, + out: &mut std::collections::HashMap>, +) { + match value { + Value::Object(map) => { + if let Some(Value::String(reference)) = map.get("reference") + && is_conditional_reference(reference) + { + out.entry(reference.clone()).or_insert(None); + } + for v in map.values() { + collect_conditional_references(v, out); + } + } + Value::Array(arr) => { + for item in arr { + collect_conditional_references(item, out); + } + } + _ => {} + } +} + +/// Rewrites collected conditional `reference` literals to their resolutions. +fn rewrite_conditional_references( + value: &mut Value, + resolved: &std::collections::HashMap>, +) { + match value { + Value::Object(map) => { + if let Some(Value::String(reference)) = map.get("reference") + && let Some(Some(target)) = resolved.get(reference) + { + map.insert("reference".to_string(), Value::String(target.clone())); + } + for v in map.values_mut() { + rewrite_conditional_references(v, resolved); + } + } + Value::Array(arr) => { + for item in arr { + rewrite_conditional_references(item, resolved); + } + } + _ => {} + } +} + fn parse_bundle_entry(entry: &Value) -> Result<(BundleEntry, Option), String> { let request = entry .get("request") diff --git a/crates/rest/tests/batch_conformance.rs b/crates/rest/tests/batch_conformance.rs index b40964311..bb83bb706 100644 --- a/crates/rest/tests/batch_conformance.rs +++ b/crates/rest/tests/batch_conformance.rs @@ -809,3 +809,102 @@ mod transaction_errors { response.assert_status(StatusCode::BAD_REQUEST); } } + +/// #459: conditional references (`Type?query`) resolve against the server's +/// content before the transaction executes — exactly one match rewrites the +/// reference; zero or several reject the bundle. They used to be stored +/// verbatim, unsearchable and unresolvable. +mod conditional_references { + use super::*; + + async fn seed_location(backend: &SqliteBackend, id: &str, ident: &str) { + let tenant = test_tenant(); + backend + .create( + &tenant, + "Location", + json!({ + "resourceType": "Location", + "id": id, + "status": "active", + "name": format!("Location {id}"), + "identifier": [{"system": "https://example.org/locs", "value": ident}] + }), + FhirVersion::R4, + ) + .await + .expect("seed location"); + } + + fn immunization_bundle() -> serde_json::Value { + json!({ + "resourceType": "Bundle", + "type": "transaction", + "entry": [{ + "fullUrl": "urn:uuid:11111111-1111-1111-1111-111111111111", + "resource": { + "resourceType": "Immunization", + "status": "completed", + "vaccineCode": {"coding": [{"system": "http://hl7.org/fhir/sid/cvx", "code": "140"}]}, + "patient": {"reference": "Patient/p1"}, + "occurrenceDateTime": "2020-01-01", + "location": {"reference": "Location?identifier=https://example.org/locs|loc-a"} + }, + "request": {"method": "POST", "url": "Immunization"} + }] + }) + } + + #[tokio::test] + async fn a_unique_match_is_rewritten_into_storage() { + let (server, backend) = create_test_server().await; + seed_patient(&backend, "p1", "CondRef").await; + seed_location(&backend, "loc-1", "loc-a").await; + + let response = server + .post("/") + .add_header(X_TENANT_ID, HeaderValue::from_static("test-tenant")) + .json(&immunization_bundle()) + .await; + response.assert_status_ok(); + + let stored = server + .get("/Immunization?_count=5") + .add_header(X_TENANT_ID, HeaderValue::from_static("test-tenant")) + .await; + let body: serde_json::Value = stored.json(); + let imm = &body["entry"][0]["resource"]; + assert_eq!( + imm["location"]["reference"], "Location/loc-1", + "the conditional reference is resolved, not stored verbatim: {imm}" + ); + } + + #[tokio::test] + async fn no_match_rejects_the_bundle() { + let (server, backend) = create_test_server().await; + seed_patient(&backend, "p1", "CondRef").await; + + let response = server + .post("/") + .add_header(X_TENANT_ID, HeaderValue::from_static("test-tenant")) + .json(&immunization_bundle()) + .await; + response.assert_status(StatusCode::BAD_REQUEST); + } + + #[tokio::test] + async fn an_ambiguous_match_rejects_the_bundle() { + let (server, backend) = create_test_server().await; + seed_patient(&backend, "p1", "CondRef").await; + seed_location(&backend, "loc-1", "loc-a").await; + seed_location(&backend, "loc-2", "loc-a").await; + + let response = server + .post("/") + .add_header(X_TENANT_ID, HeaderValue::from_static("test-tenant")) + .json(&immunization_bundle()) + .await; + response.assert_status(StatusCode::BAD_REQUEST); + } +}