diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/pivot/PivotConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/pivot/PivotConfig.java index 9d040c1180b..d681d5bf04d 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/pivot/PivotConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/pivot/PivotConfig.java @@ -42,32 +42,30 @@ public class PivotConfig implements Writeable, ToXContentObject { private static final ConstructingObjectParser LENIENT_PARSER = createParser(true); private static ConstructingObjectParser createParser(boolean lenient) { - ConstructingObjectParser parser = new ConstructingObjectParser<>(NAME, lenient, - args -> { - GroupConfig groups = (GroupConfig) args[0]; + ConstructingObjectParser parser = new ConstructingObjectParser<>(NAME, lenient, args -> { + GroupConfig groups = (GroupConfig) args[0]; - // allow "aggs" and "aggregations" but require one to be specified - // if somebody specifies both: throw - AggregationConfig aggregationConfig = null; - if (args[1] != null) { - aggregationConfig = (AggregationConfig) args[1]; - } + // allow "aggs" and "aggregations" but require one to be specified + // if somebody specifies both: throw + AggregationConfig aggregationConfig = null; + if (args[1] != null) { + aggregationConfig = (AggregationConfig) args[1]; + } - if (args[2] != null) { - if (aggregationConfig != null) { - throw new IllegalArgumentException("Found two aggregation definitions: [aggs] and [aggregations]"); - } - aggregationConfig = (AggregationConfig) args[2]; - } - if (aggregationConfig == null) { - throw new IllegalArgumentException("Required [aggregations]"); - } + if (args[2] != null) { + if (aggregationConfig != null) { + throw new IllegalArgumentException("Found two aggregation definitions: [aggs] and [aggregations]"); + } + aggregationConfig = (AggregationConfig) args[2]; + } + if (aggregationConfig == null) { + throw new IllegalArgumentException("Required [aggregations]"); + } - return new PivotConfig(groups, aggregationConfig, (Integer)args[3]); - }); + return new PivotConfig(groups, aggregationConfig, (Integer) args[3]); + }); - parser.declareObject(constructorArg(), - (p, c) -> (GroupConfig.fromXContent(p, lenient)), TransformField.GROUP_BY); + parser.declareObject(constructorArg(), (p, c) -> (GroupConfig.fromXContent(p, lenient)), TransformField.GROUP_BY); parser.declareObject(optionalConstructorArg(), (p, c) -> AggregationConfig.fromXContent(p, lenient), TransformField.AGGREGATIONS); parser.declareObject(optionalConstructorArg(), (p, c) -> AggregationConfig.fromXContent(p, lenient), TransformField.AGGS); @@ -206,17 +204,26 @@ public class PivotConfig implements Writeable, ToXContentObject { usedNames.sort(String::compareTo); for (int i = 0; i < usedNames.size() - 1; i++) { - if (usedNames.get(i+1).startsWith(usedNames.get(i) + ".")) { + if (usedNames.get(i + 1).startsWith(usedNames.get(i) + ".")) { validationFailures.add("field [" + usedNames.get(i) + "] cannot be both an object and a field"); } - if (usedNames.get(i+1).equals(usedNames.get(i))) { + if (usedNames.get(i + 1).equals(usedNames.get(i))) { validationFailures.add("duplicate field [" + usedNames.get(i) + "] detected"); } } + + for (String name : usedNames) { + if (name.startsWith(".")) { + validationFailures.add("field [" + name + "] must not start with '.'"); + } + if (name.endsWith(".")) { + validationFailures.add("field [" + name + "] must not end with '.'"); + } + } + return validationFailures; } - private static void addAggNames(AggregationBuilder aggregationBuilder, Collection names) { names.add(aggregationBuilder.getName()); aggregationBuilder.getSubAggregations().forEach(agg -> addAggNames(agg, names)); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/transform/transforms/pivot/PivotConfigTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/transform/transforms/pivot/PivotConfigTests.java index 30c310fe3e4..e9bb572f33d 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/transform/transforms/pivot/PivotConfigTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/transform/transforms/pivot/PivotConfigTests.java @@ -24,15 +24,19 @@ import static org.hamcrest.Matchers.empty; public class PivotConfigTests extends AbstractSerializingTransformTestCase { public static PivotConfig randomPivotConfig() { - return new PivotConfig(GroupConfigTests.randomGroupConfig(), + return new PivotConfig( + GroupConfigTests.randomGroupConfig(), AggregationConfigTests.randomAggregationConfig(), - randomBoolean() ? null : randomIntBetween(10, 10_000)); + randomBoolean() ? null : randomIntBetween(10, 10_000) + ); } public static PivotConfig randomInvalidPivotConfig() { - return new PivotConfig(GroupConfigTests.randomGroupConfig(), + return new PivotConfig( + GroupConfigTests.randomGroupConfig(), AggregationConfigTests.randomInvalidAggregationConfig(), - randomBoolean() ? null : randomIntBetween(10, 10_000)); + randomBoolean() ? null : randomIntBetween(10, 10_000) + ); } @Override @@ -52,44 +56,39 @@ public class PivotConfigTests extends AbstractSerializingTransformTestCase createPivotConfigFromString(pivot, false)); } public void testEmptyAggs() throws IOException { String pivot = "{" - + " \"group_by\": {" - + " \"id\": {" - + " \"terms\": {" - + " \"field\": \"id\"" - + "} } }," - + "\"aggs\": {}" - + " }"; + + " \"group_by\": {" + + " \"id\": {" + + " \"terms\": {" + + " \"field\": \"id\"" + + "} } }," + + "\"aggs\": {}" + + " }"; expectThrows(IllegalArgumentException.class, () -> createPivotConfigFromString(pivot, false)); @@ -100,12 +99,12 @@ public class PivotConfigTests extends AbstractSerializingTransformTestCase createPivotConfigFromString(pivot, false)); @@ -115,34 +114,29 @@ public class PivotConfigTests extends AbstractSerializingTransformTestCase createPivotConfigFromString(pivot, false)); } public void testDoubleAggs() { String pivot = "{" - + " \"group_by\": {" - + " \"id\": {" - + " \"terms\": {" - + " \"field\": \"id\"" - + "} } }," - + " \"aggs\": {" - + " \"avg\": {" - + " \"avg\": {" - + " \"field\": \"points\"" - + "} } }," - + " \"aggregations\": {" - + " \"avg\": {" - + " \"avg\": {" - + " \"field\": \"points\"" - + "} } }" - + "}"; + + " \"group_by\": {" + + " \"id\": {" + + " \"terms\": {" + + " \"field\": \"id\"" + + "} } }," + + " \"aggs\": {" + + " \"avg\": {" + + " \"avg\": {" + + " \"field\": \"points\"" + + "} } }," + + " \"aggregations\": {" + + " \"avg\": {" + + " \"avg\": {" + + " \"field\": \"points\"" + + "} } }" + + "}"; expectThrows(IllegalArgumentException.class, () -> createPivotConfigFromString(pivot, false)); } @@ -171,17 +165,17 @@ public class PivotConfigTests extends AbstractSerializingTransformTestCase failures = PivotConfig.aggFieldValidation(Arrays.asList(".at_start", "at_end.", ".start_and_end.")); + + assertThat( + failures, + containsInAnyOrder( + "field [.at_start] must not start with '.'", + "field [at_end.] must not end with '.'", + "field [.start_and_end.] must not start with '.'", + "field [.start_and_end.] must not end with '.'" + ) + ); } private static String dotJoin(String... fields) { @@ -210,8 +224,8 @@ public class PivotConfigTests extends AbstractSerializingTransformTestCase