fix: preserve full transaction attribute state in GrailsTransactionAttribute (web) copy constructors

[borinquenkid]

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

[Copilot]

Pull request overview

Fixes lossy copy-constructor behavior in org.grails.transaction.GrailsTransactionAttribute so that copy construction preserves transaction attribute state beyond the five TransactionDefinition fields (notably rollback rules, qualifier/labels, descriptor, and timeoutString), matching the intent described in the PR metadata and aligning behavior with the earlier related fixes.

Changes:

• Make GrailsTransactionAttribute(TransactionAttribute) delegate to the TransactionDefinition constructor and enhance the TransactionDefinition constructor to preserve attribute state (incl. rollback rules snapshotting without mutating the source).
• Ensure the RuleBasedTransactionAttribute copy path also preserves qualifier/labels/descriptor/timeoutString and Grails-specific inheritRollbackOnly.
• Add a comprehensive Spock spec covering deep-copy independence, non-mutation of sources, definition/attribute state preservation, and rollbackOn behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
grails-core/src/main/groovy/org/grails/transaction/GrailsTransactionAttribute.java	Updates copy constructors to preserve full attribute state (rollback rules, qualifier/labels, descriptor/timeoutString, Grails state) and fixes/guards trace logging.
grails-core/src/test/groovy/org/grails/transaction/GrailsTransactionAttributeSpec.groovy	Adds targeted regression tests for copy semantics, defensive copying, and rollback rule behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[borinquenkid]

The TestLens failures for GrailsUtilStackFiltererSpec > installed DefaultStackTraceFilterer emits Full Stack Trace by default and GrailsBootstrapRegistryInitializerSpec > defaults logFullStackTraceOnFilter to true on the promoted DefaultStackTraceFilterer are a pre-existing bug on 8.0.x, unrelated to this PR's changes -- reproducible on plain 8.0.x today. DefaultStackTraceFilterer.STACK_LOG routes through a jcl-over-slf4j commons-logging binding, so the tests' System.setErr() capture never observes the emitted message.

Fix: #16067 (replaces the System.err capture with a Logback appender attached directly to the logger). Once that merges, rebasing onto 8.0.x should clear this failure here.

[borinquenkid]

The CI failures on this PR (Build Grails-Core failing on ubuntu-latest/macos-latest/windows-latest, "Rerunning all Tasks") are pre-existing on 8.0.x and unrelated to this change. Both failures are the same root cause:

• GrailsBootstrapRegistryInitializerSpec > defaults logFullStackTraceOnFilter to true on the promoted DefaultStackTraceFilterer
• GrailsUtilStackFiltererSpec > installed DefaultStackTraceFilterer emits Full Stack Trace by default

Both assert on a System.err-backed ByteArrayOutputStream capture of the stack trace filterer's output, but 8.0.x now routes that output through a Logback appender instead of System.err, so the capture is empty.

This branch depends on #16067 (fix: capture STACK_LOG output via a Logback appender instead of System.err) merging into 8.0.x first. Once that lands, I'll rebase this branch onto the updated 8.0.x and CI should go green.

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: 87e23d4
▶️ Tests: 57593 executed
⚪️ Checks: 60/60 completed

---

Learn more about TestLens at testlens.app.