Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PROC-1334: Add library support for rule filtering #142

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -80,4 +80,44 @@ public static boolean versionInRange(
}
return inRange(version, start, end);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given all those code will only be used outside the proctor library. https://indeed-pte.slack.com/archives/C1F00STA9/p1688590802320449?thread_ts=1688582115.448299&cid=C1F00STA9

Can we add java doc to each of those methods to explain it is not meant to be called inside proctor lib?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, can we create a subclass and only the subclass has those extra methods?

public static MaybeBool maybeAnd(final MaybeBool op1, final MaybeBool op2) {
if (MaybeBool.FALSE == op1 || MaybeBool.FALSE == op2) {
return MaybeBool.FALSE;
}
if (MaybeBool.TRUE == op1 && MaybeBool.TRUE == op2) {
return MaybeBool.TRUE;
}
return MaybeBool.UNKNOWN;
}

public static MaybeBool maybeOr(final MaybeBool op1, final MaybeBool op2) {
if (MaybeBool.TRUE == op1 || MaybeBool.TRUE == op2) {
return MaybeBool.TRUE;
}
if (MaybeBool.FALSE == op1 && MaybeBool.FALSE == op2) {
return MaybeBool.FALSE;
}
return MaybeBool.UNKNOWN;
}

public static MaybeBool maybeNot(final MaybeBool maybeBool) {
if (MaybeBool.TRUE == maybeBool) {
return MaybeBool.FALSE;
}
if (MaybeBool.FALSE == maybeBool) {
return MaybeBool.TRUE;
}
return MaybeBool.UNKNOWN;
}

public static MaybeBool toMaybeBool(final boolean b) {
return b ? MaybeBool.TRUE : MaybeBool.FALSE;
}

public enum MaybeBool {
TRUE,
FALSE,
UNKNOWN;
}
}
@@ -0,0 +1,43 @@
package com.indeed.proctor.common;

import com.indeed.proctor.common.el.filter.PartialExpressionFactory;

import javax.annotation.Nonnull;
import javax.annotation.concurrent.NotThreadSafe;
import javax.el.FunctionMapper;
import javax.el.ValueExpression;

import java.util.Map;

/**
* Leverages RuleEvaluator to determine whether a given rule *could* match the
* given context; useful for tools that search allocations.
**/
@NotThreadSafe
public class RuleFilter {
@Nonnull
private final PartialExpressionFactory expressionFactory;
private final RuleEvaluator ruleEvaluator;

RuleFilter(
final FunctionMapper functionMapper,
@Nonnull final Map<String, Object> testConstantsMap
) {
this.expressionFactory = new PartialExpressionFactory(testConstantsMap.keySet());
this.ruleEvaluator = new RuleEvaluator(
expressionFactory,
functionMapper,
testConstantsMap);
}

public static RuleFilter createDefaultRuleFilter(final Map<String, Object> testConstantsMap) {
return new RuleFilter(RuleEvaluator.FUNCTION_MAPPER, testConstantsMap);
}

public boolean ruleCanMatch(final String rule, final Map<String, Object> values) {
expressionFactory.setContextVariables(values.keySet());
final Map<String, ValueExpression> localContext = ProctorUtils
.convertToValueExpressionMap(expressionFactory, values);
return ruleEvaluator.evaluateBooleanRuleWithValueExpr(rule, localContext);
}
}
@@ -0,0 +1,181 @@
package com.indeed.proctor.common.el.filter;

import com.indeed.proctor.common.ProctorRuleFunctions.MaybeBool;
import org.apache.el.parser.AstAnd;
import org.apache.el.parser.AstFunction;
import org.apache.el.parser.AstIdentifier;
import org.apache.el.parser.AstLiteralExpression;
import org.apache.el.parser.AstNot;
import org.apache.el.parser.AstNotEqual;
import org.apache.el.parser.AstOr;
import org.apache.el.parser.Node;
import org.apache.el.parser.NodeVisitor;
import org.apache.el.parser.SimpleNode;

import java.lang.reflect.InvocationTargetException;
import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.Map;
import java.util.Set;
import java.util.Stack;

/**
* This class traverses a tomcat el syntax tree and transforms it into a new
* expression that evaluates to true if the original expression could be true
* given only a subset of the variables referenced.
*
* Expected usage is via the static destroyUnknowns method, which visits each
* node in the tree to identify variables that are not in the variablesDefined
* set and then transforms the expression to remove those nodes. Parents of
* unknowns are recursively replaced using a
* simple 3-valued logic (https://en.wikipedia.org/wiki/Three-valued_logic) with
* maybeAnd, maybeOr, and maybeNot, with the semantics one would expect defined
* in ProctorRuleFunctions.
*
* In the end the new maybeBool expression x is wrapped in a "not false" test: x
* != false. The final expression will resolve to true if the original
* expression is definitely true or unknown (could be true) given the known
* values.
*/
class NodeHunter implements NodeVisitor {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add some javadoc in this class?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay added some explanation.

private final Set<Node> initialUnknowns = Collections.newSetFromMap(new IdentityHashMap<>());
private final Map<Node, Node> replacements = new IdentityHashMap<>();
private final Set<String> variablesDefined;

NodeHunter(final Set<String> variablesDefined) {
this.variablesDefined = variablesDefined;
}

public static Node destroyUnknowns(
final Node node,
final Set<String> variablesDefined
) throws Exception {
final NodeHunter nodeHunter = new NodeHunter(variablesDefined);
node.accept(nodeHunter);
if (nodeHunter.initialUnknowns.isEmpty()) {
// Nothing to do here
return node;
}
nodeHunter.calculateReplacements();
final Node result = nodeHunter.replaceNodes(node);
// At this point result is a maybebool, we need to convert it to a bool
final Node resultIsNotFalse = nodeHunter.wrapIsNotFalse(result);
return resultIsNotFalse;
}

private void calculateReplacements() {
final Stack<Node> nodesToDestroy = new Stack<>();
initialUnknowns.forEach(nodesToDestroy::push);
while (!nodesToDestroy.isEmpty()) {
final Node nodeToDestroy = nodesToDestroy.pop();
if (nodeToDestroy instanceof AstAnd) {
// Replace simple "and" with maybeAnd
replaceWithFunction(nodeToDestroy, "maybeAnd");
} else if (nodeToDestroy instanceof AstOr) {
// Replace simple "or" with maybeOr
replaceWithFunction(nodeToDestroy, "maybeOr");
} else if (nodeToDestroy instanceof AstNot) {
// Replace simple "not" with maybeNot
replaceWithFunction(nodeToDestroy, "maybeNot");
// } else if (nodeToDestroy instanceof AstEqual || nodeToDestroy instanceof AstNotEqual) {
// TODO: if someone compares two bools using == that would be
// weird, but we could handle it by making sure any cases that mix
// maybeBool and bool are promoted to maybeBool like we do with the
// other logical operators
} else if (!replacements.containsKey(nodeToDestroy)) {
// Anything else propagate the unknown value
//
// TODO: If a bool is used as an argument to a function we
// could try and do the function if the maybeBool is true or
// false, and only propagate the unknown if any argument is
// unknown, but that seems rare and very complicated so I
// haven't handled that case here.
final AstLiteralExpression replacement = new AstLiteralExpression(1);
replacement.setImage(MaybeBool.UNKNOWN.name());
replacements.put(nodeToDestroy, replacement);
}
if (nodeToDestroy.jjtGetParent() != null) {
nodesToDestroy.push(nodeToDestroy.jjtGetParent());
}
}
}

private void replaceWithFunction(final Node node, final String function) {
final AstFunction replacement = new AstFunction(27);
replacement.setPrefix("proctor");
replacement.setLocalName(function);
replacement.setImage("proctor:" + function);
for (int i = 0; i < node.jjtGetNumChildren(); i++) {
final Node child = node.jjtGetChild(i);
if (replacements.containsKey(child)) {
replacement.jjtAddChild(replacements.get(child), i);
} else {
final AstFunction replacementChild = new AstFunction(27);
replacementChild.setPrefix("proctor");
replacementChild.setLocalName("toMaybeBool");
replacementChild.setImage("proctor:toMaybeBool");
replacementChild.jjtAddChild(child, 0);
replacement.jjtAddChild(replacementChild, i);
}
}
replacements.put(node, replacement);
}

private Node replaceNodes(
final Node node
) throws NoSuchMethodException, InvocationTargetException, IllegalAccessException, InstantiationException {
if (replacements.containsKey(node)) {
Node newNode = node;
while (replacements.containsKey(newNode)) {
newNode = replacements.get(newNode);
}
return newNode;
}
final SimpleNode asSimpleNode = (SimpleNode) node;
for (int i = 0; i < asSimpleNode.jjtGetNumChildren(); i++) {
final Node newChild = replaceNodes(asSimpleNode.jjtGetChild(i));
asSimpleNode.jjtAddChild(newChild, i);
newChild.jjtSetParent(asSimpleNode);
}
return node;
}

@Override
public void visit(final Node node) throws Exception {
if (node instanceof AstIdentifier) {
String variable = node.getImage();
if (!variablesDefined.contains(variable)) {
initialUnknowns.add(node);
}
}
}

private Node wrapIsNotFalse(final Node node) {
final Node resultIsNotFalse = new AstNotEqual(9);
final AstLiteralExpression literalFalse = new AstLiteralExpression(1);
literalFalse.setImage(MaybeBool.FALSE.name());
literalFalse.jjtSetParent(resultIsNotFalse);
resultIsNotFalse.jjtSetParent(node.jjtGetParent());
node.jjtSetParent(resultIsNotFalse);
resultIsNotFalse.jjtAddChild(node, 0);
resultIsNotFalse.jjtAddChild(literalFalse, 1);
return resultIsNotFalse;
}

// handy utility for debugging
public static void printAST(final Node node) {
final StringBuilder stringBuilder = new StringBuilder().append("\n");
printAST(stringBuilder, 0, node);
System.err.println(stringBuilder.toString());
}

private static void printAST(final StringBuilder stringBuilder, final int depth, final Node node) {
for (int i = 0; i < depth; i++) {
stringBuilder.append(" ");
}
stringBuilder.append(node).append('(').append(node.getClass().getSimpleName()).append(")\n");
for (int i = 0; i < node.jjtGetNumChildren(); i++) {
printAST(stringBuilder, depth + 1, node.jjtGetChild(i));
}
}
}