From 35dea59ce4941603cb5d226b6ac5e43a39435a24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Tue, 4 Feb 2025 15:43:24 +0100 Subject: [PATCH] Polish "Prevent further configuration once SqlCall is compiled" See gh-33729 --- .../jdbc/core/simple/AbstractJdbcCall.java | 34 ++++---- .../jdbc/core/simple/SimpleJdbcCallTests.java | 84 +++++-------------- 2 files changed, 40 insertions(+), 78 deletions(-) diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/core/simple/AbstractJdbcCall.java b/spring-jdbc/src/main/java/org/springframework/jdbc/core/simple/AbstractJdbcCall.java index 83c1005da6..e41f625ec5 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/core/simple/AbstractJdbcCall.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/core/simple/AbstractJdbcCall.java @@ -248,16 +248,18 @@ public abstract class AbstractJdbcCall { * @param parameter the {@link SqlParameter} to add */ public void addDeclaredParameter(SqlParameter parameter) { - if(!isCompiled()) { - Assert.notNull(parameter, "The supplied parameter must not be null"); - if (!StringUtils.hasText(parameter.getName())) { - throw new InvalidDataAccessApiUsageException( - "You must specify a parameter name when declaring parameters for \"" + getProcedureName() + "\""); - } - this.declaredParameters.add(parameter); - if (logger.isDebugEnabled()) { - logger.debug("Added declared parameter for [" + getProcedureName() + "]: " + parameter.getName()); - } + if (isCompiled()) { + throw new IllegalStateException("SqlCall for " + (isFunction() ? "function" : "procedure") + + " is already compiled"); + } + Assert.notNull(parameter, "The supplied parameter must not be null"); + if (!StringUtils.hasText(parameter.getName())) { + throw new InvalidDataAccessApiUsageException( + "You must specify a parameter name when declaring parameters for \"" + getProcedureName() + "\""); + } + this.declaredParameters.add(parameter); + if (logger.isDebugEnabled()) { + logger.debug("Added declared parameter for [" + getProcedureName() + "]: " + parameter.getName()); } } @@ -267,11 +269,13 @@ public abstract class AbstractJdbcCall { * @param rowMapper the RowMapper implementation to use */ public void addDeclaredRowMapper(String parameterName, RowMapper rowMapper) { - if(!isCompiled()) { - this.declaredRowMappers.put(parameterName, rowMapper); - if (logger.isDebugEnabled()) { - logger.debug("Added row mapper for [" + getProcedureName() + "]: " + parameterName); - } + if (isCompiled()) { + throw new IllegalStateException("SqlCall for " + (isFunction() ? "function" : "procedure") + + " is already compiled"); + } + this.declaredRowMappers.put(parameterName, rowMapper); + if (logger.isDebugEnabled()) { + logger.debug("Added row mapper for [" + getProcedureName() + "]: " + parameterName); } } 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 5075d50b91..064928f1dc 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 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. @@ -16,18 +16,16 @@ package org.springframework.jdbc.core.simple; -import java.lang.reflect.Field; import java.sql.CallableStatement; import java.sql.Connection; import java.sql.DatabaseMetaData; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Types; -import java.util.List; -import java.util.Map; import javax.sql.DataSource; +import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -40,6 +38,7 @@ import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; @@ -364,78 +363,37 @@ class SimpleJdbcCallTests { verifyStatement(adder, "{call ADD_INVOICE(@AMOUNT = ?, @CUSTID = ?)}"); } - /** - * This test verifies that when declaring a parameter for a SimpleJdbcCall, - * then the parameter is added as expected. - */ - @SuppressWarnings("unchecked") @Test - void verifyUncompiledDeclareParameterIsAdded() throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException { - SimpleJdbcCall call = new SimpleJdbcCall(dataSource) - .withProcedureName("procedure_name") - .declareParameters(new SqlParameter("PARAM", Types.VARCHAR)); - - Field params = AbstractJdbcCall.class.getDeclaredField("declaredParameters"); - params.setAccessible(true); - List paramList = (List) params.get(call); - assertThat(paramList).hasSize(1).allMatch(sqlParam -> sqlParam.getName().equals("PARAM")); - } - - /** - * This verifies that once the SimpleJdbcCall is compiled, then adding - * a parameter is ignored - */ - @SuppressWarnings("unchecked") - @Test - void verifyWhenCompiledThenDeclareParameterIsIgnored() throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException { + void declareParametersCannotBeInvokedWhenCompiled() { SimpleJdbcCall call = new SimpleJdbcCall(dataSource) .withProcedureName("procedure_name") .declareParameters(new SqlParameter("PARAM", Types.VARCHAR)); call.compile(); - - call.declareParameters(new SqlParameter("Ignored Param", Types.VARCHAR)); - - Field params = AbstractJdbcCall.class.getDeclaredField("declaredParameters"); - params.setAccessible(true); - List paramList = (List) params.get(call); - assertThat(paramList).hasSize(1).allMatch(sqlParam -> sqlParam.getName().equals("PARAM")); + assertThatIllegalStateException() + .isThrownBy(() -> call.declareParameters(new SqlParameter("Ignored Param", Types.VARCHAR))) + .withMessage("SqlCall for procedure is already compiled"); } - - /** - * When adding a declared row mapper, this verifies that the declaredRowMappers - * gets the new mapper - */ - @SuppressWarnings("unchecked") + @Test - void verifyUncompiledDeclareRowMapperIsAdded() throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException { + void addDeclaredRowMapperCanBeConfigured() { SimpleJdbcCall call = new SimpleJdbcCall(dataSource) .withProcedureName("procedure_name") - .returningResultSet("result_set", (rs,i) -> new Object()); - - Field rowMappers = AbstractJdbcCall.class.getDeclaredField("declaredRowMappers"); - rowMappers.setAccessible(true); - Map> mappers = (Map>) rowMappers.get(call); - assertThat(mappers).hasSize(1).allSatisfy((key,value) -> key.equals("result_set")); + .returningResultSet("result_set", (rs, i) -> new Object()); + + assertThat(call).extracting("declaredRowMappers") + .asInstanceOf(InstanceOfAssertFactories.map(String.class, RowMapper.class)) + .containsOnlyKeys("result_set"); } - - /** - * This verifies that when adding a row mapper after the call is compiled - * then the request is ignored - */ - @SuppressWarnings("unchecked") + @Test - void verifyWhenCompiledThenDeclareRowMapperIsIgnored() throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException { + void addDeclaredRowMapperWhenCompiled() { SimpleJdbcCall call = new SimpleJdbcCall(dataSource) .withProcedureName("procedure_name") - .returningResultSet("result_set", (rs,i) -> new Object()); + .returningResultSet("result_set", (rs, i) -> new Object()); call.compile(); - - call.returningResultSet("not added", (rs,i) -> new Object()); - - Field rowMappers = AbstractJdbcCall.class.getDeclaredField("declaredRowMappers"); - rowMappers.setAccessible(true); - Map> mappers = (Map>) rowMappers.get(call); - assertThat(mappers).hasSize(1).allSatisfy((key,value) -> key.equals("result_set")); + assertThatIllegalStateException() + .isThrownBy(() -> call.returningResultSet("not added", (rs, i) -> new Object())) + .withMessage("SqlCall for procedure is already compiled"); } - + }