You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by no...@apache.org on 2018/08/13 15:11:15 UTC
lucene-solr:master: SOLR-12592: added more validation and tests
Repository: lucene-solr
Updated Branches:
refs/heads/master 767223ddd -> be4a33938
SOLR-12592: added more validation and tests
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/be4a3393
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/be4a3393
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/be4a3393
Branch: refs/heads/master
Commit: be4a33938d3c6eac12b0a575b12cafa5c781909c
Parents: 767223d
Author: Noble Paul <no...@apache.org>
Authored: Tue Aug 14 01:11:01 2018 +1000
Committer: Noble Paul <no...@apache.org>
Committed: Tue Aug 14 01:11:01 2018 +1000
----------------------------------------------------------------------
.../client/solrj/cloud/autoscaling/Clause.java | 20 ++++++++------
.../solrj/cloud/autoscaling/CoresVariable.java | 3 +-
.../cloud/autoscaling/ReplicaVariable.java | 19 +++++++++++--
.../solrj/cloud/autoscaling/TestPolicy.java | 29 ++++++++++++++------
4 files changed, 51 insertions(+), 20 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/be4a3393/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Clause.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Clause.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Clause.java
index e325d10..3a57a1f 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Clause.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Clause.java
@@ -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()
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/be4a3393/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/CoresVariable.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/CoresVariable.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/CoresVariable.java
index 4df394b..3344626 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/CoresVariable.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/CoresVariable.java
@@ -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
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/be4a3393/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/ReplicaVariable.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/ReplicaVariable.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/ReplicaVariable.java
index 6239ec3..78f32b3 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/ReplicaVariable.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/ReplicaVariable.java
@@ -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;
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/be4a3393/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java b/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java
index 19db8fc..b75e1ba 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java
@@ -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));