From 8810913f301dce7563649b9f13679cb9cb59f59d Mon Sep 17 00:00:00 2001 From: Dan Sievewright Date: Thu, 17 Oct 2024 02:12:18 -0400 Subject: [PATCH 1/2] Prevent further configuration once SqlCall is compiled This commit prevents AbstractJdbcCall to be further configured once it is compiled. See gh-33729 --- .../jdbc/core/simple/AbstractJdbcCall.java | 26 ++++--- .../jdbc/core/simple/SimpleJdbcCallTests.java | 78 +++++++++++++++++++ 2 files changed, 93 insertions(+), 11 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 2856f54985..83c1005da6 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,14 +248,16 @@ public abstract class AbstractJdbcCall { * @param parameter the {@link SqlParameter} to add */ public void addDeclaredParameter(SqlParameter parameter) { - 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()) { + 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()); + } } } @@ -265,9 +267,11 @@ public abstract class AbstractJdbcCall { * @param rowMapper the RowMapper implementation to use */ public void addDeclaredRowMapper(String parameterName, RowMapper rowMapper) { - this.declaredRowMappers.put(parameterName, rowMapper); - if (logger.isDebugEnabled()) { - logger.debug("Added row mapper for [" + getProcedureName() + "]: " + parameterName); + if(!isCompiled()) { + 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 9d4342a785..5075d50b91 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 @@ -16,12 +16,15 @@ 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; @@ -30,6 +33,7 @@ import org.junit.jupiter.api.Test; import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.jdbc.BadSqlGrammarException; +import org.springframework.jdbc.core.RowMapper; import org.springframework.jdbc.core.SqlOutParameter; import org.springframework.jdbc.core.SqlParameter; import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; @@ -360,4 +364,78 @@ 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 { + 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")); + } + + /** + * 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 { + 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")); + } + + /** + * 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 { + SimpleJdbcCall call = new SimpleJdbcCall(dataSource) + .withProcedureName("procedure_name") + .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")); + } + } 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 2/2] 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"); } - + }