[#3132] improvement(core): Use a more elegant validation of User, Group and Role NameIdentifier

core/src/main/java/com/datastrato/gravitino/authorization/AuthorizationUtils.java

@@ -69,4 +69,40 @@ public static Namespace ofGroupNamespace(String metalake) {
   public static Namespace ofUserNamespace(String metalake) {
     return Namespace.of(metalake, Entity.SYSTEM_CATALOG_RESERVED_NAME, Entity.USER_SCHEMA_NAME);
   }
+
+  public static void checkUser(NameIdentifier ident) {
+    NameIdentifier.check(ident != null, "User identifier must not be null");
+    checkUserNamespace(ident.namespace());
+  }
+
+  public static void checkGroup(NameIdentifier ident) {
+    NameIdentifier.check(ident != null, "Group identifier must not be null");
+    checkGroupNamespace(ident.namespace());
+  }
+
+  public static void checkRole(NameIdentifier ident) {
+    NameIdentifier.check(ident != null, "Role identifier must not be null");
+    checkRoleNamespace(ident.namespace());
+  }
+
+  public static void checkUserNamespace(Namespace namespace) {
+    Namespace.check(
+        namespace != null && namespace.length() == 3,
+        "User namespace must be non-null and have 3 levels, the input namespace is %s",
+        namespace);
+  }
+
+  public static void checkGroupNamespace(Namespace namespace) {
+    Namespace.check(
+        namespace != null && namespace.length() == 3,
+        "Group namespace must be non-null and have 3 levels, the input namespace is %s",
+        namespace);
+  }
+
+  public static void checkRoleNamespace(Namespace namespace) {
+    Namespace.check(
+        namespace != null && namespace.length() == 3,
+        "Role namespace must be non-null and have 3 levels, the input namespace is %s",
+        namespace);
+  }
 }

core/src/main/java/com/datastrato/gravitino/storage/relational/service/GroupMetaService.java

@@ -7,6 +7,7 @@
 import com.datastrato.gravitino.Entity;
 import com.datastrato.gravitino.HasIdentifier;
 import com.datastrato.gravitino.NameIdentifier;
+import com.datastrato.gravitino.authorization.AuthorizationUtils;
 import com.datastrato.gravitino.exceptions.NoSuchEntityException;
 import com.datastrato.gravitino.meta.GroupEntity;
 import com.datastrato.gravitino.storage.relational.mapper.GroupMetaMapper;
@@ -67,11 +68,8 @@ private Long getGroupIdByMetalakeIdAndName(Long metalakeId, String groupName) {
   }
 
   public GroupEntity getGroupByIdentifier(NameIdentifier identifier) {
-    Preconditions.checkArgument(
-        identifier != null
-            && !identifier.namespace().isEmpty()
-            && identifier.namespace().levels().length == 3,
-        "The identifier should not be null and should have three level.");
+    AuthorizationUtils.checkGroup(identifier);
+
     Long metalakeId =
         MetalakeMetaService.getInstance().getMetalakeIdByName(identifier.namespace().level(0));
     GroupPO groupPO = getGroupPOByMetalakeIdAndName(metalakeId, identifier.name());
@@ -82,11 +80,7 @@ public GroupEntity getGroupByIdentifier(NameIdentifier identifier) {
 
   public void insertGroup(GroupEntity groupEntity, boolean overwritten) {
     try {
-      Preconditions.checkArgument(
-          groupEntity.namespace() != null
-              && !groupEntity.namespace().isEmpty()
-              && groupEntity.namespace().levels().length == 3,
-          "The namespace of groupEntity should not be null and should have three level.");
+      AuthorizationUtils.checkGroup(groupEntity.nameIdentifier());
 
       Long metalakeId =
           MetalakeMetaService.getInstance().getMetalakeIdByName(groupEntity.namespace().level(0));
@@ -130,11 +124,8 @@ public void insertGroup(GroupEntity groupEntity, boolean overwritten) {
   }
 
   public boolean deleteGroup(NameIdentifier identifier) {
-    Preconditions.checkArgument(
-        identifier != null
-            && !identifier.namespace().isEmpty()
-            && identifier.namespace().levels().length == 3,
-        "The identifier should not be null and should have three level.");
+    AuthorizationUtils.checkGroup(identifier);
+
     Long metalakeId =
         MetalakeMetaService.getInstance().getMetalakeIdByName(identifier.namespace().level(0));
     Long groupId = getGroupIdByMetalakeIdAndName(metalakeId, identifier.name());
@@ -152,11 +143,7 @@ public boolean deleteGroup(NameIdentifier identifier) {
 
   public <E extends Entity & HasIdentifier> GroupEntity updateGroup(
       NameIdentifier identifier, Function<E, E> updater) {
-    Preconditions.checkArgument(
-        identifier != null
-            && !identifier.namespace().isEmpty()
-            && identifier.namespace().levels().length == 3,
-        "The identifier should not be null and should have three level.");
+    AuthorizationUtils.checkGroup(identifier);
 
     Long metalakeId =
         MetalakeMetaService.getInstance().getMetalakeIdByName(identifier.namespace().level(0));

core/src/main/java/com/datastrato/gravitino/storage/relational/service/RoleMetaService.java

@@ -6,6 +6,7 @@
 
 import com.datastrato.gravitino.Entity;
 import com.datastrato.gravitino.NameIdentifier;
+import com.datastrato.gravitino.authorization.AuthorizationUtils;
 import com.datastrato.gravitino.exceptions.NoSuchEntityException;
 import com.datastrato.gravitino.meta.RoleEntity;
 import com.datastrato.gravitino.storage.relational.mapper.GroupRoleRelMapper;
@@ -15,7 +16,6 @@
 import com.datastrato.gravitino.storage.relational.utils.ExceptionUtils;
 import com.datastrato.gravitino.storage.relational.utils.POConverters;
 import com.datastrato.gravitino.storage.relational.utils.SessionUtils;
-import com.google.common.base.Preconditions;
 import java.util.List;
 
 /** The service class for role metadata. It provides the basic database operations for role. */
@@ -70,11 +70,7 @@ public List<RolePO> listRolesByGroupId(Long groupId) {
 
   public void insertRole(RoleEntity roleEntity, boolean overwritten) {
     try {
-      Preconditions.checkArgument(
-          roleEntity.namespace() != null
-              && !roleEntity.namespace().isEmpty()
-              && roleEntity.namespace().levels().length == 3,
-          "The namespace of RoleEntity must have three levels.");
+      AuthorizationUtils.checkRole(roleEntity.nameIdentifier());
 
       Long metalakeId =
           MetalakeMetaService.getInstance().getMetalakeIdByName(roleEntity.namespace().level(0));
@@ -99,11 +95,8 @@ public void insertRole(RoleEntity roleEntity, boolean overwritten) {
   }
 
   public RoleEntity getRoleByIdentifier(NameIdentifier identifier) {
-    Preconditions.checkArgument(
-        identifier != null
-            && !identifier.namespace().isEmpty()
-            && identifier.namespace().levels().length == 3,
-        "The identifier of Role must have three levels.");
+    AuthorizationUtils.checkRole(identifier);
+
     Long metalakeId =
         MetalakeMetaService.getInstance().getMetalakeIdByName(identifier.namespace().level(0));
     RolePO rolePO = getRolePOByMetalakeIdAndName(metalakeId, identifier.name());
@@ -112,11 +105,8 @@ public RoleEntity getRoleByIdentifier(NameIdentifier identifier) {
   }
 
   public boolean deleteRole(NameIdentifier identifier) {
-    Preconditions.checkArgument(
-        identifier != null
-            && !identifier.namespace().isEmpty()
-            && identifier.namespace().levels().length == 3,
-        "The identifier of Role must have three levels.");
+    AuthorizationUtils.checkRole(identifier);
+
     Long metalakeId =
         MetalakeMetaService.getInstance().getMetalakeIdByName(identifier.namespace().level(0));
     Long roleId = getRoleIdByMetalakeIdAndName(metalakeId, identifier.name());

core/src/main/java/com/datastrato/gravitino/storage/relational/service/UserMetaService.java

@@ -7,6 +7,7 @@
 import com.datastrato.gravitino.Entity;
 import com.datastrato.gravitino.HasIdentifier;
 import com.datastrato.gravitino.NameIdentifier;
+import com.datastrato.gravitino.authorization.AuthorizationUtils;
 import com.datastrato.gravitino.exceptions.NoSuchEntityException;
 import com.datastrato.gravitino.meta.UserEntity;
 import com.datastrato.gravitino.storage.relational.mapper.UserMetaMapper;
@@ -67,11 +68,8 @@ private Long getUserIdByMetalakeIdAndName(Long metalakeId, String userName) {
   }
 
   public UserEntity getUserByIdentifier(NameIdentifier identifier) {
-    Preconditions.checkArgument(
-        identifier != null
-            && !identifier.namespace().isEmpty()
-            && identifier.namespace().levels().length == 3,
-        "The identifier should not be null and should have three level.");
+    AuthorizationUtils.checkUser(identifier);
+
     Long metalakeId =
         MetalakeMetaService.getInstance().getMetalakeIdByName(identifier.namespace().level(0));
     UserPO userPO = getUserPOByMetalakeIdAndName(metalakeId, identifier.name());
@@ -82,11 +80,7 @@ public UserEntity getUserByIdentifier(NameIdentifier identifier) {
 
   public void insertUser(UserEntity userEntity, boolean overwritten) {
     try {
-      Preconditions.checkArgument(
-          userEntity.namespace() != null
-              && !userEntity.namespace().isEmpty()
-              && userEntity.namespace().levels().length == 3,
-          "The identifier should not be null and should have three level.");
+      AuthorizationUtils.checkUser(userEntity.nameIdentifier());
 
       Long metalakeId =
           MetalakeMetaService.getInstance().getMetalakeIdByName(userEntity.namespace().level(0));
@@ -130,11 +124,8 @@ public void insertUser(UserEntity userEntity, boolean overwritten) {
   }
 
   public boolean deleteUser(NameIdentifier identifier) {
-    Preconditions.checkArgument(
-        identifier != null
-            && !identifier.namespace().isEmpty()
-            && identifier.namespace().levels().length == 3,
-        "The identifier should not be null and should have three level.");
+    AuthorizationUtils.checkUser(identifier);
+
     Long metalakeId =
         MetalakeMetaService.getInstance().getMetalakeIdByName(identifier.namespace().level(0));
     Long userId = getUserIdByMetalakeIdAndName(metalakeId, identifier.name());
@@ -151,11 +142,7 @@ public boolean deleteUser(NameIdentifier identifier) {
 
   public <E extends Entity & HasIdentifier> UserEntity updateUser(
       NameIdentifier identifier, Function<E, E> updater) {
-    Preconditions.checkArgument(
-        identifier != null
-            && !identifier.namespace().isEmpty()
-            && identifier.namespace().levels().length == 3,
-        "The identifier should not be null and should have three level.");
+    AuthorizationUtils.checkUser(identifier);
 
     Long metalakeId =
         MetalakeMetaService.getInstance().getMetalakeIdByName(identifier.namespace().level(0));

core/src/test/java/com/datastrato/gravitino/authorization/TestAuthorizationUtils.java

@@ -0,0 +1,126 @@
+/*
+ * Copyright 2024 Datastrato Pvt Ltd.
+ * This software is licensed under the Apache License version 2.
+ */
+package com.datastrato.gravitino.authorization;
+
+import com.datastrato.gravitino.NameIdentifier;
+import com.datastrato.gravitino.Namespace;
+import com.datastrato.gravitino.exceptions.IllegalNameIdentifierException;
+import com.datastrato.gravitino.exceptions.IllegalNamespaceException;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+class TestAuthorizationUtils {
+
+  String metalake = "metalake";
+
+  @Test
+  void testCreateNameIdentifier() {
+    NameIdentifier user = AuthorizationUtils.ofUser(metalake, "user");
+    NameIdentifier group = AuthorizationUtils.ofGroup(metalake, "group");
+    NameIdentifier role = AuthorizationUtils.ofRole(metalake, "role");
+
+    Assertions.assertEquals(AuthorizationUtils.ofUserNamespace(metalake), user.namespace());
+    Assertions.assertEquals("user", user.name());
+    Assertions.assertEquals(AuthorizationUtils.ofGroupNamespace(metalake), group.namespace());
+    Assertions.assertEquals("group", group.name());
+    Assertions.assertEquals(AuthorizationUtils.ofRoleNamespace(metalake), role.namespace());
+    Assertions.assertEquals("role", role.name());
+  }
+
+  @Test
+  void testCreateNameIdentifierWithInvalidArgs() {
+    Assertions.assertThrows(
+        IllegalNameIdentifierException.class, () -> AuthorizationUtils.ofUser(metalake, null));
+    Assertions.assertThrows(
+        IllegalNameIdentifierException.class, () -> AuthorizationUtils.ofUser(metalake, ""));
+    Assertions.assertThrows(
+        IllegalNameIdentifierException.class, () -> AuthorizationUtils.ofGroup(metalake, null));
+    Assertions.assertThrows(
+        IllegalNameIdentifierException.class, () -> AuthorizationUtils.ofGroup(metalake, ""));
+    Assertions.assertThrows(
+        IllegalNameIdentifierException.class, () -> AuthorizationUtils.ofRole(metalake, null));
+    Assertions.assertThrows(
+        IllegalNameIdentifierException.class, () -> AuthorizationUtils.ofRole(metalake, ""));
+  }
+
+  @Test
+  void testCreateNamespace() {
+    Namespace namespace = AuthorizationUtils.ofUserNamespace(metalake);
+    Assertions.assertEquals(3, namespace.length());
+    Assertions.assertEquals(metalake, namespace.level(0));
+    Assertions.assertEquals("system", namespace.level(1));
+    Assertions.assertEquals("user", namespace.level(2));
+
+    namespace = AuthorizationUtils.ofGroupNamespace(metalake);
+    Assertions.assertEquals(3, namespace.length());
+    Assertions.assertEquals(metalake, namespace.level(0));
+    Assertions.assertEquals("system", namespace.level(1));
+    Assertions.assertEquals("group", namespace.level(2));
+
+    namespace = AuthorizationUtils.ofRoleNamespace(metalake);
+    Assertions.assertEquals(3, namespace.length());
+    Assertions.assertEquals(metalake, namespace.level(0));
+    Assertions.assertEquals("system", namespace.level(1));
+    Assertions.assertEquals("role", namespace.level(2));
+  }
+
+  @Test
+  void testCreateNamespaceWithInvalidArgs() {
+    Assertions.assertThrows(
+        IllegalNamespaceException.class, () -> AuthorizationUtils.ofUserNamespace(null));
+    Assertions.assertThrows(
+        IllegalNamespaceException.class, () -> AuthorizationUtils.ofUserNamespace(""));
+    Assertions.assertThrows(
+        IllegalNamespaceException.class, () -> AuthorizationUtils.ofGroupNamespace(null));
+    Assertions.assertThrows(
+        IllegalNamespaceException.class, () -> AuthorizationUtils.ofGroupNamespace(""));
+    Assertions.assertThrows(
+        IllegalNamespaceException.class, () -> AuthorizationUtils.ofRoleNamespace(null));
+    Assertions.assertThrows(
+        IllegalNamespaceException.class, () -> AuthorizationUtils.ofRoleNamespace(""));
+  }
+
+  @Test
+  void testCheckNameIdentifier() {
+    NameIdentifier user = AuthorizationUtils.ofUser(metalake, "user");
+    NameIdentifier group = AuthorizationUtils.ofGroup(metalake, "group");
+    NameIdentifier role = AuthorizationUtils.ofRole(metalake, "role");
+
+    Assertions.assertDoesNotThrow(() -> AuthorizationUtils.checkUser(user));
+    Assertions.assertDoesNotThrow(() -> AuthorizationUtils.checkGroup(group));
+    Assertions.assertDoesNotThrow(() -> AuthorizationUtils.checkRole(role));
+
+    Assertions.assertThrows(
+        IllegalNameIdentifierException.class,
+        () -> AuthorizationUtils.checkUser(NameIdentifier.of("")));
+    Assertions.assertThrows(
+        IllegalNameIdentifierException.class,
+        () -> AuthorizationUtils.checkGroup(NameIdentifier.of("")));
+    Assertions.assertThrows(
+        IllegalNameIdentifierException.class,
+        () -> AuthorizationUtils.checkRole(NameIdentifier.of("")));
+  }
+
+  @Test
+  void testCheckNamespace() {
+    Namespace userNamespace = AuthorizationUtils.ofUserNamespace(metalake);
+    Namespace groupNamespace = AuthorizationUtils.ofGroupNamespace(metalake);
+    Namespace roleNamespace = AuthorizationUtils.ofRoleNamespace(metalake);
+
+    Assertions.assertDoesNotThrow(() -> AuthorizationUtils.checkUserNamespace(userNamespace));
+    Assertions.assertDoesNotThrow(() -> AuthorizationUtils.checkGroupNamespace(groupNamespace));
+    Assertions.assertDoesNotThrow(() -> AuthorizationUtils.checkRoleNamespace(roleNamespace));
+
+    Assertions.assertThrows(
+        IllegalNamespaceException.class,
+        () -> AuthorizationUtils.checkUserNamespace(Namespace.of("a", "b")));
+    Assertions.assertThrows(
+        IllegalNamespaceException.class,
+        () -> AuthorizationUtils.checkGroupNamespace(Namespace.of("a")));
+    Assertions.assertThrows(
+        IllegalNamespaceException.class,
+        () -> AuthorizationUtils.checkRoleNamespace(Namespace.of("a", "b", "c", "d")));
+  }
+}