Skip to content

refactor(deps): remove dot.guava repackaged dependency#35315

Merged
dsilvam merged 2 commits intomainfrom
fix/remove-dot-guava-repackaged-imports-35235
Apr 15, 2026
Merged

refactor(deps): remove dot.guava repackaged dependency#35315
dsilvam merged 2 commits intomainfrom
fix/remove-dot-guava-repackaged-imports-35235

Conversation

@dsilvam
Copy link
Copy Markdown
Member

@dsilvam dsilvam commented Apr 14, 2026

Summary

  • Replaces all 240 occurrences of com.dotcms.repackage.com.google.common.* imports with standard com.google.common.* equivalents across the codebase (222 in dotCMS/src, 18 in dotcms-integration/src)
  • Removes com.dotcms.lib:dot.guava:11.0.1_2 from dotCMS/pom.xml and bom/application/pom.xml
  • Fixes two API compatibility breaks surfaced by the Guava 32 upgrade:
    • Objects.toStringHelper()MoreObjects.toStringHelper() (removed in Guava 20)
    • Cache.get(key)Cache.getIfPresent(key) (removed in Guava 22)

Closes #35235 (partial — companion to the OWASP dependency upgrade PR)

Motivation

dot.guava was a dotCMS-repackaged version of Guava 11.0.1 that provided a com.dotcms.repackage.com.google.common.* namespace alongside the standard com.google.guava:guava artifact. Both coexisted on the classpath. Now that guava has been upgraded to 32.0.1-jre, dot.guava is redundant and its old APIs are a liability.

Test plan

  • ./mvnw install -pl :dotcms-core -DskipTests — BUILD SUCCESS
  • Verify no com.dotcms.repackage.com.google references remain: grep -r "com.dotcms.repackage.com.google" dotCMS/src dotcms-integration/src --include="*.java" → 0 results
  • Run integration tests for rules engine (most heavily affected area): RulesAPIImpl, RulesFactoryImpl, conditionlets

🤖 Generated with Claude Code

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 14, 2026

Claude finished @dsilvam's task in 8m 1s —— View job


  • Load dotCMS conventions
  • Get Java diff
  • Run specialized sub-agent reviews in parallel
  • Consolidate findings and post review

Review complete. Two issues were found and posted to the backend review comment.

@mbiuki mbiuki self-requested a review April 14, 2026 17:28
Copy link
Copy Markdown
Contributor

@mbiuki mbiuki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, well-scoped PR. A few notes:

Two API fixes are correct:

  • Objects.toStringHelper()MoreObjects.toStringHelper() — import correctly switched, removed in Guava 20.
  • Cache.get(key)Cache.getIfPresent(key) — right call; Cache.get(key) required a Callable loader in Guava 22+, getIfPresent is the correct no-loader equivalent.

dot.guava removed from both bom/application/pom.xml and dotCMS/pom.xml — clean removal with no loose ends.

One observation (out of scope for this PR): dot.google (com.dotcms.lib:dot.google:ukv_2) is still present in both pom files — it's a similar repackaged dependency. If it follows the same pattern as dot.guava, it may be worth evaluating for the same treatment in a follow-up.

Overall this is a straightforward import migration — low risk, high value. The build passing confirms no compile breaks. Approving.

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 14, 2026

dotCMS Backend Review

[High] dotCMS/src/main/java/com/dotmarketing/webdav/FileResourceCache.java:41

The get() method was not updated alongside clearExpiredEntries(). NullCallable.call() returns null, which causes Guava 22+ to throw InvalidCacheLoadException on every cache miss (Guava forbids a loader returning null). The exception is swallowed by the catch (Exception e) on line 42, so the method silently returns null — but every cache miss now allocates and throws an exception, a performance regression that also masks the API misuse. The NullCallable inner class at line 88 also uses raw Callable instead of Callable<Long>.

// Line 41 -- not fixed; throws InvalidCacheLoadException on every cache miss
timeOfPublishing = (Long)cache.get(key, new NullCallable());
// Line 88 -- raw type
private class NullCallable implements Callable {

Suggested fix: Replace line 41 with timeOfPublishing = cache.getIfPresent(key); (same fix already applied to clearExpiredEntries()), then delete the now-unused NullCallable inner class.


[Medium] dotCMS/src/enterprise/java/com/dotcms/enterprise/rules/RulesFactoryImpl.java:623-625

Pre-existing bug in saveRule() — the stream that should delete orphaned condition groups when a rule is updated has two defects: (1) it uses .map() with no terminal operation, so the stream is never consumed and deleteRemovedGroupFromRule() is never called; (2) the filter predicate is inverted — .filter(group -> updatedGroups.contains(group.getId())) selects groups that are still present, not those that were removed. This file was touched by this PR (imports changed), surfacing the bug for review. Orphaned ConditionGroup rows accumulate in the DB every time a rule is saved with fewer groups than before.

dbGroups.stream()
    .filter(group -> updatedGroups.contains(group.getId())) // wrong: keeps, not removes
    .map(this::deleteRemovedGroupFromRule);                 // no terminal op: never executes

Suggested fix: .filter(group -> !updatedGroups.contains(group.getId())).forEach(this::deleteRemovedGroupFromRule);


Next steps

  • High / Medium: Fix locally and push — these need your judgment
  • You can ask me to handle mechanical fixes inline: @claude fix FileResourceCache get() method or @claude fix RulesFactoryImpl saveRule stream
  • Every new push triggers a fresh review automatically

Base automatically changed from fix/security-owasp-dependency-upgrades-35235 to main April 14, 2026 20:52
Daniel Silva and others added 2 commits April 14, 2026 15:04
Replace all 222 occurrences of com.dotcms.repackage.com.google.common.*
imports with standard com.google.common.* equivalents, then remove the
com.dotcms.lib:dot.guava:11.0.1_2 artifact from dotCMS/pom.xml and BOM.

Two API compatibility fixes required for the Guava 32 upgrade:
- Objects.toStringHelper() removed in Guava 20 → MoreObjects.toStringHelper()
- Cache.get(key) removed in Guava 22 → Cache.getIfPresent(key)

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…sts (refs #35235)

18 integration test files in dotcms-integration/src/test/java were missed
in the initial migration. Replace all remaining com.dotcms.repackage.com.google.*
imports with standard com.google.* equivalents.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
@dsilvam dsilvam force-pushed the fix/remove-dot-guava-repackaged-imports-35235 branch from ee0cffd to e5151a8 Compare April 14, 2026 21:08
@github-actions github-actions bot added the Area : Backend PR changes Java/Maven backend code label Apr 14, 2026
@dsilvam dsilvam added this pull request to the merge queue Apr 15, 2026
Merged via the queue into main with commit 0c035c5 Apr 15, 2026
86 of 151 checks passed
@dsilvam dsilvam deleted the fix/remove-dot-guava-repackaged-imports-35235 branch April 15, 2026 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

fix(security): Upgrade vulnerable dependencies flagged by OWASP Dependency Check

2 participants