Skip to content

Commit a085c06

Browse files
mohsinhmohsinh
committed
DATAES-274 - prevent NPE in FacetedPageImpl
1 parent a343bf7 commit a085c06

File tree

2 files changed

+110
-59
lines changed

2 files changed

+110
-59
lines changed

‎src/main/java/org/springframework/data/elasticsearch/core/FacetedPageImpl.java

Lines changed: 77 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
import java.util.HashMap;
2020
import java.util.List;
2121
import java.util.Map;
22-
2322
import org.elasticsearch.search.aggregations.Aggregation;
23+
import org.elasticsearch.search.aggregations.Aggregations;
2424
import org.elasticsearch.search.aggregations.bucket.histogram.Histogram;
2525
import org.elasticsearch.search.aggregations.bucket.range.Range;
2626
import org.elasticsearch.search.aggregations.bucket.terms.Terms;
@@ -33,7 +33,12 @@
3333
import org.springframework.data.elasticsearch.core.facet.AbstractFacetRequest;
3434
import org.springframework.data.elasticsearch.core.facet.FacetResult;
3535
import org.springframework.data.elasticsearch.core.facet.request.RangeFacetRequest;
36-
import org.springframework.data.elasticsearch.core.facet.result.*;
36+
import org.springframework.data.elasticsearch.core.facet.result.HistogramResult;
37+
import org.springframework.data.elasticsearch.core.facet.result.IntervalUnit;
38+
import org.springframework.data.elasticsearch.core.facet.result.RangeResult;
39+
import org.springframework.data.elasticsearch.core.facet.result.StatisticalResult;
40+
import org.springframework.data.elasticsearch.core.facet.result.Term;
41+
import org.springframework.data.elasticsearch.core.facet.result.TermResult;
3742

3843
/**
3944
* Container for query result and facet results
@@ -42,6 +47,7 @@
4247
* @author Mohsin Husen
4348
* @author Artur Konczak
4449
* @author Jonathan Yan
50+
* @author Philipp Kräutli
4551
*/
4652
@Deprecated
4753
public abstract class FacetedPageImpl<T> extends PageImpl<T> implements FacetedPage<T>, AggregatedPage<T> {
@@ -84,47 +90,76 @@ private void addFacet(FacetResult facetResult) {
8490
* Lazy conversion from aggregation to old facets
8591
*/
8692
private void processAggregations() {
87-
if (facets == null) {
88-
facets = new ArrayList<>();
89-
for (Aggregation agg : getAggregations()) {
90-
if (agg instanceof Terms) {
91-
List<Term> terms = new ArrayList<>();
92-
for (Terms.Bucket t : ((Terms) agg).getBuckets()) {
93-
terms.add(new Term(t.getKeyAsString(), t.getDocCount()));
94-
}
95-
addFacet(new TermResult(agg.getName(), terms, terms.size(), ((Terms) agg).getSumOfOtherDocCounts(), 0));
96-
}
97-
if (agg instanceof Range) {
98-
List<? extends Range.Bucket> buckets = ((Range) agg).getBuckets();
99-
List<org.springframework.data.elasticsearch.core.facet.result.Range> ranges = new ArrayList<>();
100-
for (Range.Bucket b : buckets) {
101-
ExtendedStats rStats = (ExtendedStats) b.getAggregations().get(AbstractFacetRequest.INTERNAL_STATS);
102-
if (rStats != null) {
103-
Sum sum = (Sum) b.getAggregations().get(RangeFacetRequest.RANGE_INTERNAL_SUM);
104-
ranges.add(new org.springframework.data.elasticsearch.core.facet.result.Range((Double) b.getFrom(), (Double) b.getTo(), b.getDocCount(), sum != null ? sum.getValue() : rStats.getSum(), rStats.getCount(), rStats.getMin(), rStats.getMax()));
105-
} else {
106-
ranges.add(new org.springframework.data.elasticsearch.core.facet.result.Range((Double) b.getFrom(), (Double) b.getTo(), b.getDocCount(), 0, 0, 0, 0));
107-
}
108-
}
109-
addFacet(new RangeResult(agg.getName(), ranges));
110-
}
111-
if (agg instanceof ExtendedStats) {
112-
ExtendedStats stats = (ExtendedStats) agg;
113-
addFacet(new StatisticalResult(agg.getName(), stats.getCount(), stats.getMax(), stats.getMin(), stats.getAvg(), stats.getStdDeviation(), stats.getSumOfSquares(), stats.getSum(), stats.getVariance()));
114-
}
115-
if (agg instanceof Histogram) {
116-
List<IntervalUnit> intervals = new ArrayList<>();
117-
for (Histogram.Bucket h : ((Histogram) agg).getBuckets()) {
118-
ExtendedStats hStats = (ExtendedStats) h.getAggregations().get(AbstractFacetRequest.INTERNAL_STATS);
119-
if (hStats != null) {
120-
intervals.add(new IntervalUnit(((DateTime) h.getKey()).getMillis(), h.getDocCount(), h.getDocCount(), hStats.getSum(), hStats.getAvg(), hStats.getMin(), hStats.getMax()));
121-
} else {
122-
intervals.add(new IntervalUnit(((DateTime) h.getKey()).getMillis(), h.getDocCount(), h.getDocCount(), 0, 0, 0, 0));
123-
}
124-
}
125-
addFacet(new HistogramResult(agg.getName(), intervals));
126-
}
93+
if (facets != null) {
94+
return;
95+
}
96+
facets = new ArrayList<>();
97+
Aggregations aggregations = getAggregations();
98+
if (aggregations == null) {
99+
return;
100+
}
101+
for (Aggregation agg : aggregations) {
102+
processAggregation(agg);
103+
}
104+
}
105+
106+
private void processAggregation(Aggregation agg)
107+
{
108+
if (agg instanceof Terms) {
109+
processTermAggregation((Terms) agg);
110+
}
111+
if (agg instanceof Range) {
112+
processRangeAggregation((Range) agg);
113+
}
114+
if (agg instanceof ExtendedStats) {
115+
processExtendedStatsAggregation((ExtendedStats) agg);
116+
}
117+
if (agg instanceof Histogram) {
118+
processHistogramAggregation((Histogram) agg);
119+
}
120+
}
121+
122+
private void processTermAggregation(Terms agg)
123+
{
124+
List<Term> terms = new ArrayList<>();
125+
for (Terms.Bucket t : agg.getBuckets()) {
126+
terms.add(new Term(t.getKeyAsString(), t.getDocCount()));
127+
}
128+
addFacet(new TermResult(agg.getName(), terms, terms.size(), agg.getSumOfOtherDocCounts(), 0));
129+
}
130+
131+
private void processRangeAggregation(Range agg)
132+
{
133+
List<? extends Range.Bucket> buckets = ((Range) agg).getBuckets();
134+
List<org.springframework.data.elasticsearch.core.facet.result.Range> ranges = new ArrayList<>();
135+
for (Range.Bucket b : buckets) {
136+
ExtendedStats rStats = b.getAggregations().get(AbstractFacetRequest.INTERNAL_STATS);
137+
if (rStats != null) {
138+
Sum sum = b.getAggregations().get(RangeFacetRequest.RANGE_INTERNAL_SUM);
139+
ranges.add(new org.springframework.data.elasticsearch.core.facet.result.Range((Double) b.getFrom(), (Double) b.getTo(), b.getDocCount(), sum != null ? sum.getValue() : rStats.getSum(), rStats.getCount(), rStats.getMin(), rStats.getMax()));
140+
} else {
141+
ranges.add(new org.springframework.data.elasticsearch.core.facet.result.Range((Double) b.getFrom(), (Double) b.getTo(), b.getDocCount(), 0, 0, 0, 0));
142+
}
143+
}
144+
addFacet(new RangeResult(agg.getName(), ranges));
145+
}
146+
147+
private void processExtendedStatsAggregation(ExtendedStats agg)
148+
{
149+
addFacet(new StatisticalResult(agg.getName(), agg.getCount(), agg.getMax(), agg.getMin(), agg.getAvg(), agg.getStdDeviation(), agg.getSumOfSquares(), agg.getSum(), agg.getVariance()));
150+
}
151+
152+
private void processHistogramAggregation(Histogram agg)
153+
{
154+
List<IntervalUnit> intervals = new ArrayList<>();
155+
for (Histogram.Bucket h : agg.getBuckets()) {
156+
ExtendedStats hStats = h.getAggregations().get(AbstractFacetRequest.INTERNAL_STATS);
157+
if (hStats != null) {
158+
intervals.add(new IntervalUnit(((DateTime) h.getKey()).getMillis(), h.getDocCount(), h.getDocCount(), hStats.getSum(), hStats.getAvg(), hStats.getMin(), hStats.getMax()));
159+
} else {
160+
intervals.add(new IntervalUnit(((DateTime) h.getKey()).getMillis(), h.getDocCount(), h.getDocCount(), 0, 0, 0, 0));
127161
}
128162
}
163+
addFacet(new HistogramResult(agg.getName(), intervals));
129164
}
130165
}

‎src/test/java/org/springframework/data/elasticsearch/core/facet/ElasticsearchTemplateFacetTests.java

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,26 +16,33 @@
1616

1717
package org.springframework.data.elasticsearch.core.facet;
1818

19-
import static org.elasticsearch.index.query.QueryBuilders.*;
20-
import static org.hamcrest.Matchers.*;
21-
import static org.junit.Assert.*;
22-
23-
import org.elasticsearch.index.query.QueryBuilder;
24-
import org.elasticsearch.index.query.QueryBuilders;
2519
import org.junit.Before;
2620
import org.junit.Test;
2721
import org.junit.runner.RunWith;
2822
import org.springframework.beans.factory.annotation.Autowired;
2923
import org.springframework.data.elasticsearch.core.ElasticsearchTemplate;
3024
import org.springframework.data.elasticsearch.core.FacetedPage;
31-
import org.springframework.data.elasticsearch.core.facet.request.*;
32-
import org.springframework.data.elasticsearch.core.facet.result.*;
33-
import org.springframework.data.elasticsearch.core.query.DeleteQuery;
25+
import org.springframework.data.elasticsearch.core.aggregation.AggregatedPage;
26+
import org.springframework.data.elasticsearch.core.facet.request.HistogramFacetRequestBuilder;
27+
import org.springframework.data.elasticsearch.core.facet.request.NativeFacetRequest;
28+
import org.springframework.data.elasticsearch.core.facet.request.RangeFacetRequestBuilder;
29+
import org.springframework.data.elasticsearch.core.facet.request.StatisticalFacetRequestBuilder;
30+
import org.springframework.data.elasticsearch.core.facet.request.TermFacetRequestBuilder;
31+
import org.springframework.data.elasticsearch.core.facet.result.HistogramResult;
32+
import org.springframework.data.elasticsearch.core.facet.result.IntervalUnit;
33+
import org.springframework.data.elasticsearch.core.facet.result.Range;
34+
import org.springframework.data.elasticsearch.core.facet.result.RangeResult;
35+
import org.springframework.data.elasticsearch.core.facet.result.StatisticalResult;
36+
import org.springframework.data.elasticsearch.core.facet.result.Term;
37+
import org.springframework.data.elasticsearch.core.facet.result.TermResult;
3438
import org.springframework.data.elasticsearch.core.query.IndexQuery;
3539
import org.springframework.data.elasticsearch.core.query.NativeSearchQueryBuilder;
3640
import org.springframework.data.elasticsearch.core.query.SearchQuery;
3741
import org.springframework.test.context.ContextConfiguration;
3842
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
43+
import static org.elasticsearch.index.query.QueryBuilders.*;
44+
import static org.hamcrest.Matchers.*;
45+
import static org.junit.Assert.*;
3946

4047
/**
4148
* @author Rizwan Idrees
@@ -48,14 +55,14 @@
4855
@ContextConfiguration("classpath:elasticsearch-template-test.xml")
4956
public class ElasticsearchTemplateFacetTests {
5057

51-
public static final String RIZWAN_IDREES = "Rizwan Idrees";
52-
public static final String MOHSIN_HUSEN = "Mohsin Husen";
53-
public static final String JONATHAN_YAN = "Jonathan Yan";
54-
public static final String ARTUR_KONCZAK = "Artur Konczak";
55-
public static final int YEAR_2002 = 2002;
56-
public static final int YEAR_2001 = 2001;
57-
public static final int YEAR_2000 = 2000;
58-
public static final String PUBLISHED_YEARS = "publishedYears";
58+
private static final String RIZWAN_IDREES = "Rizwan Idrees";
59+
private static final String MOHSIN_HUSEN = "Mohsin Husen";
60+
private static final String JONATHAN_YAN = "Jonathan Yan";
61+
private static final String ARTUR_KONCZAK = "Artur Konczak";
62+
private static final int YEAR_2002 = 2002;
63+
private static final int YEAR_2001 = 2001;
64+
private static final int YEAR_2000 = 2000;
65+
private static final String PUBLISHED_YEARS = "publishedYears";
5966
@Autowired
6067
private ElasticsearchTemplate elasticsearchTemplate;
6168

@@ -598,5 +605,14 @@ public void shouldReturnHistogramFacetForGivenQuery() {
598605
assertThat(unit.getKey(), is(Long.valueOf(YEAR_2002)));
599606
assertThat(unit.getCount(), is(1L));
600607
}
608+
609+
@Test
610+
public void shouldNotThrowExceptionForNoFacets()
611+
{
612+
SearchQuery searchQuery = new NativeSearchQueryBuilder().withQuery(matchAllQuery()).build();
613+
AggregatedPage<ArticleEntity> result = elasticsearchTemplate.queryForPage(searchQuery, ArticleEntity.class);
614+
615+
assertThat(result.hasFacets(), is(false));
616+
}
601617
}
602618

0 commit comments

Comments
 (0)