yunhwane opened a new pull request, #10663:
URL: https://github.com/apache/gravitino/pull/10663
### What changes were proposed in this pull request?
Move `request.validate()` inside the `try` block in `alterModelVersion` and
`alterModelVersionByAlias` methods of `ModelOperations.java`, so that
`IllegalArgumentException` from invalid requests is properly caught by
`ExceptionHandlers.handleModelException()`.
### Why are the changes needed?
`request.validate()` was called before the `try/catch` block in both
`alterModelVersion` and `alterModelVersionByAlias`. When validation fails
(e.g., `{"updates": null}`), the `IllegalArgumentException` bypasses
`ExceptionHandlers.handleModelException()` and results in a 500 Internal Server
Error instead of the expected 400 Bad Request.
The duplicate `request.validate()` call inside the `try` block (within the
lambda) already exists, so removing the pre-try call is sufficient.
Fix: #10660
### Does this PR introduce _any_ user-facing change?
No. This fixes error response codes for invalid REST API requests from 500
to 400.
### How was this patch tested?
Added two unit tests:
- `testAlterModelVersionWithNullUpdates`: Sends `{"updates":null}` to the
version endpoint, asserts 400 Bad Request with `ILLEGAL_ARGUMENTS_CODE`
- `testAlterModelVersionByAliasWithNullUpdates`: Sends `{"updates":null}` to
the alias endpoint, asserts 400 Bad Request with `ILLEGAL_ARGUMENTS_CODE`
All existing `TestModelOperations` tests continue to 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]
