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

Move IScopedNode from SDK #228

Merged
merged 4 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
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
2 changes: 1 addition & 1 deletion firely-validator-api-tests.props
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
</PropertyGroup>

<PropertyGroup>
<FirelySdkVersion>5.4.1-20231211.2</FirelySdkVersion>
<FirelySdkVersion>5.4.1-20231212.3</FirelySdkVersion>
<LegacyValidatorVersion>5.1.0</LegacyValidatorVersion>
</PropertyGroup>

Expand Down
2 changes: 1 addition & 1 deletion firely-validator-api.props
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
</PropertyGroup>

<PropertyGroup>
<FirelySdkVersion>5.4.1-20231211.2</FirelySdkVersion>
<FirelySdkVersion>5.4.1-20231212.3</FirelySdkVersion>
</PropertyGroup>

<PropertyGroup Condition=" '$(Configuration)' == 'Debug' ">
Expand Down
12 changes: 4 additions & 8 deletions src/Firely.Fhir.Validation/Impl/FhirPathValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,12 @@ protected override (bool, ResultReport?) RunInvariant(IScopedNode input, Validat
{
try
{
#pragma warning disable CS0618 // Type or member is obsolete
var node = input as ScopedNode ?? new ScopedNode(input.AsTypedElement());
#pragma warning restore CS0618 // Type or member is obsolete
ScopedNode node = input.ToScopedNode();
var context = new FhirEvaluationContext(node.ResourceContext)
{
TerminologyService = new ValidateCodeServiceToTerminologyServiceAdapter(vc.ValidateCodeService)
};
return (predicate(input, context, vc), null);
return (predicate(node, context, vc), null);
}
catch (Exception e)
{
Expand Down Expand Up @@ -153,14 +151,12 @@ private CompiledExpression getDefaultCompiledExpression(FhirPathCompiler compile
}
}

private bool predicate(IScopedNode input, EvaluationContext context, ValidationContext vc)
private bool predicate(ScopedNode input, EvaluationContext context, ValidationContext vc)
{
var compiler = vc?.FhirPathCompiler ?? DefaultCompiler;
var compiledExpression = getDefaultCompiledExpression(compiler);

#pragma warning disable CS0618 // Type or member is obsolete
return compiledExpression.IsTrue(input.AsTypedElement(), context);
#pragma warning restore CS0618 // Type or member is obsolete
return compiledExpression.IsTrue(input, context);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,8 @@ private record ResolutionResult(ITypedElement? ReferencedResource, AggregationMo
ResolutionResult resolution = new(null, null, null);
List<ResultReport> evidence = new();

if (input is not ScopedNode instance)
throw new InvalidOperationException($"Cannot validate because input is not of type {nameof(ScopedNode)}.");

// First, try to resolve within this instance (in contained, Bundle.entry)
evidence.Add(resolveLocally(instance, reference, s, out resolution));
evidence.Add(resolveLocally(input.ToScopedNode(), reference, s, out resolution));

// Now that we have tried to fetch the reference locally, we have also determined the kind of
// reference we are dealing with, so check it for aggregation and versioning rules.
Expand Down
3 changes: 0 additions & 3 deletions src/Firely.Fhir.Validation/Schema/ValueElementNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
* via any medium is strictly prohibited.
*/

using Hl7.Fhir.ElementModel;
using Hl7.Fhir.Language;
using System.Collections.Generic;
using System.Linq;
Expand All @@ -20,8 +19,6 @@ public ValueElementNode(IScopedNode wrapped)
_wrapped = wrapped;
}

public IScopedNode? Parent => _wrapped.Parent;

public string Name => "value";

public string InstanceType => TypeSpecifier.ForNativeType(_wrapped.Value.GetType()).FullName;
Expand Down
36 changes: 36 additions & 0 deletions src/Firely.Fhir.Validation/Support/IScopedNode.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Copyright (c) 2023, Firely (info@fire.ly) and contributors
* See the file CONTRIBUTORS for details.
*
* This file is licensed under the BSD 3-Clause license
* available at https://raw.githubusercontent.com/FirelyTeam/firely-net-sdk/master/LICENSE
*/

#nullable enable

using Hl7.Fhir.ElementModel;

namespace Firely.Fhir.Validation
{
/// <summary>
/// An element within a tree of typed FHIR data with also a parent element.
/// </summary>
/// <remarks>
/// This interface represents FHIR data as a tree of elements, including type information either present in
/// the instance or derived from fully aware of the FHIR definitions and types
/// </remarks>
#pragma warning disable CS0618 // Type or member is obsolete
internal interface IScopedNode : IBaseElementNavigator<IScopedNode>
#pragma warning restore CS0618 // Type or member is obsolete
{
/*
*
/// <summary>
/// The parent node of this node, or null if this is the root node.
/// </summary>
IScopedNode? Parent { get; } // We don't need this probably.
*/
}
}

#nullable restore
57 changes: 57 additions & 0 deletions src/Firely.Fhir.Validation/Support/IScopedNodeExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright (c) 2023, Firely (info@fire.ly) and contributors
* See the file CONTRIBUTORS for details.
*
* This file is licensed under the BSD 3-Clause license
* available at https://raw.githubusercontent.com/FirelyTeam/firely-net-sdk/master/LICENSE
*/

#nullable enable


using Hl7.Fhir.ElementModel;
using System;
using System.Collections.Generic;
using System.Linq;

namespace Firely.Fhir.Validation
{
internal static class IScopedNodeExtensions
{
/// <summary>
/// Converts a <see cref="IScopedNode"/> to a <see cref="ITypedElement"/>.
/// </summary>
/// <param name="node">An <see cref="IScopedNode"/> node</param>
/// <returns>An implementation of <see cref="ITypedElement"/></returns>
/// <remarks>Be careful when using this method, the returned <see cref="ITypedElement"/> does not implement
/// the methods <see cref="ITypedElement.Location"/> and <see cref="ITypedElement.Definition"/>.
/// </remarks>
[Obsolete("WARNING! For internal API use only. Turning an IScopedNode into an ITypedElement will cause problems for" +
"Location and Definitions. Those properties are not implemented using this method and can cause problems " +
"elsewhere. Please don't use this method unless you know what you are doing.")]
public static ITypedElement AsTypedElement(this IScopedNode node) => node switch
{
TypedElementToIScopedNodeToAdapter adapter => adapter.ScopedNode,
ITypedElement ite => ite,
_ => new ScopedNodeToTypedElementAdapter(node)
};
//node is ITypedElement ite ? ite : new ScopedNodeToTypedElementAdapter(node);

public static ScopedNode ToScopedNode(this IScopedNode node) => node switch
{
TypedElementToIScopedNodeToAdapter adapter => adapter.ScopedNode,
_ => throw new ArgumentException("The node is not a TypedElementToIScopedNodeToAdapter")
};

/// <summary>
/// Returns the parent resource of this node, or null if this node is not part of a resource.
/// </summary>
/// <param name="nodes"></param>
/// <param name="name"></param>
/// <returns></returns>
public static IEnumerable<IScopedNode> Children(this IEnumerable<IScopedNode> nodes, string? name = null) =>
nodes.SelectMany(n => n.Children(name));
}
}

#nullable restore
56 changes: 56 additions & 0 deletions src/Firely.Fhir.Validation/Support/ScopedNodeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ namespace Firely.Fhir.Validation
/// </summary>
internal static class ScopedNodeExtensions
{

/// <summary>
/// Convert a <see cref="ITypedElement"/> to a <see cref="IScopedNode"/>.
/// </summary>
/// <param name="node">An <see cref="ITypedElement"/></param>
/// <returns></returns>
public static IScopedNode AsScopedNode(this ITypedElement node)
=> new TypedElementToIScopedNodeToAdapter(node.ToScopedNode());

/// <summary>
///
/// </summary>
Expand All @@ -25,6 +34,53 @@ public static IScopedNode ToScopedNode(this IReadOnlyDictionary<string, object>
inspector ??= ModelInspector.ForAssembly(node.GetType().Assembly);
return new ScopedNodeOnDictionary(inspector, node.GetType().Name, node);
}

internal static Quantity ParseQuantity(this IScopedNode instance)
#pragma warning disable CS0618 // Type or member is obsolete
=> instance.ParseQuantityInternal();
#pragma warning restore CS0618 // Type or member is obsolete

internal static T ParsePrimitive<T>(this IScopedNode instance) where T : PrimitiveType, new()
#pragma warning disable CS0618 // Type or member is obsolete
=> instance.ParsePrimitiveInternal<T, IScopedNode>();
#pragma warning restore CS0618 // Type or member is obsolete

internal static Coding ParseCoding(this IScopedNode instance)
#pragma warning disable CS0618 // Type or member is obsolete
=> instance.ParseCodingInternal();
#pragma warning restore CS0618 // Type or member is obsolete

internal static ResourceReference ParseResourceReference(this IScopedNode instance)
#pragma warning disable CS0618 // Type or member is obsolete
=> instance.ParseResourceReferenceInternal();
#pragma warning restore CS0618 // Type or member is obsolete

internal static CodeableConcept ParseCodeableConcept(this IScopedNode instance)
#pragma warning disable CS0618 // Type or member is obsolete
=> instance.ParseCodeableConceptInternal();
#pragma warning restore CS0618 // Type or member is obsolete

/// <summary>
/// Parses a bindeable type (code, Coding, CodeableConcept, Quantity, string, uri) into a FHIR coded datatype.
/// Extensions will be parsed from the 'value' of the (simple) extension.
/// </summary>
/// <param name="instance"></param>
/// <returns>An Element of a coded type (code, Coding or CodeableConcept) dependin on the instance type,
/// or null if no bindable instance data was found</returns>
/// <remarks>The instance type is mapped to a codable type as follows:
/// 'code' => code
/// 'Coding' => Coding
/// 'CodeableConcept' => CodeableConcept
/// 'Quantity' => Coding
/// 'Extension' => depends on value[x]
/// 'string' => code
/// 'uri' => code
/// </remarks>
internal static Element ParseBindable(this IScopedNode instance)
#pragma warning disable CS0618 // Type or member is obsolete
=> instance.ParseBindableInternal();
#pragma warning restore CS0618 // Type or member is obsolete

}

internal class ScopedNodeOnDictionary : IScopedNode
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright (c) 2023, Firely (info@fire.ly) and contributors
* See the file CONTRIBUTORS for details.
*
* This file is licensed under the BSD 3-Clause license
* available at https://raw.githubusercontent.com/FirelyTeam/firely-net-sdk/master/LICENSE
*/

#nullable enable

using Hl7.Fhir.ElementModel;
using Hl7.Fhir.Specification;
using System.Collections.Generic;
using System.Linq;

namespace Firely.Fhir.Validation
{
/// <summary>
/// An adapter from <see cref="IScopedNode"/> to <see cref="ITypedElement"/>.
/// </summary>
/// <remarks>Be careful, this adapter does not implement the <see cref="ITypedElement.Location"/> and
/// <see cref="ITypedElement.Definition"/> property.
/// </remarks>
internal class ScopedNodeToTypedElementAdapter : ITypedElement
{
private readonly IScopedNode _adaptee;

public ScopedNodeToTypedElementAdapter(IScopedNode adaptee)
{
_adaptee = adaptee;
}

public string Location => throw new System.NotImplementedException();

public IElementDefinitionSummary Definition => throw new System.NotImplementedException();

public string Name => _adaptee.Name;

public string InstanceType => _adaptee.InstanceType;

public object Value => _adaptee.Value;

public IEnumerable<ITypedElement> Children(string? name = null) =>
_adaptee.Children(name).Select(n => new ScopedNodeToTypedElementAdapter(n));
Copy link
Member

Choose a reason for hiding this comment

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

If _adaptee is a ScopedNode, we don't need to wrap it I think, as ScopedNode already implements ITypedElement?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes we do have to cast it, because the constructor of TypedElementToIScopedNodeToAdapter needs a ScopedNode as an argument. When I skip the cast, the constructor will not work anymore.
Why not make the _adaptee of type ITypedElement you would say? When we do that we have to do the cast at the property public ScopedNode ScopedNode instead. So I chose for the casting at the Children() function.

Copy link
Member

Choose a reason for hiding this comment

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

But it is not a cast, we are creating lots of new objects, which I know by testing both increases GC load and slows down validation - so I want to make sure we really need this!

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I change the class a bit. The field _adaptee is now of type ITypedElement and we have a private constructor that takes an ITypedElement. Doing so, we don't need the cast in children anymore. The cast moved to property ScopedNode.

Wrapping is still needed because this adapter implements IScopedNode. ScopedNode implements ITypedElement, but not IScopedNode.

Copy link
Member Author

Choose a reason for hiding this comment

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

I misread your comment. This about the ScopedNodeToTypedElementAdapter. I was talking about TypedElementToIScopedNodeToAdapter.

About ScopedNodeToTypedElementAdapter:

This adapter is only used by the extension method AsTypedElement. Looking at the implementation of this method we see:

public static ITypedElement AsTypedElement(this IScopedNode node) => node switch
{
    TypedElementToIScopedNodeToAdapter adapter => adapter.ScopedNode,
    ITypedElement ite => ite,
    _ => new ScopedNodeToTypedElementAdapter(node)
};

The first 2 cases already bypass the adapter. Only in rare cases the adapter is used. In fact, I ran all the tests and the adapter is never used!

}
}

#nullable restore
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
using Hl7.Fhir.ElementModel;
using System.Collections.Generic;
using System.Linq;

namespace Firely.Fhir.Validation
{
internal class TypedElementToIScopedNodeToAdapter : IScopedNode
{
private readonly ScopedNode _adaptee;

public TypedElementToIScopedNodeToAdapter(ScopedNode adaptee)
{
_adaptee = adaptee;
}

public ScopedNode ScopedNode => _adaptee;

public string Name => _adaptee.Name;

public string InstanceType => _adaptee.InstanceType;

public object Value => _adaptee.Value;

IEnumerable<IScopedNode> IBaseElementNavigator<IScopedNode>.Children(string? name) =>
_adaptee.Children(name).Cast<ScopedNode>().Select(n => new TypedElementToIScopedNodeToAdapter(n));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public void AvoidsRedoingProfileValidation()
vc.ResolveExternalReference = resolveTestData;

var validationState = new ValidationState();
var result = schemaElement!.Validate(new ScopedNode(all.ToTypedElement()), vc, validationState);
var result = schemaElement!.Validate(new ScopedNode(all.ToTypedElement()).AsScopedNode(), vc, validationState);
result.Result.Should().Be(ValidationResult.Failure);
var issues = result.Evidence.OfType<IssueAssertion>().ToList();
issues.Count.Should().Be(1); // Bundle.entry[2].resource[0] is validated twice against different profiles.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public void ValidateInstance(object instance, object testeeo, bool success, stri

static ResultReport test(object instance, IAssertion testee, ValidationContext vc)
{
var te = new ScopedNode(instance.ToTypedElement());
var te = instance.ToTypedElement().AsScopedNode();
var asserter = te.Children("entry").First().Children("resource").Children("asserter").Single();
return testee.Validate(asserter, vc);
}
Expand Down
Loading