diff --git a/crates/persistence/src/backends/elasticsearch/search/parameter_handlers/date.rs b/crates/persistence/src/backends/elasticsearch/search/parameter_handlers/date.rs index 1a73d5685..e4cbaeb29 100644 --- a/crates/persistence/src/backends/elasticsearch/search/parameter_handlers/date.rs +++ b/crates/persistence/src/backends/elasticsearch/search/parameter_handlers/date.rs @@ -4,113 +4,67 @@ use serde_json::{Value, json}; use crate::types::SearchPrefix; -/// Builds an ES query clause for a date search parameter. -pub fn build_clause(name: &str, value: &str, prefix: SearchPrefix) -> Option { - let range_condition = match prefix { - SearchPrefix::Eq => { - // Equality accounting for precision: - // "2024-01-15" matches the full day - let (lower, upper) = date_precision_range(value); - json!({ - "range": { - "search_params.date.value": { - "gte": lower, - "lt": upper - } - } - }) - } - SearchPrefix::Ne => { - let (lower, upper) = date_precision_range(value); - return Some(json!({ - "nested": { - "path": "search_params.date", - "query": { - "bool": { - "must": [ - { "term": { "search_params.date.name": name } } - ], - "must_not": [ - { - "range": { - "search_params.date.value": { - "gte": lower, - "lt": upper - } - } - } - ] - } - } - } - })); - } - SearchPrefix::Gt | SearchPrefix::Sa => { - let (_, upper) = date_precision_range(value); - json!({ - "range": { - "search_params.date.value": { - "gte": upper - } - } - }) - } - SearchPrefix::Lt | SearchPrefix::Eb => { - let (lower, _) = date_precision_range(value); - json!({ - "range": { - "search_params.date.value": { - "lt": lower - } - } - }) +/// A precision-aware comparison on one ES date field, ready to be wrapped in +/// whatever query shape the caller needs (nested for indexed parameters, a +/// bare top-level clause for `_lastUpdated`). +#[derive(Debug)] +pub(crate) enum DateRange { + /// `{ "range": { field: bounds } }` — the value must fall inside. + Within(Value), + /// `{ "range": { field: bounds } }` — the value must fall *outside* + /// (`ne`). The caller negates it with `must_not` so the negation is + /// applied at the right level of the enclosing query. + Outside(Value), +} + +/// Builds the `range` comparison for `field` at the value's inherent +/// precision: `eq` at day precision means `[day, day+1)`, `ne` its +/// complement, `gt`/`sa` start strictly after the whole period, `lt`/`eb` +/// end strictly before it, and `le` reaches the end of the period. +/// +/// A full-precision instant (`2024-01-15T10:00:00Z`) has no period, so it +/// falls back to scalar comparison rather than an empty half-open range. +pub(crate) fn field_range(field: &str, value: &str, prefix: SearchPrefix) -> DateRange { + let (lower, upper) = date_precision_range(value); + let degenerate = lower == upper; + let range = |bounds: Value| json!({ "range": { field: bounds } }); + + match prefix { + // `ap` on a date is the precision range itself: ES has no fuzzy + // date matching, and the implied period is the natural tolerance. + SearchPrefix::Eq | SearchPrefix::Ap if degenerate => { + DateRange::Within(range(json!({ "gte": lower, "lte": lower }))) } - SearchPrefix::Ge => { - let (lower, _) = date_precision_range(value); - json!({ - "range": { - "search_params.date.value": { - "gte": lower - } - } - }) + SearchPrefix::Eq | SearchPrefix::Ap => { + DateRange::Within(range(json!({ "gte": lower, "lt": upper }))) } - SearchPrefix::Le => { - let (_, upper) = date_precision_range(value); - json!({ - "range": { - "search_params.date.value": { - "lt": upper - } - } - }) + SearchPrefix::Ne if degenerate => { + DateRange::Outside(range(json!({ "gte": lower, "lte": lower }))) } - SearchPrefix::Ap => { - // Approximately: ±10% of the precision range - let (lower, upper) = date_precision_range(value); - // For approximate, we use the range itself (ES handles fuzzy matching) - json!({ - "range": { - "search_params.date.value": { - "gte": lower, - "lt": upper - } - } - }) + SearchPrefix::Ne => DateRange::Outside(range(json!({ "gte": lower, "lt": upper }))), + SearchPrefix::Gt | SearchPrefix::Sa if degenerate => { + DateRange::Within(range(json!({ "gt": lower }))) } + SearchPrefix::Gt | SearchPrefix::Sa => DateRange::Within(range(json!({ "gte": upper }))), + SearchPrefix::Lt | SearchPrefix::Eb => DateRange::Within(range(json!({ "lt": lower }))), + SearchPrefix::Ge => DateRange::Within(range(json!({ "gte": lower }))), + SearchPrefix::Le if degenerate => DateRange::Within(range(json!({ "lte": lower }))), + SearchPrefix::Le => DateRange::Within(range(json!({ "lt": upper }))), + } +} + +/// Builds an ES query clause for an indexed date search parameter. +pub fn build_clause(name: &str, value: &str, prefix: SearchPrefix) -> Option { + let name_term = json!({ "term": { "search_params.date.name": name } }); + let bool_body = match field_range("search_params.date.value", value, prefix) { + DateRange::Within(range) => json!({ "must": [name_term, range] }), + DateRange::Outside(range) => json!({ "must": [name_term], "must_not": [range] }), }; Some(json!({ "nested": { "path": "search_params.date", - "query": { - "bool": { - "must": [ - { "term": { "search_params.date.name": name } }, - range_condition - ] - } - } + "query": { "bool": bool_body } } })) } @@ -206,4 +160,51 @@ mod tests { assert!(s.contains("gte")); assert!(s.contains("2024-01-16")); // starts after precision range } + + #[test] + fn ne_is_the_negated_precision_range() { + let clause = build_clause("birthdate", "2024-01-15", SearchPrefix::Ne).unwrap(); + let bool_body = &clause["nested"]["query"]["bool"]; + assert_eq!( + bool_body["must"][0]["term"]["search_params.date.name"], + "birthdate" + ); + let range = &bool_body["must_not"][0]["range"]["search_params.date.value"]; + assert_eq!(range["gte"], "2024-01-15"); + assert_eq!(range["lt"], "2024-01-16"); + } + + #[test] + fn sa_and_eb_mirror_gt_and_lt_on_the_whole_period() { + let sa = field_range("f", "2024-01", SearchPrefix::Sa); + let DateRange::Within(sa) = sa else { + panic!("sa must be a plain range: {sa:?}") + }; + assert_eq!(sa["range"]["f"], json!({ "gte": "2024-02-01" })); + + let eb = field_range("f", "2024-01", SearchPrefix::Eb); + let DateRange::Within(eb) = eb else { + panic!("eb must be a plain range: {eb:?}") + }; + assert_eq!(eb["range"]["f"], json!({ "lt": "2024-01-01" })); + } + + #[test] + fn full_precision_instant_is_scalar_not_an_empty_range() { + let instant = "2024-01-15T10:00:00Z"; + let DateRange::Within(eq) = field_range("f", instant, SearchPrefix::Eq) else { + panic!("eq must be a plain range") + }; + assert_eq!(eq["range"]["f"], json!({ "gte": instant, "lte": instant })); + + let DateRange::Within(gt) = field_range("f", instant, SearchPrefix::Gt) else { + panic!("gt must be a plain range") + }; + assert_eq!(gt["range"]["f"], json!({ "gt": instant })); + + let DateRange::Within(le) = field_range("f", instant, SearchPrefix::Le) else { + panic!("le must be a plain range") + }; + assert_eq!(le["range"]["f"], json!({ "lte": instant })); + } } diff --git a/crates/persistence/src/backends/elasticsearch/search/query_builder.rs b/crates/persistence/src/backends/elasticsearch/search/query_builder.rs index 3f1df3b6e..80fca5c1f 100644 --- a/crates/persistence/src/backends/elasticsearch/search/query_builder.rs +++ b/crates/persistence/src/backends/elasticsearch/search/query_builder.rs @@ -268,37 +268,38 @@ impl<'a> EsQueryBuilder<'a> { } } - /// Builds a clause for the _lastUpdated special parameter. + /// Builds a clause for the `_lastUpdated` special parameter. + /// + /// Reuses the precision-aware date logic that indexed date parameters + /// get, against the top-level `last_updated` field: `eq` at day + /// precision means `[day, day+1)`, `ne` its complement, `sa`/`eb` mirror + /// `gt`/`lt` on the whole period, and comma-separated values OR together + /// (#892). Previously every value was folded into one `range` map, so + /// `ne`/`sa`/`eb`/`ap` degraded to `eq` and a second value overwrote the + /// first. fn build_last_updated_clause(&self, param: &SearchParameter) -> Option { - let mut range = serde_json::Map::new(); - for value in ¶m.values { - match value.prefix { - SearchPrefix::Eq => { - range.insert("gte".to_string(), json!(value.value)); - range.insert("lte".to_string(), json!(value.value)); - } - SearchPrefix::Gt => { - range.insert("gt".to_string(), json!(value.value)); - } - SearchPrefix::Lt => { - range.insert("lt".to_string(), json!(value.value)); - } - SearchPrefix::Ge => { - range.insert("gte".to_string(), json!(value.value)); - } - SearchPrefix::Le => { - range.insert("lte".to_string(), json!(value.value)); - } - _ => { - range.insert("gte".to_string(), json!(value.value)); - range.insert("lte".to_string(), json!(value.value)); + let mut clauses: Vec = param + .values + .iter() + .map( + |value| match date::field_range("last_updated", &value.value, value.prefix) { + date::DateRange::Within(range) => range, + date::DateRange::Outside(range) => { + json!({ "bool": { "must_not": [range] } }) + } + }, + ) + .collect(); + + match clauses.len() { + 0 => None, + 1 => clauses.pop(), + _ => Some(json!({ + "bool": { + "should": clauses, + "minimum_should_match": 1 } - } - } - if range.is_empty() { - None - } else { - Some(json!({ "range": { "last_updated": Value::Object(range) } })) + })), } } @@ -451,6 +452,63 @@ mod tests { } } + fn last_updated_query(values: Vec) -> Value { + let query = SearchQuery::new("Patient").with_parameter(SearchParameter { + name: "_lastUpdated".to_string(), + param_type: SearchParamType::Date, + modifier: None, + values, + chain: vec![], + components: vec![], + }); + let builder = EsQueryBuilder::new("acme", "Patient", "hfs_acme_patient".to_string()); + builder.build(&query).body["query"]["bool"]["must"][0].clone() + } + + #[test] + fn last_updated_eq_is_a_day_range() { + let clause = last_updated_query(vec![SearchValue::eq("2026-09-01")]); + assert_eq!( + clause["range"]["last_updated"], + json!({ "gte": "2026-09-01", "lt": "2026-09-02" }) + ); + } + + #[test] + fn last_updated_ne_is_negated_not_eq() { + // #892: `ne` fell into the default arm and matched exactly the day. + let clause = last_updated_query(vec![SearchValue::new(SearchPrefix::Ne, "2026-09-01")]); + assert_eq!( + clause["bool"]["must_not"][0]["range"]["last_updated"], + json!({ "gte": "2026-09-01", "lt": "2026-09-02" }) + ); + } + + #[test] + fn last_updated_sa_and_eb_exclude_the_named_period() { + let sa = last_updated_query(vec![SearchValue::new(SearchPrefix::Sa, "2026-09-01")]); + assert_eq!(sa["range"]["last_updated"], json!({ "gte": "2026-09-02" })); + + let eb = last_updated_query(vec![SearchValue::new(SearchPrefix::Eb, "2026-09-01")]); + assert_eq!(eb["range"]["last_updated"], json!({ "lt": "2026-09-01" })); + } + + #[test] + fn last_updated_or_list_keeps_every_value() { + // #892: a second value overwrote the first in the single range map. + let clause = last_updated_query(vec![ + SearchValue::eq("2026-09-01"), + SearchValue::eq("2026-09-03"), + ]); + let should = clause["bool"]["should"] + .as_array() + .expect("OR list -> bool.should"); + assert_eq!(should.len(), 2); + assert_eq!(clause["bool"]["minimum_should_match"], 1); + assert_eq!(should[0]["range"]["last_updated"]["gte"], "2026-09-01"); + assert_eq!(should[1]["range"]["last_updated"]["gte"], "2026-09-03"); + } + fn not_param(values: Vec) -> SearchQuery { SearchQuery::new("Patient").with_parameter(SearchParameter { name: "language".to_string(), diff --git a/crates/persistence/tests/elasticsearch_tests.rs b/crates/persistence/tests/elasticsearch_tests.rs index 81534d967..ea12e2147 100644 --- a/crates/persistence/tests/elasticsearch_tests.rs +++ b/crates/persistence/tests/elasticsearch_tests.rs @@ -2798,6 +2798,64 @@ mod es_integration { ); } + /// #892: `ne` must be the complement of the day range, not `eq`. + #[tokio::test] + async fn es_integration_search_last_updated_ne_excludes_today() { + use helios_persistence::core::SearchProvider; + use helios_persistence::types::{ + SearchParamType, SearchParameter, SearchPrefix, SearchQuery, SearchValue, + }; + + let backend = create_backend().await; + let tenant = create_tenant("test-tenant"); + + let created = backend + .create( + &tenant, + "Patient", + json!({ + "resourceType": "Patient", + "id": "lu-ne-1" + }), + FhirVersion::default(), + ) + .await + .unwrap(); + let today = created.last_modified().format("%Y-%m-%d").to_string(); + + // Wait for index refresh + tokio::time::sleep(tokio::time::Duration::from_millis(500)).await; + + let last_updated = |prefix: SearchPrefix| { + SearchQuery::new("Patient").with_parameter(SearchParameter { + name: "_lastUpdated".to_string(), + param_type: SearchParamType::Date, + modifier: None, + values: vec![SearchValue::new(prefix, &today)], + chain: vec![], + components: vec![], + }) + }; + + let eq = backend + .search(&tenant, &last_updated(SearchPrefix::Eq)) + .await + .unwrap(); + assert!( + eq.resources.items.iter().any(|r| r.id() == "lu-ne-1"), + "eq on the creation day must match the resource" + ); + + let ne = backend + .search(&tenant, &last_updated(SearchPrefix::Ne)) + .await + .unwrap(); + assert!( + ne.resources.items.iter().all(|r| r.id() != "lu-ne-1"), + "ne on the creation day must exclude the resource" + ); + } + // ======================================================================== // Full-Text Search Tests // ========================================================================