jdaugherty opened a new pull request, #16515: URL: https://github.com/apache/grails-core/pull/16515
## Description With `DISCRIMINATOR` multi-tenancy, the datastores that use GORM's `MultiTenantEventListener` restrict queries and dynamic finders to the current tenant, but not lookups by id. `GormStaticApi.get`, `read`, `exists`, `getAll` and `load` look an instance up by its key, which bypasses the `PreQueryEvent` the listener restricts, so they return an instance of another tenant: | Reading as tenant `b` an instance saved by tenant `a` | simple | MongoDB | |---|---|---| | `get` / `read` / `exists` | found | found | | `getAll` | found | not found (already a query) | | `load` / `proxy` | proxy initializes | proxy initializes | | `findById`, `countBy...` | not found | not found | GORM for Hibernate already runs `get`, `read`, `exists` and `getAll` of a multi-tenant entity as a query for the same reason. For a multi-tenant entity in `DISCRIMINATOR` mode, `GormStaticApi` now retrieves instances by id through a query, which the listener restricts to the current tenant: - `get` and `getAll` run an id query, converting the ids to the identifier type as the lookup by key does. `getAll` keeps the order of the ids, with `null` for an id that is not found. `read` delegates to `get`, as its Javadoc says, and `exists` already did. - `load` and `proxy` return a proxy that is initialized through such a query. A proxy for an instance of another tenant then fails to initialize, as a proxy for an instance that does not exist does, and `getId()` still works without initializing it. A proxy factory that cannot create such a proxy (`GroovyProxyFactory`) gets the instance itself. - Inside `Tenants.withoutId`, where the current id is the default connection source, a lookup by id stays a lookup by key and is not restricted, as with Hibernate, which disables its tenant filter there. - Without a current tenant, a lookup by id now throws `TenantNotFoundException`, as a query does. Entities that are not multi-tenant, and the `DATABASE` and `SCHEMA` modes, are unchanged. GORM for Hibernate overrides these methods and is unchanged. Because a lookup by id of a multi-tenant entity is now a query, it flushes the session first with the `AUTO` flush mode, and with the `COMMIT` flush mode it no longer finds an instance that was saved but not flushed. The upgrade notes say so. ### Documentation - Upgrade note 19 in `upgrading60x.adoc`. - The partitioned multi-tenancy section of the GORM for MongoDB guide now covers lookups by id. ### Tests - `PartitionedLookupByIdSpec` (simple datastore, `grails-datamapping-core-test`): `get`, `read`, `exists`, `getAll`, `load` and `proxy` for the current and another tenant, id conversion, `withTenant`, `withoutId`, no current tenant, the `GroovyProxyFactory` fallback, and an entity that is not multi-tenant. - `MongoLookupByIdMultiTenancySpec` (`grails-data-mongodb-core`): the same against MongoDB. Without the change, 6 of the 9 simple features and 4 of the 6 MongoDB features fail. The others cover what must not change (`withoutId`, an entity that is not multi-tenant, MongoDB's `getAll`). These pass with the change: - all tests of `grails-datamapping-core` and `grails-datamapping-core-test` (which runs the TCK against the simple datastore) - the multi-tenancy, proxy and `getAll` specs of `grails-data-mongodb-core` - `codeStyle` of `grails-datamapping-core` ### Merging up 8.0.x reworked `GormStaticApi`, so the merge up needs the change ported there. On 8.0.x, Neo4j is part of the build and its `getAll` and `load` have the same problem (its `get` already runs as a query), so the port should add a Neo4j feature as well. -- 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]
