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

Java Record support #365

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Verdent
Copy link
Contributor

@Verdent Verdent commented Dec 17, 2024

This is still WIP.

@Verdent Verdent self-assigned this Dec 17, 2024
Copy link

@njr-11 njr-11 left a comment

Choose a reason for hiding this comment

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

Excellent start on the record support! I added a question about whether to allow compact constructor and suggested a few minor grammar corrections.

Comment on lines +462 to +463
By default, public default no-arguments constructor is required for deserialization of the classes.
In the case of records, it is required to have only default constructor, which was generated for the record.
Copy link

Choose a reason for hiding this comment

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

This seems to rule out having a compact constructor to override the default constructor that is generated for a record. Was that the intent? I can see where you might want to avoid the possibility that the compact constructor reassigns a record component. But I think it's more likely users would include the compact constructor for purposes like validating the supplied values.

Here is one idea if we want to conditionally allow the compact constructor for record:

Suggested change
By default, public default no-arguments constructor is required for deserialization of the classes.
In the case of records, it is required to have only default constructor, which was generated for the record.
By default, a public default no-arguments constructor is required for deserialization of the class.
In the case of records, the record's default constructor is required for deserialization of the record. The record can include a compact constructor if the compact constructor does not reassign any of the record components.

Any instance passed to a deserialization operation must have a public or protected no-argument constructor. Implementations SHOULD throw an error if this condition is not met. This limitation does not apply to serialization operations, as well as to classes which specify explicit instantiation methods as described in section 4.5.
Any class instance passed to a deserialization operation must have a public or protected no-argument constructor. Implementations SHOULD throw an error if this condition is not met. This limitation does not apply to serialization operations, as well as to classes which specify explicit instantiation methods as described in section 4.5.

Any record instance passed to a deserialization operation must have only default generated constructor. Implementations SHOULD throw an error if this condition is not met. This limitation does not apply to serialization operations, as well as to records which specify explicit instantiation methods as described in section 4.5.
Copy link

Choose a reason for hiding this comment

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

To allow compact constructor (see prior comment),

Suggested change
Any record instance passed to a deserialization operation must have only default generated constructor. Implementations SHOULD throw an error if this condition is not met. This limitation does not apply to serialization operations, as well as to records which specify explicit instantiation methods as described in section 4.5.
Any record instance passed to a deserialization operation must have its default generated constructor (and optionally a compact constructor that does not reassign any record component values) as its only constructor. Implementations SHOULD throw an error if this condition is not met. This limitation does not apply to serialization operations, as well as to records which specify explicit instantiation methods as described in section 4.5.

@@ -490,7 +492,7 @@ For bounded parameter type, let’s use bounds `B~1~,…,B~m~`.

If `m = 1`, then the most specific parameter type MUST be derived from the given bound.

If is class or interface, the most specific parameter type MUST be the class or interface.
If is class, record or interface, the most specific parameter type MUST be the class, record or interface.
Copy link

Choose a reason for hiding this comment

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

Minor grammar correction:

Suggested change
If is class, record or interface, the most specific parameter type MUST be the class, record or interface.
If it is a class, record or interface, the most specific parameter type MUST be the class, record or interface.

* Getter is annotated with `@JsonbTransient`
** Exception is thrown if when the field or this getter are annotated with other JSON Binding annotations. Exception is not thrown if JSON Binding annotations are presented on the setter.
** Exception is thrown if when the field, record component or this getter are annotated with other JSON Binding annotations. Exception is not thrown if JSON Binding annotations are presented on the setter.
Copy link

Choose a reason for hiding this comment

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

Minor grammar corrections:

Suggested change
** Exception is thrown if when the field, record component or this getter are annotated with other JSON Binding annotations. Exception is not thrown if JSON Binding annotations are presented on the setter.
** An exception is thrown if the field, record component or this getter are annotated with other JSON Binding annotations. An exception is not thrown if JSON Binding annotations are present on the setter.

}
----

The same JSON document will be produced if @JsonbProperty annotation is placed on getter like this:
Copy link
Contributor

Choose a reason for hiding this comment

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

With records, this kind of method is not called "getter" but "accessor".

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.

4 participants