jdaugherty commented on code in PR #16723:
URL: https://github.com/apache/grails-core/pull/16723#discussion_r4408221926
##########
grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/AbstractHibernateGormStaticApi.groovy:
##########
@@ -178,9 +179,9 @@ abstract class AbstractHibernateGormStaticApi<D> extends
GormStaticApi<D> {
@Override
D load(Serializable id) {
- if (persistentEntity.isMultiTenant()) {
- // Hibernate's tenant filter does not apply to a load by
identifier, so a multi-tenant entity is loaded
- // through a query, as get(id) does, rather than handed to
whichever tenant asks for the id
+ if (persistentEntity.isMultiTenant() && multiTenancyMode ==
MultiTenancyMode.DISCRIMINATOR) {
Review Comment:
7.0.x, 7.1.x and 7.2.x have this same unconditional `isMultiTenant()` gate,
from #16703 (lines 176-177 of this file on 7.0.x), so `load` and `proxy` of a
DATABASE or SCHEMA entity run a query there as well. Their `GormStaticApi`
already has `multiTenancyMode`, so this exact change compiles on 7.0.x.
Could the Hibernate 5 part, this hunk and the two Hibernate 5 specs, land on
7.0.x and merge up, with this PR keeping the Hibernate 7 half? If this merges
to 8.0.x first, the later 7.0.x fix conflicts here and in both specs on the
merge-up. The 7.0.x `SingleTenantSpec` and `SchemaMultiTenantSpec` have the
same `then: "the correct tenant is used"` block, so the assertions port as they
are; `datastore` is a local variable there rather than a field.
##########
grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/connections/SingleTenantSpec.groovy:
##########
@@ -88,6 +88,18 @@ class SingleTenantSpec extends Specification {
}
}
+ and: 'load and proxy stay lazy for missing identifiers in DATABASE
mode'
+ SingleTenantAuthor.withNewSession {
+ def handler = datastore.mappingContext.proxyHandler
+ def loaded = SingleTenantAuthor.load(Long.MAX_VALUE)
Review Comment:
Optional. This pins that a missing id yields an uninitialized proxy, which
is the regression. A second assertion after the `Stephen King` save below,
loading that id under `moreBooks` and reading `name` through the proxy, would
also pin that the restored lazy path initializes against the tenant's own
database, which nothing in this spec exercises for `load`. Same for the SCHEMA
spec and the Hibernate 5 pair.
--
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]
