arjnklc opened a new pull request, #10816: URL: https://github.com/apache/gravitino/pull/10816
### What changes were proposed in this pull request? Guarded `AuthorizationUtils.authorizationPluginRemovePrivileges()` behind a success check in all HookDispatcher drop/purge methods: - `TableHookDispatcher.dropTable()` and `purgeTable()` - `TopicHookDispatcher.dropTopic()` - `SchemaHookDispatcher.dropSchema()` - `FilesetHookDispatcher.dropFileset()` Previously, authorization privileges were removed unconditionally, even when the underlying drop/purge operation returned `false`. ### Why are the changes needed? When a drop/purge operation fails (returns `false`), the entity still exists but its authorization privileges were already removed. This created an inconsistency between the actual entity state and its authorization metadata. Fix: #10131 ### Does this PR introduce _any_ user-facing change? No. This is an internal authorization consistency fix. The API behavior is unchanged. ### How was this patch tested? Added new unit tests following the pattern suggested in the issue: - `testDropTableShouldNotRemovePrivilegesWhenDropFails` - `testPurgeTableShouldNotRemovePrivilegesWhenPurgeFails` - `testDropTopicShouldNotRemovePrivilegesWhenDropFails` - `testDropFilesetShouldNotRemovePrivilegesWhenDropFails` Each test mocks the dispatcher to return `false` and verifies that `authorizationPluginRemovePrivileges` is never called. ``` ./gradlew :core:test --tests "org.apache.gravitino.hook.TestTableHookDispatcher" --tests "org.apache.gravitino.hook.TestTopicHookDispatcher" --tests "org.apache.gravitino.hook.TestFilesetHookDispatcher" -PskipITs ``` All tests pass. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
