[#1253] bugfix(jdbc): fix jdbc catalog store empty audit info issue

catalogs/catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/JdbcCatalogOperations.java

@@ -203,7 +203,7 @@ public JdbcSchema loadSchema(NameIdentifier ident) throws NoSuchSchemaException
     }
     Map<String, String> properties =
         load.properties() == null ? Maps.newHashMap() : Maps.newHashMap(load.properties());
-    StringIdentifier.addToProperties(id, properties);
+    StringIdentifier.newPropertiesWithId(id, properties);
     return new JdbcSchema.Builder()
         .withAuditInfo(load.auditInfo())
         .withName(load.name())
@@ -276,7 +276,7 @@ public Table loadTable(NameIdentifier tableIdent) throws NoSuchTableException {
     }
     Map<String, String> properties =
         load.properties() == null ? Maps.newHashMap() : Maps.newHashMap(load.properties());
-    StringIdentifier.addToProperties(id, properties);
+    properties = StringIdentifier.newPropertiesWithId(id, properties);
     return new JdbcTable.Builder()
         .withAuditInfo(load.auditInfo())
         .withName(tableName)

core/src/main/java/com/datastrato/gravitino/StringIdentifier.java

@@ -8,6 +8,7 @@
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
+import java.util.Collections;
 import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -88,7 +89,7 @@ public String toString() {
    * @param properties the properties to add the string identifier to
    * @return the properties with the string identifier added
    */
-  public static Map<String, String> addToProperties(
+  public static Map<String, String> newPropertiesWithId(
       StringIdentifier stringId, Map<String, String> properties) {
     if (properties == null) {
       return ImmutableMap.of(ID_KEY, stringId.toString());
@@ -100,7 +101,7 @@ public static Map<String, String> addToProperties(
               + "ignore adding the identifier to the properties",
           ID_KEY,
           properties.get(ID_KEY));
-      return properties;
+      return Collections.unmodifiableMap(properties);
     }
 
     return ImmutableMap.<String, String>builder()
@@ -115,13 +116,13 @@ public static Map<String, String> addToProperties(
    * @param properties the properties to remove the string identifier from.
    * @return the properties with the string identifier removed.
    */
-  public static Map<String, String> removeIdFromProperties(Map<String, String> properties) {
+  public static Map<String, String> newPropertiesWithoutId(Map<String, String> properties) {
     if (properties == null) {
       return null;
     }
 
     if (!properties.containsKey(ID_KEY)) {
-      return properties;
+      return Collections.unmodifiableMap(properties);
     }
 
     Map<String, String> copy = Maps.newHashMap(properties);

core/src/main/java/com/datastrato/gravitino/catalog/CatalogManager.java

@@ -258,7 +258,7 @@ public Catalog createCatalog(
             .withType(type)
             .withProvider(provider)
             .withComment(comment)
-            .withProperties(StringIdentifier.addToProperties(stringId, mergedConfig))
+            .withProperties(StringIdentifier.newPropertiesWithId(stringId, mergedConfig))
             .withAuditInfo(
                 new AuditInfo.Builder()
                     .withCreator("gravitino") /* TODO. Should change to real user */

core/src/main/java/com/datastrato/gravitino/catalog/CatalogOperationDispatcher.java

@@ -127,7 +127,8 @@ public Schema createSchema(NameIdentifier ident, String comment, Map<String, Str
     // StringIdentifier to make sure only when the operation is successful, the related
     // SchemaEntity will be visible.
     StringIdentifier stringId = StringIdentifier.fromId(uid);
-    Map<String, String> updatedProperties = StringIdentifier.addToProperties(stringId, properties);
+    Map<String, String> updatedProperties =
+        StringIdentifier.newPropertiesWithId(stringId, properties);
 
     doWithCatalog(
         catalogIdent,
@@ -421,7 +422,8 @@ public Table createTable(
     // StringIdentifier to make sure only when the operation is successful, the related
     // TableEntity will be visible.
     StringIdentifier stringId = StringIdentifier.fromId(uid);
-    Map<String, String> updatedProperties = StringIdentifier.addToProperties(stringId, properties);
+    Map<String, String> updatedProperties =
+        StringIdentifier.newPropertiesWithId(stringId, properties);
 
     doWithCatalog(
         catalogIdent,

core/src/main/java/com/datastrato/gravitino/meta/BaseMetalake.java

@@ -127,7 +127,7 @@ public EntityType type() {
    */
   @Override
   public Map<String, String> properties() {
-    return StringIdentifier.removeIdFromProperties(properties);
+    return StringIdentifier.newPropertiesWithoutId(properties);
   }
 
   /** Builder class for creating instances of {@link BaseMetalake}. */

core/src/main/java/com/datastrato/gravitino/meta/MetalakeManager.java

@@ -103,7 +103,7 @@ public BaseMetalake createMetalake(
             .withId(uid)
             .withName(ident.name())
             .withComment(comment)
-            .withProperties(StringIdentifier.addToProperties(stringId, properties))
+            .withProperties(StringIdentifier.newPropertiesWithId(stringId, properties))
             .withVersion(SchemaVersion.V_0_1)
             .withAuditInfo(
                 new AuditInfo.Builder()

core/src/test/java/com/datastrato/gravitino/TestStringIdentifier.java

@@ -74,14 +74,14 @@ public void testAddStringIdToProperties() {
     StringIdentifier stringId = StringIdentifier.fromId(uid);
 
     Map<String, String> prop = ImmutableMap.of("key1", "value1", "key2", "value2");
-    Map<String, String> propWithId = StringIdentifier.addToProperties(stringId, prop);
+    Map<String, String> propWithId = StringIdentifier.newPropertiesWithId(stringId, prop);
     Assertions.assertTrue(propWithId.containsKey(StringIdentifier.ID_KEY));
     Assertions.assertEquals(stringId.toString(), propWithId.get(StringIdentifier.ID_KEY));
     Assertions.assertEquals("value1", propWithId.get("key1"));
     Assertions.assertEquals("value2", propWithId.get("key2"));
 
     // Test if the input properties is null
-    Map<String, String> propWithId1 = StringIdentifier.addToProperties(stringId, null);
+    Map<String, String> propWithId1 = StringIdentifier.newPropertiesWithId(stringId, null);
     Assertions.assertTrue(propWithId1.containsKey(StringIdentifier.ID_KEY));
     Assertions.assertEquals(stringId.toString(), propWithId1.get(StringIdentifier.ID_KEY));
     Assertions.assertEquals(1, propWithId1.size());
@@ -90,7 +90,7 @@ public void testAddStringIdToProperties() {
     Map<String, String> prop1 =
         ImmutableMap.of("k1", "v1", StringIdentifier.ID_KEY, stringId.toString());
     StringIdentifier newStringId = StringIdentifier.fromId(12341234L);
-    Map<String, String> propWithId2 = StringIdentifier.addToProperties(newStringId, prop1);
+    Map<String, String> propWithId2 = StringIdentifier.newPropertiesWithId(newStringId, prop1);
     Assertions.assertEquals(stringId.toString(), propWithId2.get(StringIdentifier.ID_KEY));
   }
 
@@ -100,7 +100,7 @@ public void testGetStringIdFromProperties() {
     StringIdentifier stringId = StringIdentifier.fromId(uid);
 
     Map<String, String> prop = ImmutableMap.of("key1", "value1", "key2", "value2");
-    Map<String, String> propWithId = StringIdentifier.addToProperties(stringId, prop);
+    Map<String, String> propWithId = StringIdentifier.newPropertiesWithId(stringId, prop);
     StringIdentifier stringIdFromProp = StringIdentifier.fromProperties(propWithId);
     Assertions.assertEquals(stringId.id(), stringIdFromProp.id());
 

integration-test/src/test/java/com/datastrato/gravitino/integration/test/catalog/jdbc/mysql/CatalogMysqlIT.java

@@ -375,6 +375,11 @@ void testAlterAndDropMysqlTable() {
     Assertions.assertEquals("col_4", table.columns()[3].name());
     Assertions.assertEquals(Types.StringType.get(), table.columns()[3].dataType());
     Assertions.assertNull(table.columns()[3].comment());
+    Assertions.assertNotNull(table.auditInfo());
+    Assertions.assertNotNull(table.auditInfo().createTime());
+    Assertions.assertNotNull(table.auditInfo().creator());
+    Assertions.assertNotNull(table.auditInfo().lastModifiedTime());
+    Assertions.assertNotNull(table.auditInfo().lastModifier());
 
     ColumnDTO col1 =
         new ColumnDTO.Builder()

integration-test/src/test/java/com/datastrato/gravitino/integration/test/catalog/jdbc/postgresql/CatalogPostgreSqlIT.java

@@ -384,6 +384,11 @@ void testAlterAndDropPostgreSqlTable() {
     Assertions.assertEquals(Types.StringType.get(), table.columns()[3].dataType());
     Assertions.assertNull(table.columns()[3].comment());
     Assertions.assertTrue(table.columns()[3].nullable());
+    Assertions.assertNotNull(table.auditInfo());
+    Assertions.assertNotNull(table.auditInfo().createTime());
+    Assertions.assertNotNull(table.auditInfo().creator());
+    Assertions.assertNotNull(table.auditInfo().lastModifiedTime());
+    Assertions.assertNotNull(table.auditInfo().lastModifier());
 
     ColumnDTO col1 =
         new ColumnDTO.Builder()