From 197283824976e02f4aca214a97ffa1586e06b96e Mon Sep 17 00:00:00 2001 From: ianmarshall Date: Mon, 18 Jan 2021 14:03:50 -0500 Subject: [PATCH 01/10] Ensure that table indexes are not deleted during DB migration dry-run. --- .../HapiFlywayMigrateDatabaseCommandTest.java | 105 ++++++++++++++++++ .../jpa/migrate/taskdef/DropTableTask.java | 3 +- 2 files changed, 107 insertions(+), 1 deletion(-) diff --git a/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/HapiFlywayMigrateDatabaseCommandTest.java b/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/HapiFlywayMigrateDatabaseCommandTest.java index d968a11503b..a12693fc867 100644 --- a/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/HapiFlywayMigrateDatabaseCommandTest.java +++ b/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/HapiFlywayMigrateDatabaseCommandTest.java @@ -24,6 +24,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Set; import static org.apache.commons.lang3.StringUtils.isBlank; import static org.junit.jupiter.api.Assertions.*; @@ -61,9 +62,44 @@ public class HapiFlywayMigrateDatabaseCommandTest { "-n", "", "-p", "" }; + assertFalse(JdbcUtils.getTableNames(connectionProperties).contains("HFJ_RES_REINDEX_JOB")); + // Verify that HFJ_SEARCH_PARM exists along with index and foreign key dependencies. + assertTrue(JdbcUtils.getTableNames(connectionProperties).contains("HFJ_SEARCH_PARM")); + Set indexNames = JdbcUtils.getIndexNames(connectionProperties, "HFJ_SEARCH_PARM"); + assertTrue(indexNames.contains("IDX_SEARCHPARM_RESTYPE_SPNAME")); + Set foreignKeys = JdbcUtils.getForeignKeys(connectionProperties, "HFJ_SEARCH_PARM", "HFJ_RES_PARAM_PRESENT"); + assertTrue(foreignKeys.contains("FK_RESPARMPRES_SPID")); + // Verify that IDX_FORCEDID_TYPE_FORCEDID index exists on HFJ_FORCED_ID table + indexNames = JdbcUtils.getIndexNames(connectionProperties, "HFJ_FORCED_ID"); + assertTrue(indexNames.contains("IDX_FORCEDID_TYPE_FORCEDID")); + // Verify that HFJ_RES_PARAM_PRESENT has column SP_ID + Set columnNames = JdbcUtils.getColumnNames(connectionProperties, "HFJ_RES_PARAM_PRESENT"); + assertTrue(columnNames.contains("SP_ID")); + // Verify that SEQ_SEARCHPARM_ID sequence exists + Set seqNames = JdbcUtils.getSequenceNames(connectionProperties); + assertTrue(seqNames.contains("SEQ_SEARCHPARM_ID")); + // Verify that foreign key FK_SEARCHRES_RES on HFJ_SEARCH_RESULT exists + foreignKeys = JdbcUtils.getForeignKeys(connectionProperties, "HFJ_RESOURCE", "HFJ_SEARCH_RESULT"); + assertTrue(foreignKeys.contains("FK_SEARCHRES_RES")); + App.main(args); + assertTrue(JdbcUtils.getTableNames(connectionProperties).contains("HFJ_RES_REINDEX_JOB")); + // Verify that HFJ_SEARCH_PARM has been removed + assertFalse(JdbcUtils.getTableNames(connectionProperties).contains("HFJ_SEARCH_PARM")); + // Verify that IDX_FORCEDID_TYPE_FORCEDID index no longer exists on HFJ_FORCED_ID table + indexNames = JdbcUtils.getIndexNames(connectionProperties, "HFJ_FORCED_ID"); + assertFalse(indexNames.contains("IDX_FORCEDID_TYPE_FORCEDID")); + // Verify that HFJ_RES_PARAM_PRESENT no longer has column SP_ID + columnNames = JdbcUtils.getColumnNames(connectionProperties, "HFJ_RES_PARAM_PRESENT"); + assertFalse(columnNames.contains("SP_ID")); + // Verify that SEQ_SEARCHPARM_ID sequence no longer exists + seqNames = JdbcUtils.getSequenceNames(connectionProperties); + assertFalse(seqNames.contains("SEQ_SEARCHPARM_ID")); + // Verify that foreign key FK_SEARCHRES_RES on HFJ_SEARCH_RESULT no longer exists + foreignKeys = JdbcUtils.getForeignKeys(connectionProperties, "HFJ_RESOURCE", "HFJ_SEARCH_RESULT"); + assertFalse(foreignKeys.contains("FK_SEARCHRES_RES")); connectionProperties.getTxTemplate().execute(t -> { JdbcTemplate jdbcTemplate = connectionProperties.newJdbcTemplate(); @@ -118,6 +154,75 @@ public class HapiFlywayMigrateDatabaseCommandTest { }); } + @Test + public void testMigrateFrom340_dryRun() throws IOException, SQLException { + + File location = getLocation("migrator_h2_test_340_dryrun"); + + String url = "jdbc:h2:" + location.getAbsolutePath() + ";create=true"; + DriverTypeEnum.ConnectionProperties connectionProperties = DriverTypeEnum.H2_EMBEDDED.newConnectionProperties(url, "", ""); + + String initSql = "/persistence_create_h2_340.sql"; + executeSqlStatements(connectionProperties, initSql); + + seedDatabase340(connectionProperties); + + ourLog.info("**********************************************"); + ourLog.info("Done Setup, Starting Migration..."); + ourLog.info("**********************************************"); + + String[] args = new String[]{ + BaseFlywayMigrateDatabaseCommand.MIGRATE_DATABASE, + "-d", "H2_EMBEDDED", + "-u", url, + "-n", "", + "-p", "", + "-r" + }; + + // Verify that HFJ_SEARCH_PARM exists along with index and foreign key dependencies. + assertTrue(JdbcUtils.getTableNames(connectionProperties).contains("HFJ_SEARCH_PARM")); + Set indexNames = JdbcUtils.getIndexNames(connectionProperties, "HFJ_SEARCH_PARM"); + assertTrue(indexNames.contains("IDX_SEARCHPARM_RESTYPE_SPNAME")); + Set foreignKeys = JdbcUtils.getForeignKeys(connectionProperties, "HFJ_SEARCH_PARM", "HFJ_RES_PARAM_PRESENT"); + assertTrue(foreignKeys.contains("FK_RESPARMPRES_SPID")); + // Verify that IDX_FORCEDID_TYPE_FORCEDID index exists on HFJ_FORCED_ID table + indexNames = JdbcUtils.getIndexNames(connectionProperties, "HFJ_FORCED_ID"); + assertTrue(indexNames.contains("IDX_FORCEDID_TYPE_FORCEDID")); + // Verify that HFJ_RES_PARAM_PRESENT has column SP_ID + Set columnNames = JdbcUtils.getColumnNames(connectionProperties, "HFJ_RES_PARAM_PRESENT"); + assertTrue(columnNames.contains("SP_ID")); + // Verify that SEQ_SEARCHPARM_ID sequence exists + Set seqNames = JdbcUtils.getSequenceNames(connectionProperties); + assertTrue(seqNames.contains("SEQ_SEARCHPARM_ID")); + // Verify that foreign key FK_SEARCHRES_RES on HFJ_SEARCH_RESULT exists + foreignKeys = JdbcUtils.getForeignKeys(connectionProperties, "HFJ_RESOURCE", "HFJ_SEARCH_RESULT"); + assertTrue(foreignKeys.contains("FK_SEARCHRES_RES")); + + App.main(args); + + // Verify that HFJ_SEARCH_PARM still exists along with index and foreign key dependencies. + assertTrue(JdbcUtils.getTableNames(connectionProperties).contains("HFJ_SEARCH_PARM")); + indexNames = JdbcUtils.getIndexNames(connectionProperties, "HFJ_SEARCH_PARM"); + assertTrue(indexNames.contains("IDX_SEARCHPARM_RESTYPE_SPNAME")); + foreignKeys = JdbcUtils.getForeignKeys(connectionProperties, "HFJ_SEARCH_PARM", "HFJ_RES_PARAM_PRESENT"); + assertTrue(foreignKeys.contains("FK_RESPARMPRES_SPID")); + // Verify that IDX_FORCEDID_TYPE_FORCEDID index still exists on HFJ_FORCED_ID table + indexNames = JdbcUtils.getIndexNames(connectionProperties, "HFJ_FORCED_ID"); + assertTrue(indexNames.contains("IDX_FORCEDID_TYPE_FORCEDID")); + // Verify that HFJ_RES_PARAM_PRESENT still has column SP_ID + columnNames = JdbcUtils.getColumnNames(connectionProperties, "HFJ_RES_PARAM_PRESENT"); + assertTrue(columnNames.contains("SP_ID")); + // Verify that SEQ_SEARCHPARM_ID sequence still exists + seqNames = JdbcUtils.getSequenceNames(connectionProperties); + assertTrue(seqNames.contains("SEQ_SEARCHPARM_ID")); + // Verify that foreign key FK_SEARCHRES_RES on HFJ_SEARCH_RESULT still exists + foreignKeys = JdbcUtils.getForeignKeys(connectionProperties, "HFJ_RESOURCE", "HFJ_SEARCH_RESULT"); + assertTrue(foreignKeys.contains("FK_SEARCHRES_RES")); + + + } + @Test public void testMigrateFromEmptySchema() throws IOException, SQLException { diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropTableTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropTableTask.java index 440f4bf20e2..7a80a978a2e 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropTableTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropTableTask.java @@ -67,7 +67,8 @@ public class DropTableTask extends BaseTableTask { theIndexTask .setTableName(getTableName()) .setConnectionProperties(getConnectionProperties()) - .setDriverType(getDriverType()); + .setDriverType(getDriverType()) + .setDryRun(isDryRun()); for (String nextIndex : indexNames) { theIndexTask .setIndexName(nextIndex) From 26491f3bc845349bf3651c87bf48b779e00d3bad Mon Sep 17 00:00:00 2001 From: ianmarshall Date: Mon, 18 Jan 2021 14:09:06 -0500 Subject: [PATCH 02/10] Ensure that table indexes are not deleted during DB migration dry-run. --- .../5_3_0/2298-indexes-dropped-during-db-migrate-dryrun.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2298-indexes-dropped-during-db-migrate-dryrun.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2298-indexes-dropped-during-db-migrate-dryrun.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2298-indexes-dropped-during-db-migrate-dryrun.yaml new file mode 100644 index 00000000000..eecb6fb3e5c --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2298-indexes-dropped-during-db-migrate-dryrun.yaml @@ -0,0 +1,5 @@ +--- +type: fix +issue: 2298 +title: "A recent change inadvertently caused an issue with DB migration utility such that it would attempt to drop + indexes even when the `dry-run` option was specified. This has been fixed." From 8f72970063d908cf8c92210dad1bb8200493dfe3 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 18 Jan 2021 14:09:38 -0500 Subject: [PATCH 03/10] Add bean to config --- .../java/ca/uhn/fhir/jpa/bulk/job/BulkExportJobConfig.java | 6 ++++++ .../src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java | 2 ++ .../src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java | 4 ---- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/bulk/job/BulkExportJobConfig.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/bulk/job/BulkExportJobConfig.java index bfe08e51b5e..d3ebcba1c6c 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/bulk/job/BulkExportJobConfig.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/bulk/job/BulkExportJobConfig.java @@ -21,6 +21,7 @@ package ca.uhn.fhir.jpa.bulk.job; */ import ca.uhn.fhir.jpa.batch.processors.PidToIBaseResourceProcessor; +import ca.uhn.fhir.jpa.bulk.svc.BulkExportDaoSvc; import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; import org.hl7.fhir.instance.model.api.IBaseResource; import org.springframework.batch.core.Job; @@ -54,6 +55,11 @@ public class BulkExportJobConfig { @Autowired private PidToIBaseResourceProcessor myPidToIBaseResourceProcessor; + @Bean + public BulkExportDaoSvc bulkExportDaoSvc() { + return new BulkExportDaoSvc(); + } + @Bean @Lazy public Job bulkExportJob() { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java index bdffba29f46..c3b00c46da3 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java @@ -19,6 +19,7 @@ import ca.uhn.fhir.jpa.binstore.BinaryStorageInterceptor; import ca.uhn.fhir.jpa.bulk.api.IBulkDataExportSvc; import ca.uhn.fhir.jpa.bulk.provider.BulkDataExportProvider; import ca.uhn.fhir.jpa.bulk.svc.BulkDataExportSvcImpl; +import ca.uhn.fhir.jpa.bulk.svc.BulkExportDaoSvc; import ca.uhn.fhir.jpa.cache.IResourceVersionSvc; import ca.uhn.fhir.jpa.cache.ResourceVersionSvcDaoImpl; import ca.uhn.fhir.jpa.dao.DaoSearchParamProvider; @@ -220,6 +221,7 @@ public abstract class BaseConfig { return new BatchJobSubmitterImpl(); } + @Lazy @Bean public CascadingDeleteInterceptor cascadingDeleteInterceptor(FhirContext theFhirContext, DaoRegistry theDaoRegistry, IInterceptorBroadcaster theInterceptorBroadcaster) { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java index 8849f1acf06..474403f05c3 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java @@ -73,10 +73,6 @@ public class TestR4Config extends BaseJavaConfigR4 { return new CircularQueueCaptureQueriesListener(); } - @Bean - public BulkExportDaoSvc bulkExportDaoSvc() { - return new BulkExportDaoSvc(); - } @Bean public DataSource dataSource() { From 5b2530a9482ed06e0809cea3a7a0a56294b4106f Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 18 Jan 2021 14:55:33 -0500 Subject: [PATCH 04/10] Remove dead beans --- .../main/java/ca/uhn/fhir/jpa/bulk/svc/BulkExportDaoSvc.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/bulk/svc/BulkExportDaoSvc.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/bulk/svc/BulkExportDaoSvc.java index d9aa7104502..d69f8cbc235 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/bulk/svc/BulkExportDaoSvc.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/bulk/svc/BulkExportDaoSvc.java @@ -50,10 +50,6 @@ public class BulkExportDaoSvc { IBulkExportCollectionDao myBulkExportCollectionDao; @Autowired IBulkExportCollectionFileDao myBulkExportCollectionFileDao; - @Autowired - private FhirContext myFhirContext; - @Autowired - private DaoRegistry myDaoRegistry; @Transactional public void addFileToCollectionWithId(Long theCollectionEntityId, BulkExportCollectionFileEntity theFile) { @@ -65,7 +61,6 @@ public class BulkExportDaoSvc { myBulkExportCollectionFileDao.saveAndFlush(theFile); myBulkExportCollectionDao.saveAndFlush(exportCollectionEntity); } - } @Transactional From 141cd149182a0fe09b892460d926864564f00eac Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 18 Jan 2021 15:04:03 -0500 Subject: [PATCH 05/10] Add changelog --- .../changelog/5_3_0/2299-bean-missing-for-batch-export.yaml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2299-bean-missing-for-batch-export.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2299-bean-missing-for-batch-export.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2299-bean-missing-for-batch-export.yaml new file mode 100644 index 00000000000..3d53efb1607 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2299-bean-missing-for-batch-export.yaml @@ -0,0 +1,4 @@ +--- +type: fix +issue: 2299 +title: "The BulkExportDaoSvc bean was forgotten from the BaseConfig context. It has been added." From 7c6c64bfb79c7ca40bc998dccb549b7bd75c0d27 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 18 Jan 2021 15:53:49 -0500 Subject: [PATCH 06/10] Remove dead import --- .../src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java index c3b00c46da3..0c24c591ac7 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/BaseConfig.java @@ -19,7 +19,6 @@ import ca.uhn.fhir.jpa.binstore.BinaryStorageInterceptor; import ca.uhn.fhir.jpa.bulk.api.IBulkDataExportSvc; import ca.uhn.fhir.jpa.bulk.provider.BulkDataExportProvider; import ca.uhn.fhir.jpa.bulk.svc.BulkDataExportSvcImpl; -import ca.uhn.fhir.jpa.bulk.svc.BulkExportDaoSvc; import ca.uhn.fhir.jpa.cache.IResourceVersionSvc; import ca.uhn.fhir.jpa.cache.ResourceVersionSvcDaoImpl; import ca.uhn.fhir.jpa.dao.DaoSearchParamProvider; From edbc789a93253ff67633b0da99ce97c1b18ace20 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 18 Jan 2021 21:49:43 -0500 Subject: [PATCH 07/10] Add annotation to skip test classes which require docker if it is not found on the host --- .../fhir/jpa/dao/r4/BaseR4SearchLastN.java | 2 ++ ...esourceDaoR4SearchWithElasticSearchIT.java | 2 ++ ...sourceDaoR4TerminologyElasticsearchIT.java | 2 ++ ...bservationIndexedSearchParamLastNR4IT.java | 2 ++ ...lasticsearchSvcMultipleObservationsIT.java | 8 ++++++- .../TestElasticsearchContainerHelper.java | 6 +++++ hapi-fhir-jpaserver-test-utilities/pom.xml | 2 +- hapi-fhir-test-utilities/pom.xml | 7 +++++- .../docker/DockerRequiredCondition.java | 23 +++++++++++++++++++ .../test/utilities/docker/RequiresDocker.java | 15 ++++++++++++ 10 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/docker/DockerRequiredCondition.java create mode 100644 hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/docker/RequiresDocker.java diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseR4SearchLastN.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseR4SearchLastN.java index 53d167270b0..682e7283623 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseR4SearchLastN.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseR4SearchLastN.java @@ -19,6 +19,7 @@ import ca.uhn.fhir.rest.param.TokenAndListParam; import ca.uhn.fhir.rest.param.TokenOrListParam; import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; +import ca.uhn.fhir.test.utilities.docker.RequiresDocker; import ca.uhn.fhir.util.TestUtil; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.DateTimeType; @@ -50,6 +51,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.when; @ExtendWith(SpringExtension.class) +@RequiresDocker @ContextConfiguration(classes = {TestR4ConfigWithElasticsearchClient.class}) public class BaseR4SearchLastN extends BaseJpaTest { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithElasticSearchIT.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithElasticSearchIT.java index 5098d26e0a3..252df5cfdd2 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithElasticSearchIT.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithElasticSearchIT.java @@ -11,6 +11,7 @@ import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; +import ca.uhn.fhir.test.utilities.docker.RequiresDocker; import org.hamcrest.Matchers; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Bundle; @@ -59,6 +60,7 @@ import ca.uhn.fhir.validation.FhirValidator; import ca.uhn.fhir.validation.ValidationResult; @ExtendWith(SpringExtension.class) +@RequiresDocker @ContextConfiguration(classes = {TestR4ConfigWithElasticSearch.class}) @DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS) public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TerminologyElasticsearchIT.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TerminologyElasticsearchIT.java index 098754e33ea..751b0696713 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TerminologyElasticsearchIT.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TerminologyElasticsearchIT.java @@ -18,6 +18,7 @@ import ca.uhn.fhir.jpa.searchparam.registry.ISearchParamRegistry; import ca.uhn.fhir.jpa.term.api.ITermCodeSystemStorageSvc; import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; +import ca.uhn.fhir.test.utilities.docker.RequiresDocker; import ca.uhn.fhir.util.TestUtil; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.CodeSystem; @@ -45,6 +46,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; @ExtendWith(SpringExtension.class) +@RequiresDocker @ContextConfiguration(classes = {TestR4ConfigWithElasticSearch.class}) public class FhirResourceDaoR4TerminologyElasticsearchIT extends BaseJpaTest { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/PersistObservationIndexedSearchParamLastNR4IT.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/PersistObservationIndexedSearchParamLastNR4IT.java index 74a56313c45..bd686231e0c 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/PersistObservationIndexedSearchParamLastNR4IT.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/PersistObservationIndexedSearchParamLastNR4IT.java @@ -14,6 +14,7 @@ import ca.uhn.fhir.jpa.search.lastn.json.ObservationJson; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.parser.IParser; import ca.uhn.fhir.rest.param.*; +import ca.uhn.fhir.test.utilities.docker.RequiresDocker; import com.google.common.base.Charsets; import org.apache.commons.io.IOUtils; import org.aspectj.lang.annotation.Before; @@ -47,6 +48,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; @ExtendWith(SpringExtension.class) +@RequiresDocker @ContextConfiguration(classes = {TestR4ConfigWithElasticsearchClient.class}) @TestMethodOrder(MethodOrderer.OrderAnnotation.class) public class PersistObservationIndexedSearchParamLastNR4IT { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/lastn/LastNElasticsearchSvcMultipleObservationsIT.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/lastn/LastNElasticsearchSvcMultipleObservationsIT.java index 29a0c5bc70c..3937b7d316f 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/lastn/LastNElasticsearchSvcMultipleObservationsIT.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/lastn/LastNElasticsearchSvcMultipleObservationsIT.java @@ -18,6 +18,7 @@ import ca.uhn.fhir.rest.param.TokenAndListParam; import ca.uhn.fhir.rest.param.TokenOrListParam; import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.rest.param.TokenParamModifier; +import ca.uhn.fhir.test.utilities.docker.RequiresDocker; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.databind.ObjectMapper; @@ -27,6 +28,7 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; @@ -57,7 +59,8 @@ import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; -@ExtendWith(SpringExtension.class) +@ExtendWith({SpringExtension.class}) +@RequiresDocker @Testcontainers public class LastNElasticsearchSvcMultipleObservationsIT { @@ -70,9 +73,12 @@ public class LastNElasticsearchSvcMultipleObservationsIT { private final FhirContext myFhirContext = FhirContext.forCached(FhirVersionEnum.R4); + @Container public static ElasticsearchContainer elasticsearchContainer = TestElasticsearchContainerHelper.getEmbeddedElasticSearch(); + + private ElasticsearchSvcImpl elasticsearchSvc; @BeforeEach diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/lastn/config/TestElasticsearchContainerHelper.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/lastn/config/TestElasticsearchContainerHelper.java index 570d9565fec..6e5d708406c 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/lastn/config/TestElasticsearchContainerHelper.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/lastn/config/TestElasticsearchContainerHelper.java @@ -1,5 +1,10 @@ package ca.uhn.fhir.jpa.search.lastn.config; +import com.github.dockerjava.api.exception.InternalServerErrorException; +import org.junit.jupiter.api.extension.BeforeAllCallback; +import org.junit.jupiter.api.extension.ConditionEvaluationResult; +import org.junit.jupiter.api.extension.ExecutionCondition; +import org.junit.jupiter.api.extension.ExtensionContext; import org.testcontainers.elasticsearch.ElasticsearchContainer; import java.time.Duration; @@ -13,6 +18,7 @@ public class TestElasticsearchContainerHelper { public static final String ELASTICSEARCH_IMAGE = "docker.elastic.co/elasticsearch/elasticsearch:" + ELASTICSEARCH_VERSION; public static ElasticsearchContainer getEmbeddedElasticSearch() { + return new ElasticsearchContainer(ELASTICSEARCH_IMAGE) .withStartupTimeout(Duration.of(300, SECONDS)); } diff --git a/hapi-fhir-jpaserver-test-utilities/pom.xml b/hapi-fhir-jpaserver-test-utilities/pom.xml index 12cef79a094..9b4624f2f0b 100644 --- a/hapi-fhir-jpaserver-test-utilities/pom.xml +++ b/hapi-fhir-jpaserver-test-utilities/pom.xml @@ -68,5 +68,5 @@ junit-jupiter-engine compile - + diff --git a/hapi-fhir-test-utilities/pom.xml b/hapi-fhir-test-utilities/pom.xml index f7c0cbfb6ed..b0073d305a8 100644 --- a/hapi-fhir-test-utilities/pom.xml +++ b/hapi-fhir-test-utilities/pom.xml @@ -95,8 +95,13 @@ junit-jupiter-engine compile + + org.testcontainers + elasticsearch + compile + - + diff --git a/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/docker/DockerRequiredCondition.java b/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/docker/DockerRequiredCondition.java new file mode 100644 index 00000000000..799350e0cdb --- /dev/null +++ b/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/docker/DockerRequiredCondition.java @@ -0,0 +1,23 @@ +package ca.uhn.fhir.test.utilities.docker; + +import org.junit.jupiter.api.extension.ConditionEvaluationResult; +import org.junit.jupiter.api.extension.ExecutionCondition; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.testcontainers.elasticsearch.ElasticsearchContainer; + + +/** + * Execution condition which will skip test classes that require docker if it is not present on the host machine + */ +public class DockerRequiredCondition implements ExecutionCondition { + + @Override + public ConditionEvaluationResult evaluateExecutionCondition(ExtensionContext theExtensionContext) { + try { + new ElasticsearchContainer(); + return ConditionEvaluationResult.enabled("Docker is installed so we can run these tests!"); + } catch (Exception e) { + return ConditionEvaluationResult.disabled("It appears as though docker is not installed on the host machine!"); + } + } +} diff --git a/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/docker/RequiresDocker.java b/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/docker/RequiresDocker.java new file mode 100644 index 00000000000..3a0ae68812f --- /dev/null +++ b/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/docker/RequiresDocker.java @@ -0,0 +1,15 @@ +package ca.uhn.fhir.test.utilities.docker; + + +import org.junit.jupiter.api.extension.ExtendWith; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Retention(RetentionPolicy.RUNTIME) +@ExtendWith(DockerRequiredCondition.class) +@Target(ElementType.TYPE) +public @interface RequiresDocker { +} From 8d87f0f92bd343636569255b058f139bdbaf9739 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 18 Jan 2021 21:53:14 -0500 Subject: [PATCH 08/10] Simplify check --- hapi-fhir-test-utilities/pom.xml | 7 ++++++- .../test/utilities/docker/DockerRequiredCondition.java | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/hapi-fhir-test-utilities/pom.xml b/hapi-fhir-test-utilities/pom.xml index b0073d305a8..d59433874f4 100644 --- a/hapi-fhir-test-utilities/pom.xml +++ b/hapi-fhir-test-utilities/pom.xml @@ -100,8 +100,13 @@ elasticsearch compile + + org.testcontainers + testcontainers + compile + - + diff --git a/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/docker/DockerRequiredCondition.java b/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/docker/DockerRequiredCondition.java index 799350e0cdb..6edd457dc92 100644 --- a/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/docker/DockerRequiredCondition.java +++ b/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/docker/DockerRequiredCondition.java @@ -3,6 +3,7 @@ package ca.uhn.fhir.test.utilities.docker; import org.junit.jupiter.api.extension.ConditionEvaluationResult; import org.junit.jupiter.api.extension.ExecutionCondition; import org.junit.jupiter.api.extension.ExtensionContext; +import org.testcontainers.DockerClientFactory; import org.testcontainers.elasticsearch.ElasticsearchContainer; @@ -14,7 +15,7 @@ public class DockerRequiredCondition implements ExecutionCondition { @Override public ConditionEvaluationResult evaluateExecutionCondition(ExtensionContext theExtensionContext) { try { - new ElasticsearchContainer(); + DockerClientFactory.instance().isDockerAvailable(); return ConditionEvaluationResult.enabled("Docker is installed so we can run these tests!"); } catch (Exception e) { return ConditionEvaluationResult.disabled("It appears as though docker is not installed on the host machine!"); From ceb0155ab78a45a4657e3892d4b03c9506155f45 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 18 Jan 2021 22:07:12 -0500 Subject: [PATCH 09/10] Fix hamcrest inclusion via compile scope --- hapi-fhir-test-utilities/pom.xml | 11 ++++++----- .../utilities/docker/DockerRequiredCondition.java | 1 - 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hapi-fhir-test-utilities/pom.xml b/hapi-fhir-test-utilities/pom.xml index d59433874f4..0ca370abbb5 100644 --- a/hapi-fhir-test-utilities/pom.xml +++ b/hapi-fhir-test-utilities/pom.xml @@ -95,15 +95,16 @@ junit-jupiter-engine compile - - org.testcontainers - elasticsearch - compile - org.testcontainers testcontainers compile + + + org.hamcrest + hamcrest-core + + diff --git a/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/docker/DockerRequiredCondition.java b/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/docker/DockerRequiredCondition.java index 6edd457dc92..661f314bbaf 100644 --- a/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/docker/DockerRequiredCondition.java +++ b/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/docker/DockerRequiredCondition.java @@ -4,7 +4,6 @@ import org.junit.jupiter.api.extension.ConditionEvaluationResult; import org.junit.jupiter.api.extension.ExecutionCondition; import org.junit.jupiter.api.extension.ExtensionContext; import org.testcontainers.DockerClientFactory; -import org.testcontainers.elasticsearch.ElasticsearchContainer; /** From 240decc2103f05c88769eee74196848d7cf5fa76 Mon Sep 17 00:00:00 2001 From: Kevin Dougan SmileCDR <72025369+KevinDougan-SmileCDR@users.noreply.github.com> Date: Tue, 19 Jan 2021 08:44:41 -0500 Subject: [PATCH 10/10] Added an extra check for an IS NOT NULL Constraint when checking an Oracle DB Column for Nullability (#2288) * Added an extra check for an IS NOT NULL Constraint when checking an Oracle DB Column for Nullability. --- .../jpa/migrate/taskdef/ModifyColumnTask.java | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ModifyColumnTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ModifyColumnTask.java index aab6be99287..cc499500b71 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ModifyColumnTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ModifyColumnTask.java @@ -22,10 +22,14 @@ package ca.uhn.fhir.jpa.migrate.taskdef; import ca.uhn.fhir.jpa.migrate.JdbcUtils; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; +import org.intellij.lang.annotations.Language; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.jdbc.core.ColumnMapRowMapper; import java.sql.SQLException; +import java.util.List; +import java.util.Map; import java.util.Set; public class ModifyColumnTask extends BaseTableColumnTypeTask { @@ -56,7 +60,7 @@ public class ModifyColumnTask extends BaseTableColumnTypeTask { try { existingType = JdbcUtils.getColumnType(getConnectionProperties(), getTableName(), getColumnName()); - nullable = JdbcUtils.isColumnNullable(getConnectionProperties(), getTableName(), getColumnName()); + nullable = isColumnNullable(getTableName(), getColumnName()); } catch (SQLException e) { throw new InternalErrorException(e); } @@ -152,4 +156,38 @@ public class ModifyColumnTask extends BaseTableColumnTypeTask { } } + private boolean isColumnNullable(String tableName, String columnName) throws SQLException { + boolean result = JdbcUtils.isColumnNullable(getConnectionProperties(), tableName, columnName); + // Oracle sometimes stores the NULLABLE property in a Constraint, so override the result if this is an Oracle DB + switch (getDriverType()) { + case ORACLE_12C: + @Language("SQL") String findNullableConstraintSql = + "SELECT acc.owner, acc.table_name, acc.column_name, search_condition_vc " + + "FROM all_cons_columns acc, all_constraints ac " + + "WHERE acc.constraint_name = ac.constraint_name " + + "AND acc.table_name = ac.table_name " + + "AND ac.constraint_type = ? " + + "AND acc.table_name = ? " + + "AND acc.column_name = ? " + + "AND search_condition_vc = ? "; + String[] params = new String[4]; + params[0] = "C"; + params[1] = tableName.toUpperCase(); + params[2] = columnName.toUpperCase(); + params[3] = "\"" + columnName.toUpperCase() + "\" IS NOT NULL"; + List> queryResults = getConnectionProperties().getTxTemplate().execute(t -> { + return getConnectionProperties().newJdbcTemplate().query(findNullableConstraintSql, params, new ColumnMapRowMapper()); + }); + // If this query returns a row then the existence of that row indicates that a NOT NULL constraint exists + // on this Column and we must override whatever result was previously calculated and set it to false + if (queryResults != null && queryResults.size() > 0 && queryResults.get(0) != null && !queryResults.get(0).isEmpty()) { + result = false; + } + break; + default: + // Do nothing since we already initialized the variable above + break; + } + return result; + } }