Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Records as Beans #459

Merged
merged 1 commit into from
Dec 31, 2024
Merged

Support Records as Beans #459

merged 1 commit into from
Dec 31, 2024

Conversation

jodastephen
Copy link
Member

@jodastephen jodastephen commented Dec 31, 2024

  • Add support for JLS records as beans
  • Avoid Map.copyOf as iteration is random

Summary by CodeRabbit

Release Notes

  • New Features

    • Added support for Java records as beans.
    • Introduced new interfaces and classes for handling record-based beans.
    • Enhanced meta-bean registration and lookup mechanisms.
  • Improvements

    • Improved immutability handling for map returns.
    • Added better support for record components and their metadata.
    • Refined bean builder functionality for record types.
    • Updated JSON serialization formats for better compatibility.
  • Bug Fixes

    • Corrected method type variable naming in light meta-property implementation.
  • Testing

    • Added comprehensive test coverage for record bean functionality.
    • Included tests for public and private record types.

The update provides enhanced support for Java records within the Joda-Beans library, improving type safety and bean management.

Copy link

coderabbitai bot commented Dec 31, 2024

📝 Walkthrough

Walkthrough

The pull request introduces comprehensive support for Java records in the Joda-Beans library. The changes include adding new classes like RecordBean, RecordBeanBuilder, RecordMetaBean, and RecordMetaProperty to enable seamless integration of record types with the bean framework. The implementation provides mechanisms for registering, building, and managing record-based beans, with a focus on immutability and type safety. Additionally, modifications were made to existing utility classes to improve map handling and method invocation.

Changes

File Change Summary
src/main/java/org/joda/beans/JodaBeanUtils.java Modified return statements to use Collections.unmodifiableMap() instead of Map.copyOf() for map immutability.
src/main/java/org/joda/beans/MetaBeans.java Updated meta-bean lookup logic to support record classes, added handling for RecordBean.register().
src/main/java/org/joda/beans/impl/RecordBean.java New interface for implementing record beans, with static registration method.
src/main/java/org/joda/beans/impl/RecordBeanBuilder.java New builder class for constructing immutable record beans.
src/main/java/org/joda/beans/impl/RecordMetaBean.java New meta-bean implementation for record types.
src/main/java/org/joda/beans/impl/RecordMetaProperty.java New meta-property implementation for record components.
src/main/java/org/joda/beans/impl/light/LightBeanBuilder.java Updated constructor to accept a data array.
src/main/java/org/joda/beans/impl/light/LightMetaProperty.java Fixed typographical error in method type variable name.
src/test/java/org/joda/beans/TestRecordBean.java Added test class for record bean functionality.
src/test/java/org/joda/beans/sample/RecordStrIntPair.java Added sample record class for testing.
src/changes/changes.xml Added new action for supporting records in the Joda-Beans library under release version 3.0.0-RC1.

Tip

CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command @coderabbitai generate docstrings to have CodeRabbit automatically generate docstrings for your pull request. We would love to hear your feedback on Discord.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

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 (2)
src/main/java/org/joda/beans/impl/RecordBean.java (1)

52-52: Consider clarifying the interface's placement under the impl package.
Although this interface is declared public, placing it in an impl package might cause confusion for consumers regarding whether it is official public API or an internal implementation detail.

src/main/java/org/joda/beans/MetaBeans.java (1)

100-112: Race condition avoided with synchronisation.
The synchronised block correctly avoids duplicate registration for the same class. The subsequent check for cls.isRecord() within it is clear and ensures concurrency correctness.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5b8fbda and 03c75c0.

📒 Files selected for processing (10)
  • src/main/java/org/joda/beans/JodaBeanUtils.java (2 hunks)
  • src/main/java/org/joda/beans/MetaBeans.java (3 hunks)
  • src/main/java/org/joda/beans/impl/RecordBean.java (1 hunks)
  • src/main/java/org/joda/beans/impl/RecordBeanBuilder.java (1 hunks)
  • src/main/java/org/joda/beans/impl/RecordMetaBean.java (1 hunks)
  • src/main/java/org/joda/beans/impl/RecordMetaProperty.java (1 hunks)
  • src/main/java/org/joda/beans/impl/light/LightBeanBuilder.java (1 hunks)
  • src/main/java/org/joda/beans/impl/light/LightMetaProperty.java (1 hunks)
  • src/test/java/org/joda/beans/TestRecordBean.java (1 hunks)
  • src/test/java/org/joda/beans/sample/RecordStrIntPair.java (1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
src/test/java/org/joda/beans/sample/RecordStrIntPair.java (2)

Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.

  • Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
  • Validate that immutable local variables are not annotated with final unless the variable is required for use in an inner class.
  • Favour use of var keyword for type declarations. var may also be used when the value is a cast null.
  • Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
  • Propose changes that only use the Java 21 API, not the API of Guava.
  • The pattern matching instanceof expression safely handles null, returning false.

Pattern **/test/java/**/*.java: For test code, focus on:

  • Correctness of test assertions
  • Test coverage of edge cases
  • Clear test naming and documentation
  • Encourage test methods to be package-scoped where possible
  • Be more lenient with code style and minor optimisations
src/main/java/org/joda/beans/impl/RecordMetaBean.java (2)

Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.

  • Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
  • Validate that immutable local variables are not annotated with final unless the variable is required for use in an inner class.
  • Favour use of var keyword for type declarations. var may also be used when the value is a cast null.
  • Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
  • Propose changes that only use the Java 21 API, not the API of Guava.
  • The pattern matching instanceof expression safely handles null, returning false.

Pattern **/main/java/**/*.java: - This project is mature and must provide a stable backwards-compatible public Java API.

  • In the 'Walkthrough' section, you must always provide a list of up to 25 changes to the public Java API that will affect end users.
    If there are no changes, you must explicitly state that there are no changes to the public Java API in this PR.
    The public Java API is defined as public and protected methods on public classes, plus the file module-info.java.
    Provide the list by deeply analysing code flow, which incudes analysing code flow through private methods and calls to Guava and Java 21.
    Changes to be reported on include:
    • New or removed methods in the public Java API
    • Changes to method return types or parameter types in the public Java API
    • Changes to method behaviour in the public Java API that might affect consumers
  • This project uses System.out.println instead of logging
  • This project tends to prefer for loops to streams for performance reasons, however either form is acceptable.
    Do not make suggestions to change between streams and for loops or vice versa.
src/main/java/org/joda/beans/impl/light/LightMetaProperty.java (2)

Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.

  • Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
  • Validate that immutable local variables are not annotated with final unless the variable is required for use in an inner class.
  • Favour use of var keyword for type declarations. var may also be used when the value is a cast null.
  • Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
  • Propose changes that only use the Java 21 API, not the API of Guava.
  • The pattern matching instanceof expression safely handles null, returning false.

Pattern **/main/java/**/*.java: - This project is mature and must provide a stable backwards-compatible public Java API.

  • In the 'Walkthrough' section, you must always provide a list of up to 25 changes to the public Java API that will affect end users.
    If there are no changes, you must explicitly state that there are no changes to the public Java API in this PR.
    The public Java API is defined as public and protected methods on public classes, plus the file module-info.java.
    Provide the list by deeply analysing code flow, which incudes analysing code flow through private methods and calls to Guava and Java 21.
    Changes to be reported on include:
    • New or removed methods in the public Java API
    • Changes to method return types or parameter types in the public Java API
    • Changes to method behaviour in the public Java API that might affect consumers
  • This project uses System.out.println instead of logging
  • This project tends to prefer for loops to streams for performance reasons, however either form is acceptable.
    Do not make suggestions to change between streams and for loops or vice versa.
src/main/java/org/joda/beans/impl/light/LightBeanBuilder.java (2)

Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.

  • Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
  • Validate that immutable local variables are not annotated with final unless the variable is required for use in an inner class.
  • Favour use of var keyword for type declarations. var may also be used when the value is a cast null.
  • Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
  • Propose changes that only use the Java 21 API, not the API of Guava.
  • The pattern matching instanceof expression safely handles null, returning false.

Pattern **/main/java/**/*.java: - This project is mature and must provide a stable backwards-compatible public Java API.

  • In the 'Walkthrough' section, you must always provide a list of up to 25 changes to the public Java API that will affect end users.
    If there are no changes, you must explicitly state that there are no changes to the public Java API in this PR.
    The public Java API is defined as public and protected methods on public classes, plus the file module-info.java.
    Provide the list by deeply analysing code flow, which incudes analysing code flow through private methods and calls to Guava and Java 21.
    Changes to be reported on include:
    • New or removed methods in the public Java API
    • Changes to method return types or parameter types in the public Java API
    • Changes to method behaviour in the public Java API that might affect consumers
  • This project uses System.out.println instead of logging
  • This project tends to prefer for loops to streams for performance reasons, however either form is acceptable.
    Do not make suggestions to change between streams and for loops or vice versa.
src/test/java/org/joda/beans/TestRecordBean.java (2)

Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.

  • Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
  • Validate that immutable local variables are not annotated with final unless the variable is required for use in an inner class.
  • Favour use of var keyword for type declarations. var may also be used when the value is a cast null.
  • Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
  • Propose changes that only use the Java 21 API, not the API of Guava.
  • The pattern matching instanceof expression safely handles null, returning false.

Pattern **/test/java/**/*.java: For test code, focus on:

  • Correctness of test assertions
  • Test coverage of edge cases
  • Clear test naming and documentation
  • Encourage test methods to be package-scoped where possible
  • Be more lenient with code style and minor optimisations
src/main/java/org/joda/beans/impl/RecordBean.java (2)

Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.

  • Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
  • Validate that immutable local variables are not annotated with final unless the variable is required for use in an inner class.
  • Favour use of var keyword for type declarations. var may also be used when the value is a cast null.
  • Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
  • Propose changes that only use the Java 21 API, not the API of Guava.
  • The pattern matching instanceof expression safely handles null, returning false.

Pattern **/main/java/**/*.java: - This project is mature and must provide a stable backwards-compatible public Java API.

  • In the 'Walkthrough' section, you must always provide a list of up to 25 changes to the public Java API that will affect end users.
    If there are no changes, you must explicitly state that there are no changes to the public Java API in this PR.
    The public Java API is defined as public and protected methods on public classes, plus the file module-info.java.
    Provide the list by deeply analysing code flow, which incudes analysing code flow through private methods and calls to Guava and Java 21.
    Changes to be reported on include:
    • New or removed methods in the public Java API
    • Changes to method return types or parameter types in the public Java API
    • Changes to method behaviour in the public Java API that might affect consumers
  • This project uses System.out.println instead of logging
  • This project tends to prefer for loops to streams for performance reasons, however either form is acceptable.
    Do not make suggestions to change between streams and for loops or vice versa.
src/main/java/org/joda/beans/impl/RecordBeanBuilder.java (2)

Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.

  • Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
  • Validate that immutable local variables are not annotated with final unless the variable is required for use in an inner class.
  • Favour use of var keyword for type declarations. var may also be used when the value is a cast null.
  • Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
  • Propose changes that only use the Java 21 API, not the API of Guava.
  • The pattern matching instanceof expression safely handles null, returning false.

Pattern **/main/java/**/*.java: - This project is mature and must provide a stable backwards-compatible public Java API.

  • In the 'Walkthrough' section, you must always provide a list of up to 25 changes to the public Java API that will affect end users.
    If there are no changes, you must explicitly state that there are no changes to the public Java API in this PR.
    The public Java API is defined as public and protected methods on public classes, plus the file module-info.java.
    Provide the list by deeply analysing code flow, which incudes analysing code flow through private methods and calls to Guava and Java 21.
    Changes to be reported on include:
    • New or removed methods in the public Java API
    • Changes to method return types or parameter types in the public Java API
    • Changes to method behaviour in the public Java API that might affect consumers
  • This project uses System.out.println instead of logging
  • This project tends to prefer for loops to streams for performance reasons, however either form is acceptable.
    Do not make suggestions to change between streams and for loops or vice versa.
src/main/java/org/joda/beans/impl/RecordMetaProperty.java (2)

Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.

  • Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
  • Validate that immutable local variables are not annotated with final unless the variable is required for use in an inner class.
  • Favour use of var keyword for type declarations. var may also be used when the value is a cast null.
  • Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
  • Propose changes that only use the Java 21 API, not the API of Guava.
  • The pattern matching instanceof expression safely handles null, returning false.

Pattern **/main/java/**/*.java: - This project is mature and must provide a stable backwards-compatible public Java API.

  • In the 'Walkthrough' section, you must always provide a list of up to 25 changes to the public Java API that will affect end users.
    If there are no changes, you must explicitly state that there are no changes to the public Java API in this PR.
    The public Java API is defined as public and protected methods on public classes, plus the file module-info.java.
    Provide the list by deeply analysing code flow, which incudes analysing code flow through private methods and calls to Guava and Java 21.
    Changes to be reported on include:
    • New or removed methods in the public Java API
    • Changes to method return types or parameter types in the public Java API
    • Changes to method behaviour in the public Java API that might affect consumers
  • This project uses System.out.println instead of logging
  • This project tends to prefer for loops to streams for performance reasons, however either form is acceptable.
    Do not make suggestions to change between streams and for loops or vice versa.
src/main/java/org/joda/beans/MetaBeans.java (2)

Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.

  • Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
  • Validate that immutable local variables are not annotated with final unless the variable is required for use in an inner class.
  • Favour use of var keyword for type declarations. var may also be used when the value is a cast null.
  • Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
  • Propose changes that only use the Java 21 API, not the API of Guava.
  • The pattern matching instanceof expression safely handles null, returning false.

Pattern **/main/java/**/*.java: - This project is mature and must provide a stable backwards-compatible public Java API.

  • In the 'Walkthrough' section, you must always provide a list of up to 25 changes to the public Java API that will affect end users.
    If there are no changes, you must explicitly state that there are no changes to the public Java API in this PR.
    The public Java API is defined as public and protected methods on public classes, plus the file module-info.java.
    Provide the list by deeply analysing code flow, which incudes analysing code flow through private methods and calls to Guava and Java 21.
    Changes to be reported on include:
    • New or removed methods in the public Java API
    • Changes to method return types or parameter types in the public Java API
    • Changes to method behaviour in the public Java API that might affect consumers
  • This project uses System.out.println instead of logging
  • This project tends to prefer for loops to streams for performance reasons, however either form is acceptable.
    Do not make suggestions to change between streams and for loops or vice versa.
src/main/java/org/joda/beans/JodaBeanUtils.java (2)

Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.

  • Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
  • Validate that immutable local variables are not annotated with final unless the variable is required for use in an inner class.
  • Favour use of var keyword for type declarations. var may also be used when the value is a cast null.
  • Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
  • Propose changes that only use the Java 21 API, not the API of Guava.
  • The pattern matching instanceof expression safely handles null, returning false.

Pattern **/main/java/**/*.java: - This project is mature and must provide a stable backwards-compatible public Java API.

  • In the 'Walkthrough' section, you must always provide a list of up to 25 changes to the public Java API that will affect end users.
    If there are no changes, you must explicitly state that there are no changes to the public Java API in this PR.
    The public Java API is defined as public and protected methods on public classes, plus the file module-info.java.
    Provide the list by deeply analysing code flow, which incudes analysing code flow through private methods and calls to Guava and Java 21.
    Changes to be reported on include:
    • New or removed methods in the public Java API
    • Changes to method return types or parameter types in the public Java API
    • Changes to method behaviour in the public Java API that might affect consumers
  • This project uses System.out.println instead of logging
  • This project tends to prefer for loops to streams for performance reasons, however either form is acceptable.
    Do not make suggestions to change between streams and for loops or vice versa.
🔇 Additional comments (13)
src/main/java/org/joda/beans/impl/RecordBean.java (2)

65-72: Ensure reflective registration is safe in multi-threaded environments.
Multiple threads might concurrently try to register the same record meta-bean. Consider whether this method—together with any usage in the MetaBeans class—requires additional synchronisation to avoid redundant or conflicting registrations.


88-91: Default method usage is appropriate.
Providing a default metaBean() is a neat way to simplify record-based beans, ensuring they do not need boilerplate code. The approach here aligns with the library’s design.

src/main/java/org/joda/beans/impl/RecordMetaBean.java (3)

42-69: Reflection-based construction is well-structured but verify security constraints.
Using MethodHandles.Lookup and reflection is flexible. However, confirm that only trusted code is permitted to register record beans. If untrusted code can register malicious records, reflection-based handle usage may pose a security risk.


71-79: Error handling is suitable for inaccessible record components.
An IllegalArgumentException is thrown for inaccessible getters. This is consistent with the rest of the library’s approach for invalid bean definitions.


81-99: Constructor handle creation looks robust.
The code uses asSpreader and sets the return type to ImmutableBean, ensuring a consistent signature for building record instances. This mechanism appears coherent with the library’s reflection strategy.

src/test/java/org/joda/beans/sample/RecordStrIntPair.java (1)

26-28: Test record usage is concise and illustrative.
The record-based bean is straightforward, effectively demonstrating how to implement RecordBean. It aids test coverage for record integration.

src/main/java/org/joda/beans/impl/RecordBeanBuilder.java (1)

27-73: Builder implementation is clear and consistent with immutable patterns.
The RecordBeanBuilder sets property values in an array before constructing the immutable bean. This approach matches the existing Joda-Beans style.

src/main/java/org/joda/beans/impl/light/LightBeanBuilder.java (1)

42-42: Constructor Parameter Documentation Check
The new data parameter is clearly documented and aids in storing property values. This change looks logically consistent with the builder approach in this package.

src/main/java/org/joda/beans/impl/RecordMetaProperty.java (1)

1-133: Comprehensive New Class Implementation
This class appears to provide well-structured, immutable record meta-properties, with appropriate handling of getter methods and a clear exception when writing is attempted. The code aligns with record immutability in Java 21.

src/test/java/org/joda/beans/TestRecordBean.java (1)

1-149: Overall Test Coverage
The new tests thoroughly validate the functionality of record beans, including builder usage, exception handling, and JSON serialisation. They adhere to best practices, verifying both public and private record handling.

src/main/java/org/joda/beans/impl/light/LightMetaProperty.java (1)

129-130: Variable Name Correction
Renaming the local variable to methodType improves clarity. The updated invocation in lookup.findVirtual now matches the naming convention consistently throughout the method.

src/main/java/org/joda/beans/JodaBeanUtils.java (2)

408-408: Ensure immutability or document the risk of subsequent modification.

Replacing Map.copyOf(map) with Collections.unmodifiableMap(map) creates a read-only view, which can still reflect changes to the underlying LinkedHashMap. If immutability is essential, ensure map is never mutated after this point, or switch to a fully immutable map creation approach.


776-776: Confirm that shared references are not mutated elsewhere.

Similar to line 408, returning Collections.unmodifiableMap(resolved) prevents direct modifications on the returned map but does not guarantee full immutability if resolved is changed later. Verify that no further changes occur to resolved downstream to avoid unexpected behaviour.

* Add support for JLS records as beans
* Avoid `Map.copyOf` as iteration is random
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

🧹 Nitpick comments (6)
src/main/java/org/joda/beans/impl/RecordMetaProperty.java (2)

36-36: Consider verifying the completeness of JavaDoc comments.
The class-level JavaDoc clearly states its purpose, but you may wish to add further usage examples or note any limitations (for instance, that setters are unsupported). This can help future maintainers understand the context and constraints.


103-105: Revisit the exception message for clarity.
When an unknown property is encountered, it currently raises "Unknown property: ...". Consider adding additional contextual cues, for example referencing the bean class.

src/main/java/org/joda/beans/impl/RecordMetaBean.java (1)

65-67: Add more detail to the exception message.
When the record constructor is missing, it might be helpful to mention the parameter types that were attempted. This could speed up debugging for record-based beans.

src/test/java/org/joda/beans/TestRecordBean.java (1)

90-93: Ensure JSON-based tests cover all record properties.
These tests confirm correct serialisation/deserialisation for two basic properties. If more complex properties or derived properties are introduced in future, ensure coverage of edge cases such as null values.

src/main/java/org/joda/beans/MetaBeans.java (1)

100-112: LGTM! Well-implemented synchronization and record handling.

The synchronization block effectively prevents race conditions, and the record handling logic is properly implemented with appropriate type safety.

Consider extracting the record handling logic into a private method for better readability:

-            if (cls.isRecord() && ImmutableBean.class.isAssignableFrom(cls)) {
-                @SuppressWarnings({"rawtypes", "unchecked"})
-                var metaBean = RecordBean.register((Class) cls, MethodHandles.lookup());
-                return metaBean;
-            }
+            var metaBean = handleRecordClass(cls);
+            if (metaBean != null) {
+                return metaBean;
+            }

+    private static MetaBean handleRecordClass(Class<?> cls) {
+        if (cls.isRecord() && ImmutableBean.class.isAssignableFrom(cls)) {
+            @SuppressWarnings({"rawtypes", "unchecked"})
+            var metaBean = RecordBean.register((Class) cls, MethodHandles.lookup());
+            return metaBean;
+        }
+        return null;
+    }
src/changes/changes.xml (1)

Line range hint 1-1: Walkthrough of Public API Changes

  1. New public interface RecordBean that can be implemented by records
  2. Modified behaviour in MetaBeans.lookup(Class<?>) to support record classes
  3. New static method RecordBean.register(Class, MethodHandles.Lookup) for registering record meta-beans

These changes will affect end users who:

  • Want to implement beans using Java records
  • Use the meta-bean lookup functionality with record classes
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 03c75c0 and 1d6d12e.

📒 Files selected for processing (11)
  • src/changes/changes.xml (1 hunks)
  • src/main/java/org/joda/beans/JodaBeanUtils.java (2 hunks)
  • src/main/java/org/joda/beans/MetaBeans.java (3 hunks)
  • src/main/java/org/joda/beans/impl/RecordBean.java (1 hunks)
  • src/main/java/org/joda/beans/impl/RecordBeanBuilder.java (1 hunks)
  • src/main/java/org/joda/beans/impl/RecordMetaBean.java (1 hunks)
  • src/main/java/org/joda/beans/impl/RecordMetaProperty.java (1 hunks)
  • src/main/java/org/joda/beans/impl/light/LightBeanBuilder.java (1 hunks)
  • src/main/java/org/joda/beans/impl/light/LightMetaProperty.java (1 hunks)
  • src/test/java/org/joda/beans/TestRecordBean.java (1 hunks)
  • src/test/java/org/joda/beans/sample/RecordStrIntPair.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/test/java/org/joda/beans/sample/RecordStrIntPair.java
  • src/main/java/org/joda/beans/impl/light/LightBeanBuilder.java
  • src/main/java/org/joda/beans/impl/RecordBeanBuilder.java
  • src/main/java/org/joda/beans/impl/light/LightMetaProperty.java
  • src/main/java/org/joda/beans/JodaBeanUtils.java
🧰 Additional context used
📓 Path-based instructions (5)
src/main/java/org/joda/beans/impl/RecordBean.java (2)

Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.

  • Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
  • Validate that immutable local variables are not annotated with final unless the variable is required for use in an inner class.
  • Favour use of var keyword for type declarations. var may also be used when the value is a cast null.
  • Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
  • Propose changes that only use the Java 21 API, not the API of Guava.
  • The pattern matching instanceof expression safely handles null, returning false.

Pattern **/main/java/**/*.java: - This project is mature and must provide a stable backwards-compatible public Java API.

  • In the 'Walkthrough' section, you must always provide a list of up to 25 changes to the public Java API that will affect end users.
    If there are no changes, you must explicitly state that there are no changes to the public Java API in this PR.
    The public Java API is defined as public and protected methods on public classes, plus the file module-info.java.
    Provide the list by deeply analysing code flow, which incudes analysing code flow through private methods and calls to Guava and Java 21.
    Changes to be reported on include:
    • New or removed methods in the public Java API
    • Changes to method return types or parameter types in the public Java API
    • Changes to method behaviour in the public Java API that might affect consumers
  • This project uses System.out.println instead of logging
  • This project tends to prefer for loops to streams for performance reasons, however either form is acceptable.
    Do not make suggestions to change between streams and for loops or vice versa.
src/main/java/org/joda/beans/impl/RecordMetaProperty.java (2)

Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.

  • Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
  • Validate that immutable local variables are not annotated with final unless the variable is required for use in an inner class.
  • Favour use of var keyword for type declarations. var may also be used when the value is a cast null.
  • Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
  • Propose changes that only use the Java 21 API, not the API of Guava.
  • The pattern matching instanceof expression safely handles null, returning false.

Pattern **/main/java/**/*.java: - This project is mature and must provide a stable backwards-compatible public Java API.

  • In the 'Walkthrough' section, you must always provide a list of up to 25 changes to the public Java API that will affect end users.
    If there are no changes, you must explicitly state that there are no changes to the public Java API in this PR.
    The public Java API is defined as public and protected methods on public classes, plus the file module-info.java.
    Provide the list by deeply analysing code flow, which incudes analysing code flow through private methods and calls to Guava and Java 21.
    Changes to be reported on include:
    • New or removed methods in the public Java API
    • Changes to method return types or parameter types in the public Java API
    • Changes to method behaviour in the public Java API that might affect consumers
  • This project uses System.out.println instead of logging
  • This project tends to prefer for loops to streams for performance reasons, however either form is acceptable.
    Do not make suggestions to change between streams and for loops or vice versa.
src/main/java/org/joda/beans/impl/RecordMetaBean.java (2)

Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.

  • Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
  • Validate that immutable local variables are not annotated with final unless the variable is required for use in an inner class.
  • Favour use of var keyword for type declarations. var may also be used when the value is a cast null.
  • Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
  • Propose changes that only use the Java 21 API, not the API of Guava.
  • The pattern matching instanceof expression safely handles null, returning false.

Pattern **/main/java/**/*.java: - This project is mature and must provide a stable backwards-compatible public Java API.

  • In the 'Walkthrough' section, you must always provide a list of up to 25 changes to the public Java API that will affect end users.
    If there are no changes, you must explicitly state that there are no changes to the public Java API in this PR.
    The public Java API is defined as public and protected methods on public classes, plus the file module-info.java.
    Provide the list by deeply analysing code flow, which incudes analysing code flow through private methods and calls to Guava and Java 21.
    Changes to be reported on include:
    • New or removed methods in the public Java API
    • Changes to method return types or parameter types in the public Java API
    • Changes to method behaviour in the public Java API that might affect consumers
  • This project uses System.out.println instead of logging
  • This project tends to prefer for loops to streams for performance reasons, however either form is acceptable.
    Do not make suggestions to change between streams and for loops or vice versa.
src/test/java/org/joda/beans/TestRecordBean.java (2)

Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.

  • Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
  • Validate that immutable local variables are not annotated with final unless the variable is required for use in an inner class.
  • Favour use of var keyword for type declarations. var may also be used when the value is a cast null.
  • Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
  • Propose changes that only use the Java 21 API, not the API of Guava.
  • The pattern matching instanceof expression safely handles null, returning false.

Pattern **/test/java/**/*.java: For test code, focus on:

  • Correctness of test assertions
  • Test coverage of edge cases
  • Clear test naming and documentation
  • Encourage test methods to be package-scoped where possible
  • Be more lenient with code style and minor optimisations
src/main/java/org/joda/beans/MetaBeans.java (2)

Pattern **/*.java: - Review code using Java 21 standards, taking into account the rules defined by src/main/checkstyle/checkstyle.xml.

  • Validate that code indentation uses spaces, not tabs, with an indent of multiple of 4.
  • Validate that immutable local variables are not annotated with final unless the variable is required for use in an inner class.
  • Favour use of var keyword for type declarations. var may also be used when the value is a cast null.
  • Use a coding standard where multi-line expressions have operators and tenary separators at the end of line.
  • Propose changes that only use the Java 21 API, not the API of Guava.
  • The pattern matching instanceof expression safely handles null, returning false.

Pattern **/main/java/**/*.java: - This project is mature and must provide a stable backwards-compatible public Java API.

  • In the 'Walkthrough' section, you must always provide a list of up to 25 changes to the public Java API that will affect end users.
    If there are no changes, you must explicitly state that there are no changes to the public Java API in this PR.
    The public Java API is defined as public and protected methods on public classes, plus the file module-info.java.
    Provide the list by deeply analysing code flow, which incudes analysing code flow through private methods and calls to Guava and Java 21.
    Changes to be reported on include:
    • New or removed methods in the public Java API
    • Changes to method return types or parameter types in the public Java API
    • Changes to method behaviour in the public Java API that might affect consumers
  • This project uses System.out.println instead of logging
  • This project tends to prefer for loops to streams for performance reasons, however either form is acceptable.
    Do not make suggestions to change between streams and for loops or vice versa.
🔇 Additional comments (7)
src/main/java/org/joda/beans/impl/RecordBean.java (1)

52-52: Ensure that the interface remains forward-compatible.
As this interface is public, any changes in future releases, such as adding methods, could break existing implementations. Keep the surface area minimal or add default methods as necessary.

src/main/java/org/joda/beans/impl/RecordMetaBean.java (1)

42-42: Confirm internal visibility of this class.
Since this class is package-private, it is effectively internal API. Ensure this approach is intentional, as external usage could be blocked if clients rely on reflection.

src/test/java/org/joda/beans/TestRecordBean.java (1)

35-39: Good practice employing static initialisation for private record registration.
Using the static { RecordBean.register(...) } approach ensures that the meta-bean is registered once in a controlled manner. This is a tidy pattern for non-public records in tests.

src/main/java/org/joda/beans/MetaBeans.java (3)

18-18: LGTM! Required imports added for record support.

The new imports for MethodHandles and RecordBean are necessary for implementing the record support functionality.

Also applies to: 24-24


37-37: LGTM! Well-documented field choices.

The fields use appropriate concurrent collections, and the comments clearly explain why ClassValue is not used in each case.

Also applies to: 43-43


Line range hint 113-134: LGTM! Comprehensive provider annotation handling.

The provider annotation handling is well-structured with proper error handling and clear exception messages.

src/changes/changes.xml (1)

17-20: LGTM! Clear documentation of record support feature.

The changes are well-documented, clearly stating the addition of record support and the new RecordBean interface.

@jodastephen jodastephen merged commit e462e8b into main Dec 31, 2024
5 checks passed
@jodastephen jodastephen deleted the record-bean branch December 31, 2024 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

1 participant