jamesfredley commented on code in PR #15433: URL: https://github.com/apache/grails-core/pull/15433#discussion_r2836829654
########## grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/connections/DataServiceDatasourceInheritanceSpec.groovy: ########## @@ -0,0 +1,215 @@ +/* + * 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.orm.hibernate.connections + +import org.hibernate.dialect.H2Dialect +import spock.lang.AutoCleanup +import spock.lang.Shared +import spock.lang.Specification + +import grails.gorm.annotation.Entity +import grails.gorm.services.Service +import grails.gorm.transactions.Transactional +import org.grails.datastore.gorm.GormEnhancer +import org.grails.datastore.mapping.core.DatastoreUtils +import org.grails.orm.hibernate.HibernateDatastore + +class DataServiceDatasourceInheritanceSpec extends Specification { + + @Shared Map config = [ + 'dataSource.url':"jdbc:h2:mem:grailsDB;LOCK_TIMEOUT=10000", + 'dataSource.dbCreate': 'create-drop', + 'dataSource.dialect': H2Dialect.name, + 'dataSource.formatSql': 'true', + 'hibernate.flush.mode': 'COMMIT', + 'hibernate.cache.queries': 'true', + 'hibernate.hbm2ddl.auto': 'create-drop', + 'dataSources.warehouse':[url:"jdbc:h2:mem:warehouseDB;LOCK_TIMEOUT=10000"], + ] + + @Shared @AutoCleanup HibernateDatastore datastore = new HibernateDatastore( + DatastoreUtils.createPropertyResolver(config), Inventory + ) + + @Shared InventoryService inventoryService + @Shared InventoryDataService inventoryDataService + @Shared ExplicitInventoryService explicitInventoryService + + void setupSpec() { + inventoryService = datastore + .getDatastoreForConnection('warehouse') + .getService(InventoryService) + inventoryDataService = datastore + .getDatastoreForConnection('warehouse') + .getService(InventoryDataService) + explicitInventoryService = datastore + .getDatastoreForConnection('warehouse') + .getService(ExplicitInventoryService) + } Review Comment: Fixed. Added `defaultDatastoreInventoryService = datastore.getService(InventoryService)` and a new test `"service obtained from default datastore still routes to inherited datasource"` that exercises the AST-generated connection routing from the default datastore. ########## grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/connections/DataServiceDatasourceInheritanceSpec.groovy: ########## @@ -0,0 +1,215 @@ +/* + * 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.orm.hibernate.connections + +import org.hibernate.dialect.H2Dialect +import spock.lang.AutoCleanup +import spock.lang.Shared +import spock.lang.Specification + +import grails.gorm.annotation.Entity +import grails.gorm.services.Service +import grails.gorm.transactions.Transactional +import org.grails.datastore.gorm.GormEnhancer +import org.grails.datastore.mapping.core.DatastoreUtils +import org.grails.orm.hibernate.HibernateDatastore + +class DataServiceDatasourceInheritanceSpec extends Specification { + + @Shared Map config = [ + 'dataSource.url':"jdbc:h2:mem:grailsDB;LOCK_TIMEOUT=10000", + 'dataSource.dbCreate': 'create-drop', + 'dataSource.dialect': H2Dialect.name, + 'dataSource.formatSql': 'true', + 'hibernate.flush.mode': 'COMMIT', + 'hibernate.cache.queries': 'true', + 'hibernate.hbm2ddl.auto': 'create-drop', + 'dataSources.warehouse':[url:"jdbc:h2:mem:warehouseDB;LOCK_TIMEOUT=10000"], + ] + + @Shared @AutoCleanup HibernateDatastore datastore = new HibernateDatastore( + DatastoreUtils.createPropertyResolver(config), Inventory + ) + + @Shared InventoryService inventoryService + @Shared InventoryDataService inventoryDataService + @Shared ExplicitInventoryService explicitInventoryService + + void setupSpec() { + inventoryService = datastore + .getDatastoreForConnection('warehouse') + .getService(InventoryService) + inventoryDataService = datastore + .getDatastoreForConnection('warehouse') + .getService(InventoryDataService) + explicitInventoryService = datastore + .getDatastoreForConnection('warehouse') + .getService(ExplicitInventoryService) + } + + void setup() { + def api = GormEnhancer.findStaticApi(Inventory, 'warehouse') + api.withNewTransaction { + api.executeUpdate('delete from Inventory') + } + } + + void "abstract service without @Transactional(connection) inherits from domain"() { + when: "saving through a service that has no @Transactional(connection)" + def saved = inventoryService.save(new Inventory(sku: 'ABC-001', quantity: 50)) + + then: "the entity is persisted on the warehouse datasource" + saved != null + saved.id != null + saved.sku == 'ABC-001' + + and: "it exists on the warehouse datasource" + GormEnhancer.findStaticApi(Inventory, 'warehouse').withNewTransaction { + GormEnhancer.findStaticApi(Inventory, 'warehouse').count() + } == 1 + } + + void "get by ID routes to inherited datasource"() { + given: "an inventory item saved on warehouse" + def saved = inventoryService.save(new Inventory(sku: 'GET-001', quantity: 10)) + + when: "retrieving by ID" + def found = inventoryService.get(saved.id) + + then: "the correct entity is returned" + found != null + found.id == saved.id + found.sku == 'GET-001' + } + + void "delete routes to inherited datasource"() { + given: "an inventory item saved on warehouse" + def saved = inventoryService.save(new Inventory(sku: 'DEL-001', quantity: 5)) + + when: "deleting by ID" + def deleted = inventoryService.delete(saved.id) + + then: "the entity is deleted" + deleted != null + deleted.sku == 'DEL-001' + inventoryService.get(saved.id) == null + } + + void "count routes to inherited datasource"() { + given: "items saved on warehouse" + inventoryService.save(new Inventory(sku: 'CNT-001', quantity: 1)) + inventoryService.save(new Inventory(sku: 'CNT-002', quantity: 2)) + + expect: "count returns 2" + inventoryService.count() == 2 + } + + void "findBySku routes to inherited datasource"() { + given: "items saved on warehouse" + inventoryService.save(new Inventory(sku: 'FIND-001', quantity: 100)) + + when: "finding by sku" + def found = inventoryService.findBySku('FIND-001') + + then: "the correct entity is returned" + found != null + found.sku == 'FIND-001' + found.quantity == 100 + } + + void "interface service inherits datasource from domain"() { + when: "saving through an interface service with no @Transactional(connection)" + def saved = inventoryDataService.save(new Inventory(sku: 'IFACE-001', quantity: 25)) + + then: "the entity is persisted on warehouse" + saved != null + saved.id != null + + and: "retrievable through the same service" + inventoryDataService.get(saved.id) != null + } + + void "explicit @Transactional(connection) wins over domain datasource"() { + when: "saving through a service with explicit @Transactional(connection='warehouse')" + def saved = explicitInventoryService.save(new Inventory(sku: 'EXPL-001', quantity: 75)) + + then: "the entity is persisted correctly" + saved != null + saved.id != null + saved.sku == 'EXPL-001' + } Review Comment: Fixed. Replaced `ExplicitInventoryService` (which used the same `warehouse` connection as the domain) with `ExplicitArchiveInventoryService` using `@Transactional(connection = 'archive')`. The test now verifies that the explicit annotation value (`archive`) is preserved and differs from the domain mapping (`warehouse`). ########## grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/DatastoreServiceMethodInvokingFactoryBean.groovy: ########## @@ -56,14 +62,53 @@ class DatastoreServiceMethodInvokingFactoryBean extends MethodInvokingFactoryBea protected Object invokeWithTargetException() throws Exception { Object object = super.invokeWithTargetException() if (object) { - ((Service) object).setDatastore((Datastore) targetObject) + Datastore effectiveDatastore = resolveEffectiveDatastore((Datastore) targetObject) + ((Service) object).setDatastore(effectiveDatastore) if (beanFactory instanceof AutowireCapableBeanFactory) { ((AutowireCapableBeanFactory) beanFactory).autowireBeanProperties(object, AutowireCapableBeanFactory.AUTOWIRE_BY_TYPE, false) } } object } + private Datastore resolveEffectiveDatastore(Datastore defaultDatastore) { + if (!(defaultDatastore instanceof MultipleConnectionSourceCapableDatastore)) { + return defaultDatastore + } + + Class<?> domainClass = getServiceDomainClass() + if (domainClass == null || domainClass == Object) { + return defaultDatastore + } + + PersistentEntity entity = defaultDatastore.getMappingContext()?.getPersistentEntity(domainClass.getName()) + if (entity == null) { + return defaultDatastore + } + + String domainConnection = ConnectionSourcesSupport.getDefaultConnectionSourceName(entity) + if (domainConnection != null + && !ConnectionSource.DEFAULT.equals(domainConnection) + && !ConnectionSource.ALL.equals(domainConnection)) { + return ((MultipleConnectionSourceCapableDatastore) defaultDatastore).getDatastoreForConnection(domainConnection) + } Review Comment: Good catch - fixed. `resolveEffectiveDatastore()` now checks the service's `@Transactional(connection)` first via `getServiceTransactionalConnection()`. Only falls back to domain mapping if the service has no explicit connection set. This ensures the documented precedence rule holds at runtime too. ########## grails-test-examples/hibernate5/grails-data-service-multi-datasource/src/integration-test/groovy/functionaltests/DataServiceDatasourceInheritanceSpec.groovy: ########## @@ -0,0 +1,121 @@ +/* + * 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 functionaltests + +import example.Product +import example.InheritedProductService + +import org.springframework.beans.factory.annotation.Autowired + +import grails.testing.mixin.integration.Integration +import org.grails.orm.hibernate.HibernateDatastore +import spock.lang.Specification + +@Integration +class DataServiceDatasourceInheritanceSpec extends Specification { + + @Autowired + HibernateDatastore hibernateDatastore + + InheritedProductService inheritedProductService + + void setup() { + inheritedProductService = hibernateDatastore + .getDatastoreForConnection('secondary') + .getService(InheritedProductService) + } Review Comment: Fixed. Replaced manual `hibernateDatastore.getDatastoreForConnection('secondary').getService(...)` with `@Autowired InheritedProductService` so the test exercises the Spring-managed bean created by `DatastoreServiceMethodInvokingFactoryBean`. ########## grails-doc/src/en/guide/conf/dataSource/multipleDatasources.adoc: ########## @@ -260,6 +260,50 @@ Note that the datasource specified in a service has no bearing on which datasour If you have a `Foo` domain class in `dataSource1` and a `Bar` domain class in `dataSource2`, if `WahooService` uses `dataSource1`, a service method that saves a new `Foo` and a new `Bar` will only be transactional for `Foo` since they share the same datasource. The transaction won't affect the `Bar` instance. If you want both to be transactional you'd need to use two services and XA datasources for two-phase commit, e.g. with the Atomikos plugin. +===== GORM Data Service Datasource Inheritance + +When using GORM Data Services (the `@Service` annotation), the service automatically inherits the datasource from its domain class's `mapping` block. This means you do not need to explicitly specify `@Transactional(connection = '...')` if the domain class already declares its datasource. + +For example, given a domain class mapped to the `'lookup'` datasource: + +[source,groovy] +---- +class ZipCode { + + String code + + static mapping = { + datasource 'lookup' + } +} +---- + +A GORM Data Service targeting this domain class will automatically route all auto-implemented operations (save, get, delete, find, count) to the `'lookup'` datasource: + +[source,groovy] +---- +@Service(ZipCode) +abstract class ZipCodeService { + + abstract ZipCode get(Serializable id) + abstract ZipCode save(ZipCode zipCode) + abstract ZipCode delete(Serializable id) + abstract List<ZipCode> findByCode(String code) Review Comment: Fixed. Changed `findByCode` to `findAllByCode` to match the `List<ZipCode>` return type. -- 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]
