From 3d9dcdb1c2be07949d91340453c1740ffb3a7fa2 Mon Sep 17 00:00:00 2001 From: Kenneth Myhra Date: Mon, 17 Aug 2026 22:21:20 +0200 Subject: [PATCH 1/3] MongoDB: Use Moq and its capabilities instead of Stub/Mock impls We already use Moq for the Engine tests so lets start use it for or MongoDB tests as well. --- .../DatabaseMigrationRefreshServiceTests.cs | 115 ++++++++---------- .../Spark.Store.MongoDB.Tests.csproj | 1 + 2 files changed, 52 insertions(+), 64 deletions(-) diff --git a/Tests/Spark.Store.MongoDB.Tests/DatabaseMigrationRefreshServiceTests.cs b/Tests/Spark.Store.MongoDB.Tests/DatabaseMigrationRefreshServiceTests.cs index cc43551f0..e530d2f07 100644 --- a/Tests/Spark.Store.MongoDB.Tests/DatabaseMigrationRefreshServiceTests.cs +++ b/Tests/Spark.Store.MongoDB.Tests/DatabaseMigrationRefreshServiceTests.cs @@ -5,7 +5,7 @@ */ using Microsoft.Extensions.Logging; -using Spark.Engine.Store; +using Moq; using Spark.Engine.Store.Interfaces; using System; using System.Threading; @@ -20,18 +20,23 @@ public partial class DatabaseMigrationRefreshServiceTests public async Task StartAsync_RefreshesImmediately() { TaskCompletionSource refreshed = new(TaskCreationOptions.RunContinuationsAsynchronously); - StubMigrationService migrationService = new((_, _) => - { - refreshed.TrySetResult(); - return Task.CompletedTask; - }); - DatabaseMigrationRefreshService worker = CreateWorker(migrationService, TimeSpan.FromHours(1)); + Mock migrationService = new(); + migrationService + .Setup(service => service.RefreshAsync(It.IsAny())) + .Returns(() => + { + refreshed.TrySetResult(); + return Task.CompletedTask; + }); + DatabaseMigrationRefreshService worker = CreateWorker(migrationService.Object, TimeSpan.FromHours(1)); await worker.StartAsync(TestContext.Current.CancellationToken); try { await refreshed.Task.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); - Assert.Equal(1, migrationService.RefreshCount); + migrationService.Verify( + service => service.RefreshAsync(It.IsAny()), + Times.Once); } finally { @@ -43,20 +48,24 @@ public async Task StartAsync_RefreshesImmediately() public async Task ExecuteAsync_RefreshesPeriodically() { TaskCompletionSource refreshedTwice = new(TaskCreationOptions.RunContinuationsAsynchronously); - StubMigrationService migrationService = new((count, _) => - { - if (count >= 2) - refreshedTwice.TrySetResult(); + int refreshCount = 0; + Mock migrationService = new(); + migrationService + .Setup(service => service.RefreshAsync(It.IsAny())) + .Returns(() => + { + if (Interlocked.Increment(ref refreshCount) >= 2) + refreshedTwice.TrySetResult(); - return Task.CompletedTask; - }); - DatabaseMigrationRefreshService worker = CreateWorker(migrationService, TimeSpan.FromMilliseconds(10)); + return Task.CompletedTask; + }); + DatabaseMigrationRefreshService worker = CreateWorker(migrationService.Object, TimeSpan.FromMilliseconds(10)); await worker.StartAsync(TestContext.Current.CancellationToken); try { await refreshedTwice.Task.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); - Assert.True(migrationService.RefreshCount >= 2); + Assert.True(Volatile.Read(ref refreshCount) >= 2); } finally { @@ -69,20 +78,23 @@ public async Task StopAsync_CancelsActiveRefresh() { TaskCompletionSource refreshStarted = new(TaskCreationOptions.RunContinuationsAsynchronously); TaskCompletionSource refreshCancelled = new(TaskCreationOptions.RunContinuationsAsynchronously); - StubMigrationService migrationService = new(async (_, cancellationToken) => - { - refreshStarted.TrySetResult(); - try - { - await Task.Delay(Timeout.InfiniteTimeSpan, cancellationToken); - } - catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + Mock migrationService = new(); + migrationService + .Setup(service => service.RefreshAsync(It.IsAny())) + .Returns(async (CancellationToken cancellationToken) => { - refreshCancelled.TrySetResult(); - throw; - } - }); - DatabaseMigrationRefreshService worker = CreateWorker(migrationService, TimeSpan.FromHours(1)); + refreshStarted.TrySetResult(); + try + { + await Task.Delay(Timeout.InfiniteTimeSpan, cancellationToken); + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + refreshCancelled.TrySetResult(); + throw; + } + }); + DatabaseMigrationRefreshService worker = CreateWorker(migrationService.Object, TimeSpan.FromHours(1)); await worker.StartAsync(TestContext.Current.CancellationToken); await refreshStarted.Task.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); @@ -95,19 +107,23 @@ public async Task StopAsync_CancelsActiveRefresh() public async Task ExecuteAsync_WhenRefreshFails_LogsAndContinuesRefreshing() { TaskCompletionSource secondRefresh = new(TaskCreationOptions.RunContinuationsAsynchronously); - StubMigrationService migrationService = new((count, _) => + int refreshCount = 0; + Mock migrationService = new(); + migrationService.SetupGet(service => service.CurrentVersion).Returns(2); + migrationService + .Setup(service => service.RefreshAsync(It.IsAny())) + .Returns(() => { + int count = Interlocked.Increment(ref refreshCount); if (count == 1) throw new InvalidOperationException("Refresh failed."); secondRefresh.TrySetResult(); return Task.CompletedTask; - } - ); - migrationService.CurrentVersion = 2; + }); TestLogger logger = new(); DatabaseMigrationRefreshService worker = new( - migrationService, + migrationService.Object, logger, TimeSpan.FromMilliseconds(10)); @@ -116,7 +132,7 @@ public async Task ExecuteAsync_WhenRefreshFails_LogsAndContinuesRefreshing() { await secondRefresh.Task.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); - Assert.Equal(2, migrationService.CurrentVersion); + Assert.Equal(2, migrationService.Object.CurrentVersion); Assert.Contains( logger.Entries, entry => @@ -142,33 +158,4 @@ private static DatabaseMigrationRefreshService CreateWorker( ); } - private sealed class StubMigrationService : IDatabaseMigrationService - { - private readonly Func _refresh; - private int _refreshCount; - - public StubMigrationService(Func refresh) - { - _refresh = refresh; - } - - public int CurrentVersion { get; set; } - - public int RefreshCount => Volatile.Read(ref _refreshCount); - - public bool IsApplied(int version) => version > 0 && version <= CurrentVersion; - - public Task RefreshAsync(CancellationToken cancellationToken = default) - { - int count = Interlocked.Increment(ref _refreshCount); - return _refresh(count, cancellationToken); - } - - public Task RecordCompletedAsync( - DatabaseMigration migration, - CancellationToken cancellationToken = default) - { - throw new NotSupportedException(); - } - } } diff --git a/Tests/Spark.Store.MongoDB.Tests/Spark.Store.MongoDB.Tests.csproj b/Tests/Spark.Store.MongoDB.Tests/Spark.Store.MongoDB.Tests.csproj index 5a8208cf1..188a7c433 100644 --- a/Tests/Spark.Store.MongoDB.Tests/Spark.Store.MongoDB.Tests.csproj +++ b/Tests/Spark.Store.MongoDB.Tests/Spark.Store.MongoDB.Tests.csproj @@ -8,6 +8,7 @@ + all runtime; build; native; contentfiles; analyzers; buildtransitive From 9fc15d578cdee40c5f14c9547be418f94397f4ea Mon Sep 17 00:00:00 2001 From: Kenneth Myhra Date: Wed, 19 Aug 2026 21:21:14 +0200 Subject: [PATCH 2/3] MongoDB: Bootstrap fresh database at current migration --- .../DatabaseMigrationRefreshService.cs | 8 ++- .../DatabaseMigrationService.cs | 38 ++++++++++ .../DatabaseMigrationRefreshServiceTests.cs | 33 ++++++++- .../DatabaseMigrationServiceTests.cs | 71 ++++++++++++++++++- 4 files changed, 144 insertions(+), 6 deletions(-) diff --git a/Libraries/Spark.Store.MongoDB/DatabaseMigrationRefreshService.cs b/Libraries/Spark.Store.MongoDB/DatabaseMigrationRefreshService.cs index f4cfdcc0f..5d98084ed 100644 --- a/Libraries/Spark.Store.MongoDB/DatabaseMigrationRefreshService.cs +++ b/Libraries/Spark.Store.MongoDB/DatabaseMigrationRefreshService.cs @@ -42,10 +42,14 @@ internal DatabaseMigrationRefreshService( _refreshInterval = refreshInterval; } - protected override async Task ExecuteAsync(CancellationToken stoppingToken) + public override async Task StartAsync(CancellationToken cancellationToken) { - await RefreshAsync(stoppingToken).ConfigureAwait(false); + await RefreshAsync(cancellationToken).ConfigureAwait(false); + await base.StartAsync(cancellationToken).ConfigureAwait(false); + } + protected override async Task ExecuteAsync(CancellationToken stoppingToken) + { using PeriodicTimer timer = new(_refreshInterval); try { diff --git a/Libraries/Spark.Store.MongoDB/DatabaseMigrationService.cs b/Libraries/Spark.Store.MongoDB/DatabaseMigrationService.cs index 9b8700569..de71b413a 100644 --- a/Libraries/Spark.Store.MongoDB/DatabaseMigrationService.cs +++ b/Libraries/Spark.Store.MongoDB/DatabaseMigrationService.cs @@ -8,6 +8,7 @@ using MongoDB.Driver; using Spark.Engine.Store; using Spark.Engine.Store.Interfaces; +using Spark.Store.MongoDB.Search.Common; using System; using System.Collections.Generic; using System.Threading; @@ -21,8 +22,11 @@ public sealed class DatabaseMigrationService : IDatabaseMigrationService private const string CompletedAtField = "completedAt"; private readonly IMongoCollection _collection; + private readonly IMongoCollection _resources; + private readonly IMongoCollection _searchIndex; private readonly SemaphoreSlim _stateLock = new(1, 1); private IReadOnlyDictionary _appliedMigrations = new Dictionary(); + private bool _freshDatabaseCheckCompleted; private int _currentVersion; public DatabaseMigrationService(string connectionString) @@ -34,6 +38,8 @@ internal DatabaseMigrationService(IMongoDatabase database) { ArgumentNullException.ThrowIfNull(database); _collection = database.GetCollection(Collection.SchemaMigrations); + _resources = database.GetCollection(Collection.RESOURCE); + _searchIndex = database.GetCollection(MongoCollections.SEARCH_INDEX_COLLECTION); } public int CurrentVersion => Volatile.Read(ref _currentVersion); @@ -51,6 +57,21 @@ public async Task RefreshAsync(CancellationToken cancellationToken = default) .ToListAsync(cancellationToken) .ConfigureAwait(false); + if (documents.Count == 0 && !_freshDatabaseCheckCompleted) + { + bool isFreshDatabase = await IsFreshDatabaseAsync(cancellationToken).ConfigureAwait(false); + if (isFreshDatabase) + { + BsonDocument migration = await UpsertMigrationAsync( + DatabaseMigrations.StructuredStringTokenIndex, + cancellationToken) + .ConfigureAwait(false); + documents.Add(migration); + } + + _freshDatabaseCheckCompleted = true; + } + var appliedMigrations = new Dictionary(documents.Count); var expectedVersion = 1; @@ -76,6 +97,23 @@ public async Task RefreshAsync(CancellationToken cancellationToken = default) } } + private async Task IsFreshDatabaseAsync(CancellationToken cancellationToken) + { + CountOptions options = new() { Limit = 1 }; + long resourceCount = await _resources + .CountDocumentsAsync(FilterDefinition.Empty, options, cancellationToken) + .ConfigureAwait(false); + if (resourceCount != 0) + { + return false; + } + + long searchIndexCount = await _searchIndex + .CountDocumentsAsync(FilterDefinition.Empty, options, cancellationToken) + .ConfigureAwait(false); + return searchIndexCount == 0; + } + public async Task RecordCompletedAsync( DatabaseMigration migration, CancellationToken cancellationToken = default diff --git a/Tests/Spark.Store.MongoDB.Tests/DatabaseMigrationRefreshServiceTests.cs b/Tests/Spark.Store.MongoDB.Tests/DatabaseMigrationRefreshServiceTests.cs index e530d2f07..4606cd43d 100644 --- a/Tests/Spark.Store.MongoDB.Tests/DatabaseMigrationRefreshServiceTests.cs +++ b/Tests/Spark.Store.MongoDB.Tests/DatabaseMigrationRefreshServiceTests.cs @@ -44,6 +44,31 @@ public async Task StartAsync_RefreshesImmediately() } } + [Fact] + public async Task StartAsync_WaitsForInitialRefresh() + { + TaskCompletionSource refreshStarted = new(TaskCreationOptions.RunContinuationsAsynchronously); + TaskCompletionSource completeRefresh = new(TaskCreationOptions.RunContinuationsAsynchronously); + Mock migrationService = new(); + migrationService + .Setup(service => service.RefreshAsync(It.IsAny())) + .Returns(async () => + { + refreshStarted.TrySetResult(); + await completeRefresh.Task; + }); + DatabaseMigrationRefreshService worker = CreateWorker(migrationService.Object, TimeSpan.FromHours(1)); + + Task start = worker.StartAsync(TestContext.Current.CancellationToken); + await refreshStarted.Task.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); + + Assert.False(start.IsCompleted); + + completeRefresh.TrySetResult(); + await start; + await worker.StopAsync(TestContext.Current.CancellationToken); + } + [Fact] public async Task ExecuteAsync_RefreshesPeriodically() { @@ -78,11 +103,17 @@ public async Task StopAsync_CancelsActiveRefresh() { TaskCompletionSource refreshStarted = new(TaskCreationOptions.RunContinuationsAsynchronously); TaskCompletionSource refreshCancelled = new(TaskCreationOptions.RunContinuationsAsynchronously); + int refreshCount = 0; Mock migrationService = new(); migrationService .Setup(service => service.RefreshAsync(It.IsAny())) .Returns(async (CancellationToken cancellationToken) => { + if (Interlocked.Increment(ref refreshCount) == 1) + { + return; + } + refreshStarted.TrySetResult(); try { @@ -94,7 +125,7 @@ public async Task StopAsync_CancelsActiveRefresh() throw; } }); - DatabaseMigrationRefreshService worker = CreateWorker(migrationService.Object, TimeSpan.FromHours(1)); + DatabaseMigrationRefreshService worker = CreateWorker(migrationService.Object, TimeSpan.FromMilliseconds(10)); await worker.StartAsync(TestContext.Current.CancellationToken); await refreshStarted.Task.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); diff --git a/Tests/Spark.Store.MongoDB.Tests/DatabaseMigrationServiceTests.cs b/Tests/Spark.Store.MongoDB.Tests/DatabaseMigrationServiceTests.cs index dbcdf48c8..bea623f3f 100644 --- a/Tests/Spark.Store.MongoDB.Tests/DatabaseMigrationServiceTests.cs +++ b/Tests/Spark.Store.MongoDB.Tests/DatabaseMigrationServiceTests.cs @@ -7,6 +7,7 @@ using MongoDB.Bson; using MongoDB.Driver; using Spark.Engine.Store; +using Spark.Store.MongoDB.Search.Common; using System; using System.Linq; using System.Threading.Tasks; @@ -25,14 +26,76 @@ public class DatabaseMigrationServiceTests : IAsyncLifetime public ValueTask DisposeAsync() => _container.DisposeAsync(); [Fact] - public async Task RefreshAsync_WithNoPersistedMigrations_UsesVersionZero() + public async Task RefreshAsync_WithFreshDatabase_RecordsCurrentMigration() { var service = CreateService(); await service.RefreshAsync(TestContext.Current.CancellationToken); + Assert.Equal(1, service.CurrentVersion); + Assert.True(service.IsApplied(1)); + } + + [Fact] + public async Task RefreshAsync_WithUnversionedResources_UsesVersionZero() + { + string connectionString = CreateConnectionString(); + await GetDatabase(connectionString) + .GetCollection(Collection.RESOURCE) + .InsertOneAsync(new BsonDocument("resource", true), + cancellationToken: TestContext.Current.CancellationToken); + var service = new DatabaseMigrationService(connectionString); + + await service.RefreshAsync(TestContext.Current.CancellationToken); + + Assert.Equal(0, service.CurrentVersion); + Assert.False(service.IsApplied(1)); + Assert.Equal(0, await GetCollection(connectionString) + .CountDocumentsAsync(Builders.Filter.Empty, + cancellationToken: TestContext.Current.CancellationToken)); + } + + [Fact] + public async Task RefreshAsync_WithUnversionedSearchIndex_UsesVersionZero() + { + string connectionString = CreateConnectionString(); + await GetDatabase(connectionString) + .GetCollection(MongoCollections.SEARCH_INDEX_COLLECTION) + .InsertOneAsync(new BsonDocument("index", true), + cancellationToken: TestContext.Current.CancellationToken); + var service = new DatabaseMigrationService(connectionString); + + await service.RefreshAsync(TestContext.Current.CancellationToken); + Assert.Equal(0, service.CurrentVersion); Assert.False(service.IsApplied(1)); + Assert.Equal(0, await GetCollection(connectionString) + .CountDocumentsAsync(Builders.Filter.Empty, + cancellationToken: TestContext.Current.CancellationToken)); + } + + [Fact] + public async Task RefreshAsync_DoesNotReclassifyLegacyDatabaseAsFresh() + { + string connectionString = CreateConnectionString(); + IMongoCollection resources = GetDatabase(connectionString) + .GetCollection(Collection.RESOURCE); + await resources.InsertOneAsync( + new BsonDocument("resource", true), + cancellationToken: TestContext.Current.CancellationToken); + var service = new DatabaseMigrationService(connectionString); + + await service.RefreshAsync(TestContext.Current.CancellationToken); + await resources.DeleteManyAsync( + FilterDefinition.Empty, + TestContext.Current.CancellationToken); + await service.RefreshAsync(TestContext.Current.CancellationToken); + + Assert.Equal(0, service.CurrentVersion); + Assert.False(service.IsApplied(1)); + Assert.Equal(0, await GetCollection(connectionString) + .CountDocumentsAsync(Builders.Filter.Empty, + cancellationToken: TestContext.Current.CancellationToken)); } [Fact] @@ -153,7 +216,6 @@ public async Task RecordCompletedAsync_WhenPersistenceFails_DoesNotAdvanceCache( await using MongoDbContainer container = await StartMongoOrSkipAsync(); string connectionString = BuildConnectionString(container.GetConnectionString(), "migration-failure", 1); var service = new DatabaseMigrationService(connectionString); - await service.RefreshAsync(TestContext.Current.CancellationToken); await container.StopAsync(TestContext.Current.CancellationToken); await Assert.ThrowsAsync(() => @@ -199,9 +261,12 @@ private string CreateConnectionString() => }; private static IMongoCollection GetCollection(string connectionString) => - MongoDatabaseFactory.GetMongoDatabase(connectionString) + GetDatabase(connectionString) .GetCollection(Collection.SchemaMigrations); + private static IMongoDatabase GetDatabase(string connectionString) => + MongoDatabaseFactory.GetMongoDatabase(connectionString); + private static async Task StartMongoOrSkipAsync() { MongoDbContainer container = null; From d25e7ea779654499ddf8e3facfa9363e0c26c4aa Mon Sep 17 00:00:00 2001 From: Kenneth Myhra Date: Tue, 18 Aug 2026 21:37:12 +0200 Subject: [PATCH 3/3] MongoDB: Include plain query in TokenQuery only if migration is not run Only include the plainStringQuery in a token query if DatabaseMigrations.StructuredStringTokenIndex has not been run. --- .../Search/CriteriaMongoExtensions.cs | 83 ++++++++++++++----- .../Search/MongoSearcher.cs | 34 +++++++- ...grationServiceCollectionExtensionsTests.cs | 27 ++++++ .../Search/ChainedSearchErrorHandlingTests.cs | 9 +- .../Search/CriteriumQueryBuilderTests.cs | 48 ++++++++++- .../Search/MongoSearcherMigrationTests.cs | 56 +++++++++++++ .../Search/QuantitySearchIntegrationTests.cs | 9 +- 7 files changed, 241 insertions(+), 25 deletions(-) create mode 100644 Tests/Spark.Store.MongoDB.Tests/Search/MongoSearcherMigrationTests.cs diff --git a/Libraries/Spark.Store.MongoDB/Search/CriteriaMongoExtensions.cs b/Libraries/Spark.Store.MongoDB/Search/CriteriaMongoExtensions.cs index 11df0aef0..db45ad987 100644 --- a/Libraries/Spark.Store.MongoDB/Search/CriteriaMongoExtensions.cs +++ b/Libraries/Spark.Store.MongoDB/Search/CriteriaMongoExtensions.cs @@ -57,7 +57,10 @@ internal static SearchParameter FindSearchParamDefinition(this Criterium param, return param.SearchParameters?.FirstOrDefault(sp => sp.Resource == resourceType || sp.Resource == "Resource"); } - internal static FilterDefinition ToFilter(this Criterium param, string resourceType) + internal static FilterDefinition ToFilter( + this Criterium param, + string resourceType, + bool includePlainStringTokenQuery = true) { //Maybe it's a generic parameter. if (FixedQueries.TryGetValue(param.ParamName, out Func> query)) @@ -69,14 +72,24 @@ internal static FilterDefinition ToFilter(this Criterium param, st { // todo: DSTU2 - modifier not in SearchParameter - return CreateFilter(critSp, param.Operator, param.Modifier, param.Operand); + return CreateFilter( + critSp, + param.Operator, + param.Modifier, + param.Operand, + includePlainStringTokenQuery); //return null; } throw new UnknownSearchParameterException(string.Format("Resource {0} has no parameter with the name {1}.", resourceType, param.ParamName)); } - private static FilterDefinition CreateFilter(SearchParameter parameter, Operator op, String modifier, Expression operand) + private static FilterDefinition CreateFilter( + SearchParameter parameter, + Operator op, + String modifier, + Expression operand, + bool includePlainStringTokenQuery) { if (op == Operator.CHAIN) { @@ -98,7 +111,7 @@ private static FilterDefinition CreateFilter(SearchParameter param switch (parameter.Type) { case SearchParamType.Composite: - return CompositeQuery(parameter, op, modifier, valueOperand); + return CompositeQuery(parameter, op, modifier, valueOperand, includePlainStringTokenQuery); case SearchParamType.Date: return DateQuery(parameterName, op, modifier, valueOperand); case SearchParamType.Number: @@ -117,7 +130,7 @@ private static FilterDefinition CreateFilter(SearchParameter param } else if (modifier == Modifier.IDENTIFIER) { - return TokenQuery(parameterName, op, Modifier.EXACT, valueOperand); + return TokenQuery(parameterName, op, Modifier.EXACT, valueOperand, includePlainStringTokenQuery); } else { @@ -126,7 +139,7 @@ private static FilterDefinition CreateFilter(SearchParameter param case SearchParamType.String: return StringQuery(parameterName, op, modifier, valueOperand); case SearchParamType.Token: - return TokenQuery(parameterName, op, modifier, valueOperand); + return TokenQuery(parameterName, op, modifier, valueOperand, includePlainStringTokenQuery); case SearchParamType.Uri: return UriQuery(parameterName, op, modifier, valueOperand); default: @@ -327,7 +340,12 @@ private static FilterDefinition QuantityQuery(string parameterName return query; } - private static FilterDefinition TokenQuery(String parameterName, Operator optor, String modifier, ValueExpression operand) + private static FilterDefinition TokenQuery( + String parameterName, + Operator optor, + String modifier, + ValueExpression operand, + bool includePlainStringTokenQuery) { //$elemMatch only works on array values. But the MongoIndexMapper only creates an array if there are multiple values for a given parameter. //So we also construct a query for when there is only one set of values in the searchIndex, hence there is no array. @@ -349,14 +367,15 @@ private static FilterDefinition TokenQuery(String parameterName, O var arrayQueries = new List>(); var noArrayQueries = new List>{ Builders.Filter.Not(Builders.Filter.Type(parameterName, BsonType.Array))}; - var plainStringQueries = new List>{ - Builders.Filter.Type(parameterName, BsonType.String)}; + List> plainStringQueries = includePlainStringTokenQuery + ? [Builders.Filter.Type(parameterName, BsonType.String)] + : null; if (!string.IsNullOrEmpty(typedEqOperand.Value)) { noArrayQueries.Add(Builders.Filter.Eq(codefield, typedEqOperand.Value)); arrayQueries.Add(Builders.Filter.Eq("code", typedEqOperand.Value)); - plainStringQueries.Add(Builders.Filter.Eq(parameterName, typedEqOperand.Value)); + plainStringQueries?.Add(Builders.Filter.Eq(parameterName, typedEqOperand.Value)); } //Handle the system part, if present. @@ -366,28 +385,44 @@ private static FilterDefinition TokenQuery(String parameterName, O { arrayQueries.Add(Builders.Filter.Exists("system", false)); noArrayQueries.Add(Builders.Filter.Exists(systemfield, false)); - plainStringQueries.Add(Builders.Filter.Exists("system", false)); + plainStringQueries?.Add(Builders.Filter.Exists("system", false)); } else { arrayQueries.Add(Builders.Filter.Eq("system", typedEqOperand.Namespace)); noArrayQueries.Add(Builders.Filter.Eq(systemfield, typedEqOperand.Namespace)); - plainStringQueries.Add(Builders.Filter.Eq("system", typedEqOperand.Namespace)); + plainStringQueries?.Add(Builders.Filter.Eq("system", typedEqOperand.Namespace)); } } //Combine code and system var arrayEqQuery = Builders.Filter.ElemMatch(parameterName, Builders.Filter.And(arrayQueries)); var noArrayEqQuery = Builders.Filter.And(noArrayQueries); + if (!includePlainStringTokenQuery) + { + return modifier == Modifier.NOT + ? Builders.Filter.And( + Builders.Filter.Not(arrayEqQuery), + Builders.Filter.Not(noArrayEqQuery)) + : Builders.Filter.Or(arrayEqQuery, noArrayEqQuery); + } + var plainStringQuery = Builders.Filter.And(plainStringQueries); - return modifier == Modifier.NOT ? - Builders.Filter.And(Builders.Filter.Not(arrayEqQuery), - Builders.Filter.Not(noArrayEqQuery), Builders.Filter.Not(plainStringQuery)) + return modifier == Modifier.NOT + ? Builders.Filter.And( + Builders.Filter.Not(arrayEqQuery), + Builders.Filter.Not(noArrayEqQuery), + Builders.Filter.Not(plainStringQuery)) : Builders.Filter.Or(arrayEqQuery, noArrayEqQuery, plainStringQuery); } case Operator.IN: IEnumerable opMultiple = ((ChoiceValue)operand).Choices; - var queries = opMultiple.Select(choice => TokenQuery(parameterName, Operator.EQ, modifier, choice)); + var queries = opMultiple.Select(choice => TokenQuery( + parameterName, + Operator.EQ, + modifier, + choice, + includePlainStringTokenQuery)); return modifier == Modifier.NOT ? Builders.Filter.And(queries) : Builders.Filter.Or(queries); case Operator.ISNULL: return Builders.Filter.And(Builders.Filter.Eq(parameterName, BsonNull.Value), Builders.Filter.Eq(textfield, BsonNull.Value)); //We don't use Builders.Filter.NotExists, because that would exclude resources that have this field with an explicit null in it. @@ -478,7 +513,12 @@ private static FilterDefinition DateQuery(String parameterName, Op } } - private static FilterDefinition CompositeQuery(SearchParameter parameterDef, Operator optor, String modifier, ValueExpression operand) + private static FilterDefinition CompositeQuery( + SearchParameter parameterDef, + Operator optor, + String modifier, + ValueExpression operand, + bool includePlainStringTokenQuery) { if (optor == Operator.IN) { @@ -486,7 +526,12 @@ private static FilterDefinition CompositeQuery(SearchParameter par var queries = new List>(); foreach (var choice in choices.Choices) { - queries.Add(CompositeQuery(parameterDef, Operator.EQ, modifier, choice)); + queries.Add(CompositeQuery( + parameterDef, + Operator.EQ, + modifier, + choice, + includePlainStringTokenQuery)); } return Builders.Filter.Or(queries); } @@ -511,7 +556,7 @@ private static FilterDefinition CompositeQuery(SearchParameter par Operand = components[i], Modifier = modifier }; - queries.Add(subCrit.ToFilter(parameterDef.Resource)); + queries.Add(subCrit.ToFilter(parameterDef.Resource, includePlainStringTokenQuery)); } return Builders.Filter.And(queries); } diff --git a/Libraries/Spark.Store.MongoDB/Search/MongoSearcher.cs b/Libraries/Spark.Store.MongoDB/Search/MongoSearcher.cs index 5d223aecb..e4546daac 100644 --- a/Libraries/Spark.Store.MongoDB/Search/MongoSearcher.cs +++ b/Libraries/Spark.Store.MongoDB/Search/MongoSearcher.cs @@ -13,6 +13,8 @@ using Spark.Engine.Core; using Spark.Engine.Search; using Spark.Engine.Search.Types; +using Spark.Engine.Store; +using Spark.Engine.Store.Interfaces; using Spark.Store.MongoDB.Search.Common; using System; using System.Collections.Generic; @@ -27,8 +29,27 @@ public class MongoSearcher private readonly IMongoCollection _collection; private readonly ILocalhost _localhost; private readonly IFhirModel _fhirModel; + private readonly IDatabaseMigrationService _databaseMigrationService; private readonly IReferenceNormalizationService _referenceNormalizationService; + public MongoSearcher( + MongoIndexStore mongoIndexStore, + ILocalhost localhost, + IFhirModel fhirModel, + IReferenceNormalizationService referenceNormalizationService, + IDatabaseMigrationService databaseMigrationService) + { + _collection = mongoIndexStore.Collection; + _localhost = localhost; + _fhirModel = fhirModel; + _databaseMigrationService = databaseMigrationService ?? + throw new ArgumentNullException(nameof(databaseMigrationService)); + _referenceNormalizationService = referenceNormalizationService; + } + + [Obsolete( + "Use MongoSearcher(MongoIndexStore, ILocalhost, IFhirModel, IReferenceNormalizationService, IDatabaseMigrationService) instead." + )] public MongoSearcher(MongoIndexStore mongoIndexStore, ILocalhost localhost, IFhirModel fhirModel, IReferenceNormalizationService referenceNormalizationService = null) { @@ -116,6 +137,10 @@ private SearchResults KeysToSearchResults(IEnumerable keys) return results; } + internal bool IncludePlainStringTokenQuery => + _databaseMigrationService == null + || !_databaseMigrationService.IsApplied(DatabaseMigrations.StructuredStringTokenIndex.Version); + private List CollectKeys(string resourceType, IEnumerable criteria, int level = 0) { return CollectKeys(resourceType, criteria, null, level); @@ -197,9 +222,14 @@ private static SortDefinition CreateSortBy(IList<(string, SortOrde } - private static FilterDefinition CreateMongoQuery(string resourceType, SearchResults results, int level, Dictionary closedCriteria) + private FilterDefinition CreateMongoQuery( + string resourceType, + SearchResults results, + int level, + Dictionary closedCriteria) { FilterDefinition resultQuery = CriteriaMongoExtensions.ResourceFilter(resourceType, level); + bool includePlainStringTokenQuery = IncludePlainStringTokenQuery; if (closedCriteria.Count > 0) { var criteriaQueries = new List>(); @@ -209,7 +239,7 @@ private static FilterDefinition CreateMongoQuery(string resourceTy { try { - criteriaQueries.Add(crit.Value.ToFilter(resourceType)); + criteriaQueries.Add(crit.Value.ToFilter(resourceType, includePlainStringTokenQuery)); } catch (ArgumentException ex) { diff --git a/Tests/Spark.Store.MongoDB.Tests/DatabaseMigrationServiceCollectionExtensionsTests.cs b/Tests/Spark.Store.MongoDB.Tests/DatabaseMigrationServiceCollectionExtensionsTests.cs index ca7058cdb..dae9f0a57 100644 --- a/Tests/Spark.Store.MongoDB.Tests/DatabaseMigrationServiceCollectionExtensionsTests.cs +++ b/Tests/Spark.Store.MongoDB.Tests/DatabaseMigrationServiceCollectionExtensionsTests.cs @@ -6,9 +6,15 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; +using Moq; using Spark.Engine; +using Spark.Engine.Core; +using Spark.Engine.Search; +using Spark.Engine.Store; using Spark.Engine.Store.Interfaces; using Spark.Store.MongoDB.Extensions; +using Spark.Store.MongoDB.Search; +using System; using Xunit; namespace Spark.Store.MongoDB.Tests; @@ -46,6 +52,27 @@ public void AddMongoFhirStore_WhenCalledTwice_RegistersOneMigrationRefreshServic Assert.Equal(ServiceLifetime.Singleton, descriptor.Lifetime); } + [Fact] + public void AddMongoFhirStore_MongoSearcherUsesRegisteredMigrationService() + { + ServiceCollection services = new(); + Mock migrationService = new(); + migrationService + .Setup(service => service.IsApplied(DatabaseMigrations.StructuredStringTokenIndex.Version)) + .Returns(true); + Localhost localhost = new(new Uri("http://localhost/fhir")); + services.AddSingleton(migrationService.Object); + services.AddSingleton(localhost); + services.AddSingleton(new Mock().Object); + services.AddSingleton(new ReferenceNormalizationService(localhost)); + services.AddMongoFhirStore(CreateSettings()); + + using ServiceProvider provider = services.BuildServiceProvider(); + + MongoSearcher searcher = provider.GetRequiredService(); + Assert.False(searcher.IncludePlainStringTokenQuery); + } + private static StoreSettings CreateSettings() => new() { ConnectionString = "mongodb://localhost/spark" diff --git a/Tests/Spark.Store.MongoDB.Tests/Search/ChainedSearchErrorHandlingTests.cs b/Tests/Spark.Store.MongoDB.Tests/Search/ChainedSearchErrorHandlingTests.cs index c57e69aa8..c9207a3a8 100644 --- a/Tests/Spark.Store.MongoDB.Tests/Search/ChainedSearchErrorHandlingTests.cs +++ b/Tests/Spark.Store.MongoDB.Tests/Search/ChainedSearchErrorHandlingTests.cs @@ -9,9 +9,11 @@ using Hl7.Fhir.Rest; using Hl7.Fhir.Specification; using MongoDB.Driver; +using Moq; using Spark.Engine.Core; using Spark.Engine.Search; using Spark.Engine.Service.FhirServiceExtensions; +using Spark.Engine.Store.Interfaces; using Spark.Store.MongoDB.Search; using Spark.Store.MongoDB.Search.Common; using Spark.Store.MongoDB.Search.Indexer; @@ -164,7 +166,12 @@ private static async System.Threading.Tasks.Task SeedSearcherAsyn var indexStore = new MongoIndexStore(connectionString, new MongoIndexMapper()); var indexService = new IndexService(fhirModel, indexStore, new ElementIndexer(fhirModel), new ResourceResolver(fhirModel.SupportedResources, new PocoStructureDefinitionSummaryProvider())); - var searcher = new MongoSearcher(indexStore, localhost, fhirModel, new ReferenceNormalizationService(localhost)); + var searcher = new MongoSearcher( + indexStore, + localhost, + fhirModel, + new ReferenceNormalizationService(localhost), + new Mock().Object); // 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" }; diff --git a/Tests/Spark.Store.MongoDB.Tests/Search/CriteriumQueryBuilderTests.cs b/Tests/Spark.Store.MongoDB.Tests/Search/CriteriumQueryBuilderTests.cs index b0d315226..1424d4c9c 100644 --- a/Tests/Spark.Store.MongoDB.Tests/Search/CriteriumQueryBuilderTests.cs +++ b/Tests/Spark.Store.MongoDB.Tests/Search/CriteriumQueryBuilderTests.cs @@ -37,6 +37,46 @@ public void Can_Build_TokenQuery_Filter(ResourceType resourceType, string search Assert.Equal(expected, jsonFilter); } + [Theory] + [InlineData( + ResourceType.Condition, + "code", + "code=ha125", + "{ \"$or\" : [{ \"code\" : { \"$elemMatch\" : { \"code\" : \"ha125\" } } }, { \"code\" : { \"$not\" : { \"$type\" : 4 } }, \"code.code\" : \"ha125\" }] }")] + [InlineData( + ResourceType.Patient, + "gender", + "gender:not=male", + "{ \"gender\" : { \"$not\" : { \"$elemMatch\" : { \"code\" : \"male\" } } }, \"$nor\" : [{ \"gender\" : { \"$not\" : { \"$type\" : 4 } }, \"gender.code\" : \"male\" }] }")] + public void MigratedTokenQuery_OmitsPlainStringBranch( + ResourceType resourceType, + string searchParameter, + string query, + string expected) + { + string jsonFilter = BuildAndReturnQueryFilterAsJsonString( + resourceType, + searchParameter, + query, + includePlainStringTokenQuery: false); + + Assert.Equal(expected, jsonFilter); + } + + [Fact] + public void MigratedMultiValueTokenQuery_OmitsPlainStringBranchFromEveryChoice() + { + string jsonFilter = BuildAndReturnQueryFilterAsJsonString( + ResourceType.Patient, + "gender", + "gender=male,female", + includePlainStringTokenQuery: false); + + Assert.DoesNotContain("\"$type\" : 2", jsonFilter); + Assert.Contains("\"gender.code\" : \"male\"", jsonFilter); + Assert.Contains("\"gender.code\" : \"female\"", jsonFilter); + } + [Theory] [InlineData(ResourceType.RiskAssessment, "probability", "probability=0.8", "{ \"probability\" : \"0.8\" }")] [InlineData(ResourceType.RiskAssessment, "probability", "probability=eq0.8", "{ \"probability\" : \"0.8\" }")] @@ -105,7 +145,11 @@ public void Can_Build_UriQuery_Filter(ResourceType resourceType, string searchPa Assert.Equal(expected, jsonFilter); } - private string BuildAndReturnQueryFilterAsJsonString(ResourceType resourceType, string searchParameter, string query) + private string BuildAndReturnQueryFilterAsJsonString( + ResourceType resourceType, + string searchParameter, + string query, + bool includePlainStringTokenQuery = true) { var fhirModel = new FhirModel(); var bsonSerializerRegistry = new BsonSerializerRegistry(); @@ -117,7 +161,7 @@ private string BuildAndReturnQueryFilterAsJsonString(ResourceType resourceType, var criterium = Criterium.Parse(fhirModel.SearchParameters, resourceTypeAsString, keyVal.Item1, keyVal.Item2); criterium.SearchParameters.AddRange(fhirModel.FindSearchParameters(resourceTypeAsString).Where(sp => sp.Name == searchParameter)); - var filter = criterium.ToFilter(resourceType.GetLiteral()); + var filter = criterium.ToFilter(resourceType.GetLiteral(), includePlainStringTokenQuery); var jsonFilter = filter.Render(new RenderArgs(bsonSerializerRegistry.GetSerializer(), bsonSerializerRegistry)).ToJson(); return jsonFilter; diff --git a/Tests/Spark.Store.MongoDB.Tests/Search/MongoSearcherMigrationTests.cs b/Tests/Spark.Store.MongoDB.Tests/Search/MongoSearcherMigrationTests.cs new file mode 100644 index 000000000..06c9ebf50 --- /dev/null +++ b/Tests/Spark.Store.MongoDB.Tests/Search/MongoSearcherMigrationTests.cs @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2026, Incendi + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +using Spark.Engine.Core; +using Spark.Engine.Store; +using Spark.Engine.Store.Interfaces; +using Spark.Store.MongoDB.Search; +using Spark.Store.MongoDB.Search.Common; +using Spark.Store.MongoDB.Search.Indexer; +using Moq; +using System; +using Xunit; + +namespace Spark.Store.MongoDB.Tests.Search; + +public class MongoSearcherMigrationTests +{ + [Fact] + public void IncludePlainStringTokenQuery_ReflectsCurrentMigrationState() + { + int currentVersion = 0; + Mock migrationService = new(); + migrationService + .Setup(service => service.IsApplied(DatabaseMigrations.StructuredStringTokenIndex.Version)) + .Returns(() => currentVersion >= DatabaseMigrations.StructuredStringTokenIndex.Version); + MongoSearcher searcher = new( + new MongoIndexStore("mongodb://localhost/spark", new MongoIndexMapper()), + new Localhost(new Uri("http://localhost/fhir")), + new Mock().Object, + referenceNormalizationService: null, + databaseMigrationService: migrationService.Object); + + Assert.True(searcher.IncludePlainStringTokenQuery); + + currentVersion = 1; + + Assert.False(searcher.IncludePlainStringTokenQuery); + } + + [Fact] + public void LegacyConstructor_IncludesPlainStringTokenQuery() + { +#pragma warning disable CS0618 + MongoSearcher searcher = new( + new MongoIndexStore("mongodb://localhost/spark", new MongoIndexMapper()), + new Localhost(new Uri("http://localhost/fhir")), + new Mock().Object, + null); +#pragma warning restore CS0618 + + Assert.True(searcher.IncludePlainStringTokenQuery); + } +} diff --git a/Tests/Spark.Store.MongoDB.Tests/Search/QuantitySearchIntegrationTests.cs b/Tests/Spark.Store.MongoDB.Tests/Search/QuantitySearchIntegrationTests.cs index 5b8d7e68b..f8b2892d0 100644 --- a/Tests/Spark.Store.MongoDB.Tests/Search/QuantitySearchIntegrationTests.cs +++ b/Tests/Spark.Store.MongoDB.Tests/Search/QuantitySearchIntegrationTests.cs @@ -8,9 +8,11 @@ using Hl7.Fhir.Rest; using Hl7.Fhir.Specification; using MongoDB.Driver; +using Moq; using Spark.Engine.Core; using Spark.Engine.Search; using Spark.Engine.Service.FhirServiceExtensions; +using Spark.Engine.Store.Interfaces; using Spark.Store.MongoDB.Search; using Spark.Store.MongoDB.Search.Common; using Spark.Store.MongoDB.Search.Indexer; @@ -142,7 +144,12 @@ private static async Task SeedStoreAndReturnSearcherAsync(MongoDb MongoIndexStore indexStore = new(connectionString, new MongoIndexMapper()); IndexService indexService = new(fhirModel, indexStore, new ElementIndexer(fhirModel), new ResourceResolver(fhirModel.SupportedResources, new PocoStructureDefinitionSummaryProvider())); - MongoSearcher searcher = new(indexStore, localhost, fhirModel, new ReferenceNormalizationService(localhost)); + MongoSearcher searcher = new( + indexStore, + localhost, + fhirModel, + new ReferenceNormalizationService(localhost), + new Mock().Object); foreach (Resource resource in resources) {