Skip to content

Hibernate 7#15568

Open
jdaugherty wants to merge 1143 commits into8.0.xfrom
8.0.x-hibernate7
Open

Hibernate 7#15568
jdaugherty wants to merge 1143 commits into8.0.xfrom
8.0.x-hibernate7

Conversation

@jdaugherty
Copy link
Copy Markdown
Contributor

Reopening. This PR replaces #15530

borinquenkid and others added 30 commits March 29, 2026 18:27
- Revert Liquibase Hibernate 7 versions to 4.27.0 for stability.
- Add no-arg constructor to GormDatabase for Liquibase service discovery.
- Fix JdbcConnection casting in HibernateDatabase.
- Update sequence support detection in MissingSequenceChangeGenerator.
- Replace removed StringUtil.randomIdentifier in UniqueConstraintSnapshotGenerator.
- Update migration specs to support INT and INTEGER type names.
- Fix dependency conflicts and JDK 17 compatibility for Derby.
- Update SBOM license mapping for liquibase-core.
- Rename Liquibase extension classes with 'Hibernate' prefix to avoid shadowing superclass simple names (fixes NM_SAME_SIMPLE_NAME_AS_SUPERCLASS).
- Update service registration files and test sources to reflect class renames.
- Fix NP_NONNULL_RETURN_VIOLATION in HibernateConnection by throwing SQLFeatureNotSupportedException instead of returning null.
- Fix DM_DEFAULT_ENCODING by specifying UTF-8/StandardCharsets.UTF_8 in multiple files.
- Fix potential NP_NULL_ON_SOME_PATH in DatabaseMigrationCommand.groovy with explicit null checks.
- Refactor BasicValueIdCreator to take MetadataBuildingContext in constructor.
- Move generator binding logic from SimpleValueBinder.bindSimpleValue to bindBasicValue.
- Update GrailsPropertyBinder and tests to use renamed bindBasicValue method.
- Update all relevant specs to match new constructor and method signatures.
- Move identifier property resolution and sequence detection logic from SimpleIdBinder to BasicValueIdCreator.
- Simplify SimpleIdBinder.bindSimpleId by using the new encapsulated methods.
- Update relevant unit tests to verify the refactored logic.
When firstExpressionIsRequiredBoolean() is true and operator is OR
(e.g. findPublishedByTitleOrAuthor), the refactored getJunction()
incorrectly placed the boolean criterion inside the Disjunction,
producing: (boolean) OR expr1 OR expr2

Fixed to produce the correct: boolean AND (expr1 OR expr2)
by wrapping in a top-level Conjunction with a nested Disjunction.

Assisted-by: OpenCode <opencode@opencode.ai>
Reverted Thread.currentThread().getContextClassLoader() back to
getClass().getClassLoader() in DelegateAsyncTransformation. The context
classloader does not have plugin module classes during Gradle AST
compilation, causing AsyncTransactionalServiceSpec to fail.

Also removed @ignore("FIX THIS LATER") from the spec.

Assisted-by: OpenCode <opencode@opencode.ai>
Restores test coverage for hibernate5, mongodb, and core-test that was
lost in the 8.0.x-hibernate7 merge.

TCK specs:
- FindByMethodSpec: restore 16 missing test methods from spring-boot-4
- OptimisticLockingSpec: add withNewSession-based tests for mongodb
  (matching spring-boot-4 pattern) alongside new withTransaction tests
- PagedResultSpec: add core.gorm.suite to @requires
- EnumSpec: restore findByEn()/findByEnInList() single-result tests
  without gates for core-test alongside new findAllBy versions

Core-test specs:
- Fix 6 specs disabled by @IgnoreIf(core.gorm.suite): update to
  addAllDomainClasses() for new GrailsDataTckManager API, restore
  missing imports, add targeted @requires only where simple datastore
  genuinely cannot handle the query
- OneToOneWithProxiesSpec: remove unnecessary @requires gate

All three suites verified green:
  core-test, hibernate5-core, mongodb-core

Assisted-by: OpenCode <opencode@opencode.ai>
…date binding logic

- Rename BasicValueIdCreator to BasicValueCreator.
- Rename SimpleValueBinder.bindBasicValue to bindSimpleValue.
- Update SimpleValueBinder to use BasicValueCreator internally for BasicValue creation.
- Update all references, tests, and documentation to reflect the new class and method names.
Move multi-line @requires closure statements to new line after opening
brace in FindByMethodSpec. Remove unused GrailsTransactionTemplate
import from OptimisticLockingSpec.

Assisted-by: OpenCode <opencode@opencode.ai>
…rage

Add 15 new test methods (from 5 to 20 total) covering all ProxyHandler
and ProxyFactory contract methods: isInitialized (null, non-proxied,
persistent collection, Hibernate proxy, Groovy proxy), unwrap (Hibernate
proxy, Groovy proxy, non-proxy), getIdentifier (Hibernate proxy,
non-proxied, null), createProxy, and isProxy (non-Hibernate object).

The Groovy proxy isInitialized test is excluded because it depends on a
source fix to HibernateProxyHandler.isInitialized() that will be
submitted in a separate PR.

Assisted-by: OpenCode <opencode@opencode.ai>
The Hibernate 7 HibernateProxyHandler.isInitialized() was missing a check
for Groovy proxies (ProxyInstanceMetaClass), causing uninitialized Groovy
proxies to be incorrectly reported as initialized. This is because the
fallback Hibernate.isInitialized() returns true for any non-Hibernate
object.

Add GroovyProxyInterceptorLogic.isInitialized() helper that checks
ProxyInstanceMetaClass.isProxyInitiated(), and call it from
HibernateProxyHandler.isInitialized() before the Hibernate fallback.

The Hibernate 5 implementation already had this check. This restores
parity and adds the corresponding test.

Assisted-by: OpenCode <opencode@opencode.ai>
- Rename groovyProxyInit to groovyProxyInitialized for clearer state semantics
- Add ProxyInstanceMetaClass precondition assertion in Groovy proxy test

Assisted-by: OpenCode <opencode@opencode.ai>
@jdaugherty
Copy link
Copy Markdown
Contributor Author

FYI: I added this comment due to the micronaut mismatches. I'll add a work around for this for now (no javadoc so it's probably ok to force api compatibility). Hopefully they update on the micronaut side so there aren't any issues.

@jdaugherty
Copy link
Copy Markdown
Contributor Author

Running this locally, I only had 1 test failure:

image

Which was due to OOM.

@jdaugherty
Copy link
Copy Markdown
Contributor Author

@borinquenkid can you take a look at that failure?

@borinquenkid
Copy link
Copy Markdown
Member

@jdaugherty I created a local task not checked in and needed this setup to run relaibli sub.tasks.withType(Test).configureEach { t -> │
│ t.maxParallelForks = 1 │
│ t.maxHeapSize = "3g" │
│ t.jvmArgs("-XX:MaxMetaspaceSize=512m") │
│ t.forkEvery = 1 │
│ }

@jdaugherty
Copy link
Copy Markdown
Contributor Author

Still TODO:

  1. I need to understand why micronaut was choosing the wrong version of test containers (reason we put it in the bom)
  2. need to take another pass at the out of tree updates
  3. need to review hibernate / final PR state (may recreate in that case)

@jdaugherty
Copy link
Copy Markdown
Contributor Author

@borinquenkid the latest round seems to have broken a lot of tests. Can you please take a look? I'd like to do another pass at this PR when you're done so we can actually review this functionality.

@borinquenkid
Copy link
Copy Markdown
Member

I reset hard some of the changes because they touched core which affects other modules.
Here is an assessment of the limitations:

GORM Scalability Analysis & Multi-Tenancy Risk Profile

Metric Safe Operating Range High Risk Range Critical Failure
Total Domain Classes < 150 150 - 300 > 500
Total Tenants < 200 200 - 500 > 1,000
Total API Objects < 30,000 30k - 50,000 > 100,000

Summary for June Release

  • Suitability: Current code is stable for small-to-medium applications but NOT supported for Dynamic Schema SaaS due to the lack of tenant eviction/lifecycle management.
  • Mandatory Requirement: To achieve production-grade stability for June, the architecture must transition to a Class-Singleton Orchestrator where API instances are shared across all tenants.
  • Safety Ceiling: The 50,000 API object ceiling must be re-implemented as a mandatory safety gate until the orchestrator transition is complete.

@jdaugherty
Copy link
Copy Markdown
Contributor Author

TODO:

  1. reduce memory back down
  2. fix rat issues
  3. @jdaugherty to take 1 last pass before review
  4. do the review on hibernate 7

@testlens-app
Copy link
Copy Markdown

testlens-app Bot commented Apr 22, 2026

✅ All tests passed ✅

🏷️ Commit: 6e09712
▶️ Tests: 17370 executed
⚪️ Checks: 34/34 completed


Learn more about TestLens at testlens.app.

@jdaugherty
Copy link
Copy Markdown
Contributor Author

My bom changes are really separate from this review. They fix a critical mistake in 7.0.x - you have to still override the hibernate 5 version instead of just using the bom. I'm going to look at pulling those changes out of this review so that the review is focused on hibernate alone.

@jdaugherty
Copy link
Copy Markdown
Contributor Author

This PR is currently blocked by #15605

Once that PR is merged, we will merge it up and then into this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants