diff --git a/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Search/Expressions/DenormalizedPredicateRewriterTests.cs b/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Search/Expressions/DenormalizedPredicateRewriterTests.cs index 562b2264c6..09c45afe50 100644 --- a/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Search/Expressions/DenormalizedPredicateRewriterTests.cs +++ b/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Search/Expressions/DenormalizedPredicateRewriterTests.cs @@ -18,7 +18,7 @@ namespace Microsoft.Health.Fhir.SqlServer.UnitTests.Features.Search.Expressions public class DenormalizedPredicateRewriterTests { [Fact] - public void GivenExpressionWithNoTableExpressions_WhenRewritted_ReturnsOriginalExpression() + public void GivenExpressionWithNoTableExpressions_WhenRewritten_ReturnsOriginalExpression() { var inputExpression = SqlRootExpression.WithDenormalizedExpressions( Expression.Equals(FieldName.Number, null, 1)); @@ -27,7 +27,7 @@ public void GivenExpressionWithNoTableExpressions_WhenRewritted_ReturnsOriginalE } [Fact] - public void GivenExpressionWithNoDenormalizedExpressions_WhenRewritted_ReturnsOriginalExpression() + public void GivenExpressionWithNoDenormalizedExpressions_WhenRewritten_ReturnsOriginalExpression() { var inputExpression = SqlRootExpression.WithTableExpressions( new TableExpression(null, null, null, TableExpressionKind.Normal)); @@ -104,5 +104,24 @@ public void GivenExpressionWithMultipleDenormalizedExpressions_WhenRewritten_Den var visitedExpression = (SqlRootExpression)inputExpression.AcceptVisitor(DenormalizedPredicateRewriter.Instance); Assert.Equal(inputExpression.DenormalizedExpressions[1], visitedExpression.DenormalizedExpressions[0]); } + + [Theory] + [InlineData(SearchParameterNames.ResourceType)] + [InlineData(SqlSearchParameters.ResourceSurrogateIdParameterName)] + public void GivenExpressionWithDenormalizedAndChainedExpressions_WhenRewritten_DenormalisedPredicatesPromotedToChainTableExpression(string paramName) + { + var inputExpression = new SqlRootExpression( + new List + { + new TableExpression(null, null, null, TableExpressionKind.Chain, 1), + }, + new List + { + new SearchParameterExpression(new SearchParameterInfo(paramName), Expression.Equals(FieldName.String, null, "ExtractableTestParamValue")), + }); + var visitedExpression = (SqlRootExpression)inputExpression.AcceptVisitor(DenormalizedPredicateRewriter.Instance); + Assert.Equal(inputExpression.DenormalizedExpressions[0], visitedExpression.TableExpressions[0].DenormalizedPredicateOnChainRoot); + Assert.Equal(inputExpression.TableExpressions[0].ChainLevel, visitedExpression.TableExpressions[0].ChainLevel); + } } } diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/TableExpression.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/TableExpression.cs index 5ee78c3383..f75967b61c 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/TableExpression.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/TableExpression.cs @@ -15,12 +15,23 @@ namespace Microsoft.Health.Fhir.SqlServer.Features.Search.Expressions /// internal class TableExpression : Expression { + /// + /// Creates a new instance of the class. + /// + /// The search parameter query generator + /// The search expression over a columns belonging exclusively to a search parameter table. + /// Applies to the chain target if a chained expression. + /// The search expression over columns that exist the Resource table. Applies to the chain target if a chained expression. + /// The table expression kind. + /// The nesting chain nesting level of the current expression. 0 if not a chain expression. + /// The search expression over columns that exist the Resource table. Applies to the chain root if a chained expression. public TableExpression( NormalizedSearchParameterQueryGenerator searchParameterQueryGenerator, Expression normalizedPredicate, Expression denormalizedPredicate, TableExpressionKind kind, - int chainLevel = 0) + int chainLevel = 0, + Expression denormalizedPredicateOnChainRoot = null) { switch (normalizedPredicate) { @@ -39,18 +50,34 @@ internal class TableExpression : Expression DenormalizedPredicate = denormalizedPredicate; Kind = kind; ChainLevel = chainLevel; + DenormalizedPredicateOnChainRoot = denormalizedPredicateOnChainRoot; } public TableExpressionKind Kind { get; } + /// + /// The nesting chain nesting level of the current expression. 0 if not a chain expression. + /// public int ChainLevel { get; } public NormalizedSearchParameterQueryGenerator SearchParameterQueryGenerator { get; } + /// + /// The search expression over a columns belonging exclusively to a search parameter table. + /// Applies to the chain target if a chained expression. + /// public Expression NormalizedPredicate { get; } + /// + /// The search expression over columns that exist the Resource table. Applies to the chain target if a chained expression. + /// public Expression DenormalizedPredicate { get; } + /// + /// The search expression over columns that exist the Resource table. Applies to the chain root if a chained expression. + /// + public Expression DenormalizedPredicateOnChainRoot { get; } + public override TOutput AcceptVisitor(IExpressionVisitor visitor, TContext context) { return AcceptVisitor((ISqlExpressionVisitor)visitor, context); @@ -63,7 +90,7 @@ internal class TableExpression : Expression public override string ToString() { - return $"(Table {Kind} {(ChainLevel == 0 ? null : $"ChainLevel:{ChainLevel} ")}{SearchParameterQueryGenerator?.Table} Normalized:{NormalizedPredicate} Denormalized:{DenormalizedPredicate})"; + return $"(Table {Kind} {(ChainLevel == 0 ? null : $"ChainLevel:{ChainLevel} ")}{SearchParameterQueryGenerator?.Table} Normalized:{NormalizedPredicate} Denormalized:{DenormalizedPredicate} DenormalizedOnChainRoot:{DenormalizedPredicateOnChainRoot})"; } } } diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/ChainFlatteningRewriter.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/ChainFlatteningRewriter.cs index 722285e1f5..05fcb2e2c2 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/ChainFlatteningRewriter.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/ChainFlatteningRewriter.cs @@ -66,7 +66,8 @@ public override Expression VisitChained(ChainedExpression expression, (TableExpr expression, denormalizedPredicate: normalizedParameterQueryGenerator == null ? expression.Expression : null, TableExpressionKind.Chain, - context.chainLevel); + context.chainLevel, + thisTableExpression?.DenormalizedPredicateOnChainRoot); } if (normalizedParameterQueryGenerator == null) diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/DenormalizedPredicateRewriter.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/DenormalizedPredicateRewriter.cs index d6ee00686e..7429321316 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/DenormalizedPredicateRewriter.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/DenormalizedPredicateRewriter.cs @@ -22,7 +22,7 @@ internal class DenormalizedPredicateRewriter : ExpressionRewriterWithInitialCont public Expression VisitSqlRoot(SqlRootExpression expression, object context) { - if (expression.TableExpressions.Count == 0 || expression.DenormalizedExpressions.Count == 0 || expression.TableExpressions.All(t => t.Kind == TableExpressionKind.Chain) || expression.TableExpressions.All(t => t.Kind == TableExpressionKind.Include)) + if (expression.TableExpressions.Count == 0 || expression.DenormalizedExpressions.Count == 0 || expression.TableExpressions.All(t => t.Kind == TableExpressionKind.Include)) { return expression; } @@ -59,10 +59,14 @@ public Expression VisitSqlRoot(SqlRootExpression expression, object context) var newTableExpressions = new List(expression.TableExpressions.Count); foreach (var tableExpression in expression.TableExpressions) { - if (tableExpression.Kind == TableExpressionKind.Chain || tableExpression.Kind == TableExpressionKind.Include) + if (tableExpression.Kind == TableExpressionKind.Include) { newTableExpressions.Add(tableExpression); } + else if (tableExpression.Kind == TableExpressionKind.Chain) + { + newTableExpressions.Add(new TableExpression(tableExpression.SearchParameterQueryGenerator, tableExpression.NormalizedPredicate, tableExpression.DenormalizedPredicate, tableExpression.Kind, chainLevel: tableExpression.ChainLevel, denormalizedPredicateOnChainRoot: extractedDenormalizedExpression)); + } else { Expression newDenormalizedPredicate = tableExpression.DenormalizedPredicate == null diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/QueryGenerators/SqlQueryGenerator.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/QueryGenerators/SqlQueryGenerator.cs index 2db456bc71..b9ae605836 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/QueryGenerators/SqlQueryGenerator.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/QueryGenerators/SqlQueryGenerator.cs @@ -138,8 +138,8 @@ public object VisitSqlRoot(SqlRootExpression expression, SearchOptions context) public object VisitTable(TableExpression tableExpression, SearchOptions context) { - string referenceTableAlias = "ref"; - string resourceTableAlias = "r"; + const string referenceSourceTableAlias = "refSource"; + const string referenceTargetResourceTableAlias = "refTarget"; switch (tableExpression.Kind) { @@ -251,43 +251,48 @@ public object VisitTable(TableExpression tableExpression, SearchOptions context) case TableExpressionKind.Chain: var chainedExpression = (ChainedExpression)tableExpression.NormalizedPredicate; - string resourceTableAlias2 = "r2"; StringBuilder.Append("SELECT "); if (tableExpression.ChainLevel == 1) { - StringBuilder.Append(VLatest.ReferenceSearchParam.ResourceSurrogateId, referenceTableAlias).Append(" AS ").Append(chainedExpression.Reversed ? "Sid2" : "Sid1").Append(", "); + StringBuilder.Append(VLatest.ReferenceSearchParam.ResourceSurrogateId, referenceSourceTableAlias).Append(" AS ").Append(chainedExpression.Reversed ? "Sid2" : "Sid1").Append(", "); } else { StringBuilder.Append("Sid1, "); } - StringBuilder.Append(VLatest.Resource.ResourceSurrogateId, chainedExpression.Reversed && tableExpression.ChainLevel > 1 ? referenceTableAlias : resourceTableAlias).Append(" AS ").AppendLine(chainedExpression.Reversed && tableExpression.ChainLevel == 1 ? "Sid1 " : "Sid2 ") - .Append("FROM ").Append(VLatest.ReferenceSearchParam).Append(' ').AppendLine(referenceTableAlias) - .Append("INNER JOIN ").Append(VLatest.Resource).Append(' ').AppendLine(resourceTableAlias); + StringBuilder.Append(VLatest.Resource.ResourceSurrogateId, chainedExpression.Reversed && tableExpression.ChainLevel > 1 ? referenceSourceTableAlias : referenceTargetResourceTableAlias).Append(" AS ").AppendLine(chainedExpression.Reversed && tableExpression.ChainLevel == 1 ? "Sid1 " : "Sid2 ") + .Append("FROM ").Append(VLatest.ReferenceSearchParam).Append(' ').AppendLine(referenceSourceTableAlias) + .Append("INNER JOIN ").Append(VLatest.Resource).Append(' ').AppendLine(referenceTargetResourceTableAlias); using (var delimited = StringBuilder.BeginDelimitedOnClause()) { - delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ReferenceResourceTypeId, referenceTableAlias) - .Append(" = ").Append(VLatest.Resource.ResourceTypeId, resourceTableAlias); + delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ReferenceResourceTypeId, referenceSourceTableAlias) + .Append(" = ").Append(VLatest.Resource.ResourceTypeId, referenceTargetResourceTableAlias); - delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ReferenceResourceId, referenceTableAlias) - .Append(" = ").Append(VLatest.Resource.ResourceId, resourceTableAlias); + delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ReferenceResourceId, referenceSourceTableAlias) + .Append(" = ").Append(VLatest.Resource.ResourceId, referenceTargetResourceTableAlias); } - // Denormalized predicates on reverse chains need to be applied via a join to the resource table - if (tableExpression.DenormalizedPredicate != null && chainedExpression.Reversed) + // For reverse chaning, if there is a parameter on the _id search parameter, we need another join to get the resource ID of the reference source (all we have is the surrogate ID at this point) + + bool denormalizedHandledBySecondJoin = tableExpression.DenormalizedPredicate != null && chainedExpression.Reversed && tableExpression.DenormalizedPredicate.AcceptVisitor(DenormalizedExpressionContainsIdParameterVisitor.Instance, null); + + if (denormalizedHandledBySecondJoin) { - StringBuilder.Append("INNER JOIN ").Append(VLatest.Resource).Append(' ').AppendLine(resourceTableAlias2); + const string referenceSourceResourceTableAlias = "refSourceResource"; + + denormalizedHandledBySecondJoin = true; + StringBuilder.Append("INNER JOIN ").Append(VLatest.Resource).Append(' ').AppendLine(referenceSourceResourceTableAlias); using (var delimited = StringBuilder.BeginDelimitedOnClause()) { - delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ResourceSurrogateId, referenceTableAlias) - .Append(" = ").Append(VLatest.Resource.ResourceSurrogateId, resourceTableAlias2); + delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ResourceSurrogateId, referenceSourceTableAlias) + .Append(" = ").Append(VLatest.Resource.ResourceSurrogateId, referenceSourceResourceTableAlias); delimited.BeginDelimitedElement(); - tableExpression.DenormalizedPredicate?.AcceptVisitor(DispatchingDenormalizedSearchParameterQueryGenerator.Instance, GetContext(resourceTableAlias2)); + tableExpression.DenormalizedPredicate?.AcceptVisitor(DispatchingDenormalizedSearchParameterQueryGenerator.Instance, GetContext(referenceSourceResourceTableAlias)); } } @@ -297,34 +302,40 @@ public object VisitTable(TableExpression tableExpression, SearchOptions context) using (var delimited = StringBuilder.BeginDelimitedOnClause()) { - delimited.BeginDelimitedElement().Append(VLatest.Resource.ResourceSurrogateId, chainedExpression.Reversed ? resourceTableAlias : referenceTableAlias).Append(" = ").Append("Sid2"); + delimited.BeginDelimitedElement().Append(VLatest.Resource.ResourceSurrogateId, chainedExpression.Reversed ? referenceTargetResourceTableAlias : referenceSourceTableAlias).Append(" = ").Append("Sid2"); } } using (var delimited = StringBuilder.BeginDelimitedWhereClause()) { - delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.SearchParamId, referenceTableAlias) + delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.SearchParamId, referenceSourceTableAlias) .Append(" = ").Append(Parameters.AddParameter(VLatest.ReferenceSearchParam.SearchParamId, Model.GetSearchParamId(chainedExpression.ReferenceSearchParameter.Url))); - AppendHistoryClause(delimited, resourceTableAlias); - AppendHistoryClause(delimited, referenceTableAlias); + AppendHistoryClause(delimited, referenceTargetResourceTableAlias); + AppendHistoryClause(delimited, referenceSourceTableAlias); - delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ResourceTypeId, referenceTableAlias) + delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ResourceTypeId, referenceSourceTableAlias) .Append(" = ").Append(Parameters.AddParameter(VLatest.ReferenceSearchParam.ResourceTypeId, Model.GetResourceTypeId(chainedExpression.ResourceType))); - delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ReferenceResourceTypeId, referenceTableAlias) + delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ReferenceResourceTypeId, referenceSourceTableAlias) .Append(" = ").Append(Parameters.AddParameter(VLatest.ReferenceSearchParam.ReferenceResourceTypeId, Model.GetResourceTypeId(chainedExpression.TargetResourceType))); if (tableExpression.ChainLevel == 1) { // if > 1, the intersection is handled by the JOIN - AppendIntersectionWithPredecessor(delimited, tableExpression, chainedExpression.Reversed ? resourceTableAlias : referenceTableAlias); + AppendIntersectionWithPredecessor(delimited, tableExpression, chainedExpression.Reversed ? referenceTargetResourceTableAlias : referenceSourceTableAlias); + } + + if (tableExpression.DenormalizedPredicate != null && !denormalizedHandledBySecondJoin) + { + delimited.BeginDelimitedElement(); + tableExpression.DenormalizedPredicate?.AcceptVisitor(DispatchingDenormalizedSearchParameterQueryGenerator.Instance, GetContext(chainedExpression.Reversed ? referenceSourceTableAlias : referenceTargetResourceTableAlias)); } - if (tableExpression.DenormalizedPredicate != null && !chainedExpression.Reversed) + if (tableExpression.DenormalizedPredicateOnChainRoot != null) { delimited.BeginDelimitedElement(); - tableExpression.DenormalizedPredicate?.AcceptVisitor(DispatchingDenormalizedSearchParameterQueryGenerator.Instance, GetContext(resourceTableAlias)); + tableExpression.DenormalizedPredicateOnChainRoot.AcceptVisitor(DispatchingDenormalizedSearchParameterQueryGenerator.Instance, GetContext(chainedExpression.Reversed ? referenceTargetResourceTableAlias : referenceSourceTableAlias)); } } @@ -333,42 +344,42 @@ public object VisitTable(TableExpression tableExpression, SearchOptions context) var includeExpression = (IncludeExpression)tableExpression.NormalizedPredicate; StringBuilder.Append("SELECT DISTINCT "); - StringBuilder.Append(VLatest.Resource.ResourceSurrogateId, resourceTableAlias).AppendLine(" AS Sid1, 0 AS IsMatch") - .Append("FROM ").Append(VLatest.ReferenceSearchParam).Append(' ').AppendLine(referenceTableAlias) - .Append("INNER JOIN ").Append(VLatest.Resource).Append(' ').AppendLine(resourceTableAlias); + StringBuilder.Append(VLatest.Resource.ResourceSurrogateId, referenceTargetResourceTableAlias).AppendLine(" AS Sid1, 0 AS IsMatch") + .Append("FROM ").Append(VLatest.ReferenceSearchParam).Append(' ').AppendLine(referenceSourceTableAlias) + .Append("INNER JOIN ").Append(VLatest.Resource).Append(' ').AppendLine(referenceTargetResourceTableAlias); using (var delimited = StringBuilder.BeginDelimitedOnClause()) { - delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ReferenceResourceTypeId, referenceTableAlias) - .Append(" = ").Append(VLatest.Resource.ResourceTypeId, resourceTableAlias); + delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ReferenceResourceTypeId, referenceSourceTableAlias) + .Append(" = ").Append(VLatest.Resource.ResourceTypeId, referenceTargetResourceTableAlias); - delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ReferenceResourceId, referenceTableAlias) - .Append(" = ").Append(VLatest.Resource.ResourceId, resourceTableAlias); + delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ReferenceResourceId, referenceSourceTableAlias) + .Append(" = ").Append(VLatest.Resource.ResourceId, referenceTargetResourceTableAlias); } using (var delimited = StringBuilder.BeginDelimitedWhereClause()) { if (!includeExpression.WildCard) { - delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.SearchParamId, referenceTableAlias) + delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.SearchParamId, referenceSourceTableAlias) .Append(" = ").Append(Parameters.AddParameter(VLatest.ReferenceSearchParam.SearchParamId, Model.GetSearchParamId(includeExpression.ReferenceSearchParameter.Url))); if (includeExpression.TargetResourceType != null) { - delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ReferenceResourceTypeId, referenceTableAlias) + delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ReferenceResourceTypeId, referenceSourceTableAlias) .Append(" = ").Append(Parameters.AddParameter(VLatest.ReferenceSearchParam.ReferenceResourceTypeId, Model.GetResourceTypeId(includeExpression.TargetResourceType))); } } - AppendHistoryClause(delimited, resourceTableAlias); - AppendHistoryClause(delimited, referenceTableAlias); + AppendHistoryClause(delimited, referenceTargetResourceTableAlias); + AppendHistoryClause(delimited, referenceSourceTableAlias); - delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ResourceTypeId, referenceTableAlias) + delimited.BeginDelimitedElement().Append(VLatest.ReferenceSearchParam.ResourceTypeId, referenceSourceTableAlias) .Append(" = ").Append(Parameters.AddParameter(VLatest.ReferenceSearchParam.ResourceTypeId, Model.GetResourceTypeId(includeExpression.ResourceType))); // Limit the join to the main select CTE. // The main select will have max+1 items in the result set to account for paging, so we only want to join using the max amount. - delimited.BeginDelimitedElement().Append(VLatest.Resource.ResourceSurrogateId, referenceTableAlias) + delimited.BeginDelimitedElement().Append(VLatest.Resource.ResourceSurrogateId, referenceSourceTableAlias) .Append(" IN (SELECT TOP(") .Append(Parameters.AddParameter(context.MaxItemCount)) .Append(") Sid1 FROM ").Append(_cteMainSelect).Append(")"); @@ -459,5 +470,20 @@ private void AppendHistoryClause(in IndentedStringBuilder.DelimitedScope delimit StringBuilder.Append(VLatest.Resource.IsHistory, tableAlias).Append(" = 0"); } } + + /// + /// A visitor to determine if there are any references to the _id search parameter in an expression + /// + private class DenormalizedExpressionContainsIdParameterVisitor : DefaultExpressionVisitor + { + public static readonly DenormalizedExpressionContainsIdParameterVisitor Instance = new DenormalizedExpressionContainsIdParameterVisitor(); + + private DenormalizedExpressionContainsIdParameterVisitor() + : base((acc, curr) => acc || curr) + { + } + + public override bool VisitSearchParameter(SearchParameterExpression expression, object context) => expression.Parameter.Name == SearchParameterNames.Id; + } } } diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/ExportDataValidationTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/ExportDataValidationTests.cs index 64758c2167..7eca78e132 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/ExportDataValidationTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/ExportDataValidationTests.cs @@ -51,17 +51,17 @@ public async Task GivenFhirServer_WhenDataIsExported_ThenExportedDataIsSameAsDat IList blobUris = await CheckExportStatus(contentLocation); // Download exported data from storage account - Dictionary dataFromExport = await DownloadBlobAndParse(blobUris); + Dictionary<(string resourceType, string resourceId), string> dataFromExport = await DownloadBlobAndParse(blobUris); // Download all resources from fhir server Uri address = new Uri(_testFhirClient.HttpClient.BaseAddress, path); - Dictionary dataFromFhirServer = await GetResourcesFromFhirServer(address); + Dictionary<(string resourceType, string resourceId), string> dataFromFhirServer = await GetResourcesFromFhirServer(address); // Assert both data are equal Assert.True(ValidateDataFromBothSources(dataFromFhirServer, dataFromExport)); } - private bool ValidateDataFromBothSources(Dictionary dataFromServer, Dictionary dataFromStorageAccount) + private bool ValidateDataFromBothSources(Dictionary<(string resourceType, string resourceId), string> dataFromServer, Dictionary<(string resourceType, string resourceId), string> dataFromStorageAccount) { bool result = true; @@ -81,7 +81,7 @@ private bool ValidateDataFromBothSources(Dictionary dataFromServ */ int wrongCount = 0; - foreach (KeyValuePair kvp in dataFromServer) + foreach (KeyValuePair<(string resourceType, string resourceId), string> kvp in dataFromServer) { if (!dataFromStorageAccount.ContainsKey(kvp.Key)) { @@ -132,11 +132,11 @@ private async Task> CheckExportStatus(Uri contentLocation) return exportJobResult.Output.Select(x => x.FileUri).ToList(); } - private async Task> DownloadBlobAndParse(IList blobUri) + private async Task> DownloadBlobAndParse(IList blobUri) { if (blobUri == null || blobUri.Count == 0) { - return new Dictionary(); + return new Dictionary<(string resourceType, string resourceId), string>(); } // Extract storage account name from blob uri in order to get corresponding access token. @@ -145,7 +145,7 @@ private async Task> CheckExportStatus(Uri contentLocation) CloudStorageAccount cloudAccount = GetCloudStorageAccountHelper(storageAccountName); CloudBlobClient blobClient = cloudAccount.CreateCloudBlobClient(); - Dictionary resourceIdToResourceMapping = new Dictionary(); + var resourceIdToResourceMapping = new Dictionary<(string resourceType, string resourceId), string>(); foreach (Uri uri in blobUri) { @@ -162,7 +162,7 @@ private async Task> CheckExportStatus(Uri contentLocation) } JObject resource = JObject.Parse(entry); - resourceIdToResourceMapping.Add(resource["id"].ToString(), entry); + resourceIdToResourceMapping.Add((resource["resourceType"].ToString(), resource["id"].ToString()), entry); } } @@ -175,9 +175,9 @@ private async Task> CheckExportStatus(Uri contentLocation) return resourceIdToResourceMapping; } - private async Task> GetResourcesFromFhirServer(Uri requestUri) + private async Task> GetResourcesFromFhirServer(Uri requestUri) { - Dictionary resourceIdToResourceMapping = new Dictionary(); + var resourceIdToResourceMapping = new Dictionary<(string resourceType, string resourceId), string>(); while (requestUri != null) { @@ -203,13 +203,15 @@ private async Task> CheckExportStatus(Uri contentLocation) foreach (JToken entry in entries) { + string resourceType = entry["resource"]["resourceType"].ToString(); string id = entry["resource"]["id"].ToString(); + string resource = entry["resource"].ToString().Trim(); - resourceIdToResourceMapping.Add(id, resource); + resourceIdToResourceMapping.Add((resourceType, id), resource); } - // Look at whether a continutation token has been returned. + // Look at whether a continuation token has been returned. // We will always have self link. We are looking for the "next" link JArray links = (JArray)result["link"]; string nextUri = null; diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/ChainingSearchTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/ChainingSearchTests.cs index e51c9b5ffb..3205fad522 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/ChainingSearchTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/ChainingSearchTests.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; +using System.Linq; using Hl7.Fhir.Model; using Microsoft.Health.Fhir.Tests.Common.FixtureParameters; using Xunit; @@ -129,7 +130,7 @@ public async Task GivenANestedReverseChainSearchExpressionOverASimpleParameter_W } [Fact] - public async Task GivenANestedReverseChainSearchExpressionOverADenormalizedParameter_WhenSearched_ThenCorrectBundleShouldBeReturned() + public async Task GivenANestedReverseChainSearchExpressionOverTheIdDenormalizedParameter_WhenSearched_ThenCorrectBundleShouldBeReturned() { string query = $"_tag={Fixture.Tag}&_has:Group:member:_id={Fixture.PatientGroup.Id}"; @@ -138,6 +139,58 @@ public async Task GivenANestedReverseChainSearchExpressionOverADenormalizedParam ValidateBundle(bundle, Fixture.AdamsPatient, Fixture.SmithPatient, Fixture.TrumanPatient); } + [Fact] + public async Task GivenANestedReverseChainSearchExpressionOverTheTypeDenormalizedParameter_WhenSearched_ThenCorrectBundleShouldBeReturned() + { + Bundle bundle = await Client.SearchAsync(ResourceType.Patient, $"_tag={Fixture.Tag}&_has:Group:member:_type=Group"); + + Assert.NotEmpty(bundle.Entry); + + bundle = await Client.SearchAsync(ResourceType.Patient, $"_tag={Fixture.Tag}&_has:Group:member:_type=Patient"); + + Assert.Empty(bundle.Entry); + } + + [Fact] + public async Task GivenAChainedSearchExpressionWithAPredicateOnSurrogateId_WhenSearched_ThenCorrectBundleShouldBeReturned() + { + string query = $"subject:Patient._type=Patient&subject:Patient._tag={Fixture.Tag}"; + + Bundle completeBundle = await Client.SearchAsync(ResourceType.DiagnosticReport, query); + Assert.True(completeBundle.Entry.Count > 2); + + Bundle bundle = await Client.SearchAsync(ResourceType.DiagnosticReport, query, count: 1); + List resources = new List(); + resources.AddRange(bundle.Entry); + while (bundle.NextLink != null) + { + bundle = await Client.SearchAsync(bundle.NextLink.ToString()); + resources.AddRange(bundle.Entry); + } + + ValidateBundle(new Bundle { Entry = resources }, completeBundle.Entry.Select(e => e.Resource).ToArray()); + } + + [Fact] + public async Task GivenAReverseChainedSearchExpressionWithAPredicateOnSurrogateId_WhenSearched_ThenCorrectBundleShouldBeReturned() + { + string query = $"_has:Observation:patient:_tag={Fixture.Tag}"; + + Bundle completeBundle = await Client.SearchAsync(ResourceType.Patient, query); + Assert.True(completeBundle.Entry.Count > 2); + + Bundle bundle = await Client.SearchAsync(ResourceType.Patient, query, count: 1); + List resources = new List(); + resources.AddRange(bundle.Entry); + while (bundle.NextLink != null) + { + bundle = await Client.SearchAsync(bundle.NextLink.ToString()); + resources.AddRange(bundle.Entry); + } + + ValidateBundle(new Bundle { Entry = resources }, completeBundle.Entry.Select(e => e.Resource).ToArray()); + } + [Fact] public async Task GivenACombinationOfChainingReverseChainSearchExpressionOverASimpleParameter_WhenSearched_ThenCorrectBundleShouldBeReturned() { @@ -225,7 +278,8 @@ public ClassFixture(DataStore dataStore, Format format, TestFhirServerFactory te var group = new Group { Meta = new Meta { Tag = new List { new Coding("testTag", Tag) } }, - Type = Group.GroupType.Person, Actual = true, + Type = Group.GroupType.Person, + Actual = true, Member = new List { new Group.MemberComponent { Entity = new ResourceReference($"Patient/{AdamsPatient.Id}") }, @@ -254,6 +308,7 @@ Observation CreateObservation(Patient patient, CodeableConcept code) return TestFhirClient.CreateAsync( new Observation() { + Meta = meta, Status = ObservationStatus.Final, Code = code, Subject = new ResourceReference($"Patient/{patient.Id}"),