-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Task Validation fails in 5.2.0, works in 5.1.0 #2284
base: master
Are you sure you want to change the base?
Conversation
Hi @hhund - I tried the test in this PR, and the validation doesn't actually fail in 5.2.0. It does produce 3 information level messages though, which the test is labelling as a fail: {
"resourceType": "OperationOutcome",
"issue": [ {
"severity": "information",
"code": "processing",
"diagnostics": "Binding for path Task.input[0].type has no source, so can't be checked",
"location": [ "Task.input[0].type", "Line 1, Col 179" ]
}, {
"severity": "information",
"code": "processing",
"diagnostics": "Binding for path Task.input[1].type has no source, so can't be checked",
"location": [ "Task.input[1].type", "Line 1, Col 331" ]
}, {
"severity": "information",
"code": "processing",
"diagnostics": "Binding for path Task.input[2].type has no source, so can't be checked",
"location": [ "Task.input[2].type", "Line 1, Col 477" ]
} ] Those messages look accurate to me, I don't see any valueset binding for the slices you specify in the profile. |
@jamesagnew thank you for the feedback. With this unit test I was trying to create a minimal example for reproducing a potential bug we encountered while trying to upgrade the HAPI dependency of https://github.com/highmed/highmed-dsf from 5.1.0 to 5.2.0. |
@hhund sure thing, sounds good. I do want to say I really appreciate the good quality test. I wish that every bug came with something of this caliber. :) |
@jamesagnew Can you give me a hint, why the validation using 5.2.0 results in infos about missing bindings? Validating with 5.1.0 does not give any such infos and at least I thought I specified bindings at the appropriate locations. The generated Resources as JSON below, maybe you can take a look at the StructureDefinition (click to expand): CodeSystem{
"resourceType": "CodeSystem",
"url": "http://test.com/fhir/CodeSystem/test",
"version": "1.0.0",
"name": "Test_CodeSystem",
"status": "active",
"experimental": true,
"date": "2021-01-14T21:01:35+01:00",
"caseSensitive": true,
"hierarchyMeaning": "grouped-by",
"versionNeeded": false,
"content": "complete",
"concept": [ {
"code": "foo",
"display": "Foo Display",
"definition": "Foo Definition"
}, {
"code": "bar",
"display": "Bar Display",
"definition": "Bar Definition"
}, {
"code": "baz",
"display": "Baz Display",
"definition": "Baz Definition"
} ]
} ValueSet{
"resourceType": "ValueSet",
"url": "http://test.com/fhir/ValueSet/test",
"version": "1.0.0",
"name": "Test_ValueSet",
"status": "active",
"experimental": true,
"date": "2021-01-14T21:01:36+01:00",
"immutable": true,
"compose": {
"include": [ {
"system": "http://test.com/fhir/CodeSystem/test"
} ]
}
} StructureDefinition{
"resourceType": "StructureDefinition",
"url": "http://test.com/fhir/StructureDefinition/TestTask",
"name": "TestTask",
"status": "active",
"experimental": true,
"fhirVersion": "4.0.1",
"kind": "resource",
"abstract": false,
"type": "Task",
"baseDefinition": "http://hl7.org/fhir/StructureDefinition/Task",
"derivation": "constraint",
"differential": {
"element": [ {
"id": "Task.input",
"path": "Task.input",
"slicing": {
"discriminator": [ {
"type": "value",
"path": "type.coding.system"
}, {
"type": "value",
"path": "type.coding.code"
} ],
"rules": "closed"
},
"min": 3,
"max": "3"
}, {
"id": "Task.input:stringSlice1",
"path": "Task.input",
"sliceName": "stringSlice1",
"min": 1,
"max": "1"
}, {
"id": "Task.input:stringSlice1.type",
"path": "Task.input.type",
"binding": {
"strength": "required",
"valueSet": "http://test.com/fhir/ValueSet/test"
}
}, {
"id": "Task.input:stringSlice1.type.coding",
"path": "Task.input.type.coding",
"min": 1,
"max": "1"
}, {
"id": "Task.input:stringSlice1.type.coding.system",
"path": "Task.input.type.coding.system",
"min": 1,
"fixedUri": "http://test.com/fhir/CodeSystem/test"
}, {
"id": "Task.input:stringSlice1.type.coding.code",
"path": "Task.input.type.coding.code",
"min": 1,
"fixedCode": "foo"
}, {
"id": "Task.input:stringSlice1.value[x]",
"path": "Task.input.value[x]",
"fixedString": "foo-bar-baz"
}, {
"id": "Task.input:booleanSlice",
"path": "Task.input",
"sliceName": "booleanSlice",
"min": 1,
"max": "1"
}, {
"id": "Task.input:booleanSlice.type",
"path": "Task.input.type",
"binding": {
"strength": "required",
"valueSet": "http://test.com/fhir/ValueSet/test"
}
}, {
"id": "Task.input:booleanSlice.type.coding",
"path": "Task.input.type.coding",
"min": 1,
"max": "1"
}, {
"id": "Task.input:booleanSlice.type.coding.system",
"path": "Task.input.type.coding.system",
"min": 1,
"fixedUri": "http://test.com/fhir/CodeSystem/test"
}, {
"id": "Task.input:booleanSlice.type.coding.code",
"path": "Task.input.type.coding.code",
"min": 1,
"fixedCode": "bar"
}, {
"id": "Task.input:booleanSlice.value[x]",
"path": "Task.input.value[x]",
"type": [ {
"code": "boolean"
} ]
}, {
"id": "Task.input:stringSlice2",
"path": "Task.input",
"sliceName": "stringSlice2",
"min": 1,
"max": "1"
}, {
"id": "Task.input:stringSlice2.type",
"path": "Task.input.type",
"binding": {
"strength": "required",
"valueSet": "http://test.com/fhir/ValueSet/test"
}
}, {
"id": "Task.input:stringSlice2.type.coding",
"path": "Task.input.type.coding",
"min": 1,
"max": "1"
}, {
"id": "Task.input:stringSlice2.type.coding.system",
"path": "Task.input.type.coding.system",
"min": 1,
"fixedUri": "http://test.com/fhir/CodeSystem/test"
}, {
"id": "Task.input:stringSlice2.type.coding.code",
"path": "Task.input.type.coding.code",
"min": 1,
"fixedCode": "baz"
}, {
"id": "Task.input:stringSlice2.value[x]",
"path": "Task.input.value[x]",
"type": [ {
"code": "string"
} ]
} ]
}
} Task{
"resourceType": "Task",
"meta": {
"profile": [ "http://test.com/fhir/StructureDefinition/TestTask" ]
},
"status": "requested",
"intent": "order",
"input": [ {
"type": {
"coding": [ {
"system": "http://test.com/fhir/CodeSystem/test",
"code": "foo"
} ]
},
"valueString": "foo-bar-baz"
}, {
"type": {
"coding": [ {
"system": "http://test.com/fhir/CodeSystem/test",
"code": "bar"
} ]
},
"valueBoolean": true
}, {
"type": {
"coding": [ {
"system": "http://test.com/fhir/CodeSystem/test",
"code": "baz"
} ]
},
"valueString": "310fc43d-f387-4915-8ad0-9c757a943923"
} ]
} Using a code not part of the CodeSystem ('code-does-not-exist'), results in an validation error using 5.1.0 which suggests to me, that the binding is being picked up. OperationOutcome for unknown code with 5.1.0{
"resourceType": "OperationOutcome",
"issue": [ {
"severity": "error",
"code": "processing",
"diagnostics": "This element does not match any known slice defined in the profile http://test.com/fhir/StructureDefinition/TestTask and slicing is CLOSED: Task.input[2]: Does not match slice 'stringSlice1', Task.input[2]: Does not match slice 'booleanSlice', Task.input[2]: Does not match slice 'stringSlice1'",
"location": [ "Task.input[2]", "Line 1, Col 551" ]
}, {
"severity": "error",
"code": "processing",
"diagnostics": "Profile http://test.com/fhir/StructureDefinition/TestTask, Element 'Task.input[stringSlice1]': minimum required = 1, but only found 0",
"location": [ "Task", "Line 1, Col 35" ]
}, {
"severity": "information",
"code": "processing",
"diagnostics": "Binding for path Task.input[2].type has no source, so can't be checked",
"location": [ "Task.input[2].type", "Line 1, Col 557" ]
}, {
"severity": "error",
"code": "processing",
"diagnostics": "Unknown code 'http://test.com/fhir/CodeSystem/test#code-does-not-exist' for 'http://test.com/fhir/CodeSystem/test#code-does-not-exist'",
"location": [ "Task.input[2].type.coding[0]", "Line 1, Col 565" ]
} ]
} If I remove the bindings from the profile, validating with 5.1.0 gives the same info messages as 5.2.0 with bindings. OperationOutcome bindings remove from profile with 5.1.0{
"resourceType": "OperationOutcome",
"issue": [ {
"severity": "information",
"code": "processing",
"diagnostics": "Binding for path Task.input[0].type has no source, so can't be checked",
"location": [ "Task.input[0].type", "Line 1, Col 204" ]
}, {
"severity": "information",
"code": "processing",
"diagnostics": "Binding for path Task.input[1].type has no source, so can't be checked",
"location": [ "Task.input[1].type", "Line 1, Col 383" ]
}, {
"severity": "information",
"code": "processing",
"diagnostics": "Binding for path Task.input[2].type has no source, so can't be checked",
"location": [ "Task.input[2].type", "Line 1, Col 557" ]
} ]
} As mentioned in #2285 I think there is an Issue starting from HAPI 5.2.0: org.hl7.fhir.validation.instance.InstanceValidator L4158ff specifically the fact, that the validation is done twice: public void checkChild(ValidatorHostContext hostContext, List<ValidationMessage> errors, StructureDefinition profile, ElementDefinition definition,
Element resource, Element element, String actualType, NodeStack stack, boolean inCodeableConcept, boolean checkDisplayInContext, ElementInfo ei, String extensionUrl)
throws FHIRException, DefinitionException {
if (debug && ei.definition != null && ei.slice != null) {
System.out.println(Utilities.padLeft("", ' ', stack.depth())+ "Check "+ei.getPath()+" against both "+ei.definition.getId()+" and "+ei.slice.getId());
}
if (ei.definition != null) {
if (debug) {
System.out.println(Utilities.padLeft("", ' ', stack.depth())+ "Check "+ei.getPath()+" against defn "+ei.definition.getId());
}
checkChildByDefinition(hostContext, errors, profile, definition, resource, element, actualType, stack, inCodeableConcept, checkDisplayInContext, ei, extensionUrl, ei.definition, false);
}
if (ei.slice != null) {
if (debug) {
System.out.println(Utilities.padLeft("", ' ', stack.depth())+ "Check "+ei.getPath()+" against slice "+ei.slice.getId());
}
checkChildByDefinition(hostContext, errors, profile, definition, resource, element, actualType, stack, inCodeableConcept, checkDisplayInContext, ei, extensionUrl, ei.slice, true);
}
} The first Validating against our more complex HiGHmed DSF Task profiles results in very weird validation errors for 5.2.0, again with some of the elements being validated twice (once without error, once with). Update: Fixed non unique slice name in StructuredDefinition (see hhund/hapi-fhir@c201a17) no change to validation behaviour |
@jamesagnew looking at the changes merged, PR #2054 will help in suppressing the info messages. But I think it does not fix the underlying issue. In the profile above bindings are defined for all slices and the info messages should not be generated. To determine if #2285 is a bug, we should confirm that the profile used in this PR is valid. If so, I would argue that a bug was introduced into InstanceValidator in version 5.2.0. Could we maybe have @grahamegrieve take a look at this PR? |
If you're confident this is an actual bug in the validator itself, my recommendation would be to get it into a PR against the actual reference validator test cases, which live here: https://github.com/FHIR/fhir-test-cases/blob/master/validator/manifest.json This is the runner that executes these if you wanted to try it out: https://github.com/hapifhir/org.hl7.fhir.core/blob/master/org.hl7.fhir.validation/src/test/java/org/hl7/fhir/validation/tests/ValidationTests.java |
JUnit test to reproduce Issue #2285:
Validation of Task resource against profile defining multiple input slices discriminated by custom CodeSystem values fails in 5.2.0 but works in 5.1.0.
The TaskValidationTest in branch v5.2.0_task_validation_not_working based on v5.2.0 fails
while the TaskValidationTest in branch v5.1.0_task_validation_working based on v5.1.0 works.