honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-02-26 06:46:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

[Transform] disallow field and script being empty for group sources (#63313)

Browse Source 
fail validation earlier when field and script are both missing in a group source
This commit is contained in:
Hendrik Muhs 2020-10-06 16:57:33 +02:00
parent 09f7cff612
commit 058c55da6a
10 changed files with 104 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
client/rest-high-level/src/test/java/org/elasticsearch/client/transform/transforms/pivot/DateHistogramGroupSourceTests.java
View File

@ -38,7 +38,7 @@ public class DateHistogramGroupSourceTests extends AbstractXContentTestCase<Date
}
public static DateHistogramGroupSource randomDateHistogramGroupSource() {
String field = randomAlphaOfLengthBetween(1, 20);
String field = randomBoolean() ? randomAlphaOfLengthBetween(1, 20) : null;
Script script = randomBoolean() ? new Script(randomAlphaOfLengthBetween(1, 10)) : null;
return new DateHistogramGroupSource(

2
client/rest-high-level/src/test/java/org/elasticsearch/client/transform/transforms/pivot/HistogramGroupSourceTests.java
View File

@ -29,7 +29,7 @@ import java.util.function.Predicate;
public class HistogramGroupSourceTests extends AbstractXContentTestCase<HistogramGroupSource> {
public static HistogramGroupSource randomHistogramGroupSource() {
String field = randomAlphaOfLengthBetween(1, 20);
String field = randomBoolean() ? randomAlphaOfLengthBetween(1, 20) : null;
Script script = randomBoolean() ? new Script(randomAlphaOfLengthBetween(1, 10)) : null;
boolean missingBucket = randomBoolean();
double interval = randomDoubleBetween(Math.nextUp(0), Double.MAX_VALUE, false);

3
client/rest-high-level/src/test/java/org/elasticsearch/client/transform/transforms/pivot/TermsGroupSourceTests.java
View File

@ -29,8 +29,9 @@ import java.util.function.Predicate;
public class TermsGroupSourceTests extends AbstractXContentTestCase<TermsGroupSource> {
public static TermsGroupSource randomTermsGroupSource() {
String field = randomBoolean() ? randomAlphaOfLengthBetween(1, 20) : null;
Script script = randomBoolean() ? new Script(randomAlphaOfLengthBetween(1, 10)) : null;
return new TermsGroupSource(randomAlphaOfLengthBetween(1, 20), script, randomBoolean());
return new TermsGroupSource(field, script, randomBoolean());
}
@Override

69
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/pivot/GroupConfig.java
View File

@ -54,26 +54,26 @@ public class GroupConfig implements Writeable, ToXContentObject {
groups = in.readMap(StreamInput::readString, (stream) -> {
SingleGroupSource.Type groupType = SingleGroupSource.Type.fromId(stream.readByte());
switch (groupType) {
case TERMS:
return new TermsGroupSource(stream);
case HISTOGRAM:
return new HistogramGroupSource(stream);
case DATE_HISTOGRAM:
return new DateHistogramGroupSource(stream);
case GEOTILE_GRID:
return new GeoTileGroupSource(stream);
default:
throw new IOException("Unknown group type");
case TERMS:
return new TermsGroupSource(stream);
case HISTOGRAM:
return new HistogramGroupSource(stream);
case DATE_HISTOGRAM:
return new DateHistogramGroupSource(stream);
case GEOTILE_GRID:
return new GeoTileGroupSource(stream);
default:
throw new IOException("Unknown group type");
}
});
}
public Map <String, SingleGroupSource> getGroups() {
public Map<String, SingleGroupSource> getGroups() {
return groups;
}
public boolean isValid() {
return this.groups != null;
return this.groups != null && this.groups.values().stream().allMatch(SingleGroupSource::isValid);
}
@Override
@ -122,9 +122,11 @@ public class GroupConfig implements Writeable, ToXContentObject {
throw new IllegalArgumentException(TransformMessages.TRANSFORM_CONFIGURATION_PIVOT_NO_GROUP_BY);
}
} else {
try (XContentBuilder xContentBuilder = XContentFactory.jsonBuilder().map(source);
XContentParser sourceParser = XContentType.JSON.xContent().createParser(registry, LoggingDeprecationHandler.INSTANCE,
BytesReference.bytes(xContentBuilder).streamInput())) {
try (
XContentBuilder xContentBuilder = XContentFactory.jsonBuilder().map(source);
XContentParser sourceParser = XContentType.JSON.xContent()
.createParser(registry, LoggingDeprecationHandler.INSTANCE, BytesReference.bytes(xContentBuilder).streamInput())
) {
groups = parseGroupConfig(sourceParser, lenient);
} catch (Exception e) {
if (lenient) {
@ -137,8 +139,7 @@ public class GroupConfig implements Writeable, ToXContentObject {
return new GroupConfig(source, groups);
}
private static Map<String, SingleGroupSource> parseGroupConfig(final XContentParser parser,
boolean lenient) throws IOException {
private static Map<String, SingleGroupSource> parseGroupConfig(final XContentParser parser, boolean lenient) throws IOException {
Matcher validAggMatcher = AggregatorFactories.VALID_AGG_NAME.matcher("");
LinkedHashMap<String, SingleGroupSource> groups = new LinkedHashMap<>();
@ -156,8 +157,10 @@ public class GroupConfig implements Writeable, ToXContentObject {
ensureExpectedToken(XContentParser.Token.FIELD_NAME, token, parser);
String destinationFieldName = parser.currentName();
if (validAggMatcher.reset(destinationFieldName).matches() == false) {
throw new ParsingException(parser.getTokenLocation(), "Invalid group name [" + destinationFieldName
+ "]. Group names can contain any character except '[', ']', and '>'");
throw new ParsingException(
parser.getTokenLocation(),
"Invalid group name [" + destinationFieldName + "]. Group names can contain any character except '[', ']', and '>'"
);
}
token = parser.nextToken();
@ -170,20 +173,20 @@ public class GroupConfig implements Writeable, ToXContentObject {
ensureExpectedToken(XContentParser.Token.START_OBJECT, token, parser);
SingleGroupSource groupSource;
switch (groupType) {
case TERMS:
groupSource = TermsGroupSource.fromXContent(parser, lenient);
break;
case HISTOGRAM:
groupSource = HistogramGroupSource.fromXContent(parser, lenient);
break;
case DATE_HISTOGRAM:
groupSource = DateHistogramGroupSource.fromXContent(parser, lenient);
break;
case GEOTILE_GRID:
groupSource = GeoTileGroupSource.fromXContent(parser, lenient);
break;
default:
throw new ParsingException(parser.getTokenLocation(), "invalid grouping type: " + groupType);
case TERMS:
groupSource = TermsGroupSource.fromXContent(parser, lenient);
break;
case HISTOGRAM:
groupSource = HistogramGroupSource.fromXContent(parser, lenient);
break;
case DATE_HISTOGRAM:
groupSource = DateHistogramGroupSource.fromXContent(parser, lenient);
break;
case GEOTILE_GRID:
groupSource = GeoTileGroupSource.fromXContent(parser, lenient);
break;
default:
throw new ParsingException(parser.getTokenLocation(), "invalid grouping type: " + groupType);
}
parser.nextToken();

9
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/pivot/SingleGroupSource.java
View File

@ -76,6 +76,10 @@ public abstract class SingleGroupSource implements Writeable, ToXContentObject {
parser.declareString(optionalConstructorArg(), FIELD);
parser.declareObject(optionalConstructorArg(), (p, c) -> ScriptConfig.fromXContent(p, lenient), SCRIPT);
parser.declareBoolean(optionalConstructorArg(), MISSING_BUCKET);
if (lenient == false) {
// either a script or a field must be declared, or both
parser.declareRequiredFieldSet(FIELD.getPreferredName(), SCRIPT.getPreferredName());
}
}
public SingleGroupSource(final String field, final ScriptConfig scriptConfig, final boolean missingBucket) {
@ -98,6 +102,11 @@ public abstract class SingleGroupSource implements Writeable, ToXContentObject {
}
}
boolean isValid() {
// either a script or a field must be declared
return field != null || scriptConfig != null;
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();

14
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/transform/transforms/pivot/DateHistogramGroupSourceTests.java
View File

@ -30,10 +30,16 @@ public class DateHistogramGroupSourceTests extends AbstractSerializingTestCase<D
}
public static DateHistogramGroupSource randomDateHistogramGroupSource(Version version) {
String field = randomBoolean() ? null : randomAlphaOfLengthBetween(1, 20);
ScriptConfig scriptConfig = version.onOrAfter(Version.V_7_7_0)
? randomBoolean() ? null : ScriptConfigTests.randomScriptConfig()
: null;
ScriptConfig scriptConfig = null;
String field;
// either a field or a script must be specified, it's possible to have both, but disallowed to have none
if (version.onOrAfter(Version.V_7_7_0) && randomBoolean()) {
scriptConfig = ScriptConfigTests.randomScriptConfig();
field = randomBoolean() ? null : randomAlphaOfLengthBetween(1, 20);
} else {
field = randomAlphaOfLengthBetween(1, 20);
}
boolean missingBucket = version.onOrAfter(Version.V_7_10_0) ? randomBoolean() : false;
DateHistogramGroupSource dateHistogramGroupSource;

2
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/transform/transforms/pivot/GeoTileGroupSourceTests.java
View File

@ -28,7 +28,7 @@ public class GeoTileGroupSourceTests extends AbstractSerializingTestCase<GeoTile
Rectangle rectangle = GeometryTestUtils.randomRectangle();
boolean missingBucket = version.onOrAfter(Version.V_7_10_0) ? randomBoolean() : false;
return new GeoTileGroupSource(
randomBoolean() ? null : randomAlphaOfLength(10),
randomAlphaOfLength(10),
missingBucket,
randomBoolean() ? null : randomIntBetween(1, GeoTileUtils.MAX_ZOOM),
randomBoolean()

22
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/transform/transforms/pivot/GroupConfigTests.java
View File

@ -130,6 +130,28 @@ public class GroupConfigTests extends AbstractSerializingTestCase<GroupConfig> {
}
}
public void testInvalidGroupSourceValidation() throws IOException {
XContentBuilder source = JsonXContent.contentBuilder()
.startObject()
.startObject(randomAlphaOfLengthBetween(1, 20))
.startObject("terms")
.endObject()
.endObject()
.endObject();
// lenient, passes but reports invalid
try (XContentParser parser = createParser(source)) {
GroupConfig groupConfig = GroupConfig.fromXContent(parser, true);
assertFalse(groupConfig.isValid());
}
// strict throws
try (XContentParser parser = createParser(source)) {
Exception e = expectThrows(IllegalArgumentException.class, () -> GroupConfig.fromXContent(parser, false));
assertTrue(e.getMessage().startsWith("Required one of fields [field, script], but none were specified."));
}
}
private static Map<String, Object> getSource(SingleGroupSource groupSource) {
try (XContentBuilder xContentBuilder = XContentFactory.jsonBuilder()) {
XContentBuilder content = groupSource.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS);

15
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/transform/transforms/pivot/HistogramGroupSourceTests.java
View File

@ -20,10 +20,17 @@ public class HistogramGroupSourceTests extends AbstractSerializingTestCase<Histo
}
public static HistogramGroupSource randomHistogramGroupSource(Version version) {
String field = randomBoolean() ? null : randomAlphaOfLengthBetween(1, 20);
ScriptConfig scriptConfig = version.onOrAfter(Version.V_7_7_0)
? randomBoolean() ? null : ScriptConfigTests.randomScriptConfig()
: null;
ScriptConfig scriptConfig = null;
String field;
// either a field or a script must be specified, it's possible to have both, but disallowed to have none
if (version.onOrAfter(Version.V_7_7_0) && randomBoolean()) {
scriptConfig = ScriptConfigTests.randomScriptConfig();
field = randomBoolean() ? null : randomAlphaOfLengthBetween(1, 20);
} else {
field = randomAlphaOfLengthBetween(1, 20);
}
boolean missingBucket = version.onOrAfter(Version.V_7_10_0) ? randomBoolean() : false;
double interval = randomDoubleBetween(Math.nextUp(0), Double.MAX_VALUE, false);
return new HistogramGroupSource(field, scriptConfig, missingBucket, interval);

15
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/transform/transforms/pivot/TermsGroupSourceTests.java
View File

@ -20,10 +20,17 @@ public class TermsGroupSourceTests extends AbstractSerializingTestCase<TermsGrou
}
public static TermsGroupSource randomTermsGroupSource(Version version) {
String field = randomBoolean() ? null : randomAlphaOfLengthBetween(1, 20);
ScriptConfig scriptConfig = version.onOrAfter(Version.V_7_7_0)
? randomBoolean() ? null : ScriptConfigTests.randomScriptConfig()
: null;
ScriptConfig scriptConfig = null;
String field;
// either a field or a script must be specified, it's possible to have both, but disallowed to have none
if (version.onOrAfter(Version.V_7_7_0) && randomBoolean()) {
scriptConfig = ScriptConfigTests.randomScriptConfig();
field = randomBoolean() ? null : randomAlphaOfLengthBetween(1, 20);
} else {
field = randomAlphaOfLengthBetween(1, 20);
}
boolean missingBucket = version.onOrAfter(Version.V_7_10_0) ? randomBoolean() : false;
return new TermsGroupSource(field, scriptConfig, missingBucket);
}