Do not return nulls from the store

Never return null from store methods.
uber · Feb 8, 2019 · fe716e1 · fe716e1
1 parent ac8afdf
commit fe716e1
Show file tree

Hide file tree

Showing 4 changed files with 33 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
@@ -65,6 +65,11 @@ 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,14 @@ 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.
      */

diff --git a/simplestore/src/main/java/com/uber/simplestore/impl/SimpleStoreImpl.java b/simplestore/src/main/java/com/uber/simplestore/impl/SimpleStoreImpl.java
@@ -19,6 +19,7 @@
 import java.util.concurrent.Executor;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 /**
@@ -28,6 +29,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;
@@ -89,11 +91,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);
         }, orderedIoExecutor);
@@ -106,21 +107,29 @@ 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
@@ -37,10 +37,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);
         }
     }
 
@@ -49,7 +49,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();
         }
     }
 
@@ -59,7 +59,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();
         }
     }