jamesfredley opened a new pull request, #15731:
URL: https://github.com/apache/grails-core/pull/15731

   ## Summary
   
   This PR expands the Hibernate 7 functional-test path and closes several 
parity gaps found while running the functional applications against Hibernate 
7. The branch targets `8.0.x-hibernate7` and includes the functional matrix 
work plus follow-up fixes, documentation, and review-gate cleanup.
   
   The main outcome is that Hibernate 5 and Hibernate 7 functional lanes can 
run from the same workflow while known H7-incompatible general projects remain 
explicitly excluded until their association-rendering, unique-constraint, and 
datasource gaps are fixed.
   
   ## Detailed issue coverage
   
   | Area | Issue | Why it is an issue | How it manifests | Fix in this PR |
   |---|---|---|---|---|
   | Functional CI matrix | General functional tests were not consistently 
exercised against both Hibernate 5 and Hibernate 7. | Hibernate 7 regressions 
could be hidden because the broad application test suite mostly validated the 
Hibernate 5 dependency path. | CI could publish artifacts even when the H7 
functional lane was not part of the publish dependency chain. | Adds the H5 and 
H7 functional jobs to the publish dependency gate and wires the functional-test 
configuration so H7 can substitute the Hibernate 7 BOM where appropriate. |
   | H7 dependency routing | General test applications need Hibernate 7 
constraints when `-PhibernateVersion=7` is selected. | Without BOM 
substitution, applications can still resolve Hibernate 5 aligned constraints or 
incompatible dependency combinations. | H7 functional runs fail during 
dependency resolution or boot with mismatched Hibernate and Micronaut platform 
dependencies. | Redirects the default BOM to `grails-hibernate7-bom` for 
eligible general functional tests and aligns the H7 Hibernate version to 
`7.2.7.Final`. |
   | Known H7-incompatible apps | `datasources`, `views-functional-tests`, and 
`scaffolding-fields` are not safe to run as generic H7 substitution projects 
yet. | Enabling them would turn known uncovered behavior into noisy CI failures 
instead of actionable coverage. | Datasources still needs explicit H7 
duplicates for some coverage; views/scaffolding boot with substitution but fail 
on association rendering and unique-constraint behavior. | Keeps those projects 
in `h7IncompatibleProjects` and documents the reason for each exclusion 
directly in `gradle/functional-test-config.gradle`. |
   | Forge analytics dependencies | Forge analytics Postgres was using the old 
Micronaut BOM coordinate. | The old coordinate does not match the platform 
coordinate used by current Micronaut dependency management. | H7 
functional/Forge dependency resolution can fail or pull inconsistent dependency 
constraints. | Switches to `io.micronaut.platform:micronaut-platform` and 
centralizes the Testcontainers version used by the Forge H7 path. |
   | H7 application boot | Some functional test apps still carried Hibernate 5 
specific configuration. | H5-specific cache/autoconfiguration prevents the same 
app from proving H7 behavior. | H7 app boot fails before specs can exercise 
GORM behavior. | Registers H7 autoconfiguration, removes H5-specific Ehcache 
assumptions, and updates H7 functional app configuration. |
   | Safe HQL usage | Some functional specs used ambiguous plain String HQL 
forms where the H7 API expects parameterized or safer overloads. | Hibernate 7 
query construction is stricter and the GORM API intentionally steers callers 
away from unsafe interpolation patterns. | `executeQuery` or related calls fail 
with unsupported or unsafe String HQL usage. | Updates affected specs to use 
safe HQL overloads and correct application class setup. |
   | DetachedCriteria single-result behavior | Hibernate 7 throws when 
`getSingleResult()` returns more than one row. | GORM's historical behavior is 
to return the first matching row for single-result helpers where multiple rows 
can match. | `DetachedCriteria.get()` can throw `NonUniqueResultException` in 
H7 where H5 returned one result. | Adjusts H7 query handling so single-result 
paths limit to one row or otherwise match the GORM contract. |
   | Managed collection identity | Hibernate 7 is stricter about multiple 
representations of the same collection. | GORM add-to/save flows must not leave 
duplicate collection wrappers in the session. | Cascade/add-to functional specs 
can fail with `Found two representations of same collection`. | Fixes the H7 
managed-entity add-to/save path so the bidirectional collection state remains 
consistent. |
   | Aggregate query result typing | H7 enforces selected result type more 
strictly than H5. | Aggregate HQL like `avg`, `max`, `min`, and `sum` does not 
return the domain entity type. | Data service aggregate queries fail with 
incorrect result type errors. | Routes aggregate/projection HQL through query 
creation that lets Hibernate return the correct scalar type. |
   | Cross-property arithmetic | H7 SQM typing is stricter for numeric property 
comparisons. | GORM where queries comparing differently typed numeric 
expressions need explicit type handling. | Expressions such as `pageCount > 
price * 10` can fail with coercion errors. | Updates the H7 where/query 
handling so cross-property arithmetic comparisons work under H7. |
   | `findWhere` result limiting | H7 `findWhere` did not reliably limit 
duplicate matches to a single row. | `findWhere` is a single-result convenience 
API and should not allow duplicate matches to produce multiple-row failures. | 
Duplicate rows matching the property map can throw or behave differently from 
H5. | Forces `findWhere` to set `max: 1` internally and adds SQL-capture 
coverage proving the generated query includes the row limit. |
   | Null values in `findWhere` and `findAllWhere` | The generated HQL treated 
null map values as named parameters. | In SQL/HQL, `= null` does not match null 
rows; it must be `is null`. | `findWhere(nullableProperty: null)` and 
`findAllWhere(nullableProperty: null)` fail to find rows with null values. | 
Generates `is null` for null values and removes those values from named 
parameters. |
   | Property-map query safety | Property names from `findWhere` and 
`findAllWhere` were interpolated into HQL without validation. | Property-map 
keys are caller input at the API boundary and should not become arbitrary HQL 
fragments. | Invalid or malicious keys could produce malformed HQL or unsafe 
generated query text. | Validates every key against the persistent entity 
before building HQL and tests invalid property names. |
   | `getAll` ordering with convertible ids | H7 converted ids before querying 
but reconstructed results with the original id values. | When callers pass 
string ids for numeric identifiers, the returned row map is keyed by converted 
ids. | `getAll(["3", "1", "2"])` can return nulls or the wrong order even 
though those rows exist. | Reconstructs the result list using converted ids and 
documents that existing ids preserve caller order. |
   | HQL query settings | H7 HQL helpers only partially applied common GORM 
query settings. | Users expect settings such as `fetchSize`, `timeout`, 
`readOnly`, `cache`, `max`, `offset`, `flushMode`, and `lock` to behave 
consistently. | String values for `max` or `offset` can fail casts, `cache` 
might not be applied, and `lock` was not filtered from named parameters. | 
Converts string/number/boolean setting values, applies all supported settings, 
maps `lock: true` to pessimistic write locking, disables query cache for locked 
queries, and filters `lock` from query parameters. |
   | TCK spec migration | Some migrated H7 TCK specs had missing imports or 
stale registration code. | The H7 TCK specs must compile and exercise the same 
public behavior as the source TCK specs. | Specs fail at compile time or 
register the wrong domain set. | Restores required `HibernateGormDatastoreSpec` 
imports, removes duplicate setup, and narrows Spock imports. |
   | H7 has-many support class | `Something.groovy` referenced `Book` without 
the required imported package. | Test support classes must compile in isolation 
under the H7 module. | Compilation can fail because `Book` is in 
`grails.gorm.tests.hasmany`, not the support class package. | Adds the explicit 
`Book` import. |
   | Multitenancy package scanning | The multitenancy spec scanned the spec 
package instead of the package containing the support domain and services. | 
The datastore only discovers classes in the package it scans. | Services or 
domain classes can be missing when the spec starts the datastore. | Imports the 
support classes explicitly and scans `Department.getPackage()`. |
   | Public documentation | The H7 behavior changes were not reflected in the 
user-facing docs. | Users need documented contracts for query settings, finder 
null handling, property validation, lock behavior, and `getAll` ordering. | The 
reference pages understate supported settings or omit the new null/order 
behavior. | Updates the Hibernate 7 guide and Grails reference docs for HQL 
settings, `lock`, `findWhere`, `findAllWhere`, and `getAll`. |
   | Historical bug report | `H7_GORM_BUG_REPORT.md` was useful triage context 
but lacked an Apache header and read like current expected failure status. | 
New source files need ASF licensing, and stale present-tense failure statements 
mislead reviewers. | RAT/compliance review would fail, and readers could think 
fixed failures are still expected. | Adds the Apache/SPDX Markdown header, 
frames the report as historical triage rationale, and normalizes dash 
typography. |
   
   ## Verification
   
   - `git diff --check origin/8.0.x-hibernate7...HEAD` completed with no output.
   - Dash typography search in `H7_GORM_BUG_REPORT.md` completed with no output.
   - Markdown LSP diagnostics for `H7_GORM_BUG_REPORT.md` found no diagnostics.
   - Java LSP diagnostics for `HqlQueryMethods.java` and `SelectHqlQuery.java` 
found no diagnostics.
   - Groovy/AsciiDoc LSP diagnostics are not authoritative in this workspace 
because the Groovy server does not have the Gradle classpath and there is no 
AsciiDoc LSP configured.
   - `./gradlew --no-daemon --no-parallel :grails-data-hibernate7-core:test 
--tests "org.grails.orm.hibernate.HibernateGormStaticApiFindWhereSpec" --tests 
"org.grails.orm.hibernate.HibernateGormStaticApiSpec" --tests 
"org.grails.orm.hibernate.query.HqlQueryMethodsSpec" --tests 
"org.grails.orm.hibernate.query.SelectHqlQuerySpec"` passed after cleaning 
`:grails-core`: 130 tests, 130 successes, 0 failures.
   - `./gradlew --no-daemon --no-parallel 
:grails-data-hibernate7-docs:asciidoctor :grails-doc:publishGuide -x 
aggregateGroovydoc` passed and built the user manual.
   
   ## Known follow-up work not included here
   
   | Project | Current state | Follow-up needed |
   |---|---|---|
   | `grails-test-examples-datasources` | Partial H7 coverage exists in 
`grails-test-examples/hibernate7/grails-multiple-datasources`, but not all 
datasource-switching and OSIV coverage has an H7 duplicate. | Add explicit H7 
duplicate specs before enabling the general project under H7 substitution. |
   | `grails-test-examples-views-functional-tests` | Boots with H7 substitution 
but has functional failures around association rendering. | Fix the rendering 
behavior or add H7-specific coverage before removing it from 
`h7IncompatibleProjects`. |
   | `grails-test-examples-scaffolding-fields` | Boots with H7 substitution but 
has Geb failures around association rendering and unique-constraint behavior. | 
Fix those H7 behavior gaps before enabling the general project under H7 
substitution. |
   


-- 
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]

Reply via email to