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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
