diff --git a/catalogs/catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/JdbcCatalogOperations.java b/catalogs/catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/JdbcCatalogOperations.java index 404e980f8ab..61d6a520c88 100644 --- a/catalogs/catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/JdbcCatalogOperations.java +++ b/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 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 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) diff --git a/core/src/main/java/com/datastrato/gravitino/StringIdentifier.java b/core/src/main/java/com/datastrato/gravitino/StringIdentifier.java index 942ce27ab53..6ece84a80dd 100644 --- a/core/src/main/java/com/datastrato/gravitino/StringIdentifier.java +++ b/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 addToProperties( + public static Map newPropertiesWithId( StringIdentifier stringId, Map properties) { if (properties == null) { return ImmutableMap.of(ID_KEY, stringId.toString()); @@ -100,7 +101,7 @@ public static Map addToProperties( + "ignore adding the identifier to the properties", ID_KEY, properties.get(ID_KEY)); - return properties; + return Collections.unmodifiableMap(properties); } return ImmutableMap.builder() @@ -115,13 +116,13 @@ public static Map addToProperties( * @param properties the properties to remove the string identifier from. * @return the properties with the string identifier removed. */ - public static Map removeIdFromProperties(Map properties) { + public static Map newPropertiesWithoutId(Map properties) { if (properties == null) { return null; } if (!properties.containsKey(ID_KEY)) { - return properties; + return Collections.unmodifiableMap(properties); } Map copy = Maps.newHashMap(properties); diff --git a/core/src/main/java/com/datastrato/gravitino/catalog/CatalogManager.java b/core/src/main/java/com/datastrato/gravitino/catalog/CatalogManager.java index 4f41a0e7c32..7a9283add17 100644 --- a/core/src/main/java/com/datastrato/gravitino/catalog/CatalogManager.java +++ b/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 */ diff --git a/core/src/main/java/com/datastrato/gravitino/catalog/CatalogOperationDispatcher.java b/core/src/main/java/com/datastrato/gravitino/catalog/CatalogOperationDispatcher.java index 2409cefd331..6fb28d9f548 100644 --- a/core/src/main/java/com/datastrato/gravitino/catalog/CatalogOperationDispatcher.java +++ b/core/src/main/java/com/datastrato/gravitino/catalog/CatalogOperationDispatcher.java @@ -127,7 +127,8 @@ public Schema createSchema(NameIdentifier ident, String comment, Map updatedProperties = StringIdentifier.addToProperties(stringId, properties); + Map 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 updatedProperties = StringIdentifier.addToProperties(stringId, properties); + Map updatedProperties = + StringIdentifier.newPropertiesWithId(stringId, properties); doWithCatalog( catalogIdent, diff --git a/core/src/main/java/com/datastrato/gravitino/meta/BaseMetalake.java b/core/src/main/java/com/datastrato/gravitino/meta/BaseMetalake.java index f6de4bf2a54..bbe9e118ef3 100644 --- a/core/src/main/java/com/datastrato/gravitino/meta/BaseMetalake.java +++ b/core/src/main/java/com/datastrato/gravitino/meta/BaseMetalake.java @@ -127,7 +127,7 @@ public EntityType type() { */ @Override public Map properties() { - return StringIdentifier.removeIdFromProperties(properties); + return StringIdentifier.newPropertiesWithoutId(properties); } /** Builder class for creating instances of {@link BaseMetalake}. */ diff --git a/core/src/main/java/com/datastrato/gravitino/meta/MetalakeManager.java b/core/src/main/java/com/datastrato/gravitino/meta/MetalakeManager.java index 5ca502f367a..2ff1ab8c7d9 100644 --- a/core/src/main/java/com/datastrato/gravitino/meta/MetalakeManager.java +++ b/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() diff --git a/core/src/test/java/com/datastrato/gravitino/TestStringIdentifier.java b/core/src/test/java/com/datastrato/gravitino/TestStringIdentifier.java index d480c7d24e9..a4e8d2130d2 100644 --- a/core/src/test/java/com/datastrato/gravitino/TestStringIdentifier.java +++ b/core/src/test/java/com/datastrato/gravitino/TestStringIdentifier.java @@ -74,14 +74,14 @@ public void testAddStringIdToProperties() { StringIdentifier stringId = StringIdentifier.fromId(uid); Map prop = ImmutableMap.of("key1", "value1", "key2", "value2"); - Map propWithId = StringIdentifier.addToProperties(stringId, prop); + Map 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 propWithId1 = StringIdentifier.addToProperties(stringId, null); + Map 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 prop1 = ImmutableMap.of("k1", "v1", StringIdentifier.ID_KEY, stringId.toString()); StringIdentifier newStringId = StringIdentifier.fromId(12341234L); - Map propWithId2 = StringIdentifier.addToProperties(newStringId, prop1); + Map 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 prop = ImmutableMap.of("key1", "value1", "key2", "value2"); - Map propWithId = StringIdentifier.addToProperties(stringId, prop); + Map propWithId = StringIdentifier.newPropertiesWithId(stringId, prop); StringIdentifier stringIdFromProp = StringIdentifier.fromProperties(propWithId); Assertions.assertEquals(stringId.id(), stringIdFromProp.id()); diff --git a/integration-test/src/test/java/com/datastrato/gravitino/integration/test/catalog/jdbc/mysql/CatalogMysqlIT.java b/integration-test/src/test/java/com/datastrato/gravitino/integration/test/catalog/jdbc/mysql/CatalogMysqlIT.java index af2309e20a3..843edd58fe6 100644 --- a/integration-test/src/test/java/com/datastrato/gravitino/integration/test/catalog/jdbc/mysql/CatalogMysqlIT.java +++ b/integration-test/src/test/java/com/datastrato/gravitino/integration/test/catalog/jdbc/mysql/CatalogMysqlIT.java @@ -377,6 +377,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() diff --git a/integration-test/src/test/java/com/datastrato/gravitino/integration/test/catalog/jdbc/postgresql/CatalogPostgreSqlIT.java b/integration-test/src/test/java/com/datastrato/gravitino/integration/test/catalog/jdbc/postgresql/CatalogPostgreSqlIT.java index 23f564ec37e..dddfc480adb 100644 --- a/integration-test/src/test/java/com/datastrato/gravitino/integration/test/catalog/jdbc/postgresql/CatalogPostgreSqlIT.java +++ b/integration-test/src/test/java/com/datastrato/gravitino/integration/test/catalog/jdbc/postgresql/CatalogPostgreSqlIT.java @@ -385,6 +385,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()