SuperBuilder have attibute to specify access modifier for builderImpl class

[kaweston]

As per discussions here https://stackoverflow.com/questions/52734182/lombok-superbuilder-example-with-json-annotations involving Jan Rieke, it is acknowledged that there is a compatibility issue with Jackson, which by inference means requiring the explicit declaration of the builderImpl class that would otherwise be generated so that the access modifier can be made package private allowing Jackson to create a builderImpl instance

It would be nice to have the ability to specify the access modifier in the SuperBuilder annotation using the AccessLevel class.

I understand it could be argued that Jackson should support the ability to specify a method in its JsonDeserialize annotation to create a builder instance and I will likely raise a feature request to that effect also. I certainly understand if that is also your preferred option.

[janrieke]

Just a few further thoughts:
A parameter builderMethod on @JsonDeserialize would be helpful to avoid the unnecessary broadening of visibility. However, we still need @JsonPOJOBuilder on the builder implementation class because of withPrefix.
We could also add an onBuilderImpl annotation parameter for @SuperBuilder so that users don't have to copy the rather complex builder class definition from delombok.

[kaweston]

I agree with your first comment, that would be the preferred method and I will raise an enhancement request to that effect today.

Edit: FasterXML/jackson-databind#2354

Could you please elaborate on what the onBuilderImpl annotation parameter would do?

With regards to the @JsonPOJOBuilder issue, on that page I referenced Andreas gives a solution which I've been using successfully in conjunction with both @builder and @SuperBuilder annotations.

    final ObjectMapper mapper = new ObjectMapper();
    mapper.setAnnotationIntrospector(new JacksonAnnotationIntrospector() {

        @Override
        public JsonPOJOBuilder.Value findPOJOBuilderConfig(final AnnotatedClass ac) {
            if (ac.hasAnnotation(JsonPOJOBuilder.class)) {
                return super.findPOJOBuilderConfig(ac);
            }
            return new JsonPOJOBuilder.Value("build", "");
        }
    });

    return mapper;
}

[janrieke]

FYI: "Set" methods for @SuperBuilder can be prefixed with the next version (PR #2357), making @JsonPOJOBuilder (or setting a custom JacksonAnnotationIntrospector) unnecessary if you specify the prefix in @SuperBuilder.
However, you still need to broaden the visibility. Making the builder implementation class package-private by default simply feels wrong, and another annotation parameter also. So I bumped the issue FasterXML/jackson-databind#2354 again, asking if they'd accept a PR.

[janrieke]

Hmm, now I'm a bit confused. I just checked it again and it also works with the builder implementation class private. I simply cannot remember why I thought a few month ago that making it package-private is necessary. I simply may have made a mistake back then.
So from my POV, this issue can be closed.

[janrieke]

I found the problem: Eclipse's compiler does not complain about private access, but javac does. javac gives MyDto.MyDtoBuilderImpl has private access in MyDto on @JsonDeserialize. Looks like a bug in ecj to me.

Although the Jackson people seem quite positive about adding support for specifying a static factory method, lets keep this issue open until it is really implemented.

[kaweston]

@janrieke Thanks for the updates. With regards to your comment about PR #2357 its really nice to see such flexibility implemented for those who need it. For me though I'm more in agreement with @rzwitserloot and his comments at #2174 (comment) so I'll likely stick with using the JacksonAnnotationIntrospector.

I've read the comments on the Jackson issue I raised and I'm hoping they will allow the specifying of a static builder method. I'm not a fan of subverting the protection system without compelling reasons, and I don't see a valid reason when a builder method is available for use.

[rzwitserloot]

@janrieke 's the effective maintainer of the SuperBuilder feature at this point. Not because we don't care, on the contrary - because he's sending the pull requests before we even realize there's a problem :) – so I'll leave this in your capable hands and if you need anything from us, let us know.

One thought does come up: If there's this whole song and dance routine you have to go through to make (super)builder act precisely in the right way so that it works best with Jackson, maybe we should just introduce a single annotation that means: "Just make it all so that jackson is as happy as can be". I prefer to wait for the jackson feature request/PR IF that would mean you can just put @(Super)Builder on a class and jackson will just work, no further configuration needed, but if it's a mix of telling (S)B to make the class package private as well as introduce a 'with' prefix as well introduce some annotations on various generated nodes, I'm in favour of @Jacksonify @SuperBuilder or @SuperBuilder(forJackson = true) or something in that vein.

[janrieke]

A dedicated Jackson switch could be a solution. However, what I do not like is to use "with" as setterPrefix in this case just to make Jackson happy (because I do not want to further endorse this ugly default in Jackson). But that would mean generating Jackson annotations in Lombok.
That seems to be a dangerous area to me. I'll think about it.

[rzwitserloot]

@janrieke lombok features targeted specifically at third party libraries (as in, if you aren't using that library, the feature makes no sense and will probably error out due to missing dependencies) is fine, that's what the lombok.extern package is used for. It's a little bizarre that this isn't so much a dedicated feature for jackson; it's a modification of an existing feature for jackson. Hence why I'd prefer the otherwise somewhat unwieldy @Jacksonify @SuperBuilder, or possibly @JacksonSuperBuilder.

But, yes, having to cater to this crazy with standard hurts.

[janrieke]

I opened a thread on Google Groups to continue the discussion: https://groups.google.com/forum/#!topic/project-lombok/L7OVH_noUAY

[kennethjor]

For reference, I have opened a feature request with Jackson at https://github.com/FasterXML/jackson-future-ideas/issues/47.