Skip to content

Conversation

@javier-godoy
Copy link
Member

@javier-godoy javier-godoy commented Dec 1, 2025

Close #149

Summary by CodeRabbit

  • Chores
    • Added an internal string-trimming utility and removed reliance on an external string-utils import.
    • Internal cleanup to centralize trimming behavior; no API or user-facing changes expected.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 1, 2025

Walkthrough

Adds a package-local StringUtils.trimToNull implementation and removes imports of org.apache.commons.lang3.StringUtils from two classes so source compiles without the external commons-lang3 dependency.

Changes

Cohort / File(s) Summary
Import removal
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelperClassNameGenerator.java, src/main/java/com/flowingcode/vaadin/addons/gridhelpers/HeaderFooterStylesHelper.java
Removed org.apache.commons.lang3.StringUtils imports; code now relies on a local utility in the same package.
Local utility implementation
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/StringUtils.java
Added package-private StringUtils utility with static String trimToNull(String) that trims input and returns null for null or empty-after-trim strings.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify the two modified classes compile and reference the new package-local StringUtils.
  • Confirm trimToNull behavior matches Apache Commons Lang3 semantics.
  • Check for other occurrences of org.apache.commons.lang3.StringUtils that may require similar changes.

Possibly related PRs

Suggested reviewers

  • mlopezFC
  • paodb

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'refactor: remove use of lang3 trimToNull' directly describes the main change in the PR: removing the dependency on Apache Commons Lang3's trimToNull in favor of a custom implementation.
Linked Issues check ✅ Passed The PR successfully addresses issue #149 by removing imports of org.apache.commons.lang3 and providing a custom StringUtils implementation, resolving the compilation failure when building with Vaadin 25.0.0-beta7.
Out of Scope Changes check ✅ Passed All changes are directly related to resolving the compilation failure: removing problematic imports from two files and adding a custom StringUtils class to replace the missing dependency.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-149

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 091ccf8 and c03929b.

📒 Files selected for processing (3)
  • src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelperClassNameGenerator.java (0 hunks)
  • src/main/java/com/flowingcode/vaadin/addons/gridhelpers/HeaderFooterStylesHelper.java (0 hunks)
  • src/main/java/com/flowingcode/vaadin/addons/gridhelpers/StringUtils.java (1 hunks)
💤 Files with no reviewable changes (2)
  • src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelperClassNameGenerator.java
  • src/main/java/com/flowingcode/vaadin/addons/gridhelpers/HeaderFooterStylesHelper.java
🔇 Additional comments (3)
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/StringUtils.java (3)

1-37: LGTM! Dual license headers are appropriate.

The dual license headers correctly attribute both Flowing Code's ownership of the file and the Apache Software Foundation's original implementation. This is the proper way to handle code copied from Apache Commons Lang3.


39-40: Lombok dependency is properly configured.

The @UtilityClass annotation is correctly used here. Lombok is declared in pom.xml with proper import from lombok.experimental.UtilityClass. The class will have an automatically generated private constructor and be marked as final, making it an effective utility class.


42-66: Implementation and integration verified.

The trimToNull method is correctly implemented and properly integrated into the codebase. All usages in GridHelperClassNameGenerator.java (2 calls) and HeaderFooterStylesHelper.java (1 call) are in place, and the commons-lang3 dependency has been successfully removed. The JavaDoc is comprehensive with clear examples.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/StringUtils.java (1)

36-64: Make the utility class non‑instantiable (and optionally final)

Since this is a pure static utility class, Sonar is right that exposing an implicit public no‑arg constructor is unnecessary. Making the class non‑instantiable also documents intent.

Consider:

-class StringUtils {
+final class StringUtils {
+
+  private StringUtils() {
+    // Utility class; do not instantiate.
+  }

This will satisfy the Sonar warning and follows common Java utility‑class conventions.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5c0683d and 637c616.

📒 Files selected for processing (3)
  • src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelperClassNameGenerator.java (0 hunks)
  • src/main/java/com/flowingcode/vaadin/addons/gridhelpers/HeaderFooterStylesHelper.java (0 hunks)
  • src/main/java/com/flowingcode/vaadin/addons/gridhelpers/StringUtils.java (1 hunks)
💤 Files with no reviewable changes (2)
  • src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelperClassNameGenerator.java
  • src/main/java/com/flowingcode/vaadin/addons/gridhelpers/HeaderFooterStylesHelper.java
🧰 Additional context used
🪛 GitHub Check: SonarCloud Code Analysis
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/StringUtils.java

[warning] 36-36: Add a private constructor to hide the implicit public one.

See more on https://sonarcloud.io/project/issues?id=FlowingCode_GridHelpers&issues=AZrbd__M6LPYYU9wOTKj&open=AZrbd__M6LPYYU9wOTKj&pullRequest=151

🔇 Additional comments (1)
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/StringUtils.java (1)

38-62: trimToNull implementation correctly mirrors the intended behavior

The null check, trim(), and empty‑to‑null handling give you the same semantics as the original Apache Commons StringUtils.trimToNull, so this is a safe local replacement for the removed dependency.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 637c616 and 091ccf8.

📒 Files selected for processing (3)
  • src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelperClassNameGenerator.java (0 hunks)
  • src/main/java/com/flowingcode/vaadin/addons/gridhelpers/HeaderFooterStylesHelper.java (0 hunks)
  • src/main/java/com/flowingcode/vaadin/addons/gridhelpers/StringUtils.java (1 hunks)
💤 Files with no reviewable changes (2)
  • src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelperClassNameGenerator.java
  • src/main/java/com/flowingcode/vaadin/addons/gridhelpers/HeaderFooterStylesHelper.java
🔇 Additional comments (2)
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/StringUtils.java (2)

22-22: LGTM: @UtilityClass annotation addresses constructor visibility.

The use of Lombok's @UtilityClass annotation automatically makes the constructor private and the class final, which addresses the concern raised in the previous review about constructor visibility.

Also applies to: 39-40


57-66: LGTM: Implementation logic is correct and verified.

The trimToNull implementation correctly handles null input, trims whitespace, and returns null for empty strings. The logic matches the documented behavior.

Verification confirms:

  • ✓ All production code call sites (GridHelperClassNameGenerator, HeaderFooterStylesHelper) correctly resolve to the local StringUtils class
  • ✓ Lombok dependency is declared in pom.xml
  • ✓ The commons-lang3 import remaining in LazyTestData.java (test file) is only used for containsIgnoreCase(), not trimToNull, so this does not impact the change

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 2, 2025

@paodb paodb merged commit c86f04f into master Dec 2, 2025
5 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Pending release in Flowing Code Addons Dec 2, 2025
@paodb paodb deleted the fix-149 branch December 2, 2025 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Pending release

Development

Successfully merging this pull request may close these issues.

Vaadin 25: cannot compile with 25.0.0-beta7

3 participants