Skip to content

Conversation

@javier-godoy
Copy link
Member

@javier-godoy javier-godoy commented Nov 14, 2025

Summary by CodeRabbit

  • Refactor
    • Streamlined internal implementation of event handling and property management across Google Maps components. Enhanced code consistency and maintainability while preserving all public APIs and existing functionality.

@javier-godoy javier-godoy requested a review from paodb November 14, 2025 19:19
@coderabbitai
Copy link

coderabbitai bot commented Nov 14, 2025

Walkthrough

The PR adds Lombok ExtensionMethod annotation support to three Google Maps addon classes and refactors internal JsonMigration calls to use extension method syntax instead of static method invocations, with no changes to public APIs or method signatures.

Changes

Cohort / File(s) Summary
Lombok ExtensionMethod Integration and JsonMigration Refactoring
src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMap.java, src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMapMarker.java, src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMapPoly.java
Added @ExtensionMethod(value = JsonMigration.class, suppressBaseMethods = true) annotation and corresponding import to all three classes. Refactored internal method calls: replaced JsonMigration.getEventData(ev).get(...) and JsonMigration.setPropertyJson(getElement(), ...) with extension method syntax ev.getEventData().get(...) and getElement().setPropertyJson(...). Changes are internal only with no public API impact.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Consistent refactoring pattern applied uniformly across three files
  • No new business logic or algorithmic complexity
  • Mechanical transformation of static method calls to extension method syntax
  • Annotation additions are straightforward Lombok integration

Possibly related PRs

Suggested reviewers

  • mlopezFC
  • paodb

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'refactor: use lombok extension mechanism' clearly and accurately summarizes the main change—introducing Lombok ExtensionMethod support across multiple classes to enable direct method calls instead of static method invocations.
Docstring Coverage ✅ Passed Docstring coverage is 80.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 lombok-refactor

📜 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 39cc4aa and 5460b1b.

📒 Files selected for processing (3)
  • src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMap.java (8 hunks)
  • src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMapMarker.java (3 hunks)
  • src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMapPoly.java (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-08T21:28:24.972Z
Learnt from: javier-godoy
Repo: FlowingCode/GoogleMapsAddon PR: 135
File: src/test/java/com/flowingcode/vaadin/addons/googlemaps/TrackLocationDemo.java:67-70
Timestamp: 2024-10-08T21:28:24.972Z
Learning: The `getTrackLocationId` method is located at line 646 in the `GoogleMap.java` file.

Applied to files:

  • src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMap.java
🧬 Code graph analysis (1)
src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMapMarker.java (2)
src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMap.java (1)
  • SuppressWarnings (52-932)
src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMapPoly.java (1)
  • SuppressWarnings (40-213)
🔇 Additional comments (12)
src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMap.java (4)

49-49: LGTM!

The import for Lombok's ExtensionMethod is correctly added.


524-524: LGTM!

The refactoring to use extension method syntax improves readability by making the call more fluent. The logic remains functionally equivalent.


809-809: LGTM!

The refactoring to extension method syntax makes the property setting call more intuitive and readable.


58-58: Annotation correctly applies Lombok's ExtensionMethod feature from the external json-migration-helper dependency.

The @ExtensionMethod(value = JsonMigration.class, suppressBaseMethods = true) annotation properly references the JsonMigration class from the json-migration-helper dependency (com.flowingcode.vaadin:json-migration-helper:0.0.1-SNAPSHOT), which is correctly declared in pom.xml. Lombok is properly declared with version 1.18.34. The suppressBaseMethods = true parameter appropriately prevents conflicts with existing methods. No issues identified.

src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMapPoly.java (4)

38-38: LGTM: Import correctly added.

The import for ExtensionMethod is properly added to support the annotation applied to the class.


197-197: LGTM: Extension method syntax correctly applied.

The refactoring from JsonMigration.setPropertyJson(getElement(), "icons", jsonArray) to getElement().setPropertyJson("icons", jsonArray) is correct and makes the code more readable.


210-210: LGTM: Extension method syntax correctly applied.

The refactoring matches the pattern used at line 197 and is consistent with the usage in GoogleMap.java and GoogleMapMarker.java.


46-46: Confirm intentional suppression of Vaadin's native setPropertyJson method.

Verified: Vaadin's Element class natively provides setPropertyJson(String name, elemental.json.JsonValue value) as a standard Element API method. The suppressBaseMethods = true flag will suppress this native method in favor of the JsonMigration extension method from the external json-migration-helper dependency.

This pattern is consistently applied across GoogleMapPoly.java, GoogleMap.java, and GoogleMapMarker.java, suggesting it is intentional. However, confirm:

  • The purpose of replacing Vaadin's native method with the extension version
  • Whether this is necessary for compatibility with specific Vaadin versions or provides enhanced functionality
  • That the JsonMigration implementation maintains API compatibility with the native method
src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMapMarker.java (4)

38-38: LGTM: Import addition for Lombok ExtensionMethod.

The import is correctly added to support the @ExtensionMethod annotation used on line 46.


199-199: Refactoring looks correct, subject to verification.

The extension method syntax is properly applied. The behavior should remain identical to the previous JsonMigration.setPropertyJson(getElement(), "icon", icon.getJson()) call, assuming the suppressBaseMethods parameter is correctly configured.


209-209: Refactoring is consistent with line 199.

The extension method syntax is correctly applied, consistent with the refactoring at line 199.


46-46: Verify intent of suppressBaseMethods = true with the library maintainers.

Element.setPropertyJson exists in all targeted Vaadin versions (14, 23, 24, 25), so suppressBaseMethods = true consistently bypasses JsonMigration and uses the native method. However:

  • json-migration-helper (0.0.1-SNAPSHOT) is included as a dependency but explicitly bypassed by this setting
  • The library name suggests it provides compatibility/migration logic
  • No explanatory comments clarify whether this is intentional or inadvertent

Confirm with maintainers whether:

  1. JsonMigration should be used (change to suppressBaseMethods = false)
  2. Or if suppressBaseMethods = true is the correct choice and json-migration-helper serves a different purpose

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.

@sonarqubecloud
Copy link

@paodb paodb merged commit b669aed into master Nov 14, 2025
7 checks passed
@paodb paodb deleted the lombok-refactor branch November 14, 2025 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants