validation for resource and element id (#590)

* revise validation of resource and element id

* wip

Co-authored-by: markiantorno <markiantorno@gmail.com>

RELEASE_NOTES.md

@@ -0,0 +1 @@
+* Fixed ID verification issues for resources

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<Arguments> 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);
+  }
+}

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}''

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<Element> list = new ArrayList<Element>();
     bundle.getElement().getNamedChildren(ENTRY, list);

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<Arguments> 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);
-  }
 }