diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaPersistedResourceValidationSupport.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaPersistedResourceValidationSupport.java index bb27591070a..94cc05b5a58 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaPersistedResourceValidationSupport.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaPersistedResourceValidationSupport.java @@ -38,7 +38,6 @@ import ca.uhn.fhir.rest.param.UriParam; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; -import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Validate; import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IBaseResource; @@ -63,7 +62,6 @@ import java.util.concurrent.TimeUnit; import java.util.function.Supplier; import static org.apache.commons.lang3.StringUtils.isBlank; -import static org.hl7.fhir.common.hapi.validation.support.ValidationConstants.LOINC_GENERIC_VALUESET_URL_PLUS_SLASH; import static org.hl7.fhir.common.hapi.validation.support.ValidationConstants.LOINC_LOW; /** @@ -144,13 +142,11 @@ public class JpaPersistedResourceValidationSupport implements IValidationSupport * version is always pointed by the ForcedId for the no-versioned VS */ private Optional getValueSetCurrentVersion(UriType theUrl) { - if (TermReadSvcUtil.mustReturnEmptyValueSet(theUrl.getValueAsString())) return Optional.empty(); - - String forcedId = theUrl.getValue().substring(LOINC_GENERIC_VALUESET_URL_PLUS_SLASH.length()); - if (StringUtils.isBlank(forcedId)) return Optional.empty(); + Optional vsIdOpt = TermReadSvcUtil.getValueSetId(theUrl.getValueAsString()); + if (! vsIdOpt.isPresent()) return Optional.empty(); IFhirResourceDao valueSetResourceDao = myDaoRegistry.getResourceDao(myValueSetType); - IBaseResource valueSet = valueSetResourceDao.read(new IdDt("ValueSet", forcedId)); + IBaseResource valueSet = valueSetResourceDao.read(new IdDt("ValueSet", vsIdOpt.get())); return Optional.ofNullable(valueSet); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseTermReadSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseTermReadSvcImpl.java index 5b185c5726d..57c42879c4e 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseTermReadSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseTermReadSvcImpl.java @@ -181,7 +181,6 @@ import static org.apache.commons.lang3.StringUtils.isNoneBlank; import static org.apache.commons.lang3.StringUtils.isNotBlank; import static org.apache.commons.lang3.StringUtils.lowerCase; import static org.apache.commons.lang3.StringUtils.startsWithIgnoreCase; -import static org.hl7.fhir.common.hapi.validation.support.ValidationConstants.LOINC_GENERIC_VALUESET_URL_PLUS_SLASH; import static org.hl7.fhir.common.hapi.validation.support.ValidationConstants.LOINC_LOW; public abstract class BaseTermReadSvcImpl implements ITermReadSvc { @@ -2349,16 +2348,19 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { @Override public Optional findCurrentTermValueSet(String theUrl) { if (TermReadSvcUtil.isLoincNotGenericUnversionedValueSet(theUrl)) { - if (TermReadSvcUtil.mustReturnEmptyValueSet(theUrl)) return Optional.empty(); + Optional vsIdOpt = TermReadSvcUtil.getValueSetId(theUrl); + if (! vsIdOpt.isPresent()) { + return Optional.empty(); + } - String forcedId = theUrl.substring(LOINC_GENERIC_VALUESET_URL_PLUS_SLASH.length()); - if (StringUtils.isBlank(forcedId)) return Optional.empty(); - - return myTermValueSetDao.findTermValueSetByForcedId(forcedId); + return myTermValueSetDao.findTermValueSetByForcedId(vsIdOpt.get()); } List termValueSetList = myTermValueSetDao.findTermValueSetByUrl(Pageable.ofSize(1), theUrl); - if (termValueSetList.isEmpty()) return Optional.empty(); + if (termValueSetList.isEmpty()) { + return Optional.empty(); + } + return Optional.of(termValueSetList.get(0)); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermReadSvcUtil.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermReadSvcUtil.java index d45872a048e..1f965296edd 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermReadSvcUtil.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/TermReadSvcUtil.java @@ -20,24 +20,30 @@ package ca.uhn.fhir.jpa.term; * #L% */ -import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import org.apache.commons.lang3.StringUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.util.Optional; + +import static org.apache.commons.lang3.StringUtils.isBlank; import static org.hl7.fhir.common.hapi.validation.support.ValidationConstants.LOINC_GENERIC_VALUESET_URL; import static org.hl7.fhir.common.hapi.validation.support.ValidationConstants.LOINC_GENERIC_VALUESET_URL_PLUS_SLASH; import static org.hl7.fhir.common.hapi.validation.support.ValidationConstants.LOINC_LOW; public class TermReadSvcUtil { + private static final Logger ourLog = LoggerFactory.getLogger(TermReadSvcUtil.class); - public static boolean mustReturnEmptyValueSet(String theUrl) { - if (! theUrl.startsWith(LOINC_GENERIC_VALUESET_URL)) return true; + public static Optional getValueSetId(String theUrl) { + if (! theUrl.startsWith(LOINC_GENERIC_VALUESET_URL)) return Optional.empty(); if (! theUrl.startsWith(LOINC_GENERIC_VALUESET_URL_PLUS_SLASH)) { - throw new InternalErrorException("Don't know how to extract ValueSet's ForcedId from url: " + theUrl); + ourLog.error("Don't know how to extract ValueSet's ForcedId from url: " + theUrl); + return Optional.empty(); } String forcedId = theUrl.substring(LOINC_GENERIC_VALUESET_URL_PLUS_SLASH.length()); - return StringUtils.isBlank(forcedId); + return isBlank(forcedId) ? Optional.empty() : Optional.of(forcedId); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TermReadSvcUtilTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TermReadSvcUtilTest.java new file mode 100644 index 00000000000..4927edca0f6 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TermReadSvcUtilTest.java @@ -0,0 +1,102 @@ +package ca.uhn.fhir.jpa.term; + +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class TermReadSvcUtilTest { + + @Nested + public class GetValueSetId { + + @Test + void doesntStartWithLoincGenericValuesetIdReturnsEmpty() { + Optional result = TermReadSvcUtil.getValueSetId("http://boinc.org"); + assertFalse(result.isPresent()); + } + + @Test + void doesntStartWithLoincGenericValuesetIdPluSlashReturnsEmpty() { + Optional result = TermReadSvcUtil.getValueSetId("http://loinc.org/vs-something-else.ar"); + assertFalse(result.isPresent()); + } + + @Test + void startWithLoincGenericValuesetIdPluSlashButNothingElseReturnsEmpty() { + Optional result = TermReadSvcUtil.getValueSetId("http://loinc.org/vs/"); + assertFalse(result.isPresent()); + } + + @Test + void startWithLoincGenericValuesetIdPluSlashPlusIdReturnsId() { + Optional result = TermReadSvcUtil.getValueSetId("http://loinc.org/vs/radiology-playbook"); + assertTrue(result.isPresent()); + } + } + + + @Nested + public class IsLoincNotGenericUnversionedValueSet { + + @Test + void doesntContainLoincReturnsFalse() { + boolean result = TermReadSvcUtil.isLoincNotGenericUnversionedValueSet( + "http://l-oinc.org/vs/radiology-playbook"); + assertFalse(result); + } + + @Test + void containsVersionDelimiterReturnsFalse() { + boolean result = TermReadSvcUtil.isLoincNotGenericUnversionedValueSet( + "http://loinc.org/vs/radiology-playbook|v2.68"); + assertFalse(result); + } + + @Test + void isLoincGenericValueSetUrlReturnsFalse() { + boolean result = TermReadSvcUtil.isLoincNotGenericUnversionedValueSet( + "http://loinc.org/vs"); + assertFalse(result); + } + + @Test + void isLoincWithoutVersionAndNotGenericValuesetUrlReturnsTrue() { + boolean result = TermReadSvcUtil.isLoincNotGenericUnversionedValueSet( + "http://loinc.org/vs/radiology-playbook"); + assertTrue(result); + } + + } + + + @Nested + public class IsLoincNotGenericUnversionedCodeSystem { + + @Test + void doesntContainLoincReturnsFalse() { + boolean result = TermReadSvcUtil.isLoincNotGenericUnversionedCodeSystem( + "http://loin-c.org"); + assertFalse(result); + } + + @Test + void hasVersionDelimiterReturnsFalse() { + boolean result = TermReadSvcUtil.isLoincNotGenericUnversionedCodeSystem( + "http://loinc.org|v2.68"); + assertFalse(result); + } + + @Test + void containsLoincNadNoVersionDelimiterReturnsTrue() { + boolean result = TermReadSvcUtil.isLoincNotGenericUnversionedCodeSystem( + "http://loinc.org"); + assertTrue(result); + } + + } + +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/api/ITermReadSvcTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/api/ITermReadSvcTest.java index c695bf8c0aa..267d74437a0 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/api/ITermReadSvcTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/api/ITermReadSvcTest.java @@ -26,7 +26,6 @@ import ca.uhn.fhir.jpa.dao.data.ITermValueSetDao; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.term.TermReadSvcR4; import ca.uhn.fhir.jpa.term.TermReadSvcUtil; -import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import com.google.common.collect.Lists; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.r4.model.CodeSystem; @@ -96,33 +95,30 @@ class ITermReadSvcTest { @Nested - public class MustReturnEmptyValueSet { + public class GetValueSetId { @Test - void doesntStartWithGenericVSReturnsTrue() { - boolean ret = TermReadSvcUtil.mustReturnEmptyValueSet("http://boing.org"); - assertTrue(ret); + void doesntStartWithGenericVSReturnsEmpty() { + Optional vsIdOpt = TermReadSvcUtil.getValueSetId("http://boing.org"); + assertFalse(vsIdOpt.isPresent()); } @Test - void doesntStartWithGenericVSPlusSlashThrows() { - InternalErrorException thrown = assertThrows( - InternalErrorException.class, - () -> TermReadSvcUtil.mustReturnEmptyValueSet("http://loinc.org/vs-no-slash-after-vs")); - - assertTrue(thrown.getMessage().contains("Don't know how to extract ValueSet's ForcedId from url:")); + void doesntStartWithGenericVSPlusSlashReturnsEmpty() { + Optional vsIdOpt = TermReadSvcUtil.getValueSetId("http://loinc.org/vs-no-slash-after-vs"); + assertFalse(vsIdOpt.isPresent()); } @Test - void blankVsIdReturnsTrue() { - boolean ret = TermReadSvcUtil.mustReturnEmptyValueSet("http://loinc.org/vs/"); - assertTrue(ret); + void blankVsIdReturnsEmpty() { + Optional vsIdOpt = TermReadSvcUtil.getValueSetId("http://loinc.org"); + assertFalse(vsIdOpt.isPresent()); } @Test - void startsWithGenericPlusSlashPlusIdReturnsFalse() { - boolean ret = TermReadSvcUtil.mustReturnEmptyValueSet("http://loinc.org/vs/some-vs-id"); - assertFalse(ret); + void startsWithGenericPlusSlashPlusIdReturnsValid() { + Optional vsIdOpt = TermReadSvcUtil.getValueSetId("http://loinc.org/vs/some-vs-id"); + assertTrue(vsIdOpt.isPresent()); } }