Ensure DropPrimaryKeyTask respects dry-run (#6062)

* Remove code to SQL query from BaseTask.  Move code to DropPrimaryKeyTask and gate it for NOT dry-run.

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

Commit code review suggestion

Co-authored-by: Michael Buckley <michaelabuckley@gmail.com>

* Code review feedback.

* Code review feedback.

* Remove isTransactional().

---------

Co-authored-by: Michael Buckley <michaelabuckley@gmail.com>

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/6061-drop-primary-key-task-doesnt-handle-dry-run.yaml

@@ -0,0 +1,6 @@
+---
+type: fix
+issue: 6061
+title: "The latest migration for HFJ_RES_SEARCH_URL introduced in [6033](https://github.com/hapifhir/hapi-fhir/issues/6033)
+        does not fully respect dry-run.
+        This has been fixed."

hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/embedded/HapiSchemaMigrationTest.java

@@ -26,7 +26,6 @@ import javax.sql.DataSource;
 import java.sql.Connection;
 import java.sql.DatabaseMetaData;
 import java.sql.ResultSet;
-import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
 import java.sql.Types;
 import java.util.ArrayList;

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

@@ -25,7 +25,6 @@ import ca.uhn.fhir.jpa.migrate.HapiMigrationException;
 import ca.uhn.fhir.jpa.migrate.tasks.api.TaskFlagEnum;
 import ca.uhn.fhir.system.HapiSystemProperties;
 import jakarta.annotation.Nonnull;
-import jakarta.annotation.Nullable;
 import org.apache.commons.lang3.Validate;
 import org.apache.commons.lang3.builder.EqualsBuilder;
 import org.apache.commons.lang3.builder.HashCodeBuilder;
@@ -37,7 +36,6 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.dao.DataAccessException;
 import org.springframework.jdbc.core.JdbcTemplate;
-import org.springframework.jdbc.core.ResultSetExtractor;
 import org.springframework.transaction.support.TransactionTemplate;
 
 import java.sql.SQLException;
@@ -176,17 +174,6 @@ public abstract class BaseTask {
 		captureExecutedStatement(theTableName, theSql, theArguments);
 	}
 
-	protected <T> T executeSqlWithResult(
-			@Language("SQL") String theSql, ResultSetExtractor<T> theResultSetExtractor, Object... theArguments) {
-		if (myTransactional) {
-			return getConnectionProperties()
-					.getTxTemplate()
-					.execute(t -> doExecuteSqlWithResult(theSql, theResultSetExtractor, theArguments));
-		}
-
-		return doExecuteSqlWithResult(theSql, theResultSetExtractor, theArguments);
-	}
-
 	protected void executeSqlListInTransaction(String theTableName, List<String> theSqlStatements) {
 		if (!isDryRun()) {
 			Integer changes;
@@ -232,32 +219,6 @@ public abstract class BaseTask {
 		}
 	}
 
-	@Nullable
-	private <T> T doExecuteSqlWithResult(
-			@Language("SQL") String theSql, ResultSetExtractor<T> theResultSetExtractor, Object... theArguments) {
-		final JdbcTemplate jdbcTemplate = getConnectionProperties().newJdbcTemplate();
-		// 0 means no timeout -- we use this for index rebuilds that may take time.
-		jdbcTemplate.setQueryTimeout(0);
-		try {
-			T result = jdbcTemplate.query(theSql, theResultSetExtractor, theArguments);
-			if (!HapiSystemProperties.isUnitTestModeEnabled()) {
-				logInfo(ourLog, "SQL \"{}\" returned result {}", theSql, result);
-			}
-			return result;
-		} catch (DataAccessException e) {
-			if (myFlags.contains(TaskFlagEnum.FAILURE_ALLOWED)) {
-				ourLog.info(
-						"Task {} did not exit successfully on doExecuteSqlWithResult(), but task is allowed to fail",
-						getMigrationVersion());
-				ourLog.debug("Error was: {}", e.getMessage(), e);
-				return null;
-			} else {
-				throw new HapiMigrationException(
-						Msg.code(2532) + "Failed during task " + getMigrationVersion() + ": " + e, e);
-			}
-		}
-	}
-
 	protected void captureExecutedStatement(
 			String theTableName, @Language("SQL") String theSql, Object... theArguments) {
 		myExecutedStatements.add(new ExecutedStatement(mySchemaVersion, theTableName, theSql, theArguments));

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

@@ -26,7 +26,6 @@ import jakarta.annotation.Nullable;
 import org.intellij.lang.annotations.Language;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.springframework.jdbc.core.ResultSetExtractor;
 
 import java.sql.SQLException;
 
@@ -51,28 +50,14 @@ public class DropPrimaryKeyTask extends BaseTableTask {
 	private String generateSql() {
 		ourLog.debug("DropPrimaryKeyTask.generateSql()");
 
-		final ResultSetExtractor<String> resultSetExtractor = rs -> {
-			if (rs.next()) {
-				final String singleResult = rs.getString(1);
-
-				if (rs.next()) {
-					throw new IllegalArgumentException(Msg.code(2533)
-							+ "Expecting only a single result for the table primary but got multiple for task: "
-							+ getMigrationVersion());
-				}
-
-				return singleResult;
-			}
-			return null;
-		};
-
 		@Nullable
 		@Language("SQL")
 		final String primaryKeyNameSql = generatePrimaryKeyNameSql();
 
 		@Nullable
 		final String primaryKeyName = primaryKeyNameSql != null
-				? executeSqlWithResult(primaryKeyNameSql, resultSetExtractor, getTableNameWithDatabaseExpectedCase())
+				? newJdbcTemplate()
+						.queryForObject(primaryKeyNameSql, String.class, getTableNameWithDatabaseExpectedCase())
 				: null;
 
 		ourLog.debug("primaryKeyName: {} for driver: {}", primaryKeyName, getDriverType());