Do not return nulls from the store

Never return null from store methods.
uber · Feb 12, 2019 · 4e5cd70 · 4e5cd70
1 parent ea4aca5
commit 4e5cd70
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 11 deletions.
diff --git a/protosimplestore/src/main/java/com/uber/simplestore/proto/impl/SimpleProtoStoreImpl.java b/protosimplestore/src/main/java/com/uber/simplestore/proto/impl/SimpleProtoStoreImpl.java
@@ -73,6 +73,14 @@ public <T extends MessageLite> ListenableFuture<T> put(String key, @Nullable T v
         SimpleStoreConfig.getComputationExecutor());
   }
 
+  @Override
+  public ListenableFuture<Boolean> contains(String key) {
+    return Futures.transform(
+        get(key),
+        value -> value != null && value.length > 0,
+        SimpleStoreConfig.getComputationExecutor());
+  }
+
   @Override
   public ListenableFuture<String> getString(String key) {
     return simpleStore.getString(key);

diff --git a/simplestore/src/main/java/com/uber/simplestore/SimpleStore.java b/simplestore/src/main/java/com/uber/simplestore/SimpleStore.java
@@ -42,6 +42,15 @@ public interface SimpleStore extends Closeable {
   @CheckReturnValue
   ListenableFuture<byte[]> put(String key, @Nullable byte[] value);
 
+  /**
+   * Determine if a key exists in storage.
+   *
+   * @param key to check
+   * @return if key is set
+   */
+  @CheckReturnValue
+  ListenableFuture<Boolean> contains(String key);
+
   /** Delete all keys in this direct scope. */
   @CheckReturnValue
   ListenableFuture<Void> deleteAll();

diff --git a/simplestore/src/main/java/com/uber/simplestore/impl/SimpleStoreImpl.java b/simplestore/src/main/java/com/uber/simplestore/impl/SimpleStoreImpl.java
@@ -22,6 +22,7 @@ final class SimpleStoreImpl implements SimpleStore {
   private static final int OPEN = 0;
   private static final int CLOSED = 1;
   private static final int TOMBSTONED = 2;
+  private static final byte[] EMPTY_BYTES = new byte[0];
 
   private final Context context;
   private final String scope;
@@ -88,11 +89,10 @@ public ListenableFuture<byte[]> get(String key) {
             } catch (IOException e) {
               return Futures.immediateFailedFuture(e);
             }
-            if (value == null) {
-              cache.remove(key);
-            } else {
-              cache.put(key, value);
+            if (value == null || value.length == 0) {
+              value = EMPTY_BYTES;
             }
+            cache.put(key, value);
           }
           return Futures.immediateFuture(value);
         },
@@ -107,22 +107,32 @@ public ListenableFuture<byte[]> put(String key, @Nullable byte[] value) {
           if (isClosed()) {
             return Futures.immediateFailedFuture(new StoreClosedException());
           }
-          if (value == null) {
-            cache.remove(key);
+          if (value == null || value.length == 0) {
+            cache.put(key, EMPTY_BYTES);
             deleteFile(key);
+            return Futures.immediateFuture(EMPTY_BYTES);
           } else {
             cache.put(key, value);
             try {
               writeFile(key, value);
             } catch (IOException e) {
               return Futures.immediateFailedFuture(e);
             }
+            return Futures.immediateFuture(value);
           }
-          return Futures.immediateFuture(value);
         },
         orderedIoExecutor);
   }
 
+  @Override
+  public ListenableFuture<Boolean> contains(String key) {
+    requireOpen();
+    return Futures.transform(
+        get(key),
+        (value) -> value != null && value.length > 0,
+        SimpleStoreConfig.getComputationExecutor());
+  }
+
   @Override
   public ListenableFuture<Void> deleteAll() {
     requireOpen();

diff --git a/simplestore/src/test/java/com/uber/simplestore/impl/SimpleStoreImplTest.java b/simplestore/src/test/java/com/uber/simplestore/impl/SimpleStoreImplTest.java
@@ -32,10 +32,10 @@ public void reset() {
   }
 
   @Test
-  public void nullWhenMissing() throws Exception {
+  public void zeroLengthWhenMissing() throws Exception {
     try (SimpleStore store = SimpleStoreFactory.create(context, "")) {
       ListenableFuture<byte[]> future = store.get(TEST_KEY);
-      assertThat(future.get()).isNull();
+      assertThat(future.get()).hasLength(0);
     }
   }
 
@@ -44,7 +44,7 @@ public void puttingNullDeletesKey() throws Exception {
     try (SimpleStore store = SimpleStoreFactory.create(context, "")) {
       ListenableFuture<byte[]> first = store.put(TEST_KEY, new byte[1]);
       ListenableFuture<byte[]> second = store.put(TEST_KEY, null);
-      assertThat(second.get()).isNull();
+      assertThat(second.get()).isEmpty();
     }
   }
 
@@ -54,7 +54,7 @@ public void deleteAll() throws Exception {
       ListenableFuture<byte[]> first = store.put(TEST_KEY, new byte[1]);
       ListenableFuture<Void> second = store.deleteAll();
       ListenableFuture<byte[]> empty = store.get(TEST_KEY);
-      assertThat(empty.get()).isNull();
+      assertThat(empty.get()).isEmpty();
     }
   }