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

[ML] Forbid 'by' and 'over' as fieldnames for by/over (elastic/x-pack-elasticsearch#1007) 

relates elastic/x-pack-elasticsearch#1002

Original commit: elastic/x-pack-elasticsearch@ba9005a58e
This commit is contained in:
Zachary Tong 2017-04-07 16:09:22 +00:00 committed by GitHub
parent bf111dde7e
commit 8e2e26fc44
5 changed files with 104 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
plugin/src/main/java/org/elasticsearch/xpack/ml/job/config/Detector.java
View File

@ -137,6 +137,8 @@ public class Detector extends ToXContentToBytes implements Writeable {
public static final String NON_NULL_SUM = "non_null_sum";
public static final String LOW_NON_NULL_SUM = "low_non_null_sum";
public static final String HIGH_NON_NULL_SUM = "high_non_null_sum";
public static final String BY = "by";
public static final String OVER = "over";
/**
* Population variance is called varp to match Splunk
*/
@ -674,7 +676,7 @@ public class Detector extends ToXContentToBytes implements Writeable {
byFieldName));
}
// by/over field names cannot be "count" - this requirement dates back to the early
// by/over field names cannot be "count", "over', "by" - this requirement dates back to the early
// days of the Splunk app and could be removed now BUT ONLY IF THE C++ CODE IS CHANGED
// FIRST - see https://github.com/elastic/x-pack-elasticsearch/issues/858
if (COUNT.equals(byFieldName)) {
@ -686,6 +688,24 @@ public class Detector extends ToXContentToBytes implements Writeable {
OVER_FIELD_NAME_FIELD.getPreferredName()));
}
if (BY.equals(byFieldName)) {
throw new IllegalArgumentException(Messages.getMessage(Messages.JOB_CONFIG_DETECTOR_BY_DISALLOWED,
BY_FIELD_NAME_FIELD.getPreferredName()));
}
if (BY.equals(overFieldName)) {
throw new IllegalArgumentException(Messages.getMessage(Messages.JOB_CONFIG_DETECTOR_BY_DISALLOWED,
OVER_FIELD_NAME_FIELD.getPreferredName()));
}
if (OVER.equals(byFieldName)) {
throw new IllegalArgumentException(Messages.getMessage(Messages.JOB_CONFIG_DETECTOR_OVER_DISALLOWED,
BY_FIELD_NAME_FIELD.getPreferredName()));
}
if (OVER.equals(overFieldName)) {
throw new IllegalArgumentException(Messages.getMessage(Messages.JOB_CONFIG_DETECTOR_OVER_DISALLOWED,
OVER_FIELD_NAME_FIELD.getPreferredName()));
}
return new Detector(detectorDescription, function, fieldName, byFieldName, overFieldName, partitionFieldName,
useNull, excludeFrequent, detectorRules);
}

4
plugin/src/main/java/org/elasticsearch/xpack/ml/job/messages/Messages.java
View File

@ -123,6 +123,10 @@ public final class Messages {
"{0} and {1} cannot be the same: ''{2}''";
public static final String JOB_CONFIG_DETECTOR_COUNT_DISALLOWED =
"''count'' is not a permitted value for {0}";
public static final String JOB_CONFIG_DETECTOR_BY_DISALLOWED =
"''by'' is not a permitted value for {0}";
public static final String JOB_CONFIG_DETECTOR_OVER_DISALLOWED =
"''over'' is not a permitted value for {0}";
public static final String JOB_CONFIG_MAPPING_TYPE_CLASH =
"A field has a different mapping type to an existing field with the same name. " +
"Use the 'results_index_name' setting to assign the job to another index";

8
plugin/src/test/java/org/elasticsearch/xpack/ml/job/config/AnalysisConfigTests.java
View File

@ -434,20 +434,20 @@ public class AnalysisConfigTests extends AbstractSerializingTestCase<AnalysisCon
// should work now
Detector.Builder builder = new Detector.Builder("distinct_count", "somefield");
builder.setOverFieldName("over");
builder.setOverFieldName("over_field");
new AnalysisConfig.Builder(Collections.singletonList(builder.build())).build();
builder = new Detector.Builder("info_content", "somefield");
builder.setOverFieldName("over");
builder.setOverFieldName("over_field");
d = builder.build();
new AnalysisConfig.Builder(Collections.singletonList(builder.build())).build();
builder.setByFieldName("by");
builder.setByFieldName("by_field");
new AnalysisConfig.Builder(Collections.singletonList(builder.build())).build();
try {
builder = new Detector.Builder("made_up_function", "somefield");
builder.setOverFieldName("over");
builder.setOverFieldName("over_field");
new AnalysisConfig.Builder(Collections.singletonList(builder.build())).build();
assertTrue(false); // shouldn't get here
} catch (IllegalArgumentException e) {

106
plugin/src/test/java/org/elasticsearch/xpack/ml/job/config/DetectorTests.java
View File

@ -24,15 +24,15 @@ public class DetectorTests extends AbstractSerializingTestCase<Detector> {
public void testEquals_GivenEqual() {
Detector.Builder builder = new Detector.Builder("mean", "field");
builder.setByFieldName("by");
builder.setOverFieldName("over");
builder.setByFieldName("by_field");
builder.setOverFieldName("over_field");
builder.setPartitionFieldName("partition");
builder.setUseNull(false);
Detector detector1 = builder.build();
builder = new Detector.Builder("mean", "field");
builder.setByFieldName("by");
builder.setOverFieldName("over");
builder.setByFieldName("by_field");
builder.setOverFieldName("over_field");
builder.setPartitionFieldName("partition");
builder.setUseNull(false);
Detector detector2 = builder.build();
@ -60,7 +60,7 @@ public class DetectorTests extends AbstractSerializingTestCase<Detector> {
Detector.Builder builder = new Detector.Builder(detector2);
builder.setByFieldName("by2");
Condition condition = new Condition(Operator.GT, "5");
DetectionRule rule = new DetectionRule("over", "targetValue", Connective.AND,
DetectionRule rule = new DetectionRule("over_field", "targetValue", Connective.AND,
Collections.singletonList(new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, "by2", "val", condition, null)));
builder.setDetectorRules(Collections.singletonList(rule));
detector2 = builder.build();
@ -82,19 +82,19 @@ public class DetectorTests extends AbstractSerializingTestCase<Detector> {
public void testExtractAnalysisFields() {
Detector detector = createDetector().build();
assertEquals(Arrays.asList("by", "over", "partition"), detector.extractAnalysisFields());
assertEquals(Arrays.asList("by_field", "over_field", "partition"), detector.extractAnalysisFields());
Detector.Builder builder = new Detector.Builder(detector);
builder.setPartitionFieldName(null);
detector = builder.build();
assertEquals(Arrays.asList("by", "over"), detector.extractAnalysisFields());
assertEquals(Arrays.asList("by_field", "over_field"), detector.extractAnalysisFields());
builder = new Detector.Builder(detector);
Condition condition = new Condition(Operator.GT, "5");
DetectionRule rule = new DetectionRule("over", "targetValue", Connective.AND,
DetectionRule rule = new DetectionRule("over_field", "targetValue", Connective.AND,
Collections.singletonList(new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, null, null, condition, null)));
builder.setDetectorRules(Collections.singletonList(rule));
builder.setByFieldName(null);
detector = builder.build();
assertEquals(Arrays.asList("over"), detector.extractAnalysisFields());
assertEquals(Arrays.asList("over_field"), detector.extractAnalysisFields());
builder = new Detector.Builder(detector);
rule = new DetectionRule(null, null, Connective.AND,
Collections.singletonList(new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, null, null, condition, null)));
@ -107,8 +107,8 @@ public class DetectorTests extends AbstractSerializingTestCase<Detector> {
public void testExtractReferencedLists() {
Detector.Builder builder = createDetector();
builder.setDetectorRules(Arrays.asList(
new DetectionRule(null, null, Connective.OR, Arrays.asList(RuleCondition.createCategorical("by", "list1"))),
new DetectionRule(null, null, Connective.OR, Arrays.asList(RuleCondition.createCategorical("by", "list2")))));
new DetectionRule(null, null, Connective.OR, Arrays.asList(RuleCondition.createCategorical("by_field", "list1"))),
new DetectionRule(null, null, Connective.OR, Arrays.asList(RuleCondition.createCategorical("by_field", "list2")))));
Detector detector = builder.build();
assertEquals(new HashSet<>(Arrays.asList("list1", "list2")), detector.extractReferencedFilters());
@ -116,13 +116,13 @@ public class DetectorTests extends AbstractSerializingTestCase<Detector> {
private Detector.Builder createDetector() {
Detector.Builder detector = new Detector.Builder("mean", "field");
detector.setByFieldName("by");
detector.setOverFieldName("over");
detector.setByFieldName("by_field");
detector.setOverFieldName("over_field");
detector.setPartitionFieldName("partition");
detector.setUseNull(true);
Condition condition = new Condition(Operator.GT, "5");
DetectionRule rule = new DetectionRule("over", "targetValue", Connective.AND,
Collections.singletonList(new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, "by", "val", condition, null)));
DetectionRule rule = new DetectionRule("over_field", "targetValue", Connective.AND,
Collections.singletonList(new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, "by_field", "val", condition, null)));
detector.setDetectorRules(Arrays.asList(rule));
return detector;
}
@ -261,16 +261,16 @@ public class DetectorTests extends AbstractSerializingTestCase<Detector> {
private static Detector.Builder createDetectorWithValidFieldNames() {
Detector.Builder d = new Detector.Builder("metric", "field");
d.setByFieldName("by");
d.setOverFieldName("over");
d.setByFieldName("by_field");
d.setOverFieldName("over_field");
d.setPartitionFieldName("partition");
return d;
}
private static Detector.Builder createDetectorWithSpecificFieldName(String fieldName) {
Detector.Builder d = new Detector.Builder("metric", fieldName);
d.setByFieldName("by");
d.setOverFieldName("over");
d.setByFieldName("by_field");
d.setOverFieldName("over_field");
d.setPartitionFieldName("partition");
return d;
}
@ -330,7 +330,7 @@ public class DetectorTests extends AbstractSerializingTestCase<Detector> {
Detector.LOW_NON_ZERO_COUNT, Detector.LOW_NZC, Detector.HIGH_NON_ZERO_COUNT,
Detector.HIGH_NZC}) {
Detector.Builder builder = new Detector.Builder(f, null);
builder.setOverFieldName("over");
builder.setOverFieldName("over_field");
try {
builder.build();
Assert.fail("IllegalArgumentException not thrown when expected");
@ -352,7 +352,7 @@ public class DetectorTests extends AbstractSerializingTestCase<Detector> {
difference.remove(Detector.TIME_OF_WEEK);
for (String f : difference) {
Detector.Builder builder = new Detector.Builder(f, null);
builder.setOverFieldName("over");
builder.setOverFieldName("over_field");
try {
builder.build();
Assert.fail("IllegalArgumentException not thrown when expected");
@ -369,15 +369,15 @@ public class DetectorTests extends AbstractSerializingTestCase<Detector> {
for (String f : new String[]{Detector.COUNT, Detector.HIGH_COUNT,
Detector.LOW_COUNT}) {
Detector.Builder builder = new Detector.Builder(f, null);
builder.setOverFieldName("over");
builder.setOverFieldName("over_field");
builder.build();
builder.build(true);
}
for (String f : new String[]{Detector.RARE, Detector.FREQ_RARE}) {
Detector.Builder builder = new Detector.Builder(f, null);
builder.setOverFieldName("over");
builder.setByFieldName("by");
builder.setOverFieldName("over_field");
builder.setByFieldName("by_field");
builder.build();
builder.build(true);
}
@ -393,7 +393,7 @@ public class DetectorTests extends AbstractSerializingTestCase<Detector> {
Detector.LOW_NON_NULL_SUM, Detector.HIGH_NON_NULL_SUM, Detector.POPULATION_VARIANCE,
Detector.LOW_POPULATION_VARIANCE, Detector.HIGH_POPULATION_VARIANCE}) {
Detector.Builder builder = new Detector.Builder(f, "f");
builder.setOverFieldName("over");
builder.setOverFieldName("over_field");
builder.build();
try {
builder.build(true);
@ -437,7 +437,7 @@ public class DetectorTests extends AbstractSerializingTestCase<Detector> {
difference.remove(Detector.LAT_LONG);
for (String f : difference) {
Detector.Builder builder = new Detector.Builder(f, "f");
builder.setOverFieldName("over");
builder.setOverFieldName("over_field");
try {
builder.build();
Assert.fail("IllegalArgumentException not thrown when expected");
@ -461,12 +461,12 @@ public class DetectorTests extends AbstractSerializingTestCase<Detector> {
}
Detector.Builder builder = new Detector.Builder(Detector.FREQ_RARE, null);
builder.setOverFieldName("over");
builder.setOverFieldName("over_field");
builder.setByFieldName("b");
builder.build();
builder.build(true);
builder = new Detector.Builder(Detector.FREQ_RARE, null);
builder.setOverFieldName("over");
builder.setOverFieldName("over_field");
builder.setByFieldName("b");
builder.build();
@ -538,7 +538,7 @@ public class DetectorTests extends AbstractSerializingTestCase<Detector> {
builder = new Detector.Builder(Detector.FREQ_RARE, "field");
builder.setByFieldName("b");
builder.setOverFieldName("over");
builder.setOverFieldName("over_field");
try {
builder.build();
Assert.fail("IllegalArgumentException not thrown when expected");
@ -554,7 +554,7 @@ public class DetectorTests extends AbstractSerializingTestCase<Detector> {
for (String f : new String[]{Detector.HIGH_COUNT,
Detector.LOW_COUNT, Detector.NON_ZERO_COUNT, Detector.NZC}) {
builder = new Detector.Builder(f, null);
builder.setByFieldName("by");
builder.setByFieldName("by_field");
builder.build();
builder.build(true);
}
@ -562,7 +562,7 @@ public class DetectorTests extends AbstractSerializingTestCase<Detector> {
for (String f : new String[]{Detector.COUNT, Detector.HIGH_COUNT,
Detector.LOW_COUNT}) {
builder = new Detector.Builder(f, null);
builder.setOverFieldName("over");
builder.setOverFieldName("over_field");
builder.build();
builder.build(true);
}
@ -570,8 +570,8 @@ public class DetectorTests extends AbstractSerializingTestCase<Detector> {
for (String f : new String[]{Detector.HIGH_COUNT,
Detector.LOW_COUNT}) {
builder = new Detector.Builder(f, null);
builder.setByFieldName("by");
builder.setOverFieldName("over");
builder.setByFieldName("by_field");
builder.setOverFieldName("over_field");
builder.build();
builder.build(true);
}
@ -579,16 +579,16 @@ public class DetectorTests extends AbstractSerializingTestCase<Detector> {
for (String f : new String[]{Detector.NON_ZERO_COUNT, Detector.NZC}) {
try {
builder = new Detector.Builder(f, "field");
builder.setByFieldName("by");
builder.setOverFieldName("over");
builder.setByFieldName("by_field");
builder.setOverFieldName("over_field");
builder.build();
Assert.fail("IllegalArgumentException not thrown when expected");
} catch (IllegalArgumentException e) {
}
try {
builder = new Detector.Builder(f, "field");
builder.setByFieldName("by");
builder.setOverFieldName("over");
builder.setByFieldName("by_field");
builder.setOverFieldName("over_field");
builder.build(true);
Assert.fail("IllegalArgumentException not thrown when expected");
} catch (IllegalArgumentException e) {
@ -666,6 +666,38 @@ public class DetectorTests extends AbstractSerializingTestCase<Detector> {
assertEquals("'count' is not a permitted value for over_field_name", e.getMessage());
}
public void testVerify_GivenByIsBy() {
Detector.Builder detector = new Detector.Builder("count", "");
detector.setByFieldName("by");
IllegalArgumentException e = ESTestCase.expectThrows(IllegalArgumentException.class, detector::build);
assertEquals("'by' is not a permitted value for by_field_name", e.getMessage());
}
public void testVerify_GivenOverIsBy() {
Detector.Builder detector = new Detector.Builder("count", "");
detector.setOverFieldName("by");
IllegalArgumentException e = ESTestCase.expectThrows(IllegalArgumentException.class, detector::build);
assertEquals("'by' is not a permitted value for over_field_name", e.getMessage());
}
public void testVerify_GivenByIsOver() {
Detector.Builder detector = new Detector.Builder("count", "");
detector.setByFieldName("over");
IllegalArgumentException e = ESTestCase.expectThrows(IllegalArgumentException.class, detector::build);
assertEquals("'over' is not a permitted value for by_field_name", e.getMessage());
}
public void testVerify_GivenOverIsOver() {
Detector.Builder detector = new Detector.Builder("count", "");
detector.setOverFieldName("over");
IllegalArgumentException e = ESTestCase.expectThrows(IllegalArgumentException.class, detector::build);
assertEquals("'over' is not a permitted value for over_field_name", e.getMessage());
}
public void testExcludeFrequentForString() {
assertEquals(Detector.ExcludeFrequent.ALL, Detector.ExcludeFrequent.forString("all"));
assertEquals(Detector.ExcludeFrequent.ALL, Detector.ExcludeFrequent.forString("ALL"));

12
plugin/src/test/java/org/elasticsearch/xpack/ml/job/config/JobTests.java
View File

@ -161,10 +161,10 @@ public class JobTests extends AbstractSerializingTestCase<Job> {
*/
public void testAnalysisConfigRequiredFields() {
Detector.Builder d1 = new Detector.Builder("max", "field");
d1.setByFieldName("by");
d1.setByFieldName("by_field");
Detector.Builder d2 = new Detector.Builder("metric", "field2");
d2.setOverFieldName("over");
d2.setOverFieldName("over_field");
AnalysisConfig.Builder ac = new AnalysisConfig.Builder(Arrays.asList(d1.build(), d2.build()));
ac.setSummaryCountFieldName("agg");
@ -174,9 +174,9 @@ public class JobTests extends AbstractSerializingTestCase<Job> {
assertTrue(analysisFields.contains("agg"));
assertTrue(analysisFields.contains("field"));
assertTrue(analysisFields.contains("by"));
assertTrue(analysisFields.contains("by_field"));
assertTrue(analysisFields.contains("field2"));
assertTrue(analysisFields.contains("over"));
assertTrue(analysisFields.contains("over_field"));
assertFalse(analysisFields.contains("max"));
assertFalse(analysisFields.contains(""));
@ -193,10 +193,10 @@ public class JobTests extends AbstractSerializingTestCase<Job> {
assertTrue(analysisFields.contains("partition"));
assertTrue(analysisFields.contains("field"));
assertTrue(analysisFields.contains("by"));
assertTrue(analysisFields.contains("by_field"));
assertTrue(analysisFields.contains("by2"));
assertTrue(analysisFields.contains("field2"));
assertTrue(analysisFields.contains("over"));
assertTrue(analysisFields.contains("over_field"));
assertFalse(analysisFields.contains("count"));
assertFalse(analysisFields.contains("max"));