address comment

apache · Jan 2, 2025 · 7ce4635 · 7ce4635
1 parent 6f7968b
commit 7ce4635
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 27 deletions.
diff --git a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
@@ -25,10 +25,12 @@
 import java.util.Set;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
+
 import org.apache.gravitino.Catalog;
 import org.apache.gravitino.Entity;
 import org.apache.gravitino.GravitinoEnv;
 import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.MetadataObjects;
 import org.apache.gravitino.NameIdentifier;
 import org.apache.gravitino.Namespace;
 import org.apache.gravitino.catalog.CatalogManager;
@@ -257,30 +259,18 @@ public static void authorizationPluginRemovePrivileges(
     }
   }
 
-  public static Runnable createRemovePrivilegesCallback(
-      NameIdentifier ident, Entity.EntityType type) {
+  public static void removeCatalogPrivileges(Catalog catalog) {
     // If we enable authorization, we should remove the privileges about the entity in the
     // authorization plugin.
-    if (GravitinoEnv.getInstance().accessControlDispatcher() != null) {
-      MetadataObject metadataObject = NameIdentifierUtil.toMetadataObject(ident, type);
-      MetadataObjectChange removeObject = MetadataObjectChange.remove(metadataObject);
-
-      String metalake =
-          type == Entity.EntityType.METALAKE ? ident.name() : ident.namespace().level(0);
-
-      List<Catalog> catalogs = loadMetadataObjectCatalog(metalake, metadataObject);
-
-      return () -> {
-        for (Catalog catalog : catalogs) {
-          callAuthorizationPluginImpl(
-              authorizationPlugin -> {
-                authorizationPlugin.onMetadataUpdated(removeObject);
-              },
-              catalog);
-        }
-      };
-    }
-    return null;
+    MetadataObject metadataObject =
+        MetadataObjects.of(null, catalog.name(), MetadataObject.Type.CATALOG);
+    MetadataObjectChange removeObject = MetadataObjectChange.remove(metadataObject);
+
+    callAuthorizationPluginImpl(
+        authorizationPlugin -> {
+          authorizationPlugin.onMetadataUpdated(removeObject);
+        },
+        catalog);
   }
 
   public static void authorizationPluginRenamePrivileges(

diff --git a/core/src/main/java/org/apache/gravitino/hook/CatalogHookDispatcher.java b/core/src/main/java/org/apache/gravitino/hook/CatalogHookDispatcher.java
@@ -128,16 +128,15 @@ public boolean dropCatalog(NameIdentifier ident, boolean force)
       throws NonEmptyEntityException, CatalogInUseException {
     // If we call the authorization plugin after dropping catalog, we can't load the plugin of the
     // catalog
-    Runnable removePrivilegesCallback = null;
+    Catalog catalog = null;
     if (dispatcher.catalogExists(ident)) {
-      removePrivilegesCallback =
-          AuthorizationUtils.createRemovePrivilegesCallback(ident, Entity.EntityType.CATALOG);
+      catalog = dispatcher.loadCatalog(ident);
     }
 
     boolean dropped = dispatcher.dropCatalog(ident, force);
 
-    if (dropped && removePrivilegesCallback != null) {
-      removePrivilegesCallback.run();
+    if (dropped && catalog != null) {
+      AuthorizationUtils.removeCatalogPrivileges(catalog);
     }
     return dropped;
   }