Skip to content

Commit 236623f

Browse files
sbrannensbrannen
committed
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
1 parent e18fba1 commit 236623f

File tree

5 files changed

+122
-7
lines changed

5 files changed

+122
-7
lines changed

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2020 the original author or authors.
2+
* Copyright 2002-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -39,6 +39,7 @@
3939
*
4040
* @author Thomas Risberg
4141
* @author Juergen Hoeller
42+
* @author Sam Brannen
4243
* @since 2.5
4344
*/
4445
public class GenericCallMetaDataProvider implements CallMetaDataProvider {
@@ -414,8 +415,15 @@ else if ("Oracle".equals(databaseMetaData.getDatabaseProductName())) {
414415
}
415416
catch (SQLException ex) {
416417
if (logger.isWarnEnabled()) {
417-
logger.warn("Error while retrieving meta-data for procedure columns: " + ex);
418+
logger.warn("Error while retrieving meta-data for procedure columns. " +
419+
"Consider declaring explicit parameters -- for example, via SimpleJdbcCall#addDeclaredParameter().",
420+
ex);
418421
}
422+
// Although we could invoke `this.callParameterMetaData.clear()` so that
423+
// we don't retain a partial list of column names (like we do in
424+
// GenericTableMetaDataProvider.processTableColumns(...)), we choose
425+
// not to do that here, since invocation of the stored procedure will
426+
// likely fail anyway with an incorrect argument list.
419427
}
420428
}
421429

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2020 the original author or authors.
2+
* Copyright 2002-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -39,6 +39,7 @@
3939
*
4040
* @author Thomas Risberg
4141
* @author Juergen Hoeller
42+
* @author Sam Brannen
4243
* @since 2.5
4344
*/
4445
public class GenericTableMetaDataProvider implements TableMetaDataProvider {
@@ -422,8 +423,12 @@ private void processTableColumns(DatabaseMetaData databaseMetaData, TableMetaDat
422423
}
423424
catch (SQLException ex) {
424425
if (logger.isWarnEnabled()) {
425-
logger.warn("Error while retrieving meta-data for table columns: " + ex.getMessage());
426+
logger.warn("Error while retrieving meta-data for table columns. " +
427+
"Consider specifying explicit column names -- for example, via SimpleJdbcInsert#usingColumns().",
428+
ex);
426429
}
430+
// Clear the metadata so that we don't retain a partial list of column names
431+
this.tableParameterMetaData.clear();
427432
}
428433
finally {
429434
JdbcUtils.closeResultSet(tableColumns);

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -43,6 +43,7 @@
4343
*
4444
* @author Thomas Risberg
4545
* @author Juergen Hoeller
46+
* @author Sam Brannen
4647
* @since 2.5
4748
*/
4849
public class TableMetaDataContext {
@@ -302,8 +303,12 @@ public String createInsertString(String... generatedKeyNames) {
302303
}
303304
}
304305
else {
305-
throw new InvalidDataAccessApiUsageException("Unable to locate columns for table '" +
306-
getTableName() + "' so an insert statement can't be generated");
306+
String message = "Unable to locate columns for table '" + getTableName()
307+
+ "' so an insert statement can't be generated.";
308+
if (isAccessTableColumnMetaData()) {
309+
message += " Consider specifying explicit column names -- for example, via SimpleJdbcInsert#usingColumns().";
310+
}
311+
throw new InvalidDataAccessApiUsageException(message);
307312
}
308313
}
309314
String params = String.join(", ", Collections.nCopies(columnCount, "?"));

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
*
4747
* @author Thomas Risberg
4848
* @author Kiril Nugmanov
49+
* @author Sam Brannen
4950
*/
5051
class SimpleJdbcCallTests {
5152

@@ -224,6 +225,44 @@ void correctProcedureStatementNamed() throws Exception {
224225
verifyStatement(adder, "{call ADD_INVOICE(AMOUNT => ?, CUSTID => ?, NEWID => ?)}");
225226
}
226227

228+
/**
229+
* This test demonstrates that a CALL statement will still be generated if
230+
* an exception occurs while retrieving metadata, potentially resulting in
231+
* missing metadata and consequently a failure while invoking the stored
232+
* procedure.
233+
*/
234+
@Test // gh-26486
235+
void exceptionThrownWhileRetrievingColumnNamesFromMetadata() throws Exception {
236+
ResultSet proceduresResultSet = mock(ResultSet.class);
237+
ResultSet procedureColumnsResultSet = mock(ResultSet.class);
238+
239+
given(databaseMetaData.getDatabaseProductName()).willReturn("Oracle");
240+
given(databaseMetaData.getUserName()).willReturn("ME");
241+
given(databaseMetaData.storesUpperCaseIdentifiers()).willReturn(true);
242+
given(databaseMetaData.getProcedures("", "ME", "ADD_INVOICE")).willReturn(proceduresResultSet);
243+
given(databaseMetaData.getProcedureColumns("", "ME", "ADD_INVOICE", null)).willReturn(procedureColumnsResultSet);
244+
245+
given(proceduresResultSet.next()).willReturn(true, false);
246+
given(proceduresResultSet.getString("PROCEDURE_NAME")).willReturn("add_invoice");
247+
248+
given(procedureColumnsResultSet.next()).willReturn(true, true, true, false);
249+
given(procedureColumnsResultSet.getString("COLUMN_NAME")).willReturn("amount", "custid", "newid");
250+
given(procedureColumnsResultSet.getInt("DATA_TYPE"))
251+
// Return a valid data type for the first 2 columns.
252+
.willReturn(Types.INTEGER, Types.INTEGER)
253+
// 3rd time, simulate an error while retrieving metadata.
254+
.willThrow(new SQLException("error with DATA_TYPE for column 3"));
255+
256+
SimpleJdbcCall adder = new SimpleJdbcCall(dataSource).withNamedBinding().withProcedureName("add_invoice");
257+
adder.compile();
258+
// If an exception were not thrown for column 3, we would expect:
259+
// {call ADD_INVOICE(AMOUNT => ?, CUSTID => ?, NEWID => ?)}
260+
verifyStatement(adder, "{call ADD_INVOICE(AMOUNT => ?, CUSTID => ?)}");
261+
262+
verify(proceduresResultSet).close();
263+
verify(procedureColumnsResultSet).close();
264+
}
265+
227266

228267
private void verifyStatement(SimpleJdbcCall adder, String expected) {
229268
assertThat(adder.getCallString()).as("Incorrect call statement").isEqualTo(expected);

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
import java.sql.Connection;
2020
import java.sql.DatabaseMetaData;
2121
import java.sql.ResultSet;
22+
import java.sql.SQLException;
23+
import java.sql.Types;
2224
import java.util.Collections;
2325

2426
import javax.sql.DataSource;
@@ -29,6 +31,7 @@
2931

3032
import org.springframework.dao.InvalidDataAccessApiUsageException;
3133

34+
import static org.assertj.core.api.Assertions.assertThat;
3235
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
3336
import static org.mockito.BDDMockito.given;
3437
import static org.mockito.Mockito.mock;
@@ -38,6 +41,7 @@
3841
* Mock object based tests for {@link SimpleJdbcInsert}.
3942
*
4043
* @author Thomas Risberg
44+
* @author Sam Brannen
4145
*/
4246
class SimpleJdbcInsertTests {
4347

@@ -80,4 +84,58 @@ void noSuchTable() throws Exception {
8084
verify(resultSet).close();
8185
}
8286

87+
@Test // gh-26486
88+
void retrieveColumnNamesFromMetadata() throws Exception {
89+
ResultSet tableResultSet = mock(ResultSet.class);
90+
given(tableResultSet.next()).willReturn(true, false);
91+
92+
given(databaseMetaData.getUserName()).willReturn("me");
93+
given(databaseMetaData.getTables(null, null, "me", null)).willReturn(tableResultSet);
94+
95+
ResultSet columnResultSet = mock(ResultSet.class);
96+
given(databaseMetaData.getColumns(null, "me", null, null)).willReturn(columnResultSet);
97+
given(columnResultSet.next()).willReturn(true, true, false);
98+
given(columnResultSet.getString("COLUMN_NAME")).willReturn("col1", "col2");
99+
given(columnResultSet.getInt("DATA_TYPE")).willReturn(Types.VARCHAR);
100+
given(columnResultSet.getBoolean("NULLABLE")).willReturn(false);
101+
102+
SimpleJdbcInsert insert = new SimpleJdbcInsert(dataSource).withTableName("me");
103+
insert.compile();
104+
assertThat(insert.getInsertString()).isEqualTo("INSERT INTO me (col1, col2) VALUES(?, ?)");
105+
106+
verify(columnResultSet).close();
107+
verify(tableResultSet).close();
108+
}
109+
110+
@Test // gh-26486
111+
void exceptionThrownWhileRetrievingColumnNamesFromMetadata() throws Exception {
112+
ResultSet tableResultSet = mock(ResultSet.class);
113+
given(tableResultSet.next()).willReturn(true, false);
114+
115+
given(databaseMetaData.getUserName()).willReturn("me");
116+
given(databaseMetaData.getTables(null, null, "me", null)).willReturn(tableResultSet);
117+
118+
ResultSet columnResultSet = mock(ResultSet.class);
119+
given(databaseMetaData.getColumns(null, "me", null, null)).willReturn(columnResultSet);
120+
// true, true, false --> simulates processing of two columns
121+
given(columnResultSet.next()).willReturn(true, true, false);
122+
given(columnResultSet.getString("COLUMN_NAME"))
123+
// Return a column name the first time.
124+
.willReturn("col1")
125+
// Second time, simulate an error while retrieving metadata.
126+
.willThrow(new SQLException("error with col2"));
127+
given(columnResultSet.getInt("DATA_TYPE")).willReturn(Types.VARCHAR);
128+
given(columnResultSet.getBoolean("NULLABLE")).willReturn(false);
129+
130+
SimpleJdbcInsert insert = new SimpleJdbcInsert(dataSource).withTableName("me");
131+
132+
assertThatExceptionOfType(InvalidDataAccessApiUsageException.class)
133+
.isThrownBy(insert::compile)
134+
.withMessage("Unable to locate columns for table 'me' so an insert statement can't be generated. " +
135+
"Consider specifying explicit column names -- for example, via SimpleJdbcInsert#usingColumns().");
136+
137+
verify(columnResultSet).close();
138+
verify(tableResultSet).close();
139+
}
140+
83141
}

0 commit comments

Comments
 (0)