codeconsole commented on PR #16369:
URL: https://github.com/apache/grails-core/pull/16369#issuecomment-5782050191
@matrei round 2 arrived while I was pushing the five fixes and I only read
it alongside round 3, so it went unanswered for a round. Both are addressed now.
**[P1] `@Rollback`/`@Transactional` naming a tenant** — 62c7b4a139, your
first option, pushed shortly after your comment. `isMappedToConnection` passes
over a multi-tenant class, so the annotation chooses the transaction manager
and the tenant resolver keeps deciding, as before. Both example applications
pass again on Hibernate 5 and 7, unchanged, and
`ConnectionScopeMultiTenancySpec` pins the contract where I had no coverage: a
`@Transactional('bar')` method counting the current tenant's rows. The guides,
section 72 and the `connection()` javadoc state the exemption.
**[P3] `TransactionService` on the default datastore** — dd92b4502f.
`newTemplate` passes a connection only when it is not the default, so the root
datastore's service behaves as an unqualified `@Transactional` does. The nested
form is pinned: `Player.one.withTransaction {
datastore.getService(TransactionService).withTransaction { Player.count() } }
== 2`, beside the named-connection service and the root one outside a block.
**[P3] what "mapped" means** — 44d95a6427, also your first option. Both
pages now say the class has to declare the connection in its `connections`
mapping, `ALL` included, and that one declaring none stays reachable through
`Book.moreBooks` while the annotation leaves it alone; the MongoDB and Neo4j
notes say the same. The annotation now has a test on both implementations: a
`@Transactional(connection = 'test2')` service saving and counting against a
container on MongoDB, and the same against the second embedded server on Neo4j.
Both fail with the routing removed.
**Nit, two sessions in DATABASE mode** — 9265c8e1b5. Bound through
`CurrentTenantHolder.withTenant`, so only the session this method opens is
opened.
**Nit, the `default`-scope assertion** — 1103ddd72c. It nests inside a block
for another connection now, in the public form,
`ScopedTenantBook.bar.withTransaction {
ScopedTenantBook.'default'.withTransaction { … } }`, and through the registry.
**Nit, the "left alone" branch** — 1103ddd72c, in
`PartitionMultiTenancySpec`, which already had the DISCRIMINATOR setup: a
transaction and a session opened for a tenant id keep the current tenant, while
the call that names it reaches it. One caveat, since it is worth being exact:
it pins the documented behaviour and covers those lines, but it does not
distinguish the branch from its absence. With the `isConnectionSourceName`
check removed the numbers do not move, because the registry resolves a
DISCRIMINATOR tenant qualifier back to the default API anyway.
**Nit, the two `localhost` sentences** — 44d95a6427, both now `127.0.0.1`.
**The limitation line** — it went from the description with fdd67c9244; the
description no longer lists it.
You were right about the run. Mine covered seven multi-datasource
applications and not the tenant ones, which is exactly where this broke. It is
now the eight modules plus thirteen example applications, schema-per-tenant,
database-per-tenant and partitioned included on both Hibernate versions.
Run for these: 7,525 tests, 0 failures, plus `codeStyle`, the five guides
and `rat`.
--
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]
