From c66214540e84d6718b8e7c1903642024d308e333 Mon Sep 17 00:00:00 2001 From: Thorben Janssen <2053825+thjanssen@users.noreply.github.com> Date: Sat, 4 May 2024 11:23:45 +0200 Subject: [PATCH] Retain stored procedure parameter position. Store the position of each output parameter in ProcedureParameter so that it can be used when extracting them from the result set. Resolves: #3460 Original Pull Request: #3463 --- .../repository/query/ProcedureParameter.java | 11 +++++-- .../query/StoredProcedureAttributeSource.java | 33 +++++++++---------- .../query/StoredProcedureAttributes.java | 3 +- .../query/StoredProcedureJpaQuery.java | 18 +++++----- ...stgresStoredProcedureIntegrationTests.java | 25 ++++++++++++-- .../StoredProcedureAttributesUnitTests.java | 3 +- .../scripts/postgres-stored-procedures.sql | 18 ++++++++++ 7 files changed, 77 insertions(+), 34 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/ProcedureParameter.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/ProcedureParameter.java index 366a4f639..0f6ca5886 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/ProcedureParameter.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/ProcedureParameter.java @@ -28,16 +28,19 @@ import org.springframework.lang.Nullable; * * @author Gabriel Basilio * @author Greg Turnquist + * @author Thorben Janssen */ class ProcedureParameter { private final String name; + private final int position; private final ParameterMode mode; private final Class type; - ProcedureParameter(@Nullable String name, ParameterMode mode, Class type) { + ProcedureParameter(@Nullable String name, int position, ParameterMode mode, Class type) { this.name = name; + this.position = position; this.mode = mode; this.type = type; } @@ -46,6 +49,10 @@ class ProcedureParameter { return name; } + public int getPosition() { + return position; + } + public ParameterMode getMode() { return mode; } @@ -76,6 +83,6 @@ class ProcedureParameter { @Override public String toString() { - return "ProcedureParameter{" + "name='" + name + '\'' + ", mode=" + mode + ", type=" + type + '}'; + return "ProcedureParameter{" + "name='" + name + '\'' + ", position=" + position + ", mode=" + mode + ", type=" + type + '}'; } } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StoredProcedureAttributeSource.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StoredProcedureAttributeSource.java index 805672a0a..770c946f6 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StoredProcedureAttributeSource.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StoredProcedureAttributeSource.java @@ -15,17 +15,16 @@ */ package org.springframework.data.jpa.repository.query; +import jakarta.persistence.NamedStoredProcedureQueries; +import jakarta.persistence.NamedStoredProcedureQuery; +import jakarta.persistence.ParameterMode; +import jakarta.persistence.StoredProcedureParameter; + import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.stream.Collectors; - -import jakarta.persistence.NamedStoredProcedureQueries; -import jakarta.persistence.NamedStoredProcedureQuery; -import jakarta.persistence.ParameterMode; -import jakarta.persistence.StoredProcedureParameter; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.lang.Nullable; @@ -44,6 +43,7 @@ import org.springframework.util.StringUtils; * @author Jeff Sheets * @author Gabriel Basilio * @author Greg Turnquist + * @author Thorben Janssen * @since 1.6 */ enum StoredProcedureAttributeSource { @@ -119,10 +119,7 @@ enum StoredProcedureAttributeSource { } else { // try to discover the output parameter - outputParameters = extractOutputParametersFrom(namedStoredProc).stream() // - .map(namedParameter -> new ProcedureParameter(namedParameter.name(), namedParameter.mode(), - namedParameter.type())) // - .collect(Collectors.toList()); + outputParameters = extractOutputParametersFrom(namedStoredProc); } return new StoredProcedureAttributes(namedStoredProc.name(), outputParameters, true); @@ -137,33 +134,35 @@ enum StoredProcedureAttributeSource { */ private ProcedureParameter createOutputProcedureParameterFrom(Method method, Procedure procedure) { - return new ProcedureParameter(procedure.outputParameterName(), + return new ProcedureParameter(procedure.outputParameterName(), 1, procedure.refCursor() ? ParameterMode.REF_CURSOR : ParameterMode.OUT, method.getReturnType()); } /** - * Translate all the {@Link NamedStoredProcedureQuery} parameters into a {@link List} of - * {@link StoredProcedureParameter}s. + * Translate all the {@Link NamedStoredProcedureQuery} parameters into a {@link List} of {@link ProcedureParameter}s. * * @param namedStoredProc * @return */ - private List extractOutputParametersFrom(NamedStoredProcedureQuery namedStoredProc) { + private List extractOutputParametersFrom(NamedStoredProcedureQuery namedStoredProc) { - List outputParameters = new ArrayList<>(); + List outputParameters = new ArrayList<>(); + int position = 1; for (StoredProcedureParameter param : namedStoredProc.parameters()) { switch (param.mode()) { case OUT: case INOUT: case REF_CURSOR: - outputParameters.add(param); + outputParameters.add(new ProcedureParameter(param.name(), position, param.mode(), param.type())); break; case IN: default: - continue; + // not an output parameter } + + position++; } return outputParameters; diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StoredProcedureAttributes.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StoredProcedureAttributes.java index 5a01409d9..a489b490c 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StoredProcedureAttributes.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StoredProcedureAttributes.java @@ -34,6 +34,7 @@ import org.springframework.util.StringUtils; * @author Jeff Sheets * @author Jens Schauder * @author Gabriel Basilio + * @author Thorben Janssen * @since 1.6 */ class StoredProcedureAttributes { @@ -87,7 +88,7 @@ class StoredProcedureAttributes { private ProcedureParameter getParameterWithCompletedName(ProcedureParameter parameter, int i) { - return new ProcedureParameter(completeOutputParameterName(i, parameter.getName()), parameter.getMode(), + return new ProcedureParameter(completeOutputParameterName(i, parameter.getName()), parameter.getPosition(), parameter.getMode(), parameter.getType()); } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StoredProcedureJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StoredProcedureJpaQuery.java index 42008294e..87e402aa4 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StoredProcedureJpaQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StoredProcedureJpaQuery.java @@ -43,6 +43,7 @@ import org.springframework.util.StringUtils; * @author Mark Paluch * @author Jeff Sheets * @author JyotirmoyVS + * @author Thorben Janssen * @since 1.6 */ class StoredProcedureJpaQuery extends AbstractJpaQuery { @@ -119,15 +120,15 @@ class StoredProcedureJpaQuery extends AbstractJpaQuery { List outputParameters = procedureAttributes.getOutputProcedureParameters(); if (outputParameters.size() == 1) { - return extractOutputParameterValue(outputParameters.get(0), 0, storedProcedureQuery); + return extractOutputParameterValue(outputParameters.get(0), storedProcedureQuery); } Map outputValues = new HashMap<>(); - for (int i = 0; i < outputParameters.size(); i++) { - ProcedureParameter outputParameter = outputParameters.get(i); - outputValues.put(outputParameter.getName(), - extractOutputParameterValue(outputParameter, i, storedProcedureQuery)); + for (ProcedureParameter outputParameter : outputParameters) { + outputValues.put( + !outputParameter.getName().isEmpty() ? outputParameter.getName() : outputParameter.getPosition() + "", + extractOutputParameterValue(outputParameter, storedProcedureQuery)); } return outputValues; @@ -136,14 +137,12 @@ class StoredProcedureJpaQuery extends AbstractJpaQuery { /** * @return The value of an output parameter either by name or by index. */ - private Object extractOutputParameterValue(ProcedureParameter outputParameter, Integer index, + private Object extractOutputParameterValue(ProcedureParameter outputParameter, StoredProcedureQuery storedProcedureQuery) { - JpaParameters methodParameters = getQueryMethod().getParameters(); - return useNamedParameters && StringUtils.hasText(outputParameter.getName()) ? storedProcedureQuery.getOutputParameterValue(outputParameter.getName()) - : storedProcedureQuery.getOutputParameterValue(methodParameters.getNumberOfParameters() + index + 1); + : storedProcedureQuery.getOutputParameterValue(outputParameter.getPosition()); } /** @@ -236,7 +235,6 @@ class StoredProcedureJpaQuery extends AbstractJpaQuery { } /** - * * @return true if the stored procedure will use a ResultSet to return data and not output parameters */ private boolean isResultSetProcedure() { diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureIntegrationTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureIntegrationTests.java index 21a82252a..1be3894a9 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureIntegrationTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureIntegrationTests.java @@ -16,7 +16,7 @@ package org.springframework.data.jpa.repository.procedures; -import static org.assertj.core.api.Assertions.*; +import static org.assertj.core.api.Assertions.assertThat; import jakarta.persistence.Entity; import jakarta.persistence.EntityManagerFactory; @@ -28,6 +28,7 @@ import jakarta.persistence.StoredProcedureParameter; import java.math.BigDecimal; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Properties; @@ -45,7 +46,6 @@ import org.springframework.core.io.ClassPathResource; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.config.EnableJpaRepositories; import org.springframework.data.jpa.repository.query.Procedure; -import org.springframework.data.jpa.util.DisabledOnHibernate61; import org.springframework.data.jpa.util.DisabledOnHibernate62; import org.springframework.jdbc.datasource.init.DataSourceInitializer; import org.springframework.jdbc.datasource.init.ResourceDatabasePopulator; @@ -66,8 +66,8 @@ import org.testcontainers.containers.PostgreSQLContainer; * @author Gabriel Basilio * @author Greg Turnquist * @author Yanming Zhou + * @author Thorben Janssen */ -@DisabledOnHibernate61 // GH-2903 @Transactional @ExtendWith(SpringExtension.class) @ContextConfiguration(classes = PostgresStoredProcedureIntegrationTests.Config.class) @@ -150,12 +150,28 @@ class PostgresStoredProcedureIntegrationTests { new Employee(4, "Gabriel")); } + @Test // 3460 + void testPositionalInOutParameter() { + + Map results = repository.positionalInOut(1, 2); + + assertThat(results.get("2")).isEqualTo(2); + assertThat(results.get("3")).isEqualTo(3); + } + @Entity @NamedStoredProcedureQuery( // name = "get_employees_postgres", // procedureName = "get_employees", // parameters = { @StoredProcedureParameter(mode = ParameterMode.REF_CURSOR, type = void.class) }, // resultClasses = Employee.class) + @NamedStoredProcedureQuery( // + name = "positional_inout", // + procedureName = "positional_inout_parameter_issue3460", // + parameters = { @StoredProcedureParameter(mode = ParameterMode.IN, type = Integer.class), + @StoredProcedureParameter(mode = ParameterMode.INOUT, type = Integer.class), + @StoredProcedureParameter(mode = ParameterMode.OUT, type = Integer.class) }, // + resultClasses = Employee.class) public static class Employee { @Id @@ -234,6 +250,9 @@ class PostgresStoredProcedureIntegrationTests { @Procedure(name = "get_employees_postgres", refCursor = true) List entityListFromNamedProcedure(); + + @Procedure(name = "positional_inout") + Map positionalInOut(Integer in, Integer inout); } @EnableJpaRepositories(considerNestedRepositories = true, diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/StoredProcedureAttributesUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/StoredProcedureAttributesUnitTests.java index b419c940c..919f12d1a 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/StoredProcedureAttributesUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/StoredProcedureAttributesUnitTests.java @@ -27,6 +27,7 @@ import org.junit.jupiter.api.Test; * * @author Oliver Gierke * @author Jens Schauder + * @author Thorben Janssen */ class StoredProcedureAttributesUnitTests { @@ -34,7 +35,7 @@ class StoredProcedureAttributesUnitTests { void usesSyntheticOutputParameterNameForAdhocProcedureWithoutOutputName() { StoredProcedureAttributes attributes = new StoredProcedureAttributes("procedure", - new ProcedureParameter(null, ParameterMode.OUT, Long.class)); + new ProcedureParameter(null, 1, ParameterMode.OUT, Long.class)); assertThat(attributes.getOutputProcedureParameters().get(0).getName()).isEqualTo(SYNTHETIC_OUTPUT_PARAMETER_NAME); } } diff --git a/spring-data-jpa/src/test/resources/scripts/postgres-stored-procedures.sql b/spring-data-jpa/src/test/resources/scripts/postgres-stored-procedures.sql index 59097c851..ceafa45f0 100644 --- a/spring-data-jpa/src/test/resources/scripts/postgres-stored-procedures.sql +++ b/spring-data-jpa/src/test/resources/scripts/postgres-stored-procedures.sql @@ -34,3 +34,21 @@ BEGIN OPEN ref FOR SELECT * FROM employee WHERE employee.ID = 3; END; $BODY$;; + +CREATE OR REPLACE PROCEDURE get_employees_count(OUT results integer) + LANGUAGE 'plpgsql' +AS +$BODY$ +BEGIN + results = (SELECT COUNT(*) FROM employee); +END; +$BODY$;; + +CREATE OR REPLACE PROCEDURE positional_inout_parameter_issue3460(IN inParam integer, INOUT inoutParam integer, OUT outParam integer) + LANGUAGE 'plpgsql' +AS +$BODY$ +BEGIN + outParam = 3; +END; +$BODY$;; \ No newline at end of file