Fix NullPointerException when table is not found in Consensus Commit (#…

…2083)
scalar-labs · Jul 31, 2024 · 2089188 · 2089188
1 parent 09c2568
commit 2089188
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 9 deletions.
diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/CrudHandler.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/CrudHandler.java
@@ -8,6 +8,7 @@
 import com.scalar.db.api.DistributedStorage;
 import com.scalar.db.api.Get;
 import com.scalar.db.api.GetBuilder;
+import com.scalar.db.api.Operation;
 import com.scalar.db.api.Put;
 import com.scalar.db.api.Result;
 import com.scalar.db.api.Scan;
@@ -189,6 +190,7 @@ private List<Result> scanInternal(Scan originalScan) throws CrudException {
   private List<Result> createScanResults(
       Scan scan, List<String> projections, Map<Snapshot.Key, TransactionResult> results)
       throws CrudException {
+    assert scan.forNamespace().isPresent() && scan.forTable().isPresent();
     TableMetadata metadata = getTableMetadata(scan.forNamespace().get(), scan.forTable().get());
     return results.values().stream()
         .map(r -> new FilteredResult(r, projections, metadata, isIncludeMetadataEnabled))
@@ -294,17 +296,31 @@ private Scanner scanFromStorage(Scan scan) throws CrudException {
   }
 
   private Selection prepareStorageSelection(Selection selection) throws CrudException {
+    selection.clearProjections();
+    // Retrieve only the after images columns when including the metadata is disabled, otherwise
+    // retrieve all the columns
+    if (!isIncludeMetadataEnabled) {
+      LinkedHashSet<String> afterImageColumnNames =
+          getTransactionTableMetadata(selection).getAfterImageColumnNames();
+      selection.withProjections(afterImageColumnNames);
+    }
+    selection.withConsistency(Consistency.LINEARIZABLE);
+    return selection;
+  }
+
+  private TransactionTableMetadata getTransactionTableMetadata(Operation operation)
+      throws CrudException {
     try {
-      selection.clearProjections();
-      // Retrieve only the after images columns when including the metadata is disabled, otherwise
-      // retrieve all the columns
-      if (!isIncludeMetadataEnabled) {
-        LinkedHashSet<String> afterImageColumnNames =
-            tableMetadataManager.getTransactionTableMetadata(selection).getAfterImageColumnNames();
-        selection.withProjections(afterImageColumnNames);
+      TransactionTableMetadata metadata =
+          tableMetadataManager.getTransactionTableMetadata(operation);
+      if (metadata == null) {
+        assert operation.forNamespace().isPresent() && operation.forTable().isPresent();
+        throw new IllegalArgumentException(
+            CoreError.TABLE_NOT_FOUND.buildMessage(
+                ScalarDbUtils.getFullTableName(
+                    operation.forNamespace().get(), operation.forTable().get())));
       }
-      selection.withConsistency(Consistency.LINEARIZABLE);
-      return selection;
+      return metadata;
     } catch (ExecutionException e) {
       throw new CrudException(
           CoreError.GETTING_TABLE_METADATA_FAILED.buildMessage(), e, snapshot.getId());

diff --git a/core/src/test/java/com/scalar/db/transaction/consensuscommit/CrudHandlerTest.java b/core/src/test/java/com/scalar/db/transaction/consensuscommit/CrudHandlerTest.java
@@ -334,6 +334,26 @@ public void get_CalledTwiceUnderRealSnapshot_SecondTimeShouldReturnTheSameFromSn
     verify(storage).get(getForStorage);
   }
 
+  @Test
+  public void get_ForNonExistingTable_ShouldThrowIllegalArgumentException()
+      throws ExecutionException {
+    // Arrange
+    Key partitionKey = Key.ofText(ANY_NAME_1, ANY_TEXT_1);
+    Key clusteringKey = Key.ofText(ANY_NAME_2, ANY_TEXT_2);
+    Get get =
+        Get.newBuilder()
+            .namespace(ANY_NAMESPACE_NAME)
+            .table(ANY_TABLE_NAME)
+            .partitionKey(partitionKey)
+            .clusteringKey(clusteringKey)
+            .build();
+
+    when(tableMetadataManager.getTransactionTableMetadata(get)).thenReturn(null);
+
+    // Act Assert
+    assertThatThrownBy(() -> handler.get(get)).isInstanceOf(IllegalArgumentException.class);
+  }
+
   @Test
   public void scan_ResultGivenFromStorage_ShouldUpdateSnapshotAndReturn()
       throws ExecutionException, CrudException {