From ef0d7e54a4314d93170ebda163c5191958c6748b Mon Sep 17 00:00:00 2001 From: Joseph Barratt Date: Wed, 5 Jul 2023 14:20:33 -0500 Subject: [PATCH 1/2] PROC-1334: Add support for rule evaluation for partial contexts --- .../proctor/common/ProctorRuleFunctions.java | 40 ++++ .../com/indeed/proctor/common/RuleFilter.java | 43 ++++ .../proctor/common/el/filter/NodeHunter.java | 163 +++++++++++++ .../el/filter/PartialExpressionBuilder.java | 221 ++++++++++++++++++ .../el/filter/PartialExpressionFactory.java | 95 ++++++++ .../indeed/proctor/common/TestRuleFilter.java | 71 ++++++ 6 files changed, 633 insertions(+) create mode 100644 proctor-common/src/main/java/com/indeed/proctor/common/RuleFilter.java create mode 100644 proctor-common/src/main/java/com/indeed/proctor/common/el/filter/NodeHunter.java create mode 100644 proctor-common/src/main/java/com/indeed/proctor/common/el/filter/PartialExpressionBuilder.java create mode 100644 proctor-common/src/main/java/com/indeed/proctor/common/el/filter/PartialExpressionFactory.java create mode 100644 proctor-common/src/test/java/com/indeed/proctor/common/TestRuleFilter.java diff --git a/proctor-common/src/main/java/com/indeed/proctor/common/ProctorRuleFunctions.java b/proctor-common/src/main/java/com/indeed/proctor/common/ProctorRuleFunctions.java index dae020973..28890e940 100644 --- a/proctor-common/src/main/java/com/indeed/proctor/common/ProctorRuleFunctions.java +++ b/proctor-common/src/main/java/com/indeed/proctor/common/ProctorRuleFunctions.java @@ -80,4 +80,44 @@ public static boolean versionInRange( } return inRange(version, start, end); } + + 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; + } } diff --git a/proctor-common/src/main/java/com/indeed/proctor/common/RuleFilter.java b/proctor-common/src/main/java/com/indeed/proctor/common/RuleFilter.java new file mode 100644 index 000000000..a468c0394 --- /dev/null +++ b/proctor-common/src/main/java/com/indeed/proctor/common/RuleFilter.java @@ -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 testConstantsMap + ) { + this.expressionFactory = new PartialExpressionFactory(testConstantsMap.keySet()); + this.ruleEvaluator = new RuleEvaluator( + expressionFactory, + functionMapper, + testConstantsMap); + } + + public static RuleFilter createDefaultRuleFilter(final Map testConstantsMap) { + return new RuleFilter(RuleEvaluator.FUNCTION_MAPPER, testConstantsMap); + } + + public boolean ruleCanMatch(final String rule, final Map values) { + expressionFactory.setContextVariables(values.keySet()); + final Map localContext = ProctorUtils + .convertToValueExpressionMap(expressionFactory, values); + return ruleEvaluator.evaluateBooleanRuleWithValueExpr(rule, localContext); + } +} diff --git a/proctor-common/src/main/java/com/indeed/proctor/common/el/filter/NodeHunter.java b/proctor-common/src/main/java/com/indeed/proctor/common/el/filter/NodeHunter.java new file mode 100644 index 000000000..f2b5e5ff1 --- /dev/null +++ b/proctor-common/src/main/java/com/indeed/proctor/common/el/filter/NodeHunter.java @@ -0,0 +1,163 @@ +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; + +class NodeHunter implements NodeVisitor { + private final Set initialUnknowns = Collections.newSetFromMap(new IdentityHashMap<>()); + private final Map replacements = new IdentityHashMap<>(); + private final Set variablesDefined; + + NodeHunter(final Set variablesDefined) { + this.variablesDefined = variablesDefined; + } + + public static Node destroyUnknowns( + final Node node, + final Set 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 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)); + } + } +} diff --git a/proctor-common/src/main/java/com/indeed/proctor/common/el/filter/PartialExpressionBuilder.java b/proctor-common/src/main/java/com/indeed/proctor/common/el/filter/PartialExpressionBuilder.java new file mode 100644 index 000000000..7451f49d6 --- /dev/null +++ b/proctor-common/src/main/java/com/indeed/proctor/common/el/filter/PartialExpressionBuilder.java @@ -0,0 +1,221 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.indeed.proctor.common.el.filter; + +import org.apache.el.MethodExpressionImpl; +import org.apache.el.MethodExpressionLiteral; +import org.apache.el.ValueExpressionImpl; +import org.apache.el.lang.FunctionMapperFactory; +import org.apache.el.lang.VariableMapperFactory; +import org.apache.el.parser.AstDeferredExpression; +import org.apache.el.parser.AstDynamicExpression; +import org.apache.el.parser.AstFunction; +import org.apache.el.parser.AstIdentifier; +import org.apache.el.parser.AstLiteralExpression; +import org.apache.el.parser.AstValue; +import org.apache.el.parser.ELParser; +import org.apache.el.parser.Node; +import org.apache.el.parser.NodeVisitor; +import org.apache.el.parser.ParseException; +import org.apache.el.util.MessageFactory; + +import javax.el.ELContext; +import javax.el.ELException; +import javax.el.FunctionMapper; +import javax.el.MethodExpression; +import javax.el.ValueExpression; +import javax.el.VariableMapper; + +import java.io.StringReader; +import java.lang.reflect.Method; +import java.util.Set; + +/** + * Like ExpressionBuilder except removes nodes referring to missing variables + */ +class PartialExpressionBuilder implements NodeVisitor { + + private FunctionMapper fnMapper; + + private VariableMapper varMapper; + + private String expression; + + private final Set variablesDefined; + + public PartialExpressionBuilder(String expression, ELContext ctx, Set variablesDefined) + throws ELException { + this.expression = expression; + + + FunctionMapper ctxFn = ctx.getFunctionMapper(); + VariableMapper ctxVar = ctx.getVariableMapper(); + + if (ctxFn != null) { + this.fnMapper = new FunctionMapperFactory(ctxFn); + } + if (ctxVar != null) { + this.varMapper = new VariableMapperFactory(ctxVar); + } + this.variablesDefined = variablesDefined; + } + + public static final Node createNode(String expr) throws ELException { + Node n = createNodeInternal(expr); + return n; + } + + private static final Node createNodeInternal(String expr) + throws ELException { + if (expr == null) { + throw new ELException(MessageFactory.get("error.null")); + } + + // Removed caching as we translate nodes differently depending on + // variables defined + Node n; + try { + n = (new ELParser(new StringReader(expr))) + .CompositeExpression(); + + // validate composite expression + int numChildren = n.jjtGetNumChildren(); + if (numChildren == 1) { + n = n.jjtGetChild(0); + } else { + Class type = null; + Node child = null; + for (int i = 0; i < numChildren; i++) { + child = n.jjtGetChild(i); + if (child instanceof AstLiteralExpression) + continue; + if (type == null) + type = child.getClass(); + else { + if (!type.equals(child.getClass())) { + throw new ELException(MessageFactory.get( + "error.mixed", expr)); + } + } + } + } + + if (n instanceof AstDeferredExpression + || n instanceof AstDynamicExpression) { + n = n.jjtGetChild(0); + } + } catch (ParseException pe) { + throw new ELException("Error Parsing: " + expr, pe); + } + return n; + } + + private void prepare(Node node) throws ELException { + try { + node.accept(this); + } catch (Exception e) { + if (e instanceof ELException) { + throw (ELException) e; + } else { + throw (new ELException(e)); + } + } + if (this.fnMapper instanceof FunctionMapperFactory) { + this.fnMapper = ((FunctionMapperFactory) this.fnMapper).create(); + } + if (this.varMapper instanceof VariableMapperFactory) { + this.varMapper = ((VariableMapperFactory) this.varMapper).create(); + } + } + + private Node build() throws ELException { + Node n = createNodeInternal(this.expression); + try { + n = NodeHunter.destroyUnknowns(n, variablesDefined); + } catch (final Exception e) { + throw new ELException(e); + } + this.prepare(n); + if (n instanceof AstDeferredExpression + || n instanceof AstDynamicExpression) { + n = n.jjtGetChild(0); + } + return n; + } + + /* + * (non-Javadoc) + * + * @see com.sun.el.parser.NodeVisitor#visit(com.sun.el.parser.Node) + */ + @Override + public void visit(Node node) throws ELException { + if (node instanceof AstFunction) { + + AstFunction funcNode = (AstFunction) node; + + if (this.fnMapper == null) { + throw new ELException(MessageFactory.get("error.fnMapper.null")); + } + Method m = fnMapper.resolveFunction(funcNode.getPrefix(), funcNode + .getLocalName()); + if (m == null) { + throw new ELException(MessageFactory.get( + "error.fnMapper.method", funcNode.getOutputName())); + } + int pcnt = m.getParameterTypes().length; + if (node.jjtGetNumChildren() != pcnt) { + throw new ELException(MessageFactory.get( + "error.fnMapper.paramcount", funcNode.getOutputName(), + "" + pcnt, "" + node.jjtGetNumChildren())); + } + } else if (node instanceof AstIdentifier && this.varMapper != null) { + String variable = ((AstIdentifier) node).getImage(); + + // simply capture it + this.varMapper.resolveVariable(variable); + } + } + + public ValueExpression createValueExpression(Class expectedType) + throws ELException { + Node n = this.build(); + return new ValueExpressionImpl(this.expression, n, this.fnMapper, + this.varMapper, expectedType); + } + + public MethodExpression createMethodExpression(Class expectedReturnType, + Class[] expectedParamTypes) throws ELException { + Node n = this.build(); + if (!n.isParametersProvided() && expectedParamTypes == null) { + throw new NullPointerException(MessageFactory + .get("error.method.nullParms")); + } + if (n instanceof AstValue || n instanceof AstIdentifier) { + return new MethodExpressionImpl(expression, n, this.fnMapper, + this.varMapper, expectedReturnType, expectedParamTypes); + } else if (n instanceof AstLiteralExpression) { + return new MethodExpressionLiteral(expression, expectedReturnType, + expectedParamTypes); + } else { + throw new ELException("Not a Valid Method Expression: " + + expression); + } + } +} + diff --git a/proctor-common/src/main/java/com/indeed/proctor/common/el/filter/PartialExpressionFactory.java b/proctor-common/src/main/java/com/indeed/proctor/common/el/filter/PartialExpressionFactory.java new file mode 100644 index 000000000..07d109861 --- /dev/null +++ b/proctor-common/src/main/java/com/indeed/proctor/common/el/filter/PartialExpressionFactory.java @@ -0,0 +1,95 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.indeed.proctor.common.el.filter; + +import com.google.common.collect.ImmutableSet; +import org.apache.el.ValueExpressionLiteral; +import org.apache.el.lang.ELSupport; +import org.apache.el.util.MessageFactory; + +import javax.annotation.concurrent.NotThreadSafe; +import javax.el.ELContext; +import javax.el.ExpressionFactory; +import javax.el.MethodExpression; +import javax.el.ValueExpression; + +import java.util.HashSet; +import java.util.Set; + +/** + * Like ExpressionFactoryImpl except removes nodes referring to missing variables + */ +@NotThreadSafe +public class PartialExpressionFactory extends ExpressionFactory { + + private final Set testConstants; + private final Set contextVariables; + + public PartialExpressionFactory(final Set testConstants) { + super(); + this.testConstants = testConstants; + this.contextVariables = new HashSet<>(); + } + + @Override + public Object coerceToType(Object obj, Class type) { + return ELSupport.coerceToType(obj, type); + } + + @Override + public MethodExpression createMethodExpression(ELContext context, + String expression, Class expectedReturnType, + Class[] expectedParamTypes) { + PartialExpressionBuilder builder = new PartialExpressionBuilder(expression, context, getAllDefinedVariables()); + return builder.createMethodExpression(expectedReturnType, + expectedParamTypes); + } + + @Override + public ValueExpression createValueExpression(ELContext context, + String expression, Class expectedType) { + if (expectedType == null) { + throw new NullPointerException(MessageFactory + .get("error.value.expectedType")); + } + PartialExpressionBuilder builder = new PartialExpressionBuilder(expression, context, getAllDefinedVariables()); + return builder.createValueExpression(expectedType); + } + + @Override + public ValueExpression createValueExpression(Object instance, + Class expectedType) { + if (expectedType == null) { + throw new NullPointerException(MessageFactory + .get("error.value.expectedType")); + } + return new ValueExpressionLiteral(instance, expectedType); + } + + public void setContextVariables(final Set contextVariables) { + this.contextVariables.clear(); + this.contextVariables.addAll(contextVariables); + } + + private Set getAllDefinedVariables() { + return ImmutableSet.builder() + .addAll(testConstants) + .addAll(contextVariables) + .build(); + } +} diff --git a/proctor-common/src/test/java/com/indeed/proctor/common/TestRuleFilter.java b/proctor-common/src/test/java/com/indeed/proctor/common/TestRuleFilter.java new file mode 100644 index 000000000..7f4d25a44 --- /dev/null +++ b/proctor-common/src/test/java/com/indeed/proctor/common/TestRuleFilter.java @@ -0,0 +1,71 @@ +package com.indeed.proctor.common; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Lists; +import org.junit.Before; +import org.junit.Test; + +import java.util.Map; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class TestRuleFilter { + + private RuleFilter ruleFilter; + private String rule; + + @Before + public void setUp() throws Exception { + final Map testConstants = ImmutableMap.of( + "US_REMOTE_Q", Lists.newArrayList("work from home remote", "remote work from home", "remote work from home jobs on indeed", "at home work", "online", "work home online", "online work from home", "work at home", "at home", "online", "work from home", "remote"), + "US_REMOTE_L", Lists.newArrayList("work at home", "at home", "online", "work from home", "remote", "remote", "us", "remote,us", "")); + ruleFilter = RuleFilter.createDefaultRuleFilter(testConstants); + } + + @Test + public void testPartialMatchEmptyContextMatch() throws Exception { + rule = "${3<4 && (2 > 1 || proctor:versionInRange(version, '1.2.0.0', '2.4.0.0'))}"; + assertMatches(ImmutableMap.of()); + } + + @Test + public void testPartialMatchEmptyContextNoMatch() throws Exception { + rule = "${3<4 && 2<1 && proctor:versionInRange(version, '1.2.0.0', '2.4.0.0')}"; + assertDoesNotMatch(ImmutableMap.of()); + } + + @Test + public void testPartialMatchComplexRule() throws Exception { + rule = "${country == 'US' && adFormat == 'web' && hasAccount && (indeed:contains(US_REMOTE_Q, fn:toLowerCase(fn:trim(query))) || indeed:contains(US_REMOTE_L, searchLocation))}"; + assertMatches(ImmutableMap.of()); + assertMatches(ImmutableMap.of("country", "US", "adFormat", "web")); + assertDoesNotMatch(ImmutableMap.of("country", "GB", "adFormat", "web")); + assertDoesNotMatch(ImmutableMap.of("country", "US", "adFormat", "mob")); + assertDoesNotMatch(ImmutableMap.of("query", "software engineer", "searchLocation", "Austin")); + assertMatches(ImmutableMap.of("query", "remote", "searchLocation", "Austin")); + assertMatches(ImmutableMap.of("query", "software engineer", "searchLocation", "remote")); + assertMatches(ImmutableMap.of("query", "software engineer")); + assertMatches(ImmutableMap.of("searchLocation", "Austin")); + assertDoesNotMatch(ImmutableMap.of("hasAccount", false, "searchLocation", "Austin")); + } + + @Test + public void testPartialFalseSometimesHelps() throws Exception { + rule = "${!(country == 'US' && adFormat == 'web')}"; + assertDoesNotMatch(ImmutableMap.of("country", "US", "adFormat", "web")); + assertMatches(ImmutableMap.of("country", "US")); + assertMatches(ImmutableMap.of("adFormat", "web")); + assertMatches(ImmutableMap.of("country", "GB")); + assertMatches(ImmutableMap.of("country", "mob")); + assertMatches(ImmutableMap.of("country", "US", "adFormat", "mob")); + } + + private void assertMatches(final Map context) { + assertTrue(ruleFilter.ruleCanMatch(rule, context)); + } + + private void assertDoesNotMatch(final Map context) { + assertFalse(ruleFilter.ruleCanMatch(rule, context)); + } +} From 14709fbf59da95775d130e83cba9fef8e9e71875 Mon Sep 17 00:00:00 2001 From: Joseph Barratt Date: Wed, 11 Oct 2023 15:33:17 -0500 Subject: [PATCH 2/2] PROC-1334: Add javadoc for NodeHunter --- .../proctor/common/el/filter/NodeHunter.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/proctor-common/src/main/java/com/indeed/proctor/common/el/filter/NodeHunter.java b/proctor-common/src/main/java/com/indeed/proctor/common/el/filter/NodeHunter.java index f2b5e5ff1..393c30e49 100644 --- a/proctor-common/src/main/java/com/indeed/proctor/common/el/filter/NodeHunter.java +++ b/proctor-common/src/main/java/com/indeed/proctor/common/el/filter/NodeHunter.java @@ -19,6 +19,24 @@ 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 { private final Set initialUnknowns = Collections.newSetFromMap(new IdentityHashMap<>()); private final Map replacements = new IdentityHashMap<>();