Skip to content

Commit 0a5ac69

Browse files
sxhinzvcmp911de
authored andcommitted
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
1 parent 5eb9c30 commit 0a5ac69

9 files changed

+111
-34
lines changed

spring-data-jpa/src/main/java/org/springframework/data/jpa/provider/PersistenceProvider.java

-11
Original file line numberDiff line numberDiff line change
@@ -106,12 +106,6 @@ public CloseableIterator<Object> executeQueryWithResultStream(Query jpaQuery) {
106106
return new HibernateScrollableResultsIterator(jpaQuery);
107107
}
108108

109-
@Override
110-
public JpaParametersParameterAccessor getParameterAccessor(JpaParameters parameters, Object[] values,
111-
EntityManager em) {
112-
return new HibernateJpaParametersParameterAccessor(parameters, values, em);
113-
}
114-
115109
@Override
116110
public String getCommentHintKey() {
117111
return "org.hibernate.comment";
@@ -292,11 +286,6 @@ public static PersistenceProvider fromMetamodel(Metamodel metamodel) {
292286
return cacheAndReturn(metamodelType, GENERIC_JPA);
293287
}
294288

295-
public JpaParametersParameterAccessor getParameterAccessor(JpaParameters parameters, Object[] values,
296-
EntityManager em) {
297-
return new JpaParametersParameterAccessor(parameters, values);
298-
}
299-
300289
/**
301290
* Returns the placeholder to be used for simple count queries. Default implementation returns {@code x}.
302291
*

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
* @author Jens Schauder
6262
* @author Сергей Цыпанов
6363
* @author Wonchul Heo
64+
* @author Julia Lee
6465
*/
6566
public abstract class AbstractJpaQuery implements RepositoryQuery {
6667

@@ -153,7 +154,11 @@ private Object doExecute(JpaQueryExecution execution, Object[] values) {
153154

154155
private JpaParametersParameterAccessor obtainParameterAccessor(Object[] values) {
155156

156-
return provider.getParameterAccessor(method.getParameters(), values, em);
157+
if (method.isNativeQuery() && PersistenceProvider.HIBERNATE.equals(provider)) {
158+
return new HibernateJpaParametersParameterAccessor(method.getParameters(), values, em);
159+
}
160+
161+
return new JpaParametersParameterAccessor(method.getParameters(), values);
157162
}
158163

159164
protected JpaQueryExecution getExecution() {
+2-2
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,14 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16-
package org.springframework.data.jpa.provider;
16+
package org.springframework.data.jpa.repository.query;
1717

1818
import jakarta.persistence.EntityManager;
1919

2020
import org.hibernate.engine.spi.SessionFactoryImplementor;
2121
import org.hibernate.query.TypedParameterValue;
2222
import org.hibernate.type.BasicType;
2323
import org.hibernate.type.BasicTypeRegistry;
24-
import org.springframework.data.jpa.repository.query.JpaParametersParameterAccessor;
2524
import org.springframework.data.repository.query.Parameter;
2625
import org.springframework.data.repository.query.Parameters;
2726
import org.springframework.data.repository.query.ParametersParameterAccessor;
@@ -38,6 +37,7 @@
3837
* @author Robert Wilson
3938
* @author Oliver Drotbohm
4039
* @author Greg Turnquist
40+
* @author Julia Lee
4141
* @since 2.7
4242
*/
4343
class HibernateJpaParametersParameterAccessor extends JpaParametersParameterAccessor {

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/ParameterMetadataProvider.java

+8-12
Original file line numberDiff line numberDiff line change
@@ -226,31 +226,27 @@ public boolean isIsNullParameter() {
226226
/**
227227
* Prepares the object before it's actually bound to the {@link jakarta.persistence.Query;}.
228228
*
229-
* @param value must not be {@literal null}.
229+
* @param value can be {@literal null}.
230230
*/
231231
@Nullable
232-
public Object prepare(Object value) {
232+
public Object prepare(@Nullable Object value) {
233233

234-
Assert.notNull(value, "Value must not be null");
235-
236-
Object unwrapped = PersistenceProvider.unwrapTypedParameterValue(value);
237-
238-
if (unwrapped == null || expression.getJavaType() == null) {
239-
return unwrapped;
234+
if (value == null || expression.getJavaType() == null) {
235+
return value;
240236
}
241237

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

244240
switch (type) {
245241
case STARTING_WITH:
246-
return String.format("%s%%", escape.escape(unwrapped.toString()));
242+
return String.format("%s%%", escape.escape(value.toString()));
247243
case ENDING_WITH:
248-
return String.format("%%%s", escape.escape(unwrapped.toString()));
244+
return String.format("%%%s", escape.escape(value.toString()));
249245
case CONTAINING:
250246
case NOT_CONTAINING:
251-
return String.format("%%%s%%", escape.escape(unwrapped.toString()));
247+
return String.format("%%%s%%", escape.escape(value.toString()));
252248
default:
253-
return unwrapped;
249+
return value;
254250
}
255251
}
256252

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/AbstractJpaQueryTests.java

+45
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515
*/
1616
package org.springframework.data.jpa.repository.query;
1717

18+
import static org.assertj.core.api.Assertions.assertThat;
1819
import static org.mockito.ArgumentMatchers.*;
20+
import static org.mockito.ArgumentMatchers.eq;
1921
import static org.mockito.Mockito.*;
2022
import static org.springframework.data.jpa.support.EntityManagerTestUtils.*;
2123

@@ -33,6 +35,7 @@
3335
import org.junit.jupiter.api.BeforeEach;
3436
import org.junit.jupiter.api.Test;
3537
import org.junit.jupiter.api.extension.ExtendWith;
38+
import org.mockito.ArgumentCaptor;
3639

3740
import org.springframework.data.jpa.domain.sample.User;
3841
import org.springframework.data.jpa.provider.PersistenceProvider;
@@ -53,6 +56,7 @@
5356
* @author Oliver Gierke
5457
* @author Thomas Darimont
5558
* @author Mark Paluch
59+
* @author Julia Lee
5660
*/
5761
@ExtendWith(SpringExtension.class)
5862
@ContextConfiguration("classpath:infrastructure.xml")
@@ -62,12 +66,14 @@ public class AbstractJpaQueryTests {
6266

6367
private Query query;
6468
private TypedQuery<Long> countQuery;
69+
private JpaQueryExecution execution;
6570

6671
@BeforeEach
6772
@SuppressWarnings("unchecked")
6873
void setUp() {
6974
query = mock(Query.class);
7075
countQuery = mock(TypedQuery.class);
76+
execution = mock(JpaQueryExecution.class);
7177
}
7278

7379
@Test // DATADOC-97
@@ -147,6 +153,37 @@ void shouldAddEntityGraphHintForLoad() throws Exception {
147153
verify(result).setHint("jakarta.persistence.loadgraph", entityGraph);
148154
}
149155

156+
@Test // GH-3137
157+
void shouldCreateHibernateJpaParameterParametersAccessorForNativeQuery() throws Exception {
158+
159+
JpaQueryMethod queryMethod = getMethod("findByLastnameNativeQuery", String.class);
160+
161+
AbstractJpaQuery jpaQuery = new DummyJpaQuery(queryMethod, em);
162+
163+
jpaQuery.execute(new Object[] {"some last name"});
164+
165+
ArgumentCaptor<JpaParametersParameterAccessor> captor = ArgumentCaptor.forClass(JpaParametersParameterAccessor.class);
166+
verify(execution).execute(eq(jpaQuery), captor.capture());
167+
JpaParametersParameterAccessor parameterAccessor = captor.getValue();
168+
169+
assertThat(parameterAccessor).isInstanceOf(HibernateJpaParametersParameterAccessor.class);
170+
}
171+
172+
@Test // GH-3137
173+
void shouldCreateGenericJpaParameterParametersAccessorForNonNativeQuery() throws Exception {
174+
175+
JpaQueryMethod queryMethod = getMethod("findByFirstname", String.class);
176+
AbstractJpaQuery jpaQuery = new DummyJpaQuery(queryMethod, em);
177+
178+
jpaQuery.execute(new Object[] {"some first name"});
179+
180+
ArgumentCaptor<JpaParametersParameterAccessor> captor = ArgumentCaptor.forClass(JpaParametersParameterAccessor.class);
181+
verify(execution).execute(eq(jpaQuery), captor.capture());
182+
JpaParametersParameterAccessor parameterAccessor = captor.getValue();
183+
184+
assertThat(parameterAccessor).isNotInstanceOf(HibernateJpaParametersParameterAccessor.class);
185+
}
186+
150187
private JpaQueryMethod getMethod(String name, Class<?>... parameterTypes) throws Exception {
151188

152189
Method method = SampleRepository.class.getMethod(name, parameterTypes);
@@ -161,6 +198,9 @@ interface SampleRepository extends Repository<User, Integer> {
161198
@QueryHints({ @QueryHint(name = "foo", value = "bar") })
162199
List<User> findByLastname(String lastname);
163200

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

@@ -183,6 +223,11 @@ class DummyJpaQuery extends AbstractJpaQuery {
183223
super(method, em);
184224
}
185225

226+
@Override
227+
protected JpaQueryExecution getExecution() {
228+
return execution;
229+
}
230+
186231
@Override
187232
protected Query doCreateQuery(JpaParametersParameterAccessor accessor) {
188233
return query;
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package org.springframework.data.jpa.provider;
1+
package org.springframework.data.jpa.repository.query;
22

33
import jakarta.persistence.EntityManager;
44

@@ -8,6 +8,7 @@
88
import org.junit.jupiter.api.Test;
99
import org.junit.jupiter.api.extension.ExtendWith;
1010
import org.springframework.beans.factory.annotation.Autowired;
11+
import org.springframework.data.jpa.repository.query.HibernateJpaParametersParameterAccessor;
1112
import org.springframework.data.jpa.repository.query.JpaParameters;
1213
import org.springframework.test.context.ContextConfiguration;
1314
import org.springframework.test.context.junit.jupiter.SpringExtension;

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaParametersParameterAccessorTests.java

+2-4
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ void createsJpaParametersParameterAccessor() throws Exception {
4343
Method withNativeQuery = SampleRepository.class.getMethod("withNativeQuery", Integer.class);
4444
Object[] values = { null };
4545
JpaParameters parameters = new JpaParameters(withNativeQuery);
46-
JpaParametersParameterAccessor accessor = PersistenceProvider.GENERIC_JPA.getParameterAccessor(parameters, values,
47-
em);
46+
JpaParametersParameterAccessor accessor = new JpaParametersParameterAccessor(parameters, values);
4847

4948
bind(parameters, accessor);
5049

@@ -57,8 +56,7 @@ void createsHibernateParametersParameterAccessor() throws Exception {
5756
Method withNativeQuery = SampleRepository.class.getMethod("withNativeQuery", Integer.class);
5857
Object[] values = { null };
5958
JpaParameters parameters = new JpaParameters(withNativeQuery);
60-
JpaParametersParameterAccessor accessor = PersistenceProvider.HIBERNATE.getParameterAccessor(parameters, values,
61-
em);
59+
JpaParametersParameterAccessor accessor = new HibernateJpaParametersParameterAccessor(parameters, values, em);
6260

6361
bind(parameters, accessor);
6462

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/ParameterMetadataProviderUnitTests.java

+32-1
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,32 @@
2222

2323
import jakarta.persistence.criteria.CriteriaBuilder;
2424

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

29+
import org.junit.jupiter.api.Test;
30+
import org.junit.jupiter.api.extension.ExtendWith;
31+
import org.mockito.Answers;
32+
import org.mockito.Mock;
33+
import org.mockito.junit.jupiter.MockitoExtension;
34+
import org.mockito.junit.jupiter.MockitoSettings;
35+
import org.mockito.quality.Strictness;
36+
2937
/**
3038
* Unit tests for {@link ParameterMetadataProvider}.
3139
*
3240
* @author Jens Schauder
41+
* @author Julia Lee
3342
*/
43+
@ExtendWith(MockitoExtension.class)
44+
@MockitoSettings(strictness = Strictness.STRICT_STUBS)
3445
class ParameterMetadataProviderUnitTests {
3546

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

@@ -49,4 +64,20 @@ void errorMessageMentionesParametersWhenParametersAreExhausted() {
4964
.withMessageContaining("parameter");
5065
}
5166

67+
@Test // GH-3137
68+
void returnAugmentedValueForStringExpressions() {
69+
when(part.getProperty().getLeafProperty().isCollection()).thenReturn(false);
70+
71+
assertThat(createParameterMetadata(Part.Type.STARTING_WITH).prepare("starting with")).isEqualTo("starting with%");
72+
assertThat(createParameterMetadata(Part.Type.ENDING_WITH).prepare("ending with")).isEqualTo("%ending with");
73+
assertThat(createParameterMetadata(Part.Type.CONTAINING).prepare("containing")).isEqualTo("%containing%");
74+
assertThat(createParameterMetadata(Part.Type.NOT_CONTAINING).prepare("not containing")).isEqualTo("%not containing%");
75+
assertThat(createParameterMetadata(Part.Type.LIKE).prepare("%like%")).isEqualTo("%like%");
76+
assertThat(createParameterMetadata(Part.Type.IS_NULL).prepare(null)).isEqualTo(null);
77+
}
78+
79+
private ParameterMetadataProvider.ParameterMetadata createParameterMetadata(Part.Type partType) {
80+
when(part.getType()).thenReturn(partType);
81+
return new ParameterMetadataProvider.ParameterMetadata<>(parameterExpression, part, null, EscapeCharacter.DEFAULT);
82+
}
5283
}

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryWithNullLikeHibernateIntegrationTests.java

+14-2
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
*
5555
* @author Greg Turnquist
5656
* @author Yuriy Tsarkov
57+
* @author Julia Lee
5758
*/
5859
@ExtendWith(SpringExtension.class)
5960
@ContextConfiguration(classes = QueryWithNullLikeHibernateIntegrationTests.Config.class)
@@ -66,7 +67,8 @@ public class QueryWithNullLikeHibernateIntegrationTests {
6667
void setUp() {
6768
repository.saveAllAndFlush(List.of( //
6869
new EmployeeWithName("Frodo Baggins"), //
69-
new EmployeeWithName("Bilbo Baggins")));
70+
new EmployeeWithName("Bilbo Baggins"),
71+
new EmployeeWithName(null)));
7072
}
7173

7274
@Test
@@ -273,7 +275,14 @@ void mismatchedReturnTypeShouldCauseException() {
273275
@Test // GH-1184
274276
void alignedReturnTypeShouldWork() {
275277
assertThat(repository.customQueryWithAlignedReturnType()).containsExactly(new Object[][] {
276-
{ "Frodo Baggins", "Frodo Baggins with suffix" }, { "Bilbo Baggins", "Bilbo Baggins with suffix" } });
278+
{ "Frodo Baggins", "Frodo Baggins with suffix" }, { "Bilbo Baggins", "Bilbo Baggins with suffix" }, { null, null} });
279+
}
280+
281+
@Test
282+
void nullOptionalParameterShouldReturnAllEntries() {
283+
List<EmployeeWithName> result = repository.customQueryWithOptionalParameter(null);
284+
285+
assertThat(result).hasSize(3);
277286
}
278287

279288
@Transactional
@@ -291,6 +300,9 @@ public interface EmployeeWithNullLikeRepository extends JpaRepository<EmployeeWi
291300
@Query(value = "select * from EmployeeWithName as e where e.name like %:partialName%", nativeQuery = true)
292301
List<EmployeeWithName> customQueryWithNullableParamInNative(@Nullable @Param("partialName") String partialName);
293302

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

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

0 commit comments

Comments
 (0)