Skip to content

Commit

Permalink
Supports _type search parameter, search across all resource types, an…
Browse files Browse the repository at this point in the history
…d :missing modifier (#154)

or access control, we need to the resource type to be included in the search expression tree. This also opens up the possibility of supporting the _type search parameter and searches across all resource types (/?_lastUpdated=gt2010-10-01 in addition to /Observation?_lastUpdated=gt2010-10-01).

Also makes an adjustment to the search expression trees, to explicitly state which search parameter an expression tree is bound to, by introducing a SearchParameterExpression type. Previously, there was just a convention, an implicit contract between the expression parser and the data layer. This meant that (AND (AND a b) (AND c d)) was not the same as (AND a b c d), so tree rewriting would not be a safe thing to do.

Finally, we implement support for the :missing search parameter modifier.

Resolves #152
Resolves #153
  • Loading branch information
johnstairs authored Nov 1, 2018
1 parent a27da73 commit ba711e1
Show file tree
Hide file tree
Showing 44 changed files with 529 additions and 354 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,26 +41,6 @@ public FhirControllerTests()
_authorizationService);
}

[Fact]
public void GivenUIIsSupported_WhenRequestingForRoot_ThenViewResultShouldBeReturned()
{
_featureConfiguration.SupportsUI = true;

IActionResult result = _controller.Fhir();

Assert.IsType<ViewResult>(result);
}

[Fact]
public void GivenUIIsNotSupported_WhenRequestingForRoot_ThenNotFoundResultShouldBeReturned()
{
_featureConfiguration.SupportsUI = false;

IActionResult result = _controller.Fhir();

Assert.IsType<NotFoundResult>(result);
}

[Fact]
public async void GivenHardDeleteActionNotAuthorized_WhenRequestingHardDeleteAction_ThenForbiddenResultShouldBeReturned()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,16 +126,18 @@ public void GivenAFhirRequest_WhenExecutingAnValidAction_ThenLogAuditMustBeCalle
}

[Theory]
[InlineData("Create", AuditEventSubType.Create)]
[InlineData("Update", AuditEventSubType.Update)]
[InlineData("Read", AuditEventSubType.Read)]
[InlineData("SystemHistory", AuditEventSubType.HistorySystem)]
[InlineData("TypeHistory", AuditEventSubType.HistoryType)]
[InlineData("History", AuditEventSubType.HistoryInstance)]
[InlineData("VRead", AuditEventSubType.VRead)]
[InlineData("Delete", AuditEventSubType.Delete)]
[InlineData("Search", AuditEventSubType.SearchType)]
[InlineData("SearchPost", AuditEventSubType.SearchType)]
[InlineData(nameof(FhirController.Create), AuditEventSubType.Create)]
[InlineData(nameof(FhirController.Update), AuditEventSubType.Update)]
[InlineData(nameof(FhirController.Read), AuditEventSubType.Read)]
[InlineData(nameof(FhirController.SystemHistory), AuditEventSubType.HistorySystem)]
[InlineData(nameof(FhirController.TypeHistory), AuditEventSubType.HistoryType)]
[InlineData(nameof(FhirController.History), AuditEventSubType.HistoryInstance)]
[InlineData(nameof(FhirController.VRead), AuditEventSubType.VRead)]
[InlineData(nameof(FhirController.Delete), AuditEventSubType.Delete)]
[InlineData(nameof(FhirController.SearchByResourceType), AuditEventSubType.SearchType)]
[InlineData(nameof(FhirController.SearchByResourceTypePost), AuditEventSubType.SearchType)]
[InlineData(nameof(FhirController.Search), AuditEventSubType.SearchSystem)]
[InlineData(nameof(FhirController.SearchPost), AuditEventSubType.SearchSystem)]
public void GivenAFhirRequest_WhenExecutingAnValidAction_ThenCorrectRequestSubTypeMustBeSet(string methodName, string auditEventSubType)
{
var executingContext = new ActionExecutingContext(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public void GivenAResource_WhenResourceUrlIsResolvedWithHistory_ThenCorrectUrlSh
[Fact]
public void GivenANullUnsupportedSearchParams_WhenSearchUrlIsResolved_ThenCorrectUrlShouldBeReturned()
{
_urlResolver.ResolveSearchUrl(unsupportedSearchParams: null, continuationToken: null);
_urlResolver.ResolveSearchUrl("Patient", unsupportedSearchParams: null, continuationToken: null);

ValidateUrlRouteContext(
"SearchResources",
Expand Down Expand Up @@ -247,7 +247,7 @@ private void TestAndValidateRouteWithQueryParameter(
{
_httpContext.Request.QueryString = new QueryString(inputQueryString);

_urlResolver.ResolveSearchUrl(unsupportedSearchParams, continuationToken);
_urlResolver.ResolveSearchUrl("Patient", unsupportedSearchParams, continuationToken);

ValidateUrlRouteContext(
"SearchResources",
Expand Down
46 changes: 25 additions & 21 deletions src/Microsoft.Health.Fhir.Api/Controllers/FhirController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,25 +89,6 @@ public FhirController(
_featureConfiguration = uiConfiguration.Value;
}

/// <summary>
/// Default page of the FHIR endpoint
/// </summary>
[ApiExplorerSettings(IgnoreApi = true)]
[Route("")]
[AllowAnonymous]
public IActionResult Fhir()
{
_logger.LogInformation("Default route called.");

if (_featureConfiguration.SupportsUI)
{
return View("Welcome");
}

// If the UI is not supported, return NotFound.
return new NotFoundResult();
}

[ApiExplorerSettings(IgnoreApi = true)]
[Route("CustomError")]
[AllowAnonymous]
Expand Down Expand Up @@ -345,6 +326,29 @@ public async Task<IActionResult> Delete(string type, string id, [FromQuery]bool
return FhirResult.NoContent().SetETagHeader(response.WeakETag);
}

/// <summary>
/// Searches across all resource types.
/// </summary>
[Route("", Name = RouteNames.SearchAllResources)]
[AuditEventSubType(AuditEventSubType.SearchSystem)]
[Authorize(PolicyNames.ReadPolicy)]
public async Task<IActionResult> Search()
{
return await SearchByResourceType(type: null);
}

/// <summary>
/// Searches for resources.
/// </summary>
[HttpPost]
[Route(KnownRoutes.Search, Name = RouteNames.SearchAllResourcesPost)]
[AuditEventSubType(AuditEventSubType.SearchSystem)]
[Authorize(PolicyNames.ReadPolicy)]
public async Task<IActionResult> SearchPost()
{
return await SearchByResourceTypePost(type: null);
}

/// <summary>
/// Searches for resources.
/// </summary>
Expand All @@ -353,7 +357,7 @@ public async Task<IActionResult> Delete(string type, string id, [FromQuery]bool
[Route(KnownRoutes.ResourceType, Name = RouteNames.SearchResources)]
[AuditEventSubType(AuditEventSubType.SearchType)]
[Authorize(PolicyNames.ReadPolicy)]
public async Task<IActionResult> Search(string type)
public async Task<IActionResult> SearchByResourceType(string type)
{
IReadOnlyList<Tuple<string, string>> queries = Array.Empty<Tuple<string, string>>();

Expand All @@ -374,7 +378,7 @@ public async Task<IActionResult> Search(string type)
[Route(KnownRoutes.ResourceTypeSearch, Name = RouteNames.SearchResourcesPost)]
[AuditEventSubType(AuditEventSubType.SearchType)]
[Authorize(PolicyNames.ReadPolicy)]
public async Task<IActionResult> SearchPost(string type)
public async Task<IActionResult> SearchByResourceTypePost(string type)
{
var queries = new List<Tuple<string, string>>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,18 @@ public override async Task OnActionExecutionAsync(ActionExecutingContext context
{
var resourceFormat = ContentType.GetResourceFormatFromContentType(headerValue[0]);

if (!await _contentTypeService.IsFormatSupportedAsync(resourceFormat) && context.ActionDescriptor?.AttributeRouteInfo?.Name != RouteNames.SearchResourcesPost)
if (!await _contentTypeService.IsFormatSupportedAsync(resourceFormat))
{
throw new UnsupportedMediaTypeException(Resources.UnsupportedContentTypeHeader);
string routeName = context.ActionDescriptor?.AttributeRouteInfo?.Name;

switch (routeName)
{
case RouteNames.SearchResourcesPost:
case RouteNames.SearchAllResourcesPost:
break;
default:
throw new UnsupportedMediaTypeException(Resources.UnsupportedContentTypeHeader);
}
}
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ internal class KnownRoutes
private const string VidRouteSegment = "{" + KnownActionParameterNames.Vid + "}";

public const string History = "_history";
public const string Search = "_search";
public const string ResourceType = ResourceTypeRouteSegment;
public const string ResourceTypeHistory = ResourceType + "/" + History;
public const string ResourceTypeSearch = ResourceType + "/_search";
public const string ResourceTypeSearch = ResourceType + "/" + Search;
public const string ResourceTypeById = ResourceType + "/" + IdRouteSegment;
public const string ResourceTypeByIdHistory = ResourceTypeById + "/" + History;
public const string ResourceTypeByIdAndVid = ResourceTypeByIdHistory + "/" + VidRouteSegment;
Expand Down
4 changes: 4 additions & 0 deletions src/Microsoft.Health.Fhir.Api/Features/Routing/RouteNames.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,16 @@ internal static class RouteNames

internal const string SearchResources = "SearchResources";

internal const string SearchAllResources = "SearchAllResources";

internal const string History = "History";

internal const string HistoryType = "HistoryType";

internal const string HistoryTypeId = "HistoryTypeId";

internal const string SearchResourcesPost = "SearchResourcesPost";

internal const string SearchAllResourcesPost = "SearchAllResourcesPost";
}
}
4 changes: 2 additions & 2 deletions src/Microsoft.Health.Fhir.Api/Features/Routing/UrlResolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ public Uri ResolveResourceUrl(Resource resource, bool includeVersion = false)
}

/// <inheritdoc />
public Uri ResolveSearchUrl(IEnumerable<Tuple<string, string>> unsupportedSearchParams = null, string continuationToken = null)
public Uri ResolveSearchUrl(string resourceType, IEnumerable<Tuple<string, string>> unsupportedSearchParams = null, string continuationToken = null)
{
return ResolveRouteUrl(RouteNames.SearchResources, unsupportedSearchParams, continuationToken);
return ResolveRouteUrl(string.IsNullOrEmpty(resourceType) ? RouteNames.SearchAllResources : RouteNames.SearchResources, unsupportedSearchParams, continuationToken);
}

public Uri ResolveRouteUrl(string routeName, IEnumerable<Tuple<string, string>> unsupportedSearchParams = null, string continuationToken = null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

<ItemGroup>
<EmbeddedResource Include="Views\Shared\ViewJson.cshtml" />
<EmbeddedResource Include="Views\Shared\Welcome.cshtml" />
<EmbeddedResource Include="Views\Shared\_Layout.cshtml" />
<EmbeddedResource Include="Views\_ViewImports.cshtml" />
<EmbeddedResource Include="Views\_ViewStart.cshtml" />
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.Health.Fhir.Api/Modules/SearchModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public void Load(IServiceCollection services)
.AsService<IFhirElementToSearchValueTypeConverterManager>();

services.AddSingleton<ISearchIndexer, SearchIndexer>();
services.AddSingleton<ISearchValueExpressionBuilder, SearchValueExpressionBuilder>();
services.AddSingleton<ISearchParameterExpressionParser, SearchParameterExpressionParser>();
services.AddSingleton<IExpressionParser, ExpressionParser>();
services.AddSingleton<ISearchOptionsFactory, SearchOptionsFactory>();

Expand Down
12 changes: 0 additions & 12 deletions src/Microsoft.Health.Fhir.Api/Views/Shared/Welcome.cshtml

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ public void GivenAValidSearchParameterDefinitionFile_WhenBuilt_ThenCorrectListOf

ValidateSearchParameters(
searchParametersDictionary,
("_type", SearchParamType.Token, "Resource.type().name"),
("_id", SearchParamType.Token, "Resource.id"),
("balance", SearchParamType.Quantity, "Account.balance"),
("identifier", SearchParamType.Token, "Account.identifier"));
Expand All @@ -103,6 +104,7 @@ public void GivenAValidSearchParameterDefinitionFile_WhenBuilt_ThenCorrectListOf

ValidateSearchParameters(
searchParametersDictionary,
("_type", SearchParamType.Token, "Resource.type().name"),
("_id", SearchParamType.Token, "Resource.id"),
("identifier", SearchParamType.Token, "MedicationRequest.identifier | MedicationAdministration.identifier | MedicationStatement.identifier | MedicationDispense.identifier"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ namespace Microsoft.Health.Fhir.Core.UnitTests.Features.Search.Expressions.Parse
public class ExpressionParserTests
{
private readonly ISearchParameterDefinitionManager _searchParameterDefinitionManager = Substitute.For<ISearchParameterDefinitionManager>();
private readonly ISearchValueExpressionBuilder _searchValueExpressionBuilder = Substitute.For<ISearchValueExpressionBuilder>();
private readonly ISearchParameterExpressionParser _searchParameterExpressionParser = Substitute.For<ISearchParameterExpressionParser>();

private readonly ExpressionParser _expressionParser;

public ExpressionParserTests()
{
_expressionParser = new ExpressionParser(
_searchParameterDefinitionManager,
_searchValueExpressionBuilder);
_searchParameterExpressionParser);
}

[Fact]
Expand Down Expand Up @@ -243,15 +243,15 @@ public void GivenAModifier_WhenParsed_ThenExceptionShouldBeThrown()
string param1 = "ref";
string modifier = "missing";

// Practitioner is a valid resource type but is not supported by the search paramter.
// Practitioner is a valid resource type but is not supported by the search parameter.
string key = $"{param1}:{modifier}";
string value = "Seattle";

SearchParameter searchParameter = SetupSearchParameter(resourceType, param1);

Expression expression = Substitute.For<Expression>();

_searchValueExpressionBuilder.Build(searchParameter, SearchParameter.SearchModifierCode.Missing, value).Returns(expression);
_searchParameterExpressionParser.Parse(searchParameter, SearchParameter.SearchModifierCode.Missing, value).Returns(expression);

// Parse the expression.
Expression actualExpression = _expressionParser.Parse(resourceType, key, value);
Expand Down Expand Up @@ -361,7 +361,7 @@ private Expression SetupExpression(SearchParameter searchParameter, string value
{
Expression expectedExpression = Substitute.For<Expression>();

_searchValueExpressionBuilder.Build(searchParameter, null, value).Returns(expectedExpression);
_searchParameterExpressionParser.Parse(searchParameter, null, value).Returns(expectedExpression);

return expectedExpression;
}
Expand Down
Loading

0 comments on commit ba711e1

Please sign in to comment.