Merge branch 'main' into gvfs-fuse-dev

apache · Jan 3, 2025 · 63e0d7f · 63e0d7f
2 parents 0d83d95 + 2732792
commit 63e0d7f
Show file tree

Hide file tree

Showing 20 changed files with 281 additions and 72 deletions.
diff --git a/...test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java b/...test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java
@@ -121,6 +121,10 @@ void testManageUsers() {
         users.stream().map(User::name).collect(Collectors.toList()));
     Assertions.assertEquals(Lists.newArrayList("role1"), users.get(2).roles());
 
+    // ISSUE-6061: Test listUsers with revoked users
+    metalake.revokeRolesFromUser(Lists.newArrayList("role1"), username);
+    Assertions.assertEquals(3, metalake.listUsers().length);
+
     // Get a not-existed user
     Assertions.assertThrows(NoSuchUserException.class, () -> metalake.getUser("not-existed"));
 
@@ -176,6 +180,10 @@ void testManageGroups() {
         groups.stream().map(Group::name).collect(Collectors.toList()));
     Assertions.assertEquals(Lists.newArrayList("role2"), groups.get(0).roles());
 
+    // ISSUE-6061: Test listGroups with revoked groups
+    metalake.revokeRolesFromGroup(Lists.newArrayList("role2"), groupName);
+    Assertions.assertEquals(2, metalake.listGroups().length);
+
     Assertions.assertTrue(metalake.removeGroup(groupName));
     Assertions.assertFalse(metalake.removeGroup(groupName));
 

diff --git a/common/src/main/java/org/apache/gravitino/dto/requests/RoleCreateRequest.java b/common/src/main/java/org/apache/gravitino/dto/requests/RoleCreateRequest.java
@@ -79,5 +79,14 @@ public void validate() throws IllegalArgumentException {
     Preconditions.checkArgument(
         StringUtils.isNotBlank(name), "\"name\" field is required and cannot be empty");
     Preconditions.checkArgument(securableObjects != null, "\"securableObjects\" can't null ");
+    for (SecurableObjectDTO objectDTO : securableObjects) {
+      Preconditions.checkArgument(
+          StringUtils.isNotBlank(objectDTO.name()), "\" securable object name\" can't be blank");
+      Preconditions.checkArgument(
+          objectDTO.type() != null, "\" securable object type\" can't be null");
+      Preconditions.checkArgument(
+          objectDTO.privileges() != null && !objectDTO.privileges().isEmpty(),
+          "\"securable object privileges\" can't be null or empty");
+    }
   }
 }
diff --git a/...rg/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java b/...rg/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java
@@ -57,16 +57,19 @@ public String listExtendedGroupPOsByMetalakeId(Long metalakeId) {
         + " JSON_ARRAYAGG(rot.role_id) as roleIds"
         + " FROM "
         + GROUP_TABLE_NAME
-        + " gt LEFT OUTER JOIN "
+        + " gt LEFT OUTER JOIN ("
+        + " SELECT * FROM "
         + GROUP_ROLE_RELATION_TABLE_NAME
-        + " rt ON rt.group_id = gt.group_id"
-        + " LEFT OUTER JOIN "
+        + " WHERE deleted_at = 0)"
+        + " AS rt ON rt.group_id = gt.group_id"
+        + " LEFT OUTER JOIN ("
+        + " SELECT * FROM "
         + ROLE_TABLE_NAME
-        + " rot ON rot.role_id = rt.role_id"
+        + " WHERE deleted_at = 0)"
+        + " AS rot ON rot.role_id = rt.role_id"
         + " WHERE "
         + " gt.deleted_at = 0 AND"
-        + " (rot.deleted_at = 0 OR rot.deleted_at is NULL) AND"
-        + " (rt.deleted_at = 0 OR rt.deleted_at is NULL) AND gt.metalake_id = #{metalakeId}"
+        + " gt.metalake_id = #{metalakeId}"
         + " GROUP BY gt.group_id";
   }
 

diff --git a/...org/apache/gravitino/storage/relational/mapper/provider/base/UserMetaBaseSQLProvider.java b/...org/apache/gravitino/storage/relational/mapper/provider/base/UserMetaBaseSQLProvider.java
@@ -165,16 +165,19 @@ public String listExtendedUserPOsByMetalakeId(@Param("metalakeId") Long metalake
         + " JSON_ARRAYAGG(rot.role_id) as roleIds"
         + " FROM "
         + USER_TABLE_NAME
-        + " ut LEFT OUTER JOIN "
+        + " ut LEFT OUTER JOIN ("
+        + " SELECT * FROM "
         + USER_ROLE_RELATION_TABLE_NAME
-        + " rt ON rt.user_id = ut.user_id"
-        + " LEFT OUTER JOIN "
+        + " WHERE deleted_at = 0)"
+        + " AS rt ON rt.user_id = ut.user_id"
+        + " LEFT OUTER JOIN ("
+        + " SELECT * FROM "
         + ROLE_TABLE_NAME
-        + " rot ON rot.role_id = rt.role_id"
+        + " WHERE deleted_at = 0)"
+        + " AS rot ON rot.role_id = rt.role_id"
         + " WHERE "
         + " ut.deleted_at = 0 AND"
-        + " (rot.deleted_at = 0 OR rot.deleted_at is NULL) AND"
-        + " (rt.deleted_at = 0 OR rt.deleted_at is NULL) AND ut.metalake_id = #{metalakeId}"
+        + " ut.metalake_id = #{metalakeId}"
         + " GROUP BY ut.user_id";
   }
 

diff --git a/.../java/org/apache/gravitino/storage/relational/mapper/provider/h2/GroupMetaH2Provider.java b/.../java/org/apache/gravitino/storage/relational/mapper/provider/h2/GroupMetaH2Provider.java
@@ -37,16 +37,19 @@ public String listExtendedGroupPOsByMetalakeId(@Param("metalakeId") Long metalak
         + " '[' || GROUP_CONCAT('\"' || rot.role_id || '\"') || ']' as roleIds"
         + " FROM "
         + GROUP_TABLE_NAME
-        + " gt LEFT OUTER JOIN "
+        + " gt LEFT OUTER JOIN ("
+        + " SELECT * FROM "
         + GROUP_ROLE_RELATION_TABLE_NAME
-        + " rt ON rt.group_id = gt.group_id"
-        + " LEFT OUTER JOIN "
+        + " WHERE deleted_at = 0)"
+        + " AS rt ON rt.group_id = gt.group_id"
+        + " LEFT OUTER JOIN ("
+        + " SELECT * FROM "
         + ROLE_TABLE_NAME
-        + " rot ON rot.role_id = rt.role_id"
+        + " WHERE deleted_at = 0)"
+        + " AS rot ON rot.role_id = rt.role_id"
         + " WHERE "
         + " gt.deleted_at = 0 AND"
-        + " (rot.deleted_at = 0 OR rot.deleted_at is NULL) AND"
-        + " (rt.deleted_at = 0 OR rt.deleted_at is NULL) AND gt.metalake_id = #{metalakeId}"
+        + " gt.metalake_id = #{metalakeId}"
         + " GROUP BY gt.group_id";
   }
 }
diff --git a/...n/java/org/apache/gravitino/storage/relational/mapper/provider/h2/UserMetaH2Provider.java b/...n/java/org/apache/gravitino/storage/relational/mapper/provider/h2/UserMetaH2Provider.java
@@ -37,16 +37,19 @@ public String listExtendedUserPOsByMetalakeId(@Param("metalakeId") Long metalake
         + " '[' || GROUP_CONCAT('\"' || rot.role_id || '\"') || ']' as roleIds"
         + " FROM "
         + USER_TABLE_NAME
-        + " ut LEFT OUTER JOIN "
+        + " ut LEFT OUTER JOIN ("
+        + " SELECT * FROM "
         + USER_ROLE_RELATION_TABLE_NAME
-        + " rt ON rt.user_id = ut.user_id"
-        + " LEFT OUTER JOIN "
+        + " WHERE deleted_at = 0)"
+        + " AS rt ON rt.user_id = ut.user_id"
+        + " LEFT OUTER JOIN ("
+        + " SELECT * FROM "
         + ROLE_TABLE_NAME
-        + " rot ON rot.role_id = rt.role_id"
+        + " WHERE deleted_at = 0)"
+        + " AS rot ON rot.role_id = rt.role_id"
         + " WHERE "
         + " ut.deleted_at = 0 AND "
-        + "(rot.deleted_at = 0 OR rot.deleted_at is NULL) AND "
-        + "(rt.deleted_at = 0 OR rt.deleted_at is NULL) AND ut.metalake_id = #{metalakeId}"
+        + " ut.metalake_id = #{metalakeId}"
         + " GROUP BY ut.user_id";
   }
 }
diff --git a/.../gravitino/storage/relational/mapper/provider/postgresql/GroupMetaPostgreSQLProvider.java b/.../gravitino/storage/relational/mapper/provider/postgresql/GroupMetaPostgreSQLProvider.java
@@ -80,16 +80,19 @@ public String listExtendedGroupPOsByMetalakeId(Long metalakeId) {
         + " JSON_AGG(rot.role_id) as roleIds"
         + " FROM "
         + GROUP_TABLE_NAME
-        + " gt LEFT OUTER JOIN "
+        + " gt LEFT OUTER JOIN ("
+        + " SELECT * FROM "
         + GROUP_ROLE_RELATION_TABLE_NAME
-        + " rt ON rt.group_id = gt.group_id"
-        + " LEFT OUTER JOIN "
+        + " WHERE deleted_at = 0)"
+        + " AS rt ON rt.group_id = gt.group_id"
+        + " LEFT OUTER JOIN ("
+        + " SELECT * FROM "
         + ROLE_TABLE_NAME
-        + " rot ON rot.role_id = rt.role_id"
+        + " WHERE deleted_at = 0)"
+        + " AS rot ON rot.role_id = rt.role_id"
         + " WHERE "
         + " gt.deleted_at = 0 AND"
-        + " (rot.deleted_at = 0 OR rot.deleted_at is NULL) AND"
-        + " (rt.deleted_at = 0 OR rt.deleted_at is NULL) AND gt.metalake_id = #{metalakeId}"
+        + " gt.metalake_id = #{metalakeId}"
         + " GROUP BY gt.group_id";
   }
 }
diff --git a/...e/gravitino/storage/relational/mapper/provider/postgresql/UserMetaPostgreSQLProvider.java b/...e/gravitino/storage/relational/mapper/provider/postgresql/UserMetaPostgreSQLProvider.java
@@ -80,16 +80,19 @@ public String listExtendedUserPOsByMetalakeId(Long metalakeId) {
         + " JSON_AGG(rot.role_id) as roleIds"
         + " FROM "
         + USER_TABLE_NAME
-        + " ut LEFT OUTER JOIN "
+        + " ut LEFT OUTER JOIN ("
+        + " SELECT * FROM "
         + USER_ROLE_RELATION_TABLE_NAME
-        + " rt ON rt.user_id = ut.user_id"
-        + " LEFT OUTER JOIN "
+        + " WHERE deleted_at = 0)"
+        + " AS rt ON rt.user_id = ut.user_id"
+        + " LEFT OUTER JOIN ("
+        + " SELECT * FROM "
         + ROLE_TABLE_NAME
-        + " rot ON rot.role_id = rt.role_id"
+        + " WHERE deleted_at = 0)"
+        + " AS rot ON rot.role_id = rt.role_id"
         + " WHERE "
         + " ut.deleted_at = 0 AND"
-        + " (rot.deleted_at = 0 OR rot.deleted_at is NULL) AND"
-        + " (rt.deleted_at = 0 OR rt.deleted_at is NULL) AND ut.metalake_id = #{metalakeId}"
+        + " ut.metalake_id = #{metalakeId}"
         + " GROUP BY ut.user_id";
   }
 }
diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestGroupMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestGroupMetaService.java
@@ -27,6 +27,7 @@
 import java.sql.SQLException;
 import java.sql.Statement;
 import java.time.Instant;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Optional;
@@ -189,6 +190,33 @@ void testListGroups() throws IOException {
         }
       }
     }
+
+    // ISSUE-6061: Test listGroupsByNamespace with revoked users
+    Function<GroupEntity, GroupEntity> revokeUpdater =
+        group -> {
+          AuditInfo updateAuditInfo =
+              AuditInfo.builder()
+                  .withCreator(group.auditInfo().creator())
+                  .withCreateTime(group.auditInfo().createTime())
+                  .withLastModifier("revokeGroup")
+                  .withLastModifiedTime(Instant.now())
+                  .build();
+
+          return GroupEntity.builder()
+              .withNamespace(group.namespace())
+              .withId(group.id())
+              .withName(group.name())
+              .withRoleNames(Collections.emptyList())
+              .withRoleIds(Collections.emptyList())
+              .withAuditInfo(updateAuditInfo)
+              .build();
+        };
+
+    Assertions.assertNotNull(groupMetaService.updateGroup(group2.nameIdentifier(), revokeUpdater));
+    actualGroups =
+        groupMetaService.listGroupsByNamespace(
+            AuthorizationUtils.ofGroupNamespace(metalakeName), true);
+    Assertions.assertEquals(expectGroups.size(), actualGroups.size());
   }
 
   @Test

diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestUserMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestUserMetaService.java
@@ -27,6 +27,7 @@
 import java.sql.SQLException;
 import java.sql.Statement;
 import java.time.Instant;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Optional;
@@ -188,6 +189,33 @@ void testListUsers() throws IOException {
         }
       }
     }
+
+    // ISSUE-6061: Test listUsersByNamespace with revoked users
+    Function<UserEntity, UserEntity> revokeUpdater =
+        user -> {
+          AuditInfo updateAuditInfo =
+              AuditInfo.builder()
+                  .withCreator(user.auditInfo().creator())
+                  .withCreateTime(user.auditInfo().createTime())
+                  .withLastModifier("revokeUser")
+                  .withLastModifiedTime(Instant.now())
+                  .build();
+
+          return UserEntity.builder()
+              .withNamespace(user.namespace())
+              .withId(user.id())
+              .withName(user.name())
+              .withRoleNames(Collections.emptyList())
+              .withRoleIds(Collections.emptyList())
+              .withAuditInfo(updateAuditInfo)
+              .build();
+        };
+
+    Assertions.assertNotNull(userMetaService.updateUser(user1.nameIdentifier(), revokeUpdater));
+    actualUsers =
+        userMetaService.listUsersByNamespace(
+            AuthorizationUtils.ofUserNamespace(metalakeName), true);
+    Assertions.assertEquals(expectUsers.size(), actualUsers.size());
   }
 
   @Test

diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java
@@ -103,11 +103,13 @@ public Response addGroup(@PathParam("metalake") String metalake, GroupAddRequest
               TreeLockUtils.doWithTreeLock(
                   NameIdentifier.of(AuthorizationUtils.ofGroupNamespace(metalake).levels()),
                   LockType.WRITE,
-                  () ->
-                      Utils.ok(
-                          new GroupResponse(
-                              DTOConverters.toDTO(
-                                  accessControlManager.addGroup(metalake, request.getName()))))));
+                  () -> {
+                    request.validate();
+                    return Utils.ok(
+                        new GroupResponse(
+                            DTOConverters.toDTO(
+                                accessControlManager.addGroup(metalake, request.getName()))));
+                  }));
     } catch (Exception e) {
       return ExceptionHandlers.handleGroupException(
           OperationType.ADD, request.getName(), metalake, e);

diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java
@@ -113,6 +113,7 @@ public Response setOwnerForObject(
       return Utils.doAs(
           httpRequest,
           () -> {
+            request.validate();
             MetadataObjectUtil.checkMetadataObject(metalake, object);
             NameIdentifier objectIdent = MetadataObjectUtil.toEntityIdent(metalake, object);
             TreeLockUtils.doWithTreeLock(