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 d83a1ce05d42..56cbd8dcc976 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 @@ -5,6 +5,9 @@ import ca.uhn.fhir.jpa.migrate.SchemaMigrator; import ca.uhn.fhir.jpa.migrate.dao.HapiMigrationDao; import ca.uhn.fhir.jpa.migrate.entity.HapiMigrationEntity; +import ca.uhn.fhir.jpa.migrate.SchemaMigrator; +import ca.uhn.fhir.jpa.migrate.dao.HapiMigrationDao; +import ca.uhn.fhir.jpa.migrate.entity.HapiMigrationEntity; import ca.uhn.fhir.jpa.util.RandomTextUtils; import ca.uhn.fhir.system.HapiSystemProperties; import com.google.common.base.Charsets; diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_10_1/upgrade.md b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_10_1/upgrade.md new file mode 100644 index 000000000000..701baf518163 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_10_1/upgrade.md @@ -0,0 +1,5 @@ +### Major Database Change + +This release contains a migration that covers every resource. +This may take several minutes on a larger system (e.g. 10 minutes for 100 million resources). +For zero-downtime, or for larger systems, we recommend you upgrade the schema using the CLI tools. diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_10_1/version.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_10_1/version.yaml new file mode 100644 index 000000000000..a29e249409a0 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_10_1/version.yaml @@ -0,0 +1,3 @@ +--- +release-date: "2023-08-31" +codename: "Zed" diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5486-cli-migrate-database-in-dry-run-modifies-db.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5486-cli-migrate-database-in-dry-run-modifies-db.yaml index f46b55475bd1..09409e91ff7b 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5486-cli-migrate-database-in-dry-run-modifies-db.yaml +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5486-cli-migrate-database-in-dry-run-modifies-db.yaml @@ -2,5 +2,6 @@ type: fix issue: 5486 jira: SMILE-7457 +backport: 6.10.1 title: "Previously, testing database migration with cli migrate-database command in dry-run mode would insert in the migration task table. The issue has been fixed." diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5511-oracle-migration-create-index.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5511-oracle-migration-create-index.yaml index b9eda74e6d71..2a4cd3e731da 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5511-oracle-migration-create-index.yaml +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5511-oracle-migration-create-index.yaml @@ -1,6 +1,7 @@ --- type: fix issue: 5511 +backport: 6.10.1 title: "Previously, when creating an index as a part of a migration, if the index already existed with a different name on Oracle, the migration would fail. This has been fixed so that the create index migration task now recovers with a warning message if the index already exists with a different name." diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5546-bad_force_id_spaces.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5546-bad_force_id_spaces.yaml new file mode 100644 index 000000000000..714061375129 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5546-bad_force_id_spaces.yaml @@ -0,0 +1,6 @@ +--- +type: fix +issue: 5546 +backport: 6.10.1 +title: "A database migration added trailing spaces to server-assigned resource ids. + This fix removes the bad migration, and adds another migration to fix the errors." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java index dcc7c9b9c6d1..c8ae2e315dcd 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java @@ -29,6 +29,7 @@ import ca.uhn.fhir.jpa.migrate.taskdef.CalculateOrdinalDatesTask; import ca.uhn.fhir.jpa.migrate.taskdef.ColumnTypeEnum; import ca.uhn.fhir.jpa.migrate.taskdef.ForceIdMigrationCopyTask; +import ca.uhn.fhir.jpa.migrate.taskdef.ForceIdMigrationFixTask; import ca.uhn.fhir.jpa.migrate.tasks.api.BaseMigrationTasks; import ca.uhn.fhir.jpa.migrate.tasks.api.Builder; import ca.uhn.fhir.jpa.model.config.PartitionSettings; @@ -140,10 +141,19 @@ protected void init700() { // Move forced_id constraints to hfj_resource and the new fhir_id column // Note: we leave the HFJ_FORCED_ID.IDX_FORCEDID_TYPE_FID index in place to support old writers for a while. - version.addTask(new ForceIdMigrationCopyTask(version.getRelease(), "20231018.1")); + version.addTask(new ForceIdMigrationCopyTask(version.getRelease(), "20231018.1").setDoNothing(true)); Builder.BuilderWithTableName hfjResource = version.onTable("HFJ_RESOURCE"); - hfjResource.modifyColumn("20231018.2", "FHIR_ID").nonNullable(); + // commented out to make numeric space for the fix task below. + // This constraint can't be enabled until the column is fully populated, and the shipped version of 20231018.1 + // was broken. + // hfjResource.modifyColumn("20231018.2", "FHIR_ID").nonNullable(); + + // this was inserted after the release. + version.addTask(new ForceIdMigrationFixTask(version.getRelease(), "20231018.3")); + + // added back in place of 20231018.2. If 20231018.2 already ran, this is a no-op. + hfjResource.modifyColumn("20231018.4", "FHIR_ID").nonNullable(); hfjResource.dropIndex("20231027.1", "IDX_RES_FHIR_ID"); hfjResource @@ -187,6 +197,8 @@ protected void init700() { "SP_URI".toLowerCase()), "Column HFJ_SPIDX_STRING.SP_VALUE_NORMALIZED already has a collation of 'C' so doing nothing"); } + + version.addTask(new ForceIdMigrationFixTask(version.getRelease(), "20231213.1")); } protected void init680() { diff --git a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/embedded/HapiSchemaMigrationTest.java b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/embedded/HapiSchemaMigrationTest.java index e6b023dd6e05..29642613f883 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/embedded/HapiSchemaMigrationTest.java +++ b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/embedded/HapiSchemaMigrationTest.java @@ -19,16 +19,18 @@ import org.junit.jupiter.params.provider.ArgumentsSource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.jdbc.core.JdbcTemplate; import org.testcontainers.junit.jupiter.Testcontainers; import javax.sql.DataSource; import java.sql.SQLException; import java.util.Collections; +import java.util.List; import java.util.Properties; -import java.util.Set; import static ca.uhn.fhir.jpa.embedded.HapiEmbeddedDatabasesExtension.FIRST_TESTED_VERSION; import static ca.uhn.fhir.jpa.migrate.SchemaMigrator.HAPI_FHIR_MIGRATION_TABLENAME; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -75,7 +77,7 @@ public void testMigration(DriverTypeEnum theDriverType) throws SQLException { VersionEnum[] allVersions = VersionEnum.values(); - Set dataVersions = Set.of( + List dataVersions = List.of( VersionEnum.V5_2_0, VersionEnum.V5_3_0, VersionEnum.V5_4_0, @@ -105,6 +107,8 @@ public void testMigration(DriverTypeEnum theDriverType) throws SQLException { new HapiForeignKeyIndexHelper() .ensureAllForeignKeysAreIndexed(dataSource); } + + verifyForcedIdMigration(dataSource); } private static void migrate(DriverTypeEnum theDriverType, DataSource dataSource, HapiMigrationStorageSvc hapiMigrationStorageSvc, VersionEnum from, VersionEnum to) throws SQLException { @@ -123,6 +127,19 @@ private static void migrate(DriverTypeEnum theDriverType, DataSource dataSource, schemaMigrator.migrate(); } + /** + * For bug https://github.com/hapifhir/hapi-fhir/issues/5546 + */ + private void verifyForcedIdMigration(DataSource theDataSource) throws SQLException { + JdbcTemplate jdbcTemplate = new JdbcTemplate(theDataSource); + @SuppressWarnings("DataFlowIssue") + int nullCount = jdbcTemplate.queryForObject("select count(1) from hfj_resource where fhir_id is null", Integer.class); + assertEquals(0, nullCount, "no fhir_id should be null"); + int trailingSpaceCount = jdbcTemplate.queryForObject("select count(1) from hfj_resource where fhir_id <> trim(fhir_id)", Integer.class); + assertEquals(0, trailingSpaceCount, "no fhir_id should contain a space"); + } + + @Test public void testCreateMigrationTableIfRequired() throws SQLException { // Setup diff --git a/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ForceIdMigrationCopyTask.java b/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ForceIdMigrationCopyTask.java index bdf1030c6395..6a71d0322adb 100644 --- a/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ForceIdMigrationCopyTask.java +++ b/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ForceIdMigrationCopyTask.java @@ -69,7 +69,7 @@ protected void doExecute() throws SQLException { "update hfj_resource " + "set fhir_id = coalesce( " + // use first non-null value: forced_id if present, otherwise res_id " (select f.forced_id from hfj_forced_id f where f.resource_pid = res_id), " - + " cast(res_id as char(64)) " + + " cast(res_id as varchar(64)) " + " ) " + "where fhir_id is null " + "and res_id >= ? and res_id < ?", diff --git a/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ForceIdMigrationFixTask.java b/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ForceIdMigrationFixTask.java new file mode 100644 index 000000000000..86e7e21139a4 --- /dev/null +++ b/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ForceIdMigrationFixTask.java @@ -0,0 +1,121 @@ +/*- + * #%L + * HAPI FHIR Server - SQL Migration + * %% + * Copyright (C) 2014 - 2023 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% + */ +package ca.uhn.fhir.jpa.migrate.taskdef; + +import org.apache.commons.lang3.builder.EqualsBuilder; +import org.apache.commons.lang3.builder.HashCodeBuilder; +import org.apache.commons.lang3.tuple.Pair; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.jdbc.core.JdbcTemplate; + +import java.sql.SQLException; + +/** + * Fix for bad version of {@link ForceIdMigrationCopyTask} + * The earlier migration had used at cast to char instead of varchar, which is space-padded on Oracle. + * This migration includes the copy action, but also adds a trim() call to fixup the bad server-assigned ids. + */ +public class ForceIdMigrationFixTask extends BaseTask { + private static final Logger ourLog = LoggerFactory.getLogger(ForceIdMigrationFixTask.class); + + public ForceIdMigrationFixTask(String theProductVersion, String theSchemaVersion) { + super(theProductVersion, theSchemaVersion); + } + + @Override + public void validate() { + // no-op + } + + @Override + protected void doExecute() throws SQLException { + logInfo(ourLog, "Starting: migrate fhir_id from hfj_forced_id to hfj_resource.fhir_id"); + + JdbcTemplate jdbcTemplate = newJdbcTemplate(); + + Pair range = jdbcTemplate.queryForObject( + "select min(RES_ID), max(RES_ID) from HFJ_RESOURCE", + (rs, rowNum) -> Pair.of(rs.getLong(1), rs.getLong(2))); + + if (range == null || range.getLeft() == null) { + logInfo(ourLog, "HFJ_RESOURCE is empty. No work to do."); + return; + } + + // run update in batches. + int rowsPerBlock = 50; // hfj_resource has roughly 50 rows per 8k block. + int batchSize = rowsPerBlock * 2000; // a few thousand IOPS gives a batch size around a second. + ourLog.info( + "About to migrate ids from {} to {} in batches of size {}", + range.getLeft(), + range.getRight(), + batchSize); + for (long batchStart = range.getLeft(); batchStart <= range.getRight(); batchStart = batchStart + batchSize) { + long batchEnd = batchStart + batchSize; + ourLog.info("Migrating client-assigned ids for pids: {}-{}", batchStart, batchEnd); + + /* + We have several cases. Two require no action: + 1. client-assigned id, with correct value in fhir_id and row in hfj_forced_id + 2. server-assigned id, with correct value in fhir_id, no row in hfj_forced_id + And three require action: + 3. client-assigned id, no value in fhir_id, but row in hfj_forced_id + 4. server-assigned id, no value in fhir_id, and row in hfj_forced_id + 5. bad migration - server-assigned id, with wrong space-padded value in fhir_id, no row in hfj_forced_id + */ + + executeSql( + "hfj_resource", + "update hfj_resource " + + // coalesce is varargs and chooses the first non-null value, like || + " set fhir_id = coalesce( " + + + // case 5. + " trim(fhir_id), " + + + // case 3 + " (select f.forced_id from hfj_forced_id f where f.resource_pid = res_id), " + + + // case 4 - use pid as fhir_id + " cast(res_id as varchar(64)) " + + " ) " + + + // avoid useless updates on engines that don't check + // skip case 1, 2. Only check 3,4,5 + " where (fhir_id is null or fhir_id <> trim(fhir_id)) " + + + // chunk range. + " and res_id >= ? and res_id < ?", + batchStart, + batchEnd); + } + } + + @Override + protected void generateHashCode(HashCodeBuilder theBuilder) { + // no-op - this is a singleton. + } + + @Override + protected void generateEquals(EqualsBuilder theBuilder, BaseTask theOtherObject) { + // no-op - this is a singleton. + } +} diff --git a/hapi-fhir-structures-dstu2.1/pom.xml b/hapi-fhir-structures-dstu2.1/pom.xml index dac3392fc7e1..0ca9230dac66 100644 --- a/hapi-fhir-structures-dstu2.1/pom.xml +++ b/hapi-fhir-structures-dstu2.1/pom.xml @@ -71,7 +71,7 @@ ${project.version} true - + commons-codec commons-codec