From 852ff4545854ed0cbcd5a40cbe87250888e105fe Mon Sep 17 00:00:00 2001 From: Yuriy Tsarkov Date: Mon, 3 Oct 2022 05:02:38 +0300 Subject: [PATCH] Unwrapping of TypedParameterValue, return null instead of empty string for string params. Fixes #2653 Related tickets #2548 --- .../jpa/provider/PersistenceProvider.java | 3 +- .../query/ParameterMetadataProvider.java | 15 +++---- .../jpa/repository/query/StringQuery.java | 14 ++++--- ...WithNullLikeHibernateIntegrationTests.java | 41 ++++++++++++++++--- 4 files changed, 54 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/provider/PersistenceProvider.java b/src/main/java/org/springframework/data/jpa/provider/PersistenceProvider.java index 19c1fbfc9b..6a5b814f0b 100644 --- a/src/main/java/org/springframework/data/jpa/provider/PersistenceProvider.java +++ b/src/main/java/org/springframework/data/jpa/provider/PersistenceProvider.java @@ -50,6 +50,7 @@ * @author Thomas Darimont * @author Mark Paluch * @author Jens Schauder + * @author Yuriy Tsarkov */ public enum PersistenceProvider implements QueryExtractor, ProxyIdAccessor { @@ -349,7 +350,7 @@ public static Object condense(Object value) { Class typeParameterValue = ClassUtils.forName("org.hibernate.jpa.TypedParameterValue", classLoader); if (typeParameterValue.isInstance(value)) { - return ""; + return null; } } catch (ClassNotFoundException | LinkageError o_O) { return value; diff --git a/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java b/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java index 05e2c45bd4..df7a868b98 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java @@ -45,6 +45,7 @@ * @author Christoph Strobl * @author Jens Schauder * @author Andrey Kovalev + * @author Yuriy Tsarkov */ class ParameterMetadataProvider { @@ -181,6 +182,7 @@ EscapeCharacter getEscape() { * @author Oliver Gierke * @author Thomas Darimont * @author Andrey Kovalev + * @author Yuriy Tsarkov * @param */ static class ParameterMetadata { @@ -227,23 +229,22 @@ public boolean isIsNullParameter() { */ @Nullable public Object prepare(Object value) { - - Assert.notNull(value, "Value must not be null!"); + Object unwrappedValue = PersistenceProvider.condense(value); + Assert.notNull(unwrappedValue, "Value must not be null!"); Class expressionType = expression.getJavaType(); if (String.class.equals(expressionType)) { - switch (type) { case STARTING_WITH: - return String.format("%s%%", escape.escape(PersistenceProvider.condense(value).toString())); + return String.format("%s%%", escape.escape(unwrappedValue.toString())); case ENDING_WITH: - return String.format("%%%s", escape.escape(PersistenceProvider.condense(value).toString())); + return String.format("%%%s", escape.escape(unwrappedValue.toString())); case CONTAINING: case NOT_CONTAINING: - return String.format("%%%s%%", escape.escape(PersistenceProvider.condense(value).toString())); + return String.format("%%%s%%", escape.escape(unwrappedValue.toString())); default: - return PersistenceProvider.condense(value); + return unwrappedValue; } } diff --git a/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java index ce80e2409a..8d409960f1 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java @@ -49,6 +49,7 @@ * @author Jens Schauder * @author Diego Krupitza * @author Greg Turnquist + * @author Yuriy Tsarkov */ class StringQuery implements DeclaredQuery { @@ -680,6 +681,7 @@ public Object prepare(@Nullable Object value) { * * @author Oliver Gierke * @author Thomas Darimont + * @author Yuriy Tsarkov */ static class LikeParameterBinding extends ParameterBinding { @@ -764,21 +766,21 @@ public Type getType() { @Nullable @Override public Object prepare(@Nullable Object value) { - - if (value == null) { + Object unwrappedValue = PersistenceProvider.condense(value); + if (unwrappedValue == null) { return null; } switch (type) { case STARTING_WITH: - return String.format("%s%%", PersistenceProvider.condense(value)); + return String.format("%s%%", unwrappedValue); case ENDING_WITH: - return String.format("%%%s", PersistenceProvider.condense(value)); + return String.format("%%%s", unwrappedValue); case CONTAINING: - return String.format("%%%s%%", PersistenceProvider.condense(value)); + return String.format("%%%s%%", unwrappedValue); case LIKE: default: - return PersistenceProvider.condense(value); + return unwrappedValue; } } diff --git a/src/test/java/org/springframework/data/jpa/repository/query/QueryWithNullLikeHibernateIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/query/QueryWithNullLikeHibernateIntegrationTests.java index a5fac8892a..8d1049d2aa 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/QueryWithNullLikeHibernateIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/QueryWithNullLikeHibernateIntegrationTests.java @@ -16,6 +16,7 @@ package org.springframework.data.jpa.repository.query; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.util.Arrays; import java.util.List; @@ -24,6 +25,7 @@ import javax.persistence.EntityManagerFactory; import javax.sql.DataSource; +import org.junit.Ignore; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -52,13 +54,14 @@ * Verify that {@literal LIKE}s mixed with {@literal NULL}s work properly. * * @author Greg Turnquist + * @author Yuriy Tsarkov */ @ExtendWith(SpringExtension.class) @ContextConfiguration(classes = QueryWithNullLikeHibernateIntegrationTests.Config.class) @Transactional public class QueryWithNullLikeHibernateIntegrationTests { - @Autowired EmpoyeeWithNullLikeRepository repository; + @Autowired EmployeeWithNullLikeRepository repository; @BeforeEach void setUp() { @@ -98,8 +101,7 @@ void customQueryWithNullMatch() { List Employees = repository.customQueryWithNullableParam(null); - assertThat(Employees).extracting(EmployeeWithName::getName).containsExactlyInAnyOrder("Frodo Baggins", - "Bilbo Baggins"); + assertThat(Employees).extracting(EmployeeWithName::getName).isEmpty(); } @Test @@ -128,6 +130,8 @@ void derivedQueryStartsWithWithEmptyStringMatch() { } @Test + @Ignore + @Deprecated void derivedQueryStartsWithWithNullMatch() { List Employees = repository.findByNameStartsWith(null); @@ -136,6 +140,13 @@ void derivedQueryStartsWithWithNullMatch() { "Bilbo Baggins"); } + @Test // GH-2653 + void derivedQueryStartsWithWithNullFail() { + assertThatThrownBy(() -> repository.findByNameStartsWith(null)) + .hasCauseExactlyInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Value must not be null!"); + } + @Test void derivedQueryEndsWithWithMultipleMatch() { @@ -163,6 +174,8 @@ void derivedQueryEndsWithWithEmptyStringMatch() { } @Test + @Ignore + @Deprecated void derivedQueryEndsWithWithNullMatch() { List Employees = repository.findByNameEndsWith(null); @@ -171,6 +184,14 @@ void derivedQueryEndsWithWithNullMatch() { "Bilbo Baggins"); } + @Test // GH-2653 + void derivedQueryEndsWithWithNullFail() { + + assertThatThrownBy(() -> repository.findByNameStartsWith(null)) + .hasCauseExactlyInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Value must not be null!"); + } + @Test void derivedQueryContainsWithMultipleMatch() { @@ -198,6 +219,8 @@ void derivedQueryContainsWithEmptyStringMatch() { } @Test + @Ignore + @Deprecated void derivedQueryContainsWithNullMatch() { List Employees = repository.findByNameContains(null); @@ -206,6 +229,14 @@ void derivedQueryContainsWithNullMatch() { "Bilbo Baggins"); } + @Test // GH-2653 + void derivedQueryContainsWithNullFail() { + + assertThatThrownBy(() -> repository.findByNameStartsWith(null)) + .hasCauseExactlyInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Value must not be null!"); + } + @Test void derivedQueryLikeWithMultipleMatch() { @@ -233,7 +264,7 @@ void derivedQueryLikeWithEmptyStringMatch() { } @Transactional - public interface EmpoyeeWithNullLikeRepository extends JpaRepository { + public interface EmployeeWithNullLikeRepository extends JpaRepository { @Query("select e from EmployeeWithName e where e.name like %:partialName%") List customQueryWithNullableParam(@Nullable @Param("partialName") String partialName); @@ -248,7 +279,7 @@ public interface EmpoyeeWithNullLikeRepository extends JpaRepository