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

   ## Description
   
   ### Cause
   With GORM for Hibernate 7, `ManyToOneValuesBinder` bound every many-to-one 
without a fetch mapping with `FetchMode.DEFAULT`. When Hibernate cannot create 
a proxy for the associated entity, 
`FetchOptionsHelper.determineFetchStyleByMetadata` resolves `DEFAULT` to a 
`JOIN` fetch style. `determineFetchTiming` then finds `JOIN` on a lazy 
association and logs HHH000510 at startup, once for every such association.
   
   ### Fix
   As suggested in the issue, when no fetch mode is mapped, a lazy many-to-one 
is now bound with `FetchMode.SELECT`, the fetch mode that matches lazy loading. 
Eager many-to-ones (`lazy: false`) and many-to-ones with a mapped fetch mode 
are bound as before.
   
   ### Loading behaviour
   I checked that the change does not alter what gets loaded, both against 
Hibernate 7.4.10 (`FetchOptionsHelper`, `MappingModelCreationHelper`) and with 
the specs below:
   - **The associated entity can be proxied (the usual case).** `DEFAULT` 
already resolved to a `SELECT` fetch style, so nothing changes, and the 
association is still an uninitialized proxy after loading its owner.
   - **The associated entity cannot be proxied** (Hibernate logs HHH000305 
first). This is the only case that logged HHH000510. The association was 
already loaded together with its owner, because Hibernate has no proxy to use 
in its place, and it still is. The only thing removed is the 
`JOIN`-on-a-lazy-association combination that produced the warning.
   - **The associated entity is batch loadable.** Hibernate resolves it to 
`BATCH` whether the mapping is `DEFAULT` or `SELECT`.
   
   The test datastore proxies entities, so the spec reproduces the warning with 
an associated entity that has a `final` getter, which Hibernate cannot proxy. I 
did not reproduce the specific reason the associated entities in the issue's 
application have no proxy.
   
   ### Coverage
   `ManyToOneValuesBinderSpec`:
   - The existing data table now expects `SELECT` for a lazy many-to-one 
without a fetch mapping, and has a new row for an eager one, which keeps 
`DEFAULT`.
   - New: binds real domain classes and checks both the bound fetch mode and 
the fetch style Hibernate resolves. The cases are a lazy many-to-one, a lazy 
many-to-one to an entity that cannot be proxied (the HHH000510 case), a `lazy: 
false` one, and a `fetch: 'join'` one.
   - New: loads a domain instance through GORM and checks that the lazy 
association is still an uninitialized proxy and the other three are loaded.
   
   Without the fix, the three lazy cases fail. The loading spec passes both 
with and without the fix, which shows loading is unchanged.
   
   Run locally (JDK 21):
   - `./gradlew :grails-data-hibernate7-core:test`: of 3,777 tests, all passed 
except the Oracle variants of `SqlProjectionHibernate7Spec` and 
`SqlRestrictionHibernate7Spec`. Their Oracle container was OOMKilled at startup 
on my machine, where Docker has 7.7 GB and the database containers start in 
parallel. Both specs pass for every database, Oracle included, when run on 
their own.
   - `./gradlew codeStyle` and `./gradlew aggregateViolations --continue`: no 
Checkstyle, CodeNarc, PMD or repository-conventions violations. RAT flagged 
only two git-ignored files that an embedded Neo4j database had written to my 
working tree, not files in this change.
   - Not run: `./gradlew build --rerun-tasks` for the whole repository.
   
   No documentation change. The only behaviour users can see is that the 
warning is gone.
   
   Fixes #16659
   
   **Use of generative AI:** the change, the specs and this description were 
prepared with [Claude Code](https://claude.com/claude-code) (Claude Opus 5.5), 
and the commit carries a `Co-Authored-By` trailer for it, per the ASF policy on 
generative tooling.
   
   ## Contributor Checklist
   
   Please review the following checklist before submitting your pull request. 
Pull requests that do not meet these requirements may be closed without review.
   
   ### Issue and Scope
   
   - [x] This PR is linked to an existing issue that has been **acknowledged or 
approved** by the project team. If no approved issue exists, please give 
background on why this change is necessary.  Tickets are preferred for release 
change log history.
   - [x] This PR addresses the **complete scope** of the linked issue. Partial 
implementations or unfinished work should not be submitted for review.
   - [x] This PR contains a **single, focused change**. Unrelated changes 
should be submitted as separate pull requests.
   - [x] This PR targets the **correct branch** for the type of change:
       - **Patch release branches** (e.g., `7.0.x`): Bug fixes only. No new 
features or API changes.
       - **Minor release branches** (e.g., `7.1.x`): New features are welcome, 
but breaking existing APIs must be avoided.
       - **Major release branches** (e.g., `8.0.x`): Reserved for major 
changes. Breaking API changes are permitted.
   
   ### Code Quality
   
   - [x] I have **added or updated tests** that cover the changes introduced in 
this PR. All code contributions are expected to include appropriate test 
coverage.
   - [ ] I have verified that all existing tests pass by running `./gradlew 
build --rerun-tasks`. *(Ran the full `grails-data-hibernate7-core` suite 
instead; see Coverage.)*
   - [x] My code follows the project's **code style** guidelines. I have run 
`./gradlew codeStyle` and resolved any violations. See [Code 
Style](../CONTRIBUTING.md#code-style) for details.
   - [x] This PR does **not** include mass reformatting, style-only changes, or 
large-scale refactoring unless it was **explicitly approved** in the linked 
issue. Unsolicited reformatting will not be accepted.
   - [x] If generative AI tooling was used in preparing this contribution, a 
quality model was used to ensure contributions are **consistent with the 
project's quality standards**.
   
   ### Licensing and Attribution
   
   - [x] All contributed code is provided under the [Apache License 
2.0](https://www.apache.org/licenses/LICENSE-2.0), and new source files include 
the appropriate **Apache license header**.
   - [ ] I have the necessary rights to submit this contribution and confirm it 
is my own original work (see [Legal 
Notice](../CONTRIBUTING.md#i-want-to-contribute)).
   - [x] If generative AI tooling was used in preparing this contribution, I 
have followed the [Apache Software Foundation's policy on generative 
tooling](https://www.apache.org/legal/generative-tooling.html) and have 
properly attributed its use.
   
   ### Documentation
   
   - [x] If this PR introduces user-facing changes, I have included or updated 
the relevant documentation.
   - [x] If this PR adds a new feature, I have updated the **What's New** 
section of the Grails Guide.
   - [x] If this PR introduces breaking changes or changes that require user 
action during an upgrade, I have updated the **Upgrade Notes** for the 
corresponding version in the Grails Guide.
   - [x] The PR description clearly explains **what** was changed and **why**.
   
   ---
   
   > **First-time contributors:** Please read our [Contributing 
Guide](../CONTRIBUTING.md) before submitting.
   > Pull requests that appear to be auto-generated, incomplete, or unrelated 
to an approved issue may be
   > closed to help maintainers focus on reviewed and planned work. We 
appreciate your understanding.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


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