From 49eb3bb988ebbbd6ddf5329b4db52f81c96e9b72 Mon Sep 17 00:00:00 2001 From: yuqi Date: Wed, 22 Jan 2025 16:55:55 +0800 Subject: [PATCH 1/3] Fix bugs in `AuthorizationUtils` and remove `protobuf-java` from catalog Hive --- catalogs/hive-metastore-common/build.gradle.kts | 3 +++ .../apache/gravitino/authorization/AuthorizationUtils.java | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/catalogs/hive-metastore-common/build.gradle.kts b/catalogs/hive-metastore-common/build.gradle.kts index 539c8291dd1..26ec123c78b 100644 --- a/catalogs/hive-metastore-common/build.gradle.kts +++ b/catalogs/hive-metastore-common/build.gradle.kts @@ -61,6 +61,9 @@ dependencies { exclude("org.eclipse.jetty.orbit", "javax.servlet") exclude("org.openjdk.jol") exclude("org.slf4j") + // protobuf-java is not needed for Hive Metastore, and it will conflict with gcp authorizations + // library + exclude("com.google.protobuf", "protobuf-java") } implementation(libs.hadoop2.common) { exclude("*") 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 499ba5cbf1f..60ffe3e83c8 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.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 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 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); From 5c4a0530e0d802f9eed4716b3e1fc85fcdd0559d Mon Sep 17 00:00:00 2001 From: yuqi Date: Wed, 22 Jan 2025 17:07:36 +0800 Subject: [PATCH 2/3] fix --- catalogs/catalog-hive/build.gradle.kts | 2 ++ catalogs/hive-metastore-common/build.gradle.kts | 3 --- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/catalogs/catalog-hive/build.gradle.kts b/catalogs/catalog-hive/build.gradle.kts index 6a8b815ab97..b5593c6f7e4 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/catalogs/hive-metastore-common/build.gradle.kts b/catalogs/hive-metastore-common/build.gradle.kts index 26ec123c78b..539c8291dd1 100644 --- a/catalogs/hive-metastore-common/build.gradle.kts +++ b/catalogs/hive-metastore-common/build.gradle.kts @@ -61,9 +61,6 @@ dependencies { exclude("org.eclipse.jetty.orbit", "javax.servlet") exclude("org.openjdk.jol") exclude("org.slf4j") - // protobuf-java is not needed for Hive Metastore, and it will conflict with gcp authorizations - // library - exclude("com.google.protobuf", "protobuf-java") } implementation(libs.hadoop2.common) { exclude("*") From 41b2b8640ce60aad0c532f7a6fbc6df24cb1c7da Mon Sep 17 00:00:00 2001 From: yuqi Date: Wed, 22 Jan 2025 17:59:50 +0800 Subject: [PATCH 3/3] add ut --- .../authorization/TestAuthorizationUtils.java | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) 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 b602471c4d1..373785d539b 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 @@ 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 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)); + } }