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 bd67d91cf0 [#8339] feat(authz): map GROUP to GroupEntity in
MetadataIdConverter; add unit test (#8443)
bd67d91cf0 is described below
commit bd67d91cf09360a7604f053788f2d35c896bc741
Author: Minji Ryu <[email protected]>
AuthorDate: Wed Sep 10 21:09:15 2025 +1000
[#8339] feat(authz): map GROUP to GroupEntity in MetadataIdConverter; add
unit test (#8443)
### What changes were proposed in this pull request?
- Map `Entity.EntityType.GROUP` → `GroupEntity.class` in
`MetadataIdConverter`
- Add a unit test to cover the GROUP → GroupEntity code path in
`getID()`
### Why are the changes needed?
- GROUP was mapped to `Entity.class`, which is incorrect and may lead to
runtime issues
(e.g., invalid casting or resolving the wrong class in the entity
store).
- The fix ensures the converter resolves GROUP to the correct entity
class and prevents regressions.
Fix: #8339
### Does this PR introduce _any_ user-facing change?
- No user-facing/API changes.
### How was this patch tested?
- Added a unit test that verifies `getID()` returns the expected id for
GROUP.
- Local runs:
```
./gradlew spotlessApply
./gradlew :server-common:test --tests
'org.apache.gravitino.server.authorization.TestMetadataIdConverter'
./gradlew test -PskipITs
```
### Notes / Follow-ups (for reviewers)
- This PR does **not yet update**:
- `MetadataObjectUtil.checkMetadataObject()` to handle GROUP explicitly.
- The Javadoc / comments in `MetadataObject` for the newly added GROUP
type.
These will need to be clarified, and I’d like to confirm the expected
behavior first.
(e.g., GROUP as a metalake-scoped object)
I’m opening this to get feedback on the current approach before
finalizing those parts.
---
.../server/authorization/MetadataIdConverter.java | 3 ++-
.../authorization/TestMetadataIdConverter.java | 23 +++++++++++++++++++++-
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataIdConverter.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataIdConverter.java
index 4143477bd2..ded3183752 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataIdConverter.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataIdConverter.java
@@ -37,6 +37,7 @@ import org.apache.gravitino.meta.BaseMetalake;
import org.apache.gravitino.meta.CatalogEntity;
import org.apache.gravitino.meta.ColumnEntity;
import org.apache.gravitino.meta.FilesetEntity;
+import org.apache.gravitino.meta.GroupEntity;
import org.apache.gravitino.meta.ModelEntity;
import org.apache.gravitino.meta.ModelVersionEntity;
import org.apache.gravitino.meta.RoleEntity;
@@ -85,7 +86,7 @@ public class MetadataIdConverter {
.put(Entity.EntityType.MODEL_VERSION, ModelVersionEntity.class)
.put(Entity.EntityType.COLUMN, ColumnEntity.class)
.put(Entity.EntityType.USER, UserEntity.class)
- .put(Entity.EntityType.GROUP, Entity.class)
+ .put(Entity.EntityType.GROUP, GroupEntity.class)
.put(Entity.EntityType.ROLE, RoleEntity.class)
.build();
diff --git
a/server-common/src/test/java/org/apache/gravitino/server/authorization/TestMetadataIdConverter.java
b/server-common/src/test/java/org/apache/gravitino/server/authorization/TestMetadataIdConverter.java
index 3d80530bf6..e2f2e5686d 100644
---
a/server-common/src/test/java/org/apache/gravitino/server/authorization/TestMetadataIdConverter.java
+++
b/server-common/src/test/java/org/apache/gravitino/server/authorization/TestMetadataIdConverter.java
@@ -46,6 +46,7 @@ import org.apache.gravitino.meta.BaseMetalake;
import org.apache.gravitino.meta.CatalogEntity;
import org.apache.gravitino.meta.ColumnEntity;
import org.apache.gravitino.meta.FilesetEntity;
+import org.apache.gravitino.meta.GroupEntity;
import org.apache.gravitino.meta.ModelEntity;
import org.apache.gravitino.meta.SchemaEntity;
import org.apache.gravitino.meta.SchemaVersion;
@@ -68,6 +69,7 @@ public class TestMetadataIdConverter {
private NameIdentifier ident5;
private NameIdentifier ident6;
private NameIdentifier ident7;
+ private NameIdentifier ident8;
// Test Entities
private BaseMetalake entity1;
@@ -77,6 +79,7 @@ public class TestMetadataIdConverter {
private ModelEntity entity5;
private FilesetEntity entity6;
private TopicEntity entity7;
+ private GroupEntity entity8;
@BeforeAll
void initTest() throws IOException {
@@ -86,7 +89,7 @@ public class TestMetadataIdConverter {
}
@Test
- void testConvert() throws IOException, IllegalAccessException {
+ void testConvert() throws IllegalAccessException {
CatalogManager mockCatalogManager = mock(CatalogManager.class);
Object originalCatalogManager =
FieldUtils.readDeclaredField(GravitinoEnv.getInstance(),
"catalogManager", true);
@@ -141,6 +144,12 @@ public class TestMetadataIdConverter {
MetadataIdConverter.normalizeCaseSensitive(
eq(ident7), eq(Capability.Scope.TOPIC),
eq(mockCatalogManager)))
.thenReturn(ident7);
+ mockedStatic
+ .when(
+ () ->
+ MetadataIdConverter.normalizeCaseSensitive(
+ eq(ident8), eq(null), eq(mockCatalogManager)))
+ .thenReturn(ident8);
Long metalakeConvertedId =
MetadataIdConverter.getID(
@@ -198,6 +207,7 @@ public class TestMetadataIdConverter {
ident5 = NameIdentifier.of("metalake", "catalog", "schema", "model");
ident6 = NameIdentifier.of("metalake", "catalog", "schema", "fileset");
ident7 = NameIdentifier.of("metalake", "catalog", "schema", "topic");
+ ident8 = NameIdentifier.of("metalake", "group");
}
private void initTestEntities() {
@@ -217,6 +227,7 @@ public class TestMetadataIdConverter {
entity7 =
getTestTopicEntity(
7L, "topic", Namespace.of("metalake", "catalog", "schema"),
"test_topic");
+ entity8 = getTestGroupEntity(8L, "group", Namespace.of("metalake"));
}
private void initMockCache() throws IOException {
@@ -229,6 +240,7 @@ public class TestMetadataIdConverter {
when(mockStore.get(ident5, Entity.EntityType.MODEL,
ModelEntity.class)).thenReturn(entity5);
when(mockStore.get(ident6, Entity.EntityType.FILESET,
FilesetEntity.class)).thenReturn(entity6);
when(mockStore.get(ident7, Entity.EntityType.TOPIC,
TopicEntity.class)).thenReturn(entity7);
+ when(mockStore.get(ident8, Entity.EntityType.GROUP,
GroupEntity.class)).thenReturn(entity8);
}
private BaseMetalake getTestMetalake(long id, String name, String comment) {
@@ -338,4 +350,13 @@ public class TestMetadataIdConverter {
.withProperties(ImmutableMap.of())
.build();
}
+
+ private GroupEntity getTestGroupEntity(long id, String name, Namespace
namespace) {
+ return GroupEntity.builder()
+ .withId(id)
+ .withName(name)
+ .withNamespace(namespace)
+ .withAuditInfo(getTestAuditInfo())
+ .build();
+ }
}
