yuqi1129 opened a new pull request, #12876: URL: https://github.com/apache/gravitino/pull/12876
### What changes were proposed in this pull request? Adds `DROP_TABLE_AUTHORIZATION_EXPRESSION` to `AuthorizationExpressionConstants` and replaces 13 hand-written copies of three table authorization expressions with references to the shared constants: | Sites | Constant | | --- | --- | | `TableOperations#dropTable`, `IcebergTableOperations#dropTable`, `RenameTableAuthzHandler` | `DROP_TABLE_AUTHORIZATION_EXPRESSION` (new) | | `IcebergTableOperations#updateTable`, `IcebergTableRenameOperations`, four `StatisticOperations` endpoints | `MODIFY_TABLE_AUTHORIZATION_EXPRESSION` (existing) | | `IcebergTableOperations` credentials and plan, two `StatisticOperations` endpoints | `LOAD_TABLE_AUTHORIZATION_EXPRESSION` (existing) | Also removes a local variable in `RenameTableAuthzHandler` that only aliased the expression. This is behavior preserving. No privilege rule changes. Lance REST has a fourth copy of the removal expression, added in #12696, which is not merged yet. The two PRs are independent; whichever lands second, the Lance copy will be pointed at the same shared constant in a follow-up so all four surfaces read from one place. Fix: #12875 ### Why are the changes needed? A privilege change to one of these rules currently has to be repeated in every copy, and missing one is a silent authorization difference between the Gravitino, Iceberg REST and Lance REST surfaces rather than a visible failure. Two copies had already drifted in formatting (`ANY_SELECT_TABLE|| ANY_MODIFY_TABLE`, missing the space), which is evidence that they are edited independently. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? The expressions were compared before and after rather than by eye. A throwaway test ran each removed literal and its replacement constant through `AuthorizationExpressionConverter.convertToOgnlExpression`, asserted the results are identical ignoring whitespace, and parsed both with `Ognl.parseExpression` to confirm they are still valid OGNL. Whitespace around operators turned out to be the only difference, from the two `StatisticOperations` copies that were missing a space before `||`. `./gradlew :server-common:test :server:test :iceberg:iceberg-rest-server:test` — 1119 tests, all passing. `TestHttpsServerAuthentication` is a pre-existing flake and is unrelated: a different method of that class fails the same way on a branch that does not touch `server-common` at all. -- 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]
