Skip to content

Commit ee7d5fd

Browse files
gtheagthea
authored
Change FT sanitizer log level and message (#808)
1 parent 5addd46 commit ee7d5fd

File tree

3 files changed

+77
-4
lines changed

3 files changed

+77
-4
lines changed

‎src/main/java/io/split/android/client/fallback/FallbacksSanitizerImpl.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ private FallbackTreatment sanitizeGlobalTreatment(@Nullable FallbackTreatment gl
5151
}
5252

5353
if (!isValidTreatment(global)) {
54-
Logger.w("Discarded global fallback: Invalid treatment (max " + MAX_TREATMENT_LENGTH + " chars)");
54+
Logger.e("Discarded global fallback: Invalid treatment (max " + MAX_TREATMENT_LENGTH + " chars and comply with " + TREATMENT_REGEXP + ")");
5555
return null;
5656
}
5757

@@ -70,12 +70,12 @@ private Map<String, FallbackTreatment> sanitizeByFlagTreatments(Map<String, Fall
7070
FallbackTreatment treatment = entry.getValue();
7171

7272
if (!isValidFlagName(flagName)) {
73-
Logger.w("Discarded flag '" + flagName + "': Invalid flag name (max " + MAX_FLAG_NAME_LENGTH + " chars, no spaces)");
73+
Logger.e("Discarded flag '" + flagName + "': Invalid flag name (max " + MAX_FLAG_NAME_LENGTH + " chars, no spaces)");
7474
continue;
7575
}
7676

7777
if (!isValidTreatment(treatment)) {
78-
Logger.w("Discarded treatment for flag '" + flagName + "': Invalid treatment (max " + MAX_TREATMENT_LENGTH + " chars)");
78+
Logger.e("Discarded treatment for flag '" + flagName + "': Invalid treatment (max " + MAX_TREATMENT_LENGTH + " chars and comply with " + TREATMENT_REGEXP + ")");
7979
continue;
8080
}
8181

‎src/test/java/io/split/android/client/fallback/FallbacksSanitizerImplTest.java‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,18 @@
77
import org.junit.Before;
88
import org.junit.Test;
99

10+
import java.util.Deque;
1011
import java.util.HashMap;
1112
import java.util.Map;
1213

14+
import io.split.android.client.utils.logger.LogPrinterStub;
15+
import io.split.android.client.utils.logger.Logger;
16+
import io.split.android.client.utils.logger.SplitLogLevel;
17+
1318
public class FallbacksSanitizerImplTest {
1419

1520
private FallbacksSanitizerImpl mSanitizer;
21+
private LogPrinterStub mLogPrinter;
1622

1723
private static final String VALID_FLAG = "my_flag";
1824
private static final String INVALID_FLAG_WITH_SPACE = "my flag";
@@ -26,6 +32,9 @@ public class FallbacksSanitizerImplTest {
2632
@Before
2733
public void setUp() {
2834
mSanitizer = new FallbacksSanitizerImpl();
35+
mLogPrinter = new LogPrinterStub();
36+
Logger.instance().setLevel(SplitLogLevel.VERBOSE);
37+
Logger.instance().setPrinter(mLogPrinter);
2938
}
3039

3140
@Test
@@ -43,7 +52,15 @@ public void dropsInvalidFlagNamesAndTreatments() {
4352

4453
FallbackConfiguration sanitized = mSanitizer.sanitize(config);
4554

46-
// only VALID_FLAG should remain
55+
Deque<String> errors = mLogPrinter.getLoggedMessages().get(android.util.Log.ERROR);
56+
assertTrue("Expected ERROR logs to be present", errors != null && !errors.isEmpty());
57+
long invalidFlagNameCount = errors.stream().filter(m -> m.contains("Invalid flag name")).count();
58+
assertEquals(2, invalidFlagNameCount);
59+
assertTrue(errors.stream().anyMatch(m -> m.contains("Discarded flag 'my flag'")));
60+
// invalid treatment for a specific flag name and contains the full expected message
61+
assertTrue(errors.stream().anyMatch(m -> m.contains("Discarded treatment for flag 'tooLongTreatment'")));
62+
assertTrue(errors.stream().anyMatch(m -> m.contains("Invalid treatment (max 100 chars and comply with ^[0-9]+[.a-zA-Z0-9_-]*$|^[a-zA-Z]+[a-zA-Z0-9_-]*$)")));
63+
4764
assertEquals(1, sanitized.getByFlag().size());
4865
assertEquals("on", sanitized.getByFlag().get(VALID_FLAG).getTreatment());
4966
}
@@ -57,6 +74,11 @@ public void dropsInvalidGlobalTreatment() {
5774

5875
FallbackConfiguration sanitized = mSanitizer.sanitize(config);
5976

77+
// Assert error log for discarded global fallback only
78+
Deque<String> errors = mLogPrinter.getLoggedMessages().get(android.util.Log.ERROR);
79+
assertTrue("Expected ERROR logs to be present", errors != null && !errors.isEmpty());
80+
assertTrue(errors.stream().anyMatch(m -> m.contains("Discarded global fallback")));
81+
6082
assertNull(sanitized.getGlobal());
6183
assertEquals(0, sanitized.getByFlag().size());
6284
}
@@ -75,6 +97,15 @@ public void byFlagTreatmentIsDroppedWhenInvalidFormat() {
7597

7698
FallbackConfiguration sanitized = mSanitizer.sanitize(config);
7799

100+
// Assert error logs for invalid treatments under flags
101+
Deque<String> errors = mLogPrinter.getLoggedMessages().get(android.util.Log.ERROR);
102+
assertTrue("Expected ERROR logs to be present", errors != null && !errors.isEmpty());
103+
assertTrue(errors.stream().anyMatch(m -> m.contains("Discarded treatment for flag '" + VALID_FLAG + "'")));
104+
assertTrue(errors.stream().anyMatch(m -> m.contains("Invalid treatment (max 100 chars and comply with ^[0-9]+[.a-zA-Z0-9_-]*$|^[a-zA-Z]+[a-zA-Z0-9_-]*$)")));
105+
assertTrue(errors.stream().anyMatch(m -> m.contains("Discarded treatment for flag 'null_treatment'")));
106+
// Ensure no error for valid flag/treatment
107+
assertTrue(errors.stream().noneMatch(m -> m.contains("Discarded treatment for flag 'valid_num_dot'")));
108+
78109
// Only the valid one should remain
79110
assertEquals(1, sanitized.getByFlag().size());
80111
assertEquals("123.on", sanitized.getByFlag().get("valid_num_dot").getTreatment());
@@ -94,6 +125,22 @@ public void globalTreatmentIsDroppedWhenInvalidFormat() {
94125

95126
FallbackConfiguration sanitized = mSanitizer.sanitize(config);
96127

128+
// Assert error logs were emitted for invalid entries
129+
Deque<String> errorLogs = mLogPrinter.getLoggedMessages().get(android.util.Log.ERROR);
130+
assertTrue("Expected ERROR logs to be present", errorLogs != null && !errorLogs.isEmpty());
131+
boolean hasGlobalDiscard = false;
132+
boolean hasNullFlagDiscard = false;
133+
for (String msg : errorLogs) {
134+
if (msg.contains("Discarded global fallback")) {
135+
hasGlobalDiscard = true;
136+
}
137+
if (msg.contains("Discarded treatment for flag 'null_treatment'")) {
138+
hasNullFlagDiscard = true;
139+
}
140+
}
141+
assertTrue("Expected an error about discarded global fallback", hasGlobalDiscard);
142+
assertTrue("Expected an error about discarded treatment for flag 'null_treatment'", hasNullFlagDiscard);
143+
97144
assertNull(sanitized.getGlobal());
98145
// Ensure only the valid by-flag entry is preserved
99146
assertEquals(1, sanitized.getByFlag().size());
@@ -118,5 +165,9 @@ public void validFormatTreatmentIsNotDropped() {
118165
assertEquals("123.on", sanitized.getByFlag().get("numWithDot").getTreatment());
119166
assertEquals("on_1-2", sanitized.getByFlag().get(VALID_FLAG).getTreatment());
120167
assertEquals("on", sanitized.getGlobal().getTreatment());
168+
169+
// No ERROR logs expected for valid-only case
170+
Deque<String> errors4 = mLogPrinter.getLoggedMessages().get(android.util.Log.ERROR);
171+
assertTrue(errors4 == null || errors4.isEmpty());
121172
}
122173
}

‎src/test/java/io/split/android/client/utils/logger/LogPrinterStub.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,43 +2,65 @@
22

33
import android.util.Log;
44

5+
import java.util.HashMap;
56
import java.util.HashSet;
7+
import java.util.Map;
68
import java.util.Set;
9+
import java.util.concurrent.ConcurrentHashMap;
10+
import java.util.concurrent.ConcurrentLinkedDeque;
711

812
public class LogPrinterStub implements LogPrinter {
913
private final Set<Integer> calls = new HashSet<>();
14+
private final Map<Integer, ConcurrentLinkedDeque<String>> logs = new ConcurrentHashMap<>();
15+
16+
public LogPrinterStub() {
17+
// Initialize for all Android log levels: VERBOSE(2) .. ASSERT(7)
18+
for (int level = Log.VERBOSE; level <= Log.ASSERT; level++) {
19+
logs.put(level, new ConcurrentLinkedDeque<>());
20+
}
21+
}
1022

1123
@Override
1224
public void v(String tag, String msg, Throwable tr) {
25+
logs.get(Log.VERBOSE).add(msg);
1326
calls.add(Log.VERBOSE);
1427
}
1528

1629
@Override
1730
public void d(String tag, String msg, Throwable tr) {
31+
logs.get(Log.DEBUG).add(msg);
1832
calls.add(Log.DEBUG);
1933
}
2034

2135
@Override
2236
public void i(String tag, String msg, Throwable tr) {
37+
logs.get(Log.INFO).add(msg);
2338
calls.add(Log.INFO);
2439
}
2540

2641
@Override
2742
public void w(String tag, String msg, Throwable tr) {
43+
logs.get(Log.WARN).add(msg);
2844
calls.add(Log.WARN);
2945
}
3046

3147
@Override
3248
public void e(String tag, String msg, Throwable tr) {
49+
logs.get(Log.ERROR).add(msg);
3350
calls.add(Log.ERROR);
3451
}
3552

3653
@Override
3754
public void wtf(String tag, String msg, Throwable tr) {
55+
logs.get(Log.ASSERT).add(msg);
3856
calls.add(Log.ASSERT);
3957
}
4058

4159
public boolean isCalled(Integer type) {
4260
return calls.contains(type);
4361
}
62+
63+
public Map<Integer, ConcurrentLinkedDeque<String>> getLoggedMessages() {
64+
return new HashMap<>(logs);
65+
}
4466
}

0 commit comments

Comments
 (0)