Merge pull request #21 from G8XSU/keyNotFound

Throw NoSuchKeyException when key doesn't exist in get

Author: G8XSU

app/src/main/java/org/vss/api/AbstractVssApi.java

@@ -8,6 +8,7 @@
 import org.vss.ErrorResponse;
 import org.vss.KVStore;
 import org.vss.exception.ConflictException;
+import org.vss.exception.NoSuchKeyException;
 
 public abstract class AbstractVssApi {
   final KVStore kvStore;
@@ -32,6 +33,8 @@ Response toErrorResponse(Exception e) {
     } else if (e instanceof IllegalArgumentException
         || e instanceof InvalidProtocolBufferException) {
       errorCode = ErrorCode.INVALID_REQUEST_EXCEPTION;
+    } else if (e instanceof NoSuchKeyException) {
+      errorCode = ErrorCode.NO_SUCH_KEY_EXCEPTION;
     } else {
       errorCode = ErrorCode.INTERNAL_SERVER_EXCEPTION;
     }

app/src/main/java/org/vss/exception/NoSuchKeyException.java

@@ -0,0 +1,7 @@
+package org.vss.exception;
+
+public class NoSuchKeyException extends RuntimeException {
+  public NoSuchKeyException(String message) {
+    super(message);
+  }
+}

app/src/main/java/org/vss/impl/postgres/PostgresBackendImpl.java

@@ -23,6 +23,7 @@
 import org.vss.PutObjectRequest;
 import org.vss.PutObjectResponse;
 import org.vss.exception.ConflictException;
+import org.vss.exception.NoSuchKeyException;
 import org.vss.postgres.tables.records.VssDbRecord;
 
 import static org.vss.postgres.tables.VssDb.VSS_DB;
@@ -31,6 +32,11 @@
 public class PostgresBackendImpl implements KVStore {
 
   private static final int LIST_KEY_VERSIONS_MAX_PAGE_SIZE = 100;
+  private static final KeyValue DEFAULT_GLOBAL_VERSION_KV = KeyValue.newBuilder()
+      .setKey(GLOBAL_VERSION_KEY)
+      .setValue(ByteString.EMPTY)
+      .setVersion(0L)
+      .build();
   private final DSLContext context;
 
   @Inject
@@ -47,16 +53,19 @@ public GetObjectResponse get(GetObjectRequest request) {
         .fetchOne();
 
     final KeyValue keyValue;
-
-    if (vssDbRecord != null) {
+    if (vssDbRecord == null) {
+      if (GLOBAL_VERSION_KEY.equals(request.getKey())) {
+        keyValue = DEFAULT_GLOBAL_VERSION_KV;
+      } else {
+        throw new NoSuchKeyException(
+            "Specified key: " + request.getKey() + " in request does not exist.");
+      }
+    } else {
       keyValue = KeyValue.newBuilder()
           .setKey(vssDbRecord.getKey())
           .setValue(ByteString.copyFrom(vssDbRecord.getValue()))
           .setVersion(vssDbRecord.getVersion())
           .build();
-    } else {
-      keyValue = KeyValue.newBuilder()
-          .setKey(request.getKey()).build();
     }
 
     return GetObjectResponse.newBuilder()

app/src/main/proto/vss.proto

@@ -16,6 +16,9 @@ message GetObjectRequest {
 
   // `Key` for which the value is to be fetched.
   //
+  // If the specified `key` does not exist, returns `ErrorResponse` with `NO_SUCH_KEY_EXCEPTION` as
+  // `ErrorCode`.
+  //
   // Consistency Guarantee:
   // Get(read) operations against a `key` are consistent reads and will reflect all previous writes,
   // since Put/Write provides read-after-write and read-after-update consistency guarantees.
@@ -276,6 +279,9 @@ enum ErrorCode {
   // An internal server error occurred, client is probably at no fault and can safely retry this
   // error with exponential backoff.
   INTERNAL_SERVER_EXCEPTION = 3;
+
+  // NO_SUCH_KEY_EXCEPTION is used when the specified `key` in a `GetObjectRequest` does not exist.
+  NO_SUCH_KEY_EXCEPTION = 4;
 }
 
 // Represents a key-value pair to be stored or retrieved.

app/src/test/java/org/vss/AbstractKVStoreIntegrationTest.java

@@ -11,6 +11,7 @@
 import org.junit.jupiter.api.Test;
 import org.testcontainers.shaded.org.apache.commons.lang3.StringUtils;
 import org.vss.exception.ConflictException;
+import org.vss.exception.NoSuchKeyException;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
@@ -19,7 +20,6 @@
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public abstract class AbstractKVStoreIntegrationTest {
 
@@ -153,25 +153,25 @@ void putAndDeleteShouldSucceedAsAtomicTransaction() {
     assertThat(response.getVersion(), is(1L));
     assertThat(response.getValue().toStringUtf8(), is("k2v1"));
 
-    assertTrue(getObject("k1").getValue().isEmpty());
+    assertThrows(NoSuchKeyException.class, () -> getObject("k1"));
 
     // Delete fails (and hence put as well) due to mismatched version for the deleted item.
     assertThrows(ConflictException.class, () -> putAndDeleteObjects(null, List.of(kv("k3", "k3v1", 0)), List.of(kv("k2", "", 3))));
 
-    assertTrue(getObject("k3").getValue().isEmpty());
-    assertFalse(getObject("k2").getValue().isEmpty());
+    assertThrows(NoSuchKeyException.class, () -> getObject("k3"));
+    assertDoesNotThrow(() -> getObject("k2"));
 
     // Put fails (and hence delete as well) due to mismatched version for the put item.
     assertThrows(ConflictException.class, () -> putAndDeleteObjects(null, List.of(kv("k3", "k3v1", 1)), List.of(kv("k2", "", 1))));
 
-    assertTrue(getObject("k3").getValue().isEmpty());
-    assertFalse(getObject("k2").getValue().isEmpty());
+    assertThrows(NoSuchKeyException.class, () -> getObject("k3"));
+    assertDoesNotThrow(() -> getObject("k2"));
 
     // Put and delete both fail due to mismatched global version.
     assertThrows(ConflictException.class, () -> putAndDeleteObjects(2L, List.of(kv("k3", "k3v1", 0)), List.of(kv("k2", "", 1))));
 
-    assertTrue(getObject("k3").getValue().isEmpty());
-    assertFalse(getObject("k2").getValue().isEmpty());
+    assertThrows(NoSuchKeyException.class, () -> getObject("k3"));
+    assertDoesNotThrow(() -> getObject("k2"));
 
     assertThat(getObject(KVStore.GLOBAL_VERSION_KEY).getVersion(), is(0L));
   }
@@ -182,18 +182,14 @@ void deleteShouldSucceedWhenItemExists() {
     // Conditional Delete
     assertDoesNotThrow(() -> deleteObject(kv("k1", "", 1)));
 
-    KeyValue response = getObject("k1");
-    assertThat(response.getKey(), is("k1"));
-    assertTrue(response.getValue().isEmpty());
+    assertThrows(NoSuchKeyException.class, () -> getObject("k1"));
 
     assertDoesNotThrow(() -> putObjects(null, List.of(kv("k1", "k1v1", 0))));
     assertDoesNotThrow(() -> putObjects(null, List.of(kv("k1", "k1v2", 1))));
     // NonConditional Delete
     assertDoesNotThrow(() -> deleteObject(kv("k1", "", -1)));
 
-    response = getObject("k1");
-    assertThat(response.getKey(), is("k1"));
-    assertTrue(response.getValue().isEmpty());
+    assertThrows(NoSuchKeyException.class, () -> getObject("k1"));
   }
 
   @Test
@@ -207,18 +203,12 @@ void deleteShouldBeIdempotent() {
     assertDoesNotThrow(() -> deleteObject(kv("k1", "", 1)));
     assertDoesNotThrow(() -> deleteObject(kv("k1", "", 1)));
 
-    KeyValue response = getObject("k1");
-    assertThat(response.getKey(), is("k1"));
-    assertTrue(response.getValue().isEmpty());
+    assertThrows(NoSuchKeyException.class, () -> getObject("k1"));
   }
 
   @Test
-  void getShouldReturnEmptyResponseWhenKeyDoesNotExist() {
-    KeyValue response = getObject("non_existent_key");
-
-    assertThat(response.getKey(), is("non_existent_key"));
-    assertTrue(response.getValue().isEmpty());
-    assertThat(response.getVersion(), is(0L));
+  void getShouldThrowNoSuchKeyExceptionWhenKeyDoesNotExist() {
+    assertThrows(NoSuchKeyException.class, () -> getObject("non_existent_key"));
   }
 
   @Test

app/src/test/java/org/vss/api/GetObjectApiTest.java

@@ -16,6 +16,7 @@
 import org.vss.KVStore;
 import org.vss.KeyValue;
 import org.vss.exception.ConflictException;
+import org.vss.exception.NoSuchKeyException;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
@@ -80,6 +81,7 @@ private static Stream<Arguments> provideErrorTestCases() {
     return Stream.of(
         Arguments.of(new ConflictException(""), ErrorCode.CONFLICT_EXCEPTION),
         Arguments.of(new IllegalArgumentException(""), ErrorCode.INVALID_REQUEST_EXCEPTION),
+        Arguments.of(new NoSuchKeyException(""), ErrorCode.NO_SUCH_KEY_EXCEPTION),
         Arguments.of(new RuntimeException(""), ErrorCode.INTERNAL_SERVER_EXCEPTION)
     );
   }