diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractDetachedCriteriaServiceImplementor.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractDetachedCriteriaServiceImplementor.groovy index 312b60454a7..95fbef54f33 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractDetachedCriteriaServiceImplementor.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractDetachedCriteriaServiceImplementor.groovy @@ -123,6 +123,9 @@ abstract class AbstractDetachedCriteriaServiceImplementor extends AbstractReadOp } } + // domainClassNode is unused here, but kept so subclasses can override this as a + // polymorphic extension point and pick a DetachedCriteria type based on the domain class + @SuppressWarnings(['unused', 'MethodMayBeStatic']) protected ClassNode getDetachedCriteriaType(ClassNode domainClassNode) { DETACHED_CRITERIA } @@ -150,6 +153,9 @@ abstract class AbstractDetachedCriteriaServiceImplementor extends AbstractReadOp * Whether lookup by id is allowed by this implementation * @return True if it is */ + // Not static: AbstractProjectionImplementer overrides this to return false, and + // doImplement() dispatches on it polymorphically. A static method would break that override. + @SuppressWarnings('MethodMayBeStatic') protected boolean lookupById() { return true } diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractSaveImplementer.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractSaveImplementer.groovy index 2ba6864766b..73b8f21c626 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractSaveImplementer.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractSaveImplementer.groovy @@ -51,7 +51,7 @@ import static org.grails.datastore.gorm.transform.AstMethodDispatchUtils.namedAr @CompileStatic abstract class AbstractSaveImplementer extends AbstractWriteOperationImplementer { - protected Statement bindParametersAndSave(ClassNode domainClassNode, MethodNode abstractMethodNode, MethodNode newMethodNode, Parameter[] parameters, BlockStatement body, VariableExpression entityVar) { + protected Statement bindParametersAndSave(ClassNode domainClassNode, MethodNode abstractMethodNode, Parameter[] parameters, BlockStatement body, VariableExpression entityVar) { Expression argsExpression = null for (Parameter parameter in parameters) { @@ -64,8 +64,8 @@ abstract class AbstractSaveImplementer extends AbstractWriteOperationImplementer argsExpression = varX(parameter) } else { AstUtils.error( - newMethodNode.declaringClass.module.context, - newMethodNode, + abstractMethodNode.declaringClass.module.context, + abstractMethodNode, "Cannot implement method for argument [${parameterName}]. No property exists on domain class [$domainClassNode.name]" ) } diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractServiceImplementer.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractServiceImplementer.groovy index b21ceabbf53..c1b7576c5be 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractServiceImplementer.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractServiceImplementer.groovy @@ -21,7 +21,6 @@ package org.grails.datastore.gorm.services.implementers import groovy.transform.CompileStatic import org.codehaus.groovy.ast.AnnotationNode -import org.codehaus.groovy.ast.ClassHelper import org.codehaus.groovy.ast.ClassNode import org.codehaus.groovy.ast.MethodNode import org.codehaus.groovy.ast.Parameter @@ -29,12 +28,10 @@ import org.codehaus.groovy.ast.expr.Expression import org.codehaus.groovy.transform.trait.Traits import grails.gorm.multitenancy.TenantService -import grails.gorm.transactions.TransactionService import org.grails.datastore.gorm.GormEnhancer import org.grails.datastore.gorm.multitenancy.transform.TenantTransform import org.grails.datastore.gorm.services.ServiceImplementer import org.grails.datastore.gorm.transactions.transform.TransactionalTransform -import org.grails.datastore.gorm.transform.AstMethodDispatchUtils import org.grails.datastore.gorm.transform.AstPropertyResolveUtils import org.grails.datastore.mapping.core.Ordered import org.grails.datastore.mapping.model.config.GormProperties @@ -42,7 +39,6 @@ import org.grails.datastore.mapping.multitenancy.MultiTenancySettings import org.grails.datastore.mapping.multitenancy.MultiTenantCapableDatastore import org.grails.datastore.mapping.reflect.AstUtils import org.grails.datastore.mapping.services.ServiceRegistry -import org.grails.datastore.mapping.transactions.TransactionCapableDatastore import static org.codehaus.groovy.ast.ClassHelper.make import static org.codehaus.groovy.ast.tools.GeneralUtils.args @@ -116,7 +112,7 @@ abstract class AbstractServiceImplementer implements PrefixedServiceImplementer, * @return True if it is */ protected boolean isValidParameter(ClassNode domainClassNode, Parameter parameter, String parameterName) { - if (GormProperties.IDENTITY.equals(parameterName)) { + if (GormProperties.IDENTITY == parameterName) { return true } else { @@ -135,18 +131,11 @@ abstract class AbstractServiceImplementer implements PrefixedServiceImplementer, return propX(varX('this'), 'targetDatastore') } - /** - * @return The datastore expression - */ - protected Expression transactionalDatastore() { - return castX(ClassHelper.make(TransactionCapableDatastore), propX(varX('this'), 'targetDatastore')) - } - /** * @return The datastore expression */ protected Expression multiTenantDatastore() { - return castX(ClassHelper.make(MultiTenantCapableDatastore), propX(varX('this'), 'targetDatastore')) + return castX(make(MultiTenantCapableDatastore), propX(varX('this'), 'targetDatastore')) } /** @@ -156,16 +145,9 @@ abstract class AbstractServiceImplementer implements PrefixedServiceImplementer, return callD(ServiceRegistry, 'targetDatastore', 'getService', classX(make(TenantService))) } - /** - * @return The transaction service - */ - protected Expression transactionService() { - return callD(ServiceRegistry, 'targetDatastore', 'getService', classX(make(TransactionService))) - } - protected Expression findConnectionId(MethodNode methodNode) { if (TenantTransform.hasTenantAnnotation(methodNode)) { - return callD(classX(ClassHelper.make(MultiTenancySettings)), 'resolveConnectionForTenantId', args( + return callD(classX(make(MultiTenancySettings)), 'resolveConnectionForTenantId', args( propX(multiTenantDatastore(), 'multiTenancyMode'), callD(tenantService(), 'currentId') )) } @@ -180,27 +162,17 @@ abstract class AbstractServiceImplementer implements PrefixedServiceImplementer, } protected Expression buildInstanceApiLookup(ClassNode domainClass, Expression connectionId) { - return AstMethodDispatchUtils.callD( + return callD( classX(GormEnhancer), 'findInstanceApi', args(classX(domainClass), connectionId) ) } protected Expression buildStaticApiLookup(ClassNode domainClass, Expression connectionId) { - return AstMethodDispatchUtils.callD( + return callD( classX(GormEnhancer), 'findStaticApi', args(classX(domainClass), connectionId) ) } - protected Expression findInstanceApiForConnectionId(ClassNode domainClass, MethodNode methodNode) { - Expression connectionId = findConnectionId(methodNode) - if (connectionId != null) { - return buildInstanceApiLookup(domainClass, connectionId) - } - else { - return classX(domainClass.plainNodeReference) - } - } - protected Expression findStaticApiForConnectionId(ClassNode domainClass, MethodNode methodNode) { Expression connectionId = findConnectionId(methodNode) if (connectionId != null) { diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractWhereImplementer.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractWhereImplementer.groovy index 01125f4de36..0c737719d64 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractWhereImplementer.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/AbstractWhereImplementer.groovy @@ -117,6 +117,9 @@ abstract class AbstractWhereImplementer extends AbstractReadOperationImplementer } } + // domainClassNode is unused here, but kept so subclasses can override this as a + // polymorphic extension point and pick a DetachedCriteria type based on the domain class + @SuppressWarnings(['unused', 'MethodMayBeStatic']) protected ClassNode getDetachedCriteriaType(ClassNode domainClassNode) { ClassHelper.make(DetachedCriteria) } diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/DeleteImplementer.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/DeleteImplementer.groovy index 701a6858b0b..e3f2da4e76d 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/DeleteImplementer.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/DeleteImplementer.groovy @@ -64,7 +64,7 @@ class DeleteImplementer extends AbstractDetachedCriteriaServiceImplementor imple @Override protected boolean isCompatibleReturnType(ClassNode domainClass, MethodNode methodNode, ClassNode returnType, String prefix) { - return ClassHelper.VOID_TYPE.equals(returnType) || AstUtils.isSubclassOfOrImplementsInterface(returnType, Number.name) + return ClassHelper.VOID_TYPE == returnType || AstUtils.isSubclassOfOrImplementsInterface(returnType, Number.name) } @Override @@ -79,7 +79,7 @@ class DeleteImplementer extends AbstractDetachedCriteriaServiceImplementor imple @Override void implementById(ClassNode domainClassNode, MethodNode abstractMethodNode, MethodNode newMethodNode, ClassNode targetClassNode, BlockStatement body, Expression byIdLookup) { - boolean isVoidReturnType = ClassHelper.VOID_TYPE.equals(newMethodNode.returnType) + boolean isVoidReturnType = ClassHelper.VOID_TYPE == newMethodNode.returnType VariableExpression obj = varX('$obj') Expression connectionId = findConnectionId(abstractMethodNode) Statement deleteStatement @@ -115,7 +115,7 @@ class DeleteImplementer extends AbstractDetachedCriteriaServiceImplementor imple void implementWithQuery(ClassNode domainClassNode, MethodNode abstractMethodNode, MethodNode newMethodNode, ClassNode targetClassNode, BlockStatement body, VariableExpression detachedCriteriaVar, Expression queryArgs) { MethodCallExpression deleteCall = callX(detachedCriteriaVar, 'deleteAll') - boolean isVoidReturnType = ClassHelper.VOID_TYPE.equals(newMethodNode.returnType) + boolean isVoidReturnType = ClassHelper.VOID_TYPE == newMethodNode.returnType body.addStatements([ // return query.deleteAll() diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/DeleteWhereImplementer.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/DeleteWhereImplementer.groovy index bb7e3b9b0d4..c0c19258b84 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/DeleteWhereImplementer.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/DeleteWhereImplementer.groovy @@ -58,7 +58,7 @@ class DeleteWhereImplementer extends AbstractWhereImplementer { @Override protected boolean isCompatibleReturnType(ClassNode domainClass, MethodNode methodNode, ClassNode returnType, String prefix) { - return ClassHelper.VOID_TYPE.equals(returnType) || AstUtils.isSubclassOfOrImplementsInterface(returnType, Number.name) + return ClassHelper.VOID_TYPE == returnType || AstUtils.isSubclassOfOrImplementsInterface(returnType, Number.name) } @Override diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/FindAllByImplementer.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/FindAllByImplementer.groovy index 2d4e0212e60..dd80fafeda5 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/FindAllByImplementer.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/FindAllByImplementer.groovy @@ -36,7 +36,6 @@ import org.grails.datastore.gorm.finders.DynamicFinder import org.grails.datastore.gorm.finders.MatchSpec import org.grails.datastore.gorm.services.transform.ServiceTransformation import org.grails.datastore.mapping.core.Ordered -import org.grails.datastore.mapping.reflect.AstUtils import static org.codehaus.groovy.ast.ClassHelper.MAP_TYPE import static org.codehaus.groovy.ast.tools.GeneralUtils.args @@ -109,7 +108,7 @@ class FindAllByImplementer extends AbstractArrayOrIterableResultImplementer impl } if (matchSpec == null) { - AstUtils.error(abstractMethodNode.declaringClass.module.context, abstractMethodNode, ServiceTransformation.NO_IMPLEMENTATIONS_MESSAGE) + error(abstractMethodNode.declaringClass.module.context, abstractMethodNode, ServiceTransformation.NO_IMPLEMENTATIONS_MESSAGE) } else { // validate the properties diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/FindOneImplementer.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/FindOneImplementer.groovy index 8a4aadf96ab..1734421aa9a 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/FindOneImplementer.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/FindOneImplementer.groovy @@ -75,6 +75,11 @@ class FindOneImplementer extends AbstractDetachedCriteriaServiceImplementor impl ) } + // domainClassNode/newMethodNode are unused here, but kept so subclasses + // (e.g. FindAndDeleteImplementer, FindOneInterfaceProjectionImplementer) can + // override this as a polymorphic extension point and vary the query method + // by domain class/method + @SuppressWarnings('unused') protected String findMethodToInvoke(ClassNode domainClassNode, MethodNode newMethodNode) { 'find' } diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/FindOneStringQueryImplementer.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/FindOneStringQueryImplementer.groovy index 1d0887735e3..c6fbfd8b980 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/FindOneStringQueryImplementer.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/FindOneStringQueryImplementer.groovy @@ -66,6 +66,10 @@ class FindOneStringQueryImplementer extends AbstractStringQueryImplementer imple ) } + // classNode/methodNode are unused here, but kept so subclasses (e.g. + // FindOneInterfaceProjectionStringQueryImplementer) can override this as a + // polymorphic extension point and vary the query method by domain class/method + @SuppressWarnings('unused') protected String getFindMethodToInvoke(ClassNode classNode, MethodNode methodNode, ClassNode returnType) { if (AstUtils.isDomainClass(returnType)) { return 'find' diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/InterfaceProjectionBuilder.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/InterfaceProjectionBuilder.groovy index 992736c182f..0cfe93b0882 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/InterfaceProjectionBuilder.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/InterfaceProjectionBuilder.groovy @@ -60,26 +60,39 @@ trait InterfaceProjectionBuilder { @Generated boolean isInterfaceProjection(ClassNode domainClass, MethodNode methodNode, ClassNode returnType) { if (returnType.isInterface() && !returnType.packageName?.startsWith('java.')) { - List interfacePropertyNames = AstPropertyResolveUtils.getPropertyNames(returnType) + return hasCompatibleProperties(domainClass, returnType) + } + return false + } - for (prop in interfacePropertyNames) { - ClassNode existingType = AstPropertyResolveUtils.getPropertyType(domainClass, prop) - ClassNode propertyType = AstPropertyResolveUtils.getPropertyType(returnType, prop) - if (existingType == null) { - return false - } - else if (!AstUtils.isSubclassOfOrImplementsInterface(existingType, propertyType)) { - return false - } + /** + * Whether every property declared by the candidate interface has a compatible + * property of the same name on the domain class + * + * @param domainClass The domain class + * @param candidateType The interface being considered as a projection + * @return True if every property is compatible + */ + @Generated + boolean hasCompatibleProperties(ClassNode domainClass, ClassNode candidateType) { + List interfacePropertyNames = AstPropertyResolveUtils.getPropertyNames(candidateType) + + for (prop in interfacePropertyNames) { + ClassNode existingType = AstPropertyResolveUtils.getPropertyType(domainClass, prop) + ClassNode propertyType = AstPropertyResolveUtils.getPropertyType(candidateType, prop) + if (existingType == null) { + return false + } + else if (!AstUtils.isSubclassOfOrImplementsInterface(existingType, propertyType)) { + return false } - return true } - return false + return true } @Generated MethodNode buildInterfaceImpl(ClassNode interfaceNode, ClassNode declaringClass, ClassNode targetDomainClass, MethodNode abstractMethodNode) { - List getterNames = (List) AstPropertyResolveUtils.getPropertyNames(interfaceNode) + List getterNames = (List) (List) AstPropertyResolveUtils.getPropertyNames(interfaceNode) .collect() { new ConstantExpression(NameUtils.getGetterName(it)) } diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/IterableInterfaceProjectionBuilder.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/IterableInterfaceProjectionBuilder.groovy index 78bda7209a4..b7dd0453579 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/IterableInterfaceProjectionBuilder.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/IterableInterfaceProjectionBuilder.groovy @@ -31,7 +31,6 @@ import org.codehaus.groovy.ast.expr.VariableExpression import org.codehaus.groovy.ast.stmt.Statement import org.grails.datastore.gorm.services.ServiceImplementer -import org.grails.datastore.gorm.transform.AstPropertyResolveUtils import org.grails.datastore.mapping.reflect.AstGenericsUtils import org.grails.datastore.mapping.reflect.AstUtils @@ -71,20 +70,7 @@ trait IterableInterfaceProjectionBuilder extends InterfaceProjectionBuilder { if (AstUtils.isSubclassOfOrImplementsInterface(returnType, Iterable.name) || returnType.isArray()) { ClassNode genericType = AstGenericsUtils.resolveSingleGenericType(returnType) if (genericType != null && genericType.isInterface() && !genericType.packageName?.startsWith('java.')) { - - List interfacePropertyNames = AstPropertyResolveUtils.getPropertyNames(genericType) - - for (prop in interfacePropertyNames) { - ClassNode existingType = AstPropertyResolveUtils.getPropertyType(domainClass, prop) - ClassNode propertyType = AstPropertyResolveUtils.getPropertyType(genericType, prop) - if (existingType == null) { - return false - } - else if (!AstUtils.isSubclassOfOrImplementsInterface(existingType, propertyType)) { - return false - } - } - return true + return hasCompatibleProperties(domainClass, genericType) } } return false diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/SaveImplementer.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/SaveImplementer.groovy index fa35663723e..5bc6709f537 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/SaveImplementer.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/SaveImplementer.groovy @@ -84,7 +84,7 @@ class SaveImplementer extends AbstractSaveImplementer implements SingleResultSer declS(entityVar, ctorX(domainClassNode)) ) body.addStatement( - bindParametersAndSave(domainClassNode, abstractMethodNode, newMethodNode, parameters, body, entityVar) + bindParametersAndSave(domainClassNode, abstractMethodNode, parameters, body, entityVar) ) } diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/UpdateOneImplementer.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/UpdateOneImplementer.groovy index c5a563332dd..056f321c6a3 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/UpdateOneImplementer.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/implementers/UpdateOneImplementer.groovy @@ -90,7 +90,8 @@ class UpdateOneImplementer extends AbstractSaveImplementer implements SingleResu declS(entityVar, lookupCall) ) BlockStatement ifBody = block() - Statement saveStmt = bindParametersAndSave(domainClassNode, abstractMethodNode, newMethodNode, parameters[1..-1] as Parameter[], ifBody, entityVar) + Parameter[] propertyParameters = Arrays.copyOfRange(parameters, 1, parameters.length) + Statement saveStmt = bindParametersAndSave(domainClassNode, abstractMethodNode, propertyParameters, ifBody, entityVar) ifBody.addStatement(saveStmt) body.addStatement( ifS(notNullX(entityVar), diff --git a/grails-datamapping-core/src/test/groovy/grails/gorm/services/ServiceImplementerEdgeCaseSpec.groovy b/grails-datamapping-core/src/test/groovy/grails/gorm/services/ServiceImplementerEdgeCaseSpec.groovy new file mode 100644 index 00000000000..0f9433fdf2a --- /dev/null +++ b/grails-datamapping-core/src/test/groovy/grails/gorm/services/ServiceImplementerEdgeCaseSpec.groovy @@ -0,0 +1,413 @@ +/* + * 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 grails.gorm.services + +import org.codehaus.groovy.control.MultipleCompilationErrorsException +import org.grails.datastore.gorm.services.Implemented +import org.grails.datastore.gorm.services.implementers.DeleteWhereImplementer +import org.grails.datastore.gorm.services.implementers.FindAllPropertyProjectionImplementer +import org.grails.datastore.gorm.services.implementers.FindOneByImplementer +import org.grails.datastore.gorm.services.implementers.FindOneInterfaceProjectionWhereImplementer +import org.grails.datastore.gorm.services.implementers.FindOnePropertyProjectionImplementer +import org.grails.datastore.gorm.services.implementers.UpdateOneImplementer +import spock.lang.Specification + +/** + * Covers DSL shapes for {@link org.grails.datastore.gorm.services.implementers.ServiceImplementer} + * subclasses that are not otherwise exercised by {@link ServiceTransformSpec} or + * {@link WhereConnectionRoutingSpec}: update methods, {@code @Where}-annotated deletes, + * property projections and the {@code findById(id)} shortcut. + */ +class ServiceImplementerEdgeCaseSpec extends Specification { + + void 'an update method with a matching id parameter is implemented via UpdateOneImplementer'() { + when: + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity + +@Service(Foo) +interface FooService { + Foo updateFoo(Serializable id, String title) +} +@Entity +class Foo { + String title +} +''') + + then: + service.isInterface() + + when: + Class impl = service.classLoader.loadClass("\$FooServiceImplementation") + + then: + impl.getMethod('updateFoo', Serializable, String).getAnnotation(Implemented).by() == UpdateOneImplementer + } + + void 'an update method with a Map args parameter is implemented via UpdateOneImplementer'() { + when: + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity + +@Service(Foo) +interface FooService { + Foo updateFoo(Serializable id, Map args) +} +@Entity +class Foo { + String title +} +''') + + then: + service.isInterface() + + when: + Class impl = service.classLoader.loadClass("\$FooServiceImplementation") + + then: + impl.getMethod('updateFoo', Serializable, Map).getAnnotation(Implemented).by() == UpdateOneImplementer + } + + void 'an update method on a service routed to a non-default connection saves through the instance API for that connection'() { + when: + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity +import grails.gorm.transactions.Transactional + +@Service(Foo) +@Transactional(connection = 'secondary') +interface FooService { + Foo updateFoo(Serializable id, String title) +} +@Entity +class Foo { + String title + static mapping = { + datasource 'secondary' + } +} +''') + + then: + service.isInterface() + + when: + Class impl = service.classLoader.loadClass("\$FooServiceImplementation") + + then: + impl.getMethod('updateFoo', Serializable, String).getAnnotation(Implemented).by() == UpdateOneImplementer + } + + void 'an update method with a parameter that matches no domain property fails to compile'() { + when: + new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity + +@Service(Foo) +interface FooService { + Foo updateFoo(Serializable id, String notAProperty) +} +@Entity +class Foo { + String title +} +''') + + then: + MultipleCompilationErrorsException e = thrown(MultipleCompilationErrorsException) + e.message.contains('Cannot implement method for argument [notAProperty]') + } + + void 'a void @Where delete method is implemented via DeleteWhereImplementer'() { + when: + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.services.Where +import grails.gorm.annotation.Entity + +@Service(Foo) +interface FooService { + @Where({ title ==~ pattern }) + void deleteByTitle(String pattern) +} +@Entity +class Foo { + String title +} +''') + + then: + service.isInterface() + + when: + Class impl = service.classLoader.loadClass("\$FooServiceImplementation") + + then: + impl.getMethod('deleteByTitle', String).getAnnotation(Implemented).by() == DeleteWhereImplementer + } + + void 'a Number-returning @Where delete method is implemented via DeleteWhereImplementer'() { + when: + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.services.Where +import grails.gorm.annotation.Entity + +@Service(Foo) +interface FooService { + @Where({ title ==~ pattern }) + Number deleteByTitle(String pattern) +} +@Entity +class Foo { + String title +} +''') + + then: + service.isInterface() + + when: + Class impl = service.classLoader.loadClass("\$FooServiceImplementation") + + then: + impl.getMethod('deleteByTitle', String).getAnnotation(Implemented).by() == DeleteWhereImplementer + } + + void 'a @Where method returning an interface projection is implemented via FindOneInterfaceProjectionWhereImplementer'() { + when: + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.services.Where +import grails.gorm.annotation.Entity + +interface ITitle { + String getTitle() +} + +@Service(Foo) +interface FooService { + @Where({ title ==~ pattern }) + ITitle findProjectionByTitle(String pattern) +} +@Entity +class Foo { + String title +} +''') + + then: + service.isInterface() + + when: + Class impl = service.classLoader.loadClass("\$FooServiceImplementation") + + then: + impl.getMethod('findProjectionByTitle', String).getAnnotation(Implemented).by() == FindOneInterfaceProjectionWhereImplementer + } + + void 'a findAll method is implemented via FindAllPropertyProjectionImplementer'() { + when: + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity + +@Service(Foo) +interface FooService { + List findFooTitle() +} +@Entity +class Foo { + String title +} +''') + + then: + service.isInterface() + + when: + Class impl = service.classLoader.loadClass("\$FooServiceImplementation") + + then: + impl.getMethod('findFooTitle').getAnnotation(Implemented).by() == FindAllPropertyProjectionImplementer + } + + void 'a findAll method returning an array is implemented via FindAllPropertyProjectionImplementer'() { + when: + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity + +@Service(Foo) +interface FooService { + String[] findFooTitle() +} +@Entity +class Foo { + String title +} +''') + + then: + service.isInterface() + + when: + Class impl = service.classLoader.loadClass("\$FooServiceImplementation") + + then: + impl.getMethod('findFooTitle').getAnnotation(Implemented).by() == FindAllPropertyProjectionImplementer + } + + void 'a find method returning a single value is implemented via FindOnePropertyProjectionImplementer'() { + when: + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity + +@Service(Foo) +interface FooService { + String findFooTitle() +} +@Entity +class Foo { + String title +} +''') + + then: + service.isInterface() + + when: + Class impl = service.classLoader.loadClass("\$FooServiceImplementation") + + then: + impl.getMethod('findFooTitle').getAnnotation(Implemented).by() == FindOnePropertyProjectionImplementer + } + + void 'a findById(Serializable) method is implemented via a direct get(id) call in FindOneByImplementer'() { + when: + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity + +@Service(Foo) +interface FooService { + Foo findById(Serializable id) +} +@Entity +class Foo { + String title +} +''') + + then: + service.isInterface() + + when: + Class impl = service.classLoader.loadClass("\$FooServiceImplementation") + + then: + impl.getMethod('findById', Serializable).getAnnotation(Implemented).by() == FindOneByImplementer + } + + void 'a pre-existing concrete write method on an abstract class service is enhanced with a default transaction'() { + when: + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity + +@Service(Foo) +abstract class FooService { + Foo saveFoo(String title) { + Foo f = new Foo(title: title) + f.save(failOnError: true) + return f + } +} +@Entity +class Foo { + String title +} +''') + + then: + !service.isInterface() + + when: + Class impl = service.classLoader.loadClass("\$FooServiceImplementation") + + then: + impl.getMethod('saveFoo', String).getAnnotation(grails.gorm.transactions.Transactional) != null + } + + void 'a @Where delete method with an incompatible return type fails to compile'() { + when: + new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.services.Where +import grails.gorm.annotation.Entity + +@Service(Foo) +interface FooService { + @Where({ title ==~ pattern }) + String deleteByTitle(String pattern) +} +@Entity +class Foo { + String title +} +''') + + then: + MultipleCompilationErrorsException e = thrown(MultipleCompilationErrorsException) + e.message.contains('No implementations possible') + } + + void 'a save method with an id-named parameter binds it directly onto the entity'() { + when: + Class service = new GroovyClassLoader().parseClass(''' +import grails.gorm.services.Service +import grails.gorm.annotation.Entity + +@Service(Foo) +interface FooService { + Foo save(Serializable id, String title) +} +@Entity +class Foo { + String title +} +''') + + then: + service.isInterface() + + when: + Class impl = service.classLoader.loadClass("\$FooServiceImplementation") + + then: + impl.getMethod('save', Serializable, String).getAnnotation(Implemented).by() == org.grails.datastore.gorm.services.implementers.SaveImplementer + } +} diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/DefaultTenantServiceSpec.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/DefaultTenantServiceSpec.groovy new file mode 100644 index 00000000000..65f39221b26 --- /dev/null +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/DefaultTenantServiceSpec.groovy @@ -0,0 +1,205 @@ +/* + * 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 + +import org.grails.datastore.mapping.core.Datastore +import org.grails.datastore.mapping.model.DatastoreConfigurationException +import org.grails.datastore.mapping.multitenancy.AllTenantsResolver +import org.grails.datastore.mapping.multitenancy.MultiTenancySettings +import org.grails.datastore.mapping.multitenancy.MultiTenantCapableDatastore +import org.grails.datastore.mapping.multitenancy.TenantResolver +import spock.lang.Specification + +class DefaultTenantServiceSpec extends Specification { + + DefaultTenantService tenantService = new DefaultTenantService() + + void 'multiTenantDatastore throws when the datastore is not multi-tenant capable'() { + given: + tenantService.datastore = Mock(Datastore) + + when: + tenantService.eachTenant {} + + then: + DatastoreConfigurationException e = thrown(DatastoreConfigurationException) + e.message.contains('not Multi-Tenant capable') + } + + void 'currentId throws when multi tenancy mode is NONE'() { + given: + MultiTenantCapableDatastore datastore = Mock(MultiTenantCapableDatastore) + datastore.multiTenancyMode >> MultiTenancySettings.MultiTenancyMode.NONE + tenantService.datastore = datastore + + when: + tenantService.currentId() + + then: + DatastoreConfigurationException e = thrown(DatastoreConfigurationException) + e.message.contains('not configured for Multi-Tenancy') + } + + void 'currentId resolves the tenant id from the tenant resolver'() { + given: + TenantResolver tenantResolver = Mock(TenantResolver) { + resolveTenantIdentifier() >> 'tenant1' + } + MultiTenantCapableDatastore datastore = Mock(MultiTenantCapableDatastore) + datastore.multiTenancyMode >> MultiTenancySettings.MultiTenancyMode.DISCRIMINATOR + datastore.tenantResolver >> tenantResolver + tenantService.datastore = datastore + + expect: + tenantService.currentId() == 'tenant1' + } + + void 'withoutId throws when multi tenancy mode is NONE'() { + given: + MultiTenantCapableDatastore datastore = Mock(MultiTenantCapableDatastore) + datastore.multiTenancyMode >> MultiTenancySettings.MultiTenancyMode.NONE + tenantService.datastore = datastore + + when: + tenantService.withoutId { 'result' } + + then: + thrown(DatastoreConfigurationException) + } + + void 'withoutId with a shared connection mode executes the callable without a session'() { + given: + MultiTenantCapableDatastore datastore = Mock(MultiTenantCapableDatastore) + datastore.multiTenancyMode >> MultiTenancySettings.MultiTenancyMode.DISCRIMINATOR + tenantService.datastore = datastore + + when: + String result = tenantService.withoutId { -> 'result' } + + then: + result == 'result' + 0 * datastore.withSession(_) + 0 * datastore.withNewSession(_, _) + } + + void 'withoutId with a non-shared connection mode executes within a new session'() { + given: + MultiTenantCapableDatastore datastore = Mock(MultiTenantCapableDatastore) + datastore.multiTenancyMode >> MultiTenancySettings.MultiTenancyMode.DATABASE + datastore.withNewSession(_, _) >> { args -> args[1].call('session') } + tenantService.datastore = datastore + + expect: + tenantService.withoutId { 'result' } == 'result' + } + + void 'withId throws when multi tenancy mode is NONE'() { + given: + MultiTenantCapableDatastore datastore = Mock(MultiTenantCapableDatastore) + datastore.multiTenancyMode >> MultiTenancySettings.MultiTenancyMode.NONE + tenantService.datastore = datastore + + when: + tenantService.withId('tenant1') { 'result' } + + then: + thrown(DatastoreConfigurationException) + } + + void 'withId with a shared connection mode executes the callable directly'() { + given: + MultiTenantCapableDatastore datastore = Mock(MultiTenantCapableDatastore) + datastore.multiTenancyMode >> MultiTenancySettings.MultiTenancyMode.SCHEMA + tenantService.datastore = datastore + + expect: + tenantService.withId('tenant1') { tenantId -> "result-$tenantId" } == 'result-tenant1' + } + + void 'withId with a non-shared connection mode executes within a new session for the tenant'() { + given: + MultiTenantCapableDatastore datastore = Mock(MultiTenantCapableDatastore) + datastore.multiTenancyMode >> MultiTenancySettings.MultiTenancyMode.DATABASE + datastore.withNewSession('tenant1', _) >> { args -> args[1].call('session') } + tenantService.datastore = datastore + + expect: + tenantService.withId('tenant1') { 'result' } == 'result' + } + + void 'withCurrent throws when multi tenancy mode is NONE'() { + given: + MultiTenantCapableDatastore datastore = Mock(MultiTenantCapableDatastore) + datastore.multiTenancyMode >> MultiTenancySettings.MultiTenancyMode.NONE + tenantService.datastore = datastore + + when: + tenantService.withCurrent { 'result' } + + then: + thrown(DatastoreConfigurationException) + } + + void 'withCurrent executes the callable with the resolved current tenant id'() { + given: + TenantResolver tenantResolver = Mock(TenantResolver) { + resolveTenantIdentifier() >> 'tenant1' + } + MultiTenantCapableDatastore datastore = Mock(MultiTenantCapableDatastore) + datastore.multiTenancyMode >> MultiTenancySettings.MultiTenancyMode.SCHEMA + datastore.tenantResolver >> tenantResolver + tenantService.datastore = datastore + + expect: + tenantService.withCurrent { tenantId -> "result-$tenantId" } == 'result-tenant1' + } + + void 'eachTenant throws when multi tenancy mode is NONE'() { + given: + MultiTenantCapableDatastore datastore = Mock(MultiTenantCapableDatastore) + datastore.multiTenancyMode >> MultiTenancySettings.MultiTenancyMode.NONE + tenantService.datastore = datastore + + when: + tenantService.eachTenant {} + + then: + thrown(UnsupportedOperationException) + } + + void 'eachTenant with a shared connection mode invokes the callable for every resolved tenant id'() { + given: + AllTenantsResolver tenantResolver = Mock(AllTenantsResolver) { + resolveTenantIds() >> ['tenant1', 'tenant2'] + } + MultiTenantCapableDatastore datastore = Mock(MultiTenantCapableDatastore) + datastore.multiTenancyMode >> MultiTenancySettings.MultiTenancyMode.SCHEMA + datastore.tenantResolver >> tenantResolver + tenantService.datastore = datastore + + and: + List seen = [] + + when: + tenantService.eachTenant { tenantId -> seen << tenantId } + + then: + seen == ['tenant1', 'tenant2'] + } +} diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/DefaultTransactionServiceSpec.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/DefaultTransactionServiceSpec.groovy new file mode 100644 index 00000000000..99be3d643e7 --- /dev/null +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/services/DefaultTransactionServiceSpec.groovy @@ -0,0 +1,186 @@ +/* + * 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 + +import org.springframework.transaction.PlatformTransactionManager +import org.springframework.transaction.TransactionDefinition +import org.springframework.transaction.TransactionStatus +import org.springframework.transaction.TransactionSystemException + +import org.grails.datastore.mapping.core.Datastore +import org.grails.datastore.mapping.transactions.CustomizableRollbackTransactionAttribute +import org.grails.datastore.mapping.transactions.TransactionCapableDatastore +import spock.lang.Specification + +class DefaultTransactionServiceSpec extends Specification { + + DefaultTransactionService transactionService = new DefaultTransactionService() + + PlatformTransactionManager transactionManager = Mock(PlatformTransactionManager) { + getTransaction(_) >> Mock(TransactionStatus) + } + + TransactionCapableDatastore transactionCapableDatastore = Mock(TransactionCapableDatastore) { + getTransactionManager() >> transactionManager + } + + void 'withTransaction(Closure) executes the callable within a transaction'() { + given: + transactionService.datastore = transactionCapableDatastore + + when: + String result = transactionService.withTransaction { status -> 'done' } + + then: + result == 'done' + 1 * transactionManager.commit(_) + } + + void 'withTransaction(Closure) throws when the datastore does not support transactions'() { + given: + transactionService.datastore = Mock(Datastore) + + when: + transactionService.withTransaction { status -> 'done' } + + then: + TransactionSystemException e = thrown(TransactionSystemException) + e.message.contains('does not support transactions') + } + + void 'withRollback(Closure) executes the callable and rolls back'() { + given: + transactionService.datastore = transactionCapableDatastore + + expect: + transactionService.withRollback { status -> 'done' } == 'done' + } + + void 'withRollback(Closure) throws when the datastore does not support transactions'() { + given: + transactionService.datastore = Mock(Datastore) + + when: + transactionService.withRollback { status -> 'done' } + + then: + thrown(TransactionSystemException) + } + + void 'withNewTransaction(Closure) executes the callable with PROPAGATION_REQUIRES_NEW'() { + given: + transactionService.datastore = transactionCapableDatastore + + expect: + transactionService.withNewTransaction { status -> 'done' } == 'done' + } + + void 'withNewTransaction(Closure) throws when the datastore does not support transactions'() { + given: + transactionService.datastore = Mock(Datastore) + + when: + transactionService.withNewTransaction { status -> 'done' } + + then: + thrown(TransactionSystemException) + } + + void 'withTransaction(TransactionDefinition, Closure) executes the callable with the given definition'() { + given: + transactionService.datastore = transactionCapableDatastore + TransactionDefinition definition = new CustomizableRollbackTransactionAttribute() + + expect: + transactionService.withTransaction(definition) { status -> 'done' } == 'done' + } + + void 'withTransaction(TransactionDefinition, Closure) throws when the datastore does not support transactions'() { + given: + transactionService.datastore = Mock(Datastore) + TransactionDefinition definition = new CustomizableRollbackTransactionAttribute() + + when: + transactionService.withTransaction(definition) { status -> 'done' } + + then: + thrown(TransactionSystemException) + } + + void 'withTransaction(Map, Closure) builds a transaction definition from the map'() { + given: + transactionService.datastore = transactionCapableDatastore + + expect: + transactionService.withTransaction([readOnly: true]) { status -> 'done' } == 'done' + } + + void 'withTransaction(Map, Closure) throws when the datastore does not support transactions'() { + given: + transactionService.datastore = Mock(Datastore) + + when: + transactionService.withTransaction([readOnly: true]) { status -> 'done' } + + then: + thrown(TransactionSystemException) + } + + void 'withRollback(TransactionDefinition, Closure) executes the callable and rolls back'() { + given: + transactionService.datastore = transactionCapableDatastore + TransactionDefinition definition = new CustomizableRollbackTransactionAttribute() + + expect: + transactionService.withRollback(definition) { status -> 'done' } == 'done' + } + + void 'withRollback(TransactionDefinition, Closure) throws when the datastore does not support transactions'() { + given: + transactionService.datastore = Mock(Datastore) + TransactionDefinition definition = new CustomizableRollbackTransactionAttribute() + + when: + transactionService.withRollback(definition) { status -> 'done' } + + then: + thrown(TransactionSystemException) + } + + void 'withNewTransaction(TransactionDefinition, Closure) forces PROPAGATION_REQUIRES_NEW'() { + given: + transactionService.datastore = transactionCapableDatastore + TransactionDefinition definition = new CustomizableRollbackTransactionAttribute() + + expect: + transactionService.withNewTransaction(definition) { status -> 'done' } == 'done' + } + + void 'withNewTransaction(TransactionDefinition, Closure) throws when the datastore does not support transactions'() { + given: + transactionService.datastore = Mock(Datastore) + TransactionDefinition definition = new CustomizableRollbackTransactionAttribute() + + when: + transactionService.withNewTransaction(definition) { status -> 'done' } + + then: + thrown(TransactionSystemException) + } +}