From 2a2005f93bce9afa96f4538ac06dc59d28e068ee Mon Sep 17 00:00:00 2001 From: Lloyd McKenzie Date: Tue, 4 Jun 2019 20:50:49 -0400 Subject: [PATCH 1/2] 1. Cleaned up test cases to work after version changes in R5 (not sure what to do with uk one and patient-contained-org I'll work on next) 2. Added new test case for (and fixed issue with) validation problems when there are multiple target profiles declared that have the same resource type. (Previously, the validator was always choosing the first profile, even if it was invalid.) 3. Fixed problems where certain profiles were triggering silent NPEs in the validator and improved console messaging so you can tell what warnings and errors are being generated for what. --- .../fhir/r5/validation/InstanceValidator.java | 90 ++++++++++++------- .../validation/tests/ValidationTestSuite.java | 20 ++++- .../validation-examples/manifest.json | 23 +++-- .../multi-profile-same-resource-profile.xml | 80 +++++++++++++++++ .../multi-profile-same-resource.xml | 17 ++++ .../validation-examples/synthea.json | 4 +- 6 files changed, 187 insertions(+), 47 deletions(-) create mode 100644 org.hl7.fhir.validation/src/test/resources/validation-examples/multi-profile-same-resource-profile.xml create mode 100644 org.hl7.fhir.validation/src/test/resources/validation-examples/multi-profile-same-resource.xml diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/InstanceValidator.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/InstanceValidator.java index 588420095..cdca3461d 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/InstanceValidator.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/InstanceValidator.java @@ -405,7 +405,7 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat } StructureDefinition sd = null; if (profile.startsWith("#")) { - if (!rule(errors, IssueType.INVALID, element.line(), element.col(), path, sd != null, "StructureDefinition reference \"{0}\" is local, but there is not local context", profile)) { + if (!rule(errors, IssueType.INVALID, element.line(), element.col(), path, containingProfile != null, "StructureDefinition reference \"{0}\" is local, but there is not local context", profile)) { return false; } @@ -1863,33 +1863,62 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat // we validate as much as we can. First, can we infer a type from the profile? if (!type.hasTargetProfile() || type.hasTargetProfile("http://hl7.org/fhir/StructureDefinition/Resource")) ok = true; - else for (UriType u : type.getTargetProfile()) { - String pr = u.getValue(); - - String bt = getBaseType(profile, pr); - StructureDefinition sd = context.fetchResource(StructureDefinition.class, "http://hl7.org/fhir/StructureDefinition/" + bt); - if (rule(errors, IssueType.STRUCTURE, element.line(), element.col(), path, bt != null, "Unable to resolve the profile reference '" + pr + "'")) { - b.append(bt); - ok = bt.equals(ft); - if (ok && we!=null && pol.checkValid()) { - doResourceProfile(hostContext, we, pr, errors, stack.push(we, -1, null, null), path, element, profile); + else { + List candidateProfiles = new ArrayList(); + for (UriType u : type.getTargetProfile()) { + String pr = u.getValue(); + + String bt = getBaseType(profile, pr); + StructureDefinition sd = context.fetchResource(StructureDefinition.class, "http://hl7.org/fhir/StructureDefinition/" + bt); + if (rule(errors, IssueType.STRUCTURE, element.line(), element.col(), path, bt != null, "Unable to resolve the profile reference '" + pr + "'")) { + b.append(bt); + if (bt.equals(ft)) { + ok = true; + if (we!=null && pol.checkValid()) + candidateProfiles.add(sd); + } + } + } + HashMap> goodProfiles = new HashMap>(); + List> badProfiles = new ArrayList>(); + List profiles = new ArrayList(); + if (!candidateProfiles.isEmpty()) { + for (StructureDefinition sd: candidateProfiles) { + profiles.add(sd.getUrl()); + List profileErrors = new ArrayList(); + doResourceProfile(hostContext, we, sd.getUrl(), profileErrors, stack.push(we, -1, null, null), path, element, profile); + + if (hasErrors(profileErrors)) + badProfiles.add(profileErrors); + else + goodProfiles.put(sd.getUrl(), profileErrors); + if (type.hasAggregation()) { + boolean modeOk = false; + for (Enumeration mode : type.getAggregation()) { + if (mode.getValue().equals(AggregationMode.CONTAINED) && refType.equals("contained")) + modeOk = true; + else if (mode.getValue().equals(AggregationMode.BUNDLED) && refType.equals("bundled")) + modeOk = true; + else if (mode.getValue().equals(AggregationMode.REFERENCED) && (refType.equals("bundled")||refType.equals("remote"))) + modeOk = true; + } + rule(errors, IssueType.STRUCTURE, element.line(), element.col(), path, modeOk, "Reference is " + refType + " which isn't supported by the specified aggregation mode(s) for the reference"); + } + } + if (goodProfiles.size()==1) { + errors.addAll(goodProfiles.values().iterator().next()); + } else if (goodProfiles.size()==0) { + rule(errors, IssueType.STRUCTURE, element.line(), element.col(), path, false, "Unable to find matching profile among choices: " + StringUtils.join("; ", profiles)); + for (List messages : badProfiles) { + errors.addAll(messages); + } + } else { + warning(errors, IssueType.STRUCTURE, element.line(), element.col(), path, false, "Found multiple matching profiles among choices: " + StringUtils.join("; ", goodProfiles.keySet())); + for (List messages : goodProfiles.values()) { + errors.addAll(messages); + } } - } else - ok = true; // suppress following check - if (ok && type.hasAggregation()) { - boolean modeOk; - for (Enumeration mode : type.getAggregation()) { - if (mode.getValue().equals(AggregationMode.CONTAINED) && refType.equals("contained")) - ok = true; - else if (mode.getValue().equals(AggregationMode.BUNDLED) && refType.equals("bundled")) - ok = true; - else if (mode.getValue().equals(AggregationMode.REFERENCED) && (refType.equals("bundled")||refType.equals("remote"))) - ok = true; - } - rule(errors, IssueType.STRUCTURE, element.line(), element.col(), path, ok, "Reference is " + refType + " which isn't supported by the specified aggregation mode(s) for the reference"); } - if (ok) - break; } } if (!ok && type.getCode().equals("*")) { @@ -4001,14 +4030,7 @@ private boolean isAnswerRequirementFulfilled(QuestionnaireItemComponent qItem, L if (rule(errors, IssueType.STRUCTURE, ei.line(), ei.col(), ei.path, p != null, "Unknown profile " + typeProfile)) { List profileErrors = new ArrayList(); validateElement(hostContext, profileErrors, p, getElementByTail(p, tail), profile, ei.definition, resource, ei.element, type, localStack, thisIsCodeableConcept, checkDisplay); - boolean hasError = false; - for (ValidationMessage msg : profileErrors) { - if (msg.getLevel()==ValidationMessage.IssueSeverity.ERROR || msg.getLevel()==ValidationMessage.IssueSeverity.FATAL) { - hasError = true; - break; - } - } - if (hasError) + if (hasErrors(profileErrors)) badProfiles.add(profileErrors); else goodProfiles.put(typeProfile, profileErrors); diff --git a/org.hl7.fhir.validation/src/test/java/org/hl7/fhir/validation/tests/ValidationTestSuite.java b/org.hl7.fhir.validation/src/test/java/org/hl7/fhir/validation/tests/ValidationTestSuite.java index 8580d10c2..47dd84fad 100644 --- a/org.hl7.fhir.validation/src/test/java/org/hl7/fhir/validation/tests/ValidationTestSuite.java +++ b/org.hl7.fhir.validation/src/test/java/org/hl7/fhir/validation/tests/ValidationTestSuite.java @@ -101,7 +101,9 @@ public class ValidationTestSuite implements IEvaluationContext, IValidatorResour @SuppressWarnings("deprecation") @Test public void test() throws Exception { + System.out.println("Name: " + name); String v = "5.0"; + List messages = new ArrayList(); if (content.has("version")) v = content.get("version").getAsString(); @@ -138,8 +140,9 @@ public class ValidationTestSuite implements IEvaluationContext, IValidatorResour if (content.has("profiles")) { for (JsonElement je : content.getAsJsonArray("profiles")) { String p = je.getAsString(); + System.out.println("Profile: " + p); String filename = TestUtilities.resourceNameToFile("validation-examples", p); - StructureDefinition sd = loadProfile(filename, v); + StructureDefinition sd = loadProfile(filename, v, messages); val.getContext().cacheResource(sd); } } @@ -154,7 +157,7 @@ public class ValidationTestSuite implements IEvaluationContext, IValidatorResour JsonObject profile = content.getAsJsonObject("profile"); String filename = TestUtilities.resourceNameToFile("validation-examples", profile.get("source").getAsString()); v = content.has("version") ? content.get("version").getAsString() : Constants.VERSION; - StructureDefinition sd = loadProfile(filename, v); + StructureDefinition sd = loadProfile(filename, v, messages); if (name.startsWith("Json.")) val.validate(null, errorsProfile, new FileInputStream(path), FhirFormat.JSON, sd); else @@ -163,14 +166,23 @@ public class ValidationTestSuite implements IEvaluationContext, IValidatorResour } } - public StructureDefinition loadProfile(String filename, String v) throws IOException, FHIRFormatError, FileNotFoundException, FHIRException, DefinitionException { + public StructureDefinition loadProfile(String filename, String v, List messages) throws IOException, FHIRFormatError, FileNotFoundException, FHIRException, DefinitionException { StructureDefinition sd = (StructureDefinition) loadResource(filename, v); + ProfileUtilities pu = new ProfileUtilities(TestingUtilities.context(), messages, null); if (!sd.hasSnapshot()) { - ProfileUtilities pu = new ProfileUtilities(TestingUtilities.context(), null, null); StructureDefinition base = TestingUtilities.context().fetchResource(StructureDefinition.class, sd.getBaseDefinition()); pu.generateSnapshot(base, sd, sd.getUrl(), null, sd.getTitle()); // (debugging) new XmlParser().setOutputStyle(OutputStyle.PRETTY).compose(new FileOutputStream(Utilities.path("[tmp]", sd.getId()+".xml")), sd); } + for (Resource r: sd.getContained()) { + if (r instanceof StructureDefinition) { + StructureDefinition childSd = (StructureDefinition)r; + if (!childSd.hasSnapshot()) { + StructureDefinition base = TestingUtilities.context().fetchResource(StructureDefinition.class, childSd.getBaseDefinition()); + pu.generateSnapshot(base, childSd, childSd.getUrl(), null, childSd.getTitle()); + } + } + } return sd; } diff --git a/org.hl7.fhir.validation/src/test/resources/validation-examples/manifest.json b/org.hl7.fhir.validation/src/test/resources/validation-examples/manifest.json index 3ca0a6273..d84f9a365 100644 --- a/org.hl7.fhir.validation/src/test/resources/validation-examples/manifest.json +++ b/org.hl7.fhir.validation/src/test/resources/validation-examples/manifest.json @@ -360,6 +360,7 @@ ] }, "slice-by-polymorphic-type.xml": { + "version": "4.0", "errorCount": 0, "profile": { "source": "slice-by-polymorphic-type-profile.xml", @@ -411,6 +412,7 @@ } }, "slicing-types-by-string.xml": { + "version": "4.0", "errorCount": 0, "profile": { "source": "slicing-types-by-string-profile.xml", @@ -729,13 +731,20 @@ "source": "patient-translated-codes.profile.xml", "errorCount": 0, "warningCount": 0 - }, - "patient-contained-org.xml": { - "errorCount": 0, - "profile": { - "source": "patient-contained-org-profile.xml", - "errorCount": 1 - } + } + }, + "patient-contained-org.xml": { + "errorCount": 0, + "profile": { + "source": "patient-contained-org-profile.xml", + "errorCount": 1 + } + }, + "multi-profile-same-resource.xml": { + "errorCount": 0, + "profile": { + "source": "multi-profile-same-resource-profile.xml", + "errorCount": 0 } } } diff --git a/org.hl7.fhir.validation/src/test/resources/validation-examples/multi-profile-same-resource-profile.xml b/org.hl7.fhir.validation/src/test/resources/validation-examples/multi-profile-same-resource-profile.xml new file mode 100644 index 000000000..6c371e0a7 --- /dev/null +++ b/org.hl7.fhir.validation/src/test/resources/validation-examples/multi-profile-same-resource-profile.xml @@ -0,0 +1,80 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/org.hl7.fhir.validation/src/test/resources/validation-examples/multi-profile-same-resource.xml b/org.hl7.fhir.validation/src/test/resources/validation-examples/multi-profile-same-resource.xml new file mode 100644 index 000000000..16bc51430 --- /dev/null +++ b/org.hl7.fhir.validation/src/test/resources/validation-examples/multi-profile-same-resource.xml @@ -0,0 +1,17 @@ + + + + +
+
+ + + + + + + + + + + diff --git a/org.hl7.fhir.validation/src/test/resources/validation-examples/synthea.json b/org.hl7.fhir.validation/src/test/resources/validation-examples/synthea.json index fa68b8a40..b7693ffde 100644 --- a/org.hl7.fhir.validation/src/test/resources/validation-examples/synthea.json +++ b/org.hl7.fhir.validation/src/test/resources/validation-examples/synthea.json @@ -2,7 +2,7 @@ "resourceType": "Encounter", "id": "f003", "class" : { - "system" : "http://hl7.org/fhir/v3/ActCode", + "system" : "http://terminology.hl7.org/CodeSystem/v3-ActCode", "code" : "AMB" }, "text": { @@ -22,7 +22,7 @@ "reference": "Patient/f001", "display": "P. van de Heuvel" }, - "reason": { + "reasonCode": { "extension": [ { "url": "http://hl7.org/fhir/StructureDefinition/iso21090-nullFlavor", From ce26cae73617e21e1af706dac0e1c9429c9cbff2 Mon Sep 17 00:00:00 2001 From: Lloyd McKenzie Date: Wed, 5 Jun 2019 23:40:06 -0400 Subject: [PATCH 2/2] Fixed issue where wrong profile was being used, suppressed message when it wasn't needed --- .../hl7/fhir/r5/validation/InstanceValidator.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/InstanceValidator.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/InstanceValidator.java index cdca3461d..9f764ce08 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/InstanceValidator.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/r5/validation/InstanceValidator.java @@ -1864,7 +1864,7 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat if (!type.hasTargetProfile() || type.hasTargetProfile("http://hl7.org/fhir/StructureDefinition/Resource")) ok = true; else { - List candidateProfiles = new ArrayList(); + List candidateProfiles = new ArrayList(); for (UriType u : type.getTargetProfile()) { String pr = u.getValue(); @@ -1875,7 +1875,7 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat if (bt.equals(ft)) { ok = true; if (we!=null && pol.checkValid()) - candidateProfiles.add(sd); + candidateProfiles.add(pr); } } } @@ -1883,15 +1883,15 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat List> badProfiles = new ArrayList>(); List profiles = new ArrayList(); if (!candidateProfiles.isEmpty()) { - for (StructureDefinition sd: candidateProfiles) { - profiles.add(sd.getUrl()); + for (String pr: candidateProfiles) { + profiles.add(pr); List profileErrors = new ArrayList(); - doResourceProfile(hostContext, we, sd.getUrl(), profileErrors, stack.push(we, -1, null, null), path, element, profile); + doResourceProfile(hostContext, we, pr, profileErrors, stack.push(we, -1, null, null), path, element, profile); if (hasErrors(profileErrors)) badProfiles.add(profileErrors); else - goodProfiles.put(sd.getUrl(), profileErrors); + goodProfiles.put(pr, profileErrors); if (type.hasAggregation()) { boolean modeOk = false; for (Enumeration mode : type.getAggregation()) { @@ -1908,7 +1908,7 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat if (goodProfiles.size()==1) { errors.addAll(goodProfiles.values().iterator().next()); } else if (goodProfiles.size()==0) { - rule(errors, IssueType.STRUCTURE, element.line(), element.col(), path, false, "Unable to find matching profile among choices: " + StringUtils.join("; ", profiles)); + rule(errors, IssueType.STRUCTURE, element.line(), element.col(), path, profiles.size()==1, "Unable to find matching profile among choices: " + StringUtils.join("; ", profiles)); for (List messages : badProfiles) { errors.addAll(messages); }