Skip to content

Use TypedParameterValue and HibernateParameterParametersAccessor for native queries only #3173

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 3 commits 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
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-jpa-parent</artifactId>
<version>3.2.0-SNAPSHOT</version>
<version>3.2.x-gh-3137-SNAPSHOT</version>
<packaging>pom</packaging>

<name>Spring Data JPA Parent</name>
Expand Down
4 changes: 2 additions & 2 deletions spring-data-envers/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-envers</artifactId>
<version>3.2.0-SNAPSHOT</version>
<version>3.2.x-gh-3137-SNAPSHOT</version>

<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-jpa-parent</artifactId>
<version>3.2.0-SNAPSHOT</version>
<version>3.2.x-gh-3137-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
2 changes: 1 addition & 1 deletion spring-data-jpa-distribution/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-jpa-parent</artifactId>
<version>3.2.0-SNAPSHOT</version>
<version>3.2.x-gh-3137-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
4 changes: 2 additions & 2 deletions spring-data-jpa/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-jpa</artifactId>
<version>3.2.0-SNAPSHOT</version>
<version>3.2.x-gh-3137-SNAPSHOT</version>

<name>Spring Data JPA</name>
<description>Spring Data module for JPA repositories.</description>
Expand All @@ -15,7 +15,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-jpa-parent</artifactId>
<version>3.2.0-SNAPSHOT</version>
<version>3.2.x-gh-3137-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,6 @@ public CloseableIterator<Object> executeQueryWithResultStream(Query jpaQuery) {
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";
Expand Down Expand Up @@ -292,11 +286,6 @@ public static PersistenceProvider fromMetamodel(Metamodel metamodel) {
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}.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
* @author Jens Schauder
* @author Сергей Цыпанов
* @author Wonchul Heo
* @author Julia Lee
*/
public abstract class AbstractJpaQuery implements RepositoryQuery {

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 @@ -13,15 +13,14 @@
* 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;

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;
Expand All @@ -38,6 +37,7 @@
* @author Robert Wilson
* @author Oliver Drotbohm
* @author Greg Turnquist
* @author Julia Lee
* @since 2.7
*/
class HibernateJpaParametersParameterAccessor extends JpaParametersParameterAccessor {
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 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;
}
}

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,6 +38,7 @@
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;
Expand All @@ -56,6 +59,7 @@
* @author Thomas Darimont
* @author Mark Paluch
* @author Krzysztof Krason
* @author Julia Lee
*/
@ExtendWith(SpringExtension.class)
@ContextConfiguration("classpath:infrastructure.xml")
Expand All @@ -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
@@ -1,4 +1,4 @@
package org.springframework.data.jpa.provider;
package org.springframework.data.jpa.repository.query;

import jakarta.persistence.EntityManager;

Expand All @@ -8,6 +8,7 @@
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ void createsJpaParametersParameterAccessor() throws Exception {
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);

Expand All @@ -57,8 +56,7 @@ void createsHibernateParametersParameterAccessor() throws Exception {
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);

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 @@ -54,6 +54,7 @@
*
* @author Greg Turnquist
* @author Yuriy Tsarkov
* @author Julia Lee
*/
@ExtendWith(SpringExtension.class)
@ContextConfiguration(classes = QueryWithNullLikeIntegrationTests.Config.class)
Expand All @@ -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
Expand Down Expand Up @@ -273,7 +275,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 +300,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