Menu
▾
▴
[Rhq-commits] [rhq-project.org rhq] [2539] RHQ-1309 - support filtering on unset/null plugin/resource configuration properties;
|
From: <jma...@rh...> - 2008-12-25 06:59:43
|
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd"> <html xmlns="http://www.w3.org/1999/xhtml"> <head><style type="text/css"><!-- #msg DL { border : 1px #006 solid; background-color : #369; padding : 6px; color : #fff; } #msg DT { float : left; width : 6em; font-weight : bold; } #msg DL, #msg DT, #msg UL, #msg LI { font-family : arial,helvetica,sans-serif; font-size : 10pt; } h3 { font-family : arial,helvetica,sans-serif; font-size : 10pt; font-weight : bold; } #msg PRE { overflow : auto; white-space : normal; background-color : #ffc; border : 1px #fc0 solid; padding : 6px; } #msg UL, PRE, .diff { overflow : auto; } #patch h4 { font-family : arial,helvetica,sans-serif; font-size : 10pt; } #patch h4 { padding: 8px; background : #369; color : #fff; margin : 0; } #patch .propset h4, #patch .binary h4 {margin: 0;} #patch pre {padding:0;line-height:1.2em;margin:0;} #patch .diff {background:#eeeeee;padding: 0 0 10px 0;} #patch .propset .diff, #patch .binary .diff {padding: 10px 0;} #patch span {display:block;padding:0 10px;} #patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;} #patch .add {background:#ddffdd;} #patch .rem {background:#ffdddd;} #patch .lines, .info {color:#888888;background:#ffffff;} .diff { width : 100%; } #msg DL { border : 1px #006 solid; background-color : #369; padding : 6px; color : #fff; } #msg DT { float : left; width : 6em; font-weight : bold; } #msg DL, #msg DT, #msg UL, #msg LI { font-family : arial,helvetica,sans-serif; font-size : 10pt; } h3 { font-family : arial,helvetica,sans-serif; font-size : 10pt; font-weight : bold; } #msg PRE { overflow : auto; white-space : normal; background-color : #ffc; border : 1px #fc0 solid; padding : 6px; } #msg UL, PRE, .diff { overflow : auto; } #patch h4 { font-family : arial,helvetica,sans-serif; font-size : 10pt; } #patch h4 { padding: 8px; background : #369; color : #fff; margin : 0; } #patch .propset h4, #patch .binary h4 {margin: 0;} #patch pre {padding:0;line-height:1.2em;margin:0;} #patch .diff {background:#eeeeee;padding: 0 0 10px 0;} #patch .propset .diff, #patch .binary .diff {padding: 10px 0;} #patch span {display:block;padding:0 10px;} #patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;} #patch .add {background:#ddffdd;} #patch .rem {background:#ffdddd;} #patch .lines, .info {color:#888888;background:#ffffff;} .diff { width : 100%; } --></style> <title>[rhq-project.org rhq] [2539] RHQ-1309 - support filtering on unset/null plugin/resource configuration properties; </title> </head> <body> <div id="msg"> <dl> <dt>Revision</dt> <dd>2539</dd> <dt>Author</dt> <dd>jmarques</dd> <dt>Date</dt> <dd>2008-12-25 00:59:35 -0600 (Thu, 25 Dec 2008)</dd> </dl> <h3>Log Message</h3> <pre>RHQ-1309 - support filtering on unset/null plugin/resource configuration properties; in fact, this commit supports the general nullabllity of any condition that dynagroups supports; </pre> <h3>Modified Paths</h3> <ul> <li><a href="#rhqtrunkmodulesenterpriseserverjarsrcmainjavaorgrhqenterpriseserverresourcegroupdefinitionframeworkExpressionEvaluatorjava">rhq/trunk/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/resource/group/definition/framework/ExpressionEvaluator.java</a></li> <li><a href="#rhqtrunkmodulesenterpriseserverjarsrctestjavaorgrhqenterpriseserverresourcegroupdefinitionframeworktestExpressionEvaluatorTestjava">rhq/trunk/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/resource/group/definition/framework/test/ExpressionEvaluatorTest.java</a></li> </ul> </div> <div id="patch"> <h3>Diff</h3> <a id="rhqtrunkmodulesenterpriseserverjarsrcmainjavaorgrhqenterpriseserverresourcegroupdefinitionframeworkExpressionEvaluatorjava"></a> <div class="modfile"><h4>Modified: rhq/trunk/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/resource/group/definition/framework/ExpressionEvaluator.java (2538 => 2539)</h4> <pre class="diff"> <span class="info">--- rhq/trunk/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/resource/group/definition/framework/ExpressionEvaluator.java 2008-12-24 01:49:14 UTC (rev 2538) +++ rhq/trunk/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/resource/group/definition/framework/ExpressionEvaluator.java 2008-12-25 06:59:35 UTC (rev 2539) </span><span class="lines">@@ -43,7 +43,8 @@ </span><span class="cx"> private final Log log = LogFactory.getLog(ExpressionEvaluator.class); </span><span class="rem">- private static final String INVALID_EXPRESSION_FORM_MSG = "Expression must be in the form of 'condition = value' or 'groupBy condition'"; </span><span class="add">+ private static final String INVALID_EXPRESSION_FORM_MSG = "Expression must be in one of the follow forms: " + // + "'groupBy condition', 'condition = value', 'empty condition', 'not empty condition"; </span><span class="cx"> private static final String PROP_SIMPLE_ALIAS = "simple"; private static final String PROP_SIMPLE_DEF_ALIAS = "simpleDef"; </span><span class="lines">@@ -169,7 +170,6 @@ </span><span class="cx"> } try { </span><span class="rem">- isGroupBy = false; // this needs to be reset each time a new expression is added </span><span class="cx"> parseExpression(expression); expressionCount++; } catch (InvalidExpressionException iee) { </span><span class="lines">@@ -208,7 +208,7 @@ </span><span class="cx"> private enum ParseContext { BEGIN(false), // </span><span class="rem">- Pivot(false), // </span><span class="add">+ Modifier(false), // includes 'empty', 'not', and 'pivot' </span><span class="cx"> Resource(false), // ResourceParent(false), // ResourceGrandParent(false), // </span><span class="lines">@@ -216,7 +216,7 @@ </span><span class="cx"> ResourceType(false), // Availability(true), // Trait(true), // </span><span class="rem">- Configuration(true), // </span><span class="add">+ Configuration(true), // includes 'pluginConfiguration' and 'resourceConfiguration' </span><span class="cx"> StringMatch(true), // END(true); </span><span class="lines">@@ -232,13 +232,30 @@ </span><span class="cx"> } private enum ParseSubContext { </span><span class="rem">- PluginConfiguration, ResourceConfiguration; </span><span class="add">+ Negated, // only relevant for Modifier context + NotEmpty, // only relevant for Modifier context + Empty, // only relevant for Modifier context + Pivot, // only relevant for Modifier context + PluginConfiguration, // only relevant for Configuration context + ResourceConfiguration; // only relevant for Configuration context </span><span class="cx"> } </span><span class="add">+ private enum ComparisonType { + NONE, // expression in the form of 'groupBy condition' + EQUALS, // expression in the form of 'condition = value' + EMPTY, // expression in the form of 'empty value' + NOT_EMPTY; // expression in the form of 'not empty value' + } + + private enum Literal { + NULL, NOTNULL; + } + </span><span class="cx"> private ParseContext context = ParseContext.BEGIN; private ParseSubContext subcontext = null; private int parseIndex = 0; private boolean isGroupBy = false; </span><span class="add">+ private ComparisonType comparisonType = null; </span><span class="cx"> private Class<?> expressionType; private ParseContext deepestResourceContext = null; </span><span class="lines">@@ -261,11 +278,15 @@ </span><span class="cx"> /* * instead of building '= value' parsing into the below algorithm, let's chop this off early and store it; this * makes the rest of the parsing a bit simpler because some ParseContexts need the value immediately in order to </span><span class="rem">- * properly build up internal maps constructs to be used in generating the requisite JPQL statement </span><span class="add">+ * properly build up internal maps / constructs to be used in generating the requisite JPQL statement + * + * </span><span class="cx"> */ int equalsIndex = expression.lastIndexOf('='); if (equalsIndex == -1) { condition = expression; </span><span class="add">+ + //isReference </span><span class="cx"> } else { condition = expression.substring(0, equalsIndex); value = expression.substring(equalsIndex + 1).trim(); </span><span class="lines">@@ -293,8 +314,13 @@ </span><span class="cx"> */ StringBuilder normalizedSubExpressionBuilder = new StringBuilder(); for (String subExpressionToken : tokens) { </span><span class="add">+ // do not add modifiers to the normalized expression </span><span class="cx"> if (subExpressionToken.equals("groupby")) { continue; </span><span class="add">+ } else if (subExpressionToken.equals("not")) { + continue; + } else if (subExpressionToken.equals("empty")) { + continue; </span><span class="cx"> } normalizedSubExpressionBuilder.append(subExpressionToken); } </span><span class="lines">@@ -312,6 +338,8 @@ </span><span class="cx"> context = ParseContext.BEGIN; subcontext = null; parseIndex = 0; </span><span class="add">+ isGroupBy = false; // this needs to be reset each time a new expression is added + comparisonType = ComparisonType.EQUALS; // assume equals, unless "(not) empty" found during the parse </span><span class="cx"> deepestResourceContext = null; expressionType = String.class; </span><span class="lines">@@ -321,33 +349,73 @@ </span><span class="cx"> if (context == ParseContext.BEGIN) { if (nextToken.equals("resource")) { </span><span class="rem">- if (value == null) { - // filter expressions must have "= <value>" part - throw new InvalidExpressionException(INVALID_EXPRESSION_FORM_MSG); - } - validateSubExpressionAgainstPreviouslySeen(normalizedSubExpression, false); </span><span class="cx"> context = ParseContext.Resource; deepestResourceContext = context; } else if (nextToken.equals("groupby")) { </span><span class="rem">- if (value != null) { - // grouped expressions must NOT have "= <value>" part - throw new InvalidExpressionException(INVALID_EXPRESSION_FORM_MSG); - } - validateSubExpressionAgainstPreviouslySeen(normalizedSubExpression, true); - isGroupBy = true; - context = ParseContext.Pivot; </span><span class="add">+ context = ParseContext.Modifier; + subcontext = ParseSubContext.Pivot; + } else if (nextToken.equals("not")) { + context = ParseContext.Modifier; + subcontext = ParseSubContext.Negated; + // 'not' must be followed by 'empty' today, but we won't know until next parse iteration + // furthermore, we may support other forms of negated expressions in the future + } else if (nextToken.equals("empty")) { + context = ParseContext.Modifier; + subcontext = ParseSubContext.Empty; </span><span class="cx"> } else { throw new InvalidExpressionException( "Expression must either start with 'resource' or 'groupby' token"); } </span><span class="rem">- } else if (context == ParseContext.Pivot) { - if (nextToken.equals("resource")) { - context = ParseContext.Resource; - deepestResourceContext = context; </span><span class="add">+ } else if (context == ParseContext.Modifier) { + if (subcontext == ParseSubContext.Negated) { + if (nextToken.equals("empty")) { + subcontext = ParseSubContext.NotEmpty; + } else { + throw new InvalidExpressionException( + "Expression starting with 'not' must be followed by the 'empty' token"); + } </span><span class="cx"> } else { </span><span class="rem">- throw new InvalidExpressionException("Grouped expressions must be followed by the 'resource' token"); </span><span class="add">+ // first check for valid forms given the subcontext + if (subcontext == ParseSubContext.Pivot || subcontext == ParseSubContext.Empty + || subcontext == ParseSubContext.NotEmpty) { + if (value != null) { + // these specific types of 'modified' expressions must NOT HAVE "= <value>" part + throw new InvalidExpressionException(INVALID_EXPRESSION_FORM_MSG); + } + } + + // then perform individual processing based on current subcontext + if (subcontext == ParseSubContext.Pivot) { + // validates the uniqueness of the subexpression after checking for INVALID_EXPRESSION_FORM_MSG + validateSubExpressionAgainstPreviouslySeen(normalizedSubExpression, true); + isGroupBy = true; + comparisonType = ComparisonType.NONE; + } else if (subcontext == ParseSubContext.NotEmpty) { + comparisonType = ComparisonType.NOT_EMPTY; + } else if (subcontext == ParseSubContext.Empty) { + comparisonType = ComparisonType.EMPTY; + } else { + throw new InvalidExpressionException("Unknown or unsupported ParseSubContext[" + subcontext + + "] for ParseContext[" + context + "]"); + } + + if (nextToken.equals("resource")) { + context = ParseContext.Resource; + deepestResourceContext = context; + } else { + throw new InvalidExpressionException( + "Grouped expressions must be followed by the 'resource' token"); + } </span><span class="cx"> } } else if (context == ParseContext.Resource) { </span><span class="add">+ if (comparisonType == ComparisonType.EQUALS) { + if (value == null) { + // EQUALS filter expressions must HAVE "= <value>" part + throw new InvalidExpressionException(INVALID_EXPRESSION_FORM_MSG); + } + validateSubExpressionAgainstPreviouslySeen(normalizedSubExpression, false); + } + </span><span class="cx"> if (nextToken.equals("parent")) { context = ParseContext.ResourceParent; deepestResourceContext = context; </span><span class="lines">@@ -567,16 +635,39 @@ </span><span class="cx"> * it will only add data to the predicate list groupByElements if necessary, as determined by the instance-level * isGroupBy field or the explicitly overriding groupBy 3rd argument */ </span><span class="rem">- private void populatePredicateCollections(String predicateName, Object value) { </span><span class="add">+ private void populatePredicateCollections(String predicateName, Object value) throws InvalidExpressionException { </span><span class="cx"> populatePredicateCollections(predicateName, value, isGroupBy); } </span><span class="rem">- private void populatePredicateCollections(String predicateName, Object value, boolean groupBy) { </span><span class="add">+ private void populatePredicateCollections(String predicateName, Object value, boolean groupBy) + throws InvalidExpressionException { </span><span class="cx"> if (groupBy) { groupByElements.add(predicateName); } else { String argumentName = getNextArgumentName(); </span><span class="add">+ // change the value as necessary based on the comparison type + if (comparisonType == ComparisonType.EMPTY) { + /* + * a single parse context may populate several predicate collections, + * but we want to make sure we are only performing extra comparison + * computation on values representing the "empty" RHS of the expression + */ + if (value == null) { + value = Literal.NULL; + } + } else if (comparisonType == ComparisonType.NOT_EMPTY) { + // see comment for ComparisonType.EMPTY logic just above this block + if (value == null) { + value = Literal.NOTNULL; + } + } else if (comparisonType == ComparisonType.EQUALS || comparisonType == ComparisonType.NONE) { + // pass through + } else { + throw new InvalidExpressionException("Unknown or unsupported ComparisonType[" + comparisonType + + "] for predicate population"); + } + </span><span class="cx"> whereConditions.put(predicateName, argumentName); whereReplacements.put(argumentName, value); whereReplacementTypes.put(argumentName, expressionType); </span><span class="lines">@@ -832,23 +923,30 @@ </span><span class="cx"> result += " AND "; } </span><span class="rem">- String whereConditionOperator = " = "; - </span><span class="cx"> Object bindValue = whereReplacements.get(whereCondition.getValue()); </span><span class="rem">- if (bindValue != null) { - /* - * there will *not* necessarily be a replacement value ready at this point in the processing; these - * get set earlier if this is a SingleQuery, but a MultipleQuery will set these later based on the - * results of the pivoted query; so, only attempt processing here if necessary - */ - String bindValueAsString = bindValue.toString(); - if ((bindValueAsString != null) // whereConditionValue is null when whereCondition isn't a groupBy expression - && (bindValueAsString.startsWith("%") || bindValueAsString.endsWith("%"))) { - whereConditionOperator = " like "; </span><span class="add">+ if (bindValue == Literal.NOTNULL) { + result += whereCondition.getKey() + " IS NOT NULL "; + whereReplacements.remove(whereCondition.getValue()); // no longer needed, literal rendered here + } else if (bindValue == Literal.NULL) { + result += whereCondition.getKey() + " IS NULL "; + whereReplacements.remove(whereCondition.getValue()); // no longer needed, literal rendered here + } else { + String whereConditionOperator = " = "; + if (bindValue != null) { + /* + * there will *not* necessarily be a replacement value ready at this point in the processing; these + * get set earlier if this is a SingleQuery, but a MultipleQuery will set these later based on the + * results of the pivoted query; so, only attempt processing here if necessary + */ + String bindValueAsString = bindValue.toString(); + if ((bindValueAsString != null) // whereConditionValue is null when whereCondition isn't a groupBy expression + && (bindValueAsString.startsWith("%") || bindValueAsString.endsWith("%"))) { + whereConditionOperator = " LIKE "; + } </span><span class="cx"> } </span><span class="add">+ result += whereCondition.getKey() + whereConditionOperator + ":" + whereCondition.getValue() + " "; </span><span class="cx"> } </span><span class="rem">- result += whereCondition.getKey() + whereConditionOperator + ":" + whereCondition.getValue() + " "; </span><span class="cx"> first = false; } } </span><span class="lines">@@ -931,16 +1029,20 @@ </span><span class="cx"> normalizedSubExpression = stripFunctionSuffix(normalizedSubExpression); if (grouped) { if (groupedSubExpressions.contains(normalizedSubExpression)) { </span><span class="rem">- throw new InvalidExpressionException( - "Redundant 'groupby' expressions - these expressions must be unique"); </span><span class="add">+ throw new InvalidExpressionException("Redundant 'groupby' expression[" + normalizedSubExpression + + "] - these expressions must be unique"); </span><span class="cx"> } if (simpleSubExpressions.contains(normalizedSubExpression)) { </span><span class="rem">- throw new InvalidExpressionException("Can not group by the same condition you are filtering on"); </span><span class="add">+ throw new InvalidExpressionException( + "Can not group by the same condition you are filtering on, expression[" + normalizedSubExpression + + "]"); </span><span class="cx"> } groupedSubExpressions.add(normalizedSubExpression); } else { if (groupedSubExpressions.contains(normalizedSubExpression)) { </span><span class="rem">- throw new InvalidExpressionException("Can not group by the same condition you are filtering on"); </span><span class="add">+ throw new InvalidExpressionException( + "Can not group by the same condition you are filtering on, expression[" + normalizedSubExpression + + "]"); </span><span class="cx"> } simpleSubExpressions.add(normalizedSubExpression); } </span></pre></div> <a id="rhqtrunkmodulesenterpriseserverjarsrctestjavaorgrhqenterpriseserverresourcegroupdefinitionframeworktestExpressionEvaluatorTestjava"></a> <div class="modfile"><h4>Modified: rhq/trunk/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/resource/group/definition/framework/test/ExpressionEvaluatorTest.java (2538 => 2539)</h4> <pre class="diff"> <span class="info">--- rhq/trunk/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/resource/group/definition/framework/test/ExpressionEvaluatorTest.java 2008-12-24 01:49:14 UTC (rev 2538) +++ rhq/trunk/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/resource/group/definition/framework/test/ExpressionEvaluatorTest.java 2008-12-25 06:59:35 UTC (rev 2539) </span><span class="lines">@@ -118,8 +118,34 @@ </span><span class="cx"> " AND trait.value = :arg3 " + // " AND trait.schedule = sched " + // " AND trait.id.timestamp = " + // </span><span class="rem">- " (SELECT max(mdt.id.timestamp) FROM MeasurementDataTrait mdt WHERE sched.id = mdt.schedule.id)" } }; </span><span class="add">+ " (SELECT max(mdt.id.timestamp) FROM MeasurementDataTrait mdt WHERE sched.id = mdt.schedule.id)" }, </span><span class="cx"> </span><span class="add">+ { " ;" + // test empty first line, which should be allowed + "resource.name.contains = joseph; " + // + " ;" + // test empty intermediate line, which should be allowed + "resource.parent.name.contains = joseph; " + // + " ;", // test empty last line, which should be allowed, + + "SELECT res.id FROM Resource res " + // + "WHERE res.name LIKE :arg1 " + // + " AND res.parentResource.name LIKE :arg2 " }, + + { "EMPTY resource.name", + + "SELECT res.id FROM Resource res " + // + "WHERE res.name IS NULL" }, + + { "empty resource.pluginConfiguration[partition]", + + "SELECT res.id FROM Resource res " + // + " JOIN res.pluginConfiguration pluginConf, PropertySimple simple, PropertyDefinition simpleDef " + // + " JOIN res.resourceType.pluginConfigurationDefinition pluginConfDef " + // + " WHERE simple.name = :arg1 " + // + " AND simple.stringValue IS NULL " + // + " AND simple.configuration = pluginConf " + // + " AND simpleDef.configurationDefinition = pluginConfDef " + // + " AND simple.name = simpleDef.name AND simpleDef.type != 'PASSWORD' " }, }; + </span><span class="cx"> @Test(groups = "integration.session") public void testWellFormedExpressions() throws Exception { List<Integer> suppressedCases = Arrays.asList(); </span><span class="lines">@@ -141,7 +167,13 @@ </span><span class="cx"> ExpressionEvaluator evaluator = new ExpressionEvaluator(); evaluator.setTestMode(true); // to prevent actual query from happening for (String expression : inputExpressions.split(";")) { </span><span class="rem">- evaluator.addExpression(expression.trim()); </span><span class="add">+ try { + evaluator.addExpression(expression); // do not trim, evaluator must handle sloppy expressions + } catch (Exception e) { + e.printStackTrace(System.out); + assert false : "Error in TestCase[" + i + "], could not add expression[" + expression + + "], input[" + inputExpressions + "]"; + } </span><span class="cx"> } evaluator.execute(); // execute will compute the JPQL statements </span><span class="lines">@@ -154,8 +186,8 @@ </span><span class="cx"> expectedGroupResult = cleanUp(expectedGroupResult); actualGroupResult = cleanUp(actualGroupResult); </span><span class="rem">- System.out.println("Expected[" + i + "]: \"" + expectedTopResult + "\""); - System.out.println("Received[" + i + "]: \"" + actualTopResult + "\""); </span><span class="add">+ //System.out.println("Expected[" + i + "]: \"" + expectedTopResult + "\""); + //System.out.println("Received[" + i + "]: \"" + actualTopResult + "\""); </span><span class="cx"> assert expectedTopResult.equalsIgnoreCase(actualTopResult) && expectedGroupResult.equalsIgnoreCase(actualGroupResult) : "TestCase[" + i + "] = \"" + inputExpressions + "\" failed. \n" + "Expected Top Result: \"" + expectedTopResult + "\"\n" </span> </pre> </div> </div> </body> </html> |