sbglasius commented on code in PR #16154:
URL: https://github.com/apache/grails-core/pull/16154#discussion_r4144495470


##########
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/finders/DynamicFinder.java:
##########
@@ -66,13 +67,16 @@
 import org.grails.datastore.mapping.reflect.NameUtils;
 
 /**
- * Abstract base class for dynamic finders.
+ * Parses a dynamic finder method name into a {@link DynamicFinderInvocation}, 
builds the AND/OR
+ * junction of criteria for it, and exposes the shared 
argument-map/fetch/sort/detached-criteria
+ * handling used by every finder implementation. Composed (not extended) by 
the concrete finder
+ * classes in this package and in {@code grails-datamapping-rx} - see {@link 
FinderGrammar}.
  *
  * @author Graeme Rocher
  * @since 1.0
  */
-@SuppressWarnings({"rawtypes", "unchecked"})
-public abstract class DynamicFinder extends AbstractFinder implements 
QueryBuildingFinder {
+@SuppressWarnings({"rawtypes", "unchecked", "ResultOfMethodCallIgnored"})
+public class DynamicFinder implements FinderGrammar {

Review Comment:
   **Public API break.** This refactor deletes the public finder classes 
(`FindByFinder`, `FindAllByFinder`, `CountByFinder`, `FindOrCreateByFinder`, 
`FindOrSaveByFinder`, the boolean finders, `AbstractFindByFinder`, 
`AbstractFinder`). `DynamicFinder` is now concrete with a different constructor 
and no `invoke`/`doInvokeInternal`.
   
   I found no remaining users in this repo. A third-party GORM plugin or 
datastore that extends any of these will fail to compile or throw 
`NoClassDefFoundError` on upgrade. Please call this out in the PR description 
or upgrade notes.



-- 
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]

Reply via email to