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.

Sign up for GitHub

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

Jump to bottom

Fix expected result message in FhirClient #2788

Merged
merged 2 commits into from
May 23, 2024
Merged

Fix expected result message in FhirClient #2788

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a449d6
fix expected result message in FhirClient
mmsmits May 22, 2024
24c5431
Merge branch 'develop' into bugfix/2772-HttpResponse_result_does_not_…
mmsmits May 22, 2024
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
113 changes: 56 additions & 57 deletions src/Hl7.Fhir.Base/Rest/BaseFhirClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
using Hl7.Fhir.Utility;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Net;
using System.Net.Http;
Expand All @@ -28,15 +27,15 @@ namespace Hl7.Fhir.Rest;
public partial class BaseFhirClient : IDisposable
{
#region Properties

private readonly Lazy<List<HttpStatusCode>> _200Responses = new(() => Enum.GetValues(typeof(HttpStatusCode)).Cast<HttpStatusCode>().Where(n => (int)n > 199 && (int)n < 300).ToList());

private string fhirVersion => Settings?.ExplicitFhirVersion ?? Inspector.FhirVersion ??
throw new ArgumentException("The FHIR version to use cannot be derived from the assembly metadata, " +
$"use {nameof(FhirClientSettings)}.{nameof(FhirClientSettings.ExplicitFhirVersion)} instead.");

internal readonly ModelInspector Inspector;

internal HttpClientRequester Requester { get; init; }

/// <summary>
Expand Down Expand Up @@ -75,11 +74,11 @@ public Uri Endpoint
/// The body returned by the last http request as a FHIR resource (or null if the body did not have a FHIR payload).
/// </summary>
public virtual Resource? LastBodyAsResource { get; private set; }

#endregion

#region Constructors

/// <summary>
/// Creates a new client using a default endpoint
/// If the endpoint does not end with a slash (/), it will be added.
Expand Down Expand Up @@ -139,7 +138,7 @@ public BaseFhirClient(Uri endpoint, ModelInspector inspector, FhirClientSettings
{
// nothing
}

#endregion

#region Read
Expand Down Expand Up @@ -274,7 +273,7 @@ public BaseFhirClient(Uri endpoint, ModelInspector inspector, FhirClientSettings

return internalUpdateAsync(resource, upd.ToBundle(), ct);
}

private Task<TResource?> internalUpdateAsync<TResource>(TResource resource, Bundle tx, CancellationToken? ct) where TResource : Resource
{
resource.ResourceBase = Endpoint;
Expand All @@ -287,7 +286,7 @@ public BaseFhirClient(Uri endpoint, ModelInspector inspector, FhirClientSettings
{
return internalUpdateAsync(resource, tx, ct).WaitResult();
}

#endregion

#region Delete
Expand Down Expand Up @@ -335,7 +334,7 @@ public virtual async Task DeleteAsync(Resource resource, CancellationToken? ct =

await DeleteAsync(resource.ResourceIdentity(Endpoint).WithoutVersion(), ct).ConfigureAwait(false);
}

/// <summary>
/// Conditionally delete a single resource
/// </summary>
Expand All @@ -348,7 +347,7 @@ public virtual async Task ConditionalDeleteSingleAsync(SearchParams condition, s
var tx = new TransactionBuilder(Endpoint).ConditionalDeleteSingle(condition, resourceType, versionId).ToBundle();
await executeAsync<Resource>(tx, new[] { HttpStatusCode.OK, HttpStatusCode.NoContent }, ct).ConfigureAwait(false);
}

/// <summary>
/// Conditionally delete multiple resources
/// </summary>
Expand Down Expand Up @@ -458,7 +457,7 @@ public virtual async Task ConditionalDeleteMultipleAsync(SearchParams condition,
var url = new RestUrl(Endpoint).AddPath(resourceType, id);

var request = new HttpRequestMessage(new("PATCH"), url.Uri).WithFormatParameter(format);

request.Content = new StringContent(patchDocument);
request.Content.Headers.ContentType = new MediaTypeHeaderValue(format switch
{
Expand Down Expand Up @@ -583,9 +582,9 @@ public virtual async Task ConditionalDeleteMultipleAsync(SearchParams condition,
}

#endregion

#region DeleteHistory

/// <summary>
/// Delete a resource's history (all historic versions except current)
/// </summary>
Expand All @@ -600,7 +599,7 @@ public async Task DeleteHistoryAsync(Uri location, CancellationToken? ct = null)

await executeAsync<Resource>(tx, new[] { HttpStatusCode.OK, HttpStatusCode.NoContent }, ct).ConfigureAwait(false);
}

/// <summary>
/// Delete a resource's history (all historic versions except current)
/// </summary>
Expand All @@ -626,7 +625,7 @@ public async Task DeleteHistoryVersionAsync(Uri location, CancellationToken? ct

await executeAsync<Resource>(tx, new[] { HttpStatusCode.OK, HttpStatusCode.NoContent }, ct).ConfigureAwait(false);
}

/// <summary>
/// Delete a specific historical version of a resource
/// </summary>
Expand Down Expand Up @@ -666,7 +665,7 @@ public async Task DeleteHistoryVersionAsync(string location, CancellationToken?
if (operationName == null) throw Error.ArgumentNull(nameof(operationName));
return internalOperationAsync(operationName, parameters: parameters, useGet: useGet, ct: ct);
}

public virtual Task<Resource?> TypeOperationAsync<TResource>(string operationName, Parameters? parameters = null, bool useGet = false, CancellationToken? ct = null)
where TResource : Resource
{
Expand All @@ -675,7 +674,7 @@ public async Task DeleteHistoryVersionAsync(string location, CancellationToken?

return TypeOperationAsync(operationName, typeName, parameters, useGet: useGet, ct);
}

public virtual Task<Resource?> TypeOperationAsync(string operationName, string typeName, Parameters? parameters = null, bool useGet = false, CancellationToken? ct = null)
{
if (operationName == null) throw Error.ArgumentNull(nameof(operationName));
Expand Down Expand Up @@ -751,7 +750,7 @@ public async Task DeleteHistoryVersionAsync(string location, CancellationToken?
#endregion

#region Get

/// <summary>
/// Invoke a general GET on the server. If the operation fails, then this method will throw an exception
/// </summary>
Expand Down Expand Up @@ -789,7 +788,7 @@ public async Task DeleteHistoryVersionAsync(string location, CancellationToken?
{
return executeAsync<TResource>(tx, new[] { expect }, ct);
}

private async Task<TResource?> executeAsync<TResource>(Bundle tx, IEnumerable<HttpStatusCode> expect, CancellationToken? ct) where TResource : Resource
{
var cancellation = ct ?? CancellationToken.None;
Expand All @@ -804,7 +803,7 @@ public async Task DeleteHistoryVersionAsync(string location, CancellationToken?
Requester.BaseUrl,
getSerializationEngine(),
Settings.UseFhirVersionInAcceptHeader ? fhirVersion : null,
Settings,
Settings,
maybeBinaryInteraction);

using var responseMessage = await Requester.ExecuteAsync(requestMessage, cancellation).ConfigureAwait(false);
Expand All @@ -817,7 +816,7 @@ public async Task DeleteHistoryVersionAsync(string location, CancellationToken?
var cancellation = ct ?? CancellationToken.None;

cancellation.ThrowIfCancellationRequested();

using var responseMessage = await Requester.ExecuteAsync(request, cancellation).ConfigureAwait(false);

return await extractResourceFromHttpResponse<TResource>(expect, responseMessage, request);
Expand All @@ -826,14 +825,14 @@ public async Task DeleteHistoryVersionAsync(string location, CancellationToken?
#endregion

#region Utilities

// Create our own and add decompression strategy in default handler.
private static HttpClientHandler makeDefaultHandler() =>
new()
{
AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate
};

private static Uri getValidatedEndpoint(Uri endpoint)
{
if (endpoint == null) throw new ArgumentNullException(nameof(endpoint));
Expand All @@ -854,7 +853,7 @@ private static ResourceIdentity verifyResourceIdentity(Uri location, bool needId

return result;
}

// either msg or entryComponent should be set
private async Task<TResource?> extractResourceFromHttpResponse<TResource>(IEnumerable<HttpStatusCode> expect, HttpResponseMessage responseMessage, HttpRequestMessage? msg = null, Bundle.EntryComponent? entryComponent = null, bool useBinaryProtocol = false) where TResource : Resource
{
Expand Down Expand Up @@ -905,34 +904,34 @@ await ValidateResponse(responseMessage, expect, getSerializationEngine(), sugges
null => null,

// Unexpected response type in the body, throw.
_ => throw new FhirOperationException(entryComponent is not null ? unexpectedBodyTypeForBundle(entryComponent.Request) : unexpectedBodyTypeForMessage(msg!), responseMessage.StatusCode)
_ => throw new FhirOperationException(entryComponent is not null ? unexpectedBodyTypeForBundle(entryComponent.Request, execResult) : unexpectedBodyTypeForMessage(msg!, execResult), responseMessage.StatusCode)
};
static string unexpectedBodyTypeForBundle(Bundle.RequestComponent rc) => $"Operation {rc.Method} on {rc.Url} " +
$"expected a body of type {typeof(TResource).Name} but a {typeof(TResource).Name} was returned.";
static string unexpectedBodyTypeForMessage(HttpRequestMessage msg) => $"Operation {msg.Method} on {msg.RequestUri} " +
$"expected a body of type {typeof(TResource).Name} but a {typeof(TResource).Name} was returned.";

static string unexpectedBodyTypeForBundle(Bundle.RequestComponent rc, Resource result) => $"Operation {rc.Method} on {rc.Url} " +
$"expected a body of type {typeof(TResource).Name} but a {result.GetType().Name} was returned.";

static string unexpectedBodyTypeForMessage(HttpRequestMessage msg, Resource result) => $"Operation {msg.Method} on {msg.RequestUri} " +
$"expected a body of type {typeof(TResource).Name} but a {result.GetType().Name} was returned.";
}

/// <summary>
/// Validates the <see cref="HttpResponseMessage"/> and throws the appropriate exceptions.
/// It also simulates the exception-throwing behaviour of the original TypedElement-based parsers.
/// </summary>
/// <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)"/>
internal static async Task<ResponseData> ValidateResponse(
HttpResponseMessage responseMessage,
IEnumerable<HttpStatusCode> expect,
IFhirSerializationEngine engine,
string? suggestedVersionOnParseError,
bool useBinaryProtocol)
{
var responseData = (await responseMessage.ExtractResponseData(engine, useBinaryProtocol).ConfigureAwait(false))
.TranslateUnsupportedBodyTypeException(responseMessage.StatusCode)
.TranslateLegacyParserException(suggestedVersionOnParseError);
/// <summary>
/// Validates the <see cref="HttpResponseMessage"/> and throws the appropriate exceptions.
/// It also simulates the exception-throwing behaviour of the original TypedElement-based parsers.
/// </summary>
/// <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)"/>
internal static async Task<ResponseData> ValidateResponse(
HttpResponseMessage responseMessage,
IEnumerable<HttpStatusCode> expect,
IFhirSerializationEngine engine,
string? suggestedVersionOnParseError,
bool useBinaryProtocol)
{
var responseData = (await responseMessage.ExtractResponseData(engine, useBinaryProtocol).ConfigureAwait(false))
.TranslateUnsupportedBodyTypeException(responseMessage.StatusCode)
.TranslateLegacyParserException(suggestedVersionOnParseError);

// If extracting the data caused an issue, return it immediately
if (responseData.Issue is not null)
Expand Down Expand Up @@ -961,13 +960,13 @@ internal static async Task<ResponseData> ValidateResponse(

private static bool isPostOrPutOrPatch(Bundle.EntryComponent interaction) =>
interaction.Request.Method is Bundle.HTTPVerb.POST or Bundle.HTTPVerb.PUT or Bundle.HTTPVerb.PATCH;

private static bool isPostOrPutOrPatch(HttpMethod method) =>
method == HttpMethod.Post || method == HttpMethod.Put || method == new HttpMethod("PATCH");

private bool _versionChecked = false;


private IFhirSerializationEngine getSerializationEngine()
{
return Settings.SerializationEngine ?? FhirSerializationEngineFactory.Legacy.FromParserSettings(Inspector, Settings.ParserSettings ?? new());
Expand Down Expand Up @@ -1024,9 +1023,9 @@ private async Task verifyServerVersion(CancellationToken ct)

private string typeNameOrDie<TResource>() => Inspector.GetFhirTypeNameForType(typeof(TResource)) ??
throw new ArgumentException($"Type parameter {nameof(TResource)} is not a known resource.");



#endregion

#region IDisposable Support
Expand Down