Skip to content

Commit 63c7fd7

Browse files
gavinkinggavinking
committed
rethink of the previous approach to query parameter binding validation
1 parent ce1751b commit 63c7fd7

File tree

6 files changed

+164
-122
lines changed

6 files changed

+164
-122
lines changed

‎hibernate-core/src/main/java/org/hibernate/query/internal/QueryArguments.java‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,13 @@ public class QueryArguments {
2121

2222
private static boolean isInstance(Object value, JavaType<?> javaType) {
2323
try {
24-
// special handling for entity arguments due to
25-
// the possibility of an uninitialized proxy
26-
// (which we don't want or need to fetch)
27-
if ( javaType instanceof EntityJavaType<?> ) {
24+
if ( value == null ) {
25+
return true;
26+
}
27+
else if ( javaType instanceof EntityJavaType<?> ) {
28+
// special handling for entity arguments due to
29+
// the possibility of an uninitialized proxy
30+
// (which we don't want or need to fetch)
2831
final var javaTypeClass = javaType.getJavaTypeClass();
2932
final var initializer = extractLazyInitializer( value );
3033
final var valueEntityClass =
@@ -94,4 +97,31 @@ public static boolean areInstances(
9497
}
9598
return true;
9699
}
100+
101+
public static <T> T cast(Object value, JavaType<T> javaType) {
102+
if ( value == null ) {
103+
return null;
104+
}
105+
else if ( javaType instanceof EntityJavaType<?> ) {
106+
// special handling for entity arguments due to
107+
// the possibility of an uninitialized proxy
108+
// (which we don't want or need to fetch)
109+
if ( isInstance( value, javaType ) ) {
110+
// The proxy might not literally be an
111+
// instance of the entity class represented
112+
// by the unreified type T, but it is an
113+
// instance in spirit
114+
//noinspection unchecked
115+
return (T) value;
116+
}
117+
else {
118+
throw new ClassCastException( "Cannot cast to entity type '"
119+
+ javaType.getJavaTypeClass().getTypeName() + "'" );
120+
}
121+
}
122+
else {
123+
// require that the argument be assignable to the parameter
124+
return javaType.cast( javaType.coerce( value ) );
125+
}
126+
}
97127
}

‎hibernate-core/src/main/java/org/hibernate/query/internal/QueryParameterBindingImpl.java‎

Lines changed: 89 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222

2323
import jakarta.persistence.TemporalType;
2424

25-
import static org.hibernate.query.internal.QueryParameterBindingValidator.validate;
2625
import static org.hibernate.type.descriptor.java.JavaTypeHelper.isTemporal;
2726
import static org.hibernate.type.internal.BindingTypeHelper.resolveTemporalPrecision;
2827

@@ -38,11 +37,11 @@ public class QueryParameterBindingImpl<T> implements QueryParameterBinding<T> {
3837
private boolean isBound;
3938
private boolean isMultiValued;
4039

41-
private @Nullable BindableType<? super T> bindType;
40+
private @Nullable BindableType<T> bindType;
4241
private @Nullable MappingModelExpressible<T> type;
43-
private @Nullable TemporalType explicitTemporalPrecision;
42+
private @Nullable @SuppressWarnings("deprecation") TemporalType explicitTemporalPrecision;
4443

45-
private Object bindValue;
44+
private T bindValue;
4645
private Collection<? extends T> bindValues;
4746

4847
/**
@@ -60,7 +59,7 @@ protected QueryParameterBindingImpl(
6059
public QueryParameterBindingImpl(
6160
QueryParameter<T> queryParameter,
6261
SessionFactoryImplementor sessionFactory,
63-
BindableType<? super T> bindType) {
62+
@Nullable BindableType<T> bindType) {
6463
this.queryParameter = queryParameter;
6564
this.sessionFactory = sessionFactory;
6665
this.bindType = bindType;
@@ -70,13 +69,17 @@ private QueryParameterBindingTypeResolver getParameterBindingTypeResolver() {
7069
return sessionFactory.getMappingMetamodel();
7170
}
7271

72+
public TypeConfiguration getTypeConfiguration() {
73+
return sessionFactory.getTypeConfiguration();
74+
}
75+
7376
@Override
74-
public @Nullable BindableType<? super T> getBindType() {
77+
public @Nullable BindableType<T> getBindType() {
7578
return bindType;
7679
}
7780

7881
@Override
79-
public @Nullable TemporalType getExplicitTemporalPrecision() {
82+
public @Nullable @SuppressWarnings("deprecation") TemporalType getExplicitTemporalPrecision() {
8083
return explicitTemporalPrecision;
8184
}
8285

@@ -107,17 +110,14 @@ public T getBindValue() {
107110
if ( isMultiValued ) {
108111
throw new IllegalStateException( "Binding is multi-valued; illegal call to #getBindValue" );
109112
}
110-
111-
//TODO: I believe this cast is unsound due to coercion
112-
return (T) bindValue;
113+
return bindValue;
113114
}
114115

115116
@Override
116-
public void setBindValue(T value, boolean resolveJdbcTypeIfNecessary) {
117+
public void setBindValue(Object value, boolean resolveJdbcTypeIfNecessary) {
117118
if ( !handleAsMultiValue( value, null ) ) {
118-
final Object coerced = coerceIfNotJpa( value );
119-
validate( getBindType(), coerced, sessionFactory );
120-
119+
final Object coerced = coerce( value );
120+
validate( coerced );
121121
if ( value == null ) {
122122
// needed when setting a null value to the parameter of a native SQL query
123123
// TODO: this does not look like a very disciplined way to handle this
@@ -129,70 +129,73 @@ public void setBindValue(T value, boolean resolveJdbcTypeIfNecessary) {
129129
}
130130
}
131131

132-
private void bindNull(boolean resolveJdbcTypeIfNecessary) {
133-
isBound = true;
134-
bindValue = null;
135-
if ( resolveJdbcTypeIfNecessary && bindType == null ) {
136-
bindType = (BindableType<? super T>)
137-
getTypeConfiguration().getBasicTypeRegistry()
138-
.getRegisteredType( "null" );
139-
}
140-
}
141-
142-
private boolean handleAsMultiValue(T value, @Nullable BindableType<T> bindableType) {
143-
if ( queryParameter.allowsMultiValuedBinding()
144-
&& value instanceof Collection
145-
&& !( bindableType == null
146-
? isRegisteredAsBasicType( value.getClass() )
147-
: bindableType.getJavaType().isInstance( value ) ) ) {
148-
//noinspection unchecked
149-
setBindValues( (Collection<T>) value );
150-
return true;
151-
}
152-
else {
153-
return false;
154-
}
155-
}
156-
157-
private boolean isRegisteredAsBasicType(Class<?> valueClass) {
158-
return getTypeConfiguration().getBasicTypeForJavaType( valueClass ) != null;
159-
}
160-
161-
private void bindValue(Object value) {
162-
isBound = true;
163-
bindValue = value;
164-
if ( canBindValueBeSet( value, bindType ) ) {
165-
bindType = getParameterBindingTypeResolver().resolveParameterBindType( value );
166-
}
167-
}
168-
169132
@Override
170-
public void setBindValue(T value, @Nullable BindableType<T> clarifiedType) {
133+
public void setBindValue(Object value, @Nullable BindableType<T> clarifiedType) {
171134
if ( !handleAsMultiValue( value, clarifiedType ) ) {
172135
if ( clarifiedType != null ) {
173136
bindType = clarifiedType;
174137
}
175138

176139
final Object coerced = coerce( value );
177-
validate( clarifiedType, coerced, sessionFactory );
140+
validate( coerced );
178141
bindValue( coerced );
179142
}
180143
}
181144

182145
@Override
183-
public void setBindValue(T value, TemporalType temporalTypePrecision) {
146+
public void setBindValue(Object value, @SuppressWarnings("deprecation") TemporalType temporalTypePrecision) {
184147
if ( !handleAsMultiValue( value, null ) ) {
185148
if ( bindType == null ) {
186149
bindType = queryParameter.getHibernateType();
187150
}
188151

189-
final Object coerced = coerceIfNotJpa( value );
190-
validate( getBindType(), coerced, sessionFactory );
152+
final Object coerced = coerce( value );
153+
validate( coerced );
191154
bindValue( coerced );
192155
setExplicitTemporalPrecision( temporalTypePrecision );
193156
}
194157
}
195158

159+
private void bindValue(Object value) {
160+
if ( canBindValueBeSet( value, bindType ) ) {
161+
bindType = (BindableType<T>) (BindableType)
162+
getParameterBindingTypeResolver()
163+
.resolveParameterBindType( value );
164+
}
165+
bindValue = cast( value );
166+
isBound = true;
167+
}
168+
169+
private void bindNull(boolean resolveJdbcTypeIfNecessary) {
170+
isBound = true;
171+
bindValue = null;
172+
if ( resolveJdbcTypeIfNecessary && bindType == null ) {
173+
final var nullType =
174+
getTypeConfiguration().getBasicTypeRegistry()
175+
.getRegisteredType( "null" );
176+
//noinspection unchecked
177+
bindType = (BindableType<T>) nullType;
178+
}
179+
}
180+
181+
private boolean handleAsMultiValue(Object value, @Nullable BindableType<T> bindableType) {
182+
if ( queryParameter.allowsMultiValuedBinding()
183+
&& value instanceof Collection
184+
&& !( bindableType == null
185+
? isRegisteredAsBasicType( value.getClass() )
186+
: bindableType.getJavaType().isInstance( value ) ) ) {
187+
setBindValues( (Collection<?>) value );
188+
return true;
189+
}
190+
else {
191+
return false;
192+
}
193+
}
194+
195+
private boolean isRegisteredAsBasicType(Class<?> valueClass) {
196+
return getTypeConfiguration().getBasicTypeForJavaType( valueClass ) != null;
197+
}
198+
196199
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
197200
// multi-valued binding support
198201

@@ -205,22 +208,27 @@ public Collection<? extends T> getBindValues() {
205208
}
206209

207210
@Override
208-
public void setBindValues(Collection<? extends T> values) {
211+
public void setBindValues(Collection<?> values) {
209212
if ( !queryParameter.allowsMultiValuedBinding() ) {
210213
throw new IllegalArgumentException(
211214
"Illegal attempt to bind a collection value to a single-valued parameter"
212215
);
213216
}
214217

215-
this.isBound = true;
216-
this.isMultiValued = true;
218+
final var coerced = values.stream().map( this::coerce ).toList();
219+
values.forEach( this::validate );
217220

218-
this.bindValue = null;
219-
this.bindValues = values;
221+
isBound = true;
222+
isMultiValued = true;
220223

221-
final T value = firstNonNull( values );
224+
bindValue = null;
225+
bindValues = coerced.stream().map( this::cast ).toList();
226+
227+
final T value = firstNonNull( bindValues );
222228
if ( canBindValueBeSet( value, bindType ) ) {
223-
bindType = getParameterBindingTypeResolver().resolveParameterBindType( value );
229+
bindType = (BindableType<T>) (BindableType)
230+
getParameterBindingTypeResolver()
231+
.resolveParameterBindType( value );
224232
}
225233
}
226234

@@ -234,7 +242,7 @@ public void setBindValues(Collection<? extends T> values) {
234242
}
235243

236244
@Override
237-
public void setBindValues(Collection<? extends T> values, BindableType<T> clarifiedType) {
245+
public void setBindValues(Collection<?> values, BindableType<T> clarifiedType) {
238246
if ( clarifiedType != null ) {
239247
bindType = clarifiedType;
240248
}
@@ -243,14 +251,14 @@ public void setBindValues(Collection<? extends T> values, BindableType<T> clarif
243251

244252
@Override
245253
public void setBindValues(
246-
Collection<? extends T> values,
247-
TemporalType temporalTypePrecision,
254+
Collection<?> values,
255+
@SuppressWarnings("deprecation") TemporalType temporalTypePrecision,
248256
TypeConfiguration typeConfiguration) {
249257
setBindValues( values );
250258
setExplicitTemporalPrecision( temporalTypePrecision );
251259
}
252260

253-
private void setExplicitTemporalPrecision(TemporalType precision) {
261+
private void setExplicitTemporalPrecision(@SuppressWarnings("deprecation") TemporalType precision) {
254262
explicitTemporalPrecision = precision;
255263
if ( bindType == null || isTemporal( determineJavaType( bindType ) ) ) {
256264
bindType = resolveTemporalPrecision( precision, bindType, getCriteriaBuilder() );
@@ -273,32 +281,33 @@ public boolean setType(@Nullable MappingModelExpressible<T> type) {
273281
if ( bindType == null || bindType.getJavaType() == Object.class || type instanceof ModelPart ) {
274282
if ( type instanceof BindableType<?> ) {
275283
final boolean changed = bindType != null && type != bindType;
276-
bindType = (BindableType<? super T>) type;
284+
bindType = (BindableType<T>) type;
277285
return changed;
278286
}
279287
else if ( type instanceof BasicValuedMapping basicValuedMapping ) {
280288
final var jdbcMapping = basicValuedMapping.getJdbcMapping();
281289
if ( jdbcMapping instanceof BindableType<?> ) {
282290
final boolean changed = bindType != null && jdbcMapping != bindType;
283-
bindType = (BindableType<? super T>) jdbcMapping;
291+
bindType = (BindableType<T>) jdbcMapping;
284292
return changed;
285293
}
286294
}
287295
}
288296
return false;
289297
}
290298

291-
public TypeConfiguration getTypeConfiguration() {
292-
return sessionFactory.getTypeConfiguration();
299+
private T cast(Object value) {
300+
final var bindableType = getCriteriaBuilder().resolveExpressible( bindType );
301+
return bindableType == null
302+
? (T) value // YOLO
303+
: QueryArguments.cast( value, bindableType.getExpressibleJavaType() );
293304
}
294305

295-
private Object coerceIfNotJpa(T value) {
296-
return sessionFactory.getSessionFactoryOptions().getJpaCompliance().isLoadByIdComplianceEnabled()
297-
? value
298-
: coerce( value );
306+
private void validate(Object value) {
307+
QueryParameterBindingValidator.validate( getBindType(), value, sessionFactory );
299308
}
300309

301-
private Object coerce(T value) {
310+
private Object coerce(Object value) {
302311
try {
303312
if ( canValueBeCoerced( bindType ) ) {
304313
return coerce( value, bindType );
@@ -322,14 +331,10 @@ else if ( canValueBeCoerced( queryParameter.getHibernateType() ) ) {
322331
}
323332
}
324333

325-
private Object coerce(T value, BindableType<? super T> parameterType) {
326-
if ( value == null ) {
327-
return null;
328-
}
329-
else {
330-
return getCriteriaBuilder().resolveExpressible( parameterType )
331-
.getExpressibleJavaType().coerce( value );
332-
}
334+
private Object coerce(Object value, BindableType<T> parameterType) {
335+
return value == null ? null
336+
: getCriteriaBuilder().resolveExpressible( parameterType )
337+
.getExpressibleJavaType().coerce( value );
333338
}
334339

335340
private static boolean canValueBeCoerced(BindableType<?> bindType) {

‎hibernate-core/src/main/java/org/hibernate/query/internal/QueryParameterBindingValidator.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@ public static void validate(BindableType<?> parameterType, Object argument, Sess
2727
if ( parameterJavaType != null ) {
2828
if ( argument instanceof Collection<?> collection
2929
&& !Collection.class.isAssignableFrom( parameterJavaType ) ) {
30-
// we have a collection passed in where we are expecting a non-collection.
31-
// NOTE: this can happen in Hibernate's notion of "parameter list" binding
32-
// NOTE2: the case of a collection value and an expected collection (if that can even happen)
33-
// will fall through to the main check.
30+
// We have a collection passed in where we were expecting a non-collection.
31+
// NOTE: This can happen in Hibernate's notion of "parameter list" binding.
32+
// NOTE2: The case of a collection value and an expected collection
33+
// (if that can even happen) will fall through to the main check.
3434
validateCollectionValuedParameterBinding( parameterType, parameterJavaType, collection, factory );
3535
}
3636
else if ( argument.getClass().isArray() ) {

0 commit comments

Comments
 (0)