Skip to content

Fix store null values implementation. #2841

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@
import org.springframework.data.elasticsearch.core.convert.ElasticsearchConverter;
import org.springframework.util.Assert;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializationFeature;

/**
* Base class for a @{@link org.springframework.context.annotation.Configuration} class to set up the Elasticsearch
* connection using the Elasticsearch Client. This class exposes different parts of the setup as Spring beans. Deriving
Expand Down Expand Up @@ -118,7 +122,13 @@ public ElasticsearchOperations elasticsearchOperations(ElasticsearchConverter el
*/
@Bean
public JsonpMapper jsonpMapper() {
return new JacksonJsonpMapper();
// we need to create our own objectMapper that keeps null values in order to provide the storeNullValue
// functionality. The one Elasticsearch would provide removes the nulls. We remove unwanted nulls before they get
// into this mapper, so we can safely keep them here.
var objectMapper = (new ObjectMapper())
.configure(SerializationFeature.INDENT_OUTPUT, false)
.setSerializationInclusion(JsonInclude.Include.ALWAYS);
return new JacksonJsonpMapper(objectMapper);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@

import static com.github.tomakehurst.wiremock.client.WireMock.*;
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options;
import static io.specto.hoverfly.junit.dsl.HoverflyDsl.service;
import static io.specto.hoverfly.junit.verification.HoverflyVerifications.atLeast;
import static org.assertj.core.api.Assertions.assertThat;
import static io.specto.hoverfly.junit.dsl.HoverflyDsl.*;
import static io.specto.hoverfly.junit.verification.HoverflyVerifications.*;
import static org.assertj.core.api.Assertions.*;

import co.elastic.clients.elasticsearch.ElasticsearchClient;
import co.elastic.clients.transport.endpoints.BooleanResponse;
Expand Down Expand Up @@ -51,6 +51,10 @@
/**
* We need hoverfly for testing the reactive code to use a proxy. Wiremock cannot intercept the proxy calls as WebClient
* uses HTTP CONNECT on proxy requests which wiremock does not support.
* <br/>
* Note: since 5.0 we do not use the WebClient for
* the reactive code anymore, so this might be handled with two wiremocks, but there is no real need to change this test
* setup.
*
* @author Peter-Josef Meisch
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
/*
* Copyright 2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.elasticsearch.client.elc;

import static com.github.tomakehurst.wiremock.client.WireMock.*;
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.*;

import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Configuration;
import org.springframework.data.annotation.Id;
import org.springframework.data.elasticsearch.annotations.Document;
import org.springframework.data.elasticsearch.annotations.Field;
import org.springframework.data.elasticsearch.client.ClientConfiguration;
import org.springframework.data.elasticsearch.core.ElasticsearchOperations;
import org.springframework.lang.Nullable;
import org.springframework.test.context.junit.jupiter.SpringExtension;

import com.github.tomakehurst.wiremock.junit5.WireMockExtension;

/**
* Tests that need to check the data produced by the Elasticsearch client
* @author Peter-Josef Meisch
*/
@ExtendWith(SpringExtension.class)
public class ELCWiremockTests {

@RegisterExtension static WireMockExtension wireMock = WireMockExtension.newInstance()
.options(wireMockConfig()
.dynamicPort()
// needed, otherwise Wiremock goes to test/resources/mappings
.usingFilesUnderDirectory("src/test/resources/wiremock-mappings"))
.build();

@Configuration
static class Config extends ElasticsearchConfiguration {
@Override
public ClientConfiguration clientConfiguration() {
return ClientConfiguration.builder()
.connectedTo("localhost:" + wireMock.getPort())
.build();
}
}

@Autowired ElasticsearchOperations operations;

@Test // #2839
@DisplayName("should store null values if configured")
void shouldStoreNullValuesIfConfigured() {

wireMock.stubFor(put(urlPathEqualTo("/null-fields/_doc/42"))
.withRequestBody(equalToJson("""
{
"_class": "org.springframework.data.elasticsearch.client.elc.ELCWiremockTests$EntityWithNullFields",
"id": "42",
"field1": null
}
"""))
.willReturn(
aResponse()
.withStatus(200)
.withHeader("X-elastic-product", "Elasticsearch")
.withHeader("content-type", "application/vnd.elasticsearch+json;compatible-with=8")
.withBody("""
{
"_index": "null-fields",
"_id": "42",
"_version": 1,
"result": "created",
"forced_refresh": true,
"_shards": {
"total": 2,
"successful": 1,
"failed": 0
},
"_seq_no": 1,
"_primary_term": 1
}
""")));

var entity = new EntityWithNullFields();
entity.setId("42");

operations.save(entity);
// no need to assert anything, if the field1:null is not sent, we run into a 404 error
}

@Document(indexName = "null-fields")
static class EntityWithNullFields {
@Nullable
@Id private String id;
@Nullable
@Field(storeNullValue = true) private String field1;
@Nullable
@Field private String field2;

@Nullable
public String getId() {
return id;
}

public void setId(@Nullable String id) {
this.id = id;
}

@Nullable
public String getField1() {
return field1;
}

public void setField1(@Nullable String field1) {
this.field1 = field1;
}

@Nullable
public String getField2() {
return field2;
}

public void setField2(@Nullable String field2) {
this.field2 = field2;
}
}
}