Merge a6dea96fb5 into 3f6d1eb29b

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_6_0/6290-mssql-migration-trim-fix.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 6290
+title: "Previously, a specific migration task was using the `TRIM()` function, which does not exist in MSSQL 2012. This was causing migrations targeting MSSQL 2012 to fail. 
+This has been corrected and replaced with usage of a combination of LTRIM() and RTRIM(). Thanks to Primož Delopst at Better for the contribution!"

hapi-fhir-jpaserver-subscription/pom.xml

@@ -70,6 +70,11 @@
 			<artifactId>jakarta.servlet-api</artifactId>
 			<scope>provided</scope>
 		</dependency>
+		<dependency>
+			<groupId>jakarta.mail</groupId>
+			<artifactId>jakarta.mail-api</artifactId>
+			<optional>true</optional>
+		</dependency>
 
 		<!-- test dependencies -->
 		<dependency>

hapi-fhir-jpaserver-test-dstu2/src/test/java/ca/uhn/fhir/jpa/subscription/email/EmailSubscriptionDstu2Test.java

@@ -28,8 +28,8 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 
-import javax.mail.internet.InternetAddress;
-import javax.mail.internet.MimeMessage;
+import jakarta.mail.internet.InternetAddress;
+import jakarta.mail.internet.MimeMessage;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;

hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/subscription/email/EmailSenderImplTest.java

@@ -17,8 +17,8 @@ import org.junit.jupiter.api.extension.RegisterExtension;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.mail.internet.InternetAddress;
-import javax.mail.internet.MimeMessage;
+import jakarta.mail.internet.InternetAddress;
+import jakarta.mail.internet.MimeMessage;
 import java.util.Arrays;
 
 import static org.assertj.core.api.Assertions.assertThat;

hapi-fhir-jpaserver-test-dstu3/src/test/java/ca/uhn/fhir/jpa/subscription/email/EmailSubscriptionDstu3Test.java

@@ -26,8 +26,8 @@ import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.springframework.beans.factory.annotation.Autowired;
 
-import javax.mail.internet.InternetAddress;
-import javax.mail.internet.MimeMessage;
+import jakarta.mail.internet.InternetAddress;
+import jakarta.mail.internet.MimeMessage;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;

hapi-fhir-server-cds-hooks/pom.xml

@@ -79,25 +79,11 @@
 		<dependency>
 			<groupId>org.simplejavamail</groupId>
 			<artifactId>simple-java-mail</artifactId>
-			<!-- Excluded in favor of jakarta.activation:jakarta.activation-api -->
-			<exclusions>
-				<exclusion>
-					<groupId>com.sun.activation</groupId>
-					<artifactId>jakarta.activation</artifactId>
-				</exclusion>
-			</exclusions>
 		</dependency>
 		<dependency>
 			<groupId>com.icegreen</groupId>
 			<artifactId>greenmail</artifactId>
 			<scope>compile</scope>
-			<!-- Excluded in favor of jakarta.activation:jakarta.activation-api -->
-			<exclusions>
-				<exclusion>
-					<groupId>com.sun.activation</groupId>
-					<artifactId>jakarta.activation</artifactId>
-				</exclusion>
-			</exclusions>
 		</dependency>
 	</dependencies>
 

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/mail/IMailSvc.java

@@ -21,9 +21,9 @@ package ca.uhn.fhir.rest.server.mail;
 
 import jakarta.annotation.Nonnull;
 import org.simplejavamail.api.email.Email;
-import org.simplejavamail.api.mailer.AsyncResponse;
 
 import java.util.List;
+import java.util.function.Consumer;
 
 public interface IMailSvc {
 	void sendMail(@Nonnull List<Email> theEmails);
@@ -31,7 +31,5 @@ public interface IMailSvc {
 	void sendMail(@Nonnull Email theEmail);
 
 	void sendMail(
-			@Nonnull Email theEmail,
-			@Nonnull Runnable theOnSuccess,
-			@Nonnull AsyncResponse.ExceptionConsumer theErrorHandler);
+			@Nonnull Email theEmail, @Nonnull Runnable theOnSuccess, @Nonnull Consumer<Throwable> theErrorHandler);
 }

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/mail/MailSvc.java

@@ -20,12 +20,9 @@
 package ca.uhn.fhir.rest.server.mail;
 
 import jakarta.annotation.Nonnull;
-import org.apache.commons.lang3.Validate;
 import org.simplejavamail.MailException;
 import org.simplejavamail.api.email.Email;
 import org.simplejavamail.api.email.Recipient;
-import org.simplejavamail.api.mailer.AsyncResponse;
-import org.simplejavamail.api.mailer.AsyncResponse.ExceptionConsumer;
 import org.simplejavamail.api.mailer.Mailer;
 import org.simplejavamail.api.mailer.config.TransportStrategy;
 import org.simplejavamail.mailer.MailerBuilder;
@@ -33,6 +30,8 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.util.List;
+import java.util.Objects;
+import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
 public class MailSvc implements IMailSvc {
@@ -42,14 +41,14 @@ public class MailSvc implements IMailSvc {
 	private final Mailer myMailer;
 
 	public MailSvc(@Nonnull MailConfig theMailConfig) {
-		Validate.notNull(theMailConfig);
+		Objects.requireNonNull(theMailConfig);
 		myMailConfig = theMailConfig;
 		myMailer = makeMailer(myMailConfig);
 	}
 
 	@Override
 	public void sendMail(@Nonnull List<Email> theEmails) {
-		Validate.notNull(theEmails);
+		Objects.requireNonNull(theEmails);
 		theEmails.forEach(theEmail -> send(theEmail, new OnSuccess(theEmail), new ErrorHandler(theEmail)));
 	}
 
@@ -60,21 +59,23 @@ public class MailSvc implements IMailSvc {
 
 	@Override
 	public void sendMail(
-			@Nonnull Email theEmail, @Nonnull Runnable theOnSuccess, @Nonnull ExceptionConsumer theErrorHandler) {
+			@Nonnull Email theEmail, @Nonnull Runnable theOnSuccess, @Nonnull Consumer<Throwable> theErrorHandler) {
 		send(theEmail, theOnSuccess, theErrorHandler);
 	}
 
 	private void send(
-			@Nonnull Email theEmail, @Nonnull Runnable theOnSuccess, @Nonnull ExceptionConsumer theErrorHandler) {
-		Validate.notNull(theEmail);
-		Validate.notNull(theOnSuccess);
-		Validate.notNull(theErrorHandler);
+			@Nonnull Email theEmail, @Nonnull Runnable theOnSuccess, @Nonnull Consumer<Throwable> theErrorHandler) {
+		Objects.requireNonNull(theEmail);
+		Objects.requireNonNull(theOnSuccess);
+		Objects.requireNonNull(theErrorHandler);
 		try {
-			final AsyncResponse asyncResponse = myMailer.sendMail(theEmail, true);
-			if (asyncResponse != null) {
-				asyncResponse.onSuccess(theOnSuccess);
-				asyncResponse.onException(theErrorHandler);
-			}
+			myMailer.sendMail(theEmail, true).whenComplete((result, ex) -> {
+				if (ex != null) {
+					theErrorHandler.accept(ex);
+				} else {
+					theOnSuccess.run();
+				}
+			});
 		} catch (MailException e) {
 			theErrorHandler.accept(e);
 		}
@@ -117,7 +118,7 @@ public class MailSvc implements IMailSvc {
 		}
 	}
 
-	private class ErrorHandler implements ExceptionConsumer {
+	private class ErrorHandler implements Consumer<Throwable> {
 		private final Email myEmail;
 
 		private ErrorHandler(@Nonnull Email theEmail) {
@@ -125,7 +126,7 @@ public class MailSvc implements IMailSvc {
 		}
 
 		@Override
-		public void accept(Exception t) {
+		public void accept(Throwable t) {
 			ourLog.error("Email not sent" + makeMessage(myEmail), t);
 		}
 	}

hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/mail/MailSvcIT.java

@@ -4,6 +4,7 @@ import com.icegreen.greenmail.junit5.GreenMailExtension;
 import com.icegreen.greenmail.util.GreenMailUtil;
 import com.icegreen.greenmail.util.ServerSetupTest;
 import jakarta.annotation.Nonnull;
+import jakarta.mail.internet.MimeMessage;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -11,7 +12,6 @@ import org.simplejavamail.MailException;
 import org.simplejavamail.api.email.Email;
 import org.simplejavamail.email.EmailBuilder;
 
-import javax.mail.internet.MimeMessage;
 import java.util.Arrays;
 import java.util.List;
 
@@ -86,13 +86,14 @@ public class MailSvcIT {
 	@Test
 	public void testSendMailWithInvalidToAddressExpectErrorHandler() {
 		// setup
-		final Email email = withEmail("xyz");
+		String invalidEmailAdress = "xyz";
+		final Email email = withEmail(invalidEmailAdress);
 		// execute
 		fixture.sendMail(email,
 			() -> fail("Should not execute on Success"),
 			(e) -> {
 				assertTrue(e instanceof MailException);
-				assertEquals("Invalid TO address: " + email, e.getMessage());
+				assertEquals("Invalid TO address: " + invalidEmailAdress, e.getMessage());
 			});
 		// validate
 		assertTrue(ourGreenMail.waitForIncomingEmail(1000, 0));

hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/AddPrimaryKeyTask.java

@@ -24,9 +24,11 @@ import jakarta.annotation.Nonnull;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.sql.Connection;
 import java.sql.SQLException;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Optional;
 
 /**
  * Migration task that handles cross-database logic for adding a new primary key.
@@ -37,7 +39,7 @@ public class AddPrimaryKeyTask extends BaseTableTask {
 	private final List<String> myPrimaryKeyColumnsInOrder;
 
 	public AddPrimaryKeyTask(
-			String theProductVersion, String theSchemaVersion, String theTableName, String... theColumnsInOrder) {
+		String theProductVersion, String theSchemaVersion, String theTableName, String... theColumnsInOrder) {
 		super(theProductVersion, theSchemaVersion);
 		setTableName(theTableName);
 
@@ -46,32 +48,39 @@ public class AddPrimaryKeyTask extends BaseTableTask {
 
 	@Nonnull
 	private String generateSql() {
-		switch (getDriverType()) {
-			case MYSQL_5_7:
-			case MARIADB_10_1:
-			case POSTGRES_9_4:
-			case DERBY_EMBEDDED:
-			case H2_EMBEDDED:
-			case ORACLE_12C:
-			case MSSQL_2012:
-			case COCKROACHDB_21_1:
-				return String.format(
+		try (Connection connection = getConnectionProperties().getDataSource().getConnection()) {
+			switch (getDriverType()) {
+				case MYSQL_5_7:
+				case MARIADB_10_1:
+				case POSTGRES_9_4:
+				case DERBY_EMBEDDED:
+				case H2_EMBEDDED:
+				case ORACLE_12C:
+				case MSSQL_2012:
+				case COCKROACHDB_21_1:
+					return String.format(
 						"ALTER TABLE %s ADD PRIMARY KEY (%s)",
-						getTableName(), String.join(", ", myPrimaryKeyColumnsInOrder));
-			default:
-				throw new IllegalStateException(String.format(
+						Optional.of(connection.getSchema())
+							.map(schema -> String.format("%s.%s", schema, getTableName()))
+							.orElse(getTableName()),
+						String.join(", ", myPrimaryKeyColumnsInOrder));
+				default:
+					throw new IllegalStateException(String.format(
 						"%s Unknown driver type.  Cannot add primary key for task %s",
 						Msg.code(2531), getMigrationVersion()));
+			}
+		} catch (SQLException e) {
+			throw new IllegalStateException(e);
 		}
 	}
 
 	@Override
 	protected void doExecute() throws SQLException {
 		logInfo(
-				ourLog,
-				"Going to add a primary key on table {} for columns {}",
-				getTableName(),
-				myPrimaryKeyColumnsInOrder);
+			ourLog,
+			"Going to add a primary key on table {} for columns {}",
+			getTableName(),
+			myPrimaryKeyColumnsInOrder);
 
 		executeSql(getTableName(), generateSql());
 	}

hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropPrimaryKeyTask.java

@@ -20,14 +20,17 @@
 package ca.uhn.fhir.jpa.migrate.taskdef;
 
 import ca.uhn.fhir.i18n.Msg;
-import ca.uhn.fhir.jpa.migrate.DriverTypeEnum;
+import com.google.common.collect.ImmutableList;
 import jakarta.annotation.Nonnull;
 import jakarta.annotation.Nullable;
 import org.intellij.lang.annotations.Language;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.sql.Connection;
+import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.util.Optional;
 
 /**
  * Migration task that handles cross-database logic for dropping a primary key.
@@ -52,27 +55,13 @@ public class DropPrimaryKeyTask extends BaseTableTask {
 
 		@Nullable
 		@Language("SQL")
-		final String primaryKeyNameSql = generatePrimaryKeyNameSql();
-
-		@Nullable
-		final String primaryKeyName = primaryKeyNameSql != null
-				? newJdbcTemplate()
-						.queryForObject(primaryKeyNameSql, String.class, getTableNameWithDatabaseExpectedCase())
-				: null;
+		final String primaryKeyName = getPrimaryKeyName();
 
 		ourLog.debug("primaryKeyName: {} for driver: {}", primaryKeyName, getDriverType());
 
 		return generateDropPrimaryKeySql(primaryKeyName);
 	}
 
-	private String getTableNameWithDatabaseExpectedCase() {
-		if (DriverTypeEnum.ORACLE_12C == getDriverType()) {
-			return getTableName().toUpperCase();
-		}
-
-		return getTableName().toLowerCase();
-	}
-
 	@Override
 	protected void doExecute() throws SQLException {
 		logInfo(ourLog, "Going to DROP the PRIMARY KEY on table {}", getTableName());
@@ -81,56 +70,63 @@ public class DropPrimaryKeyTask extends BaseTableTask {
 	}
 
 	private String generateDropPrimaryKeySql(@Nullable String thePrimaryKeyName) {
-		switch (getDriverType()) {
-			case MARIADB_10_1:
-			case DERBY_EMBEDDED:
-			case H2_EMBEDDED:
-				@Language("SQL")
-				final String sqlH2 = "ALTER TABLE %s DROP PRIMARY KEY";
-				return String.format(sqlH2, getTableName());
-			case POSTGRES_9_4:
-			case ORACLE_12C:
-			case MSSQL_2012:
-			case MYSQL_5_7:
-				assert thePrimaryKeyName != null;
-				@Language("SQL")
-				final String sql = "ALTER TABLE %s DROP CONSTRAINT %s";
-				return String.format(sql, getTableName(), thePrimaryKeyName);
-			default:
-				throw new IllegalStateException(String.format(
+		try (Connection connection = getConnectionProperties().getDataSource().getConnection()) {
+			switch (getDriverType()) {
+				case MARIADB_10_1:
+				case DERBY_EMBEDDED:
+				case H2_EMBEDDED:
+					@Language("SQL")
+					final String sqlH2 = "ALTER TABLE %s DROP PRIMARY KEY";
+					return String.format(
+						sqlH2,
+						Optional.of(connection.getSchema())
+							.map(schema -> String.format("%s.%s", schema, getTableName()))
+							.orElse(getTableName()));
+				case POSTGRES_9_4:
+				case ORACLE_12C:
+				case MSSQL_2012:
+				case MYSQL_5_7:
+					assert thePrimaryKeyName != null;
+					@Language("SQL")
+					final String sql = "ALTER TABLE %s DROP CONSTRAINT %s";
+					return String.format(
+						sql,
+						Optional.of(connection.getSchema())
+							.map(schema -> String.format("%s.%s", schema, getTableName()))
+							.orElse(getTableName()),
+						thePrimaryKeyName);
+				default:
+					throw new IllegalStateException(String.format(
 						"%s Unknown driver type: %s.  Cannot drop primary key: %s for task %s",
 						Msg.code(2529), getDriverType(), getMigrationVersion(), getTableName()));
+			}
+
+		} catch (SQLException e) {
+			throw new IllegalStateException(e);
 		}
 	}
 
-	@Language("SQL")
+	@SuppressWarnings({"NestedTryStatement", "MethodWithMultipleLoops"})
 	@Nullable
-	private String generatePrimaryKeyNameSql() {
-		switch (getDriverType()) {
-			case MYSQL_5_7:
-			case MARIADB_10_1:
-			case DERBY_EMBEDDED:
-			case COCKROACHDB_21_1:
-			case H2_EMBEDDED:
-				return null; // Irrelevant:  We don't need to run the SQL for these databases.
-			case POSTGRES_9_4:
-				return "SELECT constraint_name " + "FROM information_schema.table_constraints "
-						+ "WHERE table_schema = 'public' "
-						+ "AND constraint_type = 'PRIMARY KEY' "
-						+ "AND table_name = ?";
-			case ORACLE_12C:
-				return "SELECT constraint_name " + "FROM user_constraints "
-						+ "WHERE constraint_type = 'P' "
-						+ "AND table_name = ?";
-			case MSSQL_2012:
-				return "SELECT tc.constraint_name " + "FROM information_schema.table_constraints tc "
-						+ "JOIN information_schema.constraint_column_usage ccu ON tc.constraint_name = ccu.constraint_name "
-						+ "WHERE tc.constraint_type = 'PRIMARY KEY' "
-						+ "AND  tc.table_name = ?";
-			default:
-				throw new IllegalStateException(String.format(
-						"%s Unknown driver type: %s  Cannot find primary key to drop for task %s",
-						Msg.code(2530), getDriverType(), getMigrationVersion()));
+	private String getPrimaryKeyName() {
+		String primaryKey = null;
+		try (Connection connection = getConnectionProperties().getDataSource().getConnection()) {
+			for (String tableName : ImmutableList.of(
+				getTableName().toLowerCase(), getTableName().toUpperCase())) {
+				try (ResultSet resultSet = connection
+					.getMetaData()
+					.getPrimaryKeys(connection.getCatalog(), connection.getSchema(), tableName)) {
+					while (resultSet.next()) {
+						primaryKey = resultSet.getString(6);
+					}
+				} catch (SQLException e) {
+					throw new IllegalStateException(e);
+				}
+			}
+		} catch (SQLException e) {
+			throw new IllegalStateException(e);
 		}
+
+		return primaryKey;
 	}
 }

pom.xml

@@ -869,6 +869,7 @@
 		<developer>
 			<id>delopst</id>
 			<name>Primož Delopst</name>
+			<organization>Better</organization>
 		</developer>
 		<developer>
 			<id>Zach Smith</id>
@@ -1160,27 +1161,38 @@
 			<dependency>
 				<groupId>org.simplejavamail</groupId>
 				<artifactId>simple-java-mail</artifactId>
-				<version>6.6.1</version>
+				<version>8.11.2</version>
 				<exclusions>
 					<exclusion>
-						<groupId>com.sun.activation</groupId>
-						<artifactId>jakarta.activation-api</artifactId>
+						<groupId>com.github.bbottema</groupId>
+						<artifactId>jetbrains-runtime-annotations</artifactId>
 					</exclusion>
 					<exclusion>
-						<groupId>com.sun.activation</groupId>
-						<artifactId>jakarta.activation</artifactId>
+						<groupId>jakarta.mail</groupId>
+						<artifactId>jakarta.mail-api</artifactId>
+					</exclusion>
+				</exclusions>
+			</dependency>
+			<dependency>
+				<groupId>jakarta.mail</groupId>
+				<artifactId>jakarta.mail-api</artifactId>
+				<version>2.1.3</version>
+			</dependency>
+			<dependency>
+				<groupId>com.icegreen</groupId>
+				<artifactId>greenmail</artifactId>
+				<version>2.1.0-rc-1</version>
+				<exclusions>
+					<exclusion>
+						<groupId>jakarta.mail</groupId>
+						<artifactId>jakarta.mail-api</artifactId>
 					</exclusion>
 				</exclusions>
 			</dependency>
-			<dependency>
-				<groupId>com.icegreen</groupId>
-				<artifactId>greenmail</artifactId>
-				<version>1.6.4</version>
-			</dependency>
 			<dependency>
 				<groupId>com.icegreen</groupId>
 				<artifactId>greenmail-junit5</artifactId>
-				<version>1.6.4</version>
+				<version>2.1.0-rc-1</version>
 				<scope>compile</scope>
 			</dependency>
 			<!-- mail end -->