fix(audience): log debug instead of warning for missing attribute value (#463)

Author: jaeopt

core-api/src/main/java/com/optimizely/ab/config/audience/match/ExactMatch.java

@@ -1,6 +1,6 @@
 /**
  *
- *    Copyright 2018-2020, Optimizely and contributors
+ *    Copyright 2018-2020, 2022, Optimizely and contributors
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -28,6 +28,8 @@
 class ExactMatch implements Match {
     @Nullable
     public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException {
+        if (attributeValue == null) return null;
+
         if (isValidNumber(attributeValue)) {
             if (isValidNumber(conditionValue)) {
                 return NumberComparator.compareUnsafe(attributeValue, conditionValue) == 0;
@@ -39,7 +41,7 @@ public Boolean eval(Object conditionValue, Object attributeValue) throws Unexpec
             throw new UnexpectedValueTypeException();
         }
 
-        if (attributeValue == null || attributeValue.getClass() != conditionValue.getClass()) {
+        if (attributeValue.getClass() != conditionValue.getClass()) {
             return null;
         }
 

core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionEqualsMatch.java

@@ -1,6 +1,6 @@
 /**
  *
- *    Copyright 2020, Optimizely and contributors
+ *    Copyright 2020, 2022, Optimizely and contributors
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -24,6 +24,7 @@
 class SemanticVersionEqualsMatch implements Match {
     @Nullable
     public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException {
+        if (attributeValue == null) return null;  // stay silent (no WARNING) when attribute value is missing or empty.
         return SemanticVersion.compare(attributeValue, conditionValue) == 0;
     }
 }

core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGEMatch.java

@@ -1,6 +1,6 @@
 /**
  *
- *    Copyright 2020, Optimizely and contributors
+ *    Copyright 2020, 2022, Optimizely and contributors
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -25,6 +25,7 @@
 class SemanticVersionGEMatch implements Match {
     @Nullable
     public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException {
+        if (attributeValue == null) return null;  // stay silent (no WARNING) when attribute value is missing or empty.
         return SemanticVersion.compare(attributeValue, conditionValue) >= 0;
     }
 }

core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGTMatch.java

@@ -1,6 +1,6 @@
 /**
  *
- *    Copyright 2020, Optimizely and contributors
+ *    Copyright 2020, 2022, Optimizely and contributors
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -24,6 +24,7 @@
 class SemanticVersionGTMatch implements Match {
     @Nullable
     public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException {
+        if (attributeValue == null) return null;  // stay silent (no WARNING) when attribute value is missing or empty.
         return SemanticVersion.compare(attributeValue, conditionValue) > 0;
     }
 }

core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLEMatch.java

@@ -1,6 +1,6 @@
 /**
  *
- *    Copyright 2020, Optimizely and contributors
+ *    Copyright 2020, 2022, Optimizely and contributors
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -25,6 +25,7 @@
 class SemanticVersionLEMatch implements Match {
     @Nullable
     public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException {
+        if (attributeValue == null) return null;  // stay silent (no WARNING) when attribute value is missing or empty.
         return SemanticVersion.compare(attributeValue, conditionValue) <= 0;
     }
 }

core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLTMatch.java

@@ -1,6 +1,6 @@
 /**
  *
- *    Copyright 2020, Optimizely and contributors
+ *    Copyright 2020, 2022, Optimizely and contributors
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -24,6 +24,7 @@
 class SemanticVersionLTMatch implements Match {
     @Nullable
     public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException {
+        if (attributeValue == null) return null;  // stay silent (no WARNING) when attribute value is missing or empty.
         return SemanticVersion.compare(attributeValue, conditionValue) < 0;
     }
 }

core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java

@@ -1,6 +1,6 @@
 /**
  *
- *    Copyright 2016-2020, Optimizely and contributors
+ *    Copyright 2016-2020, 2022, Optimizely and contributors
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@
 package com.optimizely.ab.config.audience;
 
 import ch.qos.logback.classic.Level;
+import com.fasterxml.jackson.databind.deser.std.MapEntryDeserializer;
 import com.optimizely.ab.internal.LogbackVerifier;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.junit.Before;
@@ -191,16 +192,35 @@ public void unexpectedAttributeTypeNull() throws Exception {
             "Audience condition \"{name='browser_type', type='custom_attribute', match='gt', value=20}\" evaluated to UNKNOWN because a null value was passed for user attribute \"browser_type\"");
     }
 
-
     /**
-     * Verify that UserAttribute.evaluate returns null on missing attribute value.
+     * Verify that UserAttribute.evaluate returns null (and log debug message) on missing attribute value.
      */
     @Test
-    public void missingAttribute() throws Exception {
-        UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", "gt", 20);
-        assertNull(testInstance.evaluate(null, Collections.EMPTY_MAP));
-        logbackVerifier.expectMessage(Level.DEBUG,
-            "Audience condition \"{name='browser_type', type='custom_attribute', match='gt', value=20}\" evaluated to UNKNOWN because no value was passed for user attribute \"browser_type\"");
+    public void missingAttribute_returnsNullAndLogDebugMessage() throws Exception {
+        // check with all valid value types for each match
+
+        Map<String, Object[]> items = new HashMap<>();
+        items.put("exact", new Object[]{"string", 123, true});
+        items.put("substring", new Object[]{"string"});
+        items.put("gt", new Object[]{123, 5.3});
+        items.put("ge", new Object[]{123, 5.3});
+        items.put("lt", new Object[]{123, 5.3});
+        items.put("le", new Object[]{123, 5.3});
+        items.put("semver_eq", new Object[]{"1.2.3"});
+        items.put("semver_ge", new Object[]{"1.2.3"});
+        items.put("semver_gt", new Object[]{"1.2.3"});
+        items.put("semver_le", new Object[]{"1.2.3"});
+        items.put("semver_lt", new Object[]{"1.2.3"});
+
+        for (Map.Entry<String, Object[]> entry : items.entrySet()) {
+            for (Object value : entry.getValue()) {
+                UserAttribute testInstance = new UserAttribute("n", "custom_attribute", entry.getKey(), value);
+                assertNull(testInstance.evaluate(null, Collections.EMPTY_MAP));
+                String valueStr = (value instanceof String) ? ("'" + value + "'") : value.toString();
+                logbackVerifier.expectMessage(Level.DEBUG,
+                    "Audience condition \"{name='n', type='custom_attribute', match='" + entry.getKey() + "', value=" + valueStr + "}\" evaluated to UNKNOWN because no value was passed for user attribute \"n\"");
+            }
+        }
     }
 
     /**

core-api/src/test/java/com/optimizely/ab/config/audience/match/SemanticVersionTest.java

@@ -1,6 +1,6 @@
 /**
  *
- *    Copyright 2020, Optimizely and contributors
+ *    Copyright 2020, 2022, Optimizely and contributors
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -26,7 +26,6 @@ public class SemanticVersionTest {
     @Rule
     public ExpectedException thrown = ExpectedException.none();
 
-
     @Test
     public void semanticVersionInvalidOnlyDash() throws Exception {
         thrown.expect(Exception.class);
@@ -165,4 +164,16 @@ public void testGreaterThan() throws Exception {
         assertTrue(SemanticVersion.compare("3.7.1-prerelease-prerelease+rc", "3.7.1-prerelease+build") > 0);
         assertTrue(SemanticVersion.compare("3.7.1-beta.2", "3.7.1-beta.1") > 0);
     }
+
+    @Test
+    public void testSilentForNullOrMissingAttributesValues() throws Exception {
+        // SemanticVersionMatcher will throw UnexpectedValueType exception for invalid condition or attribute values (this exception is handled to log WARNING messages).
+        // But, for missing (or null) attribute value, it should not throw the exception.
+        assertNull(new SemanticVersionEqualsMatch().eval("1.2.3", null));
+        assertNull(new SemanticVersionGEMatch().eval("1.2.3", null));
+        assertNull(new SemanticVersionGTMatch().eval("1.2.3", null));
+        assertNull(new SemanticVersionLEMatch().eval("1.2.3", null));
+        assertNull(new SemanticVersionLTMatch().eval("1.2.3", null));
+    }
+
 }