jamesfredley commented on PR #16299: URL: https://github.com/apache/grails-core/pull/16299#issuecomment-5534506436
Independent check of the review feedback (not taking the review text as given): The Hibernate 5 factory **was** looking up a bean named `default` (`ConnectionSource.DEFAULT`). The DataSource bean is `dataSource`, so `setApplicationContext` never ran on the reporter's Hibernate 5 path. That claim was valid. `setDataSourceConnectionSource` already preferred a restart TCCL, so swapping `CLASSLOADERS` alone on that else-branch was equivalent. The real Hibernate 5 hole was taking the wrong setter, which left `CLASSLOADERS` on `connectionSource.getClass().getClassLoader()` (the base loader) whenever TCCL was not already a `RestartClassLoader`. Hibernate also re-resolves entity `Class` objects by name during SessionFactory construction (`ReflectHelper.classForName` uses TCCL). That is why this follow-up sets TCCL around `super.buildSessionFactory`, not only `AvailableSettings.CLASSLOADERS`. Updates in c8945fd6c3: - Hibernate 5 factory looks up `dataSource` / `dataSource_<name>` and sets `dataSourceName` **before** `setApplicationContext`, so named sources inject `dataSource_secondary` rather than the default DataSource. - Hibernate 5 and 7 use `preferRestartClassLoader`, re-resolve at SessionFactory build, and wrap TCCL around `super.buildSessionFactory` (restored in `finally`). - `CLASSLOADERS` is left unset when the application context loader is null and DevTools is not active. - Helper: FQCN match first, case-sensitive simple-name fallback, descendant-loader guard, `resolve()` deprecated. - Tests for factory bean wiring, named DataSource injection, loader identity, null-loader fallback, and `resolveSessionFactoryClassLoader`. - Docs: dropped the `Role.count()` symptom sentence. Targeted tests passed in `grails-datastore-core`, `grails-data-hibernate5-core`, and `grails-data-hibernate7-core`. Module `codeStyle` is clean. Review threads from that pass are resolved. I did not run a generated Forge app with `gorm-hibernate5` + spring-security-core + DevTools end-to-end in this follow-up; the factory bean-name miss and TCCL wrap are covered by unit tests against the actual bootstrap path. -- 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]
