Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change hash to string #4768

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ string ValueToString()
return $"(Field{BinaryOperator} {(ComponentIndex == null ? null : $"[{ComponentIndex}].")}{FieldName} {ValueToString()})";
}

public override string ToValueInsensitiveString()
{
return $"(Field{BinaryOperator} {(ComponentIndex == null ? null : $"[{ComponentIndex}].")}{FieldName})";
}

public override void AddValueInsensitiveHashCode(ref HashCode hashCode)
{
hashCode.Add(typeof(BinaryExpression));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ public override string ToString()
return $"({(Reversed ? "Reverse " : string.Empty)}Chain {ReferenceSearchParameter.Code}:{string.Join(", ", TargetResourceTypes)} {Expression})";
}

public override string ToValueInsensitiveString()
{
return $"({(Reversed ? "Reverse " : string.Empty)}Chain {ReferenceSearchParameter.Code}:{string.Join(", ", TargetResourceTypes)} {Expression.ToValueInsensitiveString()})";
}

public override void AddValueInsensitiveHashCode(ref HashCode hashCode)
{
hashCode.Add(typeof(ChainedExpression));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ public override string ToString()
return $"(Compartment {CompartmentType} '{CompartmentId}')";
}

public override string ToValueInsensitiveString()
{
return $"(Compartment {CompartmentType} '{CompartmentId}')";
}

public override void AddValueInsensitiveHashCode(ref HashCode hashCode)
{
hashCode.Add(typeof(CompartmentSearchExpression));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,9 +316,12 @@ public static SmartCompartmentSearchExpression SmartCompartmentSearch(string com
/// <inheritdoc />
public abstract override string ToString();

public abstract string ToValueInsensitiveString();

/// <summary>
/// Accumulates a "value-insensitive" hash code of this instance, meaning it ignores parameterizable values.
/// For example, date=2013&amp;name=Smith and date=2014&amp;name=Trudeau would have the same hash code.
/// HashCodes change after a restart, don't use this for something that needs to be consistent between restarts.
/// </summary>
/// <param name="hashCode">The HashCode instance to accumulate into</param>
public abstract void AddValueInsensitiveHashCode(ref HashCode hashCode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ public override string ToString()
return $"({(ComponentIndex == null ? null : $"[{ComponentIndex}].")}{FieldName} IN ({string.Join(", ", Values)}))";
}

public override string ToValueInsensitiveString()
{
return $"({(ComponentIndex == null ? null : $"[{ComponentIndex}].")}{FieldName} IN )";
}

public override void AddValueInsensitiveHashCode(ref HashCode hashCode)
{
hashCode.Add(typeof(InExpression<T>));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,19 @@
var paramName = ReferenceSearchParameter != null ? $" {ReferenceSearchParameter.Code}" : string.Empty;
return $"({reversed}Include{iterate}{wildcard}{paramName}{targetType})";
}


// Check this one, I don't think the toString is correct
public override string ToValueInsensitiveString()
{
var targetType = TargetResourceType != null ? $":{TargetResourceType}" : string.Empty;
var iterate = Iterate ? " Iterate" : string.Empty;
var reversed = Reversed ? "Reversed " : string.Empty;
var wildcard = WildCard ? " Wildcard" : string.Empty;
var paramName = ReferenceSearchParameter != null ? $" {ReferenceSearchParameter.Code}" : string.Empty;
return $"({reversed}Include{iterate}{wildcard}{paramName}{targetType})";
}

private IReadOnlyCollection<string> GetRequiredResources()
{
if (Reversed)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ public override string ToString()
return $"(MissingField {(ComponentIndex == null ? null : $"[{ComponentIndex}].")}{FieldName})";
}

public override string ToValueInsensitiveString()
{
return $"(MissingField {(ComponentIndex == null ? null : $"[{ComponentIndex}].")}{FieldName})";
}

public override void AddValueInsensitiveHashCode(ref HashCode hashCode)
{
hashCode.Add(typeof(MissingFieldException));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ public override string ToString()
return $"({(!IsMissing ? "Not" : null)}MissingParam {Parameter.Name})";
}

public override string ToValueInsensitiveString()
{
return $"({(!IsMissing ? "Not" : null)}MissingParam {Parameter.Name})";
}

public override void AddValueInsensitiveHashCode(ref HashCode hashCode)
{
hashCode.Add(typeof(MissingSearchParameterExpression));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ public override string ToString()
return $"({MultiaryOperation} {string.Join(' ', Expressions)})";
}

public override string ToValueInsensitiveString()
{
return $"({MultiaryOperation} {string.Join(' ', Expressions.Select(x => x.ToValueInsensitiveString()))})";
}

public override void AddValueInsensitiveHashCode(ref HashCode hashCode)
{
hashCode.Add(typeof(MultiaryExpression));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ public override string ToString()
return $"(Not {Expression})";
}

public override string ToValueInsensitiveString()
{
return $"(Not {Expression.ToValueInsensitiveString()})";
}

public override void AddValueInsensitiveHashCode(ref HashCode hashCode)
{
hashCode.Add(typeof(NotExpression));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ public override string ToString()
return $"(Param {Parameter.Code} {Expression})";
}

public override string ToValueInsensitiveString()
{
return $"(Param {Parameter.Code} {Expression.ToValueInsensitiveString()})";
}

public override void AddValueInsensitiveHashCode(ref HashCode hashCode)
{
hashCode.Add(typeof(SearchParameterExpression));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ public override string ToString()
return $"(Sort Param: {Parameter.Code})";
}

public override string ToValueInsensitiveString()
{
return $"(Sort Param: {Parameter.Code})";
}

public override void AddValueInsensitiveHashCode(ref HashCode hashCode)
{
hashCode.Add(typeof(SortExpression));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ public override string ToString()
return $"(String{StringOperator}{(IgnoreCase ? "IgnoreCase" : null)} {(ComponentIndex == null ? null : $"[{ComponentIndex}].")}{FieldName} '{Value}')";
}

public override string ToValueInsensitiveString()
{
return $"(String{StringOperator}{(IgnoreCase ? "IgnoreCase" : null)} {(ComponentIndex == null ? null : $"[{ComponentIndex}].")}{FieldName})";
}

public override void AddValueInsensitiveHashCode(ref HashCode hashCode)
{
hashCode.Add(typeof(StringExpression));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ public override string ToString()
return $"(Union ({Operator}) {Expressions} {string.Join(' ', Expressions)})";
}

public override string ToValueInsensitiveString()
{
return $"(Union ({Operator}) {string.Join(' ', Expressions.Select(x => x.ToValueInsensitiveString()))})";
}

public override void AddValueInsensitiveHashCode(ref HashCode hashCode)
{
hashCode.Add(typeof(UnionExpression));
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ 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();
Expand All @@ -91,7 +90,6 @@ public SqlServerSearchService(
SchemaInformation schemaInformation,
RequestContextAccessor<IFhirRequestContext> requestContextAccessor,
ICompressedRawResourceConverter compressedRawResourceConverter,
ISqlQueryHashCalculator queryHashCalculator,
IParameterStore parameterStore,
ILogger<SqlServerSearchService> logger)
: base(searchOptionsFactory, fhirDataStore, logger)
Expand All @@ -116,7 +114,6 @@ public SqlServerSearchService(
_smartCompartmentSearchRewriter = smartCompartmentSearchRewriter;
_chainFlatteningRewriter = chainFlatteningRewriter;
_sqlRetryService = sqlRetryService;
_queryHashCalculator = queryHashCalculator;
_parameterStore = parameterStore;
_logger = logger;

Expand Down Expand Up @@ -366,7 +363,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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,6 @@ public static IFhirServerBuilder AddSqlServer(this IFhirServerBuilder fhirServer
.AsSelf()
.AsImplementedInterfaces();

services.Add<SqlQueryHashCalculator>()
.Singleton()
.AsImplementedInterfaces();

services.Add<SqlServerSearchService>()
.Scoped()
.AsSelf()
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand All @@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Loading