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

Support accessors without get/is/set #1959

Open
t1 opened this issue Nov 12, 2023 · 12 comments
Open

Support accessors without get/is/set #1959

t1 opened this issue Nov 12, 2023 · 12 comments
Labels
enhancement New feature or request Server This issue applies to the Server side

Comments

@t1
Copy link
Collaborator

t1 commented Nov 12, 2023

In Java records, the components (a.k.a. properties) can be accessed without a get prefix as it was specified for JavaBeans. Lombok calls this prefix-less accessor style "fluent".

We obviously don't want to support that style by default. But if I annotate any method with @Name or @JsonbProperty, it should be added to the list of fields. But this doesn't work, yet.

Additional Ideas:

  • We could also automatically add fluent accessors that have been generated by Lombok (IIUC, Lombok can generate a @Generated annotation).
  • We could add a config to allow this style by default.

What do you think?

@t1 t1 added enhancement New feature or request Server This issue applies to the Server side labels Nov 12, 2023
@t1
Copy link
Collaborator Author

t1 commented Nov 12, 2023

Maybe we could also extend this to support parameterised accessors, i.e. a field in a Person class @Name("appellation") String appellation(Locale locale) ... would result in a schema type Person { appellation(locale: Locale): String ... }

@jmartisk
Copy link
Member

Please don't get me started on Lombok :)

How would we tell whether the method annotated @Name is a setter or a getter? And how would we find the counterpart?

Maybe we could also extend this to support parameterised accessors, i.e. a field in a Person class @Name("appellation") String appellation(Locale locale) ... would result in a schema type Person { appellation(locale: Locale): String ... }

This looks like just a different way to make a source field, but you would lose the batching option

@t1
Copy link
Collaborator Author

t1 commented Nov 14, 2023

You're right. I haven't thought about that. So let's drop the parameterised getters.

But then we can easily distinguish a getter from a setter by looking at the parameters: if it has none, it's a getter, otherwise a setter. And I think we don't have to find a counterpart, because a Type only looks at getters, while an Input only looks at setters, doesn't it?

@jmartisk
Copy link
Member

And what if you have a class that is both input and output?

@t1
Copy link
Collaborator Author

t1 commented Nov 14, 2023

It still has getters for the Type and setters for the Input 😁

@jmartisk
Copy link
Member

If it is both an input and output, and it has a method with @Name, then that method is what?
I don't know, it seems like further complicating an already quite complicated model (we allow public fields, or private fields with getter and setter). I'm not completely convinced it's worth it. Also we use JSON-B for (de)serializing, which I'm not sure how it will play together with this.

@t1
Copy link
Collaborator Author

t1 commented Nov 14, 2023

I think I'll have to look at the code to see how complicated it would get.

And IIRC, we use only the annotations from JSON-B, not the implementation.

@jmartisk
Copy link
Member

And IIRC, we use only the annotations from JSON-B, not the implementation.

That is in the client. Server uses JSON-B itself

@t1
Copy link
Collaborator Author

t1 commented Nov 14, 2023

Hmmm... and how do we force JSONB to consider the @Name annotation? There is no generic metadata model in JSONB, is it?

@jmartisk
Copy link
Member

look at the io.smallrye.graphql.json.GraphQLNamingStrategy class, we use this to convey some extra information to the mapper

@phillip-kruger
Copy link
Member

Yes we use JSONB in the server. I still wanted to change that to only support the annotations and not directly the impl, and rather use Jackson, or make it plug-able, but I never got around to that.

@t1
Copy link
Collaborator Author

t1 commented Nov 15, 2023

I see. So there is a metadata mechanism. Cool.
But I think we should stick to JSONB: that's the standard. I understand Jackson is faster, but that's only because Yasson is, more or less, only the RI for JSONB and not really optimized for speed. IIRC, there was an initiative to make Jackson a compliant JSONB implementation. I don't know what happened there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Server This issue applies to the Server side
Projects
None yet
Development

No branches or pull requests

3 participants