feat(api): Ideas/examples for a possible optional JsonType API#691
feat(api): Ideas/examples for a possible optional JsonType API#691
Conversation
These 4 suggestions show possible implementations for the support of OptionalJsonProperty without relying on a big number of manual constructors as suggested by the creators of Jackson. Consequently, none of those implementations are working at this point and should only be seen as examples of how an API could look like, especially because the rest of the API would be hidden, and only the annotation are used by the users of the Telestion System.
|
The current state is documented here: |
|
I believe that the cleanest, most readable, and most flexible approach to go about this is to override the getter methods in the records. Then, it looks something like this: public record Room(@JsonProperty(defaultValue = "10") int size) implements JsonRecord {
public Room {
if (size() < 0) {
throw new IllegalArgumentException("Size must not be negative");
}
}
public int size() {
return Objects.requireNonNullElse(size, 10);
}
}By "only" having the one constructor (and no need for a default constructor anymore), we can have additional validation in this constructor (as you can see here) which we can also use to determine if a message on the event bus is a valid object of that type (in this case, This also doesn't require developers to learn additional concepts (such as the handling of default constructors or new annotations), can handle complex objects, etc. The only downside I see with this, which compared to the other methods is in my opinion far less severe in terms of issues, is that you have to be careful to call the actual getter function (instead of the parameter itself) inside the record's functions (e.g., in the validation |
Optional JsonProperties for the Configurations of the Telestion Verticles
Problem
As soon as verticles get complexer, optional JsonProperties for the defining Configuration are helpful to reduce the work by the configurating party - the end user - who expectedly has little to no experience at all with the Telestion Ecosystem. Therefore, these should be deemed a highly important role, making the project a lot more appealing to annoyed software developers, being woken up at 9 am (very early for a developer) to fix the missing parts of the configurations... 😆
Current state
At the moment, there is no special support for this feature apart from the native support out of the box from the Jackson Library or additional libraries. Unfortunately, the Jackson API does not easily support default parameters for inferring other than for documenation reasons as the Java Type System and its stubborness makes a good design difficult (or even impossible) and no possible solution. To be fair, there is one possibility to add those to the existing system. One has to add an additional constructor without one of the fields, which does not only stack up with more and more record parameters (problem 1), but also imposes problems with parameters of the same type in one class (problem 2). An example showcasing both of those problems is seen below. We want to give Default parameters to every parameter:
Note that with this solution and the massive amounts of equal implementations also small mistakes as shown in the constructor for
String param1lead to fast bugs in the development of more complex verticles. Here, obviously the 43 should be a 42. This obviously can also be targetted by constant static variables, but imho this is still a bad style of implementation if you need an extra mechanic to fix a problem that shouldn't even exist in the first place.Implementation Suggestions
In all cases, the manual overhead of adding default information to each of the Record values should be as low as possible. Therefore, an annotation-based approach is suggested which complements the existing
@JsonPropertyfrom Jackson. As seen in the examples, the Annotations are only added additionally to the fields. In a future release, it could also be thought of combining those additional Annotations with the JsonProperty into one Property, reducing the overhead even further. In the background a manual ObjectMapper (already provided by the Jackson API) could be used, which could work hand in hand with the proposed new Launcher API by @fussel178 - the registering would take place in this new Launcher API at the start of the Application. Currently, up to this point, the registering alternatively would take place in the Telestion class in the application package.Details about the implementation
These 4 suggestions show possible implementations for the support of OptionalJsonProperty without relying on a big number of manual constructors as suggested by the creators of Jackson. Consequently, none of those implementations are working at this point and should only be seen as examples of how an API could look like, especially because the rest of the API would be hidden, and only the annotation are used by the users of the Telestion System.
Meta information about this PR
Related links
As far as I know, no current Issue is targetting this problem.
CLA
I would love some feedback and an open discussion about this problem! 👀