[#1047] improvement(core): Fix the issue of StringIdentifier in metal…

…ake property (#1188)

### What changes were proposed in this pull request?

Remove the `stringId` from metalake property map. 

### Why are the changes needed?

`gravitino.identifier` is generated internally and should not be visible
to users.

Fix: #1047 

### Does this PR introduce _any_ user-facing change?

N/A

### How was this patch tested?

Existing UTs can cover it.

Co-authored-by: Qi Yu <[email protected]>

common/src/main/java/com/datastrato/gravitino/dto/MetalakeDTO.java

@@ -7,14 +7,13 @@
 import com.datastrato.gravitino.Audit;
 import com.datastrato.gravitino.Metalake;
 import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Objects;
 import com.google.common.base.Preconditions;
 import java.util.Map;
 import javax.annotation.Nullable;
-import lombok.EqualsAndHashCode;
 import lombok.ToString;
 
 /** Represents a Metalake Data Transfer Object (DTO) that implements the Metalake interface. */
-@EqualsAndHashCode
 @ToString
 public class MetalakeDTO implements Metalake {
 
@@ -132,4 +131,40 @@ public MetalakeDTO build() {
       return new MetalakeDTO(name, comment, properties, audit);
     }
   }
+
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) {
+      return true;
+    }
+    if (o == null || getClass() != o.getClass()) {
+      return false;
+    }
+    MetalakeDTO that = (MetalakeDTO) o;
+    return Objects.equal(name, that.name)
+        && Objects.equal(comment, that.comment)
+        && propertyEqual(properties, that.properties)
+        && Objects.equal(audit, that.audit);
+  }
+
+  private boolean propertyEqual(Map<String, String> p1, Map<String, String> p2) {
+    if (p1 == null && p2 == null) {
+      return true;
+    }
+
+    if (p1 != null && p1.isEmpty() && p2 == null) {
+      return true;
+    }
+
+    if (p2 != null && p2.isEmpty() && p1 == null) {
+      return true;
+    }
+
+    return java.util.Objects.equals(p1, p2);
+  }
+
+  @Override
+  public int hashCode() {
+    return Objects.hashCode(name, comment, audit);
+  }
 }

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

@@ -7,6 +7,7 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
 import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -108,6 +109,27 @@ public static Map<String, String> addToProperties(
         .build();
   }
 
+  /**
+   * Remove StringIdentifier from properties.
+   *
+   * @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) {
+    if (properties == null) {
+      return null;
+    }
+
+    if (!properties.containsKey(ID_KEY)) {
+      return properties;
+    }
+
+    Map<String, String> copy = Maps.newHashMap(properties);
+    copy.remove(ID_KEY);
+
+    return ImmutableMap.<String, String>builder().putAll(copy).build();
+  }
+
   public static StringIdentifier fromProperties(Map<String, String> properties) {
     if (properties == null) {
       return null;

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

@@ -10,6 +10,7 @@
 import com.datastrato.gravitino.Field;
 import com.datastrato.gravitino.HasIdentifier;
 import com.datastrato.gravitino.Metalake;
+import com.datastrato.gravitino.StringIdentifier;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -126,7 +127,7 @@ public EntityType type() {
    */
   @Override
   public Map<String, String> properties() {
-    return properties;
+    return StringIdentifier.removeIdFromProperties(properties);
   }
 
   /** Builder class for creating instances of {@link BaseMetalake}. */

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

@@ -52,9 +52,8 @@ public MetalakeManager(EntityStore store, IdGenerator idGenerator) {
   @Override
   public BaseMetalake[] listMetalakes() {
     try {
-      return store
-          .list(Namespace.empty(), BaseMetalake.class, EntityType.METALAKE)
-          .toArray(new BaseMetalake[0]);
+      return store.list(Namespace.empty(), BaseMetalake.class, EntityType.METALAKE).stream()
+          .toArray(BaseMetalake[]::new);
     } catch (IOException ioe) {
       LOG.error("Listing Metalakes failed due to storage issues.", ioe);
       throw new RuntimeException(ioe);

core/src/main/java/com/datastrato/gravitino/storage/kv/KvGarbageCollector.java

@@ -44,7 +44,7 @@ public final class KvGarbageCollector implements Closeable {
       new ScheduledThreadPoolExecutor(
           2,
           r -> {
-            Thread t = new Thread(r, "KvEntityStore-Garbage-Collector-%d");
+            Thread t = new Thread(r, "KvEntityStore-Garbage-Collector");
             t.setDaemon(true);
             return t;
           },

core/src/test/java/com/datastrato/gravitino/meta/TestMetalakeManager.java

@@ -173,8 +173,6 @@ private void testProperties(Map<String, String> expectedProps, Map<String, Strin
           Assertions.assertEquals(v, testProps.get(k));
         });
 
-    Assertions.assertTrue(testProps.containsKey(StringIdentifier.ID_KEY));
-    StringIdentifier StringId = StringIdentifier.fromString(testProps.get(StringIdentifier.ID_KEY));
-    Assertions.assertEquals(StringId.toString(), testProps.get(StringIdentifier.ID_KEY));
+    Assertions.assertFalse(testProps.containsKey(StringIdentifier.ID_KEY));
   }
 }