task(Index Operations were not using a singleton to keep cache) Refs:#34938

[fabrizzio-dotCMS]

Summary

Fixes a bug where ContentFactoryIndexOperationsOS was creating a new OSQueryCache instance on every instantiation instead of using the shared singleton managed by CacheLocator. This caused the OS query cache to be bypassed in production, defeating its purpose.

Changes

Bug fix

• ContentFactoryIndexOperationsOS — constructor now fetches the cache via CacheLocator.getOSQueryCache() instead of new OSQueryCache(), ensuring all instances share the same cache

Cache infrastructure

• CacheLocator — added getOSQueryCache() accessor and registered OSQueryCache in the CacheIndex enum, mirroring the existing ESQueryCache pattern

Cleanup

• CacheUtil.java — deleted dead utility class (Moveh2dbDir) that referenced contentletIndexAPI.timestampFormatter and was unused

Testing

No new tests added. The fix follows the same pattern as ESQueryCache which is already in use. To verify:

1. Enable OS migration (phase 1+)
2. Confirm repeated identical queries hit the cache (cache hit metrics in Hazelcast/JMX)
3. Confirm CacheLocator.getOSQueryCache() returns the same instance across calls

Breaking Changes

None.

This PR fixes: #34938

This PR fixes: #34938

[claude]

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

---

Rollback Safety Analysis

• Gather context and read rollback-unsafe categories
• Get full PR diff
• Analyze changes against unsafe categories
• Apply appropriate label and post findings

---

Result: ✅ Safe to Rollback

The label AI: Safe To Rollback has been applied.

Changes analyzed:

File	Change
ContentFactoryIndexOperationsOS.java	Constructor refactored to use CacheLocator.getOSQueryCache() singleton instead of new OSQueryCache()
CacheLocator.java	Added OSQueryCache entry to CacheIndex enum and getOSQueryCache() factory method
CacheUtil.java	Deleted unused utility class (h2db directory management, already had its core logic commented out)

Category-by-category verdict:

• C-1 Structural Data Model Change — No DB schema changes. ✅
• C-2 Elasticsearch Mapping Change — No ES mapping changes. ✅
• C-3 Content JSON Model Version Bump — No JSON model changes. ✅
• C-4 DROP TABLE/COLUMN — No DDL changes at all. ✅
• H-1 One-Way Data Migration — No data transformations. ✅
• H-2 RENAME TABLE/COLUMN — No renames. ✅
• H-3 PK Restructuring — No constraint changes. ✅
• H-4 New ContentType Field Type — No new field types. ✅
• H-5 Binary Storage Provider Change — No storage provider changes. ✅
• H-6 DROP PROCEDURE/FUNCTION — No stored procedures affected. ✅
• H-7 NOT NULL Column Without Default — No column additions. ✅
• M-1 Non-Broadening Column Type Change — No type changes. ✅
• M-2 Push Publishing Bundle Format — No bundle format changes. ✅
• M-3 REST/GraphQL API Contract Change — No API contract changes. ✅
• M-4 OSGi Plugin API Breakage — getOSQueryCache() is a new additive method; no existing interfaces removed or changed. ✅

This is a pure in-memory cache refactoring. Rolling back to N-1 would simply revert to creating a fresh OSQueryCache instance per ContentFactoryIndexOperationsOS constructor call — no persistent state is affected.

[claude]

Claude finished @fabrizzio-dotCMS's task in 1m 11s —— View job

---

PR Review

• Gather context
• Read changed files
• Analyze changes
• Post review

---

Summary: Clean, minimal fix. The core problem — ContentFactoryIndexOperationsOS creating a new OSQueryCache() on every instantiation instead of sharing a singleton — is correctly resolved by routing through CacheLocator, which uses Locator.getServiceInstance() with double-checked locking.

Issues found:

1. Indentation inconsistency in CacheLocator.java

The new entries in the CacheIndex enum and switch statement use 4-space indentation while the surrounding code uses tabs. This creates inconsistency. CacheLocator.java:492

// existing entries use tabs:
	ESQueryCache("ESQueryCache"),
// new entry uses spaces:
    OSQueryCache("OSQueryCache"),

2. OSQueryCache constructor called inside CacheLocator.createService() calls back into CacheLocator

OSQueryCache() no-arg constructor calls CacheLocator.getCacheAdministrator(). This is the same pattern as ESQueryCache and should be safe since adminCache is initialized before service instances are created, but it's worth being aware of this circular dependency.

3. Locator.cache is a raw HashMap — pre-existing

Locator.java:21 uses HashMap which is not thread-safe. Reads outside the synchronized block (line 33, 38) can theoretically race. This is pre-existing and not introduced by this PR, but it means the singleton guarantee in getServiceInstance has a pre-existing weakness (the outer containsKey check before the synchronized block is unguarded). Not a blocker for this PR.

CacheUtil.java deletion: Confirmed no references exist in the codebase. Safe to delete.

Overall: The fix is correct and the approach matches existing patterns (ESQueryCache). Just clean up the indentation before merge.

[wezell]

Good. love the red lines