honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SQL Migration: Support lowercase for SQL columns and overridden column type/driver type SQL type string rules (#5534) 

* 1. Allow callers to choose uppercase or lowercase column names.  Default uppercase.
2. Allow callers to pass custom sorting rules:  Default no special sorting.

* Spotless.

* Msg.code.

* Get rid of factory method and fix constructor.

* Maintain old constructor for backward compatibility.

* Fix JDK 11 compile error, spotless, and class header.

* Introduce a mechanism to override default SQL column mappings by column type and driver type.

* Add error message code.  Tweak column type algorithm.

* Replace javax.annotation with jakarta.annotation.

* Spotless.

* Fix unit test by restoring FOO_COLUMN assertion.

* Add changelog.

* Code review feedback.

* spotless

* Fix Msg.code().

* Add unit test.
This commit is contained in:
Luke deGruchy 2023-12-06 09:05:36 -05:00 committed by GitHub
parent f5cb674345
commit 5fa20438b6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 337 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5536-sql-migration-lowercase-and-custom-column-type-sql.yaml Normal file
View File

@ -0,0 +1,4 @@
---
type: add
issue: 5536
title: "In code: Support lowercase for SQL columns and overridden column type/driver type SQL type string rules"

4
hapi-fhir-sql-migrate/pom.xml
View File

@ -35,6 +35,10 @@
<groupId>org.hibernate.orm</groupId>
<artifactId>hibernate-core</artifactId>
</dependency>
<dependency>
<groupId>jakarta.annotation</groupId>
<artifactId>jakarta.annotation-api</artifactId>
</dependency>
<dependency>
<groupId>jakarta.transaction</groupId>
<artifactId>jakarta.transaction-api</artifactId>

12
hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/AddColumnTask.java
View File

@ -31,6 +31,10 @@ public class AddColumnTask extends BaseTableColumnTypeTask {
private static final Logger ourLog = LoggerFactory.getLogger(AddColumnTask.class);
public static AddColumnTask lowerCase(Set<ColumnDriverMappingOverride> theColumnDriverMappingOverrides) {
return new AddColumnTask(null, null, ColumnNameCase.ALL_LOWER, theColumnDriverMappingOverrides);
}
public AddColumnTask() {
this(null, null);
setDryRun(true);
@ -41,6 +45,14 @@ public class AddColumnTask extends BaseTableColumnTypeTask {
super(theProductVersion, theSchemaVersion);
}
private AddColumnTask(
String theProductVersion,
String theSchemaVersion,
ColumnNameCase theColumnNameCase,
Set<ColumnDriverMappingOverride> theColumnDriverMappingOverrides) {
super(theProductVersion, theSchemaVersion, theColumnNameCase, theColumnDriverMappingOverrides);
}
@Override
public void validate() {
super.validate();

25
hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/AddTableByColumnTask.java
View File

@ -29,7 +29,9 @@ import org.slf4j.LoggerFactory;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.stream.Collectors;
public class AddTableByColumnTask extends BaseTableTask {
@ -38,15 +40,26 @@ public class AddTableByColumnTask extends BaseTableTask {
private final List<AddColumnTask> myAddColumnTasks = new ArrayList<>();
private List<String> myPkColumns;
private final List<ForeignKeyContainer> myFKColumns = new ArrayList<>();
private final Comparator<AddColumnTask> myColumnSortingRules;
public AddTableByColumnTask() {
this(null, null);
this(null);
}
public AddTableByColumnTask(Comparator<AddColumnTask> theColumnSortingRules) {
this(null, null, theColumnSortingRules);
setDryRun(true);
myCheckForExistingTables = false;
}
public AddTableByColumnTask(String theProductVersion, String theSchemaVersion) {
this(theProductVersion, theSchemaVersion, null);
}
private AddTableByColumnTask(
String theProductVersion, String theSchemaVersion, Comparator<AddColumnTask> theColumnSortingRules) {
super(theProductVersion, theSchemaVersion);
myColumnSortingRules = theColumnSortingRules;
}
@Override
@ -83,7 +96,7 @@ public class AddTableByColumnTask extends BaseTableTask {
sb.append(" ");
}
for (AddColumnTask next : myAddColumnTasks) {
for (AddColumnTask next : getOrderedAddColumnTasks()) {
next.setDriverType(getDriverType());
next.setTableName(getTableName());
next.validate();
@ -194,4 +207,12 @@ public class AddTableByColumnTask extends BaseTableTask {
theBuilder.append(myAddColumnTasks);
theBuilder.append(myPkColumns);
}
private List<AddColumnTask> getOrderedAddColumnTasks() {
if (myColumnSortingRules == null) {
return myAddColumnTasks;
}
return myAddColumnTasks.stream().sorted(myColumnSortingRules).collect(Collectors.toUnmodifiableList());
}
}

28
hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTableColumnTask.java
View File

@ -19,12 +19,15 @@
*/
package ca.uhn.fhir.jpa.migrate.taskdef;
import ca.uhn.fhir.i18n.Msg;
import org.apache.commons.lang3.Validate;
import org.apache.commons.lang3.builder.EqualsBuilder;
import org.apache.commons.lang3.builder.HashCodeBuilder;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
public abstract class BaseTableColumnTask extends BaseTableTask {
@ -35,8 +38,19 @@ public abstract class BaseTableColumnTask extends BaseTableTask {
// If a concrete class decides to, they can define a custom WHERE clause for the task.
protected String myWhereClause;
private final ColumnNameCase myColumnNameCase;
public BaseTableColumnTask(String theProductVersion, String theSchemaVersion) {
super(theProductVersion, theSchemaVersion);
this(theProductVersion, theSchemaVersion, ColumnNameCase.ALL_UPPER, Collections.emptySet());
}
BaseTableColumnTask(
String theProductVersion,
String theSchemaVersion,
ColumnNameCase theColumnNameCase,
Set<ColumnDriverMappingOverride> theColumnDriverMappingOverrides) {
super(theProductVersion, theSchemaVersion, theColumnDriverMappingOverrides);
myColumnNameCase = theColumnNameCase;
}
public String getColumnName() {
@ -44,7 +58,17 @@ public abstract class BaseTableColumnTask extends BaseTableTask {
}
public BaseTableColumnTask setColumnName(String theColumnName) {
myColumnName = theColumnName.toUpperCase();
switch (myColumnNameCase) {
case ALL_UPPER:
myColumnName = theColumnName.toUpperCase();
break;
case ALL_LOWER:
myColumnName = theColumnName.toLowerCase();
break;
default:
throw new IllegalArgumentException(Msg.code(2448)
+ " Unknown ColumnNameCase was passed when setting column name case: " + myColumnNameCase);
}
return this;
}

11
hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTableColumnTypeTask.java
View File

@ -19,11 +19,12 @@
*/
package ca.uhn.fhir.jpa.migrate.taskdef;
import jakarta.annotation.Nullable;
import org.apache.commons.lang3.Validate;
import org.apache.commons.lang3.builder.EqualsBuilder;
import org.apache.commons.lang3.builder.HashCodeBuilder;
import javax.annotation.Nullable;
import java.util.Set;
public abstract class BaseTableColumnTypeTask extends BaseTableColumnTask {
private ColumnTypeEnum myColumnType;
@ -37,6 +38,14 @@ public abstract class BaseTableColumnTypeTask extends BaseTableColumnTask {
super(theProductVersion, theSchemaVersion);
}
BaseTableColumnTypeTask(
String theProductVersion,
String theSchemaVersion,
ColumnNameCase theColumnNameCase,
Set<ColumnDriverMappingOverride> theColumnDriverMappingOverrides) {
super(theProductVersion, theSchemaVersion, theColumnNameCase, theColumnDriverMappingOverrides);
}
public ColumnTypeEnum getColumnType() {
return myColumnType;
}

52
hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTableTask.java
View File

@ -19,17 +19,37 @@
*/
package ca.uhn.fhir.jpa.migrate.taskdef;
import ca.uhn.fhir.i18n.Msg;
import jakarta.annotation.Nonnull;
import org.apache.commons.lang3.Validate;
import org.apache.commons.lang3.builder.EqualsBuilder;
import org.apache.commons.lang3.builder.HashCodeBuilder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
public abstract class BaseTableTask extends BaseTask {
private static final Logger ourLog = LoggerFactory.getLogger(BaseTableTask.class);
private String myTableName;
private final List<ColumnDriverMappingOverride> myColumnDriverMappingOverrides;
public BaseTableTask(String theProductVersion, String theSchemaVersion) {
this(theProductVersion, theSchemaVersion, Collections.emptySet());
}
public BaseTableTask(
String theProductVersion,
String theSchemaVersion,
Set<ColumnDriverMappingOverride> theColumnDriverMappingOverrides) {
super(theProductVersion, theSchemaVersion);
myColumnDriverMappingOverrides = new ArrayList<>(theColumnDriverMappingOverrides);
}
public String getTableName() {
@ -54,13 +74,12 @@ public abstract class BaseTableTask extends BaseTask {
}
protected String getSqlType(ColumnTypeEnum theColumnType, Long theColumnLength) {
String retVal = ColumnTypeToDriverTypeToSqlType.getColumnTypeToDriverTypeToSqlType()
.get(theColumnType)
.get(getDriverType());
final String retVal = getColumnSqlWithToken(theColumnType);
Objects.requireNonNull(retVal);
if (theColumnType == ColumnTypeEnum.STRING) {
retVal = retVal.replace("?", Long.toString(theColumnLength));
return retVal.replace("?", Long.toString(theColumnLength));
}
return retVal;
@ -70,4 +89,29 @@ public abstract class BaseTableTask extends BaseTask {
protected void generateHashCode(HashCodeBuilder theBuilder) {
theBuilder.append(myTableName);
}
@Nonnull
private String getColumnSqlWithToken(ColumnTypeEnum theColumnType) {
final List<ColumnDriverMappingOverride> eligibleOverrides = myColumnDriverMappingOverrides.stream()
.filter(override -> override.getColumnType() == theColumnType)
.filter(override -> override.getDriverType() == getDriverType())
.collect(Collectors.toUnmodifiableList());
if (eligibleOverrides.size() > 1) {
ourLog.info("There is more than one eligible override: {}. Picking the first one", eligibleOverrides);
}
if (eligibleOverrides.size() == 1) {
return eligibleOverrides.get(0).getColumnTypeSql();
}
if (!ColumnTypeToDriverTypeToSqlType.getColumnTypeToDriverTypeToSqlType()
.containsKey(theColumnType)) {
throw new IllegalArgumentException(Msg.code(2449) + "Column type does not exist: " + theColumnType);
}
return ColumnTypeToDriverTypeToSqlType.getColumnTypeToDriverTypeToSqlType()
.get(theColumnType)
.get(getDriverType());
}
}

88
hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ColumnDriverMappingOverride.java Normal file
View File

@ -0,0 +1,88 @@
/*-
* #%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 ca.uhn.fhir.jpa.migrate.DriverTypeEnum;
import java.util.Objects;
import java.util.StringJoiner;
import javax.annotation.Nonnull;
/**
* Capture a single SQL column type text override to the logic in {@link ColumnDriverMappingOverride}, namely,
* by column type and driver type.
* <p/>
* Several overrides can be passed down together at the same time to override said logic.
*/
public class ColumnDriverMappingOverride {
private final ColumnTypeEnum myColumnType;
private final DriverTypeEnum myDriverType;
private final String myColumnTypeSql;
public ColumnDriverMappingOverride(
@Nonnull ColumnTypeEnum theColumnType,
@Nonnull DriverTypeEnum theDriverType,
@Nonnull String theColumnTypeSql) {
myColumnType = theColumnType;
myDriverType = theDriverType;
myColumnTypeSql = theColumnTypeSql;
}
public ColumnTypeEnum getColumnType() {
return myColumnType;
}
public DriverTypeEnum getDriverType() {
return myDriverType;
}
public String getColumnTypeSql() {
return myColumnTypeSql;
}
@Override
public boolean equals(Object theO) {
if (this == theO) {
return true;
}
if (theO == null || getClass() != theO.getClass()) {
return false;
}
ColumnDriverMappingOverride that = (ColumnDriverMappingOverride) theO;
return myColumnType == that.myColumnType
&& myDriverType == that.myDriverType
&& Objects.equals(myColumnTypeSql, that.myColumnTypeSql);
}
@Override
public int hashCode() {
return Objects.hash(myColumnType, myDriverType, myColumnTypeSql);
}
@Override
public String toString() {
return new StringJoiner(", ", ColumnDriverMappingOverride.class.getSimpleName() + "[", "]")
.add("myColumnType=" + myColumnType)
.add("myDriverType=" + myDriverType)
.add("myColumnTypeSql='" + myColumnTypeSql + "'")
.toString();
}
}

28
hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ColumnNameCase.java Normal file
View File

@ -0,0 +1,28 @@
/*-
* #%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;
/**
* Determine whether to display column names in upper case, lower case, or eventually some form of mixed case.
*/
public enum ColumnNameCase {
ALL_UPPER,
ALL_LOWER
}

94
hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddTableByColumnTaskTest.java
View File

@ -5,16 +5,20 @@ import ca.uhn.fhir.jpa.migrate.JdbcUtils;
import ca.uhn.fhir.jpa.migrate.tasks.api.BaseMigrationTasks;
import ca.uhn.fhir.jpa.migrate.tasks.api.Builder;
import ca.uhn.fhir.util.VersionEnum;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
import java.sql.SQLException;
import java.util.Collections;
import java.util.Comparator;
import java.util.Set;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.is;
public class AddTableByColumnTaskTest extends BaseTest {
@ -42,6 +46,96 @@ public class AddTableByColumnTaskTest extends BaseTest {
assertThat(indexes.toString(), indexes, containsInAnyOrder("IDX_BONJOUR"));
}
@Test
public void testLowercaseColumnsNoOverridesDefaultSorting() {
final String tableName = "table_3_columns";
final String columnName1 = "a_column";
final String columnName3 = "z_column";
final String columnNameId = "id";
final DriverTypeEnum driverType = DriverTypeEnum.MSSQL_2012;
final ColumnTypeEnum columnType = ColumnTypeEnum.STRING;
final AddTableByColumnTask addTableByColumnTask = new AddTableByColumnTask();
addTableByColumnTask.setTableName(tableName);
addTableByColumnTask.setDriverType(driverType);
addTableByColumnTask.setPkColumns(Collections.singletonList(columnNameId));
addTableByColumnTask.addAddColumnTask(buildAddColumnTask(driverType, columnType, tableName, columnName3, true, 10, Collections.emptySet()));
addTableByColumnTask.addAddColumnTask(buildAddColumnTask(driverType, columnType, tableName, columnNameId, false, 25, Collections.emptySet()));
addTableByColumnTask.addAddColumnTask(buildAddColumnTask(driverType, columnType, tableName, columnName1, true, 20, Collections.emptySet()));
final String actualCreateTableSql = addTableByColumnTask.generateSQLCreateScript();
assertThat("CREATE TABLE table_3_columns ( z_column varchar(10), id varchar(25) not null, a_column varchar(20), PRIMARY KEY (id) )", is(actualCreateTableSql));;
}
@Test
public void testLowercaseColumnsNvarcharOverrideDefaultSorting() {
final String tableName = "table_3_columns";
final String columnName1 = "a_column";
final String columnName3 = "z_column";
final String columnNameId = "id";
final DriverTypeEnum driverType = DriverTypeEnum.MSSQL_2012;
final ColumnTypeEnum columnType = ColumnTypeEnum.STRING;
final ColumnDriverMappingOverride override = new ColumnDriverMappingOverride(columnType, driverType, "nvarchar(?)");
final AddTableByColumnTask addTableByColumnTask = new AddTableByColumnTask();
addTableByColumnTask.setTableName(tableName);
addTableByColumnTask.setDriverType(driverType);
addTableByColumnTask.setPkColumns(Collections.singletonList(columnNameId));
addTableByColumnTask.addAddColumnTask(buildAddColumnTask(driverType, columnType, tableName, columnName3, true, 10, Collections.singleton(override)));
addTableByColumnTask.addAddColumnTask(buildAddColumnTask(driverType, columnType, tableName, columnNameId, false, 25, Collections.singleton(override)));
addTableByColumnTask.addAddColumnTask(buildAddColumnTask(driverType, columnType, tableName, columnName1, true, 20, Collections.singleton(override)));
final String actualCreateTableSql = addTableByColumnTask.generateSQLCreateScript();
assertThat("CREATE TABLE table_3_columns ( z_column nvarchar(10), id nvarchar(25) not null, a_column nvarchar(20), PRIMARY KEY (id) )", is(actualCreateTableSql));;
}
@Test
public void testLowercaseColumnsNoOverridesCustomSorting() {
final String tableName = "table_4_columns";
final String columnName1 = "a_column";
final String columnName2 = "b_column";
final String columnName3 = "z_column";
final String columnNameId = "id";
final DriverTypeEnum driverType = DriverTypeEnum.MSSQL_2012;
final ColumnTypeEnum columnType = ColumnTypeEnum.STRING;
final ColumnDriverMappingOverride override = new ColumnDriverMappingOverride(columnType, driverType, "nvarchar(?)");
final Comparator<AddColumnTask> comparator = (theTask1, theTask2) -> {
if (columnNameId.equals(theTask1.getColumnName())) {
return -1;
}
return theTask1.getColumnName().compareTo(theTask2.getColumnName());
};
final AddTableByColumnTask addTableByColumnTask = new AddTableByColumnTask(comparator);
addTableByColumnTask.setTableName(tableName);
addTableByColumnTask.setDriverType(driverType);
addTableByColumnTask.setPkColumns(Collections.singletonList(columnNameId));
addTableByColumnTask.addAddColumnTask(buildAddColumnTask(driverType, columnType, tableName, columnName3, true, 10, Collections.singleton(override)));
addTableByColumnTask.addAddColumnTask(buildAddColumnTask(driverType, columnType, tableName, columnName2, false, 15, Collections.singleton(override)));
addTableByColumnTask.addAddColumnTask(buildAddColumnTask(driverType, columnType, tableName, columnName1, true, 20, Collections.singleton(override)));
addTableByColumnTask.addAddColumnTask(buildAddColumnTask(driverType, columnType, tableName, columnNameId, false, 25, Collections.singleton(override)));
final String actualCreateTableSql = addTableByColumnTask.generateSQLCreateScript();
assertThat("CREATE TABLE table_4_columns ( id nvarchar(25) not null, a_column nvarchar(20), b_column nvarchar(15) not null, z_column nvarchar(10), PRIMARY KEY (id) )", is(actualCreateTableSql));;
}
private static AddColumnTask buildAddColumnTask(DriverTypeEnum theDriverTypeEnum, ColumnTypeEnum theColumnTypeEnum, String theTableName, String theColumnName, boolean theNullable, int theColumnLength, Set<ColumnDriverMappingOverride> theColumnDriverMappingOverrides) {
final AddColumnTask task = AddColumnTask.lowerCase(theColumnDriverMappingOverrides);
task.setTableName(theTableName);
task.setColumnName(theColumnName);
task.setColumnType(theColumnTypeEnum);
task.setDriverType(theDriverTypeEnum);
task.setNullable(theNullable);
task.setColumnLength(theColumnLength);
return task;
}
private static class MyMigrationTasks extends BaseMigrationTasks<VersionEnum> {
public MyMigrationTasks() {
Builder v = forVersion(VersionEnum.V3_5_0);