From 10070c1c3c1ee0dc054d21143a6099d5d17e114f Mon Sep 17 00:00:00 2001 From: qiang_liu Date: Sun, 8 Sep 2024 11:45:23 +0800 Subject: [PATCH 01/16] #4873 support list group --- .../dto/responses/GroupListResponse.java | 74 ++++++++++++++++++ .../AccessControlDispatcher.java | 8 ++ .../authorization/AccessControlManager.java | 6 ++ .../authorization/UserGroupManager.java | 27 +++++++ .../hook/AccessControlHookDispatcher.java | 6 ++ .../server/web/rest/GroupOperations.java | 37 +++++++++ .../server/web/rest/TestGroupOperations.java | 75 ++++++++++++++++--- 7 files changed, 224 insertions(+), 9 deletions(-) create mode 100644 common/src/main/java/org/apache/gravitino/dto/responses/GroupListResponse.java diff --git a/common/src/main/java/org/apache/gravitino/dto/responses/GroupListResponse.java b/common/src/main/java/org/apache/gravitino/dto/responses/GroupListResponse.java new file mode 100644 index 00000000000..e6cc6ed8d76 --- /dev/null +++ b/common/src/main/java/org/apache/gravitino/dto/responses/GroupListResponse.java @@ -0,0 +1,74 @@ +/* + * 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.dto.responses; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Preconditions; +import java.util.Arrays; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.ToString; +import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.dto.authorization.GroupDTO; + +/** Represents a response for a list of groups. */ +@Getter +@ToString +@EqualsAndHashCode(callSuper = true) +public class GroupListResponse extends BaseResponse { + + @JsonProperty("groups") + private final GroupDTO[] groups; + + /** + * Constructor for GroupListResponse. + * + * @param groups The array of group DTOs. + */ + public GroupListResponse(GroupDTO[] groups) { + super(0); + this.groups = groups; + } + + /** Default constructor for GroupListResponse. (Used for Jackson deserialization.) */ + public GroupListResponse() { + super(); + this.groups = null; + } + + /** + * Validates the response data. + * + * @throws IllegalArgumentException if the name or audit is not set. + */ + @Override + public void validate() throws IllegalArgumentException { + super.validate(); + + Preconditions.checkArgument(groups != null, "groups must not be null"); + Arrays.stream(groups) + .forEach( + group -> { + Preconditions.checkArgument( + StringUtils.isNotBlank(group.name()), "group 'name' must not be null and empty"); + Preconditions.checkArgument( + group.auditInfo() != null, "group 'auditInfo' must not be null"); + }); + } +} diff --git a/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java b/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java index fabc8acaaf3..e955948dfa7 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java @@ -20,6 +20,7 @@ import java.util.List; import java.util.Map; +import org.apache.gravitino.Namespace; import org.apache.gravitino.exceptions.GroupAlreadyExistsException; import org.apache.gravitino.exceptions.NoSuchGroupException; import org.apache.gravitino.exceptions.NoSuchMetalakeException; @@ -109,6 +110,13 @@ Group addGroup(String metalake, String group) Group getGroup(String metalake, String group) throws NoSuchGroupException, NoSuchMetalakeException; + /** + * List groups + * + * @return The list of groups + */ + Group[] listGroup(Namespace namespace); + /** * Grant roles to a user. * diff --git a/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java b/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java index 8c6a73346d0..9d61f7a62d7 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java @@ -24,6 +24,7 @@ import org.apache.gravitino.Config; import org.apache.gravitino.Configs; import org.apache.gravitino.EntityStore; +import org.apache.gravitino.Namespace; import org.apache.gravitino.exceptions.GroupAlreadyExistsException; import org.apache.gravitino.exceptions.NoSuchGroupException; import org.apache.gravitino.exceptions.NoSuchMetalakeException; @@ -85,6 +86,11 @@ public Group getGroup(String metalake, String group) return userGroupManager.getGroup(metalake, group); } + @Override + public Group[] listGroup(Namespace namespace) { + return userGroupManager.listGroups(namespace); + } + @Override public User grantRolesToUser(String metalake, List roles, String user) throws NoSuchUserException, NoSuchRoleException, NoSuchMetalakeException { 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..cb52af27198 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java @@ -23,11 +23,15 @@ import java.time.Instant; import java.util.Collections; import org.apache.gravitino.Entity; +import org.apache.gravitino.Entity.EntityType; import org.apache.gravitino.EntityAlreadyExistsException; import org.apache.gravitino.EntityStore; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.Namespace; import org.apache.gravitino.exceptions.GroupAlreadyExistsException; import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.exceptions.NoSuchGroupException; +import org.apache.gravitino.exceptions.NoSuchMetalakeException; import org.apache.gravitino.exceptions.NoSuchUserException; import org.apache.gravitino.exceptions.UserAlreadyExistsException; import org.apache.gravitino.meta.AuditInfo; @@ -46,6 +50,7 @@ class UserGroupManager { private static final Logger LOG = LoggerFactory.getLogger(UserGroupManager.class); + private static final String METALAKE_DOES_NOT_EXIST_MSG = "Metalake %s does not exist"; private final EntityStore store; private final IdGenerator idGenerator; @@ -165,4 +170,26 @@ Group getGroup(String metalake, String group) { throw new RuntimeException(ioe); } } + + private void checkMetalakeExists(NameIdentifier ident) throws NoSuchMetalakeException { + try { + if (!store.exists(ident, EntityType.METALAKE)) { + throw new NoSuchMetalakeException(METALAKE_DOES_NOT_EXIST_MSG, ident); + } + } catch (IOException e) { + LOG.error("Failed to do storage operation", e); + throw new RuntimeException(e); + } + } + + Group[] listGroups(Namespace namespace) { + NameIdentifier groupIdent = NameIdentifier.of(namespace.levels()); + checkMetalakeExists(groupIdent); + try { + return store.list(namespace, GroupEntity.class, EntityType.GROUP).toArray(new Group[0]); + } catch (Exception ioe) { + LOG.error("Listing Groups failed due to storage issues.", ioe); + throw new RuntimeException(ioe); + } + } } diff --git a/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java b/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java index 44dc491a722..c5bcca57ed0 100644 --- a/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java @@ -22,6 +22,7 @@ import java.util.Map; import org.apache.gravitino.Entity; import org.apache.gravitino.GravitinoEnv; +import org.apache.gravitino.Namespace; import org.apache.gravitino.authorization.AccessControlDispatcher; import org.apache.gravitino.authorization.AuthorizationUtils; import org.apache.gravitino.authorization.Group; @@ -86,6 +87,11 @@ public Group getGroup(String metalake, String group) return dispatcher.getGroup(metalake, group); } + @Override + public Group[] listGroup(Namespace namespace) { + return dispatcher.listGroup(namespace); + } + @Override public User grantRolesToUser(String metalake, List roles, String user) throws NoSuchUserException, NoSuchRoleException, NoSuchMetalakeException { diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java index 537bafb9e78..74932b28d1e 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java @@ -20,6 +20,7 @@ import com.codahale.metrics.annotation.ResponseMetered; import com.codahale.metrics.annotation.Timed; +import java.util.Arrays; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.DELETE; import javax.ws.rs.GET; @@ -31,9 +32,13 @@ import javax.ws.rs.core.Response; import org.apache.gravitino.GravitinoEnv; import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.Namespace; import org.apache.gravitino.authorization.AccessControlDispatcher; import org.apache.gravitino.authorization.AuthorizationUtils; +import org.apache.gravitino.authorization.Group; +import org.apache.gravitino.dto.authorization.GroupDTO; import org.apache.gravitino.dto.requests.GroupAddRequest; +import org.apache.gravitino.dto.responses.GroupListResponse; import org.apache.gravitino.dto.responses.GroupResponse; import org.apache.gravitino.dto.responses.RemoveResponse; import org.apache.gravitino.dto.util.DTOConverters; @@ -134,4 +139,36 @@ public Response removeGroup( return ExceptionHandlers.handleGroupException(OperationType.REMOVE, group, metalake, e); } } + + @GET + @Produces("application/vnd.gravitino.v1+json") + @Timed(name = "list-group." + MetricNames.HTTP_PROCESS_DURATION, absolute = true) + @ResponseMetered(name = "list-group", absolute = true) + public Response listGroups(@PathParam("metalake") String metalake) { + LOG.info("Received list groups request."); + try { + return Utils.doAs( + httpRequest, + () -> { + Namespace groupNS = AuthorizationUtils.ofGroupNamespace(metalake); + Group[] groups = + TreeLockUtils.doWithTreeLock( + NameIdentifier.of(metalake), + LockType.READ, + () -> accessControlManager.listGroup(groupNS)); + GroupDTO[] groupDTOS; + if (groups == null) { + groupDTOS = new GroupDTO[0]; + } else { + groupDTOS = Arrays.stream(groups).map(DTOConverters::toDTO).toArray(GroupDTO[]::new); + } + LOG.info("List {} groups in Gravitino", groupDTOS.length); + return Utils.ok(new GroupListResponse(groupDTOS)); + }); + + } catch (Exception e) { + return ExceptionHandlers.handleGroupException( + OperationType.LIST, Namespace.empty().toString(), metalake, e); + } + } } 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..b24042d2366 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 @@ -34,15 +34,19 @@ import javax.ws.rs.core.Application; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import javax.ws.rs.core.Response.Status; import org.apache.commons.lang3.reflect.FieldUtils; import org.apache.gravitino.Config; import org.apache.gravitino.GravitinoEnv; +import org.apache.gravitino.Namespace; import org.apache.gravitino.authorization.AccessControlManager; +import org.apache.gravitino.authorization.AuthorizationUtils; import org.apache.gravitino.authorization.Group; import org.apache.gravitino.dto.authorization.GroupDTO; import org.apache.gravitino.dto.requests.GroupAddRequest; import org.apache.gravitino.dto.responses.ErrorConstants; import org.apache.gravitino.dto.responses.ErrorResponse; +import org.apache.gravitino.dto.responses.GroupListResponse; import org.apache.gravitino.dto.responses.GroupResponse; import org.apache.gravitino.dto.responses.RemoveResponse; import org.apache.gravitino.exceptions.GroupAlreadyExistsException; @@ -119,7 +123,7 @@ public void testAddGroup() { .accept("application/vnd.gravitino.v1+json") .post(Entity.entity(req, MediaType.APPLICATION_JSON_TYPE)); - Assertions.assertEquals(Response.Status.OK.getStatusCode(), resp.getStatus()); + Assertions.assertEquals(Status.OK.getStatusCode(), resp.getStatus()); Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp.getMediaType()); GroupResponse groupResponse = resp.readEntity(GroupResponse.class); @@ -138,7 +142,7 @@ public void testAddGroup() { .accept("application/vnd.gravitino.v1+json") .post(Entity.entity(req, MediaType.APPLICATION_JSON_TYPE)); - Assertions.assertEquals(Response.Status.NOT_FOUND.getStatusCode(), resp1.getStatus()); + Assertions.assertEquals(Status.NOT_FOUND.getStatusCode(), resp1.getStatus()); Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, resp1.getMediaType()); ErrorResponse errorResponse = resp1.readEntity(ErrorResponse.class); @@ -153,7 +157,7 @@ public void testAddGroup() { .accept("application/vnd.gravitino.v1+json") .post(Entity.entity(req, MediaType.APPLICATION_JSON_TYPE)); - Assertions.assertEquals(Response.Status.CONFLICT.getStatusCode(), resp2.getStatus()); + Assertions.assertEquals(Status.CONFLICT.getStatusCode(), resp2.getStatus()); ErrorResponse errorResponse1 = resp2.readEntity(ErrorResponse.class); Assertions.assertEquals(ErrorConstants.ALREADY_EXISTS_CODE, errorResponse1.getCode()); @@ -168,8 +172,7 @@ public void testAddGroup() { .accept("application/vnd.gravitino.v1+json") .post(Entity.entity(req, MediaType.APPLICATION_JSON_TYPE)); - Assertions.assertEquals( - Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp3.getStatus()); + Assertions.assertEquals(Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp3.getStatus()); ErrorResponse errorResponse2 = resp3.readEntity(ErrorResponse.class); Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE, errorResponse2.getCode()); @@ -251,6 +254,61 @@ private Group buildGroup(String group) { .build(); } + @Test + public void testListGroup() { + Group group = buildGroup("group0"); + Group group2 = buildGroup("group1"); + Group[] groups = {group, group2}; + Namespace groupNS = AuthorizationUtils.ofGroupNamespace("metalake1"); + + when(manager.listGroup(groupNS)).thenReturn(groups); + + Response resp = + target("/metalakes/metalake1/groups") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .get(); + + Assertions.assertEquals(Status.OK.getStatusCode(), resp.getStatus()); + + GroupListResponse groupListResponse = resp.readEntity(GroupListResponse.class); + Assertions.assertEquals(0, groupListResponse.getCode()); + GroupDTO[] groupDTOS = groupListResponse.getGroups(); + for (int i = 0; i < groupDTOS.length; i++) { + Assertions.assertEquals("group" + i, groupDTOS[i].name()); + Assertions.assertNotNull(groupDTOS[i].roles()); + Assertions.assertTrue(groupDTOS[i].roles().isEmpty()); + } + + // Test to throw NoSuchMetalakeException + doThrow(new NoSuchMetalakeException("mock error")).when(manager).listGroup(groupNS); + Response resp1 = + target("/metalakes/metalake1/groups") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .get(); + + Assertions.assertEquals(Status.NOT_FOUND.getStatusCode(), resp1.getStatus()); + + ErrorResponse errorResponse = resp1.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.NOT_FOUND_CODE, errorResponse.getCode()); + Assertions.assertEquals(NoSuchMetalakeException.class.getSimpleName(), errorResponse.getType()); + + // Test to throw internal RuntimeException + doThrow(new RuntimeException("mock error")).when(manager).listGroup(groupNS); + Response resp2 = + target("/metalakes/metalake1/groups") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .get(); + + Assertions.assertEquals(Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp2.getStatus()); + + ErrorResponse errorResponse2 = resp2.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE, errorResponse2.getCode()); + Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResponse2.getType()); + } + @Test public void testRemoveGroup() { when(manager.removeGroup(any(), any())).thenReturn(true); @@ -261,7 +319,7 @@ public void testRemoveGroup() { .accept("application/vnd.gravitino.v1+json") .delete(); - Assertions.assertEquals(Response.Status.OK.getStatusCode(), resp.getStatus()); + Assertions.assertEquals(Status.OK.getStatusCode(), resp.getStatus()); RemoveResponse removeResponse = resp.readEntity(RemoveResponse.class); Assertions.assertEquals(0, removeResponse.getCode()); Assertions.assertTrue(removeResponse.removed()); @@ -274,7 +332,7 @@ public void testRemoveGroup() { .accept("application/vnd.gravitino.v1+json") .delete(); - Assertions.assertEquals(Response.Status.OK.getStatusCode(), resp2.getStatus()); + Assertions.assertEquals(Status.OK.getStatusCode(), resp2.getStatus()); RemoveResponse removeResponse2 = resp2.readEntity(RemoveResponse.class); Assertions.assertEquals(0, removeResponse2.getCode()); Assertions.assertFalse(removeResponse2.removed()); @@ -286,8 +344,7 @@ public void testRemoveGroup() { .accept("application/vnd.gravitino.v1+json") .delete(); - Assertions.assertEquals( - Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp3.getStatus()); + Assertions.assertEquals(Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp3.getStatus()); ErrorResponse errorResponse = resp3.readEntity(ErrorResponse.class); Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE, errorResponse.getCode()); From eb8cba6529420514309ea22f972724c8792a4c2a Mon Sep 17 00:00:00 2001 From: qiang_liu Date: Wed, 25 Sep 2024 22:34:58 +0800 Subject: [PATCH 02/16] #4873 support list group --- .../gravitino/client/GravitinoClient.java | 9 +++ .../gravitino/client/GravitinoMetalake.java | 38 +++++++++++++ .../gravitino/client/TestUserGroup.java | 56 +++++++++++++++++++ .../test/authorization/AccessControlIT.java | 17 ++++++ .../gravitino/dto/util/DTOConverters.java | 8 +++ .../AccessControlDispatcher.java | 4 +- .../authorization/AccessControlManager.java | 9 ++- .../authorization/UserGroupManager.java | 16 +++++- .../hook/AccessControlHookDispatcher.java | 10 +++- .../storage/relational/JDBCBackend.java | 2 + .../mapper/GroupMetaBaseSQLProvider.java | 7 +++ .../relational/mapper/GroupMetaMapper.java | 7 +++ .../mapper/GroupMetaSQLProviderFactory.java | 4 ++ .../relational/po/ExtendedGroupPO.java | 56 +++++++++++++++++++ .../relational/service/GroupMetaService.java | 14 +++++ .../relational/utils/POConverters.java | 1 + .../server/web/rest/GroupOperations.java | 3 +- .../server/web/rest/TestGroupOperations.java | 7 +-- 18 files changed, 254 insertions(+), 14 deletions(-) create mode 100644 core/src/main/java/org/apache/gravitino/storage/relational/po/ExtendedGroupPO.java diff --git a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java index 9b7769200be..2df377739d1 100644 --- a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java +++ b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java @@ -207,6 +207,15 @@ public Group getGroup(String group) throws NoSuchGroupException, NoSuchMetalakeE return getMetalake().getGroup(group); } + + public Group[] listGroups(){ + return getMetalake().listGroups(); + } + + public String[] listGroupNames(){ + return getMetalake().listGroupNames(); + } + /** * Gets a Role. * diff --git a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java index f13958cb526..037e8d9a9f1 100644 --- a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java +++ b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java @@ -60,6 +60,7 @@ import org.apache.gravitino.dto.responses.DropResponse; import org.apache.gravitino.dto.responses.EntityListResponse; import org.apache.gravitino.dto.responses.ErrorResponse; +import org.apache.gravitino.dto.responses.GroupListResponse; import org.apache.gravitino.dto.responses.GroupResponse; import org.apache.gravitino.dto.responses.NameListResponse; import org.apache.gravitino.dto.responses.OwnerResponse; @@ -582,6 +583,43 @@ public Group getGroup(String group) throws NoSuchGroupException, NoSuchMetalakeE return resp.getGroup(); } + + /** + * Lists the groups + * + * @return The Group list + */ + public Group[] listGroups() { + Map params = new HashMap<>(); + GroupListResponse resp = + restClient.get( + String.format(API_METALAKES_GROUPS_PATH,name(),BLANK_PLACE_HOLDER), + params, + GroupListResponse.class, + Collections.emptyMap(), + ErrorHandlers.groupErrorHandler() + ); + resp.validate(); + return resp.getGroups(); + } + + /** + * Lists the group names + * + * @return The Group Name List + */ + public String[] listGroupNames(){ + NameListResponse resp = restClient.get( + String.format(API_METALAKES_GROUPS_PATH,name(), + BLANK_PLACE_HOLDER), + NameListResponse.class, + Collections.emptyMap(), + ErrorHandlers.groupErrorHandler() + ); + resp.validate(); + return resp.getNames(); + } + /** * Gets a Role. * diff --git a/clients/client-java/src/test/java/org/apache/gravitino/client/TestUserGroup.java b/clients/client-java/src/test/java/org/apache/gravitino/client/TestUserGroup.java index f3885a05f9c..b505e73a552 100644 --- a/clients/client-java/src/test/java/org/apache/gravitino/client/TestUserGroup.java +++ b/clients/client-java/src/test/java/org/apache/gravitino/client/TestUserGroup.java @@ -23,7 +23,11 @@ import static javax.servlet.http.HttpServletResponse.SC_OK; import static org.apache.hc.core5.http.HttpStatus.SC_SERVER_ERROR; +import com.fasterxml.jackson.core.JsonProcessingException; import java.time.Instant; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import org.apache.gravitino.authorization.Group; import org.apache.gravitino.authorization.User; import org.apache.gravitino.dto.AuditDTO; @@ -33,8 +37,10 @@ import org.apache.gravitino.dto.requests.GroupAddRequest; import org.apache.gravitino.dto.requests.UserAddRequest; import org.apache.gravitino.dto.responses.ErrorResponse; +import org.apache.gravitino.dto.responses.GroupListResponse; import org.apache.gravitino.dto.responses.GroupResponse; import org.apache.gravitino.dto.responses.MetalakeResponse; +import org.apache.gravitino.dto.responses.NameListResponse; import org.apache.gravitino.dto.responses.RemoveResponse; import org.apache.gravitino.dto.responses.UserResponse; import org.apache.gravitino.exceptions.GroupAlreadyExistsException; @@ -42,6 +48,7 @@ import org.apache.gravitino.exceptions.NoSuchMetalakeException; import org.apache.gravitino.exceptions.NoSuchUserException; import org.apache.gravitino.exceptions.UserAlreadyExistsException; +import org.apache.hadoop.security.Groups; import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.http.Method; import org.junit.jupiter.api.Assertions; @@ -273,6 +280,55 @@ public void testRemoveGroups() throws Exception { Assertions.assertThrows(RuntimeException.class, () -> gravitinoClient.removeGroup(groupName)); } + + @Test + public void testListGroupNames() throws JsonProcessingException { + String groupPath = withSlash(String.format(API_METALAKES_GROUPS_PATH,metalakeName,"")); + NameListResponse listResponse = new NameListResponse(new String[] {"group1","group2"}); + buildMockResource(Method.GET,groupPath,null,listResponse,SC_OK); + Assertions.assertArrayEquals(new String[]{"group1","group2"},gravitinoClient.listGroupNames()); + ErrorResponse errRespNoMetaLake = ErrorResponse.notFound(NoSuchMetalakeException.class.getSimpleName(),"metalake not found"); + buildMockResource(Method.GET,groupPath,null,errRespNoMetaLake,SC_NOT_FOUND); + Exception ex = Assertions.assertThrows( + NoSuchMetalakeException.class,()->{ + gravitinoClient.listGroupNames(); + } + ); + Assertions.assertEquals("metalake not found",ex.getMessage()); + + // Test RuntimeException + ErrorResponse errResp = ErrorResponse.internalError("internal error"); + buildMockResource(Method.GET,groupPath,null,errResp,SC_SERVER_ERROR); + + Assertions.assertThrows(RuntimeException.class,()->gravitinoClient.listGroupNames()); + } + + @Test + public void testListGroups() throws JsonProcessingException { + String groupPath = withSlash(String.format(API_METALAKES_GROUPS_PATH,metalakeName,"")); + GroupDTO group1 = mockGroupDTO("group1"); + GroupDTO group2 = mockGroupDTO("group2"); + GroupDTO group3 = mockGroupDTO("group3"); + Map params = new HashMap<>(); + GroupListResponse listResponse = new GroupListResponse(new GroupDTO[]{group1,group2,group3}); + buildMockResource(Method.GET,groupPath,params,null,listResponse,SC_OK); + + Group[] groups = gravitinoClient.listGroups(); + Assertions.assertEquals(3,groups.length); + assertGroup(group1,groups[0]); + assertGroup(group2,groups[1]); + assertGroup(group3,groups[2]); + ErrorResponse errResNoMetaLake = ErrorResponse.notFound(NoSuchMetalakeException.class.getSimpleName(),"metalake not found"); + buildMockResource(Method.GET,groupPath,params,null,errResNoMetaLake,SC_NOT_FOUND); + Exception ex = Assertions.assertThrows(NoSuchMetalakeException.class,()-> gravitinoClient.listGroups()); + Assertions.assertEquals("metalake not found",ex.getMessage()); + // Test RuntimeException + ErrorResponse errResp = ErrorResponse.internalError("internal error"); + buildMockResource(Method.GET,groupPath,params,null,errResp,SC_SERVER_ERROR); + Assertions.assertThrows(RuntimeException.class,()->gravitinoClient.listGroups()); + } + + private UserDTO mockUserDTO(String name) { return UserDTO.builder() .withName(name) diff --git a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java index 76f2c1b0fe8..7a0d0a3178a 100644 --- a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java +++ b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java @@ -20,8 +20,12 @@ import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import java.util.Arrays; import java.util.Collections; +import java.util.Comparator; +import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import org.apache.gravitino.Configs; import org.apache.gravitino.auth.AuthConstants; import org.apache.gravitino.authorization.Group; @@ -97,8 +101,21 @@ void testManageGroups() { // Get a not-existed group Assertions.assertThrows(NoSuchGroupException.class, () -> metalake.getGroup("not-existed")); + + // List groups + String anotherGroups = "group2#456"; + metalake.addGroup(anotherGroups); + String[] groupNames = metalake.listGroupNames(); + Arrays.sort(groupNames); + Assertions.assertEquals(Lists.newArrayList(groupName,anotherGroups),Arrays.asList(groupNames)); + + + + Assertions.assertTrue(metalake.removeGroup(groupName)); + Assertions.assertTrue(metalake.removeGroup(anotherGroups)); Assertions.assertFalse(metalake.removeGroup(groupName)); + Assertions.assertFalse(metalake.removeGroup(anotherGroups)); } @Test diff --git a/common/src/main/java/org/apache/gravitino/dto/util/DTOConverters.java b/common/src/main/java/org/apache/gravitino/dto/util/DTOConverters.java index d83460af182..5ea0740fb08 100644 --- a/common/src/main/java/org/apache/gravitino/dto/util/DTOConverters.java +++ b/common/src/main/java/org/apache/gravitino/dto/util/DTOConverters.java @@ -422,6 +422,14 @@ public static GroupDTO toDTO(Group group) { .build(); } + + public static GroupDTO[] toDTOs(Group[] groups){ + if (ArrayUtils.isEmpty(groups)){ + return new GroupDTO[0]; + } + return Arrays.stream(groups).map(DTOConverters::toDTO).toArray(GroupDTO[]::new); + } + /** * Converts a role implementation to a RoleDTO. * diff --git a/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java b/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java index e955948dfa7..66e5473b36d 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java @@ -115,7 +115,9 @@ Group getGroup(String metalake, String group) * * @return The list of groups */ - Group[] listGroup(Namespace namespace); + Group[] listGroup(String metalake); + + String[] listGroupNames(String metalake); /** * Grant roles to a user. diff --git a/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java b/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java index 38f248a8769..3f10444f24d 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java @@ -87,8 +87,13 @@ public Group getGroup(String metalake, String group) } @Override - public Group[] listGroup(Namespace namespace) { - return userGroupManager.listGroups(namespace); + public Group[] listGroup(String metalake) { + return userGroupManager.listGroups(metalake); + } + + @Override + public String[] listGroupNames(String metalake) { + return userGroupManager.listGroupNames(metalake); } @Override 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 cb52af27198..3f69ad7ee11 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java @@ -21,11 +21,14 @@ import com.google.common.collect.Lists; import java.io.IOException; import java.time.Instant; +import java.util.Arrays; import java.util.Collections; +import java.util.Set; import org.apache.gravitino.Entity; import org.apache.gravitino.Entity.EntityType; import org.apache.gravitino.EntityAlreadyExistsException; import org.apache.gravitino.EntityStore; +import org.apache.gravitino.Field; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.Namespace; import org.apache.gravitino.exceptions.GroupAlreadyExistsException; @@ -39,6 +42,7 @@ import org.apache.gravitino.meta.UserEntity; import org.apache.gravitino.storage.IdGenerator; import org.apache.gravitino.utils.PrincipalUtils; +import org.glassfish.jersey.internal.guava.Sets; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -182,9 +186,10 @@ private void checkMetalakeExists(NameIdentifier ident) throws NoSuchMetalakeExce } } - Group[] listGroups(Namespace namespace) { - NameIdentifier groupIdent = NameIdentifier.of(namespace.levels()); - checkMetalakeExists(groupIdent); + Group[] listGroups(String metalake) { + AuthorizationUtils.checkMetalakeExists(metalake); + Namespace namespace = AuthorizationUtils.ofGroupNamespace(metalake); + try { return store.list(namespace, GroupEntity.class, EntityType.GROUP).toArray(new Group[0]); } catch (Exception ioe) { @@ -192,4 +197,9 @@ Group[] listGroups(Namespace namespace) { throw new RuntimeException(ioe); } } + + String[] listGroupNames(String metalake){ + return Arrays.stream(listGroups(metalake)).map(Group::name).toArray(String[]::new); + } + } diff --git a/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java b/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java index c5bcca57ed0..95223a3ab01 100644 --- a/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java @@ -88,10 +88,16 @@ public Group getGroup(String metalake, String group) } @Override - public Group[] listGroup(Namespace namespace) { - return dispatcher.listGroup(namespace); + public Group[] listGroup(String metalake) { + return dispatcher.listGroup(metalake); } + @Override + public String[] listGroupNames(String metalake) { + return dispatcher.listGroupNames(metalake); + } + + @Override public User grantRolesToUser(String metalake, List roles, String user) throws NoSuchUserException, NoSuchRoleException, NoSuchMetalakeException { diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java b/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java index b23c7667388..3be4649793d 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java @@ -104,6 +104,8 @@ public List list( return (List) TopicMetaService.getInstance().listTopicsByNamespace(namespace); case TAG: return (List) TagMetaService.getInstance().listTagsByNamespace(namespace); + case GROUP: + return (List) GroupMetaService.getInstance().listGroupsByNamespace(namespace); default: throw new UnsupportedEntityTypeException( "Unsupported entity type: %s for list operation", entityType); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaBaseSQLProvider.java index 686bc42c565..559a41f7732 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaBaseSQLProvider.java @@ -34,6 +34,13 @@ public String selectGroupIdBySchemaIdAndName( + " AND deleted_at = 0"; } + public String listGroupsByMetalakeId(@Param("metalakeId") Long metalakeId){ + return "SELECT group_id as groupId, group_name as groupName, metalake_id as metalakeId," + + " audit_info as auditInfo, current_version as currentVersion, last_version as lastVersion," + + " deleted_at as deletedAt FROM " + GROUP_TABLE_NAME + " WHERE metalake_id = #{metalakeId}" + + " AND deleted_at = 0"; + } + public String selectGroupMetaByMetalakeIdAndName( @Param("metalakeId") Long metalakeId, @Param("groupName") String name) { return "SELECT group_id as groupId, group_name as groupName," diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaMapper.java index 5743095dd72..f8e46dfd22b 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaMapper.java @@ -20,6 +20,7 @@ package org.apache.gravitino.storage.relational.mapper; import java.util.List; +import org.apache.gravitino.authorization.Group; import org.apache.gravitino.storage.relational.po.GroupPO; import org.apache.ibatis.annotations.DeleteProvider; import org.apache.ibatis.annotations.InsertProvider; @@ -51,6 +52,12 @@ Long selectGroupIdBySchemaIdAndName( GroupPO selectGroupMetaByMetalakeIdAndName( @Param("metalakeId") Long metalakeId, @Param("groupName") String name); + @SelectProvider( + type = GroupMetaSQLProviderFactory.class, + method = "listGroupsByMetalakeId") + List selectGroupsMetaByMetalakeId(@Param("metalakeId") Long metalakeId); + + @InsertProvider(type = GroupMetaSQLProviderFactory.class, method = "insertGroupMeta") void insertGroupMeta(@Param("groupMeta") GroupPO groupPO); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java index 97cc87f975e..5bc4718d0db 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java @@ -82,6 +82,10 @@ public static String updateGroupMeta( public static String listGroupsByRoleId(@Param("roleId") Long roleId) { return getProvider().listGroupsByRoleId(roleId); } + public static String listGroupsByMetalake(@Param("metalakeId")Long metalakeId){ + + return getProvider().listGroupsByMetalakeId(metalakeId); + } public static String deleteGroupMetasByLegacyTimeline( @Param("legacyTimeline") Long legacyTimeline, @Param("limit") int limit) { diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/po/ExtendedGroupPO.java b/core/src/main/java/org/apache/gravitino/storage/relational/po/ExtendedGroupPO.java new file mode 100644 index 00000000000..01710533673 --- /dev/null +++ b/core/src/main/java/org/apache/gravitino/storage/relational/po/ExtendedGroupPO.java @@ -0,0 +1,56 @@ +/* + * 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.po; + +import java.util.Objects; + +public class ExtendedGroupPO extends GroupPO{ + + private String groupNames; + private String groupIds; + + public String getGroupNames() { + return groupNames; + } + + + + public String getGroupIds() { + return groupIds; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + + if (!(o instanceof ExtendedGroupPO)){ + return false; + } + ExtendedGroupPO that = (ExtendedGroupPO) o; + return Objects.equals(getGroupNames(), that.getGroupNames()) && Objects.equals(getGroupIds(), + that.getGroupIds()); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), getGroupIds(), getGroupNames()); + } +} 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..7c585904a4f 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 @@ -32,10 +32,12 @@ import org.apache.gravitino.Entity; import org.apache.gravitino.HasIdentifier; import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.Namespace; import org.apache.gravitino.authorization.AuthorizationUtils; import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.meta.GroupEntity; import org.apache.gravitino.meta.RoleEntity; +import org.apache.gravitino.meta.UserEntity; import org.apache.gravitino.storage.relational.mapper.GroupMetaMapper; import org.apache.gravitino.storage.relational.mapper.GroupRoleRelMapper; import org.apache.gravitino.storage.relational.mapper.OwnerMetaMapper; @@ -45,6 +47,7 @@ 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.h2.engine.Session; /** The service class for group metadata. It provides the basic database operations for group. */ public class GroupMetaService { @@ -249,6 +252,17 @@ public GroupEntity updateGroup( return newEntity; } + public List listGroupsByNamespace(Namespace namespace){ + AuthorizationUtils.checkUserNamespace(namespace); + String metalakeName = namespace.level(0); + Long metalakeId = MetalakeMetaService.getInstance().getMetalakeIdByName(metalakeName); + List groupPOS = SessionUtils.getWithoutCommit( + GroupMetaMapper.class, mapper->mapper.selectGroupsMetaByMetalakeId(metalakeId) + ); + return groupPOS.stream().map(po->POConverters.fromGroupPO(po,AuthorizationUtils.ofUserNamespace(metalakeName))).collect( + Collectors.toList()); + } + public int deleteGroupMetasByLegacyTimeline(long legacyTimeline, int limit) { int[] groupDeletedCount = new int[] {0}; int[] groupRoleRelDeletedCount = new int[] {0}; 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..03eb4f6da25 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 @@ -762,6 +762,7 @@ public static GroupEntity fromGroupPO( } } + /** * Initialize UserRoleRelPO * diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java index 74932b28d1e..82ed93ef880 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java @@ -150,12 +150,11 @@ public Response listGroups(@PathParam("metalake") String metalake) { return Utils.doAs( httpRequest, () -> { - Namespace groupNS = AuthorizationUtils.ofGroupNamespace(metalake); Group[] groups = TreeLockUtils.doWithTreeLock( NameIdentifier.of(metalake), LockType.READ, - () -> accessControlManager.listGroup(groupNS)); + () -> accessControlManager.listGroup(metalake)); GroupDTO[] groupDTOS; if (groups == null) { groupDTOS = new GroupDTO[0]; 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 b24042d2366..a9e4bd2b8eb 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 @@ -259,9 +259,8 @@ public void testListGroup() { Group group = buildGroup("group0"); Group group2 = buildGroup("group1"); Group[] groups = {group, group2}; - Namespace groupNS = AuthorizationUtils.ofGroupNamespace("metalake1"); - when(manager.listGroup(groupNS)).thenReturn(groups); + when(manager.listGroup("metalake1")).thenReturn(groups); Response resp = target("/metalakes/metalake1/groups") @@ -281,7 +280,7 @@ public void testListGroup() { } // Test to throw NoSuchMetalakeException - doThrow(new NoSuchMetalakeException("mock error")).when(manager).listGroup(groupNS); + doThrow(new NoSuchMetalakeException("mock error")).when(manager).listGroup("metalake1"); Response resp1 = target("/metalakes/metalake1/groups") .request(MediaType.APPLICATION_JSON_TYPE) @@ -295,7 +294,7 @@ public void testListGroup() { Assertions.assertEquals(NoSuchMetalakeException.class.getSimpleName(), errorResponse.getType()); // Test to throw internal RuntimeException - doThrow(new RuntimeException("mock error")).when(manager).listGroup(groupNS); + doThrow(new RuntimeException("mock error")).when(manager).listGroup("metalake1"); Response resp2 = target("/metalakes/metalake1/groups") .request(MediaType.APPLICATION_JSON_TYPE) From 361481245c5511ed55dc65928e2a09b24a9703ab Mon Sep 17 00:00:00 2001 From: qiang_liu Date: Thu, 26 Sep 2024 20:47:30 +0800 Subject: [PATCH 03/16] #4873 support list group --- .../gravitino/authorization/UserGroupManager.java | 5 ++++- .../java/org/apache/gravitino/meta/GroupEntity.java | 12 ++++++++++++ .../provider/base/GroupMetaBaseSQLProvider.java | 12 +++++++----- 3 files changed, 23 insertions(+), 6 deletions(-) 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 9646069d853..b9b26dd7a7e 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java @@ -223,8 +223,11 @@ Group[] listGroups(String metalake) { AuthorizationUtils.checkMetalakeExists(metalake); Namespace namespace = AuthorizationUtils.ofGroupNamespace(metalake); + Set skippingFields = Sets.newHashSet(); + skippingFields.add(GroupEntity.ROLE_IDS); + skippingFields.add(GroupEntity.ROLE_NAMES); try { - return store.list(namespace, GroupEntity.class, EntityType.GROUP).toArray(new Group[0]); + return store.list(namespace, GroupEntity.class, EntityType.GROUP, skippingFields).toArray(new Group[0]); } catch (Exception ioe) { LOG.error("Listing Groups failed due to storage issues.", ioe); throw new RuntimeException(ioe); 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..7cde463a8d3 100644 --- a/core/src/main/java/org/apache/gravitino/meta/GroupEntity.java +++ b/core/src/main/java/org/apache/gravitino/meta/GroupEntity.java @@ -23,12 +23,14 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import org.apache.gravitino.Auditable; import org.apache.gravitino.Entity; import org.apache.gravitino.Field; import org.apache.gravitino.HasIdentifier; import org.apache.gravitino.Namespace; import org.apache.gravitino.authorization.Group; +import org.glassfish.jersey.internal.guava.Sets; public class GroupEntity implements Group, Entity, Auditable, HasIdentifier { @@ -73,6 +75,16 @@ public Map fields() { return Collections.unmodifiableMap(fields); } + public static Set fieldSet(){ + Set fields = Sets.newHashSet(); + fields.add(ID); + fields.add(NAME); + fields.add(AUDIT_INFO); + fields.add(ROLE_NAMES); + fields.add(ROLE_IDS); + return Collections.unmodifiableSet(fields); + } + /** * Returns the name of the group. * diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java index fc3706c8c35..41a281fde0b 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java @@ -21,6 +21,7 @@ import static org.apache.gravitino.storage.relational.mapper.GroupMetaMapper.GROUP_TABLE_NAME; import static org.apache.gravitino.storage.relational.mapper.RoleMetaMapper.GROUP_ROLE_RELATION_TABLE_NAME; +import org.apache.gravitino.storage.relational.mapper.MetalakeMetaMapper; import org.apache.gravitino.storage.relational.po.GroupPO; import org.apache.ibatis.annotations.Param; @@ -34,11 +35,12 @@ public String selectGroupIdBySchemaIdAndName( + " AND deleted_at = 0"; } - public String listGroupsByMetalakeId(@Param("metalakeId") Long metalakeId){ - return "SELECT group_id as groupId, group_name as groupName, metalake_id as metalakeId," - + " audit_info as auditInfo, current_version as currentVersion, last_version as lastVersion," - + " deleted_at as deletedAt FROM " + GROUP_TABLE_NAME + " WHERE metalake_id = #{metalakeId}" - + " AND deleted_at = 0"; + public String listGroupsByMetalake(@Param("metalakeName") String metalakeName){ + return "SELECT gt.group_id as groupId, gt.group_name as groupName, gt.metalake_id as metalakeId," + + " gt.audit_info as auditInfo, gt.current_version as currentVersion, gt.last_version as lastVersion," + + " gt.deleted_at as deletedAt FROM " + GROUP_TABLE_NAME + " gt JOIN " + + MetalakeMetaMapper.TABLE_NAME + " mt ON gt.metalake_id = mt.metalakeId + WHERE mt.metalake_name = #{metalake_name}" + + " AND gt.deleted_at = 0 AND mt.deleted_at = 0"; } public String selectGroupMetaByMetalakeIdAndName( From 4e14f9dda9a43fdb8650be64ee27beacdd7367cf Mon Sep 17 00:00:00 2001 From: qiang_liu Date: Thu, 26 Sep 2024 20:56:08 +0800 Subject: [PATCH 04/16] #4873 support list group format --- .../gravitino/storage/relational/service/GroupMetaService.java | 2 +- .../apache/gravitino/server/web/rest/TestGroupOperations.java | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) 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 7c585904a4f..0b251696b72 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 @@ -259,7 +259,7 @@ public List listGroupsByNamespace(Namespace namespace){ List groupPOS = SessionUtils.getWithoutCommit( GroupMetaMapper.class, mapper->mapper.selectGroupsMetaByMetalakeId(metalakeId) ); - return groupPOS.stream().map(po->POConverters.fromGroupPO(po,AuthorizationUtils.ofUserNamespace(metalakeName))).collect( + return groupPOS.stream().map(po->POConverters.fromGroupPO(po,Collections.emptyList(),AuthorizationUtils.ofUserNamespace(metalakeName))).collect( Collectors.toList()); } 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 a9e4bd2b8eb..b3af1478ddc 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 @@ -38,9 +38,7 @@ import org.apache.commons.lang3.reflect.FieldUtils; import org.apache.gravitino.Config; import org.apache.gravitino.GravitinoEnv; -import org.apache.gravitino.Namespace; import org.apache.gravitino.authorization.AccessControlManager; -import org.apache.gravitino.authorization.AuthorizationUtils; import org.apache.gravitino.authorization.Group; import org.apache.gravitino.dto.authorization.GroupDTO; import org.apache.gravitino.dto.requests.GroupAddRequest; From a0206a9d41897d3e4991d94108f298fa744f9686 Mon Sep 17 00:00:00 2001 From: qiang_liu Date: Thu, 26 Sep 2024 20:56:48 +0800 Subject: [PATCH 05/16] #4873 support list group : format --- .../java/org/apache/gravitino/dto/util/DTOConverters.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/common/src/main/java/org/apache/gravitino/dto/util/DTOConverters.java b/common/src/main/java/org/apache/gravitino/dto/util/DTOConverters.java index 19f8aa36860..96039b0077a 100644 --- a/common/src/main/java/org/apache/gravitino/dto/util/DTOConverters.java +++ b/common/src/main/java/org/apache/gravitino/dto/util/DTOConverters.java @@ -422,9 +422,8 @@ public static GroupDTO toDTO(Group group) { .build(); } - - public static GroupDTO[] toDTOs(Group[] groups){ - if (ArrayUtils.isEmpty(groups)){ + public static GroupDTO[] toDTOs(Group[] groups) { + if (ArrayUtils.isEmpty(groups)) { return new GroupDTO[0]; } return Arrays.stream(groups).map(DTOConverters::toDTO).toArray(GroupDTO[]::new); From 95034a37ae822ea8be76fd997420e1f115356921 Mon Sep 17 00:00:00 2001 From: qiang_liu Date: Thu, 26 Sep 2024 20:57:42 +0800 Subject: [PATCH 06/16] #4873 support list group : format --- .../authorization/AccessControlDispatcher.java | 1 - .../authorization/AccessControlManager.java | 1 - .../authorization/UserGroupManager.java | 8 ++++---- .../hook/AccessControlHookDispatcher.java | 2 -- .../org/apache/gravitino/meta/GroupEntity.java | 2 +- .../relational/mapper/GroupMetaMapper.java | 6 +----- .../mapper/GroupMetaSQLProviderFactory.java | 3 ++- .../base/GroupMetaBaseSQLProvider.java | 9 ++++++--- .../storage/relational/po/ExtendedGroupPO.java | 10 ++++------ .../relational/service/GroupMetaService.java | 18 ++++++++++-------- .../storage/relational/utils/POConverters.java | 1 - 11 files changed, 28 insertions(+), 33 deletions(-) diff --git a/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java b/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java index 41ab4901c24..cda2ee04a5b 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java @@ -20,7 +20,6 @@ import java.util.List; import java.util.Map; -import org.apache.gravitino.Namespace; import org.apache.gravitino.exceptions.GroupAlreadyExistsException; import org.apache.gravitino.exceptions.NoSuchGroupException; import org.apache.gravitino.exceptions.NoSuchMetalakeException; diff --git a/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java b/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java index 81df6c5f8b5..862200c02ba 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java @@ -24,7 +24,6 @@ import org.apache.gravitino.Config; import org.apache.gravitino.Configs; import org.apache.gravitino.EntityStore; -import org.apache.gravitino.Namespace; import org.apache.gravitino.exceptions.GroupAlreadyExistsException; import org.apache.gravitino.exceptions.NoSuchGroupException; import org.apache.gravitino.exceptions.NoSuchMetalakeException; 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 337960a2402..1ded6592769 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java @@ -27,7 +27,6 @@ import org.apache.gravitino.Entity.EntityType; import org.apache.gravitino.EntityAlreadyExistsException; import org.apache.gravitino.EntityStore; -import org.apache.gravitino.Namespace; import org.apache.gravitino.Field; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.Namespace; @@ -221,15 +220,16 @@ Group[] listGroups(String metalake) { skippingFields.add(GroupEntity.ROLE_IDS); skippingFields.add(GroupEntity.ROLE_NAMES); try { - return store.list(namespace, GroupEntity.class, EntityType.GROUP, skippingFields).toArray(new Group[0]); + return store + .list(namespace, GroupEntity.class, EntityType.GROUP, skippingFields) + .toArray(new Group[0]); } catch (Exception ioe) { LOG.error("Listing Groups failed due to storage issues.", ioe); throw new RuntimeException(ioe); } } - String[] listGroupNames(String metalake){ + String[] listGroupNames(String metalake) { return Arrays.stream(listGroups(metalake)).map(Group::name).toArray(String[]::new); } - } diff --git a/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java b/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java index 4a085bbc0ef..f6c2717b3f6 100644 --- a/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java @@ -22,7 +22,6 @@ import java.util.Map; import org.apache.gravitino.Entity; import org.apache.gravitino.GravitinoEnv; -import org.apache.gravitino.Namespace; import org.apache.gravitino.authorization.AccessControlDispatcher; import org.apache.gravitino.authorization.AuthorizationUtils; import org.apache.gravitino.authorization.Group; @@ -107,7 +106,6 @@ public String[] listGroupNames(String metalake) { return dispatcher.listGroupNames(metalake); } - @Override public User grantRolesToUser(String metalake, List roles, String user) throws NoSuchUserException, NoSuchRoleException, NoSuchMetalakeException { 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 7cde463a8d3..ac26213a305 100644 --- a/core/src/main/java/org/apache/gravitino/meta/GroupEntity.java +++ b/core/src/main/java/org/apache/gravitino/meta/GroupEntity.java @@ -75,7 +75,7 @@ public Map fields() { return Collections.unmodifiableMap(fields); } - public static Set fieldSet(){ + public static Set fieldSet() { Set fields = Sets.newHashSet(); fields.add(ID); fields.add(NAME); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaMapper.java index f8e46dfd22b..a7b50bfbd43 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaMapper.java @@ -20,7 +20,6 @@ package org.apache.gravitino.storage.relational.mapper; import java.util.List; -import org.apache.gravitino.authorization.Group; import org.apache.gravitino.storage.relational.po.GroupPO; import org.apache.ibatis.annotations.DeleteProvider; import org.apache.ibatis.annotations.InsertProvider; @@ -52,12 +51,9 @@ Long selectGroupIdBySchemaIdAndName( GroupPO selectGroupMetaByMetalakeIdAndName( @Param("metalakeId") Long metalakeId, @Param("groupName") String name); - @SelectProvider( - type = GroupMetaSQLProviderFactory.class, - method = "listGroupsByMetalakeId") + @SelectProvider(type = GroupMetaSQLProviderFactory.class, method = "listGroupsByMetalakeId") List selectGroupsMetaByMetalakeId(@Param("metalakeId") Long metalakeId); - @InsertProvider(type = GroupMetaSQLProviderFactory.class, method = "insertGroupMeta") void insertGroupMeta(@Param("groupMeta") GroupPO groupPO); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java index 2f061863388..5aa0d92da23 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java @@ -83,7 +83,8 @@ public static String updateGroupMeta( public static String listGroupsByRoleId(@Param("roleId") Long roleId) { return getProvider().listGroupsByRoleId(roleId); } - public static String listGroupsByMetalake(@Param("metalakeId")Long metalakeId){ + + public static String listGroupsByMetalake(@Param("metalakeId") Long metalakeId) { return getProvider().listGroupsByMetalakeId(metalakeId); } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java index 41a281fde0b..47c3ac3a896 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java @@ -35,11 +35,14 @@ public String selectGroupIdBySchemaIdAndName( + " AND deleted_at = 0"; } - public String listGroupsByMetalake(@Param("metalakeName") String metalakeName){ + public String listGroupsByMetalake(@Param("metalakeName") String metalakeName) { return "SELECT gt.group_id as groupId, gt.group_name as groupName, gt.metalake_id as metalakeId," + " gt.audit_info as auditInfo, gt.current_version as currentVersion, gt.last_version as lastVersion," - + " gt.deleted_at as deletedAt FROM " + GROUP_TABLE_NAME + " gt JOIN " + - MetalakeMetaMapper.TABLE_NAME + " mt ON gt.metalake_id = mt.metalakeId + WHERE mt.metalake_name = #{metalake_name}" + + " gt.deleted_at as deletedAt FROM " + + GROUP_TABLE_NAME + + " gt JOIN " + + MetalakeMetaMapper.TABLE_NAME + + " mt ON gt.metalake_id = mt.metalakeId + WHERE mt.metalake_name = #{metalake_name}" + " AND gt.deleted_at = 0 AND mt.deleted_at = 0"; } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/po/ExtendedGroupPO.java b/core/src/main/java/org/apache/gravitino/storage/relational/po/ExtendedGroupPO.java index 01710533673..b331fb6b3c5 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/po/ExtendedGroupPO.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/po/ExtendedGroupPO.java @@ -20,7 +20,7 @@ import java.util.Objects; -public class ExtendedGroupPO extends GroupPO{ +public class ExtendedGroupPO extends GroupPO { private String groupNames; private String groupIds; @@ -29,8 +29,6 @@ public String getGroupNames() { return groupNames; } - - public String getGroupIds() { return groupIds; } @@ -41,12 +39,12 @@ public boolean equals(Object o) { return true; } - if (!(o instanceof ExtendedGroupPO)){ + if (!(o instanceof ExtendedGroupPO)) { return false; } ExtendedGroupPO that = (ExtendedGroupPO) o; - return Objects.equals(getGroupNames(), that.getGroupNames()) && Objects.equals(getGroupIds(), - that.getGroupIds()); + return Objects.equals(getGroupNames(), that.getGroupNames()) + && Objects.equals(getGroupIds(), that.getGroupIds()); } @Override 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 0b251696b72..05ebf0e0073 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 @@ -37,7 +37,6 @@ import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.meta.GroupEntity; import org.apache.gravitino.meta.RoleEntity; -import org.apache.gravitino.meta.UserEntity; import org.apache.gravitino.storage.relational.mapper.GroupMetaMapper; import org.apache.gravitino.storage.relational.mapper.GroupRoleRelMapper; import org.apache.gravitino.storage.relational.mapper.OwnerMetaMapper; @@ -47,7 +46,6 @@ 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.h2.engine.Session; /** The service class for group metadata. It provides the basic database operations for group. */ public class GroupMetaService { @@ -252,15 +250,19 @@ public GroupEntity updateGroup( return newEntity; } - public List listGroupsByNamespace(Namespace namespace){ + public List listGroupsByNamespace(Namespace namespace) { AuthorizationUtils.checkUserNamespace(namespace); String metalakeName = namespace.level(0); Long metalakeId = MetalakeMetaService.getInstance().getMetalakeIdByName(metalakeName); - List groupPOS = SessionUtils.getWithoutCommit( - GroupMetaMapper.class, mapper->mapper.selectGroupsMetaByMetalakeId(metalakeId) - ); - return groupPOS.stream().map(po->POConverters.fromGroupPO(po,Collections.emptyList(),AuthorizationUtils.ofUserNamespace(metalakeName))).collect( - Collectors.toList()); + List groupPOS = + SessionUtils.getWithoutCommit( + GroupMetaMapper.class, mapper -> mapper.selectGroupsMetaByMetalakeId(metalakeId)); + return groupPOS.stream() + .map( + po -> + POConverters.fromGroupPO( + po, Collections.emptyList(), AuthorizationUtils.ofUserNamespace(metalakeName))) + .collect(Collectors.toList()); } public int deleteGroupMetasByLegacyTimeline(long legacyTimeline, int limit) { 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 c2b97a9154a..da1f3d06a3b 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 @@ -814,7 +814,6 @@ public static GroupEntity fromGroupPO( } } - /** * Initialize UserRoleRelPO * From ed9cc9ddd6b989dab5dbfdb6ae80611ba719ec19 Mon Sep 17 00:00:00 2001 From: qiang_liu Date: Thu, 26 Sep 2024 21:13:59 +0800 Subject: [PATCH 07/16] #4873 support list group --- .../gravitino/client/GravitinoMetalake.java | 4 ++-- .../authorization/UserGroupManager.java | 24 +++++++++++-------- .../mapper/GroupMetaSQLProviderFactory.java | 5 ++-- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java index 9aa4b2b4195..5740367a6ca 100644 --- a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java +++ b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java @@ -634,7 +634,7 @@ public Group[] listGroups() { Map params = new HashMap<>(); GroupListResponse resp = restClient.get( - String.format(API_METALAKES_GROUPS_PATH,name(),BLANK_PLACE_HOLDER), + String.format(API_METALAKES_GROUPS_PATH,name(),BLANK_PLACEHOLDER), params, GroupListResponse.class, Collections.emptyMap(), @@ -652,7 +652,7 @@ public Group[] listGroups() { public String[] listGroupNames(){ NameListResponse resp = restClient.get( String.format(API_METALAKES_GROUPS_PATH,name(), - BLANK_PLACE_HOLDER), + BLANK_PLACEHOLDER), NameListResponse.class, Collections.emptyMap(), ErrorHandlers.groupErrorHandler() 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 1ded6592769..e9e4e555d17 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java @@ -27,7 +27,6 @@ import org.apache.gravitino.Entity.EntityType; import org.apache.gravitino.EntityAlreadyExistsException; import org.apache.gravitino.EntityStore; -import org.apache.gravitino.Field; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.Namespace; import org.apache.gravitino.exceptions.GroupAlreadyExistsException; @@ -213,23 +212,28 @@ private void checkMetalakeExists(NameIdentifier ident) throws NoSuchMetalakeExce } Group[] listGroups(String metalake) { - AuthorizationUtils.checkMetalakeExists(metalake); - Namespace namespace = AuthorizationUtils.ofGroupNamespace(metalake); + return listGroupInternal(metalake, true); + } - Set skippingFields = Sets.newHashSet(); - skippingFields.add(GroupEntity.ROLE_IDS); - skippingFields.add(GroupEntity.ROLE_NAMES); + private Group[] listGroupInternal(String metalake, boolean allFields) { try { + AuthorizationUtils.checkMetalakeExists(metalake); + Namespace namespace = AuthorizationUtils.ofUserNamespace(metalake); return store - .list(namespace, GroupEntity.class, EntityType.GROUP, skippingFields) + .list(namespace, GroupEntity.class, EntityType.GROUP, allFields) .toArray(new Group[0]); - } catch (Exception ioe) { - LOG.error("Listing Groups failed due to storage issues.", ioe); + } catch (NoSuchEntityException e) { + LOG.error("Metalake {} does not exist", metalake, e); + throw new NoSuchMetalakeException(METALAKE_DOES_NOT_EXIST_MSG, metalake); + } catch (IOException ioe) { + LOG.error("Listing group under metalake {} failed due to storage issues", metalake, ioe); throw new RuntimeException(ioe); } } String[] listGroupNames(String metalake) { - return Arrays.stream(listGroups(metalake)).map(Group::name).toArray(String[]::new); + return Arrays.stream(listGroupInternal(metalake, false)) + .map(Group::name) + .toArray(String[]::new); } } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java index 5aa0d92da23..72da18265c4 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java @@ -84,9 +84,8 @@ public static String listGroupsByRoleId(@Param("roleId") Long roleId) { return getProvider().listGroupsByRoleId(roleId); } - public static String listGroupsByMetalake(@Param("metalakeId") Long metalakeId) { - - return getProvider().listGroupsByMetalakeId(metalakeId); + public static String listGroupsByMetalake(@Param("metalakeName") String metalakeName) { + return getProvider().listGroupsByMetalake(metalakeName); } public static String deleteGroupMetasByLegacyTimeline( From 2e24f4bea5af6ed0436b71682f820cba2009bd38 Mon Sep 17 00:00:00 2001 From: qiang_liu Date: Thu, 26 Sep 2024 21:22:20 +0800 Subject: [PATCH 08/16] #4873 support list group add javaDoc --- .../org/apache/gravitino/client/GravitinoClient.java | 10 ++++++++++ .../org/apache/gravitino/client/GravitinoMetalake.java | 2 ++ .../authorization/AccessControlDispatcher.java | 9 +++++++++ 3 files changed, 21 insertions(+) diff --git a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java index d79deaaeefd..5bcdccdbc8f 100644 --- a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java +++ b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java @@ -228,10 +228,20 @@ public Group getGroup(String group) throws NoSuchGroupException, NoSuchMetalakeE } + /** + * List the groups. + * @return The Group list + * @throws NoSuchMetalakeException If the Metalake with the given name does not exist. + */ public Group[] listGroups(){ return getMetalake().listGroups(); } + /** + * List the group names. + * @return The group names list. + * @throws NoSuchMetalakeException If the Metalake with the given name does not exist. + */ public String[] listGroupNames(){ return getMetalake().listGroupNames(); } diff --git a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java index 5740367a6ca..5efbd7532f7 100644 --- a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java +++ b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java @@ -629,6 +629,7 @@ public Group getGroup(String group) throws NoSuchGroupException, NoSuchMetalakeE * Lists the groups * * @return The Group list + * @throws NoSuchMetalakeException If the Metalake with the given name does not exist. */ public Group[] listGroups() { Map params = new HashMap<>(); @@ -648,6 +649,7 @@ public Group[] listGroups() { * Lists the group names * * @return The Group Name List + * @throws NoSuchMetalakeException If the Metalake with the given name does not exist. */ public String[] listGroupNames(){ NameListResponse resp = restClient.get( diff --git a/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java b/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java index cda2ee04a5b..4e9da93a41c 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java @@ -130,10 +130,19 @@ Group getGroup(String metalake, String group) /** * List groups * + * @param metalake The Metalake of the Group. * @return The list of groups + * @throws NoSuchMetalakeException If the Metalake with the given name does not exist. */ Group[] listGroup(String metalake); + /** + * List group names + * + * @param metalake The Metalake of the Group. + * @return The list of group names + * @throws NoSuchMetalakeException If the Metalake with the given name does not exist. + */ String[] listGroupNames(String metalake); /** From 81a8d83825b345066bed0a051df0ad8df20f26aa Mon Sep 17 00:00:00 2001 From: Rory Date: Fri, 27 Sep 2024 13:57:44 +0800 Subject: [PATCH 09/16] Align to list users --- .../gravitino/client/GravitinoClient.java | 7 +- .../gravitino/client/GravitinoMetalake.java | 27 +++-- .../gravitino/client/TestUserGroup.java | 66 +++++------ .../test/authorization/AccessControlIT.java | 31 +++-- .../AccessControlDispatcher.java | 2 +- .../authorization/AccessControlManager.java | 4 +- .../authorization/UserGroupManager.java | 12 -- .../hook/AccessControlHookDispatcher.java | 6 +- .../apache/gravitino/meta/GroupEntity.java | 12 -- .../storage/relational/JDBCBackend.java | 2 +- .../relational/mapper/GroupMetaMapper.java | 10 +- .../mapper/GroupMetaSQLProviderFactory.java | 11 +- .../base/GroupMetaBaseSQLProvider.java | 28 ++++- .../provider/h2/GroupMetaH2Provider.java | 52 +++++++++ .../GroupMetaPostgreSQLProvider.java | 26 +++++ .../relational/po/ExtendedGroupPO.java | 23 ++-- .../relational/service/GroupMetaService.java | 40 +++++-- .../relational/utils/POConverters.java | 54 ++++++++- .../TestAccessControlManager.java | 22 ++++ .../service/TestGroupMetaService.java | 72 ++++++++++++ .../server/web/rest/GroupOperations.java | 26 ++--- .../server/web/rest/TestGroupOperations.java | 108 +++++++++++++----- 22 files changed, 477 insertions(+), 164 deletions(-) create mode 100644 core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/h2/GroupMetaH2Provider.java diff --git a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java index 5bcdccdbc8f..a8a46ff8f47 100644 --- a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java +++ b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java @@ -227,22 +227,23 @@ public Group getGroup(String group) throws NoSuchGroupException, NoSuchMetalakeE return getMetalake().getGroup(group); } - /** * List the groups. + * * @return The Group list * @throws NoSuchMetalakeException If the Metalake with the given name does not exist. */ - public Group[] listGroups(){ + public Group[] listGroups() throws NoSuchMetalakeException { return getMetalake().listGroups(); } /** * List the group names. + * * @return The group names list. * @throws NoSuchMetalakeException If the Metalake with the given name does not exist. */ - public String[] listGroupNames(){ + public String[] listGroupNames() throws NoSuchMetalakeException { return getMetalake().listGroupNames(); } diff --git a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java index 5efbd7532f7..3354bf2a6c6 100644 --- a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java +++ b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java @@ -624,23 +624,23 @@ public Group getGroup(String group) throws NoSuchGroupException, NoSuchMetalakeE return resp.getGroup(); } - /** * Lists the groups * * @return The Group list * @throws NoSuchMetalakeException If the Metalake with the given name does not exist. */ - public Group[] listGroups() { - Map params = new HashMap<>(); + public Group[] listGroups() throws NoSuchMetalakeException { + Map params = new HashMap<>(); + params.put("details", "true"); + GroupListResponse resp = restClient.get( - String.format(API_METALAKES_GROUPS_PATH,name(),BLANK_PLACEHOLDER), + String.format(API_METALAKES_GROUPS_PATH, name(), BLANK_PLACEHOLDER), params, GroupListResponse.class, Collections.emptyMap(), - ErrorHandlers.groupErrorHandler() - ); + ErrorHandlers.groupErrorHandler()); resp.validate(); return resp.getGroups(); } @@ -651,14 +651,13 @@ public Group[] listGroups() { * @return The Group Name List * @throws NoSuchMetalakeException If the Metalake with the given name does not exist. */ - public String[] listGroupNames(){ - NameListResponse resp = restClient.get( - String.format(API_METALAKES_GROUPS_PATH,name(), - BLANK_PLACEHOLDER), - NameListResponse.class, - Collections.emptyMap(), - ErrorHandlers.groupErrorHandler() - ); + public String[] listGroupNames() throws NoSuchMetalakeException { + NameListResponse resp = + restClient.get( + String.format(API_METALAKES_GROUPS_PATH, name(), BLANK_PLACEHOLDER), + NameListResponse.class, + Collections.emptyMap(), + ErrorHandlers.groupErrorHandler()); resp.validate(); return resp.getNames(); } diff --git a/clients/client-java/src/test/java/org/apache/gravitino/client/TestUserGroup.java b/clients/client-java/src/test/java/org/apache/gravitino/client/TestUserGroup.java index ccf8dcbada5..354419cb475 100644 --- a/clients/client-java/src/test/java/org/apache/gravitino/client/TestUserGroup.java +++ b/clients/client-java/src/test/java/org/apache/gravitino/client/TestUserGroup.java @@ -28,8 +28,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; -import java.util.Collections; -import java.util.Map; import org.apache.gravitino.authorization.Group; import org.apache.gravitino.authorization.User; import org.apache.gravitino.dto.AuditDTO; @@ -51,7 +49,6 @@ import org.apache.gravitino.exceptions.NoSuchMetalakeException; import org.apache.gravitino.exceptions.NoSuchUserException; import org.apache.gravitino.exceptions.UserAlreadyExistsException; -import org.apache.hadoop.security.Groups; import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.http.Method; import org.junit.jupiter.api.Assertions; @@ -333,55 +330,58 @@ public void testRemoveGroups() throws Exception { Assertions.assertThrows(RuntimeException.class, () -> gravitinoClient.removeGroup(groupName)); } - @Test public void testListGroupNames() throws JsonProcessingException { - String groupPath = withSlash(String.format(API_METALAKES_GROUPS_PATH,metalakeName,"")); - NameListResponse listResponse = new NameListResponse(new String[] {"group1","group2"}); - buildMockResource(Method.GET,groupPath,null,listResponse,SC_OK); - Assertions.assertArrayEquals(new String[]{"group1","group2"},gravitinoClient.listGroupNames()); - ErrorResponse errRespNoMetaLake = ErrorResponse.notFound(NoSuchMetalakeException.class.getSimpleName(),"metalake not found"); - buildMockResource(Method.GET,groupPath,null,errRespNoMetaLake,SC_NOT_FOUND); - Exception ex = Assertions.assertThrows( - NoSuchMetalakeException.class,()->{ - gravitinoClient.listGroupNames(); - } - ); - Assertions.assertEquals("metalake not found",ex.getMessage()); + String groupPath = withSlash(String.format(API_METALAKES_GROUPS_PATH, metalakeName, "")); + NameListResponse listResponse = new NameListResponse(new String[] {"group1", "group2"}); + buildMockResource(Method.GET, groupPath, null, listResponse, SC_OK); + Assertions.assertArrayEquals( + new String[] {"group1", "group2"}, gravitinoClient.listGroupNames()); + ErrorResponse errRespNoMetaLake = + ErrorResponse.notFound(NoSuchMetalakeException.class.getSimpleName(), "metalake not found"); + buildMockResource(Method.GET, groupPath, null, errRespNoMetaLake, SC_NOT_FOUND); + Exception ex = + Assertions.assertThrows( + NoSuchMetalakeException.class, + () -> { + gravitinoClient.listGroupNames(); + }); + Assertions.assertEquals("metalake not found", ex.getMessage()); // Test RuntimeException ErrorResponse errResp = ErrorResponse.internalError("internal error"); - buildMockResource(Method.GET,groupPath,null,errResp,SC_SERVER_ERROR); + buildMockResource(Method.GET, groupPath, null, errResp, SC_SERVER_ERROR); - Assertions.assertThrows(RuntimeException.class,()->gravitinoClient.listGroupNames()); + Assertions.assertThrows(RuntimeException.class, () -> gravitinoClient.listGroupNames()); } @Test public void testListGroups() throws JsonProcessingException { - String groupPath = withSlash(String.format(API_METALAKES_GROUPS_PATH,metalakeName,"")); + String groupPath = withSlash(String.format(API_METALAKES_GROUPS_PATH, metalakeName, "")); GroupDTO group1 = mockGroupDTO("group1"); GroupDTO group2 = mockGroupDTO("group2"); GroupDTO group3 = mockGroupDTO("group3"); - Map params = new HashMap<>(); - GroupListResponse listResponse = new GroupListResponse(new GroupDTO[]{group1,group2,group3}); - buildMockResource(Method.GET,groupPath,params,null,listResponse,SC_OK); + Map params = new HashMap<>(); + GroupListResponse listResponse = new GroupListResponse(new GroupDTO[] {group1, group2, group3}); + buildMockResource(Method.GET, groupPath, params, null, listResponse, SC_OK); Group[] groups = gravitinoClient.listGroups(); - Assertions.assertEquals(3,groups.length); - assertGroup(group1,groups[0]); - assertGroup(group2,groups[1]); - assertGroup(group3,groups[2]); - ErrorResponse errResNoMetaLake = ErrorResponse.notFound(NoSuchMetalakeException.class.getSimpleName(),"metalake not found"); - buildMockResource(Method.GET,groupPath,params,null,errResNoMetaLake,SC_NOT_FOUND); - Exception ex = Assertions.assertThrows(NoSuchMetalakeException.class,()-> gravitinoClient.listGroups()); - Assertions.assertEquals("metalake not found",ex.getMessage()); + Assertions.assertEquals(3, groups.length); + assertGroup(group1, groups[0]); + assertGroup(group2, groups[1]); + assertGroup(group3, groups[2]); + ErrorResponse errResNoMetaLake = + ErrorResponse.notFound(NoSuchMetalakeException.class.getSimpleName(), "metalake not found"); + buildMockResource(Method.GET, groupPath, params, null, errResNoMetaLake, SC_NOT_FOUND); + Exception ex = + Assertions.assertThrows(NoSuchMetalakeException.class, () -> gravitinoClient.listGroups()); + Assertions.assertEquals("metalake not found", ex.getMessage()); // Test RuntimeException ErrorResponse errResp = ErrorResponse.internalError("internal error"); - buildMockResource(Method.GET,groupPath,params,null,errResp,SC_SERVER_ERROR); - Assertions.assertThrows(RuntimeException.class,()->gravitinoClient.listGroups()); + buildMockResource(Method.GET, groupPath, params, null, errResp, SC_SERVER_ERROR); + Assertions.assertThrows(RuntimeException.class, () -> gravitinoClient.listGroups()); } - private UserDTO mockUserDTO(String name) { return UserDTO.builder() .withName(name) diff --git a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java index 8da88464e19..742eae25981 100644 --- a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java +++ b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java @@ -133,21 +133,38 @@ void testManageGroups() { // Get a not-existed group Assertions.assertThrows(NoSuchGroupException.class, () -> metalake.getGroup("not-existed")); + Map properties = Maps.newHashMap(); + properties.put("k1", "v1"); + SecurableObject metalakeObject = + SecurableObjects.ofMetalake( + metalakeName, Lists.newArrayList(Privileges.CreateCatalog.allow())); + + // Test the group with the role + metalake.createRole("role2", properties, Lists.newArrayList(metalakeObject)); + metalake.grantRolesToGroup(Lists.newArrayList("role2"), groupName); // List groups - String anotherGroups = "group2#456"; - metalake.addGroup(anotherGroups); + String anotherGroup = "group2#456"; + metalake.addGroup(anotherGroup); String[] groupNames = metalake.listGroupNames(); Arrays.sort(groupNames); - Assertions.assertEquals(Lists.newArrayList(groupName,anotherGroups),Arrays.asList(groupNames)); - - + Assertions.assertEquals(Lists.newArrayList(groupName, anotherGroup), Arrays.asList(groupNames)); + List groups = + Arrays.stream(metalake.listGroups()) + .sorted(Comparator.comparing(Group::name)) + .collect(Collectors.toList()); + Assertions.assertEquals( + Lists.newArrayList(groupName, anotherGroup), + groups.stream().map(Group::name).collect(Collectors.toList())); + Assertions.assertEquals(Lists.newArrayList("role2"), groups.get(0).roles()); Assertions.assertTrue(metalake.removeGroup(groupName)); - Assertions.assertTrue(metalake.removeGroup(anotherGroups)); Assertions.assertFalse(metalake.removeGroup(groupName)); - Assertions.assertFalse(metalake.removeGroup(anotherGroups)); + + // clean up + metalake.removeGroup(anotherGroup); + metalake.deleteRole("role2"); } @Test diff --git a/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java b/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java index 4e9da93a41c..0dc65909a5c 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java @@ -134,7 +134,7 @@ Group getGroup(String metalake, String group) * @return The list of groups * @throws NoSuchMetalakeException If the Metalake with the given name does not exist. */ - Group[] listGroup(String metalake); + Group[] listGroups(String metalake); /** * List group names diff --git a/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java b/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java index 862200c02ba..067d3845613 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java @@ -95,12 +95,12 @@ public Group getGroup(String metalake, String group) } @Override - public Group[] listGroup(String metalake) { + public Group[] listGroups(String metalake) throws NoSuchMetalakeException { return userGroupManager.listGroups(metalake); } @Override - public String[] listGroupNames(String metalake) { + public String[] listGroupNames(String metalake) throws NoSuchMetalakeException { return userGroupManager.listGroupNames(metalake); } 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 e9e4e555d17..65043732a87 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java @@ -27,7 +27,6 @@ import org.apache.gravitino.Entity.EntityType; import org.apache.gravitino.EntityAlreadyExistsException; import org.apache.gravitino.EntityStore; -import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.Namespace; import org.apache.gravitino.exceptions.GroupAlreadyExistsException; import org.apache.gravitino.exceptions.NoSuchEntityException; @@ -200,17 +199,6 @@ Group getGroup(String metalake, String group) { } } - private void checkMetalakeExists(NameIdentifier ident) throws NoSuchMetalakeException { - try { - if (!store.exists(ident, EntityType.METALAKE)) { - throw new NoSuchMetalakeException(METALAKE_DOES_NOT_EXIST_MSG, ident); - } - } catch (IOException e) { - LOG.error("Failed to do storage operation", e); - throw new RuntimeException(e); - } - } - Group[] listGroups(String metalake) { return listGroupInternal(metalake, true); } diff --git a/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java b/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java index f6c2717b3f6..a057af86691 100644 --- a/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java @@ -97,12 +97,12 @@ public Group getGroup(String metalake, String group) } @Override - public Group[] listGroup(String metalake) { - return dispatcher.listGroup(metalake); + public Group[] listGroups(String metalake) throws NoSuchMetalakeException { + return dispatcher.listGroups(metalake); } @Override - public String[] listGroupNames(String metalake) { + public String[] listGroupNames(String metalake) throws NoSuchMetalakeException { return dispatcher.listGroupNames(metalake); } 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 ac26213a305..d9e4b633d54 100644 --- a/core/src/main/java/org/apache/gravitino/meta/GroupEntity.java +++ b/core/src/main/java/org/apache/gravitino/meta/GroupEntity.java @@ -23,14 +23,12 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Set; import org.apache.gravitino.Auditable; import org.apache.gravitino.Entity; import org.apache.gravitino.Field; import org.apache.gravitino.HasIdentifier; import org.apache.gravitino.Namespace; import org.apache.gravitino.authorization.Group; -import org.glassfish.jersey.internal.guava.Sets; public class GroupEntity implements Group, Entity, Auditable, HasIdentifier { @@ -75,16 +73,6 @@ public Map fields() { return Collections.unmodifiableMap(fields); } - public static Set fieldSet() { - Set fields = Sets.newHashSet(); - fields.add(ID); - fields.add(NAME); - fields.add(AUDIT_INFO); - fields.add(ROLE_NAMES); - fields.add(ROLE_IDS); - return Collections.unmodifiableSet(fields); - } - /** * Returns the name of the group. * diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java b/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java index 7daaf00bf0d..fb1084d8e48 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java @@ -109,7 +109,7 @@ public List list( case ROLE: return (List) RoleMetaService.getInstance().listRolesByNamespace(namespace); case GROUP: - return (List) GroupMetaService.getInstance().listGroupsByNamespace(namespace); + return (List) GroupMetaService.getInstance().listGroupsByNamespace(namespace, allFields); default: throw new UnsupportedEntityTypeException( "Unsupported entity type: %s for list operation", entityType); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaMapper.java index a7b50bfbd43..ae554a2a436 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaMapper.java @@ -20,6 +20,7 @@ package org.apache.gravitino.storage.relational.mapper; import java.util.List; +import org.apache.gravitino.storage.relational.po.ExtendedGroupPO; import org.apache.gravitino.storage.relational.po.GroupPO; import org.apache.ibatis.annotations.DeleteProvider; import org.apache.ibatis.annotations.InsertProvider; @@ -51,8 +52,13 @@ Long selectGroupIdBySchemaIdAndName( GroupPO selectGroupMetaByMetalakeIdAndName( @Param("metalakeId") Long metalakeId, @Param("groupName") String name); - @SelectProvider(type = GroupMetaSQLProviderFactory.class, method = "listGroupsByMetalakeId") - List selectGroupsMetaByMetalakeId(@Param("metalakeId") Long metalakeId); + @SelectProvider(type = GroupMetaSQLProviderFactory.class, method = "listGroupPOsByMetalake") + List listGroupPOsByMetalake(@Param("metalakeName") String metalakeName); + + @SelectProvider( + type = GroupMetaSQLProviderFactory.class, + method = "listExtendedGroupPOsByMetalakeId") + List listExtendedGroupPOsByMetalakeId(Long metalakeId); @InsertProvider(type = GroupMetaSQLProviderFactory.class, method = "insertGroupMeta") void insertGroupMeta(@Param("groupMeta") GroupPO groupPO); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java index 72da18265c4..591ac2e9a1c 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java @@ -22,6 +22,7 @@ import java.util.Map; import org.apache.gravitino.storage.relational.JDBCBackend.JDBCBackendType; import org.apache.gravitino.storage.relational.mapper.provider.base.GroupMetaBaseSQLProvider; +import org.apache.gravitino.storage.relational.mapper.provider.h2.GroupMetaH2Provider; import org.apache.gravitino.storage.relational.mapper.provider.postgresql.GroupMetaPostgreSQLProvider; import org.apache.gravitino.storage.relational.po.GroupPO; import org.apache.gravitino.storage.relational.session.SqlSessionFactoryHelper; @@ -47,8 +48,6 @@ public static GroupMetaBaseSQLProvider getProvider() { static class GroupMetaMySQLProvider extends GroupMetaBaseSQLProvider {} - static class GroupMetaH2Provider extends GroupMetaBaseSQLProvider {} - public static String selectGroupIdBySchemaIdAndName( @Param("metalakeId") Long metalakeId, @Param("groupName") String name) { return getProvider().selectGroupIdBySchemaIdAndName(metalakeId, name); @@ -84,8 +83,12 @@ public static String listGroupsByRoleId(@Param("roleId") Long roleId) { return getProvider().listGroupsByRoleId(roleId); } - public static String listGroupsByMetalake(@Param("metalakeName") String metalakeName) { - return getProvider().listGroupsByMetalake(metalakeName); + public static String listGroupPOsByMetalake(@Param("metalakeName") String metalakeName) { + return getProvider().listGroupPOsByMetalake(metalakeName); + } + + public static String listExtendedGroupPOsByMetalakeId(@Param("metalakeId") Long metalakeId) { + return getProvider().listExtendedGroupPOsByMetalakeId(metalakeId); } public static String deleteGroupMetasByLegacyTimeline( diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java index 47c3ac3a896..2e4cd25954f 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java @@ -20,6 +20,7 @@ import static org.apache.gravitino.storage.relational.mapper.GroupMetaMapper.GROUP_TABLE_NAME; import static org.apache.gravitino.storage.relational.mapper.RoleMetaMapper.GROUP_ROLE_RELATION_TABLE_NAME; +import static org.apache.gravitino.storage.relational.mapper.RoleMetaMapper.ROLE_TABLE_NAME; import org.apache.gravitino.storage.relational.mapper.MetalakeMetaMapper; import org.apache.gravitino.storage.relational.po.GroupPO; @@ -35,17 +36,40 @@ public String selectGroupIdBySchemaIdAndName( + " AND deleted_at = 0"; } - public String listGroupsByMetalake(@Param("metalakeName") String metalakeName) { + public String listGroupPOsByMetalake(@Param("metalakeName") String metalakeName) { return "SELECT gt.group_id as groupId, gt.group_name as groupName, gt.metalake_id as metalakeId," + " gt.audit_info as auditInfo, gt.current_version as currentVersion, gt.last_version as lastVersion," + " gt.deleted_at as deletedAt FROM " + GROUP_TABLE_NAME + " gt JOIN " + MetalakeMetaMapper.TABLE_NAME - + " mt ON gt.metalake_id = mt.metalakeId + WHERE mt.metalake_name = #{metalake_name}" + + " mt ON gt.metalake_id = mt.metalake_id WHERE mt.metalake_name = #{metalakeName}" + " AND gt.deleted_at = 0 AND mt.deleted_at = 0"; } + public String listExtendedGroupPOsByMetalakeId(Long metalakeId) { + return "SELECT gt.group_id as groupId, gt.group_name as groupName," + + " gt.metalake_id as metalakeId," + + " gt.audit_info as auditInfo," + + " gt.current_version as currentVersion, gt.last_version as lastVersion," + + " gt.deleted_at as deletedAt," + + " JSON_ARRAYAGG(rot.role_name) as roleNames," + + " JSON_ARRAYAGG(rot.role_id) as roleIds" + + " FROM " + + GROUP_TABLE_NAME + + " gt LEFT OUTER JOIN " + + GROUP_ROLE_RELATION_TABLE_NAME + + " rt ON rt.group_id = gt.group_id" + + " LEFT OUTER JOIN " + + ROLE_TABLE_NAME + + " rot ON rot.role_id = rt.role_id" + + " WHERE " + + " gt.deleted_at = 0 AND" + + " (rot.deleted_at = 0 OR rot.deleted_at is NULL) AND" + + " (rt.deleted_at = 0 OR rt.deleted_at is NULL) AND gt.metalake_id = #{metalakeId}" + + " GROUP BY gt.group_id"; + } + public String selectGroupMetaByMetalakeIdAndName( @Param("metalakeId") Long metalakeId, @Param("groupName") String name) { return "SELECT group_id as groupId, group_name as groupName," diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/h2/GroupMetaH2Provider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/h2/GroupMetaH2Provider.java new file mode 100644 index 00000000000..175d9d8ae9a --- /dev/null +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/h2/GroupMetaH2Provider.java @@ -0,0 +1,52 @@ +/* + * 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.mapper.provider.h2; + +import static org.apache.gravitino.storage.relational.mapper.GroupMetaMapper.GROUP_TABLE_NAME; +import static org.apache.gravitino.storage.relational.mapper.RoleMetaMapper.GROUP_ROLE_RELATION_TABLE_NAME; +import static org.apache.gravitino.storage.relational.mapper.RoleMetaMapper.ROLE_TABLE_NAME; + +import org.apache.gravitino.storage.relational.mapper.provider.base.GroupMetaBaseSQLProvider; +import org.apache.ibatis.annotations.Param; + +public class GroupMetaH2Provider extends GroupMetaBaseSQLProvider { + @Override + public String listExtendedGroupPOsByMetalakeId(@Param("metalakeId") Long metalakeId) { + return "SELECT gt.group_id as groupId, gt.group_name as groupName," + + " gt.metalake_id as metalakeId," + + " gt.audit_info as auditInfo," + + " gt.current_version as currentVersion, gt.last_version as lastVersion," + + " gt.deleted_at as deletedAt," + + " '[' || GROUP_CONCAT('\"' || rot.role_name || '\"') || ']' as roleNames," + + " '[' || GROUP_CONCAT('\"' || rot.role_id || '\"') || ']' as roleIds" + + " FROM " + + GROUP_TABLE_NAME + + " gt LEFT OUTER JOIN " + + GROUP_ROLE_RELATION_TABLE_NAME + + " rt ON rt.group_id = gt.group_id" + + " LEFT OUTER JOIN " + + ROLE_TABLE_NAME + + " rot ON rot.role_id = rt.role_id" + + " WHERE " + + " gt.deleted_at = 0 AND" + + " (rot.deleted_at = 0 OR rot.deleted_at is NULL) AND" + + " (rt.deleted_at = 0 OR rt.deleted_at is NULL) AND gt.metalake_id = #{metalakeId}" + + " GROUP BY gt.group_id"; + } +} diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/GroupMetaPostgreSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/GroupMetaPostgreSQLProvider.java index 4dddcad42be..51cf47bf7d7 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/GroupMetaPostgreSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/GroupMetaPostgreSQLProvider.java @@ -19,6 +19,8 @@ package org.apache.gravitino.storage.relational.mapper.provider.postgresql; import static org.apache.gravitino.storage.relational.mapper.GroupMetaMapper.GROUP_TABLE_NAME; +import static org.apache.gravitino.storage.relational.mapper.RoleMetaMapper.GROUP_ROLE_RELATION_TABLE_NAME; +import static org.apache.gravitino.storage.relational.mapper.RoleMetaMapper.ROLE_TABLE_NAME; import org.apache.gravitino.storage.relational.mapper.provider.base.GroupMetaBaseSQLProvider; import org.apache.gravitino.storage.relational.po.GroupPO; @@ -66,4 +68,28 @@ public String insertGroupMetaOnDuplicateKeyUpdate(GroupPO groupPO) { + " last_version = #{groupMeta.lastVersion}," + " deleted_at = #{groupMeta.deletedAt}"; } + + @Override + public String listExtendedGroupPOsByMetalakeId(Long metalakeId) { + return "SELECT gt.group_id as groupId, gt.group_name as groupName," + + " gt.metalake_id as metalakeId," + + " gt.audit_info as auditInfo," + + " gt.current_version as currentVersion, gt.last_version as lastVersion," + + " gt.deleted_at as deletedAt," + + " JSON_AGG(rot.role_name) as roleNames," + + " JSON_AGG(rot.role_id) as roleIds" + + " FROM " + + GROUP_TABLE_NAME + + " gt LEFT OUTER JOIN " + + GROUP_ROLE_RELATION_TABLE_NAME + + " rt ON rt.group_id = gt.group_id" + + " LEFT OUTER JOIN " + + ROLE_TABLE_NAME + + " rot ON rot.role_id = rt.role_id" + + " WHERE " + + " gt.deleted_at = 0 AND" + + " (rot.deleted_at = 0 OR rot.deleted_at is NULL) AND" + + " (rt.deleted_at = 0 OR rt.deleted_at is NULL) AND gt.metalake_id = #{metalakeId}" + + " GROUP BY gt.group_id"; + } } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/po/ExtendedGroupPO.java b/core/src/main/java/org/apache/gravitino/storage/relational/po/ExtendedGroupPO.java index b331fb6b3c5..390a0039833 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/po/ExtendedGroupPO.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/po/ExtendedGroupPO.java @@ -20,17 +20,22 @@ import java.util.Objects; +/** + * ExtendedGroupPO add extra roleNames and roleIds for GroupPO. This PO is only used for reading the + * data from multiple joined tables. The PO won't be written to database. So we don't need the inner + * class Builder. + */ public class ExtendedGroupPO extends GroupPO { - private String groupNames; - private String groupIds; + private String roleNames; + private String roleIds; - public String getGroupNames() { - return groupNames; + public String getRoleNames() { + return roleNames; } - public String getGroupIds() { - return groupIds; + public String getRoleIds() { + return roleIds; } @Override @@ -43,12 +48,12 @@ public boolean equals(Object o) { return false; } ExtendedGroupPO that = (ExtendedGroupPO) o; - return Objects.equals(getGroupNames(), that.getGroupNames()) - && Objects.equals(getGroupIds(), that.getGroupIds()); + return Objects.equals(getRoleIds(), that.getRoleIds()) + && Objects.equals(getRoleNames(), that.getRoleNames()); } @Override public int hashCode() { - return Objects.hash(super.hashCode(), getGroupIds(), getGroupNames()); + return Objects.hash(super.hashCode(), getRoleIds(), getRoleNames()); } } 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 05ebf0e0073..4329b3a0a10 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 @@ -40,6 +40,7 @@ import org.apache.gravitino.storage.relational.mapper.GroupMetaMapper; import org.apache.gravitino.storage.relational.mapper.GroupRoleRelMapper; import org.apache.gravitino.storage.relational.mapper.OwnerMetaMapper; +import org.apache.gravitino.storage.relational.po.ExtendedGroupPO; import org.apache.gravitino.storage.relational.po.GroupPO; import org.apache.gravitino.storage.relational.po.GroupRoleRelPO; import org.apache.gravitino.storage.relational.po.RolePO; @@ -250,19 +251,34 @@ public GroupEntity updateGroup( return newEntity; } - public List listGroupsByNamespace(Namespace namespace) { - AuthorizationUtils.checkUserNamespace(namespace); + public List listGroupsByNamespace(Namespace namespace, boolean allFields) { + AuthorizationUtils.checkGroupNamespace(namespace); String metalakeName = namespace.level(0); - Long metalakeId = MetalakeMetaService.getInstance().getMetalakeIdByName(metalakeName); - List groupPOS = - SessionUtils.getWithoutCommit( - GroupMetaMapper.class, mapper -> mapper.selectGroupsMetaByMetalakeId(metalakeId)); - return groupPOS.stream() - .map( - po -> - POConverters.fromGroupPO( - po, Collections.emptyList(), AuthorizationUtils.ofUserNamespace(metalakeName))) - .collect(Collectors.toList()); + + if (allFields) { + Long metalakeId = MetalakeMetaService.getInstance().getMetalakeIdByName(metalakeName); + List groupPOs = + SessionUtils.getWithoutCommit( + GroupMetaMapper.class, mapper -> mapper.listExtendedGroupPOsByMetalakeId(metalakeId)); + return groupPOs.stream() + .map( + po -> + POConverters.fromExtendedGroupPO( + po, AuthorizationUtils.ofGroupNamespace(metalakeName))) + .collect(Collectors.toList()); + } else { + List groupPOs = + SessionUtils.getWithoutCommit( + GroupMetaMapper.class, mapper -> mapper.listGroupPOsByMetalake(metalakeName)); + return groupPOs.stream() + .map( + po -> + POConverters.fromGroupPO( + po, + Collections.emptyList(), + AuthorizationUtils.ofGroupNamespace(metalakeName))) + .collect(Collectors.toList()); + } } public int deleteGroupMetasByLegacyTimeline(long legacyTimeline, int limit) { 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 da1f3d06a3b..088af2ea4b5 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 @@ -49,6 +49,7 @@ import org.apache.gravitino.meta.TopicEntity; import org.apache.gravitino.meta.UserEntity; import org.apache.gravitino.storage.relational.po.CatalogPO; +import org.apache.gravitino.storage.relational.po.ExtendedGroupPO; import org.apache.gravitino.storage.relational.po.ExtendedUserPO; import org.apache.gravitino.storage.relational.po.FilesetPO; import org.apache.gravitino.storage.relational.po.FilesetVersionPO; @@ -733,7 +734,7 @@ public static UserEntity fromUserPO(UserPO userPO, List rolePOs, Namespa /** * Convert {@link ExtendedUserPO} to {@link UserEntity} * - * @param userPO CombinedUserPo object to be converted + * @param userPO ExtendedUserPo object to be converted * @param namespace Namespace object to be associated with the user * @return UserEntity object from ExtendedUserPO object */ @@ -814,6 +815,57 @@ public static GroupEntity fromGroupPO( } } + /** + * Convert {@link ExtendedGroupPO} to {@link GroupEntity} + * + * @param groupPO ExtendedUserPo object to be converted + * @param namespace Namespace object to be associated with the user + * @return UserEntity object from ExtendedUserPO object + */ + public static GroupEntity fromExtendedGroupPO(ExtendedGroupPO groupPO, Namespace namespace) { + try { + GroupEntity.Builder builder = + GroupEntity.builder() + .withId(groupPO.getGroupId()) + .withName(groupPO.getGroupName()) + .withNamespace(namespace) + .withAuditInfo( + JsonUtils.anyFieldMapper().readValue(groupPO.getAuditInfo(), AuditInfo.class)); + + if (StringUtils.isNotBlank(groupPO.getRoleNames())) { + List roleNamesFromJson = + JsonUtils.anyFieldMapper().readValue(groupPO.getRoleNames(), List.class); + List roleNames = + roleNamesFromJson.stream().filter(StringUtils::isNotBlank).collect(Collectors.toList()); + if (!roleNames.isEmpty()) { + builder.withRoleNames(roleNames); + } + } + + if (StringUtils.isNotBlank(groupPO.getRoleIds())) { + // Different JSON AGG from backends will produce different types data, we + // can only use Object. PostSQL produces the data with type Long. H2 produces + // the data with type String. + List roleIdsFromJson = + JsonUtils.anyFieldMapper().readValue(groupPO.getRoleIds(), List.class); + List roleIds = + roleIdsFromJson.stream() + .filter(Objects::nonNull) + .map(String::valueOf) + .filter(StringUtils::isNotBlank) + .map(Long::valueOf) + .collect(Collectors.toList()); + + if (!roleIds.isEmpty()) { + builder.withRoleIds(roleIds); + } + } + return builder.build(); + } catch (JsonProcessingException e) { + throw new RuntimeException("Failed to deserialize json object:", e); + } + } + /** * Initialize UserRoleRelPO * diff --git a/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManager.java b/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManager.java index 6dfaf54fecf..ff0f5a58e1a 100644 --- a/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManager.java +++ b/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManager.java @@ -294,6 +294,28 @@ public void testGetGroup() { Assertions.assertTrue(exception.getMessage().contains("Group not-exist does not exist")); } + @Test + public void testListGroupss() { + accessControlManager.addGroup("metalake_list", "testList1"); + accessControlManager.addGroup("metalake_list", "testList2"); + + // Test to list groups + String[] expectGroupNames = new String[] {"testList1", "testList2"}; + String[] actualGroupNames = accessControlManager.listGroupNames("metalake_list"); + Arrays.sort(actualGroupNames); + Assertions.assertArrayEquals(expectGroupNames, actualGroupNames); + Group[] groups = accessControlManager.listGroups("metalake_list"); + Arrays.sort(groups, Comparator.comparing(Group::name)); + Assertions.assertArrayEquals( + expectGroupNames, Arrays.stream(groups).map(Group::name).toArray(String[]::new)); + + // Test with NoSuchMetalakeException + Assertions.assertThrows( + NoSuchMetalakeException.class, () -> accessControlManager.listGroupNames("no-exist")); + Assertions.assertThrows( + NoSuchMetalakeException.class, () -> accessControlManager.listGroups("no-exist")); + } + @Test public void testRemoveGroup() { accessControlManager.addGroup(METALAKE, "testRemove"); 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..2a7b671faae 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 @@ -27,6 +27,7 @@ import java.sql.SQLException; import java.sql.Statement; import java.time.Instant; +import java.util.Comparator; import java.util.List; import java.util.Optional; import java.util.function.Function; @@ -119,6 +120,77 @@ void getGroupByIdentifier() throws IOException { Sets.newHashSet(group2.roleNames()), Sets.newHashSet(actualGroup.roleNames())); } + @Test + void testListGroups() throws IOException { + AuditInfo auditInfo = + AuditInfo.builder().withCreator("creator").withCreateTime(Instant.now()).build(); + BaseMetalake metalake = + createBaseMakeLake(RandomIdGenerator.INSTANCE.nextId(), metalakeName, auditInfo); + backend.insert(metalake, false); + + CatalogEntity catalog = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), Namespace.of(metalakeName), "catalog", auditInfo); + backend.insert(catalog, false); + + GroupEntity group1 = + createGroupEntity( + RandomIdGenerator.INSTANCE.nextId(), + AuthorizationUtils.ofUserNamespace(metalakeName), + "group1", + auditInfo); + + RoleEntity role1 = + createRoleEntity( + RandomIdGenerator.INSTANCE.nextId(), + AuthorizationUtils.ofRoleNamespace("metalake"), + "role1", + auditInfo, + "catalog"); + backend.insert(role1, false); + + RoleEntity role2 = + createRoleEntity( + RandomIdGenerator.INSTANCE.nextId(), + AuthorizationUtils.ofRoleNamespace("metalake"), + "role2", + auditInfo, + "catalog"); + backend.insert(role2, false); + + GroupEntity group2 = + createGroupEntity( + RandomIdGenerator.INSTANCE.nextId(), + AuthorizationUtils.ofUserNamespace("metalake"), + "group2", + auditInfo, + Lists.newArrayList(role1.name(), role2.name()), + Lists.newArrayList(role1.id(), role2.id())); + + backend.insert(group1, false); + backend.insert(group2, false); + + GroupMetaService groupMetaService = GroupMetaService.getInstance(); + List actualGroups = + groupMetaService.listGroupsByNamespace( + AuthorizationUtils.ofUserNamespace(metalakeName), true); + actualGroups.sort(Comparator.comparing(GroupEntity::name)); + List expectGroups = Lists.newArrayList(group1, group2); + Assertions.assertEquals(expectGroups.size(), actualGroups.size()); + for (int index = 0; index < expectGroups.size(); index++) { + Assertions.assertEquals(expectGroups.get(index).name(), actualGroups.get(index).name()); + if (expectGroups.get(index).roleNames() == null) { + Assertions.assertNull(actualGroups.get(index).roleNames()); + } else { + Assertions.assertEquals( + expectGroups.get(index).roleNames().size(), actualGroups.get(index).roleNames().size()); + for (String roleName : expectGroups.get(index).roleNames()) { + Assertions.assertTrue(actualGroups.get(index).roleNames().contains(roleName)); + } + } + } + } + @Test void insertGroup() throws IOException { AuditInfo auditInfo = diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java index 82ed93ef880..12cf769932e 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java @@ -20,14 +20,15 @@ import com.codahale.metrics.annotation.ResponseMetered; import com.codahale.metrics.annotation.Timed; -import java.util.Arrays; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.DELETE; +import javax.ws.rs.DefaultValue; import javax.ws.rs.GET; import javax.ws.rs.POST; import javax.ws.rs.Path; import javax.ws.rs.PathParam; import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; import javax.ws.rs.core.Context; import javax.ws.rs.core.Response; import org.apache.gravitino.GravitinoEnv; @@ -35,11 +36,10 @@ import org.apache.gravitino.Namespace; import org.apache.gravitino.authorization.AccessControlDispatcher; import org.apache.gravitino.authorization.AuthorizationUtils; -import org.apache.gravitino.authorization.Group; -import org.apache.gravitino.dto.authorization.GroupDTO; import org.apache.gravitino.dto.requests.GroupAddRequest; import org.apache.gravitino.dto.responses.GroupListResponse; import org.apache.gravitino.dto.responses.GroupResponse; +import org.apache.gravitino.dto.responses.NameListResponse; import org.apache.gravitino.dto.responses.RemoveResponse; import org.apache.gravitino.dto.util.DTOConverters; import org.apache.gravitino.lock.LockType; @@ -144,25 +144,21 @@ public Response removeGroup( @Produces("application/vnd.gravitino.v1+json") @Timed(name = "list-group." + MetricNames.HTTP_PROCESS_DURATION, absolute = true) @ResponseMetered(name = "list-group", absolute = true) - public Response listGroups(@PathParam("metalake") String metalake) { + public Response listGroups( + @PathParam("metalake") String metalake, + @QueryParam("details") @DefaultValue("false") boolean verbose) { LOG.info("Received list groups request."); try { return Utils.doAs( httpRequest, () -> { - Group[] groups = - TreeLockUtils.doWithTreeLock( - NameIdentifier.of(metalake), - LockType.READ, - () -> accessControlManager.listGroup(metalake)); - GroupDTO[] groupDTOS; - if (groups == null) { - groupDTOS = new GroupDTO[0]; + if (verbose) { + return Utils.ok( + new GroupListResponse( + DTOConverters.toDTOs(accessControlManager.listGroups(metalake)))); } else { - groupDTOS = Arrays.stream(groups).map(DTOConverters::toDTO).toArray(GroupDTO[]::new); + return Utils.ok(new NameListResponse(accessControlManager.listGroupNames(metalake))); } - LOG.info("List {} groups in Gravitino", groupDTOS.length); - return Utils.ok(new GroupListResponse(groupDTOS)); }); } catch (Exception e) { 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 b3af1478ddc..6025d1105ea 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 @@ -46,6 +46,7 @@ import org.apache.gravitino.dto.responses.ErrorResponse; import org.apache.gravitino.dto.responses.GroupListResponse; import org.apache.gravitino.dto.responses.GroupResponse; +import org.apache.gravitino.dto.responses.NameListResponse; import org.apache.gravitino.dto.responses.RemoveResponse; import org.apache.gravitino.exceptions.GroupAlreadyExistsException; import org.apache.gravitino.exceptions.NoSuchGroupException; @@ -242,66 +243,101 @@ public void testGetGroup() { Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResponse2.getType()); } - private Group buildGroup(String group) { - return GroupEntity.builder() - .withId(1L) - .withName(group) - .withRoleNames(Collections.emptyList()) - .withAuditInfo( - AuditInfo.builder().withCreator("creator").withCreateTime(Instant.now()).build()) - .build(); + @Test + public void testListGroupNames() { + when(manager.listGroupNames(any())).thenReturn(new String[] {"group"}); + + Response resp = + target("/metalakes/metalake1/groups/") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .get(); + Assertions.assertEquals(Response.Status.OK.getStatusCode(), resp.getStatus()); + + NameListResponse listResponse = resp.readEntity(NameListResponse.class); + Assertions.assertEquals(0, listResponse.getCode()); + + Assertions.assertEquals(1, listResponse.getNames().length); + Assertions.assertEquals("group", listResponse.getNames()[0]); + + // Test to throw NoSuchMetalakeException + doThrow(new NoSuchMetalakeException("mock error")).when(manager).listGroupNames(any()); + Response resp1 = + target("/metalakes/metalake1/groups/") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .get(); + + Assertions.assertEquals(Response.Status.NOT_FOUND.getStatusCode(), resp1.getStatus()); + + ErrorResponse errorResponse = resp1.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.NOT_FOUND_CODE, errorResponse.getCode()); + Assertions.assertEquals(NoSuchMetalakeException.class.getSimpleName(), errorResponse.getType()); + + // Test to throw internal RuntimeException + doThrow(new RuntimeException("mock error")).when(manager).listGroupNames(any()); + Response resp3 = + target("/metalakes/metalake1/groups") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .get(); + + Assertions.assertEquals( + Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp3.getStatus()); + + ErrorResponse errorResponse2 = resp3.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE, errorResponse2.getCode()); + Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResponse2.getType()); } @Test - public void testListGroup() { - Group group = buildGroup("group0"); - Group group2 = buildGroup("group1"); - Group[] groups = {group, group2}; - - when(manager.listGroup("metalake1")).thenReturn(groups); + public void testListGroups() { + Group group = buildGroup("user"); + when(manager.listGroups(any())).thenReturn(new Group[] {group}); Response resp = - target("/metalakes/metalake1/groups") + target("/metalakes/metalake1/groups/") + .queryParam("details", "true") .request(MediaType.APPLICATION_JSON_TYPE) .accept("application/vnd.gravitino.v1+json") .get(); + Assertions.assertEquals(Response.Status.OK.getStatusCode(), resp.getStatus()); - Assertions.assertEquals(Status.OK.getStatusCode(), resp.getStatus()); + GroupListResponse listResponse = resp.readEntity(GroupListResponse.class); + Assertions.assertEquals(0, listResponse.getCode()); - GroupListResponse groupListResponse = resp.readEntity(GroupListResponse.class); - Assertions.assertEquals(0, groupListResponse.getCode()); - GroupDTO[] groupDTOS = groupListResponse.getGroups(); - for (int i = 0; i < groupDTOS.length; i++) { - Assertions.assertEquals("group" + i, groupDTOS[i].name()); - Assertions.assertNotNull(groupDTOS[i].roles()); - Assertions.assertTrue(groupDTOS[i].roles().isEmpty()); - } + Assertions.assertEquals(1, listResponse.getGroups().length); + Assertions.assertEquals(group.name(), listResponse.getGroups()[0].name()); + Assertions.assertEquals(group.roles(), listResponse.getGroups()[0].roles()); // Test to throw NoSuchMetalakeException - doThrow(new NoSuchMetalakeException("mock error")).when(manager).listGroup("metalake1"); + doThrow(new NoSuchMetalakeException("mock error")).when(manager).listGroups(any()); Response resp1 = - target("/metalakes/metalake1/groups") + target("/metalakes/metalake1/groups/") + .queryParam("details", "true") .request(MediaType.APPLICATION_JSON_TYPE) .accept("application/vnd.gravitino.v1+json") .get(); - Assertions.assertEquals(Status.NOT_FOUND.getStatusCode(), resp1.getStatus()); + Assertions.assertEquals(Response.Status.NOT_FOUND.getStatusCode(), resp1.getStatus()); ErrorResponse errorResponse = resp1.readEntity(ErrorResponse.class); Assertions.assertEquals(ErrorConstants.NOT_FOUND_CODE, errorResponse.getCode()); Assertions.assertEquals(NoSuchMetalakeException.class.getSimpleName(), errorResponse.getType()); // Test to throw internal RuntimeException - doThrow(new RuntimeException("mock error")).when(manager).listGroup("metalake1"); - Response resp2 = + doThrow(new RuntimeException("mock error")).when(manager).listGroups(any()); + Response resp3 = target("/metalakes/metalake1/groups") + .queryParam("details", "true") .request(MediaType.APPLICATION_JSON_TYPE) .accept("application/vnd.gravitino.v1+json") .get(); - Assertions.assertEquals(Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp2.getStatus()); + Assertions.assertEquals( + Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp3.getStatus()); - ErrorResponse errorResponse2 = resp2.readEntity(ErrorResponse.class); + ErrorResponse errorResponse2 = resp3.readEntity(ErrorResponse.class); Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE, errorResponse2.getCode()); Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResponse2.getType()); } @@ -347,4 +383,14 @@ public void testRemoveGroup() { Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE, errorResponse.getCode()); Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResponse.getType()); } + + private Group buildGroup(String group) { + return GroupEntity.builder() + .withId(1L) + .withName(group) + .withRoleNames(Collections.emptyList()) + .withAuditInfo( + AuditInfo.builder().withCreator("creator").withCreateTime(Instant.now()).build()) + .build(); + } } From 4a85ff7bfddb3714dee896c5f83900e6e08d5b5f Mon Sep 17 00:00:00 2001 From: Rory Date: Fri, 27 Sep 2024 15:09:36 +0800 Subject: [PATCH 10/16] Fix wrong use --- .../authorization/UserGroupManager.java | 2 +- .../service/TestGroupMetaService.java | 18 +++++++++--------- .../server/web/rest/TestGroupOperations.java | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) 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 65043732a87..f5353f5e4d9 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java @@ -206,7 +206,7 @@ Group[] listGroups(String metalake) { private Group[] listGroupInternal(String metalake, boolean allFields) { try { AuthorizationUtils.checkMetalakeExists(metalake); - Namespace namespace = AuthorizationUtils.ofUserNamespace(metalake); + Namespace namespace = AuthorizationUtils.ofGroupNamespace(metalake); return store .list(namespace, GroupEntity.class, EntityType.GROUP, allFields) .toArray(new Group[0]); 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 2a7b671faae..77cd9d110bc 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 @@ -136,7 +136,7 @@ void testListGroups() throws IOException { GroupEntity group1 = createGroupEntity( RandomIdGenerator.INSTANCE.nextId(), - AuthorizationUtils.ofUserNamespace(metalakeName), + AuthorizationUtils.ofGroupNamespace(metalakeName), "group1", auditInfo); @@ -161,7 +161,7 @@ void testListGroups() throws IOException { GroupEntity group2 = createGroupEntity( RandomIdGenerator.INSTANCE.nextId(), - AuthorizationUtils.ofUserNamespace("metalake"), + AuthorizationUtils.ofGroupNamespace("metalake"), "group2", auditInfo, Lists.newArrayList(role1.name(), role2.name()), @@ -173,7 +173,7 @@ void testListGroups() throws IOException { GroupMetaService groupMetaService = GroupMetaService.getInstance(); List actualGroups = groupMetaService.listGroupsByNamespace( - AuthorizationUtils.ofUserNamespace(metalakeName), true); + AuthorizationUtils.ofGroupNamespace(metalakeName), true); actualGroups.sort(Comparator.comparing(GroupEntity::name)); List expectGroups = Lists.newArrayList(group1, group2); Assertions.assertEquals(expectGroups.size(), actualGroups.size()); @@ -315,7 +315,7 @@ void insertGroup() throws IOException { GroupEntity group3Overwrite = createGroupEntity( group1.id(), - AuthorizationUtils.ofUserNamespace(metalakeName), + AuthorizationUtils.ofGroupNamespace(metalakeName), "group3Overwrite", auditInfo, Lists.newArrayList(role3.name()), @@ -332,7 +332,7 @@ void insertGroup() throws IOException { GroupEntity group4Overwrite = createGroupEntity( group1.id(), - AuthorizationUtils.ofUserNamespace(metalakeName), + AuthorizationUtils.ofGroupNamespace(metalakeName), "group4Overwrite", auditInfo); Assertions.assertDoesNotThrow(() -> groupMetaService.insertGroup(group4Overwrite, true)); @@ -851,7 +851,7 @@ void deleteGroupMetasByLegacyTimeline() throws IOException { GroupEntity group1 = createGroupEntity( RandomIdGenerator.INSTANCE.nextId(), - AuthorizationUtils.ofUserNamespace(metalakeName), + AuthorizationUtils.ofGroupNamespace(metalakeName), "group1", auditInfo, Lists.newArrayList(role1.name(), role2.name()), @@ -859,7 +859,7 @@ void deleteGroupMetasByLegacyTimeline() throws IOException { GroupEntity group2 = createGroupEntity( RandomIdGenerator.INSTANCE.nextId(), - AuthorizationUtils.ofUserNamespace(metalakeName), + AuthorizationUtils.ofGroupNamespace(metalakeName), "group2", auditInfo, Lists.newArrayList(role1.name(), role2.name()), @@ -867,7 +867,7 @@ void deleteGroupMetasByLegacyTimeline() throws IOException { GroupEntity group3 = createGroupEntity( RandomIdGenerator.INSTANCE.nextId(), - AuthorizationUtils.ofUserNamespace(metalakeName), + AuthorizationUtils.ofGroupNamespace(metalakeName), "group3", auditInfo, Lists.newArrayList(role1.name(), role2.name()), @@ -875,7 +875,7 @@ void deleteGroupMetasByLegacyTimeline() throws IOException { GroupEntity group4 = createGroupEntity( RandomIdGenerator.INSTANCE.nextId(), - AuthorizationUtils.ofUserNamespace(metalakeName), + AuthorizationUtils.ofGroupNamespace(metalakeName), "group4", auditInfo, Lists.newArrayList(role1.name(), role2.name()), 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 6025d1105ea..77f0cf97988 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 @@ -292,7 +292,7 @@ public void testListGroupNames() { @Test public void testListGroups() { - Group group = buildGroup("user"); + Group group = buildGroup("group"); when(manager.listGroups(any())).thenReturn(new Group[] {group}); Response resp = From 9df97a12a2514a4fc9a9228a19dcbf32fad1bb1b Mon Sep 17 00:00:00 2001 From: Rory Date: Fri, 27 Sep 2024 15:15:58 +0800 Subject: [PATCH 11/16] add comment --- .../gravitino/dto/util/DTOConverters.java | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/common/src/main/java/org/apache/gravitino/dto/util/DTOConverters.java b/common/src/main/java/org/apache/gravitino/dto/util/DTOConverters.java index 96039b0077a..38224493b71 100644 --- a/common/src/main/java/org/apache/gravitino/dto/util/DTOConverters.java +++ b/common/src/main/java/org/apache/gravitino/dto/util/DTOConverters.java @@ -422,13 +422,6 @@ public static GroupDTO toDTO(Group group) { .build(); } - public static GroupDTO[] toDTOs(Group[] groups) { - if (ArrayUtils.isEmpty(groups)) { - return new GroupDTO[0]; - } - return Arrays.stream(groups).map(DTOConverters::toDTO).toArray(GroupDTO[]::new); - } - /** * Converts a role implementation to a RoleDTO. * @@ -698,6 +691,19 @@ public static UserDTO[] toDTOs(User[] users) { return Arrays.stream(users).map(DTOConverters::toDTO).toArray(UserDTO[]::new); } + /** + * Converts an array of Groups to an array of GroupDTOs. + * + * @param groups The groups to be converted. + * @return The array of GroupDTOs. + */ + public static GroupDTO[] toDTOs(Group[] groups) { + if (ArrayUtils.isEmpty(groups)) { + return new GroupDTO[0]; + } + return Arrays.stream(groups).map(DTOConverters::toDTO).toArray(GroupDTO[]::new); + } + /** * Converts a DistributionDTO to a Distribution. * From 90d13dced7d9cf65c295e79c8a9eb76c5fcb4dad Mon Sep 17 00:00:00 2001 From: Rory Date: Fri, 27 Sep 2024 15:27:18 +0800 Subject: [PATCH 12/16] fix comment --- .../apache/gravitino/storage/relational/utils/POConverters.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 088af2ea4b5..c58b79ba73b 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 @@ -820,7 +820,7 @@ public static GroupEntity fromGroupPO( * * @param groupPO ExtendedUserPo object to be converted * @param namespace Namespace object to be associated with the user - * @return UserEntity object from ExtendedUserPO object + * @return GroupEntity object from ExtendedGroupPO object */ public static GroupEntity fromExtendedGroupPO(ExtendedGroupPO groupPO, Namespace namespace) { try { From 97dc884aa4fc02434ee462c42b4b1fbde89f77cc Mon Sep 17 00:00:00 2001 From: Rory Date: Fri, 27 Sep 2024 15:29:37 +0800 Subject: [PATCH 13/16] fix comment --- .../gravitino/storage/relational/utils/POConverters.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 c58b79ba73b..f6392127b36 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 @@ -734,7 +734,7 @@ public static UserEntity fromUserPO(UserPO userPO, List rolePOs, Namespa /** * Convert {@link ExtendedUserPO} to {@link UserEntity} * - * @param userPO ExtendedUserPo object to be converted + * @param userPO ExtendedUserPO object to be converted * @param namespace Namespace object to be associated with the user * @return UserEntity object from ExtendedUserPO object */ @@ -818,7 +818,7 @@ public static GroupEntity fromGroupPO( /** * Convert {@link ExtendedGroupPO} to {@link GroupEntity} * - * @param groupPO ExtendedUserPo object to be converted + * @param groupPO ExtendedGroupPO object to be converted * @param namespace Namespace object to be associated with the user * @return GroupEntity object from ExtendedGroupPO object */ From 9c893d879d65c265ca0101f418ef590df59a8c7a Mon Sep 17 00:00:00 2001 From: Rory Date: Fri, 27 Sep 2024 17:22:22 +0800 Subject: [PATCH 14/16] address comment --- .../gravitino/client/TestUserGroup.java | 5 +- .../authorization/UserGroupManager.java | 47 ++++++++++--------- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/clients/client-java/src/test/java/org/apache/gravitino/client/TestUserGroup.java b/clients/client-java/src/test/java/org/apache/gravitino/client/TestUserGroup.java index 354419cb475..ff98b2ca6c7 100644 --- a/clients/client-java/src/test/java/org/apache/gravitino/client/TestUserGroup.java +++ b/clients/client-java/src/test/java/org/apache/gravitino/client/TestUserGroup.java @@ -342,10 +342,7 @@ public void testListGroupNames() throws JsonProcessingException { buildMockResource(Method.GET, groupPath, null, errRespNoMetaLake, SC_NOT_FOUND); Exception ex = Assertions.assertThrows( - NoSuchMetalakeException.class, - () -> { - gravitinoClient.listGroupNames(); - }); + NoSuchMetalakeException.class, () -> gravitinoClient.listGroupNames()); Assertions.assertEquals("metalake not found", ex.getMessage()); // Test RuntimeException 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 f5353f5e4d9..bda17652dfe 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java @@ -125,23 +125,6 @@ User[] listUsers(String metalake) { return listUsersInternal(metalake, true /* allFields */); } - private User[] listUsersInternal(String metalake, boolean allFields) { - try { - AuthorizationUtils.checkMetalakeExists(metalake); - - Namespace namespace = AuthorizationUtils.ofUserNamespace(metalake); - return store - .list(namespace, UserEntity.class, Entity.EntityType.USER, allFields) - .toArray(new User[0]); - } catch (NoSuchEntityException e) { - LOG.error("Metalake {} does not exist", metalake, e); - throw new NoSuchMetalakeException(METALAKE_DOES_NOT_EXIST_MSG, metalake); - } catch (IOException ioe) { - LOG.error("Listing user under metalake {} failed due to storage issues", metalake, ioe); - throw new RuntimeException(ioe); - } - } - Group addGroup(String metalake, String group) throws GroupAlreadyExistsException { try { AuthorizationUtils.checkMetalakeExists(metalake); @@ -203,6 +186,30 @@ Group[] listGroups(String metalake) { return listGroupInternal(metalake, true); } + + String[] listGroupNames(String metalake) { + return Arrays.stream(listGroupInternal(metalake, false)) + .map(Group::name) + .toArray(String[]::new); + } + + private User[] listUsersInternal(String metalake, boolean allFields) { + try { + AuthorizationUtils.checkMetalakeExists(metalake); + + Namespace namespace = AuthorizationUtils.ofUserNamespace(metalake); + return store + .list(namespace, UserEntity.class, Entity.EntityType.USER, allFields) + .toArray(new User[0]); + } catch (NoSuchEntityException e) { + LOG.error("Metalake {} does not exist", metalake, e); + throw new NoSuchMetalakeException(METALAKE_DOES_NOT_EXIST_MSG, metalake); + } catch (IOException ioe) { + LOG.error("Listing user under metalake {} failed due to storage issues", metalake, ioe); + throw new RuntimeException(ioe); + } + } + private Group[] listGroupInternal(String metalake, boolean allFields) { try { AuthorizationUtils.checkMetalakeExists(metalake); @@ -218,10 +225,4 @@ private Group[] listGroupInternal(String metalake, boolean allFields) { throw new RuntimeException(ioe); } } - - String[] listGroupNames(String metalake) { - return Arrays.stream(listGroupInternal(metalake, false)) - .map(Group::name) - .toArray(String[]::new); - } } From eb35f965f400e3f4298a0a5435a271cdae86587e Mon Sep 17 00:00:00 2001 From: Rory Date: Fri, 27 Sep 2024 17:26:01 +0800 Subject: [PATCH 15/16] fix style --- .../apache/gravitino/authorization/UserGroupManager.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) 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 bda17652dfe..cd852ab66a7 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java @@ -186,11 +186,10 @@ Group[] listGroups(String metalake) { return listGroupInternal(metalake, true); } - String[] listGroupNames(String metalake) { return Arrays.stream(listGroupInternal(metalake, false)) - .map(Group::name) - .toArray(String[]::new); + .map(Group::name) + .toArray(String[]::new); } private User[] listUsersInternal(String metalake, boolean allFields) { @@ -199,8 +198,8 @@ private User[] listUsersInternal(String metalake, boolean allFields) { Namespace namespace = AuthorizationUtils.ofUserNamespace(metalake); return store - .list(namespace, UserEntity.class, Entity.EntityType.USER, allFields) - .toArray(new User[0]); + .list(namespace, UserEntity.class, Entity.EntityType.USER, allFields) + .toArray(new User[0]); } catch (NoSuchEntityException e) { LOG.error("Metalake {} does not exist", metalake, e); throw new NoSuchMetalakeException(METALAKE_DOES_NOT_EXIST_MSG, metalake); From 81b225f4d5a4a5edfd002d86201dd7ac15d31fe9 Mon Sep 17 00:00:00 2001 From: Rory Date: Fri, 27 Sep 2024 18:00:17 +0800 Subject: [PATCH 16/16] address comment --- .../org/apache/gravitino/dto/responses/GroupListResponse.java | 2 +- .../mapper/provider/base/GroupMetaBaseSQLProvider.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/src/main/java/org/apache/gravitino/dto/responses/GroupListResponse.java b/common/src/main/java/org/apache/gravitino/dto/responses/GroupListResponse.java index e6cc6ed8d76..271fb9a92ba 100644 --- a/common/src/main/java/org/apache/gravitino/dto/responses/GroupListResponse.java +++ b/common/src/main/java/org/apache/gravitino/dto/responses/GroupListResponse.java @@ -66,7 +66,7 @@ public void validate() throws IllegalArgumentException { .forEach( group -> { Preconditions.checkArgument( - StringUtils.isNotBlank(group.name()), "group 'name' must not be null and empty"); + StringUtils.isNotBlank(group.name()), "group 'name' must not be blank"); Preconditions.checkArgument( group.auditInfo() != null, "group 'auditInfo' must not be null"); }); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java index 2e4cd25954f..a52e1b86144 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java @@ -43,7 +43,7 @@ public String listGroupPOsByMetalake(@Param("metalakeName") String metalakeName) + GROUP_TABLE_NAME + " gt JOIN " + MetalakeMetaMapper.TABLE_NAME - + " mt ON gt.metalake_id = mt.metalake_id WHERE mt.metalake_name = #{metalakeName}" + + " mt ON gt.metalake_id = mt.metalake_id WHERE mt.metalake_name = #{metalakeName}" + " AND gt.deleted_at = 0 AND mt.deleted_at = 0"; }