Merge pull request #59 from launchdarkly/eb/ch15275/bad-operator-npe

treat unsupported operators as a non-match, don't throw NPE

Author: eli-darkly

src/main/java/com/launchdarkly/client/Clause.java

@@ -75,9 +75,11 @@ boolean matchesUser(FeatureStore store, LDUser user) {
   }
 
   private boolean matchAny(JsonPrimitive userValue) {
-    for (JsonPrimitive v : values) {
-      if (op.apply(userValue, v)) {
-        return true;
+    if (op != null) {
+      for (JsonPrimitive v : values) {
+        if (op.apply(userValue, v)) {
+          return true;
+        }
       }
     }
     return false;

src/test/java/com/launchdarkly/client/FeatureFlagTest.java

@@ -1,5 +1,11 @@
 package com.launchdarkly.client;
 
+import com.google.gson.Gson;
+import com.google.gson.JsonArray;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
+
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -13,6 +19,7 @@
 import static com.launchdarkly.client.VersionedDataKind.FEATURES;
 import static com.launchdarkly.client.VersionedDataKind.SEGMENTS;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 
 public class FeatureFlagTest {
@@ -243,6 +250,48 @@ public void clauseCanBeNegated() throws Exception {
     assertEquals(jbool(false), f.evaluate(user, featureStore).getValue());
   }
 
+  @Test
+  public void clauseWithUnsupportedOperatorStringIsUnmarshalledWithNullOperator() throws Exception {
+    // This just verifies that GSON will give us a null in this case instead of throwing an exception,
+    // so we fail as gracefully as possible if a new operator type has been added in the application
+    // and the SDK hasn't been upgraded yet.
+    String badClauseJson = "{\"attribute\":\"name\",\"operator\":\"doesSomethingUnsupported\",\"values\":[\"x\"]}";
+    Gson gson = new Gson();
+    Clause clause = gson.fromJson(badClauseJson, Clause.class);
+    assertNotNull(clause);
+    
+    JsonElement json = gson.toJsonTree(clause);
+    String expectedJson = "{\"attribute\":\"name\",\"values\":[\"x\"],\"negate\":false}";
+    assertEquals(gson.fromJson(expectedJson, JsonElement.class), json);
+  }
+  
+  @Test
+  public void clauseWithNullOperatorDoesNotMatch() throws Exception {
+    Clause badClause = new Clause("name", null, Arrays.asList(js("Bob")), false);
+    FeatureFlag f = booleanFlagWithClauses(badClause);
+    LDUser user = new LDUser.Builder("key").name("Bob").build();
+    
+    assertEquals(jbool(false), f.evaluate(user, featureStore).getValue());
+  }
+  
+  @Test
+  public void clauseWithNullOperatorDoesNotStopSubsequentRuleFromMatching() throws Exception {
+    Clause badClause = new Clause("name", null, Arrays.asList(js("Bob")), false);
+    Rule badRule = new Rule(Arrays.asList(badClause), 1, null);
+    Clause goodClause = new Clause("name", Operator.in, Arrays.asList(js("Bob")), false);
+    Rule goodRule = new Rule(Arrays.asList(goodClause), 1, null);
+    FeatureFlag f = new FeatureFlagBuilder("feature")
+        .on(true)
+        .rules(Arrays.asList(badRule, goodRule))
+        .fallthrough(fallthroughVariation(0))
+        .offVariation(0)
+        .variations(jbool(false), jbool(true))
+        .build();
+    LDUser user = new LDUser.Builder("key").name("Bob").build();
+    
+    assertEquals(jbool(true), f.evaluate(user, featureStore).getValue());
+  }
+  
   @Test
   public void testSegmentMatchClauseRetrievesSegmentFromStore() throws Exception {
     Segment segment = new Segment.Builder("segkey")