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

[Validator] Also validate the instance type when the definition has children #2236

Merged
merged 4 commits into from
Sep 14, 2022
Merged

[Validator] Also validate the instance type when the definition has children #2236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8138441
fix: validator did not validate the instance type when the definition…
mmsmits Sep 13, 2022
0f78701
Merge branch 'develop-stu3' into bugfix/2183-also-validate-parent-typ…
mmsmits Sep 13, 2022
82fa665
move null check on instanceType to still allow validation on custom r…
mmsmits Sep 14, 2022
172ba57
Adjusted error message
mmsmits Sep 14, 2022
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
77 changes: 76 additions & 1 deletion src/Hl7.Fhir.Specification.Tests/Validation/IssuesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Hl7.Fhir.Model;
using FluentAssertions;
using Hl7.Fhir.Model;
using Hl7.Fhir.Serialization;
using Hl7.Fhir.Specification.Source;
using Hl7.Fhir.Validation;
Expand Down Expand Up @@ -30,5 +31,79 @@ public async Tasks.Task Issue474StartdateIs0001_01_01()
var report = validator.Validate(pat);
Assert.IsTrue(report.Success);
}


//Test for issue #2183 on Github, type check didn't fail because definition has children.
[TestMethod]
public void Issue2183TypeCheck()
{

var observationDef = new StructureDefinition()
{
Url = "http://fire.ly/fhir/Sd/observationQuantity",
Name = "observationQuantity",
Type = "Observation",
Status = PublicationStatus.Draft,
Kind = StructureDefinition.StructureDefinitionKind.Resource,
FhirVersion = "3.0.2",
Derivation = StructureDefinition.TypeDerivationRule.Constraint,
BaseDefinition = "http://hl7.org/fhir/StructureDefinition/Observation",
Abstract = false,
Differential = new()
{
Element = new()
{
new()
{
Path = "Observation.valueQuantity",
SliceName = "valueQuantity",
Type = new()
{
new()
{
Code = "Quantity"
}
}
},
new()
{
Path = "Observation.valueQuantity.code",
Fixed = new Code("mm[Hg]")
}
}
}
};

var observation = new Observation
{
Meta = new()
{
Profile = new[] { "http://fire.ly/fhir/Sd/observationQuantity" }
},
Status = ObservationStatus.Unknown,
Code = new()
{
Text = "test"
},
Value = new FhirString("foo")
};

var settings = new ValidationSettings()
{
ResourceResolver = new MultiResolver(new InMemoryProfileResolver(observationDef), ZipSource.CreateValidationSource()),
GenerateSnapshot = true,
GenerateSnapshotSettings = new()
{
GenerateSnapshotForExternalProfiles = true,
ForceRegenerateSnapshots = true
}
};

var validator = new Validator(settings);
var outcome = validator.Validate(observation);

outcome.Success.Should().Be(false);
}
}
}

2 changes: 1 addition & 1 deletion src/Hl7.Fhir.Specification/Validation/ChildConstraintValidationExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ internal static OperationOutcome ValidateChildConstraints(this Validator validat
validator.Trace(outcome, "Start validation of inlined child constraints for '{0}'".FormatWith(definition.Path), Issue.PROCESSING_PROGRESS, instance);

// validate the type on the parent of children. If this is a reference type, it will follow that reference as well
outcome.Add(validator.ValidateTypeReferences(definition.Current.Type, instance, state, validateProfiles: false));
outcome.Add(validator.ValidateType(definition.Current, instance, state, validateProfiles: false));

var matchResult = ChildNameMatcher.Match(definition, instance);

Expand Down
53 changes: 34 additions & 19 deletions src/Hl7.Fhir.Specification/Validation/TypeRefValidationExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace Hl7.Fhir.Validation
{
internal static class TypeRefValidationExtensions
{
internal static OperationOutcome ValidateType(this Validator validator, ElementDefinition definition, ScopedNode instance, ValidationState state)
internal static OperationOutcome ValidateType(this Validator validator, ElementDefinition definition, ScopedNode instance, ValidationState state, bool validateProfiles)
{
var outcome = new OperationOutcome();

Expand All @@ -32,46 +32,61 @@ internal static OperationOutcome ValidateType(this Validator validator, ElementD
var typeRefs = definition.Type.Where(tr => tr.Code != null);
var choices = typeRefs.Select(tr => tr.Code).Distinct();

if (choices.Count() > 1)

if (instance.InstanceType != null)
{
if (instance.InstanceType != null)
//find out what type is present in the instance data
// (e.g. deceased[Boolean], or _resourceType in json). This is exposed by IElementNavigator.TypeName.
var instanceType = ModelInfo.FhirTypeNameToFhirType(instance.InstanceType);

if (choices.Count() > 1)
{
// This is a choice type, find out what type is present in the instance data
// (e.g. deceased[Boolean], or _resourceType in json). This is exposed by IElementNavigator.TypeName.
var instanceType = ModelInfo.FhirTypeNameToFhirType(instance.InstanceType);
if (instanceType != null)
if (instanceType is not null)
{
// In fact, the next statements are just an optimalization, without them, we would do an ANY validation
// The next statements are just an optimalization, without them, we would do an ANY validation
// against *all* choices, what we do here is pre-filtering for sensible choices, and report if there isn't
// any.
var applicableChoices = typeRefs.Where(tr => !tr.Code.StartsWith("http:"))
.Where(tr => ModelInfo.IsInstanceTypeFor(ModelInfo.FhirTypeNameToFhirType(tr.Code).Value,
instanceType.Value));
.Where(tr => ModelInfo.IsInstanceTypeFor(ModelInfo.FhirTypeNameToFhirType(tr.Code).Value, instanceType.Value));

// Instance typename must be one of the applicable types in the choice
if (applicableChoices.Any())
{
outcome.Include(validator.ValidateTypeReferences(applicableChoices, instance, state));
outcome.Include(validator.ValidateTypeReferences(applicableChoices, instance, state, validateProfiles));
}
else
{
var choiceList = String.Join(",", choices.Select(t => "'" + t + "'"));
validator.Trace(outcome, $"Type specified in the instance ('{instance.InstanceType}') is not one of the allowed choices ({choiceList})",
Issue.CONTENT_ELEMENT_HAS_INCORRECT_TYPE, instance);
Issue.CONTENT_ELEMENT_HAS_INCORRECT_TYPE, instance);
}
}
else
{
Copy link
Member

Choose a reason for hiding this comment

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

This block should move to line 41 (so just under var instanceType = ModelIInfo.FhirTypeNameToFhirType(..) : it's applicable to both choice and non-choice elements.

validator.Trace(outcome, $"Instance indicates the element is of type '{instance.InstanceType}', which is not a known FHIR core type.",
Issue.CONTENT_ELEMENT_CHOICE_INVALID_INSTANCE_TYPE, instance);
Issue.CONTENT_ELEMENT_CHOICE_INVALID_INSTANCE_TYPE, instance);
}
}
else if (choices.Count() == 1)
{
if (instanceType is not null)
{
// Check if instance is of correct type
var isCorrectType = ModelInfo.IsInstanceTypeFor(ModelInfo.FhirTypeNameToFhirType(choices.Single()).Value, instanceType.Value);
if (!isCorrectType)
{
validator.Trace(outcome, $"Type specified in the instance ('{instance.InstanceType}') is not of the expected type ('{choices.Single()}')",
Issue.CONTENT_ELEMENT_HAS_INCORRECT_TYPE, instance);
}
}
// Only one type present in list of typerefs, all of the typerefs are candidates
outcome.Include(validator.ValidateTypeReferences(typeRefs, instance, state, validateProfiles));
}
else
validator.Trace(outcome, "ElementDefinition is a choice or contains a polymorphic type constraint, but the instance does not indicate its actual type",
Issue.CONTENT_ELEMENT_CANNOT_DETERMINE_TYPE, instance);
}
else if (choices.Count() == 1)
else
{
// Only one type present in list of typerefs, all of the typerefs are candidates
outcome.Include(validator.ValidateTypeReferences(typeRefs, instance, state));
validator.Trace(outcome, "Cannot determine the data type of this instance.",
Issue.CONTENT_ELEMENT_CANNOT_DETERMINE_TYPE, instance);
}

return outcome;
Expand Down
4 changes: 2 additions & 2 deletions src/Hl7.Fhir.Specification/Validation/Validator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ private OperationOutcome validateElement(ElementDefinitionNavigator definition,
// we need to validate based on the actual type of the instance
if (isInlineChildren && elementConstraints.IsResourcePlaceholder())
{
outcome.Add(this.ValidateType(elementConstraints, instance, state));
outcome.Add(this.ValidateType(elementConstraints, instance, state, true));
}
}

Expand All @@ -272,7 +272,7 @@ private OperationOutcome validateElement(ElementDefinitionNavigator definition,
// No inline-children, so validation depends on the presence of a <type> or <contentReference>
if (elementConstraints.Type != null || elementConstraints.ContentReference != null)
{
outcome.Add(this.ValidateType(elementConstraints, instance, state));
outcome.Add(this.ValidateType(elementConstraints, instance, state, true));
outcome.Add(ValidateNameReference(elementConstraints, definition, instance, state));
}
else
Expand Down