You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "jamesfredley (via GitHub)" <gi...@apache.org> on 2026/02/28 23:05:18 UTC
[PR] Merge 7.1.x into 8.0.x [grails-core]
jamesfredley opened a new pull request, #15472:
URL: https://github.com/apache/grails-core/pull/15472
(no comment)
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Merge 7.1.x into 8.0.x [grails-core]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #15472:
URL: https://github.com/apache/grails-core/pull/15472#discussion_r2868002387
##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/DatastoreServiceMethodInvokingFactoryBean.groovy:
##########
@@ -54,16 +58,86 @@ class DatastoreServiceMethodInvokingFactoryBean extends MethodInvokingFactoryBea
@Override
protected Object invokeWithTargetException() throws Exception {
- Object object = super.invokeWithTargetException()
+ def object = super.invokeWithTargetException()
if (object) {
- ((Service) object).setDatastore((Datastore) targetObject)
+ def effectiveDatastore = resolveEffectiveDatastore((Datastore) targetObject)
+ ((Service) object).datastore = effectiveDatastore
if (beanFactory instanceof AutowireCapableBeanFactory) {
- ((AutowireCapableBeanFactory) beanFactory).autowireBeanProperties(object, AutowireCapableBeanFactory.AUTOWIRE_BY_TYPE, false)
+ ((AutowireCapableBeanFactory) beanFactory).autowireBeanProperties(
+ object,
+ AutowireCapableBeanFactory.AUTOWIRE_BY_TYPE,
+ false
+ )
}
}
object
}
+ private Datastore resolveEffectiveDatastore(Datastore defaultDatastore) {
+ if (!(defaultDatastore instanceof MultipleConnectionSourceCapableDatastore)) {
+ return defaultDatastore
+ }
+
+ // 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)
+ }
+
+ // Fall back to domain class mapping datasource
+ def domainClass = serviceDomainClass
+ if (domainClass == null || domainClass == Object) {
+ return defaultDatastore
+ }
+
+ def entity = defaultDatastore.mappingContext?.getPersistentEntity(domainClass.name)
+ if (entity == null) {
+ return defaultDatastore
+ }
+
+ def domainConnection = ConnectionSourcesSupport.getDefaultConnectionSourceName(entity)
+ if (domainConnection != null
+ && ConnectionSource.DEFAULT != domainConnection
+ && ConnectionSource.ALL != domainConnection) {
+ return ((MultipleConnectionSourceCapableDatastore) defaultDatastore)
+ .getDatastoreForConnection(domainConnection)
+ }
Review 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.
##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/DatastoreServiceMethodInvokingFactoryBean.groovy:
##########
@@ -29,10 +28,15 @@ import org.springframework.beans.factory.config.MethodInvokingFactoryBean
import org.springframework.lang.Nullable
import org.grails.datastore.mapping.core.Datastore
+import org.grails.datastore.mapping.core.connections.ConnectionSource
+import org.grails.datastore.mapping.core.connections.ConnectionSourcesSupport
+import org.grails.datastore.mapping.core.connections.MultipleConnectionSourceCapableDatastore
import org.grails.datastore.mapping.services.Service
/**
- * 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
Review 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.
```suggestion
* Variant of {@link MethodInvokingFactoryBean} which returns the correct
```
##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/DatastoreServiceMethodInvokingFactoryBean.groovy:
##########
@@ -54,16 +58,86 @@ class DatastoreServiceMethodInvokingFactoryBean extends MethodInvokingFactoryBea
@Override
protected Object invokeWithTargetException() throws Exception {
- Object object = super.invokeWithTargetException()
+ def object = super.invokeWithTargetException()
if (object) {
- ((Service) object).setDatastore((Datastore) targetObject)
+ def effectiveDatastore = resolveEffectiveDatastore((Datastore) targetObject)
+ ((Service) object).datastore = effectiveDatastore
if (beanFactory instanceof AutowireCapableBeanFactory) {
- ((AutowireCapableBeanFactory) beanFactory).autowireBeanProperties(object, AutowireCapableBeanFactory.AUTOWIRE_BY_TYPE, false)
+ ((AutowireCapableBeanFactory) beanFactory).autowireBeanProperties(
+ object,
+ AutowireCapableBeanFactory.AUTOWIRE_BY_TYPE,
+ false
+ )
}
}
object
}
+ private Datastore resolveEffectiveDatastore(Datastore defaultDatastore) {
+ if (!(defaultDatastore instanceof MultipleConnectionSourceCapableDatastore)) {
+ return defaultDatastore
+ }
+
+ // 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)
+ }
Review 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 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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Merge 7.1.x into 8.0.x [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley merged PR #15472:
URL: https://github.com/apache/grails-core/pull/15472
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org