CodecsAutoConfiguration may overwrite existing customizations of the default codes

[rstoyanchev]

Currently CodecsAutoConfiguration replaces Jackson codecs even though all it means to do is set the ObjectMapper but until now that's all the underlying API allowed. In Spring Framework 5.3.4 it will be possible to set the ObjectMapper without replacing the Jackson codec instances and CodecsAutoConfiguration should take advantage of that.

This will help with the changes for spring-projects/spring-framework#26212 after which Spring HATEOAS will be able to register additional ObjectMapper instances on the same default Jackson codecs for its own RepresentationalModel class without interfering with Boot trying to set the default ObjectMapper.

[wilkinsona]

I've taken a look at implementing this and I don't understand the benefit for a couple of reasons.

With the change in Framework 5.3.4 it's possible to build on Boot's configuration of the encoder and decoder without any changes in Boot. Boot sets the Jackson 2 JSON encoder and decoder in a CodecCustomizer with order 0. Anything that wants to fine tune this setup can do so in a customiser with lower precedence. If we change things so that Boot customises rather than replaces, it would technically be a breaking change. That's hard to justify when the desired end result already appears to be possible.

The javadoc for registerObjectMappersForType states that it "effectively turns off use of the default ObjectMapper" so Boot's auto-configured ObjectMapper will be lost. I guess you could get the default ObjectMapper and register it again for the mime types returned by getDecodableMimeTypes() or getEncodableMimeTypes() but that feels rather clunky and it feels like there's plenty of scope for getting it wrong. Looking at the changes made in Spring HATEOAS, I can't see any sign of the existing mime types and object mapper being preserved. This API is new to me, though, and I've only looked at that commit on GitHub, so I may have missed something.

[rstoyanchev]

Thanks for taking a look.

First one clarification. The registerObjectMappersForType indeed turns off the default ObjectMapper but only for the registered class. For all other types the default ObjectMapper is still in effect. For example Spring HATEOAS can take charge of the handling of RepresentationModel and that should have no impact for any other class.

I see your point that Spring HATEOAS can register its own CodecCustomizer at a lower priority. If I recall correctly from discussing with @odrotbohm, the challenge was that the customization is done with a WebMvcConfigurer, i.e. without a dependency on Boot. I'm not entirely sure which one would go first in that case.

I thought of this as a relatively straight-forward change when I suggested it but maybe I'm not seeing something. Boot currently relies on the Jackson2CodecSupport constructor, passing an ObjectMapper and empty media types. Instead it could just set the ObjectMapper on the instance created internally with the default constructor. Isn't this reasonably safe?

[odrotbohm]

The problem with our previous approach (a custom converter / codec ordered before the default ones) was that the default one would end up opting in rendering RepresentationModel for (hyper)media types which Spring HATEOAS is not actually set up for, effectively producing invalid responses. Take an application enabling HATEOAS with HAL but a client asking for HAL FORMS. The default HMC/ codec would happily render that but produce something that is not HAL FORMS. The API Rossen introduced allow us to avoid the dedicated converters / codecs but rather tweak the default ones to use a dedicated ObjectMapper equipped with the necessary extensions when a particular (super)type is requested. It also allows to return an appropriate status code if the media type / type combination is not actually supported. See spring-projects/spring-framework#26212 for details.

Thus, the HATEOAS components registering the support for the hypermedia types still consume a potentially Boot configured ObjectMapper as they did before. Also, those customized mappers are only used if HATEOAS specific types are used as top-level types. That said, the configuration assumes the HMC / codec instance already being prepared by Boot at the time of our configuration or at least not being replaced afterwards.

I might be missing a part of the story but isn't what's suggested to exchange the ObjectMapper instance on the default JSON codec rather than replacing the entire codec at once?

Anything that wants to fine tune this setup can do so in a customiser with lower precedence.

Anything downstream of Spring Boot that is, yes.

I see your point that Spring HATEOAS can register its own CodecCustomizer at a lower priority.

Spring HATEOAS unfortunately can't as the interface is part of Boot, not Spring Framework.

[wilkinsona]

First one clarification. The registerObjectMappersForType indeed turns off the default ObjectMapper but only for the registered class

Thanks, Rossen. That goes a long way to addressing my concerns. I've just re-read the javadoc and managed not to skip over "for the given class" this time. Sorry for the noise there.

the customization is done with a WebMvcConfigurer, i.e. without a dependency on Boot. I'm not entirely sure which one would go first in that case

I think the order is undefined at the moment. Boot's WebFluxConfigurer that applies its codec customization is unordered and, I believe, HATEOAS's is too. I think that's a bug in Boot as the equivalent WebMvcConfigurer is @Order(0) to allow other configurers to go before or after it. I've opened #25302.

I might be missing a part of the story but isn't what's suggested to exchange the ObjectMapper instance on the default JSON codec rather than replacing the entire codec at once?

Yes. The problem is that, theoretically at least, there could be someone relying upon the complete replacement. I do think it's quite unlikely, and could probably be worked around with some changes to their app, but, as with #25302 that I've just opened, we need to be a little bit wary of breaking something in a maintenance release.

If we fix #25302, we may not need to do anything here as Boot's WebFluxConfigurer, and therefore its CodecCustomizers, will be applied before HATEOAS's WebFluxConfigurer. This would rely on HATEOAS's WebFluxConfigurer being ordered with precedence lower than zero. That feels OK to me as, presumably, its ordering is going to have to be documented anyway so that non-Boot users know how to order their own WebFluxConfigurer with respect to HATEOAS's.

[odrotbohm]

I do think it's quite unlikely, and could probably be worked around with some changes to their app, but, as with #25302 that I've just opened, we need to be a little bit wary of breaking something in a maintenance release.

We've only rolled those changes into the upcoming 1.3 of Spring HATEOAS, i.e. I think it'd be fine to only tweak this for Boot 2.5.

[wilkinsona]

I've taken another look at this and have been trying to write a test that checks that existing customizations aren't overwritten. I've failed to do so because configureDefaultCodec chains the consumers together and applies them all in initCodec. I've got a CodecCustomizer that calls configureDefaultCodec with a Consumer that, when called, registers an ObjectMapper for a custom type. This customizer goes first. After this, the customizer defined in CodecsAutoConfiguration is called and it replaces the Jackson encoder and decoder. However, this replacement does not cause the registration of the custom type ObjectMapper to be lost. It isn't lost because the Consumer that configures it is called again when the Jackson encoder and decoder are replaced.

Unfortunately, that leaves me back to where I was earlier not understanding the need for this change. What have I missed this time please, @rstoyanchev? FWIW, I'm also struggling to understand the motivation for the codec customization being as complex as it is. I suspect there are some use cases of which I am unaware, but it does seem quite complicated even for something that felt like it should be reasonably simple.

[rstoyanchev]

Apologies for the slow reply.

For how to test, I imagine something like this:

List<HttpMessageReader<?>> readers = codecConfigurer.getReaders();
List<HttpMessageWriter<?>> writers = codecConfigurer.getWriters();

// Inspect codecs before...

codecConfigurer.defaultCodecs().configureDefaultCodec(codec -> {
	// customize...
});

readers = codecConfigurer.getReaders();
writers = codecConfigurer.getWriters();

// Inspect codecs after...

For the second question on why replace the Jackson encoder and decoder instances vs just set the ObjectMapper, indeed there is very little one could customize directly on those codecs. Well, that and it could be a sub-class I suppose. That said, this is the situation for HATEOAS, which needs to customize the Jackson codecs through CodecConfigurer, i.e. without depending on Boot.

I suppose if it becomes possible to order WebMvcConfigurer then HATEOAS would have to after Boot in order to customize the Jackson codec instances set by Boot? It just seemed to me like Boot customizing only the ObjectMapper was less invasive and yet sufficient, and therefore preferable.

[wilkinsona]

Thanks, Rossen. I don't think I did a very good job of explaining the problem. The crux of it is that it doesn't seem to matter what Boot does as the end result is always the same.

Here are four tests that hopefully illustrate this:

import java.util.List;
import java.util.stream.Collectors;

import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.jupiter.api.Test;

import org.springframework.http.codec.CodecConfigurer;
import org.springframework.http.codec.DecoderHttpMessageReader;
import org.springframework.http.codec.EncoderHttpMessageWriter;
import org.springframework.http.codec.json.Jackson2CodecSupport;
import org.springframework.http.codec.json.Jackson2JsonDecoder;
import org.springframework.http.codec.json.Jackson2JsonEncoder;
import org.springframework.http.codec.support.DefaultServerCodecConfigurer;
import org.springframework.util.MimeType;

import static org.assertj.core.api.Assertions.assertThat;

class CodecCustomizationTests {

	private final ObjectMapper bootObjectMapper = new ObjectMapper();

	private final ObjectMapper hateoasObjectMapper = new ObjectMapper();

	private final CodecConfigurer codecConfigurer = new DefaultServerCodecConfigurer();

	@Test
	void hateoasCustomizesThenBootReplaces() {
		hateoasCustomization(this.codecConfigurer);
		bootReplacement(this.codecConfigurer);
		verifyJacksonCodecs(this.codecConfigurer);
	}

	@Test
	void bootReplacesThenHateoasCustomizes() {
		bootReplacement(this.codecConfigurer);
		hateoasCustomization(this.codecConfigurer);
		verifyJacksonCodecs(this.codecConfigurer);
	}

	@Test
	void hateoasCustomizesThenBootCustomizes() {
		hateoasCustomization(this.codecConfigurer);
		bootCustomization(this.codecConfigurer);
		verifyJacksonCodecs(this.codecConfigurer);
	}

	@Test
	void bootCustomizesThenHateoasCustomizes() {
		bootCustomization(this.codecConfigurer);
		hateoasCustomization(this.codecConfigurer);
		verifyJacksonCodecs(this.codecConfigurer);
	}

	private void bootReplacement(CodecConfigurer codecConfigurer) {
		codecConfigurer.defaultCodecs().jackson2JsonDecoder(new Jackson2JsonDecoder(this.bootObjectMapper));
		codecConfigurer.defaultCodecs().jackson2JsonEncoder(new Jackson2JsonEncoder(this.bootObjectMapper));
	}

	private void bootCustomization(CodecConfigurer codecConfigurer) {
		codecConfigurer.defaultCodecs().configureDefaultCodec((codec) -> {
			if (codec instanceof Jackson2JsonDecoder || codec instanceof Jackson2JsonEncoder) {
				((Jackson2CodecSupport) codec).setObjectMapper(this.bootObjectMapper);
			}
		});
	}

	private void hateoasCustomization(CodecConfigurer codecConfigurer) {
		codecConfigurer.defaultCodecs().configureDefaultCodec((codec) -> {
			if (codec instanceof Jackson2JsonDecoder || codec instanceof Jackson2JsonEncoder) {
				((Jackson2CodecSupport) codec).registerObjectMappersForType(CustomType.class, (registrar) -> {
					registrar.put(MimeType.valueOf("application/custom+json"), this.hateoasObjectMapper);
				});
			}
		});
	}

	private void verifyJacksonCodecs(CodecConfigurer codecConfigurer) {
		List<Jackson2CodecSupport> readerDecoders = codecConfigurer.getReaders().stream()
				.filter(DecoderHttpMessageReader.class::isInstance).map(DecoderHttpMessageReader.class::cast)
				.map(DecoderHttpMessageReader::getDecoder).filter(Jackson2JsonDecoder.class::isInstance)
				.map(Jackson2CodecSupport.class::cast).collect(Collectors.toList());
		verifyJacksonCodecs(readerDecoders);
		List<Jackson2CodecSupport> writerEncoders = codecConfigurer.getWriters().stream()
				.filter(EncoderHttpMessageWriter.class::isInstance).map(EncoderHttpMessageWriter.class::cast)
				.map(EncoderHttpMessageWriter::getEncoder).filter(Jackson2JsonEncoder.class::isInstance)
				.map(Jackson2CodecSupport.class::cast).collect(Collectors.toList());
		verifyJacksonCodecs(writerEncoders);
	}

	private void verifyJacksonCodecs(List<Jackson2CodecSupport> jacksonCodecs) {
		assertThat(jacksonCodecs).isNotEmpty();
		assertThat(jacksonCodecs).allSatisfy((codec) -> {
			assertThat(codec.getObjectMappersForType(CustomType.class).get(MimeType.valueOf("application/custom+json")))
					.isEqualTo(this.hateoasObjectMapper);
			assertThat(codec.getObjectMapper()).isEqualTo(this.bootObjectMapper);
		});
	}

	static class CustomType {

	}

}

As far as I can tell, it makes no difference if Boot goes before or after HATEOAS or if it customises the existing codec's ObjectMapper or configures an entirely new ObjectMapper. The end result is a Jackson codec with an ObjectMapper configured with HATEOAS's type-specific ObjectMapper.

As I said above, I may well have missed something as I'm struggling to understand the need for the various different ways of doings things. Perhaps there's a subtlety here that the tests above don't cover.

[rstoyanchev]

I see now what you meant.

On closer look, indeed the intention is that the order in which a default codec might be replaced and when some other default codec setting might change, doesn't matter. The configureDefaultCodec callback is no different. It shouldn't matter if it is set before or after a default codec is replaced.

In other words, you're right that as long as HATEOAS uses this callback it is guaranteed to be applied. That makes this a non-issue at least as defined, in the context of HATEOAS. Given this, Boot can still choose to use the callback and only update the ObjectMapper in order to allow someone else to replace the actual codec instance but that's a slightly different goal than what I wrote in the original description.

[wilkinsona]

Boot can still choose to use the callback and only update the ObjectMapper in order to allow someone else to replace the actual codec instance

As things stand, I'm not sure that it's worth making this change. As demonstrated above, even if Boot only updates the ObjectMapper, if someone else replaces the codec instance Boot's customization with then be applied anyway. If an unforeseen problem is identified that would be solved by making the change we can reconsider, but until then I think it's best if we leave things as they are.