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
spring-projects · May 13, 2024 · c662145 · c662145
1 parent 3eb9c4b
commit c662145
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 34 deletions.
diff --git a/...a-jpa/src/main/java/org/springframework/data/jpa/repository/query/ProcedureParameter.java b/...a-jpa/src/main/java/org/springframework/data/jpa/repository/query/ProcedureParameter.java
@@ -28,16 +28,19 @@
  *
  * @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 @@ public String getName() {
 		return name;
 	}
 
+	public int getPosition() {
+		return position;
+	}
+
 	public ParameterMode getMode() {
 		return mode;
 	}
@@ -76,6 +83,6 @@ public int hashCode() {
 
 	@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/...in/java/org/springframework/data/jpa/repository/query/StoredProcedureAttributeSource.java b/...in/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 @@
  * @author Jeff Sheets
  * @author Gabriel Basilio
  * @author Greg Turnquist
+ * @author Thorben Janssen
  * @since 1.6
  */
 enum StoredProcedureAttributeSource {
@@ -119,10 +119,7 @@ private StoredProcedureAttributes newProcedureAttributesFrom(Method method, Name
 		} 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 @@ private StoredProcedureAttributes newProcedureAttributesFrom(Method method, Name
 	 */
 	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<StoredProcedureParameter> extractOutputParametersFrom(NamedStoredProcedureQuery namedStoredProc) {
+	private List<ProcedureParameter> extractOutputParametersFrom(NamedStoredProcedureQuery namedStoredProc) {
 
-		List<StoredProcedureParameter> outputParameters = new ArrayList<>();
+		List<ProcedureParameter> 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/...rc/main/java/org/springframework/data/jpa/repository/query/StoredProcedureAttributes.java b/...rc/main/java/org/springframework/data/jpa/repository/query/StoredProcedureAttributes.java
@@ -34,6 +34,7 @@
  * @author Jeff Sheets
  * @author Jens Schauder
  * @author Gabriel Basilio
+ * @author Thorben Janssen
  * @since 1.6
  */
 class StoredProcedureAttributes {
@@ -87,7 +88,7 @@ private List<ProcedureParameter> getParametersWithCompletedNames(List<ProcedureP
 
 	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/.../src/main/java/org/springframework/data/jpa/repository/query/StoredProcedureJpaQuery.java b/.../src/main/java/org/springframework/data/jpa/repository/query/StoredProcedureJpaQuery.java
@@ -43,6 +43,7 @@
  * @author Mark Paluch
  * @author Jeff Sheets
  * @author JyotirmoyVS
+ * @author Thorben Janssen
  * @since 1.6
  */
 class StoredProcedureJpaQuery extends AbstractJpaQuery {
@@ -119,15 +120,15 @@ Object extractOutputValue(StoredProcedureQuery storedProcedureQuery) {
 		List<ProcedureParameter> outputParameters = procedureAttributes.getOutputProcedureParameters();
 
 		if (outputParameters.size() == 1) {
-			return extractOutputParameterValue(outputParameters.get(0), 0, storedProcedureQuery);
+			return extractOutputParameterValue(outputParameters.get(0), storedProcedureQuery);
 		}
 
 		Map<String, Object> 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 @@ Object extractOutputValue(StoredProcedureQuery storedProcedureQuery) {
 	/**
 	 * @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 @@ private StoredProcedureQuery createAdhocStoredProcedureQuery() {
 	}
 
 	/**
-	 *
 	 * @return true if the stored procedure will use a ResultSet to return data and not output parameters
 	 */
 	private boolean isResultSetProcedure() {

diff --git a/...ringframework/data/jpa/repository/procedures/PostgresStoredProcedureIntegrationTests.java b/...ringframework/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 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.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 @@
  * @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 @@ void testEntityListFromNamedProcedure() {
 				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 @@ public interface EmployeeRepositoryWithRefCursor extends JpaRepository<Employee,
 
 		@Procedure(name = "get_employees_postgres", refCursor = true)
 		List<Employee> entityListFromNamedProcedure();
+
+		@Procedure(name = "positional_inout")
+		Map positionalInOut(Integer in, Integer inout);
 	}
 
 	@EnableJpaRepositories(considerNestedRepositories = true,

diff --git a/...ava/org/springframework/data/jpa/repository/query/StoredProcedureAttributesUnitTests.java b/...ava/org/springframework/data/jpa/repository/query/StoredProcedureAttributesUnitTests.java
@@ -27,14 +27,15 @@
  *
  * @author Oliver Gierke
  * @author Jens Schauder
+ * @author Thorben Janssen
  */
 class StoredProcedureAttributesUnitTests {
 
 	@Test // DATAJPA-681
 	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
@@ -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$;;