Skip to content

Commit c965c34

Browse files
odrotbohmxhaggi
authored andcommitted
DATAES-462 - Polishing.
SimpleElasticsearchPersistentProperty now already checks for the correct type of score properties. Added unit tests for that. Also added unit tests for SimpleElasticsearchPersistentEntity rejecting more than one score property being present. Additional non-null assertions on components that are required so that we can remove superfluous null checks. A bit o formatting, Javadoc, missing @SInCE tags and license headers. Original pull request: spring-projects#207.
1 parent 7fb3eaf commit c965c34

15 files changed

+159
-26
lines changed

src/main/java/org/springframework/data/elasticsearch/annotations/Score.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@
1313
* Specifies that this field is used for storing the document score.
1414
*
1515
* @author Sascha Woo
16+
* @since 3.1
1617
*/
1718
@Retention(RetentionPolicy.RUNTIME)
1819
@Target(ElementType.FIELD)
1920
@Documented
2021
@Inherited
2122
@ReadOnlyProperty
22-
public @interface Score {
23-
}
23+
public @interface Score {}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import org.springframework.data.elasticsearch.ElasticsearchException;
2121
import org.springframework.util.StringUtils;
22+
import org.springframework.util.Assert;
2223

2324
/**
2425
* @author Artur Konczak
@@ -28,6 +29,9 @@ public abstract class AbstractResultMapper implements ResultsMapper {
2829
private EntityMapper entityMapper;
2930

3031
public AbstractResultMapper(EntityMapper entityMapper) {
32+
33+
Assert.notNull(entityMapper, "EntityMapper must not be null!");
34+
3135
this.entityMapper = entityMapper;
3236
}
3337

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

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2014-2017 the original author or authors.
2+
* Copyright 2014-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -57,25 +57,33 @@
5757
*/
5858
public class DefaultResultMapper extends AbstractResultMapper {
5959

60-
private MappingContext<? extends ElasticsearchPersistentEntity<?>, ElasticsearchPersistentProperty> mappingContext;
60+
private final MappingContext<? extends ElasticsearchPersistentEntity<?>, ElasticsearchPersistentProperty> mappingContext;
6161

6262
public DefaultResultMapper() {
6363
this(new SimpleElasticsearchMappingContext());
6464
}
6565

6666
public DefaultResultMapper(MappingContext<? extends ElasticsearchPersistentEntity<?>, ElasticsearchPersistentProperty> mappingContext) {
67+
6768
super(new DefaultEntityMapper(mappingContext));
69+
70+
Assert.notNull(mappingContext, "MappingContext must not be null!");
71+
6872
this.mappingContext = mappingContext;
6973
}
7074

7175
public DefaultResultMapper(EntityMapper entityMapper) {
72-
super(entityMapper);
76+
this(new SimpleElasticsearchMappingContext(), entityMapper);
7377
}
7478

7579
public DefaultResultMapper(
7680
MappingContext<? extends ElasticsearchPersistentEntity<?>, ElasticsearchPersistentProperty> mappingContext,
7781
EntityMapper entityMapper) {
82+
7883
super(entityMapper);
84+
85+
Assert.notNull(mappingContext, "MappingContext must not be null!");
86+
7987
this.mappingContext = mappingContext;
8088
}
8189

@@ -97,6 +105,7 @@ public <T> AggregatedPage<T> mapResults(SearchResponse response, Class<T> clazz,
97105
setPersistentEntityId(result, hit.getId(), clazz);
98106
setPersistentEntityVersion(result, hit.getVersion(), clazz);
99107
setPersistentEntityScore(result, hit.getScore(), clazz);
108+
100109
populateScriptFields(result, hit);
101110
results.add(result);
102111
}
@@ -183,7 +192,9 @@ public <T> LinkedList<T> mapResults(MultiGetResponse responses, Class<T> clazz)
183192
}
184193

185194
private <T> void setPersistentEntityId(T result, String id, Class<T> clazz) {
186-
if (mappingContext != null && clazz.isAnnotationPresent(Document.class)) {
195+
196+
if (clazz.isAnnotationPresent(Document.class)) {
197+
187198
ElasticsearchPersistentEntity<?> persistentEntity = mappingContext.getRequiredPersistentEntity(clazz);
188199
ElasticsearchPersistentProperty idProperty = persistentEntity.getIdProperty();
189200

@@ -195,7 +206,9 @@ private <T> void setPersistentEntityId(T result, String id, Class<T> clazz) {
195206
}
196207

197208
private <T> void setPersistentEntityVersion(T result, long version, Class<T> clazz) {
198-
if (mappingContext != null && clazz.isAnnotationPresent(Document.class)) {
209+
210+
if (clazz.isAnnotationPresent(Document.class)) {
211+
199212
ElasticsearchPersistentEntity<?> persistentEntity = mappingContext.getPersistentEntity(clazz);
200213
ElasticsearchPersistentProperty versionProperty = persistentEntity.getVersionProperty();
201214

@@ -210,14 +223,17 @@ private <T> void setPersistentEntityVersion(T result, long version, Class<T> cla
210223
}
211224

212225
private <T> void setPersistentEntityScore(T result, float score, Class<T> clazz) {
213-
if (mappingContext != null && clazz.isAnnotationPresent(Document.class)) {
214-
ElasticsearchPersistentEntity<?> persistentEntity = mappingContext.getRequiredPersistentEntity(clazz);
215-
ElasticsearchPersistentProperty scoreProperty = persistentEntity.getScoreProperty();
216-
Class<?> type = scoreProperty.getType();
217226

218-
if (scoreProperty != null && (type == Float.class || type == Float.TYPE)) {
219-
persistentEntity.getPropertyAccessor(result).setProperty(scoreProperty, score);
227+
if (clazz.isAnnotationPresent(Document.class)) {
228+
229+
ElasticsearchPersistentEntity<?> entity = mappingContext.getRequiredPersistentEntity(clazz);
230+
231+
if (!entity.hasScoreProperty()) {
232+
return;
220233
}
234+
235+
entity.getPropertyAccessor(result) //
236+
.setProperty(entity.getScoreProperty(), score);
221237
}
222238
}
223239
}

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

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,18 @@
1-
1+
/*
2+
* Copyright 2018 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
216
package org.springframework.data.elasticsearch.core;
317

418
import org.springframework.data.domain.Page;
@@ -12,5 +26,4 @@
1226
public interface ScoredPage<T> extends Page<T> {
1327

1428
float getMaxScore();
15-
1629
}

src/main/java/org/springframework/data/elasticsearch/core/mapping/ElasticsearchPersistentEntity.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2013-2017 the original author or authors.
2+
* Copyright 2013-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -25,6 +25,7 @@
2525
* @author Mohsin Husen
2626
* @author Mark Paluch
2727
* @author Sascha Woo
28+
* @author Oliver Gierke
2829
*/
2930
public interface ElasticsearchPersistentEntity<T> extends PersistentEntity<T, ElasticsearchPersistentProperty> {
3031

@@ -57,6 +58,7 @@ public interface ElasticsearchPersistentEntity<T> extends PersistentEntity<T, El
5758
* {@literal true}, {@link #getScoreProperty()} will return a non-{@literal null} value.
5859
*
5960
* @return false when {@link ElasticsearchPersistentEntity} does not define a score property.
61+
* @since 3.1
6062
*/
6163
boolean hasScoreProperty();
6264

@@ -66,6 +68,7 @@ public interface ElasticsearchPersistentEntity<T> extends PersistentEntity<T, El
6668
*
6769
* @return the score {@link ElasticsearchPersistentProperty} of the {@link PersistentEntity} or {@literal null} if not
6870
* defined.
71+
* @since 3.1
6972
*/
7073
@Nullable
7174
ElasticsearchPersistentProperty getScoreProperty();

src/main/java/org/springframework/data/elasticsearch/core/mapping/ElasticsearchPersistentProperty.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2013 the original author or authors.
2+
* Copyright 2013-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -24,8 +24,8 @@
2424
* @author Rizwan Idrees
2525
* @author Mohsin Husen
2626
* @author Sascha Woo
27+
* @author Oliver Gierke
2728
*/
28-
2929
public interface ElasticsearchPersistentProperty extends PersistentProperty<ElasticsearchPersistentProperty> {
3030

3131
String getFieldName();
@@ -38,6 +38,7 @@ public interface ElasticsearchPersistentProperty extends PersistentProperty<Elas
3838
* current property is the version property of that {@link ElasticsearchPersistentEntity} under consideration.
3939
*
4040
* @return
41+
* @since 3.1
4142
*/
4243
boolean isScoreProperty();
4344

src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentEntity.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ public void addPersistentProperty(ElasticsearchPersistentProperty property) {
183183
}
184184

185185
if (property.isScoreProperty()) {
186+
186187
ElasticsearchPersistentProperty scoreProperty = this.scoreProperty;
187188

188189
if (scoreProperty != null) {
@@ -191,8 +192,6 @@ public void addPersistentProperty(ElasticsearchPersistentProperty property) {
191192
+ "as version. Check your mapping configuration!", property.getField(), scoreProperty.getField()));
192193
}
193194

194-
Assert.isTrue(property.getType() == Float.class || property.getType() == Float.TYPE, "Score property must be of type float!");
195-
196195
this.scoreProperty = property;
197196
}
198197
}

src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentProperty.java

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2013-2017 the original author or authors.
2+
* Copyright 2013-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -15,16 +15,17 @@
1515
*/
1616
package org.springframework.data.elasticsearch.core.mapping;
1717

18+
import java.util.Arrays;
1819
import java.util.HashSet;
1920
import java.util.Set;
2021

2122
import org.springframework.data.elasticsearch.annotations.Score;
2223
import org.springframework.data.mapping.Association;
24+
import org.springframework.data.mapping.MappingException;
2325
import org.springframework.data.mapping.PersistentEntity;
2426
import org.springframework.data.mapping.model.AnnotationBasedPersistentProperty;
2527
import org.springframework.data.mapping.model.Property;
2628
import org.springframework.data.mapping.model.SimpleTypeHolder;
27-
import org.springframework.data.util.Lazy;
2829

2930
/**
3031
* Elasticsearch specific {@link org.springframework.data.mapping.PersistentProperty} implementation processing
@@ -33,14 +34,15 @@
3334
* @author Mohsin Husen
3435
* @author Mark Paluch
3536
* @author Sascha Woo
37+
* @author Oliver Gierke
3638
*/
3739
public class SimpleElasticsearchPersistentProperty extends
3840
AnnotationBasedPersistentProperty<ElasticsearchPersistentProperty> implements ElasticsearchPersistentProperty {
3941

4042
private static final Set<Class<?>> SUPPORTED_ID_TYPES = new HashSet<>();
4143
private static final Set<String> SUPPORTED_ID_PROPERTY_NAMES = new HashSet<>();
4244

43-
private final Lazy<Boolean> isScore = Lazy.of(() -> isAnnotationPresent(Score.class));
45+
private final boolean isScore;
4446

4547
static {
4648
SUPPORTED_ID_TYPES.add(String.class);
@@ -50,7 +52,14 @@ public class SimpleElasticsearchPersistentProperty extends
5052

5153
public SimpleElasticsearchPersistentProperty(Property property,
5254
PersistentEntity<?, ElasticsearchPersistentProperty> owner, SimpleTypeHolder simpleTypeHolder) {
55+
5356
super(property, owner, simpleTypeHolder);
57+
58+
this.isScore = isAnnotationPresent(Score.class);
59+
60+
if (isScore && !Arrays.asList(Float.TYPE, Float.class).contains(getType())) {
61+
throw new MappingException(String.format("Score property %s must be either of type float or Float!", property.getName()));
62+
}
5463
}
5564

5665
@Override
@@ -68,8 +77,12 @@ protected Association<ElasticsearchPersistentProperty> createAssociation() {
6877
return null;
6978
}
7079

80+
/*
81+
* (non-Javadoc)
82+
* @see org.springframework.data.elasticsearch.core.mapping.ElasticsearchPersistentProperty#isScoreProperty()
83+
*/
7184
@Override
7285
public boolean isScoreProperty() {
73-
return isScore.get();
86+
return isScore;
7487
}
7588
}

src/main/java/org/springframework/data/elasticsearch/core/query/AbstractQuery.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,11 +154,21 @@ public SearchType getSearchType() {
154154
return searchType;
155155
}
156156

157+
/*
158+
* (non-Javadoc)
159+
* @see org.springframework.data.elasticsearch.core.query.Query#getTrackScores()
160+
*/
157161
@Override
158162
public boolean getTrackScores() {
159163
return trackScores;
160164
}
161165

166+
/**
167+
* Configures whether to track scores.
168+
*
169+
* @param trackScores
170+
* @since 3.1
171+
*/
162172
public void setTrackScores(boolean trackScores) {
163173
this.trackScores = trackScores;
164174
}

src/main/java/org/springframework/data/elasticsearch/core/query/NativeSearchQueryBuilder.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,11 @@ public NativeSearchQueryBuilder withMinScore(float minScore) {
127127
return this;
128128
}
129129

130+
/**
131+
* @param trackScores whether to track scores.
132+
* @return
133+
* @since 3.1
134+
*/
130135
public NativeSearchQueryBuilder withTrackScores(boolean trackScores) {
131136
this.trackScores = trackScores;
132137
return this;

src/main/java/org/springframework/data/elasticsearch/core/query/Query.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ public interface Query {
130130
* Get if scores will be computed and tracked, regardless of whether sorting on a field. Defaults to <tt>false</tt>.
131131
*
132132
* @return
133+
* @since 3.1
133134
*/
134135
boolean getTrackScores();
135136

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import org.springframework.data.elasticsearch.entities.SampleEntity;
4646

4747
import static java.util.Arrays.asList;
48+
import static org.assertj.core.api.Assertions.*;
4849
import static org.hamcrest.Matchers.*;
4950
import static org.junit.Assert.*;
5051
import static org.mockito.Mockito.*;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder;
3636
import org.elasticsearch.search.fetch.subphase.highlight.HighlightField;
3737
import org.elasticsearch.search.sort.FieldSortBuilder;
38-
import org.elasticsearch.search.sort.SortBuilder;
3938
import org.elasticsearch.search.sort.SortBuilders;
4039
import org.elasticsearch.search.sort.SortOrder;
4140
import org.hamcrest.Matchers;
@@ -1509,6 +1508,7 @@ public void shouldReturnDocumentAboveMinimalScoreGivenQuery() {
15091508

15101509
@Test // DATAES-462
15111510
public void shouldReturnScores() {
1511+
15121512
// given
15131513
List<IndexQuery> indexQueries = new ArrayList<>();
15141514

0 commit comments

Comments
 (0)