Add handling for system namespace to CheckedDistributedStorageAdmin

scalar-labs · Dec 7, 2023 · f3e1c1b · f3e1c1b
1 parent 2162b59
commit f3e1c1b
Show file tree

Hide file tree

Showing 7 changed files with 104 additions and 8 deletions.
diff --git a/core/src/main/java/com/scalar/db/common/CheckedDistributedStorageAdmin.java b/core/src/main/java/com/scalar/db/common/CheckedDistributedStorageAdmin.java
@@ -2,26 +2,33 @@
 
 import com.scalar.db.api.DistributedStorageAdmin;
 import com.scalar.db.api.TableMetadata;
+import com.scalar.db.config.DatabaseConfig;
 import com.scalar.db.exception.storage.ExecutionException;
 import com.scalar.db.io.DataType;
 import com.scalar.db.util.ScalarDbUtils;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 import javax.annotation.Nullable;
 
 public class CheckedDistributedStorageAdmin implements DistributedStorageAdmin {
 
   private final DistributedStorageAdmin admin;
+  private final String systemNamespaceName;
 
   @SuppressFBWarnings("EI_EXPOSE_REP2")
-  public CheckedDistributedStorageAdmin(DistributedStorageAdmin admin) {
+  public CheckedDistributedStorageAdmin(DistributedStorageAdmin admin, DatabaseConfig config) {
     this.admin = admin;
+    systemNamespaceName = config.getSystemNamespaceName();
   }
 
   @Override
   public void createNamespace(String namespace, Map<String, String> options)
       throws ExecutionException {
+    if (systemNamespaceName.equals(namespace)) {
+      throw new IllegalArgumentException(namespace + " is the system namespace name");
+    }
     if (namespaceExists(namespace)) {
       throw new IllegalArgumentException("Namespace already exists: " + namespace);
     }
@@ -70,6 +77,9 @@ public void dropTable(String namespace, String table) throws ExecutionException
 
   @Override
   public void dropNamespace(String namespace) throws ExecutionException {
+    if (systemNamespaceName.equals(namespace)) {
+      throw new IllegalArgumentException(namespace + " is the system namespace name");
+    }
     if (!namespaceExists(namespace)) {
       throw new IllegalArgumentException("Namespace does not exist: " + namespace);
     }
@@ -189,6 +199,9 @@ public Set<String> getNamespaceTableNames(String namespace) throws ExecutionExce
 
   @Override
   public boolean namespaceExists(String namespace) throws ExecutionException {
+    if (systemNamespaceName.equals(namespace)) {
+      return true;
+    }
     try {
       return admin.namespaceExists(namespace);
     } catch (ExecutionException e) {
@@ -278,7 +291,14 @@ public void addNewColumnToTable(
   @Override
   public Set<String> getNamespaceNames() throws ExecutionException {
     try {
-      return admin.getNamespaceNames();
+      Set<String> namespaceNames = admin.getNamespaceNames();
+      if (namespaceNames.contains(systemNamespaceName)) {
+        return namespaceNames;
+      }
+
+      namespaceNames = new HashSet<>(namespaceNames);
+      namespaceNames.add(systemNamespaceName);
+      return namespaceNames;
     } catch (ExecutionException e) {
       throw new ExecutionException("Getting the namespace names failed", e);
     }

diff --git a/core/src/main/java/com/scalar/db/storage/cassandra/CassandraProvider.java b/core/src/main/java/com/scalar/db/storage/cassandra/CassandraProvider.java
@@ -19,6 +19,6 @@ public DistributedStorage createDistributedStorage(DatabaseConfig config) {
 
   @Override
   public DistributedStorageAdmin createDistributedStorageAdmin(DatabaseConfig config) {
-    return new CheckedDistributedStorageAdmin(new CassandraAdmin(config));
+    return new CheckedDistributedStorageAdmin(new CassandraAdmin(config), config);
   }
 }
diff --git a/core/src/main/java/com/scalar/db/storage/cosmos/CosmosProvider.java b/core/src/main/java/com/scalar/db/storage/cosmos/CosmosProvider.java
@@ -19,6 +19,6 @@ public DistributedStorage createDistributedStorage(DatabaseConfig config) {
 
   @Override
   public DistributedStorageAdmin createDistributedStorageAdmin(DatabaseConfig config) {
-    return new CheckedDistributedStorageAdmin(new CosmosAdmin(config));
+    return new CheckedDistributedStorageAdmin(new CosmosAdmin(config), config);
   }
 }
diff --git a/core/src/main/java/com/scalar/db/storage/dynamo/DynamoProvider.java b/core/src/main/java/com/scalar/db/storage/dynamo/DynamoProvider.java
@@ -19,7 +19,6 @@ public DistributedStorage createDistributedStorage(DatabaseConfig config) {
 
   @Override
   public DistributedStorageAdmin createDistributedStorageAdmin(DatabaseConfig config) {
-    // Set the namespace check to false because DynamoDB does not support namespaces.
-    return new CheckedDistributedStorageAdmin(new DynamoAdmin(config));
+    return new CheckedDistributedStorageAdmin(new DynamoAdmin(config), config);
   }
 }
diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/JdbcProvider.java b/core/src/main/java/com/scalar/db/storage/jdbc/JdbcProvider.java
@@ -19,6 +19,6 @@ public DistributedStorage createDistributedStorage(DatabaseConfig config) {
 
   @Override
   public DistributedStorageAdmin createDistributedStorageAdmin(DatabaseConfig config) {
-    return new CheckedDistributedStorageAdmin(new JdbcAdmin(config));
+    return new CheckedDistributedStorageAdmin(new JdbcAdmin(config), config);
   }
 }
diff --git a/core/src/main/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdmin.java b/core/src/main/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdmin.java
@@ -21,7 +21,7 @@ public class JdbcTransactionAdmin implements DistributedTransactionAdmin {
 
   @Inject
   public JdbcTransactionAdmin(DatabaseConfig databaseConfig) {
-    jdbcAdmin = new CheckedDistributedStorageAdmin(new JdbcAdmin(databaseConfig));
+    jdbcAdmin = new CheckedDistributedStorageAdmin(new JdbcAdmin(databaseConfig), databaseConfig);
   }
 
   @VisibleForTesting

diff --git a/core/src/test/java/com/scalar/db/common/CheckedDistributedStorageAdminTest.java b/core/src/test/java/com/scalar/db/common/CheckedDistributedStorageAdminTest.java
@@ -0,0 +1,77 @@
+package com.scalar.db.common;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.when;
+
+import com.scalar.db.api.DistributedStorageAdmin;
+import com.scalar.db.config.DatabaseConfig;
+import com.scalar.db.exception.storage.ExecutionException;
+import java.util.Collections;
+import java.util.Set;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+public class CheckedDistributedStorageAdminTest {
+
+  private static final String SYSTEM_NAMESPACE = "scalardb";
+
+  @Mock private DistributedStorageAdmin admin;
+  @Mock private DatabaseConfig databaseConfig;
+
+  private CheckedDistributedStorageAdmin checkedAdmin;
+
+  @BeforeEach
+  public void setUp() throws Exception {
+    MockitoAnnotations.openMocks(this).close();
+
+    // Arrange
+    when(databaseConfig.getSystemNamespaceName()).thenReturn(SYSTEM_NAMESPACE);
+    checkedAdmin = new CheckedDistributedStorageAdmin(admin, databaseConfig);
+  }
+
+  @Test
+  public void createNamespace_SystemNamespaceNameGiven_ShouldThrowIllegalArgumentException() {
+    // Arrange
+
+    // Act Assert
+    assertThatThrownBy(() -> checkedAdmin.createNamespace(SYSTEM_NAMESPACE))
+        .isInstanceOf(IllegalArgumentException.class);
+  }
+
+  @Test
+  public void dropNamespace_SystemNamespaceNameGiven_ShouldThrowIllegalArgumentException() {
+    // Arrange
+
+    // Act Assert
+    assertThatThrownBy(() -> checkedAdmin.dropNamespace(SYSTEM_NAMESPACE))
+        .isInstanceOf(IllegalArgumentException.class);
+  }
+
+  @Test
+  public void namespaceExists_SystemNamespaceNameGiven_ShouldReturnTrue()
+      throws ExecutionException {
+    // Arrange
+
+    // Act
+    boolean actual = checkedAdmin.namespaceExists(SYSTEM_NAMESPACE);
+
+    // Assert
+    assertThat(actual).isTrue();
+  }
+
+  @Test
+  public void getNamespaceNames_ShouldReturnListWithSystemNamespaceName()
+      throws ExecutionException {
+    // Arrange
+    when(admin.getNamespaceNames()).thenReturn(Collections.emptySet());
+
+    // Act
+    Set<String> actual = checkedAdmin.getNamespaceNames();
+
+    // Assert
+    assertThat(actual).containsExactly(SYSTEM_NAMESPACE);
+  }
+}