Fix

Author: brfrn169

core/src/main/java/com/scalar/db/common/CoreError.java

@@ -377,16 +377,16 @@ public enum CoreError implements ScalarDbError {
       ""),
   MULTI_STORAGE_STORAGE_NOT_FOUND(
       Category.USER_ERROR, "0084", "Storage not found. Storage: %s", "", ""),
-  JDBC_NAMESPACE_NAME_NOT_ACCEPTABLE(
-      Category.USER_ERROR, "0085", "The namespace name is not acceptable. Namespace: %s", "", ""),
-  JDBC_TABLE_NAME_NOT_ACCEPTABLE(
-      Category.USER_ERROR, "0086", "The table name is not acceptable. Table: %s", "", ""),
-  JDBC_IMPORT_NOT_SUPPORTED(
+  JDBC_SQLITE_NAMESPACE_NAME_NOT_ACCEPTABLE(
       Category.USER_ERROR,
-      "0087",
-      "Importing tables is not allowed in the RDB engine. RDB engine: %s",
+      "0085",
+      "The namespace name is not acceptable in SQLite. Namespace: %s",
       "",
       ""),
+  JDBC_SQLITE_TABLE_NAME_NOT_ACCEPTABLE(
+      Category.USER_ERROR, "0086", "The table name is not acceptable in SQLite. Table: %s", "", ""),
+  JDBC_SQLITE_IMPORT_NOT_SUPPORTED(
+      Category.USER_ERROR, "0087", "Importing tables is not allowed in SQLite", "", ""),
   JDBC_IMPORT_TABLE_WITHOUT_PRIMARY_KEY(
       Category.USER_ERROR, "0088", "The %s table must have a primary key", "", ""),
   JDBC_RDB_ENGINE_NOT_SUPPORTED(

core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java

@@ -147,10 +147,8 @@ static void execute(
   @Override
   public void createNamespace(String namespace, Map<String, String> options)
       throws ExecutionException {
-    if (!rdbEngine.isValidNamespaceOrTableName(namespace)) {
-      throw new IllegalArgumentException(
-          CoreError.JDBC_NAMESPACE_NAME_NOT_ACCEPTABLE.buildMessage(namespace));
-    }
+    rdbEngine.throwIfInvalidNamespaceName(namespace);
+
     try (Connection connection = dataSource.getConnection()) {
       execute(connection, rdbEngine.createSchemaSqls(namespace));
       createNamespacesTableIfNotExists(connection);
@@ -182,10 +180,8 @@ void createTableInternal(
       TableMetadata metadata,
       boolean ifNotExists)
       throws SQLException {
-    if (!rdbEngine.isValidNamespaceOrTableName(table)) {
-      throw new IllegalArgumentException(
-          CoreError.JDBC_TABLE_NAME_NOT_ACCEPTABLE.buildMessage(table));
-    }
+    rdbEngine.throwIfInvalidTableName(table);
+
     String createTableStatement = "CREATE TABLE " + encloseFullTableName(schema, table) + "(";
     // Order the columns for their creation by (partition keys >> clustering keys >> other columns)
     LinkedHashSet<String> sortedColumnNames =
@@ -572,11 +568,6 @@ TableMetadata getImportTableMetadata(
     TableMetadata.Builder builder = TableMetadata.newBuilder();
     boolean primaryKeyExists = false;
 
-    if (!rdbEngine.isImportable()) {
-      throw new UnsupportedOperationException(
-          CoreError.JDBC_IMPORT_NOT_SUPPORTED.buildMessage(rdbEngine.getClass().getName()));
-    }
-
     try (Connection connection = dataSource.getConnection()) {
       rdbEngine.setConnectionToReadOnly(connection, true);
 
@@ -634,6 +625,8 @@ public void importTable(
       Map<String, String> options,
       Map<String, DataType> overrideColumnsType)
       throws ExecutionException {
+    rdbEngine.throwIfImportNotSupported();
+
     try (Connection connection = dataSource.getConnection()) {
       TableMetadata tableMetadata = getImportTableMetadata(namespace, table, overrideColumnsType);
       createNamespacesTableIfNotExists(connection);
@@ -852,10 +845,8 @@ private boolean tableExistsInternal(Connection connection, String namespace, Str
   @Override
   public void repairNamespace(String namespace, Map<String, String> options)
       throws ExecutionException {
-    if (!rdbEngine.isValidNamespaceOrTableName(namespace)) {
-      throw new IllegalArgumentException(
-          CoreError.JDBC_NAMESPACE_NAME_NOT_ACCEPTABLE.buildMessage(namespace));
-    }
+    rdbEngine.throwIfInvalidNamespaceName(namespace);
+
     try (Connection connection = dataSource.getConnection()) {
       createSchemaIfNotExists(connection, namespace);
       createNamespacesTableIfNotExists(connection);
@@ -869,6 +860,8 @@ public void repairNamespace(String namespace, Map<String, String> options)
   public void repairTable(
       String namespace, String table, TableMetadata metadata, Map<String, String> options)
       throws ExecutionException {
+    rdbEngine.throwIfInvalidNamespaceName(table);
+
     try (Connection connection = dataSource.getConnection()) {
       createTableInternal(connection, namespace, table, metadata, true);
       addTableMetadata(connection, namespace, table, metadata, true, true);

core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlite.java

@@ -167,8 +167,19 @@ public DataType getDataTypeForScalarDbInternal(
   }
 
   @Override
-  public boolean isValidNamespaceOrTableName(String namespaceOrTableName) {
-    return !namespaceOrTableName.contains(NAMESPACE_SEPARATOR);
+  public void throwIfInvalidNamespaceName(String namespaceName) {
+    if (namespaceName.contains(NAMESPACE_SEPARATOR)) {
+      throw new IllegalArgumentException(
+          CoreError.JDBC_SQLITE_NAMESPACE_NAME_NOT_ACCEPTABLE.buildMessage(namespaceName));
+    }
+  }
+
+  @Override
+  public void throwIfInvalidTableName(String tableName) {
+    if (tableName.contains(NAMESPACE_SEPARATOR)) {
+      throw new IllegalArgumentException(
+          CoreError.JDBC_SQLITE_TABLE_NAME_NOT_ACCEPTABLE.buildMessage(tableName));
+    }
   }
 
   @Override
@@ -310,8 +321,9 @@ public Driver getDriver() {
   }
 
   @Override
-  public boolean isImportable() {
-    return false;
+  public void throwIfImportNotSupported() {
+    throw new UnsupportedOperationException(
+        CoreError.JDBC_SQLITE_IMPORT_NOT_SUPPORTED.buildMessage());
   }
 
   @Override

core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java

@@ -73,9 +73,9 @@ DataType getDataTypeForScalarDb(
 
   String[] createSchemaIfNotExistsSqls(String fullSchema);
 
-  default boolean isValidNamespaceOrTableName(String tableName) {
-    return true;
-  }
+  default void throwIfInvalidNamespaceName(String namespaceName) {}
+
+  default void throwIfInvalidTableName(String tableName) {}
 
   String createTableInternalPrimaryKeyClause(
       boolean hasDescClusteringOrder, TableMetadata metadata);
@@ -165,9 +165,7 @@ default String encloseFullTableName(String schema, String table) {
 
   Driver getDriver();
 
-  default boolean isImportable() {
-    return true;
-  }
+  default void throwIfImportNotSupported() {}
 
   /**
    * Return properly-preprocessed like pattern for each underlying database.

core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTest.java

@@ -3644,20 +3644,16 @@ private void getNamespaceNames_forX_ShouldReturnNamespaceNames(
   }
 
   @ParameterizedTest
-  @EnumSource(RdbEngine.class)
-  public void getImportTableMetadata_ForX_ShouldWorkProperly(RdbEngine rdbEngine)
+  @EnumSource(
+      value = RdbEngine.class,
+      mode = Mode.EXCLUDE,
+      names = {
+        "SQLITE",
+      })
+  public void getImportTableMetadata_ForXBesidesSqlite_ShouldWorkProperly(RdbEngine rdbEngine)
       throws SQLException, ExecutionException {
-    if (rdbEngine.equals(RdbEngine.SQLITE)) {
-      getImportTableMetadata_ForSQLite_ShouldThrowUnsupportedOperationException(rdbEngine);
-    } else {
-      getImportTableMetadata_ForOtherThanSQLite_ShouldWorkProperly(
-          rdbEngine, prepareSqlForTableCheck(rdbEngine, NAMESPACE, TABLE));
-    }
-  }
+    String expectedCheckTableExistStatement = prepareSqlForTableCheck(rdbEngine, NAMESPACE, TABLE);
 
-  private void getImportTableMetadata_ForOtherThanSQLite_ShouldWorkProperly(
-      RdbEngine rdbEngine, String expectedCheckTableExistStatement)
-      throws SQLException, ExecutionException {
     // Arrange
     Statement checkTableExistStatement = mock(Statement.class);
     DatabaseMetaData metadata = mock(DatabaseMetaData.class);
@@ -3749,16 +3745,6 @@ public Boolean answer(InvocationOnMock invocation) {
             eq(DataType.FLOAT));
   }
 
-  private void getImportTableMetadata_ForSQLite_ShouldThrowUnsupportedOperationException(
-      RdbEngine rdbEngine) {
-    // Arrange
-    JdbcAdmin admin = createJdbcAdminFor(rdbEngine);
-
-    // Act Assert
-    assertThatThrownBy(() -> admin.getImportTableMetadata(NAMESPACE, TABLE, Collections.emptyMap()))
-        .isInstanceOf(UnsupportedOperationException.class);
-  }
-
   @Test
   public void getImportTableMetadata_PrimaryKeyNotExistsForX_ShouldThrowExecutionException()
       throws SQLException {

core/src/test/java/com/scalar/db/storage/jdbc/RdbEngineSqliteTest.java

@@ -1,5 +1,7 @@
 package com.scalar.db.storage.jdbc;
 
+import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.api.AssertionsForClassTypes.catchThrowable;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -121,12 +123,26 @@ void isConflict_False() {
   }
 
   @Test
-  void isValidNamespaceOrTableName_True() {
-    assertTrue(rdbEngine.isValidNamespaceOrTableName("a_b"));
+  void throwIfInvalidNamespaceName_ShouldNotThrowAnyException() {
+    assertThatCode(() -> rdbEngine.throwIfInvalidNamespaceName("a-b")).doesNotThrowAnyException();
   }
 
   @Test
-  void isValidNamespaceOrTableName_False_WhenContainsNamespaceSeparator() {
-    assertFalse(rdbEngine.isValidNamespaceOrTableName("a$b"));
+  void
+      throwIfInvalidNamespaceName_WhenContainsNamespaceSeparator_ShouldThrowIllegalArgumentException() {
+    assertThatThrownBy(() -> rdbEngine.throwIfInvalidNamespaceName("a$b"))
+        .isInstanceOf(IllegalArgumentException.class);
+  }
+
+  @Test
+  void throwIfInvalidTableName_ShouldNotThrowAnyException() {
+    assertThatCode(() -> rdbEngine.throwIfInvalidTableName("a-b")).doesNotThrowAnyException();
+  }
+
+  @Test
+  void
+      throwIfInvalidTableName_WhenContainsNamespaceSeparator_ShouldThrowIllegalArgumentException() {
+    assertThatThrownBy(() -> rdbEngine.throwIfInvalidTableName("a$b"))
+        .isInstanceOf(IllegalArgumentException.class);
   }
 }