apache/gravitino

[Improvement] TableHookDispatcher removes auth privileges when dropTable fails

Open

#10.131 aperta il 3 mar 2026

Vedi su GitHub
 (5 commenti) (0 reazioni) (1 assegnatario)Java (887 fork)auto 404
good first issueimprovement

Metriche repository

Star
 (3058 star)
Metriche merge PR
 (Metriche PR in attesa)

Descrizione

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.

Guida contributor