Skip to content

Commit

Permalink
Restrict TypedParameterValue usage to native queries only.
Browse files Browse the repository at this point in the history
Use Hibernate parameter accessor for native queries only to avoid affecting JPA queries.

See #3137
  • Loading branch information
sxhinzvc committed Sep 25, 2023
1 parent 09a7e7a commit dfc70d4
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
* @author Greg Turnquist
* @since 2.7
*/
class HibernateJpaParametersParameterAccessor extends JpaParametersParameterAccessor {
public class HibernateJpaParametersParameterAccessor extends JpaParametersParameterAccessor {

private final BasicTypeRegistry typeHelper;

Expand All @@ -51,7 +51,7 @@ class HibernateJpaParametersParameterAccessor extends JpaParametersParameterAcce
* @param values must not be {@literal null}.
* @param em must not be {@literal null}.
*/
HibernateJpaParametersParameterAccessor(Parameters<?, ?> parameters, Object[] values, EntityManager em) {
public HibernateJpaParametersParameterAccessor(Parameters<?, ?> parameters, Object[] values, EntityManager em) {

super(parameters, values);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.util.stream.Collectors;

import org.springframework.core.convert.converter.Converter;
import org.springframework.data.jpa.provider.HibernateJpaParametersParameterAccessor;
import org.springframework.data.jpa.provider.PersistenceProvider;
import org.springframework.data.jpa.repository.EntityGraph;
import org.springframework.data.jpa.repository.query.JpaQueryExecution.CollectionExecution;
Expand Down Expand Up @@ -153,7 +154,11 @@ private Object doExecute(JpaQueryExecution execution, Object[] values) {

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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,31 +233,27 @@ public boolean isIsNullParameter() {
/**
* Prepares the object before it's actually bound to the {@link jakarta.persistence.Query;}.
*
* @param value must not be {@literal null}.
* @param value the value to be prepared.
*/
@Nullable
public Object prepare(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;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -36,7 +38,9 @@
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.HibernateJpaParametersParameterAccessor;
import org.springframework.data.jpa.provider.PersistenceProvider;
import org.springframework.data.jpa.repository.EntityGraph;
import org.springframework.data.jpa.repository.EntityGraph.EntityGraphType;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -150,6 +156,37 @@ void shouldAddEntityGraphHintForLoad() throws Exception {
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);
Expand All @@ -164,6 +201,9 @@ interface SampleRepository extends Repository<User, Integer> {
@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);

Expand All @@ -186,6 +226,11 @@ class DummyJpaQuery extends AbstractJpaQuery {
super(method, em);
}

@Override
protected JpaQueryExecution getExecution() {
return execution;
}

@Override
protected Query doCreateQuery(JpaParametersParameterAccessor accessor) {
return query;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,32 @@

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() {

Expand All @@ -49,4 +64,20 @@ void errorMessageMentionesParametersWhenParametersAreExhausted() {
.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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,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
Expand Down Expand Up @@ -273,7 +274,14 @@ void mismatchedReturnTypeShouldCauseException() {
@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
Expand All @@ -291,6 +299,9 @@ public interface EmployeeWithNullLikeRepository extends JpaRepository<EmployeeWi
@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);
Expand Down

0 comments on commit dfc70d4

Please sign in to comment.