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 b8a845229 [#3963][followup] feat(core): Call the method ownerset of
authorization plugin (#4623)
b8a845229 is described below
commit b8a845229ba491c175319994d5a36c6d31aafc3d
Author: roryqi <[email protected]>
AuthorDate: Thu Aug 22 19:29:20 2024 +0800
[#3963][followup] feat(core): Call the method ownerset of authorization
plugin (#4623)
### What changes were proposed in this pull request?
This is the follow-up pull request of #3963.
#3963 add authorization plugin to set owner. This pull request is to
call them.
### Why are the changes needed?
Fix: #3963
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Add new UT.
---
.../java/org/apache/gravitino/GravitinoEnv.java | 2 +-
.../authorization/AuthorizationUtils.java | 55 +++++++++++++++-------
.../authorization/FutureGrantManager.java | 19 +++++++-
.../gravitino/authorization/OwnerManager.java | 32 +++++++++----
.../gravitino/authorization/PermissionManager.java | 8 ++--
.../gravitino/authorization/RoleManager.java | 4 +-
.../apache/gravitino/connector/BaseCatalog.java | 5 +-
.../authorization/TestFutureGrantManager.java | 27 ++++++++++-
.../gravitino/authorization/TestOwnerManager.java | 16 +++++++
.../gravitino/server/web/rest/OwnerOperations.java | 5 +-
10 files changed, 133 insertions(+), 40 deletions(-)
diff --git a/core/src/main/java/org/apache/gravitino/GravitinoEnv.java
b/core/src/main/java/org/apache/gravitino/GravitinoEnv.java
index 04de93186..a27df8710 100644
--- a/core/src/main/java/org/apache/gravitino/GravitinoEnv.java
+++ b/core/src/main/java/org/apache/gravitino/GravitinoEnv.java
@@ -426,7 +426,7 @@ public class GravitinoEnv {
this.accessControlDispatcher = accessControlHookDispatcher;
this.ownerManager = new OwnerManager(entityStore);
- this.futureGrantManager = new FutureGrantManager(entityStore);
+ this.futureGrantManager = new FutureGrantManager(entityStore,
ownerManager);
} else {
this.accessControlDispatcher = null;
this.ownerManager = null;
diff --git
a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
index 6dd42e628..c182980b8 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
@@ -140,45 +140,62 @@ public class AuthorizationUtils {
// Every catalog has one authorization plugin, we should avoid calling
// underlying authorization repeatedly. So we use a set to record which
// catalog has been called the authorization plugin.
- public static void callAuthorizationPlugin(
+ public static void callAuthorizationPluginForSecurableObjects(
String metalake,
List<SecurableObject> securableObjects,
Set<String> catalogsAlreadySet,
Consumer<AuthorizationPlugin> consumer) {
CatalogManager catalogManager =
GravitinoEnv.getInstance().catalogManager();
for (SecurableObject securableObject : securableObjects) {
- if (needApplyAllAuthorizationPlugin(securableObject)) {
+ if (needApplyAuthorizationPluginAllCatalogs(securableObject)) {
Catalog[] catalogs =
catalogManager.listCatalogsInfo(Namespace.of(metalake));
for (Catalog catalog : catalogs) {
- callAuthorizationPluginImpl(catalogsAlreadySet, consumer, catalog);
+ callAuthorizationPluginImpl(consumer, catalog);
}
- } else if (supportsSingleAuthorizationPlugin(securableObject.type())) {
+ } else if (needApplyAuthorization(securableObject.type())) {
NameIdentifier catalogIdent =
NameIdentifierUtil.getCatalogIdentifier(
MetadataObjectUtil.toEntityIdent(metalake, securableObject));
Catalog catalog = catalogManager.loadCatalog(catalogIdent);
- callAuthorizationPluginImpl(catalogsAlreadySet, consumer, catalog);
+ if (!catalogsAlreadySet.contains(catalog.name())) {
+ catalogsAlreadySet.add(catalog.name());
+ callAuthorizationPluginImpl(consumer, catalog);
+ }
+ }
+ }
+ }
+
+ public static void callAuthorizationPluginForMetadataObject(
+ String metalake, MetadataObject metadataObject,
Consumer<AuthorizationPlugin> consumer) {
+ CatalogManager catalogManager =
GravitinoEnv.getInstance().catalogManager();
+ if (needApplyAuthorizationPluginAllCatalogs(metadataObject.type())) {
+ Catalog[] catalogs =
catalogManager.listCatalogsInfo(Namespace.of(metalake));
+ for (Catalog catalog : catalogs) {
+ callAuthorizationPluginImpl(consumer, catalog);
}
+ } else if (needApplyAuthorization(metadataObject.type())) {
+ NameIdentifier catalogIdent =
+ NameIdentifierUtil.getCatalogIdentifier(
+ MetadataObjectUtil.toEntityIdent(metalake, metadataObject));
+ Catalog catalog = catalogManager.loadCatalog(catalogIdent);
+ callAuthorizationPluginImpl(consumer, catalog);
}
}
private static void callAuthorizationPluginImpl(
- Set<String> catalogsAlreadySet, Consumer<AuthorizationPlugin> consumer,
Catalog catalog) {
- if (!catalogsAlreadySet.contains(catalog.name())) {
- catalogsAlreadySet.add(catalog.name());
-
- if (catalog instanceof BaseCatalog) {
- BaseCatalog baseCatalog = (BaseCatalog) catalog;
- if (baseCatalog.getAuthorizationPlugin() != null) {
- consumer.accept(baseCatalog.getAuthorizationPlugin());
- }
+ Consumer<AuthorizationPlugin> consumer, Catalog catalog) {
+
+ if (catalog instanceof BaseCatalog) {
+ BaseCatalog baseCatalog = (BaseCatalog) catalog;
+ if (baseCatalog.getAuthorizationPlugin() != null) {
+ consumer.accept(baseCatalog.getAuthorizationPlugin());
}
}
}
- public static boolean needApplyAllAuthorizationPlugin(SecurableObject
securableObject) {
- // TODO: Add supportsSecurableObjects method for every privilege to
simplify this code
+ public static boolean
needApplyAuthorizationPluginAllCatalogs(SecurableObject securableObject) {
+ // TODO: Add `supportsSecurableObjects` method for every privilege to
simplify this code
if (securableObject.type() == MetadataObject.Type.METALAKE) {
List<Privilege> privileges = securableObject.privileges();
for (Privilege privilege : privileges) {
@@ -190,7 +207,11 @@ public class AuthorizationUtils {
return false;
}
- private static boolean supportsSingleAuthorizationPlugin(MetadataObject.Type
type) {
+ private static boolean
needApplyAuthorizationPluginAllCatalogs(MetadataObject.Type type) {
+ return type == MetadataObject.Type.METALAKE;
+ }
+
+ private static boolean needApplyAuthorization(MetadataObject.Type type) {
return type != MetadataObject.Type.ROLE && type !=
MetadataObject.Type.METALAKE;
}
}
diff --git
a/core/src/main/java/org/apache/gravitino/authorization/FutureGrantManager.java
b/core/src/main/java/org/apache/gravitino/authorization/FutureGrantManager.java
index 1745d53d9..c24817ea5 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/FutureGrantManager.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/FutureGrantManager.java
@@ -23,10 +23,13 @@ import com.google.common.collect.Maps;
import java.io.IOException;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import org.apache.gravitino.Entity;
import org.apache.gravitino.EntityStore;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.MetadataObjects;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.SupportsRelationOperations;
import org.apache.gravitino.connector.BaseCatalog;
@@ -45,13 +48,25 @@ import org.glassfish.jersey.internal.guava.Sets;
*/
public class FutureGrantManager {
EntityStore entityStore;
+ OwnerManager ownerManager;
- public FutureGrantManager(EntityStore entityStore) {
+ public FutureGrantManager(EntityStore entityStore, OwnerManager
ownerManager) {
this.entityStore = entityStore;
+ this.ownerManager = ownerManager;
}
public void grantNewlyCreatedCatalog(String metalake, BaseCatalog catalog) {
try {
+ MetadataObject metalakeObject =
+ MetadataObjects.of(null, metalake, MetadataObject.Type.METALAKE);
+ Optional<Owner> ownerOptional = ownerManager.getOwner(metalake,
metalakeObject);
+ ownerOptional.ifPresent(
+ owner -> {
+ AuthorizationPlugin authorizationPlugin =
catalog.getAuthorizationPlugin();
+ if (authorizationPlugin != null) {
+ authorizationPlugin.onOwnerSet(metalakeObject, null, owner);
+ }
+ });
Map<UserEntity, Set<RoleEntity>> userGrantRoles = Maps.newHashMap();
Map<GroupEntity, Set<RoleEntity>> groupGrantRoles = Maps.newHashMap();
@@ -69,7 +84,7 @@ public class FutureGrantManager {
boolean supportsFutureGrant = false;
for (SecurableObject object : role.securableObjects()) {
- if (AuthorizationUtils.needApplyAllAuthorizationPlugin(object)) {
+ if
(AuthorizationUtils.needApplyAuthorizationPluginAllCatalogs(object)) {
supportsFutureGrant = true;
break;
}
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 ec1b26438..04285954e 100644
--- a/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java
+++ b/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java
@@ -68,7 +68,11 @@ public class OwnerManager {
public void setOwner(
String metalake, MetadataObject metadataObject, String ownerName,
Owner.Type ownerType) {
try {
+ Optional<Owner> originOwner = getOwner(metalake, metadataObject);
+
NameIdentifier objectIdent = MetadataObjectUtil.toEntityIdent(metalake,
metadataObject);
+ OwnerImpl newOwner = new OwnerImpl();
+
if (ownerType == Owner.Type.USER) {
NameIdentifier ownerIdent = AuthorizationUtils.ofUser(metalake,
ownerName);
TreeLockUtils.doWithTreeLock(
@@ -86,6 +90,9 @@ public class OwnerManager {
true);
return null;
});
+
+ newOwner.name = ownerName;
+ newOwner.type = Owner.Type.USER;
} else if (ownerType == Owner.Type.GROUP) {
NameIdentifier ownerIdent = AuthorizationUtils.ofGroup(metalake,
ownerName);
TreeLockUtils.doWithTreeLock(
@@ -103,7 +110,16 @@ public class OwnerManager {
true);
return null;
});
+
+ newOwner.name = ownerName;
+ newOwner.type = Owner.Type.GROUP;
}
+
+ AuthorizationUtils.callAuthorizationPluginForMetadataObject(
+ metalake,
+ metadataObject,
+ authorizationPlugin ->
+ authorizationPlugin.onOwnerSet(metadataObject,
originOwner.orElse(null), newOwner));
} catch (NoSuchEntityException nse) {
LOG.warn(
"Metadata object {} or owner {} is not found",
metadataObject.fullName(), ownerName, nse);
@@ -124,16 +140,12 @@ public class OwnerManager {
OwnerImpl owner = new OwnerImpl();
NameIdentifier ident = MetadataObjectUtil.toEntityIdent(metalake,
metadataObject);
List<? extends Entity> entities =
- TreeLockUtils.doWithTreeLock(
- ident,
- LockType.READ,
- () ->
- store
- .relationOperations()
- .listEntitiesByRelation(
- SupportsRelationOperations.Type.OWNER_REL,
- ident,
- MetadataObjectUtil.toEntityType(metadataObject)));
+ store
+ .relationOperations()
+ .listEntitiesByRelation(
+ SupportsRelationOperations.Type.OWNER_REL,
+ ident,
+ MetadataObjectUtil.toEntityType(metadataObject));
if (entities.isEmpty()) {
return Optional.empty();
diff --git
a/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java
b/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java
index 01c87d030..edb02cdce 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java
@@ -112,7 +112,7 @@ class PermissionManager {
Set<String> catalogs = Sets.newHashSet();
for (Role grantedRole : roleEntitiesToGrant) {
- AuthorizationUtils.callAuthorizationPlugin(
+ AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
grantedRole.securableObjects(),
catalogs,
@@ -191,7 +191,7 @@ class PermissionManager {
Set<String> catalogs = Sets.newHashSet();
for (Role grantedRole : roleEntitiesToGrant) {
- AuthorizationUtils.callAuthorizationPlugin(
+ AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
grantedRole.securableObjects(),
catalogs,
@@ -269,7 +269,7 @@ class PermissionManager {
Set<String> catalogs = Sets.newHashSet();
for (Role grantedRole : roleEntitiesToRevoke) {
- AuthorizationUtils.callAuthorizationPlugin(
+ AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
grantedRole.securableObjects(),
catalogs,
@@ -349,7 +349,7 @@ class PermissionManager {
Set<String> catalogs = Sets.newHashSet();
for (Role grantedRole : roleEntitiesToRevoke) {
- AuthorizationUtils.callAuthorizationPlugin(
+ AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
grantedRole.securableObjects(),
catalogs,
diff --git
a/core/src/main/java/org/apache/gravitino/authorization/RoleManager.java
b/core/src/main/java/org/apache/gravitino/authorization/RoleManager.java
index b1539d019..3edcfaa57 100644
--- a/core/src/main/java/org/apache/gravitino/authorization/RoleManager.java
+++ b/core/src/main/java/org/apache/gravitino/authorization/RoleManager.java
@@ -109,7 +109,7 @@ class RoleManager {
store.put(roleEntity, false /* overwritten */);
cache.put(roleEntity.nameIdentifier(), roleEntity);
- AuthorizationUtils.callAuthorizationPlugin(
+ AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
roleEntity.securableObjects(),
Sets.newHashSet(),
@@ -145,7 +145,7 @@ class RoleManager {
try {
RoleEntity roleEntity = store.get(ident, Entity.EntityType.ROLE,
RoleEntity.class);
- AuthorizationUtils.callAuthorizationPlugin(
+ AuthorizationUtils.callAuthorizationPluginForSecurableObjects(
metalake,
roleEntity.securableObjects(),
Sets.newHashSet(),
diff --git a/core/src/main/java/org/apache/gravitino/connector/BaseCatalog.java
b/core/src/main/java/org/apache/gravitino/connector/BaseCatalog.java
index 56c8f7920..45a7b9098 100644
--- a/core/src/main/java/org/apache/gravitino/connector/BaseCatalog.java
+++ b/core/src/main/java/org/apache/gravitino/connector/BaseCatalog.java
@@ -196,7 +196,10 @@ public abstract class BaseCatalog<T extends BaseCatalog>
private BaseAuthorization<?> createAuthorizationPluginInstance() {
String authorizationProvider =
- (String) catalogPropertiesMetadata().getOrDefault(conf,
AUTHORIZATION_PROVIDER);
+ catalogPropertiesMetadata().containsProperty(AUTHORIZATION_PROVIDER)
+ ? (String) catalogPropertiesMetadata().getOrDefault(conf,
AUTHORIZATION_PROVIDER)
+ : null;
+
if (authorizationProvider == null) {
LOG.info("Authorization provider is not set!");
return null;
diff --git
a/core/src/test/java/org/apache/gravitino/authorization/TestFutureGrantManager.java
b/core/src/test/java/org/apache/gravitino/authorization/TestFutureGrantManager.java
index 87acd0223..b07e42ac7 100644
---
a/core/src/test/java/org/apache/gravitino/authorization/TestFutureGrantManager.java
+++
b/core/src/test/java/org/apache/gravitino/authorization/TestFutureGrantManager.java
@@ -30,6 +30,7 @@ import com.google.common.collect.Lists;
import java.io.IOException;
import java.time.Instant;
import java.util.Collections;
+import java.util.Optional;
import org.apache.gravitino.Entity;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.MetadataObject;
@@ -49,6 +50,7 @@ import org.junit.jupiter.api.Test;
public class TestFutureGrantManager {
private static EntityStore entityStore = mock(EntityStore.class);
+ private static OwnerManager ownerManager = mock(OwnerManager.class);
private static String METALAKE = "metalake";
private static AuthorizationPlugin authorizationPlugin;
private static BaseMetalake metalakeEntity =
@@ -72,10 +74,11 @@ public class TestFutureGrantManager {
@Test
void testGrantNormally() throws IOException {
- FutureGrantManager manager = new FutureGrantManager(entityStore);
+ FutureGrantManager manager = new FutureGrantManager(entityStore,
ownerManager);
SupportsRelationOperations relationOperations =
mock(SupportsRelationOperations.class);
when(entityStore.relationOperations()).thenReturn(relationOperations);
+ when(ownerManager.getOwner(any(), any())).thenReturn(Optional.empty());
// test no securable objects
RoleEntity roleEntity = mock(RoleEntity.class);
@@ -100,9 +103,25 @@ public class TestFutureGrantManager {
manager.grantNewlyCreatedCatalog(METALAKE, catalog);
verify(authorizationPlugin, never()).onGrantedRolesToUser(any(), any());
verify(authorizationPlugin, never()).onGrantedRolesToGroup(any(), any());
+ verify(authorizationPlugin, never()).onOwnerSet(any(), any(), any());
// test only grant users
reset(authorizationPlugin);
+ when(ownerManager.getOwner(any(), any()))
+ .thenReturn(
+ Optional.of(
+ new Owner() {
+ @Override
+ public String name() {
+ return "test";
+ }
+
+ @Override
+ public Type type() {
+ return Type.USER;
+ }
+ }));
+
SecurableObject securableObject = mock(SecurableObject.class);
when(securableObject.type()).thenReturn(MetadataObject.Type.METALAKE);
when(securableObject.privileges())
@@ -111,6 +130,7 @@ public class TestFutureGrantManager {
when(roleEntity.nameIdentifier()).thenReturn(AuthorizationUtils.ofRole(METALAKE,
"role1"));
manager.grantNewlyCreatedCatalog(METALAKE, catalog);
+ verify(authorizationPlugin).onOwnerSet(any(), any(), any());
verify(authorizationPlugin).onGrantedRolesToUser(any(), any());
verify(authorizationPlugin, never()).onGrantedRolesToGroup(any(), any());
@@ -128,6 +148,7 @@ public class TestFutureGrantManager {
Entity.EntityType.ROLE))
.thenReturn(Lists.newArrayList(groupEntity));
manager.grantNewlyCreatedCatalog(METALAKE, catalog);
+ verify(authorizationPlugin).onOwnerSet(any(), any(), any());
verify(authorizationPlugin, never()).onGrantedRolesToUser(any(), any());
verify(authorizationPlugin).onGrantedRolesToGroup(any(), any());
@@ -144,6 +165,7 @@ public class TestFutureGrantManager {
Entity.EntityType.ROLE))
.thenReturn(Lists.newArrayList(groupEntity));
manager.grantNewlyCreatedCatalog(METALAKE, catalog);
+ verify(authorizationPlugin).onOwnerSet(any(), any(), any());
verify(authorizationPlugin).onGrantedRolesToUser(any(), any());
verify(authorizationPlugin).onGrantedRolesToGroup(any(), any());
@@ -152,13 +174,14 @@ public class TestFutureGrantManager {
when(securableObject.privileges())
.thenReturn(Lists.newArrayList(Privileges.CreateCatalog.allow()));
manager.grantNewlyCreatedCatalog(METALAKE, catalog);
+ verify(authorizationPlugin).onOwnerSet(any(), any(), any());
verify(authorizationPlugin, never()).onGrantedRolesToUser(any(), any());
verify(authorizationPlugin, never()).onGrantedRolesToGroup(any(), any());
}
@Test
void testGrantWithException() throws IOException {
- FutureGrantManager manager = new FutureGrantManager(entityStore);
+ FutureGrantManager manager = new FutureGrantManager(entityStore,
ownerManager);
SupportsRelationOperations relationOperations =
mock(SupportsRelationOperations.class);
when(entityStore.relationOperations()).thenReturn(relationOperations);
doThrow(new IOException("mock error"))
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 bf8c57580..83a562f64 100644
---
a/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java
+++
b/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java
@@ -40,12 +40,16 @@ import java.util.Collections;
import java.util.UUID;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.reflect.FieldUtils;
+import org.apache.gravitino.Catalog;
import org.apache.gravitino.Config;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.EntityStoreFactory;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.MetadataObjects;
+import org.apache.gravitino.catalog.CatalogManager;
+import org.apache.gravitino.connector.BaseCatalog;
+import org.apache.gravitino.connector.authorization.AuthorizationPlugin;
import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
import org.apache.gravitino.exceptions.NotFoundException;
import org.apache.gravitino.lock.LockManager;
@@ -75,6 +79,8 @@ public class TestOwnerManager {
private static IdGenerator idGenerator;
private static OwnerManager ownerManager;
+ private static CatalogManager catalogManager =
Mockito.mock(CatalogManager.class);
+ private static AuthorizationPlugin authorizationPlugin =
Mockito.mock(AuthorizationPlugin.class);
@BeforeAll
public static void setUp() throws IOException, IllegalAccessException {
@@ -98,6 +104,7 @@ public class TestOwnerManager {
Mockito.doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL);
FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new
LockManager(config), true);
+ FieldUtils.writeField(GravitinoEnv.getInstance(), "catalogManager",
catalogManager, true);
entityStore = EntityStoreFactory.createEntityStore(config);
entityStore.initialize(config);
@@ -137,6 +144,10 @@ public class TestOwnerManager {
entityStore.put(groupEntity, false /* overwritten*/);
ownerManager = new OwnerManager(entityStore);
+ BaseCatalog catalog = Mockito.mock(BaseCatalog.class);
+ Mockito.when(catalogManager.listCatalogsInfo(Mockito.any()))
+ .thenReturn(new Catalog[] {catalog});
+
Mockito.when(catalog.getAuthorizationPlugin()).thenReturn(authorizationPlugin);
}
@AfterAll
@@ -155,6 +166,8 @@ public class TestOwnerManager {
MetadataObject metalakeObject =
MetadataObjects.of(Lists.newArrayList(METALAKE),
MetadataObject.Type.METALAKE);
Assertions.assertFalse(ownerManager.getOwner(METALAKE,
metalakeObject).isPresent());
+ Mockito.verify(authorizationPlugin, Mockito.never())
+ .onOwnerSet(Mockito.any(), Mockito.any(), Mockito.any());
// Test not-existed metadata object
MetadataObject notExistObject =
@@ -164,13 +177,16 @@ public class TestOwnerManager {
// Test to set the user as the owner
ownerManager.setOwner(METALAKE, metalakeObject, USER, Owner.Type.USER);
+ Mockito.verify(authorizationPlugin).onOwnerSet(Mockito.any(),
Mockito.any(), Mockito.any());
Owner owner = ownerManager.getOwner(METALAKE, metalakeObject).get();
Assertions.assertEquals(USER, owner.name());
Assertions.assertEquals(Owner.Type.USER, owner.type());
// Test to set the group as the owner
+ Mockito.reset(authorizationPlugin);
ownerManager.setOwner(METALAKE, metalakeObject, GROUP, Owner.Type.GROUP);
+ Mockito.verify(authorizationPlugin).onOwnerSet(Mockito.any(),
Mockito.any(), Mockito.any());
// Test not-existed metadata object
Assertions.assertThrows(
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java
index fb5e69198..d4fe66b7f 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java
@@ -78,7 +78,10 @@ public class OwnerOperations {
return Utils.doAs(
httpRequest,
() -> {
- Optional<Owner> owner = ownerManager.getOwner(metalake, object);
+ NameIdentifier ident = MetadataObjectUtil.toEntityIdent(metalake,
object);
+ Optional<Owner> owner =
+ TreeLockUtils.doWithTreeLock(
+ ident, LockType.READ, () ->
ownerManager.getOwner(metalake, object));
if (owner.isPresent()) {
return Utils.ok(new
OwnerResponse(DTOConverters.toDTO(owner.get())));
} else {
