Create CDI Interceptors to handle DB Annotations on Bean classes by spbolton · Pull Request #34946 · dotCMS/core

spbolton · 2026-03-11T13:31:35Z

Why: CDI Beans Need Working Interceptors

ByteBuddy annotations (@WrapInTransaction, @CloseDBIfOpened, etc.) fail silently on CDI beans. Weld proxies don't copy method annotations, so ByteBuddy's class matcher (declaresMethod(isAnnotatedWith(...))) never sees them. This was the root cause of the connection leak fixed in #34134, and affects 7+ known classes including UniqueFieldDataBaseUtil, JobQueueManagerAPIImpl, and MetricsAPIImpl.

Adding CDI interceptor bindings ensures these annotations work correctly on @ApplicationScoped beans — preventing silent failures, connection leaks, and transaction boundary violations.

Secondary Benefits

DRY shared logic into handlers — The same transaction/connection logic previously lived in both ByteBuddy advice classes and inline call sites. Extracting it into reusable handler classes (WrapInTransactionHandler, CloseDBIfOpenedHandler, etc.) means the CDI interceptors and ByteBuddy advice share a single implementation.

Clean ByteBuddy advice and SPI abstraction — Decoupling the interceptor layer from core implementations via SPI interfaces (DatabaseConnectionOps, TransactionOps, LicenseOps) enables future extraction to a utility module with no core dependency. This also opens the door to build-time annotation processing, which would improve startup performance and unblock code coverage analysis currently hindered by ByteBuddy's runtime instrumentation.

Proposed Changes

Add CDI interceptors for all 6 annotations (WrapInTransactionInterceptor, CloseDBIfOpenedInterceptor, CloseDBInterceptor, ExternalTransactionInterceptor, LogTimeInterceptor, EnterpriseFeatureInterceptor)
Mark annotations with @InterceptorBinding to enable CDI interception
Extract shared logic from ByteBuddy advice into reusable handler classes
Create SPI interfaces (DatabaseConnectionOps, TransactionOps, LicenseOps, InterceptorLogger) to decouple from core
Implement core SPI providers (CoreDatabaseConnectionOps, CoreTransactionOps, etc.)
Update ByteBuddy advice classes to delegate to handlers
Register CDI interceptors in beans.xml
Simplify LocalTransaction and DbConnectionFactory to use new handlers

Compatibility

100% behavioral compatibility maintained. ByteBuddy continues to handle non-CDI classes; CDI interceptors handle CDI beans. Both use the same handlers and lifecycle guards to prevent double-processing.

Checklist

No new tests required — existing ByteBuddy and transaction tests cover the refactored logic
Translations
Security Implications Contemplated

Additional Info

https://claude.ai/code/session_01EFJR5jtJLuoAYBNef4aAPs

This PR fixes: #34482

…o match DbConnectionFactory DbConnectionFactory.closeAndCommit() throws DotDataException but the new DatabaseConnectionOps interface and CoreDatabaseConnectionOps implementation declared the method without any checked exception, causing a compilation failure. Propagate the exception through the interface, implementation, and CloseDBHandler.onExit() — all callers already handle checked exceptions. Includes all changes from PR #34946 (refactor interceptors to DRY shared logic into handlers). https://claude.ai/code/session_018LvtrjfQSzLxzEyG7RRCcy

wezell · 2026-03-12T01:16:04Z

Uggh. Of course it was this.

https://dotcms.slack.com/archives/C096E0QAM7G/p1770064096388029

https://dotcms.slack.com/archives/C096E0QAM7G/p1772468621287769?thread_ts=1772468558.957369&cid=C096E0QAM7G

wezell

See my comments.

@logtime

…ations #34482 ByteBuddy annotations fail silently on @ApplicationScoped CDI beans because Weld proxies don't copy method annotations. The class matcher using declaresMethod(isAnnotatedWith(...)) cannot detect annotations on proxy classes, causing connection leaks. This adds CDI interceptor bindings for all 6 annotations: - @CloseDBIfOpened, @WrapInTransaction, @CloseDB, @ExternalTransaction, @EnterpriseFeature, @logtime Each annotation now carries @InterceptorBinding (with @Nonbinding on members) and has a corresponding CDI interceptor in com.dotcms.business.cdi that replicates the ByteBuddy advice logic using @AroundInvoke. A ThreadLocal-based InterceptorGuard prevents double-processing when both ByteBuddy (non-CDI classes) and CDI interceptors (managed beans) are active on the same call chain. https://claude.ai/code/session_01EFJR5jtJLuoAYBNef4aAPs

…tion Introduces an SPI layer (com.dotcms.business.interceptor) that decouples ByteBuddy advice and CDI interceptors from core classes like DbConnectionFactory, HibernateUtil, LicenseUtil, Config, and Logger. New SPI interfaces: - DatabaseConnectionOps: abstracts DbConnectionFactory - TransactionOps: abstracts HibernateUtil/LocalTransaction - LicenseOps: abstracts LicenseUtil - InterceptorLogger: abstracts dotCMS Logger Shared handler classes (used by BOTH ByteBuddy advice AND CDI interceptors): - CloseDBIfOpenedHandler, CloseDBHandler, WrapInTransactionHandler, ExternalTransactionHandler, EnterpriseFeatureHandler, LogTimeHandler Core implementations (CoreDatabaseConnectionOps, CoreTransactionOps, CoreLicenseOps, CoreInterceptorLogger) delegate to the real static classes and are registered at startup via InterceptorServiceProvider.init(). No-op defaults are used when core implementations are not registered, enabling future extraction to a utility module with zero core dependency. This makes ByteBuddy advice and CDI interceptors truly DRY — both delegate to the same handler methods via the same SPI interfaces. https://claude.ai/code/session_01EFJR5jtJLuoAYBNef4aAPs

… lambda wrappers Fix inconsistencies between annotation handlers and lambda wrappers: - Annotation path now uses setAutoCommit(true) + closeConnection() instead of closeSessionSilently(), preventing connections returned to pool with autocommit=false - Annotation path now captures stack trace for transaction interruption detection (was passing null) - onError handlers now just rollback; callers rethrow the original exception without wrapping in DotDataException Delegate lambda wrappers to shared handler code: - LocalTransaction.wrapReturn/wrap -> WrapInTransactionHandler - LocalTransaction.externalizeTransaction -> ExternalTransactionHandler - DbConnectionFactory.wrapConnection -> CloseDBIfOpenedHandler Add lambda convenience methods to handlers: - WrapInTransactionHandler.wrapReturn/wrap - ExternalTransactionHandler.externalizeTransaction - CloseDBIfOpenedHandler.wrapConnection (preserves interrupt status) https://claude.ai/code/session_01EFJR5jtJLuoAYBNef4aAPs

DbConnectionFactory.closeAndCommit() throws DotDataException but the DatabaseConnectionOps interface and CoreDatabaseConnectionOps declared the method without any checked exception, causing a compilation failure. Propagate the exception through the interface, implementation, and CloseDBHandler.onExit(). https://claude.ai/code/session_018LvtrjfQSzLxzEyG7RRCcy

…ction() HibernateUtil.rollbackTransaction() throws DotHibernateException but the TransactionOps interface declares rollbackTransaction() with no throws. Wrap the call in try-catch and rethrow as RuntimeException to satisfy the compiler without changing the interface or callers. Made-with: Cursor

…nState The refactor replaced ExternalTransactionAdvice.TransactionInfo with ExternalTransactionHandler.ExternalTransactionState. The advice enter/exit methods are now static. Update testExternalTransactionAnnotation to use the new API. Made-with: Cursor

CDI interceptors cannot proxy final methods (WELD-001504). The @CloseDB on VelocityServlet.service() caused the servlet to fail initialization, which broke page rendering and the Cache-Control header — failing the Postman PagesResourceTests "Check Cache-Control" test. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…by design The handler enter/exit state pattern already handles nesting correctly: - WrapInTransaction: startLocalTransactionIfNeeded() returns false if already in transaction, so nested calls skip commit/cleanup - CloseDBIfOpened: onEnter() returns false for isNewConnection if connection exists, so onExit skips close - ExternalTransaction: always creates new connection (nesting by design) - EnterpriseFeature/LogTime: idempotent operations ByteBuddy and CDI are also mutually exclusive — Weld proxies don't copy method annotations, so ByteBuddy's declaresMethod(isAnnotatedWith) matcher never matches CDI beans. Removes unnecessary ThreadLocal and simplifies all 6 CDI interceptors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…icenseException Preserves behavioral compatibility with existing catch blocks that expect DotInvalidLicenseException from @EnterpriseFeature checks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Original handleException() used sneaky-throw to rethrow all exception types as-is. The refactored version was wrapping RuntimeExceptions in DotDataException, changing semantics for callers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Original WrapInTransactionAdvice called closeSessionSilently() on error during enter. The refactored handler only closed the JDBC connection, potentially leaving a Hibernate session open. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Eliminates duplicated setup/commit/rollback/cleanup logic that was manually reimplemented without onEnter()'s error handling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- DotInvalidLicenseException type preserved from @EnterpriseFeature - RuntimeException passthrough in wrapReturn - Transaction commit/rollback lifecycle - External transaction connection isolation and restoration Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

This reverts commit fab7da0.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…ons" This reverts commit 572ca00.

Weld's Tomcat integration (WELD-ENV-001100) processes all servlets through WeldInstanceManager, scanning for interceptor bindings even on non-CDI-bean servlets. WELD-001504 rejects final methods that carry interceptor annotations like @CloseDB. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Include @RequestCost in the handler+interceptor pattern established for all other annotation-based interceptors. This enables cost tracking on CDI-managed beans, not just ByteBuddy-instrumented classes. - Add @InterceptorBinding to @RequestCost annotation - Extract shared logic into RequestCostHandler - Create RequestCostInterceptor for CDI beans - Update RequestCostAdvice to delegate to handler - Register in beans.xml under Observability group Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…otcms.cdi.business Per review feedback — the existing com.dotcms.cdi package is the natural home for CDI interceptors. Using com.dotcms.cdi.business keeps them co-located. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

wezell

Looks good.

…rces (#34961) ## Summary Makes `DbConnectionFactory.getConnection()` safe for try-with-resources by wrapping the returned connection in a `ManagedConnection` that only closes when it owns the connection lifecycle. Additionally fixes **all identity comparisons** (`!=`) on connections that break when `getConnection()` returns a new wrapper object each call: - **`DotConnectionWrapper.equals()`** — fallback now compares `internalConnection.equals(obj)` instead of `super.equals(obj)`, correctly handling raw-vs-wrapped connection comparisons - **`LocalTransaction.handleTransactionInteruption()`** — `!=` → `.equals()` - **`CoreTransactionOps.handleTransactionInterruption()`** — `!=` → `.equals()` (bug latent in #34946, only exposed by ManagedConnection wrapper) ## Dependency > **⚠️ This branch is rebased on top of #34946. That PR must merge to `main` first.** > > Once #34946 is merged, the commits from that PR will already be on `main` and this branch will merge cleanly with only its own changes. Until then, the PR diff will show #34946's commits as well. ## Changes (this PR only) ### `ManagedConnection.java` (new) - Extends `DotConnectionWrapper`, tracks `ownsConnection` flag - `close()` is a no-op for borrowed connections; calls `closeSilently()` for owned ones ### `DbConnectionFactory.getConnection()` - Returns `new ManagedConnection(connection, isNewConnection)` instead of raw connection - Try-with-resources is now safe whether or not a ThreadLocal connection already exists ### `DotConnectionWrapper.equals()` - Fixed fallback: `internalConnection.equals(obj)` instead of `super.equals(obj)` - Enables correct equality when comparing wrapped connections to raw connections (e.g., from `setConnection()`) ### `LocalTransaction.handleTransactionInteruption()` - Changed `DbConnectionFactory.getConnection() != conn` to `!DbConnectionFactory.getConnection().equals(conn)` - Without this, every transaction would trigger a spurious "Transaction broken" warning ### `CoreTransactionOps.handleTransactionInterruption()` - Same `!=` → `.equals()` fix for the inline check path (when `threadStack == null`) ## Test plan - [ ] `./mvnw install -pl :dotcms-core -DskipTests` — builds clean ✅ - [ ] Run `DbConnectionFactoryTest` integration test - [ ] Verify no spurious "Transaction broken" warnings in logs during any integration test - [ ] Grep for remaining `getConnection() !=` patterns: `grep -rn "getConnection() !=" dotCMS/src/main/java/` Closes #34489 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>

) ### Why: CDI Beans Need Working Interceptors ByteBuddy annotations (`@WrapInTransaction`, `@CloseDBIfOpened`, etc.) **fail silently on CDI beans**. Weld proxies don't copy method annotations, so ByteBuddy's class matcher (`declaresMethod(isAnnotatedWith(...))`) never sees them. This was the root cause of the connection leak fixed in #34134, and affects 7+ known classes including `UniqueFieldDataBaseUtil`, `JobQueueManagerAPIImpl`, and `MetricsAPIImpl`. Adding CDI interceptor bindings ensures these annotations work correctly on `@ApplicationScoped` beans — preventing silent failures, connection leaks, and transaction boundary violations. ### Secondary Benefits **DRY shared logic into handlers** — The same transaction/connection logic previously lived in both ByteBuddy advice classes and inline call sites. Extracting it into reusable handler classes (`WrapInTransactionHandler`, `CloseDBIfOpenedHandler`, etc.) means the CDI interceptors and ByteBuddy advice share a single implementation. **Clean ByteBuddy advice and SPI abstraction** — Decoupling the interceptor layer from core implementations via SPI interfaces (`DatabaseConnectionOps`, `TransactionOps`, `LicenseOps`) enables future extraction to a utility module with no core dependency. This also opens the door to **build-time annotation processing**, which would improve startup performance and unblock code coverage analysis currently hindered by ByteBuddy's runtime instrumentation. ### Proposed Changes * Add CDI interceptors for all 6 annotations (`WrapInTransactionInterceptor`, `CloseDBIfOpenedInterceptor`, `CloseDBInterceptor`, `ExternalTransactionInterceptor`, `LogTimeInterceptor`, `EnterpriseFeatureInterceptor`) * Mark annotations with `@InterceptorBinding` to enable CDI interception * Extract shared logic from ByteBuddy advice into reusable handler classes * Create SPI interfaces (`DatabaseConnectionOps`, `TransactionOps`, `LicenseOps`, `InterceptorLogger`) to decouple from core * Implement core SPI providers (`CoreDatabaseConnectionOps`, `CoreTransactionOps`, etc.) * Update ByteBuddy advice classes to delegate to handlers * Register CDI interceptors in `beans.xml` * Simplify `LocalTransaction` and `DbConnectionFactory` to use new handlers ### Compatibility 100% behavioral compatibility maintained. ByteBuddy continues to handle non-CDI classes; CDI interceptors handle CDI beans. Both use the same handlers and lifecycle guards to prevent double-processing. ### Checklist - [x] No new tests required — existing ByteBuddy and transaction tests cover the refactored logic - [ ] Translations - [ ] Security Implications Contemplated ### Additional Info https://claude.ai/code/session_01EFJR5jtJLuoAYBNef4aAPs This PR fixes: #34482 --------- Co-authored-by: Claude <noreply@anthropic.com>

…rces (#34961) ## Summary Makes `DbConnectionFactory.getConnection()` safe for try-with-resources by wrapping the returned connection in a `ManagedConnection` that only closes when it owns the connection lifecycle. Additionally fixes **all identity comparisons** (`!=`) on connections that break when `getConnection()` returns a new wrapper object each call: - **`DotConnectionWrapper.equals()`** — fallback now compares `internalConnection.equals(obj)` instead of `super.equals(obj)`, correctly handling raw-vs-wrapped connection comparisons - **`LocalTransaction.handleTransactionInteruption()`** — `!=` → `.equals()` - **`CoreTransactionOps.handleTransactionInterruption()`** — `!=` → `.equals()` (bug latent in #34946, only exposed by ManagedConnection wrapper) ## Dependency > **⚠️ This branch is rebased on top of #34946. That PR must merge to `main` first.** > > Once #34946 is merged, the commits from that PR will already be on `main` and this branch will merge cleanly with only its own changes. Until then, the PR diff will show #34946's commits as well. ## Changes (this PR only) ### `ManagedConnection.java` (new) - Extends `DotConnectionWrapper`, tracks `ownsConnection` flag - `close()` is a no-op for borrowed connections; calls `closeSilently()` for owned ones ### `DbConnectionFactory.getConnection()` - Returns `new ManagedConnection(connection, isNewConnection)` instead of raw connection - Try-with-resources is now safe whether or not a ThreadLocal connection already exists ### `DotConnectionWrapper.equals()` - Fixed fallback: `internalConnection.equals(obj)` instead of `super.equals(obj)` - Enables correct equality when comparing wrapped connections to raw connections (e.g., from `setConnection()`) ### `LocalTransaction.handleTransactionInteruption()` - Changed `DbConnectionFactory.getConnection() != conn` to `!DbConnectionFactory.getConnection().equals(conn)` - Without this, every transaction would trigger a spurious "Transaction broken" warning ### `CoreTransactionOps.handleTransactionInterruption()` - Same `!=` → `.equals()` fix for the inline check path (when `threadStack == null`) ## Test plan - [ ] `./mvnw install -pl :dotcms-core -DskipTests` — builds clean ✅ - [ ] Run `DbConnectionFactoryTest` integration test - [ ] Verify no spurious "Transaction broken" warnings in logs during any integration test - [ ] Grep for remaining `getConnection() !=` patterns: `grep -rn "getConnection() !=" dotCMS/src/main/java/` Closes #34489 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>

github-project-automation Bot added this to dotCMS - Product Planning Mar 11, 2026

github-actions Bot mentioned this pull request Mar 11, 2026

Implement CDI Interceptors for @CloseDBIfOpened and @WrapInTransaction #34482

Closed

8 tasks

github-actions Bot added the Area : Backend PR changes Java/Maven backend code label Mar 11, 2026

spbolton changed the title ~~Refactor interceptors to DRY shared logic into handlers~~ Create CDI Interceptors to handle DB Annotations on Bean classes Mar 11, 2026

wezell requested changes Mar 12, 2026

View reviewed changes

Comment thread dotCMS/src/main/java/com/dotcms/business/bytebuddy/ByteBuddyFactory.java

spbolton mentioned this pull request Mar 12, 2026

fix: make DbConnectionFactory.getConnection() safe for try-with-resources #34961

Merged

4 tasks

spbolton enabled auto-merge March 12, 2026 14:35

fabrizzio-dotCMS reviewed Mar 12, 2026

View reviewed changes

Comment thread dotCMS/src/main/java/com/dotcms/business/cdi/LogTimeInterceptor.java Outdated

claude and others added 19 commits March 13, 2026 10:27

fix: throw DotInvalidLicenseException instead of custom InsufficientL…

8bc187a

…icenseException Preserves behavioral compatibility with existing catch blocks that expect DotInvalidLicenseException from @EnterpriseFeature checks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

refactor: compose externalizeTransaction() from lifecycle methods

81b8b2c

Eliminates duplicated setup/commit/rollback/cleanup logic that was manually reimplemented without onEnter()'s error handling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Revert "fix: remove final from methods with CDI-intercepted annotations"

9d1e3a2

This reverts commit fab7da0.

test: add InterceptorHandlerTest to MainSuite2a

34e40af

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Reapply "fix: remove final from methods with CDI-intercepted annotati…

22f5135

…ons" This reverts commit 572ca00.

spbolton force-pushed the claude/fix-issue-34482-AY8pM branch from 1581c0a to 1c98fab Compare March 13, 2026 10:28

wezell approved these changes Mar 13, 2026

View reviewed changes

fabrizzio-dotCMS approved these changes Mar 13, 2026

View reviewed changes

spbolton added this pull request to the merge queue Mar 13, 2026

Merged via the queue into main with commit 7b785d8 Mar 13, 2026
49 checks passed

spbolton deleted the claude/fix-issue-34482-AY8pM branch March 13, 2026 17:19

sfreudenthaler added AI: Not Safe To Rollback and removed AI: Not Safe To Rollback labels Mar 17, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create CDI Interceptors to handle DB Annotations on Bean classes#34946

Create CDI Interceptors to handle DB Annotations on Bean classes#34946
spbolton merged 19 commits intomainfrom
claude/fix-issue-34482-AY8pM

spbolton commented Mar 11, 2026 •

edited

Loading

Uh oh!

wezell commented Mar 12, 2026 •

edited

Loading

Uh oh!

wezell left a comment

Uh oh!

Uh oh!

Uh oh!

wezell left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

spbolton commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why: CDI Beans Need Working Interceptors

Secondary Benefits

Proposed Changes

Compatibility

Checklist

Additional Info

Uh oh!

wezell commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wezell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wezell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

spbolton commented Mar 11, 2026 •

edited

Loading

wezell commented Mar 12, 2026 •

edited

Loading