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

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

Closed
wants to merge 1 commit into from

Conversation

maths22
Copy link

@maths22 maths22 commented Apr 25, 2024

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
Copy link
Member

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
Copy link
Author

maths22 commented Apr 27, 2024

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
Copy link
Member

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

@maths22
Copy link
Author

maths22 commented Apr 27, 2024

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
Copy link
Member

dzikoysk commented Apr 27, 2024

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
Copy link
Author

maths22 commented Apr 27, 2024

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

@dzikoysk
Copy link
Member

I've reported it in #217 to gather some extra feedback on this. In case of further related issues, keep me posted there.

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

Successfully merging this pull request may close these issues.

2 participants