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

Fixed issue where bundle extension could return null when resource exists #19

Merged
merged 2 commits into from
Jan 3, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
9 changes: 9 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -328,3 +328,12 @@ ASALocalRun/

# MFractors (Xamarin productivity tool) working folder
.mfractor/

## Mac OS generated
.DS_Store
.DS_Store?
._*
.Spotlight-V100
.Trashes
ehthumbs.db
Thumbs.db
17 changes: 12 additions & 5 deletions src/lib/Microsoft.Health.Extensions.Fhir.R4/BundleExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,21 +41,28 @@ public static IEnumerable<TResource> ReadFromBundle<TResource>(this Bundle bundl
}
}

public static TResource ReadOneFromBundle<TResource>(this Bundle bundle, bool throwOnMultipleFound = true)
public static async Task<TResource> ReadOneFromBundleWithContinuationAsync<TResource>(this Bundle bundle, IFhirClient fhirClient, bool throwOnMultipleFound = true)
where TResource : Resource, new()
{
var bundleCount = bundle?.Entry?.Count ?? 0;
if (bundleCount == 0)
if (bundle == null)
{
return null;
}

var resources = await bundle?.ReadFromBundleWithContinuationAsync<TResource>(fhirClient, 2);

var resourceCount = resources.Count();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, but best to use .ToArray to avoid multiple enumerations over the same collection (happens with Count & FirstOrDefault). Impact is minimal since ReadFromBundleWithContinuation is using a materialized collection behind the scenes (list in this case) but if that ever changes we should re-evaluate.

We also may want to look at using async enumerables going forward as well https://docs.microsoft.com/en-us/archive/msdn-magazine/2019/november/csharp-iterating-with-async-enumerables-in-csharp-8

if (resourceCount == 0)
{
return null;
}

if (throwOnMultipleFound && bundleCount > 1)
if (throwOnMultipleFound && resourceCount > 1)
{
throw new MultipleResourceFoundException<TResource>();
}

return bundle.Entry.ByResourceType<TResource>().FirstOrDefault();
return resources.FirstOrDefault();
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ protected static async Task<TResource> GetResourceByIdentityAsync<TResource>(IFh
EnsureArg.IsNotNull(identifier, nameof(identifier));
var searchParams = identifier.ToSearchParams();
var result = await client.SearchAsync<TResource>(searchParams).ConfigureAwait(false);
return result.ReadOneFromBundle<TResource>();
return await result.ReadOneFromBundleWithContinuationAsync<TResource>(client);
}

protected static async Task<TResource> CreateResourceByIdentityAsync<TResource>(IFhirClient client, Model.Identifier identifier, Action<TResource, Model.Identifier> propertySetter)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ public virtual async Task<string> SaveObservationAsync(ILookupTemplate<IFhirTemp
.SetAbsoluteExpiration(DateTimeOffset.UtcNow.AddHours(1))
.SetSize(1)
.SetValue(result)
.Dispose();
.Dispose();

return result.Id;
}

Expand Down Expand Up @@ -137,7 +137,7 @@ public virtual Model.Observation MergeObservation(ILookupTemplate<IFhirTemplate>
{
var searchParams = identifier.ToSearchParams();
var result = await _client.SearchAsync<Model.Observation>(searchParams).ConfigureAwait(false);
return result.ReadOneFromBundle<Model.Observation>();
return await result.ReadOneFromBundleWithContinuationAsync<Model.Observation>(_client);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

using System;
namalu marked this conversation as resolved.
Show resolved Hide resolved
using System.Threading.Tasks;
using Xunit;
using Hl7.Fhir.Model;
using Hl7.Fhir.Rest;
using System.Collections.Generic;
using NSubstitute;

namespace Microsoft.Health.Extensions.Fhir.R4.UnitTests
{
public class BundleExtensionsTests
{
[Fact]
public async void GivenNoEntriesAndNoContinuationToken_ReadOneFromBundleWithContinuationAsync_ThenNullIsReturned_Test()
{
var bundle = new Bundle();
bundle.Entry = new List<Bundle.EntryComponent>();
bundle.Link = new List<Bundle.LinkComponent>();

var client = Substitute.For<IFhirClient>();
client.ContinueAsync(Arg.Any<Bundle>()).Returns(System.Threading.Tasks.Task.FromResult<Bundle>(null));

Assert.Null(await bundle.ReadOneFromBundleWithContinuationAsync<Observation>(client));
}

[Fact]
public async void GivenOneEntryAndNoContinuationToken_ReadOneFromBundleWithContinuationAsync_ThenResourceIsReturned_Test()
{
var bundle = new Bundle();
bundle.Entry = new List<Bundle.EntryComponent>();
bundle.Link = new List<Bundle.LinkComponent>();

var observation = new Observation();
var entry = new Bundle.EntryComponent();
entry.Resource = observation;
bundle.Entry.Add(entry);

var client = Substitute.For<IFhirClient>();
client.ContinueAsync(Arg.Any<Bundle>()).Returns(System.Threading.Tasks.Task.FromResult<Bundle>(null));

Assert.Equal(observation, await bundle.ReadOneFromBundleWithContinuationAsync<Observation>(client));
}

[Fact]
public async void GivenOneEntryAfterContinuationToken_ReadOneFromBundleWithContinuationAsync_ThenResourceIsReturned_Test()
{
var bundle = new Bundle();
bundle.Entry = new List<Bundle.EntryComponent>();
bundle.Link = new List<Bundle.LinkComponent>();

var continuationBundle = new Bundle();
continuationBundle.Entry = new List<Bundle.EntryComponent>();
continuationBundle.Link = new List<Bundle.LinkComponent>();

var observation = new Observation();
var entry = new Bundle.EntryComponent();
entry.Resource = observation;
continuationBundle.Entry.Add(entry);

var client = Substitute.For<IFhirClient>();
client.ContinueAsync(Arg.Any<Bundle>()).Returns(ret => System.Threading.Tasks.Task.FromResult(continuationBundle), ret => System.Threading.Tasks.Task.FromResult<Bundle>(null));

Assert.Equal(observation, await bundle.ReadOneFromBundleWithContinuationAsync<Observation>(client));
}

[Fact]
public async void GivenTwoEntriesAndNoContinuationToken_ReadOneFromBundleWithContinuationAsync_ThenThrows_Test()
{
var bundle = new Bundle();
bundle.Entry = new List<Bundle.EntryComponent>();
bundle.Link = new List<Bundle.LinkComponent>();

var observation1 = new Observation();
var entry1 = new Bundle.EntryComponent();
entry1.Resource = observation1;
bundle.Entry.Add(entry1);

var observation2 = new Observation();
var entry2 = new Bundle.EntryComponent();
entry2.Resource = observation2;
bundle.Entry.Add(entry2);

var client = Substitute.For<IFhirClient>();
client.ContinueAsync(Arg.Any<Bundle>()).Returns(System.Threading.Tasks.Task.FromResult<Bundle>(null));

await Assert.ThrowsAsync<MultipleResourceFoundException<Observation>>(() => bundle.ReadOneFromBundleWithContinuationAsync<Observation>(client));
}

[Fact]
public async void GivenOneEntryBeforeAndAfterContinuationToken_ReadOneFromBundleWithContinuationAsync_ThenThrows_Test()
{
var bundle = new Bundle();
bundle.Entry = new List<Bundle.EntryComponent>();
bundle.Link = new List<Bundle.LinkComponent>();

var observation1 = new Observation();
var entry1 = new Bundle.EntryComponent();
entry1.Resource = observation1;
bundle.Entry.Add(entry1);

var continuationBundle = new Bundle();
continuationBundle.Entry = new List<Bundle.EntryComponent>();
continuationBundle.Link = new List<Bundle.LinkComponent>();

var observation2 = new Observation();
var entry2 = new Bundle.EntryComponent();
entry2.Resource = observation2;
continuationBundle.Entry.Add(entry2);

var client = Substitute.For<IFhirClient>();
client.ContinueAsync(Arg.Any<Bundle>()).Returns(ret => System.Threading.Tasks.Task.FromResult(continuationBundle), ret => System.Threading.Tasks.Task.FromResult<Bundle>(null));

await Assert.ThrowsAsync<MultipleResourceFoundException<Observation>>(() => bundle.ReadOneFromBundleWithContinuationAsync<Observation>(client));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,13 @@
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Hl7.Fhir.R4" Version="1.2.1" />
<PackageReference Include="NSubstitute" Version="4.2.0" />
</ItemGroup>
<ItemGroup>
<AdditionalFiles Include="..\..\stylecop.json" Link="stylecop.json" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\..\src\lib\Microsoft.Health.Extensions.Fhir.R4\Microsoft.Health.Extensions.Fhir.R4.csproj" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public void GivenPopulatedEvent_WhenConvert_ThenTokenWithNonSerializedBodyReturn
}

[Theory]
[FileData(@"TestInput\data_IotHubPayloadExample.json")]
[FileData(@"TestInput/data_IotHubPayloadExample.json")]
public void GivenIoTCentralPopulatedEvent_WhenConvert_ThenTokenWithNonSerializedBodyAndPropertiesReturned_Test(string json)
{
var evt = EventDataTestHelper.BuildEventFromJson(json);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace Microsoft.Health.Fhir.Ingest.Template
public class CodeValueFhirTemplateFactoryTests
{
[Theory]
[FileData(@"TestInput\data_CodeValueFhirTemplate_SampledData.json")]
[FileData(@"TestInput/data_CodeValueFhirTemplate_SampledData.json")]
public void GivenValidTemplateJsonWithValueSampledDataType_WhenFactoryCreate_ThenTemplateCreated_Test(string json)
{
var templateContainer = JsonConvert.DeserializeObject<TemplateContainer>(json);
Expand Down Expand Up @@ -46,7 +46,7 @@ public void GivenValidTemplateJsonWithValueSampledDataType_WhenFactoryCreate_The
}

[Theory]
[FileData(@"TestInput\data_CodeValueFhirTemplate_Components.json")]
[FileData(@"TestInput/data_CodeValueFhirTemplate_Components.json")]
public void GivenValidTemplateJsonWithComponentValueSampledDataType_WhenFactoryCreate_ThenTemplateCreated_Test(string json)
{
var templateContainer = JsonConvert.DeserializeObject<TemplateContainer>(json);
Expand Down Expand Up @@ -100,7 +100,7 @@ public void GivenValidTemplateJsonWithComponentValueSampledDataType_WhenFactoryC
}

[Theory]
[FileData(@"TestInput\data_CodeValueFhirTemplate_CodeableConceptData.json")]
[FileData(@"TestInput/data_CodeValueFhirTemplate_CodeableConceptData.json")]
public void GivenValidTemplateJsonWithCodeableConceptDataType_WhenFactoryCreate_ThenTemplateCreated_Test(string json)
{
var templateContainer = JsonConvert.DeserializeObject<TemplateContainer>(json);
Expand Down Expand Up @@ -141,7 +141,7 @@ public void GivenValidTemplateJsonWithCodeableConceptDataType_WhenFactoryCreate_
}

[Theory]
[FileData(@"TestInput\data_CodeValueFhirTemplate_Quantity.json")]
[FileData(@"TestInput/data_CodeValueFhirTemplate_Quantity.json")]
public void GivenValidTemplateJsonWithComponentValueQuantityType_WhenFactoryCreate_ThenTemplateCreated_Test(string json)
{
var templateContainer = JsonConvert.DeserializeObject<TemplateContainer>(json);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,22 @@ namespace Microsoft.Health.Fhir.Ingest.Template
public class CollectionContentTemplateFactoryTests
{
[Theory]
[FileData(@"TestInput\data_CollectionContentTemplateEmpty.json")]
[FileData(@"TestInput/data_CollectionContentTemplateEmpty.json")]
public void GivenEmptyConfig_WhenCreate_ThenInvalidTemplateException_Test(string json)
{
Assert.Throws<InvalidTemplateException>(() => CollectionContentTemplateFactory.Default.Create(json));
}

[Theory]
[FileData(@"TestInput\data_CollectionContentTemplateEmptyWithType.json")]
[FileData(@"TestInput/data_CollectionContentTemplateEmptyWithType.json")]
public void GivenEmptyTemplateCollection_WhenCreate_ThenTemplateReturned_Test(string json)
{
var template = CollectionContentTemplateFactory.Default.Create(json);
Assert.NotNull(template);
}

[Theory]
[FileData(@"TestInput\data_CollectionContentTemplateMultipleMocks.json")]
[FileData(@"TestInput/data_CollectionContentTemplateMultipleMocks.json")]
public void GivenInputWithMatchingFactories_WhenCreate_ThenTemplateReturned_Test(string json)
{
IContentTemplate nullReturn = null;
Expand All @@ -48,7 +48,7 @@ public void GivenInputWithMatchingFactories_WhenCreate_ThenTemplateReturned_Test
}

[Theory]
[FileData(@"TestInput\data_CollectionContentTemplateMultipleMocks.json")]
[FileData(@"TestInput/data_CollectionContentTemplateMultipleMocks.json")]
public void GivenInputWithNoMatchingFactories_WhenCreate_ThenException_Test(string json)
{
IContentTemplate nullReturn = null;
Expand All @@ -67,7 +67,7 @@ public void GivenInputWithNoMatchingFactories_WhenCreate_ThenException_Test(stri
}

[Theory]
[FileData(@"TestInput\data_CollectionFhirTemplateMixed.json")]
[FileData(@"TestInput/data_CollectionFhirTemplateMixed.json")]
public void GivenInputWithMultipleTemplates_WhenCreate_ThenTemplateReturn_Test(string json)
{
var template = CollectionContentTemplateFactory.Default.Create(json);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,22 @@ namespace Microsoft.Health.Fhir.Ingest.Template
public class CollectionFhirTemplateFactoryTests
{
[Theory]
[FileData(@"TestInput\data_CollectionFhirTemplateEmpty.json")]
[FileData(@"TestInput/data_CollectionFhirTemplateEmpty.json")]
public void GivenEmptyConfig_WhenCreate_ThenInvalidTemplateException_Test(string json)
{
Assert.Throws<InvalidTemplateException>(() => CollectionFhirTemplateFactory.Default.Create(json));
}

[Theory]
[FileData(@"TestInput\data_CollectionFhirTemplateEmptyWithType.json")]
[FileData(@"TestInput/data_CollectionFhirTemplateEmptyWithType.json")]
public void GivenEmptyTemplateCollection_WhenCreate_ThenTemplateReturned_Test(string json)
{
var template = CollectionFhirTemplateFactory.Default.Create(json);
Assert.NotNull(template);
}

[Theory]
[FileData(@"TestInput\data_CollectionFhirTemplateMultipleMocks.json")]
[FileData(@"TestInput/data_CollectionFhirTemplateMultipleMocks.json")]
public void GivenInputWithRegisteredFactories_WhenCreate_ThenTemplateReturned_Test(string json)
{
IFhirTemplate nullReturn = null;
Expand Down Expand Up @@ -56,7 +56,7 @@ public void GivenInputWithRegisteredFactories_WhenCreate_ThenTemplateReturned_Te
}

[Theory]
[FileData(@"TestInput\data_CollectionFhirTemplateMultipleMocks.json")]
[FileData(@"TestInput/data_CollectionFhirTemplateMultipleMocks.json")]
public void GivenInputWithUnregisteredFactories_WhenCreate_ThenException_Test(string json)
{
IFhirTemplate nullReturn = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace Microsoft.Health.Fhir.Ingest.Template
public class IotJsonPathContentTemplateFactoryTests
{
[Theory]
[FileData(@"TestInput\data_IotJsonPathContentTemplateValid.json")]
[FileData(@"TestInput/data_IotJsonPathContentTemplateValid.json")]
public void GivenValidTemplateJson_WhenFactoryCreate_ThenTemplateCreated_Test(string json)
{
var templateContainer = JsonConvert.DeserializeObject<TemplateContainer>(json);
Expand All @@ -39,7 +39,7 @@ public void GivenValidTemplateJson_WhenFactoryCreate_ThenTemplateCreated_Test(st
}

[Theory]
[FileData(@"TestInput\data_IotJsonPathContentTemplateValidWithOptional.json")]
[FileData(@"TestInput/data_IotJsonPathContentTemplateValidWithOptional.json")]
public void GivenValidTemplateJsonWithOptionalExpressions_WhenFactoryCreate_ThenTemplateCreated_Test(string json)
{
var templateContainer = JsonConvert.DeserializeObject<TemplateContainer>(json);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public class IotJsonPathContentTemplateTests
};

[Theory]
[FileData(@"TestInput\data_IotHubPayloadExample.json")]
[FileData(@"TestInput/data_IotHubPayloadExample.json")]
public void GivenTemplateAndSingleValidToken_WhenGetMeasurements_ThenSingleMeasurementReturned_Test(string eventJson)
{
var evt = EventDataTestHelper.BuildEventFromJson(eventJson);
Expand All @@ -58,7 +58,7 @@ public void GivenTemplateAndSingleValidToken_WhenGetMeasurements_ThenSingleMeasu
}

[Theory]
[FileData(@"TestInput\data_IotHubPayloadMultiValueExample.json")]
[FileData(@"TestInput/data_IotHubPayloadMultiValueExample.json")]
public void GivenTemplateAndSingleMultiValueValidToken_WhenGetMeasurements_ThenSingleMeasurementReturned_Test(string eventJson)
{
var evt = EventDataTestHelper.BuildEventFromJson(eventJson);
Expand Down Expand Up @@ -88,7 +88,7 @@ public void GivenTemplateAndSingleMultiValueValidToken_WhenGetMeasurements_ThenS
}

[Theory]
[FileData(@"TestInput\data_IoTHubPayloadExampleMissingCreateTime.json")]
[FileData(@"TestInput/data_IoTHubPayloadExampleMissingCreateTime.json")]
public void GivenTemplateAndSingleValidTokenWithoutCreationTime_WhenGetMeasurements_ThenSingleMeasurementReturned_Test(string eventJson)
{
var evt = EventDataTestHelper.BuildEventFromJson(eventJson);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace Microsoft.Health.Fhir.Ingest.Template
public class JsonContentTemplateFactoryTests
{
[Theory]
[FileData(@"TestInput\data_JsonPathContentTemplateValid.json")]
[FileData(@"TestInput/data_JsonPathContentTemplateValid.json")]
public void GivenValidTemplateJson_WhenFactoryCreate_ThenTemplateCreated_Test(string json)
{
var templateContainer = JsonConvert.DeserializeObject<TemplateContainer>(json);
Expand All @@ -39,7 +39,7 @@ public void GivenValidTemplateJson_WhenFactoryCreate_ThenTemplateCreated_Test(st
}

[Theory]
[FileData(@"TestInput\data_JsonPathContentTemplateValidWithOptional.json")]
[FileData(@"TestInput/data_JsonPathContentTemplateValidWithOptional.json")]
public void GivenValidTemplateJsonWithOptionalExpressions_WhenFactoryCreate_ThenTemplateCreated_Test(string json)
{
var templateContainer = JsonConvert.DeserializeObject<TemplateContainer>(json);
Expand Down