Merged
Conversation
…ices @service(DomainClass) now automatically inherits the datasource from the domain class's mapping block, so developers no longer need to specify @transactional(connection = '...') when the domain already declares its datasource. Explicit @transactional(connection) on the service still takes precedence. Two-layer implementation: - AST (ServiceTransformation): parses the domain's static mapping closure for datasource/connection calls and propagates to the generated service impl via @transactional(connection) - Runtime (DatastoreServiceMethodInvokingFactoryBean): resolves the domain's datasource from the mapping context and sets the correct datastore on the service instance Assisted-by: Claude Code <Claude@Claude.ai>
…tasource-inheritance
- Fix runtime precedence: resolveEffectiveDatastore() now checks service @transactional(connection) before falling back to domain mapping - Fix functional test to use @Autowired instead of manual service retrieval - Add integration test for service obtained from default datastore - Improve explicit-wins test to verify annotation preservation with different connection values (archive vs warehouse) - Fix docs: findByCode -> findAllByCode for List return type Assisted-by: Claude Code <Claude@Claude.ai>
…guide Assisted-by: Claude Code <Claude@Claude.ai>
…sformation for inherited datasource services When a @service data service auto-inherits its datasource from the domain class mapping, ServiceTransformation creates a synthetic impl class and adds @transactional(connection='secondary') to it. However, TransactionalTransform runs in SEMANTIC_ANALYSIS phase before the impl class exists, so it never weaves getTransactionManager() with the correct connection-aware logic onto that impl. At runtime this caused 'No Session found for current thread' because the default transaction manager was used instead of the secondary one. Fix: ServiceTransformation now directly generates the getTransactionManager() method on the impl class in applyDomainConnectionToService(), mirroring exactly what TransactionalTransform would have done for Service-implementing classes with a connection attribute. Assisted-by: Claude Code <Claude@Claude.ai>
… classpath pollution Override loadDataServices() via an anonymous subclass (makeInitializer factory) so that SoftServiceLoader no longer picks up data services (e.g. TestService with @transactional(connection = "books")) when running single-connection MongoDB initializer tests. This avoids the need to change any production code. Reverts the try-catch approach in DatastoreServiceMethodInvokingFactoryBean in favor of test-level isolation, matching the pattern established in PR #15429. Assisted-by: OpenCode <opencode@opencode.ai>
…rnal API in test Replace GormEnhancer.findStaticApi(Inventory, 'warehouse') calls with the idiomatic Inventory.warehouse namespace syntax in the unit test. This aligns with the functional test in the same PR which already uses the public API pattern (Product.secondary), and avoids testing through internal APIs. Assisted-by: Claude Code <Claude@Claude.ai>
…tasource-inheritance # Conflicts: # grails-doc/src/en/guide/conf/dataSource/multipleDatasources.adoc
…heritance feat: auto-inherit datasource from domain class in @service data services
Contributor
There was a problem hiding this comment.
Pull request overview
This PR merges 7.1.x into 8.0.x and, as part of that, updates GORM Data Services to automatically inherit datasource routing from the target domain class mapping when no explicit @Transactional(connection=...) is present, with accompanying tests and documentation updates.
Changes:
- Add datasource/connection inheritance for
@Service(Domain)data services at compile-time (AST transform) and at runtime when resolving the effective datastore. - Add new unit/integration tests covering inherited routing and explicit-override precedence.
- Update multi-datasource documentation to describe the new inheritance behavior and adjust prior guidance.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| grails-test-examples/hibernate5/grails-data-service-multi-datasource/src/integration-test/groovy/functionaltests/DataServiceDatasourceInheritanceSpec.groovy | New integration spec asserting data service methods route correctly via inherited datasource. |
| grails-test-examples/hibernate5/grails-data-service-multi-datasource/grails-app/services/example/InheritedProductService.groovy | New data service without explicit @Transactional(connection) to exercise inheritance. |
| grails-doc/src/en/guide/services/declarativeTransactions/transactionsMultiDataSource.adoc | Adds a note explaining datasource inheritance for GORM Data Services. |
| grails-doc/src/en/guide/conf/dataSource/multipleDatasources.adoc | Adds a dedicated inheritance section and updates earlier statements about required @Transactional(connection). |
| grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/DatastoreServiceMethodInvokingFactoryBean.groovy | Resolves and injects an “effective” datastore based on service/domain connection routing. |
| grails-datamapping-core/src/test/groovy/grails/gorm/services/ConnectionRoutingServiceTransformSpec.groovy | Adds AST-level tests for inherited connection annotation behavior and precedence rules. |
| grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/services/transform/ServiceTransformation.groovy | Extends the AST transform to infer domain connection from mapping and weave connection-aware tx manager logic. |
| grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/connections/DataServiceDatasourceInheritanceSpec.groovy | New Hibernate integration test covering inherited routing and default-datastore service retrieval. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+101
to
+107
| def domainConnection = ConnectionSourcesSupport.getDefaultConnectionSourceName(entity) | ||
| if (domainConnection != null | ||
| && ConnectionSource.DEFAULT != domainConnection | ||
| && ConnectionSource.ALL != domainConnection) { | ||
| return ((MultipleConnectionSourceCapableDatastore) defaultDatastore) | ||
| .getDatastoreForConnection(domainConnection) | ||
| } |
There was a problem hiding this comment.
Same concern for the domain-mapping path: getDatastoreForConnection(domainConnection) can be null depending on datastore implementation. It would be safer to explicitly handle null (throwing a configuration error that includes the missing connection name and service type) instead of silently setting a null datastore.
|
|
||
| /** | ||
| * Variant of {#link MethodInvokingFactoryBean} which returns the correct data service type instead of {@code java.lang.Object} so the Autowire with type works correctly. | ||
| * Variant of {#link MethodInvokingFactoryBean} which returns the correct |
There was a problem hiding this comment.
Groovydoc/Javadoc link tag is malformed: {#link MethodInvokingFactoryBean} should be {@link MethodInvokingFactoryBean} (and similarly keep the tag on a single token) so the generated docs render correctly.
Suggested change
| * Variant of {#link MethodInvokingFactoryBean} which returns the correct | |
| * Variant of {@link MethodInvokingFactoryBean} which returns the correct |
Comment on lines
+81
to
+88
| // Check for explicit @Transactional(connection=...) on the service first - it takes precedence | ||
| String serviceConnection = getServiceTransactionalConnection() | ||
| if (serviceConnection != null | ||
| && ConnectionSource.DEFAULT != serviceConnection | ||
| && ConnectionSource.ALL != serviceConnection) { | ||
| return ((MultipleConnectionSourceCapableDatastore) defaultDatastore) | ||
| .getDatastoreForConnection(serviceConnection) | ||
| } |
There was a problem hiding this comment.
getDatastoreForConnection(serviceConnection) may return null for some MultipleConnectionSourceCapableDatastore implementations (e.g. Neo4j returns datastoresByConnectionSource.get(...) without validation). Consider guarding against null and throwing a clear configuration exception (or falling back to defaultDatastore) so the service doesn’t end up with a null datastore reference.
This was referenced Feb 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.