Skip to content

Unwrapping of TypedParameterValue, return null instead of empty strin… #2655

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
* @author Thomas Darimont
* @author Mark Paluch
* @author Jens Schauder
* @author Yuriy Tsarkov
*/
public enum PersistenceProvider implements QueryExtractor, ProxyIdAccessor {

Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
* @author Christoph Strobl
* @author Jens Schauder
* @author Andrey Kovalev
* @author Yuriy Tsarkov
*/
class ParameterMetadataProvider {

Expand Down Expand Up @@ -181,6 +182,7 @@ EscapeCharacter getEscape() {
* @author Oliver Gierke
* @author Thomas Darimont
* @author Andrey Kovalev
* @author Yuriy Tsarkov
* @param <T>
*/
static class ParameterMetadata<T> {
Expand Down Expand Up @@ -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<? extends T> 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;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
* @author Jens Schauder
* @author Diego Krupitza
* @author Greg Turnquist
* @author Yuriy Tsarkov
*/
class StringQuery implements DeclaredQuery {

Expand Down Expand Up @@ -680,6 +681,7 @@ public Object prepare(@Nullable Object value) {
*
* @author Oliver Gierke
* @author Thomas Darimont
* @author Yuriy Tsarkov
*/
static class LikeParameterBinding extends ParameterBinding {

Expand Down Expand Up @@ -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;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -98,8 +101,7 @@ void customQueryWithNullMatch() {

List<EmployeeWithName> Employees = repository.customQueryWithNullableParam(null);

assertThat(Employees).extracting(EmployeeWithName::getName).containsExactlyInAnyOrder("Frodo Baggins",
"Bilbo Baggins");
assertThat(Employees).extracting(EmployeeWithName::getName).isEmpty();
}

@Test
Expand Down Expand Up @@ -128,6 +130,8 @@ void derivedQueryStartsWithWithEmptyStringMatch() {
}

@Test
@Ignore
@Deprecated
void derivedQueryStartsWithWithNullMatch() {

List<EmployeeWithName> Employees = repository.findByNameStartsWith(null);
Expand All @@ -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() {

Expand Down Expand Up @@ -163,6 +174,8 @@ void derivedQueryEndsWithWithEmptyStringMatch() {
}

@Test
@Ignore
@Deprecated
void derivedQueryEndsWithWithNullMatch() {

List<EmployeeWithName> Employees = repository.findByNameEndsWith(null);
Expand All @@ -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() {

Expand Down Expand Up @@ -198,6 +219,8 @@ void derivedQueryContainsWithEmptyStringMatch() {
}

@Test
@Ignore
@Deprecated
void derivedQueryContainsWithNullMatch() {

List<EmployeeWithName> Employees = repository.findByNameContains(null);
Expand All @@ -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() {

Expand Down Expand Up @@ -233,7 +264,7 @@ void derivedQueryLikeWithEmptyStringMatch() {
}

@Transactional
public interface EmpoyeeWithNullLikeRepository extends JpaRepository<EmployeeWithName, Integer> {
public interface EmployeeWithNullLikeRepository extends JpaRepository<EmployeeWithName, Integer> {

@Query("select e from EmployeeWithName e where e.name like %:partialName%")
List<EmployeeWithName> customQueryWithNullableParam(@Nullable @Param("partialName") String partialName);
Expand All @@ -248,7 +279,7 @@ public interface EmpoyeeWithNullLikeRepository extends JpaRepository<EmployeeWit
}

@EnableJpaRepositories(considerNestedRepositories = true, //
includeFilters = @Filter(type = FilterType.ASSIGNABLE_TYPE, classes = EmpoyeeWithNullLikeRepository.class))
includeFilters = @Filter(type = FilterType.ASSIGNABLE_TYPE, classes = EmployeeWithNullLikeRepository.class))
@EnableTransactionManagement
static class Config {

Expand Down