[apache#1760] Fix(core): Fix bug when introducing fileset api in Cata…

…logOperationDispatcher (apache#1762) ### What changes were proposed in this pull request? Using the correct PropertiesMetadata to check the value when doing operations. ### Why are the changes needed? This is bug introduced in apache#1667 , fix it here. Fix: apache#1706 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Add UTs.
ch3yne · Jan 29, 2024 · 23b045e · 23b045e
1 parent ee8d4a7
commit 23b045e
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 5 deletions.
diff --git a/core/src/main/java/com/datastrato/gravitino/catalog/CatalogOperationDispatcher.java b/core/src/main/java/com/datastrato/gravitino/catalog/CatalogOperationDispatcher.java
@@ -398,7 +398,7 @@ public Table loadTable(NameIdentifier ident) throws NoSuchTableException {
           .withHiddenPropertiesSet(
               getHiddenPropertyNames(
                   catalogIdentifier,
-                  HasPropertyMetadata::filesetPropertiesMetadata,
+                  HasPropertyMetadata::tablePropertiesMetadata,
                   table.properties()));
     }
 
@@ -667,7 +667,9 @@ public Fileset loadFileset(NameIdentifier ident) throws NoSuchFilesetException {
     return EntityCombinedFileset.of(fileset)
         .withHiddenPropertiesSet(
             getHiddenPropertyNames(
-                catalogIdent, HasPropertyMetadata::tablePropertiesMetadata, fileset.properties()));
+                catalogIdent,
+                HasPropertyMetadata::filesetPropertiesMetadata,
+                fileset.properties()));
   }
 
   @Override
@@ -705,7 +707,7 @@ public Fileset createFileset(
         .withHiddenPropertiesSet(
             getHiddenPropertyNames(
                 catalogIdent,
-                HasPropertyMetadata::tablePropertiesMetadata,
+                HasPropertyMetadata::filesetPropertiesMetadata,
                 createdFileset.properties()));
   }
 
@@ -725,7 +727,7 @@ public Fileset alterFileset(NameIdentifier ident, FilesetChange... changes)
         .withHiddenPropertiesSet(
             getHiddenPropertyNames(
                 catalogIdent,
-                HasPropertyMetadata::tablePropertiesMetadata,
+                HasPropertyMetadata::filesetPropertiesMetadata,
                 alteredFileset.properties()));
   }
 

diff --git a/core/src/test/java/com/datastrato/gravitino/TestCatalogOperations.java b/core/src/test/java/com/datastrato/gravitino/TestCatalogOperations.java
@@ -63,7 +63,7 @@ public TestCatalogOperations(Map<String, String> config) {
     filesets = Maps.newHashMap();
     tablePropertiesMetadata = new TestBasePropertiesMetadata();
     schemaPropertiesMetadata = new TestBasePropertiesMetadata();
-    filesetPropertiesMetadata = new TestBasePropertiesMetadata();
+    filesetPropertiesMetadata = new TestFilesetPropertiesMetadata();
     this.config = config;
   }
 

diff --git a/core/src/test/java/com/datastrato/gravitino/TestFilesetPropertiesMetadata.java b/core/src/test/java/com/datastrato/gravitino/TestFilesetPropertiesMetadata.java
@@ -0,0 +1,42 @@
+/*
+ * Copyright 2024 Datastrato Pvt Ltd.
+ * This software is licensed under the Apache License version 2.
+ */
+package com.datastrato.gravitino;
+
+import com.datastrato.gravitino.catalog.PropertyEntry;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
+import java.util.List;
+import java.util.Map;
+
+public class TestFilesetPropertiesMetadata extends TestBasePropertiesMetadata {
+
+  public static final String TEST_FILESET_HIDDEN_KEY = "fileset_key";
+
+  private static final Map<String, PropertyEntry<?>> TEST_FILESET_PROPERTY;
+
+  static {
+    List<PropertyEntry<?>> tablePropertyMetadata =
+        ImmutableList.of(
+            PropertyEntry.stringPropertyEntry(
+                TEST_FILESET_HIDDEN_KEY,
+                "test fileset required k1 property",
+                false,
+                false,
+                "test",
+                true,
+                false));
+
+    TEST_FILESET_PROPERTY = Maps.uniqueIndex(tablePropertyMetadata, PropertyEntry::getName);
+  }
+
+  @Override
+  protected Map<String, PropertyEntry<?>> specificPropertyEntries() {
+    return ImmutableMap.<String, PropertyEntry<?>>builder()
+        .putAll(super.specificPropertyEntries())
+        .putAll(TEST_FILESET_PROPERTY)
+        .build();
+  }
+}
diff --git a/core/src/test/java/com/datastrato/gravitino/catalog/TestCatalogOperationDispatcher.java b/core/src/test/java/com/datastrato/gravitino/catalog/TestCatalogOperationDispatcher.java
@@ -8,6 +8,7 @@
 import static com.datastrato.gravitino.Entity.EntityType.TABLE;
 import static com.datastrato.gravitino.StringIdentifier.ID_KEY;
 import static com.datastrato.gravitino.TestBasePropertiesMetadata.COMMENT_KEY;
+import static com.datastrato.gravitino.TestFilesetPropertiesMetadata.TEST_FILESET_HIDDEN_KEY;
 import static com.datastrato.gravitino.catalog.BasePropertiesMetadata.GRAVITINO_MANAGED_ENTITY;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -678,6 +679,7 @@ private void testProperties(Map<String, String> expectedProps, Map<String, Strin
         });
     Assertions.assertFalse(testProps.containsKey(StringIdentifier.ID_KEY));
     Assertions.assertFalse(testProps.containsKey(GRAVITINO_MANAGED_ENTITY));
+    Assertions.assertFalse(testProps.containsKey(TEST_FILESET_HIDDEN_KEY));
   }
 
   private void testPropertyException(Executable operation, String... errorMessage) {