diff --git a/src/Spark.Engine.Test/Search/CriteriumTests.cs b/src/Spark.Engine.Test/Search/CriteriumTests.cs index 8f766138b..e03e956bb 100644 --- a/src/Spark.Engine.Test/Search/CriteriumTests.cs +++ b/src/Spark.Engine.Test/Search/CriteriumTests.cs @@ -8,6 +8,7 @@ using System; using Microsoft.VisualStudio.TestTools.UnitTesting; using Hl7.Fhir.Model; +using Spark.Engine.Core; using Spark.Search.Support; @@ -128,6 +129,74 @@ public void ParseChain() Assert.IsTrue(crit.Operand is UntypedValue); } + [TestMethod] + public void ParseChainStripsComparatorPrefixOnInnerParameter() + { + // subject:Patient.birthdate=ge1974-12-25 : the comparator prefix belongs to the inner + // parameter (Patient.birthdate), so the chain must be resolved against Patient - not the + // outer Observation - for the prefix to be recognised, applied, and stripped from the value. + var crit = Criterium.Parse("Observation", "subject:Patient.birthdate", "ge1974-12-25"); + + Assert.AreEqual(Operator.CHAIN, crit.Operator); + Assert.AreEqual("Patient", crit.Modifier); + + var inner = crit.Operand as Criterium; + Assert.IsNotNull(inner); + Assert.AreEqual("birthdate", inner.ParamName); + Assert.AreEqual(Operator.GTE, inner.Operator); + Assert.AreEqual("1974-12-25", inner.Operand.ToString()); + } + + [TestMethod] + [DataRow("ge1974-12-25", Operator.GTE, "1974-12-25")] + [DataRow("le2000-01-01", Operator.LTE, "2000-01-01")] + [DataRow("gt2000-01-01", Operator.GT, "2000-01-01")] + [DataRow("lt2000-01-01", Operator.LT, "2000-01-01")] + [DataRow("eq2000-01-01", Operator.EQ, "2000-01-01")] + [DataRow("ne2000-01-01", Operator.NOT_EQUAL, "2000-01-01")] + [DataRow("sa2000-01-01", Operator.STARTS_AFTER, "2000-01-01")] + [DataRow("eb2000-01-01", Operator.ENDS_BEFORE, "2000-01-01")] + [DataRow("ap2000-01-01", Operator.APPROX, "2000-01-01")] + [DataRow("2000-01-01", Operator.EQ, "2000-01-01")] // no prefix: operator stays EQ, value untouched + public void ParseChainAppliesComparatorPrefixToInnerParameter(string value, Operator expectedOperator, string expectedOperand) + { + // The comparator prefix belongs to the inner parameter (Patient.birthdate), so it must be + // recognised and applied there - regardless of which prefix is used. + var crit = Criterium.Parse("Observation", "subject:Patient.birthdate", value); + + var inner = crit.Operand as Criterium; + Assert.IsNotNull(inner); + Assert.AreEqual(expectedOperator, inner.Operator); + Assert.AreEqual(expectedOperand, inner.Operand.ToString()); + } + + [TestMethod] + public void ParseUntypedChainAppliesComparatorPrefixOnInnerParameter() + { + // subject.birthdate=ge1974-12-25 : no explicit :Patient type, so the target must be resolved + // from the reference's declared targets for the prefix to be recognised and applied. + var crit = Criterium.Parse("Observation", "subject.birthdate", "ge1974-12-25"); + + var inner = crit.Operand as Criterium; + Assert.IsNotNull(inner); + Assert.AreEqual("birthdate", inner.ParamName); + Assert.AreEqual(Operator.GTE, inner.Operator); + Assert.AreEqual("1974-12-25", inner.Operand.ToString()); + } + + [TestMethod] + public void ParseChainKeepsModifierOnInnerParameter() + { + // A modifier on the inner parameter (name:exact) belongs to the inner criterium, not the chain. + var crit = Criterium.Parse("Observation", "subject:Patient.name:exact", "Smith"); + + var inner = crit.Operand as Criterium; + Assert.IsNotNull(inner); + Assert.AreEqual("name", inner.ParamName); + Assert.AreEqual("exact", inner.Modifier); + Assert.AreEqual("Smith", inner.Operand.ToString()); + } + [TestMethod] public void SerializeChain() { @@ -411,4 +480,4 @@ public void HandleComposites() Assert.AreEqual(14.8M, ((UntypedValue)comp1.Components[1]).AsNumberValue().Value); Assert.AreEqual("http://somesuch.org|NOK", ((UntypedValue)crit1.Choices[1]).AsTokenValue().ToString()); } -} \ No newline at end of file +} diff --git a/src/Spark.Engine/Search/ValueExpressionTypes/Criterium.cs b/src/Spark.Engine/Search/ValueExpressionTypes/Criterium.cs index 7874f04de..e1f2697e3 100644 --- a/src/Spark.Engine/Search/ValueExpressionTypes/Criterium.cs +++ b/src/Spark.Engine/Search/ValueExpressionTypes/Criterium.cs @@ -138,6 +138,25 @@ private static Tuple pathToKeyModifTuple(string pathPart) return Tuple.Create(name, modifier); } + private static string ResolveReferenceTargetType(string resourceType, string referenceParam, string innerParam) + { + if (string.IsNullOrEmpty(resourceType) || string.IsNullOrEmpty(referenceParam) || string.IsNullOrEmpty(innerParam)) + return null; + + ModelInfo.SearchParamDefinition reference = ModelInfo.SearchParameters.Find( + p => (p.Resource == resourceType || p.Resource == nameof(Resource)) && p.Name == referenceParam); + if (reference?.Target == null) + return null; + + List targets = reference.Target.Select(Hl7.Fhir.Utility.EnumUtility.GetLiteral).ToList(); + + string preferred = targets.FirstOrDefault(t => ModelInfo.SearchParameters.CanHaveOperatorPrefix(t, innerParam)); + if (preferred != null) + return preferred; + + return targets.FirstOrDefault(t => ModelInfo.SearchParameters.Exists(p => p.Resource == t && p.Name == innerParam)); + } + private static Criterium fromPathTuples(IEnumerable> path, string value, string resourceType = null) { var first = path.First(); @@ -150,7 +169,8 @@ private static Criterium fromPathTuples(IEnumerable> path, if (path.Count() > 1) { type = Operator.CHAIN; - operand = fromPathTuples(path.Skip(1), value, resourceType); + string innerResourceType = modifier ?? ResolveReferenceTargetType(resourceType, name, path.Skip(1).First().Item1) ?? resourceType; + operand = fromPathTuples(path.Skip(1), value, innerResourceType); } // :missing modifier is actually not a real modifier and is turned into diff --git a/src/Spark.Mongo.Tests/Search/ChainedSearchErrorHandlingTests.cs b/src/Spark.Mongo.Tests/Search/ChainedSearchErrorHandlingTests.cs new file mode 100644 index 000000000..3a6d90d55 --- /dev/null +++ b/src/Spark.Mongo.Tests/Search/ChainedSearchErrorHandlingTests.cs @@ -0,0 +1,202 @@ +/* + * Copyright (c) 2026, Incendi + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +using Hl7.Fhir.Model; +using Hl7.Fhir.Rest; +using MongoDB.Driver; +using Spark.Engine.Core; +using Spark.Engine.Search; +using Spark.Engine.Service.FhirServiceExtensions; +using Spark.Mongo.Search.Common; +using Spark.Mongo.Search.Indexer; +using Spark.Search.Mongo; +using System; +using System.Threading; +using Testcontainers.MongoDb; +using Xunit; +using Assert = Xunit.Assert; +using Task = System.Threading.Tasks.Task; + +namespace Spark.Mongo.Tests.Search; + +/// +/// End-to-end behaviour of a chained search whose inner sub-query targets Patient. Both tests seed +/// four Observations - two whose Patient is born after the cut-off, two before. Needs Docker +/// (Testcontainers); tagged Integration so the cross-platform unit run skips it, while it still runs +/// locally via a normal `dotnet test`. +/// +[Trait("Category", "Integration")] +public class ChainedSearchErrorHandlingTests +{ + private const string BaseUri = "http://localhost/"; + + /// + /// A comparator prefix on the inner parameter (Patient.birthdate=ge...) is stripped and applied, so + /// only the two Patients born after the cut-off match - leaving exactly two of the four Observations. + /// + [SkippableFact] + public async Task Chained_search_applies_comparator_prefix_on_inner_parameter() + { + Skip.IfNot(OperatingSystem.IsLinux()); + + var container = await StartMongoOrSkipAsync(); + try + { + var searcher = await SeedSearcherAsync(container); + + var results = await searcher.SearchAsync("Observation", + new SearchParams().Add("subject:Patient.birthdate", "ge1974-12-25")); + + Assert.Equal(2, results.MatchCount); + } + finally + { + await container.DisposeAsync(); + } + } + + /// + /// The same comparator prefix on an untyped chain (subject.birthdate, no explicit :Patient type) is + /// resolved against the reference's target and applied, again leaving exactly two of four Observations. + /// + [SkippableFact] + public async Task Chained_search_applies_comparator_prefix_on_untyped_inner_parameter() + { + Skip.IfNot(OperatingSystem.IsLinux()); + + var container = await StartMongoOrSkipAsync(); + try + { + var searcher = await SeedSearcherAsync(container); + + var results = await searcher.SearchAsync("Observation", + new SearchParams().Add("subject.birthdate", "ge1974-12-25")); + + Assert.Equal(2, results.MatchCount); + } + finally + { + await container.DisposeAsync(); + } + } + + /// + /// An unsupported modifier on the inner parameter (Patient.name:above) makes the inner sub-query + /// fail; that failure must surface as an exception, not be swallowed into an unfiltered result. + /// + [SkippableFact] + public async Task Chained_search_throws_when_inner_parameter_has_unsupported_modifier() + { + Skip.IfNot(OperatingSystem.IsLinux()); + + var container = await StartMongoOrSkipAsync(); + try + { + var searcher = await SeedSearcherAsync(container); + + await Assert.ThrowsAsync(() => searcher.SearchAsync("Observation", + new SearchParams().Add("subject:Patient.name:above", "Smith"))); + } + finally + { + await container.DisposeAsync(); + } + } + + /// + /// Reference checking rewrites a non-chained reference search into an internal chained lookup. + /// The inner lookup uses internal_justid for bare ids and internal_id for typed/full references. + /// + [SkippableTheory] + [InlineData("subject", "p1")] + [InlineData("subject:Patient", "p1")] + [InlineData("subject", "Patient/p1")] + public async Task Reference_search_with_reference_check_uses_internal_reference_lookup(string parameterName, string parameterValue) + { + Skip.IfNot(OperatingSystem.IsLinux()); + + var container = await StartMongoOrSkipAsync(); + try + { + var searcher = await SeedSearcherAsync(container); + var searchSettings = new SearchSettings + { + CheckReferences = true, + CheckReferencesFor = ["Observation.subject"] + }; + + var results = await searcher.SearchAsync("Observation", + new SearchParams().Add(parameterName, parameterValue), + searchSettings); + + Assert.False(results.HasErrors); + Assert.Equal(1, results.MatchCount); + Assert.Single(results); + Assert.Equal("http://localhost/Observation/o1/_history/1", results[0]); + } + finally + { + await container.DisposeAsync(); + } + } + + private static async System.Threading.Tasks.Task StartMongoOrSkipAsync() + { + // Building/starting the container probes the Docker endpoint; on a host without Docker + // (e.g. the macOS/Windows CI runners) that throws, so skip rather than fail. + MongoDbContainer container = null; + try + { + container = new MongoDbBuilder("mongo:8.2.7").Build(); + await container.StartAsync(CancellationToken.None); + return container; + } + catch (Exception ex) + { + if (container != null) + await container.DisposeAsync(); + //Assert.Skip($"Docker/Testcontainers not available: {ex.Message}"); + return null; // unreachable: Assert.Skip throws. + } + } + + private static async System.Threading.Tasks.Task SeedSearcherAsync(MongoDbContainer container) + { + var connectionString = BuildConnectionString(container.GetConnectionString(), "sparktest"); + + // Wire up the real index/search object graph. + IFhirModel fhirModel = new FhirModel(); + ILocalhost localhost = new Localhost(new Uri(BaseUri)); + var indexStore = new MongoIndexStore(connectionString, new MongoIndexMapper()); + + var indexService = new IndexService(fhirModel, indexStore, new ElementIndexer(fhirModel), new ResourceResolver()); + var searcher = new MongoSearcher(indexStore, localhost, fhirModel, new ReferenceNormalizationService(localhost)); + + // Two patients born after the cut-off and two before, each with one Observation. + var birthdates = new[] { "2000-01-01", "2001-01-01", "1950-01-01", "1951-01-01" }; + for (var i = 0; i < birthdates.Length; i++) + { + await indexService.IndexResourceAsync( + new Patient { Id = $"p{i}", BirthDate = birthdates[i] }, + new Key(BaseUri, "Patient", $"p{i}", "1")); + await indexService.IndexResourceAsync( + new Observation { Id = $"o{i}", Subject = new ResourceReference($"Patient/p{i}") }, + new Key(BaseUri, "Observation", $"o{i}", "1")); + } + + return searcher; + } + + private static string BuildConnectionString(string raw, string databaseName) + { + var builder = new MongoUrlBuilder(raw) { DatabaseName = databaseName }; + if (!string.IsNullOrEmpty(builder.Username) && string.IsNullOrEmpty(builder.AuthenticationSource)) + { + builder.AuthenticationSource = "admin"; + } + return builder.ToMongoUrl().ToString(); + } +} diff --git a/src/Spark.Mongo.Tests/Spark.Mongo.Tests.csproj b/src/Spark.Mongo.Tests/Spark.Mongo.Tests.csproj index d3b657024..ccbc59ef9 100644 --- a/src/Spark.Mongo.Tests/Spark.Mongo.Tests.csproj +++ b/src/Spark.Mongo.Tests/Spark.Mongo.Tests.csproj @@ -8,11 +8,13 @@ + all runtime; build; native; contentfiles; analyzers; buildtransitive + diff --git a/src/Spark.Mongo/Search/Searcher/CriteriaMongoExtensions.cs b/src/Spark.Mongo/Search/Searcher/CriteriaMongoExtensions.cs index ffa1bd8ea..71aba5c8b 100644 --- a/src/Spark.Mongo/Search/Searcher/CriteriaMongoExtensions.cs +++ b/src/Spark.Mongo/Search/Searcher/CriteriaMongoExtensions.cs @@ -62,7 +62,7 @@ internal static FilterDefinition ToFilter(this Criterium param, st //return null; } - throw new ArgumentException(string.Format("Resource {0} has no parameter with the name {1}.", resourceType, param.ParamName)); + throw new UnknownSearchParameterException(string.Format("Resource {0} has no parameter with the name {1}.", resourceType, param.ParamName)); } private static FilterDefinition CreateFilter(ModelInfo.SearchParamDefinition parameter, Operator op, String modifier, Expression operand) @@ -529,4 +529,4 @@ private static FilterDefinition SafeIn(string parameterName, BsonA return Builders.Filter.In(parameterName, values); return FalseQuery(); } -} \ No newline at end of file +} diff --git a/src/Spark.Mongo/Search/Searcher/MongoSearcher.cs b/src/Spark.Mongo/Search/Searcher/MongoSearcher.cs index 4c868018e..2c213e487 100644 --- a/src/Spark.Mongo/Search/Searcher/MongoSearcher.cs +++ b/src/Spark.Mongo/Search/Searcher/MongoSearcher.cs @@ -230,7 +230,7 @@ private static FilterDefinition CreateMongoQuery(string resourceTy { criteriaQueries.Add(crit.Value.ToFilter(resourceType)); } - catch (ArgumentException ex) + catch (UnknownSearchParameterException ex) { if (results == null) throw; //The exception *will* be caught on the highest level. results.AddIssue(String.Format("Parameter [{0}] was ignored for the reason: {1}.", crit.Key.ToString(), ex.Message), OperationOutcome.IssueSeverity.Warning); @@ -261,7 +261,7 @@ private Dictionary CloseChainedCriteria(string resourceTyp closedCriteria.Add(c.Clone(), CloseCriterium(c, resourceType, level)); //CK: We don't pass the SearchResults on to the (recursive) CloseCriterium. We catch any exceptions only on the highest level. } - catch (ArgumentException ex) + catch (UnknownSearchParameterException ex) { if (results == null) throw; //The exception *will* be caught on the highest level. results.AddIssue(String.Format("Parameter [{0}] was ignored for the reason: {1}.", c.ToString(), ex.Message), OperationOutcome.IssueSeverity.Warning); @@ -291,7 +291,7 @@ private async Task> CloseChainedCriteriaAsync(s closedCriteria.Add(c.Clone(), await CloseCriteriumAsync(c, resourceType, level).ConfigureAwait(false)); //CK: We don't pass the SearchResults on to the (recursive) CloseCriterium. We catch any exceptions only on the highest level. } - catch (ArgumentException ex) + catch (UnknownSearchParameterException ex) { if (results == null) throw; //The exception *will* be caught on the highest level. results.AddIssue(String.Format("Parameter [{0}] was ignored for the reason: {1}.", c.ToString(), ex.Message), OperationOutcome.IssueSeverity.Warning); @@ -329,7 +329,7 @@ private Criterium CloseCriterium(Criterium crit, string resourceType, int level) var keys = CollectKeys(target, new List { innerCriterium }, ++level); allKeys.AddRange(keys.Select(k => k.ToString())); } - catch (Exception ex) + catch (UnknownSearchParameterException ex) { errors.Add(ex); } @@ -337,7 +337,7 @@ private Criterium CloseCriterium(Criterium crit, string resourceType, int level) if (errors.Count == targeted.Count()) { //It is possible that some of the targets don't support the current parameter. But if none do, there is a serious problem. - throw new ArgumentException(String.Format("None of the possible target resources support querying for parameter {0}", crit.ParamName)); + throw new UnknownSearchParameterException(String.Format("None of the possible target resources support querying for parameter {0}", crit.ParamName)); } crit.Operator = Operator.IN; crit.Operand = new ChoiceValue(allKeys.Select(k => new UntypedValue(k))); @@ -365,7 +365,7 @@ private async Task CloseCriteriumAsync(Criterium crit, string resourc var keys = await CollectKeysAsync(target, new List { innerCriterium }, ++level).ConfigureAwait(false); allKeys.AddRange(keys.Select(k => k.ToString())); } - catch (Exception ex) + catch (UnknownSearchParameterException ex) { errors.Add(ex); } @@ -373,7 +373,7 @@ private async Task CloseCriteriumAsync(Criterium crit, string resourc if (errors.Count == targeted.Count()) { //It is possible that some of the targets don't support the current parameter. But if none do, there is a serious problem. - throw new ArgumentException(String.Format("None of the possible target resources support querying for parameter {0}", crit.ParamName)); + throw new UnknownSearchParameterException(String.Format("None of the possible target resources support querying for parameter {0}", crit.ParamName)); } crit.Operator = Operator.IN; crit.Operand = new ChoiceValue(allKeys.Select(k => new UntypedValue(k))); @@ -760,4 +760,4 @@ private List parseCriteria(string resourceType, SearchParams searchCo return result; } -} \ No newline at end of file +} diff --git a/src/Spark.Mongo/Search/Searcher/UnknownSearchParameterException.cs b/src/Spark.Mongo/Search/Searcher/UnknownSearchParameterException.cs new file mode 100644 index 000000000..b51f60dac --- /dev/null +++ b/src/Spark.Mongo/Search/Searcher/UnknownSearchParameterException.cs @@ -0,0 +1,16 @@ +/* + * Copyright (c) 2026, Incendi + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +using System; + +namespace Spark.Search.Mongo; + +internal sealed class UnknownSearchParameterException : ArgumentException +{ + public UnknownSearchParameterException(string message) : base(message) + { + } +}