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

@GenIgnore non-canonical getters/setters of Redis[Connect]Options.endpoints #439

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

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Mar 22, 2024

The RedisOptions class has a field endpoints of type List<String> for which multiple accessors and mutators exist:

  • getEndpoints(): the canonical getter
  • setEndpoints(): the canonical setter
  • getEndpoint(): returns the first element of the list
  • setEndpoint(): turns the list into a single-element list
  • addEndpoint(): adds one element to the list
  • setConnectionString(): same as setEndpoint()
  • addConnectionString(): same as addEndpoint()

The RedisConnectOptions class, which is public but is only used internally, has the same problem.

All these methods are treated as individual properties by the code generator that produces the JSON serialization/deserialization classes, even though they are all backed by a single field. Since the code generator considers properties in declaration order (in upcoming Vert.x 5; it is lexicographic order in Vert.x 4), we had to reorder the methods so that serialization and deserialization does not lose information.

That is arguably a provisional solution, as it is very much not obvious that declaration order matters. This commit provides a permanent solution: all the non-canonical methods are marked as @GenIgnore. Only the getEndpoints() and setEndpoints() methods are used for serialization/deserialization.

This is a breaking change, so it shall not be backported to Vert.x 4 and shall be documented as such.

…points

The `RedisOptions` class has a field `endpoints` of type `List<String>`
for which multiple accessors and mutators exist:

- `getEndpoints()`: the canonical getter
- `setEndpoints()`: the canonical setter
- `getEndpoint()`: returns the first element of the list
- `setEndpoint()`: turns the list into a single-element list
- `addEndpoint()`: adds one element to the list
- `setConnectionString()`: same as `setEndpoint()`
- `addConnectionString()`: same as `addEndpoint()`

The `RedisConnectOptions` class, which is `public` but is only used internally,
has the same problem.

All these methods are treated as individual properties by the code generator
that produces the JSON serialization/deserialization classes, even though
they are all backed by a single field. Since the code generator considers
properties in declaration order (in upcoming Vert.x 5; it is lexicographic
order in Vert.x 4), we had to reorder the methods so that serialization
and deserialization does not lose information.

That is arguably a provisional solution, as it is very much not obvious that
declaration order matters. This commit provides a permanent solution: all
the non-canonical methods are marked as `@GenIgnore`. Only the `getEndpoints()`
and `setEndpoints()` methods are used for serialization/deserialization.

This is a breaking change, so it shall not be backported to Vert.x 4 and
shall be documented as such.
@Ladicek Ladicek requested a review from vietj March 22, 2024 10:26
@Ladicek
Copy link
Contributor Author

Ladicek commented Mar 22, 2024

This is my proposal for https://github.com/vert-x3/wiki/wiki/5.0.0-Deprecations-and-breaking-changes:

Vert.x Redis Client

JSON serialization/deserialization of RedisOptions

The RedisOptions class has multiple methods to configure the Redis endpoints. These methods are still available, but the JSON format of this class has changed to only include one canonical field: endpoints.

If you rely on the JSON form of RedisOptions objects, note that the endpoint, connectionString and connectionStrings members of the JSON object are no longer recognized by the deserializer and are no longer generated by the serializer. Ensure that the necessary information is present in the endpoints member.

If you do not rely on the JSON form of RedisOptions objects, this change does not affect you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant