From 81a4f0c78f34c9bfd71fdbdea366510ddcd9c93a Mon Sep 17 00:00:00 2001 From: "juan.marchionatto" Date: Wed, 22 Sep 2021 09:40:25 -0400 Subject: [PATCH] Rename method for clarity. Increase test coverage. --- ...JpaPersistedResourceValidationSupport.java | 2 +- .../fhir/jpa/term/BaseTermReadSvcImpl.java | 2 +- .../uhn/fhir/jpa/term/api/ITermReadSvc.java | 2 +- ...ersistedResourceValidationSupportTest.java | 135 +++++++++++ .../jpa/term/TermVersionAdapterSvcR4Test.java | 77 +++++++ .../fhir/jpa/term/api/ITermReadSvcTest.java | 218 ++++++++++++++++++ 6 files changed, 433 insertions(+), 3 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/JpaPersistedResourceValidationSupportTest.java create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TermVersionAdapterSvcR4Test.java create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/api/ITermReadSvcTest.java 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 f6234960c7f..d755a9cb9ba 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 @@ -125,7 +125,7 @@ public class JpaPersistedResourceValidationSupport implements IValidationSupport private Optional getCodeSystemCurrentVersion(UriType theUrl) { if (! theUrl.getValueAsString().contains("loinc")) return Optional.empty(); - return myTermReadSvc.readByForcedId("loinc"); + return myTermReadSvc.readCodeSystemByForcedId("loinc"); } 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 cd15e180327..e403fe0717c 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 @@ -2558,7 +2558,7 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { @Override - public Optional readByForcedId(String theForcedId) { + public Optional readCodeSystemByForcedId(String theForcedId) { @SuppressWarnings("unchecked") List resultList = (List) myEntityManager.createQuery( "select f.myResource from ForcedId f " + diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/api/ITermReadSvc.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/api/ITermReadSvc.java index fe6adeffee9..c619deb8f0e 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/api/ITermReadSvc.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/api/ITermReadSvc.java @@ -136,6 +136,6 @@ public interface ITermReadSvc extends IValidationSupport { */ boolean isLoincNotGenericUnversionedValueSet(String theUrl); - Optional readByForcedId(String theForcedId); + Optional readCodeSystemByForcedId(String theForcedId); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/JpaPersistedResourceValidationSupportTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/JpaPersistedResourceValidationSupportTest.java new file mode 100644 index 00000000000..20250d77327 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/JpaPersistedResourceValidationSupportTest.java @@ -0,0 +1,135 @@ +package ca.uhn.fhir.jpa.dao; + +/*- + * #%L + * HAPI FHIR JPA Server + * %% + * Copyright (C) 2014 - 2021 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.support.IValidationSupport; +import ca.uhn.fhir.jpa.api.dao.DaoRegistry; +import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; +import ca.uhn.fhir.jpa.term.api.ITermReadSvc; +import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; +import com.github.benmanes.caffeine.cache.Cache; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.r4.model.CodeSystem; +import org.hl7.fhir.r4.model.ValueSet; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.test.util.ReflectionTestUtils; + +import java.util.function.Function; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.isA; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class JpaPersistedResourceValidationSupportTest { + + private FhirContext theFhirContext = FhirContext.forR4(); + + @Mock private ITermReadSvc myTermReadSvc; + @Mock private DaoRegistry myDaoRegistry; + @Mock private Cache myLoadCache; + @Mock private IFhirResourceDao myValueSetResourceDao; + + + private IValidationSupport testedClass = + new JpaPersistedResourceValidationSupport(theFhirContext); + + private Class myCodeSystemType = CodeSystem.class; + private Class myValueSetType = ValueSet.class; + + + @BeforeEach + public void setup() { + ReflectionTestUtils.setField(testedClass, "myTermReadSvc", myTermReadSvc); + ReflectionTestUtils.setField(testedClass, "myDaoRegistry", myDaoRegistry); + ReflectionTestUtils.setField(testedClass, "myLoadCache", myLoadCache); + ReflectionTestUtils.setField(testedClass, "myCodeSystemType", myCodeSystemType); + ReflectionTestUtils.setField(testedClass, "myValueSetType", myValueSetType); + } + + + @Nested + public class FetchCodeSystemTests { + + @Test + void fetchCodeSystemMustUseForcedId() { + when(myTermReadSvc.isLoincNotGenericUnversionedCodeSystem(anyString())).thenReturn(true); + + testedClass.fetchCodeSystem("string-containing-loinc"); + + verify(myTermReadSvc, times(1)).readCodeSystemByForcedId("loinc"); + verify(myLoadCache, never()).get(anyString(), isA(Function.class)); + } + + + @Test + void fetchCodeSystemMustNotUseForcedId() { + when(myTermReadSvc.isLoincNotGenericUnversionedCodeSystem(anyString())).thenReturn(false); + + testedClass.fetchCodeSystem("string-not-containing-l-o-i-n-c"); + + verify(myTermReadSvc, never()).readCodeSystemByForcedId("loinc"); + verify(myLoadCache, times(1)).get(anyString(), isA(Function.class)); + } + + } + + + @Nested + public class FetchValueSetTests { + + @Test + void fetchValueSetMustUseForcedId() { + when(myTermReadSvc.isLoincNotGenericUnversionedValueSet(anyString())).thenReturn(true); + when(myDaoRegistry.getResourceDao(ValueSet.class)).thenReturn(myValueSetResourceDao); + + ResourceNotFoundException thrown = assertThrows( + ResourceNotFoundException.class, + () -> testedClass.fetchValueSet("string-containing-loinc")); + + assertTrue(thrown.getMessage().contains("Couldn't find current version ValueSet for url")); + } + + + @Test + void fetchValueSetMustNotUseForcedId() { + when(myTermReadSvc.isLoincNotGenericUnversionedValueSet(anyString())).thenReturn(false); + + testedClass.fetchValueSet("string-not-containing-l-o-i-n-c"); + + verify(myLoadCache, times(1)).get(anyString(), isA(Function.class)); + } + + } + + +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TermVersionAdapterSvcR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TermVersionAdapterSvcR4Test.java new file mode 100644 index 00000000000..f942f1df0a2 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/TermVersionAdapterSvcR4Test.java @@ -0,0 +1,77 @@ +package ca.uhn.fhir.jpa.term; + +/*- + * #%L + * HAPI FHIR JPA Server + * %% + * Copyright (C) 2014 - 2021 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + +import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; +import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; +import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; +import org.hl7.fhir.r4.model.CodeSystem; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.test.util.ReflectionTestUtils; + +import java.security.InvalidParameterException; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + + +@ExtendWith(MockitoExtension.class) +class TermVersionAdapterSvcR4Test { + + private final TermVersionAdapterSvcR4 testedClass = new TermVersionAdapterSvcR4(); + + @Mock private IFhirResourceDao myCodeSystemResourceDao; + @Mock ServletRequestDetails theRequestDetails; + @Mock DaoMethodOutcome theDaoMethodOutcome; + + @Test + void createOrUpdateCodeSystemMustHaveId() { + org.hl7.fhir.r4.model.CodeSystem codeSystem = new CodeSystem(); + codeSystem.setUrl("a-loinc-system"); + + InvalidParameterException thrown = assertThrows( + InvalidParameterException.class, + () -> testedClass.createOrUpdateCodeSystem(codeSystem, new ServletRequestDetails())); + + assertTrue(thrown.getMessage().contains("LOINC CodeSystem must have an 'ID' element")); + } + + + @Test + void createOrUpdateCodeSystemWithIdNoException() { + ReflectionTestUtils.setField(testedClass, "myCodeSystemResourceDao", myCodeSystemResourceDao); + + org.hl7.fhir.r4.model.CodeSystem codeSystem = new CodeSystem(); + codeSystem.setUrl("a-loinc-system").setId("loinc"); + + when(myCodeSystemResourceDao.update(codeSystem, theRequestDetails)).thenReturn(theDaoMethodOutcome); + + testedClass.createOrUpdateCodeSystem(codeSystem, theRequestDetails); + + verify(myCodeSystemResourceDao, Mockito.times(1)).update(codeSystem, theRequestDetails); + } +} 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 new file mode 100644 index 00000000000..d47e53736ec --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/term/api/ITermReadSvcTest.java @@ -0,0 +1,218 @@ +package ca.uhn.fhir.jpa.term.api; + +import ca.uhn.fhir.jpa.api.dao.DaoRegistry; +import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; +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.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; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Answers; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.data.domain.Pageable; +import org.springframework.test.util.ReflectionTestUtils; + +import javax.persistence.EntityManager; +import javax.persistence.NonUniqueResultException; +import java.util.Collections; +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isA; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class ITermReadSvcTest { + + private final ITermReadSvc testedClass = new TermReadSvcR4(); + + @Mock private ITermValueSetDao myTermValueSetDao; + @Mock private DaoRegistry myDaoRegistry; + @Mock private IFhirResourceDao myFhirResourceDao; + + + @Nested + public class FindCurrentTermValueSet { + + @BeforeEach + public void setup() { + ReflectionTestUtils.setField(testedClass, "myTermValueSetDao", myTermValueSetDao); + } + + @Test + void forLoinc() { + String valueSetId = "a-loinc-value-set"; + testedClass.findCurrentTermValueSet("http://loinc.org/vs/" + valueSetId); + + verify(myTermValueSetDao, times(1)).findTermValueSetByForcedId(valueSetId); + verify(myTermValueSetDao, never()).findTermValueSetByUrl(isA(Pageable.class), anyString()); + } + + @Test + void forNotLoinc() { + String valueSetId = "not-a-loin-c-value-set"; + testedClass.findCurrentTermValueSet("http://not-loin-c.org/vs/" + valueSetId); + + verify(myTermValueSetDao, never()).findTermValueSetByForcedId(valueSetId); + verify(myTermValueSetDao, times(1)).findTermValueSetByUrl(isA(Pageable.class), anyString()); + } + } + + + @Nested + public class MustReturnEmptyValueSet { + + @Test + void doesntStartWithGenericVSReturnsTrue() { + boolean ret = testedClass.mustReturnEmptyValueSet("http://boing.org"); + assertTrue(ret); + } + + @Test + void doesntStartWithGenericVSPlusSlashThrows() { + InternalErrorException thrown = assertThrows( + InternalErrorException.class, + () -> testedClass.mustReturnEmptyValueSet("http://loinc.org/vs-no-slash-after-vs")); + + assertTrue(thrown.getMessage().contains("Don't know how to extract ForcedId from url:")); + } + + @Test + void blankVsIdReturnsTrue() { + boolean ret = testedClass.mustReturnEmptyValueSet("http://loinc.org/vs/"); + assertTrue(ret); + } + + @Test + void startsWithGenericPlusSlashPlusIdReturnsFalse() { + boolean ret = testedClass.mustReturnEmptyValueSet("http://loinc.org/vs/some-vs-id"); + assertFalse(ret); + } + + } + + + @Nested + public class IsLoincNotGenericUnversionedCodeSystem { + + @Test + void doesntContainLoincReturnsFalse() { + boolean ret = testedClass.isLoincNotGenericUnversionedCodeSystem("http://boing.org"); + assertFalse(ret); + } + + @Test + void hasVersionReturnsFalse() { + boolean ret = testedClass.isLoincNotGenericUnversionedCodeSystem("http://boing.org|v2.68"); + assertFalse(ret); + } + + @Test + void containsLoincAndNoVersionReturnsTrue() { + boolean ret = testedClass.isLoincNotGenericUnversionedCodeSystem("http://anything-plus-loinc.org"); + assertTrue(ret); + } + } + + @Nested + public class IsLoincNotGenericUnversionedValueSet { + + @Test + void notLoincReturnsFalse() { + boolean ret = testedClass.isLoincNotGenericUnversionedValueSet("http://anything-but-loin-c.org"); + assertFalse(ret); + } + + @Test + void isLoincAndHasVersionReturnsFalse() { + boolean ret = testedClass.isLoincNotGenericUnversionedValueSet("http://loinc.org|v2.67"); + assertFalse(ret); + } + + @Test + void isLoincNoVersionButEqualsGenericValueSetUrlReturnsFalse() { + boolean ret = testedClass.isLoincNotGenericUnversionedValueSet("http://loinc.org/vs"); + assertFalse(ret); + } + + @Test + void isLoincNoVersionStartsWithGenericValueSetPlusSlashPlusIdReturnsTrue() { + boolean ret = testedClass.isLoincNotGenericUnversionedValueSet("http://loinc.org/vs/vs-id"); + assertTrue(ret); + } + + } + + + @Nested + public class ReadByForcedId { + + @Mock(answer = Answers.RETURNS_DEEP_STUBS) + private EntityManager myEntityManager; + + @Mock private ResourceTable resource1; + @Mock private ResourceTable resource2; + @Mock private IBaseResource myCodeSystemResource; + + + @BeforeEach + public void setup() { + ReflectionTestUtils.setField(testedClass, "myEntityManager", myEntityManager); + } + + + @Test + void getNoneReturnsOptionalEmpty() { + when(myEntityManager.createQuery(anyString()).getResultList()) + .thenReturn(Collections.emptyList()); + + Optional result = testedClass.readCodeSystemByForcedId("a-cs-id"); + assertFalse(result.isPresent()); + } + + @Test + void getMultipleThrows() { + when(myEntityManager.createQuery(anyString()).getResultList()) + .thenReturn(Lists.newArrayList(resource1, resource2)); + + NonUniqueResultException thrown = assertThrows( + NonUniqueResultException.class, + () -> testedClass.readCodeSystemByForcedId("a-cs-id")); + + assertTrue(thrown.getMessage().contains("More than one CodeSystem is pointed by forcedId:")); + } + + @Test + void getOneConvertToResource() { + ReflectionTestUtils.setField(testedClass, "myDaoRegistry", myDaoRegistry); + + when(myEntityManager.createQuery(anyString()).getResultList()) + .thenReturn(Lists.newArrayList(resource1)); + when(myDaoRegistry.getResourceDao("CodeSystem")).thenReturn(myFhirResourceDao); + when(myFhirResourceDao.toResource(resource1, false)).thenReturn(myCodeSystemResource); + + + testedClass.readCodeSystemByForcedId("a-cs-id"); + + + verify(myFhirResourceDao, times(1)).toResource(any(), eq(false)); + } + + } + +}