build!: upgrade vaadin to 24.9.8 AND feat!: replace setClassNameGenerator with setPartNameGenerator AND refactor: avoid use of commons.lang3 AND docs: enhance class documentation in LegacyTwinColGrid AND docs: fix double brace in javadoc in LegacyTwinColGrid AND build: set version to 4.0.0-SNAPSHOT AND chore/ci #205
Conversation
WalkthroughUpgrades project version and runtime (Java 17/21, Vaadin 24/25), replaces javax.servlet with jakarta.servlet-api, removes prerelease profiles, renames Grid API methods from setClassNameGenerator to setPartNameGenerator, adds internal String/CharSequence utilities, updates license/plugin excludes, and adds license headers to tests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/flowingcode/vaadin/addons/twincolgrid/FilterableTwinColumn.java (1)
139-144: Method override mismatch with parent class refactor causes compilation failure.The parent class
TwinColumn.javahas renamedsetClassNameGeneratortosetPartNameGenerator(line 256). This override inFilterableTwinColumn.javastill uses the old method name and callssuper.setClassNameGenerator()at line 142, which no longer exists in the parent class. This will cause a compilation error.Rename the method and update the super call:
@Override - public FilterableTwinColumn<T> setClassNameGenerator( - SerializableFunction<T, String> classNameGenerator) { - super.setClassNameGenerator(classNameGenerator); + public FilterableTwinColumn<T> setPartNameGenerator( + SerializableFunction<T, String> partNameGenerator) { + super.setPartNameGenerator(partNameGenerator); return this; }
🧹 Nitpick comments (1)
src/main/java/com/flowingcode/vaadin/addons/twincolgrid/FilterableTwinColumn.java (1)
90-95: Return type inconsistency breaks fluent API pattern.This override returns
TwinColumn<T>instead ofFilterableTwinColumn<T>, unlike all other overridden methods in this class. This breaks the fluent chaining pattern for users ofFilterableTwinColumn.🔎 Proposed fix for consistency
@Override - public <V extends Comparable<? super V>> TwinColumn<T> setComparator( + public <V extends Comparable<? super V>> FilterableTwinColumn<T> setComparator( ValueProvider<T, V> keyExtractor) { super.setComparator(keyExtractor); return this; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
pom.xmlsrc/main/java/com/flowingcode/vaadin/addons/twincolgrid/CharSequenceUtils.javasrc/main/java/com/flowingcode/vaadin/addons/twincolgrid/FilterableTwinColumn.javasrc/main/java/com/flowingcode/vaadin/addons/twincolgrid/LegacyTwinColGrid.javasrc/main/java/com/flowingcode/vaadin/addons/twincolgrid/StringUtils.javasrc/main/java/com/flowingcode/vaadin/addons/twincolgrid/TwinColGrid.javasrc/main/java/com/flowingcode/vaadin/addons/twincolgrid/TwinColumn.javasrc/test/java/com/flowingcode/vaadin/addons/twincolgrid/CompatibilityExtension.javasrc/test/java/com/flowingcode/vaadin/addons/twincolgrid/TwinColGridListAdapterTest.javasrc/test/java/com/flowingcode/vaadin/addons/twincolgrid/TwinColGridTest.java
💤 Files with no reviewable changes (1)
- src/main/java/com/flowingcode/vaadin/addons/twincolgrid/TwinColGrid.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/com/flowingcode/vaadin/addons/twincolgrid/StringUtils.java (1)
src/main/java/com/flowingcode/vaadin/addons/twincolgrid/CharSequenceUtils.java (1)
UtilityClass(40-101)
src/main/java/com/flowingcode/vaadin/addons/twincolgrid/CharSequenceUtils.java (1)
src/main/java/com/flowingcode/vaadin/addons/twincolgrid/StringUtils.java (1)
UtilityClass(40-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-vaadin25
🔇 Additional comments (18)
src/main/java/com/flowingcode/vaadin/addons/twincolgrid/LegacyTwinColGrid.java (3)
33-44: Javadoc is clear and properly formatted.The class documentation effectively explains the backward compatibility purpose and migration path. The
@deprecatedtag aligns with the runtime@Deprecatedannotation and provides clear guidance to maintainers and users.
61-192: Constructor implementations are consistent and well-structured.All 8 constructors properly initialize the deprecated legacy fields (
leftGrid,rightGrid, and their data providers) from the parent class accessors. The delegation to parent constructors is appropriate, and the consistent initialization pattern makes the code maintainable.
194-454: Deprecated methods are properly delegated to new API.The delegation pattern is consistently applied throughout. Each deprecated method correctly maps to its replacement in the parent class or grid accessors, includes helpful javadoc migration guidance, and returns
thisfor fluent API compatibility where applicable. The override ofsetDataProvider(lines 313–316) correctly maintains the legacyleftGridDataProviderfield.src/main/java/com/flowingcode/vaadin/addons/twincolgrid/FilterableTwinColumn.java (1)
1-19: License header addition looks good.Standard Apache 2.0 license header properly formatted.
src/test/java/com/flowingcode/vaadin/addons/twincolgrid/CompatibilityExtension.java (1)
1-19: License header addition looks good.The Apache 2.0 license header is properly formatted and consistent with the project's licensing requirements.
src/test/java/com/flowingcode/vaadin/addons/twincolgrid/TwinColGridTest.java (1)
1-19: License header addition looks good.The Apache 2.0 license header is properly formatted and consistent with the project's licensing requirements.
src/test/java/com/flowingcode/vaadin/addons/twincolgrid/TwinColGridListAdapterTest.java (1)
1-19: License header addition looks good.The Apache 2.0 license header is properly formatted and consistent with the project's licensing requirements.
pom.xml (5)
6-6: Version bump to 4.0.0-SNAPSHOT is appropriate.The major version bump correctly signals the breaking changes in this release (API rename, Java 17 baseline, servlet API migration).
12-17: Dependency upgrades align with Vaadin 24 requirements.The Vaadin 24.9.8, Java 17, and Jetty 11 upgrades are consistent and appropriate for the migration.
159-162: License plugin excludes are appropriate.Excluding
main/bundles/**andmain/frontend/**from license processing is reasonable, as these directories typically contain generated or third-party frontend assets.
373-400: v25 profile provides good forward compatibility testing.The v25 profile with Java 21 and Vaadin 25.0.0 enables testing compatibility with the next major version, which is valuable for ensuring the API changes are future-proof.
131-135: No changes needed. Vaadin 24.9.8 requires Jakarta Servlet API version 6, making jakarta.servlet-api 6.0.0 the correct dependency version.src/main/java/com/flowingcode/vaadin/addons/twincolgrid/TwinColumn.java (2)
1-19: License header addition looks good.The Apache 2.0 license header is properly formatted and consistent with the project's licensing requirements.
244-260: API change from setClassNameGenerator to setPartNameGenerator correctly addresses Vaadin 25 compatibility.The method rename aligns with Vaadin's migration from class name generators to part name generators for CSS styling.
setPartNameGeneratoris available in Vaadin 24.7.3+ and confirmed as the required replacement in Vaadin 25, wheresetClassNameGeneratorwas deprecated and removed. The javadoc accurately documents the behavior, including null handling, space-separated part names, and proper combination with Grid-level generators.src/main/java/com/flowingcode/vaadin/addons/twincolgrid/StringUtils.java (2)
1-36: Dual license headers are appropriate for Apache Commons-derived code.The dual licensing (Flowing Code Apache 2.0 + ASF Apache 2.0) correctly acknowledges that this code is derived from Apache Commons Lang3. This is the proper way to attribute derivative works.
40-55: containsIgnoreCase implementation is correct.The logic correctly implements case-insensitive substring search:
- Proper null handling (returns false)
- Iterates through all valid starting positions
- Delegates to CharSequenceUtils.regionMatches for the actual comparison
This successfully replaces the Apache Commons Lang3 dependency.
src/main/java/com/flowingcode/vaadin/addons/twincolgrid/CharSequenceUtils.java (2)
1-36: Dual license headers are appropriate for Apache Commons-derived code.The dual licensing (Flowing Code Apache 2.0 + ASF Apache 2.0) correctly acknowledges that this code is derived from Apache Commons Lang3. This is the proper way to attribute derivative works.
55-99: regionMatches implementation is correct and well-optimized.The implementation properly handles:
- String optimization by delegating to native String.regionMatches (lines 57-59)
- Proper bounds validation (lines 64-76)
- Correct case-insensitive character comparison (lines 91-95)
The logic matches String.regionMatches semantics and successfully replaces the Apache Commons Lang3 dependency.
javier-godoy
force-pushed
the
4.0.0-snapshot
branch
from
c0f6066 to
76efd58
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
pom.xml (1)
89-117: Consider updating or removing the outdated comment.The comment references "Vaadin 13 compatibility mode in V14" which is no longer relevant for Vaadin 24+. The webjars exclusions themselves may still be valid, but the explanatory comment is misleading.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pom.xmlsrc/main/java/com/flowingcode/vaadin/addons/twincolgrid/FilterableTwinColumn.javasrc/main/java/com/flowingcode/vaadin/addons/twincolgrid/TwinColumn.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-vaadin23
- GitHub Check: build-vaadin14
- GitHub Check: build-vaadin24
- GitHub Check: build-vaadin25
🔇 Additional comments (5)
pom.xml (2)
373-388: v25 profile looks well-structured.The profile correctly overrides the Vaadin version to 25.0.0, Java version to 21, and includes the
vaadin-devdependency for development support. Note that the Jetty version compatibility concern raised earlier also applies to this profile.
6-6: Version bump and Vaadin upgrade look correct.The 4.0.0-SNAPSHOT version appropriately signals breaking changes (setClassNameGenerator → setPartNameGenerator API change). The Vaadin 24.9.8 baseline and Java 17 requirements are consistent with current Vaadin 24 LTS recommendations.
Also applies to: 12-14
src/main/java/com/flowingcode/vaadin/addons/twincolgrid/FilterableTwinColumn.java (1)
1-19: License header update looks correct.The copyright year range has been updated to 2017-2025, which is appropriate for the current release cycle.
src/main/java/com/flowingcode/vaadin/addons/twincolgrid/TwinColumn.java (2)
22-24: Grid import addition is correct.The
Gridimport on line 23 is required for the javadoc@linkreference toGrid#setPartNameGeneratoron line 248.
243-260: API change from setClassNameGenerator to setPartNameGenerator is correctly implemented.The method properly delegates to both columns and maintains fluent API pattern. Javadoc comprehensively documents null return behavior, space-separated part names, and interaction with
Grid#setPartNameGenerator. The@throws NullPointerExceptiondocumentation correctly reflects thatColumn.setPartNameGeneratorenforces non-null input per Vaadin's API contract.
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
| <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding> | ||
| <jetty.version>9.4.36.v20210114</jetty.version> | ||
| <jetty.version>11.0.26</jetty.version> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Jetty 11.0 Jakarta Servlet 6.0 compatibility
💡 Result:
Short answer: Jetty 11 implements Jakarta Servlet 5.0 — it is not compatible with Jakarta Servlet 6.0. For Servlet 6.0 support you must use Jetty 12 (or later). [1][2]
Notes:
- Jetty 11.x = Servlet 5.0 / Jakarta EE 9. [1][2]
- Jetty 12.x implements Servlet 6.0 (and newer 12.1.x adds Servlet 6.1/EE11 support). [1][2]
- Many frameworks (e.g., Spring Boot 3) that require Jakarta Servlet 6.0 will not work with the Jetty 11 runtime without downgrading the servlet API or upgrading Jetty. [3]
Sources:
- Eclipse Jetty version table / downloads. [1]
- Jetty 11→12 migration / Jetty 12 docs. [2]
- Reports/examples of incompatibility (Spring Boot + Jetty 11 vs Servlet 6). [3]
🌐 Web query:
Jetty 12 Jakarta Servlet API version requirements
💡 Result:
Short answer — Jetty 12 itself is servlet‑API agnostic; servlet support is provided by separate "EE environments." Jetty 12 ships/works with these environments and Servlet API versions:
- EE8: Servlet 4.0 (javax.*). [1][2]
- EE9: Servlet 5.0 (jakarta.*) — with deprecated features. [1][2]
- EE10: Servlet 6.0 (jakarta.*) — full EE10 environment. Use the ee10 modules/artifacts (e.g. org.eclipse.jetty.ee10:jetty-ee10-servlet). [1][3]
Note: Jetty 12.1 adds Jakarta EE 11 / Jakarta Servlet 6.1 support if you need that. [4]
References: Jetty docs and release notes. [1] https://jetty.org/docs/jetty/12/index.html [2] https://webtide.com/introducing-jetty-12/ [3] https://jetty.org/docs/jetty/12/programming-guide/server/http.html [4] https://jetty.org/docs/jetty/12.1/programming-guide/migration/12.0-to-12.1.html
Jetty 11.x is incompatible with Jakarta Servlet 6.0 — upgrade to Jetty 12.x.
Jetty 11 supports only Jakarta Servlet 5.0 (Jakarta EE 9), but the pom.xml declares jakarta.servlet-api:6.0.0 (Jakarta EE 10). This mismatch will cause runtime failures. Use Jetty 12 with the ee10 modules for Jakarta Servlet 6.0 compatibility.
Also applies to: 131-135, and the v25 profile (line 373+) which inherits the same jetty.version.
🤖 Prompt for AI Agents
In pom.xml around line 17 (and also update the occurrences at lines ~131-135 and
in the v25 profile at ~373+), the project pins jetty.version to 11.0.26 which is
incompatible with jakarta.servlet-api:6.0.0; change the jetty.version property
to a Jetty 12.x release (for example 12.0.x — pick the current stable 12.0.y) so
Jetty matches Jakarta Servlet 6.0, and update the same version reference in the
other locations/profiles to keep them consistent with the ee10 modules.
There was a problem hiding this comment.
@coderabbitai Please create an issue about it, since we would have to apply the resolution in other add-ons.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue about Jetty/Servlet API incompatibility
gh issue create \
--repo FlowingCode/TwinColGridAddon \
--title "Upgrade Jetty to 12.x for Jakarta Servlet 6.0 compatibility" \
--body "## Problem
The project currently pins \`jetty.version\` to 11.0.26, but Jetty 11.x only supports Jakarta Servlet 5.0 (Jakarta EE 9). The pom.xml declares \`jakarta.servlet-api:6.0.0\` (Jakarta EE 10), which creates an incompatibility that will cause runtime failures.
## Solution
Upgrade to Jetty 12.x which provides Jakarta Servlet 6.0 support via the EE10 modules (e.g., \`org.eclipse.jetty.ee10:jetty-ee10-servlet\`).
## Context
This issue was identified during review of PR #205 and needs to be applied across multiple add-ons maintained by FlowingCode.
**References:**
- PR: https://github.com/FlowingCode/TwinColGridAddon/pull/205
- Review comment: https://github.com/FlowingCode/TwinColGridAddon/pull/205#discussion_r2651631758
- Requested by: @javier-godoy" \
--assignee javier-godoy
Length of output: 58
@javier-godoy I've created issue #207 to track the Jetty 12.x upgrade for Jakarta Servlet 6.0 compatibility:
🔗 https://github.com/FlowingCode/TwinColGridAddon/issues/207
The issue includes references to this PR and comment, and is assigned to you. It can serve as a tracking item for applying the same resolution across other FlowingCode add-ons.
There was a problem hiding this comment.
There was a problem hiding this comment.
@javier-godoy Thank you for creating the broader tracking issue! That's the right approach to coordinate the Jetty 12.x upgrade across all affected add-ons. Issue #142 in AddonsInternal will ensure consistent resolution across the organization.
src/main/java/com/flowingcode/vaadin/addons/twincolgrid/FilterableTwinColumn.java
Show resolved
Hide resolved
javier-godoy
force-pushed
the
4.0.0-snapshot
branch
from
76efd58 to
edec196
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pom.xml (1)
17-17: Jetty 11.x is incompatible with Jakarta Servlet 6.0.This issue was already flagged in previous review comments. Jetty 11 supports only Jakarta Servlet 5.0, but the pom declares
jakarta.servlet-api:6.0.0(lines 131-135). Upgrade to Jetty 12.x for Jakarta Servlet 6.0 compatibility.This also applies to the v25 profile at line 379.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pom.xmlsrc/main/java/com/flowingcode/vaadin/addons/twincolgrid/FilterableTwinColumn.javasrc/main/java/com/flowingcode/vaadin/addons/twincolgrid/TwinColumn.java
🧰 Additional context used
🪛 GitHub Actions: Java CI with Maven
pom.xml
[error] 1-1: vaadin-maven-plugin:prepare-frontend failed due to API incompatibility: com/vaadin/hilla/maven/PrepareFrontendMojo has been compiled by a more recent Java runtime (class file version 61.0) than the current runtime (55.0). This causes the plugin to fail during frontend preparation.
[warning] 1-1: The requested profile "v23" could not be activated because it does not exist.
🔇 Additional comments (6)
src/main/java/com/flowingcode/vaadin/addons/twincolgrid/FilterableTwinColumn.java (1)
140-144: LGTM! Previous review concern has been addressed.The override correctly implements
setPartNameGeneratorwith the parameter now properly namedpartNameGenerator(matching both the method name and parent class API). The delegation tosuper.setPartNameGenerator(partNameGenerator)is consistent and the covariant return type maintains the fluent API pattern.src/main/java/com/flowingcode/vaadin/addons/twincolgrid/TwinColumn.java (1)
244-260: LGTM! API rename correctly addresses Vaadin 25 compatibility.The method has been properly renamed from
setClassNameGeneratortosetPartNameGenerator, with updated Javadoc that accurately describes CSS part name generation. The implementation correctly delegates to both columns and preserves method chaining. No references to the old method name remain in the codebase.pom.xml (4)
6-6: LGTM: Appropriate major version bump.The version bump to 4.0.0-SNAPSHOT correctly reflects the breaking changes in this PR (Vaadin 24/25 upgrade, Java 17/21 baseline, API method renames).
159-162: LGTM: Appropriate license exclusions.Excluding
main/bundles/**andmain/frontend/**is correct, as these directories contain generated or frontend resources that don't require license headers.
373-388: Java 21 requirement is mandatory for Vaadin 25.0.0; Jetty 11.0.26 is incompatible with Jakarta Servlet 6.0.The v25 profile correctly sets Java 21, which is the minimum required version for Vaadin 25.0.0. The
vaadin-devoptional dependency is appropriate for this profile. However, Jetty 11.0.26 targets Jakarta Servlet 5.0 (Jakarta EE 9) and is not compatible with applications requiring Servlet 6.0 (Jakarta EE 10); Jetty 12.x is needed for Servlet 6.0 support. Ensure the CI/CD pipeline uses Java 21 or higher when activating this profile.
12-14: No action required. The workflow is already correctly configured with Java 17 for thebuild-vaadin24job in.github/workflows/maven.yml, which is the appropriate job for building with Vaadin 24.9.8. The GitHub Actions configuration matches the pom.xml settings.Likely an incorrect or invalid review comment.
| <groupId>com.flowingcode.vaadin.addons.demo</groupId> | ||
| <artifactId>commons-demo</artifactId> | ||
| <version>3.8.0</version> | ||
| <version>5.0.0</version> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
commons-demo com.flowingcode.vaadin.addons.demo Maven Central repository versions
💡 Result:
Versions for com.flowingcode.vaadin.addons.demo:commons-demo available on Maven Central (examples with release dates): 4.3.0 (Sep 26, 2025), 4.2.0 (Jan 15, 2025), 4.1.0 (Jul 22, 2024), 4.0.0 (Feb 05, 2024), 3.10.0 (Feb 05, 2024), 3.9.0 (Oct 12, 2023), 3.8.0 (Sep 07, 2023). See Maven Central listing. [1]
Source:
[1] mvnrepository — com.flowingcode.vaadin.addons.demo » commons-demo.
Version 5.0.0 does not exist in Maven Central—use 4.3.0 instead.
The commons-demo:5.0.0 reference cannot be resolved. Maven Central only provides up to version 4.3.0 (released Sep 26, 2025). Update line 145 to:
<version>4.3.0</version>
This represents a safe version upgrade from 3.8.0 to the current stable release.
🤖 Prompt for AI Agents
In pom.xml around line 145 the dependency version is set to 5.0.0 which does not
exist on Maven Central; update the version element to 4.3.0 so the dependency
resolves (replace 5.0.0 with 4.3.0 on line 145).
There was a problem hiding this comment.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the existence of commons-demo version 5.0.0 on Maven Central
curl -s -o /dev/null -w "%{http_code}" "https://repo1.maven.org/maven2/com/flowingcode/vaadin/addons/demo/commons-demo/5.0.0/commons-demo-5.0.0.pom"
Length of output: 3
@javier-godoy You're absolutely right, my apologies for the incorrect information. I've confirmed that commons-demo version 5.0.0 exists on Maven Central (the URL returns HTTP 200). The version bump from 3.8.0 to 5.0.0 is valid.
It appears my initial web search results were outdated and didn't include the December 15, 2025 release. The dependency version is correct as specified in the PR.
Close #203
Close #204
Summary by CodeRabbit
New Features
Refactor
Chores
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.