honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Align ParsedAggregation meta to InternalAggregation behaviour 

Empty meta gets printed out, which means that if the request contains an empty meta object, that is returned with the response as well. On the other hand null, meaning when the object is not in the request, is not printed out. ParsedAggregation used to not print out empty metadata, and didn't allow the null value. Aligned behaviour to the existing behaviour from InternalAggregation.
This commit is contained in:
javanna 2017-04-07 21:54:46 +02:00 committed by Luca Cavanna
parent 39e791291e
commit 306ef086c5
2 changed files with 17 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
core/src/main/java/org/elasticsearch/search/aggregations/ParsedAggregation.java
View File

@ -25,7 +25,6 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
import java.io.IOException; import java.io.IOException;
import java.util.Collections; import java.util.Collections;
import java.util.HashMap;
import java.util.Map; import java.util.Map;
/** /**
@ -36,12 +35,12 @@ public abstract class ParsedAggregation implements Aggregation, ToXContent {
//TODO move CommonFields out of InternalAggregation //TODO move CommonFields out of InternalAggregation
protected static void declareCommonFields(ObjectParser<? extends ParsedAggregation, Void> objectParser) { protected static void declareCommonFields(ObjectParser<? extends ParsedAggregation, Void> objectParser) {
objectParser.declareObject((parsedAgg, metadata) -> parsedAgg.metadata.putAll(metadata), objectParser.declareObject((parsedAgg, metadata) -> parsedAgg.metadata = Collections.unmodifiableMap(metadata),
(parser, context) -> parser.map(), InternalAggregation.CommonFields.META); (parser, context) -> parser.map(), InternalAggregation.CommonFields.META);
} }
String name; String name;
final Map<String, Object> metadata = new HashMap<>(); Map<String, Object> metadata;
@Override @Override
public final String getName() { public final String getName() {
@ -50,7 +49,7 @@ public abstract class ParsedAggregation implements Aggregation, ToXContent {
@Override @Override
public final Map<String, Object> getMetaData() { public final Map<String, Object> getMetaData() {
return Collections.unmodifiableMap(metadata); return metadata;
} }
/** /**
@ -67,9 +66,9 @@ public abstract class ParsedAggregation implements Aggregation, ToXContent {
//TODO move TYPED_KEYS_DELIMITER constant out of InternalAggregation //TODO move TYPED_KEYS_DELIMITER constant out of InternalAggregation
// Concatenates the type and the name of the aggregation (ex: top_hits#foo) // Concatenates the type and the name of the aggregation (ex: top_hits#foo)
builder.startObject(String.join(InternalAggregation.TYPED_KEYS_DELIMITER, getType(), name)); builder.startObject(String.join(InternalAggregation.TYPED_KEYS_DELIMITER, getType(), name));
if (metadata.isEmpty() == false) { if (this.metadata != null) {
builder.field(InternalAggregation.CommonFields.META.getPreferredName()); builder.field(InternalAggregation.CommonFields.META.getPreferredName());
builder.map(metadata); builder.map(this.metadata);
} }
doXContentBody(builder, params); doXContentBody(builder, params);
builder.endObject(); builder.endObject();

17
core/src/test/java/org/elasticsearch/search/aggregations/ParsedAggregationTests.java
View File

@ -29,6 +29,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.rest.action.search.RestSearchAction;
import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.rest.FakeRestRequest; import org.elasticsearch.test.rest.FakeRestRequest;
@ -46,15 +47,18 @@ public class ParsedAggregationTests extends ESTestCase {
//TODO maybe this test will no longer be needed once we have real tests for ParsedAggregation subclasses //TODO maybe this test will no longer be needed once we have real tests for ParsedAggregation subclasses
public void testParse() throws IOException { public void testParse() throws IOException {
String name = randomAlphaOfLengthBetween(5, 10); String name = randomAlphaOfLengthBetween(5, 10);
int numMetas = randomIntBetween(0, 5); Map<String, Object> meta = null;
Map<String, Object> meta = new HashMap<>(numMetas); if (randomBoolean()) {
for (int i = 0; i < numMetas; i++) { int numMetas = randomIntBetween(0, 5);
meta.put(randomAlphaOfLengthBetween(3, 10), randomAlphaOfLengthBetween(3, 10)); meta = new HashMap<>(numMetas);
for (int i = 0; i < numMetas; i++) {
meta.put(randomAlphaOfLengthBetween(3, 10), randomAlphaOfLengthBetween(3, 10));
}
} }
TestInternalAggregation testAgg = new TestInternalAggregation(name, meta); TestInternalAggregation testAgg = new TestInternalAggregation(name, meta);
XContentType xContentType = randomFrom(XContentType.values()); XContentType xContentType = randomFrom(XContentType.values());
FakeRestRequest params = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY) FakeRestRequest params = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY)
.withParams(Collections.singletonMap("typed_keys", "true")).build(); .withParams(Collections.singletonMap(RestSearchAction.TYPED_KEYS_PARAM, "true")).build();
BytesReference bytesAgg = XContentHelper.toXContent(testAgg, xContentType, params, randomBoolean()); BytesReference bytesAgg = XContentHelper.toXContent(testAgg, xContentType, params, randomBoolean());
try (XContentParser parser = createParser(xContentType.xContent(), bytesAgg)) { try (XContentParser parser = createParser(xContentType.xContent(), bytesAgg)) {
parser.nextToken(); parser.nextToken();
@ -69,6 +73,9 @@ public class ParsedAggregationTests extends ESTestCase {
assertThat(parsedAgg, instanceOf(TestParsedAggregation.class)); assertThat(parsedAgg, instanceOf(TestParsedAggregation.class));
assertEquals(testAgg.getName(), parsedAgg.getName()); assertEquals(testAgg.getName(), parsedAgg.getName());
assertEquals(testAgg.getMetaData(), parsedAgg.getMetaData()); assertEquals(testAgg.getMetaData(), parsedAgg.getMetaData());
if (meta != null) {
expectThrows(UnsupportedOperationException.class, () -> parsedAgg.getMetaData().put("test", "test"));
}
BytesReference finalAgg = XContentHelper.toXContent((ToXContent) parsedAgg, xContentType, randomBoolean()); BytesReference finalAgg = XContentHelper.toXContent((ToXContent) parsedAgg, xContentType, randomBoolean());
assertToXContentEquivalent(bytesAgg, finalAgg, xContentType); assertToXContentEquivalent(bytesAgg, finalAgg, xContentType);
} }