DATAES-747 - ElasticsearchConfigurationSupport does not set customConversions into the MappingElasticsearchConverter.

Original PR: spring-projects#395

Author: sothawo

src/main/asciidoc/reference/elasticsearch-object-mapping.adoc

@@ -3,12 +3,15 @@
 
 Spring Data Elasticsearch Object Mapping is the process that maps a Java object - the domain entity - into the JSON representation that is stored in Elasticsearch and back.
 
-Earlier versions of Spring Data Elasticsearch used a Jackson based conversion, Spring Data Elasticsearch 3.2.x introduced the <<elasticsearch.mapping.meta-model>>. As of version 4.0 only the Meta Object Mapping is used, the Jackson based mapper is not available anymore.
+Earlier versions of Spring Data Elasticsearch used a Jackson based conversion, Spring Data Elasticsearch 3.2.x introduced the <<elasticsearch.mapping.meta-model>>. As of version 4.0 only the Meta Object Mapping is used, the Jackson based mapper is not available anymore and the `MappingElasticsearchConverter` is used.
 
 The main reasons for the removal of the Jackson based mapper are:
 
 * Custom mappings of fields needed to be done with annotations like `@JsonFormat` or `@JsonInclude`. This often caused problems when the same object was used in different JSON based datastores or sent over a JSON based API.
 * Custom field types and formats also need to be stored into the Elasticsearch index mappings. The Jackson based annotations did not fully provide all the information that is necessary to represent the types of Elasticsearch.
+* Fields must be mapped not only when converting fromand to entities, but also an query argument, returned data and on other places.
+
+Using the `MappingElasticsearchConverter` now covers all these cases.
 
 
 [[elasticsearch.mapping.meta-model]]
@@ -20,7 +23,7 @@ This allows to register `Converter` instances for specific domain type mapping.
 [[elasticsearch.mapping.meta-model.annotations]]
 === Mapping Annotation Overview
 
-The `ElasticsearchEntityMapper` can use metadata to drive the mapping of objects to documents. The metadata is taken from the entities properties which can be annotated.
+The `MappingElasticsearchConverter` uses metadata to drive the mapping of objects to documents. The metadata is taken from the entity's properties which can be annotated.
 
 The following annotations are available:
 
@@ -206,25 +209,14 @@ public class Config extends AbstractElasticsearchConfiguration {
     return RestClients.create(ClientConfiguration.create("localhost:9200")).rest();
   }
 
-  @Bean
-  @Override
-  public EntityMapper entityMapper() {
-
-    ElasticsearchEntityMapper entityMapper = new ElasticsearchEntityMapper(
-      elasticsearchMappingContext(), new DefaultConversionService());
-    entityMapper.setConversions(elasticsearchCustomConversions());  <1>
-
-  	return entityMapper;
-  }
-
   @Bean
   @Override
   public ElasticsearchCustomConversions elasticsearchCustomConversions() {
     return new ElasticsearchCustomConversions(
-      Arrays.asList(new AddressToMap(), new MapToAddress()));       <2>
+      Arrays.asList(new AddressToMap(), new MapToAddress()));       <1>
   }
 
-  @WritingConverter                                                 <3>
+  @WritingConverter                                                 <2>
   static class AddressToMap implements Converter<Address, Map<String, Object>> {
 
     @Override
@@ -238,7 +230,7 @@ public class Config extends AbstractElasticsearchConfiguration {
     }
   }
 
-  @ReadingConverter                                                 <4>
+  @ReadingConverter                                                 <3>
   static class MapToAddress implements Converter<Map<String, Object>, Address> {
 
     @Override
@@ -258,8 +250,7 @@ public class Config extends AbstractElasticsearchConfiguration {
   "localidad" : { "lat" : 34.118347, "lon" : -118.3026284 }
 }
 ----
-<1> Register `ElasticsearchCustomConversions` with the `EntityMapper`.
-<2> Add `Converter` implementations.
-<3> Set up the `Converter` used for writing `DomainType` to Elasticsearch.
-<4> Set up the `Converter` used for reading `DomainType` from search result.
+<1> Add `Converter` implementations.
+<2> Set up the `Converter` used for writing `DomainType` to Elasticsearch.
+<3> Set up the `Converter` used for reading `DomainType` from search result.
 ====

src/main/asciidoc/reference/elasticsearch-operations.adoc

@@ -41,7 +41,7 @@ public class TransportClientConfig extends ElasticsearchConfigurationSupport {
 
   @Bean(name = {"elasticsearchOperations", "elasticsearchTemplate"})
   public ElasticsearchTemplate elasticsearchTemplate() throws UnknownHostException { <2>
-  	return new ElasticsearchTemplate(elasticsearchClient(), entityMapper());
+  	return new ElasticsearchTemplate(elasticsearchClient());
   }
 }
 ----

src/main/java/org/springframework/data/elasticsearch/config/ElasticsearchConfigurationSupport.java

@@ -48,7 +48,10 @@ public class ElasticsearchConfigurationSupport {
 	@Bean
 	public ElasticsearchConverter elasticsearchEntityMapper(
 			SimpleElasticsearchMappingContext elasticsearchMappingContext) {
-		return new MappingElasticsearchConverter(elasticsearchMappingContext);
+		MappingElasticsearchConverter elasticsearchConverter = new MappingElasticsearchConverter(
+				elasticsearchMappingContext);
+		elasticsearchConverter.setConversions(elasticsearchCustomConversions());
+		return elasticsearchConverter;
 	}
 
 	/**

src/main/java/org/springframework/data/elasticsearch/core/AbstractElasticsearchTemplate.java

@@ -326,7 +326,16 @@ private Long getEntityVersion(Object entity) {
 	}
 
 	private <T> IndexQuery getIndexQuery(T entity) {
-		return new IndexQueryBuilder().withObject(entity).withId(getEntityId(entity)).withVersion(getEntityVersion(entity))
+		String id = getEntityId(entity);
+
+		if (id != null) {
+			id = elasticsearchConverter.convertId(id);
+		}
+
+		return new IndexQueryBuilder() //
+				.withId(id) //
+				.withVersion(getEntityVersion(entity)) //
+				.withObject(entity) //
 				.build();
 	}
 

src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchRestTemplate.java

@@ -173,7 +173,7 @@ public void bulkUpdate(List<UpdateQuery> queries, BulkOptions bulkOptions, Index
 
 	@Override
 	public String delete(String id, IndexCoordinates index) {
-		DeleteRequest request = new DeleteRequest(index.getIndexName(), id);
+		DeleteRequest request = new DeleteRequest(index.getIndexName(), elasticsearchConverter.convertId(id));
 		try {
 			return client.delete(request, RequestOptions.DEFAULT).getId();
 		} catch (IOException e) {

src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java

@@ -165,7 +165,8 @@ public void bulkUpdate(List<UpdateQuery> queries, BulkOptions bulkOptions, Index
 
 	@Override
 	public String delete(String id, IndexCoordinates index) {
-		return client.prepareDelete(index.getIndexName(), IndexCoordinates.TYPE, id).execute().actionGet().getId();
+		return client.prepareDelete(index.getIndexName(), IndexCoordinates.TYPE, elasticsearchConverter.convertId(id))
+				.execute().actionGet().getId();
 	}
 
 	@Override

src/test/java/org/springframework/data/elasticsearch/core/RequestFactoryTest.java renamed to src/test/java/org/springframework/data/elasticsearch/core/RequestFactoryTests.java

@@ -40,7 +40,7 @@
 /**
  * @author Peter-Josef Meisch
  */
-class RequestFactoryTest {
+class RequestFactoryTests {
 
 	@Nullable private static RequestFactory requestFactory;
 	@Nullable private static MappingElasticsearchConverter converter;
@@ -74,7 +74,7 @@ void shouldBuildSearchWithGeoSortSort() throws JSONException {
 				"          \"query_string\": {" + //
 				"            \"query\": \"Smith\"," + //
 				"            \"fields\": [" + //
-				"              \"first-name^1.0\"" + //
+				"              \"last-name^1.0\"" + //
 				"            ]" + //
 				"          }" + //
 				"        }" + //

src/test/java/org/springframework/data/elasticsearch/core/index/MappingContextBaseTests.java

@@ -34,7 +34,7 @@ private ElasticsearchConverter setupElasticsearchConverter() {
 
 	private SimpleElasticsearchMappingContext setupMappingContext() {
 
-		SimpleElasticsearchMappingContext mappingContext = new ElasticsearchConfigurationSupport() {}
+		SimpleElasticsearchMappingContext mappingContext = new ElasticsearchConfigurationSupport()
 				.elasticsearchMappingContext();
 		mappingContext.initialize();
 		return mappingContext;