Skip to content

Commit

Permalink
Issue OpenLiberty#28366 workaround to reject multiple results of Elem…
Browse files Browse the repository at this point in the history 
…entCollection attribute, and otherwise improve conversions
  • Loading branch information
@njr-11
njr-11 committed Jan 17, 2025
1 parent 845b04c commit 9d282b6
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 11 deletions.
22 changes: 22 additions & 0 deletions ...tence/resources/io/openliberty/data/internal/persistence/resources/CWWKDMessages.nlsprops
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1016,3 +1016,25 @@ CWWKD1101.attr.subset.mismatch.explanation=When Java records are used to \
CWWKD1101.attr.subset.mismatch.useraction=Rename the record components to \
match valid entity attribute names, or use the Select annotation to explicitly \
request entity attributes by their name.

CWWKD1102.incompat.query.result=CWWKD1102E: The {0} method of the {1} repository \
has the {2} return type, but the Jakarta Persistence provider performed a \
query with results of the {3} type. Check if the Jakarta Persistence provider \
allows querying for the types of entity attributes that are selected by the \
{0} method.
CWWKD1102.incompat.query.result.explanation=Jakarta Persistence providers might \
not allow returning all types of entity attributes.
CWWKD1102.incompat.query.result.useraction=Modify the query to retrieve the whole \
entity or a different set of entity attributes that are allowed by the Jakarta \
Persistence provider.

CWWKD1103.incompat.query.result=CWWKD1103E: The {0} method of the {1} repository \
has the {2} return type, but the Jakarta Persistence provider performed a \
{3} query with results of the {4} type. Check if the Jakarta Persistence provider \
allows querying for the types of entity attributes that are selected by the \
{0} method.
CWWKD1103.incompat.query.result.explanation=Jakarta Persistence providers might \
not allow returning all types of entity attributes.
CWWKD1103.incompat.query.result.useraction=Modify the query to retrieve the whole \
entity or a different set of entity attributes that are allowed by the Jakarta \
Persistence provider.
77 changes: 68 additions & 9 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 @@ -611,7 +611,7 @@ int computeOffset(PageRequest pagination) {
* return type.
*
* @param value value to convert.
* @param type type to convert to.
* @param toType type to convert to.
* @param failIfNotConverted whether or not to fail if unable to convert.
* @return converted value.
*/
Expand Down Expand Up @@ -788,8 +788,9 @@ else if ("false".equalsIgnoreCase(str))
} else if (value instanceof List &&
Iterable.class.isAssignableFrom(toType)) {
return convertToIterable((List<?>) value,
toType,
singleTypeElementType,
toType);
null);
}

if (failIfNotConverted) {
Expand Down Expand Up @@ -834,15 +835,26 @@ private void convertFail(Number value, long min, long max) {
* Convert the results list into an Iterable of the specified type.
*
* @param results results of a find or save operation.
* @param elementType the type of each element if a find operation.
* Can be NULL if a save operation.
* @param iterableType the desired type of Iterable.
* @param elementType the type of each element, or null.
* Always null if not a find operation.
* @param query the query if available.
* Always null if not a find operation.
* @return results converted to an Iterable of the specified type.
*/
@Trivial
final Iterable<?> convertToIterable(List<?> results,
Class<?> iterableType,
Class<?> elementType,
Class<?> iterableType) {
jakarta.persistence.Query query) {
final boolean trace = TraceComponent.isAnyTracingEnabled();
if (trace && tc.isEntryEnabled())
Tr.entry(this, tc, "convertToIterable",
loggable(results),
"to " + iterableType.getName(),
elementType == null ? "of ?" : ("of " + elementType.getName()),
query);

Collection<Object> list;
if (iterableType.isInterface()) {
if (iterableType.isAssignableFrom(ArrayList.class))
Expand Down Expand Up @@ -899,8 +911,21 @@ else if (iterableType.isAssignableFrom(LinkedHashSet.class))
list.add(element);
}
} else {
list.addAll(results);
for (Object element : results) {
if (elementType != null && !elementType.isInstance(element)) {
Object converted = convert(element, elementType, false);
// EclipseLink returns wrong values when selecting
// ElementCollection attributes instead of rejecting it as
// unsupported. Raise an error instead.
if (converted == element)
throw excIncompatibleQueryResult(results, query);
}
list.add(element);
}
}

if (trace && tc.isEntryEnabled())
Tr.exit(this, tc, "convertToIterable", loggable(list));
return list;
}

Expand Down Expand Up @@ -1083,6 +1108,40 @@ private MappingException excExtraMethodArgNamedParams(Set<String> extras,
method.getAnnotation(Query.class).value());
}

/**
* Constructs an UnsupportedOperationException for an error where the
* repository method return type does not match the query results.
* On reason this might happen is when EclipseLink returns wrong values
* when selecting ElementCollection attributes instead of rejecting
* it as unsupported.
*
* @param results list of at least 1 result.
* @param query jakarta.persistence.Query, a String, or null.
* @return UnsupportedOperationException.
*/
@Trivial
UnsupportedOperationException excIncompatibleQueryResult(List<?> results,
Object query) {
String r = results.getClass().getName() +
"<" + results.get(0).getClass().getName() + ">";

if (query == null)
return exc(UnsupportedOperationException.class,
"CWWKD1102.incompat.query.result",
method.getName(),
repositoryInterface.getName(),
method.getGenericReturnType().getTypeName(),
r);
else
return exc(UnsupportedOperationException.class,
"CWWKD1103.incompat.query.result",
method.getName(),
repositoryInterface.getName(),
method.getGenericReturnType().getTypeName(),
query instanceof String ? query : query.getClass().getName(),
r);
}

/**
* Check if the cause of the lacking named parameter is a mispositioned
* special parameter. If so, raises UnsupportedOperationException.
Expand Down Expand Up @@ -1313,7 +1372,7 @@ else if (multiType.isInstance(results))
else if (Stream.class.equals(multiType))
returnValue = results.stream();
else if (Iterable.class.isAssignableFrom(multiType))
returnValue = convertToIterable(results, null, multiType);
returnValue = convertToIterable(results, multiType, null, null);
else if (Iterator.class.equals(multiType))
returnValue = results.iterator();
else
Expand Down Expand Up @@ -3801,7 +3860,7 @@ else if (multiType.isInstance(results))
else if (Stream.class.equals(multiType))
returnValue = results.stream();
else if (Iterable.class.isAssignableFrom(multiType))
returnValue = convertToIterable(results, null, multiType);
returnValue = convertToIterable(results, multiType, null, null);
else if (Iterator.class.equals(multiType))
returnValue = results.iterator();
else
Expand Down Expand Up @@ -4301,7 +4360,7 @@ else if (multiType.isInstance(results))
else if (Stream.class.equals(multiType))
returnValue = results.stream();
else if (Iterable.class.isAssignableFrom(multiType))
returnValue = convertToIterable(results, null, multiType);
returnValue = convertToIterable(results, multiType, null, null);
else if (Iterator.class.equals(multiType))
returnValue = results.iterator();
else
Expand Down
6 changes: 4 additions & 2 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 @@ -850,12 +850,14 @@ else if (DoubleStream.class.equals(multiType))
returnValue = oneResult(queryInfo, results);
} else if (multiType != null &&
multiType.isInstance(results) &&
(results.isEmpty() || !(results.get(0) instanceof Object[]))) {
(results.isEmpty() || singleType.isInstance(results.get(0)) &&
!(results.get(0) instanceof Object[]))) {
returnValue = results;
} else if (multiType != null && Iterable.class.isAssignableFrom(multiType)) {
returnValue = queryInfo.convertToIterable(results,
multiType,
singleType,
multiType);
query);
} else if (Iterator.class.equals(multiType)) {
returnValue = results.iterator();
} else if (queryInfo.returnArrayType != null) {
Expand Down

0 comments on commit 9d282b6

Please sign in to comment.