fix(content-drive): user-cache sizing + warm-up to cut per-row DB lookups (#37186)

[ihoffmann-dot]

Summary

• Raises cache.userdotcmscache.size from the effectively-1000 default to 4000 (FR-002), sized against a reference ~1,828 active-user base.
• Adds a warm-up pass (BrowserAPIImpl#warmUpUserCache) that resolves each distinct modUser/owner on a listing page once, ahead of parallel row hydration, cutting redundant per-row UserAPI#loadUserById calls (User Story 1).
• Dedupes the mod-user-name lookup already done once per row (DefaultTransformStrategy#resolveModUserName, User Story 2).
• Adds an orphan-user fallback (FR-004a): a listing row whose modUser/lockedBy user was deleted no longer fails the whole page — it now degrades to N/A for that row only.
• Adds a @VisibleForTesting DB-lookup counter (UserFactoryImpl) used only by the new integration test to assert warm-up actually reduces lookups to one per distinct author.

Test plan

• ./mvnw test -pl :dotcms-core -Dtest=UserFactoryImplTest,BrowserAPIImplTest,UserCacheConfigTest,DefaultTransformStrategyTest
• just test-integration-ide
• ./mvnw verify -pl :dotcms-integration -Dcoreit.test.skip=false -Dit.test=BrowserAPITest
• Confirm no System.out/System.getProperty/System.getenv introduced (checked via diff at implementation time)

Branched off the approved spec branch per this repo's Spec-Kit flow (spec.md-only in PR1, not merged to main yet).

🤖 Generated with Claude Code

This PR fixes: #37186

[claude]

Claude finished @ihoffmann-dot's task in 3m 0s —— View job

---

Code Review — PR #37394 (content-drive user-cache sizing + warm-up)

• Read the PR diff
• Analyze main code changes (BrowserAPIImpl, UserFactoryImpl, DefaultTransformStrategy, config)
• Analyze test changes
• Post review

I traced the full transform/hydration path and verified the cache region key, the dedup logic, and the warm-up placement against the surrounding code.

New Issues

• 🟡 Medium: dotcms-integration/src/test/java/com/dotcms/browser/BrowserAPITest.java:~2680 — alter table user_ disable trigger all is table-level DDL that affects every session against the shared DB, not just this test's connection. For the brief window between disable and re-enable, FK enforcement on user_ is off process-wide. It's correctly wrapped in try/finally, but the enable only runs after the delete; if the integration suite is ever run with any parallel/concurrent DB activity, a sibling test touching user_ during that window could bypass constraints.

  • Assumption: dotcms-integration runs tests serially in a single JVM against one DB (which is the current setup), so this is not a live problem today.
  • What to verify: that no integration test parallelism is enabled for this module now or planned. If it is, scope the delete differently (e.g. SET session_replication_role = replica on this connection only, or delete via a path that doesn't trip the parent-side trigger).

• 🟡 Medium: dotCMS/src/main/java/com/dotmarketing/business/UserFactoryImpl.java:118 / dotcms-integration/.../BrowserAPITest.java:2600 — dbLookupCount is a process-wide static incremented on every loadUserById cache miss JVM-wide. The integration assertion getDbLookupCountForTesting() == expectedIds.size() (exactly 3) assumes nothing else in the process resolves a not-yet-cached user between reset() and the assertion. getPaginatedContents runs parallel hydration chunks and other listing machinery on background threads; any incidental cold-user resolution (or a concurrently-running test) would push the count above 3 and fail.

  • Assumption: in the current serial integration run, the only cold resolutions during this call are the 3 seeded authors (system/other users are already warm from AbstractDataGen's static init), so it passes deterministically.
  • What to verify: the test is stable across repeated CI runs. If it flakes, assert >= expectedIds.size() on the cold pass isn't enough — consider assertEquals(0, ...) on the warm second pass (already present) as the primary signal and relax the cold-pass exact count.

Notes (non-blocking, no action required)

• DefaultTransformStrategy.resolveModUserName correctly reuses the value addAuditProperties writes under MOD_USER_NAME_KEY. I confirmed the ordering: transform() runs addCommonProperties (→ addAuditProperties) before addVersionProperties, both resolve the same modUser id, so the reused value is identical to what the old unconditional loadUserById produced. The only behavioral change is the intended FR-004a degrade-to-N/A instead of throwing NoSuchUserException — consistent with addAuditProperties' existing Try.getOrNull pattern. ✅
• Config key is correct: UserCacheImpl.primaryGroup = "UserDotCMSCache" → property cache.userdotcmscache.size. The "dead email region" comment is accurate (add keys emailGroup by email address, get reads it by primaryGroup + key, so entries are never read back). ✅
• warmUpUserCache per-id failure handling (Try.run().onFailure → Logger.debug) correctly isolates a single orphan from aborting the warm-up, leaving the per-row FR-004a fallback to handle the row later. ✅
• collectWarmUpUserIds is pure/side-effect-free, null-safe via UtilMethods.isSet, and deterministically ordered (LinkedHashSet). ✅
• No System.out/getProperty/getenv; Logger/Config/APILocator conventions followed; @VisibleForTesting public-vs-package visibility is justified by the cross-module integration test. ✅

No blocking issues. The two medium items are test-robustness concerns that only bite if integration test concurrency changes — worth a glance but they don't block merge.

· issue-37186-content-drive-user-cache-sizing-impl