Skip to content

Commit

Permalink
Adding some UT/E2E test for $includes operation.
Browse files Browse the repository at this point in the history
  • Loading branch information
tarunmathew12 committed Feb 13, 2025
1 parent ec46199 commit 123023c
Show file tree
Hide file tree
Showing 17 changed files with 3,907 additions and 59 deletions.
13 changes: 4 additions & 9 deletions src/Microsoft.Health.Fhir.Api/Features/Routing/UrlResolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -368,27 +368,22 @@ private Uri GetRouteUri(HttpContext httpContext, string routeName, RouteValueDic
pathBase += "/";
}

if (address == null)
{
uriString = _linkGenerator.GetUriByRouteValues(
uriString = address == null ?
_linkGenerator.GetUriByRouteValues(
httpContext,
routeName,
routeValues,
scheme,
new HostString(host),
pathBase);
}
else
{
uriString = _linkGenerator.GetUriByAddress<RouteValuesAddress>(
pathBase) :
_linkGenerator.GetUriByAddress<RouteValuesAddress>(
httpContext,
address,
address.ExplicitValues,
address.AmbientValues,
scheme,
new HostString(host),
pathBase);
}
}

return new Uri(uriString);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,13 @@ public static class KnownQueryParameterNames
public const string MaxCount = "_maxCount";

/// <summary>
/// The include continuation token parameter.
/// The $includes continuation token parameter.
/// </summary>
public const string IncludesContinuationToken = "includesCt";

/// <summary>
/// The $includes count parameter.
/// </summary>
public const string IncludesCount = "_includesCount";
}
}
4 changes: 4 additions & 0 deletions src/Microsoft.Health.Fhir.Core/Models/KnownResourceTypes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ public static class KnownResourceTypes

public const string CareTeam = "CareTeam";

public const string MedicationDispense = "MedicationDispense";

public const string MedicationRequest = "MedicationRequest";

public const string All = "all";
}
}
29 changes: 28 additions & 1 deletion src/Microsoft.Health.Fhir.Core/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 11 additions & 3 deletions src/Microsoft.Health.Fhir.Core/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -764,8 +764,16 @@
<value>The number of included results exceeded the limit of {0}</value>
<comment>{0} is the limit of how many include results the service will return.</comment>
</data>
</root>
<data name="UnsupportedIncludesOperation" xml:space="preserve">
<value>The '$includes' operation is not supported for CosmosDB data store.</value>
<value>The '$includes' operation is not supported.</value>
</data>
</root>
<data name="SearchParamaterIncludesCountExceedLimit" xml:space="preserve">
<value>The '_includesCount' parameter exceeds limit configured for server. Current limit is {0} while the `_includesCount` parameter set to {1}.</value>
</data>
<data name="InvalidSearchIncludesCountSpecified" xml:space="preserve">
<value>The '_includesCount' parameter value must be an integer greater than zero.</value>
</data>
<data name="MissingIncludesContinuationToken" xml:space="preserve">
<value>"The 'includesCt' parameter must be provided for the $includes operation."</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public class BundleFactoryTests
private readonly BundleFactory _bundleFactory;

private const string _continuationToken = "ct";
private const string _includesContinuationToken = "includesCt";
private const string _resourceUrlFormat = "http://resource/{0}";
private static readonly string _correlationId = Guid.NewGuid().ToString();
private static readonly Uri _selfUrl = new Uri("http://self/");
Expand Down Expand Up @@ -224,5 +225,44 @@ public void GivenAHistoryResultWithAllHttpVerbs_WhenCreateHistoryBundle_ThenBund
Assert.NotNull(actual.ToPoco<Bundle>().Entry[0].Response.Status);
}
}

[Theory]
[InlineData(RouteNames.SearchResources, _includesContinuationToken)]
[InlineData(RouteNames.Includes, _includesContinuationToken)]
[InlineData(RouteNames.SearchResources, null)]
[InlineData(RouteNames.Includes, null)]
public void GivenASearchResultWithIncludesContinuationToken_WhenCreateSearchBundle_ThenCorrectBundleShouldBeReturned(string routeName, string includesContinuationToken)
{
_fhirRequestContextAccessor.RequestContext.RouteName = routeName;

var encodedContinuationToken = ContinuationTokenEncoder.Encode(_continuationToken);
_urlResolver.ResolveRouteUrl(_unsupportedSearchParameters, null, encodedContinuationToken, true).Returns(_nextUrl);
_urlResolver.ResolveRouteUrl(_unsupportedSearchParameters).Returns(_selfUrl);

var includesUrl = "https://localhost/Patient/$include";
var encodedIncludesContinuationToken = includesContinuationToken != null ? ContinuationTokenEncoder.Encode(includesContinuationToken) : null;
_urlResolver.ResolveRouteUrl(
Arg.Any<IReadOnlyCollection<Tuple<string, string>>>(),
Arg.Any<IReadOnlyList<(SearchParameterInfo searchParameterInfo, SortOrder sortOrder)>>(),
Arg.Is<string>(x => x == null),
Arg.Any<bool>(),
Arg.Is<string>(x => x == encodedIncludesContinuationToken),
Arg.Any<string>(),
Arg.Any<IDictionary<string, object>>()).Returns(new Uri(includesUrl));

var searchResult = new SearchResult(new SearchResultEntry[0], _continuationToken, null, _unsupportedSearchParameters, null, includesContinuationToken);
var actual = _bundleFactory.CreateSearchBundle(searchResult);

if (routeName != RouteNames.Includes)
{
Assert.Equal(_nextUrl.OriginalString, actual.Scalar<string>("Bundle.link.where(relation='next').url"));
Assert.Equal(includesContinuationToken != null ? includesUrl : null, actual.Scalar<string>("Bundle.link.where(relation='related').url"));
}
else
{
Assert.Equal(includesContinuationToken != null ? includesUrl : null, actual.Scalar<string>("Bundle.link.where(relation='next').url"));
Assert.Null(actual.Scalar<string>("Bundle.link.where(relation='related').url"));
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ private void CreateLinks(SearchResult result, Bundle bundle)
new Bundle.LinkComponent()
{
Relation = "related",
Url = url.AbsoluteUri,
Url = url?.AbsoluteUri,
});
}
catch (UriFormatException)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,12 @@ public SearchOptions Create(
searchOptions.IgnoreSearchParamHash = queryParameters != null && queryParameters.Any(_ => _.Item1 == KnownQueryParameterNames.IgnoreSearchParamHash && _.Item2 != null);

string continuationToken = null;
string includesContinuationToken = null;
string feedRange = null;

// $includes related parameters
string includesContinuationToken = null;
int? includesCount = null;

var searchParams = new SearchParams();
var unsupportedSearchParameters = new List<Tuple<string, string>>();
bool setDefaultBundleTotal = true;
Expand Down Expand Up @@ -229,15 +232,20 @@ public SearchOptions Create(
string.Format(Core.Resources.MultipleQueryParametersNotAllowed, KnownQueryParameterNames.IncludesContinuationToken));
}

if (!isIncludesOperation)
{
// TODO: should this case be ignored or throw an exception? The Hl7 doc says, we should ignore it in general. (https://www.hl7.org/fhir/R4/search.html#errors)
// throw new BadRequestException(string.Format(Core.Resources.InvalidContinuationToken, query.Item2, SupportedTotalTypes));
}

includesContinuationToken = ContinuationTokenEncoder.Decode(query.Item2);
setDefaultBundleTotal = false;
}
else if (string.Equals(query.Item1, KnownQueryParameterNames.IncludesCount, StringComparison.OrdinalIgnoreCase))
{
if (int.TryParse(query.Item2, out int count) && count > 0)
{
includesCount = count;
}
else
{
throw new BadRequestException(Core.Resources.InvalidSearchIncludesCountSpecified);
}
}
else
{
// Parse the search parameters.
Expand All @@ -255,7 +263,7 @@ public SearchOptions Create(

if (isIncludesOperation && string.IsNullOrEmpty(includesContinuationToken))
{
throw new BadRequestException("'includesCt' parameter must be provided for $includes operation.");
throw new BadRequestException(Core.Resources.MissingIncludesContinuationToken);
}

searchOptions.ContinuationToken = continuationToken;
Expand Down Expand Up @@ -301,7 +309,23 @@ public SearchOptions Create(
searchOptions.MaxItemCount = _featureConfiguration.DefaultItemCountPerSearch;
}

searchOptions.IncludeCount = _featureConfiguration.DefaultIncludeCountPerSearch;
if (includesCount.HasValue && includesCount <= _featureConfiguration.DefaultIncludeCountPerSearch)
{
searchOptions.IncludeCount = includesCount.Value;
}
else
{
if (includesCount.HasValue)
{
_contextAccessor.RequestContext?.BundleIssues.Add(
new OperationOutcomeIssue(
OperationOutcomeConstants.IssueSeverity.Information,
OperationOutcomeConstants.IssueType.Informational,
string.Format(Core.Resources.SearchParamaterIncludesCountExceedLimit, _featureConfiguration.DefaultIncludeCountPerSearch, includesCount)));
}

searchOptions.IncludeCount = _featureConfiguration.DefaultIncludeCountPerSearch;
}

if (searchParams.Elements?.Any() == true && searchParams.Summary != null && searchParams.Summary != SummaryType.False)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ internal class IncludeRewriter : SqlExpressionRewriterWithInitialContext<object>
{
internal static readonly IncludeRewriter Instance = new IncludeRewriter();

private static readonly SearchParamTableExpression IncludeUnionAllExpression = new SearchParamTableExpression(null, null, SearchParamTableExpressionKind.IncludeUnionAll);
private static readonly SearchParamTableExpression IncludeLimitExpression = new SearchParamTableExpression(null, null, SearchParamTableExpressionKind.IncludeLimit);
protected static readonly SearchParamTableExpression IncludeUnionAllExpression = new SearchParamTableExpression(null, null, SearchParamTableExpressionKind.IncludeUnionAll);
protected static readonly SearchParamTableExpression IncludeLimitExpression = new SearchParamTableExpression(null, null, SearchParamTableExpressionKind.IncludeLimit);

public override Expression VisitSqlRoot(SqlRootExpression expression, object context)
{
Expand Down Expand Up @@ -63,7 +63,7 @@ public override Expression VisitSqlRoot(SqlRootExpression expression, object con
return new SqlRootExpression(reorderedExpressions, expression.ResourceTableExpressions);
}

private static List<SearchParamTableExpression> SortIncludeIterateExpressions(List<SearchParamTableExpression> expressions)
protected static List<SearchParamTableExpression> SortIncludeIterateExpressions(List<SearchParamTableExpression> expressions)
{
// Based on Khan's algorithm. See https://en.wikipedia.org/wiki/Topological_sorting.
// The search queries are acyclic.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,9 @@

namespace Microsoft.Health.Fhir.SqlServer.Features.Search.Expressions.Visitors
{
internal class IncludesOperationRewriter : SqlExpressionRewriterWithInitialContext<object>
internal class IncludesOperationRewriter : IncludeRewriter
{
internal static readonly IncludesOperationRewriter Instance = new IncludesOperationRewriter();

private static readonly SearchParamTableExpression IncludeLimitExpression = new SearchParamTableExpression(
null,
null,
SearchParamTableExpressionKind.IncludeLimit);

private static readonly SearchParamTableExpression IncludeUnionAllExpression = new SearchParamTableExpression(
null,
null,
SearchParamTableExpressionKind.IncludeUnionAll);
internal static new readonly IncludesOperationRewriter Instance = new IncludesOperationRewriter();

public override Expression VisitSqlRoot(SqlRootExpression expression, object context)
{
Expand Down Expand Up @@ -51,10 +41,5 @@ public override Expression VisitSqlRoot(SqlRootExpression expression, object con
var reorderedExpressions = nonIncludeExpressions.Concat(sortedIncludeExpressions).Concat(new[] { IncludeUnionAllExpression, IncludeLimitExpression }).ToList();
return new SqlRootExpression(reorderedExpressions, expression.ResourceTableExpressions);
}

private IEnumerable<SearchParamTableExpression> SortIncludeIterateExpressions(List<SearchParamTableExpression> includeIterateExpressions)
{
throw new NotImplementedException();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -781,12 +781,8 @@ private void HandleTableKindInclude(

StringBuilder.Append("SELECT DISTINCT ");

if (includeExpression.Reversed || context.IsIncludesOperation)
{
// In case its revinclude, we limit the number of returned items as the resultset size is potentially
// unbounded. we ask for +1 so in the limit expression we know if to mark at truncated...
StringBuilder.Append("TOP (").Append(Parameters.AddParameter(context.IncludeCount + 1, includeInHash: false)).Append(") ");
}
// Adding 1 to the include count for detecting a case of truncated "include" resources.
StringBuilder.Append("TOP (").Append(Parameters.AddParameter(context.IncludeCount + 1, includeInHash: false)).Append(") ");

var table = !includeExpression.Reversed ? referenceTargetResourceTableAlias : referenceSourceTableAlias;

Expand Down Expand Up @@ -911,6 +907,7 @@ private void HandleTableKindInclude(
{
var tableAlias = includeExpression.Reversed ? referenceSourceTableAlias : referenceTargetResourceTableAlias;
delimited.BeginDelimitedElement()
.Append("(")
.Append(VLatest.Resource.ResourceTypeId, tableAlias)
.Append(" > ")
.Append(includesContinuationToken.IncludeResourceTypeId)
Expand All @@ -922,7 +919,7 @@ private void HandleTableKindInclude(
.Append(VLatest.ReferenceSearchParam.ResourceSurrogateId, tableAlias)
.Append(" > ")
.Append(includesContinuationToken.IncludeResourceSurrogateId)
.Append(")");
.Append("))");
}

var scope = delimited.BeginDelimitedElement();
Expand Down Expand Up @@ -996,15 +993,8 @@ private void HandleTableKindInclude(

private void HandleTableKindIncludeLimit(SearchOptions context)
{
var includeCount = context.IncludeCount;
if (context.IsIncludesOperation)
{
// Adding 1 for detecting an IsPartial resource if any.
includeCount++;
}

StringBuilder.Append("SELECT DISTINCT TOP (")
.Append(Parameters.AddParameter(includeCount, includeInHash: false))
.Append(Parameters.AddParameter(context.IncludeCount + 1, includeInHash: false))
.Append(") T1, Sid1, IsMatch, ");

StringBuilder.Append("CASE WHEN count_big(*) over() > ")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
<EmbeddedResource Include="TestFiles\Normative\Import-DupPatientTemplate.ndjson" />
<EmbeddedResource Include="TestFiles\Normative\Import-Patient.ndjson" />
<EmbeddedResource Include="TestFiles\Normative\Import-SinglePatientTemplate.ndjson" />
<EmbeddedResource Include="TestFiles\Normative\Includes-TestResources.json" />
<EmbeddedResource Include="TestFiles\Normative\Medication.json" />
<EmbeddedResource Include="TestFiles\Normative\MemberMatch.json" />
<EmbeddedResource Include="TestFiles\Normative\PatientWithUnknownElements.json" />
Expand Down Expand Up @@ -206,6 +207,7 @@
<EmbeddedResource Include="TestFiles\R5\DocumentReference-example-with-base64.json" />
<EmbeddedResource Include="TestFiles\R5\Encounter-For-Patient-f001.json" />
<EmbeddedResource Include="TestFiles\R5\Immunization.json" />
<EmbeddedResource Include="TestFiles\R5\Includes-TestResources.json" />
<EmbeddedResource Include="TestFiles\R5\InvalidCompartmentDefinition.json" />
<EmbeddedResource Include="TestFiles\R5\Location-example-hq.json" />
<EmbeddedResource Include="TestFiles\R5\MedicinalProductDefinition.json" />
Expand Down
Loading

0 comments on commit 123023c

Please sign in to comment.