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 builderClassName in @SuperBuilder #2370

Closed
kennethjor opened this issue Feb 17, 2020 · 13 comments
Closed

Support builderClassName in @SuperBuilder #2370

kennethjor opened this issue Feb 17, 2020 · 13 comments

Comments

@kennethjor
Copy link

Describe the feature
The @Builder annotation supports the builderClassName parameter. I would be nice it @SuperBuilder did as well.

Additional context
I did check the latest version of Lombok and didn't see the parameter in there. However, it does appear that this option is supported through the config file, as shown in HandleSuperBuilder.java.

@janrieke
Copy link
Contributor

There is a reason why there is no such parameter:

  1. There are two builder classes. So either the parameter must affect both, or we need two parameters, making it even more complex.
  2. SuperBuilders in subclasses rely on the builder class name of the superclass. There is no way to detect a differing builder class name in the superclass, so we'd need another parameter to specify that.

So all in all this is too complex both for users and for us to maintain.

That being said, could you describe your usecase? Maybe we could find another solution.

@janrieke
Copy link
Contributor

janrieke commented Feb 17, 2020

PS: Allowing setting this via config is kind of a compromise: It will change the names consistently, so unless you use SuperBuilder across project borders it will just work.

@rzwitserloot
Copy link
Collaborator

It'd have to be one heck of a use-case, given the complexities as @janrieke stated. I'm pre-closing this, but feel free to answer with novel solutions or dire use cases and we can always re-evaluate :)

@kennethjor
Copy link
Author

@janmaterne @rzwitserloot Those reasons make perfect sense! I would agree it's not applicable to be able to change the name of the builder class when using @SuperBuilder.

To elaborate though, my use case is I want some of my models to be extendable (hence @SuperBuilder), but I also want them to work nicely with Jackson. As you know, this requires me to define the builder class manually, which is very ugly for classes with @SuperBuilder. I have opened a feature request with the Jackson team (https://github.com/FasterXML/jackson-future-ideas/issues/47) for a change to make this easier. It's probably better solved over there.

I'm also aware of the recent setterPrefix parameter on @Builder, which would be nice to have on @SuperBuilder. I see you have a discussion going for that at #2154.

Thank you for your quick responses and great work :)

@janrieke
Copy link
Contributor

janrieke commented Feb 17, 2020

setterPrefix will be on @SuperBuilder with the next release. Furthermore, we are working on a Lombok annotation that automatically inserts all necessary Jackson annotations into the generated code to make @(Super)Builder work in Jackson (and without having to use Jackson's defaults like the "with" prefix).

@janrieke
Copy link
Contributor

janrieke commented Mar 10, 2020

@Jacksonized @(Super)Builder will be in the next release. It will insert all necessary Jackson annotations to make Jackson use the generated builder when deserializing. No need for manual modifications to the generated builder code any more.

@kennethjor
Copy link
Author

@janrieke Sweet, thank you!

@Bittuw
Copy link

Bittuw commented May 28, 2020

@janrieke, What about field-level Jackson annotation?

@janrieke
Copy link
Contributor

@Bittuw In 1.18.12, only @JsonProperty and @JsonSetter are copied from the field to the builder's setter method, but in the next release this will be extended to include:

  • @JsonDeserialize
  • @JsonIgnore
  • @JsonAlias
  • @JsonView
  • @JacksonInject
  • @JacksonXmlProperty
  • @JsonAnySetter (only for @Singular methods)

Are you missing another annotation in that list?

@Bittuw
Copy link

Bittuw commented May 28, 2020

@janrieke, Hmm, for my classes @JsonProperty does not copied to builder class.
UPDATE-1 - Double hmm, it does not work properly with @Singular
UPDATE-2 - If @Singular#value is empty - there is always empty collection. If value equals with fields name - my type cannot be decoded with application/json into Spring WebClient. If value is not equals to field name - Jackson cannot deserialize this object.
(This is EDGE)

@janrieke
Copy link
Contributor

As already said in #2479, it's probably because the current edge does not contain that feature, yet. It's only in the sourcecode of the master branch at the moment.

@dellanira8
Copy link

@janrieke. Thanks for your great work.
Currently I'm struggling to get @SuperBuilder, @Jacksonized and @JacksonInject working, so that an additional field in my DTO is initialized besides my JSON data. Is there any update to support JacksonInject/JsonDeserialize or similiar mechanism?
In my case the extra field is required to configure a field method set via @Getter(lazy=true).
If you know another way to inject a java object to Jackson mapper just for configuration purpose, just let me know.

@janrieke
Copy link
Contributor

janrieke commented Sep 11, 2020

I don't see why that should not be working with the current edge snapshot, as @JacksonInject is copied to the builder's setter method. I think it's best to create a question at StackOverflow with a minimal example; I'll have a look then.

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

No branches or pull requests

5 participants