xunliu commented on code in PR #4894:
URL: https://github.com/apache/gravitino/pull/4894#discussion_r1775093805
##########
core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java:
##########
@@ -117,26 +114,21 @@ User getUser(String metalake, String user) throws
NoSuchUserException {
}
String[] listUserNames(String metalake) {
- Set<Field> skippingFields = Sets.newHashSet();
- skippingFields.add(UserEntity.ROLE_NAMES);
- skippingFields.add(UserEntity.ROLE_IDS);
- return Arrays.stream(listUsersInternal(metalake, skippingFields))
- .map(User::name)
- .toArray(String[]::new);
+ return Arrays.stream(listUsersInternal(metalake,
false)).map(User::name).toArray(String[]::new);
}
User[] listUsers(String metalake) {
- return listUsersInternal(metalake, Collections.emptySet());
+ return listUsersInternal(metalake, true);
Review Comment:
Better add a common.
```
return listUsersInternal(metalake, true/* allFields */);
```
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java:
##########
@@ -237,6 +239,26 @@ private List<SecurableObjectPO>
listSecurableObjectsByRoleId(Long roleId) {
SecurableObjectMapper.class, mapper ->
mapper.listSecurableObjectsByRoleId(roleId));
}
+ public List<RoleEntity> listRolesByNamespace(Namespace namespace, boolean
allFields) {
+ AuthorizationUtils.checkRoleNamespace(namespace);
+ String metalakeName = namespace.level(0);
+
+ if (allFields) {
+ throw new IllegalArgumentException("Don't support list all the fields of
roles");
+ }
Review Comment:
Can we remove this `allFields` param in the `public List<RoleEntity>
listRolesByNamespace(Namespace namespace, boolean allFields)` ?
##########
core/src/main/java/org/apache/gravitino/EntityStore.java:
##########
@@ -66,7 +64,7 @@ public interface EntityStore extends Closeable {
*/
default <E extends Entity & HasIdentifier> List<E> list(
Namespace namespace, Class<E> type, EntityType entityType) throws
IOException {
- return list(namespace, type, entityType, Collections.emptySet());
+ return list(namespace, type, entityType, true);
Review Comment:
Better add a common.
```
return list(namespace, type, entityType, true/* allFields */);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
