8.0.x Hibernate 7 Publishing Fixes & Bom Addition#15510
Draft
jdaugherty wants to merge 218 commits into8.0.x-hibernate7from
Draft
8.0.x Hibernate 7 Publishing Fixes & Bom Addition#15510jdaugherty wants to merge 218 commits into8.0.x-hibernate7from
jdaugherty wants to merge 218 commits into8.0.x-hibernate7from
Conversation
GrailsOpenSessionInViewInterceptor only registered a session for the default datasource SessionFactory. Calling withSession on a secondary datasource during a web request threw 'No Session found for current thread' because no session was bound for that SessionFactory. The interceptor now iterates all connection sources from the HibernateDatastore and opens/binds sessions for each non-default datasource in preHandle, flushes them in postHandle, and unbinds/closes them in afterCompletion. Existing sessions that are already bound (e.g. by a transaction) are left untouched. Fixes #14333 Fixes #11798 Assisted-by: Claude Code <Claude@Claude.ai>
Wrap the additional session cleanup loop in try-finally so that super.afterCompletion() always runs even if closing an additional session throws. Add per-session try-catch around closeSession with error logging, matching HibernatePersistenceContextInterceptor.destroy() pattern. Assisted-by: Claude Code <Claude@Claude.ai>
Include the datasource connection name in debug and error log messages for GrailsOpenSessionInViewInterceptor to aid multi-datasource debugging. Add a Geb integration test in the datasources test module that verifies OSIV keeps the secondary datasource session open during GSP view rendering, allowing lazy-loaded associations to be accessed without LazyInitializationException. Assisted-by: Claude Code <Claude@Claude.ai>
Use Book.secondary.findByTitle instead of Book.secondary.first to avoid picking up stale records from other integration tests sharing the same H2 database. Assisted-by: Claude Code <Claude@Claude.ai>
Add two new TCK specs that validate GORM Data Service CRUD operations route correctly to secondary datasources across all datastore implementations: - DataServiceConnectionRoutingSpec: 16 tests covering save, get, count, delete, findByName, findAllByName, and constructor-style save through both abstract class and interface service patterns, plus cross-datasource isolation verification. - DataServiceMultiTenantConnectionRoutingSpec: 5 tests covering DISCRIMINATOR multi-tenancy combined with secondary datasource routing, verifying tenant isolation for save, get, count, delete, and findByName. Adds multi-tenant + multi-datasource infrastructure to TCK: - GrailsDataTckManager: new supportsMultiTenantMultiDataSource(), setupMultiTenantMultiDataSource(), cleanupMultiTenantMultiDataSource(), getServiceForMultiTenantConnection() methods - GrailsDataHibernate5TckManager: DISCRIMINATOR mode with SystemPropertyTenantResolver and dual H2 in-memory databases - GrailsDataMongoTckManager: DISCRIMINATOR mode with SystemPropertyTenantResolver and dual MongoDB databases Fixes AbstractDatastoreInitializer.loadDataServices() to filter discovered services by mapped domain classes, preventing classpath pollution from unrelated @service implementations causing startup failures in standalone initializers (e.g., MongoDbDataStoreSpringInitializerSpec). Assisted-by: Claude Code <Claude@Claude.ai>
Add four new TCK specs that test GORM operations at the domain level via GormEnhancer API, and verify cross-layer consistency between domain operations and Data Service operations: - DomainMultiDataSourceSpec: 8 tests for direct save, get, count, list, criteria query, delete on secondary datasource, plus cross-datasource isolation. - DomainMultiTenantMultiDataSourceSpec: 5 tests for DISCRIMINATOR multi-tenancy with direct domain operations on secondary datasource, including tenant-scoped count, criteria query, and isolation. - CrossLayerMultiDataSourceSpec: 5 tests verifying data written at the domain level is visible through Data Services and vice versa, including delete propagation and count consistency. - CrossLayerMultiTenantMultiDataSourceSpec: 3 tests verifying tenant-isolated data is consistent across domain API and Data Service layers. Assisted-by: Claude Code <Claude@Claude.ai>
…ices @service(DomainClass) now automatically inherits the datasource from the domain class's mapping block, so developers no longer need to specify @transactional(connection = '...') when the domain already declares its datasource. Explicit @transactional(connection) on the service still takes precedence. Two-layer implementation: - AST (ServiceTransformation): parses the domain's static mapping closure for datasource/connection calls and propagates to the generated service impl via @transactional(connection) - Runtime (DatastoreServiceMethodInvokingFactoryBean): resolves the domain's datasource from the mapping context and sets the correct datastore on the service instance Assisted-by: Claude Code <Claude@Claude.ai>
…tasource-inheritance
- Fix runtime precedence: resolveEffectiveDatastore() now checks service @transactional(connection) before falling back to domain mapping - Fix functional test to use @Autowired instead of manual service retrieval - Add integration test for service obtained from default datastore - Improve explicit-wins test to verify annotation preservation with different connection values (archive vs warehouse) - Fix docs: findByCode -> findAllByCode for List return type Assisted-by: Claude Code <Claude@Claude.ai>
…guide Assisted-by: Claude Code <Claude@Claude.ai>
… and functional test suites Verify that GORM static methods (executeQuery, withCriteria, createCriteria, executeUpdate, withTransaction) route to the correct datasource for entities mapped to non-default datasources. Covers the allQualifiers() fix in 4e04e96. Assisted-by: Claude Code <Claude@Claude.ai>
…iTenant routing, and CRUD connection fixes Add documentation reflecting the post-fix state after PRs #15393, #15395, and #15396 are merged: - Add @CompileStatic + injected @service property example (PR #15396) - Add Multi-Tenancy with explicit datasource section (PR #15393) - List all CRUD methods that respect connection routing (PR #15395) - Soften IMPORTANT boxes to NOTE with authoritative tone Assisted-by: Claude Code <Claude@Claude.ai>
…list Add @Where-annotated methods, @Query-annotated methods, and DetachedCriteria-based queries to the list of auto-implemented methods that respect the connection parameter in multi-datasource Data Services documentation. Assisted-by: Claude Code <Claude@Claude.ai>
Domain static methods like Book.executeQuery(), Book.createCriteria(), and Book.withCriteria() route to the correct datasource automatically when the domain declares a non-default datasource in its mapping block. Remove all references to GormEnhancer.findStaticApi() from user-facing documentation since it is an internal API that users no longer need. Assisted-by: Claude Code <Claude@Claude.ai>
…cription Remove GormEnhancer class name from multi-tenancy explanation since it is an internal implementation detail. Fix description of how @service properties are populated to say autowiring by type instead of datastore.getService(). Assisted-by: Claude Code <Claude@Claude.ai>
# Conflicts: # grails-forge/grails-forge-core/src/main/java/org/grails/forge/feature/reloading/SpringBootDevTools.java # grails-forge/grails-forge-core/src/test/groovy/org/grails/forge/feature/assetPipeline/AssetPipelineSpec.groovy # grails-forge/grails-forge-core/src/test/groovy/org/grails/forge/feature/spring/SpringBootVirtualThreadsSpec.groovy
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Move the DetachedCriteria.count() projection workaround into the Query abstraction layer. The base Query.countResults() falls back to list().size() when user-defined projections exist. H5 overrides in AbstractHibernateQuery to check its own HibernateProjectionList. H7 inherits the base default for now, with an optimization path available via Hibernate 7's derived-table JPA Criteria API. DetachedCriteria.count() now delegates to a single query.countResults() call with no backend-specific branching. Assisted-by: Claude Code <Claude@Claude.ai>
Copy test specs added on the 7.0.x branch for bug fixes and features that were never ported to the Hibernate 7 module. Includes query specs (BasicCollectionInQuery, DetachedCriteriaProjectionNullAssociation, WhereQueryBugFix, WhereQueryOldIssueVerification, SqlQuery, RLike, HibernateValidation), multi-datasource specs (DataServiceDatasource Inheritance, DataServiceMultiDataSource, DataServiceMultiTenantMulti DataSource, WhereQueryMultiDataSource, MultiDataSourceSession), ExistsCrossJoinSpec, and SimpleHibernateProxyHandlerSpec. Assisted-by: Claude Code <Claude@Claude.ai>
Add 17 missing test methods to HibernateProxyHandler7Spec covering null handling, persistent collections, associations, createProxy, unwrap, and deprecated methods. Port 5 ByteBuddy proxy tests from H5 covering CompileStatic id checks, dynamic getId, truthy checks, association id checks, and isDirty. All 5 are marked @PendingFeature because Hibernate 7's ByteBuddyInterceptor initializes proxies on getMetaClass() and there is no H7-compatible replacement for yakworks hibernate-groovy-proxy. Assisted-by: Claude Code <Claude@Claude.ai>
Add grails-data-service-multi-datasource and grails-multitenant-multi- datasource test apps for H7, ported from H5 with dependency updates. Copy SecondaryBookController, UrlMappings, and MultiDataSourceWithSession Spec to the existing grails-multiple-datasources H7 app. Add BookPages Geb page objects and update BookControllerSpec to use them instead of inline selectors. Register new test apps in settings.gradle. Assisted-by: Claude Code <Claude@Claude.ai>
Add supportsMultipleDataSources(), setupMultiDataSource(), and multi-tenant multi-datasource methods to GrailsDataHibernate7TckManager, matching the H5 implementation. This enables 34 TCK tests that were previously skipped due to the missing multi-datasource test harness. Port 2 missing test methods to MultipleDataSourceConnectionsSpec: 'static GORM operations' (@PendingFeature due to executeQuery named parameter binding on non-default datasource) and 'ALL mapped entity withNewSession' (passes). Port JPA @onetomany test to TwoUnidirectionalHasManySpec (@PendingFeature due to non-nullable join column generation in H7). Assisted-by: Claude Code <Claude@Claude.ai>
Remove WhereQueryConnectionRoutingSpec from the skipped tests table since multi-datasource TCK support is now implemented. The spec was previously skipped because the test manager did not wire setupMultiDataSource(). Assisted-by: Claude Code <Claude@Claude.ai>
This comment has been minimized.
This comment has been minimized.
Contributor
|
I pushed commits addressing
Known issues after these commits: H7 Test Gaps - Detailed Analysis1. ByteBuddy Proxy Initialization (5 tests)Root cause: Hibernate 7's Stack trace path: Why H5 worked (partially): In H5, Why yakworks 1.1 can't fix H7: The yakworks Fix path: Write a Hibernate 7-compatible Groovy interceptor that overrides Affected tests:
2. JPA @onetomany Unidirectional Join Table (1 test)Root cause: Hibernate 7 generates a single join table insert into EcmMaskJpa_JpaUser (EcmMaskJpa_id, createdUsers_id) values (?, ?)
-- Fails: NULL not allowed for column "UPDATEDUSERS_ID"Why: Hibernate 7 merges the two @OneToMany(cascade = CascadeType.ALL)
@JoinTable(name = "ecm_mask_created_users")
Set<JpaUser> createdUsers = []
@OneToMany(cascade = CascadeType.ALL)
@JoinTable(name = "ecm_mask_updated_users")
Set<JpaUser> updatedUsers = []Affected test:
3. executeQuery Named Parameters on Multi-Datasource Entity (1 test)Root cause: Why: The
|
…nant spec The H7 test pattern uses explicit cleanup() to reset system properties instead of Spock's @RestoreSystemProperties annotation. Assisted-by: Claude Code <Claude@Claude.ai>
This comment has been minimized.
This comment has been minimized.
Override countResults() in H7 HibernateQuery to use JpaSelectCriteria.from(Subquery) for proper derived-table counting instead of the list().size() fallback. Add populateSubquery() to JpaCriteriaQueryCreator that builds the inner query with aliased projections as required by Hibernate 7's SqmDerivedRoot. Restore the performance warning log in H5 AbstractHibernateQuery.countResults(). Add DetachedCriteriaCountSpec with 6 test methods covering count with/without projections, groupProperty, aggregate, and filtered criteria - passes on both H5 and H7. Assisted-by: Claude Code <Claude@Claude.ai>
This comment has been minimized.
This comment has been minimized.
Replace count(col_0) with count(literal(1)) to avoid undercounting when the first projected column is nullable. Widen JpaFromProvider constructor from JpaCriteriaQuery to AbstractQuery so populateSubquery() can pass the subquery directly instead of null, preventing NPE when association criteria generate additional roots. Assisted-by: Claude Code <Claude@Claude.ai>
🚨 TestLens detected 33 failed tests 🚨Here is what you can do:
Test Summary
🏷️ Commit: d191944 Test Failures (first 5 of 33)DefaultInputRenderingSpec > input for #{required ? 'a required' : 'an optional'} #description property #{required ? 'has' : 'does not have'} a no-selection option > input for #{required ? 'a required' : 'an optional'} many-to-one property #{required ? 'has' : 'does not have'} a no-selection option (:grails-fields:test in CI / Build Grails-Core (ubuntu-latest, 25))DefaultInputRenderingSpec > input for #{required ? 'a required' : 'an optional'} #description property #{required ? 'has' : 'does not have'} a no-selection option > input for #{required ? 'a required' : 'an optional'} many-to-one property #{required ? 'has' : 'does not have'} a no-selection option (:grails-fields:test in CI / Build Grails-Core (ubuntu-latest, 25))DefaultInputRenderingSpec > input for #{required ? 'a required' : 'an optional'} #description property #{required ? 'has' : 'does not have'} a no-selection option > input for #{required ? 'a required' : 'an optional'} one-to-one property #{required ? 'has' : 'does not have'} a no-selection option (:grails-fields:test in CI / Build Grails-Core (ubuntu-latest, 25))DefaultInputRenderingSpec > input for #{required ? 'a required' : 'an optional'} #description property #{required ? 'has' : 'does not have'} a no-selection option > input for #{required ? 'a required' : 'an optional'} one-to-one property #{required ? 'has' : 'does not have'} a no-selection option (:grails-fields:test in CI / Build Grails-Core (ubuntu-latest, 25))DefaultInputRenderingSpec > input for a #description property does have `.id` at the end of the name > input for a many-to-one property does have `.id` at the end of the name (:grails-fields:test in CI / Build Grails-Core (ubuntu-latest, 25))Muted TestsNote Checks are currently running using the configuration below. Select tests to mute in this pull request: 🔲 BookControllerSpec > Test that the delete action deletes an instance if it exists Reuse successful test results: 🔲 ♻️ Only rerun the tests that failed or were muted before Click the checkbox to trigger a rerun: 🔲 Rerun jobs Learn more about TestLens at testlens.app. |
Resolve all checkstyle violations in grails-data-hibernate7-core (287 errors across 77 files), grails-data-hibernate5-core (3 errors), and grails-datastore-core (4 errors). Fixes include: expanding star imports, correcting import ordering, moving operators to previous line, fixing indentation, adding empty line separators, adding newlines at end of files, fixing whitespace around braces, and removing unnecessary semicolons. Assisted-by: Claude Code <Claude@Claude.ai>
Remove unused GormQueryOperations import from GormStaticApi. Add missing trailing newlines to NamedCriteriaProxy and FindByMethodSpec. Assisted-by: Claude Code <Claude@Claude.ai>
…st app MultiDataSourceWithSessionSpec requires micronaut-http-client and micronaut-serde-jackson for integration tests, matching the H5 test app. Assisted-by: Claude Code <Claude@Claude.ai>
The Core Projects code style check pipes Gradle output through tee into build/codestyle-output.log, but the build/ directory may not exist at the start of CI. Add mkdir -p to create the directory and reports subdirectory before tee starts writing. Introduced in 7f13d5e. Assisted-by: Claude Code <Claude@Claude.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Current Progress:
TODO: