From d62e70c209a8b71b980e6d640ccacf42cba51e24 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Thu, 13 Aug 2020 05:45:49 -0400 Subject: [PATCH 1/5] Add test --- .../rest/server/ServerExceptionDstu3Test.java | 92 +++++++++++++++++-- 1 file changed, 82 insertions(+), 10 deletions(-) diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/ServerExceptionDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/ServerExceptionDstu3Test.java index 0d9eb02415c..86163674490 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/ServerExceptionDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/ServerExceptionDstu3Test.java @@ -1,12 +1,14 @@ package ca.uhn.fhir.rest.server; import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.interceptor.api.HookParams; +import ca.uhn.fhir.interceptor.api.IAnonymousInterceptor; +import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.rest.annotation.Create; import ca.uhn.fhir.rest.annotation.ResourceParam; import ca.uhn.fhir.rest.annotation.Search; import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.server.exceptions.AuthenticationException; -import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.test.utilities.JettyUtil; @@ -29,11 +31,13 @@ import org.hl7.fhir.dstu3.model.OperationOutcome.IssueType; import org.hl7.fhir.dstu3.model.Patient; import org.hl7.fhir.instance.model.api.IBaseResource; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.Collections; import java.util.List; import java.util.concurrent.TimeUnit; @@ -44,11 +48,17 @@ import static org.junit.jupiter.api.Assertions.assertEquals; public class ServerExceptionDstu3Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ServerExceptionDstu3Test.class); - public static BaseServerResponseException ourException; + public static Exception ourException; private static CloseableHttpClient ourClient; private static FhirContext ourCtx = FhirContext.forDstu3(); private static int ourPort; private static Server ourServer; + private static RestfulServer ourServlet; + + @AfterEach + public void after() { + ourException = null; + } @Test public void testAddHeadersNotFound() throws Exception { @@ -56,8 +66,8 @@ public class ServerExceptionDstu3Test { OperationOutcome operationOutcome = new OperationOutcome(); operationOutcome.addIssue().setCode(IssueType.BUSINESSRULE); - ourException = new ResourceNotFoundException("SOME MESSAGE"); - ourException.addResponseHeader("X-Foo", "BAR BAR"); + ourException = new ResourceNotFoundException("SOME MESSAGE") + .addResponseHeader("X-Foo", "BAR BAR"); HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient"); @@ -99,6 +109,65 @@ public class ServerExceptionDstu3Test { } + @Test + public void testMethodThrowsNonHapiUncheckedExceptionHandledCleanly() throws Exception { + + ourException = new NullPointerException("Hello"); + + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_format=json"); + try (CloseableHttpResponse status = ourClient.execute(httpGet)) { + assertEquals(500, status.getStatusLine().getStatusCode()); + byte[] responseContentBytes = IOUtils.toByteArray(status.getEntity().getContent()); + String responseContent = new String(responseContentBytes, Charsets.UTF_8); + ourLog.info(status.getStatusLine().toString()); + ourLog.info(responseContent); + assertThat(responseContent, containsString("\"diagnostics\":\"Failed to call access method: java.lang.NullPointerException: Hello\"")); + } + + } + + @Test + public void testMethodThrowsNonHapiCheckedExceptionHandledCleanly() throws Exception { + + ourException = new IOException("Hello"); + + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_format=json"); + try (CloseableHttpResponse status = ourClient.execute(httpGet)) { + assertEquals(500, status.getStatusLine().getStatusCode()); + byte[] responseContentBytes = IOUtils.toByteArray(status.getEntity().getContent()); + String responseContent = new String(responseContentBytes, Charsets.UTF_8); + ourLog.info(status.getStatusLine().toString()); + ourLog.info(responseContent); + assertThat(responseContent, containsString("\"diagnostics\":\"Failed to call access method: java.io.IOException: Hello\"")); + } + + } + + @Test + public void testInterceptorThrowsNonHapiUncheckedExceptionHandledCleanly() throws Exception { + + ourServlet.getInterceptorService().registerAnonymousInterceptor(Pointcut.SERVER_INCOMING_REQUEST_PRE_HANDLED, new IAnonymousInterceptor() { + @Override + public void invoke(Pointcut thePointcut, HookParams theArgs) { + throw new NullPointerException("Hello"); + } + }); + + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_format=json"); + try (CloseableHttpResponse status = ourClient.execute(httpGet)) { + assertEquals(500, status.getStatusLine().getStatusCode()); + byte[] responseContentBytes = IOUtils.toByteArray(status.getEntity().getContent()); + String responseContent = new String(responseContentBytes, Charsets.UTF_8); + ourLog.info(status.getStatusLine().toString()); + ourLog.info(responseContent); + assertThat(responseContent, containsString("\"diagnostics\":\"Hello\"")); + } + + ourServlet.getInterceptorService().unregisterAllInterceptors(); + + } + + @Test public void testPostWithNoBody() throws IOException { @@ -143,7 +212,10 @@ public class ServerExceptionDstu3Test { } @Search() - public List search() { + public List search() throws Exception { + if (ourException == null) { + return Collections.emptyList(); + } throw ourException; } @@ -168,15 +240,15 @@ public class ServerExceptionDstu3Test { DummyPatientResourceProvider patientProvider = new DummyPatientResourceProvider(); ServletHandler proxyHandler = new ServletHandler(); - RestfulServer servlet = new RestfulServer(ourCtx); - servlet.setPagingProvider(new FifoMemoryPagingProvider(10)); + ourServlet = new RestfulServer(ourCtx); + ourServlet.setPagingProvider(new FifoMemoryPagingProvider(10)); - servlet.setResourceProviders(patientProvider); - ServletHolder servletHolder = new ServletHolder(servlet); + ourServlet.setResourceProviders(patientProvider); + ServletHolder servletHolder = new ServletHolder(ourServlet); proxyHandler.addServletWithMapping(servletHolder, "/*"); ourServer.setHandler(proxyHandler); JettyUtil.startServer(ourServer); - ourPort = JettyUtil.getPortForStartedServer(ourServer); + ourPort = JettyUtil.getPortForStartedServer(ourServer); PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(5000, TimeUnit.MILLISECONDS); HttpClientBuilder builder = HttpClientBuilder.create(); From f3d5b69da028bdc36129cc4584d1e222814fcb44 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 13 Aug 2020 05:47:43 -0400 Subject: [PATCH 2/5] Bump flyway-core from 6.4.1 to 6.5.4 (#2039) Bumps [flyway-core](https://github.com/flyway/flyway) from 6.4.1 to 6.5.4. - [Release notes](https://github.com/flyway/flyway/releases) - [Commits](https://github.com/flyway/flyway/compare/flyway-6.4.1...flyway-6.5.4) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index f19cc7e4789..502cc36b392 100644 --- a/pom.xml +++ b/pom.xml @@ -717,7 +717,7 @@ 9.4.30.v20200611 3.0.2 - 6.4.1 + 6.5.4 5.4.14.Final From 89f68353eed8f6c13311fa2c29921c80c242701e Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Thu, 13 Aug 2020 05:47:56 -0400 Subject: [PATCH 3/5] Add changelog --- .../ca/uhn/hapi/fhir/changelog/5_2_0/changes.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_2_0/changes.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_2_0/changes.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_2_0/changes.yaml new file mode 100644 index 00000000000..2b3be2df14c --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_2_0/changes.yaml @@ -0,0 +1,8 @@ +--- +- item: + type: "add" + title: "The version of a few dependencies have been bumped to the latest versions + (dependent HAPI modules listed in brackets): +
    +
  • Flyway (JPA): 6.4.1-> 6.5.4
    • +
" From a3951b551fd72c32f5f52a09f8f4da68a1d45220 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Thu, 13 Aug 2020 17:36:04 -0400 Subject: [PATCH 4/5] Add release details for 5.1.0 --- .../resources/ca/uhn/hapi/fhir/changelog/5_1_0/version.yaml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/version.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/version.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/version.yaml new file mode 100644 index 00000000000..8178f8fe4fb --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/version.yaml @@ -0,0 +1,3 @@ +--- +release-date: "2020-08-13" +codename: "Manticore" From 7dd7f2b92329b302717df05da60a7a53cbe46c1d Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Sun, 16 Aug 2020 20:48:39 -0400 Subject: [PATCH 5/5] resource pid userdata fix (#2044) * fixed resource UserData RESOURCE_PID for expunge hook (was incorrectly pointing to ResourceHistoryTable pid instead of ResourceTable pid. * fix merge --- .../java/ca/uhn/fhir/jpa/api/dao/IDao.java | 3 +- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 7 +- .../provider/r4/HookInterceptorR4Test.java | 108 ++++++++++++++++-- 3 files changed, 101 insertions(+), 17 deletions(-) diff --git a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IDao.java b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IDao.java index bc9f9e12396..ca1c690f468 100644 --- a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IDao.java +++ b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/dao/IDao.java @@ -34,8 +34,9 @@ import java.util.Collection; * time to time, even within minor point releases. */ public interface IDao { + String RESOURCE_PID_KEY = "RESOURCE_PID"; - MetadataKeyResourcePid RESOURCE_PID = new MetadataKeyResourcePid("RESOURCE_PID"); + MetadataKeyResourcePid RESOURCE_PID = new MetadataKeyResourcePid(RESOURCE_PID_KEY); MetadataKeyCurrentlyReindexing CURRENTLY_REINDEXING = new MetadataKeyCurrentlyReindexing("CURRENTLY_REINDEXING"); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index 0628ff48933..7afbb87d637 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -79,7 +79,6 @@ import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.storage.TransactionDetails; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; -import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor.ActionRequestDetails; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; @@ -114,10 +113,8 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.stereotype.Repository; import org.springframework.transaction.PlatformTransactionManager; -import org.springframework.transaction.support.TransactionCallback; import org.springframework.transaction.support.TransactionSynchronizationAdapter; import org.springframework.transaction.support.TransactionSynchronizationManager; -import org.springframework.transaction.support.TransactionTemplate; import javax.annotation.PostConstruct; import javax.persistence.EntityManager; @@ -636,7 +633,7 @@ public abstract class BaseHapiFhirDao extends BaseStora ResourceMetadataKeyEnum.VERSION.put(res, Long.toString(theEntity.getVersion())); ResourceMetadataKeyEnum.PUBLISHED.put(res, theEntity.getPublished()); ResourceMetadataKeyEnum.UPDATED.put(res, theEntity.getUpdated()); - IDao.RESOURCE_PID.put(res, theEntity.getId()); + IDao.RESOURCE_PID.put(res, theEntity.getResourceId()); Collection tags = theTagList; if (theEntity.isHasTags()) { @@ -708,7 +705,7 @@ public abstract class BaseHapiFhirDao extends BaseStora res.setId(res.getIdElement().withVersion(theVersion.toString())); res.getMeta().setLastUpdated(theEntity.getUpdatedDate()); - IDao.RESOURCE_PID.put(res, theEntity.getId()); + IDao.RESOURCE_PID.put(res, theEntity.getResourceId()); Collection tags = theTagList; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/HookInterceptorR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/HookInterceptorR4Test.java index 040655c5158..80857380f8d 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/HookInterceptorR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/HookInterceptorR4Test.java @@ -1,13 +1,25 @@ package ca.uhn.fhir.jpa.provider.r4; import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.jpa.api.config.DaoConfig; +import ca.uhn.fhir.jpa.api.dao.IDao; +import ca.uhn.fhir.jpa.dao.index.IdHelperService; +import ca.uhn.fhir.jpa.model.util.JpaConstants; import ca.uhn.fhir.rest.api.MethodOutcome; import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.r4.model.BooleanType; +import org.hl7.fhir.r4.model.Parameters; import org.hl7.fhir.r4.model.Patient; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import java.util.Collections; import java.util.concurrent.atomic.AtomicLong; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -15,16 +27,24 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; public class HookInterceptorR4Test extends BaseResourceProviderR4Test { + private static final Logger ourLog = LoggerFactory.getLogger(HookInterceptorR4Test.class); - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(HookInterceptorR4Test.class); + @Autowired + IdHelperService myIdHelperService; -// @Override -// @AfterEach -// public void after( ) throws Exception { -// super.after(); -// -// myInterceptorRegistry.unregisterAllInterceptors(); -// } + @BeforeEach + public void before() throws Exception { + super.before(); + + myDaoConfig.setExpungeEnabled(true); + } + + @AfterEach + public void after() throws Exception { + myDaoConfig.setExpungeEnabled(new DaoConfig().isExpungeEnabled()); + + super.after(); + } @Test public void testOP_PRESTORAGE_RESOURCE_CREATED_ModifyResource() { @@ -69,7 +89,7 @@ public class HookInterceptorR4Test extends BaseResourceProviderR4Test { AtomicLong pid = new AtomicLong(); myInterceptorRegistry.registerAnonymousInterceptor(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED, (thePointcut, t) -> { IAnyResource resource = (IAnyResource) t.get(IBaseResource.class, 0); - Long resourcePid = (Long) resource.getUserData("RESOURCE_PID"); + Long resourcePid = (Long) resource.getUserData(IDao.RESOURCE_PID_KEY); assertNotNull(resourcePid, "Expecting RESOURCE_PID to be set on resource user data."); pid.set(resourcePid); }); @@ -77,6 +97,72 @@ public class HookInterceptorR4Test extends BaseResourceProviderR4Test { assertTrue(pid.get() > 0); } + + @Test + public void testSTORAGE_PRECOMMIT_RESOURCE_CREATED_hasCorrectPid() { + AtomicLong pid = new AtomicLong(); + myInterceptorRegistry.registerAnonymousInterceptor(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED, (thePointcut, t) -> { + IAnyResource resource = (IAnyResource) t.get(IBaseResource.class, 0); + Long resourcePid = (Long) resource.getUserData(IDao.RESOURCE_PID_KEY); + assertNotNull(resourcePid, "Expecting RESOURCE_PID to be set on resource user data."); + pid.set(resourcePid); + }); + IIdType savedPatientId = myClient.create().resource(new Patient()).execute().getId(); + Long savedPatientPid = myIdHelperService.resolveResourcePersistentIdsWithCache(null, Collections.singletonList(savedPatientId)).get(0).getIdAsLong(); + assertEquals(savedPatientPid.longValue(), pid.get()); + } + + @Test + public void testSTORAGE_PRESTORAGE_EXPUNGE_RESOURCE_hasCorrectPid() { + AtomicLong pid = new AtomicLong(); + myInterceptorRegistry.registerAnonymousInterceptor(Pointcut.STORAGE_PRESTORAGE_EXPUNGE_RESOURCE, (thePointcut, t) -> { + IAnyResource resource = (IAnyResource) t.get(IBaseResource.class, 0); + Long resourcePid = (Long) resource.getUserData(IDao.RESOURCE_PID_KEY); + assertNotNull(resourcePid, "Expecting RESOURCE_PID to be set on resource user data."); + pid.set(resourcePid); + }); + IIdType savedPatientId = myClient.create().resource(new Patient()).execute().getId(); + Long savedPatientPid = myIdHelperService.resolveResourcePersistentIdsWithCache(null, Collections.singletonList(savedPatientId)).get(0).getIdAsLong(); + + myClient.delete().resourceById(savedPatientId).execute(); + Parameters parameters = new Parameters(); + + parameters.addParameter().setName(JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_DELETED_RESOURCES).setValue(new BooleanType(true)); + myClient + .operation() + .onInstance(savedPatientId) + .named(JpaConstants.OPERATION_EXPUNGE) + .withParameters(parameters) + .execute(); + + assertEquals(savedPatientPid.longValue(), pid.get()); + } + + + @Test + public void testSTORAGE_PRECOMMIT_RESOURCE_UPDATED_hasCorrectPid() { + AtomicLong pidOld = new AtomicLong(); + AtomicLong pidNew = new AtomicLong(); + Patient patient = new Patient(); + IIdType savedPatientId = myClient.create().resource(patient).execute().getId(); + patient.setId(savedPatientId); + myInterceptorRegistry.registerAnonymousInterceptor(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, (thePointcut, t) -> { + IAnyResource resourceOld = (IAnyResource) t.get(IBaseResource.class, 0); + IAnyResource resourceNew = (IAnyResource) t.get(IBaseResource.class, 1); + Long resourceOldPid = (Long) resourceOld.getUserData(IDao.RESOURCE_PID_KEY); + Long resourceNewPid = (Long) resourceNew.getUserData(IDao.RESOURCE_PID_KEY); + assertNotNull(resourceOldPid, "Expecting RESOURCE_PID to be set on resource user data."); + assertNotNull(resourceNewPid, "Expecting RESOURCE_PID to be set on resource user data."); + pidOld.set(resourceOldPid); + pidNew.set(resourceNewPid); + }); + patient.setActive(true); + myClient.update().resource(patient).execute(); + Long savedPatientPid = myIdHelperService.resolveResourcePersistentIdsWithCache(null, Collections.singletonList(savedPatientId)).get(0).getIdAsLong(); + assertEquals(savedPatientPid.longValue(), pidOld.get()); + assertEquals(savedPatientPid.longValue(), pidNew.get()); + } + @Test public void testSTORAGE_PRECOMMIT_RESOURCE_UPDATED_hasPid() { AtomicLong oldPid = new AtomicLong(); @@ -84,12 +170,12 @@ public class HookInterceptorR4Test extends BaseResourceProviderR4Test { myInterceptorRegistry.registerAnonymousInterceptor(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, (thePointcut, t) -> { IAnyResource oldResource = (IAnyResource) t.get(IBaseResource.class, 0); - Long oldResourcePid = (Long) oldResource.getUserData("RESOURCE_PID"); + Long oldResourcePid = (Long) oldResource.getUserData(IDao.RESOURCE_PID_KEY); assertNotNull(oldResourcePid, "Expecting RESOURCE_PID to be set on resource user data."); oldPid.set(oldResourcePid); IAnyResource newResource = (IAnyResource) t.get(IBaseResource.class, 1); - Long newResourcePid = (Long) newResource.getUserData("RESOURCE_PID"); + Long newResourcePid = (Long) newResource.getUserData(IDao.RESOURCE_PID_KEY); assertNotNull(newResourcePid, "Expecting RESOURCE_PID to be set on resource user data."); newPid.set(newResourcePid); });