You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "jdaugherty (via GitHub)" <gi...@apache.org> on 2026/10/02 13:32:15 UTC
[PR] Backport #16154 to 8.0.x: replace finders inheritance hierarchy with composition [grails-core]
jdaugherty opened a new pull request, #16478:
URL: https://github.com/apache/grails-core/pull/16478
## Summary
Backports #16154 (merged to `8.1.x` as `ced76b400d`) to `8.0.x`. Fixes #15789 on `8.0.x`.
The dynamic finders in `org.grails.datastore.gorm.finders` (grails-datamapping-core) no longer form the 4-level `AbstractFinder -> DynamicFinder -> AbstractFindByFinder -> concrete finder` hierarchy. `SingleResultFinder`, `ListResultFinder` and `CountFinder` are flat classes, created through static factories, that compose a `DynamicFinder`. `DynamicFinder` is now only the method-name grammar and criteria builder.
- `findOrCreateBy*` and `findOrSaveBy*` share one implementation of building an instance from the `Equal` expressions (`SingleResultFinder#constructFromEqualExpressions`). The old `FindOrSaveByFinder` copy of that branch could never run, because `FindOrCreateByFinder` had already built and conditionally saved the instance
- `countBy*Or*` resolves association arguments to their identifiers, as `countBy*And*` does: the OR path now goes through `Query.add(Junction, Criterion)`. This fixes the MongoDB under-count tracked in #15789, and the `@PendingFeature` on `DisjunctionQuerySpec` is removed
- `AbstractDetachedCriteria#dynamicFinders` is typed `List<FinderMethod>`, matching what the list holds
- `findAllBy*` behaves as it does on `8.0.x` today. It applies no `distinct()` projection, which GORM for Neo4j does not support
- Every other existing quirk is kept and re-asserted in tests rather than silently fixed, including the `And`/`Or` literal-split collision and the empty-property-name crash on an operator collision
## Public API changes
This removes public classes from `org.grails.datastore.gorm.finders`: `AbstractFinder`, `AbstractFindByFinder`, `FindByFinder`, `FindByBooleanFinder`, `FindAllByFinder`, `FindAllByBooleanFinder`, `FindOrCreateByFinder`, `FindOrSaveByFinder` and `CountByFinder`. `DynamicFinder` is now a concrete grammar class: it no longer implements `FinderMethod`, has a different constructor, and has no `invoke`/`doInvokeInternal`.
Applications are unaffected. A third-party GORM datastore or plugin that extends or instantiates any of the removed classes must switch to the `SingleResultFinder`/`ListResultFinder`/`CountFinder` factory methods. They accept a `Datastore`, a `DatastoreResolver` plus `MappingContext` (the form `DefaultGormApiFactory` registers), or a bare `MappingContext`. This is documented in the Grails 8 upgrade guide, `upgrading80x.adoc` section 82.
## How this differs from #16154
The core sources and specs on `8.0.x` match the `8.1.x` base #16154 was merged onto, and so do all 25 files outside the module that use the finders package. So the net #16154 diff applies to `grails-datamapping-core` and the Mongo spec unchanged, apart from these differences:
- **No RxGORM changes.** `grails-datamapping-rx` is commented out in `settings.gradle` on `8.0.x` (it was re-enabled on `8.1.x` only), so the `org.grails.gorm.rx.finders` half of #16154 is left out. The upgrade guide section omits the sentence about the RxGORM classes
- **Upgrade guide section number.** #16154 added the section as "21" after section 80 on `8.1.x`. Here it follows section 81 as 82
- **License headers.** #16154 replaced the ASF header of `ListOrderByFinder` with a legacy "the original author or authors" header, and gave the new `FinderSupport` the same legacy header. Both carry the ASF header here. `DynamicFinder` already had the legacy header on `8.0.x` and is left as it is
The merge-up into `8.1.x` should take the `8.0.x` side of `upgrading80x.adoc` and `FinderSupport.java`. `ListOrderByFinder.java` will not conflict, because only `8.1.x` changed its header, so `8.1.x` keeps the legacy header until it is fixed there.
## Test plan
- [x] `:grails-datamapping-core:test --tests 'org.grails.datastore.gorm.finders.*'` - 286 tests, 0 failures
- [x] `:grails-datamapping-core-test:test` with the dynamic finder, `ListOrderBy`, `DetachedCriteria` and TCK `Find*`/`ListOrderBySpec` specs - 65 tests, 0 failures, 1 skipped
- [x] `:grails-data-neo4j-core:test` TCK `FindByMethodSpec`, `RLikeSpec`, `ListOrderBySpec` - 27 tests, 0 failures
- [x] `:grails-data-hibernate5-core:test` and `:grails-data-hibernate7-core:test` `FindByMethodSpec` and `RLikeSpec` - 26 tests each, 0 failures. The only failure each reported is the suite class's `NoTestsDiscoveredException`, which the `--tests` filter causes
- [x] `:grails-data-mongodb-core:test --tests '*DisjunctionQuerySpec'` - 3 tests, 0 failures, including the formerly pending OR count
- [x] `:grails-datamapping-core:codeStyle`, `:grails-data-mongodb-core:codeStyle`, `validateRepositoryConventions`, `rat` - clean
- [ ] Full module suites and the Mongo/Neo4j functional jobs were not run locally and are left to CI. They were green for #16154 on `8.1.x`
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Backport #16154 to 8.0.x: replace finders inheritance hierarchy with composition [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty merged PR #16478:
URL: https://github.com/apache/grails-core/pull/16478
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Backport #16154 to 8.0.x: replace finders inheritance hierarchy with composition [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on PR #16478:
URL: https://github.com/apache/grails-core/pull/16478#issuecomment-5955850243
Thanks @matrei. Finding 1 is fixed in c8d4040e4d, which landed on `8.0.x` with this PR in 6b8d71c7c7.
`CountFinder.applyCriteriaAndCount` no longer calls `configureQueryWithArguments`, so `countBy*` ignores a trailing argument map again, as `CountByFinder` did. The `countBy*Or*` change for #15789 doesn't depend on it.
- `FindByMethodSpec` has a new TCK feature that asserts `countByFirstName` and `countByFirstNameOrLastName` return the unpaged count with `[max: 2, offset: 3]`, `[sort: 'age', order: 'desc']`, both combined, and `[sort: 'nope']`. It passes on the simple datastore, Hibernate 5, Hibernate 7, MongoDB and Neo4j.
- `CountFinderSpec` pins it at unit level: with any of those maps, or `[cache: true, lock: true]`, nothing reaches `max`, `offset`, `order`, `cache` or `lock` on the query.
With the `configureQueryWithArguments` call put back, all five `CountFinderSpec` rows fail, and every TCK row fails on the simple datastore, Hibernate 5, or both.
`upgrading80x.adoc` conflicted with #16452, which took section 82 shortly before this landed, so the finder section is now 83.
The `8.1.x` side goes in with the merge-up: `RxCountFinder` will call `configureQueryWithArguments` itself so RxGORM keeps applying the map, and section 83 will keep the RxGORM sentence from your second point.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Backport #16154 to 8.0.x: replace finders inheritance hierarchy with composition [grails-core]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #16478:
URL: https://github.com/apache/grails-core/pull/16478#issuecomment-5954409888
## [Codecov](https://app.codecov.io/gh/apache/grails-core/pull/16478?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `99.25094%` with `2 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 59.1166%. Comparing base ([`f0ddfef`](https://app.codecov.io/gh/apache/grails-core/commit/f0ddfef924ad09263e7f3605abec4f331e5887c1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`7bb6de9`](https://app.codecov.io/gh/apache/grails-core/commit/7bb6de9e568d15ea780f9b14d84d036dc4ae726e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 23 commits behind head on 8.0.x.
| [Files with missing lines](https://app.codecov.io/gh/apache/grails-core/pull/16478?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...g/grails/datastore/gorm/finders/DynamicFinder.java](https://app.codecov.io/gh/apache/grails-core/pull/16478?src=pr&el=tree&filepath=grails-datamapping-core%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fgorm%2Ffinders%2FDynamicFinder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGFtYXBwaW5nLWNvcmUvc3JjL21haW4vZ3Jvb3Z5L29yZy9ncmFpbHMvZGF0YXN0b3JlL2dvcm0vZmluZGVycy9EeW5hbWljRmluZGVyLmphdmE=) | 97.5309% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/grails-core/pull/16478?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/grails-core/pull/16478?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## 8.0.x #16478 +/- ##
==================================================
+ Coverage 58.9446% 59.1166% +0.1720%
- Complexity 24451 24592 +141
==================================================
Files 2175 2170 -5
Lines 107389 107562 +173
Branches 19562 19607 +45
==================================================
+ Hits 63300 63587 +287
+ Misses 35223 35117 -106
+ Partials 8866 8858 -8
```
| [Files with missing lines](https://app.codecov.io/gh/apache/grails-core/pull/16478?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...grails/datastore/gorm/DefaultGormApiFactory.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16478?src=pr&el=tree&filepath=grails-datamapping-core%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fgorm%2FDefaultGormApiFactory.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGFtYXBwaW5nLWNvcmUvc3JjL21haW4vZ3Jvb3Z5L29yZy9ncmFpbHMvZGF0YXN0b3JlL2dvcm0vRGVmYXVsdEdvcm1BcGlGYWN0b3J5Lmdyb292eQ==) | `100.0000% <100.0000%> (ø)` | |
| [...oovy/org/grails/datastore/gorm/GormEnhancer.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16478?src=pr&el=tree&filepath=grails-datamapping-core%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fgorm%2FGormEnhancer.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGFtYXBwaW5nLWNvcmUvc3JjL21haW4vZ3Jvb3Z5L29yZy9ncmFpbHMvZGF0YXN0b3JlL2dvcm0vR29ybUVuaGFuY2VyLmdyb292eQ==) | `74.4526% <100.0000%> (ø)` | |
| [...org/grails/datastore/gorm/finders/CountFinder.java](https://app.codecov.io/gh/apache/grails-core/pull/16478?src=pr&el=tree&filepath=grails-datamapping-core%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fgorm%2Ffinders%2FCountFinder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGFtYXBwaW5nLWNvcmUvc3JjL21haW4vZ3Jvb3Z5L29yZy9ncmFpbHMvZGF0YXN0b3JlL2dvcm0vZmluZGVycy9Db3VudEZpbmRlci5qYXZh) | `100.0000% <100.0000%> (ø)` | |
| [...atastore/gorm/finders/DynamicFinderInvocation.java](https://app.codecov.io/gh/apache/grails-core/pull/16478?src=pr&el=tree&filepath=grails-datamapping-core%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fgorm%2Ffinders%2FDynamicFinderInvocation.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGFtYXBwaW5nLWNvcmUvc3JjL21haW4vZ3Jvb3Z5L29yZy9ncmFpbHMvZGF0YXN0b3JlL2dvcm0vZmluZGVycy9EeW5hbWljRmluZGVySW52b2NhdGlvbi5qYXZh) | `100.0000% <ø> (ø)` | |
| [...g/grails/datastore/gorm/finders/FinderSupport.java](https://app.codecov.io/gh/apache/grails-core/pull/16478?src=pr&el=tree&filepath=grails-datamapping-core%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fgorm%2Ffinders%2FFinderSupport.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGFtYXBwaW5nLWNvcmUvc3JjL21haW4vZ3Jvb3Z5L29yZy9ncmFpbHMvZGF0YXN0b3JlL2dvcm0vZmluZGVycy9GaW5kZXJTdXBwb3J0LmphdmE=) | `100.0000% <100.0000%> (ø)` | |
| [...ails/datastore/gorm/finders/ListOrderByFinder.java](https://app.codecov.io/gh/apache/grails-core/pull/16478?src=pr&el=tree&filepath=grails-datamapping-core%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fgorm%2Ffinders%2FListOrderByFinder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGFtYXBwaW5nLWNvcmUvc3JjL21haW4vZ3Jvb3Z5L29yZy9ncmFpbHMvZGF0YXN0b3JlL2dvcm0vZmluZGVycy9MaXN0T3JkZXJCeUZpbmRlci5qYXZh) | `100.0000% <100.0000%> (+13.7931%)` | :arrow_up: |
| [...rails/datastore/gorm/finders/ListResultFinder.java](https://app.codecov.io/gh/apache/grails-core/pull/16478?src=pr&el=tree&filepath=grails-datamapping-core%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fgorm%2Ffinders%2FListResultFinder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGFtYXBwaW5nLWNvcmUvc3JjL21haW4vZ3Jvb3Z5L29yZy9ncmFpbHMvZGF0YXN0b3JlL2dvcm0vZmluZGVycy9MaXN0UmVzdWx0RmluZGVyLmphdmE=) | `100.0000% <100.0000%> (ø)` | |
| [...rails/datastore/gorm/finders/MethodExpression.java](https://app.codecov.io/gh/apache/grails-core/pull/16478?src=pr&el=tree&filepath=grails-datamapping-core%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fgorm%2Ffinders%2FMethodExpression.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGFtYXBwaW5nLWNvcmUvc3JjL21haW4vZ3Jvb3Z5L29yZy9ncmFpbHMvZGF0YXN0b3JlL2dvcm0vZmluZGVycy9NZXRob2RFeHByZXNzaW9uLmphdmE=) | `97.2678% <100.0000%> (+34.1099%)` | :arrow_up: |
| [...ils/datastore/gorm/finders/SingleResultFinder.java](https://app.codecov.io/gh/apache/grails-core/pull/16478?src=pr&el=tree&filepath=grails-datamapping-core%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fgorm%2Ffinders%2FSingleResultFinder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGFtYXBwaW5nLWNvcmUvc3JjL21haW4vZ3Jvb3Z5L29yZy9ncmFpbHMvZGF0YXN0b3JlL2dvcm0vZmluZGVycy9TaW5nbGVSZXN1bHRGaW5kZXIuamF2YQ==) | `100.0000% <100.0000%> (ø)` | |
| [...orm/query/criteria/AbstractDetachedCriteria.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16478?src=pr&el=tree&filepath=grails-datamapping-core%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fgorm%2Fquery%2Fcriteria%2FAbstractDetachedCriteria.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGFtYXBwaW5nLWNvcmUvc3JjL21haW4vZ3Jvb3Z5L29yZy9ncmFpbHMvZGF0YXN0b3JlL2dvcm0vcXVlcnkvY3JpdGVyaWEvQWJzdHJhY3REZXRhY2hlZENyaXRlcmlhLmdyb292eQ==) | `74.9254% <ø> (ø)` | |
| ... and [1 more](https://app.codecov.io/gh/apache/grails-core/pull/16478?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
... and [13 files with indirect coverage changes](https://app.codecov.io/gh/apache/grails-core/pull/16478/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org