Skip to content

Commit

Permalink
Guard potential NPE in TupleConverter.
Browse files Browse the repository at this point in the history
Also fix a broken reference in javadoc.

Original Pull Request: #3654
  • Loading branch information
christophstrobl committed Dec 17, 2024
1 parent 5d41ada commit 7eb313e
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import jakarta.persistence.TupleElement;
import jakarta.persistence.TypedQuery;

import java.lang.reflect.Constructor;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
Expand Down Expand Up @@ -54,6 +55,7 @@
import org.springframework.jdbc.support.JdbcUtils;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
import org.springframework.util.ReflectionUtils;

/**
Expand Down Expand Up @@ -345,7 +347,7 @@ public TupleConverter(ReturnedType type, boolean nativeQuery) {
&& !type.getInputProperties().isEmpty();

if (this.dtoProjection) {
this.preferredConstructor = PreferredConstructorDiscoverer.discover(String.class);
this.preferredConstructor = PreferredConstructorDiscoverer.discover(type.getReturnedType());
} else {
this.preferredConstructor = null;
}
Expand Down Expand Up @@ -373,18 +375,35 @@ public Object convert(Object source) {

Object[] ctorArgs = new Object[type.getInputProperties().size()];

boolean containsNullValue = false;
for (int i = 0; i < type.getInputProperties().size(); i++) {
ctorArgs[i] = tuple.get(i);
Object value = tuple.get(i);
ctorArgs[i] = value;
if (!containsNullValue && value == null) {
containsNullValue = true;
}
}

try {

if (preferredConstructor.getParameterCount() == ctorArgs.length) {
if (preferredConstructor != null && preferredConstructor.getParameterCount() == ctorArgs.length) {
return BeanUtils.instantiateClass(preferredConstructor.getConstructor(), ctorArgs);
}

return BeanUtils.instantiateClass(type.getReturnedType()
.getConstructor(Arrays.stream(ctorArgs).map(Object::getClass).toArray(Class<?>[]::new)), ctorArgs);
Constructor<?> ctor = null;

if (!containsNullValue) { // let's seem if we have an argument type match
ctor = type.getReturnedType()
.getConstructor(Arrays.stream(ctorArgs).map(Object::getClass).toArray(Class<?>[]::new));
}

if (ctor == null) { // let's see if there's more general constructor we could use that accepts our args
ctor = findFirstMatchingConstructor(ctorArgs);
}

if (ctor != null) {
return BeanUtils.instantiateClass(ctor, ctorArgs);
}
} catch (ReflectiveOperationException e) {
ReflectionUtils.handleReflectionException(e);
}
Expand All @@ -393,6 +412,30 @@ public Object convert(Object source) {
return new TupleBackedMap(tupleWrapper.apply(tuple));
}

@Nullable
private Constructor<?> findFirstMatchingConstructor(Object[] ctorArgs) {

for (Constructor<?> ctor : type.getReturnedType().getDeclaredConstructors()) {

if (ctor.getParameterCount() == ctorArgs.length) {
boolean itsAMatch = true;
for (int i = 0; i < ctor.getParameterCount(); i++) {
if (ctorArgs[i] == null) {
continue;
}
if (!ClassUtils.isAssignable(ctor.getParameterTypes()[i], ctorArgs[i].getClass())) {
itsAMatch = false;
break;
}
}
if (itsAMatch) {
return ctor;
}
}
}
return null;
}

/**
* A {@link Map} implementation which delegates all calls to a {@link Tuple}. Depending on the provided
* {@link Tuple} implementation it might return the same value for various keys of which only one will appear in the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public interface QueryEnhancer {
* @param sort the sort specification to apply.
* @param alias the alias to be used in the order by clause. May be {@literal null} or empty.
* @return the modified query string.
* @deprecated since 3.5, use {@link #rewrite(Sort, ReturnedType)} instead.
* @deprecated since 3.5, use {@link #rewrite(QueryRewriteInformation)} instead.
*/
@Deprecated(since = "3.5", forRemoval = true)
String applySorting(Sort sort, @Nullable String alias);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@

import org.springframework.data.jpa.repository.query.AbstractJpaQuery.TupleConverter;
import org.springframework.data.projection.ProjectionFactory;
import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
import org.springframework.data.repository.CrudRepository;
import org.springframework.data.repository.core.RepositoryMetadata;
import org.springframework.data.repository.core.support.DefaultRepositoryMetadata;
Expand Down Expand Up @@ -113,6 +114,16 @@ void findsValuesForAllVariantsSupportedByTheTuple() {
softly.assertAll();
}

@Test // GH-3076
void dealsWithNullsInArgumetents() {

ReturnedType returnedType = ReturnedType.of(WithPC.class, DomainType.class, new SpelAwareProxyProjectionFactory());

when(tuple.get(eq(0))).thenReturn("one");
when(tuple.get(eq(1))).thenReturn(null);
new TupleConverter(returnedType).convert(tuple);
}

interface SampleRepository extends CrudRepository<Object, Long> {
String someMethod();
}
Expand Down Expand Up @@ -177,4 +188,18 @@ public String getAlias() {
}
}
}

static class DomainType {
String one, two, three;
}

static class WithPC {
String one;
String two;

public WithPC(String one, String two) {
this.one = one;
this.two = two;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ This query gets rewritten to `SELECT new UserDto(u.firstname, u.lastname) FROM U

[WARNING]
====
JPQL constructor expressions must not contain aliases for selected columns.
JPQL constructor expressions must not contain aliases for selected columns and query rewriting will not remove them for you.
While `SELECT u as user, count(u.roles) as roleCount FROM USER u …` is a valid query for interface-based projections that rely on column names from the returned `Tuple`, the same construct is invalid when requesting a DTO where it needs to be `SELECT u, count(u.roles) FROM USER u …`. +
Some persistence providers may be lenient about this, others not.
====
Expand Down

0 comments on commit 7eb313e

Please sign in to comment.