[apache#6004] fix: use fullName instead of names.get(0) when get role (…

…apache#6057) What changes were proposed in this pull request? use fullName instead of names.get(0) when get role Why are the changes needed? Fix: apache#6004 Does this PR introduce any user-facing change? NO How was this patch tested? existing ut
justinmclean · Jan 7, 2025 · 2019fcb · 2019fcb
1 parent d5a29a4
commit 2019fcb
Show file tree

Hide file tree

Showing 4 changed files with 114 additions and 2 deletions.
diff --git a/api/src/main/java/org/apache/gravitino/MetadataObjects.java b/api/src/main/java/org/apache/gravitino/MetadataObjects.java
@@ -21,6 +21,7 @@
 import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Splitter;
+import java.util.Collections;
 import java.util.List;
 import javax.annotation.Nullable;
 import org.apache.commons.lang3.StringUtils;
@@ -151,6 +152,9 @@ public static MetadataObject parse(String fullName, MetadataObject.Type type) {
         StringUtils.isNotBlank(fullName), "Metadata object full name cannot be blank");
 
     List<String> parts = DOT_SPLITTER.splitToList(fullName);
+    if (type == MetadataObject.Type.ROLE) {
+      return MetadataObjects.of(Collections.singletonList(fullName), MetadataObject.Type.ROLE);
+    }
 
     return MetadataObjects.of(parts, type);
   }

diff --git a/api/src/test/java/org/apache/gravitino/TestMetadataObjects.java b/api/src/test/java/org/apache/gravitino/TestMetadataObjects.java
@@ -84,4 +84,19 @@ public void testColumnObject() {
             MetadataObjects.of(
                 Lists.newArrayList("catalog", "schema", "table"), MetadataObject.Type.COLUMN));
   }
+
+  @Test
+  public void testRoleObject() {
+    MetadataObject roleObject = MetadataObjects.of(null, "role.test", MetadataObject.Type.ROLE);
+    Assertions.assertEquals("role.test", roleObject.fullName());
+
+    MetadataObject roleObject1 = MetadataObjects.of(null, "role", MetadataObject.Type.ROLE);
+    Assertions.assertEquals("role", roleObject1.fullName());
+
+    MetadataObject roleObject2 = MetadataObjects.parse("role.test", MetadataObject.Type.ROLE);
+    Assertions.assertEquals("role.test", roleObject2.fullName());
+
+    MetadataObject roleObject3 = MetadataObjects.parse("role", MetadataObject.Type.ROLE);
+    Assertions.assertEquals("role", roleObject3.fullName());
+  }
 }
diff --git a/.../src/main/java/org/apache/gravitino/storage/relational/service/MetadataObjectService.java b/.../src/main/java/org/apache/gravitino/storage/relational/service/MetadataObjectService.java
@@ -50,10 +50,10 @@ public static long getMetadataObjectId(
       return MetalakeMetaService.getInstance().getMetalakeIdByName(fullName);
     }
 
-    List<String> names = DOT_SPLITTER.splitToList(fullName);
     if (type == MetadataObject.Type.ROLE) {
-      return RoleMetaService.getInstance().getRoleIdByMetalakeIdAndName(metalakeId, names.get(0));
+      return RoleMetaService.getInstance().getRoleIdByMetalakeIdAndName(metalakeId, fullName);
     }
+    List<String> names = DOT_SPLITTER.splitToList(fullName);
 
     long catalogId =
         CatalogMetaService.getInstance().getCatalogIdByMetalakeIdAndName(metalakeId, names.get(0));

diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/TestJDBCBackend.java b/core/src/test/java/org/apache/gravitino/storage/relational/TestJDBCBackend.java
@@ -81,6 +81,7 @@
 import org.apache.gravitino.storage.RandomIdGenerator;
 import org.apache.gravitino.storage.relational.mapper.GroupMetaMapper;
 import org.apache.gravitino.storage.relational.mapper.UserMetaMapper;
+import org.apache.gravitino.storage.relational.service.MetalakeMetaService;
 import org.apache.gravitino.storage.relational.service.RoleMetaService;
 import org.apache.gravitino.storage.relational.session.SqlSessionFactoryHelper;
 import org.apache.gravitino.storage.relational.utils.SessionUtils;
@@ -952,6 +953,98 @@ public void testMetaLifeCycleFromCreationToDeletion() throws IOException {
     assertEquals(1, listFilesetVersions(anotherFileset.id()).size());
   }
 
+  @Test
+  public void testGetRoleIdByMetalakeIdAndName() throws IOException {
+    AuditInfo auditInfo =
+        AuditInfo.builder().withCreator("creator").withCreateTime(Instant.now()).build();
+    String metalakeName = "testMetalake";
+    String catalogName = "catalog";
+    String roleNameWithDot = "role.with.dot";
+    String roleNameWithoutDot = "roleWithoutDot";
+
+    BaseMetalake metalake =
+        createBaseMakeLake(RandomIdGenerator.INSTANCE.nextId(), metalakeName, auditInfo);
+    backend.insert(metalake, false);
+
+    CatalogEntity catalog =
+        createCatalog(
+            RandomIdGenerator.INSTANCE.nextId(),
+            NamespaceUtil.ofCatalog(metalakeName),
+            catalogName,
+            auditInfo);
+    backend.insert(catalog, false);
+
+    RoleEntity roleWithDot =
+        createRoleEntity(
+            RandomIdGenerator.INSTANCE.nextId(),
+            AuthorizationUtils.ofRoleNamespace(metalakeName),
+            roleNameWithDot,
+            auditInfo,
+            catalogName);
+    backend.insert(roleWithDot, false);
+
+    RoleEntity roleWithoutDot =
+        createRoleEntity(
+            RandomIdGenerator.INSTANCE.nextId(),
+            AuthorizationUtils.ofRoleNamespace(metalakeName),
+            roleNameWithoutDot,
+            auditInfo,
+            catalogName);
+    backend.insert(roleWithoutDot, false);
+
+    Long metalakeId = MetalakeMetaService.getInstance().getMetalakeIdByName(metalakeName);
+
+    Long roleIdWithDot =
+        RoleMetaService.getInstance().getRoleIdByMetalakeIdAndName(metalakeId, roleNameWithDot);
+    assertEquals(roleWithDot.id(), roleIdWithDot);
+
+    Long roleIdWithoutDot =
+        RoleMetaService.getInstance().getRoleIdByMetalakeIdAndName(metalakeId, roleNameWithoutDot);
+    assertEquals(roleWithoutDot.id(), roleIdWithoutDot);
+  }
+
+  @Test
+  public void testInsertRelationWithDotInRoleName() throws IOException {
+    AuditInfo auditInfo =
+        AuditInfo.builder().withCreator("creator").withCreateTime(Instant.now()).build();
+    String metalakeName = "testMetalake";
+    String catalogName = "catalog";
+    String roleNameWithDot = "role.with.dot";
+
+    BaseMetalake metalake =
+        createBaseMakeLake(RandomIdGenerator.INSTANCE.nextId(), metalakeName, auditInfo);
+    backend.insert(metalake, false);
+
+    CatalogEntity catalog =
+        createCatalog(
+            RandomIdGenerator.INSTANCE.nextId(),
+            NamespaceUtil.ofCatalog(metalakeName),
+            catalogName,
+            auditInfo);
+    backend.insert(catalog, false);
+
+    RoleEntity role =
+        createRoleEntity(
+            RandomIdGenerator.INSTANCE.nextId(),
+            AuthorizationUtils.ofRoleNamespace(metalakeName),
+            roleNameWithDot,
+            auditInfo,
+            catalogName);
+    backend.insert(role, false);
+
+    UserEntity user =
+        createUserEntity(
+            RandomIdGenerator.INSTANCE.nextId(),
+            AuthorizationUtils.ofUserNamespace(metalakeName),
+            "user",
+            auditInfo);
+    backend.insert(user, false);
+
+    backend.insertRelation(
+        OWNER_REL, role.nameIdentifier(), role.type(), user.nameIdentifier(), user.type(), true);
+    assertEquals(1, countActiveOwnerRel(user.id()));
+  }
+
   private boolean legacyRecordExistsInDB(Long id, Entity.EntityType entityType) {
     String tableName;
     String idColumnName;