(gravitino) branch main updated: [#13397] fix(core): notify the authorizer when setOwners sets the first owner (#13398)

[jshao]

This is an automated email from the ASF dual-hosted git repository.

jerryshao 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 9a8a847e40 [#13397] fix(core): notify the authorizer when setOwners 
sets the first owner (#13398)
9a8a847e40 is described below

commit 9a8a847e403802216919b1f8379e042802768462
Author: YangJie <yangji...@baidu.com>
AuthorDate: Thu Sep 24 02:11:59 2026 -0400

    [#13397] fix(core): notify the authorizer when setOwners sets the first 
owner (#13398)
    
    ### What changes were proposed in this pull request?
    
    `OwnerManager.setOwners` now calls
    `notifyOwnerChange(originOwner.orElse(null), metalake, mo)`
    unconditionally, instead of only when a previous owner exists. This
    aligns the batch path with the single-object `setOwner` path.
    
    ### Why are the changes needed?
    
    The batch path skipped the authorizer notification when setting the
    first owner on an ownerless object, for example the automatic owner
    assignment when a schema is created, so a custom `GravitinoAuthorizer`
    never received `handleMetadataOwnerChange` for that case. The SPI
    contract documents `oldOwnerId` as `null` on the first owner, and the
    single-object path already behaves that way.
    
    Fix: #13397
    
    ### Does this PR introduce _any_ user-facing change?
    
    No.
    
    ### How was this patch tested?
    
    Added `TestOwnerManager.testInitialOwnerSetByBatchNotifiesAuthorizer`,
    which pins that a first-owner assignment through `setOwners` notifies
    the authorizer with a `null` old owner. It fails against the pre-fix
    code.
    
    Co-authored-by: Jerry Shao <jerrys...@datastrato.com>
---
 .../gravitino/authorization/OwnerManager.java      |  2 +-
 .../gravitino/authorization/TestOwnerManager.java  | 39 ++++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

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 af83b65806..fc27c94718 100644
--- a/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java
+++ b/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java
@@ -210,7 +210,7 @@ public class OwnerManager implements OwnerDispatcher {
               mo,
               authorizationPlugin ->
                   authorizationPlugin.onOwnerSet(mo, originOwner.orElse(null), 
newOwner));
-          originOwner.ifPresent(owner -> notifyOwnerChange(owner, metalake, 
mo));
+          notifyOwnerChange(originOwner.orElse(null), metalake, mo);
         } catch (RuntimeException re) {
           LOG.warn(
               "Failed to notify authorization plugin for metadata object {} 
during batch setOwners",
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 9bdb976327..832b00c993 100644
--- 
a/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java
+++ 
b/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java
@@ -290,4 +290,43 @@ public class TestOwnerManager {
           GravitinoEnv.getInstance(), "gravitinoAuthorizer", 
originalAuthorizer, true);
     }
   }
+
+  @Test
+  @Order(5)
+  public void testInitialOwnerSetByBatchNotifiesAuthorizer()
+      throws IllegalAccessException, IOException {
+    String catalogName = "catalog_initial_owner_notify_batch";
+    AuditInfo audit = 
AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build();
+    CatalogEntity catalog =
+        CatalogEntity.builder()
+            .withId(idGenerator.nextId())
+            .withName(catalogName)
+            .withNamespace(Namespace.of(METALAKE))
+            .withType(Catalog.Type.RELATIONAL)
+            .withProvider("test")
+            .withAuditInfo(audit)
+            .build();
+    entityStore.put(catalog, false);
+
+    GravitinoAuthorizer authorizer = Mockito.mock(GravitinoAuthorizer.class);
+    GravitinoAuthorizer originalAuthorizer = 
GravitinoEnv.getInstance().gravitinoAuthorizer();
+    FieldUtils.writeField(GravitinoEnv.getInstance(), "gravitinoAuthorizer", 
authorizer, true);
+    try {
+      MetadataObject catalogObject =
+          MetadataObjects.of(Lists.newArrayList(catalogName), 
MetadataObject.Type.CATALOG);
+
+      ownerManager.setOwners(
+          METALAKE, Collections.singletonList(catalogObject), USER, 
Owner.Type.USER);
+
+      Mockito.verify(authorizer)
+          .handleMetadataOwnerChange(
+              Mockito.eq(METALAKE),
+              Mockito.isNull(),
+              Mockito.eq(NameIdentifier.of(METALAKE, catalogName)),
+              Mockito.eq(Entity.EntityType.CATALOG));
+    } finally {
+      FieldUtils.writeField(
+          GravitinoEnv.getInstance(), "gravitinoAuthorizer", 
originalAuthorizer, true);
+    }
+  }
 }