diff --git a/conformance/fhir-ig-us-core/src/test/java/com/ibm/fhir/ig/us/core/test/v311/ConformanceTest.java b/conformance/fhir-ig-us-core/src/test/java/com/ibm/fhir/ig/us/core/test/v311/ConformanceTest.java index 7f07de902b3..ab55762115b 100644 --- a/conformance/fhir-ig-us-core/src/test/java/com/ibm/fhir/ig/us/core/test/v311/ConformanceTest.java +++ b/conformance/fhir-ig-us-core/src/test/java/com/ibm/fhir/ig/us/core/test/v311/ConformanceTest.java @@ -341,12 +341,11 @@ public void testUSCoreEthnicityExtension4() throws Exception { issues.forEach(System.out::println); - // one for generated-ext-1: Extension must conform to definition 'http://hl7.org/fhir/us/core/StructureDefinition/us-core-race' - // and one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..." - Assert.assertEquals(countErrors(issues), 4); + // one for generated-us-core-patient-Patient.extension + // and one for the corresponding "not a valid member of ValueSet http://hl7.org/fhir/us/core/ValueSet/detailed-ethnicity|3.1.1" + Assert.assertEquals(countErrors(issues), 2); Assert.assertEquals(countWarnings(issues), 0); - // one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..." - Assert.assertEquals(countInformation(issues), 3); + Assert.assertEquals(countInformation(issues), 1); } /** @@ -527,12 +526,11 @@ public void testUSCoreRaceExtension4() throws Exception { issues.forEach(System.out::println); - // one for generated-ext-1: Extension must conform to definition 'http://hl7.org/fhir/us/core/StructureDefinition/us-core-race' - // and one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..." - Assert.assertEquals(countErrors(issues), 4); + // one for generated-us-core-patient-Patient.extension + // and one for the corresponding "not a valid member of ValueSet http://hl7.org/fhir/us/core/ValueSet/detailed-race|3.1.1" + Assert.assertEquals(countErrors(issues), 2); Assert.assertEquals(countWarnings(issues), 1); - // one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..." - Assert.assertEquals(countInformation(issues), 3); + Assert.assertEquals(countInformation(issues), 1); } } \ No newline at end of file diff --git a/conformance/fhir-ig-us-core/src/test/java/com/ibm/fhir/ig/us/core/test/v400/ConformanceTest.java b/conformance/fhir-ig-us-core/src/test/java/com/ibm/fhir/ig/us/core/test/v400/ConformanceTest.java index 09d670decd6..5051857a738 100644 --- a/conformance/fhir-ig-us-core/src/test/java/com/ibm/fhir/ig/us/core/test/v400/ConformanceTest.java +++ b/conformance/fhir-ig-us-core/src/test/java/com/ibm/fhir/ig/us/core/test/v400/ConformanceTest.java @@ -333,12 +333,11 @@ public void testUSCoreEthnicityExtension4() throws Exception { issues.forEach(System.out::println); - // one for generated-ext-1: Extension must conform to definition 'http://hl7.org/fhir/us/core/StructureDefinition/us-core-race' - // and one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..." - Assert.assertEquals(countErrors(issues), 4); + // one for generated-us-core-patient-Patient.extension + // and one for the corresponding "not a valid member of ValueSet http://hl7.org/fhir/us/core/ValueSet/detailed-ethnicity|4.0.0" + Assert.assertEquals(countErrors(issues), 2); Assert.assertEquals(countWarnings(issues), 1); - // one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..." - Assert.assertEquals(countInformation(issues), 3); + Assert.assertEquals(countInformation(issues), 1); } /** @@ -519,12 +518,11 @@ public void testUSCoreRaceExtension4() throws Exception { issues.forEach(System.out::println); - // one for generated-ext-1: Extension must conform to definition 'http://hl7.org/fhir/us/core/StructureDefinition/us-core-race' - // and one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..." - Assert.assertEquals(countErrors(issues), 4); + // one for generated-us-core-patient-Patient.extension + // and one for the corresponding "not a valid member of ValueSet http://hl7.org/fhir/us/core/ValueSet/detailed-race|4.0.0" + Assert.assertEquals(countErrors(issues), 2); Assert.assertEquals(countWarnings(issues), 1); - // one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..." - Assert.assertEquals(countInformation(issues), 3); + Assert.assertEquals(countInformation(issues), 1); } @Test diff --git a/conformance/fhir-ig-us-core/src/test/java/com/ibm/fhir/ig/us/core/test/v500/ConformanceTest.java b/conformance/fhir-ig-us-core/src/test/java/com/ibm/fhir/ig/us/core/test/v500/ConformanceTest.java index 52b543ccc19..803ad580920 100644 --- a/conformance/fhir-ig-us-core/src/test/java/com/ibm/fhir/ig/us/core/test/v500/ConformanceTest.java +++ b/conformance/fhir-ig-us-core/src/test/java/com/ibm/fhir/ig/us/core/test/v500/ConformanceTest.java @@ -333,12 +333,11 @@ public void testUSCoreEthnicityExtension4() throws Exception { issues.forEach(System.out::println); - // one for generated-ext-1: Extension must conform to definition 'http://hl7.org/fhir/us/core/StructureDefinition/us-core-race' - // and one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..." - Assert.assertEquals(countErrors(issues), 4); + // one for generated-us-core-patient-Patient.extension + // and one for the corresponding "not a valid member of ValueSet http://hl7.org/fhir/us/core/ValueSet/detailed-ethnicity" + Assert.assertEquals(countErrors(issues), 2); Assert.assertEquals(countWarnings(issues), 1); - // one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..." - Assert.assertEquals(countInformation(issues), 3); + Assert.assertEquals(countInformation(issues), 1); } /** @@ -519,12 +518,11 @@ public void testUSCoreRaceExtension4() throws Exception { issues.forEach(System.out::println); - // one for generated-ext-1: Extension must conform to definition 'http://hl7.org/fhir/us/core/StructureDefinition/us-core-race' - // and one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..." - Assert.assertEquals(countErrors(issues), 4); + // one for generated-us-core-patient-Patient.extension + // and one for the corresponding "not a valid member of ValueSet http://hl7.org/fhir/us/core/ValueSet/detailed-race" + Assert.assertEquals(countErrors(issues), 2); Assert.assertEquals(countWarnings(issues), 1); - // one for each version of the extension due to the generated constraint "conformsTo('a'|1) or conformsTo('a'|2) or conformsTo('a'|3)..." - Assert.assertEquals(countInformation(issues), 3); + Assert.assertEquals(countInformation(issues), 1); } @Test diff --git a/fhir-profile/src/main/java/com/ibm/fhir/profile/ConstraintGenerator.java b/fhir-profile/src/main/java/com/ibm/fhir/profile/ConstraintGenerator.java index 0b39711ded4..a284cb74e35 100644 --- a/fhir-profile/src/main/java/com/ibm/fhir/profile/ConstraintGenerator.java +++ b/fhir-profile/src/main/java/com/ibm/fhir/profile/ConstraintGenerator.java @@ -73,6 +73,7 @@ public List generate() { List constraints = new ArrayList<>(); String url = profile.getUrl().getValue(); + boolean isProfile = !"Extension".equals(url); String prefix = url.substring(url.lastIndexOf("/") + 1); int index = 1; @@ -82,13 +83,27 @@ public List generate() { log.finest("Element definition -> constraint expression:"); for (Node child : tree.root.children) { try { + String constraintId; + if (isProfile && hasExtensionConstraint(child.elementDefinition)) { + // We include both the path and the profile URL in the constraint id so that + // we can pull these out during validation and avoid redundant extension validation + Type type = getTypes(child.elementDefinition).get(0); + String profile = getProfilesWithoutVersion(type).get(0); + + // https://www.rfc-editor.org/rfc/rfc3986.html#appendix-C explicitly mentions angle brackets + // as a good way to embed URIs in plain text. + // Other safe options include whitespace, curly braces, angle brackets, and/or backslash. + constraintId = "generated-" + prefix + "~" + child.elementDefinition.getPath().getValue() + "<" + profile + ">"; + } else { + constraintId = "generated-" + prefix + "-" + index; + index++; + } String expr = generate(child); if (generated.contains(expr)) { continue; } String description = "Constraint violation: " + expr; - constraints.add(constraint("generated-" + prefix + "-" + index, expr, description)); - index++; + constraints.add(constraint(constraintId, expr, description)); generated.add(expr); } catch (ConstraintGenerationException e) { log.warning("Constraint was not generated due to the following error: " + e.getMessage() + " (elementDefinition: " + e.getElementDefinition().getId() + ", profile: " + url + ")"); @@ -492,7 +507,7 @@ private String generate(Node node) { /** * Guard calls to this method with calls to {@link #hasExtensionConstraint(ElementDefinition)} - * to ensure that the node's ElementDefinition has a single type with a single profile. + * to ensure that the node's ElementDefinition has a single type with a single profile value. */ private String generateExtensionConstraint(Node node) { StringBuilder sb = new StringBuilder(); @@ -500,9 +515,12 @@ private String generateExtensionConstraint(Node node) { ElementDefinition elementDefinition = node.elementDefinition; Type type = getTypes(elementDefinition).get(0); - String profile = getProfilesWithoutVersion(type).get(0); + String profile = type.getProfile().get(0).getValue(); + String profileWithoutVersion = profile.contains("|") ? profile.substring(0, profile.lastIndexOf("|")) : profile; - sb.append("extension('").append(profile).append("')").append(cardinality(node, sb.toString())); + sb.append("extension('").append(profileWithoutVersion).append("')").append(cardinality(node, sb.toString())); + sb.append(" and "); + sb.append("extension('").append(profileWithoutVersion).append("')").append(".all(conformsTo('" + profile + "'))"); return sb.toString(); } @@ -610,7 +628,7 @@ private String generatePatternValueConstraint(Node node, boolean discriminator) */ private String generateProfileConstraint(Node node) { StringBuilder sb = new StringBuilder(); - + ElementDefinition elementDefinition = node.elementDefinition; List types = getTypes(elementDefinition); @@ -862,6 +880,10 @@ private boolean hasExtensionConstraint(ElementDefinition elementDefinition) { return false; } + if (profile.get(0).getValue() == null) { + return false; + } + return true; } diff --git a/fhir-validation/src/main/java/com/ibm/fhir/validation/FHIRValidator.java b/fhir-validation/src/main/java/com/ibm/fhir/validation/FHIRValidator.java index 9e10f3d2c78..db6d82d3a74 100644 --- a/fhir-validation/src/main/java/com/ibm/fhir/validation/FHIRValidator.java +++ b/fhir-validation/src/main/java/com/ibm/fhir/validation/FHIRValidator.java @@ -39,7 +39,6 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.stream.Collectors; import com.ibm.fhir.model.annotation.Constraint; import com.ibm.fhir.model.annotation.Constraint.FHIRPathConstraintValidator; @@ -64,7 +63,6 @@ import com.ibm.fhir.path.visitor.FHIRPathDefaultNodeVisitor; import com.ibm.fhir.profile.ProfileSupport; import com.ibm.fhir.registry.FHIRRegistry; -import com.ibm.fhir.registry.resource.FHIRRegistryResource; import com.ibm.fhir.validation.exception.FHIRValidationException; import net.jcip.annotations.NotThreadSafe; @@ -332,64 +330,73 @@ private void validate(FHIRPathResourceNode resourceNode) { constraints.addAll(ProfileSupport.getConstraints(profiles, resourceType)); } - // add instance-specific extension constraints - PathAwareCollectingVisitor extCollector = new PathAwareCollectingVisitor(Extension.class); - resourceNode.resource().accept(extCollector); - Map pathToExtension = extCollector.getResult(); - - // for option A below: find all the versions of the extension in the registry - Map> profileVersions = collectProfileVersions(pathToExtension.values()); + // path (with array indices) -> Extension instance (which never have a url version suffix) + Map pathToExtension = gatherInstanceExtensions(resourceNode); + // path (with no array indices) -> Extension URLs (with no version suffix) + Map> alreadyGeneratedExtensionConstraints = gatherExtensionConstraints(constraints); + // for each extension in the resource instance for (Entry entry : pathToExtension.entrySet()) { String path = entry.getKey(); Extension e = entry.getValue(); String url = e.getUrl(); if (isAbsolute(url)) { - // Option A: find all versions of the extension and pass validation if the instance conforms to at least one - // Option B: introspect the existing constraints and only add this one if the extension is not covered by profile constraints + // remove the array indices from the path + String reducedPath = path.replaceAll("\\[[0-9]+\\]", ""); - // Option A: conformance to any one version of the extension is sufficient - if (profileVersions.containsKey(url)) { - String constraint = profileVersions.get(url).stream() - .map(v -> "conformsTo('" + url + "|" + v + "')") - .collect(Collectors.joining(" or ")); + // if the profile validation already has this covered, then move on to the next extension + if (alreadyGeneratedExtensionConstraints.containsKey(reducedPath) + && alreadyGeneratedExtensionConstraints.get(reducedPath).contains(url)) { + continue; + } - constraints.add(Constraint.Factory.createConstraint("generated-ext-1", Constraint.LEVEL_RULE, path, - "Extension must conform to at least one definition of '" + url + "'", constraint, SOURCE_VALIDATOR, false, true)); + // otherwise generate the instance-specifc conformsTo constraint (or add a warning if we don't have a definition for it) + if (FHIRRegistry.getInstance().hasResource(url, StructureDefinition.class)) { + constraints.add(Constraint.Factory.createConstraint("generated-ext~" + path + "<" + url + ">", Constraint.LEVEL_RULE, path, + "Extension must conform to definition '" + url + "'", + "conformsTo('" + url + "')", SOURCE_VALIDATOR, false, true)); } else { issues.add(issue(IssueSeverity.WARNING, IssueType.NOT_SUPPORTED, "Extension definition '" + url + "' is not supported", null, path)); } - - // Option B: conditionally add a conformsTo constraint for the default/latest version of this extension -// if (FHIRRegistry.getInstance().hasResource(url, StructureDefinition.class)) { -// constraints.add(Constraint.Factory.createConstraint("generated-ext-2", Constraint.LEVEL_RULE, path, -// "Extension must conform to definition '" + url + "'", -// "conformsTo('" + url + "')", SOURCE_VALIDATOR, false, true)); -// } else { -// issues.add(issue(IssueSeverity.WARNING, IssueType.NOT_SUPPORTED, "Extension definition '" + url + "' is not supported", null, path)); -// } } } validate(resourceNode, constraints); } - private Map> collectProfileVersions(Collection extensions) { - Map> profileVersions = new HashMap<>(); + /** + * Collect all the extensions from the resource instance and index them by their simple FHIRPath paths + */ + private Map gatherInstanceExtensions(FHIRPathResourceNode resourceNode) { + PathAwareCollectingVisitor extCollector = new PathAwareCollectingVisitor(Extension.class); + resourceNode.resource().accept(extCollector); + Map pathToExtension = extCollector.getResult(); + return pathToExtension; + } - Set uniqueExtensionUrls = extensions.stream() - .map(e -> e.getUrl()) - .distinct() - .collect(Collectors.toSet()); + /** + * Gather all the constraint ids and process the specially-marked extension ones into a map + * from the paths to the set of corresponding extension URLs (with no version suffix). + */ + private Map> gatherExtensionConstraints(List constraints) { + Map> alreadyGeneratedExtensionConstraints = new HashMap<>(); + for (Constraint c : constraints) { + int urlMark = c.id().lastIndexOf("<"); + if (urlMark <= 0 || !c.id().endsWith(">")) { + continue; + } - // RegistryResourceProviders have a method thats basically just what we want, but unfortunately the registry does not - for (FHIRRegistryResource rr : FHIRRegistry.getInstance().getRegistryResources(StructureDefinition.class)) { - if (uniqueExtensionUrls.contains(rr.getUrl()) && rr.getVersion() != null) { - profileVersions.computeIfAbsent(rr.getUrl(), x -> new HashSet<>()).add(rr.getVersion().toString()); + String firstPart = c.id().substring(0, urlMark); + int pathMark = firstPart.lastIndexOf("~"); + if (pathMark <= 0) { + continue; } - } + String path = firstPart.substring(pathMark + 1); - return profileVersions; + String url = c.id().substring(urlMark + 1, c.id().length() - 1); + alreadyGeneratedExtensionConstraints.computeIfAbsent(path, t -> new HashSet<>()).add(url); + } + return alreadyGeneratedExtensionConstraints; } private void validateProfileReferences(FHIRPathResourceNode resourceNode, List profiles, boolean resourceAsserted) {