jamesfredley opened a new pull request, #16329: URL: https://github.com/apache/grails-core/pull/16329
> Relocated from [jamesfredley/grails-core#5](https://github.com/jamesfredley/grails-core/pull/5). That follow-up was opened on an archive fork by mistake; Grails PRs belong on `apache/grails-core`. Parent: apache/grails-core#16312. Follow-up to the review on #16312, targeting `fix/dynamic-finder-sort-validation`. ## Blocking items - **Alias sorting regression.** `validateSortProperty` rejected any first path segment that is not a persistent property of the root entity, which is what a criteria / where-query alias is. `WhereQueryWithAssociationSortSpec` failed in `grails-data-hibernate7-core` and `grails-data-hibernate5-core`, and `createCriteria().list(sort: 'c.name') { createAlias('club', 'c') }` failed the same way. A first segment that is not a persistent property is now accepted on the shape check alone and left to the query implementation; segments beneath a known property must still resolve through the mapping. - **`fetch` keys in `HqlListQueryBuilder`.** They are now validated the same way as sort keys before being concatenated into HQL. ## Smaller points - `validateSortProperty` is `private static`; one generic message that does not echo the caller-supplied value; composite identities checked explicitly via `getCompositeIdentity()`. - The duplicated regex is replaced by `NameUtils.isValidPropertyPath`, shared by both modules and based on `Character.isJavaIdentifierStart/Part`, so `$` and non-ASCII identifiers are accepted; ignorable control characters are not. The constant is gone from the `ARGUMENT_*` block. - `resolvePersistentEntity` is reused in the `sortObject == null && orderParam != null` branch. - Blank sort properties are rejected (no more `order by , e.name`), and `normalizeDirection` trims, so `order: ' desc'` keeps working. - Sort-map direction: each entry keeps taking its direction from its value (matching `applySortForMap`), with the same `asc` fallback for a `null` value in both overloads. This is tested and documented rather than reverted. ## Tests - `SortArgumentValidationSpec` (hibernate7-core) runs against real mappings through `list()`, dynamic finders, where queries and criteria: `sort: 'id'` / `'version'`, a subclass sorted by an inherited property, an embedded path, association paths including `club.id`, composite identity members, the `mapping { sort }` default, `createAlias` and where-query aliases, direction normalization, map directions, valid join fetches, and the rejected sort / order / fetch values. - `DynamicFinderCoverageSpec` covers nested association traversal, identity and composite identity, a non-association mid-path, a basic collection with no associated entity, alias-shaped roots, the map-direction change and both `populateArgumentsForCriteria` overloads. - `HqlListQueryBuilderSpec` covers the blank-key and trimmed-direction edges, `$` identifiers, and injected / unknown fetch keys. - `NameUtilsSpec` covers the shared path check. ## Docs `grails-doc` `list.adoc` and the Hibernate 7 / Hibernate 5 `finders.adoc` describe the accepted `sort`, `order` and `fetch` values and the alias pass-through. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
