diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ce831b55..7151e91d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,11 @@ All notable changes to the LaunchDarkly Java SDK will be documented in this file _This release was broken and should not be used._ +## [2.6.1] - 2018-03-01 +### Fixed +- Improved performance when evaluating flags with custom attributes, by avoiding an unnecessary caught exception (thanks, [rbalamohan](https://github.com/launchdarkly/java-client/issues/113)). + + ## [2.6.0] - 2018-02-12 ## Added - Adds support for a future LaunchDarkly feature, coming soon: semantic version user attributes. diff --git a/gradle.properties b/gradle.properties index 452e60520..ecd500f40 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,3 +1,3 @@ -version=3.0.2 +version=3.0.3 ossrhUsername= ossrhPassword= diff --git a/src/main/java/com/launchdarkly/client/Clause.java b/src/main/java/com/launchdarkly/client/Clause.java index fa4055fb6..65cb756dc 100644 --- a/src/main/java/com/launchdarkly/client/Clause.java +++ b/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; diff --git a/src/main/java/com/launchdarkly/client/FeatureFlagBuilder.java b/src/main/java/com/launchdarkly/client/FeatureFlagBuilder.java index 120764862..d06c903d6 100644 --- a/src/main/java/com/launchdarkly/client/FeatureFlagBuilder.java +++ b/src/main/java/com/launchdarkly/client/FeatureFlagBuilder.java @@ -3,6 +3,7 @@ import com.google.gson.JsonElement; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; class FeatureFlagBuilder { @@ -84,6 +85,10 @@ FeatureFlagBuilder variations(List variations) { return this; } + FeatureFlagBuilder variations(JsonElement... variations) { + return variations(Arrays.asList(variations)); + } + FeatureFlagBuilder deleted(boolean deleted) { this.deleted = deleted; return this; diff --git a/src/main/java/com/launchdarkly/client/RedisFeatureStore.java b/src/main/java/com/launchdarkly/client/RedisFeatureStore.java index 90e91b3f3..1065269aa 100644 --- a/src/main/java/com/launchdarkly/client/RedisFeatureStore.java +++ b/src/main/java/com/launchdarkly/client/RedisFeatureStore.java @@ -1,19 +1,6 @@ package com.launchdarkly.client; -import static com.launchdarkly.client.VersionedDataKind.FEATURES; - -import java.io.IOException; -import java.net.URI; -import java.util.HashMap; -import java.util.Map; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Optional; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; @@ -24,6 +11,20 @@ import com.google.common.util.concurrent.ThreadFactoryBuilder; import com.google.gson.Gson; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; + +import static com.launchdarkly.client.VersionedDataKind.FEATURES; + import redis.clients.jedis.Jedis; import redis.clients.jedis.JedisPool; import redis.clients.jedis.JedisPoolConfig; @@ -38,12 +39,15 @@ public class RedisFeatureStore implements FeatureStore { private static final String DEFAULT_PREFIX = "launchdarkly"; private static final String INIT_KEY = "$initialized$"; private static final String CACHE_REFRESH_THREAD_POOL_NAME_FORMAT = "RedisFeatureStore-cache-refresher-pool-%d"; + private static final Gson gson = new Gson(); + private final JedisPool pool; private LoadingCache> cache; private final LoadingCache initCache = createInitCache(); private String prefix; private ListeningExecutorService executorService; - + private UpdateListener updateListener; + private static class CacheKey { final VersionedDataKind kind; final String key; @@ -102,10 +106,6 @@ private void setPrefix(String prefix) { } } - private void createCache(long cacheTimeSecs) { - createCache(cacheTimeSecs, false, false); - } - private void createCache(long cacheTimeSecs, boolean refreshStaleValues, boolean asyncRefresh) { if (cacheTimeSecs > 0) { if (refreshStaleValues) { @@ -120,7 +120,9 @@ private CacheLoader> createDefaultCacheLoader( return new CacheLoader>() { @Override public Optional load(CacheKey key) throws Exception { - return Optional.fromNullable(getRedis(key.kind, key.key)); + try (Jedis jedis = pool.getResource()) { + return Optional.fromNullable(getRedisEvenIfDeleted(key.kind, key.key, jedis)); + } } }; } @@ -169,7 +171,13 @@ public T get(VersionedDataKind kind, String key) { if (cache != null) { item = (T) cache.getUnchecked(new CacheKey(kind, key)).orNull(); } else { - item = getRedis(kind, key); + try (Jedis jedis = pool.getResource()) { + item = getRedisEvenIfDeleted(kind, key, jedis); + } + } + if (item != null && item.isDeleted()) { + logger.debug("[get] Key: {} has been deleted in \"{}\". Returning null", key, kind.getNamespace()); + return null; } if (item != null) { logger.debug("[get] Key: {} with version: {} found in \"{}\".", key, item.getVersion(), kind.getNamespace()); @@ -182,7 +190,6 @@ public Map all(VersionedDataKind kind) { try (Jedis jedis = pool.getResource()) { Map allJson = jedis.hgetAll(itemsKey(kind)); Map result = new HashMap<>(); - Gson gson = new Gson(); for (Map.Entry entry : allJson.entrySet()) { T item = gson.fromJson(entry.getValue(), kind.getItemClass()); @@ -197,7 +204,6 @@ public Map all(VersionedDataKind kind) { @Override public void init(Map, Map> allData) { try (Jedis jedis = pool.getResource()) { - Gson gson = new Gson(); Transaction t = jedis.multi(); for (Map.Entry, Map> entry: allData.entrySet()) { @@ -216,63 +222,54 @@ public void init(Map, Map> @Override public void delete(VersionedDataKind kind, String key, int version) { - Jedis jedis = null; - try { - Gson gson = new Gson(); - jedis = pool.getResource(); - String baseKey = itemsKey(kind); - jedis.watch(baseKey); - - VersionedData item = getRedis(kind, key, jedis); - - if (item != null && item.getVersion() >= version) { - logger.warn("Attempted to delete key: {} version: {}" + - " with a version that is the same or older: {} in \"{}\"", - key, item.getVersion(), version, kind.getNamespace()); - return; - } - - VersionedData deletedItem = kind.makeDeletedItem(key, version); - jedis.hset(baseKey, key, gson.toJson(deletedItem)); - - if (cache != null) { - cache.invalidate(new CacheKey(kind, key)); - } - } finally { - if (jedis != null) { - jedis.unwatch(); - jedis.close(); - } - } + T deletedItem = kind.makeDeletedItem(key, version); + updateItemWithVersioning(kind, deletedItem); } - + @Override public void upsert(VersionedDataKind kind, T item) { - Jedis jedis = null; - try { - jedis = pool.getResource(); - Gson gson = new Gson(); - String baseKey = itemsKey(kind); - jedis.watch(baseKey); - - VersionedData old = getRedisEvenIfDeleted(kind, item.getKey(), jedis); - - if (old != null && old.getVersion() >= item.getVersion()) { - logger.warn("Attempted to update key: {} version: {}" + - " with a version that is the same or older: {} in \"{}\"", - item.getKey(), old.getVersion(), item.getVersion(), kind.getNamespace()); - return; - } - - jedis.hset(baseKey, item.getKey(), gson.toJson(item)); + updateItemWithVersioning(kind, item); + } - if (cache != null) { - cache.invalidate(new CacheKey(kind, item.getKey())); - } - } finally { - if (jedis != null) { - jedis.unwatch(); - jedis.close(); + private void updateItemWithVersioning(VersionedDataKind kind, T newItem) { + while (true) { + Jedis jedis = null; + try { + jedis = pool.getResource(); + String baseKey = itemsKey(kind); + jedis.watch(baseKey); + + if (updateListener != null) { + updateListener.aboutToUpdate(baseKey, newItem.getKey()); + } + + VersionedData oldItem = getRedisEvenIfDeleted(kind, newItem.getKey(), jedis); + + if (oldItem != null && oldItem.getVersion() >= newItem.getVersion()) { + logger.debug("Attempted to {} key: {} version: {}" + + " with a version that is the same or older: {} in \"{}\"", + newItem.isDeleted() ? "delete" : "update", + newItem.getKey(), oldItem.getVersion(), newItem.getVersion(), kind.getNamespace()); + return; + } + + Transaction tx = jedis.multi(); + tx.hset(baseKey, newItem.getKey(), gson.toJson(newItem)); + List result = tx.exec(); + if (result.isEmpty()) { + // if exec failed, it means the watch was triggered and we should retry + logger.debug("Concurrent modification detected, retrying"); + continue; + } + + if (cache != null) { + cache.invalidate(new CacheKey(kind, newItem.getKey())); + } + } finally { + if (jedis != null) { + jedis.unwatch(); + jedis.close(); + } } } } @@ -323,23 +320,7 @@ private Boolean getInit() { } } - private T getRedis(VersionedDataKind kind, String key) { - try (Jedis jedis = pool.getResource()) { - return getRedis(kind, key, jedis); - } - } - - private T getRedis(VersionedDataKind kind, String key, Jedis jedis) { - T item = getRedisEvenIfDeleted(kind, key, jedis); - if (item != null && item.isDeleted()) { - logger.debug("[get] Key: {} has been deleted in \"{}\". Returning null", key, kind.getNamespace()); - return null; - } - return item; - } - private T getRedisEvenIfDeleted(VersionedDataKind kind, String key, Jedis jedis) { - Gson gson = new Gson(); String json = jedis.hget(itemsKey(kind), key); if (json == null) { @@ -354,4 +335,12 @@ private static JedisPoolConfig getPoolConfig() { return new JedisPoolConfig(); } + static interface UpdateListener { + void aboutToUpdate(String baseKey, String itemKey); + } + + @VisibleForTesting + void setUpdateListener(UpdateListener updateListener) { + this.updateListener = updateListener; + } } diff --git a/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java b/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java index ebebffbf8..d5c59d101 100644 --- a/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java +++ b/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java @@ -16,7 +16,7 @@ * {@link RedisFeatureStoreBuilder} calls can be chained, enabling the following pattern: * * 
- * RedisFeatureStoreBuilder builder = new RedisFeatureStoreBuilder("host", 443, 60)
+ * RedisFeatureStore store = new RedisFeatureStoreBuilder("host", 443, 60)
  *      .refreshStaleValues(true)
  *      .asyncRefresh(true)
  *      .socketTimeout(200)
diff --git a/src/test/java/com/launchdarkly/client/FeatureFlagTest.java b/src/test/java/com/launchdarkly/client/FeatureFlagTest.java
index 9f4c568d8..522c86bc1 100644
--- a/src/test/java/com/launchdarkly/client/FeatureFlagTest.java
+++ b/src/test/java/com/launchdarkly/client/FeatureFlagTest.java
@@ -1,7 +1,9 @@
 package com.launchdarkly.client;
 
+import com.google.gson.Gson;
+import com.google.gson.JsonArray;
 import com.google.gson.JsonElement;
-import com.google.gson.JsonPrimitive;
+import com.google.gson.JsonObject;
 
 import org.junit.Assert;
 import org.junit.Before;
@@ -9,12 +11,21 @@
 
 import java.util.Arrays;
 
+import static com.launchdarkly.client.TestUtil.booleanFlagWithClauses;
+import static com.launchdarkly.client.TestUtil.fallthroughVariation;
+import static com.launchdarkly.client.TestUtil.jbool;
+import static com.launchdarkly.client.TestUtil.jint;
+import static com.launchdarkly.client.TestUtil.js;
 import static com.launchdarkly.client.VersionedDataKind.FEATURES;
 import static com.launchdarkly.client.VersionedDataKind.SEGMENTS;
-import static java.util.Collections.singletonList;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 
 public class FeatureFlagTest {
 
+  private static LDUser BASE_USER = new LDUser.Builder("x").build();
+  
   private FeatureStore featureStore;
 
   @Before
@@ -23,51 +34,264 @@ public void before() {
   }
 
   @Test
-  public void testPrereqDoesNotExist() throws EvaluationException {
-    String keyA = "keyA";
-    String keyB = "keyB";
-    FeatureFlag f1 = newFlagWithPrereq(keyA, keyB);
-
-    featureStore.upsert(FEATURES, f1);
-    LDUser user = new LDUser.Builder("userKey").build();
-    FeatureFlag.EvalResult actual = f1.evaluate(user, featureStore);
-
-    Assert.assertNull(actual.getValue());
-    Assert.assertNotNull(actual.getPrerequisiteEvents());
-    Assert.assertEquals(0, actual.getPrerequisiteEvents().size());
+  public void flagReturnsOffVariationIfFlagIsOff() throws Exception {
+    FeatureFlag f = new FeatureFlagBuilder("feature")
+        .on(false)
+        .offVariation(1)
+        .fallthrough(fallthroughVariation(0))
+        .variations(js("fall"), js("off"), js("on"))
+        .build();
+    FeatureFlag.EvalResult result = f.evaluate(BASE_USER, featureStore);
+    
+    assertEquals(js("off"), result.getValue());
+    assertEquals(0, result.getPrerequisiteEvents().size());
   }
 
   @Test
-  public void testPrereqCollectsEventsForPrereqs() throws EvaluationException {
-    String keyA = "keyA";
-    String keyB = "keyB";
-    String keyC = "keyC";
-    FeatureFlag flagA = newFlagWithPrereq(keyA, keyB);
-    FeatureFlag flagB = newFlagWithPrereq(keyB, keyC);
-    FeatureFlag flagC = newFlagOff(keyC);
-
-    featureStore.upsert(FEATURES, flagA);
-    featureStore.upsert(FEATURES, flagB);
-    featureStore.upsert(FEATURES, flagC);
-
-    LDUser user = new LDUser.Builder("userKey").build();
+  public void flagReturnsNullIfFlagIsOffAndOffVariationIsUnspecified() throws Exception {
+    FeatureFlag f = new FeatureFlagBuilder("feature")
+        .on(false)
+        .fallthrough(fallthroughVariation(0))
+        .variations(js("fall"), js("off"), js("on"))
+        .build();
+    FeatureFlag.EvalResult result = f.evaluate(BASE_USER, featureStore);
+    
+    assertNull(result.getValue());
+    assertEquals(0, result.getPrerequisiteEvents().size());
+  }
+  
+  @Test
+  public void flagReturnsOffVariationIfPrerequisiteIsNotFound() throws Exception {
+    FeatureFlag f0 = new FeatureFlagBuilder("feature0")
+        .on(true)
+        .prerequisites(Arrays.asList(new Prerequisite("feature1", 1)))
+        .fallthrough(fallthroughVariation(0))
+        .offVariation(1)
+        .variations(js("fall"), js("off"), js("on"))
+        .build();
+    FeatureFlag.EvalResult result = f0.evaluate(BASE_USER, featureStore);
+    
+    assertEquals(js("off"), result.getValue());
+    assertEquals(0, result.getPrerequisiteEvents().size());
+  }
+  
+  @Test
+  public void flagReturnsOffVariationAndEventIfPrerequisiteIsNotMet() throws Exception {
+    FeatureFlag f0 = new FeatureFlagBuilder("feature0")
+        .on(true)
+        .prerequisites(Arrays.asList(new Prerequisite("feature1", 1)))
+        .fallthrough(fallthroughVariation(0))
+        .offVariation(1)
+        .variations(js("fall"), js("off"), js("on"))
+        .version(1)
+        .build();
+    FeatureFlag f1 = new FeatureFlagBuilder("feature1")
+        .on(true)
+        .fallthrough(fallthroughVariation(0))
+        .variations(js("nogo"), js("go"))
+        .version(2)
+        .build();
+    featureStore.upsert(FEATURES, f1);        
+    FeatureFlag.EvalResult result = f0.evaluate(BASE_USER, featureStore);
+    
+    assertEquals(js("off"), result.getValue());
+    
+    assertEquals(1, result.getPrerequisiteEvents().size());
+    FeatureRequestEvent event = result.getPrerequisiteEvents().get(0);
+    assertEquals(f1.getKey(), event.key);
+    assertEquals("feature", event.kind);
+    assertEquals(js("nogo"), event.value);
+    assertEquals(f1.getVersion(), event.version.intValue());
+    assertEquals(f0.getKey(), event.prereqOf);
+  }
 
-    FeatureFlag.EvalResult flagAResult = flagA.evaluate(user, featureStore);
-    Assert.assertNotNull(flagAResult);
-    Assert.assertNull(flagAResult.getValue());
-    Assert.assertEquals(2, flagAResult.getPrerequisiteEvents().size());
+  @Test
+  public void flagReturnsFallthroughVariationAndEventIfPrerequisiteIsMetAndThereAreNoRules() throws Exception {
+    FeatureFlag f0 = new FeatureFlagBuilder("feature0")
+        .on(true)
+        .prerequisites(Arrays.asList(new Prerequisite("feature1", 1)))
+        .fallthrough(fallthroughVariation(0))
+        .offVariation(1)
+        .variations(js("fall"), js("off"), js("on"))
+        .version(1)
+        .build();
+    FeatureFlag f1 = new FeatureFlagBuilder("feature1")
+        .on(true)
+        .fallthrough(fallthroughVariation(1))
+        .variations(js("nogo"), js("go"))
+        .version(2)
+        .build();
+    featureStore.upsert(FEATURES, f1);        
+    FeatureFlag.EvalResult result = f0.evaluate(BASE_USER, featureStore);
+    
+    assertEquals(js("fall"), result.getValue());
+    assertEquals(1, result.getPrerequisiteEvents().size());
+    
+    FeatureRequestEvent event = result.getPrerequisiteEvents().get(0);
+    assertEquals(f1.getKey(), event.key);
+    assertEquals("feature", event.kind);
+    assertEquals(js("go"), event.value);
+    assertEquals(f1.getVersion(), event.version.intValue());
+    assertEquals(f0.getKey(), event.prereqOf);
+  }
 
-    FeatureFlag.EvalResult flagBResult = flagB.evaluate(user, featureStore);
-    Assert.assertNotNull(flagBResult);
-    Assert.assertNull(flagBResult.getValue());
-    Assert.assertEquals(1, flagBResult.getPrerequisiteEvents().size());
+  @Test
+  public void multipleLevelsOfPrerequisitesProduceMultipleEvents() throws Exception {
+    FeatureFlag f0 = new FeatureFlagBuilder("feature0")
+        .on(true)
+        .prerequisites(Arrays.asList(new Prerequisite("feature1", 1)))
+        .fallthrough(fallthroughVariation(0))
+        .offVariation(1)
+        .variations(js("fall"), js("off"), js("on"))
+        .version(1)
+        .build();
+    FeatureFlag f1 = new FeatureFlagBuilder("feature1")
+        .on(true)
+        .prerequisites(Arrays.asList(new Prerequisite("feature2", 1)))
+        .fallthrough(fallthroughVariation(1))
+        .variations(js("nogo"), js("go"))
+        .version(2)
+        .build();
+    FeatureFlag f2 = new FeatureFlagBuilder("feature2")
+        .on(true)
+        .fallthrough(fallthroughVariation(1))
+        .variations(js("nogo"), js("go"))
+        .version(3)
+        .build();
+    featureStore.upsert(FEATURES, f1);        
+    featureStore.upsert(FEATURES, f2);        
+    FeatureFlag.EvalResult result = f0.evaluate(BASE_USER, featureStore);
+    
+    assertEquals(js("fall"), result.getValue());    
+    assertEquals(2, result.getPrerequisiteEvents().size());
+    
+    FeatureRequestEvent event0 = result.getPrerequisiteEvents().get(0);
+    assertEquals(f2.getKey(), event0.key);
+    assertEquals("feature", event0.kind);
+    assertEquals(js("go"), event0.value);
+    assertEquals(f2.getVersion(), event0.version.intValue());
+    assertEquals(f1.getKey(), event0.prereqOf);
 
-    FeatureFlag.EvalResult flagCResult = flagC.evaluate(user, featureStore);
-    Assert.assertNotNull(flagCResult);
-    Assert.assertEquals(null, flagCResult.getValue());
-    Assert.assertEquals(0, flagCResult.getPrerequisiteEvents().size());
+    FeatureRequestEvent event1 = result.getPrerequisiteEvents().get(1);
+    assertEquals(f1.getKey(), event1.key);
+    assertEquals("feature", event1.kind);
+    assertEquals(js("go"), event1.value);
+    assertEquals(f1.getVersion(), event1.version.intValue());
+    assertEquals(f0.getKey(), event1.prereqOf);
   }
-
+  
+  @Test
+  public void flagMatchesUserFromTargets() throws Exception {
+    FeatureFlag f = new FeatureFlagBuilder("feature")
+        .on(true)
+        .targets(Arrays.asList(new Target(Arrays.asList("whoever", "userkey"), 2)))
+        .fallthrough(fallthroughVariation(0))
+        .offVariation(1)
+        .variations(js("fall"), js("off"), js("on"))
+        .build();
+    LDUser user = new LDUser.Builder("userkey").build();
+    FeatureFlag.EvalResult result = f.evaluate(user, featureStore);
+    
+    assertEquals(js("on"), result.getValue());
+    assertEquals(0, result.getPrerequisiteEvents().size());
+  }
+  
+  @Test
+  public void flagMatchesUserFromRules() throws Exception {
+    Clause clause = new Clause("key", Operator.in, Arrays.asList(js("userkey")), false);
+    Rule rule = new Rule(Arrays.asList(clause), 2, null);
+    FeatureFlag f = new FeatureFlagBuilder("feature")
+        .on(true)
+        .rules(Arrays.asList(rule))
+        .fallthrough(fallthroughVariation(0))
+        .offVariation(1)
+        .variations(js("fall"), js("off"), js("on"))
+        .build();
+    LDUser user = new LDUser.Builder("userkey").build();
+    FeatureFlag.EvalResult result = f.evaluate(user, featureStore);
+    
+    assertEquals(js("on"), result.getValue());
+    assertEquals(0, result.getPrerequisiteEvents().size());
+  }
+  
+  @Test
+  public void clauseCanMatchBuiltInAttribute() throws Exception {
+    Clause clause = new Clause("name", Operator.in, Arrays.asList(js("Bob")), false);
+    FeatureFlag f = booleanFlagWithClauses(clause);
+    LDUser user = new LDUser.Builder("key").name("Bob").build();
+    
+    assertEquals(jbool(true), f.evaluate(user, featureStore).getValue());
+  }
+  
+  @Test
+  public void clauseCanMatchCustomAttribute() throws Exception {
+    Clause clause = new Clause("legs", Operator.in, Arrays.asList(jint(4)), false);
+    FeatureFlag f = booleanFlagWithClauses(clause);
+    LDUser user = new LDUser.Builder("key").custom("legs", 4).build();
+    
+    assertEquals(jbool(true), f.evaluate(user, featureStore).getValue());
+  }
+  
+  @Test
+  public void clauseReturnsFalseForMissingAttribute() throws Exception {
+    Clause clause = new Clause("legs", Operator.in, Arrays.asList(jint(4)), false);
+    FeatureFlag f = booleanFlagWithClauses(clause);
+    LDUser user = new LDUser.Builder("key").name("Bob").build();
+    
+    assertEquals(jbool(false), f.evaluate(user, featureStore).getValue());
+  }
+  
+  @Test
+  public void clauseCanBeNegated() throws Exception {
+    Clause clause = new Clause("name", Operator.in, Arrays.asList(js("Bob")), true);
+    FeatureFlag f = booleanFlagWithClauses(clause);
+    LDUser user = new LDUser.Builder("key").name("Bob").build();
+    
+    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")
@@ -80,7 +304,7 @@ public void testSegmentMatchClauseRetrievesSegmentFromStore() throws Exception {
     LDUser user = new LDUser.Builder("foo").build();
     
     FeatureFlag.EvalResult result = flag.evaluate(user, featureStore);
-    Assert.assertEquals(new JsonPrimitive(true), result.getValue());
+    assertEquals(jbool(true), result.getValue());
   }
 
   @Test
@@ -89,34 +313,11 @@ public void testSegmentMatchClauseFallsThroughIfSegmentNotFound() throws Excepti
     LDUser user = new LDUser.Builder("foo").build();
     
     FeatureFlag.EvalResult result = flag.evaluate(user, featureStore);
-    Assert.assertEquals(new JsonPrimitive(false), result.getValue());
+    assertEquals(jbool(false), result.getValue());
   }
-  
-  private FeatureFlag newFlagWithPrereq(String featureKey, String prereqKey) {
-    return new FeatureFlagBuilder(featureKey)
-        .prerequisites(singletonList(new Prerequisite(prereqKey, 0)))
-        .variations(Arrays.asList(new JsonPrimitive(0), new JsonPrimitive(1)))
-        .fallthrough(new VariationOrRollout(0, null))
-        .on(true)
-        .build();
-  }
-
-  private FeatureFlag newFlagOff(String featureKey) {
-    return new FeatureFlagBuilder(featureKey)
-        .variations(Arrays.asList(new JsonPrimitive(0), new JsonPrimitive(1)))
-        .fallthrough(new VariationOrRollout(0, null))
-        .on(false)
-        .build();
-  }
-  
+ 
   private FeatureFlag segmentMatchBooleanFlag(String segmentKey) {
-    Clause clause = new Clause("", Operator.segmentMatch, Arrays.asList(new JsonPrimitive(segmentKey)), false);
-    Rule rule = new Rule(Arrays.asList(clause), 1, null);
-    return new FeatureFlagBuilder("key")
-        .variations(Arrays.asList(new JsonPrimitive(false), new JsonPrimitive(true)))
-        .fallthrough(new VariationOrRollout(0, null))
-        .on(true)
-        .rules(Arrays.asList(rule))
-        .build();
+    Clause clause = new Clause("", Operator.segmentMatch, Arrays.asList(js(segmentKey)), false);
+    return booleanFlagWithClauses(clause);
   }
 }
diff --git a/src/test/java/com/launchdarkly/client/RedisFeatureStoreTest.java b/src/test/java/com/launchdarkly/client/RedisFeatureStoreTest.java
index 90b194f76..cbe103cec 100644
--- a/src/test/java/com/launchdarkly/client/RedisFeatureStoreTest.java
+++ b/src/test/java/com/launchdarkly/client/RedisFeatureStoreTest.java
@@ -1,8 +1,19 @@
 package com.launchdarkly.client;
 
-import java.net.URI;
+import com.google.gson.Gson;
 
+import org.junit.Assert;
 import org.junit.Before;
+import org.junit.Test;
+
+import java.net.URI;
+import java.util.HashMap;
+import java.util.Map;
+
+import static com.launchdarkly.client.VersionedDataKind.FEATURES;
+import static java.util.Collections.singletonMap;
+
+import redis.clients.jedis.Jedis;
 
 public class RedisFeatureStoreTest extends FeatureStoreTestBase {
 
@@ -10,4 +21,63 @@ public class RedisFeatureStoreTest extends FeatureStoreTestBase flags = singletonMap(flag.getKey(), flag);
+    Map, Map> allData = new HashMap<>();
+    allData.put(FEATURES, flags);
+    store.init(allData);
+  }
+  
+  private RedisFeatureStore.UpdateListener makeConcurrentModifier(final Jedis otherClient, final FeatureFlag flag,
+    final int startVersion, final int endVersion) {
+    final Gson gson = new Gson();
+    return new RedisFeatureStore.UpdateListener() {
+      int versionCounter = startVersion;
+      @Override
+      public void aboutToUpdate(String baseKey, String itemKey) {
+        if (versionCounter <= endVersion) {
+          FeatureFlag newVer = new FeatureFlagBuilder(flag).version(versionCounter).build();
+          versionCounter++;
+          otherClient.hset(baseKey, flag.getKey(), gson.toJson(newVer));
+        }
+      }
+    };
+  }
 }
diff --git a/src/test/java/com/launchdarkly/client/TestUtil.java b/src/test/java/com/launchdarkly/client/TestUtil.java
new file mode 100644
index 000000000..af0c39ad9
--- /dev/null
+++ b/src/test/java/com/launchdarkly/client/TestUtil.java
@@ -0,0 +1,35 @@
+package com.launchdarkly.client;
+
+import com.google.gson.JsonPrimitive;
+
+import java.util.Arrays;
+
+public class TestUtil {
+
+  public static JsonPrimitive js(String s) {
+    return new JsonPrimitive(s);
+  }
+
+  public static JsonPrimitive jint(int n) {
+    return new JsonPrimitive(n);
+  }
+  
+  public static JsonPrimitive jbool(boolean b) {
+    return new JsonPrimitive(b);
+  }
+  
+  public static VariationOrRollout fallthroughVariation(int variation) {
+    return new VariationOrRollout(variation, null);
+  }
+  
+  public static FeatureFlag booleanFlagWithClauses(Clause... clauses) {
+    Rule rule = new Rule(Arrays.asList(clauses), 1, null);
+    return new FeatureFlagBuilder("feature")
+        .on(true)
+        .rules(Arrays.asList(rule))
+        .fallthrough(fallthroughVariation(0))
+        .offVariation(0)
+        .variations(jbool(false), jbool(true))
+        .build();
+  }
+}