javalin / javalin-openapi

Annotation processor for compile-time OpenAPI & JsonSchema, with out-of-the-box support for Javalin 5.x, Swagger & ReDoc
https://github.com/javalin/javalin-openapi/wiki
Apache License 2.0
45 stars 17 forks source link

Jackson JsonNode reflection generates duplicate value in 'required' array #182

Closed akram-hameed closed 1 year ago

akram-hameed commented 1 year ago

No snippet for this, sorry, but the behaviour is very simple to explain:

  1. Define a type that contains a property that is a Jackson JsonNode
  2. Mark a controller with an annotation of OpenApiContent with from your type
  3. View generated OpenAPI docs, required field in the schema for the discovered JsonNode type will contain two entries for the empty property (derived from isEmpty method).

Potential reason for this: JsonNode is an abstract class and isEmpty is overridden in child classes. BUT: there are other properties treated similarly and they are not duplicated in the output.

Overall though: what is a better way to deal with this? Most of the discovered fields for the JsonNode (and other types, mind you) are certainly not required. I'd be inclined to lean on the side of permittivity myself: don't put anything in required unless the user opts in somehow (obviously does not help for library classes like this one!!).

dzikoysk commented 1 year ago

Well, the issue here is that I'm not quite sure why there's a JsonNode as a property in the dto that will be serialized/deserialized to body. Responses should be mapped to plain Java objects, and that's how this plugin interprets them at compile-time. By default, each non-null property is marked as required, and it makes sense, because we don't want to assign null/missing properties to type-safe non-null fields in our data classes.

akram-hameed commented 1 year ago

Not disagreeing that in a perfect world, everything could be converted to a POJO or POKO, but in this case, it's a convenience to treat the JsonNode as an arbitrary piece of JSON that will be interpreted by a piece of software further down the chain. All we care about is that it is in fact, legal JSON.

Perhaps the solution to my issue is to remove the ambiguity here for the parsing: I always expect a JSON object, therefore I can choose a specialisation of JsonNode (ObjectNode) which will presumably help method resolution for required. Even so, maybe now is a good time to ask a question: if i wanted to instruct the plugin to never attempt to derive required fields etc for a given type, is there a way to do that?

I presume I'd have to influence kapt rather than add a runtime call someplace?

dzikoysk commented 1 year ago

I guess you can just do sth like that:

@OpenApiPropertyType(Object::class, nullable = Nullability.NULLABLE)
JsonNode jsonProperty;

If it may contain various nested json object. Object will tell OpenApi to not generate any fields from JsonNode & nullable type will exclude it from required properties.