From 7113c7ac667d880d87182e1c7ccfbadb433c6be2 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Sat, 20 Aug 2016 09:38:34 +0200 Subject: [PATCH] DATAES-281 - Polishing. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../core/DefaultResultMapper.java | 19 +++--- .../core/ElasticsearchTemplate.java | 29 ++++----- .../core/DefaultResultMapperTests.java | 37 +++++++++++- .../ImmutableElasticsearchRepository.java | 14 ++--- ...ImmutableElasticsearchRepositoryTests.java | 60 +++++++------------ .../immutable/ImmutableEntity.java | 41 ++++--------- 6 files changed, 96 insertions(+), 104 deletions(-) diff --git a/src/main/java/org/springframework/data/elasticsearch/core/DefaultResultMapper.java b/src/main/java/org/springframework/data/elasticsearch/core/DefaultResultMapper.java index 851690faa..ff19a1d98 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/DefaultResultMapper.java +++ b/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; @@ -35,9 +34,6 @@ import org.elasticsearch.action.get.MultiGetResponse; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.SearchHitField; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.elasticsearch.ElasticsearchException; import org.springframework.data.elasticsearch.annotations.Document; @@ -52,11 +48,11 @@ import org.springframework.data.mapping.context.MappingContext; /** * @author Artur Konczak * @author Petar Tahchiev + * @author Young Gu + * @author Oliver Gierke */ public class DefaultResultMapper extends AbstractResultMapper { - private static final Logger LOG = LoggerFactory.getLogger(DefaultResultMapper.class); - private MappingContext, ElasticsearchPersistentProperty> mappingContext; public DefaultResultMapper() { @@ -176,12 +172,15 @@ public class DefaultResultMapper extends AbstractResultMapper { } private void setPersistentEntityId(T result, String id, Class 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); + persistentEntity.getPropertyAccessor(result).setProperty(idProperty, id); } } } diff --git a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java b/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java index 00a9ce0f1..0618f7f7f 100755 --- a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java +++ b/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; @@ -101,6 +100,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 { @@ -1095,26 +1096,18 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, Applicati } private String getPersistentEntityId(Object entity) { - PersistentProperty idProperty = getPersistentEntityFor(entity.getClass()).getIdProperty(); - if (idProperty != null) { - Method getter = idProperty.getGetter(); - if (getter != null) { - try { - Object id = getter.invoke(entity); - if (id != null) { - return String.valueOf(id); - } - } catch (Throwable t) { - t.printStackTrace(); - } - } - } - return null; + + ElasticsearchPersistentEntity persistentEntity = getPersistentEntityFor(entity.getClass()); + Object identifier = persistentEntity.getIdentifierAccessor(entity).getIdentifier(); + + return identifier == null ? null : String.valueOf(identifier); } 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); diff --git a/src/test/java/org/springframework/data/elasticsearch/core/DefaultResultMapperTests.java b/src/test/java/org/springframework/data/elasticsearch/core/DefaultResultMapperTests.java index e3f371600..542df6ac2 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/DefaultResultMapperTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/DefaultResultMapperTests.java @@ -19,8 +19,14 @@ import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; import static org.mockito.Mockito.*; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.RequiredArgsConstructor; +import lombok.Value; + import java.util.*; +import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.databind.util.ArrayIterator; import org.elasticsearch.action.get.GetResponse; import org.elasticsearch.action.search.SearchResponse; @@ -35,8 +41,13 @@ import org.junit.Before; import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.springframework.data.annotation.AccessType; +import org.springframework.data.annotation.Id; import org.springframework.data.domain.Page; +import org.springframework.data.elasticsearch.annotations.Document; +import org.springframework.data.elasticsearch.core.DefaultResultMapperTests.ImmutableEntity; import org.springframework.data.elasticsearch.core.aggregation.AggregatedPage; +import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext; import org.springframework.data.elasticsearch.entities.Car; /** @@ -53,7 +64,7 @@ public class DefaultResultMapperTests { @Before public void init() { MockitoAnnotations.initMocks(this); - resultMapper = new DefaultResultMapper(); + resultMapper = new DefaultResultMapper(new SimpleElasticsearchMappingContext()); } @Test @@ -132,6 +143,22 @@ public class DefaultResultMapperTests { assertThat(result.getModel(), is("Grat")); 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 Aggregation createCarAggregation() { Aggregation aggregation = mock(Terms.class); @@ -166,4 +193,12 @@ public class DefaultResultMapperTests { result.put("model", new InternalSearchHitField("model", Arrays.asList(model))); return result; } + + @Document(indexName = "someIndex") + @NoArgsConstructor(force = true) + @Getter + static class ImmutableEntity { + + private final String id, name; + } } diff --git a/src/test/java/org/springframework/data/elasticsearch/immutable/ImmutableElasticsearchRepository.java b/src/test/java/org/springframework/data/elasticsearch/immutable/ImmutableElasticsearchRepository.java index e3885c60b..41f39be1c 100644 --- a/src/test/java/org/springframework/data/elasticsearch/immutable/ImmutableElasticsearchRepository.java +++ b/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.elasticsearch.repository.ElasticsearchRepository; - -import java.util.List; +import org.springframework.data.repository.CrudRepository; /** - * @author Rizwan Idrees - * @author Mohsin Husen + * @author Young Gu + * @author Oliver Gierke */ -public interface ImmutableElasticsearchRepository extends ElasticsearchRepository { -} +public interface ImmutableElasticsearchRepository extends CrudRepository {} diff --git a/src/test/java/org/springframework/data/elasticsearch/immutable/ImmutableElasticsearchRepositoryTests.java b/src/test/java/org/springframework/data/elasticsearch/immutable/ImmutableElasticsearchRepositoryTests.java index 24f4a41cd..afc9eb84c 100644 --- a/src/test/java/org/springframework/data/elasticsearch/immutable/ImmutableElasticsearchRepositoryTests.java +++ b/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 org.hamcrest.core.IsEqual; +import static org.hamcrest.Matchers.*; +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.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.data.elasticsearch.core.ElasticsearchOperations; 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 Mohsin Husen + * @author Young Gu + * @author Oliver Gierke */ - @RunWith(SpringJUnit4ClassRunner.class) -@ContextConfiguration("classpath:/immutable-repository-test.xml") +@ContextConfiguration("classpath:immutable-repository-test.xml") public class ImmutableElasticsearchRepositoryTests { - @Autowired - private ImmutableElasticsearchRepository repository; - - @Autowired - private ElasticsearchTemplate elasticsearchTemplate; + @Autowired ImmutableElasticsearchRepository repository; + @Autowired ElasticsearchOperations operations; @Before public void before() { - elasticsearchTemplate.deleteIndex(ImmutableEntity.class); - elasticsearchTemplate.createIndex(ImmutableEntity.class); - elasticsearchTemplate.refresh(ImmutableEntity.class); + + operations.deleteIndex(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())); } - } diff --git a/src/test/java/org/springframework/data/elasticsearch/immutable/ImmutableEntity.java b/src/test/java/org/springframework/data/elasticsearch/immutable/ImmutableEntity.java index cc34650ff..b03a5fd21 100644 --- a/src/test/java/org/springframework/data/elasticsearch/immutable/ImmutableEntity.java +++ b/src/test/java/org/springframework/data/elasticsearch/immutable/ImmutableEntity.java @@ -15,41 +15,24 @@ */ package org.springframework.data.elasticsearch.immutable; -import lombok.*; -import org.springframework.data.annotation.AccessType; -import org.springframework.data.annotation.Id; -import org.springframework.data.annotation.Version; +import lombok.Getter; +import lombok.NoArgsConstructor; + 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 Mohsin Husen + * @author Young Gu + * @author Oliver Gierke */ -@AccessType(AccessType.Type.FIELD) -@Document(indexName = "test-index", type = "immutable-entity", shards = 1, replicas = 0, refreshInterval = "-1") +@Document(indexName = "test-index") +@NoArgsConstructor(force = true) +@Getter public class ImmutableEntity { - - @Id - private String id; - private String name; - - private ImmutableEntity() { - } - + private final String id, name; + public ImmutableEntity(String name) { + + this.id = null; this.name = name; } - - public String getId() { - return id; - } - - public String getName() { - return name; - } - }