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

Address issue #184, observations not being updated when changed #185

Merged
merged 3 commits into from
Apr 18, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -15,45 +15,56 @@

namespace Microsoft.Health.Extensions.Fhir
{
public static class ServiceCollectionExtensions
public static class FhirClientExtensions
{
public static void AddFhirClient(this IServiceCollection serviceCollection, IConfiguration configuration)
public static IServiceCollection AddFhirClient(this IServiceCollection serviceCollection, IConfiguration configuration)
{
EnsureArg.IsNotNull(serviceCollection, nameof(serviceCollection));
EnsureArg.IsNotNull(configuration, nameof(configuration));

var url = new Uri(configuration.GetValue<string>("FhirService:Url"));
bool useManagedIdentity = configuration.GetValue<bool>("FhirClient:UseManagedIdentity");

serviceCollection.AddSingleton(typeof(ITelemetryLogger), typeof(IomtTelemetryLogger));
var serviceProvider = serviceCollection.BuildServiceProvider();
var logger = serviceProvider.GetRequiredService<ITelemetryLogger>();

serviceCollection.AddHttpClient<IFhirClient, FhirClient>(client =>
serviceCollection.AddHttpClient<IFhirClient, FhirClient>((client, sp) =>
{
client.BaseAddress = url;
client.Timeout = TimeSpan.FromSeconds(60);

var logger = sp.GetRequiredService<ITelemetryLogger>();

// Using discard because we don't need result
var fhirClient = new FhirClient(client);
_ = fhirClient.ValidateFhirClientAsync(logger);

return fhirClient;
})
.AddAuthenticationHandler(serviceCollection, logger, url, useManagedIdentity);
}

public static void AddNamedManagedIdentityCredentialProvider(this IServiceCollection serviceCollection)
{
EnsureArg.IsNotNull(serviceCollection, nameof(serviceCollection));
.AddAuthenticationHandler(serviceCollection, url, useManagedIdentity);

serviceCollection.TryAddSingleton<ManagedIdentityAuthService>();
return serviceCollection;
}

public static void AddNamedOAuth2ClientCredentialProvider(this IServiceCollection serviceCollection)
public static void AddAuthenticationHandler(
this IHttpClientBuilder httpClientBuilder,
IServiceCollection services,
Uri uri,
bool useManagedIdentity)
{
EnsureArg.IsNotNull(serviceCollection, nameof(serviceCollection));
EnsureArg.IsNotNull(httpClientBuilder, nameof(httpClientBuilder));
EnsureArg.IsNotNull(services, nameof(services));
EnsureArg.IsNotNull(uri, nameof(uri));

serviceCollection.TryAddSingleton<OAuthConfidentialClientAuthService>();
if (useManagedIdentity)
{
services.TryAddSingleton(new ManagedIdentityAuthService());
httpClientBuilder.AddHttpMessageHandler(sp =>
new BearerTokenAuthorizationMessageHandler(uri, sp.GetRequiredService<ManagedIdentityAuthService>(), sp.GetRequiredService<ITelemetryLogger>()));
}
else
{
services.TryAddSingleton(new OAuthConfidentialClientAuthService());
httpClientBuilder.AddHttpMessageHandler(sp =>
new BearerTokenAuthorizationMessageHandler(uri, sp.GetRequiredService<OAuthConfidentialClientAuthService>(), sp.GetRequiredService<ITelemetryLogger>()));
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

namespace Microsoft.Health.Extensions.Fhir
{
public static class FhirClientValidator
public static class FhirClientValidatorExtensions
{
public static async Task<bool> ValidateFhirClientAsync(
this IFhirClient client,
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

using EnsureThat;
using Hl7.Fhir.Model;

namespace Microsoft.Health.Extensions.Fhir
{
public static class ObservationExtensions
{
/// <summary>
/// Compares two observations to see if they are different. If they are different the <paramref name="updatedObservation"/> status is changed to Amended.
/// </summary>
/// <param name="originalObservation">The original unmodified observation.</param>
/// <param name="updatedObservation">The potentially modified observation.</param>
/// <returns>Returns true if the <paramref name="updatedObservation"/> is different than the <paramref name="originalObservation"/>. Otherwise false is returned.</returns>
public static bool AmendIfChanged(this Observation originalObservation, Observation updatedObservation)
{
EnsureArg.IsNotNull(originalObservation, nameof(originalObservation));
EnsureArg.IsNotNull(updatedObservation, nameof(updatedObservation));
EnsureArg.IsFalse(originalObservation == updatedObservation, optsFn: o => o.WithMessage($"Parameters {nameof(originalObservation)} and {nameof(updatedObservation)} are the same reference."));

if (!originalObservation.IsExactly(updatedObservation))
{
updatedObservation.Status = ObservationStatus.Amended;
return true;
}

return false;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

using EnsureThat;
using Hl7.Fhir.Model;

namespace Microsoft.Health.Extensions.Fhir
{
public static class ResourceExtensions
{
/// <summary>
/// Performs full deep copy of the resource.
/// </summary>
/// <typeparam name="TResource">Type of resource to return.</typeparam>
/// <param name="resource">Resource to copy.</param>
/// <returns>New resource object with the contents of the original.</returns>
public static TResource FullCopy<TResource>(this TResource resource)
Copy link
Contributor

Choose a reason for hiding this comment

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

I only see this referenced in unit tests. Is that intended?

Copy link
Member Author

Choose a reason for hiding this comment

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

Covered in other comment but yes for now it is due to project compatibility issues.

where TResource : class, IDeepCopyable
{
EnsureArg.IsNotNull(resource, nameof(resource));

return resource.DeepCopy() as TResource;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,42 +128,42 @@ protected override Observation MergeObservationImpl(CodeValueFhirTemplate templa
EnsureArg.IsNotNull(grp, nameof(grp));
EnsureArg.IsNotNull(existingObservation, nameof(existingObservation));

existingObservation.Status = ObservationStatus.Amended;
var mergedObservation = existingObservation.DeepCopy() as Observation;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be using the new extension method?:

var mergedObservation = existingObservation.FullCopy()

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately we can't right now. This project is .NET 2.0 standard where as the Extenstions,Fhir.R4 project is .NET 6.0 and can't be downgraded due to dependencies. This project needs to stay .NET Standard till we can move our RP to .NET 6.0.

I could have introduced a completely separate project just for this method but it seemed like overkill.


existingObservation.Category = null;
mergedObservation.Category = null;
if (template?.Category?.Count > 0)
{
existingObservation.Category = ResolveCategory(template.Category);
mergedObservation.Category = ResolveCategory(template.Category);
}

var values = grp.GetValues();
(DateTime start, DateTime end) observationPeriod = GetObservationPeriod(existingObservation);
(DateTime start, DateTime end) observationPeriod = GetObservationPeriod(mergedObservation);

// Update observation value
if (!string.IsNullOrWhiteSpace(template?.Value?.ValueName) && values.TryGetValue(template?.Value?.ValueName, out var obValues))
{
existingObservation.Value = _valueProcessor.MergeValue(template.Value, CreateMergeData(grp.Boundary, observationPeriod, obValues), existingObservation.Value);
mergedObservation.Value = _valueProcessor.MergeValue(template.Value, CreateMergeData(grp.Boundary, observationPeriod, obValues), mergedObservation.Value);
}

// Update observation component values
if (template?.Components?.Count > 0)
{
if (existingObservation.Component == null)
if (mergedObservation.Component == null)
{
existingObservation.Component = new List<Observation.ComponentComponent>(template.Components.Count);
mergedObservation.Component = new List<Observation.ComponentComponent>(template.Components.Count);
}

foreach (var component in template.Components)
{
if (values.TryGetValue(component.Value.ValueName, out var compValues))
{
var foundComponent = existingObservation.Component
var foundComponent = mergedObservation.Component
.Where(c => c.Code.Coding.Any(code => code.Code == component.Value.ValueName && code.System == FhirImportService.ServiceSystem))
.FirstOrDefault();

if (foundComponent == null)
{
existingObservation.Component.Add(
mergedObservation.Component.Add(
new Observation.ComponentComponent
{
Code = ResolveCode(component.Value.ValueName, component.Codes),
Expand All @@ -189,9 +189,9 @@ protected override Observation MergeObservationImpl(CodeValueFhirTemplate templa
observationPeriod.end = grp.Boundary.End;
}

existingObservation.Effective = observationPeriod.ToPeriod();
mergedObservation.Effective = observationPeriod.ToPeriod();

return existingObservation;
return mergedObservation;
}

protected override IEnumerable<IObservationGroup> CreateObservationGroupsImpl(CodeValueFhirTemplate template, IMeasurementGroup measurementGroup)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ public virtual async Task<string> SaveObservationAsync(ILookupTemplate<IFhirTemp
// Merge the new data with the existing Observation.
var mergedObservation = MergeObservation(config, existingObservation, observationGroup);

// Check to see if there are any changes after merging.
if (mergedObservation.IsExactly(existingObservation))
// Check to see if there are any changes after merging and update the Status to ammended if changed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit. Typo in amend

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, fixed in latest version.

if (!existingObservation.AmendIfChanged(mergedObservation))
{
// There are no changes to the Observation - Do not update.
return (existingObservation, ResourceOperation.NoOperation);
Expand Down
Loading