From 2833407de3ebd261226564c3300be178e21e1cf9 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 22 Jan 2025 19:19:01 +0800 Subject: [PATCH] [#6349] fix(core,hive-catalog): Fix bugs in AuthorizationUtils and remove 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 --- 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 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/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); 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)); + } }