Skip to content

Commit d8df159

Browse files
authored
fix(audience): log debug instead of warning for missing attribute value (#463)
1 parent 206ae8b commit d8df159

File tree

8 files changed

+55
-17
lines changed

8 files changed

+55
-17
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
*
3-
* Copyright 2018-2020, Optimizely and contributors
3+
* Copyright 2018-2020, 2022, Optimizely and contributors
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -28,6 +28,8 @@
2828
class ExactMatch implements Match {
2929
@Nullable
3030
public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException {
31+
if (attributeValue == null) return null;
32+
3133
if (isValidNumber(attributeValue)) {
3234
if (isValidNumber(conditionValue)) {
3335
return NumberComparator.compareUnsafe(attributeValue, conditionValue) == 0;
@@ -39,7 +41,7 @@ public Boolean eval(Object conditionValue, Object attributeValue) throws Unexpec
3941
throw new UnexpectedValueTypeException();
4042
}
4143

42-
if (attributeValue == null || attributeValue.getClass() != conditionValue.getClass()) {
44+
if (attributeValue.getClass() != conditionValue.getClass()) {
4345
return null;
4446
}
4547

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
*
3-
* Copyright 2020, Optimizely and contributors
3+
* Copyright 2020, 2022, Optimizely and contributors
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -24,6 +24,7 @@
2424
class SemanticVersionEqualsMatch implements Match {
2525
@Nullable
2626
public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException {
27+
if (attributeValue == null) return null; // stay silent (no WARNING) when attribute value is missing or empty.
2728
return SemanticVersion.compare(attributeValue, conditionValue) == 0;
2829
}
2930
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
*
3-
* Copyright 2020, Optimizely and contributors
3+
* Copyright 2020, 2022, Optimizely and contributors
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -25,6 +25,7 @@
2525
class SemanticVersionGEMatch implements Match {
2626
@Nullable
2727
public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException {
28+
if (attributeValue == null) return null; // stay silent (no WARNING) when attribute value is missing or empty.
2829
return SemanticVersion.compare(attributeValue, conditionValue) >= 0;
2930
}
3031
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
*
3-
* Copyright 2020, Optimizely and contributors
3+
* Copyright 2020, 2022, Optimizely and contributors
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -24,6 +24,7 @@
2424
class SemanticVersionGTMatch implements Match {
2525
@Nullable
2626
public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException {
27+
if (attributeValue == null) return null; // stay silent (no WARNING) when attribute value is missing or empty.
2728
return SemanticVersion.compare(attributeValue, conditionValue) > 0;
2829
}
2930
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
*
3-
* Copyright 2020, Optimizely and contributors
3+
* Copyright 2020, 2022, Optimizely and contributors
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -25,6 +25,7 @@
2525
class SemanticVersionLEMatch implements Match {
2626
@Nullable
2727
public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException {
28+
if (attributeValue == null) return null; // stay silent (no WARNING) when attribute value is missing or empty.
2829
return SemanticVersion.compare(attributeValue, conditionValue) <= 0;
2930
}
3031
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
*
3-
* Copyright 2020, Optimizely and contributors
3+
* Copyright 2020, 2022, Optimizely and contributors
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -24,6 +24,7 @@
2424
class SemanticVersionLTMatch implements Match {
2525
@Nullable
2626
public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException {
27+
if (attributeValue == null) return null; // stay silent (no WARNING) when attribute value is missing or empty.
2728
return SemanticVersion.compare(attributeValue, conditionValue) < 0;
2829
}
2930
}

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

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
*
3-
* Copyright 2016-2020, Optimizely and contributors
3+
* Copyright 2016-2020, 2022, Optimizely and contributors
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@
1717
package com.optimizely.ab.config.audience;
1818

1919
import ch.qos.logback.classic.Level;
20+
import com.fasterxml.jackson.databind.deser.std.MapEntryDeserializer;
2021
import com.optimizely.ab.internal.LogbackVerifier;
2122
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
2223
import org.junit.Before;
@@ -191,16 +192,35 @@ public void unexpectedAttributeTypeNull() throws Exception {
191192
"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\"");
192193
}
193194

194-
195195
/**
196-
* Verify that UserAttribute.evaluate returns null on missing attribute value.
196+
* Verify that UserAttribute.evaluate returns null (and log debug message) on missing attribute value.
197197
*/
198198
@Test
199-
public void missingAttribute() throws Exception {
200-
UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", "gt", 20);
201-
assertNull(testInstance.evaluate(null, Collections.EMPTY_MAP));
202-
logbackVerifier.expectMessage(Level.DEBUG,
203-
"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\"");
199+
public void missingAttribute_returnsNullAndLogDebugMessage() throws Exception {
200+
// check with all valid value types for each match
201+
202+
Map<String, Object[]> items = new HashMap<>();
203+
items.put("exact", new Object[]{"string", 123, true});
204+
items.put("substring", new Object[]{"string"});
205+
items.put("gt", new Object[]{123, 5.3});
206+
items.put("ge", new Object[]{123, 5.3});
207+
items.put("lt", new Object[]{123, 5.3});
208+
items.put("le", new Object[]{123, 5.3});
209+
items.put("semver_eq", new Object[]{"1.2.3"});
210+
items.put("semver_ge", new Object[]{"1.2.3"});
211+
items.put("semver_gt", new Object[]{"1.2.3"});
212+
items.put("semver_le", new Object[]{"1.2.3"});
213+
items.put("semver_lt", new Object[]{"1.2.3"});
214+
215+
for (Map.Entry<String, Object[]> entry : items.entrySet()) {
216+
for (Object value : entry.getValue()) {
217+
UserAttribute testInstance = new UserAttribute("n", "custom_attribute", entry.getKey(), value);
218+
assertNull(testInstance.evaluate(null, Collections.EMPTY_MAP));
219+
String valueStr = (value instanceof String) ? ("'" + value + "'") : value.toString();
220+
logbackVerifier.expectMessage(Level.DEBUG,
221+
"Audience condition \"{name='n', type='custom_attribute', match='" + entry.getKey() + "', value=" + valueStr + "}\" evaluated to UNKNOWN because no value was passed for user attribute \"n\"");
222+
}
223+
}
204224
}
205225

206226
/**

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
*
3-
* Copyright 2020, Optimizely and contributors
3+
* Copyright 2020, 2022, Optimizely and contributors
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -26,7 +26,6 @@ public class SemanticVersionTest {
2626
@Rule
2727
public ExpectedException thrown = ExpectedException.none();
2828

29-
3029
@Test
3130
public void semanticVersionInvalidOnlyDash() throws Exception {
3231
thrown.expect(Exception.class);
@@ -165,4 +164,16 @@ public void testGreaterThan() throws Exception {
165164
assertTrue(SemanticVersion.compare("3.7.1-prerelease-prerelease+rc", "3.7.1-prerelease+build") > 0);
166165
assertTrue(SemanticVersion.compare("3.7.1-beta.2", "3.7.1-beta.1") > 0);
167166
}
167+
168+
@Test
169+
public void testSilentForNullOrMissingAttributesValues() throws Exception {
170+
// SemanticVersionMatcher will throw UnexpectedValueType exception for invalid condition or attribute values (this exception is handled to log WARNING messages).
171+
// But, for missing (or null) attribute value, it should not throw the exception.
172+
assertNull(new SemanticVersionEqualsMatch().eval("1.2.3", null));
173+
assertNull(new SemanticVersionGEMatch().eval("1.2.3", null));
174+
assertNull(new SemanticVersionGTMatch().eval("1.2.3", null));
175+
assertNull(new SemanticVersionLEMatch().eval("1.2.3", null));
176+
assertNull(new SemanticVersionLTMatch().eval("1.2.3", null));
177+
}
178+
168179
}

0 commit comments

Comments
 (0)