From f8cd8d2ebe110b21582398a9bed7f7ffda7eb591 Mon Sep 17 00:00:00 2001 From: Bauke Scholtz Date: Tue, 3 Nov 2026 07:42:36 -0400 Subject: [PATCH] Order by a @OneToMany or @ElementCollection field via a subquery Ordering on a field of a @OneToMany was left out of the SQL whenever PostponedFetchRoot was used, and PostponedFetchRoot sorted only the entities of the current page in memory. Ordering on a field of an @ElementCollection joined the collection, which returned an entity once per element, so the same entity could show up on two pages. The entities are now ordered by the smallest value of the field among the elements when ascending, or else by the largest one, via a correlated subquery which also applies the required criteria on the collection. The ID is added as tiebreaker. The subquery is wrapped in COALESCE, as EclipseLink does not accept a subquery as order expression. In a query which is distinct because it joins a collection, the order is skipped with a warning, unless the collection is fetched. Value based paging does not apply to such an ordering. The query is no longer distinct merely because it has optional criteria, only when it joins a collection. Fixes #61 Co-Authored-By: Claude Opus 5.5 --- .../omnifaces/persistence/model/dto/Page.java | 6 +- .../service/BaseEntityService.java | 207 +++++++++++++++--- .../service/PostponedFetchRoot.java | 55 +---- .../persistence/test/OmniPersistenceIT.java | 98 +++++++++ 4 files changed, 290 insertions(+), 76 deletions(-) diff --git a/src/main/java/org/omnifaces/persistence/model/dto/Page.java b/src/main/java/org/omnifaces/persistence/model/dto/Page.java index 7c9db68..f200ab6 100644 --- a/src/main/java/org/omnifaces/persistence/model/dto/Page.java +++ b/src/main/java/org/omnifaces/persistence/model/dto/Page.java @@ -228,7 +228,8 @@ public int getLimit() { * such record. Otherwise it has exactly the keys of {@link #getOrdering()}, in the same order, and map value represents the value of that property in the * record, which may be null. If not empty, then value based paging will be performed instead of offset based paging when applicable. Value * based paging is not applicable when the query is fine-grained with a QueryBuilder of {@link BaseEntityService}, or on Hibernate when the - * query is fine-grained with a MappedQueryBuilder and the ordering contains an aggregated field. + * query is fine-grained with a MappedQueryBuilder and the ordering contains an aggregated field, or when the ordering contains a field of a + * @OneToMany or @ElementCollection. * * @return The ordering values of the record bounding the page, or an empty map if there is none. * @since 1.2 @@ -494,6 +495,9 @@ private void checkRangeNotSet() { /** * Set the ordering. This can be invoked multiple times and will be remembered in same order. The default ordering is {"id",false}. + *

+ * A field of a @OneToMany or @ElementCollection orders the entities by the smallest value among the elements when ascending, + * or else by the largest one. This ordering is skipped when the query is distinct because it joins a collection, unless that collection is fetched. * * @param field The field. May not be null. * @param ascending Whether to sort ascending. diff --git a/src/main/java/org/omnifaces/persistence/service/BaseEntityService.java b/src/main/java/org/omnifaces/persistence/service/BaseEntityService.java index 660cb02..08ca42c 100644 --- a/src/main/java/org/omnifaces/persistence/service/BaseEntityService.java +++ b/src/main/java/org/omnifaces/persistence/service/BaseEntityService.java @@ -200,6 +200,7 @@ public abstract class BaseEntityService & Serializable, private static final String LOG_FINER_QUERY_RESULT = "Query result: %s, estimatedTotalNumberOfResults=%s"; private static final String LOG_FINE_COMPUTED_TYPE_MAPPING = "Computed type mapping for %s: <%s, %s>"; private static final String LOG_WARNING_ILLEGAL_CRITERIA_VALUE = "Cannot parse predicate for %s(%s) = %s(%s), skipping!"; + private static final String LOG_WARNING_UNSUPPORTED_DISTINCT_COLLECTION_ORDER = "Cannot order by %s in a distinct query which does not select it, skipping! The query is distinct because it joins a collection. Consider using DTO mapping or DB view instead."; private static final String LOG_WARNING_UNSUPPORTED_SUBQUERY_PREDICATE = "Cannot create subquery predicate for %s(%s) = %s, skipping! The estimatedTotalNumberOfResults may be slightly inaccurate. Consider using DTO mapping or DB view instead."; private static final String LOG_SEVERE_CONSTRAINT_VIOLATION = "jakarta.validation.ConstraintViolation: @%s %s#%s %s on %s"; private static final String EXCEPTION_ILLEGAL_MAPPING = "You must return a getter-path mapping from MappedQueryBuilder"; @@ -1909,8 +1910,9 @@ private TypedQuery buildEntityQuery(PageBuilder pageBuilder, var entityQueryRoot = buildRoot(entityQuery, pageBuilder.getPage()); pageBuilder.setEntityQueryRoot(entityQueryRoot); var pathResolver = buildSelection(pageBuilder, entityQuery, entityQueryRoot, criteriaBuilder); - buildOrderBy(pageBuilder, entityQuery, criteriaBuilder, pathResolver); - return buildTypedQuery(pageBuilder, entityQuery, entityQueryRoot, buildRestrictions(pageBuilder, entityQuery, criteriaBuilder, pathResolver)); + var parameters = buildRestrictions(pageBuilder, entityQuery, criteriaBuilder, pathResolver); + buildOrderBy(pageBuilder, entityQuery, criteriaBuilder, pathResolver, parameters); + return buildTypedQuery(pageBuilder, entityQuery, entityQueryRoot, parameters); } private TypedQuery buildCountQuery(PageBuilder pageBuilder, CriteriaBuilder criteriaBuilder) { @@ -2035,8 +2037,8 @@ private PathResolver buildSelection(PageBuilder pageBuilder, Ab } else if (pageBuilder.getResultType() == entityType) { pageBuilder.shouldBuildCountSubquery(mapping != null); // mapping is empty but not null when getPage(..., QueryBuilder) is used. - pageBuilder.canBuildValueBasedPagingPredicate(mapping == null); // when mapping is not null, we cannot reliably determine if ordering contains - // aggregated fields, so value based paging cannot be reliably used. + // A collection ordering is a correlated subquery, and a non-null mapping may aggregate, so value based paging cannot be reliably used then. + pageBuilder.canBuildValueBasedPagingPredicate(mapping == null && !containsCollectionOrdering(pageBuilder.getPage())); return new RootPathResolver(root, idAttribute.getName(), elementCollections, manyOrOneToOnes); } else { @@ -2066,7 +2068,7 @@ private void buildRange(PageBuilder pageBuilder, Query query, R // Sorting actions ------------------------------------------------------------------------------------------------ private void buildOrderBy( - PageBuilder pageBuilder, CriteriaQuery criteriaQuery, CriteriaBuilder criteriaBuilder, PathResolver pathResolver + PageBuilder pageBuilder, CriteriaQuery criteriaQuery, CriteriaBuilder criteriaBuilder, PathResolver pathResolver, Map parameters ) { var page = pageBuilder.getPage(); @@ -2078,16 +2080,154 @@ private void buildOrderBy( var reversed = pageBuilder.canBuildValueBasedPagingPredicate() && page.isReversed(); var root = pathResolver.get(null); - var skipOneToManyOrdering = root instanceof PostponedFetchRoot; // PostponedFetchRoot is only created when page.getLimit() < MAX_VALUE or always for - // EclipseLink; non-paged queries don't suffer from join row inflation. - criteriaQuery.orderBy( - stream(ordering) - .filter(order -> !skipOneToManyOrdering || !oneToManys.test(order.getKey())) // @OneToMany ordering is handled in-memory by runPostponedFetches - // in executeQuery; skip it here to avoid adding a join that - // inflates the row count. - .map(order -> buildOrder(order, criteriaBuilder, pathResolver, reversed)) - .collect(toList()) - ); + var orders = new ArrayList(ordering.size() + 1); + var collectionOrdered = false; + var tiebreakerAscending = false; + + for (var order : ordering.entrySet()) { + var collectionPath = pathResolver instanceof MappedPathResolver ? null : findCollectionPath(order.getKey()); + + if (collectionPath == null) { + orders.add(buildOrder(order, criteriaBuilder, pathResolver, reversed)); + continue; + } + + collectionOrdered = true; + tiebreakerAscending = order.getValue(); + + if (!(root instanceof PostponedFetchRoot) && isFetched((From) root, collectionPath)) { + // The fetched elements are selected, also in a distinct query. + orders.add(buildOrder(order, criteriaBuilder, pathResolver, reversed)); + } + else if (criteriaQuery.isDistinct()) { + logger.log(WARNING, () -> format(LOG_WARNING_UNSUPPORTED_DISTINCT_COLLECTION_ORDER, order.getKey())); + } + else { + orders.add(buildCollectionOrder(order, collectionPath, page, criteriaQuery, criteriaBuilder, pathResolver, parameters)); + } + } + + if (collectionOrdered && !ordering.containsKey(ID) && !ordering.containsKey(idAttribute.getName())) { + // The ID keeps the page order stable when collection elements tie. Its direction is that of the last collection order. + var id = pathResolver.get(idAttribute.getName()); + orders.add(tiebreakerAscending ? criteriaBuilder.asc(id) : criteriaBuilder.desc(id)); + } + + criteriaQuery.orderBy(orders); + } + + private boolean containsCollectionOrdering(Page page) { + return page.getOrdering().keySet().stream().anyMatch(field -> findCollectionPath(field) != null); + } + + /** + * Returns the path of the @OneToMany or @ElementCollection field which given field is or is nested in, or null when + * it is in none of them. + */ + private String findCollectionPath(String field) { + var attributes = field.split("\\."); + + for (var i = 1; i <= attributes.length; i++) { + var path = String.join(".", Arrays.copyOf(attributes, i)); + + if (elementCollections.test(path) || i < attributes.length && oneToManys.test(path + "." + attributes[i])) { + return path; + } + } + + return null; + } + + private static boolean isFetched(From root, String collectionPath) { + FetchParent parent = root; + + for (var attribute : collectionPath.split("\\.")) { + var fetch = getFetches(parent).stream().filter(f -> f.getAttribute() != null && f.getAttribute().getName().equals(attribute)).findFirst(); + + if (fetch.isEmpty()) { + return false; + } + + parent = fetch.get(); + } + + return true; + } + + /** + * Builds the order of given ordering on a field of a @OneToMany or @ElementCollection. The root entities are ordered by the + * smallest value of the field among the elements of the collection when ascending, or else by the largest one. Only the elements matching the required + * criteria on the collection are considered, with multiple values and a negation applied per element; optional criteria are not. A plain order on the field + * would JOIN the collection and multiply the rows. Where entities without elements are placed, depends on how the database orders null. + *

+ * The order is a correlated subquery wrapped in COALESCE with a NULL fallback, because EclipseLink does not accept a subquery as + * order expression, and SQL Server does not accept COALESCE with only one argument. + */ + @SuppressWarnings({ "unchecked", "rawtypes" }) + private Order buildCollectionOrder( + Entry order, String collectionPath, Page page, CriteriaQuery criteriaQuery, CriteriaBuilder criteriaBuilder, + PathResolver pathResolver, Map parameters + ) + { + var subquery = criteriaQuery.subquery(Comparable.class); + Root subqueryRoot = subquery.from(entityType); + From collection = subqueryRoot; + + for (var attribute : collectionPath.split("\\.")) { + collection = collection.join(attribute); + } + + var joins = new HashMap>(); + var predicates = new ArrayList(); + predicates.add(criteriaBuilder.equal(subqueryRoot.get(idAttribute.getName()), pathResolver.get(idAttribute.getName()))); + + for (var criteria : page.getRequiredCriteria().entrySet()) { + var field = criteria.getKey(); + + if (field.equals(collectionPath) || field.startsWith(collectionPath + ".")) { + var path = getElementPath(collection, collectionPath, field, joins); + var predicate = buildElementPredicate( + path, path.getJavaType(), criteria.getValue(), criteriaBuilder, new UncheckedParameterBuilder(field, criteriaBuilder, parameters) + ); + + if (predicate != null) { + predicates.add(predicate); + } + } + } + + Expression field = getElementPath(collection, collectionPath, order.getKey(), joins); + subquery.select(order.getValue() ? criteriaBuilder.least(field) : criteriaBuilder.greatest(field)).where(toArray(predicates)); + var expression = criteriaBuilder.function("COALESCE", field.getJavaType(), subquery, criteriaBuilder.nullLiteral(field.getJavaType())); + return order.getValue() ? criteriaBuilder.asc(expression) : criteriaBuilder.desc(expression); + } + + /** + * Returns the path of given field of an element of given collection. JOINs are shared via given map, so that the criteria and the order on the elements + * apply to the same elements. + */ + private Expression getElementPath(From collection, String collectionPath, String field, Map> joins) { + if (field.equals(collectionPath)) { + return collection; + } + + var attributes = field.substring(collectionPath.length() + 1).split("\\."); + var last = attributes.length - 1; + var path = collectionPath; + From from = collection; + + for (var i = 0; i <= last; i++) { + if (i == last && !elementCollections.test(field)) { + return from.get(attributes[i]); + } + + var parent = from; + var attribute = attributes[i]; + path += "." + attribute; + from = joins.computeIfAbsent(path, k -> parent.join(attribute)); + } + + return from; } private static Order buildOrder(Entry order, CriteriaBuilder criteriaBuilder, PathResolver pathResolver, boolean reversed) { @@ -2147,10 +2287,7 @@ criteriaBuilder, restriction, buildValueBasedPagingPredicate(page, criteriaBuild if (restriction != null) { var distinct = !(pathResolver instanceof MappedPathResolver) // DTO queries have GROUP BY from aggregations; DISTINCT is redundant and causes // OpenJPA to wrap the query losing projected columns. - && (!optionalPredicates.isEmpty() // Optional (OR/global) predicates may span @ElementCollection fields; buildPredicate adds a root JOIN per - // such field unless isBuiltAsArrayPredicate, multiplying rows; DISTINCT deduplicates entities before LIMIT - // is applied. - || hasCollectionJoins((From) pathResolver.get(null))); // Collection JOINs multiply rows; DISTINCT keeps LIMIT on entity rows. + && hasCollectionJoins((From) pathResolver.get(null)); // Collection JOINs multiply rows; DISTINCT keeps LIMIT on entity rows. query.distinct(distinct).where(conjunctRestrictionsIfNecessary(criteriaBuilder, query.getRestriction(), restriction)); } @@ -2238,7 +2375,7 @@ else if (elementCollections.test(field)) { } else { var items = unwrapNot(criteria); - return oneToManys.test(field) && (isOneToManySubqueried(query, isNegated(criteria)) || items instanceof Iterable || items.getClass().isArray()); + return oneToManys.test(field) && (isOneToManySubqueried(query, isNegated(criteria)) || isMultiValued(items)); } } @@ -2261,7 +2398,7 @@ else if (elementCollections.test(field)) { predicate = buildElementCollectionPredicate(alias, path, type, field, value, negated, query, criteriaBuilder, pathResolver, parameterBuilder); negationApplied = true; } - else if (value instanceof Iterable || value.getClass().isArray() || oneToManys.test(field) && isOneToManySubqueried(query, negated)) { + else if (isMultiValued(value) || oneToManys.test(field) && isOneToManySubqueried(query, negated)) { predicate = buildArrayPredicate(path, type, field, value, negated, query, criteriaBuilder, pathResolver, parameterBuilder); negationApplied = true; } @@ -2501,18 +2638,31 @@ private Predicate buildCollectionSubqueryPredicate( } /** - * Builds the predicate of given item on given path of an element of a @OneToMany or @ElementCollection in a correlated subquery. + * Builds the predicate of given criteria on given path of an element of a @OneToMany or @ElementCollection in a correlated + * subquery. Multiple values are ORed. */ private Predicate buildElementPredicate( - Expression path, Class type, Object item, CriteriaBuilder criteriaBuilder, ParameterBuilder parameterBuilder + Expression path, Class type, Object criteria, CriteriaBuilder criteriaBuilder, ParameterBuilder parameterBuilder ) { - var value = unwrapNot(item); + var value = unwrapNot(criteria); Predicate predicate; if (value == null) { predicate = criteriaBuilder.isNull(path); } + else if (isMultiValued(value)) { + var predicates = stream(value) + .map(item -> buildElementPredicate(path, type, item, criteriaBuilder, parameterBuilder)) + .filter(Objects::nonNull) + .toList(); + + if (predicates.isEmpty()) { + return null; + } + + predicate = criteriaBuilder.or(toArray(predicates)); + } else if (isEntity(value)) { predicate = criteriaBuilder.equal(path, parameterBuilder.create(value)); } @@ -2520,7 +2670,14 @@ else if (isEntity(value)) { predicate = createElementCollectionCriteria(type, value).build(path, criteriaBuilder, parameterBuilder); } - return predicate != null && isNegated(item) ? criteriaBuilder.not(predicate) : predicate; + return predicate != null && isNegated(criteria) ? criteriaBuilder.not(predicate) : predicate; + } + + /** + * Returns whether given value holds multiple values, which Streams#stream(Object) of OmniUtils streams one by one. + */ + private static boolean isMultiValued(Object value) { + return value instanceof Iterable || value instanceof Object[] || value instanceof int[] || value instanceof long[] || value instanceof double[]; } private static Criteria createElementCollectionCriteria(Class type, Object value) { diff --git a/src/main/java/org/omnifaces/persistence/service/PostponedFetchRoot.java b/src/main/java/org/omnifaces/persistence/service/PostponedFetchRoot.java index d2f49bc..5b54769 100644 --- a/src/main/java/org/omnifaces/persistence/service/PostponedFetchRoot.java +++ b/src/main/java/org/omnifaces/persistence/service/PostponedFetchRoot.java @@ -18,7 +18,6 @@ import static java.util.Comparator.nullsLast; import static java.util.Objects.requireNonNullElse; import static java.util.stream.Collectors.toCollection; -import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toMap; import static org.omnifaces.utils.reflect.Reflections.invokeGetter; import static org.omnifaces.utils.reflect.Reflections.invokeSetter; @@ -174,7 +173,7 @@ public List runPostponedFetches( initializeNestedFetches(distinctEntities, fetchedEntities); var fetchedEntityProperties = new ArrayList>(); - distinctEntities = buildAndSortPostponedFetches(page, distinctEntities, fetchPaths, fetchedEntities, fetchedEntityProperties); + buildPostponedFetches(page, distinctEntities, fetchPaths, fetchedEntities, fetchedEntityProperties); entityManager.clear(); // Clear before applying the copies, or the provider flushes them. applyPostponedFetches(distinctEntities, fetchPaths, fetchedEntityProperties); // Set filtered+sorted copies onto the now-detached entities. return distinctEntities; @@ -223,28 +222,14 @@ else if (object != null && index < properties.length) { } /** - * Processes "postponed" fetches by copying, filtering, and sorting child collections/entities. Finally, re-sorts the parent entities based on the first - * element of their sorted child lists. + * Processes "postponed" fetches by copying, filtering, and sorting child collections/entities. The order of the entities is left as the paged query + * returned it. */ - private static List buildAndSortPostponedFetches( + private static void buildPostponedFetches( Page page, List entities, List fetchPaths, Map> fetchedEntities, List> fetchedEntityProperties ) { - var rows = entities.stream().map(entity -> processEntityFetches(entity, fetchPaths, fetchedEntities, page)).collect(toList()); - Comparator> rowComparator = buildRowComparator(page, fetchPaths); - - if (rowComparator != null) { - rows.sort(rowComparator); - } - - var sortedEntities = new ArrayList(entities.size()); - - for (var row : rows) { - sortedEntities.add(row.entity); - fetchedEntityProperties.add(row.fetchedEntityProperties); - } - - return sortedEntities; + entities.forEach(entity -> fetchedEntityProperties.add(processEntityFetches(entity, fetchPaths, fetchedEntities, page).fetchedEntityProperties())); } private record EntityRow(T entity, List fetchedEntityProperties) { @@ -275,36 +260,6 @@ private static EntityRow processEntityFetches( return new EntityRow<>(entity, copies); } - private static Comparator> buildRowComparator(Page page, List paths) { - Comparator> rowComparator = null; - - for (int i = 0; i < paths.size(); i++) { - var itemComp = buildFetchItemComparator(paths.get(i), page.getOrdering()); - - if (itemComp == null) { - continue; - } - - final int fetchIndex = i; - Comparator> propertyComparator = (rowA, rowB) -> { - var propertyA = rowA.fetchedEntityProperties.get(fetchIndex); - var propertyB = rowB.fetchedEntityProperties.get(fetchIndex); - - if (propertyA instanceof List listA && propertyB instanceof List listB) { - var firstA = listA.isEmpty() ? null : listA.get(0); - var firstB = listB.isEmpty() ? null : listB.get(0); - return itemComp.compare(firstA, firstB); - } - - return 0; - }; - - rowComparator = (rowComparator == null) ? propertyComparator : rowComparator.thenComparing(propertyComparator); - } - - return rowComparator; - } - @SuppressWarnings("unchecked") private static Comparator buildFetchItemComparator(String fetchPath, Map ordering) { return ordering.entrySet().stream() diff --git a/src/test/java/org/omnifaces/persistence/test/OmniPersistenceIT.java b/src/test/java/org/omnifaces/persistence/test/OmniPersistenceIT.java index f68ffa5..4de8713 100644 --- a/src/test/java/org/omnifaces/persistence/test/OmniPersistenceIT.java +++ b/src/test/java/org/omnifaces/persistence/test/OmniPersistenceIT.java @@ -38,6 +38,7 @@ import java.util.Objects; import java.util.Set; import java.util.function.BiFunction; +import java.util.function.Function; import java.util.stream.Stream; import jakarta.inject.Inject; @@ -738,6 +739,103 @@ void testPageWithGroupsSortedAscending() { result.forEach(p -> assertFalse(p.getGroups().isEmpty(), "Groups collection is populated")); } + @Test + void testPageOrderedByOneToManyAcrossPages() { + var all = personService().getAllWithPhones(); + + for (var ascending : List.of(true, false)) { + Function key = person -> extreme(person.getPhones().stream().map(Phone::getNumber), ascending); + BiFunction page = (offset, limit) -> Page.with().range(offset, limit).orderBy("phones.number", ascending).build(); + assertOrderedAcrossPages(all, key, ascending, (offset, limit) -> personService().getPage(page.apply(offset, limit), false)); + assertOrderedAcrossPages(all, key, ascending, (offset, limit) -> personService().getPageWithPhones(page.apply(offset, limit), false)); + } + } + + @Test + void testPageOrderedByOneToManyWithCriteriaOnIt() { + var all = personService().getAllWithPhones(); + Function key = person -> extreme(person.getPhones().stream().map(Phone::getNumber).filter(number -> number.contains("11")), true); + BiFunction page = (offset, limit) -> Page.with() + .range(offset, limit).allMatch(Map.of("phones.number", Like.contains("11"))).orderBy("phones.number", true).build(); + assertOrderedAcrossPages(all, key, true, (offset, limit) -> personService().getPage(page.apply(offset, limit), false)); + assertOrderedAcrossPages(all, key, true, (offset, limit) -> personService().getPageWithPhones(page.apply(offset, limit), false)); + } + + @Test + void testPageOrderedByElementCollectionNestedInOneToMany() { + var all = personService().getAllWithGroups(); + Function key = person -> extreme(person.getGroups().stream().map(Group::name), true); // Every phone is owned by the person itself. + BiFunction page = (offset, limit) -> Page.with().range(offset, limit).orderBy("phones.owner.groups", true).build(); + assertOrderedAcrossPages(all, key, true, (offset, limit) -> personService().getPage(page.apply(offset, limit), false)); + } + + @Test + void testPageOrderedByOneToManyWithListAndNegatedCriteriaOnIt() { + var all = personService().getAllWithPhones(); + Function key = person -> extreme(person.getPhones().stream().map(Phone::getNumber), true); + + BiFunction listPage = (offset, limit) -> Page.with() + .range(offset, limit).allMatch(Map.of("phones.number", List.of(Like.startsWith("0")))).orderBy("phones.number", true).build(); + assertOrderedAcrossPages(all, key, true, (offset, limit) -> personService().getPage(listPage.apply(offset, limit), false)); + + var withoutOne = all.stream().filter(person -> person.getPhones().stream().noneMatch(phone -> phone.getNumber().contains("1"))).toList(); + BiFunction negatedPage = (offset, limit) -> Page.with() + .range(offset, limit).allMatch(Map.of("phones.number", Not.value(Like.contains("1")))).orderBy("phones.number", true).build(); + assertOrderedAcrossPages(withoutOne, key, true, (offset, limit) -> personService().getPage(negatedPage.apply(offset, limit), false)); + } + + @Test + void testPageOrderedByOneToManyWithOptionalCriteria() { + var matching = personService().getAllWithPhones().stream() + .filter(person -> person.getEmail().contains("1") || person.getPhones().stream().anyMatch(phone -> phone.getNumber().contains("11"))) + .toList(); + Function key = person -> extreme(person.getPhones().stream().map(Phone::getNumber), true); + var criteria = Map.of("phones.number", Like.contains("11"), "email", Like.contains("1")); + BiFunction page = (offset, limit) -> Page.with().range(offset, limit).anyMatch(criteria).orderBy("phones.number", true).build(); + assertOrderedAcrossPages(matching, key, true, (offset, limit) -> personService().getPage(page.apply(offset, limit), false)); + } + + @Test + void testPageOrderOnElementCollectionWithOptionalCriteriaIsStable() { + var criteria = Map.of("groups", Group.USER, "email", Like.contains("1")); + var ids = Stream.of(0, 10) + .flatMap(offset -> personService().getPage(Page.with().range(offset, 10).anyMatch(criteria).orderBy("groups", true).build(), false).stream()) + .map(Person::getId).toList(); + assertEquals(20, ids.size(), "Two full pages"); + assertEquals(ids.size(), ids.stream().distinct().count(), "Each entity is on only one page"); + } + + @Test + void testPageOrderedByElementCollectionAcrossPages() { + var all = personService().getAllWithGroups(); + + for (var ascending : List.of(true, false)) { + Function key = person -> extreme(person.getGroups().stream().map(Group::name), ascending); // @Enumerated(STRING) + BiFunction page = (offset, limit) -> Page.with().range(offset, limit).orderBy("groups", ascending).build(); + assertOrderedAcrossPages(all, key, ascending, (offset, limit) -> personService().getPage(page.apply(offset, limit), false)); + assertOrderedAcrossPages(all, key, ascending, (offset, limit) -> personService().getPageWithGroups(page.apply(offset, limit), false)); + } + } + + private static > K extreme(Stream values, boolean smallest) { + return values.reduce((left, right) -> (left.compareTo(right) <= 0) == smallest ? left : right).orElse(null); + } + + private static > void assertOrderedAcrossPages( + List all, Function key, boolean ascending, BiFunction> pageLoader + ) + { + var keys = new HashMap(); + all.forEach(person -> keys.put(person.getId(), key.apply(person))); + var ids = Stream.concat(pageLoader.apply(0, 10).stream(), pageLoader.apply(10, 10).stream()).map(Person::getId).toList(); + assertEquals(ids.size(), ids.stream().distinct().count(), "Each entity is on only one page"); + + Comparator order = ascending ? Comparator.naturalOrder() : Comparator.reverseOrder(); + var actualKeys = ids.stream().map(keys::get).toList(); + var expectedKeys = keys.values().stream().filter(Objects::nonNull).sorted(order).limit(ids.size()).toList(); + assertEquals(expectedKeys, actualKeys, "Entities are ordered across pages by the " + (ascending ? "smallest" : "largest") + " element"); + } + @Test void testPageWithPhonesSortedByNumberAscending() { var result = personService().getPageWithPhones(Page.with().range(0, 10).orderBy("phones.number", true).build(), false);