This is an automated email from the ASF dual-hosted git repository. borinquenkid pushed a commit to branch test/document-datamapping-core-transformers in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit 55e638495f84ac9d937a4e06888ea6d933793b43 Author: Walter Duque de Estrada <[email protected]> AuthorDate: Thu Aug 13 12:59:28 2026 -0500 Add test coverage for ServiceTransformation's implementer-adapter and descriptor-writing paths ServiceTransformation sat at 77%/58% coverage. Add tests for several previously-unexercised paths: - The constructor-validation error on abstract data services (confirmed already safe - it uses addErrorAndContinue, not the crash-prone pattern found and fixed elsewhere in this branch). - ServiceImplementerAdapter loading/deduplication and the AdaptedImplementer handling, via test-only ServiceLoader-registered fixtures (support/) that only ever match a deliberately obscure method name so they can't interfere with any other @Service in the module's test suite. - generateServiceDescriptor's real file-writing path (creating and appending to a META-INF/services descriptor), using a real target directory pointed at a temp dir so nothing leaks into the real build output. - Domain mapping-closure resolution edge cases (non-closure mapping value, unrelated leading statements before the datasource call, a mapping closure that never calls datasource) and the generated-method-replaces-user-override cleanup path, added to the existing ConnectionRoutingServiceTransformSpec alongside its other mapping/connection-routing coverage. - priority(). Two branches (the implementers=/adapters= annotation members) were left uncovered: ServiceTransformation.LOADED_IMPLEMENTORS is a static field populated once per test JVM by whichever @Service compiles first, which makes those branches unreachable without either depending on test execution order or reflectively resetting internal state - both of which this branch's testing conventions rule out. Coverage moves 77% -> 88% instruction, 58% -> 71% branch. Co-Authored-By: Claude Sonnet 5 <[email protected]> --- .../ConnectionRoutingServiceTransformSpec.groovy | 144 ++++++++++++++++ .../transform/ServiceTransformationSpec.groovy | 183 +++++++++++++++++++++ .../support/AdaptedProbeServiceImplementer.groovy | 64 +++++++ .../support/NoOpServiceImplementerAdapter.groovy | 41 +++++ .../support/ProbeServiceImplementer.groovy | 63 +++++++ .../support/ProbeServiceImplementerAdapter.groovy | 43 +++++ ...ails.datastore.gorm.services.ServiceImplementer | 1 + ...tastore.gorm.services.ServiceImplementerAdapter | 2 + 8 files changed, 541 insertions(+) diff --git a/grails-datamapping-core/src/test/groovy/grails/gorm/services/ConnectionRoutingServiceTransformSpec.groovy b/grails-datamapping-core/src/test/groovy/grails/gorm/services/ConnectionRoutingServiceTransformSpec.groovy index 1c1aa441ac..bbdd6170cb 100644 --- a/grails-datamapping-core/src/test/groovy/grails/gorm/services/ConnectionRoutingServiceTransformSpec.groovy +++ b/grails-datamapping-core/src/test/groovy/grails/gorm/services/ConnectionRoutingServiceTransformSpec.groovy @@ -593,4 +593,148 @@ class TxItem { impl.getAnnotation(Transactional).connection() == 'secondary' } + void "test service with a domain mapping value that is not a closure gets no connection annotation"() { + when: "a domain declares a 'mapping' field whose value is not a closure at all" + def service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity + +@Service(NonClosureItem) +abstract class NonClosureItemService { + + abstract NonClosureItem save(NonClosureItem item) +} + +@Entity +class NonClosureItem { + String name + + static mapping = 'not-a-closure' +} +''') + + then: "the class compiles without errors" + !service.isInterface() + + when: "the implementation is loaded" + def impl = service.classLoader.loadClass('$NonClosureItemServiceImplementation') + + then: "no @Transactional(connection) is added since the mapping value can't be inspected as a closure" + impl != null + def txAnn = impl.getAnnotation(Transactional) + txAnn == null || txAnn.connection() == '' + } + + void "test datasource is still resolved from a mapping closure containing unrelated statements"() { + when: "the mapping closure contains statements that aren't simple method calls before the datasource call" + def service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity + +@Service(MixedStatementItem) +abstract class MixedStatementItemService { + + abstract MixedStatementItem save(MixedStatementItem item) +} + +@Entity +class MixedStatementItem { + String name + + static mapping = { + if (true) { } + def unused = 1 + datasource 'secondary' + } +} +''') + + then: "the class compiles without errors" + !service.isInterface() + + when: "the implementation is loaded" + def impl = service.classLoader.loadClass('$MixedStatementItemServiceImplementation') + + then: "the datasource is still correctly resolved despite the unrelated leading statements" + impl != null + impl.getAnnotation(Transactional) != null + impl.getAnnotation(Transactional).connection() == 'secondary' + } + + void "test service with a mapping closure that never calls datasource gets no connection annotation"() { + when: "the mapping closure has statements but none of them call datasource/connection/connections" + def service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity + +@Service(NoDatasourceCallItem) +abstract class NoDatasourceCallItemService { + + abstract NoDatasourceCallItem save(NoDatasourceCallItem item) +} + +@Entity +class NoDatasourceCallItem { + String name + + static mapping = { + id generator: 'assigned' + } +} +''') + + then: "the class compiles without errors" + !service.isInterface() + + when: "the implementation is loaded" + def impl = service.classLoader.loadClass('$NoDatasourceCallItemServiceImplementation') + + then: "no @Transactional(connection) is added" + impl != null + def txAnn = impl.getAnnotation(Transactional) + txAnn == null || txAnn.connection() == '' + } + + void "test a user-declared getTransactionManager is replaced by the generated connection-aware one"() { + when: "an abstract service overrides getTransactionManager and its domain uses a non-default datasource" + def service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity +import org.springframework.transaction.PlatformTransactionManager + +@Service(OverrideTxItem) +abstract class OverrideTxItemService { + + PlatformTransactionManager getTransactionManager() { + return null + } + + abstract OverrideTxItem save(OverrideTxItem item) +} + +@Entity +class OverrideTxItem { + String name + + static mapping = { + datasource 'secondary' + } +} +''') + + then: "the class compiles without errors" + !service.isInterface() + + when: "the implementation is loaded and instantiated" + def impl = service.classLoader.loadClass('$OverrideTxItemServiceImplementation') + def instance = impl.getDeclaredConstructor().newInstance() + + and: "getTransactionManager() is invoked" + instance.getTransactionManager() + + then: "the call routes through the generated connection-aware lookup rather than the user's override " + + "(which simply returned null), failing because no datastore is configured in this test" + thrown(IllegalStateException) + } + } diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/transform/ServiceTransformationSpec.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/transform/ServiceTransformationSpec.groovy new file mode 100644 index 0000000000..1584e36fa4 --- /dev/null +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/transform/ServiceTransformationSpec.groovy @@ -0,0 +1,183 @@ +/* + * 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.datastore.gorm.services.transform + +import org.codehaus.groovy.control.CompilerConfiguration +import org.codehaus.groovy.control.MultipleCompilationErrorsException +import spock.lang.TempDir + +import org.apache.grails.common.compiler.GroovyTransformOrder +import org.grails.datastore.gorm.services.Implemented +import org.grails.datastore.gorm.services.transform.support.ProbeServiceImplementer +import spock.lang.Specification + +/** + * Covers compile-time behaviour of {@link ServiceTransformation} that the higher level + * {@code @Service} usage specs (eg. {@code grails.gorm.services.ServiceTransformSpec}) don't + * otherwise exercise: constructor validation on abstract data services, resolution of methods + * through a {@code ServiceImplementerAdapter}, the {@code META-INF/services} descriptor writer, and + * the transform's declared priority. + */ +class ServiceTransformationSpec extends Specification { + + @TempDir + File targetDirectory + + void "test an abstract data service with an explicit constructor produces a clean compile error"() { + when: 'an abstract data service declares a constructor' + new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity + +@Service(Foo) +abstract class FooService { + + FooService() { + } + + abstract Foo find(Serializable id) +} +@Entity +class Foo { + String title +} +''') + + then: 'a clean compilation error is raised rather than a crash' + def e = thrown(MultipleCompilationErrorsException) + e.message.contains('Abstract data Services should not define constructors') + } + + void "test an abstract data service without a constructor compiles cleanly"() { + when: 'an abstract data service declares no constructor' + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity + +@Service(Foo) +abstract class FooServiceNoCtor { + + abstract Foo find(Serializable id) +} +@Entity +class Foo { + String title +} +''') + + then: 'no error is raised' + !service.isInterface() + } + + void "test a method resolved through a ServiceImplementerAdapter is annotated with the adapted implementer"() { + when: 'a @Service interface declares a method that only a ServiceLoader-registered adapter can implement' + Class service = new GroovyClassLoader().parseClass(""" +import grails.gorm.services.Service +import grails.gorm.annotation.Entity + +@Service(Foo) +interface ProbeService { + Object ${ProbeServiceImplementer.TARGET_METHOD_NAME}() +} +@Entity +class Foo { + String title +} +""") + + then: 'the interface compiles cleanly' + service.isInterface() + + when: 'the implementation is loaded' + Class impl = service.classLoader.loadClass('$ProbeServiceImplementation') + + then: 'the method was implemented via the adapted implementer' + impl.getMethod(ProbeServiceImplementer.TARGET_METHOD_NAME) + .getAnnotation(Implemented) + .by() == ProbeServiceImplementer + } + + void "test a concrete @Service class generates a descriptor without going through the interface/abstract-class impl path"() { + when: 'a @Service annotation is applied directly to a concrete (non-abstract) class, compiled dynamically' + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service + +@Service +class ConcreteProbeService { + void doStuff() { + } +} +''') + + then: 'the class is used as-is, with no separate $...Implementation class generated' + !service.isInterface() + org.grails.datastore.mapping.services.Service.isAssignableFrom(service) + } + + void "test priority returns the data service transform order"() { + expect: + new ServiceTransformation().priority() == GroovyTransformOrder.DATA_SERVICE_ORDER + } + + void "test an exposed @Service writes a META-INF services descriptor to the compiler's target directory"() { + given: 'a source file compiled with a real target directory, so the file-writing descriptor path runs' + File sourceFile = new File(targetDirectory, 'DescriptorProbeServices.groovy') + sourceFile.text = ''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity + +@Service(Foo) +interface DescriptorFooService { + Foo find(Serializable id) +} + +@Service(Bar) +interface DescriptorBarService { + Bar find(Serializable id) +} + +@Entity +class Foo { + String title +} +@Entity +class Bar { + String title +} +''' + File outputDirectory = new File(targetDirectory, 'classes-out') + outputDirectory.mkdirs() + CompilerConfiguration config = new CompilerConfiguration() + config.setTargetDirectory(outputDirectory) + GroovyClassLoader gcl = new GroovyClassLoader(getClass().classLoader, config) + + when: 'the source file is compiled' + gcl.parseClass(sourceFile) + + then: 'a META-INF/services descriptor is written under the configured target directory' + File descriptor = new File(outputDirectory, 'META-INF/services/org.grails.datastore.mapping.services.Service') + descriptor.exists() + + and: 'it lists both generated implementation classes, appended rather than overwritten' + List<String> entries = descriptor.text.split('\\r?\\n') as List<String> + entries.contains('$DescriptorFooServiceImplementation') + entries.contains('$DescriptorBarServiceImplementation') + entries.size() == 2 + } +} diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/transform/support/AdaptedProbeServiceImplementer.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/transform/support/AdaptedProbeServiceImplementer.groovy new file mode 100644 index 0000000000..4bfa2e9d17 --- /dev/null +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/transform/support/AdaptedProbeServiceImplementer.groovy @@ -0,0 +1,64 @@ +/* + * 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.datastore.gorm.services.transform.support + +import groovy.transform.CompileStatic +import org.codehaus.groovy.ast.ClassNode +import org.codehaus.groovy.ast.MethodNode + +import org.grails.datastore.gorm.services.ServiceImplementer +import org.grails.datastore.gorm.services.implementers.AdaptedImplementer +import org.grails.datastore.mapping.core.Ordered + +/** + * The adapted form of {@link ProbeServiceImplementer} produced by {@link ProbeServiceImplementerAdapter}. + * Declares an order lower than the default (un-ordered) precedence of {@link ProbeServiceImplementer} + * so that {@code ServiceTransformation} always tries this adapted implementer first, guaranteeing its + * {@code AdaptedImplementer} branch is exercised deterministically rather than depending on collection + * ordering. + */ +@CompileStatic +class AdaptedProbeServiceImplementer implements ServiceImplementer, AdaptedImplementer, Ordered { + + private final ServiceImplementer adapted + + AdaptedProbeServiceImplementer(ServiceImplementer adapted) { + this.adapted = adapted + } + + @Override + ServiceImplementer getAdapted() { + return adapted + } + + @Override + int getOrder() { + return 0 + } + + @Override + boolean doesImplement(ClassNode domainClass, MethodNode methodNode) { + return adapted.doesImplement(domainClass, methodNode) + } + + @Override + void implement(ClassNode domainClassNode, MethodNode abstractMethodNode, MethodNode newMethodNode, ClassNode targetClassNode) { + adapted.implement(domainClassNode, abstractMethodNode, newMethodNode, targetClassNode) + } +} diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/transform/support/NoOpServiceImplementerAdapter.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/transform/support/NoOpServiceImplementerAdapter.groovy new file mode 100644 index 0000000000..db683c1c82 --- /dev/null +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/transform/support/NoOpServiceImplementerAdapter.groovy @@ -0,0 +1,41 @@ +/* + * 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.datastore.gorm.services.transform.support + +import groovy.transform.CompileStatic + +import org.grails.datastore.gorm.services.ServiceImplementer +import org.grails.datastore.gorm.services.ServiceImplementerAdapter + +/** + * A second, deliberately inert {@link ServiceImplementerAdapter} registered via + * {@code META-INF/services} purely so that {@code ServiceTransformation} ever has more than one + * adapter to de-duplicate, exercising the {@code unique { it.class.name }} call it makes over the + * loaded adapters. It never adapts anything. + * + * @see ProbeServiceImplementerAdapter + */ +@CompileStatic +class NoOpServiceImplementerAdapter implements ServiceImplementerAdapter { + + @Override + ServiceImplementer adapt(ServiceImplementer implementer) { + return null + } +} diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/transform/support/ProbeServiceImplementer.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/transform/support/ProbeServiceImplementer.groovy new file mode 100644 index 0000000000..f497be0bae --- /dev/null +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/transform/support/ProbeServiceImplementer.groovy @@ -0,0 +1,63 @@ +/* + * 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.datastore.gorm.services.transform.support + +import groovy.transform.CompileStatic +import org.codehaus.groovy.ast.ClassNode +import org.codehaus.groovy.ast.MethodNode +import org.codehaus.groovy.ast.stmt.BlockStatement + +import org.grails.datastore.gorm.services.ServiceImplementer + +import static org.codehaus.groovy.ast.tools.GeneralUtils.constX +import static org.codehaus.groovy.ast.tools.GeneralUtils.returnS + +/** + * Test-only {@link ServiceImplementer} registered via {@code META-INF/services} so that + * {@code ServiceTransformation} loads it through the same {@link ServiceLoader} mechanism used by + * real GORM implementation modules (see {@code grails-datamapping-rx}). It exists purely to give + * {@link ProbeServiceImplementerAdapter} something to adapt, exercising the + * {@code org.grails.datastore.gorm.services.implementers.AdaptedImplementer} handling in + * {@code ServiceTransformation}. + * <p> + * It only ever matches an intentionally obscure method name so it can never interfere with any + * other {@code @Service} compiled elsewhere in this module's test suite - the {@code ServiceLoader} + * lookup in {@code ServiceTransformation} is cached for the lifetime of the test JVM, so this + * implementer becomes part of every subsequent {@code @Service} compilation once loaded. + * + * @see ProbeServiceImplementerAdapter + * @see AdaptedProbeServiceImplementer + */ +@CompileStatic +class ProbeServiceImplementer implements ServiceImplementer { + + static final String TARGET_METHOD_NAME = 'zzzProbeAdapterOnlyMethod' + + @Override + boolean doesImplement(ClassNode domainClass, MethodNode methodNode) { + return methodNode.name == TARGET_METHOD_NAME + } + + @Override + void implement(ClassNode domainClassNode, MethodNode abstractMethodNode, MethodNode newMethodNode, ClassNode targetClassNode) { + BlockStatement body = new BlockStatement() + body.addStatement(returnS(constX(null))) + newMethodNode.code = body + } +} diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/transform/support/ProbeServiceImplementerAdapter.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/transform/support/ProbeServiceImplementerAdapter.groovy new file mode 100644 index 0000000000..5403179160 --- /dev/null +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/transform/support/ProbeServiceImplementerAdapter.groovy @@ -0,0 +1,43 @@ +/* + * 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.datastore.gorm.services.transform.support + +import groovy.transform.CompileStatic + +import org.grails.datastore.gorm.services.ServiceImplementer +import org.grails.datastore.gorm.services.ServiceImplementerAdapter + +/** + * Test-only {@link ServiceImplementerAdapter}, registered via {@code META-INF/services}, that wraps + * {@link ProbeServiceImplementer} instances into an {@link AdaptedProbeServiceImplementer}. This mirrors + * how {@code grails-datamapping-rx} registers its own adapter in production, allowing this module's + * tests to exercise {@code ServiceTransformation}'s adapter-loading and {@code AdaptedImplementer} + * handling without depending on another module. + */ +@CompileStatic +class ProbeServiceImplementerAdapter implements ServiceImplementerAdapter { + + @Override + ServiceImplementer adapt(ServiceImplementer implementer) { + if (implementer instanceof ProbeServiceImplementer) { + return new AdaptedProbeServiceImplementer(implementer) + } + return null + } +} diff --git a/grails-datamapping-core/src/test/resources/META-INF/services/org.grails.datastore.gorm.services.ServiceImplementer b/grails-datamapping-core/src/test/resources/META-INF/services/org.grails.datastore.gorm.services.ServiceImplementer new file mode 100644 index 0000000000..0e54f7b735 --- /dev/null +++ b/grails-datamapping-core/src/test/resources/META-INF/services/org.grails.datastore.gorm.services.ServiceImplementer @@ -0,0 +1 @@ +org.grails.datastore.gorm.services.transform.support.ProbeServiceImplementer diff --git a/grails-datamapping-core/src/test/resources/META-INF/services/org.grails.datastore.gorm.services.ServiceImplementerAdapter b/grails-datamapping-core/src/test/resources/META-INF/services/org.grails.datastore.gorm.services.ServiceImplementerAdapter new file mode 100644 index 0000000000..8713da32c0 --- /dev/null +++ b/grails-datamapping-core/src/test/resources/META-INF/services/org.grails.datastore.gorm.services.ServiceImplementerAdapter @@ -0,0 +1,2 @@ +org.grails.datastore.gorm.services.transform.support.ProbeServiceImplementerAdapter +org.grails.datastore.gorm.services.transform.support.NoOpServiceImplementerAdapter
