ElasticsearchClientAutoConfiguration causes global ObjectMapper to be overwritten

[manofthepeace]

Tested with springboot 3.0.0

When the new ElasticsearchClient is in the classpath, it triggers the ElasticsearchClientAutoConfiguration which then overrides the application objectmapper / the one configured via Jackson2ObjectMapperBuilder.

Reproducer for this issue; https://github.com/manofthepeace/spring3-elasticClient-mapperissue

Steps to reproduce;

1. Run the test via mvn test the test will pass
2. modify the pom.xml and uncomment the elasticsearch-java dependency
3. run the test via mvn test the test will fail
4. in TestingWebApplication.java use @SpringBootApplication(exclude = ElasticsearchClientAutoConfiguration.class)
5. run the test via mvn test the test will pass

Expected behaviour;
the RestClientTransport should use a new ObjectMapper, or somehow a user provided one, but not the one that spring-mvc / the global one is using. Can be done with new JacksonJsonpMapper() or by passing an objectMapper to the JacksonJsonpMapper constructor.

[mhalbritter]

You specify the version of elasticsearch-java in your pom.xml:

        <dependency>
            <groupId>co.elastic.clients</groupId>
            <artifactId>elasticsearch-java</artifactId>
            <version>7.17.7</version>
        </dependency>

When removing the version and letting Spring Boot take care of the version, the problem goes away.

[mhalbritter]

it looks like that your version takes the passed ObjectMapper in org.springframework.boot.autoconfigure.elasticsearch.ElasticsearchClientConfigurations.JacksonJsonpMapperConfiguration#jacksonJsonpMapper and reconfigures it. The version boot uses (8.5.1 at the time of writing) doesn't do that anymore.

[manofthepeace]

@mhalbritter thanks you for your time, and your answer.

I find that really unfortunate that spring tries to do some auto configuration when the user specifies his own libraries (not a starter). The elasticsearch client is not backward compatible, like at all, so using the one provided with springboot is not an option for us as we need to support a large array of elastic server version (not that large, but 5.X, 6.X and 7.X), and we cannot just lag behind on spring versions because of elastic server (out of our control).

I would really appreciate if the autoconfiguration runs if one of the class of the starter is present, not the one from elasticsearch. I must say it was quite a painful debugging trying to understand why our rest api stopped providing the right responses, and now all our microservice that uses ES will need that specific exclude, which I find a bit dirty.

I understand that adding a starter would modify the behaviour in my app, but providing libs out of spring, I think they should be left untouched, for me the really seem like an issue.

I also think it is risky to let elasticsearch library do anything with the global object mapper, maybe it does do anything to it in 8.5.1, but what tell they wont in the future. I still think it needs its own, a user might want to parse differently the Es reponses than what is done via rest.

Again, thank you for your time.

[mhalbritter]

You can still use the old client if you really want to. The auto-configuration is built in a way that it backs off if you provide your own JsonpMapper, which can be done like this:

@Configuration(proxyBeanMethods = false)
class JacksonJsonpMapperConfiguration {
	@Bean
	JacksonJsonpMapper jacksonJsonpMapper() {
		return new JacksonJsonpMapper();
	}
}

Does this work for you?

And yes, i agree that ElasticSearch modifying the passed ObjectMapper is not good. I'll flag it for discussion to see what the rest of the team thinks.

[mhalbritter]

So we talked about that today, and we decided that all options we have are bad:

Either we change the default from not passing in the global ObjectMapper, which breaks everyone who's relying on the current behaviour.

Or we let it be and live with that the global object mapper is used for WebMVC and ElasticSearch and whatever else and if you reconfigure it, you reconfigure it in all places.

That the elasticsearch client reconfigures the given ObjectMapper is a bug which they fixed.

I'll create a ticket to maybe flip the default in Spring Boot 3.1: not using the global ObjectMapper for ElasticSearch but using the no-args constructor of JacksonJsonpMapper.

[manofthepeace]

I agree with all the above. I do also think it is/was a bug in the elasticsearch client, which sadly wasn't backported.

I tried your suggestion, while it does work, it still go ahead and created a foobar ElasticsearchClient bean. I am saying foobar, as no RestClient config are in the project/properties, so that client bean just wont ever work, which I think is another somewhat odd behaviour, as it takes up some resources.

I think the best in my case is to disable the bean via AutoConfigurationImportSelector, so all microservices can have that exclusion and let me deal with it manually, and this way I do not have the extra objectmapper, that also does that its fair share of resources + the ElasticsearchClient bean.

In my app, I have a wrapper bean around that client, from that wrapper bean, I can select, for now, the highlevelclient and the new client, so we can migrate smoothly and slowly, and since it is used in non spring component in some places, it's easier not not have to pass both all over the place, but just the wrapper. Hence why the autoconfig goes ahead and creates the bean.. (still think its odd to create a bean when the underneath restclient hasn't been configured at all)

Thanks for the help, and your time.

[mhalbritter]

If I understand this correctly, you want to use the elasticsearch client on your own and not the spring (boot) provided stuff around that. In that case the correct solution is to exclude the auto-configuration so that it doesn't run at all. You will then not have any beans you don't need in your context.

[manofthepeace]

This is exact. It is easier to maintain having multiple branches with different es client version to support multiple es server versions, but all those branches benefits having the latest and greatest boot version and what it brings.

[wilkinsona]

A disadvantage of Elasticsearch no longer changing the configuration of the passed-in ObjectMapper is that setting spring.jackson.serialization.indent-output to true breaks bulk operations as the source for each document has to be on a single line.

[wilkinsona]

A workaround is to conditionally create a copy of the ObjectMapper and modify it to make it compatible with Elastic:

@Bean
JacksonJsonpMapper jacksonJsonpMapper(ObjectMapper objectMapper) {
	return new JacksonJsonpMapper(makeElasticCompatible(objectMapper));
}

private ObjectMapper makeElasticCompatible(ObjectMapper objectMapper) {
	if (!objectMapper.isEnabled(SerializationFeature.INDENT_OUTPUT)) {
		return objectMapper;
	}
	return objectMapper.copy().disable(SerializationFeature.INDENT_OUTPUT);
}

I wonder if we should do this in Boot or perhaps suggest it to the Elastic team?

[philwebb]

We think we should implement this in our auto-configuration. If a user wants a custom mapper they can override the co.elastic.clients.json.jackson.JacksonJsonpMapper bean (possibly using any co.elastic.clients.json.JsonpMapper implementation).

[steklopod]

Version 3.0.3 broke my working code 😢

This bean did not fix problem:

@Configuration(proxyBeanMethods = false)
class ElasticJsonConfig{
    @Bean
    fun jacksonJsonpMapper() = JacksonJsonpMapper(
        jacksonObjectMapper().findAndRegisterModules()
            .disable(FAIL_ON_UNKNOWN_PROPERTIES)
    )
}

Rollback to spring boot 3.0.2 works perfect.

---

I fixed it by updating elastic-search config file in .../src/main/resources/settings/settings.json. I removed this row (it is unknown property):

{
        "autocomplete_search": {"tokenizer": "lowercase"}
}

[wilkinsona]

@steklopod Sorry to hear about the breakage. It should be possible to restore the previous behavior using a bean like this:

@Bean
JacksonJsonpMapper jacksonJsonpMapper(ObjectMapper objectMapper) {
    return new JacksonJsonpMapper(objectMapper);
}

Elastic will then use the auto-configured ObjectMapper again. Note that this will cause problems if you use output indenting.

If the above doesn't work, please open a new issue with a minimal sample that reproduces the problem.

[steklopod]

Thank you @wilkinsona for your feedback.

Honestly, this is not a big issue for me. Just wanted to share info with you. The interesting thing is that is only fails in github-actions ci. Not on local mac.

It seems like in version 3.0.3 there are some new validation rules for elastic config settings.json. I mean this file: @Setting(settingPath = "/settings/settings.json"). If there is an unknown property in linux env - it fails an app.

[wilkinsona]

We don't do anything with that file in Spring Boot. It's quite possible that something's changed in this area in Elastic client code though.