diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/GenericCallMetaDataProvider.java b/spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/GenericCallMetaDataProvider.java index f0b6991f682..8a9b2d0a3fb 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/GenericCallMetaDataProvider.java +++ b/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. } } diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/GenericTableMetaDataProvider.java b/spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/GenericTableMetaDataProvider.java index d5c7727839a..5228cb9b05c 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/GenericTableMetaDataProvider.java +++ b/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); diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/TableMetaDataContext.java b/spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/TableMetaDataContext.java index 908b165befa..13e582a113d 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/TableMetaDataContext.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/TableMetaDataContext.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. @@ -43,6 +43,7 @@ import org.springframework.util.CollectionUtils; * * @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, "?")); diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/core/simple/SimpleJdbcCallTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/core/simple/SimpleJdbcCallTests.java index b2589fa56d5..42df4595207 100644 --- a/spring-jdbc/src/test/java/org/springframework/jdbc/core/simple/SimpleJdbcCallTests.java +++ b/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); diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertTests.java index 023f6054f4a..d965d598f01 100644 --- a/spring-jdbc/src/test/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertTests.java +++ b/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(); + } + }