Merge pull request #158 from zacharygoodwin/PROC-1444

PROC-1444: Check nested map values for rule evaluation
indeedeng · Mar 6, 2024 · 60df00f · 60df00f
2 parents 7b86574 + 2fa5320
commit 60df00f
Show file tree

Hide file tree

Showing 3 changed files with 105 additions and 0 deletions.
diff --git a/proctor-common/src/main/java/com/indeed/proctor/common/RuleEvaluator.java b/proctor-common/src/main/java/com/indeed/proctor/common/RuleEvaluator.java
@@ -21,9 +21,11 @@
 import javax.el.ListELResolver;
 import javax.el.MapELResolver;
 import javax.el.ValueExpression;
+import javax.el.ValueReference;
 import javax.el.VariableMapper;
 import java.util.HashSet;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 
 /**
@@ -229,6 +231,26 @@ static void checkRuleIsBooleanType(
         // To support users writing rules, be more strict here in requiring the type of the
         // value to be expected before coercion
         Class<?> type = ve.getType(elContext);
+
+        // if object is map check what the base value type is
+        if (Objects.equals(type, Object.class)) {
+            final ValueReference valueReference = ve.getValueReference(elContext);
+            final Object base = valueReference.getBase();
+            final Object property = valueReference.getProperty();
+            try {
+                if (base instanceof Map<?, ?>) {
+                    final Map<?, ?> map = (Map<?, ?>) base;
+                    type = map.get(property).getClass();
+                }
+            } catch (final NullPointerException e) {
+                throw new IllegalArgumentException(
+                        "Received nested value which does not exist in context map: value "
+                                + property
+                                + " in rule "
+                                + rule);
+            }
+        }
+
         if (ClassUtils.isPrimitiveWrapper(type)) {
             type = ClassUtils.wrapperToPrimitive(type);
         }

diff --git a/proctor-common/src/test/java/com/indeed/proctor/common/TestRuleEvaluator.java b/proctor-common/src/test/java/com/indeed/proctor/common/TestRuleEvaluator.java
@@ -9,6 +9,7 @@
 
 import javax.el.ELException;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.Map;
 
 import static java.util.Collections.emptyMap;
@@ -187,6 +188,78 @@ public void testElExpressionsShouldBeAvailable() {
         }
     }
 
+    @Test
+    public void testElExpressionsShouldBeAvailableNestedMap() {
+        final Map<String, Boolean> value = new HashMap<>();
+        value.put("bar", true);
+        final Map<String, Map<String, Object>> traits =
+                singletonMap("baz", singletonMap("alpha", true));
+        final Map<String, Object> values = new HashMap<>();
+        values.put("foo", value);
+        values.put("traits", traits);
+        {
+            final String rule = "${foo.bar == true}";
+            assertTrue(
+                    "rule '" + rule + "' should be true for " + values,
+                    ruleEvaluator.evaluateBooleanRule(rule, values));
+        }
+        {
+            final String rule = "${foo.bar}";
+            assertTrue(
+                    "rule '" + rule + "' should be true for " + values,
+                    ruleEvaluator.evaluateBooleanRule(rule, values));
+        }
+        {
+            final String rule = "${!foo.bar}";
+            assertFalse(
+                    "rule '" + rule + "' should be true for " + values,
+                    ruleEvaluator.evaluateBooleanRule(rule, values));
+        }
+        {
+            final String rule = "${traits.baz.alpha}";
+            assertTrue(
+                    "rule '" + rule + "' should be true for " + values,
+                    ruleEvaluator.evaluateBooleanRule(rule, values));
+        }
+
+        // check false cases
+        value.put("bar", false);
+        {
+            final String rule = "${foo.bar == true}";
+            assertFalse(
+                    "rule '" + rule + "' should be true for " + values,
+                    ruleEvaluator.evaluateBooleanRule(rule, values));
+        }
+        {
+            final String rule = "${foo.bar}";
+            assertFalse(
+                    "rule '" + rule + "' should be true for " + values,
+                    ruleEvaluator.evaluateBooleanRule(rule, values));
+        }
+    }
+
+    @Test
+    public void testElExpressionsShouldBeAvailableNestedMapErrors() {
+        final Map<String, Map<String, String>> traits =
+                singletonMap("baz", singletonMap("alpha", "123"));
+        final Map<String, Object> values = new HashMap<>();
+        values.put("traits", traits);
+        {
+            final String rule = "${traits.baz.alpha}";
+            assertThatThrownBy(() -> ruleEvaluator.evaluateBooleanRule(rule, values))
+                    .isInstanceOf(IllegalArgumentException.class)
+                    .hasMessageContaining(
+                            "Received non-boolean return value: class java.lang.String from rule");
+        }
+        {
+            final String rule = "${traits.fakeValue}";
+            assertThatThrownBy(() -> ruleEvaluator.evaluateBooleanRule(rule, values))
+                    .isInstanceOf(IllegalArgumentException.class)
+                    .hasMessageContaining(
+                            "Received nested value which does not exist in context map: value fakeValue in rule ${traits.fakeValue}");
+        }
+    }
+
     public static class Temp {
         public String getY() {
             return "barY";

diff --git a/proctor-common/src/test/java/com/indeed/proctor/common/TestRuleVerifyUtils.java b/proctor-common/src/test/java/com/indeed/proctor/common/TestRuleVerifyUtils.java
@@ -112,6 +112,16 @@ public void testVerifyRulesNormal() {
                 new String[] {});
     }
 
+    @Test
+    public void testVerifyRulesNestedMapBoolean() {
+        expectValidRule(
+                "${foo.bar}",
+                new Object[][] {
+                    {"foo", Collections.singletonMap("bar", true)},
+                },
+                new String[] {});
+    }
+
     @Test
     public void testValidRulesWithMethodCall() {
         expectValidRule(