Skip to content

Commit 35dea59

Browse files
committed
Polish "Prevent further configuration once SqlCall is compiled"
See gh-33729
1 parent 8810913 commit 35dea59

File tree

2 files changed

+40
-78
lines changed

2 files changed

+40
-78
lines changed

spring-jdbc/src/main/java/org/springframework/jdbc/core/simple/AbstractJdbcCall.java

+19-15
Original file line numberDiff line numberDiff line change
@@ -248,16 +248,18 @@ protected CallableStatementCreatorFactory getCallableStatementFactory() {
248248
* @param parameter the {@link SqlParameter} to add
249249
*/
250250
public void addDeclaredParameter(SqlParameter parameter) {
251-
if(!isCompiled()) {
252-
Assert.notNull(parameter, "The supplied parameter must not be null");
253-
if (!StringUtils.hasText(parameter.getName())) {
254-
throw new InvalidDataAccessApiUsageException(
255-
"You must specify a parameter name when declaring parameters for \"" + getProcedureName() + "\"");
256-
}
257-
this.declaredParameters.add(parameter);
258-
if (logger.isDebugEnabled()) {
259-
logger.debug("Added declared parameter for [" + getProcedureName() + "]: " + parameter.getName());
260-
}
251+
if (isCompiled()) {
252+
throw new IllegalStateException("SqlCall for " + (isFunction() ? "function" : "procedure") +
253+
" is already compiled");
254+
}
255+
Assert.notNull(parameter, "The supplied parameter must not be null");
256+
if (!StringUtils.hasText(parameter.getName())) {
257+
throw new InvalidDataAccessApiUsageException(
258+
"You must specify a parameter name when declaring parameters for \"" + getProcedureName() + "\"");
259+
}
260+
this.declaredParameters.add(parameter);
261+
if (logger.isDebugEnabled()) {
262+
logger.debug("Added declared parameter for [" + getProcedureName() + "]: " + parameter.getName());
261263
}
262264
}
263265

@@ -267,11 +269,13 @@ public void addDeclaredParameter(SqlParameter parameter) {
267269
* @param rowMapper the RowMapper implementation to use
268270
*/
269271
public void addDeclaredRowMapper(String parameterName, RowMapper<?> rowMapper) {
270-
if(!isCompiled()) {
271-
this.declaredRowMappers.put(parameterName, rowMapper);
272-
if (logger.isDebugEnabled()) {
273-
logger.debug("Added row mapper for [" + getProcedureName() + "]: " + parameterName);
274-
}
272+
if (isCompiled()) {
273+
throw new IllegalStateException("SqlCall for " + (isFunction() ? "function" : "procedure") +
274+
" is already compiled");
275+
}
276+
this.declaredRowMappers.put(parameterName, rowMapper);
277+
if (logger.isDebugEnabled()) {
278+
logger.debug("Added row mapper for [" + getProcedureName() + "]: " + parameterName);
275279
}
276280
}
277281

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

+21-63
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 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.
@@ -16,18 +16,16 @@
1616

1717
package org.springframework.jdbc.core.simple;
1818

19-
import java.lang.reflect.Field;
2019
import java.sql.CallableStatement;
2120
import java.sql.Connection;
2221
import java.sql.DatabaseMetaData;
2322
import java.sql.ResultSet;
2423
import java.sql.SQLException;
2524
import java.sql.Types;
26-
import java.util.List;
27-
import java.util.Map;
2825

2926
import javax.sql.DataSource;
3027

28+
import org.assertj.core.api.InstanceOfAssertFactories;
3129
import org.junit.jupiter.api.BeforeEach;
3230
import org.junit.jupiter.api.Test;
3331

@@ -40,6 +38,7 @@
4038

4139
import static org.assertj.core.api.Assertions.assertThat;
4240
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
41+
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
4342
import static org.mockito.BDDMockito.given;
4443
import static org.mockito.Mockito.atLeastOnce;
4544
import static org.mockito.Mockito.mock;
@@ -364,78 +363,37 @@ void correctSybaseFunctionStatementNamed() throws Exception {
364363
verifyStatement(adder, "{call ADD_INVOICE(@AMOUNT = ?, @CUSTID = ?)}");
365364
}
366365

367-
/**
368-
* This test verifies that when declaring a parameter for a SimpleJdbcCall,
369-
* then the parameter is added as expected.
370-
*/
371-
@SuppressWarnings("unchecked")
372-
@Test
373-
void verifyUncompiledDeclareParameterIsAdded() throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException {
374-
SimpleJdbcCall call = new SimpleJdbcCall(dataSource)
375-
.withProcedureName("procedure_name")
376-
.declareParameters(new SqlParameter("PARAM", Types.VARCHAR));
377-
378-
Field params = AbstractJdbcCall.class.getDeclaredField("declaredParameters");
379-
params.setAccessible(true);
380-
List<SqlParameter> paramList = (List<SqlParameter>) params.get(call);
381-
assertThat(paramList).hasSize(1).allMatch(sqlParam -> sqlParam.getName().equals("PARAM"));
382-
}
383-
384-
/**
385-
* This verifies that once the SimpleJdbcCall is compiled, then adding
386-
* a parameter is ignored
387-
*/
388-
@SuppressWarnings("unchecked")
389366
@Test
390-
void verifyWhenCompiledThenDeclareParameterIsIgnored() throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException {
367+
void declareParametersCannotBeInvokedWhenCompiled() {
391368
SimpleJdbcCall call = new SimpleJdbcCall(dataSource)
392369
.withProcedureName("procedure_name")
393370
.declareParameters(new SqlParameter("PARAM", Types.VARCHAR));
394371
call.compile();
395-
396-
call.declareParameters(new SqlParameter("Ignored Param", Types.VARCHAR));
397-
398-
Field params = AbstractJdbcCall.class.getDeclaredField("declaredParameters");
399-
params.setAccessible(true);
400-
List<SqlParameter> paramList = (List<SqlParameter>) params.get(call);
401-
assertThat(paramList).hasSize(1).allMatch(sqlParam -> sqlParam.getName().equals("PARAM"));
372+
assertThatIllegalStateException()
373+
.isThrownBy(() -> call.declareParameters(new SqlParameter("Ignored Param", Types.VARCHAR)))
374+
.withMessage("SqlCall for procedure is already compiled");
402375
}
403-
404-
/**
405-
* When adding a declared row mapper, this verifies that the declaredRowMappers
406-
* gets the new mapper
407-
*/
408-
@SuppressWarnings("unchecked")
376+
409377
@Test
410-
void verifyUncompiledDeclareRowMapperIsAdded() throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException {
378+
void addDeclaredRowMapperCanBeConfigured() {
411379
SimpleJdbcCall call = new SimpleJdbcCall(dataSource)
412380
.withProcedureName("procedure_name")
413-
.returningResultSet("result_set", (rs,i) -> new Object());
414-
415-
Field rowMappers = AbstractJdbcCall.class.getDeclaredField("declaredRowMappers");
416-
rowMappers.setAccessible(true);
417-
Map<String, RowMapper<?>> mappers = (Map<String, RowMapper<?>>) rowMappers.get(call);
418-
assertThat(mappers).hasSize(1).allSatisfy((key,value) -> key.equals("result_set"));
381+
.returningResultSet("result_set", (rs, i) -> new Object());
382+
383+
assertThat(call).extracting("declaredRowMappers")
384+
.asInstanceOf(InstanceOfAssertFactories.map(String.class, RowMapper.class))
385+
.containsOnlyKeys("result_set");
419386
}
420-
421-
/**
422-
* This verifies that when adding a row mapper after the call is compiled
423-
* then the request is ignored
424-
*/
425-
@SuppressWarnings("unchecked")
387+
426388
@Test
427-
void verifyWhenCompiledThenDeclareRowMapperIsIgnored() throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException {
389+
void addDeclaredRowMapperWhenCompiled() {
428390
SimpleJdbcCall call = new SimpleJdbcCall(dataSource)
429391
.withProcedureName("procedure_name")
430-
.returningResultSet("result_set", (rs,i) -> new Object());
392+
.returningResultSet("result_set", (rs, i) -> new Object());
431393
call.compile();
432-
433-
call.returningResultSet("not added", (rs,i) -> new Object());
434-
435-
Field rowMappers = AbstractJdbcCall.class.getDeclaredField("declaredRowMappers");
436-
rowMappers.setAccessible(true);
437-
Map<String, RowMapper<?>> mappers = (Map<String, RowMapper<?>>) rowMappers.get(call);
438-
assertThat(mappers).hasSize(1).allSatisfy((key,value) -> key.equals("result_set"));
394+
assertThatIllegalStateException()
395+
.isThrownBy(() -> call.returningResultSet("not added", (rs, i) -> new Object()))
396+
.withMessage("SqlCall for procedure is already compiled");
439397
}
440-
398+
441399
}

0 commit comments

Comments
 (0)