jdaugherty commented on code in PR #16154:
URL: https://github.com/apache/grails-core/pull/16154#discussion_r4146004859
##########
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:
Called out now. The PR description has a "Public API changes" section
listing the removed classes and the `DynamicFinder` changes, and a091ca07e7
adds section 21 to the Grails 8 upgrade guide (`upgrading80x.adoc`) with the
factory-method replacements for third-party datastores and plugins.
##########
grails-datamapping-rx/src/test/groovy/org/grails/gorm/rx/finders/RxSingleResultFinderSpec.groovy:
##########
@@ -0,0 +1,325 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.grails.gorm.rx.finders
+
+import grails.gorm.rx.RxEntity
+import org.grails.datastore.mapping.core.Session
+import org.grails.datastore.mapping.model.MappingContext
+import org.grails.datastore.mapping.model.PersistentEntity
+import org.grails.datastore.mapping.model.PersistentProperty
+import org.grails.datastore.mapping.query.Query
+import org.grails.datastore.rx.RxDatastoreClient
+import org.springframework.core.convert.support.DefaultConversionService
+import rx.Observable
+import spock.lang.Specification
+
+/**
+ * Exercises {@link RxSingleResultFinder} - the rx-module mirror of {@link
+ * org.grails.datastore.gorm.finders.SingleResultFinder}, composing {@link
+ * org.grails.datastore.gorm.finders.DynamicFinder} rather than extending
core's finder classes as
+ * the previous per-type rx finder classes
(FindByFinder/FindByBooleanFinder/FindOrCreateByFinder/
+ * FindOrSaveByFinder) did. Unlike those classes' own specs, which hand-built a
+ * DynamicFinderInvocation and called the package-private doInvokeInternal
directly, these tests go
+ * through the real public invoke() entry point with real method names -
doInvokeInternal no longer
+ * exists as a separate seam, and this also means the method-name parsing
itself is now exercised,
+ * not just the query-building/execution half.
+ */
+class RxSingleResultFinderSpec extends Specification {
+
+ RxDatastoreClient datastoreClient = Mock()
+ MappingContext mappingContext = Mock()
+ PersistentEntity entity = Mock()
+ Query query = Mock()
+
+ PersistentProperty titleProperty = Stub(PersistentProperty) {
+ getName() >> 'title'
+ getType() >> String
+ }
+ PersistentProperty nameProperty = Stub(PersistentProperty) {
+ getName() >> 'name'
+ getType() >> String
+ }
+ PersistentProperty activeProperty = Stub(PersistentProperty) {
+ getName() >> 'active'
+ getType() >> Boolean
+ }
+ PersistentProperty cityProperty = Stub(PersistentProperty) {
+ getName() >> 'city'
+ getType() >> String
+ }
+
+ def setup() {
+ entity.getMappingContext() >> mappingContext
+ mappingContext.getConversionService() >> new DefaultConversionService()
+ mappingContext.getPersistentEntity(_) >> entity
+ entity.getPropertyByName('title') >> titleProperty
+ entity.getPropertyByName('name') >> nameProperty
+ entity.getPropertyByName('city') >> cityProperty
+ entity.getPropertyByName('active') >> activeProperty
+ query.getEntity() >> entity
+ datastoreClient.getMappingContext() >> mappingContext
+ }
+
+ void "findBy obtains its mapping context from the datastore client when
constructed"() {
+ when:
+ RxSingleResultFinder.findBy(datastoreClient)
+
+ then:
+ 1 * datastoreClient.getMappingContext() >> mappingContext
+ }
+
+ void "findBy isMethodMatch matches findBy* method names"() {
+ expect:
+
RxSingleResultFinder.findBy(datastoreClient).isMethodMatch('findByTitle')
+
!RxSingleResultFinder.findBy(datastoreClient).isMethodMatch('somethingElse')
+ }
+
+ void "findBy finds by building and executing a query via the RX datastore
client instead of a session"() {
+ given:
+ def finder = RxSingleResultFinder.findBy(datastoreClient)
+
+ when:
+ def result = finder.invoke(Book, 'findByTitle', ['Shogun'] as Object[])
+
+ then:
+ 1 * datastoreClient.createQuery(Book) >> query
+ 1 * query.add({ Query.Criterion it -> it instanceof Query.Junction })
+ 1 * query.singleResult() >> new Book(title: 'Shogun')
+ result.title == 'Shogun'
Review Comment:
The findBy features now stub `singleResult()` with an `Observable` and
assert on the emitted entity; the remaining two that still returned a bare
value were updated in a091ca07e7.
--
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]