Skip to content

Commit 6fb88a2

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 f02100c commit 6fb88a2

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
@@ -233,31 +233,27 @@ public boolean isIsNullParameter() {
233233
/**
234234
* Prepares the object before it's actually bound to the {@link jakarta.persistence.Query;}.
235235
*
236-
* @param value must not be {@literal null}.
236+
* @param value can be {@literal null}.
237237
*/
238238
@Nullable
239-
public Object prepare(Object value) {
239+
public Object prepare(@Nullable Object value) {
240240

241-
Assert.notNull(value, "Value must not be null");
242-
243-
Object unwrapped = PersistenceProvider.unwrapTypedParameterValue(value);
244-
245-
if (unwrapped == null || expression.getJavaType() == null) {
246-
return unwrapped;
241+
if (value == null || expression.getJavaType() == null) {
242+
return value;
247243
}
248244

249245
if (String.class.equals(expression.getJavaType()) && !noWildcards) {
250246

251247
switch (type) {
252248
case STARTING_WITH:
253-
return String.format("%s%%", escape.escape(unwrapped.toString()));
249+
return String.format("%s%%", escape.escape(value.toString()));
254250
case ENDING_WITH:
255-
return String.format("%%%s", escape.escape(unwrapped.toString()));
251+
return String.format("%%%s", escape.escape(value.toString()));
256252
case CONTAINING:
257253
case NOT_CONTAINING:
258-
return String.format("%%%s%%", escape.escape(unwrapped.toString()));
254+
return String.format("%%%s%%", escape.escape(value.toString()));
259255
default:
260-
return unwrapped;
256+
return value;
261257
}
262258
}
263259

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

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

1818
import static org.assertj.core.api.Assumptions.assumeThat;
19+
import static org.assertj.core.api.Assertions.assertThat;
1920
import static org.mockito.ArgumentMatchers.any;
21+
import static org.mockito.ArgumentMatchers.eq;
2022
import static org.mockito.Mockito.mock;
2123
import static org.mockito.Mockito.never;
2224
import static org.mockito.Mockito.verify;
@@ -36,6 +38,7 @@
3638
import org.junit.jupiter.api.BeforeEach;
3739
import org.junit.jupiter.api.Test;
3840
import org.junit.jupiter.api.extension.ExtendWith;
41+
import org.mockito.ArgumentCaptor;
3942
import org.springframework.data.jpa.domain.sample.User;
4043
import org.springframework.data.jpa.provider.PersistenceProvider;
4144
import org.springframework.data.jpa.repository.EntityGraph;
@@ -56,6 +59,7 @@
5659
* @author Thomas Darimont
5760
* @author Mark Paluch
5861
* @author Krzysztof Krason
62+
* @author Julia Lee
5963
*/
6064
@ExtendWith(SpringExtension.class)
6165
@ContextConfiguration("classpath:infrastructure.xml")
@@ -65,12 +69,14 @@ class AbstractJpaQueryTests {
6569

6670
private Query query;
6771
private TypedQuery<Long> countQuery;
72+
private JpaQueryExecution execution;
6873

6974
@BeforeEach
7075
@SuppressWarnings("unchecked")
7176
void setUp() {
7277
query = mock(Query.class);
7378
countQuery = mock(TypedQuery.class);
79+
execution = mock(JpaQueryExecution.class);
7480
}
7581

7682
@Test // DATADOC-97
@@ -150,6 +156,37 @@ void shouldAddEntityGraphHintForLoad() throws Exception {
150156
verify(result).setHint("jakarta.persistence.loadgraph", entityGraph);
151157
}
152158

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

155192
Method method = SampleRepository.class.getMethod(name, parameterTypes);
@@ -164,6 +201,9 @@ interface SampleRepository extends Repository<User, Integer> {
164201
@QueryHints({ @QueryHint(name = "foo", value = "bar") })
165202
List<User> findByLastname(String lastname);
166203

204+
@org.springframework.data.jpa.repository.Query(value = "select u from User u where u.lastname = ?1", nativeQuery = true)
205+
List<User> findByLastnameNativeQuery(String lastname);
206+
167207
@QueryHints(value = { @QueryHint(name = "bar", value = "foo") }, forCounting = false)
168208
List<User> findByFirstname(String firstname);
169209

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

229+
@Override
230+
protected JpaQueryExecution getExecution() {
231+
return execution;
232+
}
233+
189234
@Override
190235
protected Query doCreateQuery(JpaParametersParameterAccessor accessor) {
191236
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/QueryWithNullLikeIntegrationTests.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 = QueryWithNullLikeIntegrationTests.Config.class)
@@ -66,7 +67,8 @@ class QueryWithNullLikeIntegrationTests {
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)