diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/6079-term-concept-designation-val-over-2000-chars.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/6079-term-concept-designation-val-over-2000-chars.yaml new file mode 100644 index 00000000000..5cf9c32f84d --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/6079-term-concept-designation-val-over-2000-chars.yaml @@ -0,0 +1,5 @@ +--- +type: add +issue: 6079 +jira: SMILE-8644 +title: "It is now possible to save TermConceptDesignations with values over 2,000 characters." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConceptDesignation.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConceptDesignation.java index 10181fe1379..28e0b3e86a4 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConceptDesignation.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermConceptDesignation.java @@ -35,8 +35,10 @@ import jakarta.persistence.SequenceGenerator; import jakarta.persistence.Table; import org.apache.commons.lang3.builder.ToStringBuilder; import org.apache.commons.lang3.builder.ToStringStyle; +import org.hibernate.Length; import java.io.Serializable; +import java.util.Objects; import static org.apache.commons.lang3.StringUtils.left; import static org.apache.commons.lang3.StringUtils.length; @@ -82,8 +84,11 @@ public class TermConceptDesignation implements Serializable { @Column(name = "USE_DISPLAY", nullable = true, length = MAX_LENGTH) private String myUseDisplay; - @Column(name = "VAL", nullable = false, length = MAX_VAL_LENGTH) + @Column(name = "VAL", nullable = true, length = MAX_VAL_LENGTH) private String myValue; + + @Column(name = "VAL_VC", nullable = true, length = Length.LONG32) + private String myValueVc; /** * TODO: Make this non-null * @@ -144,14 +149,11 @@ public class TermConceptDesignation implements Serializable { } public String getValue() { - return myValue; + return Objects.nonNull(myValueVc) ? myValueVc : myValue; } - public TermConceptDesignation setValue(@Nonnull String theValue) { - ValidateUtil.isNotBlankOrThrowIllegalArgument(theValue, "theValue must not be null or empty"); - ValidateUtil.isNotTooLongOrThrowIllegalArgument( - theValue, MAX_VAL_LENGTH, "Value exceeds maximum length (" + MAX_VAL_LENGTH + "): " + length(theValue)); - myValue = theValue; + public TermConceptDesignation setValue(@Nonnull String theValueVc) { + myValueVc = theValueVc; return this; } 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 258d8122008..88516c9faed 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 @@ -440,6 +440,18 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { t.getString("IDX_STRING"))) .setColumnName("HASH_COMPLETE")); } + + { + version.onTable("TRM_CONCEPT_DESIG") + .modifyColumn("20240705.10", "VAL") + .nullable() + .withType(ColumnTypeEnum.STRING, 2000); + + version.onTable("TRM_CONCEPT_DESIG") + .addColumn("20240705.20", "VAL_VC") + .nullable() + .type(ColumnTypeEnum.TEXT); + } } protected void init720() { diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TerminologyTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TerminologyTest.java index 391f7f0b58c..fac5f68ef0c 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TerminologyTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TerminologyTest.java @@ -6,6 +6,7 @@ import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; import ca.uhn.fhir.jpa.entity.TermCodeSystemVersion; import ca.uhn.fhir.jpa.entity.TermConcept; +import ca.uhn.fhir.jpa.entity.TermConceptDesignation; import ca.uhn.fhir.jpa.entity.TermConceptParentChildLink.RelationshipTypeEnum; import ca.uhn.fhir.jpa.model.dao.JpaPid; import ca.uhn.fhir.jpa.model.entity.ResourceTable; @@ -48,8 +49,10 @@ import java.util.Date; import java.util.List; import java.util.Set; import java.util.stream.Collectors; +import java.util.stream.IntStream; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.fail; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -1374,6 +1377,27 @@ public class FhirResourceDaoR4TerminologyTest extends BaseJpaR4Test { } + @Test + void termConceptDesignationOver2000CharVal() { + final String stringWith8000Chars = IntStream.range(0, 8000) + .mapToObj(anInt -> "B") + .collect(Collectors.joining()); + + final TermConceptDesignation termConceptDesignation8000Chars = new TermConceptDesignation() + .setValue(stringWith8000Chars); + myTermConceptDesignationDao.save(termConceptDesignation8000Chars); + + final List allTermConceptDesignations = myTermConceptDesignationDao.findAll(); + + assertThat(allTermConceptDesignations) + .hasSize(1); + + final TermConceptDesignation termConceptDesignationFromDb = allTermConceptDesignations.get(0); + + assertThat(termConceptDesignationFromDb.getValue()) + .isEqualTo(stringWith8000Chars); + } + private ArrayList toCodesContains(List theContains) { ArrayList retVal = new ArrayList<>(); for (ValueSetExpansionContainsComponent next : theContains) { 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 b63ffc46b60..48e264da584 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 @@ -36,6 +36,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; 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; @@ -58,10 +59,14 @@ public class HapiSchemaMigrationTest { private static final String METADATA_IS_NULLABLE_YES = "YES"; private static final String TABLE_HFJ_RES_SEARCH_URL = "HFJ_RES_SEARCH_URL"; + private static final String TABLE_TRM_CONCEPT_DESIG = "TRM_CONCEPT_DESIG"; private static final String COLUMN_RES_SEARCH_URL = "RES_SEARCH_URL"; private static final String COLUMN_PARTITION_ID = "PARTITION_ID"; private static final String COLUMN_PARTITION_DATE = "PARTITION_DATE"; + private static final String COLUMN_VAL = "VAL"; + private static final String COLUMN_VAL_VC = "VAL_VC"; + private static final String NULL_PLACEHOLDER = "[NULL]"; static { @@ -120,6 +125,8 @@ public class HapiSchemaMigrationTest { verifyForcedIdMigration(dataSource); verifyHfjResSearchUrlMigration(database, theDriverType); + + verifyTrm_Concept_Desig(database, theDriverType); } /** @@ -176,8 +183,6 @@ public class HapiSchemaMigrationTest { } } - ourLog.info("6145: actualColumnResults: {}", actualColumnResults); - final List> actualPrimaryKeyResults = new ArrayList<>(); try (final ResultSet primaryKeyResultSet = tableMetaData.getPrimaryKeys(null, null, TABLE_HFJ_RES_SEARCH_URL)) { @@ -207,6 +212,61 @@ public class HapiSchemaMigrationTest { } } + private void verifyTrm_Concept_Desig(JpaEmbeddedDatabase theDatabase, DriverTypeEnum theDriverType) throws SQLException { + final List> allCount = theDatabase.query(String.format("SELECT count(*) FROM %s", TABLE_TRM_CONCEPT_DESIG)); + final List> nonNullValCount = theDatabase.query(String.format("SELECT count(*) FROM %s WHERE %s IS NOT NULL", TABLE_TRM_CONCEPT_DESIG, COLUMN_VAL)); + final List> nullValVcCount = theDatabase.query(String.format("SELECT count(*) FROM %s WHERE %s IS NULL", TABLE_TRM_CONCEPT_DESIG, COLUMN_VAL_VC)); + + assertThat(nonNullValCount).hasSize(1); + final Collection queryResultValuesVal = nonNullValCount.get(0).values(); + assertThat(queryResultValuesVal).hasSize(1); + final Object queryResultValueVal = queryResultValuesVal.iterator().next(); + assertThat(queryResultValueVal).isInstanceOf(Number.class); + if (queryResultValueVal instanceof Number queryResultNumber) { + assertThat(queryResultNumber.intValue()).isEqualTo(1); + } + + assertThat(nullValVcCount).hasSize(1); + final Collection queryResultValuesValVc = nullValVcCount.get(0).values(); + assertThat(queryResultValuesValVc).hasSize(1); + final Object queryResultValueValVc = queryResultValuesValVc.iterator().next(); + assertThat(queryResultValueValVc).isInstanceOf(Number.class); + if (queryResultValueValVc instanceof Number queryResultNumber) { + assertThat(queryResultNumber.intValue()).isEqualTo(1); + } + + final Object allCountValue = allCount.get(0).values().iterator().next(); + if (allCountValue instanceof Number allCountNumber) { + assertThat(allCountNumber.intValue()).isEqualTo(1); + } + + try (final Connection connection = theDatabase.getDataSource().getConnection()) { + final DatabaseMetaData tableMetaData = connection.getMetaData(); + + final List> actualColumnResults = new ArrayList<>(); + try (final ResultSet columnsResultSet = tableMetaData.getColumns(null, null, getTableNameWithDbSpecificCase(theDriverType, TABLE_TRM_CONCEPT_DESIG), null)) { + while (columnsResultSet.next()) { + final Map columnMap = new HashMap<>(); + actualColumnResults.add(columnMap); + + extractAndAddToMap(columnsResultSet, columnMap, METADATA_COLUMN_NAME); + extractAndAddToMap(columnsResultSet, columnMap, METADATA_DATA_TYPE); + extractAndAddToMap(columnsResultSet, columnMap, METADATA_IS_NULLABLE); + extractAndAddToMap(columnsResultSet, columnMap, METADATA_DEFAULT_VALUE); + } + + assertThat(actualColumnResults).contains(addExpectedColumnMetadata(COLUMN_VAL, Integer.toString(Types.VARCHAR), METADATA_IS_NULLABLE_YES, null)); + assertThat(actualColumnResults).contains(addExpectedColumnMetadata(COLUMN_VAL_VC, getExpectedSqlTypeForValVc(theDriverType), METADATA_IS_NULLABLE_YES, null)); + } + } + } + + private String getTableNameWithDbSpecificCase(DriverTypeEnum theDriverType, String theTableName) { + return Set.of(DriverTypeEnum.ORACLE_12C, DriverTypeEnum.H2_EMBEDDED).contains(theDriverType) + ? theTableName + : theTableName.toLowerCase(); + } + @Nonnull private Map addExpectedColumnMetadata(String theColumnName, String theDataType, String theNullable, @Nullable String theDefaultValue) { return Map.of(METADATA_COLUMN_NAME, theColumnName, @@ -234,6 +294,12 @@ public class HapiSchemaMigrationTest { : Integer.toString(Types.DATE); } + private String getExpectedSqlTypeForValVc(DriverTypeEnum theDriverType) { + return Set.of(DriverTypeEnum.ORACLE_12C, DriverTypeEnum.H2_EMBEDDED).contains(theDriverType) + ? Integer.toString(Types.CLOB) + : Integer.toString(Types.VARCHAR); + } + private void extractAndAddToMap(ResultSet theResultSet, Map theMap, String theColumn) throws SQLException { theMap.put(theColumn, Optional.ofNullable(theResultSet.getString(theColumn)) .map(defaultValueNonNull -> defaultValueNonNull.equals("((-1))") ? "-1" : defaultValueNonNull) // MSSQL returns "((-1))" for default value diff --git a/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/HapiMigrator.java b/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/HapiMigrator.java index 17ee41c8032..59ab2b85eb2 100644 --- a/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/HapiMigrator.java +++ b/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/HapiMigrator.java @@ -211,7 +211,8 @@ public class HapiMigrator { if (isBlank(description)) { description = theTask.getClass().getSimpleName(); } - String prefix = "Failure executing task \"" + description + "\", aborting! Cause: "; + String prefix = String.format( + "Failure executing task '%s', for driver: %s, aborting! Cause: ", description, getDriverType()); throw new HapiMigrationException(Msg.code(47) + prefix + e, theMigrationResult, e); } } diff --git a/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddColumnTest.java b/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddColumnTest.java index d63e28684d6..0a2f9e910ce 100644 --- a/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddColumnTest.java +++ b/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddColumnTest.java @@ -1,5 +1,7 @@ package ca.uhn.fhir.jpa.migrate.taskdef; +import ca.uhn.fhir.i18n.Msg; +import ca.uhn.fhir.jpa.migrate.DriverTypeEnum; import ca.uhn.fhir.jpa.migrate.HapiMigrationException; import ca.uhn.fhir.jpa.migrate.JdbcUtils; import ca.uhn.fhir.jpa.migrate.tasks.api.BaseMigrationTasks; @@ -77,7 +79,7 @@ public class AddColumnTest extends BaseTest { @ParameterizedTest(name = "{index}: {0}") @MethodSource("data") public void testAddColumnToNonExistentTable_Failing(Supplier theTestDatabaseDetails) { - before(theTestDatabaseDetails); + final DriverTypeEnum driverType = before(theTestDatabaseDetails); BaseMigrationTasks tasks = new BaseMigrationTasks<>(); tasks @@ -92,7 +94,12 @@ public class AddColumnTest extends BaseTest { getMigrator().migrate(); fail(); } catch (HapiMigrationException e) { - assertThat(e.getMessage()).startsWith("HAPI-0047: Failure executing task \"Add column FOO_COLUMN on table FOO_TABLE\", aborting! Cause: ca.uhn.fhir.jpa.migrate.HapiMigrationException: HAPI-0061: Failed during task 4.0.0.2001.01: "); + final String expectedError = + String.format("%sFailure executing task 'Add column FOO_COLUMN on table FOO_TABLE', for driver: %s, aborting! Cause: ca.uhn.fhir.jpa.migrate.HapiMigrationException: %sFailed during task 4.0.0.2001.01: ", + Msg.code(47), + driverType.name(), + Msg.code(61)); + assertThat(e.getMessage()).startsWith(expectedError); } } diff --git a/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTest.java b/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTest.java index 2bc06ae4775..c49da5137a3 100644 --- a/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTest.java +++ b/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTest.java @@ -107,7 +107,7 @@ public abstract class BaseTest { return retVal.stream(); } - public void before(Supplier theTestDatabaseDetails) { + public DriverTypeEnum before(Supplier theTestDatabaseDetails) { TestDatabaseDetails testDatabaseDetails = theTestDatabaseDetails.get(); myUrl = testDatabaseDetails.myUrl; myConnectionProperties = testDatabaseDetails.myConnectionProperties; @@ -117,6 +117,8 @@ public abstract class BaseTest { myHapiMigrationDao.createMigrationTableIfRequired(); myHapiMigrationStorageSvc = new HapiMigrationStorageSvc(myHapiMigrationDao); + + return testDatabaseDetails.getDriverType(); } public String getUrl() { diff --git a/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/RenameColumnTaskTest.java b/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/RenameColumnTaskTest.java index 08996c0d715..8896231bb10 100644 --- a/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/RenameColumnTaskTest.java +++ b/hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/RenameColumnTaskTest.java @@ -1,6 +1,7 @@ package ca.uhn.fhir.jpa.migrate.taskdef; import ca.uhn.fhir.i18n.Msg; +import ca.uhn.fhir.jpa.migrate.DriverTypeEnum; import ca.uhn.fhir.jpa.migrate.HapiMigrationException; import ca.uhn.fhir.jpa.migrate.JdbcUtils; import org.junit.jupiter.params.ParameterizedTest; @@ -11,7 +12,6 @@ import java.util.Set; import java.util.function.Supplier; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; public class RenameColumnTaskTest extends BaseTest { @@ -148,7 +148,7 @@ public class RenameColumnTaskTest extends BaseTest { @ParameterizedTest(name = "{index}: {0}") @MethodSource("data") public void testBothExistDeleteTargetFirstDataExistsInSourceAndTarget(Supplier theTestDatabaseDetails) { - before(theTestDatabaseDetails); + final DriverTypeEnum driverType = before(theTestDatabaseDetails); executeSql("create table SOMETABLE (PID bigint not null, TEXTCOL varchar(255), myTextCol varchar(255))"); executeSql("INSERT INTO SOMETABLE (PID, TEXTCOL, myTextCol) VALUES (123, 'AAA', 'BBB')"); @@ -165,7 +165,12 @@ public class RenameColumnTaskTest extends BaseTest { getMigrator().migrate(); fail(); } catch (HapiMigrationException e) { - assertEquals(Msg.code(47) + "Failure executing task \"Drop an index\", aborting! Cause: java.sql.SQLException: " + Msg.code(54) + "Can not rename SOMETABLE.myTextCol to TEXTCOL because both columns exist and data exists in TEXTCOL", e.getMessage()); + final String expectedError = + String.format("%sFailure executing task 'Drop an index', for driver: %s, aborting! Cause: java.sql.SQLException: %sCan not rename SOMETABLE.myTextCol to TEXTCOL because both columns exist and data exists in TEXTCOL", + Msg.code(47), + driverType.name(), + Msg.code(54)); + assertThat(e.getMessage()).isEqualTo(expectedError); } } @@ -234,7 +239,7 @@ public class RenameColumnTaskTest extends BaseTest { @ParameterizedTest(name = "{index}: {0}") @MethodSource("data") public void testNeitherColumnExists(Supplier theTestDatabaseDetails) { - before(theTestDatabaseDetails); + final DriverTypeEnum driverType = before(theTestDatabaseDetails); executeSql("create table SOMETABLE (PID bigint not null)"); @@ -248,7 +253,12 @@ public class RenameColumnTaskTest extends BaseTest { getMigrator().migrate(); fail(); } catch (HapiMigrationException e) { - assertEquals(Msg.code(47) + "Failure executing task \"RenameColumnTask\", aborting! Cause: java.sql.SQLException: " + Msg.code(56) + "Can not rename SOMETABLE.myTextCol to TEXTCOL because neither column exists!", e.getMessage()); + final String expectedError = + String.format("%sFailure executing task 'RenameColumnTask', for driver: %s, aborting! Cause: java.sql.SQLException: %sCan not rename SOMETABLE.myTextCol to TEXTCOL because neither column exists!", + Msg.code(47), + driverType.name(), + Msg.code(56)); + assertThat(e.getMessage()).isEqualTo(expectedError); } @@ -274,7 +284,7 @@ public class RenameColumnTaskTest extends BaseTest { @ParameterizedTest(name = "{index}: {0}") @MethodSource("data") public void testBothColumnsExist(Supplier theTestDatabaseDetails) { - before(theTestDatabaseDetails); + final DriverTypeEnum driverType = before(theTestDatabaseDetails); executeSql("create table SOMETABLE (PID bigint not null, PID2 bigint)"); @@ -288,7 +298,12 @@ public class RenameColumnTaskTest extends BaseTest { getMigrator().migrate(); fail(); } catch (HapiMigrationException e) { - assertEquals(Msg.code(47) + "Failure executing task \"RenameColumnTask\", aborting! Cause: java.sql.SQLException: " + Msg.code(55) + "Can not rename SOMETABLE.PID to PID2 because both columns exist!", e.getMessage()); + final String expectedError = + String.format("%sFailure executing task 'RenameColumnTask', for driver: %s, aborting! Cause: java.sql.SQLException: %sCan not rename SOMETABLE.PID to PID2 because both columns exist!", + Msg.code(47), + driverType.name(), + Msg.code(55)); + assertThat(e.getMessage()).isEqualTo(expectedError); }