Merge pull request #1614 from Boereck/fix/canonical_split_base_validator

Fix/canonical split base validator

org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/CanonicalPair.java

@@ -1,26 +1,82 @@
 package org.hl7.fhir.utilities;
 
+import org.apache.commons.lang3.StringUtils;
+
+/**
+ * Abstraction that splits a canonical in form of {@code <url>|<version>} into a URL and a version part.
+ */
 public class CanonicalPair {
 
-  private String url;
+  private final String url;
-  private String version;
+  private final String version;
   
+  /**
+   * Static factory method, that invokes the {@link CanonicalPair#CanonicalPair(String) CanonicalPair constructor}
+   * with the given argument.
+   * @param target the canonical to be split. May be {@code null}.
+   * @return new instance of CanonicalPair
+   */
+  public static CanonicalPair of(String target) {
+    return new CanonicalPair(target);
+  }
+
+  /**
+   * Wraps the given canonical and if needed splits off the version part.<p>
+   * The given parameter {@code target} is expected to be a canonical.
+   * @param target canonical to create a url, version pair from. May be {@code null}.
+   */
   public CanonicalPair(String target) {
-    if (target != null && target.contains("|")) {
+    int pipeIndex = target != null ? target.indexOf('|') : -1;
-      this.url = target.substring(0, target.indexOf("|"));
+    if (pipeIndex >= 0) {
-      this.version = target.substring(target.indexOf("|")+1);     
+      this.url = target.substring(0, pipeIndex);
+      this.version = target.substring(pipeIndex+1);
     } else {
       this.url = target;
       this.version = null;
     }
   }
 
+  /**
+   * Returns the URL part  of the canonical (everything before the {@code "|"} character, or the complete
+   * canonical if the character is not present). If the source 
+   * @return URL part of the source canonical. May be {@code null}, if source canonical was {@code null}
+   */
   public String getUrl() {
     return url;
   }
 
+  /**
+   * Returns the version part of the source canonical (everything after the {@code "|"} character.
+   * @return version part of the canonical, may be {@code null}, if canonical was {@code null}, or canonical contains no 
+   *   {@code "|"} character.
+   */
   public String getVersion() {
     return version;
   }
 
+  /**
+   * Determines if the version part of the canonical is not {@code null}
+   * @return {@code true} if version is not {@code null}, otherwise {@code false}
+   */
+  public boolean hasVersion() {
+    return version != null;
+  }
+
+  /**
+   * Returns the version of this pair, or the parameter {@code alternative},
+   * if the version of this pair is {@code null}.
+   * @param alternative to be returned from this method if the encapsulated version is {@code null}.
+   * @return either the held version, or {@code alternative}, if version is {@code null}
+   */
+  public String getVersionOr(String alternative) {
+    return hasVersion() ? version : alternative;
+  }
+
+  /**
+   * Determines if the encapsulated version of this pair is not {@code null} and not an empty string.
+   * @return {@code true} if the version of this pair is not {@code null} and not an empty string, {@code false} otherwise
+   */
+  public boolean hasNonEmptyVersion() {
+    return StringUtils.isNotEmpty(version);
+  }
 }

org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/CanonicalPairTest.java

@@ -0,0 +1,90 @@
+package org.hl7.fhir.utilities;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import org.junit.jupiter.api.Test;
+
+class CanonicalPairTest {
+
+  @Test
+  void testCanonicalNull() {
+    var canonical = new CanonicalPair(null);
+    assertNull(canonical.getUrl());
+    assertNull(canonical.getVersion());
+    assertFalse(canonical.hasVersion());
+    assertFalse(canonical.hasNonEmptyVersion());
+    String expectedVer = "1.0";
+    String actualVer = canonical.getVersionOr(expectedVer);
+    assertEquals(expectedVer, actualVer);
+  }
+  
+  @Test
+  void testCanonicalEmpty() {
+    var url = "";
+    var canonical = new CanonicalPair(url);
+    assertEquals(url, canonical.getUrl());
+    assertFalse(canonical.hasVersion());
+    assertFalse(canonical.hasNonEmptyVersion());
+    String expectedVer = "1.0";
+    String actualVer = canonical.getVersionOr(expectedVer);
+    assertEquals(expectedVer, actualVer);
+  }
+  
+  @Test
+  void testCanonicalWithoutVersion() {
+    var url = "https://www.test.org";
+    var canonical = new CanonicalPair(url);
+    assertEquals(url, canonical.getUrl());
+    assertNull(canonical.getVersion());
+    assertFalse(canonical.hasVersion());
+    assertFalse(canonical.hasNonEmptyVersion());
+    String expectedVer = "1.0";
+    String actualVer = canonical.getVersionOr(expectedVer);
+    assertEquals(expectedVer, actualVer);
+  }
+  
+  @Test
+  void testCanonicalWithEmptyVersion() {
+    var expectedUrl = "https://www.test.org";
+    var url = expectedUrl + "|";
+    var canonical = new CanonicalPair(url);
+    assertEquals(expectedUrl, canonical.getUrl());
+    assertEquals("", canonical.getVersion());
+    assertTrue(canonical.hasVersion());
+    assertFalse(canonical.hasNonEmptyVersion());
+    String alternativeVer = "1.0";
+    String actualVer = canonical.getVersionOr(alternativeVer);
+    assertEquals("", actualVer);
+  }
+
+  @Test
+  void testCanonicalWithVersion() {
+    var expectedUrl = "https://www.test.org";
+    var expectedVersion = "2.6";
+    var url = expectedUrl + "|" + expectedVersion;
+    var canonical = new CanonicalPair(url);
+    assertEquals(expectedUrl, canonical.getUrl());
+    assertEquals(expectedVersion, canonical.getVersion());
+    assertTrue(canonical.hasVersion());
+    assertTrue(canonical.hasNonEmptyVersion());
+    String alternativeVer = "1.0";
+    String actualVer = canonical.getVersionOr(alternativeVer);
+    assertEquals(expectedVersion, actualVer);
+  }
+
+  @Test
+  void testCanonicalWithVersionIncludingPipe() {
+    var expectedUrl = "https://www.test.org";
+    var expectedVersion = "2024|05";
+    var url = expectedUrl + "|" + expectedVersion;
+    var canonical = new CanonicalPair(url);
+    assertEquals(expectedUrl, canonical.getUrl());
+    assertEquals(expectedVersion, canonical.getVersion());
+    assertTrue(canonical.hasVersion());
+    assertTrue(canonical.hasNonEmptyVersion());
+    String alternativeVer = "1.0";
+    String actualVer = canonical.getVersionOr(alternativeVer);
+    assertEquals(expectedVersion, actualVer);
+  }
+  
+}

org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/BaseValidator.java

@@ -1428,25 +1428,7 @@ public class BaseValidator implements IValidationContextResourceLoader {
     return null;
   }
   
-  protected String versionFromCanonical(String system) {
-    if (system == null) {
-      return null;
-    } else if (system.contains("|")) {
-      return system.substring(0, system.indexOf("|"));
-    } else {
-      return system;
-    }
-  }
 
-  protected String systemFromCanonical(String system) {
-    if (system == null) {
-      return null;
-    } else if (system.contains("|")) {
-      return system.substring(system.indexOf("|")+1);
-    } else {
-      return system;
-    }
-  }
   
   @Override
   public Resource loadContainedResource(List<ValidationMessage> errors, String path, Element resource, String id, Class<? extends Resource> class1) throws FHIRException {

org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/type/CodeSystemValidator.java

@@ -13,6 +13,7 @@ import org.hl7.fhir.r5.model.CodeSystem;
 import org.hl7.fhir.r5.model.CodeSystem.ConceptDefinitionComponent;
 import org.hl7.fhir.r5.model.ValueSet;
 import org.hl7.fhir.r5.terminologies.CodeSystemUtilities;
+import org.hl7.fhir.utilities.CanonicalPair;
 import org.hl7.fhir.utilities.Utilities;
 import org.hl7.fhir.utilities.VersionUtilities;
 import org.hl7.fhir.utilities.i18n.I18nConstants;
@@ -620,7 +621,8 @@ public class CodeSystemValidator  extends BaseValidator {
   private boolean validateSupplementConcept(List<ValidationMessage> errors, Element concept, NodeStack stack, String supp, ValidationOptions options) {
     String code = concept.getChildValue("code");
     if (!Utilities.noString(code)) {
-      org.hl7.fhir.r5.terminologies.utilities.ValidationResult res = context.validateCode(options, systemFromCanonical(supp), versionFromCanonical(supp), code, null);
+      var canonical = new CanonicalPair(supp);
+      org.hl7.fhir.r5.terminologies.utilities.ValidationResult res = context.validateCode(options, canonical.getUrl(), canonical.getVersion(), code, null);
       return rule(errors, NO_RULE_DATE, IssueType.BUSINESSRULE, stack.getLiteralPath(), res.isOk(), I18nConstants.CODESYSTEM_CS_SUPP_INVALID_CODE, supp, code);
     } else {
       return true;

org.hl7.fhir.validation/src/test/java/org/hl7/fhir/validation/ResourceValidationTests.java

@@ -1,5 +1,9 @@
 package org.hl7.fhir.validation;
 
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.MatcherAssert.assertThat;
+
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.util.ArrayList;
@@ -10,6 +14,7 @@ import org.hl7.fhir.r5.formats.XmlParser;
 import org.hl7.fhir.r5.model.Resource;
 import org.hl7.fhir.r5.test.utils.TestingUtilities;
 import org.hl7.fhir.utilities.FhirPublication;
+import org.hl7.fhir.utilities.i18n.I18nConstants;
 import org.hl7.fhir.utilities.settings.FhirSettings;
 import org.hl7.fhir.utilities.validation.ValidationMessage;
 import org.hl7.fhir.validation.instance.InstanceValidator;
@@ -25,7 +30,7 @@ public class ResourceValidationTests {
   private static InstanceValidator val;
 
 
-  private void runTest(String filename) throws IOException, FileNotFoundException, Exception {
+  private List<ValidationMessage> runTest(String filename) throws IOException, FileNotFoundException, Exception {
     TestingUtilities.injectCorePackageLoader();
     if (val == null) {
       ctxt = TestingUtilities.getSharedWorkerContext();
@@ -37,6 +42,7 @@ public class ResourceValidationTests {
     Resource res = (Resource) new XmlParser().parse(TestingUtilities.loadTestResourceStream("r5", filename));
     val.validate(val, errors, res);
     Assertions.assertNotNull(errors);
+    return errors;
   }
 
 
@@ -123,4 +129,26 @@ public class ResourceValidationTests {
   }
 
   
+  @Test
+  public void testCodesystemSupplementsVersion() throws Exception {
+    List<ValidationMessage> errors = runTest("codesystem-example-supplement-version.xml");
+    assertNoErrors(errors);
+  }
+
+
+  private void assertNoErrors(List<ValidationMessage> errors) {
+    List<String> errorMessages = new ArrayList<>();
+    for(ValidationMessage message : errors) {
+      // we will skip the message that WG citation is needed
+      if(I18nConstants.VALIDATION_HL7_WG_NEEDED.equals(message.getMessageId())) {
+        continue;
+      }
+      if(message.getLevel().isError()) {
+        errorMessages.add(message.getMessage());
+      }
+    }
+    assertThat("No error message expected in validation outcome.", errorMessages, is(empty()));
+  }
+
+
 }