This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 97b61ca0d6 [#6349] fix(core,hive-catalog): Fix bugs in
AuthorizationUtils and remove protobuf-java from catalog hive (#6351)
97b61ca0d6 is described below
commit 97b61ca0d6c7c46affae64e568301a4e6bf31337
Author: Qi Yu <y...@datastrato.com>
AuthorDate: Wed Jan 22 18:59:21 2025 +0800
[#6349] fix(core,hive-catalog): Fix bugs in AuthorizationUtils and remove
protobuf-java from catalog hive (#6351)
### 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?
UT.
---
catalogs/catalog-hive/build.gradle.kts | 2 ++
.../authorization/AuthorizationUtils.java | 5 +--
.../authorization/TestAuthorizationUtils.java | 37 ++++++++++++++++++++++
3 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/catalogs/catalog-hive/build.gradle.kts
b/catalogs/catalog-hive/build.gradle.kts
index 6a8b815ab9..b5593c6f7e 100644
--- 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
index 499ba5cbf1..60ffe3e83c 100644
---
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.Set;
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 class AuthorizationUtils {
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 class AuthorizationUtils {
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
index b602471c4d..373785d539 100644
---
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 @@ class TestAuthorizationUtils {
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));
+ }
}
