Skip to content

Commit

Permalink
Do not return nulls from the store
Browse files Browse the repository at this point in the history
Never return null from store methods.
kurtisnelson committed Feb 8, 2019
1 parent ac8afdf commit fe716e1
Showing 4 changed files with 33 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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.
*/
Original file line number Diff line number Diff line change
@@ -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();
Original file line number Diff line number Diff line change
@@ -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();
}
}

0 comments on commit fe716e1

Please sign in to comment.