apache/gravitino

[Improvement] TableHookDispatcher removes auth privileges when dropTable fails

Open

#10,131 创建于 2026年3月3日

在 GitHub 查看
 (5 评论) (0 反应) (1 负责人)Java (887 fork)auto 404
good first issueimprovement

仓库指标

Star
 (3,058 star)
PR 合并指标
 (PR 指标待抓取)

描述

What would you like to be improved?

TableHookDispatcher.dropTable(...) and purgeTable(...) call AuthorizationUtils.authorizationPluginRemovePrivileges(...) unconditionally. If the underlying dispatcher returns false (table not dropped/purged), privileges are still removed, causing authorization metadata to become inconsistent with the table's actual state.

How should we improve?

A possible solution is to guard privilege removal behind operation success:

  • In dropTable(...), call authorizationPluginRemovePrivileges(...) only when dropped == true.
  • In purgeTable(...), call authorizationPluginRemovePrivileges(...) only when purged == true.

Here's a unit test to help:

@Test
public void testDropTableShouldNotRemovePrivilegesWhenDropFails() {
  TableDispatcher dispatcher = Mockito.mock(TableDispatcher.class);
  TableHookDispatcher hookDispatcher = new TableHookDispatcher(dispatcher);
  NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "schema", "table");

  Mockito.when(dispatcher.dropTable(ident)).thenReturn(false);

  try (MockedStatic<AuthorizationUtils> authz = Mockito.mockStatic(AuthorizationUtils.class)) {
    authz
        .when(() -> AuthorizationUtils.getMetadataObjectLocation(ident, Entity.EntityType.TABLE))
        .thenReturn(ImmutableList.of("/test"));

    boolean dropped = hookDispatcher.dropTable(ident);

    Assertions.assertFalse(dropped);
    authz.verify(
        () ->
            AuthorizationUtils.authorizationPluginRemovePrivileges(
                ident, Entity.EntityType.TABLE, ImmutableList.of("/test")),
        Mockito.never());
  }
}

Place in core/src/test/java/org/apache/gravitino/hook/TestTableHookDispatcher.java. Some imports will also need to be added.

贡献者指南