fix: GormEnhancer.allQualifiers() overrides explicit datasource declarations for MultiTenant entities

[jamesfredley]

Summary

GormEnhancer.allQualifiers() unconditionally expands qualifiers to all connection sources for any MultiTenant entity, even when the entity declares an explicit non-default datasource (e.g., datasource 'secondary'). This causes silent data routing to the wrong database under DISCRIMINATOR multi-tenancy mode.

The fix preserves explicit datasource qualifiers for MultiTenant entities instead of replacing them with DEFAULT + all connections.

Root Cause

In GormEnhancer.allQualifiers(), the MultiTenant check unconditionally triggers qualifier expansion:

if ((MultiTenant.isAssignableFrom(entity.javaClass) || qualifiers.contains(ConnectionSource.ALL))
    && (datastore instanceof ConnectionSourcesProvider)) {
    qualifiers.clear()                        // CLEARS the explicit 'secondary' mapping
    qualifiers.add(ConnectionSource.DEFAULT)   // Replaces with DEFAULT
    // adds all connection sources from ConnectionSourcesProvider
}

This treats MultiTenant the same as ConnectionSource.ALL. It is intended for DATABASE multi-tenancy mode (each tenant = separate connection), but incorrectly fires for DISCRIMINATOR mode where all tenants share one database and the entity has an explicit non-default datasource declaration.

When findTenantId() returns ConnectionSource.DEFAULT in DISCRIMINATOR mode, findStaticApi() resolves to the DEFAULT qualifier - routing all data to the wrong database with no error or warning.

Fix

A single conditional change in GormEnhancer.allQualifiers() (grails-datamapping-core):

boolean isMultiTenant = MultiTenant.isAssignableFrom(entity.javaClass)
boolean hasExplicitAll = qualifiers.contains(ConnectionSource.ALL)
boolean hasExplicitNonDefaultDatasource = isMultiTenant &&
        !hasExplicitAll &&
        qualifiers.size() > 0 &&
        !qualifiers.equals(ConnectionSourcesSupport.DEFAULT_CONNECTION_SOURCE_NAMES)

if ((isMultiTenant || hasExplicitAll) && !hasExplicitNonDefaultDatasource
    && (datastore instanceof ConnectionSourcesProvider)) {

Qualifier expansion now only fires for:

• MultiTenant entities on the default datasource (DATABASE multi-tenancy)
• Entities declared with ConnectionSource.ALL

When a MultiTenant entity has an explicit non-default datasource (e.g., datasource 'secondary'), the declared qualifier is preserved.

Steps To Reproduce

1. Create a Grails 7 app with two H2 datasources (default + secondary)
2. Configure DISCRIMINATOR multi-tenancy
3. Create a domain class that implements MultiTenant with datasource 'secondary'
4. Call .save() on an instance of that domain class
5. Observe that data is written to the default database, not the secondary one

Standalone reproducer: https://github.com/jamesfredley/grails-multitenant-datasource-bug

Test Coverage

21 tests across 3 levels:

Unit tests - GormEnhancerAllQualifiersSpec (7 tests)

Focused tests for allQualifiers() logic using mock datastores and entities:

• MultiTenant with explicit non-default datasource - preserves qualifier
• MultiTenant with default datasource - expands to all qualifiers
• MultiTenant with ConnectionSource.ALL - expands to all qualifiers
• Non-MultiTenant with explicit datasource - preserves qualifier
• Non-MultiTenant with default datasource - keeps default only
• Non-MultiTenant with ConnectionSource.ALL - expands to all qualifiers
• MultiTenant with multiple explicit datasources - preserves all qualifiers

Hibernate integration tests - DataServiceMultiTenantMultiDataSourceSpec (7 tests)

Full Hibernate datastore tests with two in-memory H2 databases (default + analytics):

• Schema creation on correct (analytics) datasource
• save() routes to analytics with tenant isolation
• get() retrieves from analytics datasource
• count() scoped to current tenant on analytics
• delete() removes from analytics datasource
• findByName with tenant isolation on analytics
• GormEnhancer qualifier resolution for unqualified and explicit paths
• GormEnhancer aggregate HQL routing to analytics

Functional test app - MultiTenantMultiDataSourceSpec (7 tests)

Full Grails application (grails-multitenant-multi-datasource) with @Integration tests:

• Schema creation on secondary datasource (not default)
• save() routes to secondary datasource
• get() by ID routes to secondary datasource
• count() scoped to current tenant
• delete() removes from secondary datasource
• findByName with tenant isolation on secondary
• findAllByName routes to secondary datasource

Changed Files

File	Change
grails-datamapping-core/.../GormEnhancer.groovy	Fix: preserve explicit non-default datasource qualifiers for MultiTenant entities
grails-datamapping-core/.../GormEnhancerAllQualifiersSpec.groovy	New: 7 unit tests for allQualifiers()
grails-data-hibernate5/.../DataServiceMultiTenantMultiDataSourceSpec.groovy	New: 7 Hibernate integration tests
grails-test-examples/.../grails-multitenant-multi-datasource/	New: functional test app (domain, service, config, 7 integration tests)
settings.gradle	Register functional test app

Environment Information

• Grails: 7.0.7
• Spring Boot: 3.5.10
• Groovy: 4.0.30
• JDK: 17+

[jdaugherty]

I worry mocking APIs in GORM unit tests will be unmaintainable in the longer run - since you are mocking just that its called in a certain way. You already have a functional test for this scenario, why not add it too?

[jdaugherty]

I thought I had submitted a review for this, but I may have gotten it mixed up with so many PRs being opened.

The test is highly relying on mocking, which is not a reliable approach long term. We need to at least add a functional test for this scenario (it sounds like you already have one too).

If we add the functional test, I'm good to merge this with teh mocking, otherwise we need to rework the unit test to not be so fragile.

[Copilot]

Pull request overview

Fixes qualifier expansion in GormEnhancer.allQualifiers() so that MultiTenant domain classes with an explicit non-default datasource mapping (e.g. datasource 'secondary') keep that qualifier instead of being forcibly expanded to DEFAULT + all connections, preventing silent routing to the wrong database in DISCRIMINATOR multi-tenancy mode.

Changes:

• Update GormEnhancer.allQualifiers() to preserve explicit non-default datasource qualifiers for MultiTenant entities.
• Add unit-level Spock coverage for allQualifiers() across MultiTenant/non-MultiTenant and explicit/default/ALL mappings.
• Add an integration-style Hibernate spec intended to exercise Data Services with DISCRIMINATOR multi-tenancy + secondary datasource.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/GormEnhancer.groovy	Adjusts qualifier expansion logic to avoid overriding explicit non-default datasource mappings for MultiTenant entities.
grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/GormEnhancerAllQualifiersSpec.groovy	Adds focused unit tests validating the new allQualifiers() behavior across key combinations.
grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/connections/DataServiceMultiTenantMultiDataSourceSpec.groovy	Adds a Hibernate-backed spec around Data Services + multi-tenancy + secondary datasource behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jamesfredley]

Updated with Functional test and PR feedback

[matrei]

This was a pretty significant bug! Nice catch.

[jamesfredley]

@matrei Thank you for the test cleanup.