Skip to content

Commit

Permalink
Merge branch 'master' into personal/poadhikari/negative_tests_added
Browse files Browse the repository at this point in the history
  • Loading branch information
Pooja Adhikari committed Dec 10, 2018
2 parents e9a58b1 + f7f7c15 commit 686b655
Show file tree
Hide file tree
Showing 14 changed files with 256 additions and 129 deletions.
12 changes: 12 additions & 0 deletions .github/ISSUE_TEMPLATE/user-story.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
name: User story
about: Tell us something new you want to do
labels:

---

**User story**
As a [type of user], I want [some functionality] so that [benefit].

**Acceptance criteria**
1. When I do [A], then [B] happens.
8 changes: 8 additions & 0 deletions PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
## Description
Describe the changes in this PR.

## Related issues
Addresses [issue #].

## Testing
Describe how this change was tested.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.Primitives;
using Microsoft.Health.Fhir.Api.Features.Routing;
using Microsoft.Health.Fhir.Core.Features.Context;
using NSubstitute;
using Xunit;

Expand All @@ -23,7 +24,9 @@ public class UrlResolverTests
private const string Scheme = "http";
private const string Host = "test";
private const string ContinuationTokenQueryParamName = "ct";
private const string DefaultRouteName = "Route";

private readonly IFhirRequestContextAccessor _fhirRequestContextAccessor = Substitute.For<IFhirRequestContextAccessor>();
private readonly IUrlHelperFactory _urlHelperFactory = Substitute.For<IUrlHelperFactory>();
private readonly IHttpContextAccessor _httpContextAccessor = Substitute.For<IHttpContextAccessor>();
private readonly IActionContextAccessor _actionContextAccessor = Substitute.For<IActionContextAccessor>();
Expand All @@ -38,7 +41,13 @@ public class UrlResolverTests

public UrlResolverTests()
{
_urlResolver = new UrlResolver(_urlHelperFactory, _httpContextAccessor, _actionContextAccessor);
_urlResolver = new UrlResolver(
_fhirRequestContextAccessor,
_urlHelperFactory,
_httpContextAccessor,
_actionContextAccessor);

_fhirRequestContextAccessor.FhirRequestContext.RouteName = DefaultRouteName;

_httpContextAccessor.HttpContext.Returns(_httpContext);

Expand Down Expand Up @@ -103,11 +112,10 @@ public void GivenAResource_WhenResourceUrlIsResolvedWithHistory_ThenCorrectUrlSh
[Fact]
public void GivenANullUnsupportedSearchParams_WhenSearchUrlIsResolved_ThenCorrectUrlShouldBeReturned()
{
_urlResolver.ResolveSearchUrl("Patient", unsupportedSearchParams: null, continuationToken: null);
_urlResolver.ResolveRouteUrl(unsupportedSearchParams: null, continuationToken: null);

ValidateUrlRouteContext(
"SearchResources",
routeValues =>
routeValuesValidator: routeValues =>
{
Assert.Empty(routeValues);
});
Expand Down Expand Up @@ -247,11 +255,10 @@ private void TestAndValidateRouteWithQueryParameter(
{
_httpContext.Request.QueryString = new QueryString(inputQueryString);

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

ValidateUrlRouteContext(
"SearchResources",
routeValues =>
routeValuesValidator: routeValues =>
{
foreach (KeyValuePair<string, object> expectedRouteValue in expectedRouteValues)
{
Expand All @@ -267,7 +274,7 @@ private void TestAndValidateRouteWithQueryParameter(
});
}

private void ValidateUrlRouteContext(string routeName, Action<RouteValueDictionary> routeValuesValidator = null)
private void ValidateUrlRouteContext(string routeName = DefaultRouteName, Action<RouteValueDictionary> routeValuesValidator = null)
{
Assert.NotNull(_capturedUrlRouteContext);
Assert.Equal(routeName, _capturedUrlRouteContext.RouteName);
Expand Down
22 changes: 14 additions & 8 deletions src/Microsoft.Health.Fhir.Api/Features/Routing/UrlResolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using EnsureThat;
using Hl7.Fhir.Model;
Expand All @@ -15,6 +16,7 @@
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.Primitives;
using Microsoft.Health.Fhir.Core.Features;
using Microsoft.Health.Fhir.Core.Features.Context;
using Microsoft.Health.Fhir.Core.Features.Routing;

namespace Microsoft.Health.Fhir.Api.Features.Routing
Expand All @@ -24,6 +26,7 @@ namespace Microsoft.Health.Fhir.Api.Features.Routing
/// </summary>
public class UrlResolver : IUrlResolver
{
private readonly IFhirRequestContextAccessor _fhirRequestContextAccessor;
private readonly IUrlHelperFactory _urlHelperFactory;

// If we update the search implementation to not use these, we should remove
Expand All @@ -35,15 +38,22 @@ public class UrlResolver : IUrlResolver
/// <summary>
/// Initializes a new instance of the <see cref="UrlResolver"/> class.
/// </summary>
/// <param name="fhirRequestContextAccessor">The FHIR request context accessor.</param>
/// <param name="urlHelperFactory">The ASP.NET Core URL helper factory.</param>
/// <param name="httpContextAccessor">The ASP.NET Core HTTP context accessor.</param>
/// <param name="actionContextAccessor">The ASP.NET Core Action context accessor.</param>
public UrlResolver(IUrlHelperFactory urlHelperFactory, IHttpContextAccessor httpContextAccessor, IActionContextAccessor actionContextAccessor)
public UrlResolver(
IFhirRequestContextAccessor fhirRequestContextAccessor,
IUrlHelperFactory urlHelperFactory,
IHttpContextAccessor httpContextAccessor,
IActionContextAccessor actionContextAccessor)
{
EnsureArg.IsNotNull(fhirRequestContextAccessor, nameof(fhirRequestContextAccessor));
EnsureArg.IsNotNull(urlHelperFactory, nameof(urlHelperFactory));
EnsureArg.IsNotNull(httpContextAccessor, nameof(httpContextAccessor));
EnsureArg.IsNotNull(actionContextAccessor, nameof(actionContextAccessor));

_fhirRequestContextAccessor = fhirRequestContextAccessor;
_urlHelperFactory = urlHelperFactory;
_httpContextAccessor = httpContextAccessor;
_actionContextAccessor = actionContextAccessor;
Expand Down Expand Up @@ -101,15 +111,11 @@ public Uri ResolveResourceUrl(Resource resource, bool includeVersion = false)
return new Uri(uriString);
}

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

public Uri ResolveRouteUrl(string routeName, IEnumerable<Tuple<string, string>> unsupportedSearchParams = null, string continuationToken = null)
{
EnsureArg.IsNotNullOrEmpty(routeName, nameof(routeName));
Debug.Assert(!string.IsNullOrWhiteSpace(routeName), "The routeName should not be null or empty.");

var routeValues = new RouteValueDictionary();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public SearchServiceTests()
_searchService = new TestSearchService(_searchOptionsFactory, _bundleFactory, _dataStore);
_rawResourceFactory = new RawResourceFactory(new FhirJsonSerializer());

_urlResolver.ResolveSearchUrl(Arg.Any<string>(), Arg.Any<IEnumerable<Tuple<string, string>>>()).Returns(SearchUrl);
_urlResolver.ResolveRouteUrl(Arg.Any<IEnumerable<Tuple<string, string>>>()).Returns(SearchUrl);

_correlationId = Guid.NewGuid().ToString();
_fhirRequestContextAccessor.FhirRequestContext.CorrelationId.Returns(_correlationId);
Expand All @@ -68,7 +68,7 @@ public async Task GivenAnySearching_WhenSearched_ThenSelfLinkShouldBeReturned()

_searchService.SearchImplementation = options => new SearchResult(new ResourceWrapper[0], null);

_urlResolver.ResolveSearchUrl(resourceType: null, unsupportedSearchParams: null, continuationToken: null).Returns(SearchUrl);
_urlResolver.ResolveRouteUrl(unsupportedSearchParams: null, continuationToken: null).Returns(SearchUrl);

Bundle actual = await _searchService.SearchAsync(resourceType, null);

Expand Down Expand Up @@ -139,7 +139,7 @@ public async Task GivenMoreMatchesAvailable_WhenSearching_ThenSearchReturnsBundl

_searchService.SearchImplementation = options => new SearchResult(new ResourceWrapper[0], searchToken);

_urlResolver.ResolveSearchUrl(resourceType, unsupportedSearchParams: null, continuationToken: searchToken).Returns(continuationLink);
_urlResolver.ResolveRouteUrl(unsupportedSearchParams: null, continuationToken: searchToken).Returns(continuationLink);

Bundle actual = await _searchService.SearchAsync(resourceType, null);

Expand Down Expand Up @@ -170,7 +170,7 @@ public async Task GivenResourceId_WhenSearchingHistoryWithSinceButNoResults_Then
var resourceWrapper =
new ResourceWrapper(observation, _rawResourceFactory.Create(observation), _resourceRequest, false, null, null, null);
_searchService.SearchImplementation = options => new SearchResult(new ResourceWrapper[0], null);
_urlResolver.ResolveRouteUrl(Arg.Any<string>(), Arg.Any<IEnumerable<Tuple<string, string>>>()).Returns(new Uri("http://narwhal"));
_urlResolver.ResolveRouteUrl(Arg.Any<IEnumerable<Tuple<string, string>>>()).Returns(new Uri("http://narwhal"));

_dataStore.GetAsync(Arg.Any<ResourceKey>(), Arg.Any<CancellationToken>()).Returns(resourceWrapper);

Expand Down
12 changes: 1 addition & 11 deletions src/Microsoft.Health.Fhir.Core/Features/Routing/IUrlResolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,12 @@ public interface IUrlResolver
/// <returns>The URL for the given <paramref name="resource"/>.</returns>
Uri ResolveResourceUrl(Resource resource, bool includeVersion = false);

/// <summary>
/// Resolves the search URL.
/// </summary>
/// <param name="resourceType">The resource type route segment, or null if the search is across all resource types.</param>
/// <param name="unsupportedSearchParams">A list of unsupported search parameters.</param>
/// <param name="continuationToken">The continuation token.</param>
/// <returns>The search URL.</returns>
Uri ResolveSearchUrl(string resourceType, IEnumerable<Tuple<string, string>> unsupportedSearchParams = null, string continuationToken = null);

/// <summary>
/// Resolves the URL for the specified route
/// </summary>
/// <param name="routeName">Name of the route</param>
/// <param name="unsupportedSearchParams">A list of unsupported search parameters.</param>
/// <param name="continuationToken">The continuation token.</param>
/// <returns>The URL.</returns>
Uri ResolveRouteUrl(string routeName, IEnumerable<Tuple<string, string>> unsupportedSearchParams = null, string continuationToken = null);
Uri ResolveRouteUrl(IEnumerable<Tuple<string, string>> unsupportedSearchParams = null, string continuationToken = null);
}
}
18 changes: 7 additions & 11 deletions src/Microsoft.Health.Fhir.Core/Features/Search/BundleFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public BundleFactory(IUrlResolver urlResolver, IFhirRequestContextAccessor fhirR
_fhirRequestContextAccessor = fhirRequestContextAccessor;
}

public Bundle CreateSearchBundle(string resourceType, IEnumerable<Tuple<string, string>> unsupportedSearchParams, SearchResult result)
public Bundle CreateSearchBundle(IEnumerable<Tuple<string, string>> unsupportedSearchParameters, SearchResult result)
{
// Create the bundle from the result.
var bundle = new Bundle();
Expand Down Expand Up @@ -56,19 +56,17 @@ public Bundle CreateSearchBundle(string resourceType, IEnumerable<Tuple<string,

if (result.ContinuationToken != null)
{
bundle.NextLink = _urlResolver.ResolveSearchUrl(
resourceType,
unsupportedSearchParams,
bundle.NextLink = _urlResolver.ResolveRouteUrl(
unsupportedSearchParameters,
result.ContinuationToken);
}
}

// Add the self link to indicate which search parameters were used.
bundle.SelfLink = _urlResolver.ResolveSearchUrl(resourceType, unsupportedSearchParams);
bundle.SelfLink = _urlResolver.ResolveRouteUrl(unsupportedSearchParameters);

bundle.Id = _fhirRequestContextAccessor.FhirRequestContext.CorrelationId;
bundle.Type = Bundle.BundleType.Searchset;
bundle.Total = result?.TotalCount;
bundle.Meta = new Meta
{
LastUpdated = Clock.UtcNow,
Expand All @@ -77,7 +75,7 @@ public Bundle CreateSearchBundle(string resourceType, IEnumerable<Tuple<string,
return bundle;
}

public Bundle CreateHistoryBundle(IEnumerable<Tuple<string, string>> unsupportedSearchParams, SearchResult result)
public Bundle CreateHistoryBundle(IEnumerable<Tuple<string, string>> unsupportedSearchParameters, SearchResult result)
{
// Create the bundle from the result.
var bundle = new Bundle();
Expand Down Expand Up @@ -111,18 +109,16 @@ public Bundle CreateHistoryBundle(IEnumerable<Tuple<string, string>> unsupported
if (result.ContinuationToken != null)
{
bundle.NextLink = _urlResolver.ResolveRouteUrl(
_fhirRequestContextAccessor.FhirRequestContext.RouteName,
unsupportedSearchParams,
unsupportedSearchParameters,
result.ContinuationToken);
}
}

// Add the self link to indicate which search parameters were used.
bundle.SelfLink = _urlResolver.ResolveRouteUrl(_fhirRequestContextAccessor.FhirRequestContext.RouteName, unsupportedSearchParams);
bundle.SelfLink = _urlResolver.ResolveRouteUrl(unsupportedSearchParameters);

bundle.Id = _fhirRequestContextAccessor.FhirRequestContext.CorrelationId;
bundle.Type = Bundle.BundleType.History;
bundle.Total = result?.TotalCount;
bundle.Meta = new Meta
{
LastUpdated = Clock.UtcNow,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace Microsoft.Health.Fhir.Core.Features.Search
{
public interface IBundleFactory
{
Bundle CreateSearchBundle(string resourceType, IEnumerable<Tuple<string, string>> unsupportedSearchParams, SearchResult result);
Bundle CreateSearchBundle(IEnumerable<Tuple<string, string>> unsupportedSearchParams, SearchResult result);

Bundle CreateHistoryBundle(IEnumerable<Tuple<string, string>> unsupportedSearchParams, SearchResult result);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public async Task<Bundle> SearchAsync(
// Execute the actual search.
SearchResult result = await SearchInternalAsync(searchOptions, cancellationToken);

return _bundleFactory.CreateSearchBundle(resourceType, searchOptions.UnsupportedSearchParams, result);
return _bundleFactory.CreateSearchBundle(searchOptions.UnsupportedSearchParams, result);
}

/// <inheritdoc />
Expand All @@ -64,7 +64,7 @@ public async Task<Bundle> SearchCompartmentAsync(string compartmentType, string
// Execute the actual search.
SearchResult result = await SearchInternalAsync(searchOptions, cancellationToken);

return _bundleFactory.CreateSearchBundle(resourceType, searchOptions.UnsupportedSearchParams, result);
return _bundleFactory.CreateSearchBundle(searchOptions.UnsupportedSearchParams, result);
}

public async Task<Bundle> SearchHistoryAsync(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

using Hl7.Fhir.Model;
using Microsoft.Health.Fhir.Tests.Common;
using Microsoft.Health.Fhir.Web;
using Xunit;
using Task = System.Threading.Tasks.Task;

namespace Microsoft.Health.Fhir.Tests.E2E.Rest
{
public class CompartmentTestFixture : HttpIntegrationTestFixture<Startup>, IAsyncLifetime
{
public Patient Patient { get; private set; }

public Observation Observation { get; private set; }

public Device Device { get; private set; }

public DeviceComponent DeviceComponent { get; private set; }

public Encounter Encounter { get; private set; }

public Condition Condition { get; private set; }

public async Task InitializeAsync()
{
// Create various resources.
Patient = await FhirClient.CreateAsync(Samples.GetJsonSample<Patient>("Patient-f001"));

string patientReference = $"Patient/{Patient.Id}";

Observation observationToCreate = Samples.GetJsonSample<Observation>("Observation-For-Patient-f001");

observationToCreate.Subject.Reference = patientReference;

Observation = await FhirClient.CreateAsync(observationToCreate);

Encounter encounterToCreate = Samples.GetJsonSample<Encounter>("Encounter-For-Patient-f001");

encounterToCreate.Subject.Reference = patientReference;

Encounter = await FhirClient.CreateAsync(encounterToCreate);

Condition conditionToCreate = Samples.GetJsonSample<Condition>("Condition-For-Patient-f001");

conditionToCreate.Subject.Reference = patientReference;

Condition = await FhirClient.CreateAsync(conditionToCreate);

Device = await FhirClient.CreateAsync(Samples.GetJsonSample<Device>("Device-d1"));

DeviceComponent deviceComponent = Samples.GetJsonSample<DeviceComponent>("DeviceComponent-For-Device-d1");

deviceComponent.Source.Reference = $"Device/{Device.Id}";

DeviceComponent = await FhirClient.CreateAsync(deviceComponent);
}

public Task DisposeAsync()
{
return Task.CompletedTask;
}
}
}
Loading

0 comments on commit 686b655

Please sign in to comment.