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

   **Step 1 (PR-B) prerequisite for Hibernate 7 work** - extracted from the 
staging branch so it can be reviewed on its own merits.
   
   ## Context
   
   This change was originally pulled forward into the hibernate7 staging branch 
as commit `e3cad414db` on PR #15654. Reviewers (@matrei, @sbglasius) correctly 
pointed out it is unrelated to the Hibernate 7 clone and should be a standalone 
PR. Extracting it here against `8.0.x` so it can land on its own and then flow 
naturally into both the staging branch (PR #15654) and Step 2 (PR #15568).
   
   ## Scope
   
   Nine files in `grails-async/` with defensive coding improvements:
   
   ### 1. Null-safety guards
   
   - [`AbstractPromiseFactory.createPromise(List, 
List)`](grails-async/core/src/main/groovy/grails/async/factory/AbstractPromiseFactory.groovy):
 return an empty `PromiseList` when `closures` is `null` rather than NPE on 
`.size()`.
   - [`FutureTaskPromise.set(T)` and 
`setException(Throwable)`](grails-async/core/src/main/groovy/org/grails/async/factory/future/FutureTaskPromise.groovy):
 null-check `successCallbacks` and `failureCallbacks` before 
synchronizing/iterating.
   
   ### 2. Exception handling
   
   - 
[`DelegateAsyncTransformation.getTransformer()`](grails-async/core/src/main/groovy/org/grails/async/transform/internal/DelegateAsyncTransformation.java):
 catch `Exception` (renamed to `ignored`) instead of `Throwable`. Catching 
`Throwable` swallows `Error`s that should propagate (`OutOfMemoryError`, 
`StackOverflowError`, etc.) and is widely considered bad style. @sbglasius 
called this out explicitly in his review of the staging PR.
   
   ### 3. Static analysis hints
   
   - Add `@Override` annotations on `DelegateAsyncTransformation.visit` and 
`NoopDelegateAsyncTransactionalMethodTransformer.transformTransactionalMethod`.
   - Use constant-on-left equality (`VOID.equals(name)` instead of 
`name.equals(VOID)`) to avoid NPE if name is null.
   - Change `copyParameters(Parameter[])` to varargs 
`copyParameters(Parameter...)` for caller convenience.
   
   ## Why a separate PR
   
   PR #15654 (Step 1) is meant to be a near-pure clone of `hibernate5` → 
`hibernate7`. PR #15568 (Step 2) is the actual Hibernate 7 logic. Both 
reviewers asked for cleanup of this nature to be split out so the review effort 
on Step 1 and Step 2 can stay focused on hibernate-related diffs.
   
   Authorship preserved from the original commit by @jdaugherty.
   
   ## Related
   
   - Step 1 PR-A: #15654 (the hibernate7 clone, already updated to remove 
`logback.groovy` and the unrelated `stepByStep.adoc` change)
   - Step 1 PR-C: MongoDatastoreSpec base class + spec refactor (forthcoming)
   - Step 1 PR-D: DetachedCriteriaSpec TCK style cleanup (forthcoming)
   - Step 2: #15568 (the actual Hibernate 7 logic - blocked on Step 1 
prerequisites)
   


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