refactor(search): convert ContentletSearch to immutable interface

[fabrizzio-dotCMS]

Summary

Converts ContentletSearch from a mutable POJO into a @Value.Immutable interface backed by ImmutableContentletSearch, eliminating all setters and enforcing construction-time initialization. All 6 instantiation sites are migrated to the builder pattern. Existing getXxx() accessors are preserved unchanged, so the ~35 read-only call sites require no modifications.

Changes

Backend — model

• ContentletSearch rewritten as @Value.Immutable interface; removes all 5 setters (setId, setInode, setIdentifier, setIndex, setScore)
• String fields annotated @Nullable; getScore() annotated @Value.Default (returns 0.0f) to preserve the original primitive default

Backend — instantiation sites (6 files)

• ESSearchAPIImpl — new ContentletSearch() + setInode → ImmutableContentletSearch.builder().inode(...).build()
• ESContentletAPIImpl — full 5-field builder
• ESContentletScrollImpl — full 5-field builder
• OSContentletScrollImpl — full 5-field builder
• ContentUtils — 2-field builder (inode + identifier)
• PaginatedContentletsIntegrationTest — test helper migrated to builder

Testing

• ./mvnw install -pl :dotcms-core -DskipTests — compiles clean
• No new tests added; existing integration test helper in PaginatedContentletsIntegrationTest updated to use the builder

To verify locally:

1. ./mvnw install -pl :dotcms-core -DskipTests — should produce no errors
2. Search for new ContentletSearch() in the codebase — should return zero results

Breaking Changes

new ContentletSearch() and all setXxx() calls are no longer valid. Any OSGi plugin or external code that instantiates ContentletSearch directly must migrate to ImmutableContentletSearch.builder(). All known sites within this repo are updated in this PR.

Relates to #35609

🤖 Generated with Claude Code

[claude]

Claude finished @fabrizzio-dotCMS's task in 2m 33s —— View job

---

PR Review: ContentletSearch immutable conversion

• Read changed files
• Check ContentletSearch interface design
• Audit all call sites for correctness
• Look for semantic regressions
• Post review

---

Clean refactor overall. Mechanics are sound and all 25 call sites are consistently migrated. A few items worth a second look:

1. PR description is stale (cosmetic, but misleading)

The description claims bare accessor style (inode(), identifier(), id(), index(), score()), but commit a72282f7 reverted that — the interface keeps getXxx() style (ContentletSearch.java:9-24). All call sites and ContentletSearch::getInode method references rely on the getXxx style. The code is internally consistent; only the description is wrong.

2. Semantic change: equals/hashCode is now field-based, not identity-based

The old class had default Object identity semantics for equals/hashCode. The Immutables-generated implementation is structural (by all fields). This isn't documented in the PR.

I searched for Set<ContentletSearch>, Map<ContentletSearch, …>, and explicit .equals(/contains-style usage. None found in the codebase, so this should be safe in practice — but it's a behavior shift, not a pure mechanical refactor. Worth a one-liner in the PR description.

3. toString format changes

Old: ContentletSearch [id=…, inode=…, identifier=…, index=…, score=…]
New Immutables default: ContentletSearch{getId=…, getInode=…, …} (and includes the getXxx prefix in the output because of the accessor naming).

No callers parse this string, and grep didn't surface log assertions depending on it. Cosmetic regression for anyone reading logs; not blocking.

4. Nullable string inode propagated to ESSearchAPIImpl

At ESSearchAPIImpl.java:69-95, the builder is called with only .inode(sourceMap.get("inode").toString()). Other fields stay null. Then conwrap.getInode() is added to inodes list which is passed to findContentlets(inodes). Pre-existing behavior, not introduced here — but now getInode() is annotated @Nullable, which means any future static-analysis pass against this code path will flag a possible NPE. Not a regression, just a heads-up that the type system has tightened.

5. @Value.Default on score preserves 0.0f default — good

Old float score; field defaulted to 0.0f; @Value.Default returns the same. Behavior preserved across the six instantiation sites (some of which never set score).

6. Field order in builder is fine but inconsistent with old write order

In ESContentletAPIImpl.java:1689-1694 and the two scroll impls, builder calls go .id().index().identifier().inode().score(). Old code wrote in the same order. Consistent — no concern.

---

TL;DR: Code change is low-risk and well-executed. Update the PR description to reflect the actual accessor style and to call out the equals/hashCode semantic change. No blocking issues.
· Branch: issue-35609/contentlet-search-immutable

[claude]

Pull Request Unsafe to Rollback!!!

• Category: M-4 — OSGi Plugin API Breakage
• Risk Level: 🟡 MEDIUM
• Why it's unsafe: com.dotmarketing.common.model.ContentletSearch is part of an OSGi-exported package (declared in dotCMS/src/main/resources/osgi/osgi-extra.conf and dotCMS/src/main/resources/osgi/system/osgi-extra.conf), so it is part of the public surface that third-party OSGi plugins compile against. This PR converts it from a concrete public class to a public interface and removes the implicit no-arg constructor plus every setter (setId, setInode, setIdentifier, setIndex, setScore). Existing plugins compiled against the old class that do new ContentletSearch() will throw InstantiationError (cannot instantiate an interface) and plugins that call any setter will throw NoSuchMethodError. Symmetrically, any plugin re-compiled against N (using ImmutableContentletSearch.builder()) will fail on N-1 after rollback because com.dotmarketing.common.model.ImmutableContentletSearch does not exist there — NoClassDefFoundError at plugin activation. Read-side access (getId(), getInode(), getIdentifier(), getIndex(), getScore()) is preserved, so consumers that only iterate results from ContentletAPI.searchIndex(...) are unaffected, but any plugin that constructs or mutates the DTO is broken in at least one direction.
• Code that makes it unsafe:
  • dotCMS/src/main/java/com/dotmarketing/common/model/ContentletSearch.java — file rewritten from public class ContentletSearch { ... setInode(...) ... setId(...) ... } to @Value.Immutable public interface ContentletSearch { ... }. The no-arg constructor and all setters disappear at the bytecode level.
  • Call sites that previously did new ContentletSearch(); conwrapper.setInode(...) are migrated to ImmutableContentletSearch.builder().inode(...).build() in:
    • dotCMS/src/enterprise/java/com/dotcms/enterprise/priv/ESSearchAPIImpl.java (lines ~76–82)
    • dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java (lines ~1686–1696)
    • dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletScrollImpl.java (lines ~197–206)
    • dotCMS/src/main/java/com/dotcms/content/index/opensearch/OSContentletScrollImpl.java (lines ~238–247)
    • dotCMS/src/main/java/com/dotcms/rendering/velocity/viewtools/content/util/ContentUtils.java (lines ~411–416)
• Alternative (if possible): Use the two-release zero-downtime API change pattern from M-4 / H-2. In release N, keep ContentletSearch as a concrete class and add the immutable variant alongside it (e.g., keep the existing setters/constructor as deprecated thin shims that delegate, or expose ImmutableContentletSearch as a parallel type while leaving the legacy class intact). Migrate internal call sites to the builder without removing the legacy surface, so plugins compiled against either N or N-1 continue to link. Only remove the deprecated constructor and setters in a later release (N+1), once N-2 — the last version with plugins still expecting the mutable shape — is outside the rollback window.

🤖 Generated with Claude Code - View job run