This is an automated email from the ASF dual-hosted git repository.
jerryshao 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 364a4edb77 [#12654] fix(core): Fix fileset schema location resolution
when schema properties omit location (#12655)
364a4edb77 is described below
commit 364a4edb77def2505622ec2d5565841640755272
Author: Tanay Paul <[email protected]>
AuthorDate: Thu Aug 27 13:14:11 2026 +0530
[#12654] fix(core): Fix fileset schema location resolution when schema
properties omit location (#12655)
### What changes were proposed in this pull request?
Fix the fileset schema location resolution logic in
`AuthorizationUtils.getMetadataObjectLocation` so that when a schema
does not specify an explicit `location` property (inheriting the
catalog's location), it correctly falls back to `catalogLocation + "/" +
schema.name()`.
### Why are the changes needed?
Previously, the fallback check `else if
(catalogObj.properties().containsKey(FILESET_CATALOG_LOCATION))` was
nested inside `if
(schema.properties().containsKey(FILESET_SCHEMA_LOCATION)`. When a
schema relied on catalog location inheritance (where
`schema.properties()` does not have the `"location"` key), the outer
condition evaluated to `false`, returning an empty location list and
preventing authorization plugins (e.g. Ranger HDFS) from generating or
cleaning up storage path policies.
Fix: #12654
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Expanded
`TestAuthorizationUtils.testGetSchemaTypeMetadataObjectLocation` to
cover all scenarios:
1. Schema without `location` property (inheriting catalog location).
2. Schema with `null` properties map.
3. Schema with empty string `location` property.
4. Schema with explicit `location` property override.
5. Catalog and schema both without `location` property.
---
.../authorization/AuthorizationUtils.java | 24 +++++++++--------
.../authorization/TestAuthorizationUtils.java | 31 ++++++++++++++++++++--
2 files changed, 42 insertions(+), 13 deletions(-)
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 8ca88fb5da..aee06ea128 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
@@ -735,19 +735,21 @@ public class AuthorizationUtils {
case FILESET:
if ("fileset".equals(catalogObj.provider())) {
- if (schema.properties().containsKey(FILESET_SCHEMA_LOCATION)) {
- String schemaLocation =
schema.properties().get(FILESET_SCHEMA_LOCATION);
- if (StringUtils.isNotBlank(schemaLocation)) {
+ String schemaLocation =
+ schema.properties() == null
+ ? null
+ : schema.properties().get(FILESET_SCHEMA_LOCATION);
+ if (StringUtils.isNotBlank(schemaLocation)) {
+ locations.add(schemaLocation);
+ } else if (catalogObj.properties() != null
+ &&
catalogObj.properties().containsKey(FILESET_CATALOG_LOCATION)) {
+ String catalogLocation =
catalogObj.properties().get(FILESET_CATALOG_LOCATION);
+ if (StringUtils.isNotBlank(catalogLocation)) {
+ schemaLocation = catalogLocation + "/" + schema.name();
locations.add(schemaLocation);
- } else if
(catalogObj.properties().containsKey(FILESET_CATALOG_LOCATION)) {
- String catalogLocation =
catalogObj.properties().get(FILESET_CATALOG_LOCATION);
- if (StringUtils.isNotBlank(catalogLocation)) {
- schemaLocation = catalogLocation + "/" + schema.name();
- locations.add(schemaLocation);
- }
- } else {
- LOG.warn("Schema {} location is not found", ident);
}
+ } else {
+ LOG.warn("Schema {} location is not found", ident);
}
}
break;
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 48bae76cbd..31d0368490 100644
---
a/core/src/test/java/org/apache/gravitino/authorization/TestAuthorizationUtils.java
+++
b/core/src/test/java/org/apache/gravitino/authorization/TestAuthorizationUtils.java
@@ -22,6 +22,7 @@ import static org.apache.gravitino.Catalog.Type.FILESET;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
+import java.util.Collections;
import java.util.List;
import org.apache.commons.lang3.reflect.FieldUtils;
import org.apache.gravitino.Catalog;
@@ -322,9 +323,7 @@ class TestAuthorizationUtils {
Catalog catalog = Mockito.mock(Catalog.class);
Schema schema = Mockito.mock(Schema.class);
- Mockito.when(schema.properties()).thenReturn(ImmutableMap.of("location",
""));
Mockito.when(schema.name()).thenReturn("testSchema");
- Mockito.when(catalog.properties()).thenReturn(ImmutableMap.of("location",
"catalogLocation"));
Mockito.when(catalog.provider()).thenReturn("fileset");
Mockito.when(catalog.type()).thenReturn(FILESET);
Mockito.when(schemaDispatcher.loadSchema(Mockito.any())).thenReturn(schema);
@@ -337,18 +336,46 @@ class TestAuthorizationUtils {
FieldUtils.writeField(
GravitinoEnv.getInstance(), "internalSchemaDispatcher",
schemaDispatcher, true);
+ // Case 1: Schema has no location property (inherits catalog location)
+ Mockito.when(schema.properties()).thenReturn(Collections.emptyMap());
+ Mockito.when(catalog.properties()).thenReturn(ImmutableMap.of("location",
"catalogLocation"));
List<String> locations =
AuthorizationUtils.getMetadataObjectLocation(
NameIdentifier.of("catalog", "schema", "fileset"),
Entity.EntityType.SCHEMA);
Assertions.assertEquals(1, locations.size());
Assertions.assertEquals("catalogLocation/testSchema", locations.get(0));
+ // Case 2: Schema has null properties (inherits catalog location)
+ Mockito.when(schema.properties()).thenReturn(null);
+ locations =
+ AuthorizationUtils.getMetadataObjectLocation(
+ NameIdentifier.of("catalog", "schema", "fileset"),
Entity.EntityType.SCHEMA);
+ Assertions.assertEquals(1, locations.size());
+ Assertions.assertEquals("catalogLocation/testSchema", locations.get(0));
+
+ // Case 3: Schema location is empty string (falls back to catalog location)
+ Mockito.when(schema.properties()).thenReturn(ImmutableMap.of("location",
""));
+ locations =
+ AuthorizationUtils.getMetadataObjectLocation(
+ NameIdentifier.of("catalog", "schema", "fileset"),
Entity.EntityType.SCHEMA);
+ Assertions.assertEquals(1, locations.size());
+ Assertions.assertEquals("catalogLocation/testSchema", locations.get(0));
+
+ // Case 4: Schema has explicit location (takes precedence over catalog
location)
Mockito.when(schema.properties()).thenReturn(ImmutableMap.of("location",
"schemaLocation"));
locations =
AuthorizationUtils.getMetadataObjectLocation(
NameIdentifier.of("catalog", "schema", "fileset"),
Entity.EntityType.SCHEMA);
Assertions.assertEquals(1, locations.size());
Assertions.assertEquals("schemaLocation", locations.get(0));
+
+ // Case 5: Neither schema nor catalog has location property
+ Mockito.when(schema.properties()).thenReturn(Collections.emptyMap());
+ Mockito.when(catalog.properties()).thenReturn(Collections.emptyMap());
+ locations =
+ AuthorizationUtils.getMetadataObjectLocation(
+ NameIdentifier.of("catalog", "schema", "fileset"),
Entity.EntityType.SCHEMA);
+ Assertions.assertEquals(0, locations.size());
}
@Test
