Restrict TypedParameterValue usage to native queries only.

Use Hibernate parameter accessor for native queries only to avoid affecting JPQL queries. Co-locate Hibernate-specific parameters accessor as the same package as JPA parameters accessor. Remove Parameter Accessor reference from Persistence Provider since it's created in AbstractJpaQuery. Closes #3137 Original Pull Request: #3173
2023-09-25 09:02:47 -04:00
parent 7cdf53f184
commit 814d214a27
9 changed files with 111 additions and 34 deletions
--- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/provider/PersistenceProvider.java
+++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/provider/PersistenceProvider.java
@@ -106,12 +106,6 @@ public enum PersistenceProvider implements QueryExtractor, ProxyIdAccessor, Quer
 			return new HibernateScrollableResultsIterator(jpaQuery);
 		}

-		@Override
-		public JpaParametersParameterAccessor getParameterAccessor(JpaParameters parameters, Object[] values,
-				EntityManager em) {
-			return new HibernateJpaParametersParameterAccessor(parameters, values, em);
-		}
-
 		@Override
 		public String getCommentHintKey() {
 			return "org.hibernate.comment";
@@ -292,11 +286,6 @@ public enum PersistenceProvider implements QueryExtractor, ProxyIdAccessor, Quer
 		return cacheAndReturn(metamodelType, GENERIC_JPA);
 	}

-	public JpaParametersParameterAccessor getParameterAccessor(JpaParameters parameters, Object[] values,
-			EntityManager em) {
-		return new JpaParametersParameterAccessor(parameters, values);
-	}
-
 	/**
 	 * Returns the placeholder to be used for simple count queries. Default implementation returns {@code x}.
 	 *
--- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java
+++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java
@@ -61,6 +61,7 @@ import org.springframework.util.Assert;
 * @author Jens Schauder
 * @author Сергей Цыпанов
 * @author Wonchul Heo
+ * @author Julia Lee
 */
 public abstract class AbstractJpaQuery implements RepositoryQuery {

@@ -153,7 +154,11 @@ public abstract class AbstractJpaQuery implements RepositoryQuery {

 	private JpaParametersParameterAccessor obtainParameterAccessor(Object[] values) {

-		return provider.getParameterAccessor(method.getParameters(), values, em);
+		if (method.isNativeQuery() && PersistenceProvider.HIBERNATE.equals(provider)) {
+			return new HibernateJpaParametersParameterAccessor(method.getParameters(), values, em);
+		}
+
+		return new JpaParametersParameterAccessor(method.getParameters(), values);
 	}

 	protected JpaQueryExecution getExecution() {
--- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HibernateJpaParametersParameterAccessor.java
+++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HibernateJpaParametersParameterAccessor.java
@@ -13,7 +13,7 @@
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
-package org.springframework.data.jpa.provider;
+package org.springframework.data.jpa.repository.query;

 import jakarta.persistence.EntityManager;

@@ -21,7 +21,6 @@ import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.query.TypedParameterValue;
 import org.hibernate.type.BasicType;
 import org.hibernate.type.BasicTypeRegistry;
-import org.springframework.data.jpa.repository.query.JpaParametersParameterAccessor;
 import org.springframework.data.repository.query.Parameter;
 import org.springframework.data.repository.query.Parameters;
 import org.springframework.data.repository.query.ParametersParameterAccessor;
@@ -38,6 +37,7 @@ import org.springframework.lang.Nullable;
 * @author Robert Wilson
 * @author Oliver Drotbohm
 * @author Greg Turnquist
+ * @author Julia Lee
 * @since 2.7
 */
 class HibernateJpaParametersParameterAccessor extends JpaParametersParameterAccessor {
--- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java
+++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java
@@ -233,31 +233,27 @@ class ParameterMetadataProvider {
 		/**
 		 * Prepares the object before it's actually bound to the {@link jakarta.persistence.Query;}.
 		 *
-		 * @param value must not be {@literal null}.
+		 * @param value can be {@literal null}.
 		 */
 		@Nullable
-		public Object prepare(Object value) {
+		public Object prepare(@Nullable Object value) {

-			Assert.notNull(value, "Value must not be null");
-
-			Object unwrapped = PersistenceProvider.unwrapTypedParameterValue(value);
-
-			if (unwrapped == null || expression.getJavaType() == null) {
-				return unwrapped;
+			if (value == null || expression.getJavaType() == null) {
+				return value;
 			}

 			if (String.class.equals(expression.getJavaType()) && !noWildcards) {

 				switch (type) {
 					case STARTING_WITH:
-						return String.format("%s%%", escape.escape(unwrapped.toString()));
+						return String.format("%s%%", escape.escape(value.toString()));
 					case ENDING_WITH:
-						return String.format("%%%s", escape.escape(unwrapped.toString()));
+						return String.format("%%%s", escape.escape(value.toString()));
 					case CONTAINING:
 					case NOT_CONTAINING:
-						return String.format("%%%s%%", escape.escape(unwrapped.toString()));
+						return String.format("%%%s%%", escape.escape(value.toString()));
 					default:
-						return unwrapped;
+						return value;
 				}
 			}

--- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/AbstractJpaQueryTests.java
+++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/AbstractJpaQueryTests.java
@@ -16,7 +16,9 @@
 package org.springframework.data.jpa.repository.query;

 import static org.assertj.core.api.Assumptions.assumeThat;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
@@ -36,6 +38,7 @@ import java.util.List;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
 import org.springframework.data.jpa.domain.sample.User;
 import org.springframework.data.jpa.provider.PersistenceProvider;
 import org.springframework.data.jpa.repository.EntityGraph;
@@ -56,6 +59,7 @@ import org.springframework.transaction.annotation.Transactional;
 * @author Thomas Darimont
 * @author Mark Paluch
 * @author Krzysztof Krason
+ * @author Julia Lee
 */
@ExtendWith(SpringExtension.class)
@ContextConfiguration("classpath:infrastructure.xml")
@@ -65,12 +69,14 @@ class AbstractJpaQueryTests {

 	private Query query;
 	private TypedQuery<Long> countQuery;
+	private JpaQueryExecution execution;

 	@BeforeEach
 	@SuppressWarnings("unchecked")
 	void setUp() {
 		query = mock(Query.class);
 		countQuery = mock(TypedQuery.class);
+		execution = mock(JpaQueryExecution.class);
 	}

 	@Test // DATADOC-97
@@ -150,6 +156,37 @@ class AbstractJpaQueryTests {
 		verify(result).setHint("jakarta.persistence.loadgraph", entityGraph);
 	}

+	@Test // GH-3137
+	void shouldCreateHibernateJpaParameterParametersAccessorForNativeQuery() throws Exception {
+
+		JpaQueryMethod queryMethod = getMethod("findByLastnameNativeQuery", String.class);
+
+		AbstractJpaQuery jpaQuery = new DummyJpaQuery(queryMethod, em);
+
+		jpaQuery.execute(new Object[] {"some last name"});
+
+		ArgumentCaptor<JpaParametersParameterAccessor> captor = ArgumentCaptor.forClass(JpaParametersParameterAccessor.class);
+		verify(execution).execute(eq(jpaQuery), captor.capture());
+		JpaParametersParameterAccessor parameterAccessor = captor.getValue();
+
+		assertThat(parameterAccessor).isInstanceOf(HibernateJpaParametersParameterAccessor.class);
+	}
+
+	@Test // GH-3137
+	void shouldCreateGenericJpaParameterParametersAccessorForNonNativeQuery() throws Exception {
+
+		JpaQueryMethod queryMethod = getMethod("findByFirstname", String.class);
+		AbstractJpaQuery jpaQuery = new DummyJpaQuery(queryMethod, em);
+
+		jpaQuery.execute(new Object[] {"some first name"});
+
+		ArgumentCaptor<JpaParametersParameterAccessor> captor = ArgumentCaptor.forClass(JpaParametersParameterAccessor.class);
+		verify(execution).execute(eq(jpaQuery), captor.capture());
+		JpaParametersParameterAccessor parameterAccessor = captor.getValue();
+
+		assertThat(parameterAccessor).isNotInstanceOf(HibernateJpaParametersParameterAccessor.class);
+	}
+
 	private JpaQueryMethod getMethod(String name, Class<?>... parameterTypes) throws Exception {

 		Method method = SampleRepository.class.getMethod(name, parameterTypes);
@@ -164,6 +201,9 @@ class AbstractJpaQueryTests {
 		@QueryHints({ @QueryHint(name = "foo", value = "bar") })
 		List<User> findByLastname(String lastname);

+		@org.springframework.data.jpa.repository.Query(value = "select u from User u where u.lastname = ?1", nativeQuery = true)
+		List<User> findByLastnameNativeQuery(String lastname);
+
 		@QueryHints(value = { @QueryHint(name = "bar", value = "foo") }, forCounting = false)
 		List<User> findByFirstname(String firstname);

@@ -186,6 +226,11 @@ class AbstractJpaQueryTests {
 			super(method, em);
 		}

+		@Override
+		protected JpaQueryExecution getExecution() {
+			return execution;
+		}
+
 		@Override
 		protected Query doCreateQuery(JpaParametersParameterAccessor accessor) {
 			return query;
--- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HibernateJpaParametersParameterAccessorUnitTests.java
+++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HibernateJpaParametersParameterAccessorUnitTests.java
@@ -1,4 +1,4 @@
-package org.springframework.data.jpa.provider;
+package org.springframework.data.jpa.repository.query;

 import jakarta.persistence.EntityManager;

@@ -8,6 +8,7 @@ import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.data.jpa.repository.query.HibernateJpaParametersParameterAccessor;
 import org.springframework.data.jpa.repository.query.JpaParameters;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit.jupiter.SpringExtension;
--- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaParametersParameterAccessorTests.java
+++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaParametersParameterAccessorTests.java
@@ -43,8 +43,7 @@ class JpaParametersParameterAccessorTests {
 		Method withNativeQuery = SampleRepository.class.getMethod("withNativeQuery", Integer.class);
 		Object[] values = { null };
 		JpaParameters parameters = new JpaParameters(withNativeQuery);
-		JpaParametersParameterAccessor accessor = PersistenceProvider.GENERIC_JPA.getParameterAccessor(parameters, values,
-				em);
+		JpaParametersParameterAccessor accessor = new JpaParametersParameterAccessor(parameters, values);

 		bind(parameters, accessor);

@@ -57,8 +56,7 @@ class JpaParametersParameterAccessorTests {
 		Method withNativeQuery = SampleRepository.class.getMethod("withNativeQuery", Integer.class);
 		Object[] values = { null };
 		JpaParameters parameters = new JpaParameters(withNativeQuery);
-		JpaParametersParameterAccessor accessor = PersistenceProvider.HIBERNATE.getParameterAccessor(parameters, values,
-				em);
+		JpaParametersParameterAccessor accessor = new HibernateJpaParametersParameterAccessor(parameters, values, em);

 		bind(parameters, accessor);

--- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderUnitTests.java
+++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderUnitTests.java
@@ -22,17 +22,32 @@ import java.util.Collections;

 import jakarta.persistence.criteria.CriteriaBuilder;

-import org.junit.jupiter.api.Test;
+import org.eclipse.persistence.internal.jpa.querydef.ParameterExpressionImpl;
 import org.springframework.data.repository.query.Parameters;
 import org.springframework.data.repository.query.parser.Part;

+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Answers;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
+
 /**
 * Unit tests for {@link ParameterMetadataProvider}.
 *
 * @author Jens Schauder
+ * @author Julia Lee
 */
+@ExtendWith(MockitoExtension.class)
+@MockitoSettings(strictness = Strictness.STRICT_STUBS)
 class ParameterMetadataProviderUnitTests {

+	@Mock(answer = Answers.RETURNS_DEEP_STUBS) Part part;
+
+	private ParameterExpressionImpl parameterExpression = new ParameterExpressionImpl(null, String.class);
+
 	@Test // DATAJPA-863
 	void errorMessageMentionesParametersWhenParametersAreExhausted() {

@@ -49,4 +64,20 @@ class ParameterMetadataProviderUnitTests {
 				.withMessageContaining("parameter");
 	}

+	@Test // GH-3137
+	void returnAugmentedValueForStringExpressions() {
+		when(part.getProperty().getLeafProperty().isCollection()).thenReturn(false);
+
+		assertThat(createParameterMetadata(Part.Type.STARTING_WITH).prepare("starting with")).isEqualTo("starting with%");
+		assertThat(createParameterMetadata(Part.Type.ENDING_WITH).prepare("ending with")).isEqualTo("%ending with");
+		assertThat(createParameterMetadata(Part.Type.CONTAINING).prepare("containing")).isEqualTo("%containing%");
+		assertThat(createParameterMetadata(Part.Type.NOT_CONTAINING).prepare("not containing")).isEqualTo("%not containing%");
+		assertThat(createParameterMetadata(Part.Type.LIKE).prepare("%like%")).isEqualTo("%like%");
+		assertThat(createParameterMetadata(Part.Type.IS_NULL).prepare(null)).isEqualTo(null);
+	}
+
+	private ParameterMetadataProvider.ParameterMetadata createParameterMetadata(Part.Type partType) {
+		when(part.getType()).thenReturn(partType);
+		return new ParameterMetadataProvider.ParameterMetadata<>(parameterExpression, part, null, EscapeCharacter.DEFAULT);
+	}
 }
--- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryWithNullLikeIntegrationTests.java
+++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryWithNullLikeIntegrationTests.java
@@ -54,6 +54,7 @@ import org.springframework.transaction.annotation.Transactional;
 *
 * @author Greg Turnquist
 * @author Yuriy Tsarkov
+ * @author Julia Lee
 */
@ExtendWith(SpringExtension.class)
@ContextConfiguration(classes = QueryWithNullLikeIntegrationTests.Config.class)
@@ -66,7 +67,8 @@ class QueryWithNullLikeIntegrationTests {
 	void setUp() {
 		repository.saveAllAndFlush(List.of( //
 				new EmployeeWithName("Frodo Baggins"), //
-				new EmployeeWithName("Bilbo Baggins")));
+				new EmployeeWithName("Bilbo Baggins"),
+				new EmployeeWithName(null)));
 	}

 	@Test
@@ -273,7 +275,14 @@ class QueryWithNullLikeIntegrationTests {
 	@Test // GH-1184
 	void alignedReturnTypeShouldWork() {
 		assertThat(repository.customQueryWithAlignedReturnType()).containsExactly(new Object[][] {
-				{ "Frodo Baggins", "Frodo Baggins with suffix" }, { "Bilbo Baggins", "Bilbo Baggins with suffix" } });
+				{ "Frodo Baggins", "Frodo Baggins with suffix" }, { "Bilbo Baggins", "Bilbo Baggins with suffix" }, { null, null} });
+	}
+
+	@Test
+	void nullOptionalParameterShouldReturnAllEntries() {
+		List<EmployeeWithName> result = repository.customQueryWithOptionalParameter(null);
+
+		assertThat(result).hasSize(3);
 	}

 	@Transactional
@@ -291,6 +300,9 @@ class QueryWithNullLikeIntegrationTests {
 		@Query(value = "select * from EmployeeWithName as e where e.name like %:partialName%", nativeQuery = true)
 		List<EmployeeWithName> customQueryWithNullableParamInNative(@Nullable @Param("partialName") String partialName);

+		@Query("select e from EmployeeWithName e where (:partialName is null or e.name like %:partialName%)")
+		List<EmployeeWithName> customQueryWithOptionalParameter(@Nullable @Param("partialName") String partialName);
+
 		List<EmployeeWithName> findByNameStartsWith(@Nullable String partialName);

 		List<EmployeeWithName> findByNameEndsWith(@Nullable String partialName);