fix: preserve full transaction attribute state in CustomizableRollbackTransactionAttribute copy constructors

[borinquenkid]

fix: preserve full transaction attribute state in CustomizableRollbackTransactionAttribute copy constructors

Split out of the GormRegistry consolidation per review discussion on #15779 (this class's copy semantics are an independent bug fix and deserve their own review).

Problem

The copy constructors of CustomizableRollbackTransactionAttribute were lossy:

• The TransactionAttribute/TransactionDefinition overloads copied only the five TransactionDefinition fields (propagation, isolation, timeout, readOnly, name) — rollback rules, qualifier, labels, descriptor, and timeoutString were silently dropped. In practice this meant a NoRollbackRuleAttribute configured on an attribute passed into GrailsTransactionTemplate or TransactionService.withTransaction/withNewTransaction was ignored: every exception rolled back regardless.
• The RuleBasedTransactionAttribute overload copied almost nothing — not even the definition fields — because it never called super(other).
• The previous implementation's copyFrom helper also mutated the source object: calling getRollbackRules() on a RuleBasedTransactionAttribute lazily installs a new list into it, and setLabels(other.getLabels()) aliased the label collection between source and copy.
• Two trace log statements in this .java file used GString-style placeholders ("$ex", "$winner") that never interpolate in Java.

Fix

• The RuleBasedTransactionAttribute/CustomizableRollbackTransactionAttribute overloads now delegate to Spring's own copy constructor via super(other), which snapshots the rule list from the field (never the lazily-materializing getter).
• The TransactionDefinition/TransactionAttribute overloads copy the definition fields and then recover the dynamic type: rules are snapshotted through a temporary RuleBasedTransactionAttribute copy (so the source is never mutated), and attribute-level state is copied explicitly.
• descriptor, timeoutString, and qualifier are carried over explicitly — as of Spring Framework 7, DefaultTransactionAttribute(TransactionAttribute) copies only the TransactionDefinition fields, so these would otherwise be lost.
• labels gets a defensive copy (setLabels stores the given reference).
• connection and inheritRollbackOnly survive every copy path when the source is a CustomizableRollbackTransactionAttribute.
• Log placeholders fixed; the remaining unguarded trace call is now behind isTraceEnabled().

Behavior change (release-note material)

@Transactional / @ReadOnly / @Rollback semantics are unchanged — the AST transform builds the attribute directly and never goes through these copy constructors.

For programmatic API users: an application that passes its own rule-bearing TransactionAttribute (e.g. a RuleBasedTransactionAttribute with NoRollbackRuleAttributes) into DomainClass.withTransaction(definition), TransactionService.withTransaction(definition), or withNewTransaction(definition) will now have those rules honored: an exception matching a no-rollback rule commits the transaction (the exception still propagates). Previously the rules were silently dropped and every exception rolled back. The change is one-directional — no scenario turns a commit into a rollback; with no matching rule the class still rolls back on every exception, checked or unchecked, as documented.

Suggested upgrade note:

Rollback rules configured on a TransactionAttribute passed to the programmatic transaction APIs (withTransaction, withNewTransaction, GrailsTransactionTemplate) are now honored. Previously noRollbackFor-style rules supplied this way were silently ignored and every exception caused a rollback.

Tests

• CustomizableRollbackTransactionAttributeSpec (13 tests): deep-copy independence of rule lists and labels in both directions, non-mutation of the source's internal rule list, preservation of every definition- and attribute-level property across all four constructor dispatch paths (including statically-dispatched TransactionDefinition/TransactionAttribute entry points via @CompileStatic helpers), and the rollback-on-everything default.
• TransactionRollbackRulePropagationSpec (4 tests): behavior through the public APIs — GrailsTransactionTemplate.execute and DefaultTransactionService.withNewTransaction against a real H2 DataSourceTransactionManager — asserting commit-vs-rollback outcomes and REQUIRES_NEW propagation.

All fix-specific assertions fail against the previous implementation (verified — the tests are not vacuous).

Known follow-up (out of scope)

grails.gorm.transactions.GrailsTransactionAttribute (grails-datamapping-core) and org.grails.transaction.GrailsTransactionAttribute (grails-core) contain the same lossy copy-constructor pattern on their TransactionAttribute/TransactionDefinition overloads. Fixing them belongs in a separate change with its own tests.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Fixes lossy copy-constructor behavior in CustomizableRollbackTransactionAttribute so that rule-bearing TransactionAttribute/TransactionDefinition inputs preserve rollback rules and attribute metadata (qualifier/labels/descriptor/timeoutString), and so copying does not mutate the source object. This aligns programmatic transaction APIs (e.g., withTransaction, withNewTransaction, GrailsTransactionTemplate) with the caller-provided rollback semantics.

Changes:

• Reworked CustomizableRollbackTransactionAttribute copy constructors to preserve full attribute state (including rollback rules, qualifier/labels, descriptor/timeoutString, and Grails-specific fields).
• Fixed Java trace logging placeholders and guarded the remaining trace call.
• Added unit + integration-style Spock specs to validate deep-copy behavior and end-to-end commit/rollback outcomes via public APIs.

Reviewed changes

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

File	Description
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/transactions/CustomizableRollbackTransactionAttribute.java	Makes copy construction non-lossy and non-mutating; preserves Spring attribute state + Grails custom fields; fixes trace logging.
grails-datastore-core/src/test/groovy/org/grails/datastore/mapping/transactions/CustomizableRollbackTransactionAttributeSpec.groovy	Adds comprehensive unit coverage for copy semantics, defensive copies, and rollback rule behavior.
grails-datamapping-core/src/test/groovy/grails/gorm/transactions/TransactionRollbackRulePropagationSpec.groovy	Verifies rollback-rule propagation through public programmatic transaction APIs using a real DataSourceTransactionManager.

---

💡 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, "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. (Confirmed the same two tests fail identically on #16064 and #16065, which are also based on 8.0.x.)

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: a1594a5
▶️ Tests: 15836 executed
⚪️ Checks: 60/60 completed

---

Learn more about TestLens at testlens.app.