From 289d5bd6bce270ab09ad6075b47ebc8a42c641d2 Mon Sep 17 00:00:00 2001 From: roryqi Date: Sat, 7 Sep 2024 00:24:02 +0800 Subject: [PATCH] [#4608] feat(core): Supports to load fields lazily (#4690) ### What changes were proposed in this pull request? Supports to load fields lazily. ### Why are the changes needed? Fix: #4608 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Add new uts and pass the existing uts --- .../ranger/RangerAuthorizationPlugin.java | 6 +- .../ranger/integration/test/RangerHiveIT.java | 42 ++---- .../authorization/PermissionManager.java | 82 +++++------ .../authorization/UserGroupManager.java | 4 +- .../apache/gravitino/meta/GroupEntity.java | 58 ++++---- .../org/apache/gravitino/meta/RoleEntity.java | 41 +++--- .../org/apache/gravitino/meta/UserEntity.java | 58 ++++---- .../gravitino/proto/GroupEntitySerDe.java | 69 ---------- .../gravitino/proto/ProtoEntitySerDe.java | 13 +- .../gravitino/proto/RoleEntitySerDe.java | 115 ---------------- .../gravitino/proto/UserEntitySerDe.java | 69 ---------- .../storage/kv/BinaryEntityKeyEncoder.java | 9 -- .../relational/service/GroupMetaService.java | 26 ++-- .../relational/service/OwnerMetaService.java | 6 +- .../relational/service/RoleMetaService.java | 42 +----- .../relational/service/UserMetaService.java | 25 ++-- .../relational/utils/POConverters.java | 63 +++++---- .../relational/utils/SupplierUtils.java | 114 ++++++++++++++++ ...estAccessControlManagerForPermissions.java | 2 +- .../authorization/TestOwnerManager.java | 6 +- .../org/apache/gravitino/meta/TestEntity.java | 98 +++++++++++++- .../gravitino/proto/TestEntityProtoSerDe.java | 113 ---------------- .../gravitino/storage/TestEntityStorage.java | 5 +- .../storage/memory/TestMemoryEntityStore.java | 5 +- .../storage/relational/TestJDBCBackend.java | 35 ++--- .../service/TestGroupMetaService.java | 126 +++++++---------- .../service/TestRoleMetaService.java | 35 ++--- .../service/TestUserMetaService.java | 127 +++++++----------- .../server/web/rest/TestGroupOperations.java | 2 +- .../web/rest/TestPermissionOperations.java | 27 ++-- .../server/web/rest/TestUserOperations.java | 2 +- 31 files changed, 555 insertions(+), 870 deletions(-) delete mode 100644 core/src/main/java/org/apache/gravitino/proto/GroupEntitySerDe.java delete mode 100644 core/src/main/java/org/apache/gravitino/proto/RoleEntitySerDe.java delete mode 100644 core/src/main/java/org/apache/gravitino/proto/UserEntitySerDe.java create mode 100644 core/src/main/java/org/apache/gravitino/storage/relational/utils/SupplierUtils.java diff --git a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java index 75692e987b8..03e18bef914 100644 --- a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java +++ b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java @@ -194,8 +194,7 @@ public Boolean onOwnerSet(MetadataObject metadataObject, Owner preOwner, Owner n UserEntity.builder() .withId(1L) .withName(newOwner.name()) - .withRoleNames(Collections.emptyList()) - .withRoleIds(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withAuditInfo(auditInfo) .build(); onUserAdded(userEntity); @@ -204,8 +203,7 @@ public Boolean onOwnerSet(MetadataObject metadataObject, Owner preOwner, Owner n GroupEntity.builder() .withId(1L) .withName(newOwner.name()) - .withRoleNames(Collections.emptyList()) - .withRoleIds(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withAuditInfo(auditInfo) .build(); onGroupAdded(groupEntity); diff --git a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java index 79f66ef287a..844b89b3008 100644 --- a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java +++ b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java @@ -483,8 +483,7 @@ public void testOnGrantedRolesToUser() { UserEntity.builder() .withId(1L) .withName(userName1) - .withRoleNames(Collections.emptyList()) - .withRoleIds(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withAuditInfo(auditInfo) .build(); Assertions.assertTrue( @@ -502,8 +501,7 @@ public void testOnGrantedRolesToUser() { UserEntity.builder() .withId(1L) .withName(userName2) - .withRoleNames(Collections.emptyList()) - .withRoleIds(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withAuditInfo(auditInfo) .build(); Assertions.assertTrue( @@ -525,8 +523,7 @@ public void testOnRevokedRolesFromUser() { UserEntity.builder() .withId(1L) .withName(userName1) - .withRoleNames(Collections.emptyList()) - .withRoleIds(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withAuditInfo(auditInfo) .build(); Assertions.assertTrue( @@ -555,8 +552,7 @@ public void testOnGrantedRolesToGroup() { GroupEntity.builder() .withId(1L) .withName(groupName1) - .withRoleNames(Collections.emptyList()) - .withRoleIds(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withAuditInfo(auditInfo) .build(); Assertions.assertTrue( @@ -574,8 +570,7 @@ public void testOnGrantedRolesToGroup() { GroupEntity.builder() .withId(1L) .withName(groupName2) - .withRoleNames(Collections.emptyList()) - .withRoleIds(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withAuditInfo(auditInfo) .build(); Assertions.assertTrue( @@ -598,8 +593,7 @@ public void testOnRevokedRolesFromGroup() { GroupEntity.builder() .withId(1L) .withName(groupName1) - .withRoleNames(Collections.emptyList()) - .withRoleIds(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withAuditInfo(auditInfo) .build(); Assertions.assertTrue( @@ -680,8 +674,7 @@ public void testCreateUser() { .withId(0L) .withName(currentFunName()) .withAuditInfo(auditInfo) - .withRoleIds(null) - .withRoleNames(null) + .withRoles(Collections.emptyList()) .build(); Assertions.assertTrue(rangerAuthPlugin.onUserAdded(user)); Assertions.assertTrue(rangerAuthPlugin.onUserAcquired(user)); @@ -696,8 +689,7 @@ public void testCreateGroup() { .withId(0L) .withName(currentFunName()) .withAuditInfo(auditInfo) - .withRoleIds(null) - .withRoleNames(null) + .withRoles(Collections.emptyList()) .build(); Assertions.assertTrue(rangerAuthPlugin.onGroupAdded(group)); @@ -769,8 +761,7 @@ public void testCombinationOperation() { UserEntity.builder() .withId(1L) .withName(userName1) - .withRoleNames(Collections.emptyList()) - .withRoleIds(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withAuditInfo(auditInfo) .build(); Assertions.assertTrue( @@ -786,8 +777,7 @@ public void testCombinationOperation() { UserEntity.builder() .withId(1L) .withName(userName2) - .withRoleNames(Collections.emptyList()) - .withRoleIds(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withAuditInfo(auditInfo) .build(); Assertions.assertTrue( @@ -800,8 +790,7 @@ public void testCombinationOperation() { UserEntity.builder() .withId(1L) .withName(userName3) - .withRoleNames(Collections.emptyList()) - .withRoleIds(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withAuditInfo(auditInfo) .build(); Assertions.assertTrue( @@ -833,8 +822,7 @@ public void testCombinationOperation() { GroupEntity.builder() .withId(1L) .withName(groupName1) - .withRoleNames(Collections.emptyList()) - .withRoleIds(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withAuditInfo(auditInfo) .build(); Assertions.assertTrue( @@ -852,8 +840,7 @@ public void testCombinationOperation() { GroupEntity.builder() .withId(1L) .withName(groupName2) - .withRoleNames(Collections.emptyList()) - .withRoleIds(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withAuditInfo(auditInfo) .build(); Assertions.assertTrue( @@ -871,8 +858,7 @@ public void testCombinationOperation() { GroupEntity.builder() .withId(1L) .withName(groupName3) - .withRoleNames(Collections.emptyList()) - .withRoleIds(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withAuditInfo(auditInfo) .build(); Assertions.assertTrue( diff --git a/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java b/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java index edb02cdcec5..479bb069e3f 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java @@ -22,9 +22,11 @@ import static org.apache.gravitino.authorization.AuthorizationUtils.USER_DOES_NOT_EXIST_MSG; import com.google.common.collect.Lists; +import com.google.common.collect.Maps; import java.io.IOException; import java.time.Instant; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; @@ -70,14 +72,10 @@ User grantRolesToUser(String metalake, List roles, String user) { UserEntity.class, Entity.EntityType.USER, userEntity -> { - List roleEntities = Lists.newArrayList(); - if (userEntity.roleNames() != null) { - for (String role : userEntity.roleNames()) { - roleEntities.add(roleManager.getRole(metalake, role)); - } - } - List roleNames = Lists.newArrayList(toRoleNames(roleEntities)); - List roleIds = Lists.newArrayList(toRoleIds(roleEntities)); + List roleEntities = Lists.newArrayList(userEntity.roleEntities()); + + List roleIds = + roleEntities.stream().map(RoleEntity::id).collect(Collectors.toList()); for (RoleEntity roleEntityToGrant : roleEntitiesToGrant) { if (roleIds.contains(roleEntityToGrant.id())) { @@ -87,8 +85,8 @@ User grantRolesToUser(String metalake, List roles, String user) { user, metalake); } else { - roleNames.add(roleEntityToGrant.name()); roleIds.add(roleEntityToGrant.id()); + roleEntities.add(roleEntityToGrant); } } @@ -104,8 +102,7 @@ User grantRolesToUser(String metalake, List roles, String user) { .withNamespace(userEntity.namespace()) .withId(userEntity.id()) .withName(userEntity.name()) - .withRoleNames(roleNames) - .withRoleIds(roleIds) + .withRoles(roleEntities) .withAuditInfo(auditInfo) .build(); }); @@ -149,13 +146,8 @@ Group grantRolesToGroup(String metalake, List roles, String group) { GroupEntity.class, Entity.EntityType.GROUP, groupEntity -> { - List roleEntities = Lists.newArrayList(); - if (groupEntity.roleNames() != null) { - for (String role : groupEntity.roleNames()) { - roleEntities.add(roleManager.getRole(metalake, role)); - } - } - List roleNames = Lists.newArrayList(toRoleNames(roleEntities)); + List roleEntities = Lists.newArrayList(groupEntity.roleEntities()); + List roleIds = Lists.newArrayList(toRoleIds(roleEntities)); for (RoleEntity roleEntityToGrant : roleEntitiesToGrant) { @@ -166,8 +158,8 @@ Group grantRolesToGroup(String metalake, List roles, String group) { group, metalake); } else { - roleNames.add(roleEntityToGrant.name()); roleIds.add(roleEntityToGrant.id()); + roleEntities.add(roleEntityToGrant); } } @@ -183,8 +175,7 @@ Group grantRolesToGroup(String metalake, List roles, String group) { .withId(groupEntity.id()) .withNamespace(groupEntity.namespace()) .withName(groupEntity.name()) - .withRoleNames(roleNames) - .withRoleIds(roleIds) + .withRoles(roleEntities) .withAuditInfo(auditInfo) .build(); }); @@ -228,19 +219,18 @@ Group revokeRolesFromGroup(String metalake, List roles, String group) { GroupEntity.class, Entity.EntityType.GROUP, groupEntity -> { - List roleEntities = Lists.newArrayList(); - if (groupEntity.roleNames() != null) { - for (String role : groupEntity.roleNames()) { - roleEntities.add(roleManager.getRole(metalake, role)); - } + List roleEntities = groupEntity.roleEntities(); + + // Avoid loading securable objects + Map roleEntitiesMap = Maps.newHashMap(); + for (RoleEntity entity : roleEntities) { + roleEntitiesMap.put(entity.id(), entity); } - List roleNames = Lists.newArrayList(toRoleNames(roleEntities)); - List roleIds = Lists.newArrayList(toRoleIds(roleEntities)); for (RoleEntity roleEntityToRevoke : roleEntitiesToRevoke) { - roleNames.remove(roleEntityToRevoke.name()); - boolean removed = roleIds.remove(roleEntityToRevoke.id()); - if (!removed) { + if (roleEntitiesMap.containsKey(roleEntityToRevoke.id())) { + roleEntitiesMap.remove(roleEntityToRevoke.id()); + } else { LOG.warn( "Failed to revoke, role {} does not exist in the group {} of metalake {}", roleEntityToRevoke.name(), @@ -261,8 +251,7 @@ Group revokeRolesFromGroup(String metalake, List roles, String group) { .withNamespace(groupEntity.namespace()) .withId(groupEntity.id()) .withName(groupEntity.name()) - .withRoleNames(roleNames) - .withRoleIds(roleIds) + .withRoles(Lists.newArrayList(roleEntitiesMap.values())) .withAuditInfo(auditInfo) .build(); }); @@ -308,20 +297,18 @@ User revokeRolesFromUser(String metalake, List roles, String user) { UserEntity.class, Entity.EntityType.USER, userEntity -> { - List roleEntities = Lists.newArrayList(); - if (userEntity.roleNames() != null) { - for (String role : userEntity.roleNames()) { - roleEntities.add(roleManager.getRole(metalake, role)); - } - } + List roleEntities = userEntity.roleEntities(); - List roleNames = Lists.newArrayList(toRoleNames(roleEntities)); - List roleIds = Lists.newArrayList(toRoleIds(roleEntities)); + // Avoid loading securable objects + Map roleEntitiesMap = Maps.newHashMap(); + for (RoleEntity entity : roleEntities) { + roleEntitiesMap.put(entity.id(), entity); + } for (RoleEntity roleEntityToRevoke : roleEntitiesToRevoke) { - roleNames.remove(roleEntityToRevoke.name()); - boolean removed = roleIds.remove(roleEntityToRevoke.id()); - if (!removed) { + if (roleEntitiesMap.containsKey(roleEntityToRevoke.id())) { + roleEntitiesMap.remove(roleEntityToRevoke.id()); + } else { LOG.warn( "Failed to revoke, role {} doesn't exist in the user {} of metalake {}", roleEntityToRevoke.name(), @@ -341,8 +328,7 @@ User revokeRolesFromUser(String metalake, List roles, String user) { .withId(userEntity.id()) .withNamespace(userEntity.namespace()) .withName(userEntity.name()) - .withRoleNames(roleNames) - .withRoleIds(roleIds) + .withRoles(Lists.newArrayList(roleEntitiesMap.values())) .withAuditInfo(auditInfo) .build(); }); @@ -373,10 +359,6 @@ User revokeRolesFromUser(String metalake, List roles, String user) { } } - private List toRoleNames(List roleEntities) { - return roleEntities.stream().map(RoleEntity::name).collect(Collectors.toList()); - } - private List toRoleIds(List roleEntities) { return roleEntities.stream().map(RoleEntity::id).collect(Collectors.toList()); } diff --git a/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java b/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java index 09427668969..eef947f22e3 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java @@ -63,7 +63,7 @@ User addUser(String metalake, String name) throws UserAlreadyExistsException { .withId(idGenerator.nextId()) .withName(name) .withNamespace(AuthorizationUtils.ofUserNamespace(metalake)) - .withRoleNames(Lists.newArrayList()) + .withRoles(Lists.newArrayList()) .withAuditInfo( AuditInfo.builder() .withCreator(PrincipalUtils.getCurrentPrincipal().getName()) @@ -117,7 +117,7 @@ Group addGroup(String metalake, String group) throws GroupAlreadyExistsException .withId(idGenerator.nextId()) .withName(group) .withNamespace(AuthorizationUtils.ofGroupNamespace(metalake)) - .withRoleNames(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withAuditInfo( AuditInfo.builder() .withCreator(PrincipalUtils.getCurrentPrincipal().getName()) diff --git a/core/src/main/java/org/apache/gravitino/meta/GroupEntity.java b/core/src/main/java/org/apache/gravitino/meta/GroupEntity.java index d9e4b633d54..047c2bcce99 100644 --- a/core/src/main/java/org/apache/gravitino/meta/GroupEntity.java +++ b/core/src/main/java/org/apache/gravitino/meta/GroupEntity.java @@ -23,6 +23,8 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.function.Supplier; +import java.util.stream.Collectors; import org.apache.gravitino.Auditable; import org.apache.gravitino.Entity; import org.apache.gravitino.Field; @@ -38,11 +40,8 @@ public class GroupEntity implements Group, Entity, Auditable, HasIdentifier { public static final Field NAME = Field.required("name", String.class, "The name of the group entity."); - public static final Field ROLE_NAMES = - Field.optional("role_names", List.class, "The role names of the group entity."); - - public static final Field ROLE_IDS = - Field.optional("role_ids", List.class, "The role names of the group entity."); + public static final Field ROLES_SUPPLIER = + Field.required("roles_supplier", Supplier.class, "The roles supplier of the group entity."); public static final Field AUDIT_INFO = Field.required("audit_info", AuditInfo.class, "The audit details of the group entity."); @@ -50,9 +49,9 @@ public class GroupEntity implements Group, Entity, Auditable, HasIdentifier { private Long id; private String name; private AuditInfo auditInfo; - private List roleNames; - private List roleIds; private Namespace namespace; + // The roles is a lazy field to avoid unnecessary cost. + private Supplier> rolesSupplier = Collections::emptyList; private GroupEntity() {} @@ -67,8 +66,7 @@ public Map fields() { fields.put(ID, id); fields.put(NAME, name); fields.put(AUDIT_INFO, auditInfo); - fields.put(ROLE_NAMES, roleNames); - fields.put(ROLE_IDS, roleIds); + fields.put(ROLES_SUPPLIER, rolesSupplier); return Collections.unmodifiableMap(fields); } @@ -130,25 +128,20 @@ public AuditInfo auditInfo() { */ @Override public List roles() { - return roleNames; - } + if (roleEntities() == null) { + return null; + } - /** - * Returns the role names of the group entity. - * - * @return The role names of the group entity. - */ - public List roleNames() { - return roleNames; + return roleEntities().stream().map(RoleEntity::name).collect(Collectors.toList()); } /** - * Returns the role ids of the group entity. + * Returns the role entities of the group entity. * - * @return The role ids of the group entity. + * @return The role entities of the group entity. */ - public List roleIds() { - return roleIds; + public List roleEntities() { + return rolesSupplier.get(); } @Override @@ -161,13 +154,12 @@ public boolean equals(Object o) { && Objects.equals(name, that.name) && Objects.equals(namespace, that.namespace) && Objects.equals(auditInfo, that.auditInfo) - && Objects.equals(roleNames, that.roleNames) - && Objects.equals(roleIds, that.roleIds); + && Objects.equals(roles(), that.roles()); } @Override public int hashCode() { - return Objects.hash(id, name, auditInfo, roleNames, roleIds); + return Objects.hash(id, name, auditInfo, roles()); } public static Builder builder() { @@ -215,24 +207,26 @@ public Builder withAuditInfo(AuditInfo auditInfo) { } /** - * Sets the role names of the group entity. + * Sets the roles of the group entity. * * @param roles The role names of the group entity. * @return The builder instance. */ - public Builder withRoleNames(List roles) { - groupEntity.roleNames = roles; + public Builder withRoles(List roles) { + if (roles != null) { + groupEntity.rolesSupplier = () -> roles; + } return this; } /** - * Sets the role ids of the group entity. + * Sets the roles supplier of the group entity. * - * @param roleIds The role ids of the group entity. + * @param supplier The roles supplier of the group entity. * @return The builder instance. */ - public Builder withRoleIds(List roleIds) { - groupEntity.roleIds = roleIds; + public Builder withRolesSupplier(Supplier> supplier) { + groupEntity.rolesSupplier = supplier; return this; } diff --git a/core/src/main/java/org/apache/gravitino/meta/RoleEntity.java b/core/src/main/java/org/apache/gravitino/meta/RoleEntity.java index d1b3bbfe99f..eda0bc89903 100644 --- a/core/src/main/java/org/apache/gravitino/meta/RoleEntity.java +++ b/core/src/main/java/org/apache/gravitino/meta/RoleEntity.java @@ -24,6 +24,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.function.Supplier; import org.apache.gravitino.Auditable; import org.apache.gravitino.Entity; import org.apache.gravitino.Field; @@ -46,15 +47,18 @@ public class RoleEntity implements Role, Entity, Auditable, HasIdentifier { public static final Field AUDIT_INFO = Field.required("audit_info", AuditInfo.class, "The audit details of the role entity."); - public static final Field SECURABLE_OBJECT = - Field.required("securable_objects", List.class, "The securable objects of the role entity."); + public static final Field SECURABLE_OBJECTS_SUPPLIER = + Field.required( + "securable_objects_supplier", + Supplier.class, + "The securable objects supplier of the role entity."); private Long id; private String name; private Map properties; private AuditInfo auditInfo; private Namespace namespace; - private List securableObjects; + private Supplier> securableObjectsSupplier = () -> null; /** * The name of the role. @@ -91,16 +95,7 @@ public Map properties() { */ @Override public List securableObjects() { - // The securable object is a special kind of entities. Some entity types aren't the securable - // object, such as - // User, Role, etc. - // The securable object identifier must be unique. - // Gravitino assumes that the identifiers of the entities may be the same if they have different - // types. - // So one type of them can't be the securable object at least if there are the two same - // identifier - // entities . - return securableObjects; + return securableObjectsSupplier.get(); } /** @@ -115,7 +110,7 @@ public Map fields() { fields.put(NAME, name); fields.put(AUDIT_INFO, auditInfo); fields.put(PROPERTIES, properties); - fields.put(SECURABLE_OBJECT, securableObjects); + fields.put(SECURABLE_OBJECTS_SUPPLIER, securableObjectsSupplier); return Collections.unmodifiableMap(fields); } @@ -151,12 +146,12 @@ public boolean equals(Object o) { && Objects.equals(namespace, that.namespace) && Objects.equals(auditInfo, that.auditInfo) && Objects.equals(properties, that.properties) - && Objects.equals(securableObjects, that.securableObjects); + && Objects.equals(securableObjects(), that.securableObjects()); } @Override public int hashCode() { - return Objects.hash(id, name, properties, auditInfo, securableObjects, namespace); + return Objects.hash(id, name, properties, auditInfo, securableObjects(), namespace); } /** @@ -231,7 +226,19 @@ public Builder withAuditInfo(AuditInfo auditInfo) { * @return The builder instance. */ public Builder withSecurableObjects(List securableObjects) { - roleEntity.securableObjects = ImmutableList.copyOf(securableObjects); + roleEntity.securableObjectsSupplier = () -> ImmutableList.copyOf(securableObjects); + return this; + } + + /** + * Sets the securable objects supplier of the role entity. + * + * @param securableObjectsSupplier The securable objects supplier of the role entity. + * @return The builder instance. + */ + public Builder withSecurableObjectsSupplier( + Supplier> securableObjectsSupplier) { + roleEntity.securableObjectsSupplier = securableObjectsSupplier; return this; } diff --git a/core/src/main/java/org/apache/gravitino/meta/UserEntity.java b/core/src/main/java/org/apache/gravitino/meta/UserEntity.java index c71d731a99e..7e12bd2d64a 100644 --- a/core/src/main/java/org/apache/gravitino/meta/UserEntity.java +++ b/core/src/main/java/org/apache/gravitino/meta/UserEntity.java @@ -23,6 +23,8 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.function.Supplier; +import java.util.stream.Collectors; import lombok.ToString; import org.apache.gravitino.Auditable; import org.apache.gravitino.Entity; @@ -44,18 +46,15 @@ public class UserEntity implements User, Entity, Auditable, HasIdentifier { public static final Field AUDIT_INFO = Field.required("audit_info", AuditInfo.class, "The audit details of the user entity."); - public static final Field ROLE_NAMES = - Field.optional("role_names", List.class, "The role names of the user entity"); - - public static final Field ROLE_IDS = - Field.optional("role_ids", List.class, "The role ids of the user entity"); + public static final Field ROLES_SUPPLIER = + Field.required("roles_supplier", Supplier.class, "The roles supplier of the user entity"); private Long id; private String name; private AuditInfo auditInfo; - private List roleNames; - private List roleIds; private Namespace namespace; + // The roles is a lazy field to avoid unnecessary cost. + private Supplier> rolesSupplier = Collections::emptyList; private UserEntity() {} @@ -70,8 +69,7 @@ public Map fields() { fields.put(ID, id); fields.put(NAME, name); fields.put(AUDIT_INFO, auditInfo); - fields.put(ROLE_NAMES, roleNames); - fields.put(ROLE_IDS, roleIds); + fields.put(ROLES_SUPPLIER, rolesSupplier); return Collections.unmodifiableMap(fields); } @@ -133,25 +131,16 @@ public AuditInfo auditInfo() { */ @Override public List roles() { - return roleNames; - } - - /** - * Returns the role names of the user entity. - * - * @return The role names of the user entity. - */ - public List roleNames() { - return roleNames; + return roleEntities().stream().map(RoleEntity::name).collect(Collectors.toList()); } /** - * Returns the role ids of the user entity. + * Returns the role entities of the user entity. * - * @return The role ids of the user entity. + * @return The role entities of the user entity. */ - public List roleIds() { - return roleIds; + public List roleEntities() { + return rolesSupplier.get(); } @Override @@ -164,13 +153,12 @@ public boolean equals(Object o) { && Objects.equals(name, that.name) && Objects.equals(namespace, that.namespace) && Objects.equals(auditInfo, that.auditInfo) - && Objects.equals(roleNames, that.roleNames) - && Objects.equals(roleIds, that.roleIds); + && Objects.equals(roles(), that.roles()); } @Override public int hashCode() { - return Objects.hash(id, name, auditInfo, roleNames, roleIds); + return Objects.hash(id, name, auditInfo, roles()); } public static Builder builder() { @@ -218,24 +206,26 @@ public Builder withAuditInfo(AuditInfo auditInfo) { } /** - * Sets the role names of the user entity. + * Sets the roles of the user entity. * - * @param roles The role names of the user entity. + * @param roles The role entities of the user entity. * @return The builder instance. */ - public Builder withRoleNames(List roles) { - userEntity.roleNames = roles; + public Builder withRoles(List roles) { + if (roles != null) { + userEntity.rolesSupplier = () -> roles; + } return this; } /** - * Sets the role ids of the user entity. + * Sets the roles supplier of the user entity. * - * @param roleIds The role ids of the user entity. + * @param supplier The roles supplier of the user entity. * @return The builder instance. */ - public Builder withRoleIds(List roleIds) { - userEntity.roleIds = roleIds; + public Builder withRolesSupplier(Supplier> supplier) { + userEntity.rolesSupplier = supplier; return this; } diff --git a/core/src/main/java/org/apache/gravitino/proto/GroupEntitySerDe.java b/core/src/main/java/org/apache/gravitino/proto/GroupEntitySerDe.java deleted file mode 100644 index 46eb9c90841..00000000000 --- a/core/src/main/java/org/apache/gravitino/proto/GroupEntitySerDe.java +++ /dev/null @@ -1,69 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.gravitino.proto; - -import java.util.Collection; -import org.apache.gravitino.Namespace; -import org.apache.gravitino.meta.GroupEntity; - -public class GroupEntitySerDe implements ProtoSerDe { - - @Override - public Group serialize(GroupEntity groupEntity) { - Group.Builder builder = - Group.newBuilder() - .setId(groupEntity.id()) - .setName(groupEntity.name()) - .setAuditInfo(new AuditInfoSerDe().serialize(groupEntity.auditInfo())); - - if (isCollectionNotEmpty(groupEntity.roles())) { - builder.addAllRoleNames(groupEntity.roles()); - } - - if (isCollectionNotEmpty(groupEntity.roleIds())) { - builder.addAllRoleIds(groupEntity.roleIds()); - } - - return builder.build(); - } - - @Override - public GroupEntity deserialize(Group group, Namespace namespace) { - GroupEntity.Builder builder = - GroupEntity.builder() - .withId(group.getId()) - .withName(group.getName()) - .withNamespace(namespace) - .withAuditInfo(new AuditInfoSerDe().deserialize(group.getAuditInfo(), namespace)); - - if (group.getRoleNamesCount() > 0) { - builder.withRoleNames(group.getRoleNamesList()); - } - - if (group.getRoleIdsCount() > 0) { - builder.withRoleIds(group.getRoleIdsList()); - } - - return builder.build(); - } - - private boolean isCollectionNotEmpty(Collection collection) { - return collection != null && !collection.isEmpty(); - } -} diff --git a/core/src/main/java/org/apache/gravitino/proto/ProtoEntitySerDe.java b/core/src/main/java/org/apache/gravitino/proto/ProtoEntitySerDe.java index 3dc5cb1c297..a4b01a896d8 100644 --- a/core/src/main/java/org/apache/gravitino/proto/ProtoEntitySerDe.java +++ b/core/src/main/java/org/apache/gravitino/proto/ProtoEntitySerDe.java @@ -53,11 +53,6 @@ public class ProtoEntitySerDe implements EntitySerDe { .put( "org.apache.gravitino.meta.TopicEntity", "org.apache.gravitino.proto.TopicEntitySerDe") - .put("org.apache.gravitino.meta.UserEntity", "org.apache.gravitino.proto.UserEntitySerDe") - .put( - "org.apache.gravitino.meta.GroupEntity", - "org.apache.gravitino.proto.GroupEntitySerDe") - .put("org.apache.gravitino.meta.RoleEntity", "org.apache.gravitino.proto.RoleEntitySerDe") .build(); private static final Map ENTITY_TO_PROTO = @@ -75,13 +70,7 @@ public class ProtoEntitySerDe implements EntitySerDe { "org.apache.gravitino.meta.FilesetEntity", "org.apache.gravitino.proto.Fileset", "org.apache.gravitino.meta.TopicEntity", - "org.apache.gravitino.proto.Topic", - "org.apache.gravitino.meta.UserEntity", - "org.apache.gravitino.proto.User", - "org.apache.gravitino.meta.GroupEntity", - "org.apache.gravitino.proto.Group", - "org.apache.gravitino.meta.RoleEntity", - "org.apache.gravitino.proto.Role"); + "org.apache.gravitino.proto.Topic"); private final Map, ProtoSerDe> entityToSerDe; diff --git a/core/src/main/java/org/apache/gravitino/proto/RoleEntitySerDe.java b/core/src/main/java/org/apache/gravitino/proto/RoleEntitySerDe.java deleted file mode 100644 index a54f4505d7f..00000000000 --- a/core/src/main/java/org/apache/gravitino/proto/RoleEntitySerDe.java +++ /dev/null @@ -1,115 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.gravitino.proto; - -import com.google.common.collect.Lists; -import java.util.List; -import java.util.stream.Collectors; -import org.apache.gravitino.MetadataObject; -import org.apache.gravitino.Namespace; -import org.apache.gravitino.authorization.Privilege; -import org.apache.gravitino.authorization.Privileges; -import org.apache.gravitino.authorization.SecurableObject; -import org.apache.gravitino.authorization.SecurableObjects; -import org.apache.gravitino.meta.RoleEntity; - -public class RoleEntitySerDe implements ProtoSerDe { - - /** - * Serializes the provided entity into its corresponding Protocol Buffer message representation. - * - * @param roleEntity The entity to be serialized. - * @return The Protocol Buffer message representing the serialized entity. - */ - @Override - public Role serialize(RoleEntity roleEntity) { - Role.Builder builder = - Role.newBuilder() - .setId(roleEntity.id()) - .setName(roleEntity.name()) - .setAuditInfo(new AuditInfoSerDe().serialize(roleEntity.auditInfo())); - - for (SecurableObject securableObject : roleEntity.securableObjects()) { - builder.addSecurableObjects( - org.apache.gravitino.proto.SecurableObject.newBuilder() - .setFullName(securableObject.fullName()) - .setType(securableObject.type().name()) - .addAllPrivilegeConditions( - securableObject.privileges().stream() - .map(Privilege::condition) - .map(Privilege.Condition::name) - .collect(Collectors.toList())) - .addAllPrivilegeNames( - securableObject.privileges().stream() - .map(Privilege::name) - .map(Privilege.Name::name) - .collect(Collectors.toList())) - .build()); - } - - if (roleEntity.properties() != null && !roleEntity.properties().isEmpty()) { - builder.putAllProperties(roleEntity.properties()); - } - - return builder.build(); - } - - /** - * Deserializes the provided Protocol Buffer message into its corresponding entity representation. - * - * @param role The Protocol Buffer message to be deserialized. - * @return The entity representing the deserialized Protocol Buffer message. - */ - @Override - public RoleEntity deserialize(Role role, Namespace namespace) { - List securableObjects = Lists.newArrayList(); - - for (int index = 0; index < role.getSecurableObjectsCount(); index++) { - List privileges = Lists.newArrayList(); - org.apache.gravitino.proto.SecurableObject object = role.getSecurableObjects(index); - for (int privIndex = 0; privIndex < object.getPrivilegeConditionsCount(); privIndex++) { - if (Privilege.Condition.ALLOW.name().equals(object.getPrivilegeConditions(privIndex))) { - privileges.add(Privileges.allow(object.getPrivilegeNames(privIndex))); - } else { - privileges.add(Privileges.deny(object.getPrivilegeNames(privIndex))); - } - } - - SecurableObject securableObject = - SecurableObjects.parse( - object.getFullName(), MetadataObject.Type.valueOf(object.getType()), privileges); - - securableObjects.add(securableObject); - } - - RoleEntity.Builder builder = - RoleEntity.builder() - .withId(role.getId()) - .withName(role.getName()) - .withNamespace(namespace) - .withSecurableObjects(securableObjects) - .withAuditInfo(new AuditInfoSerDe().deserialize(role.getAuditInfo(), namespace)); - - if (!role.getPropertiesMap().isEmpty()) { - builder.withProperties(role.getPropertiesMap()); - } - - return builder.build(); - } -} diff --git a/core/src/main/java/org/apache/gravitino/proto/UserEntitySerDe.java b/core/src/main/java/org/apache/gravitino/proto/UserEntitySerDe.java deleted file mode 100644 index e859ee3b881..00000000000 --- a/core/src/main/java/org/apache/gravitino/proto/UserEntitySerDe.java +++ /dev/null @@ -1,69 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.gravitino.proto; - -import java.util.Collection; -import org.apache.gravitino.Namespace; -import org.apache.gravitino.meta.UserEntity; - -public class UserEntitySerDe implements ProtoSerDe { - - @Override - public User serialize(UserEntity userEntity) { - User.Builder builder = - User.newBuilder() - .setId(userEntity.id()) - .setName(userEntity.name()) - .setAuditInfo(new AuditInfoSerDe().serialize(userEntity.auditInfo())); - - if (isCollectionNotEmpty(userEntity.roles())) { - builder.addAllRoleNames(userEntity.roles()); - } - - if (isCollectionNotEmpty(userEntity.roleIds())) { - builder.addAllRoleIds(userEntity.roleIds()); - } - - return builder.build(); - } - - @Override - public UserEntity deserialize(User user, Namespace namespace) { - UserEntity.Builder builder = - UserEntity.builder() - .withId(user.getId()) - .withName(user.getName()) - .withNamespace(namespace) - .withAuditInfo(new AuditInfoSerDe().deserialize(user.getAuditInfo(), namespace)); - - if (user.getRoleNamesCount() > 0) { - builder.withRoleNames(user.getRoleNamesList()); - } - - if (user.getRoleIdsCount() > 0) { - builder.withRoleIds(user.getRoleIdsList()); - } - - return builder.build(); - } - - private boolean isCollectionNotEmpty(Collection collection) { - return collection != null && !collection.isEmpty(); - } -} diff --git a/core/src/main/java/org/apache/gravitino/storage/kv/BinaryEntityKeyEncoder.java b/core/src/main/java/org/apache/gravitino/storage/kv/BinaryEntityKeyEncoder.java index fa6fb48c636..2dd0e00b0cf 100644 --- a/core/src/main/java/org/apache/gravitino/storage/kv/BinaryEntityKeyEncoder.java +++ b/core/src/main/java/org/apache/gravitino/storage/kv/BinaryEntityKeyEncoder.java @@ -20,13 +20,10 @@ import static org.apache.gravitino.Entity.EntityType.CATALOG; import static org.apache.gravitino.Entity.EntityType.FILESET; -import static org.apache.gravitino.Entity.EntityType.GROUP; import static org.apache.gravitino.Entity.EntityType.METALAKE; -import static org.apache.gravitino.Entity.EntityType.ROLE; import static org.apache.gravitino.Entity.EntityType.SCHEMA; import static org.apache.gravitino.Entity.EntityType.TABLE; import static org.apache.gravitino.Entity.EntityType.TOPIC; -import static org.apache.gravitino.Entity.EntityType.USER; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; @@ -93,12 +90,6 @@ public class BinaryEntityKeyEncoder implements EntityKeyEncoder { new String[] {TABLE.getShortName() + "/", "/", "/", "/"}, FILESET, new String[] {FILESET.getShortName() + "/", "/", "/", "/"}, - USER, - new String[] {USER.getShortName() + "/", "/", "/", "/"}, - GROUP, - new String[] {GROUP.getShortName() + "/", "/", "/", "/"}, - ROLE, - new String[] {ROLE.getShortName() + "/", "/", "/", "/"}, TOPIC, new String[] {TOPIC.getShortName() + "/", "/", "/", "/"}); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/GroupMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/GroupMetaService.java index 2ffc10dac59..2762d55a7fd 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/GroupMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/GroupMetaService.java @@ -22,10 +22,8 @@ import com.google.common.collect.Lists; import com.google.common.collect.Sets; import java.io.IOException; -import java.util.Collections; import java.util.List; import java.util.Objects; -import java.util.Optional; import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; @@ -41,10 +39,10 @@ import org.apache.gravitino.storage.relational.mapper.OwnerMetaMapper; import org.apache.gravitino.storage.relational.po.GroupPO; import org.apache.gravitino.storage.relational.po.GroupRoleRelPO; -import org.apache.gravitino.storage.relational.po.RolePO; import org.apache.gravitino.storage.relational.utils.ExceptionUtils; import org.apache.gravitino.storage.relational.utils.POConverters; import org.apache.gravitino.storage.relational.utils.SessionUtils; +import org.apache.gravitino.storage.relational.utils.SupplierUtils; /** The service class for group metadata. It provides the basic database operations for group. */ public class GroupMetaService { @@ -92,9 +90,9 @@ public GroupEntity getGroupByIdentifier(NameIdentifier identifier) { Long metalakeId = MetalakeMetaService.getInstance().getMetalakeIdByName(identifier.namespace().level(0)); GroupPO groupPO = getGroupPOByMetalakeIdAndName(metalakeId, identifier.name()); - List rolePOs = RoleMetaService.getInstance().listRolesByGroupId(groupPO.getGroupId()); - return POConverters.fromGroupPO(groupPO, rolePOs, identifier.namespace()); + return POConverters.fromGroupPO( + groupPO, SupplierUtils.createRolePOsSupplier(groupPO), identifier.namespace()); } public List listGroupsByRoleIdent(NameIdentifier roleIdent) { @@ -107,7 +105,7 @@ public List listGroupsByRoleIdent(NameIdentifier roleIdent) { po -> POConverters.fromGroupPO( po, - Collections.emptyList(), + SupplierUtils.createRolePOsSupplier(po), AuthorizationUtils.ofGroupNamespace(roleIdent.namespace().level(0)))) .collect(Collectors.toList()); } @@ -121,7 +119,8 @@ public void insertGroup(GroupEntity groupEntity, boolean overwritten) throws IOE GroupPO.Builder builder = GroupPO.builder().withMetalakeId(metalakeId); GroupPO GroupPO = POConverters.initializeGroupPOWithVersion(groupEntity, builder); - List roleIds = Optional.ofNullable(groupEntity.roleIds()).orElse(Lists.newArrayList()); + List roleIds = + groupEntity.roleEntities().stream().map(RoleEntity::id).collect(Collectors.toList()); List groupRoleRelPOS = POConverters.initializeGroupRoleRelsPOWithVersion(groupEntity, roleIds); @@ -186,10 +185,10 @@ public GroupEntity updateGroup( Long metalakeId = MetalakeMetaService.getInstance().getMetalakeIdByName(identifier.namespace().level(0)); GroupPO oldGroupPO = getGroupPOByMetalakeIdAndName(metalakeId, identifier.name()); - List rolePOs = - RoleMetaService.getInstance().listRolesByGroupId(oldGroupPO.getGroupId()); + GroupEntity oldGroupEntity = - POConverters.fromGroupPO(oldGroupPO, rolePOs, identifier.namespace()); + POConverters.fromGroupPO( + oldGroupPO, SupplierUtils.createRolePOsSupplier(oldGroupPO), identifier.namespace()); GroupEntity newEntity = (GroupEntity) updater.apply((E) oldGroupEntity); Preconditions.checkArgument( @@ -199,11 +198,10 @@ public GroupEntity updateGroup( oldGroupEntity.id()); Set oldRoleIds = - oldGroupEntity.roleIds() == null - ? Sets.newHashSet() - : Sets.newHashSet(oldGroupEntity.roleIds()); + oldGroupEntity.roleEntities().stream().map(RoleEntity::id).collect(Collectors.toSet()); + Set newRoleIds = - newEntity.roleIds() == null ? Sets.newHashSet() : Sets.newHashSet(newEntity.roleIds()); + newEntity.roleEntities().stream().map(RoleEntity::id).collect(Collectors.toSet()); Set insertRoleIds = Sets.difference(newRoleIds, oldRoleIds); Set deleteRoleIds = Sets.difference(oldRoleIds, newRoleIds); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/OwnerMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/OwnerMetaService.java index 1118467b0a7..502b427f045 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/OwnerMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/OwnerMetaService.java @@ -18,7 +18,6 @@ */ package org.apache.gravitino.storage.relational.service; -import java.util.Collections; import java.util.Optional; import org.apache.gravitino.Entity; import org.apache.gravitino.MetadataObject; @@ -30,6 +29,7 @@ import org.apache.gravitino.storage.relational.po.UserPO; import org.apache.gravitino.storage.relational.utils.POConverters; import org.apache.gravitino.storage.relational.utils.SessionUtils; +import org.apache.gravitino.storage.relational.utils.SupplierUtils; import org.apache.gravitino.utils.NameIdentifierUtil; /** This class is an utilization class to retrieve owner relation. */ @@ -58,7 +58,7 @@ public Optional getOwner(NameIdentifier identifier, Entity.EntityType ty return Optional.of( POConverters.fromUserPO( userPO, - Collections.emptyList(), + SupplierUtils.createRolePOsSupplier(userPO), AuthorizationUtils.ofUserNamespace(NameIdentifierUtil.getMetalake(identifier)))); } @@ -71,7 +71,7 @@ public Optional getOwner(NameIdentifier identifier, Entity.EntityType ty return Optional.of( POConverters.fromGroupPO( groupPO, - Collections.emptyList(), + SupplierUtils.createRolePOsSupplier(groupPO), AuthorizationUtils.ofGroupNamespace(NameIdentifierUtil.getMetalake(identifier)))); } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java index 4560b74e0d0..743eb6338d2 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java @@ -39,14 +39,12 @@ import org.apache.gravitino.storage.relational.utils.ExceptionUtils; import org.apache.gravitino.storage.relational.utils.POConverters; import org.apache.gravitino.storage.relational.utils.SessionUtils; +import org.apache.gravitino.storage.relational.utils.SupplierUtils; import org.apache.gravitino.utils.NameIdentifierUtil; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** The service class for role metadata. It provides the basic database operations for role. */ public class RoleMetaService { - private static final Logger LOG = LoggerFactory.getLogger(RoleMetaService.class); private static final RoleMetaService INSTANCE = new RoleMetaService(); public static RoleMetaService getInstance() { @@ -109,33 +107,12 @@ public List listRolesByMetadataObjectIdentAndType( .map( po -> POConverters.fromRolePO( - po, listSecurableObjects(po), AuthorizationUtils.ofRoleNamespace(metalake))) + po, + SupplierUtils.createSecurableObjectsSupplier(po), + AuthorizationUtils.ofRoleNamespace(metalake))) .collect(Collectors.toList()); } - private List listSecurableObjects(RolePO po) { - List securableObjectPOs = listSecurableObjectsByRoleId(po.getRoleId()); - List securableObjects = Lists.newArrayList(); - - for (SecurableObjectPO securableObjectPO : securableObjectPOs) { - String fullName = - MetadataObjectService.getMetadataObjectFullName( - securableObjectPO.getType(), securableObjectPO.getMetadataObjectId()); - if (fullName != null) { - securableObjects.add( - POConverters.fromSecurableObjectPO( - fullName, securableObjectPO, getType(securableObjectPO.getType()))); - } else { - LOG.info( - "The securable object {} {} may be deleted", - securableObjectPO.getMetadataObjectId(), - securableObjectPO.getType()); - } - } - - return securableObjects; - } - public List listRolesByGroupId(Long groupId) { return SessionUtils.getWithoutCommit( RoleMetaMapper.class, mapper -> mapper.listRolesByGroupId(groupId)); @@ -197,9 +174,8 @@ public RoleEntity getRoleByIdentifier(NameIdentifier identifier) { MetalakeMetaService.getInstance().getMetalakeIdByName(identifier.namespace().level(0)); RolePO rolePO = getRolePOByMetalakeIdAndName(metalakeId, identifier.name()); - List securableObjects = listSecurableObjects(rolePO); - - return POConverters.fromRolePO(rolePO, securableObjects, identifier.namespace()); + return POConverters.fromRolePO( + rolePO, SupplierUtils.createSecurableObjectsSupplier(rolePO), identifier.namespace()); } public boolean deleteRole(NameIdentifier identifier) { @@ -232,7 +208,7 @@ public boolean deleteRole(NameIdentifier identifier) { return true; } - private List listSecurableObjectsByRoleId(Long roleId) { + public List listSecurableObjectsByRoleId(Long roleId) { return SessionUtils.getWithoutCommit( SecurableObjectMapper.class, mapper -> mapper.listSecurableObjectsByRoleId(roleId)); } @@ -273,10 +249,6 @@ public int deleteRoleMetasByLegacyTimeline(long legacyTimeline, int limit) { + securableObjectsCount[0]; } - private MetadataObject.Type getType(String type) { - return MetadataObject.Type.valueOf(type); - } - private String getEntityType(SecurableObject securableObject) { return securableObject.type().name(); } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/UserMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/UserMetaService.java index e7d0a435a1b..67bbfbce3df 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/UserMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/UserMetaService.java @@ -22,10 +22,8 @@ import com.google.common.collect.Lists; import com.google.common.collect.Sets; import java.io.IOException; -import java.util.Collections; import java.util.List; import java.util.Objects; -import java.util.Optional; import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; @@ -39,12 +37,12 @@ import org.apache.gravitino.storage.relational.mapper.OwnerMetaMapper; import org.apache.gravitino.storage.relational.mapper.UserMetaMapper; import org.apache.gravitino.storage.relational.mapper.UserRoleRelMapper; -import org.apache.gravitino.storage.relational.po.RolePO; import org.apache.gravitino.storage.relational.po.UserPO; import org.apache.gravitino.storage.relational.po.UserRoleRelPO; import org.apache.gravitino.storage.relational.utils.ExceptionUtils; import org.apache.gravitino.storage.relational.utils.POConverters; import org.apache.gravitino.storage.relational.utils.SessionUtils; +import org.apache.gravitino.storage.relational.utils.SupplierUtils; /** The service class for user metadata. It provides the basic database operations for user. */ public class UserMetaService { @@ -92,9 +90,9 @@ public UserEntity getUserByIdentifier(NameIdentifier identifier) { Long metalakeId = MetalakeMetaService.getInstance().getMetalakeIdByName(identifier.namespace().level(0)); UserPO userPO = getUserPOByMetalakeIdAndName(metalakeId, identifier.name()); - List rolePOs = RoleMetaService.getInstance().listRolesByUserId(userPO.getUserId()); - return POConverters.fromUserPO(userPO, rolePOs, identifier.namespace()); + return POConverters.fromUserPO( + userPO, SupplierUtils.createRolePOsSupplier(userPO), identifier.namespace()); } public List listUsersByRoleIdent(NameIdentifier roleIdent) { @@ -107,7 +105,7 @@ public List listUsersByRoleIdent(NameIdentifier roleIdent) { po -> POConverters.fromUserPO( po, - Collections.emptyList(), + SupplierUtils.createRolePOsSupplier(po), AuthorizationUtils.ofUserNamespace(roleIdent.namespace().level(0)))) .collect(Collectors.toList()); } @@ -121,7 +119,8 @@ public void insertUser(UserEntity userEntity, boolean overwritten) throws IOExce UserPO.Builder builder = UserPO.builder().withMetalakeId(metalakeId); UserPO userPO = POConverters.initializeUserPOWithVersion(userEntity, builder); - List roleIds = Optional.ofNullable(userEntity.roleIds()).orElse(Lists.newArrayList()); + List roleIds = + userEntity.roleEntities().stream().map(RoleEntity::id).collect(Collectors.toList()); List userRoleRelPOs = POConverters.initializeUserRoleRelsPOWithVersion(userEntity, roleIds); @@ -185,8 +184,9 @@ public UserEntity updateUser( Long metalakeId = MetalakeMetaService.getInstance().getMetalakeIdByName(identifier.namespace().level(0)); UserPO oldUserPO = getUserPOByMetalakeIdAndName(metalakeId, identifier.name()); - List rolePOs = RoleMetaService.getInstance().listRolesByUserId(oldUserPO.getUserId()); - UserEntity oldUserEntity = POConverters.fromUserPO(oldUserPO, rolePOs, identifier.namespace()); + UserEntity oldUserEntity = + POConverters.fromUserPO( + oldUserPO, SupplierUtils.createRolePOsSupplier(oldUserPO), identifier.namespace()); UserEntity newEntity = (UserEntity) updater.apply((E) oldUserEntity); Preconditions.checkArgument( @@ -196,11 +196,10 @@ public UserEntity updateUser( oldUserEntity.id()); Set oldRoleIds = - oldUserEntity.roleIds() == null - ? Sets.newHashSet() - : Sets.newHashSet(oldUserEntity.roleIds()); + oldUserEntity.roleEntities().stream().map(RoleEntity::id).collect(Collectors.toSet()); + Set newRoleIds = - newEntity.roleIds() == null ? Sets.newHashSet() : Sets.newHashSet(newEntity.roleIds()); + newEntity.roleEntities().stream().map(RoleEntity::id).collect(Collectors.toSet()); Set insertRoleIds = Sets.difference(newRoleIds, oldRoleIds); Set deleteRoleIds = Sets.difference(oldRoleIds, newRoleIds); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java b/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java index 82d739a41cc..c21ef1df08e 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java @@ -24,11 +24,13 @@ import java.time.Instant; import java.util.List; import java.util.Map; +import java.util.function.Supplier; import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.apache.gravitino.Catalog; import org.apache.gravitino.MetadataObject; import org.apache.gravitino.Namespace; +import org.apache.gravitino.authorization.AuthorizationUtils; import org.apache.gravitino.authorization.Privilege; import org.apache.gravitino.authorization.Privileges; import org.apache.gravitino.authorization.SecurableObject; @@ -699,29 +701,31 @@ public static UserPO updateUserPOWithVersion(UserPO oldUserPO, UserEntity newUse * Convert {@link UserPO} to {@link UserEntity} * * @param userPO UserPo object to be converted - * @param rolePOs list of rolePO + * @param rolePOsSupplier Supplier for the list of rolePO * @param namespace Namespace object to be associated with the user * @return UserEntity object from UserPO object */ - public static UserEntity fromUserPO(UserPO userPO, List rolePOs, Namespace namespace) { + public static UserEntity fromUserPO( + UserPO userPO, Supplier> rolePOsSupplier, Namespace namespace) { try { - List roleNames = - rolePOs.stream().map(RolePO::getRoleName).collect(Collectors.toList()); - List roleIds = rolePOs.stream().map(RolePO::getRoleId).collect(Collectors.toList()); - UserEntity.Builder builder = UserEntity.builder() .withId(userPO.getUserId()) .withName(userPO.getUserName()) .withNamespace(namespace) .withAuditInfo( - JsonUtils.anyFieldMapper().readValue(userPO.getAuditInfo(), AuditInfo.class)); - if (!roleNames.isEmpty()) { - builder.withRoleNames(roleNames); - } - if (!roleIds.isEmpty()) { - builder.withRoleIds(roleIds); - } + JsonUtils.anyFieldMapper().readValue(userPO.getAuditInfo(), AuditInfo.class)) + .withRolesSupplier( + () -> + rolePOsSupplier.get().stream() + .map( + po -> + fromRolePO( + po, + SupplierUtils.createSecurableObjectsSupplier(po), + AuthorizationUtils.ofRoleNamespace(namespace.level(0)))) + .collect(Collectors.toList())); + return builder.build(); } catch (JsonProcessingException e) { throw new RuntimeException("Failed to deserialize json object:", e); @@ -732,30 +736,31 @@ public static UserEntity fromUserPO(UserPO userPO, List rolePOs, Namespa * Convert {@link GroupPO} to {@link GroupEntity} * * @param groupPO GroupPO object to be converted - * @param rolePOs list of rolePO + * @param rolePOsSupplier Supplier for the list of rolePO * @param namespace Namespace object to be associated with the group * @return GroupEntity object from GroupPO object */ public static GroupEntity fromGroupPO( - GroupPO groupPO, List rolePOs, Namespace namespace) { + GroupPO groupPO, Supplier> rolePOsSupplier, Namespace namespace) { try { - List roleNames = - rolePOs.stream().map(RolePO::getRoleName).collect(Collectors.toList()); - List roleIds = rolePOs.stream().map(RolePO::getRoleId).collect(Collectors.toList()); - GroupEntity.Builder builder = GroupEntity.builder() .withId(groupPO.getGroupId()) .withName(groupPO.getGroupName()) .withNamespace(namespace) .withAuditInfo( - JsonUtils.anyFieldMapper().readValue(groupPO.getAuditInfo(), AuditInfo.class)); - if (!roleNames.isEmpty()) { - builder.withRoleNames(roleNames); - } - if (!roleIds.isEmpty()) { - builder.withRoleIds(roleIds); - } + JsonUtils.anyFieldMapper().readValue(groupPO.getAuditInfo(), AuditInfo.class)) + .withRolesSupplier( + () -> + rolePOsSupplier.get().stream() + .map( + po -> + fromRolePO( + po, + SupplierUtils.createSecurableObjectsSupplier(po), + AuthorizationUtils.ofRoleNamespace(namespace.level(0)))) + .collect(Collectors.toList())); + return builder.build(); } catch (JsonProcessingException e) { throw new RuntimeException("Failed to deserialize json object:", e); @@ -918,14 +923,16 @@ public static SecurableObject fromSecurableObjectPO( } public static RoleEntity fromRolePO( - RolePO rolePO, List securableObjects, Namespace namespace) { + RolePO rolePO, + Supplier> securableObjectsSupplier, + Namespace namespace) { try { return RoleEntity.builder() .withId(rolePO.getRoleId()) .withName(rolePO.getRoleName()) .withNamespace(namespace) .withProperties(JsonUtils.anyFieldMapper().readValue(rolePO.getProperties(), Map.class)) - .withSecurableObjects(securableObjects) + .withSecurableObjectsSupplier(securableObjectsSupplier) .withAuditInfo( JsonUtils.anyFieldMapper().readValue(rolePO.getAuditInfo(), AuditInfo.class)) .build(); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/utils/SupplierUtils.java b/core/src/main/java/org/apache/gravitino/storage/relational/utils/SupplierUtils.java new file mode 100644 index 00000000000..16c662c476d --- /dev/null +++ b/core/src/main/java/org/apache/gravitino/storage/relational/utils/SupplierUtils.java @@ -0,0 +1,114 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.storage.relational.utils; + +import com.google.common.collect.Lists; +import java.util.List; +import java.util.function.Supplier; +import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.authorization.SecurableObject; +import org.apache.gravitino.storage.relational.po.GroupPO; +import org.apache.gravitino.storage.relational.po.RolePO; +import org.apache.gravitino.storage.relational.po.SecurableObjectPO; +import org.apache.gravitino.storage.relational.po.UserPO; +import org.apache.gravitino.storage.relational.service.MetadataObjectService; +import org.apache.gravitino.storage.relational.service.RoleMetaService; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/* This class is a utilization class for creating kinds of suppliers */ +public class SupplierUtils { + + private static final Logger LOG = LoggerFactory.getLogger(SupplierUtils.class); + + private SupplierUtils() {} + + public static Supplier> createRolePOsSupplier(UserPO userPO) { + return new Supplier>() { + private List rolePOs; + private boolean waitToLoad = true; + + @Override + public List get() { + if (waitToLoad) { + rolePOs = RoleMetaService.getInstance().listRolesByUserId(userPO.getUserId()); + waitToLoad = false; + } + + return rolePOs; + } + }; + } + + public static Supplier> createRolePOsSupplier(GroupPO groupPO) { + return new Supplier>() { + private List rolePOS; + private boolean waitToLoad = true; + + @Override + public List get() { + if (waitToLoad) { + rolePOS = RoleMetaService.getInstance().listRolesByGroupId(groupPO.getGroupId()); + waitToLoad = false; + } + + return rolePOS; + } + }; + } + + public static Supplier> createSecurableObjectsSupplier(RolePO rolePO) { + return new Supplier>() { + private List securableObjects; + + private boolean waitToLoad = true; + + @Override + public List get() { + if (waitToLoad) { + List securableObjectPOs = + RoleMetaService.getInstance().listSecurableObjectsByRoleId(rolePO.getRoleId()); + + securableObjects = Lists.newArrayList(); + + for (SecurableObjectPO securableObjectPO : securableObjectPOs) { + String fullName = + MetadataObjectService.getMetadataObjectFullName( + securableObjectPO.getType(), securableObjectPO.getMetadataObjectId()); + if (fullName != null) { + securableObjects.add( + POConverters.fromSecurableObjectPO( + fullName, + securableObjectPO, + MetadataObject.Type.valueOf(securableObjectPO.getType()))); + } else { + LOG.info( + "The securable object {} {} may be deleted", + securableObjectPO.getMetadataObjectId(), + securableObjectPO.getType()); + } + } + waitToLoad = false; + } + + return securableObjects; + } + }; + } +} diff --git a/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManagerForPermissions.java b/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManagerForPermissions.java index ce3c4e90c9d..946cebf972e 100644 --- a/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManagerForPermissions.java +++ b/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManagerForPermissions.java @@ -157,7 +157,7 @@ public void testGrantRoleToUser() { String notExist = "not-exist"; User user = accessControlManager.getUser(METALAKE, USER); - Assertions.assertNull(user.roles()); + Assertions.assertTrue(user.roles().isEmpty()); reset(authorizationPlugin); diff --git a/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java b/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java index 83a562f640d..0b0b93ef056 100644 --- a/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java +++ b/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java @@ -125,8 +125,7 @@ public static void setUp() throws IOException, IllegalAccessException { UserEntity.builder() .withId(idGenerator.nextId()) .withName(USER) - .withRoleNames(Collections.emptyList()) - .withRoleIds(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withNamespace(AuthorizationUtils.ofUserNamespace(METALAKE)) .withAuditInfo(audit) .build(); @@ -136,8 +135,7 @@ public static void setUp() throws IOException, IllegalAccessException { GroupEntity.builder() .withId(idGenerator.nextId()) .withName(GROUP) - .withRoleNames(Collections.emptyList()) - .withRoleIds(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withNamespace(AuthorizationUtils.ofUserNamespace(METALAKE)) .withAuditInfo(audit) .build(); diff --git a/core/src/test/java/org/apache/gravitino/meta/TestEntity.java b/core/src/test/java/org/apache/gravitino/meta/TestEntity.java index f4c32dca718..2e24a7eb013 100644 --- a/core/src/test/java/org/apache/gravitino/meta/TestEntity.java +++ b/core/src/test/java/org/apache/gravitino/meta/TestEntity.java @@ -21,10 +21,15 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import java.time.Instant; +import java.util.Collections; +import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Supplier; import org.apache.gravitino.Catalog; import org.apache.gravitino.Field; import org.apache.gravitino.authorization.Privileges; +import org.apache.gravitino.authorization.SecurableObject; import org.apache.gravitino.authorization.SecurableObjects; import org.apache.gravitino.file.Fileset; import org.junit.jupiter.api.Assertions; @@ -237,19 +242,48 @@ public void testUser() { .withId(userId) .withName(userName) .withAuditInfo(auditInfo) - .withRoleNames(Lists.newArrayList("role")) + .withRoles( + Lists.newArrayList( + RoleEntity.builder() + .withId(1L) + .withName("role") + .withAuditInfo(auditInfo) + .build())) .build(); Map fields = testUserEntity.fields(); Assertions.assertEquals(userId, fields.get(UserEntity.ID)); Assertions.assertEquals(userName, fields.get(UserEntity.NAME)); Assertions.assertEquals(auditInfo, fields.get(UserEntity.AUDIT_INFO)); - Assertions.assertEquals(Lists.newArrayList("role"), fields.get(UserEntity.ROLE_NAMES)); + Assertions.assertEquals( + Lists.newArrayList( + RoleEntity.builder().withId(1L).withName("role").withAuditInfo(auditInfo).build()), + ((Supplier>) fields.get(UserEntity.ROLES_SUPPLIER)).get()); UserEntity testUserEntityWithoutFields = UserEntity.builder().withId(userId).withName(userName).withAuditInfo(auditInfo).build(); - Assertions.assertNull(testUserEntityWithoutFields.roles()); + Assertions.assertTrue(testUserEntityWithoutFields.roles().isEmpty()); + + // Test lazily loading + AtomicBoolean hasCallRolesSupplier = new AtomicBoolean(false); + + UserEntity userWithSupplier = + UserEntity.builder() + .withId(userId) + .withName(userName) + .withAuditInfo(auditInfo) + .withRolesSupplier( + () -> { + hasCallRolesSupplier.set(true); + return Collections.emptyList(); + }) + .build(); + + Assertions.assertFalse(hasCallRolesSupplier.get()); + + userWithSupplier.roles(); + Assertions.assertTrue(hasCallRolesSupplier.get()); } @Test @@ -258,19 +292,48 @@ public void testGroup() { GroupEntity.builder() .withId(groupId) .withName(groupName) + .withRoles( + Lists.newArrayList( + RoleEntity.builder() + .withId(1L) + .withName("role") + .withAuditInfo(auditInfo) + .build())) .withAuditInfo(auditInfo) - .withRoleNames(Lists.newArrayList("role")) .build(); Map fields = group.fields(); Assertions.assertEquals(groupId, fields.get(GroupEntity.ID)); Assertions.assertEquals(groupName, fields.get(GroupEntity.NAME)); Assertions.assertEquals(auditInfo, fields.get(GroupEntity.AUDIT_INFO)); - Assertions.assertEquals(Lists.newArrayList("role"), fields.get(GroupEntity.ROLE_NAMES)); + Assertions.assertEquals( + Lists.newArrayList( + RoleEntity.builder().withId(1L).withName("role").withAuditInfo(auditInfo).build()), + ((Supplier>) fields.get(GroupEntity.ROLES_SUPPLIER)).get()); GroupEntity groupWithoutFields = GroupEntity.builder().withId(userId).withName(userName).withAuditInfo(auditInfo).build(); - Assertions.assertNull(groupWithoutFields.roles()); + Assertions.assertTrue(groupWithoutFields.roles().isEmpty()); + + AtomicBoolean hasCallRolesSupplier = new AtomicBoolean(false); + + // Test lazily loading + GroupEntity groupWithSupplier = + GroupEntity.builder() + .withId(userId) + .withName(userName) + .withAuditInfo(auditInfo) + .withRolesSupplier( + () -> { + hasCallRolesSupplier.set(true); + return Collections.emptyList(); + }) + .build(); + + Assertions.assertFalse(hasCallRolesSupplier.get()); + + groupWithSupplier.roles(); + Assertions.assertTrue(hasCallRolesSupplier.get()); } @Test @@ -296,7 +359,8 @@ public void testRole() { Lists.newArrayList( SecurableObjects.ofCatalog( catalogName, Lists.newArrayList(Privileges.UseCatalog.allow()))), - fields.get(RoleEntity.SECURABLE_OBJECT)); + ((Supplier>) fields.get(RoleEntity.SECURABLE_OBJECTS_SUPPLIER)) + .get()); RoleEntity roleWithoutFields = RoleEntity.builder() @@ -309,6 +373,26 @@ public void testRole() { catalogName, Lists.newArrayList(Privileges.UseCatalog.allow())))) .build(); Assertions.assertNull(roleWithoutFields.properties()); + + // Test lazily loading + AtomicBoolean hasCallSecurableObjectsSupplier = new AtomicBoolean(false); + + RoleEntity roleWithSupplier = + RoleEntity.builder() + .withId(userId) + .withName(userName) + .withAuditInfo(auditInfo) + .withSecurableObjectsSupplier( + () -> { + hasCallSecurableObjectsSupplier.set(true); + return null; + }) + .build(); + + Assertions.assertFalse(hasCallSecurableObjectsSupplier.get()); + + roleWithSupplier.securableObjects(); + Assertions.assertTrue(hasCallSecurableObjectsSupplier.get()); } @Test diff --git a/core/src/test/java/org/apache/gravitino/proto/TestEntityProtoSerDe.java b/core/src/test/java/org/apache/gravitino/proto/TestEntityProtoSerDe.java index 1d4bfaa17d9..226199a0d93 100644 --- a/core/src/test/java/org/apache/gravitino/proto/TestEntityProtoSerDe.java +++ b/core/src/test/java/org/apache/gravitino/proto/TestEntityProtoSerDe.java @@ -19,23 +19,15 @@ package org.apache.gravitino.proto; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Lists; import java.io.IOException; import java.time.Instant; import java.util.Map; import org.apache.gravitino.Catalog; -import org.apache.gravitino.Entity; import org.apache.gravitino.EntitySerDe; import org.apache.gravitino.EntitySerDeFactory; import org.apache.gravitino.Namespace; -import org.apache.gravitino.authorization.Privileges; -import org.apache.gravitino.authorization.SecurableObject; -import org.apache.gravitino.authorization.SecurableObjects; import org.apache.gravitino.file.Fileset; -import org.apache.gravitino.meta.GroupEntity; -import org.apache.gravitino.meta.RoleEntity; import org.apache.gravitino.meta.SchemaVersion; -import org.apache.gravitino.meta.UserEntity; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -323,110 +315,5 @@ public void testEntitiesSerDe() throws IOException { Assertions.assertEquals(topicEntity1, topicEntityFromBytes1); Assertions.assertNull(topicEntityFromBytes1.comment()); Assertions.assertNull(topicEntityFromBytes1.properties()); - - // Test UserEntity - Namespace userNamespace = - Namespace.of("metalake", Entity.SYSTEM_CATALOG_RESERVED_NAME, Entity.USER_SCHEMA_NAME); - Long userId = 1L; - String userName = "user"; - UserEntity userEntity = - UserEntity.builder() - .withId(userId) - .withName(userName) - .withNamespace(userNamespace) - .withAuditInfo(auditInfo) - .withRoleNames(Lists.newArrayList("role")) - .withRoleIds(Lists.newArrayList(1L)) - .build(); - byte[] userBytes = protoEntitySerDe.serialize(userEntity); - UserEntity userEntityFromBytes = - protoEntitySerDe.deserialize(userBytes, UserEntity.class, userNamespace); - Assertions.assertEquals(userEntity, userEntityFromBytes); - - UserEntity userEntityWithoutFields = - UserEntity.builder() - .withId(userId) - .withName(userName) - .withNamespace(userNamespace) - .withAuditInfo(auditInfo) - .build(); - userBytes = protoEntitySerDe.serialize(userEntityWithoutFields); - userEntityFromBytes = protoEntitySerDe.deserialize(userBytes, UserEntity.class, userNamespace); - Assertions.assertEquals(userEntityWithoutFields, userEntityFromBytes); - Assertions.assertNull(userEntityWithoutFields.roles()); - Assertions.assertNull(userEntityWithoutFields.roleIds()); - - // Test GroupEntity - Namespace groupNamespace = - Namespace.of("metalake", Entity.SYSTEM_CATALOG_RESERVED_NAME, Entity.GROUP_SCHEMA_NAME); - Long groupId = 1L; - String groupName = "group"; - - GroupEntity group = - GroupEntity.builder() - .withId(groupId) - .withName(groupName) - .withNamespace(groupNamespace) - .withAuditInfo(auditInfo) - .withRoleNames(Lists.newArrayList("role")) - .withRoleIds(Lists.newArrayList(1L)) - .build(); - byte[] groupBytes = protoEntitySerDe.serialize(group); - GroupEntity groupFromBytes = - protoEntitySerDe.deserialize(groupBytes, GroupEntity.class, groupNamespace); - Assertions.assertEquals(group, groupFromBytes); - - GroupEntity groupWithoutFields = - GroupEntity.builder() - .withId(groupId) - .withName(groupName) - .withNamespace(groupNamespace) - .withAuditInfo(auditInfo) - .build(); - groupBytes = protoEntitySerDe.serialize(groupWithoutFields); - groupFromBytes = protoEntitySerDe.deserialize(groupBytes, GroupEntity.class, groupNamespace); - Assertions.assertEquals(groupWithoutFields, groupFromBytes); - Assertions.assertNull(groupWithoutFields.roles()); - Assertions.assertNull(groupWithoutFields.roleIds()); - - // Test RoleEntity - Namespace roleNamespace = - Namespace.of("metalake", Entity.SYSTEM_CATALOG_RESERVED_NAME, Entity.ROLE_SCHEMA_NAME); - Long roleId = 1L; - String roleName = "testRole"; - String anotherCatalogName = "anotherCatalog"; - SecurableObject securableObject = - SecurableObjects.ofCatalog( - catalogName, - Lists.newArrayList(Privileges.UseCatalog.allow(), Privileges.CreateSchema.deny())); - SecurableObject anotherSecurableObject = - SecurableObjects.ofCatalog( - anotherCatalogName, Lists.newArrayList(Privileges.UseCatalog.allow())); - - RoleEntity roleEntity = - RoleEntity.builder() - .withId(roleId) - .withName(roleName) - .withNamespace(roleNamespace) - .withAuditInfo(auditInfo) - .withSecurableObjects(Lists.newArrayList(securableObject, anotherSecurableObject)) - .withProperties(props) - .build(); - byte[] roleBytes = protoEntitySerDe.serialize(roleEntity); - RoleEntity roleFromBytes = - protoEntitySerDe.deserialize(roleBytes, RoleEntity.class, roleNamespace); - Assertions.assertEquals(roleEntity, roleFromBytes); - - RoleEntity roleWithoutFields = - RoleEntity.builder() - .withId(1L) - .withName(roleName) - .withNamespace(roleNamespace) - .withAuditInfo(auditInfo) - .withSecurableObjects(Lists.newArrayList(securableObject, anotherSecurableObject)) - .build(); - roleBytes = protoEntitySerDe.serialize(roleWithoutFields); - roleFromBytes = protoEntitySerDe.deserialize(roleBytes, RoleEntity.class, roleNamespace); - Assertions.assertEquals(roleWithoutFields, roleFromBytes); } } diff --git a/core/src/test/java/org/apache/gravitino/storage/TestEntityStorage.java b/core/src/test/java/org/apache/gravitino/storage/TestEntityStorage.java index bdf95ff9639..d233ac5bafc 100644 --- a/core/src/test/java/org/apache/gravitino/storage/TestEntityStorage.java +++ b/core/src/test/java/org/apache/gravitino/storage/TestEntityStorage.java @@ -46,6 +46,7 @@ import java.sql.Statement; import java.time.Instant; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.UUID; import org.apache.commons.io.FileUtils; @@ -1234,7 +1235,7 @@ private static UserEntity createUser(Long id, String metalake, String name, Audi .withNamespace(AuthorizationUtils.ofUserNamespace(metalake)) .withName(name) .withAuditInfo(auditInfo) - .withRoleNames(null) + .withRoles(Collections.emptyList()) .build(); } @@ -1245,7 +1246,7 @@ private static GroupEntity createGroup( .withNamespace(AuthorizationUtils.ofGroupNamespace(metalake)) .withName(name) .withAuditInfo(auditInfo) - .withRoleNames(null) + .withRoles(Collections.emptyList()) .build(); } diff --git a/core/src/test/java/org/apache/gravitino/storage/memory/TestMemoryEntityStore.java b/core/src/test/java/org/apache/gravitino/storage/memory/TestMemoryEntityStore.java index 7cff7bcc553..4d41d2f2523 100644 --- a/core/src/test/java/org/apache/gravitino/storage/memory/TestMemoryEntityStore.java +++ b/core/src/test/java/org/apache/gravitino/storage/memory/TestMemoryEntityStore.java @@ -22,6 +22,7 @@ import com.google.common.collect.Maps; import java.io.IOException; import java.time.Instant; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.concurrent.locks.Lock; @@ -224,7 +225,7 @@ public void testEntityStoreAndRetrieve() throws Exception { .withName("user") .withNamespace(Namespace.of("metalake", "catalog", "db")) .withAuditInfo(auditInfo) - .withRoleNames(null) + .withRoles(Collections.emptyList()) .build(); GroupEntity groupEntity = @@ -233,7 +234,7 @@ public void testEntityStoreAndRetrieve() throws Exception { .withName("group") .withNamespace(AuthorizationUtils.ofGroupNamespace("metalake")) .withAuditInfo(auditInfo) - .withRoleNames(null) + .withRoles(Collections.emptyList()) .build(); RoleEntity roleEntity = 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 index 26430d2fb5e..d72f542fa1f 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/TestJDBCBackend.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/TestJDBCBackend.java @@ -45,6 +45,7 @@ import java.sql.Statement; import java.time.Instant; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -600,8 +601,7 @@ public void testMetaLifeCycleFromCreationToDeletion() throws IOException { AuthorizationUtils.ofUserNamespace("metalake"), "user", auditInfo, - Lists.newArrayList(role.name()), - Lists.newArrayList(role.id())); + Lists.newArrayList(role)); backend.insert(user, false); GroupEntity group = @@ -610,8 +610,7 @@ public void testMetaLifeCycleFromCreationToDeletion() throws IOException { AuthorizationUtils.ofGroupNamespace("metalake"), "group", auditInfo, - Lists.newArrayList(role.name()), - Lists.newArrayList(role.id())); + Lists.newArrayList(role)); backend.insert(group, false); TagEntity tag = @@ -688,8 +687,7 @@ public void testMetaLifeCycleFromCreationToDeletion() throws IOException { AuthorizationUtils.ofUserNamespace("another-metalake"), "another-user", auditInfo, - Lists.newArrayList(anotherRole.name()), - Lists.newArrayList(anotherRole.id())); + Lists.newArrayList(anotherRole)); backend.insert(anotherUser, false); GroupEntity anotherGroup = @@ -698,8 +696,7 @@ public void testMetaLifeCycleFromCreationToDeletion() throws IOException { AuthorizationUtils.ofGroupNamespace("another-metalake"), "another-group", auditInfo, - Lists.newArrayList(anotherRole.name()), - Lists.newArrayList(anotherRole.id())); + Lists.newArrayList(anotherRole)); backend.insert(anotherGroup, false); TagEntity anotherTagEntity = @@ -1112,8 +1109,7 @@ public static UserEntity createUserEntity( .withId(id) .withName(name) .withNamespace(namespace) - .withRoleNames(null) - .withRoleIds(null) + .withRoles(Collections.emptyList()) .withAuditInfo(auditInfo) .build(); } @@ -1123,14 +1119,12 @@ public static UserEntity createUserEntity( Namespace namespace, String name, AuditInfo auditInfo, - List roleNames, - List roleIds) { + List roleEntities) { return UserEntity.builder() .withId(id) .withName(name) .withNamespace(namespace) - .withRoleNames(roleNames) - .withRoleIds(roleIds) + .withRoles(roleEntities) .withAuditInfo(auditInfo) .build(); } @@ -1156,25 +1150,18 @@ public static GroupEntity createGroupEntity( .withId(id) .withName(name) .withNamespace(namespace) - .withRoleNames(null) - .withRoleIds(null) + .withRoles(Collections.emptyList()) .withAuditInfo(auditInfo) .build(); } public static GroupEntity createGroupEntity( - Long id, - Namespace namespace, - String name, - AuditInfo auditInfo, - List roleNames, - List roleIds) { + Long id, Namespace namespace, String name, AuditInfo auditInfo, List roles) { return GroupEntity.builder() .withId(id) .withName(name) .withNamespace(namespace) - .withRoleNames(roleNames) - .withRoleIds(roleIds) + .withRoles(roles) .withAuditInfo(auditInfo) .build(); } 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 index 22246ba0cf3..d6b33095045 100644 --- 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 @@ -30,6 +30,7 @@ import java.util.List; import java.util.Optional; import java.util.function.Function; +import java.util.stream.Collectors; import org.apache.gravitino.Entity; import org.apache.gravitino.EntityAlreadyExistsException; import org.apache.gravitino.Namespace; @@ -110,13 +111,11 @@ void getGroupByIdentifier() throws IOException { AuthorizationUtils.ofGroupNamespace(metalakeName), "group2", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); groupMetaService.insertGroup(group2, false); GroupEntity actualGroup = groupMetaService.getGroupByIdentifier(group2.nameIdentifier()); Assertions.assertEquals(group2.name(), actualGroup.name()); - Assertions.assertEquals( - Sets.newHashSet(group2.roleNames()), Sets.newHashSet(actualGroup.roleNames())); + Assertions.assertEquals(Sets.newHashSet(group2.roles()), Sets.newHashSet(actualGroup.roles())); } @Test @@ -194,13 +193,11 @@ void insertGroup() throws IOException { AuthorizationUtils.ofGroupNamespace(metalakeName), "group2", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); Assertions.assertDoesNotThrow(() -> groupMetaService.insertGroup(group2, false)); GroupEntity actualGroup = groupMetaService.getGroupByIdentifier(group2.nameIdentifier()); Assertions.assertEquals(group2.name(), actualGroup.name()); - Assertions.assertEquals( - Sets.newHashSet(group2.roleNames()), Sets.newHashSet(actualGroup.roleNames())); + Assertions.assertEquals(Sets.newHashSet(group2.roles()), Sets.newHashSet(actualGroup.roles())); // insert duplicate group with roles GroupEntity group2Exist = @@ -219,17 +216,16 @@ void insertGroup() throws IOException { AuthorizationUtils.ofGroupNamespace(metalakeName), "group2Overwrite", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); Assertions.assertDoesNotThrow(() -> groupMetaService.insertGroup(group2Overwrite, true)); GroupEntity actualOverwriteGroup2 = groupMetaService.getGroupByIdentifier(group2Overwrite.nameIdentifier()); Assertions.assertEquals("group2Overwrite", actualOverwriteGroup2.name()); - Assertions.assertEquals(2, actualOverwriteGroup2.roleNames().size()); + Assertions.assertEquals(2, actualOverwriteGroup2.roles().size()); Assertions.assertEquals( Sets.newHashSet(role1.name(), role2.name()), - Sets.newHashSet(actualOverwriteGroup2.roleNames())); + Sets.newHashSet(actualOverwriteGroup2.roles())); // insert overwrite user with 1 role RoleEntity role3 = @@ -246,15 +242,14 @@ void insertGroup() throws IOException { AuthorizationUtils.ofUserNamespace(metalakeName), "group3Overwrite", auditInfo, - Lists.newArrayList(role3.name()), - Lists.newArrayList(role3.id())); + Lists.newArrayList(role3)); Assertions.assertDoesNotThrow(() -> groupMetaService.insertGroup(group3Overwrite, true)); GroupEntity actualOverwriteGroup3 = groupMetaService.getGroupByIdentifier(group3Overwrite.nameIdentifier()); Assertions.assertEquals("group3Overwrite", actualOverwriteGroup3.name()); - Assertions.assertEquals(1, actualOverwriteGroup3.roleNames().size()); - Assertions.assertEquals("role3", actualOverwriteGroup3.roleNames().get(0)); + Assertions.assertEquals(1, actualOverwriteGroup3.roles().size()); + Assertions.assertEquals("role3", actualOverwriteGroup3.roles().get(0)); // insert overwrite user with 0 roles GroupEntity group4Overwrite = @@ -268,7 +263,7 @@ void insertGroup() throws IOException { GroupEntity actualOverwriteGroup4 = groupMetaService.getGroupByIdentifier(group4Overwrite.nameIdentifier()); Assertions.assertEquals("group4Overwrite", actualOverwriteGroup4.name()); - Assertions.assertNull(actualOverwriteGroup4.roleNames()); + Assertions.assertTrue(actualOverwriteGroup4.roles().isEmpty()); } @Test @@ -341,8 +336,7 @@ void deleteGroup() throws IOException { AuthorizationUtils.ofGroupNamespace(metalakeName), "group2", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); groupMetaService.insertGroup(group2, false); List rolePOs = SessionUtils.doWithCommitAndFetchResult( @@ -350,8 +344,7 @@ void deleteGroup() throws IOException { Assertions.assertEquals(2, rolePOs.size()); GroupEntity actualGroup = groupMetaService.getGroupByIdentifier(group2.nameIdentifier()); Assertions.assertEquals(group2.name(), actualGroup.name()); - Assertions.assertEquals( - Sets.newHashSet(group2.roleNames()), Sets.newHashSet(actualGroup.roleNames())); + Assertions.assertEquals(Sets.newHashSet(group2.roles()), Sets.newHashSet(actualGroup.roles())); Assertions.assertTrue(groupMetaService.deleteGroup(group2.nameIdentifier())); Assertions.assertThrows( @@ -400,13 +393,11 @@ void updateGroup() throws IOException { AuthorizationUtils.ofGroupNamespace(metalakeName), "group1", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); groupMetaService.insertGroup(group1, false); GroupEntity actualGroup = groupMetaService.getGroupByIdentifier(group1.nameIdentifier()); Assertions.assertEquals(group1.name(), actualGroup.name()); - Assertions.assertEquals( - Sets.newHashSet(group1.roleNames()), Sets.newHashSet(actualGroup.roleNames())); + Assertions.assertEquals(Sets.newHashSet(group1.roles()), Sets.newHashSet(actualGroup.roles())); RoleEntity role3 = createRoleEntity( @@ -428,17 +419,14 @@ void updateGroup() throws IOException { .withLastModifiedTime(Instant.now()) .build(); - List roleNames = Lists.newArrayList(group.roleNames()); - List roleIds = Lists.newArrayList(group.roleIds()); - roleNames.add(role3.name()); - roleIds.add(role3.id()); + List roleEntities = Lists.newArrayList(group.roleEntities()); + roleEntities.add(role3); return GroupEntity.builder() .withNamespace(group.namespace()) .withId(group.id()) .withName(group.name()) - .withRoleNames(roleNames) - .withRoleIds(roleIds) + .withRoles(roleEntities) .withAuditInfo(updateAuditInfo) .build(); }; @@ -449,9 +437,10 @@ void updateGroup() throws IOException { Assertions.assertEquals(group1.id(), grantGroup.id()); Assertions.assertEquals(group1.name(), grantGroup.name()); Assertions.assertEquals( - Sets.newHashSet("role1", "role2", "role3"), Sets.newHashSet(grantGroup.roleNames())); + Sets.newHashSet("role1", "role2", "role3"), Sets.newHashSet(grantGroup.roles())); Assertions.assertEquals( - Sets.newHashSet(role1.id(), role2.id(), role3.id()), Sets.newHashSet(grantGroup.roleIds())); + Sets.newHashSet(role1.id(), role2.id(), role3.id()), + grantGroup.roleEntities().stream().map(RoleEntity::id).collect(Collectors.toSet())); Assertions.assertEquals("creator", grantGroup.auditInfo().creator()); Assertions.assertEquals("grantGroup", grantGroup.auditInfo().lastModifier()); @@ -466,17 +455,16 @@ void updateGroup() throws IOException { .withLastModifiedTime(Instant.now()) .build(); - List roleNames = Lists.newArrayList(group.roleNames()); - List roleIds = Lists.newArrayList(group.roleIds()); - roleIds.remove(roleNames.indexOf("role2")); - roleNames.remove("role2"); + List roleEntities = Lists.newArrayList(group.roleEntities()); + List roleNames = + roleEntities.stream().map(RoleEntity::name).collect(Collectors.toList()); + roleEntities.remove(roleNames.indexOf("role2")); return GroupEntity.builder() .withNamespace(group.namespace()) .withId(group.id()) .withName(group.name()) - .withRoleNames(roleNames) - .withRoleIds(roleIds) + .withRoles(roleEntities) .withAuditInfo(updateAuditInfo) .build(); }; @@ -487,9 +475,10 @@ void updateGroup() throws IOException { Assertions.assertEquals(group1.id(), revokeGroup.id()); Assertions.assertEquals(group1.name(), revokeGroup.name()); Assertions.assertEquals( - Sets.newHashSet("role1", "role3"), Sets.newHashSet(revokeGroup.roleNames())); + Sets.newHashSet("role1", "role3"), Sets.newHashSet(revokeGroup.roles())); Assertions.assertEquals( - Sets.newHashSet(role1.id(), role3.id()), Sets.newHashSet(revokeGroup.roleIds())); + Sets.newHashSet(role1.id(), role3.id()), + revokeGroup.roleEntities().stream().map(RoleEntity::id).collect(Collectors.toSet())); Assertions.assertEquals("creator", revokeGroup.auditInfo().creator()); Assertions.assertEquals("revokeGroup", revokeGroup.auditInfo().lastModifier()); @@ -513,19 +502,16 @@ void updateGroup() throws IOException { .withLastModifiedTime(Instant.now()) .build(); - List roleNames = Lists.newArrayList(group.roleNames()); - List roleIds = Lists.newArrayList(group.roleIds()); - roleIds.remove(roleNames.indexOf("role3")); - roleNames.remove("role3"); - roleIds.add(role4.id()); - roleNames.add(role4.name()); + List roleNames = Lists.newArrayList(group.roles()); + List roleEntities = Lists.newArrayList(group.roleEntities()); + roleEntities.remove(roleNames.indexOf("role3")); + roleEntities.add(role4); return GroupEntity.builder() .withNamespace(group.namespace()) .withId(group.id()) .withName(group.name()) - .withRoleNames(roleNames) - .withRoleIds(roleIds) + .withRoles(roleEntities) .withAuditInfo(updateAuditInfo) .build(); }; @@ -536,9 +522,10 @@ void updateGroup() throws IOException { Assertions.assertEquals(group1.id(), grantRevokeGroup.id()); Assertions.assertEquals(group1.name(), grantRevokeGroup.name()); Assertions.assertEquals( - Sets.newHashSet("role1", "role4"), Sets.newHashSet(grantRevokeGroup.roleNames())); + Sets.newHashSet("role1", "role4"), Sets.newHashSet(grantRevokeGroup.roles())); Assertions.assertEquals( - Sets.newHashSet(role1.id(), role4.id()), Sets.newHashSet(grantRevokeGroup.roleIds())); + Sets.newHashSet(role1.id(), role4.id()), + grantRevokeGroup.roleEntities().stream().map(RoleEntity::id).collect(Collectors.toSet())); Assertions.assertEquals("creator", grantRevokeGroup.auditInfo().creator()); Assertions.assertEquals("grantRevokeUser", grantRevokeGroup.auditInfo().lastModifier()); @@ -553,15 +540,13 @@ void updateGroup() throws IOException { .withLastModifiedTime(Instant.now()) .build(); - List roleNames = Lists.newArrayList(group.roleNames()); - List roleIds = Lists.newArrayList(group.roleIds()); + List roleEntities = Lists.newArrayList(group.roleEntities()); return GroupEntity.builder() .withNamespace(group.namespace()) .withId(group.id()) .withName(group.name()) - .withRoleNames(roleNames) - .withRoleIds(roleIds) + .withRoles(roleEntities) .withAuditInfo(updateAuditInfo) .build(); }; @@ -571,9 +556,10 @@ void updateGroup() throws IOException { Assertions.assertEquals(group1.id(), noUpdaterGroup.id()); Assertions.assertEquals(group1.name(), noUpdaterGroup.name()); Assertions.assertEquals( - Sets.newHashSet("role1", "role4"), Sets.newHashSet(noUpdaterGroup.roleNames())); + Sets.newHashSet("role1", "role4"), Sets.newHashSet(noUpdaterGroup.roles())); Assertions.assertEquals( - Sets.newHashSet(role1.id(), role4.id()), Sets.newHashSet(noUpdaterGroup.roleIds())); + Sets.newHashSet(role1.id(), role4.id()), + noUpdaterGroup.roleEntities().stream().map(RoleEntity::id).collect(Collectors.toSet())); Assertions.assertEquals("creator", noUpdaterGroup.auditInfo().creator()); Assertions.assertEquals("grantRevokeUser", noUpdaterGroup.auditInfo().lastModifier()); @@ -581,7 +567,7 @@ void updateGroup() throws IOException { RoleMetaService.getInstance().deleteRole(role1.nameIdentifier()); GroupEntity groupEntity = GroupMetaService.getInstance().getGroupByIdentifier(group1.nameIdentifier()); - Assertions.assertEquals(Sets.newHashSet("role4"), Sets.newHashSet(groupEntity.roleNames())); + Assertions.assertEquals(Sets.newHashSet("role4"), Sets.newHashSet(groupEntity.roles())); } @Test @@ -629,16 +615,14 @@ void deleteMetalake() throws IOException { AuthorizationUtils.ofGroupNamespace(metalakeName), "group1", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); GroupEntity group2 = createGroupEntity( RandomIdGenerator.INSTANCE.nextId(), AuthorizationUtils.ofGroupNamespace(metalakeName), "group2", auditInfo, - Lists.newArrayList(role3.name()), - Lists.newArrayList(role3.id())); + Lists.newArrayList(role3)); groupMetaService.insertGroup(group1, false); groupMetaService.insertGroup(group2, false); @@ -710,16 +694,14 @@ void deleteMetalakeCascade() throws IOException { AuthorizationUtils.ofGroupNamespace(metalakeName), "group1", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); GroupEntity group2 = createGroupEntity( RandomIdGenerator.INSTANCE.nextId(), AuthorizationUtils.ofGroupNamespace(metalakeName), "group2", auditInfo, - Lists.newArrayList(role3.name()), - Lists.newArrayList(role3.id())); + Lists.newArrayList(role3)); groupMetaService.insertGroup(group1, false); groupMetaService.insertGroup(group2, false); @@ -782,32 +764,28 @@ void deleteGroupMetasByLegacyTimeline() throws IOException { AuthorizationUtils.ofUserNamespace(metalakeName), "group1", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); GroupEntity group2 = createGroupEntity( RandomIdGenerator.INSTANCE.nextId(), AuthorizationUtils.ofUserNamespace(metalakeName), "group2", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); GroupEntity group3 = createGroupEntity( RandomIdGenerator.INSTANCE.nextId(), AuthorizationUtils.ofUserNamespace(metalakeName), "group3", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); GroupEntity group4 = createGroupEntity( RandomIdGenerator.INSTANCE.nextId(), AuthorizationUtils.ofUserNamespace(metalakeName), "group4", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); groupMetaService.insertGroup(group1, false); groupMetaService.insertGroup(group2, false); groupMetaService.insertGroup(group3, false); diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestRoleMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestRoleMetaService.java index d811b8b598f..1db24de1c9b 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestRoleMetaService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestRoleMetaService.java @@ -301,41 +301,36 @@ void deleteRole() throws IOException { AuthorizationUtils.ofGroupNamespace(metalakeName), "group1", auditInfo, - Lists.newArrayList(role2.name()), - Lists.newArrayList(role2.id())); + Lists.newArrayList(role2)); GroupEntity group2 = createGroupEntity( RandomIdGenerator.INSTANCE.nextId(), AuthorizationUtils.ofGroupNamespace(metalakeName), "group2", auditInfo, - Lists.newArrayList(role2.name()), - Lists.newArrayList(role2.id())); + Lists.newArrayList(role2)); groupMetaService.insertGroup(group1, false); groupMetaService.insertGroup(group2, false); Assertions.assertEquals( group1.name(), groupMetaService.getGroupByIdentifier(group1.nameIdentifier()).name()); Assertions.assertEquals( - group1.roleNames(), - groupMetaService.getGroupByIdentifier(group1.nameIdentifier()).roleNames()); + group1.roles(), groupMetaService.getGroupByIdentifier(group1.nameIdentifier()).roles()); Assertions.assertEquals( group2.name(), groupMetaService.getGroupByIdentifier(group2.nameIdentifier()).name()); Assertions.assertEquals( - group2.roleNames(), - groupMetaService.getGroupByIdentifier(group2.nameIdentifier()).roleNames()); + group2.roles(), groupMetaService.getGroupByIdentifier(group2.nameIdentifier()).roles()); UserEntity user1 = createUserEntity( RandomIdGenerator.INSTANCE.nextId(), AuthorizationUtils.ofUserNamespace(metalakeName), "user1", auditInfo, - Lists.newArrayList(role2.name()), - Lists.newArrayList(role2.id())); + Lists.newArrayList(role2)); userMetaService.insertUser(user1, false); Assertions.assertEquals( user1.name(), userMetaService.getUserByIdentifier(user1.nameIdentifier()).name()); Assertions.assertEquals( - user1.roleNames(), userMetaService.getUserByIdentifier(user1.nameIdentifier()).roleNames()); + user1.roles(), userMetaService.getUserByIdentifier(user1.nameIdentifier()).roles()); Assertions.assertTrue(roleMetaService.deleteRole(role2.nameIdentifier())); Assertions.assertThrows( @@ -434,16 +429,14 @@ void deleteMetalake() throws IOException { AuthorizationUtils.ofGroupNamespace(metalakeName), "group1", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); UserEntity user1 = createUserEntity( RandomIdGenerator.INSTANCE.nextId(), AuthorizationUtils.ofUserNamespace(metalakeName), "user1", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); roleMetaService.insertRole(role1, false); roleMetaService.insertRole(role2, false); groupMetaService.insertGroup(group1, false); @@ -546,16 +539,14 @@ void deleteMetalakeCascade() throws IOException { AuthorizationUtils.ofGroupNamespace(metalakeName), "group1", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); UserEntity user1 = createUserEntity( RandomIdGenerator.INSTANCE.nextId(), AuthorizationUtils.ofUserNamespace(metalakeName), "user1", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); roleMetaService.insertRole(role1, false); roleMetaService.insertRole(role2, false); groupMetaService.insertGroup(group1, false); @@ -658,8 +649,7 @@ void deleteRoleMetasByLegacyTimeline() throws IOException { AuthorizationUtils.ofUserNamespace(metalakeName), "user1", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); userMetaService.insertUser(user1, false); GroupEntity group1 = @@ -668,8 +658,7 @@ void deleteRoleMetasByLegacyTimeline() throws IOException { AuthorizationUtils.ofUserNamespace(metalakeName), "group1", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); groupMetaService.insertGroup(group1, false); // hard delete before soft delete 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 index 326ccfc2da3..90e675e06df 100644 --- 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 @@ -30,6 +30,7 @@ import java.util.List; import java.util.Optional; import java.util.function.Function; +import java.util.stream.Collectors; import org.apache.gravitino.Entity; import org.apache.gravitino.EntityAlreadyExistsException; import org.apache.gravitino.Namespace; @@ -109,13 +110,11 @@ void getUserByIdentifier() throws IOException { AuthorizationUtils.ofUserNamespace(metalakeName), "user2", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); userMetaService.insertUser(user2, false); UserEntity actualUser = userMetaService.getUserByIdentifier(user2.nameIdentifier()); Assertions.assertEquals(user2.name(), actualUser.name()); - Assertions.assertEquals( - Sets.newHashSet(user2.roleNames()), Sets.newHashSet(actualUser.roleNames())); + Assertions.assertEquals(Sets.newHashSet(user2.roles()), Sets.newHashSet(actualUser.roles())); } @Test @@ -193,13 +192,11 @@ void insertUser() throws IOException { AuthorizationUtils.ofUserNamespace(metalakeName), "user2", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); Assertions.assertDoesNotThrow(() -> userMetaService.insertUser(user2, false)); UserEntity actualUser = userMetaService.getUserByIdentifier(user2.nameIdentifier()); Assertions.assertEquals(user2.name(), actualUser.name()); - Assertions.assertEquals( - Sets.newHashSet(user2.roleNames()), Sets.newHashSet(actualUser.roleNames())); + Assertions.assertEquals(Sets.newHashSet(user2.roles()), Sets.newHashSet(actualUser.roles())); // insert duplicate user with roles UserEntity user2Exist = @@ -218,17 +215,15 @@ void insertUser() throws IOException { AuthorizationUtils.ofUserNamespace(metalakeName), "user2Overwrite", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); Assertions.assertDoesNotThrow(() -> userMetaService.insertUser(user2Overwrite, true)); UserEntity actualOverwriteUser2 = userMetaService.getUserByIdentifier(user2Overwrite.nameIdentifier()); Assertions.assertEquals("user2Overwrite", actualOverwriteUser2.name()); - Assertions.assertEquals(2, actualOverwriteUser2.roleNames().size()); + Assertions.assertEquals(2, actualOverwriteUser2.roles().size()); Assertions.assertEquals( - Sets.newHashSet(role1.name(), role2.name()), - Sets.newHashSet(actualOverwriteUser2.roleNames())); + Sets.newHashSet(role1.name(), role2.name()), Sets.newHashSet(actualOverwriteUser2.roles())); // insert overwrite user with 1 role RoleEntity role3 = @@ -245,15 +240,14 @@ void insertUser() throws IOException { AuthorizationUtils.ofUserNamespace(metalakeName), "user3Overwrite", auditInfo, - Lists.newArrayList(role3.name()), - Lists.newArrayList(role3.id())); + Lists.newArrayList(role3)); Assertions.assertDoesNotThrow(() -> userMetaService.insertUser(user3Overwrite, true)); UserEntity actualOverwriteUser3 = userMetaService.getUserByIdentifier(user3Overwrite.nameIdentifier()); Assertions.assertEquals("user3Overwrite", actualOverwriteUser3.name()); - Assertions.assertEquals(1, actualOverwriteUser3.roleNames().size()); - Assertions.assertEquals("role3", actualOverwriteUser3.roleNames().get(0)); + Assertions.assertEquals(1, actualOverwriteUser3.roles().size()); + Assertions.assertEquals("role3", actualOverwriteUser3.roles().get(0)); // insert overwrite user with 0 roles UserEntity user4Overwrite = @@ -267,7 +261,7 @@ void insertUser() throws IOException { UserEntity actualOverwriteUser4 = userMetaService.getUserByIdentifier(user4Overwrite.nameIdentifier()); Assertions.assertEquals("user4Overwrite", actualOverwriteUser4.name()); - Assertions.assertNull(actualOverwriteUser4.roleNames()); + Assertions.assertTrue(actualOverwriteUser4.roles().isEmpty()); } @Test @@ -338,8 +332,7 @@ void deleteUser() throws IOException { AuthorizationUtils.ofUserNamespace(metalakeName), "user2", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); userMetaService.insertUser(user2, false); List rolePOs = SessionUtils.doWithCommitAndFetchResult( @@ -347,8 +340,7 @@ void deleteUser() throws IOException { Assertions.assertEquals(2, rolePOs.size()); UserEntity actualUser = userMetaService.getUserByIdentifier(user2.nameIdentifier()); Assertions.assertEquals(user2.name(), actualUser.name()); - Assertions.assertEquals( - Sets.newHashSet(user2.roleNames()), Sets.newHashSet(actualUser.roleNames())); + Assertions.assertEquals(Sets.newHashSet(user2.roles()), Sets.newHashSet(actualUser.roles())); Assertions.assertTrue(userMetaService.deleteUser(user2.nameIdentifier())); Assertions.assertThrows( @@ -397,13 +389,11 @@ void updateUser() throws IOException { AuthorizationUtils.ofUserNamespace(metalakeName), "user1", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); userMetaService.insertUser(user1, false); UserEntity actualUser = userMetaService.getUserByIdentifier(user1.nameIdentifier()); Assertions.assertEquals(user1.name(), actualUser.name()); - Assertions.assertEquals( - Sets.newHashSet(user1.roleNames()), Sets.newHashSet(actualUser.roleNames())); + Assertions.assertEquals(Sets.newHashSet(user1.roles()), Sets.newHashSet(actualUser.roles())); RoleEntity role3 = createRoleEntity( @@ -425,17 +415,14 @@ void updateUser() throws IOException { .withLastModifiedTime(Instant.now()) .build(); - List roleNames = Lists.newArrayList(user.roleNames()); - List roleIds = Lists.newArrayList(user.roleIds()); - roleNames.add(role3.name()); - roleIds.add(role3.id()); + List roleEntities = Lists.newArrayList(user.roleEntities()); + roleEntities.add(role3); return UserEntity.builder() .withNamespace(user.namespace()) .withId(user.id()) .withName(user.name()) - .withRoleNames(roleNames) - .withRoleIds(roleIds) + .withRoles(roleEntities) .withAuditInfo(updateAuditInfo) .build(); }; @@ -446,9 +433,10 @@ void updateUser() throws IOException { Assertions.assertEquals(user1.id(), grantUser.id()); Assertions.assertEquals(user1.name(), grantUser.name()); Assertions.assertEquals( - Sets.newHashSet("role1", "role2", "role3"), Sets.newHashSet(grantUser.roleNames())); + Sets.newHashSet("role1", "role2", "role3"), Sets.newHashSet(grantUser.roles())); Assertions.assertEquals( - Sets.newHashSet(role1.id(), role2.id(), role3.id()), Sets.newHashSet(grantUser.roleIds())); + Sets.newHashSet(role1.id(), role2.id(), role3.id()), + grantUser.roleEntities().stream().map(RoleEntity::id).collect(Collectors.toSet())); Assertions.assertEquals("creator", grantUser.auditInfo().creator()); Assertions.assertEquals("grantUser", grantUser.auditInfo().lastModifier()); @@ -463,17 +451,15 @@ void updateUser() throws IOException { .withLastModifiedTime(Instant.now()) .build(); - List roleNames = Lists.newArrayList(user.roleNames()); - List roleIds = Lists.newArrayList(user.roleIds()); - roleIds.remove(roleNames.indexOf("role2")); - roleNames.remove("role2"); + List roleNames = Lists.newArrayList(user.roles()); + List roleEntities = Lists.newArrayList(user.roleEntities()); + roleEntities.remove(roleNames.indexOf("role2")); return UserEntity.builder() .withNamespace(user.namespace()) .withId(user.id()) .withName(user.name()) - .withRoleNames(roleNames) - .withRoleIds(roleIds) + .withRoles(roleEntities) .withAuditInfo(updateAuditInfo) .build(); }; @@ -483,10 +469,10 @@ void updateUser() throws IOException { UserMetaService.getInstance().getUserByIdentifier(user1.nameIdentifier()); Assertions.assertEquals(user1.id(), revokeUser.id()); Assertions.assertEquals(user1.name(), revokeUser.name()); + Assertions.assertEquals(Sets.newHashSet("role1", "role3"), Sets.newHashSet(revokeUser.roles())); Assertions.assertEquals( - Sets.newHashSet("role1", "role3"), Sets.newHashSet(revokeUser.roleNames())); - Assertions.assertEquals( - Sets.newHashSet(role1.id(), role3.id()), Sets.newHashSet(revokeUser.roleIds())); + Sets.newHashSet(role1.id(), role3.id()), + revokeUser.roleEntities().stream().map(RoleEntity::id).collect(Collectors.toSet())); Assertions.assertEquals("creator", revokeUser.auditInfo().creator()); Assertions.assertEquals("revokeUser", revokeUser.auditInfo().lastModifier()); @@ -510,19 +496,16 @@ void updateUser() throws IOException { .withLastModifiedTime(Instant.now()) .build(); - List roleNames = Lists.newArrayList(user.roleNames()); - List roleIds = Lists.newArrayList(user.roleIds()); - roleIds.remove(roleNames.indexOf("role3")); - roleNames.remove("role3"); - roleIds.add(role4.id()); - roleNames.add(role4.name()); + List roleNames = Lists.newArrayList(user.roles()); + List roleEntities = Lists.newArrayList(user.roleEntities()); + roleEntities.remove(roleNames.indexOf("role3")); + roleEntities.add(role4); return UserEntity.builder() .withNamespace(user.namespace()) .withId(user.id()) .withName(user.name()) - .withRoleNames(roleNames) - .withRoleIds(roleIds) + .withRoles(roleEntities) .withAuditInfo(updateAuditInfo) .build(); }; @@ -533,9 +516,10 @@ void updateUser() throws IOException { Assertions.assertEquals(user1.id(), grantRevokeUser.id()); Assertions.assertEquals(user1.name(), grantRevokeUser.name()); Assertions.assertEquals( - Sets.newHashSet("role1", "role4"), Sets.newHashSet(grantRevokeUser.roleNames())); + Sets.newHashSet("role1", "role4"), Sets.newHashSet(grantRevokeUser.roles())); Assertions.assertEquals( - Sets.newHashSet(role1.id(), role4.id()), Sets.newHashSet(grantRevokeUser.roleIds())); + Sets.newHashSet(role1.id(), role4.id()), + grantRevokeUser.roleEntities().stream().map(RoleEntity::id).collect(Collectors.toSet())); Assertions.assertEquals("creator", grantRevokeUser.auditInfo().creator()); Assertions.assertEquals("grantRevokeUser", grantRevokeUser.auditInfo().lastModifier()); @@ -549,15 +533,13 @@ void updateUser() throws IOException { .withLastModifiedTime(Instant.now()) .build(); - List roleNames = Lists.newArrayList(user.roleNames()); - List roleIds = Lists.newArrayList(user.roleIds()); + List roleEntities = Lists.newArrayList(user.roleEntities()); return UserEntity.builder() .withNamespace(user.namespace()) .withId(user.id()) .withName(user.name()) - .withRoleNames(roleNames) - .withRoleIds(roleIds) + .withRoles(roleEntities) .withAuditInfo(updateAuditInfo) .build(); }; @@ -567,9 +549,10 @@ void updateUser() throws IOException { Assertions.assertEquals(user1.id(), noUpdaterUser.id()); Assertions.assertEquals(user1.name(), noUpdaterUser.name()); Assertions.assertEquals( - Sets.newHashSet("role1", "role4"), Sets.newHashSet(noUpdaterUser.roleNames())); + Sets.newHashSet("role1", "role4"), Sets.newHashSet(noUpdaterUser.roles())); Assertions.assertEquals( - Sets.newHashSet(role1.id(), role4.id()), Sets.newHashSet(noUpdaterUser.roleIds())); + Sets.newHashSet(role1.id(), role4.id()), + noUpdaterUser.roleEntities().stream().map(RoleEntity::id).collect(Collectors.toSet())); Assertions.assertEquals("creator", noUpdaterUser.auditInfo().creator()); Assertions.assertEquals("grantRevokeUser", noUpdaterUser.auditInfo().lastModifier()); @@ -577,7 +560,7 @@ void updateUser() throws IOException { RoleMetaService.getInstance().deleteRole(role1.nameIdentifier()); UserEntity userEntity = UserMetaService.getInstance().getUserByIdentifier(user1.nameIdentifier()); - Assertions.assertEquals(Sets.newHashSet("role4"), Sets.newHashSet(userEntity.roleNames())); + Assertions.assertEquals(Sets.newHashSet("role4"), Sets.newHashSet(userEntity.roles())); } @Test @@ -625,16 +608,14 @@ void deleteMetalake() throws IOException { AuthorizationUtils.ofUserNamespace(metalakeName), "user1", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); UserEntity user2 = createUserEntity( RandomIdGenerator.INSTANCE.nextId(), AuthorizationUtils.ofUserNamespace(metalakeName), "user2", auditInfo, - Lists.newArrayList(role3.name()), - Lists.newArrayList(role3.id())); + Lists.newArrayList(role3)); userMetaService.insertUser(user1, false); userMetaService.insertUser(user2, false); @@ -706,16 +687,14 @@ void deleteMetalakeCascade() throws IOException { AuthorizationUtils.ofUserNamespace(metalakeName), "user1", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); UserEntity user2 = createUserEntity( RandomIdGenerator.INSTANCE.nextId(), AuthorizationUtils.ofUserNamespace(metalakeName), "user2", auditInfo, - Lists.newArrayList(role3.name()), - Lists.newArrayList(role3.id())); + Lists.newArrayList(role3)); userMetaService.insertUser(user1, false); userMetaService.insertUser(user2, false); @@ -778,32 +757,28 @@ void deleteUserMetasByLegacyTimeline() throws IOException { AuthorizationUtils.ofUserNamespace(metalakeName), "user1", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); UserEntity user2 = createUserEntity( RandomIdGenerator.INSTANCE.nextId(), AuthorizationUtils.ofUserNamespace(metalakeName), "user2", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); UserEntity user3 = createUserEntity( RandomIdGenerator.INSTANCE.nextId(), AuthorizationUtils.ofUserNamespace(metalakeName), "user3", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); UserEntity user4 = createUserEntity( RandomIdGenerator.INSTANCE.nextId(), AuthorizationUtils.ofUserNamespace(metalakeName), "user4", auditInfo, - Lists.newArrayList(role1.name(), role2.name()), - Lists.newArrayList(role1.id(), role2.id())); + Lists.newArrayList(role1, role2)); userMetaService.insertUser(user1, false); userMetaService.insertUser(user2, false); userMetaService.insertUser(user3, false); diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestGroupOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestGroupOperations.java index c3b34bc6bff..6a4b8e95354 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestGroupOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestGroupOperations.java @@ -245,7 +245,7 @@ private Group buildGroup(String group) { return GroupEntity.builder() .withId(1L) .withName(group) - .withRoleNames(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withAuditInfo( AuditInfo.builder().withCreator("creator").withCreateTime(Instant.now()).build()) .build(); diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java index 76dba3edd4b..39ed06ebbcf 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java @@ -52,6 +52,7 @@ import org.apache.gravitino.lock.LockManager; import org.apache.gravitino.meta.AuditInfo; import org.apache.gravitino.meta.GroupEntity; +import org.apache.gravitino.meta.RoleEntity; import org.apache.gravitino.meta.UserEntity; import org.apache.gravitino.rest.RESTUtils; import org.glassfish.hk2.utilities.binding.AbstractBinder; @@ -110,12 +111,18 @@ protected void configure() { @Test public void testGrantRolesToUser() { + RoleEntity roleEntity = + RoleEntity.builder() + .withId(1L) + .withAuditInfo( + AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build()) + .withName("roles") + .build(); UserEntity userEntity = UserEntity.builder() .withId(1L) .withName("user") - .withRoleNames(Lists.newArrayList("roles")) - .withRoleIds(Lists.newArrayList(1L)) + .withRoles(Lists.newArrayList(roleEntity)) .withAuditInfo( AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build()) .build(); @@ -207,12 +214,18 @@ public void testGrantRolesToUser() { @Test public void testGrantRolesToGroup() { + RoleEntity roleEntity = + RoleEntity.builder() + .withId(1L) + .withName("roles") + .withAuditInfo( + AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build()) + .build(); GroupEntity groupEntity = GroupEntity.builder() .withId(1L) .withName("group") - .withRoleNames(Lists.newArrayList("roles")) - .withRoleIds(Lists.newArrayList(1L)) + .withRoles(Lists.newArrayList(roleEntity)) .withAuditInfo( AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build()) .build(); @@ -311,8 +324,7 @@ public void testRevokeRolesFromUser() { UserEntity.builder() .withId(1L) .withName("user") - .withRoleNames(Lists.newArrayList()) - .withRoleIds(Lists.newArrayList(1L)) + .withRoles(Lists.newArrayList()) .withAuditInfo( AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build()) .build(); @@ -356,8 +368,7 @@ public void testRevokeRolesFromGroup() { GroupEntity.builder() .withId(1L) .withName("group") - .withRoleNames(Lists.newArrayList()) - .withRoleIds(Lists.newArrayList(1L)) + .withRoles(Lists.newArrayList()) .withAuditInfo( AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build()) .build(); diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestUserOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestUserOperations.java index d3209e0e22c..b13c0175b0b 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestUserOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestUserOperations.java @@ -246,7 +246,7 @@ private User buildUser(String user) { return UserEntity.builder() .withId(1L) .withName(user) - .withRoleNames(Collections.emptyList()) + .withRoles(Collections.emptyList()) .withAuditInfo( AuditInfo.builder().withCreator("creator").withCreateTime(Instant.now()).build()) .build();