[Cherry-pick to branch-1.3] [#12879] fix(server): return accurate HTTP statuses for unsupported operations (#12880) - #12924
Merged
Conversation
…perations (#12880) ### What changes were proposed in this pull request? - Classify REST failures by semantics at the common exception-handling boundary: - Return HTTP 400 when statistics endpoints receive a non-table object type. - Return HTTP 409 when an operation conflicts with object state, including unmodifiable statistics and non-empty IdP groups. - Return HTTP 501 for recognized but unsupported server or connector capabilities, including handlers that use the common fallback. - Preserve the authorization-disabled filter's existing HTTP 405 contract through a dedicated helper instead of sharing the unsupported-capability helper. - Align the built-in IdP mapper so unsupported operations return 501 and unexpected `IllegalStateException` returns 500. - Keep the existing application error codes so Java clients continue reconstructing the same exceptions. - Update OpenAPI contracts and make Web v2 recognize unsupported operations by application error code, with HTTP 501 as a fallback, so it works with both old and new servers. ### Why are the changes needed? HTTP 405 means the request method is not supported by the target resource. The reported failures use valid registered methods and instead represent invalid input, resource conflicts, or unavailable functionality. The old mapping was also inconsistent because resource handlers without a dedicated branch returned HTTP 500 for the same unsupported-capability exception. The fix centralizes the fallback while classifying more specific domain exceptions before the generic `UnsupportedOperationException` branch. Fix: #12879 ### Does this PR introduce _any_ user-facing change? Yes. Invalid statistics object types return HTTP 400, state conflicts return HTTP 409, and unsupported REST capabilities return HTTP 501. Response payload codes remain compatible. The authorization-disabled filter keeps its existing HTTP 405 behavior. ### How was this patch tested? - `./gradlew spotlessApply` - `./gradlew :server-common:test --tests org.apache.gravitino.server.web.TestUtils -PskipITs -PskipDockerTests=true` - `./gradlew :server:test --tests org.apache.gravitino.server.web.filter.TestAccessControlNotAllowedFilter --tests org.apache.gravitino.server.web.rest.TestExceptionHandlers --tests org.apache.gravitino.server.web.rest.TestStatisticOperations -PskipITs -PskipDockerTests=true` - `./gradlew :clients:client-java:test --tests org.apache.gravitino.client.TestSupportsStatistics --tests org.apache.gravitino.client.TestSupportsPartitionStatistics -PskipITs -PskipDockerTests=true` - `./gradlew :plugins:idp-basic:test --tests org.apache.gravitino.idp.web.TestIdpRESTUtils --tests org.apache.gravitino.idp.TestIdpUserGroupManager -PskipITs -PskipDockerTests=true` - `./gradlew :plugins:idp-basic:test --tests org.apache.gravitino.idp.integration.test.IdpRESTApiIT -PskipDockerTests=true` - `./gradlew :docs:build -PskipITs -PskipDockerTests=true` - `pnpm exec vitest run src/lib/utils/axios/unsupportedOperation.test.js` - `pnpm prettier:check` - `pnpm lint` # Conflicts: # server-common/src/test/java/org/apache/gravitino/server/web/TestUtils.java # server/src/main/java/org/apache/gravitino/server/web/rest/ExceptionHandlers.java # server/src/test/java/org/apache/gravitino/server/web/rest/TestExceptionHandlers.java
github-actions
Bot
added
cherry-pick
Keep only what 8e41ced introduced. The conflicting hunks also carried main-only context that branch-1.3 never had: the ConnectionFailedException and OptimisticLockException branches in BaseExceptionHandler, their imports, and testOptimisticLockConflictReturnsConflict. Those come from other commits and are dropped here rather than back-ported as a side effect of this one. Add the ErrorConstants import to TestUtils, which main already had and branch-1.3 did not, so the new assertions compile. Claude-Session: https://claude.ai/code/session_01EPJduJLZAREBg1bKxybe1e
yuqi1129
changed the title
jerryshao
removed
the
cherry-pick-conflict
Author
Code Coverage Report
Files |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-pick Information:
branch-1.3Do not merge until conflict markers are resolved and the
cherry-pick-conflictlabel is removed.Please review and resolve the conflicts before merging.