Jackson and (Super)Builder

[Jan Rieke]

Many lombok users also use Jackson to (de)serialize JSON. One particular issue is that people want their DTOs to be immutable, requiring an @AllArgsConstructor with @ConstructorProperties. As that annotation may not be available in Java 9+, using a builder is a good alternative (also because you do not need a public @AllArgsConstructor with all that parameters).


That is possible, but you have to either use "with" as setter prefix in the builder (ugly), or customize the generated builder with @JsonPOJOBuilder(withPrefix="") (manual work, especially nasty for @SuperBuilder because of the complexity of the generated code). Furthermore, you have to advise Jackson to use the (generated) builder class via @JsonDeserialize(builder=MyDto.MyDtoBuilder.class). In case of @SuperBuilder, this is even more difficult, because you need to put the BuilderImpl class in there, which is private by default and thus not accessible in the @JsonDeserialize annotation.

As discussed here on GitHub, lombok could add a new annotation that simplifies this use-case. I'll call it @Jacksonized for now. One way to go could be to cater to the Jackson needs if @Jacksonized is present (like using "with" as prefix then). I don't like that.

Instead I propose the following approach whenever @Jacksonized  is present at a place where there is also a @(Super)Builder annotation:

1. Insert (or modify if already present) @JsonDeserialize(builder=MyDto.MyDtoBuilder[Impl].class) on the class.

2. Copy Jackson-related configuration annotations (like @JsonIgnoreProperties) to the builder class. According to the Jackson docs, this is necessary.

3. Insert @JsonPOJOBuilder(withPrefix="") to avoid the ugly "with" prefix in the builder.

4. For @SuperBuilder, make the builder implemention class package-private.

What do you think?

[Mat Jaggard]

As a constant user of both Lombok and Jackson and an immutability proponent, I like this idea. One item I have an issue with is point 1 of the described behaviour - Lombok should insert things if they're not present but never modify if they are in my opinion. I know this is slightly limiting but with the aim of "Least Surprising" this is important.

Currently we have an @NoArgsConstructor(force=true) which is definitely non-ideal and also doesn't work by default in the later versions of Jackson because it doesn't modify private final fields anymore.

+1 from me.

Mat.

--
You received this message because you are subscribed to the Google Groups "Project Lombok" group.
To unsubscribe from this group and stop receiving emails from it, send an email to project-lombo...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/project-lombok/3d52dc61-42e2-4f40-a328-d48dd902d701%40googlegroups.com.

[Mat Jaggard]

Apologies, I should have read the thread first. Is there any Issue for Jackson to remove the "with" prefix requirement?

[Reinier Zwitserloot]

Jan is on the case and I believe the jackson team is on board, but the annotation to explicitly set the with prefix to the empty string won't hurt even if they do update this, and we still have the aspect of 'copy over various annotations' and 'add the @JsonDeserialize' annotation (which in passing also needs for the builder impl to become package private), so even if jackson updates to alleviate that need, this annotation still makes sense.

Do we go with @Jacksonized @Builder and @Jacksonized @SuperBuilder, or do we go with @JacksonizedBuilder and @JacksonizedSuperBuilder?

To unsubscribe from this group and stop receiving emails from it, send an email to project-lombok+unsubscribe@googlegroups.com.

[Jan Rieke]

One item I have an issue with is point 1 of the described behaviour - Lombok should insert things if they're not present but never modify if they are in my opinion. I know this is slightly limiting but with the aim of "Least Surprising" this is important.

 But wouldn't it be equally surprising if the specially crafted Jackson builder just does not work? Just because the user forgot to add the automatically generated builder to the existing @JsonDeserialize.

Is there any Issue for Jackson to remove the "with" prefix requirement?

No, there's just an issue on GitHub to allow specifying a builder method (instead of a builder class), which would allow us to keep the SuperBuilder impl class private.

I doubt that the Jackson people will remove that "with" default, because it would be a large breaking change affecting almost everyone using builders with Jackson. Even if they'd do it, it will take years until the new version arrives in actual projects: Many Jackson users get their Jackson dependency from Spring, and upgrading the Spring version is typically not done very often in real life. In contrast, upgrading lombok is easy to do on a project-by-project basis. So I think we need that feature anyway. And as Reinier said: It doesn't hurt generating it even if they'd change the default.

Do we go with @Jacksonized @Builder and @Jacksonized @SuperBuilder, or do we go with @JacksonizedBuilder and @JacksonizedSuperBuilder?

I prefer @Jacksonized @[Super]Builder, because

a) we do not have to make nearly identical copies of two annotations (AFAIK annotations cannot extend others, right?), and there are no annotation params we want to remove,

b) indicate that the generated (super)builders are still compatible with non-Jacksonized builders, and

c) may later use @Jacksonized for other features (e.g. emit a warning when there is only an @AllArgsConstructor on a DTO, a frequent problem users have after upgrading lombok).

But that's up to you. I don't have strong feelings on that topic. :)

[Reinier Zwitserloot]

I'm convinced on all points.

[Jan Rieke]

The implementation of the javac part is already working quite well. Before I proceed just a quick design question, so I do not follow a direction you do not like, Reinier.

As both HandleBuilder and HandleSuperBuilder are already really complex, I wanted the implementation to be separate from them. So I created a JacksonizedHandler that does all the work (it runs after the builder handlers, so all the builder code is already there). That handler needs information from the @(Super)Builder annotations for generating the @JsonPOJOBuilder anntation (the setter prefix and the build method name), but the builder handlers already remove the annotations.

Solution 1: Don't remove the @(Super)Builder annotation in the builder handlers, but create a late removal handler (like with HandleBuilderDefaultRemove). That's how I implemented it now, but I'm unsure of the performance impact of this approach.

Solution 2: Guess the information from the generated code.

Is it ok for you to proceed with solution 1?

[Jan Rieke]

On Monday, February 24, 2020 at 10:59:57 AM UTC+1, Jan Rieke wrote:

Solution 1: Don't remove the @(Super)Builder annotation in the builder handlers, but create a late removal handler (like with HandleBuilderDefaultRemove). That's how I implemented it now, but I'm unsure of the performance impact of this approach.

BTW: This also breaks the test cases BuilderDefaultsWarnings, BuilderSingularNoAuto, BuilderSingularNoAutoWithSetterPrefix, but just because the @Builder annotation is not removed anymore. I think that's because HandleBuilder emits an error and subsequent handlers for the same annotation are not called any more. Is that a problem?

[Reinier Zwitserloot]

Nope, sounds good, carry on.

[Jan Rieke]

I created a PR: https://github.com/rzwitserloot/lombok/pull/2387