fix(unique-fields): remove @CloseDBIfOpened to restore atomicity with contentlet save

[fabrizzio-dotCMS]

Problem

Closes #35566

DBUniqueFieldValidationStrategy.innerValidate() was annotated with both @WrapInTransaction and @CloseDBIfOpened. The combination has a critical side-effect:

• @CloseDBIfOpened closes any existing JDBC connection and opens a fresh one, isolated from the caller's transaction.
• @WrapInTransaction then starts and commits a new transaction on that fresh connection.

This means the INSERT INTO unique_fields commits independently of the outer contentlet save transaction. If the outer transaction is later rolled back (e.g., a failure mid-save, or a startup migration task that rolls back), the unique_fields row persists as a permanent orphan — a hash pointing to a contentlet that no longer exists.

Outer transaction (contentlet save):
  ├── INSERT contentlet          → connection A (outer tx)
  ├── innerValidate()
  │   ├── @CloseDBIfOpened opens connection B
  │   ├── INSERT unique_fields   → connection B ← COMMITS HERE ✗
  │   └── closes connection B
  └── ROLLBACK outer tx
       ├── contentlet rolled back ✓
       └── unique_fields row stays ✗  (already committed on B)

On the next contentlet save or migration run, findVariable() returns empty (no contentlet exists), but publish() / save() hits a duplicate-key violation on unique_fields_pkey, incorrectly blocking a valid operation.

Fix

Remove @CloseDBIfOpened from innerValidate() and innerValidateInPreview().

With only @WrapInTransaction, the interceptor joins the caller's existing transaction if one is active. The unique_fields INSERT and the contentlet save are now atomic — a rollback of the outer transaction rolls back both.

When no outer transaction is present, @WrapInTransaction starts its own, preserving the existing behavior.

Outer transaction (contentlet save):
  ├── INSERT contentlet          → connection A (outer tx)
  ├── innerValidate() (@WrapInTransaction joins outer tx)
  │   └── INSERT unique_fields   → connection A (same tx) ✓
  └── COMMIT/ROLLBACK outer tx
       ├── contentlet committed/rolled back ✓
       └── unique_fields committed/rolled back ✓  (atomic)

Impact

• Prevents new orphaned unique_fields rows from being created on any contentlet save rollback.
• Existing orphans in the database are not cleaned up by this PR (a separate cleanup utility or migration would be needed for that).
• The companion PR fix(lang-var-migration): use JDBC savepoint to isolate unique_fields failures #35565 addresses the cascade blast radius caused by existing orphans in the language variable migration.

Files changed

• dotCMS/src/main/java/com/dotcms/contenttype/business/uniquefields/extratable/DBUniqueFieldValidationStrategy.java
  • Removed @CloseDBIfOpened from innerValidate() and innerValidateInPreview()
  • Removed unused import com.dotcms.business.CloseDBIfOpened

Test plan

• Save a contentlet with a unique field — verify unique_fields row is inserted
• Roll back the save (e.g., via an outer transaction) — verify the unique_fields row is also absent after rollback
• Run existing DBUniqueFieldValidationStrategy integration tests — no regressions
• Verify that duplicate-key enforcement still works correctly after the change

🤖 Generated with Claude Code

[claude]

Claude finished @fabrizzio-dotCMS's task in 56s —— View job

---

Review

• Read changed file and diff
• Analyze transaction semantics
• Review for correctness, risks, and missing cases

---

The fix is correct. WrapInTransactionHandler.onEnter() confirms the join-or-start semantics: isLocalTransaction = txOps.startLocalTransactionIfNeeded() — if an outer transaction already owns the connection, isLocalTransaction is false and neither commit nor rollback fires. Removing @CloseDBIfOpened is the right call.

---

No test for the rollback scenario. The entire justification for this change is that a rolled-back outer transaction should also roll back the unique_fields row. But DBUniqueFieldValidationStrategyTest has no test covering this. The test plan is a checklist, not an implemented test. This is the one thing that should have shipped with the PR — a test that opens a transaction, calls innerValidate, rolls the transaction back, and asserts the unique_fields table is empty afterward.

innerValidateInPreview gets @WrapInTransaction for a read-only method. It calls uniqueFieldDataBaseUtil.get() and then checkUniqueFieldDuplication() — no writes. @WrapInTransaction on a read-only path is harmless but unnecessarily acquires a connection/transaction context. This is pre-existing; the annotation was there before this PR. Not a blocker, but worth cleaning up at some point.

afterSaved, afterPublish, afterUnpublish, cleanUp have no transaction annotations. They write to unique_fields and depend entirely on the caller's transaction context. This was the same before this PR and isn't a regression — but it means the atomicity guarantee only holds for the innerValidate path. Any caller invoking afterSaved or cleanUp outside a transaction will still produce orphans on failure. Not blocking this PR, but the invariant should be documented.

isDuplicatedKeyError hardcodes a PostgreSQL error string ("ERROR: duplicate key value violates unique constraint \"unique_fields_pkey\""). Pre-existing issue not introduced here.

---

Summary: The code change is correct and the analysis in the PR description is accurate. The main gap is the missing integration test for rollback atomicity — the entire premise of the fix — which leaves the behavior unverified by CI.