Revert "Add new SQL hash calculation (#4737)" (#4770)

This reverts commit cb53a35.
microsoft · Jan 14, 2025 · bc6b453 · bc6b453
1 parent 6f44856
commit bc6b453
Show file tree

Hide file tree

Showing 11 changed files with 79 additions and 107 deletions.
diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/ISqlQueryHashCalculator.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/ISqlQueryHashCalculator.cs
@@ -0,0 +1,17 @@
+// -------------------------------------------------------------------------------------------------
+// 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
+    {
+        /// <summary>
+        /// Given a string that represents a SQL query, this returns the calculated hash of that query.
+        /// </summary>
+        /// <param name="query">The SQL query as text</param>
+        /// <returns>A string hash value.</returns>
+        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
@@ -0,0 +1,17 @@
+// -------------------------------------------------------------------------------------------------
+// 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
@@ -3,12 +3,7 @@
 // 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
 {
@@ -44,30 +39,5 @@ 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
@@ -69,6 +69,7 @@ internal class SqlServerSearchService : SearchService
         private readonly ICompressedRawResourceConverter _compressedRawResourceConverter;
         private readonly RequestContextAccessor<IFhirRequestContext> _requestContextAccessor;
         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();
@@ -90,6 +91,7 @@ public SqlServerSearchService(
             SchemaInformation schemaInformation,
             RequestContextAccessor<IFhirRequestContext> requestContextAccessor,
             ICompressedRawResourceConverter compressedRawResourceConverter,
+            ISqlQueryHashCalculator queryHashCalculator,
             IParameterStore parameterStore,
             ILogger<SqlServerSearchService> logger)
             : base(searchOptionsFactory, fhirDataStore, logger)
@@ -114,6 +116,7 @@ public SqlServerSearchService(
             _smartCompartmentSearchRewriter = smartCompartmentSearchRewriter;
             _chainFlatteningRewriter = chainFlatteningRewriter;
             _sqlRetryService = sqlRetryService;
+            _queryHashCalculator = queryHashCalculator;
             _parameterStore = parameterStore;
             _logger = logger;
 
@@ -363,7 +366,7 @@ await _sqlRetryService.ExecuteSql(
                             SqlCommandSimplifier.RemoveRedundantParameters(stringBuilder, sqlCommand.Parameters, _logger);
 
                             var queryText = stringBuilder.ToString();
-                            var queryHash = clonedSearchOptions.GetHash();
+                            var queryHash = _queryHashCalculator.CalculateHash(queryText);
                             _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,6 +73,10 @@ 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
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,7 +23,6 @@
     <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" />
@@ -50,5 +49,6 @@
     <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,7 +48,6 @@
 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;
@@ -126,12 +125,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,8 +187,7 @@ 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.ReadableLogger.TryGetLatestLog("SQL Search Service query hash:", out var hashValue) ? hashValue.Substring(31) : null;
-            Assert.NotNull(hash);
+            var hash = _fixture.SqlQueryHashCalculator.MostRecentSqlHash;
 
             // assert an sproc was not used
             Assert.False(await CheckIfSprocUsed(hash));
@@ -200,16 +199,11 @@ 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);
-                var newHash = _fixture.ReadableLogger.TryGetLatestLog("SQL Search Service query hash:", out var newHashValue) ? newHashValue.Substring(31) : null;
-                Assert.Equal(hash, newHash);
+                Assert.Equal(hash, _fixture.SqlQueryHashCalculator.MostRecentSqlHash);
                 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,7 +77,6 @@ public class SqlServerFhirStorageTestsFixture : IServiceProvider, IAsyncLifetime
         private SupportedSearchParameterDefinitionManager _supportedSearchParameterDefinitionManager;
         private SearchParameterStatusManager _searchParameterStatusManager;
         private SqlQueueClient _sqlQueueClient;
-        private ReadableLogger<SqlServerSearchService> _readableLogger;
 
         public SqlServerFhirStorageTestsFixture()
             : this(SchemaVersionConstants.Max, GetDatabaseName())
@@ -126,6 +125,8 @@ 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)}";
@@ -277,7 +278,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));
 
-            _readableLogger = new ReadableLogger<SqlServerSearchService>();
+            SqlQueryHashCalculator = new TestSqlHashCalculator();
 
             _searchService = new SqlServerSearchService(
                 searchOptionsFactory,
@@ -294,8 +295,9 @@ public async Task InitializeAsync()
                 SchemaInformation,
                 _fhirRequestContextAccessor,
                 new CompressedRawResourceConverter(),
+                SqlQueryHashCalculator,
                 new SqlServerParameterStore(SqlConnectionBuilder, NullLogger<SqlServerParameterStore>.Instance),
-                _readableLogger);
+                NullLogger<SqlServerSearchService>.Instance);
 
             ISearchParameterSupportResolver searchParameterSupportResolver = Substitute.For<ISearchParameterSupportResolver>();
             searchParameterSupportResolver.IsSearchParameterSupported(Arg.Any<SearchParameterInfo>()).Returns((true, false));
@@ -411,9 +413,9 @@ object IServiceProvider.GetService(Type serviceType)
                 return _sqlQueueClient;
             }
 
-            if (serviceType == typeof(ReadableLogger<SqlServerSearchService>))
+            if (serviceType == typeof(TestSqlHashCalculator))
             {
-                return _readableLogger;
+                return SqlQueryHashCalculator as TestSqlHashCalculator;
             }
 
             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
@@ -0,0 +1,25 @@
+// -------------------------------------------------------------------------------------------------
+// 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;
+        }
+    }
+}