matrei opened a new pull request, #16827:
URL: https://github.com/apache/grails-core/pull/16827
## Description
Fixes #16801.
In DATABASE mode, a `@Transactional` method of a class annotated with
`@CurrentTenant` failed with a `TenantException` when called inside
`Tenants.withId` while the tenant resolver resolves another tenant, as one that
resolves the tenant from the web request does outside a request:
```groovy
@CurrentTenant
@Transactional
class WidgetService {
void save(String name) {
new Widget(name: name).save(flush: true)
}
}
Tenants.withId('test') {
widgetService.save('widget')
}
// TenantException: Cannot resolve a single current tenant: [default, test]
are bound on this thread for different datastores. ...
```
These are two bugs from #16066, and a third with the same cause.
### `Tenants` and `@CurrentTenant` inside `Tenants.withId`
`Tenants.withId(tenantId)` binds the tenant for the datastore of the default
connection. Inside it, `Tenants.currentId()` and the other methods of `Tenants`
that take no datastore, and a `@CurrentTenant` class that is not a data
service, looked their datastore up through
`GormApiResolver.findSingleDatastore()`. With more than one connection that
resolved the current tenant, so in DATABASE mode it returned the datastore of
the tenant's connection. No tenant is bound for that datastore, so they took
the tenant the resolver resolves:
```groovy
// DATABASE mode, a tenant resolver that resolves 'two'
Tenants.withId('one') {
Tenants.currentId() // 'two'
}
```
A `@CurrentTenant` method then bound that tenant for the datastore of `one`.
The thread held two tenants, and a `@Transactional` method failed as above.
With a resolver that resolves no tenant, the `@CurrentTenant` method failed
with a `TenantNotFoundException`, and with a tenant the datastore has no
connection for, with a `ConfigurationException`.
`findSingleDatastore()` now returns the datastore of the default connection
whenever one is registered, as on 7.x, where it returned the single datastore
registered by type. Its callers all want that datastore: `Tenants`,
`@CurrentTenant`, the fallback of `findServiceDatastore`, and the generated
`getTargetDatastore()`, which calls `getDatastoreForConnection` on it. A
`@CurrentTenant` data service was not affected, as `@CurrentTenant` looks its
datastore up through `findServiceDatastore`.
### `Tenants.withId` with a domain class
`Tenants.withId(domainClass, tenantId)` looked the datastore up through
`findDatastore(domainClass)`, which in DATABASE mode resolves the current
tenant as well, the one bound or the one the resolver resolves. It bound the
given tenant for the datastore of that tenant, and failed:
```groovy
// DATABASE mode, a tenant resolver that resolves 'two'
Tenants.withId(Team, 'one') {
new Team(name: 'Wolves').save(flush: true)
}
// ConfigurationException: No datastore found for connection named [one]
```
It now binds the tenant for the datastore the domain class is mapped to,
looked up through `findServiceDatastore`, as `Tenants.withTenant(domainClass,
tenantId)` does too.
### The transaction manager of a tenant
The `getTransactionManager()` that `@Transactional` and data services
generate looked up the tenant with `CurrentTenantHolder.get()`, which throws a
`TenantException` when different tenants are bound for different datastores.
That is what the spec in the issue shows. It now calls
`GormRegistry.findCurrentTenantTransactionManager`, which looks up the tenant
bound for the default datastore, or for the datastore of the data service, and
returns the transaction manager of the connection of that name, as before.
The lookup moved from the generated code into `GormRegistry`. Classes
compiled against 8.0.0 keep the unscoped lookup until they are recompiled. The
change to `findSingleDatastore()` applies to them without that, and covers the
case in the issue.
### Tests
- `CurrentTenantTransformSpec` (`grails-datamapping-core-test`) has five new
features, in DATABASE mode on GORM's in-memory datastore: for
`Tenants.currentId()` and a `@CurrentTenant` method inside `Tenants.withId`
while the resolver resolves another tenant, for a `@Transactional` method while
another datastore has a different tenant, and for `Tenants.withId` with a
domain class, at the top level and inside `Tenants.withId`. All fail without
the change.
- `DatabaseMultiTenantCurrentTenantTransactionalSpec`
(`grails-data-hibernate5-core`, new) covers the case in the issue on GORM for
Hibernate 5, with a `@CurrentTenant` `@Transactional` class and a resolver that
resolves `default`. Both features fail without the change.
- `GormApiResolverSpec` has two new features for `findSingleDatastore()`,
with a tenant bound for the default datastore and with a session of another
connection bound. Both fail without the change. The feature
`findSingleDatastore delegates to findDatastore when more than one qualifier is
registered` now says what it checks.
- `GormRegistrySpec` has four features for
`findCurrentTenantTransactionManager`.
- `TransactionalTransformSpec` and `ServiceTransformSpec` have a new feature
each for the generated `getTransactionManager()` while another datastore has a
different tenant. Both fail without the change.
These pass with the change:
- all tests of `grails-datamapping-core`, `grails-datamapping-core-test`,
`grails-data-simple`, `grails-datastore-web`, `grails-data-hibernate5-core`,
`grails-data-hibernate7-core`, `grails-data-mongodb-core` and
`grails-data-neo4j-core`;
- `codeStyle` for `grails-datamapping-core` and
`grails-data-hibernate5-core`.
--
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]
