borinquenkid opened a new pull request, #16064: URL: https://github.com/apache/grails-core/pull/16064
# fix: preserve full transaction attribute state in GrailsTransactionAttribute copy constructors Follow-up to #16063 (`fix/customizable-rollback-tx-attribute-copy`), which fixed the same bug in `org.grails.datastore.mapping.transactions.CustomizableRollbackTransactionAttribute`. That PR's "known follow-up" section named this class as carrying the identical pattern; this PR is that fix. ## Problem `grails.gorm.transactions.GrailsTransactionAttribute` (the attribute type used by `GrailsTransactionTemplate`) had lossy copy constructors: - The `(TransactionAttribute)` overload copied only the five `TransactionDefinition` fields (propagation, isolation, timeout, readOnly, name) — **rollback rules, qualifier, labels, descriptor, and `timeoutString` were silently dropped**. - The `(TransactionDefinition)` overload had the same gap. - The `(RuleBasedTransactionAttribute)` overload called `super(other)` (so definition fields and rules were copied) but still dropped qualifier, labels, descriptor, and `timeoutString`. In practice, a `NoRollbackRuleAttribute` configured on an attribute passed into `GrailsTransactionTemplate` was ignored: every exception rolled back regardless of the configured rule. ## Fix Same pattern as the CRTA fix (`4042a87d54`), adapted to this class's `@CompileStatic` Groovy source: - `(TransactionAttribute)` now delegates to `(TransactionDefinition)`. - `(TransactionDefinition)` copies the five definition fields, then recovers the dynamic type via `instanceof`: `copyAttributeState` for `TransactionAttribute` sources, and a rollback-rules snapshot through a temporary `new RuleBasedTransactionAttribute(other).getRollbackRules()` — never the source's lazy getter, so the source is never mutated. - `(RuleBasedTransactionAttribute)` does `super(other)` + `copyAttributeState` + `copyGrailsState`. - New private helpers `copyAttributeState` (descriptor/timeoutString when source is `DefaultTransactionAttribute`, qualifier, defensive `new ArrayList<String>(labels)` copy) and `copyGrailsState` (`inheritRollbackOnly`, this class's only Grails-specific field). No other production files touched — `CustomizableRollbackTransactionAttribute` and `GrailsTransactionTemplate` are unmodified. ## Behavior change (release-note material) Same shape as the CRTA change: an application that passes its own rule-bearing `TransactionAttribute` into `GrailsTransactionTemplate` will now have those rules **honored** — an exception matching a `NoRollbackRuleAttribute` commits instead of rolling back. Previously the rules were silently dropped and every exception rolled back. One-directional: no scenario turns a commit into a rollback. ## Tests `GrailsTransactionAttributeSpec` (13 tests, modeled on `CustomizableRollbackTransactionAttributeSpec`): rule-list and labels deep-copy independence in both directions with explicit non-mutation-of-source assertions (`source.getRollbackRules().is(originalList)`), qualifier/labels/`inheritRollbackOnly` preservation, all five definition fields, descriptor/`timeoutString`, a plain `RuleBasedTransactionAttribute` source, rollback-on behavior (no-rollback rule honored, deepest rule wins, rollback-everything default), and the statically-dispatched `TransactionDefinition`/`TransactionAttribute` entry points via `@CompileStatic` private static helpers. No end-to-end `GrailsTransactionTemplate` rules test is included here: `GrailsTransactionTemplate`'s internal conversion to `CustomizableRollbackTransactionAttribute` is fixed on the sibling CRTA branch, not this one, so an end-to-end test would depend on both PRs merging together. Verified separately with both fixes applied. - `:grails-datamapping-core:test` — 306 tests, 0 failures. - `:grails-datamapping-core:codeStyle` — clean. 🤖 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]
