Hibernate 7 - Step 2#15568
Conversation
- PropertyConfigSpec: 14 new tests covering column(Closure) with firstColumnIsColumnCopy, getJoinTableColumnConfig, configureExisting with pre-existing columns, column-delegate getters (getEnumType, getSqlType, getIndexName, getLength, getPrecision), toString, and clone with typeParams - HibernateAssociationQuerySpec: add negation() test covering L88 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tructor test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…der branch Add test covering L77-78: when componentBinder is set on the binder and the property is HibernateEmbeddedCollectionProperty, bindCollectionSecondPass delegates to componentBinder.bindEmbeddedCollectionComponent() and sets the result as the collection element. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix 17 plain executeUpdate('...') calls across 7 specs in
grails-test-examples/gorm to use executeUpdate('...', [:]).
H7's HibernateGormStaticApi rejects plain CharSequence args
(requires either a GString with interpolated params or the
Map overload).
- Add getDomainClasses() override to BookHibernateSpec in app1.
H7's HibernateSpec uses HibernateDatastoreSpringInitializer
which requires explicit domain class declaration; H5 auto-detected
via classpath scanning.
- Remove grails-test-examples-app1 and grails-test-examples-gorm
from h7IncompatibleProjects list — both now run cleanly under
Hibernate 7 via dependency substitution.
Remaining excluded: datasources (ChainedTransactionManager),
views-functional-tests (HAL/JSON diffs), scaffolding-fields
(grails-fields rendering diffs).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…urn types, cross-property arithmetic Bug 2: HibernateQueryExecutor.singleResult() now catches both org.hibernate.NonUniqueResultException and jakarta.persistence.NonUniqueResultException (H7 throws the JPA variant; the original catch missed it) and returns the first result instead of propagating. Bug 4: HqlQueryContext.aggregateTargetClass() now returns precise types per function: count() → Long, avg() → Double, sum/min/max() → Number. Previously all aggregates were bound to Long, causing QueryTypeMismatchException in H7's strict SQM type checking. Bug 5: Cross-property arithmetic in where-DSL (e.g. pageCount > price * 10) was silently dropped — the RHS property reference was coerced to a literal. Fixed via: - PropertyReference: Groovy wrapper returned by propertyMissing for numeric properties; *, +, -, / operators produce a PropertyArithmetic value object - PropertyArithmetic: value type carrying (propertyName, Operator, operand) - HibernateDetachedCriteria: H7-only DetachedCriteria subclass that overrides propertyMissing to return PropertyReference for numeric properties, and newInstance() to preserve the subtype through cloning - HibernateGormStaticApi: overrides where/whereLazy/whereAny to use HibernateDetachedCriteria as the closure delegate - PredicateGenerator: resolveNumericExpression() detects PropertyArithmetic and builds cb.prod/sum/diff/quot(path, operand) instead of a literal H5 and MongoDB are unaffected — all new types are confined to grails-data-hibernate7. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ve on managed entities H7 enforces strict collection identity during flush. GORM's addTo* and save() flow had two failure modes: 1. When an entity is already managed in the current Hibernate session, calling session.merge() causes H7 to create a second PersistentCollection for the same role+key alongside the one already tracked in the session cache -> 'Found two representations of same collection'. Fix (HibernateGormInstanceApi.performMerge): check session.contains(target) before merging. If the entity is already managed, skip merge entirely; dirty-checking and cascade will handle children on flush. 2. When addTo* is called on a managed entity, GormEntity.addTo uses direct field access (reflector.getProperty) which bypasses H7's bytecode-enhanced interceptor, sees null, and creates a plain ArrayList on the field. H7's session cache already tracks a PersistentBag/Set for that role -> two representations on the next save. Fix (HibernateEntity.addTo): override addTo in the H7 trait; for managed entities (id != null), trigger the H7 interceptor via InvokerHelper.getProperty to obtain the live PersistentCollection before delegating to GormEntity.super.addTo. Fix (HibernateEntityTransformation): re-target the concrete addToXxx generated methods so their internal addTo call dispatches through HibernateEntity.addTo rather than being hard-wired to GormEntity.addTo. Fix (HibernateGormInstanceApi.reconcileCollections): detect stale PersistentCollections (session != current session) and replace them with plain collections before merge, covering any edge cases where the H7 interceptor path is not taken. Adds AddToManagedEntitySpec with 4 tests covering: - addTo on an already-persisted entity - multiple addTo on a fresh transient entity - modify child + save twice - removeFrom + save Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…and proper applicationClass - Replace executeQuery(plainString) and executeUpdate(plainString) calls with the (String, Map) overloads (empty map for parameterless queries). HibernateGormStaticApi intentionally rejects plain String in the no-arg overload to prevent HQL injection; parameterless static queries must use the Map overload. - Add applicationClass = Application to @Integration so the spec shares the same application context and transaction manager as the other specs in this module, preventing test-data bleed between specs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts: # NOTICE # dependencies.gradle # gradle/publish-root-config.gradle
…ibernate7 # Conflicts: # .idea/codeStyles/Project.xml # NOTICE # build-logic/plugins/build.gradle # dependencies.gradle # gradle/publish-root-config.gradle
Following the pattern used in Hibernate 5, I have moved the vendored Spring Framework ORM Hibernate 7 integration classes from the core module to a dedicated spring-orm module:
- Created Module: grails-data-hibernate7-spring-orm (located in grails-data-hibernate7/spring-orm).
- Moved Classes: All classes in org.grails.orm.hibernate.support.hibernate7 (including HibernateTransactionManager, HibernateTemplate, LocalSessionFactoryBean, etc.) are now in the new module.
- Updated Dependencies:
- Added the new module to settings.gradle.
- Updated grails-data-hibernate7-core, grails-plugin, boot-plugin, and dbmigration to depend on the new spring-orm module.
- Updated gradle/publish-root-config.gradle to ensure the new module is included in the publishing process.
- HibernateDetachedCriteria.isNumericPropertyType: box primitive types before the Number.isAssignableFrom check so that domains declaring numeric properties as primitives (int/long/double/float/short/byte) work correctly in where-DSL arithmetic expressions. Method is protected to allow subclass overrides. - ToManyEntityMultiTenantFilterBinder.bind: add null guard for getHibernateAssociatedEntity() return value to prevent NullPointerException on partially-resolved associations. - grails-data-hibernate7/README.md: add grails-data-hibernate7-spring-orm to the Module Structure table. - Tests: new HibernateDetachedCriteriaSpec covering boxed and all 6 primitive numeric types, non-numeric delegation, and unknown property. Added null-associated-entity test to ToManyEntityMultiTenantFilterBinderSpec. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts: # grails-data-mongodb/core/src/test/groovy/org/apache/grails/data/mongo/core/GrailsDataMongoTckManager.groovy # grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BeforeUpdatePropertyPersistenceSpec.groovy # grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/CustomAutoTimestampSpec.groovy # grails-datamapping-core-test/src/test/groovy/org/grails/datastore/gorm/NestedCriteriaWithNamedQuerySpec.groovy # grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/GormStaticApi.groovy # grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/query/NamedCriteriaProxy.groovy # grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/NamedQuerySpec.groovy # grails-test-suite-uber/src/test/groovy/grails/compiler/DomainClassWithInnerClassUsingStaticCompilationSpec.groovy
This comment has been minimized.
This comment has been minimized.
…s any hibernate 5 or hibernate 7 project
This is a structural problem: the Grails doc build is a single pass and can only pull from one GORM data module at a time (H5 or H7), but MongoDB is independent of both. There are three realistic Option A — separate doc builds (recommended long-term) Option B — conditional AsciiDoc includes (quick fix) Option C — link out to standalone GORM docs (unblock now) @jamesfredley Is Option A (separate builds) is on the roadmap or Option B (build flag) is acceptable for M2. |
The ASF infrastructure gateway requires all third-party actions to use commit SHAs listed in apache/infrastructure-actions approved_patterns.yml. The previously used hash (4d9f0ba, v5.0.0) was not in the approved list, causing startup_failure on pull_request and workflow_dispatch events. Update all 10 workflow files to use 50e97c2 (v6.1.0), the latest ASF-approved commit SHA for gradle/actions/setup-gradle. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tyMapping
The TCK tests for MongoDB were failing because `getStoredAs()` in the `IdentityMapping` interface returned a default of `null`, and `DefaultIdentityMapping` did not override it. This caused the
MongoDB engine to lose track of explicit `storedAs` values (like `ObjectId`), breaking coercion.
This commit overrides `getStoredAs()` in `DefaultIdentityMapping` to return `getMappedForm().getStoredAs()`, restoring the intended behavior.
|
To help guide the review of this PR, I want to provide a high-level overview of the major architectural changes and design decisions introduced in this branch. Given the magnitude of the changes, this
These architectural shifts are the primary drivers of the footprint of this PR. The accompanying style adjustments (CodeNarc, Checkstyle, PMD) were applied mechanically to ensure the new monorepo Let me know if you have any questions about specific implementation details in these areas!*** |
…le-quotes inside single-quoted strings
Three regex bugs in registerCodenarcFixTask:
1. SpaceAroundMapEntryColon matched the first ':' of '::' method references,
turning String::trim into String: :trim.
Fix: add (?!:) lookahead so only standalone map-entry colons are matched.
2. UnnecessaryGString fused the closing quote of a GString with the opening
quote of an adjacent plain string (e.g. obj."${name}"("arg") became
obj."${name}'('arg")).
Fix: add (?<!}) lookbehind so a double-quote preceded by '}' is never
treated as an opening delimiter.
3. UnnecessaryGString matched double-quoted content inside single-quoted strings
(e.g. 'format "hh:mm:ss"'), corrupting the outer string.
Fix: the regex alternation now first matches and skips complete single-quoted
strings so their embedded double-quoted content is never converted.
Tests added for all three cases.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Assisted-by: opencode:openai/gpt-5.5 oracle
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 8.0.x #15568 +/- ##
===================================================
- Coverage 48.4322% 19.0184% -29.4138%
+ Complexity 15265 288 -14977
===================================================
Files 1875 57 -1818
Lines 85852 3423 -82429
Branches 14972 597 -14375
===================================================
- Hits 41580 651 -40929
+ Misses 37863 2642 -35221
+ Partials 6409 130 -6279 🚀 New features to boost your workflow:
|
Resolved dependencies.gradle by taking the latest base dependency versions for Spring Boot, Spock, and Sitemesh while retaining Hibernate 7 BOM-specific dependency sections. Assisted-by: opencode:openai/gpt-5.5 oracle
jamesfredley
force-pushed
the
8.0.x-hibernate7
branch
from
a8e9fc9 to
8452458
Compare
|
I found a list of items here that I think are important and will have a PR against this PR in a few hours. |
Resolve dependencies.gradle conflict: take mongodb 5.8.0 from 8.0.x (aligned with the Spring Boot 4.1 BOM) and keep the reactor.version 3.8.5 entry added on this branch. Assisted-by: claude-code:claude-opus-4-8
@jamesfredley Ok, I started reviewing, but I'll hold off until you are done. |
|
@matrei @jdaugherty @borinquenkid #15731 is the list of items I found. Feel free to commit to that branch and anything can be added or subtracted. The PR description will be update in a few minutes with a full breaking changes table between h5.6 and h7.2. Hopefully CI will finish in that PR, if not feel free to push fixes. |
✅ All tests passed ✅🏷️ Commit: 76340d7 Learn more about TestLens at testlens.app. |
|
@jamesfredley I have said it before but I am highly against module substitution to expand test coverage. For multiple reasons:
We should just copy tests to expand coverage. |
|
Thinking ahead to the broader roadmap: we also need to start actively planning and working toward Hibernate 8. With that in mind, framing Grails 8.0 as a permanent, dual-baseline bridge isn't sustainable. It forces us to carry too much legacy weight while trying to leapfrog forward. Instead, we should use this PR to clean house and establish a clear, fast-moving release strategy: The Proposed Lifecycle Compromise
Why This Splitting of the Difference Works
By establishing this hard cutoff, we protect developer ergonomics, clean up the CI pipeline, and immediately clear the runway to start targeting Hibernate 8 features. |
|
@borinquenkid until there is wider adoption or a dependency change that forces the retirement of 5.x, I do not see a need to preemptively retire the other code line. We should first release dual code lines and evaluate after. |
I think I did not make myself clear: G8 supports 5 and 7, minor patches to 5 no new features. Release quicly a G8.1 with new features and drop H5.. 5 is just our thing for this release. That way we can do things @jamesfredley way just to get it out the door and then cut off immediately and do things your way. Moving forward we should be able to just have one hibernate module once spring 5 comes in the horizon we will have a branch for H8 or whatever is relevant at the time. |
|
Our release policy forbids dropping support for a feature in a minor release |
Fair enough. So that would mean dropping H5 for Grails 9, SpringBoot 4.1 |
4 participants
Reopening. This PR replaces #15530
Please note that I split off a prerequisite PR #15654
This allows us to better see what changed between hibernate 5 & 7 - otherwise the hibernate 7 code looks like it was just added instead of changed. Commit a47d8cb is the original commit prior to this split if we need to compare this branch to that for any reason (mistakes, etc).