[#6349] fix(core,hive-catalog): Fix bugs in AuthorizationUtils and re…

…move protobuf-java from catalog hive (#6352) ### What changes were proposed in this pull request? - Fix some working not null judge logic in AuthorizationUtils - Remove `protobuf-java` jar from hive distribution package ### Why are the changes needed? It's bug that need to fix Fix: #6349 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? N/A Co-authored-by: Qi Yu <[email protected]>
apache · Jan 22, 2025 · 2833407 · 2833407
1 parent efd3b36
commit 2833407
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 2 deletions.
diff --git a/catalogs/catalog-hive/build.gradle.kts b/catalogs/catalog-hive/build.gradle.kts
@@ -158,6 +158,8 @@ tasks {
       exclude("guava-*.jar")
       exclude("log4j-*.jar")
       exclude("slf4j-*.jar")
+      // Exclude the following jars to avoid conflict with the jars in authorization-gcp
+      exclude("protobuf-java-*.jar")
     }
     into("$rootDir/distribution/package/catalogs/hive/libs")
   }

diff --git a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
@@ -29,6 +29,7 @@
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.gravitino.Catalog;
 import org.apache.gravitino.Entity;
 import org.apache.gravitino.GravitinoEnv;
@@ -476,7 +477,7 @@ public static List<String> getMetadataObjectLocation(
               Schema schema = GravitinoEnv.getInstance().schemaDispatcher().loadSchema(ident);
               if (schema.properties().containsKey(HiveConstants.LOCATION)) {
                 String schemaLocation = schema.properties().get(HiveConstants.LOCATION);
-                if (schemaLocation != null && schemaLocation.isEmpty()) {
+                if (StringUtils.isNotBlank(schemaLocation)) {
                   locations.add(schemaLocation);
                 } else {
                   LOG.warn("Schema %s location is not found", ident);
@@ -497,7 +498,7 @@ public static List<String> getMetadataObjectLocation(
               Table table = GravitinoEnv.getInstance().tableDispatcher().loadTable(ident);
               if (table.properties().containsKey(HiveConstants.LOCATION)) {
                 String tableLocation = table.properties().get(HiveConstants.LOCATION);
-                if (tableLocation != null && tableLocation.isEmpty()) {
+                if (StringUtils.isNotBlank(tableLocation)) {
                   locations.add(tableLocation);
                 } else {
                   LOG.warn("Table %s location is not found", ident);

diff --git a/core/src/test/java/org/apache/gravitino/authorization/TestAuthorizationUtils.java b/core/src/test/java/org/apache/gravitino/authorization/TestAuthorizationUtils.java
@@ -18,16 +18,25 @@
  */
 package org.apache.gravitino.authorization;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
 import java.util.List;
+import org.apache.commons.lang3.reflect.FieldUtils;
+import org.apache.gravitino.Catalog;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.GravitinoEnv;
 import org.apache.gravitino.NameIdentifier;
 import org.apache.gravitino.Namespace;
+import org.apache.gravitino.catalog.CatalogDispatcher;
+import org.apache.gravitino.catalog.TableDispatcher;
 import org.apache.gravitino.exceptions.IllegalNameIdentifierException;
 import org.apache.gravitino.exceptions.IllegalNamespaceException;
 import org.apache.gravitino.meta.AuditInfo;
 import org.apache.gravitino.meta.RoleEntity;
+import org.apache.gravitino.rel.Table;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
 
 class TestAuthorizationUtils {
 
@@ -210,4 +219,32 @@ void testFilteredSecurableObjects() {
     Assertions.assertTrue(filteredRole.securableObjects().contains(schema2Object));
     Assertions.assertTrue(filteredRole.securableObjects().contains(table2Object));
   }
+
+  @Test
+  void testGetMetadataObjectLocation() throws IllegalAccessException {
+    CatalogDispatcher catalogDispatcher = Mockito.mock(CatalogDispatcher.class);
+    TableDispatcher tableDispatcher = Mockito.mock(TableDispatcher.class);
+    Catalog catalog = Mockito.mock(Catalog.class);
+    Table table = Mockito.mock(Table.class);
+
+    Mockito.when(table.properties()).thenReturn(ImmutableMap.of("location", "gs://bucket/1"));
+    Mockito.when(catalog.provider()).thenReturn("hive");
+    Mockito.when(catalogDispatcher.loadCatalog(Mockito.any())).thenReturn(catalog);
+    Mockito.when(tableDispatcher.loadTable(Mockito.any())).thenReturn(table);
+
+    FieldUtils.writeField(GravitinoEnv.getInstance(), "catalogDispatcher", catalogDispatcher, true);
+    FieldUtils.writeField(GravitinoEnv.getInstance(), "tableDispatcher", tableDispatcher, true);
+
+    List<String> locations =
+        AuthorizationUtils.getMetadataObjectLocation(
+            NameIdentifier.of("catalog", "schema", "table"), Entity.EntityType.TABLE);
+    Assertions.assertEquals(1, locations.size());
+    Assertions.assertEquals("gs://bucket/1", locations.get(0));
+
+    locations =
+        AuthorizationUtils.getMetadataObjectLocation(
+            NameIdentifier.of("catalog", "schema", "fileset"), Entity.EntityType.TABLE);
+    Assertions.assertEquals(1, locations.size());
+    Assertions.assertEquals("gs://bucket/1", locations.get(0));
+  }
 }