borinquenkid opened a new pull request, #16065: URL: https://github.com/apache/grails-core/pull/16065
# 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 `org.grails.transaction.GrailsTransactionAttribute` (grails-core) had the same lossy copy constructors as its GORM-module namesake and the CRTA class: - The `(TransactionAttribute)` overload copied only the five `TransactionDefinition` fields — **rollback rules, qualifier, labels, descriptor, and `timeoutString` were silently dropped**. - The `(TransactionDefinition)` overload had the same gap. - The `(RuleBasedTransactionAttribute)` overload called `super(other)` (copying definition fields and rules) but still dropped qualifier, labels, descriptor, and `timeoutString`. - Two trace log statements used GString-style placeholders (`"$ex"`, `"$winner"`) that never interpolate in `.java` source, and a third trace statement logged unconditionally without an `isTraceEnabled()` guard. This class has no in-repo constructor callers — `grep`ing `new GrailsTransactionAttribute(` across the whole repo turns up only `GrailsTransactionTemplate.groovy:55`, which resolves to the same-package GORM twin (`grails.gorm.transactions.GrailsTransactionAttribute`, fixed separately in #16064), not this class. This class is public API consumed by downstream plugins and applications that construct or extend it directly. ## Fix Same pattern as the CRTA fix (`4042a87d54`): - `(TransactionAttribute)` now delegates to `(TransactionDefinition)`. - `(TransactionDefinition)` copies the five definition fields, then recovers the dynamic type via pattern-matching `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<>(labels)` copy) and `copyGrailsState` (`inheritRollbackOnly`, this class's only Grails-specific field — it has no `connection` field, unlike the CRTA class). - Log placeholders fixed to string concatenation; the previously-unguarded trace call is now behind `isTraceEnabled()`. No other production files touched. ## Behavior change (release-note material) Same shape as the CRTA change: downstream code that passes its own rule-bearing `TransactionAttribute` into this class's copy constructors 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. Since this class has no in-repo callers, the practical impact is scoped to plugin/application code outside this repository. ## Tests `GrailsTransactionAttributeSpec` (13 tests, modeled on `CustomizableRollbackTransactionAttributeSpec`): rule-list and labels deep-copy independence in both directions with explicit non-mutation-of-source assertions, 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. - `:grails-core:test` — 475 tests, 0 failures. - `:grails-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]
