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

#2910 Make Base implement IScopedNode - phase 1 #2913

Open
wants to merge 30 commits into
base: develop-6.0
Choose a base branch
from

Conversation

Kasdejong
Copy link
Contributor

@Kasdejong Kasdejong commented Oct 16, 2024

Description

This first step makes POCO's implement ITypedElement. Since we no longer supply an ITypedElement.Definition, we had to patch some of the parsers to get all unit-tests back to work. In the end, these changes to the parsers will be reverted, they are just made to be able to continue testing with our extensive set of tests on ITypedElement.

Related issues

#2894, #2895

Testing

Tested by making Base.ToTypedElement() point to this implementation. This is temporary.

@ewoutkramer ewoutkramer changed the title Spike/make resource implement iscopednode #2910 Make resource implement iscopednode Oct 16, 2024
@ewoutkramer ewoutkramer changed the title #2910 Make resource implement iscopednode #2910 Make resource implement IScopednode - phase 1 Oct 16, 2024
@ewoutkramer ewoutkramer changed the title #2910 Make resource implement IScopednode - phase 1 #2910 Make Base implement IScopednode - phase 1 Oct 16, 2024
@ewoutkramer ewoutkramer changed the title #2910 Make Base implement IScopednode - phase 1 #2910 Make Base implement IScopedNode - phase 1 Oct 16, 2024
@Kasdejong Kasdejong changed the base branch from develop to develop-6.0 October 16, 2024 15:08
@Kasdejong Kasdejong marked this pull request as ready for review October 16, 2024 15:10
src/Hl7.Fhir.Base/ElementModel/PocoElementNode.cs Outdated Show resolved Hide resolved
src/Hl7.Fhir.Base/FhirPath/CompiledExpression.cs Outdated Show resolved Hide resolved
src/Hl7.Fhir.Base/FhirPath/Expressions/Closure.cs Outdated Show resolved Hide resolved
src/Hl7.Fhir.Base/FhirPath/FhirPathCompiler.cs Outdated Show resolved Hide resolved
src/Hl7.Fhir.Base/Model/Base.TypedElement.cs Show resolved Hide resolved
@@ -15,7 +15,7 @@ namespace Hl7.Fhir.ElementModel
public static class TypedElementExtensions
{
public static ITypedElement ToTypedElement(this Base @base, string? rootName = null)
=> @base.ToTypedElement(ModelInfo.ModelInspector, rootName);
=> @base.WithScopeInfo(@base.BuildRoot(rootName));
Copy link
Member

Choose a reason for hiding this comment

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

Flag for removal after spike.

src/firely-net-sdk.props Outdated Show resolved Hide resolved
@Kasdejong
Copy link
Contributor Author

Serialization is still binary breaking, but not source breaking. It now takes Base instead of Resource

…node

# Conflicts:
#	src/Hl7.Fhir.Base/CompatibilitySuppressions.xml
#	src/Hl7.Fhir.Base/Model/Base.cs
@@ -1,12 +1,334 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- https://learn.microsoft.com/en-us/dotnet/fundamentals/package-validation/diagnostic-ids -->
<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
<Suppression>
Copy link
Member

Choose a reason for hiding this comment

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

Make sure you add info about the breaking changes to the PR, including alternatives that people can use.

@@ -90,7 +90,7 @@ public static Quantity ParseQuantity(this ITypedElement instance)

[Obsolete("WARNING! Intended for internal API usage exclusively, interface IBaseElementNavigator can be changed in " +
"the near future.")]
public static Quantity ParseQuantityInternal<T>(this IBaseElementNavigator<T> instance) where T : IBaseElementNavigator<T>
public static Quantity ParseQuantityInternal(this ITypedElement instance)
Copy link
Member

Choose a reason for hiding this comment

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

The introduction of IBaseElementNavigator made it necessary to make these public, I think they can be internal again - in any case the reference to IBaseElementNavigator in the Obsolete should be changed!

// Wellioht is er ook nog iets met de directe properties "Extension.url" en "Element.id" die van een
// system type zijn ipv een FHIR type.
string? ITypedElement.InstanceType =>
((IStructureDefinitionSummary)
Copy link
Member

Choose a reason for hiding this comment

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

We should not need a ModelInspector anymore.

@@ -73,7 +73,7 @@ public static class SerializationEngineExtensions
/// </summary>
/// <exception cref="InvalidOperationException">Thrown if the underlying engine is a legacy engine</exception>
/// <exception cref="DeserializationFailedException">Thrown if a FHIR error was encountered in the data</exception>
public static Resource? SerializeReaderToXml(this IFhirSerializationEngine engine, XmlReader reader)
public static Base? SerializeReaderToXml(this IFhirSerializationEngine engine, XmlReader reader)
Copy link
Member

Choose a reason for hiding this comment

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

Ok, so a breaking change, make sure to put it in the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants