diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/ISqlQueryHashCalculator.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/ISqlQueryHashCalculator.cs deleted file mode 100644 index 34ddc55067..0000000000 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/ISqlQueryHashCalculator.cs +++ /dev/null @@ -1,17 +0,0 @@ -// ------------------------------------------------------------------------------------------------- -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. -// ------------------------------------------------------------------------------------------------- - -namespace Microsoft.Health.Fhir.SqlServer.Features.Search -{ - public interface ISqlQueryHashCalculator - { - /// - /// Given a string that represents a SQL query, this returns the calculated hash of that query. - /// - /// The SQL query as text - /// A string hash value. - string CalculateHash(string query); - } -} diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlQueryHashCalculator.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlQueryHashCalculator.cs deleted file mode 100644 index 76725c7e90..0000000000 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlQueryHashCalculator.cs +++ /dev/null @@ -1,17 +0,0 @@ -// ------------------------------------------------------------------------------------------------- -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. -// ------------------------------------------------------------------------------------------------- - -using Microsoft.Health.Core.Extensions; - -namespace Microsoft.Health.Fhir.SqlServer.Features.Search -{ - internal class SqlQueryHashCalculator : ISqlQueryHashCalculator - { - public string CalculateHash(string query) - { - return query.ComputeHash(); - } - } -} diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchOptions.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchOptions.cs index a1a6566a21..890ca122a5 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchOptions.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchOptions.cs @@ -3,7 +3,12 @@ // Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. // ------------------------------------------------------------------------------------------------- +using System; +using System.Linq; +using System.Security.Cryptography; +using Microsoft.Health.Core.Extensions; using Microsoft.Health.Fhir.Core.Features.Search; +using Microsoft.Health.Fhir.Core.Models; namespace Microsoft.Health.Fhir.SqlServer.Features.Search { @@ -39,5 +44,30 @@ public SqlSearchOptions(SearchOptions searchOptions) /// Performs a shallow clone of this instance /// public SqlSearchOptions CloneSqlSearchOptions() => (SqlSearchOptions)MemberwiseClone(); + + /// + /// Hashes the search option to indicate if two search options will return the same results. + /// UnsupportedSearchParams isn't inlcuded in the has because it isn't used in the actual search + /// + /// A hash of the search options + public string GetHash() + { + var expressionHash = default(HashCode); + Expression?.AddValueInsensitiveHashCode(ref expressionHash); + + var sort = Sort?.Aggregate(string.Empty, (string result, (SearchParameterInfo param, SortOrder order) input) => + { + return result + $"{input.param.Url}_{input.order}_"; + }); + + var queryHints = QueryHints?.Aggregate(string.Empty, (string result, (string param, string value) input) => + { + return result + $"{input.param}_{input.value}_"; + }); + + var hashString = $"{ContinuationToken}_{FeedRange}_{CountOnly}_{IgnoreSearchParamHash}_{IncludeTotal}_{MaxItemCount}_{MaxItemCountSpecifiedByClient}_{IncludeCount}_{ResourceVersionTypes}_{OnlyIds}_{IsLargeAsyncOperation}_{SortQuerySecondPhase}_{IsSortWithFilter}_{DidWeSearchForSortValue}_{SortHasMissingModifier}_{expressionHash.ToHashCode()}_{sort}_{queryHints}"; + + return hashString.ComputeHash(); + } } } diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs index a64fc6ac83..5b61d12661 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs @@ -70,7 +70,6 @@ internal class SqlServerSearchService : SearchService private readonly RequestContextAccessor _requestContextAccessor; private const int _defaultNumberOfColumnsReadFromResult = 11; private readonly SearchParameterInfo _fakeLastUpdate = new SearchParameterInfo(SearchParameterNames.LastUpdated, SearchParameterNames.LastUpdated); - private readonly ISqlQueryHashCalculator _queryHashCalculator; private readonly IParameterStore _parameterStore; private static ResourceSearchParamStats _resourceSearchParamStats; private static object _locker = new object(); @@ -92,7 +91,6 @@ public SqlServerSearchService( SchemaInformation schemaInformation, RequestContextAccessor requestContextAccessor, ICompressedRawResourceConverter compressedRawResourceConverter, - ISqlQueryHashCalculator queryHashCalculator, IParameterStore parameterStore, ILogger logger) : base(searchOptionsFactory, fhirDataStore, logger) @@ -117,7 +115,6 @@ public SqlServerSearchService( _smartCompartmentSearchRewriter = smartCompartmentSearchRewriter; _chainFlatteningRewriter = chainFlatteningRewriter; _sqlRetryService = sqlRetryService; - _queryHashCalculator = queryHashCalculator; _parameterStore = parameterStore; _logger = logger; @@ -365,7 +362,7 @@ await _sqlRetryService.ExecuteSql( SqlCommandSimplifier.RemoveRedundantParameters(stringBuilder, sqlCommand.Parameters, _logger); var queryText = stringBuilder.ToString(); - var queryHash = _queryHashCalculator.CalculateHash(queryText); + var queryHash = clonedSearchOptions.GetHash(); _logger.LogInformation("SQL Search Service query hash: {QueryHash}", queryHash); var customQuery = CustomQueries.CheckQueryHash(connection, queryHash, _logger); diff --git a/src/Microsoft.Health.Fhir.SqlServer/Registration/FhirServerBuilderSqlServerRegistrationExtensions.cs b/src/Microsoft.Health.Fhir.SqlServer/Registration/FhirServerBuilderSqlServerRegistrationExtensions.cs index 22dbea0a7b..2d648bff18 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Registration/FhirServerBuilderSqlServerRegistrationExtensions.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Registration/FhirServerBuilderSqlServerRegistrationExtensions.cs @@ -73,10 +73,6 @@ public static IFhirServerBuilder AddSqlServer(this IFhirServerBuilder fhirServer .AsSelf() .AsImplementedInterfaces(); - services.Add() - .Singleton() - .AsImplementedInterfaces(); - services.Add() .Scoped() .AsSelf() diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Helpers/ReadableLogger.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Helpers/ReadableLogger.cs new file mode 100644 index 0000000000..6bc03072eb --- /dev/null +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Helpers/ReadableLogger.cs @@ -0,0 +1,59 @@ +// ------------------------------------------------------------------------------------------------- +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. +// ------------------------------------------------------------------------------------------------- + +using System; +using System.Collections.Generic; +using System.Linq; +using Microsoft.Extensions.Logging; + +namespace Microsoft.Health.Fhir.Tests.Integration +{ + public class ReadableLogger : ILogger + { + private List _logs = new List(); + + public IDisposable BeginScope(TState state) + { + throw new NotImplementedException(); + } + + public bool IsEnabled(LogLevel logLevel) + { + throw new NotImplementedException(); + } + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) + { + _logs.Add(formatter(state, exception)); + } + + public void LogError(string message) + { + _logs.Add(message); + } + + public void LogInformation(string message) + { + _logs.Add(message); + } + + public void LogWarning(string message) + { + _logs.Add(message); + } + + public bool TryGetLatestLog(string content, out string match) + { + if (_logs.Any(l => l.Contains(content))) + { + match = _logs.FindLast(l => l.Contains(content)); + return true; + } + + match = null; + return false; + } + } +} diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Microsoft.Health.Fhir.Shared.Tests.Integration.projitems b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Microsoft.Health.Fhir.Shared.Tests.Integration.projitems index 6dae7f7621..52dbbe5401 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Microsoft.Health.Fhir.Shared.Tests.Integration.projitems +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Microsoft.Health.Fhir.Shared.Tests.Integration.projitems @@ -23,6 +23,7 @@ + @@ -49,6 +50,5 @@ - \ No newline at end of file diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/FhirStorageTestsFixture.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/FhirStorageTestsFixture.cs index bbef6e8fe3..4522299557 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/FhirStorageTestsFixture.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/FhirStorageTestsFixture.cs @@ -48,6 +48,7 @@ using Microsoft.Health.Fhir.Core.Models; using Microsoft.Health.Fhir.Core.UnitTests; using Microsoft.Health.Fhir.Core.UnitTests.Extensions; +using Microsoft.Health.Fhir.SqlServer.Features.Search; using Microsoft.Health.Fhir.Tests.Common; using Microsoft.Health.Fhir.Tests.Common.FixtureParameters; using Microsoft.Health.Fhir.Tests.Common.Mocks; @@ -125,12 +126,12 @@ internal FhirStorageTestsFixture(IServiceProvider fixture) public RequestContextAccessor FhirRequestContextAccessor => _fixture.GetRequiredService>(); - public TestSqlHashCalculator SqlQueryHashCalculator => _fixture.GetRequiredService(); - public GetResourceHandler GetResourceHandler { get; set; } public IQueueClient QueueClient => _fixture.GetRequiredService(); + internal ReadableLogger ReadableLogger => _fixture.GetRequiredService>(); + public void Dispose() { (_fixture as IDisposable)?.Dispose(); diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlComplexQueryTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlComplexQueryTests.cs index 6eee24e19e..2b34337f11 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlComplexQueryTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlComplexQueryTests.cs @@ -187,7 +187,8 @@ public async Task GivenASqlQuery_IfAStoredProcExistsWithMatchingHash_ThenStoredP // Query before adding an sproc to the database await _fixture.SearchService.SearchAsync(KnownResourceTypes.Patient, query, CancellationToken.None); - var hash = _fixture.SqlQueryHashCalculator.MostRecentSqlHash; + var hash = _fixture.ReadableLogger.TryGetLatestLog("SQL Search Service query hash:", out var hashValue) ? hashValue.Substring(31) : null; + Assert.NotNull(hash); // assert an sproc was not used Assert.False(await CheckIfSprocUsed(hash)); @@ -199,11 +200,16 @@ public async Task GivenASqlQuery_IfAStoredProcExistsWithMatchingHash_ThenStoredP // Query after adding an sproc to the database var sw = Stopwatch.StartNew(); var sprocWasUsed = false; + + // Change parameter values to test that hash is independent of parameter values + query = new[] { Tuple.Create("birthdate", "gt1900-01-01"), Tuple.Create("birthdate", "lt2000-01-01"), Tuple.Create("address-city", "Town"), Tuple.Create("address-state", "State") }; + while (sw.Elapsed.TotalSeconds < 100) // previous single try after 1.1 sec delay was not reliable. { await Task.Delay(300); await _fixture.SearchService.SearchAsync(KnownResourceTypes.Patient, query, CancellationToken.None); - Assert.Equal(hash, _fixture.SqlQueryHashCalculator.MostRecentSqlHash); + var newHash = _fixture.ReadableLogger.TryGetLatestLog("SQL Search Service query hash:", out var newHashValue) ? newHashValue.Substring(31) : null; + Assert.Equal(hash, newHash); if (await CheckIfSprocUsed(hash)) { sprocWasUsed = true; diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerFhirStorageTestsFixture.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerFhirStorageTestsFixture.cs index 4be4aa54a3..9355c4fb35 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerFhirStorageTestsFixture.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerFhirStorageTestsFixture.cs @@ -77,6 +77,7 @@ public class SqlServerFhirStorageTestsFixture : IServiceProvider, IAsyncLifetime private SupportedSearchParameterDefinitionManager _supportedSearchParameterDefinitionManager; private SearchParameterStatusManager _searchParameterStatusManager; private SqlQueueClient _sqlQueueClient; + private ReadableLogger _readableLogger; public SqlServerFhirStorageTestsFixture() : this(SchemaVersionConstants.Max, GetDatabaseName()) @@ -125,8 +126,6 @@ internal SqlServerFhirStorageTestsFixture(int maximumSupportedSchemaVersion, str internal SchemaInformation SchemaInformation { get; private set; } - internal ISqlQueryHashCalculator SqlQueryHashCalculator { get; private set; } - internal static string GetDatabaseName(string test = null) { return $"{ModelInfoProvider.Version}{(test == null ? string.Empty : $"_{test}")}_{DateTimeOffset.UtcNow.ToString("s").Replace("-", string.Empty).Replace(":", string.Empty)}_{Guid.NewGuid().ToString().Replace("-", string.Empty)}"; @@ -278,7 +277,7 @@ public async Task InitializeAsync() var compartmentSearchRewriter = new CompartmentSearchRewriter(new Lazy(() => compartmentDefinitionManager), new Lazy(() => _searchParameterDefinitionManager)); var smartCompartmentSearchRewriter = new SmartCompartmentSearchRewriter(compartmentSearchRewriter, new Lazy(() => _searchParameterDefinitionManager)); - SqlQueryHashCalculator = new TestSqlHashCalculator(); + _readableLogger = new ReadableLogger(); _searchService = new SqlServerSearchService( searchOptionsFactory, @@ -295,9 +294,8 @@ public async Task InitializeAsync() SchemaInformation, _fhirRequestContextAccessor, new CompressedRawResourceConverter(), - SqlQueryHashCalculator, new SqlServerParameterStore(SqlConnectionBuilder, NullLogger.Instance), - NullLogger.Instance); + _readableLogger); ISearchParameterSupportResolver searchParameterSupportResolver = Substitute.For(); searchParameterSupportResolver.IsSearchParameterSupported(Arg.Any()).Returns((true, false)); @@ -413,9 +411,9 @@ object IServiceProvider.GetService(Type serviceType) return _sqlQueueClient; } - if (serviceType == typeof(TestSqlHashCalculator)) + if (serviceType == typeof(ReadableLogger)) { - return SqlQueryHashCalculator as TestSqlHashCalculator; + return _readableLogger; } return null; diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/TestSqlHashCalculator.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/TestSqlHashCalculator.cs deleted file mode 100644 index 9e7922ecf0..0000000000 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/TestSqlHashCalculator.cs +++ /dev/null @@ -1,25 +0,0 @@ -// ------------------------------------------------------------------------------------------------- -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. -// ------------------------------------------------------------------------------------------------- - -using Microsoft.Health.Core.Extensions; -using Microsoft.Health.Fhir.SqlServer.Features.Search; - -namespace Microsoft.Health.Fhir.Tests.Integration.Persistence -{ - public class TestSqlHashCalculator : ISqlQueryHashCalculator - { - public string MostRecentSqlQuery { get; set; } - - public string MostRecentSqlHash { get; set; } - - public string CalculateHash(string query) - { - MostRecentSqlQuery = query; - MostRecentSqlHash = query.ComputeHash(); - - return MostRecentSqlHash; - } - } -}