Skip to content

Commit b357c1b

Browse files
authored
[7.x] Fix NPE when building exception messages for aggregations (#59156) (#59334)
1 parent cf75299 commit b357c1b

File tree

2 files changed

+72
-6
lines changed

2 files changed

+72
-6
lines changed

server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
*/
1919
package org.elasticsearch.search.aggregations.support;
2020

21-
import org.elasticsearch.index.mapper.MappedFieldType;
2221
import org.elasticsearch.index.query.QueryShardContext;
2322
import org.elasticsearch.search.SearchModule;
2423
import org.elasticsearch.search.aggregations.AggregationExecutionException;
@@ -123,11 +122,10 @@ public AggregatorSupplier getAggregator(ValuesSourceConfig valuesSourceConfig, S
123122
aggregatorRegistry.get(aggregationName)
124123
);
125124
if (supplier == null) {
126-
// TODO: push building the description into ValuesSourceConfig
127-
MappedFieldType fieldType = valuesSourceConfig.fieldContext().fieldType();
128-
String fieldDescription = fieldType.typeName();
129-
throw new IllegalArgumentException("Field [" + fieldType.name() + "] of type [" + fieldDescription +
130-
"] is not supported for aggregation [" + aggregationName + "]"); }
125+
throw new IllegalArgumentException(
126+
valuesSourceConfig.getDescription() + " is not supported for aggregation [" + aggregationName + "]"
127+
);
128+
}
131129
return supplier;
132130
}
133131
throw new AggregationExecutionException("Unregistered Aggregation [" + aggregationName + "]");
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.search.aggregations.support;
21+
22+
import org.elasticsearch.index.query.QueryShardContext;
23+
import org.elasticsearch.script.AggregationScript;
24+
import org.elasticsearch.test.ESTestCase;
25+
import org.mockito.Mockito;
26+
27+
import java.util.Collections;
28+
29+
import static org.mockito.Mockito.mock;
30+
import static org.mockito.Mockito.when;
31+
32+
public class ValuesSourceRegistryTests extends ESTestCase {
33+
34+
public void testAggregatorNotFoundException() {
35+
final QueryShardContext queryShardContext = mock(QueryShardContext.class);
36+
final AggregationScript.Factory mockAggScriptFactory = mock(AggregationScript.Factory.class);
37+
when(mockAggScriptFactory.newFactory(Mockito.any(), Mockito.any())).thenReturn(mock(AggregationScript.LeafFactory.class));
38+
when(queryShardContext.compile(Mockito.any(), Mockito.any())).thenReturn(mockAggScriptFactory);
39+
40+
ValuesSourceConfig fieldOnly = ValuesSourceConfig.resolve(
41+
queryShardContext,
42+
null,
43+
"field",
44+
null,
45+
null,
46+
null,
47+
null,
48+
CoreValuesSourceType.BYTES
49+
);
50+
51+
ValuesSourceConfig scriptOnly = ValuesSourceConfig.resolve(
52+
queryShardContext,
53+
null,
54+
null,
55+
mockScript("fakeScript"),
56+
null,
57+
null,
58+
null,
59+
CoreValuesSourceType.BYTES
60+
);
61+
ValuesSourceRegistry registry = new ValuesSourceRegistry(
62+
Collections.singletonMap("bogus", Collections.emptyList()),
63+
null);
64+
expectThrows(IllegalArgumentException.class, () -> registry.getAggregator(fieldOnly, "bogus"));
65+
expectThrows(IllegalArgumentException.class, () -> registry.getAggregator(scriptOnly, "bogus"));
66+
}
67+
68+
}

0 commit comments

Comments
 (0)