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 <ror...@apache.org>
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 {

Reply via email to