jamesfredley commented on code in PR #15474:
URL: https://github.com/apache/grails-core/pull/15474#discussion_r2868039889
##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/DatastoreServiceMethodInvokingFactoryBean.groovy:
##########
@@ -83,8 +84,16 @@ class DatastoreServiceMethodInvokingFactoryBean extends
MethodInvokingFactoryBea
if (serviceConnection != null
&& ConnectionSource.DEFAULT != serviceConnection
&& ConnectionSource.ALL != serviceConnection) {
- return ((MultipleConnectionSourceCapableDatastore)
defaultDatastore)
+ Datastore resolved = ((MultipleConnectionSourceCapableDatastore)
defaultDatastore)
.getDatastoreForConnection(serviceConnection)
+ if (resolved == null) {
+ throw new ConfigurationException(
+ "DataSource not found for connection name
[${serviceConnection}] " +
+ "specified via @Transactional on service
[${serviceClass.name}]. " +
+ 'Please check your multiple data sources configuration
and try again.'
Review Comment:
The existing `HibernateDatastore.getDatastoreForConnection()` already uses
the same "DataSource not found" wording in its own `ConfigurationException`.
Keeping our messages consistent with that convention. A terminology change
would be a broader codebase concern, not something to address in this fix.
##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/DatastoreServiceMethodInvokingFactoryBean.groovy:
##########
@@ -83,8 +84,16 @@ class DatastoreServiceMethodInvokingFactoryBean extends
MethodInvokingFactoryBea
if (serviceConnection != null
&& ConnectionSource.DEFAULT != serviceConnection
&& ConnectionSource.ALL != serviceConnection) {
- return ((MultipleConnectionSourceCapableDatastore)
defaultDatastore)
+ Datastore resolved = ((MultipleConnectionSourceCapableDatastore)
defaultDatastore)
.getDatastoreForConnection(serviceConnection)
+ if (resolved == null) {
+ throw new ConfigurationException(
+ "DataSource not found for connection name
[${serviceConnection}] " +
+ "specified via @Transactional on service
[${serviceClass.name}]. " +
+ 'Please check your multiple data sources configuration
and try again.'
+ )
Review Comment:
The domain class is intentionally omitted here. In this code path the
connection comes from the explicit `@Transactional(connection="...")`
annotation on the service - the domain class mapping is irrelevant. Including
it would mislead users into thinking the domain mapping caused the problem. The
second path (domain mapping inheritance) does include the domain class because
there it is the source of the connection name.
##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/DatastoreServiceMethodInvokingFactoryBean.groovy:
##########
@@ -102,8 +111,17 @@ class DatastoreServiceMethodInvokingFactoryBean extends
MethodInvokingFactoryBea
if (domainConnection != null
&& ConnectionSource.DEFAULT != domainConnection
&& ConnectionSource.ALL != domainConnection) {
- return ((MultipleConnectionSourceCapableDatastore)
defaultDatastore)
+ Datastore resolved = ((MultipleConnectionSourceCapableDatastore)
defaultDatastore)
.getDatastoreForConnection(domainConnection)
+ if (resolved == null) {
+ throw new ConfigurationException(
+ "DataSource not found for connection name
[${domainConnection}] " +
+ "mapped on domain class [${domainClass.name}] " +
+ "used by service [${serviceClass.name}]. " +
+ 'Please check your multiple data sources configuration
and try again.'
Review Comment:
Same as above - consistent with the existing `HibernateDatastore` error
messages.
--
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]
