DATAES-281 - Polishing.

Added unit test for change in DefaultResultMapperTests. Tweaked and simplified integration tests.

Make use of IdentifierAccessor in ElasticsearcTemplate.getPersistentEntityId(…). Added missing generics in DefaultResultMapper.

Removed unused imports, fixed copyright ranges, authors.

Original pull request: #156.

pom.xml

@@ -130,7 +130,7 @@
         <dependency>
             <groupId>org.projectlombok</groupId>
             <artifactId>lombok</artifactId>
-            <version>1.16.4</version>
+            <version>${lombok}</version>
             <scope>test</scope>
         </dependency>
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2014 the original author or authors.
+ * Copyright 2014-2016 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.
@@ -18,7 +18,6 @@ package org.springframework.data.elasticsearch.core;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
-import java.lang.reflect.Method;
 import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -48,6 +47,8 @@ import org.springframework.data.mapping.context.MappingContext;
 
 /**
  * @author Artur Konczak
+ * @author Young Gu
+ * @author Oliver Gierke
  */
 public class DefaultResultMapper extends AbstractResultMapper {
 
@@ -169,9 +170,12 @@ public class DefaultResultMapper extends AbstractResultMapper {
 	}
 
 	private <T> void setPersistentEntityId(T result, String id, Class<T> clazz) {
+
 		if (mappingContext != null && clazz.isAnnotationPresent(Document.class)) {
-			ElasticsearchPersistentEntity persistentEntity = mappingContext.getPersistentEntity(clazz);
+
-			PersistentProperty idProperty = persistentEntity.getIdProperty();
+			ElasticsearchPersistentEntity<?> persistentEntity = mappingContext.getPersistentEntity(clazz);
+			PersistentProperty<?> idProperty = persistentEntity.getIdProperty();
+			
 			// Only deal with String because ES generated Ids are strings !
 			if (idProperty != null && idProperty.getType().isAssignableFrom(String.class)) {
 				persistentEntity.getPropertyAccessor(result).setProperty(idProperty, id);

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

@@ -28,7 +28,6 @@ import static org.springframework.util.CollectionUtils.isEmpty;
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
-import java.lang.reflect.Method;
 import java.util.*;
 
 import org.elasticsearch.action.ListenableActionFuture;
@@ -98,6 +97,8 @@ import org.springframework.util.Assert;
  * @author Artur Konczak
  * @author Kevin Leturc
  * @author Mason Chan
+ * @author Young Gu
+ * @author Oliver Gierke
  */
 
 public class ElasticsearchTemplate implements ElasticsearchOperations, ApplicationContextAware {
@@ -1081,26 +1082,18 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, Applicati
 	}
 
 	private String getPersistentEntityId(Object entity) {
-		PersistentProperty idProperty = getPersistentEntityFor(entity.getClass()).getIdProperty();
+		
-		if (idProperty != null) {
+		ElasticsearchPersistentEntity<?> persistentEntity = getPersistentEntityFor(entity.getClass());
-			Method getter = idProperty.getGetter();
+		Object identifier = persistentEntity.getIdentifierAccessor(entity).getIdentifier();
-			if (getter != null) {
+		
-				try {
+		return identifier == null ? null : String.valueOf(identifier);
-					Object id = getter.invoke(entity);
-					if (id != null) {
-						return String.valueOf(id);
-					}
-				} catch (Throwable t) {
-					t.printStackTrace();
-				}
-			}
-		}
-		return null;
 	}
 
 	private void setPersistentEntityId(Object entity, String id) {
-		ElasticsearchPersistentEntity persistentEntity = getPersistentEntityFor(entity.getClass());
+		
-		PersistentProperty idProperty = persistentEntity.getIdProperty();
+		ElasticsearchPersistentEntity<?> persistentEntity = getPersistentEntityFor(entity.getClass());
+		PersistentProperty<?> idProperty = persistentEntity.getIdProperty();
+		
 		// Only deal with String because ES generated Ids are strings !
 		if (idProperty != null && idProperty.getType().isAssignableFrom(String.class)) {
 			persistentEntity.getPropertyAccessor(entity).setProperty(idProperty,id);

src/test/java/org/springframework/data/elasticsearch/core/DefaultResultMapperTests.java

@@ -19,11 +19,13 @@ import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
 
+import lombok.Getter;
+import lombok.NoArgsConstructor;
+
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
 
-import com.fasterxml.jackson.databind.util.ArrayIterator;
 import org.elasticsearch.action.get.GetResponse;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.search.SearchHit;
@@ -35,8 +37,12 @@ import org.junit.Test;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.springframework.data.domain.Page;
+import org.springframework.data.elasticsearch.annotations.Document;
+import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext;
 import org.springframework.data.elasticsearch.entities.Car;
 
+import com.fasterxml.jackson.databind.util.ArrayIterator;
+
 /**
  * @author Artur Konczak
  * @author Mohsin Husen
@@ -51,7 +57,7 @@ public class DefaultResultMapperTests {
 	@Before
 	public void init() {
 		MockitoAnnotations.initMocks(this);
-		resultMapper = new DefaultResultMapper();
+		resultMapper = new DefaultResultMapper(new SimpleElasticsearchMappingContext());
 	}
 
 	@Test
@@ -105,6 +111,22 @@ public class DefaultResultMapperTests {
 		assertThat(result.getName(), is("Ford"));
 	}
 	
+	/**
+	 * @see DATAES-281.
+	 */
+	@Test
+	public void setsIdentifierOnImmutableType() {
+		
+		GetResponse response = mock(GetResponse.class);
+		when(response.getSourceAsString()).thenReturn("{}");
+		when(response.getId()).thenReturn("identifier");
+		
+		ImmutableEntity result = resultMapper.mapResult(response, ImmutableEntity.class);
+		
+		assertThat(result, is(notNullValue()));
+		assertThat(result.getId(), is("identifier"));
+	}
+
 	private SearchHit createCarHit(String name, String model) {
 		SearchHit hit = mock(SearchHit.class);
 		when(hit.sourceAsString()).thenReturn(createJsonCar(name, model));
@@ -132,4 +154,12 @@ public class DefaultResultMapperTests {
 		result.put("model", new InternalSearchHitField("model", Arrays.<Object>asList(model)));
 		return result;
 	}
+	
+	@Document(indexName = "someIndex")
+	@NoArgsConstructor(force = true)
+	@Getter
+	static class ImmutableEntity {
+
+		private final String id, name;
+	}
 }

src/test/java/org/springframework/data/elasticsearch/immutable/ImmutableElasticsearchRepository.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013 the original author or authors.
+ * Copyright 2016 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.
@@ -15,14 +15,10 @@
  */
 package org.springframework.data.elasticsearch.immutable;
 
-import org.springframework.data.elasticsearch.entities.SampleEntity;
+import org.springframework.data.repository.CrudRepository;
-import org.springframework.data.elasticsearch.repository.ElasticsearchRepository;
-
-import java.util.List;
 
 /**
- * @author Rizwan Idrees
+ * @author Young Gu
- * @author Mohsin Husen
+ * @author Oliver Gierke
  */
-public interface ImmutableElasticsearchRepository extends ElasticsearchRepository<ImmutableEntity, String> {
+public interface ImmutableElasticsearchRepository extends CrudRepository<ImmutableEntity, String> {}
-}

src/test/java/org/springframework/data/elasticsearch/immutable/ImmutableElasticsearchRepositoryTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2016 the original author or authors.
+ * Copyright 2016 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.
@@ -15,65 +15,51 @@
  */
 package org.springframework.data.elasticsearch.immutable;
 
-import com.google.common.collect.Lists;
+import static org.hamcrest.Matchers.*;
-import org.hamcrest.core.IsEqual;
+import static org.junit.Assert.*;
+
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.data.domain.Page;
+import org.springframework.data.elasticsearch.core.ElasticsearchOperations;
-import org.springframework.data.domain.PageRequest;
-import org.springframework.data.domain.Sort;
-import org.springframework.data.elasticsearch.core.ElasticsearchTemplate;
-import org.springframework.data.elasticsearch.core.query.NativeSearchQueryBuilder;
-import org.springframework.data.elasticsearch.core.query.SearchQuery;
-import org.springframework.data.elasticsearch.entities.SampleEntity;
-import org.springframework.data.elasticsearch.repositories.sample.SampleElasticsearchRepository;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
 
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-
-import static org.apache.commons.lang.RandomStringUtils.randomNumeric;
-import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
-import static org.elasticsearch.index.query.QueryBuilders.termQuery;
-import static org.hamcrest.Matchers.*;
-import static org.junit.Assert.*;
-
 /**
- * @author Rizwan Idrees
+ * @author Young Gu
- * @author Mohsin Husen
+ * @author Oliver Gierke
  */
-
 @RunWith(SpringJUnit4ClassRunner.class)
-@ContextConfiguration("classpath:/immutable-repository-test.xml")
+@ContextConfiguration("classpath:immutable-repository-test.xml")
 public class ImmutableElasticsearchRepositoryTests {
 
-	@Autowired
+	@Autowired ImmutableElasticsearchRepository repository;
-	private ImmutableElasticsearchRepository repository;
+	@Autowired ElasticsearchOperations operations;
-
-	@Autowired
-	private ElasticsearchTemplate elasticsearchTemplate;
 
 
 	@Before
 	public void before() {
-		elasticsearchTemplate.deleteIndex(ImmutableEntity.class);
+		
-		elasticsearchTemplate.createIndex(ImmutableEntity.class);
+		operations.deleteIndex(ImmutableEntity.class);
-		elasticsearchTemplate.refresh(ImmutableEntity.class);
+		operations.createIndex(ImmutableEntity.class);
+		operations.refresh(ImmutableEntity.class);
 	}
+	
+	/**
+	 * @see DATAES-281
+	 */
 	@Test
 	public void shouldSaveAndFindImmutableDocument() {
+		
 		// when
 		ImmutableEntity entity = repository.save(new ImmutableEntity("test name"));
 		assertThat(entity.getId(), is(notNullValue()));
+		
 		// then
 		ImmutableEntity entityFromElasticSearch = repository.findOne(entity.getId());
-		assertThat(entityFromElasticSearch.getName(), new IsEqual("test name"));
-		assertThat(entityFromElasticSearch.getId(), new IsEqual(entity.getId()));
 		
+		assertThat(entityFromElasticSearch.getName(), is("test name"));
+		assertThat(entityFromElasticSearch.getId(), is(entity.getId()));
 	}
-
 }

src/test/java/org/springframework/data/elasticsearch/immutable/ImmutableEntity.java

@@ -15,41 +15,24 @@
  */
 package org.springframework.data.elasticsearch.immutable;
 
-import lombok.*;
+import lombok.Getter;
-import org.springframework.data.annotation.AccessType;
+import lombok.NoArgsConstructor;
-import org.springframework.data.annotation.Id;
+
-import org.springframework.data.annotation.Version;
 import org.springframework.data.elasticsearch.annotations.Document;
-import org.springframework.data.elasticsearch.annotations.Field;
-import org.springframework.data.elasticsearch.annotations.FieldType;
-import org.springframework.data.elasticsearch.annotations.ScriptedField;
-import org.springframework.data.elasticsearch.core.geo.GeoPoint;
 
 /**
- * @author Rizwan Idrees
+ * @author Young Gu
- * @author Mohsin Husen
+ * @author Oliver Gierke
  */
-@AccessType(AccessType.Type.FIELD)
+@Document(indexName = "test-index")
-@Document(indexName = "test-index", type = "immutable-entity", shards = 1, replicas = 0, refreshInterval = "-1")
+@NoArgsConstructor(force = true)
+@Getter
 public class ImmutableEntity {
-
+	private final String id, name;
-	@Id
-	private String id;
-	private String name;
-
-	private ImmutableEntity() {
-	}
 	
 	public ImmutableEntity(String name) {
+		
+		this.id = null;
 		this.name = name;
 	}
-
-	public String getId() {
-		return id;
-	}
-
-	public String getName() {
-		return name;
-	}
-
 }