fix comments and uts

apache · Feb 6, 2024 · 2512912 · 2512912
1 parent 4283282
commit 2512912
Show file tree

Hide file tree

Showing 11 changed files with 191 additions and 110 deletions.
diff --git a/core/src/main/java/com/datastrato/gravitino/Configs.java b/core/src/main/java/com/datastrato/gravitino/Configs.java
@@ -64,6 +64,36 @@ public interface Configs {
           .checkValue(StringUtils::isNotBlank, ConfigConstants.NOT_BLANK_ERROR_MSG)
           .createWithDefault(DEFAULT_ENTITY_RELATIONAL_STORE);
 
+  ConfigEntry<String> ENTRY_RELATIONAL_MYSQL_BACKEND_URL =
+      new ConfigBuilder(MYSQL_ENTITY_STORE_URL_KEY)
+          .doc("Connection URL of `MySQLBackend`")
+          .version("0.5.0")
+          .stringConf()
+          .checkValue(StringUtils::isNotBlank, ConfigConstants.NOT_BLANK_ERROR_MSG)
+          .create();
+
+  ConfigEntry<String> ENTRY_RELATIONAL_MYSQL_BACKEND_DRIVER_NAME =
+      new ConfigBuilder(MYSQL_ENTITY_STORE_DRIVER_NAME_KEY)
+          .doc("Driver Name of `MySQLBackend`")
+          .version("0.5.0")
+          .stringConf()
+          .checkValue(StringUtils::isNotBlank, ConfigConstants.NOT_BLANK_ERROR_MSG)
+          .create();
+
+  ConfigEntry<String> ENTRY_RELATIONAL_MYSQL_BACKEND_USERNAME =
+      new ConfigBuilder(MYSQL_ENTITY_STORE_USERNAME_KEY)
+          .doc("Username of `MySQLBackend`")
+          .version("0.5.0")
+          .stringConf()
+          .createWithDefault("");
+
+  ConfigEntry<String> ENTRY_RELATIONAL_MYSQL_BACKEND_PASSWORD =
+      new ConfigBuilder(MYSQL_ENTITY_STORE_PASSWORD_KEY)
+          .doc("Password of `MySQLBackend`")
+          .version("0.5.0")
+          .stringConf()
+          .createWithDefault("");
+
   ConfigEntry<String> ENTRY_KV_ROCKSDB_BACKEND_PATH =
       new ConfigBuilder(ENTITY_KV_ROCKSDB_BACKEND_PATH_KEY)
           .doc("Directory path of `RocksDBKvBackend`")

diff --git a/core/src/main/java/com/datastrato/gravitino/storage/relational/mysql/MySQLBackend.java b/core/src/main/java/com/datastrato/gravitino/storage/relational/mysql/MySQLBackend.java
@@ -73,8 +73,9 @@ public <E extends Entity & HasIdentifier> List<E> list(
                 String.format("Unsupported entity type: %s for list operation", entityType));
         }
       } catch (Throwable t) {
-        SqlSessions.closeSqlSession();
         throw new RuntimeException(t);
+      } finally {
+        SqlSessions.closeSqlSession();
       }
     }
   }
@@ -98,8 +99,9 @@ public boolean exists(NameIdentifier ident, Entity.EntityType entityType) {
                 String.format("Unsupported entity type: %s for exists operation", entityType));
         }
       } catch (Throwable t) {
-        SqlSessions.closeSqlSession();
         throw new RuntimeException(t);
+      } finally {
+        SqlSessions.closeSqlSession();
       }
     }
   }
@@ -198,8 +200,9 @@ public <E extends Entity & HasIdentifier> E get(
                 String.format("Unsupported entity type: %s for get operation", entityType));
         }
       } catch (Throwable t) {
-        SqlSessions.closeSqlSession();
         throw new RuntimeException(t);
+      } finally {
+        SqlSessions.closeSqlSession();
       }
     }
   }

diff --git a/core/src/main/java/com/datastrato/gravitino/storage/relational/mysql/po/MetalakePO.java b/core/src/main/java/com/datastrato/gravitino/storage/relational/mysql/po/MetalakePO.java
@@ -5,7 +5,6 @@
 
 package com.datastrato.gravitino.storage.relational.mysql.po;
 
-import com.fasterxml.jackson.core.JsonProcessingException;
 import com.google.common.base.Objects;
 
 public class MetalakePO {
@@ -114,17 +113,17 @@ public MetalakePO.Builder withMetalakeComment(String comment) {
       return this;
     }
 
-    public MetalakePO.Builder withProperties(String properties) throws JsonProcessingException {
+    public MetalakePO.Builder withProperties(String properties) {
       metalakePO.properties = properties;
       return this;
     }
 
-    public MetalakePO.Builder withAuditInfo(String auditInfo) throws JsonProcessingException {
+    public MetalakePO.Builder withAuditInfo(String auditInfo) {
       metalakePO.auditInfo = auditInfo;
       return this;
     }
 
-    public MetalakePO.Builder withVersion(String version) throws JsonProcessingException {
+    public MetalakePO.Builder withVersion(String version) {
       metalakePO.schemaVersion = version;
       return this;
     }

diff --git a/...va/com/datastrato/gravitino/storage/relational/mysql/session/SqlSessionFactoryHelper.java b/...va/com/datastrato/gravitino/storage/relational/mysql/session/SqlSessionFactoryHelper.java
@@ -48,10 +48,10 @@ static void setSqlSessionFactory(SqlSessionFactory sessionFactory) {
   @SuppressWarnings("deprecation")
   public void init(Config config) {
     BasicDataSource dataSource = new BasicDataSource();
-    dataSource.setUrl(config.getRawString(Configs.MYSQL_ENTITY_STORE_URL_KEY));
-    dataSource.setDriverClassName(config.getRawString(Configs.MYSQL_ENTITY_STORE_DRIVER_NAME_KEY));
-    dataSource.setUsername(config.getRawString(Configs.MYSQL_ENTITY_STORE_USERNAME_KEY, ""));
-    dataSource.setPassword(config.getRawString(Configs.MYSQL_ENTITY_STORE_PASSWORD_KEY, ""));
+    dataSource.setUrl(config.get(Configs.ENTRY_RELATIONAL_MYSQL_BACKEND_URL));
+    dataSource.setDriverClassName(config.get(Configs.ENTRY_RELATIONAL_MYSQL_BACKEND_DRIVER_NAME));
+    dataSource.setUsername(config.get(Configs.ENTRY_RELATIONAL_MYSQL_BACKEND_USERNAME));
+    dataSource.setPassword(config.get(Configs.ENTRY_RELATIONAL_MYSQL_BACKEND_PASSWORD));
     // close the auto commit, so that need manual commit
     dataSource.setDefaultAutoCommit(false);
     dataSource.setMaxWaitMillis(1000L);

diff --git a/.../src/main/java/com/datastrato/gravitino/storage/relational/mysql/session/SqlSessions.java b/.../src/main/java/com/datastrato/gravitino/storage/relational/mysql/session/SqlSessions.java
@@ -85,7 +85,7 @@ public static void closeSqlSession() {
    * @return the Mapper object.
    * @param <T> the type of the Mapper object.
    */
-  public static <T> T getMapper(Class className) {
+  public static <T> T getMapper(Class<?> className) {
     return (T) getSqlSession().getMapper(className);
   }
 }
diff --git a/core/src/main/resources/mysql/mysql_init.sql b/core/src/main/resources/mysql/mysql_init.sql
@@ -1,3 +1,8 @@
+--
+-- Copyright 2024 Datastrato Pvt Ltd.
+-- This software is licensed under the Apache License version 2.
+--
+
 CREATE TABLE IF NOT EXISTS `metalake_meta`
 (
     `id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'metalake id',

diff --git a/.../src/test/java/com/datastrato/gravitino/storage/relational/TestRelationalEntityStore.java b/.../src/test/java/com/datastrato/gravitino/storage/relational/TestRelationalEntityStore.java
@@ -8,9 +8,9 @@
 import static com.datastrato.gravitino.Configs.DEFAULT_ENTITY_RELATIONAL_STORE;
 import static com.datastrato.gravitino.Configs.ENTITY_RELATIONAL_STORE;
 import static com.datastrato.gravitino.Configs.ENTITY_STORE;
-import static com.datastrato.gravitino.Configs.MYSQL_ENTITY_STORE_DRIVER_NAME_KEY;
-import static com.datastrato.gravitino.Configs.MYSQL_ENTITY_STORE_URL_KEY;
-import static com.datastrato.gravitino.Configs.MYSQL_ENTITY_STORE_USERNAME_KEY;
+import static com.datastrato.gravitino.Configs.ENTRY_RELATIONAL_MYSQL_BACKEND_DRIVER_NAME;
+import static com.datastrato.gravitino.Configs.ENTRY_RELATIONAL_MYSQL_BACKEND_URL;
+import static com.datastrato.gravitino.Configs.ENTRY_RELATIONAL_MYSQL_BACKEND_USERNAME;
 import static com.datastrato.gravitino.Configs.RELATIONAL_ENTITY_STORE;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -69,10 +69,10 @@ public static void setUp() {
     Config config = Mockito.mock(Config.class);
     Mockito.when(config.get(ENTITY_STORE)).thenReturn(RELATIONAL_ENTITY_STORE);
     Mockito.when(config.get(ENTITY_RELATIONAL_STORE)).thenReturn(DEFAULT_ENTITY_RELATIONAL_STORE);
-    Mockito.when(config.getRawString(MYSQL_ENTITY_STORE_URL_KEY))
+    Mockito.when(config.get(ENTRY_RELATIONAL_MYSQL_BACKEND_URL))
         .thenReturn(String.format("jdbc:h2:%s;DB_CLOSE_DELAY=-1;MODE=MYSQL", DB_DIR));
-    Mockito.when(config.getRawString(MYSQL_ENTITY_STORE_USERNAME_KEY)).thenReturn("sa");
-    Mockito.when(config.getRawString(MYSQL_ENTITY_STORE_DRIVER_NAME_KEY))
+    Mockito.when(config.get(ENTRY_RELATIONAL_MYSQL_BACKEND_USERNAME)).thenReturn("sa");
+    Mockito.when(config.get(ENTRY_RELATIONAL_MYSQL_BACKEND_DRIVER_NAME))
         .thenReturn("org.h2.Driver");
     entityStore = EntityStoreFactory.createEntityStore(config);
     entityStore.initialize(config);

diff --git a/.../session/TestSqlSessionFactoryHelper.java → ...ational/mysql/session/TestSqlSession.java b/.../session/TestSqlSessionFactoryHelper.java → ...ational/mysql/session/TestSqlSession.java
@@ -8,12 +8,13 @@
 import static com.datastrato.gravitino.Configs.DEFAULT_ENTITY_RELATIONAL_STORE;
 import static com.datastrato.gravitino.Configs.ENTITY_RELATIONAL_STORE;
 import static com.datastrato.gravitino.Configs.ENTITY_STORE;
-import static com.datastrato.gravitino.Configs.MYSQL_ENTITY_STORE_DRIVER_NAME_KEY;
-import static com.datastrato.gravitino.Configs.MYSQL_ENTITY_STORE_URL_KEY;
-import static com.datastrato.gravitino.Configs.MYSQL_ENTITY_STORE_USERNAME_KEY;
+import static com.datastrato.gravitino.Configs.ENTRY_RELATIONAL_MYSQL_BACKEND_DRIVER_NAME;
+import static com.datastrato.gravitino.Configs.ENTRY_RELATIONAL_MYSQL_BACKEND_URL;
+import static com.datastrato.gravitino.Configs.ENTRY_RELATIONAL_MYSQL_BACKEND_USERNAME;
 import static com.datastrato.gravitino.Configs.RELATIONAL_ENTITY_STORE;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import com.datastrato.gravitino.Config;
@@ -22,14 +23,15 @@
 import java.sql.SQLException;
 import java.util.UUID;
 import org.apache.commons.dbcp2.BasicDataSource;
+import org.apache.ibatis.session.SqlSession;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
 
-public class TestSqlSessionFactoryHelper {
+public class TestSqlSession {
   private static final String MYSQL_STORE_PATH =
       "/tmp/gravitino_test_entityStore_" + UUID.randomUUID().toString().replace("-", "");
   private static final String DB_DIR = MYSQL_STORE_PATH + "/testdb";
@@ -47,16 +49,16 @@ public static void setUp() {
     config = Mockito.mock(Config.class);
     Mockito.when(config.get(ENTITY_STORE)).thenReturn(RELATIONAL_ENTITY_STORE);
     Mockito.when(config.get(ENTITY_RELATIONAL_STORE)).thenReturn(DEFAULT_ENTITY_RELATIONAL_STORE);
-    Mockito.when(config.getRawString(MYSQL_ENTITY_STORE_URL_KEY))
+    Mockito.when(config.get(ENTRY_RELATIONAL_MYSQL_BACKEND_URL))
         .thenReturn(String.format("jdbc:h2:%s;DB_CLOSE_DELAY=-1;MODE=MYSQL", DB_DIR));
-    Mockito.when(config.getRawString(MYSQL_ENTITY_STORE_USERNAME_KEY)).thenReturn("sa");
-    Mockito.when(config.getRawString(MYSQL_ENTITY_STORE_DRIVER_NAME_KEY))
+    Mockito.when(config.get(ENTRY_RELATIONAL_MYSQL_BACKEND_USERNAME)).thenReturn("sa");
+    Mockito.when(config.get(ENTRY_RELATIONAL_MYSQL_BACKEND_DRIVER_NAME))
         .thenReturn("org.h2.Driver");
   }
 
   @BeforeEach
   public void init() {
-    SqlSessionFactoryHelper.setSqlSessionFactory(null);
+    SqlSessionFactoryHelper.getInstance().init(config);
   }
 
   @AfterEach
@@ -70,6 +72,7 @@ public static void tearDown() throws IOException {
     if (dir.exists()) {
       dir.delete();
     }
+    SqlSessionFactoryHelper.setSqlSessionFactory(null);
   }
 
   @Test
@@ -80,6 +83,7 @@ public void testGetInstance() {
 
   @Test
   public void testInit() throws SQLException {
+    SqlSessionFactoryHelper.setSqlSessionFactory(null);
     SqlSessionFactoryHelper.getInstance().init(config);
     assertNotNull(SqlSessionFactoryHelper.getInstance().getSqlSessionFactory());
     BasicDataSource dataSource =
@@ -90,13 +94,38 @@ public void testInit() throws SQLException {
                 .getEnvironment()
                 .getDataSource();
     assertEquals("org.h2.Driver", dataSource.getDriverClassName());
-    assertEquals(config.getRawString(MYSQL_ENTITY_STORE_URL_KEY), dataSource.getUrl());
+    assertEquals(config.get(ENTRY_RELATIONAL_MYSQL_BACKEND_URL), dataSource.getUrl());
   }
 
   @Test
   public void testGetSqlSessionFactoryWithoutInit() {
+    SqlSessionFactoryHelper.setSqlSessionFactory(null);
     assertThrows(
         IllegalStateException.class,
         () -> SqlSessionFactoryHelper.getInstance().getSqlSessionFactory());
   }
+
+  @Test
+  public void testOpenAndCloseSqlSession() {
+    SqlSession session = SqlSessions.getSqlSession();
+    assertNotNull(session);
+    SqlSessions.closeSqlSession();
+    assertNull(SqlSessions.getSessions().get());
+  }
+
+  @Test
+  public void testOpenAndCommitAndCloseSqlSession() {
+    SqlSession session = SqlSessions.getSqlSession();
+    assertNotNull(session);
+    SqlSessions.commitAndCloseSqlSession();
+    assertNull(SqlSessions.getSessions().get());
+  }
+
+  @Test
+  public void testOpenAndRollbackAndCloseSqlSession() {
+    SqlSession session = SqlSessions.getSqlSession();
+    assertNotNull(session);
+    SqlSessions.rollbackAndCloseSqlSession();
+    assertNull(SqlSessions.getSessions().get());
+  }
 }
diff --git a/.../test/java/com/datastrato/gravitino/storage/relational/mysql/session/TestSqlSessions.java b/.../test/java/com/datastrato/gravitino/storage/relational/mysql/session/TestSqlSessions.java