MartinJesusDev commented on PR #16625: URL: https://github.com/apache/grails-core/pull/16625#issuecomment-6178478488
Updated the branch with the review follow-up. Three commits on top of `776c6d948c`: - `72c44111e5` `test: exercise the built proxy and the lazy association for trait default values` - the regression spec (h5 + h7) now initializes the loaded proxy through dynamic dispatch, reads the trait defaults off the loaded target, unwraps it through the proxy handler, and covers a lazy many-to-one to an entity whose trait declares default values. - `49fa7e809e` `test: cover the shared ByteBuddyState wiring in GrailsBytecodeProviderSpec` - the tautological assertion is gone; two factories built from the same provider build and instantiate the proxy for the same entity and share one proxy class, observed through `getProxy(...)` on a real session instead of reflecting on the private field. - `87a3f936e5` `fix: reuse the Groovy proxy wrapper per entity and class loader on Hibernate 5` - the wrapper is cached in a static `TypeCache.WithInlineExpunction<TypeCache.SimpleKey>`. One note on the key: Hibernate 5.6 names generated proxy classes randomly and Grails builds a new provider per SessionFactory, so the stock class never repeats and keying on it would never hit; the cache is keyed on (entity, interfaces) and skips regenerating the stock class on a hit, which is what actually yields one wrapper per entity and class loader - the outcome Hibernate 7 gets from its shared `ByteBuddyState`. Verified locally on the new head: `grails-data-hibernate5-core` 1093 tests / 0 failures, `grails-data-hibernate7-core` 3660 / 0 failures, the spring-security ACL integration app, `codeStyle`, `rat` and `validateRepositoryConventions`. The full-repo `aggregateViolations` reports are clean (Checkstyle, CodeNarc, PMD, repository conventions; SpotBugs is disabled). The only test failures in that run were locale/timezone artifacts of this machine (`LANG=es_ES`, Madrid timezone against CI's en-US/UTC) in modules unrelated to the change; all of them pass with `LC_ALL=C.UTF-8 TZ=UTC`, including a full re-run of `app1` (711 tests / 0 failures). While covering the deserialization path we found two pre-existing issues outside this change, now described in the PR body: on Hibernate 5 `SerializableProxy.readResolve()` uses the global bytecode provider (stock proxy, so the original #16600 failure remains for these entities); on Hibernate 7 it casts the provider to `BytecodeProviderImpl`, which `GrailsBytecodeProvider` is not, so deserializing a proxy throws `IllegalStateException`. How would you like them tracked - two separate issues (one per module), a single issue, or should the Hibernate 7 provider type be addressed in this PR? I did not want to widen the change without your call. (The Windows shard 1 failure on the previous head was a Maven Central 403 during plugin resolution, with no test failures; this push re-runs CI.) -- 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]
