Add new SQL hash calculation (#4737)

microsoft · Dec 6, 2024 · cb53a35 · cb53a35
1 parent c3a3c19
commit cb53a35
Show file tree

Hide file tree

Showing 11 changed files with 107 additions and 79 deletions.
diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/ISqlQueryHashCalculator.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/ISqlQueryHashCalculator.cs
diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlQueryHashCalculator.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlQueryHashCalculator.cs
diff --git 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
         /// </summary>
         public SqlSearchOptions CloneSqlSearchOptions() => (SqlSearchOptions)MemberwiseClone();
+
+        /// <summary>
+        /// 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
+        /// </summary>
+        /// <returns>A hash of the search options</returns>
+        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
@@ -70,7 +70,6 @@ internal class SqlServerSearchService : SearchService
         private readonly RequestContextAccessor<IFhirRequestContext> _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<IFhirRequestContext> requestContextAccessor,
             ICompressedRawResourceConverter compressedRawResourceConverter,
-            ISqlQueryHashCalculator queryHashCalculator,
             IParameterStore parameterStore,
             ILogger<SqlServerSearchService> 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/...ft.Health.Fhir.SqlServer/Registration/FhirServerBuilderSqlServerRegistrationExtensions.cs b/...ft.Health.Fhir.SqlServer/Registration/FhirServerBuilderSqlServerRegistrationExtensions.cs
@@ -73,10 +73,6 @@ public static IFhirServerBuilder AddSqlServer(this IFhirServerBuilder fhirServer
                 .AsSelf()
                 .AsImplementedInterfaces();
 
-            services.Add<SqlQueryHashCalculator>()
-                .Singleton()
-                .AsImplementedInterfaces();
-
             services.Add<SqlServerSearchService>()
                 .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
@@ -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<T> : ILogger<T>
+    {
+        private List<string> _logs = new List<string>();
+
+        public IDisposable BeginScope<TState>(TState state)
+        {
+            throw new NotImplementedException();
+        }
+
+        public bool IsEnabled(LogLevel logLevel)
+        {
+            throw new NotImplementedException();
+        }
+
+        public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> 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/...th.Fhir.Shared.Tests.Integration/Microsoft.Health.Fhir.Shared.Tests.Integration.projitems b/...th.Fhir.Shared.Tests.Integration/Microsoft.Health.Fhir.Shared.Tests.Integration.projitems
@@ -23,6 +23,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Features\Operations\Reindex\ReindexJobTests.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Features\Operations\Reindex\ReindexSearchTests.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Features\Smart\SmartSearchTests.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Helpers\ReadableLogger.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Persistence\SqlAzurePipelinesWorkloadIdentityAuthenticationProvider.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Persistence\SqlComplexQueryTests.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Persistence\FhirStorageVersioningPolicyTests.cs" />
@@ -49,6 +50,5 @@
     <Compile Include="$(MSBuildThisFileDirectory)Persistence\SqlServerSqlCompatibilityTests.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Persistence\QueueClientTests.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Persistence\SqlServerTransactionScopeTests.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)Persistence\TestSqlHashCalculator.cs" />
   </ItemGroup>
 </Project>
diff --git 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<IFhirRequestContext> FhirRequestContextAccessor => _fixture.GetRequiredService<RequestContextAccessor<IFhirRequestContext>>();
 
-        public TestSqlHashCalculator SqlQueryHashCalculator => _fixture.GetRequiredService<TestSqlHashCalculator>();
-
         public GetResourceHandler GetResourceHandler { get; set; }
 
         public IQueueClient QueueClient => _fixture.GetRequiredService<IQueueClient>();
 
+        internal ReadableLogger<SqlServerSearchService> ReadableLogger => _fixture.GetRequiredService<ReadableLogger<SqlServerSearchService>>();
+
         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
@@ -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/...soft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerFhirStorageTestsFixture.cs b/...soft.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<SqlServerSearchService> _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<ICompartmentDefinitionManager>(() => compartmentDefinitionManager), new Lazy<ISearchParameterDefinitionManager>(() => _searchParameterDefinitionManager));
             var smartCompartmentSearchRewriter = new SmartCompartmentSearchRewriter(compartmentSearchRewriter, new Lazy<ISearchParameterDefinitionManager>(() => _searchParameterDefinitionManager));
 
-            SqlQueryHashCalculator = new TestSqlHashCalculator();
+            _readableLogger = new ReadableLogger<SqlServerSearchService>();
 
             _searchService = new SqlServerSearchService(
                 searchOptionsFactory,
@@ -295,9 +294,8 @@ public async Task InitializeAsync()
                 SchemaInformation,
                 _fhirRequestContextAccessor,
                 new CompressedRawResourceConverter(),
-                SqlQueryHashCalculator,
                 new SqlServerParameterStore(SqlConnectionBuilder, NullLogger<SqlServerParameterStore>.Instance),
-                NullLogger<SqlServerSearchService>.Instance);
+                _readableLogger);
 
             ISearchParameterSupportResolver searchParameterSupportResolver = Substitute.For<ISearchParameterSupportResolver>();
             searchParameterSupportResolver.IsSearchParameterSupported(Arg.Any<SearchParameterInfo>()).Returns((true, false));
@@ -413,9 +411,9 @@ object IServiceProvider.GetService(Type serviceType)
                 return _sqlQueueClient;
             }
 
-            if (serviceType == typeof(TestSqlHashCalculator))
+            if (serviceType == typeof(ReadableLogger<SqlServerSearchService>))
             {
-                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