This is an automated email from the ASF dual-hosted git repository. liuxun pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push: new f3b4c3a40 [#4374] feat(client): Add owner operations for client (#4397) f3b4c3a40 is described below commit f3b4c3a405fc9229806bcb520fc9c6bba0ab9440 Author: roryqi <ror...@apache.org> AuthorDate: Wed Aug 7 18:17:08 2024 +0800 [#4374] feat(client): Add owner operations for client (#4397) ### What changes were proposed in this pull request? Add owner operations for client ### Why are the changes needed? Fix: #4374 ### Does this PR introduce _any_ user-facing change? I will add the document later. ### How was this patch tested? Add UTS. --- .../exceptions/NoSuchMetadataObjectException.java | 48 ++++++++ .../org/apache/gravitino/client/ErrorHandlers.java | 40 ++++++ .../apache/gravitino/client/GravitinoClient.java | 30 +++++ .../apache/gravitino/client/GravitinoMetalake.java | 59 +++++++++ .../org/apache/gravitino/client/TestOwner.java | 135 +++++++++++++++++++++ .../gravitino/authorization/OwnerManager.java | 3 +- .../storage/relational/mapper/GroupMetaMapper.java | 12 -- .../storage/relational/mapper/OwnerMetaMapper.java | 59 +++++++-- .../storage/relational/mapper/UserMetaMapper.java | 12 -- .../relational/service/GroupMetaService.java | 24 ++-- .../relational/service/OwnerMetaService.java | 41 ++++--- .../relational/service/UserMetaService.java | 24 ++-- .../gravitino/authorization/TestOwnerManager.java | 13 ++ .../relational/service/TestGroupMetaService.java | 17 +++ .../relational/service/TestUserMetaService.java | 15 +++ 15 files changed, 447 insertions(+), 85 deletions(-) diff --git a/api/src/main/java/org/apache/gravitino/exceptions/NoSuchMetadataObjectException.java b/api/src/main/java/org/apache/gravitino/exceptions/NoSuchMetadataObjectException.java new file mode 100644 index 000000000..97330f1a7 --- /dev/null +++ b/api/src/main/java/org/apache/gravitino/exceptions/NoSuchMetadataObjectException.java @@ -0,0 +1,48 @@ +/* + * 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.exceptions; + +import com.google.errorprone.annotations.FormatMethod; +import com.google.errorprone.annotations.FormatString; + +/** Exception thrown when a metadata object with specified name doesn't exist. */ +public class NoSuchMetadataObjectException extends NotFoundException { + /** + * Constructs a new exception with the specified detail message. + * + * @param message the detail message. + * @param args the arguments to the message. + */ + @FormatMethod + public NoSuchMetadataObjectException(@FormatString String message, Object... args) { + super(message, args); + } + + /** + * Constructs a new exception with the specified detail message and cause. + * + * @param cause the cause. + * @param message the detail message. + * @param args the arguments to the message. + */ + @FormatMethod + public NoSuchMetadataObjectException(Throwable cause, String message, Object... args) { + super(cause, message, args); + } +} diff --git a/clients/client-java/src/main/java/org/apache/gravitino/client/ErrorHandlers.java b/clients/client-java/src/main/java/org/apache/gravitino/client/ErrorHandlers.java index f98ee7708..4741e8462 100644 --- a/clients/client-java/src/main/java/org/apache/gravitino/client/ErrorHandlers.java +++ b/clients/client-java/src/main/java/org/apache/gravitino/client/ErrorHandlers.java @@ -35,6 +35,7 @@ import org.apache.gravitino.exceptions.MetalakeAlreadyExistsException; import org.apache.gravitino.exceptions.NoSuchCatalogException; import org.apache.gravitino.exceptions.NoSuchFilesetException; import org.apache.gravitino.exceptions.NoSuchGroupException; +import org.apache.gravitino.exceptions.NoSuchMetadataObjectException; import org.apache.gravitino.exceptions.NoSuchMetalakeException; import org.apache.gravitino.exceptions.NoSuchPartitionException; import org.apache.gravitino.exceptions.NoSuchRoleException; @@ -188,6 +189,15 @@ public class ErrorHandlers { return TagErrorHandler.INSTANCE; } + /** + * Creates an error handler specific to Owner operations. + * + * @return A Consumer representing the Owner error handler. + */ + public static Consumer<ErrorResponse> ownerErrorHandler() { + return OwnerErrorHandler.INSTANCE; + } + private ErrorHandlers() {} /** @@ -713,6 +723,36 @@ public class ErrorHandlers { } } + /** Error handler specific to Owner operations. */ + @SuppressWarnings("FormatStringAnnotation") + private static class OwnerErrorHandler extends RestErrorHandler { + + private static final OwnerErrorHandler INSTANCE = new OwnerErrorHandler(); + + @Override + public void accept(ErrorResponse errorResponse) { + String errorMessage = formatErrorMessage(errorResponse); + + switch (errorResponse.getCode()) { + case ErrorConstants.ILLEGAL_ARGUMENTS_CODE: + throw new IllegalArgumentException(errorMessage); + + case ErrorConstants.NOT_FOUND_CODE: + if (errorResponse.getType().equals(NoSuchMetadataObjectException.class.getSimpleName())) { + throw new NoSuchMetadataObjectException(errorMessage); + } else { + throw new NotFoundException(errorMessage); + } + + case ErrorConstants.INTERNAL_ERROR_CODE: + throw new RuntimeException(errorMessage); + + default: + super.accept(errorResponse); + } + } + } + /** Generic error handler for REST requests. */ private static class RestErrorHandler extends ErrorHandler { private static final ErrorHandler INSTANCE = new RestErrorHandler(); 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 335117cac..962bb09e1 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 @@ -22,10 +22,13 @@ package org.apache.gravitino.client; import com.google.common.base.Preconditions; import java.util.List; import java.util.Map; +import java.util.Optional; import org.apache.gravitino.Catalog; import org.apache.gravitino.CatalogChange; +import org.apache.gravitino.MetadataObject; import org.apache.gravitino.SupportsCatalogs; import org.apache.gravitino.authorization.Group; +import org.apache.gravitino.authorization.Owner; import org.apache.gravitino.authorization.Role; import org.apache.gravitino.authorization.SecurableObject; import org.apache.gravitino.authorization.User; @@ -33,10 +36,12 @@ import org.apache.gravitino.exceptions.CatalogAlreadyExistsException; import org.apache.gravitino.exceptions.GroupAlreadyExistsException; import org.apache.gravitino.exceptions.NoSuchCatalogException; import org.apache.gravitino.exceptions.NoSuchGroupException; +import org.apache.gravitino.exceptions.NoSuchMetadataObjectException; import org.apache.gravitino.exceptions.NoSuchMetalakeException; import org.apache.gravitino.exceptions.NoSuchRoleException; import org.apache.gravitino.exceptions.NoSuchTagException; import org.apache.gravitino.exceptions.NoSuchUserException; +import org.apache.gravitino.exceptions.NotFoundException; import org.apache.gravitino.exceptions.RoleAlreadyExistsException; import org.apache.gravitino.exceptions.TagAlreadyExistsException; import org.apache.gravitino.exceptions.UserAlreadyExistsException; @@ -308,6 +313,31 @@ public class GravitinoClient extends GravitinoClientBase return getMetalake().revokeRolesFromGroup(roles, group); } + /** + * Get the owner of a metadata object. + * + * @param object The metadata object + * @return The owner of the metadata object. If the metadata object doesn't set the owner, it will + * return Optional.empty(). + * @throws NoSuchMetadataObjectException If the metadata object is not found. + */ + public Optional<Owner> getOwner(MetadataObject object) throws NoSuchMetadataObjectException { + return getMetalake().getOwner(object); + } + + /** + * Set the owner of a metadata object. + * + * @param object The metadata object. + * @param ownerName The name of the owner + * @param ownerType The type of the owner, The owner can be a user or a group. + * @throws NotFoundException If the metadata object isn't found or the owner doesn't exist. + */ + public void setOwner(MetadataObject object, String ownerName, Owner.Type ownerType) + throws NotFoundException { + getMetalake().setOwner(object, ownerName, ownerType); + } + /** * Creates a new builder for constructing a GravitinoClient. * 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 7d527b1ec..b7fd5d943 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 @@ -24,14 +24,18 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; +import java.util.Optional; import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.apache.gravitino.Catalog; import org.apache.gravitino.CatalogChange; +import org.apache.gravitino.MetadataObject; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.SupportsCatalogs; import org.apache.gravitino.authorization.Group; +import org.apache.gravitino.authorization.Owner; import org.apache.gravitino.authorization.Role; import org.apache.gravitino.authorization.SecurableObject; import org.apache.gravitino.authorization.User; @@ -42,6 +46,7 @@ import org.apache.gravitino.dto.requests.CatalogCreateRequest; import org.apache.gravitino.dto.requests.CatalogUpdateRequest; import org.apache.gravitino.dto.requests.CatalogUpdatesRequest; import org.apache.gravitino.dto.requests.GroupAddRequest; +import org.apache.gravitino.dto.requests.OwnerSetRequest; import org.apache.gravitino.dto.requests.RoleCreateRequest; import org.apache.gravitino.dto.requests.RoleGrantRequest; import org.apache.gravitino.dto.requests.RoleRevokeRequest; @@ -57,8 +62,10 @@ import org.apache.gravitino.dto.responses.EntityListResponse; import org.apache.gravitino.dto.responses.ErrorResponse; import org.apache.gravitino.dto.responses.GroupResponse; import org.apache.gravitino.dto.responses.NameListResponse; +import org.apache.gravitino.dto.responses.OwnerResponse; import org.apache.gravitino.dto.responses.RemoveResponse; import org.apache.gravitino.dto.responses.RoleResponse; +import org.apache.gravitino.dto.responses.SetResponse; import org.apache.gravitino.dto.responses.TagListResponse; import org.apache.gravitino.dto.responses.TagResponse; import org.apache.gravitino.dto.responses.UserResponse; @@ -66,10 +73,12 @@ import org.apache.gravitino.exceptions.CatalogAlreadyExistsException; import org.apache.gravitino.exceptions.GroupAlreadyExistsException; import org.apache.gravitino.exceptions.NoSuchCatalogException; import org.apache.gravitino.exceptions.NoSuchGroupException; +import org.apache.gravitino.exceptions.NoSuchMetadataObjectException; import org.apache.gravitino.exceptions.NoSuchMetalakeException; import org.apache.gravitino.exceptions.NoSuchRoleException; import org.apache.gravitino.exceptions.NoSuchTagException; import org.apache.gravitino.exceptions.NoSuchUserException; +import org.apache.gravitino.exceptions.NotFoundException; import org.apache.gravitino.exceptions.RoleAlreadyExistsException; import org.apache.gravitino.exceptions.TagAlreadyExistsException; import org.apache.gravitino.exceptions.UserAlreadyExistsException; @@ -88,6 +97,7 @@ public class GravitinoMetalake extends MetalakeDTO implements SupportsCatalogs, private static final String API_METALAKES_USERS_PATH = "api/metalakes/%s/users/%s"; private static final String API_METALAKES_GROUPS_PATH = "api/metalakes/%s/groups/%s"; private static final String API_METALAKES_ROLES_PATH = "api/metalakes/%s/roles/%s"; + private static final String API_METALAKES_OWNERS_PATH = "api/metalakes/%s/owners/%s"; private static final String BLANK_PLACE_HOLDER = ""; private static final String API_METALAKES_TAGS_PATH = "api/metalakes/%s/tags"; @@ -762,6 +772,55 @@ public class GravitinoMetalake extends MetalakeDTO implements SupportsCatalogs, return resp.getGroup(); } + /** + * Get the owner of a metadata object. + * + * @param object The metadata object + * @return The owner of the metadata object. If the metadata object doesn't set the owner, it will + * return Optional.empty(). + * @throws NoSuchMetadataObjectException If the metadata object is not found. + */ + public Optional<Owner> getOwner(MetadataObject object) throws NoSuchMetadataObjectException { + OwnerResponse resp = + restClient.get( + String.format( + API_METALAKES_OWNERS_PATH, + this.name(), + String.format( + "%s/%s", object.type().name().toLowerCase(Locale.ROOT), object.fullName())), + OwnerResponse.class, + Collections.emptyMap(), + ErrorHandlers.ownerErrorHandler()); + resp.validate(); + return Optional.ofNullable(resp.getOwner()); + } + + /** + * Set the owner of a metadata object. + * + * @param object The metadata object. + * @param ownerName The name of the owner + * @param ownerType The type of the owner, The owner can be a user or a group. + * @throws NotFoundException If the metadata object isn't found or the owner doesn't exist. + */ + public void setOwner(MetadataObject object, String ownerName, Owner.Type ownerType) + throws NotFoundException { + OwnerSetRequest request = new OwnerSetRequest(ownerName, ownerType); + request.validate(); + SetResponse resp = + restClient.put( + String.format( + API_METALAKES_OWNERS_PATH, + this.name(), + String.format( + "%s/%s", object.type().name().toLowerCase(Locale.ROOT), object.fullName())), + request, + SetResponse.class, + Collections.emptyMap(), + ErrorHandlers.ownerErrorHandler()); + resp.validate(); + } + static class Builder extends MetalakeDTO.Builder<Builder> { private RESTClient restClient; diff --git a/clients/client-java/src/test/java/org/apache/gravitino/client/TestOwner.java b/clients/client-java/src/test/java/org/apache/gravitino/client/TestOwner.java new file mode 100644 index 000000000..dc9d801b2 --- /dev/null +++ b/clients/client-java/src/test/java/org/apache/gravitino/client/TestOwner.java @@ -0,0 +1,135 @@ +/* + * 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.client; + +import static org.apache.hc.core5.http.HttpStatus.SC_NOT_FOUND; +import static org.apache.hc.core5.http.HttpStatus.SC_OK; + +import com.fasterxml.jackson.core.JsonProcessingException; +import java.time.Instant; +import java.util.Locale; +import java.util.Optional; +import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.MetadataObjects; +import org.apache.gravitino.authorization.Owner; +import org.apache.gravitino.dto.AuditDTO; +import org.apache.gravitino.dto.MetalakeDTO; +import org.apache.gravitino.dto.authorization.OwnerDTO; +import org.apache.gravitino.dto.requests.OwnerSetRequest; +import org.apache.gravitino.dto.responses.ErrorResponse; +import org.apache.gravitino.dto.responses.MetalakeResponse; +import org.apache.gravitino.dto.responses.OwnerResponse; +import org.apache.gravitino.dto.responses.SetResponse; +import org.apache.gravitino.exceptions.NoSuchMetadataObjectException; +import org.apache.gravitino.exceptions.NotFoundException; +import org.apache.hc.core5.http.HttpStatus; +import org.apache.hc.core5.http.Method; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +public class TestOwner extends TestBase { + + private static final String metalakeName = "testMetalake"; + private static final String API_OWNER_PATH = "api/metalakes/%s/owners/%s"; + private static GravitinoClient gravitinoClient; + + @BeforeAll + public static void setUp() throws Exception { + TestBase.setUp(); + + TestGravitinoMetalake.createMetalake(client, metalakeName); + + MetalakeDTO mockMetalake = + MetalakeDTO.builder() + .withName(metalakeName) + .withComment("comment") + .withAudit( + AuditDTO.builder().withCreator("creator").withCreateTime(Instant.now()).build()) + .build(); + MetalakeResponse resp = new MetalakeResponse(mockMetalake); + buildMockResource(Method.GET, "/api/metalakes/" + metalakeName, null, resp, HttpStatus.SC_OK); + + gravitinoClient = + GravitinoClient.builder("http://127.0.0.1:" + mockServer.getLocalPort()) + .withMetalake(metalakeName) + .withVersionCheckDisabled() + .build(); + } + + @Test + public void testGetOwner() throws JsonProcessingException { + MetadataObject object = MetadataObjects.of(null, metalakeName, MetadataObject.Type.METALAKE); + String ownerPath = + withSlash( + String.format( + API_OWNER_PATH, + metalakeName, + String.format( + "%s/%s", object.type().name().toLowerCase(Locale.ROOT), object.fullName()))); + OwnerDTO ownerDTO = OwnerDTO.builder().withName("test").withType(Owner.Type.USER).build(); + OwnerResponse resp = new OwnerResponse(ownerDTO); + buildMockResource(Method.GET, ownerPath, null, resp, SC_OK); + Optional<Owner> owner = gravitinoClient.getOwner(object); + Assertions.assertTrue(owner.isPresent()); + Assertions.assertEquals("test", owner.get().name()); + Assertions.assertEquals(Owner.Type.USER, owner.get().type()); + + // no owner case + OwnerResponse noOwnerResp = new OwnerResponse(null); + buildMockResource(Method.GET, ownerPath, null, noOwnerResp, SC_OK); + Assertions.assertFalse(gravitinoClient.getOwner(object).isPresent()); + + // no such metadata object exception + ErrorResponse errorResp1 = + ErrorResponse.notFound(NoSuchMetadataObjectException.class.getSimpleName(), "not found"); + buildMockResource(Method.GET, ownerPath, null, errorResp1, SC_NOT_FOUND); + Throwable ex1 = + Assertions.assertThrows( + NoSuchMetadataObjectException.class, () -> gravitinoClient.getOwner(object)); + Assertions.assertTrue(ex1.getMessage().contains("not found")); + } + + @Test + public void testSetOwner() throws JsonProcessingException { + MetadataObject object = MetadataObjects.of(null, metalakeName, MetadataObject.Type.METALAKE); + String ownerPath = + withSlash( + String.format( + API_OWNER_PATH, + metalakeName, + String.format( + "%s/%s", object.type().name().toLowerCase(Locale.ROOT), object.fullName()))); + OwnerSetRequest request = new OwnerSetRequest("test", Owner.Type.USER); + SetResponse response = new SetResponse(true); + buildMockResource(Method.PUT, ownerPath, request, response, SC_OK); + + Assertions.assertDoesNotThrow(() -> gravitinoClient.setOwner(object, "test", Owner.Type.USER)); + + // not found exception + ErrorResponse errorResp1 = + ErrorResponse.notFound(NotFoundException.class.getSimpleName(), "not found"); + buildMockResource(Method.PUT, ownerPath, request, errorResp1, SC_NOT_FOUND); + Throwable ex1 = + Assertions.assertThrows( + NotFoundException.class, + () -> gravitinoClient.setOwner(object, "test", Owner.Type.USER)); + Assertions.assertTrue(ex1.getMessage().contains("not found")); + } +} diff --git a/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java b/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java index 9fa1694e9..f79164285 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java +++ b/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java @@ -27,6 +27,7 @@ import org.apache.gravitino.MetadataObject; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.SupportsRelationOperations; import org.apache.gravitino.exceptions.NoSuchEntityException; +import org.apache.gravitino.exceptions.NoSuchMetadataObjectException; import org.apache.gravitino.exceptions.NotFoundException; import org.apache.gravitino.lock.LockType; import org.apache.gravitino.lock.TreeLockUtils; @@ -168,7 +169,7 @@ public class OwnerManager { } return Optional.of(owner); } catch (NoSuchEntityException nse) { - throw new NotFoundException( + throw new NoSuchMetadataObjectException( "The metadata object of %s isn't found", metadataObject.fullName()); } catch (IOException ioe) { LOG.info("Fail to get the owner of entity {}", metadataObject.fullName(), ioe); 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 ca29a0c5c..6250c2a9a 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 @@ -60,18 +60,6 @@ public interface GroupMetaMapper { GroupPO selectGroupMetaByMetalakeIdAndName( @Param("metalakeId") Long metalakeId, @Param("groupName") String name); - @Select( - "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 group_id = #{groupId}" - + " AND deleted_at = 0") - GroupPO selectGroupMetaById(@Param("groupId") Long groupId); - @Insert( "INSERT INTO " + GROUP_TABLE_NAME diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/OwnerMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/OwnerMetaMapper.java index a3dacf486..11c288324 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/OwnerMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/OwnerMetaMapper.java @@ -18,7 +18,9 @@ */ package org.apache.gravitino.storage.relational.mapper; +import org.apache.gravitino.storage.relational.po.GroupPO; import org.apache.gravitino.storage.relational.po.OwnerRelPO; +import org.apache.gravitino.storage.relational.po.UserPO; import org.apache.ibatis.annotations.Delete; import org.apache.ibatis.annotations.Insert; import org.apache.ibatis.annotations.Param; @@ -38,20 +40,44 @@ public interface OwnerMetaMapper { String OWNER_TABLE_NAME = "owner_meta"; @Select( - "SELECT metalake_id as metalakeId," - + " owner_id as ownerId," - + " owner_type as ownerType," - + " metadata_object_id as metadataObjectId," - + " metadata_object_type as metadataObjectType," - + " audit_info as auditInfo," - + " current_version as currentVersion, last_version as lastVersion," - + " deleted_at as deletedAt" + "SELECT ut.user_id as userId," + + " ut.user_name as userName," + + " ut.metalake_id as metalakeId," + + " ut.audit_info as auditInfo," + + " ut.current_version as currentVersion," + + " ut.last_version as lastVersion," + + " ut.deleted_at as deletedAt" + " FROM " + OWNER_TABLE_NAME - + " WHERE metadata_object_id = #{metadataObjectId} AND" - + " metadata_object_type = #{metadataObjectType}" - + " AND deleted_at = 0") - OwnerRelPO selectOwnerMetaByMetadataObjectIdAndType( + + " ot JOIN " + + UserMetaMapper.USER_TABLE_NAME + + " ut ON ut.user_id = ot.owner_id" + + " WHERE ot.metadata_object_id = #{metadataObjectId} AND" + + " ot.metadata_object_type = #{metadataObjectType} AND" + + " ot.owner_type = 'USER' AND" + + " ot.deleted_at = 0 AND ut.deleted_at = 0") + UserPO selectUserOwnerMetaByMetadataObjectIdAndType( + @Param("metadataObjectId") Long metadataObjectId, + @Param("metadataObjectType") String metadataObjectType); + + @Select( + "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 " + + OWNER_TABLE_NAME + + " ot JOIN " + + GroupMetaMapper.GROUP_TABLE_NAME + + " gt ON gt.group_id = ot.owner_id" + + " WHERE ot.metadata_object_id = #{metadataObjectId} AND" + + " ot.metadata_object_type = #{metadataObjectType} AND" + + " ot.owner_type = 'GROUP' AND" + + " ot.deleted_at = 0 AND gt.deleted_at = 0") + GroupPO selectGroupOwnerMetaByMetadataObjectIdAndType( @Param("metadataObjectId") Long metadataObjectId, @Param("metadataObjectType") String metadataObjectType); @@ -83,6 +109,15 @@ public interface OwnerMetaMapper { @Param("metadataObjectId") Long metadataObjectId, @Param("metadataObjectType") String metadataObjectType); + @Update( + "UPDATE " + + OWNER_TABLE_NAME + + " SET deleted_at = (UNIX_TIMESTAMP() * 1000.0)" + + " + EXTRACT(MICROSECOND FROM CURRENT_TIMESTAMP(3)) / 1000" + + " WHERE owner_id = #{ownerId} AND owner_type = #{ownerType} AND deleted_at = 0") + void softDeleteOwnerRelByOwnerIdAndType( + @Param("ownerId") Long ownerId, @Param("ownerType") String ownerType); + @Update( "UPDATE " + OWNER_TABLE_NAME diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/UserMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/UserMetaMapper.java index e7c24aee6..e7b442c09 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/UserMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/UserMetaMapper.java @@ -60,18 +60,6 @@ public interface UserMetaMapper { UserPO selectUserMetaByMetalakeIdAndName( @Param("metalakeId") Long metalakeId, @Param("userName") String name); - @Select( - "SELECT user_id as userId, user_name as userName," - + " metalake_id as metalakeId," - + " audit_info as auditInfo," - + " current_version as currentVersion, last_version as lastVersion," - + " deleted_at as deletedAt" - + " FROM " - + USER_TABLE_NAME - + " WHERE user_id = #{userId}" - + " AND deleted_at = 0") - UserPO selectUserMetaById(@Param("userId") Long userId); - @Insert( "INSERT INTO " + USER_TABLE_NAME 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 a1dc6283d..81dbda4f8 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/GroupMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/GroupMetaService.java @@ -22,7 +22,6 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import com.google.common.collect.Sets; import java.io.IOException; -import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.Optional; @@ -36,6 +35,7 @@ import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.meta.GroupEntity; 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.GroupPO; import org.apache.gravitino.storage.relational.po.GroupRoleRelPO; import org.apache.gravitino.storage.relational.po.RolePO; @@ -94,20 +94,6 @@ public class GroupMetaService { return POConverters.fromGroupPO(groupPO, rolePOs, identifier.namespace()); } - public GroupEntity getGroupById(String metalake, Long groupId) { - GroupPO groupPO = - SessionUtils.getWithoutCommit( - GroupMetaMapper.class, mapper -> mapper.selectGroupMetaById(groupId)); - if (groupPO == null) { - throw new NoSuchEntityException( - NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE, - Entity.EntityType.GROUP.name().toLowerCase(), - String.valueOf(groupId)); - } - return POConverters.fromGroupPO( - groupPO, Collections.emptyList(), AuthorizationUtils.ofGroupNamespace(metalake)); - } - public void insertGroup(GroupEntity groupEntity, boolean overwritten) throws IOException { try { AuthorizationUtils.checkGroup(groupEntity.nameIdentifier()); @@ -165,7 +151,13 @@ public class GroupMetaService { () -> SessionUtils.doWithoutCommit( GroupRoleRelMapper.class, - mapper -> mapper.softDeleteGroupRoleRelByGroupId(groupId))); + mapper -> mapper.softDeleteGroupRoleRelByGroupId(groupId)), + () -> + SessionUtils.doWithoutCommit( + OwnerMetaMapper.class, + mapper -> + mapper.softDeleteOwnerRelByOwnerIdAndType( + groupId, Entity.EntityType.GROUP.name()))); return true; } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/OwnerMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/OwnerMetaService.java index 3354e7a95..c25c9997c 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/OwnerMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/OwnerMetaService.java @@ -18,12 +18,16 @@ */ package org.apache.gravitino.storage.relational.service; +import java.util.Collections; import java.util.Optional; import org.apache.gravitino.Entity; import org.apache.gravitino.MetadataObject; import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.authorization.AuthorizationUtils; import org.apache.gravitino.storage.relational.mapper.OwnerMetaMapper; +import org.apache.gravitino.storage.relational.po.GroupPO; import org.apache.gravitino.storage.relational.po.OwnerRelPO; +import org.apache.gravitino.storage.relational.po.UserPO; import org.apache.gravitino.storage.relational.utils.POConverters; import org.apache.gravitino.storage.relational.utils.SessionUtils; import org.apache.gravitino.utils.NameIdentifierUtil; @@ -44,28 +48,33 @@ public class OwnerMetaService { MetalakeMetaService.getInstance().getMetalakeIdByName(getMetalake(identifier)); Long entityId = getEntityId(metalakeId, identifier, type); - OwnerRelPO ownerRelPO = + UserPO userPO = SessionUtils.getWithoutCommit( OwnerMetaMapper.class, - mapper -> mapper.selectOwnerMetaByMetadataObjectIdAndType(entityId, type.name())); + mapper -> mapper.selectUserOwnerMetaByMetadataObjectIdAndType(entityId, type.name())); - if (ownerRelPO == null) { - return Optional.empty(); + if (userPO != null) { + return Optional.of( + POConverters.fromUserPO( + userPO, + Collections.emptyList(), + AuthorizationUtils.ofUserNamespace(getMetalake(identifier)))); } - switch (Entity.EntityType.valueOf(ownerRelPO.getOwnerType())) { - case USER: - return Optional.of( - UserMetaService.getInstance() - .getUserById(getMetalake(identifier), ownerRelPO.getOwnerId())); - case GROUP: - return Optional.of( - GroupMetaService.getInstance() - .getGroupById(getMetalake(identifier), ownerRelPO.getOwnerId())); - default: - throw new IllegalArgumentException( - String.format("Owner type doesn't support %s", ownerRelPO.getOwnerType())); + GroupPO groupPO = + SessionUtils.getWithoutCommit( + OwnerMetaMapper.class, + mapper -> mapper.selectGroupOwnerMetaByMetadataObjectIdAndType(entityId, type.name())); + + if (groupPO != null) { + return Optional.of( + POConverters.fromGroupPO( + groupPO, + Collections.emptyList(), + AuthorizationUtils.ofGroupNamespace(getMetalake(identifier)))); } + + return Optional.empty(); } public void setOwner( diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/UserMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/UserMetaService.java index 935ed87b9..f39386175 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/UserMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/UserMetaService.java @@ -22,7 +22,6 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import com.google.common.collect.Sets; import java.io.IOException; -import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.Optional; @@ -34,6 +33,7 @@ import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.authorization.AuthorizationUtils; import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.meta.UserEntity; +import org.apache.gravitino.storage.relational.mapper.OwnerMetaMapper; import org.apache.gravitino.storage.relational.mapper.UserMetaMapper; import org.apache.gravitino.storage.relational.mapper.UserRoleRelMapper; import org.apache.gravitino.storage.relational.po.RolePO; @@ -150,24 +150,16 @@ public class UserMetaService { UserMetaMapper.class, mapper -> mapper.softDeleteUserMetaByUserId(userId)), () -> SessionUtils.doWithoutCommit( - UserRoleRelMapper.class, mapper -> mapper.softDeleteUserRoleRelByUserId(userId))); + UserRoleRelMapper.class, mapper -> mapper.softDeleteUserRoleRelByUserId(userId)), + () -> + SessionUtils.doWithoutCommit( + OwnerMetaMapper.class, + mapper -> + mapper.softDeleteOwnerRelByOwnerIdAndType( + userId, Entity.EntityType.USER.name()))); return true; } - public UserEntity getUserById(String metalake, Long userId) { - UserPO userPO = - SessionUtils.getWithoutCommit( - UserMetaMapper.class, mapper -> mapper.selectUserMetaById(userId)); - if (userPO == null) { - throw new NoSuchEntityException( - NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE, - Entity.EntityType.USER.name().toLowerCase(), - String.valueOf(userId)); - } - return POConverters.fromUserPO( - userPO, Collections.emptyList(), AuthorizationUtils.ofUserNamespace(metalake)); - } - public <E extends Entity & HasIdentifier> UserEntity updateUser( NameIdentifier identifier, Function<E, E> updater) throws IOException { AuthorizationUtils.checkUser(identifier); diff --git a/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java b/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java index 57623ea89..bf8c57580 100644 --- a/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java +++ b/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java @@ -46,6 +46,8 @@ import org.apache.gravitino.EntityStoreFactory; import org.apache.gravitino.GravitinoEnv; import org.apache.gravitino.MetadataObject; import org.apache.gravitino.MetadataObjects; +import org.apache.gravitino.exceptions.NoSuchMetadataObjectException; +import org.apache.gravitino.exceptions.NotFoundException; import org.apache.gravitino.lock.LockManager; import org.apache.gravitino.meta.AuditInfo; import org.apache.gravitino.meta.BaseMetalake; @@ -154,6 +156,12 @@ public class TestOwnerManager { MetadataObjects.of(Lists.newArrayList(METALAKE), MetadataObject.Type.METALAKE); Assertions.assertFalse(ownerManager.getOwner(METALAKE, metalakeObject).isPresent()); + // Test not-existed metadata object + MetadataObject notExistObject = + MetadataObjects.of(Lists.newArrayList("not-exist"), MetadataObject.Type.CATALOG); + Assertions.assertThrows( + NoSuchMetadataObjectException.class, () -> ownerManager.getOwner(METALAKE, notExistObject)); + // Test to set the user as the owner ownerManager.setOwner(METALAKE, metalakeObject, USER, Owner.Type.USER); @@ -164,6 +172,11 @@ public class TestOwnerManager { // Test to set the group as the owner ownerManager.setOwner(METALAKE, metalakeObject, GROUP, Owner.Type.GROUP); + // Test not-existed metadata object + Assertions.assertThrows( + NotFoundException.class, + () -> ownerManager.setOwner(METALAKE, notExistObject, GROUP, Owner.Type.GROUP)); + owner = ownerManager.getOwner(METALAKE, metalakeObject).get(); Assertions.assertEquals(GROUP, owner.name()); Assertions.assertEquals(Owner.Type.GROUP, owner.type()); 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 240f6298e..22246ba0c 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 @@ -28,7 +28,9 @@ import java.sql.SQLException; import java.sql.Statement; import java.time.Instant; import java.util.List; +import java.util.Optional; import java.util.function.Function; +import org.apache.gravitino.Entity; import org.apache.gravitino.EntityAlreadyExistsException; import org.apache.gravitino.Namespace; import org.apache.gravitino.authorization.AuthorizationUtils; @@ -283,6 +285,7 @@ class TestGroupMetaService extends TestJDBCBackend { GroupMetaService groupMetaService = GroupMetaService.getInstance(); RoleMetaService roleMetaService = RoleMetaService.getInstance(); + OwnerMetaService ownerMetaService = OwnerMetaService.getInstance(); // delete group GroupEntity group1 = @@ -296,11 +299,25 @@ class TestGroupMetaService extends TestJDBCBackend { () -> groupMetaService.getGroupByIdentifier(group1.nameIdentifier())); Assertions.assertDoesNotThrow(() -> groupMetaService.insertGroup(group1, false)); Assertions.assertEquals(group1, groupMetaService.getGroupByIdentifier(group1.nameIdentifier())); + + // Set the owner of the metalake + ownerMetaService.setOwner( + metalake.nameIdentifier(), metalake.type(), group1.nameIdentifier(), group1.type()); + Optional<Entity> entity = ownerMetaService.getOwner(metalake.nameIdentifier(), metalake.type()); + Assertions.assertTrue(entity.isPresent()); + Assertions.assertEquals(group1, entity.get()); + + // Delete the group Assertions.assertTrue(groupMetaService.deleteGroup(group1.nameIdentifier())); + Assertions.assertThrows( NoSuchEntityException.class, () -> groupMetaService.getGroupByIdentifier(group1.nameIdentifier())); + // Test owner deletion + entity = ownerMetaService.getOwner(metalake.nameIdentifier(), metalake.type()); + Assertions.assertFalse(entity.isPresent()); + // delete group with roles RoleEntity role1 = createRoleEntity( diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestUserMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestUserMetaService.java index 1834ed16f..326ccfc2d 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestUserMetaService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestUserMetaService.java @@ -28,7 +28,9 @@ import java.sql.SQLException; import java.sql.Statement; import java.time.Instant; import java.util.List; +import java.util.Optional; import java.util.function.Function; +import org.apache.gravitino.Entity; import org.apache.gravitino.EntityAlreadyExistsException; import org.apache.gravitino.Namespace; import org.apache.gravitino.authorization.AuthorizationUtils; @@ -282,6 +284,7 @@ class TestUserMetaService extends TestJDBCBackend { UserMetaService userMetaService = UserMetaService.getInstance(); RoleMetaService roleMetaService = RoleMetaService.getInstance(); + OwnerMetaService ownerMetaService = OwnerMetaService.getInstance(); // delete user UserEntity user1 = @@ -295,11 +298,23 @@ class TestUserMetaService extends TestJDBCBackend { () -> userMetaService.getUserByIdentifier(user1.nameIdentifier())); Assertions.assertDoesNotThrow(() -> userMetaService.insertUser(user1, false)); Assertions.assertEquals(user1, userMetaService.getUserByIdentifier(user1.nameIdentifier())); + + // Set the owner of the metalake + ownerMetaService.setOwner( + metalake.nameIdentifier(), metalake.type(), user1.nameIdentifier(), user1.type()); + Optional<Entity> entity = ownerMetaService.getOwner(metalake.nameIdentifier(), metalake.type()); + Assertions.assertTrue(entity.isPresent()); + Assertions.assertEquals(user1, entity.get()); + Assertions.assertTrue(userMetaService.deleteUser(user1.nameIdentifier())); Assertions.assertThrows( NoSuchEntityException.class, () -> userMetaService.getUserByIdentifier(user1.nameIdentifier())); + // Test owner deletion + entity = ownerMetaService.getOwner(metalake.nameIdentifier(), metalake.type()); + Assertions.assertFalse(entity.isPresent()); + // delete user with roles RoleEntity role1 = createRoleEntity(