From 4d1f75920d318f971cd078dc99763a9b964d3a5e Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Sat, 28 Aug 2021 01:28:47 +1000 Subject: [PATCH] validation for resource and element id (#590) * revise validation of resource and element id * wip Co-authored-by: markiantorno --- RELEASE_NOTES.md | 1 + .../fhir/r5/formats/FormatUtilitiesTest.java | 32 +++++++++++++++++++ .../src/main/resources/Messages.properties | 2 +- .../instance/InstanceValidator.java | 31 +++++------------- .../instance/InstanceValidatorTest.java | 16 ---------- 5 files changed, 42 insertions(+), 40 deletions(-) create mode 100644 org.hl7.fhir.r5/src/test/java/org/hl7/fhir/r5/formats/FormatUtilitiesTest.java diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index e69de29bb..d537656d1 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -0,0 +1 @@ +* Fixed ID verification issues for resources \ No newline at end of file diff --git a/org.hl7.fhir.r5/src/test/java/org/hl7/fhir/r5/formats/FormatUtilitiesTest.java b/org.hl7.fhir.r5/src/test/java/org/hl7/fhir/r5/formats/FormatUtilitiesTest.java new file mode 100644 index 000000000..b7ab1d337 --- /dev/null +++ b/org.hl7.fhir.r5/src/test/java/org/hl7/fhir/r5/formats/FormatUtilitiesTest.java @@ -0,0 +1,32 @@ +package org.hl7.fhir.r5.formats; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.*; + +class FormatUtilitiesTest { + + private static Stream provideIdsWithOutcomes() { + return Stream.of( + Arguments.of("1234", true), + Arguments.of("12-34", true), + Arguments.of("12_34", false), + Arguments.of("12.34", true), + Arguments.of("12/34", false), + Arguments.of("1234#", false), + Arguments.of("31415926535897932384626433832795028841971693993751058209749445923", false) // 65 digits + ); + } + + @ParameterizedTest + @MethodSource("provideIdsWithOutcomes") + void isValidIdTest(String id, boolean expected) { + Assertions.assertEquals(FormatUtilities.isValidId(id), expected); + } +} \ No newline at end of file diff --git a/org.hl7.fhir.utilities/src/main/resources/Messages.properties b/org.hl7.fhir.utilities/src/main/resources/Messages.properties index f5c72b8c7..11b382064 100644 --- a/org.hl7.fhir.utilities/src/main/resources/Messages.properties +++ b/org.hl7.fhir.utilities/src/main/resources/Messages.properties @@ -118,7 +118,7 @@ Reference_REF_NoType = Unable to determine type of target resource Reference_REF_NotFound_Bundle = Bundled or contained reference not found within the bundle/resource {0} Reference_REF_ResourceType = Matching reference for reference {0} has resourceType {1} Reference_REF_WrongTarget = The type ''{0}'' is not a valid Target for this element (must be one of {1}) -Resource_RES_ID_Malformed = Resource id not formatted correctly +Resource_RES_ID_Malformed = Invalid Resource id Resource_RES_ID_Missing = Resource requires an id, but none is present Resource_RES_ID_Prohibited = Resource has an id, but none is allowed Terminology_PassThrough_TX_Message = {0} for ''{1}#{2}'' diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java index e5581180c..8dfc11abc 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java @@ -2056,7 +2056,7 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat } } } - if (type.equals(ID)) { + if (type.equals(ID) && !"Resource.id".equals(context.getBase().getPath())) { // work around an old issue with ElementDefinition.id if (!context.getPath().equals("ElementDefinition.id")) { rule(errors, IssueType.INVALID, e.line(), e.col(), path, FormatUtilities.isValidId(e.primitiveValue()), I18nConstants.TYPE_SPECIFIC_CHECKS_DT_ID_VALID, e.primitiveValue()); @@ -5173,33 +5173,18 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat rule(errors, IssueType.INVALID, element.line(), element.col(), stack.getLiteralPath(), false, I18nConstants.RESOURCE_RES_ID_MISSING); } else if (idstatus == IdStatus.PROHIBITED && (element.getNamedChild(ID) != null)) { rule(errors, IssueType.INVALID, element.line(), element.col(), stack.getLiteralPath(), false, I18nConstants.RESOURCE_RES_ID_PROHIBITED); - } else if ((idstatus == IdStatus.OPTIONAL || idstatus == IdStatus.REQUIRED) - && (element.getNamedChild(ID) != null) - && (!idFormattedCorrectly(element.getNamedChild(ID).getValue()))) { - rule(errors, IssueType.INVALID, element.line(), element.col(), stack.getLiteralPath(), false, I18nConstants.RESOURCE_RES_ID_MALFORMED); + } + if (element.getNamedChild(ID) != null) { + Element eid = element.getNamedChild(ID); + if (eid.getProperty() != null && eid.getProperty().getDefinition() != null && eid.getProperty().getDefinition().getBase().getPath().equals("Resource.id")) { + NodeStack ns = stack.push(eid, -1, eid.getProperty().getDefinition(), null); + rule(errors, IssueType.INVALID, eid.line(), eid.col(), ns.getLiteralPath(), FormatUtilities.isValidId(eid.primitiveValue()), I18nConstants.RESOURCE_RES_ID_MALFORMED); + } } start(hostContext, errors, element, element, defn, stack); // root is both definition and type } } - /** - * FHIR IDs are constrained to any combination of upper- or lower-case ASCII letters ('A'..'Z', and 'a'..'z', - * numerals ('0'..'9'), '-' and '.', with a length limit of 64 characters. (This might be an integer, an un-prefixed - * OID, UUID or any other identifier pattern that meets these constraints.) - * - * As a heads up, a null String will pass this check. - * - * @param id The id to verify conformance to specification requirements. - * @return {@link Boolean#TRUE} if the passed in ID is a valid resource ID, {@link Boolean#FALSE} otherwise. - */ - protected static boolean idFormattedCorrectly(@Nonnull String id) { - String idRegex = "[A-Za-z0-9\\-\\.]{1,64}"; - Pattern pattern = Pattern.compile(idRegex); - Matcher matcher = pattern.matcher(id); - return matcher.matches(); - } - - private NodeStack getFirstEntry(NodeStack bundle) { List list = new ArrayList(); bundle.getElement().getNamedChildren(ENTRY, list); diff --git a/org.hl7.fhir.validation/src/test/java/org/hl7/fhir/validation/instance/InstanceValidatorTest.java b/org.hl7.fhir.validation/src/test/java/org/hl7/fhir/validation/instance/InstanceValidatorTest.java index ad2aa6688..f5e9291c3 100644 --- a/org.hl7.fhir.validation/src/test/java/org/hl7/fhir/validation/instance/InstanceValidatorTest.java +++ b/org.hl7.fhir.validation/src/test/java/org/hl7/fhir/validation/instance/InstanceValidatorTest.java @@ -13,21 +13,5 @@ import static org.junit.jupiter.api.Assertions.*; class InstanceValidatorTest { - private static Stream provideIdsWithOutcomes() { - return Stream.of( - Arguments.of("1234", true), - Arguments.of("12-34", true), - Arguments.of("12_34", false), - Arguments.of("12.34", true), - Arguments.of("12/34", false), - Arguments.of("1234#", false), - Arguments.of("31415926535897932384626433832795028841971693993751058209749445923", false) // 65 digits - ); - } - @ParameterizedTest - @MethodSource("provideIdsWithOutcomes") - void idFormattedCorrectly(String id, boolean expected) { - Assertions.assertEquals(InstanceValidator.idFormattedCorrectly(id), expected); - } } \ No newline at end of file