SOLR-12592: added more validation and tests

This commit is contained in:
Noble Paul 2018-08-14 01:11:01 +10:00
parent 767223ddd3
commit be4a33938d
4 changed files with 51 additions and 20 deletions

View File

@ -253,14 +253,14 @@ public class Clause implements MapWriter, Comparable<Clause> {
new SealedClause(this, computedValueEvaluator);
}
Condition parse(String s, Map m) {
Condition parse(String s, Map<String,Object> m) {
Object expectedVal = null;
ComputedType computedType = null;
Object val = m.get(s);
Type varType = VariableBase.getTagType(s);
if (varType.meta.isHidden()) {
throw new IllegalArgumentException(formatString("''{0}'' is not allowed in a policy rule : ''{1}'' ", varType.tagName, toJSONString(m)));
throwExp(m,"''{0}'' is not allowed", varType.tagName);
}
try {
String conditionName = s.trim();
@ -270,8 +270,7 @@ public class Clause implements MapWriter, Comparable<Clause> {
expectedVal = Policy.ANY;
} else if (val instanceof List) {
if (!varType.meta.supportArrayVals()) {
throw new IllegalArgumentException(formatString("array values are not supported for {0} in clause {1}",
conditionName, toJSONString(m)));
throwExp(m, "array values are not supported for {0}", conditionName);
}
expectedVal = readListVal(m, (List) val, varType, conditionName);
operand = Operand.IN;
@ -286,14 +285,12 @@ public class Clause implements MapWriter, Comparable<Clause> {
computedType = t;
strVal = changedVal;
if (varType == null || !varType.supportedComputedTypes.contains(computedType)) {
throw new IllegalArgumentException(formatString("''{0}'' is not allowed for variable : ''{1}'' , in clause : ''{2}'' ",
t, conditionName, toJSONString(m)));
throwExp(m,"''{0}'' is not allowed for variable : ''{1}''",t,conditionName);
}
}
}
if (computedType == null && ((String) val).charAt(0) == '#' && !varType.wildCards.contains(val)) {
throw new IllegalArgumentException(formatString("''{0}'' is not an allowed value for ''{1}'' , in clause : ''{2}'' . Supported value is : {3}",
val, conditionName, toJSONString(m), varType.wildCards));
throwExp(m, "''{0}'' is not an allowed value for ''{1}'', supported value is : {2} ", val, conditionName, varType.wildCards );
}
operand = varType == null ? operand : varType.getOperand(operand, strVal, computedType);
@ -309,10 +306,15 @@ public class Clause implements MapWriter, Comparable<Clause> {
} catch (IllegalArgumentException iae) {
throw iae;
} catch (Exception e) {
throw new IllegalArgumentException("Invalid tag : " + s + ":" + val, e);
throwExp(m, "Invalid tag : {0} ",s );
return null;
}
}
public void throwExp(Map<String, Object> clause, String msg, Object... args) {
throw new IllegalArgumentException("syntax error in clause :"+ toJSONString(clause)+ " , msg: "+ formatString(msg, args));
}
private List readListVal(Map m, List val, Type varType, String conditionName) {
List list = val;
list = (List) list.stream()

View File

@ -104,8 +104,9 @@ public class CoresVariable extends VariableBase {
} else {
return "cores: '#EQUAL' can be used only with node: '#ANY', node :[....]";
}
} else {
return ReplicaVariable.checkNonEqualOp(condition);
}
return null;
}
@Override

View File

@ -88,13 +88,28 @@ class ReplicaVariable extends VariableBase {
} else {
return "'replica': '#EQUAL` must be used with 'node':'#ANY'";
}
}
if (condition.computedType == ComputedType.ALL) {
} else if (condition.computedType == ComputedType.ALL) {
if (condition.getClause().tag != null && (condition.getClause().getTag().op == Operand.IN ||
condition.getClause().getTag().op == Operand.WILDCARD)) {
return StrUtils.formatString("array value or wild card cannot be used for tag {0} with replica : '#ALL'",
condition.getClause().tag.getName());
}
} else {
return checkNonEqualOp(condition);
}
return null;
}
static String checkNonEqualOp(Condition condition) {
if (condition.computedType == null &&
condition.val instanceof RangeVal &&
condition.op != Operand.RANGE_EQUAL) {
return "non-integer values cannot have any other operators";
}
if(condition.computedType == ComputedType.PERCENT && condition.op != Operand.RANGE_EQUAL){
return "percentage values cannot have any other operators";
}
return null;
}

View File

@ -766,9 +766,7 @@ public class TestPolicy extends SolrTestCaseJ4 {
assertEquals(Operand.EQUAL, REPLICA.getOperand(Operand.EQUAL, "2", null));
assertEquals(Operand.NOT_EQUAL, REPLICA.getOperand(Operand.NOT_EQUAL, "2", null));
assertEquals(Operand.RANGE_EQUAL, REPLICA.getOperand(Operand.EQUAL, "2.1", null));
assertEquals(Operand.RANGE_NOT_EQUAL, REPLICA.getOperand(Operand.NOT_EQUAL, "2.1", null));
assertEquals(Operand.RANGE_EQUAL, REPLICA.getOperand(Operand.EQUAL, "2.01", null));
assertEquals(Operand.RANGE_NOT_EQUAL, REPLICA.getOperand(Operand.NOT_EQUAL, "2.01", null));
Clause clause = Clause.create("{replica: '1.23', node:'#ANY'}");
assertTrue(clause.getReplica().isPass(2));
@ -781,11 +779,9 @@ public class TestPolicy extends SolrTestCaseJ4 {
assertTrue(clause.getReplica().isPass(0));
assertFalse(clause.getReplica().isPass(2));
clause = Clause.create("{replica: '!1.23', node:'#ANY'}");
assertFalse(clause.getReplica().isPass(2));
assertFalse(clause.getReplica().isPass(1));
assertTrue(clause.getReplica().isPass(0));
assertTrue(clause.getReplica().isPass(3));
expectThrows(IllegalArgumentException.class,
() -> Clause.create("{replica: '!1.23', node:'#ANY'}"));
clause = Clause.create("{replica: 1.23, node:'#ANY'}");
assertTrue(clause.getReplica().isPass(2));
@ -897,6 +893,23 @@ public class TestPolicy extends SolrTestCaseJ4 {
clause = Clause.create("{cores: '14%' , node:[node1, node2, node3]}");
assertEquals(Operand.IN, clause.getTag().op);
expectThrows(IllegalArgumentException.class,
() -> Clause.create("{replica: '!14%' , node:'#ANY'}"));
expectThrows(IllegalArgumentException.class,
() -> Clause.create("{cores: '!14%' , node:[node1, node2, node3]}"));
expectThrows(IllegalArgumentException.class,
() -> Clause.create("{cores: '!1.66' , node:'#ANY'}"));
expectThrows(IllegalArgumentException.class,
() -> Clause.create("{replica: '<14%' , node:'#ANY'}"));
expectThrows(IllegalArgumentException.class,
() -> Clause.create("{replica: '>14%' , node:'#ANY'}"));
expectThrows(IllegalArgumentException.class,
() -> Clause.create("{cores: '>14%' , node:'#ANY'}"));
}
@ -2437,7 +2450,7 @@ public class TestPolicy extends SolrTestCaseJ4 {
" 'node':'10.0.0.6:8983_solr'," +
" 'cores':2}}}";
autoScalingjson = " { cluster-policy:[" +
" { replica :'<51%',node:'#ANY' , type: TLOG } ,{ replica :'<51%',node:'#ANY' , type: PULL } ]," +
" { replica :'50%',node:'#ANY' , type: TLOG } ,{ replica :'50%',node:'#ANY' , type: PULL } ]," +
" cluster-preferences :[{ minimize : cores }]}";
autoScalingConfig = new AutoScalingConfig((Map<String, Object>) Utils.fromJSONString(autoScalingjson));
session = autoScalingConfig.getPolicy().createSession(cloudManagerWithData(dataproviderdata));