SouthwestAsiaFloat commented on code in PR #6411:
URL: https://github.com/apache/shenyu/pull/6411#discussion_r3524607386


##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/controller/SwaggerImportController.java:
##########
@@ -74,14 +81,19 @@ public ShenyuAdminResult importSwagger(@Valid @RequestBody 
final SwaggerImportRe
      * @return the result of config import
      */
     @PostMapping("import/mcp")
-    public ShenyuAdminResult importMcpConfig(@Valid @RequestBody final 
SwaggerImportRequest request) {
+    public ResponseEntity<ShenyuAdminResult> importMcpConfig(@Valid 
@RequestBody final SwaggerImportRequest request) {

Review Comment:
   I changed the response type because the PR description says that invalid or 
oversized import requests should return HTTP 400. The original return type 
(ShenyuAdminResult) only carries the business error code in the response body, 
and when returned directly, Spring MVC still responds with HTTP 200 by default. 
Using ResponseEntity<ShenyuAdminResult> allows the endpoint to return both an 
HTTP 400 status and the existing ShenyuAdminResult response body.
   
   If the expected behavior is to keep the existing admin API convention (i.e. 
returning HTTP 200 with ShenyuAdminResult.code = 400 for business errors), I’m 
happy to change it back and update the implementation accordingly.
   
   My apologies if I misunderstood the expected behavior.



-- 
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]

Reply via email to