Skip to content

Commit

Permalink
Issue OpenLiberty#28366 single result of ElementCollection workaround
Browse files Browse the repository at this point in the history
  • Loading branch information
@njr-11
njr-11 committed Jan 16, 2025
1 parent 9d0d201 commit 749b349
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 61 deletions.
115 changes: 61 additions & 54 deletions ...rty.data.internal.persistence/src/io/openliberty/data/internal/persistence/QueryInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,22 @@ public static enum Type {
final Class<?> returnArrayType;

/**
* The type of a single result obtained by the query.
* For example, a single result of a query that returns List<MyEntity> is of the type MyEntity.
* The type of a single result obtained by the query. For example,
* A query that returns List<MyEntity> has singleType MyEntity.
* A query that returns List<ArrayList<String>> has singleType ArrayList<String>.
* A query that returns Optional<String[]> has singleType String[].
*/
final Class<?> singleType;

/**
* Element type of singleType when singleType is an array or collection.
* Null if singleType is not an array or collection. For example,
* A query that returns List<MyEntity> has singleTypeElementType null.
* A query that returns List<ArrayList<String>> has singleTypeElementType String.
* A query that returns Optional<String[]> has singleTypeElementType String.
*/
final Class<?> singleTypeElementType;

/**
* Positions of Sort, Sort[], and Order parameters.
* When there are no parameters specifying sort criteria dynamically,
Expand Down Expand Up @@ -265,25 +276,6 @@ public static enum Type {
*/
boolean validateResult;

/**
* Constructor for the withJPQL method.
*/
private QueryInfo(Class<?> repositoryInterface,
Method method,
Class<?> entityParamType,
boolean isOptional,
Class<?> multiType,
Class<?> returnArrayType,
Class<?> singleType) {
this.method = method;
this.entityParamType = entityParamType;
this.isOptional = isOptional;
this.multiType = multiType;
this.repositoryInterface = repositoryInterface;
this.returnArrayType = returnArrayType;
this.singleType = singleType;
}

/**
* Construct partially complete query information.
*
Expand Down Expand Up @@ -374,11 +366,18 @@ public QueryInfo(Class<?> repositoryInterface,

singleType = type;

if ((singleType.isArray() || Iterable.class.isAssignableFrom(singleType)) &&
++d < depth)
singleTypeElementType = returnTypeAtDepth.get(d);
else
singleTypeElementType = null;

if (trace && tc.isEntryEnabled())
Tr.exit(this, tc, "<init>", new Object[] { this,
"result isOptional? " + isOptional,
"result multiType: " + multiType,
"result singleType: " + singleType });
"result singleType: " + singleType,
" element: " + singleTypeElementType });
}

/**
Expand All @@ -392,9 +391,43 @@ public QueryInfo(Class<?> repositoryInterface, Method method, Type type) {
this.repositoryInterface = repositoryInterface;
this.returnArrayType = null;
this.singleType = null;
this.singleTypeElementType = null;
this.type = type;
}

/**
* Construct a copy of a source QueryInfo, but with different JPQL and sorts.
*
* @param source QueryInfo from which to copy.
* @param jpql JPQL to use instead of the JPQL from source.
* @param sorts Sorts to use instead of the sorts from source.
*/
QueryInfo(QueryInfo source, String jpql, List<Sort<Object>> sorts) {
entityInfo = source.entityInfo;
entityParamType = source.entityParamType;
entityVar = source.entityVar;
entityVar_ = source.entityVar_;
hasWhere = source.hasWhere;
isOptional = source.isOptional;
this.jpql = jpql;
jpqlAfterCursor = source.jpqlAfterCursor;
jpqlBeforeCursor = source.jpqlBeforeCursor;
jpqlCount = source.jpqlCount;
jpqlDelete = source.jpqlDelete;
jpqlParamCount = source.jpqlParamCount;
jpqlParamNames = source.jpqlParamNames;
maxResults = source.maxResults;
method = source.method;
multiType = source.multiType;
repositoryInterface = source.repositoryInterface;
returnArrayType = source.returnArrayType;
singleType = source.singleType;
singleTypeElementType = source.singleTypeElementType;
this.sorts = sorts;
type = source.type;
validateParams = source.validateParams;
}

/**
* Adds Sort criteria to the end of the tracked list of sort criteria.
*
Expand Down Expand Up @@ -752,6 +785,11 @@ else if (value instanceof CharSequence) {
else if ("false".equalsIgnoreCase(str))
return false;
}
} else if (value instanceof List &&
Iterable.class.isAssignableFrom(toType)) {
return convertToIterable((List<?>) value,
singleTypeElementType,
toType);
}

if (failIfNotConverted) {
Expand Down Expand Up @@ -856,7 +894,7 @@ else if (iterableType.isAssignableFrom(LinkedHashSet.class))
Object[] a = (Object[]) results.get(0);
for (int i = 0; i < a.length; i++) {
Object element = a[i];
if (!elementType.isInstance(element))
if (elementType != null && !elementType.isInstance(element))
element = convert(element, elementType, true);
list.add(element);
}
Expand Down Expand Up @@ -5043,35 +5081,4 @@ void validateSort(Sort<?> sort) {
repositoryInterface.getName());
}
}

/**
* Copy of query information, but with updated JPQL and sort criteria.
*/
QueryInfo withJPQL(String jpql, List<Sort<Object>> sorts) {
QueryInfo q = new QueryInfo( //
repositoryInterface, //
method, //
entityParamType, //
isOptional, //
multiType, //
returnArrayType, //
singleType);
q.entityInfo = entityInfo;
q.entityVar = entityVar;
q.entityVar_ = entityVar_;
q.hasWhere = hasWhere;
q.jpql = jpql;
q.jpqlAfterCursor = jpqlAfterCursor;
q.jpqlBeforeCursor = jpqlBeforeCursor;
q.jpqlCount = jpqlCount;
q.jpqlDelete = jpqlDelete;
q.maxResults = maxResults;
q.jpqlParamCount = jpqlParamCount;
q.jpqlParamNames = jpqlParamNames;
q.sorts = sorts;
q.type = type;
q.validateParams = validateParams;
q.validateParams = validateResult;
return q;
}
}
20 changes: 15 additions & 5 deletions ...ata.internal.persistence/src/io/openliberty/data/internal/persistence/RepositoryImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -700,9 +700,10 @@ else if ("toString".equals(methodName))
if (pageReq == null ||
pageReq.mode() == PageRequest.Mode.OFFSET) {
// offset pagination can be a starting point for cursor pagination
queryInfo = queryInfo.withJPQL(q.append(order).toString(), sortList);
String jpql = q.append(order).toString();
queryInfo = new QueryInfo(queryInfo, jpql, sortList);
} else { // CURSOR_NEXT or CURSOR_PREVIOUS
queryInfo = queryInfo.withJPQL(null, sortList);
queryInfo = new QueryInfo(queryInfo, null, sortList);
queryInfo.generateCursorQueries(q, forward ? order : null, forward ? null : order);
}
}
Expand Down Expand Up @@ -731,7 +732,7 @@ else if ("toString".equals(methodName))
if (TraceComponent.isAnyTracingEnabled() && tc.isDebugEnabled())
Tr.debug(this, tc, "createQuery", queryInfo.jpql, entityInfo.entityClass.getName());

TypedQuery<?> query = em.createQuery(queryInfo.jpql, queryInfo.entityInfo.entityClass);
jakarta.persistence.Query query = em.createQuery(queryInfo.jpql);
queryInfo.setParameters(query, args);

if (queryInfo.type == QueryInfo.Type.FIND_AND_DELETE)
Expand Down Expand Up @@ -847,7 +848,9 @@ else if (DoubleStream.class.equals(multiType))
returnValue = null;
} else if (multiType == null && entityInfo.entityClass.equals(singleType)) {
returnValue = oneResult(queryInfo, results);
} else if (multiType != null && multiType.isInstance(results) && (results.isEmpty() || !(results.get(0) instanceof Object[]))) {
} else if (multiType != null &&
multiType.isInstance(results) &&
(results.isEmpty() || !(results.get(0) instanceof Object[]))) {
returnValue = results;
} else if (multiType != null && Iterable.class.isAssignableFrom(multiType)) {
returnValue = queryInfo.convertToIterable(results,
Expand Down Expand Up @@ -933,7 +936,14 @@ else if (DoubleStream.class.equals(multiType))
} else if (results.isEmpty()) {
throw excEmptyResult(method);
} else { // single result of other type
returnValue = oneResult(queryInfo, results);
if (Iterable.class.isAssignableFrom(singleType) &&
!(results.get(0) instanceof Iterable))
// workaround for EclipseLink wrongly returning
// ElementCollection as separate individual elements
// as shown in #30575
returnValue = results;
else
returnValue = oneResult(queryInfo, results);
if (returnValue != null &&
!singleType.isAssignableFrom(returnValue.getClass()))
returnValue = queryInfo.convert(returnValue,
Expand Down
19 changes: 19 additions & 0 deletions ...internal_fat/test-applications/DataTestApp/src/test/jakarta/data/web/DataTestServlet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4364,6 +4364,25 @@ public void testQueryByMethodNameWithoutBy() {
assertEquals(List.of(), vehicles.findAllOrderByPriceDescVinIdAsc());
}

/**
* Repository Query method that selects and returns a single ArrayList attribute
*/
@Test
public void testQueryReturnsArrayListAttribute() {
assertEquals(new ArrayList<String>(List.of("X", "L", "I")),
primes.romanNumeralSymbolsAsArrayList(41).orElseThrow());
}

/**
* Repository Query method that selects a single attribute of type ArrayList
* and returns it as a Collection.
*/
@Test
public void testQueryReturnsArrayListAttributeAsCollection() {
assertEquals(List.of("X", "X", "X", "V", "I", "I"),
primes.romanNumeralSymbolsAsCollection(37).orElseThrow());
}

/**
* Use a Repository method that has the Query annotation and has a return type
* that uses a Java record indicating to select a subset of entity attributes.
Expand Down
6 changes: 6 additions & 0 deletions ...rty.data.internal_fat/test-applications/DataTestApp/src/test/jakarta/data/web/Primes.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,12 @@ Page<String> romanNumeralsWithin(long min1, long max1,
long min2, long max2,
PageRequest pageRequest);

@Query("SELECT romanNumeralSymbols WHERE numberId = ?1")
Optional<ArrayList<String>> romanNumeralSymbolsAsArrayList(long num);

@Query("SELECT romanNumeralSymbols WHERE numberId = ?1")
Optional<Collection<String>> romanNumeralSymbolsAsCollection(long num);

@Query("SELECT hex WHERE numberId=:id")
Optional<Character> singleHexDigit(long id);

Expand Down
5 changes: 3 additions & 2 deletions ...internal_fat_jpa/test-applications/DataJPATestApp/src/test/jakarta/data/jpa/web/City.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2023 IBM Corporation and others.
* Copyright (c) 2023,2025 IBM Corporation and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
Expand All @@ -23,7 +23,8 @@
@Entity
@IdClass(CityId.class)
public class City {
// TODO uncomment to reproduce EclipseLink bug with selecting an attribute that is a collection type.
// TODO uncomment to reproduce EclipseLink bugs #28589, #29475
// that select an attribute that is a collection type.
//@ElementCollection(fetch = FetchType.EAGER)
public Set<Integer> areaCodes;

Expand Down

0 comments on commit 749b349

Please sign in to comment.