fix(page-cache): include original request URI in cache key for vanity URL 200-forwards (#35696)

[dsilvam]

What does this PR do?

Fixes a page cache key collision where multiple vanity URL 200-forward requests routed to the same detail page all shared an identical cache key. The first request warmed the cache and every subsequent request — regardless of the actual incoming URL — received that cached response.

Closes #35696

Why is this a problem?

VelocityLiveMode.buildCacheParameters() composed the cache key from pageUrl (the forwarded-to path), pageInode, vanityUrlId, etc. — none of which differ between /store/123/acme/catalog/ and /store/456/globex/catalog/ when both forward to the same page via the same vanity URL rule.

On multi-node clusters with round-robin load balancing, different nodes cached different affiliates' content under the same key, causing wrong content on every other request for the full TTL duration (up to 1 hour).

What is the fix?

When VANITY_URL_OBJECT is present on the request and isForward() == true, include RequestDispatcher.FORWARD_REQUEST_URI as an originalUri: component in the cache key. This is the original browser URL set by Tomcat when CMSFilter forwards to VelocityServlet via RequestDispatcher.forward().

String originalRequestUri = (request.getAttribute(VANITY_URL_OBJECT) != null
        && ((CachedVanityUrl) request.getAttribute(VANITY_URL_OBJECT)).isForward())
        ? (String) request.getAttribute(RequestDispatcher.FORWARD_REQUEST_URI)
        : null;

Non-vanity pages and vanity redirects (301/302) are unaffected — null is filtered out of the key by PageCacheParameters.

Relation to #34879

PR #34879 fixed the same collision class for URL-mapped contentlets via WIKI_CONTENTLET_INODE. The vanity URL 200-forward path was not addressed by that fix.

Testing

• New integration test: VelocityLiveModeTest#vanityForwardDifferentOriginalUriProducesDifferentCacheKeys
• Verified locally against customer's dev environment (Freshdesk Experiments Portlet — Screen 3: View Results #37004) — different affiliate URLs now produce different cache keys and serve correct content

Checklist

• Tests added
• Existing tests pass
• No new public API changes (package-private visibility change on buildCacheParameters() for testability only)

[claude]

Claude finished @dsilvam's task in 3m 18s —— View job

---

dotCMS Backend Review — Complete

• Loaded dotCMS conventions (CLAUDE.md, Java/DB/REST standards, security principles)
• Pulled Java diff (HEAD~1...HEAD, since the PR is already merged onto this branch tip)
• Reviewed across the four domains (security, database, java standards, REST)
• Posted consolidated findings as a separate `` comment
• Submit formal PR review — skipped: formal PR approvals are outside this bot's permissions

Summary

The PR is a tightly scoped fix and the diff holds up well. No security, database, or REST API concerns. Two 🟡 Medium code-quality findings posted in the dedicated review comment:

1. VelocityLiveMode.java:217-232 — request.getAttribute(VANITY_URL_OBJECT) and its cast to CachedVanityUrl are repeated 4×/2× across two adjacent expressions; extract a local for readability.
2. VelocityLiveModeTest.java:54-62 — after adding @BeforeClass to prepare(), the explicit prepare(); call inside the configs() data provider became dead code. Harmless (init is idempotent) but worth removing.

Neither is blocking. The cache-key fix itself is correct: originalRequestUri == null is filtered out by PageCacheParameters.filterNulls(...), so non-vanity and 301/302 paths are unaffected as the PR claims.
• fix/35696-vanity-url-page-cache-key-collision

[claude]

🔍 dotCMS Backend Review

[🟡 Medium] dotCMS/src/main/java/com/dotcms/rendering/velocity/servlet/VelocityLiveMode.java:217-232

request.getAttribute(VANITY_URL_OBJECT) is invoked 4× and cast to CachedVanityUrl 2× across two adjacent expressions. Extracting the cached vanity into a local variable removes the duplication and makes the forward-vs-redirect intent more obvious to readers.

String vanityUrl = request.getAttribute(VANITY_URL_OBJECT) != null
        ? ((CachedVanityUrl) request.getAttribute(VANITY_URL_OBJECT)).vanityUrlId
        : "";

String originalRequestUri = (request.getAttribute(VANITY_URL_OBJECT) != null
        && ((CachedVanityUrl) request.getAttribute(VANITY_URL_OBJECT)).isForward())
        ? (String) request.getAttribute(RequestDispatcher.FORWARD_REQUEST_URI)
        : null;

💡 Suggested fix:

final CachedVanityUrl vanity = (CachedVanityUrl) request.getAttribute(VANITY_URL_OBJECT);
final String vanityUrl = vanity != null ? vanity.vanityUrlId : "";
final String originalRequestUri = (vanity != null && vanity.isForward())
        ? (String) request.getAttribute(RequestDispatcher.FORWARD_REQUEST_URI)
        : null;

---

[🟡 Medium] dotcms-integration/src/test/java/com/dotcms/rendering/velocity/servlet/VelocityLiveModeTest.java:54-62

prepare() now carries @BeforeClass, but configs() still calls prepare() explicitly on line 62. That call is now dead code — JUnit will already have invoked prepare() before any test runs. It's safe today only because IntegrationTestInitService.init() is gated by initCompleted.compareAndSet(false, true). Future readers will wonder why both are needed.

@BeforeClass
public static void prepare() throws Exception {
    IntegrationTestInitService.getInstance().init();
}

@DataProvider(format = "%m: %p[0]")
public static Object[] configs() throws Exception {
    prepare(); // redundant — @BeforeClass already ran

💡 Suggested fix: Drop the prepare(); call from configs().

---

Next steps

• 🔴 / 🟠 Fix locally and push — these need your judgment
• 🟡 You can ask me to handle mechanical fixes inline: @claude fix <issue description> in <File.java>
• Every new push triggers a fresh review automatically