Do not retain partial column metadata in SimpleJdbcInsert

Prior to this commit, if an SQLException was thrown while retrieving
column metadata from the database, SimpleJdbcInsert would generate an
INSERT statement that was syntactically valid but missing columns,
which could lead to data silently missing in the database (for nullable
columns).

This commit fixes this by clearing all collected column metadata if an
SQLException is thrown while processing the metadata. The result is
that an InvalidDataAccessApiUsageException will be thrown later while
generating the INSERT statement. The exception message now contains an
additional hint to make use of SimpleJdbcInsert#usingColumns() in order
to ensure that all required columns are included in the generated
INSERT statement.

SimpleJdbcCall can also encounter an SQLException while retrieving
column metadata for a stored procedure/function, but an exception is
not thrown since a later invocation of the stored procedure/function
will likely fail anyway due to missing arguments. Consequently, this
commit only improves the warning level log message by including a hint
to make use of SimpleJdbcCall#addDeclaredParameter().

Closes gh-26486

spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/GenericCallMetaDataProvider.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2021 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -39,6 +39,7 @@ import org.springframework.util.StringUtils;
  *
  * @author Thomas Risberg
  * @author Juergen Hoeller
+ * @author Sam Brannen
  * @since 2.5
  */
 public class GenericCallMetaDataProvider implements CallMetaDataProvider {
@@ -414,8 +415,15 @@ public class GenericCallMetaDataProvider implements CallMetaDataProvider {
 		}
 		catch (SQLException ex) {
 			if (logger.isWarnEnabled()) {
-				logger.warn("Error while retrieving meta-data for procedure columns: " + ex);
+				logger.warn("Error while retrieving meta-data for procedure columns. " +
+						"Consider declaring explicit parameters -- for example, via SimpleJdbcCall#addDeclaredParameter().",
+						ex);
 			}
+			// Although we could invoke `this.callParameterMetaData.clear()` so that
+			// we don't retain a partial list of column names (like we do in
+			// GenericTableMetaDataProvider.processTableColumns(...)), we choose
+			// not to do that here, since invocation of the stored procedure will
+			// likely fail anyway with an incorrect argument list.
 		}
 	}
 

spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/GenericTableMetaDataProvider.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2021 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -39,6 +39,7 @@ import org.springframework.lang.Nullable;
  *
  * @author Thomas Risberg
  * @author Juergen Hoeller
+ * @author Sam Brannen
  * @since 2.5
  */
 public class GenericTableMetaDataProvider implements TableMetaDataProvider {
@@ -422,8 +423,12 @@ public class GenericTableMetaDataProvider implements TableMetaDataProvider {
 		}
 		catch (SQLException ex) {
 			if (logger.isWarnEnabled()) {
-				logger.warn("Error while retrieving meta-data for table columns: " + ex.getMessage());
+				logger.warn("Error while retrieving meta-data for table columns. " +
+						"Consider specifying explicit column names -- for example, via SimpleJdbcInsert#usingColumns().",
+						ex);
 			}
+			// Clear the metadata so that we don't retain a partial list of column names
+			this.tableParameterMetaData.clear();
 		}
 		finally {
 			JdbcUtils.closeResultSet(tableColumns);

spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/TableMetaDataContext.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2021 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -43,6 +43,7 @@ import org.springframework.util.Assert;
  *
  * @author Thomas Risberg
  * @author Juergen Hoeller
+ * @author Sam Brannen
  * @since 2.5
  */
 public class TableMetaDataContext {
@@ -302,8 +303,12 @@ public class TableMetaDataContext {
 				}
 			}
 			else {
-				throw new InvalidDataAccessApiUsageException("Unable to locate columns for table '" +
-						getTableName() + "' so an insert statement can't be generated");
+				String message = "Unable to locate columns for table '" + getTableName()
+						+ "' so an insert statement can't be generated.";
+				if (isAccessTableColumnMetaData()) {
+					message += " Consider specifying explicit column names -- for example, via SimpleJdbcInsert#usingColumns().";
+				}
+				throw new InvalidDataAccessApiUsageException(message);
 			}
 		}
 		String params = String.join(", ", Collections.nCopies(columnCount, "?"));

spring-jdbc/src/test/java/org/springframework/jdbc/core/simple/SimpleJdbcCallTests.java

@@ -46,6 +46,7 @@ import static org.mockito.Mockito.verify;
  *
  * @author Thomas Risberg
  * @author Kiril Nugmanov
+ * @author Sam Brannen
  */
 class SimpleJdbcCallTests {
 
@@ -224,6 +225,44 @@ class SimpleJdbcCallTests {
 		verifyStatement(adder, "{call ADD_INVOICE(AMOUNT => ?, CUSTID => ?, NEWID => ?)}");
 	}
 
+	/**
+	 * This test demonstrates that a CALL statement will still be generated if
+	 * an exception occurs while retrieving metadata, potentially resulting in
+	 * missing metadata and consequently a failure while invoking the stored
+	 * procedure.
+	 */
+	@Test  // gh-26486
+	void exceptionThrownWhileRetrievingColumnNamesFromMetadata() throws Exception {
+		ResultSet proceduresResultSet = mock(ResultSet.class);
+		ResultSet procedureColumnsResultSet = mock(ResultSet.class);
+
+		given(databaseMetaData.getDatabaseProductName()).willReturn("Oracle");
+		given(databaseMetaData.getUserName()).willReturn("ME");
+		given(databaseMetaData.storesUpperCaseIdentifiers()).willReturn(true);
+		given(databaseMetaData.getProcedures("", "ME", "ADD_INVOICE")).willReturn(proceduresResultSet);
+		given(databaseMetaData.getProcedureColumns("", "ME", "ADD_INVOICE", null)).willReturn(procedureColumnsResultSet);
+
+		given(proceduresResultSet.next()).willReturn(true, false);
+		given(proceduresResultSet.getString("PROCEDURE_NAME")).willReturn("add_invoice");
+
+		given(procedureColumnsResultSet.next()).willReturn(true, true, true, false);
+		given(procedureColumnsResultSet.getString("COLUMN_NAME")).willReturn("amount", "custid", "newid");
+		given(procedureColumnsResultSet.getInt("DATA_TYPE"))
+			// Return a valid data type for the first 2 columns.
+			.willReturn(Types.INTEGER, Types.INTEGER)
+			// 3rd time, simulate an error while retrieving metadata.
+			.willThrow(new SQLException("error with DATA_TYPE for column 3"));
+
+		SimpleJdbcCall adder = new SimpleJdbcCall(dataSource).withNamedBinding().withProcedureName("add_invoice");
+		adder.compile();
+		// If an exception were not thrown for column 3, we would expect:
+		// {call ADD_INVOICE(AMOUNT => ?, CUSTID => ?, NEWID => ?)}
+		verifyStatement(adder, "{call ADD_INVOICE(AMOUNT => ?, CUSTID => ?)}");
+
+		verify(proceduresResultSet).close();
+		verify(procedureColumnsResultSet).close();
+	}
+
 
 	private void verifyStatement(SimpleJdbcCall adder, String expected) {
 		assertThat(adder.getCallString()).as("Incorrect call statement").isEqualTo(expected);

spring-jdbc/src/test/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertTests.java

@@ -19,6 +19,8 @@ package org.springframework.jdbc.core.simple;
 import java.sql.Connection;
 import java.sql.DatabaseMetaData;
 import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Types;
 import java.util.Collections;
 
 import javax.sql.DataSource;
@@ -29,6 +31,7 @@ import org.junit.jupiter.api.Test;
 
 import org.springframework.dao.InvalidDataAccessApiUsageException;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
@@ -38,6 +41,7 @@ import static org.mockito.Mockito.verify;
  * Mock object based tests for {@link SimpleJdbcInsert}.
  *
  * @author Thomas Risberg
+ * @author Sam Brannen
  */
 class SimpleJdbcInsertTests {
 
@@ -80,4 +84,58 @@ class SimpleJdbcInsertTests {
 		verify(resultSet).close();
 	}
 
+	@Test  // gh-26486
+	void retrieveColumnNamesFromMetadata() throws Exception {
+		ResultSet tableResultSet = mock(ResultSet.class);
+		given(tableResultSet.next()).willReturn(true, false);
+
+		given(databaseMetaData.getUserName()).willReturn("me");
+		given(databaseMetaData.getTables(null, null, "me", null)).willReturn(tableResultSet);
+
+		ResultSet columnResultSet = mock(ResultSet.class);
+		given(databaseMetaData.getColumns(null, "me", null, null)).willReturn(columnResultSet);
+		given(columnResultSet.next()).willReturn(true, true, false);
+		given(columnResultSet.getString("COLUMN_NAME")).willReturn("col1", "col2");
+		given(columnResultSet.getInt("DATA_TYPE")).willReturn(Types.VARCHAR);
+		given(columnResultSet.getBoolean("NULLABLE")).willReturn(false);
+
+		SimpleJdbcInsert insert = new SimpleJdbcInsert(dataSource).withTableName("me");
+		insert.compile();
+		assertThat(insert.getInsertString()).isEqualTo("INSERT INTO me (col1, col2) VALUES(?, ?)");
+
+		verify(columnResultSet).close();
+		verify(tableResultSet).close();
+	}
+
+	@Test  // gh-26486
+	void exceptionThrownWhileRetrievingColumnNamesFromMetadata() throws Exception {
+		ResultSet tableResultSet = mock(ResultSet.class);
+		given(tableResultSet.next()).willReturn(true, false);
+
+		given(databaseMetaData.getUserName()).willReturn("me");
+		given(databaseMetaData.getTables(null, null, "me", null)).willReturn(tableResultSet);
+
+		ResultSet columnResultSet = mock(ResultSet.class);
+		given(databaseMetaData.getColumns(null, "me", null, null)).willReturn(columnResultSet);
+		// true, true, false --> simulates processing of two columns
+		given(columnResultSet.next()).willReturn(true, true, false);
+		given(columnResultSet.getString("COLUMN_NAME"))
+			// Return a column name the first time.
+			.willReturn("col1")
+			// Second time, simulate an error while retrieving metadata.
+			.willThrow(new SQLException("error with col2"));
+		given(columnResultSet.getInt("DATA_TYPE")).willReturn(Types.VARCHAR);
+		given(columnResultSet.getBoolean("NULLABLE")).willReturn(false);
+
+		SimpleJdbcInsert insert = new SimpleJdbcInsert(dataSource).withTableName("me");
+
+		assertThatExceptionOfType(InvalidDataAccessApiUsageException.class)
+			.isThrownBy(insert::compile)
+			.withMessage("Unable to locate columns for table 'me' so an insert statement can't be generated. " +
+						"Consider specifying explicit column names -- for example, via SimpleJdbcInsert#usingColumns().");
+
+		verify(columnResultSet).close();
+		verify(tableResultSet).close();
+	}
+
 }