Serializable Wrappers

[scottf]

It's impractical to make configuration object serializable because it would make any changes like an addition of a field NOT backward compatible because the serialVersionUID would change, versioning the entire class, requiring a semver major version.

So instead,

• relevant classes are ensured to be JsonSerializable if not already
• builders are modified to accept json.
• wrapper classes have been added that use the json to serialize and deserialize

[somratdnutanix]

Thanks @scottf .
The changes look solid.
Please ensure the tests cover all possible configurations, including edge cases, for both serialization and deserialization. We can include tests for error scenarios such as handling invalid JSON input.
Once its merged, I can refactor the connector code to use this instead of what we have used now.

[scottf]

Please ensure the tests cover all possible configurations, including edge cases, for both serialization and deserialization

What edge cases? If the json is invalid, it will throw a parse error. Also since I run the json through the builder invalid data should be caught there but...I don't think it's necessary to handle edge cases. The expectation is that the json comes from the objects' toJson method so should never be invalid. The unit tests all test round trip of the serialization.
Is there something I am missing?

[somratdnutanix]

Okay, then it should be pretty much covered @scottf.
Looks good. 👍

[somratdnutanix]

It's impractical to make configuration object serializable because it would make any changes like an addition of a field NOT backward compatible because the serialVersionUID would change, versioning the entire class, requiring a semver major version.

So instead,

• relevant classes are ensured to be JsonSerializable if not already
• builders are modified to accept json.
• wrapper classes have been added that use the json to serialize and deserialize

thanks @scottf for this.
we won't have to use proxy pattern for each field now.
will refactor our connector code and be back with you.

[somratdnutanix]

got these warning when I tried to build Maven locally, mostly due to missing Javadoc comments, @scottf .

> Task :javadoc ConsumerConfiguration.java:697: warning: no @param for json public Builder json(String json) throws JsonParseException { ^ ConsumerConfiguration.java:697: warning: no @return public Builder json(String json) throws JsonParseException { ^ ConsumerConfiguration.java:697: warning: no @throws for io.nats.client.support.JsonParseException public Builder json(String json) throws JsonParseException { ^ ConsumerConfiguration.java:704: warning: no @param for v public Builder jsonValue(JsonValue v) { ^ ConsumerConfiguration.java:704: warning: no @return public Builder jsonValue(JsonValue v) { ^ ConsumerConfiguration.java:650: warning: no @param for cc public Builder(ConsumerConfiguration cc) { ^ BaseConsumeOptions.java:124: warning: no @param for v public B jsonValue(JsonValue v) { ^ BaseConsumeOptions.java:124: warning: no @return public B jsonValue(JsonValue v) { ^ 8 warnings

[scottf]

got these warning when I tried to build Maven locally, mostly due to missing Javadoc comments, @scottf
Warnings right? I'll fix the java doc, thanks