Support adding rather than replacing modules in Jackson2ObjectMapperBuilder

[AQS-DTheuke]

spring-framework/spring-web/src/main/java/org/springframework/http/converter/json/Jackson2ObjectMapperBuilder.java

Lines 570 to 574 in 67b1420

	public Jackson2ObjectMapperBuilder modulesToInstall(Module... modules) {
	this.modules = Arrays.asList(modules);
	this.findWellKnownModules = true;
	return this;
	}

Currently the Jackson2ObjectMapperBuilderCustomizer is only able to replace the modules that should be installed; however, it is possible to define multiple Spring Boot Jackson2ObjectMapperBuilderCustomizer beans that could each wish to add a Jackson module. However the later customizers will always remove the modules configured by the previous instance. It isn't possible to get the current list of modules either and extend that.

We wish to separate our customizers by their related modules/source libraries (api-docs, error-handling, ..., and the application itself).

So it would be nice if there was a addModules(toInstall) method that would add the modules to the list instead of replacing it.

Most of the other methods such as serializers(JsonSerializer<?>...) already work like that:

spring-framework/spring-web/src/main/java/org/springframework/http/converter/json/Jackson2ObjectMapperBuilder.java

Lines 354 to 363 in 67b1420

	public Jackson2ObjectMapperBuilder serializers(JsonSerializer<?>... serializers) {
	for (JsonSerializer<?> serializer : serializers) {
	Class<?> handledType = serializer.handledType();
	if (handledType == null || handledType == Object.class) {
	throw new IllegalArgumentException("Unknown handled type in " + serializer.getClass().getName());
	}
	this.serializers.put(serializer.handledType(), serializer);
	}
	return this;
	}

Thanks for your awesome work.

[aooohan]

@sbrannen hi,Sam. Does this issue need to be achieved? I personally think it would be more flexible to provide a method to add a Module separately, if it need to be implemented, can I be assigned? I would be happy to provide PR.

[sbrannen]

@aooohan, we will discuss within the team whether we wish to add such a method.

Please note that the waiting-for-triage label signals that the team has not yet reached a decision on the issue.

[aooohan]

@aooohan, we will discuss within the team whether we wish to add such a method.

Please note that the waiting-for-triage label signals that the team has not yet reached a decision on the issue.

Alright, thank you for your reply.

[rstoyanchev]

We could maybe add a Consumer based overloaded variant modulesToInstall(Consumer<List<Module>> modules) that lets you customize the configured set rather than set the modules.

[AQS-DTheuke]

Please keep it simple. We are already in a BuilderCustomizer.

[rstoyanchev]

Note that the builder currently has two methods for modules, modules(List<Module> modules) and modulesToInstall(Module... modules), which have different semantics and are mutually exclusive. It's a bit more challenging to come up with something that is clearly aligned with one or the other. The proposed addModules sounds like it is aligned with modules but your example was for modulesToInstall so this is already ambiguous.

An overloaded method with a Consumer allows for a clear alignment with one vs the other method, and has the benefit of provoding full control over the list of modules, including where to insert, add, remove, etc.

[AQS-DTheuke]

Please note that you always need two methods (unless you refactor the whole thing to avoid the findWellKnownModules in these methods, which might be a good idea).
modules(...) is exclusive. modulesToInstall(...) is inclusive.
And I am referring to both for consistency reasons, IMO its just somewhat redundant to always explicitly mention both.

Semantically, I wish to just add a module to the builder, so I would appreciate it, if there is a method for exactly that (similar to all the other existing methods). I explicitly added the two code examples in the issue description to show the difference in their implementation. Most of the other methods look like the serializer one (are adders not setters).
IMO in the next major version of spring, this should be addressed further by renaming modules() to replaceModules(...) or setModules(...) and renaming addModules(...) to modules(...) so they behave the same as the other methods and don't disrupt developer expectations.

This is the workaround I currently use:

@Bean
// Custom error details
public Jackson2ObjectMapperBuilderCustomizer apiErrorCustomizer() {
    // We cannot use the module directly
    // because the module methods are only setters and not adders
    // builder#serializers adds the passed serializers instead
    return builder -> builder.serializers(ApiErrorModule.requiredSerializers())
                             .deserializers(ApiErrorModule.requiredDeserializers());
}

// --------------------- in another configuration class/artifact

@Bean
// "2000" -> "2000-01-01" <= x < "2001-01-01"
public Jackson2ObjectMapperBuilderCustomizer dateishRangeCustomizer() {
    return builder -> builder.serializers(DatishRangeModule.requiredSerializers())
                             .deserializers(DatishRangeModule.requiredDeserializers());
}

[rstoyanchev]

We could consider making modules and modulesToInstall additive in 6.0 to align them with how other methods in the builder work which accept multiple registrations.

[rstoyanchev]

Team Decision: we've decided to go with the Consumer based variants, which cover a wider range of cases without the potential for side effects from changing how the methods behave currently. This does mean an additional nested lambda, but still a reasonable compromise overall:

@Bean
public Jackson2ObjectMapperBuilderCustomizer customizer() {
    return builder -> builder.modules(list -> list.add(...));
}

As a result, the methods can be added immediately in the 5.3.x branch.