Skip to content

Commit

Permalink
Merge pull request #2819 from FirelyTeam/bugfix/2786-binary-data-cont…
Browse files Browse the repository at this point in the history 
…ent-issues

Either set binary.data or binary.content when parsing binaries in the FhirClient
  • Loading branch information
@mmsmits
mmsmits authored Jul 24, 2024
2 parents 6544788 + f617372 commit 7738a9e
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 21 deletions.
16 changes: 10 additions & 6 deletions src/Hl7.Fhir.Base/Rest/BaseFhirClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Hl7.Fhir.Introspection;
using Hl7.Fhir.Model;
using Hl7.Fhir.Serialization;
using Hl7.Fhir.Specification;
using Hl7.Fhir.Utility;
using System;
using System.Collections.Generic;
Expand Down Expand Up @@ -860,9 +861,9 @@ private static ResourceIdentity verifyResourceIdentity(Uri location, bool needId
if (msg is null && entryComponent is null) throw new ArgumentException("Either msg or entryComponent should be set");
// Validate the response and throw the appropriate exceptions. Also, if we have *not* verified the FHIR version
// of the server, add a suggestion about this in the (legacy) parsing exception.
var suggestedVersionOnParseError = !Settings.VerifyFhirVersion ? fhirVersion : null;

(LastResult, LastBody, LastBodyAsText, LastBodyAsResource, var issue) =
await ValidateResponse(responseMessage, expect, getSerializationEngine(), suggestedVersionOnParseError, useBinaryProtocol)
await ValidateResponse(responseMessage, expect, getSerializationEngine(), !Settings.VerifyFhirVersion, fhirVersion, useBinaryProtocol)
.ConfigureAwait(false);

// If an error occurred while trying to interpret and validate the response, we will bail out now.
Expand Down Expand Up @@ -921,17 +922,20 @@ static string unexpectedBodyTypeForMessage(HttpRequestMessage msg, Resource resu
/// <exception cref="FhirOperationException">The body content type could not be handled or the response status indicated failure, or we received an unexpected success status.</exception>
/// <exception cref="FormatException">Thrown when the original ITypedElement-based parsers are used and a parse exception occurred.</exception>
/// <exception cref="DeserializationFailedException">Thrown when a newer parsers is used and a parse exception occurred.</exception>
/// <seealso cref="HttpContentParsers.ExtractResponseData(HttpResponseMessage, IFhirSerializationEngine, bool)"/>
/// <seealso cref="HttpContentParsers.ExtractResponseData(HttpResponseMessage, IFhirSerializationEngine, bool, FhirRelease)"/>
internal static async Task<ResponseData> ValidateResponse(
HttpResponseMessage responseMessage,
IEnumerable<HttpStatusCode> expect,
IFhirSerializationEngine engine,
string? suggestedVersionOnParseError,
bool suggestVersionOnParsingError,
string fhirVersion,
bool useBinaryProtocol)
{
var responseData = (await responseMessage.ExtractResponseData(engine, useBinaryProtocol).ConfigureAwait(false))
var fhirRelease = FhirReleaseParser.Parse(fhirVersion);

var responseData = (await responseMessage.ExtractResponseData(engine, useBinaryProtocol, fhirRelease).ConfigureAwait(false))
.TranslateUnsupportedBodyTypeException(responseMessage.StatusCode)
.TranslateLegacyParserException(suggestedVersionOnParseError);
.TranslateLegacyParserException(suggestVersionOnParsingError ? fhirVersion : null);

// If extracting the data caused an issue, return it immediately
if (responseData.Issue is not null)
Expand Down
17 changes: 11 additions & 6 deletions src/Hl7.Fhir.Base/Rest/HttpContentParsers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Hl7.Fhir.ElementModel;
using Hl7.Fhir.Model;
using Hl7.Fhir.Serialization;
using Hl7.Fhir.Specification;
using Hl7.Fhir.Utility;
using System;
using System.Linq;
Expand Down Expand Up @@ -147,7 +148,7 @@ internal record ResponseData(Bundle.ResponseComponent Response, byte[]? BodyData
/// <remarks>If the status of the response indicates failure, this function will be lenient and return the body data
/// instead of throwing an <see cref="UnsupportedBodyTypeException"/> when the content type or content itself is not recognizable as FHIR. This improves
/// the chances of capturing diagnostic (non-FHIR) bodies returned by the server when an operation fails.</remarks>
internal static async Task<ResponseData> ExtractResponseData(this HttpResponseMessage message, IFhirSerializationEngine ser, bool expectBinaryProtocol)
internal static async Task<ResponseData> ExtractResponseData(this HttpResponseMessage message, IFhirSerializationEngine ser, bool expectBinaryProtocol, FhirRelease fhirRelease)
{
var component = message.ExtractResponseComponent();
var data = await message.Content.ReadAsByteArrayAsync().ConfigureAwait(false);
Expand All @@ -159,14 +160,14 @@ internal static async Task<ResponseData> ExtractResponseData(this HttpResponseMe
// If this is not binary data, try to capture the body as text
if (!expectBinaryProtocol)
result = result with { BodyText = await message.Content.GetBodyAsString().ConfigureAwait(false) };

var usesFhirFormat = ContentType.GetResourceFormatFromContentType(message.Content.GetContentType()) != ResourceFormat.Unknown;

try
{
var resource = message switch
{
{ IsSuccessStatusCode: true } when expectBinaryProtocol && !usesFhirFormat => await ReadBinaryDataFromMessage(message).ConfigureAwait(false),
{ IsSuccessStatusCode: true } when expectBinaryProtocol && !usesFhirFormat => await ReadBinaryDataFromMessage(message, fhirRelease).ConfigureAwait(false),
{ IsSuccessStatusCode: true } => await ReadResourceFromMessage(message.Content, ser).ConfigureAwait(false),
{ IsSuccessStatusCode: false } => await ReadOutcomeFromMessage(message.Content, ser).ConfigureAwait(false),
};
Expand All @@ -179,7 +180,7 @@ internal static async Task<ResponseData> ExtractResponseData(this HttpResponseMe
}

// Sets the Resource.ResourceBase to the location given in the RequestUri of the response message.
if (result.BodyResource is not null && (message.Headers.Location?.OriginalString ?? message.GetRequestUri()?.OriginalString) is {} location)
if (result.BodyResource is not null && (message.Headers.Location?.OriginalString ?? message.GetRequestUri()?.OriginalString) is { } location)
{
var ri = new ResourceIdentity(location);
result.BodyResource.ResourceBase = ri.HasBaseUri && ri.Form == ResourceIdentityForm.AbsoluteRestUrl
Expand All @@ -196,11 +197,15 @@ internal static async Task<ResponseData> ExtractResponseData(this HttpResponseMe
/// </summary>
/// <returns>A <see cref="Binary"/> resource containing the streamed binary data. The resource's
/// metadata will be retrieved from the appropriate HTTP headers.</returns>
public static async Task<Binary> ReadBinaryDataFromMessage(this HttpResponseMessage message)
public static async Task<Binary> ReadBinaryDataFromMessage(this HttpResponseMessage message, FhirRelease fhirRelease)
{
var result = new Binary();

result.Data = result.Content = await message.Content.ReadAsByteArrayAsync().ConfigureAwait(false);
if (fhirRelease == FhirRelease.STU3)
result.Content = await message.Content.ReadAsByteArrayAsync().ConfigureAwait(false);
else
result.Data = await message.Content.ReadAsByteArrayAsync().ConfigureAwait(false);

result.ContentType = message.Content.GetContentType();
result.SecurityContext = message.GetSecurityContext() is { } reference ? new ResourceReference(reference) : null;
result.Meta ??= new();
Expand Down
23 changes: 14 additions & 9 deletions src/Hl7.Fhir.Support.Poco.Tests/Rest/ResponseMessageTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public async Task SetAndExtractRelevantHeaders()
response.Headers.Location = new Uri("http://nu.nl");
response.Headers.TryAddWithoutValidation("Test-key", "Test-value");

var extracted = await response.ExtractResponseData(engine, expectBinaryProtocol: false);
var extracted = await response.ExtractResponseData(engine, expectBinaryProtocol: false, TESTINSPECTOR.FhirRelease);

extracted.BodyText.Should().Be(DEFAULT_XML);
engine.SerializeToXml(extracted.BodyResource!).Should().Be(DEFAULT_XML);
Expand All @@ -117,7 +117,7 @@ public async Task SetAndExtractRelevantHeaders()
public async Task GetEmptyResponse()
{
var response = new HttpResponseMessage(HttpStatusCode.Conflict);
var components = await response.ExtractResponseData(POCOENGINE, expectBinaryProtocol: false);
var components = await response.ExtractResponseData(POCOENGINE, expectBinaryProtocol: false, fhirRelease: Specification.FhirRelease.R4);

components.Response.Status.Should().Be("409");
components.BodyData.Should().BeNull();
Expand All @@ -128,7 +128,7 @@ public async Task GetEmptyResponse()
private static async Task check(HttpResponseMessage response, IFhirSerializationEngine engine,
bool hasResource = false, Type? expectedIssue = null, string? messagePattern = null, string? notMessagePattern = null)
{
var components = await response.ExtractResponseData(engine, expectBinaryProtocol: false).ConfigureAwait(false);
var components = await response.ExtractResponseData(engine, expectBinaryProtocol: false, fhirRelease: Specification.FhirRelease.R4).ConfigureAwait(false);
await checkResult(response, components, engine, hasResource, expectedIssue, messagePattern, notMessagePattern);
}

Expand Down Expand Up @@ -299,7 +299,7 @@ await assertIssue<FhirOperationException>(response, "Operation was unsuccessful,
public async Task TurnsNewParsingFailureIntoDFE()
{
var response = makeXmlMessage(xml: """<Unknown><active value="true" /></Unknown>""");
await assertIssue<DeserializationFailedException>(response, "*Unknown type 'Unknown' found in root property*", engine: POCOENGINE, version: "1.0.0");
await assertIssue<DeserializationFailedException>(response, "*Unknown type 'Unknown' found in root property*", engine: POCOENGINE, suggestVersionOnParsingError: true, version: "1.0.0");
}

[TestMethod]
Expand All @@ -308,17 +308,17 @@ public async Task TurnsLegacyParsingFailureIntoFE()
var response = makeXmlMessage(xml: """<Unknown><active value="true" /></Unknown>""");
await assertIssue<FormatException>(response, "*Cannot locate type information for type 'Unknown'*", engine: ELEMENTENGINE, notmatch: "*with FHIR version 1.0.0*");
await assertIssue<StructuralTypeException>(response, "*Cannot locate type information for type 'Unknown'*" +
"Are you connected to a FHIR server with FHIR version 1.0.0*", version: "1.0.0", engine: ELEMENTENGINE);
"Are you connected to a FHIR server with FHIR version 1.0.0*", true, version: "1.0.0", engine: ELEMENTENGINE);

response = makeJsonMessage(json: """{ "resourceType": "Patient", "activex": 4 }""");
await assertIssue<StructuralTypeException>(response, "*Encountered unknown element 'activex' at location*", engine: ELEMENTENGINE);
}

private static async Task assertIssue<T>(HttpResponseMessage response, string match, string? version = null,
private static async Task assertIssue<T>(HttpResponseMessage response, string match, bool suggestVersionOnParsingError = false, string? version = null,
IFhirSerializationEngine? engine = null, string? notmatch = null)
where T : Exception
{
var result = await BaseFhirClient.ValidateResponse(response, new[] { HttpStatusCode.OK }, engine ?? POCOENGINE, version, useBinaryProtocol: false);
var result = await BaseFhirClient.ValidateResponse(response, new[] { HttpStatusCode.OK }, engine ?? POCOENGINE, suggestVersionOnParsingError, (suggestVersionOnParsingError) ? version! : "1.0.0", useBinaryProtocol: false);
await checkResult(response, result, engine ?? POCOENGINE, hasResource: false, expectedIssue: typeof(T), messagePattern: match, notMessagePattern: notmatch);
}

Expand All @@ -336,15 +336,20 @@ public async Task GetBinaryBody()
msg.SetSecurityContext("http://nu.nl");
msg.SetVersionFromETag("123");

var b = await msg.ReadBinaryDataFromMessage();
var b = await msg.ReadBinaryDataFromMessage(Specification.FhirRelease.R4);

b.Content.Should().BeEquivalentTo(data);
b.Content.Should().BeNull();
b.Data.Should().BeEquivalentTo(data);
b.SecurityContext.Reference.Should().Be("http://nu.nl");
b.ContentType.Should().Be("application/crap");
b.Meta.VersionId.Should().Be("123");
b.Meta.LastUpdated.Should().Be(when);
b.Id.Should().Be("4");

b = await msg.ReadBinaryDataFromMessage(Specification.FhirRelease.STU3);
b.Content.Should().BeEquivalentTo(data);
b.Data.Should().BeNull();

}
}
}
Expand Down

0 comments on commit 7738a9e

Please sign in to comment.