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

Allow putting OpenApi annotations on fields in addition to methods #216

Closed maths22 closed 4 months ago

maths22 commented 4 months ago

Most of the property attributes work fine and make sense on fields, but don't currently support being put there. This fixes that limitation, which is useful when using @OpenApiByFields. (My immediate use case is OpenApiDescription, but the others seemed useful too.)

dzikoysk commented 4 months ago

Not opening this API for fields was a conscious decision to enforce people to using properties for this. It seems to be a valid choice, because every use case I've seen so far could be covered by this. The only place where it's not working is a raw java class with a set of public fields - for that use case, I've added @OpenApiByFields, but I don't really want this particular workaround to affect the rest of the API 🤔

maths22 commented 4 months ago

I feel like if OpenApiByFields is going to be part of the API then you should be able to use fields in the same ways as property accessors. I have an API that was built around GSON serialization, and so all the objects only have fields since that is all GSON needs/wants. It would seem silly to me to need to create unused accessor methods just to have somewhere to stick OpenAPI annotations.

dzikoysk commented 4 months ago

Aren't Java records dedicated for such use-cases?

maths22 commented 4 months ago

In modern java applications, yes, though there are a lot of classes (like the ones I am working with now) that predate the existence of records. I guess I just don't really see a good reason for the OpenApi plugin to make life harder for you if you use fields.

Edit: having just tried to convert one of the use cases in my API to use records, it doesn't work because java records cannot inherit from other records, so while they may fit many/most use cases they don't fit all.

dzikoysk commented 4 months ago

I'm still kinda disagreeing with the direction of this change - especially that we've just confirmed that it's a matter of legacy code. Did you consider using a @CustomAnnotation? It could work like this:

@Target(FUNCTION, PROPERTY_GETTER, PROPERTY_SETTER, FIELD, CLASS)
@Retention(RUNTIME)
@CustomAnnotation
annotation class OpenApiFieldDescription(
    val description: String
)

As far as I remember, it should be processed when fields processing is enabled.

maths22 commented 4 months ago

Thank you for pointing out that I could use @CustomAnnotation to do this. That seems to work correctly for me.

dzikoysk commented 4 months ago

I've reported it in https://github.com/javalin/javalin-openapi/issues/217 to gather some extra feedback on this. In case of further related issues, keep me posted there.