From b2550f410eeb88cc89d089f12589e05fb95906ad Mon Sep 17 00:00:00 2001 From: daidai Date: Sun, 29 Dec 2024 12:18:23 +0800 Subject: [PATCH 1/2] Unify web api exception return value specification to json. --- .../web/controllers/BaseControllerAdvice.java | 55 +++++++++++++------ .../controllers/PdxBasedCrudController.java | 23 ++++++-- 2 files changed, 55 insertions(+), 23 deletions(-) diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/BaseControllerAdvice.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/BaseControllerAdvice.java index e874e587b640..fb3ba9228b2a 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/BaseControllerAdvice.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/BaseControllerAdvice.java @@ -19,6 +19,7 @@ import org.apache.logging.log4j.Logger; import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; import org.springframework.security.access.AccessDeniedException; import org.springframework.web.HttpRequestMethodNotSupportedException; import org.springframework.web.bind.annotation.ControllerAdvice; @@ -65,8 +66,10 @@ protected String getRestApiVersion() { @ExceptionHandler({RegionNotFoundException.class, ResourceNotFoundException.class}) @ResponseBody @ResponseStatus(HttpStatus.NOT_FOUND) - public String handle(final RuntimeException e) { - return convertErrorAsJson(e.getMessage()); + public ResponseEntity handle(final RuntimeException e) { + return ResponseEntity.status(HttpStatus.NOT_FOUND) + .contentType(APPLICATION_JSON_UTF8) + .body(convertErrorAsJson(e.getMessage())); } /** @@ -80,8 +83,10 @@ public String handle(final RuntimeException e) { @ExceptionHandler({MalformedJsonException.class}) @ResponseBody @ResponseStatus(HttpStatus.BAD_REQUEST) - public String handleException(final RuntimeException e) { - return convertErrorAsJson(e.getMessage()); + public ResponseEntity handleException(final RuntimeException e) { + return ResponseEntity.status(HttpStatus.BAD_REQUEST) + .contentType(APPLICATION_JSON_UTF8) + .body(convertErrorAsJson(e.getMessage())); } /** @@ -95,8 +100,10 @@ public String handleException(final RuntimeException e) { @ExceptionHandler(GemfireRestException.class) @ResponseBody @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) - public String handleException(final GemfireRestException ge) { - return convertErrorAsJson(ge); + public ResponseEntity handleException(final GemfireRestException ge) { + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .contentType(APPLICATION_JSON_UTF8) + .body(convertErrorAsJson(ge)); } /** @@ -111,8 +118,10 @@ public String handleException(final GemfireRestException ge) { @ExceptionHandler(DataTypeNotSupportedException.class) @ResponseBody @ResponseStatus(HttpStatus.NOT_ACCEPTABLE) - public String handleException(final DataTypeNotSupportedException tns) { - return convertErrorAsJson(tns.getMessage()); + public ResponseEntity handleException(final DataTypeNotSupportedException tns) { + return ResponseEntity.status(HttpStatus.NOT_ACCEPTABLE) + .contentType(APPLICATION_JSON_UTF8) + .body(convertErrorAsJson(tns.getMessage())); } /** @@ -127,8 +136,10 @@ public String handleException(final DataTypeNotSupportedException tns) { @ExceptionHandler(HttpRequestMethodNotSupportedException.class) @ResponseBody @ResponseStatus(HttpStatus.METHOD_NOT_ALLOWED) - public String handleException(final HttpRequestMethodNotSupportedException e) { - return convertErrorAsJson(e.getMessage()); + public ResponseEntity handleException(final HttpRequestMethodNotSupportedException e) { + return ResponseEntity.status(HttpStatus.METHOD_NOT_ALLOWED) + .contentType(APPLICATION_JSON_UTF8) + .body(convertErrorAsJson(e.getMessage())); } /** @@ -142,8 +153,10 @@ public String handleException(final HttpRequestMethodNotSupportedException e) { @ExceptionHandler(AccessDeniedException.class) @ResponseBody @ResponseStatus(HttpStatus.FORBIDDEN) - public String handleException(final AccessDeniedException cause) { - return convertErrorAsJson(cause.getMessage()); + public ResponseEntity handleException(final AccessDeniedException cause) { + return ResponseEntity.status(HttpStatus.FORBIDDEN) + .contentType(APPLICATION_JSON_UTF8) + .body(convertErrorAsJson(cause.getMessage())); } /** @@ -156,8 +169,10 @@ public String handleException(final AccessDeniedException cause) { @ExceptionHandler(NotAuthorizedException.class) @ResponseBody @ResponseStatus(HttpStatus.FORBIDDEN) - public String handleException(final NotAuthorizedException cause) { - return convertErrorAsJson(cause.getMessage()); + public ResponseEntity handleException(final NotAuthorizedException cause) { + return ResponseEntity.status(HttpStatus.FORBIDDEN) + .contentType(APPLICATION_JSON_UTF8) + .body(convertErrorAsJson(cause.getMessage())); } /** @@ -170,8 +185,10 @@ public String handleException(final NotAuthorizedException cause) { @ExceptionHandler(EntityNotFoundException.class) @ResponseBody @ResponseStatus(HttpStatus.NOT_FOUND) - public String handleException(final EntityNotFoundException cause) { - return convertErrorAsJson(cause.getMessage()); + public ResponseEntity handleException(final EntityNotFoundException cause) { + return ResponseEntity.status(HttpStatus.NOT_FOUND) + .contentType(APPLICATION_JSON_UTF8) + .body(convertErrorAsJson(cause.getMessage())); } /** @@ -185,7 +202,7 @@ public String handleException(final EntityNotFoundException cause) { @ExceptionHandler(Throwable.class) @ResponseBody @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) - public String handleException(final Throwable cause) { + public ResponseEntity handleException(final Throwable cause) { final StringWriter stackTraceWriter = new StringWriter(); cause.printStackTrace(new PrintWriter(stackTraceWriter)); final String stackTrace = stackTraceWriter.toString(); @@ -194,7 +211,9 @@ public String handleException(final Throwable cause) { logger.debug(stackTrace); } - return convertErrorAsJson(cause.getMessage()); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .contentType(APPLICATION_JSON_UTF8) + .body(convertErrorAsJson(cause.getMessage())); } } diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java index 8cfa93c2f0da..42587d72bb49 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java @@ -195,7 +195,9 @@ private ResponseEntity getAllRegionData(String region, String limit) { if (maxLimit < 0) { String errorMessage = String.format("Negative limit param (%1$s) is not valid!", maxLimit); - return new ResponseEntity<>(convertErrorAsJson(errorMessage), HttpStatus.BAD_REQUEST); + return ResponseEntity.status(HttpStatus.BAD_REQUEST) + .contentType(APPLICATION_JSON_UTF8) + .body(convertErrorAsJson(errorMessage)); } int mapSize = keys.size(); @@ -210,11 +212,14 @@ private ResponseEntity getAllRegionData(String region, String limit) { // limit param is not specified in proper format. set the HTTPHeader // for BAD_REQUEST String errorMessage = String.format("limit param (%1$s) is not valid!", limit); - return new ResponseEntity<>(convertErrorAsJson(errorMessage), HttpStatus.BAD_REQUEST); + return ResponseEntity.status(HttpStatus.BAD_REQUEST) + .contentType(APPLICATION_JSON_UTF8) + .body(convertErrorAsJson(errorMessage)); } } headers.set(HttpHeaders.CONTENT_LOCATION, toUri(region, keyList).toASCIIString()); + headers.setContentType(APPLICATION_JSON_UTF8); return new ResponseEntity>(data, headers, HttpStatus.OK); } @@ -277,7 +282,9 @@ private ResponseEntity getRegionKeys(String region, String ignoreMissingKey, String errorMessage = String.format( "ignoreMissingKey param (%1$s) is not valid. valid usage is ignoreMissingKey=true!", ignoreMissingKey); - return new ResponseEntity<>(convertErrorAsJson(errorMessage), HttpStatus.BAD_REQUEST); + return ResponseEntity.status(HttpStatus.BAD_REQUEST) + .contentType(APPLICATION_JSON_UTF8) + .body(convertErrorAsJson(errorMessage)); } final Map valueObjs = getValues(region, keys); @@ -365,17 +372,21 @@ public ResponseEntity update(@PathVariable("region") String region, String errorMessage = String.format( "The op parameter (%1$s) is not valid. Valid values are PUT, REPLACE, or CAS.", opValue); - return new ResponseEntity<>(convertErrorAsJson(errorMessage), HttpStatus.BAD_REQUEST); + return ResponseEntity.status(HttpStatus.BAD_REQUEST) + .contentType(APPLICATION_JSON_UTF8) + .body(convertErrorAsJson(errorMessage)); } if (keys.length > 1) { updateMultipleKeys(region, keys, json); HttpHeaders headers = new HttpHeaders(); + headers.setContentType(APPLICATION_JSON_UTF8); headers.setLocation(toUri(region, StringUtils.arrayToCommaDelimitedString(keys))); return new ResponseEntity<>(headers, HttpStatus.OK); } else { // put case Object existingValue = updateSingleKey(region, keys[0], json, opValue); final HttpHeaders headers = new HttpHeaders(); + headers.setContentType(APPLICATION_JSON_UTF8); headers.setLocation(toUri(region, keys[0])); return new ResponseEntity<>(existingValue, headers, (existingValue == null ? HttpStatus.OK : HttpStatus.CONFLICT)); @@ -433,7 +444,9 @@ public ResponseEntity updateKeys(@PathVariable("region") final String encoded String errorMessage = String.format( "The op parameter (%1$s) is not valid. Valid values are PUT, CREATE, REPLACE, or CAS.", opValue); - return new ResponseEntity<>(convertErrorAsJson(errorMessage), HttpStatus.BAD_REQUEST); + return ResponseEntity.status(HttpStatus.BAD_REQUEST) + .contentType(APPLICATION_JSON_UTF8) + .body(convertErrorAsJson(errorMessage)); } if (decodedKeys.length > 1) { From 91310470c314598e8585b421fbc62239beedd987 Mon Sep 17 00:00:00 2001 From: daidai Date: Fri, 3 Jan 2025 02:17:11 +0800 Subject: [PATCH 2/2] Unify the contentType setting method. --- .../controllers/AbstractBaseController.java | 12 +++++++ .../web/controllers/BaseControllerAdvice.java | 36 +++++-------------- .../controllers/PdxBasedCrudController.java | 21 ++++------- 3 files changed, 27 insertions(+), 42 deletions(-) diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java index 8a0d5b6461a1..d1ab4ecc6044 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java @@ -765,6 +765,18 @@ private T introspectAndConvert(final T value) { return value; } + public ResponseEntity convertErrorAsJson(HttpStatus status, String errorMessage) { + return ResponseEntity.status(status) + .contentType(APPLICATION_JSON_UTF8) + .body(convertErrorAsJson(errorMessage)); + } + + public ResponseEntity convertErrorAsJson(HttpStatus status, Throwable t) { + return ResponseEntity.status(status) + .contentType(APPLICATION_JSON_UTF8) + .body(convertErrorAsJson(t)); + } + String convertErrorAsJson(String errorMessage) { return ("{" + "\"cause\"" + ":" + "\"" + errorMessage + "\"" + "}"); } diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/BaseControllerAdvice.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/BaseControllerAdvice.java index fb3ba9228b2a..a7dcbcf8b4c1 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/BaseControllerAdvice.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/BaseControllerAdvice.java @@ -67,9 +67,7 @@ protected String getRestApiVersion() { @ResponseBody @ResponseStatus(HttpStatus.NOT_FOUND) public ResponseEntity handle(final RuntimeException e) { - return ResponseEntity.status(HttpStatus.NOT_FOUND) - .contentType(APPLICATION_JSON_UTF8) - .body(convertErrorAsJson(e.getMessage())); + return convertErrorAsJson(HttpStatus.NOT_FOUND, e.getMessage()); } /** @@ -84,9 +82,7 @@ public ResponseEntity handle(final RuntimeException e) { @ResponseBody @ResponseStatus(HttpStatus.BAD_REQUEST) public ResponseEntity handleException(final RuntimeException e) { - return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .contentType(APPLICATION_JSON_UTF8) - .body(convertErrorAsJson(e.getMessage())); + return convertErrorAsJson(HttpStatus.BAD_REQUEST, e.getMessage()); } /** @@ -101,9 +97,7 @@ public ResponseEntity handleException(final RuntimeException e) { @ResponseBody @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) public ResponseEntity handleException(final GemfireRestException ge) { - return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .contentType(APPLICATION_JSON_UTF8) - .body(convertErrorAsJson(ge)); + return convertErrorAsJson(HttpStatus.INTERNAL_SERVER_ERROR, ge); } /** @@ -119,9 +113,7 @@ public ResponseEntity handleException(final GemfireRestException ge) { @ResponseBody @ResponseStatus(HttpStatus.NOT_ACCEPTABLE) public ResponseEntity handleException(final DataTypeNotSupportedException tns) { - return ResponseEntity.status(HttpStatus.NOT_ACCEPTABLE) - .contentType(APPLICATION_JSON_UTF8) - .body(convertErrorAsJson(tns.getMessage())); + return convertErrorAsJson(HttpStatus.NOT_ACCEPTABLE, tns.getMessage()); } /** @@ -137,9 +129,7 @@ public ResponseEntity handleException(final DataTypeNotSupportedExceptio @ResponseBody @ResponseStatus(HttpStatus.METHOD_NOT_ALLOWED) public ResponseEntity handleException(final HttpRequestMethodNotSupportedException e) { - return ResponseEntity.status(HttpStatus.METHOD_NOT_ALLOWED) - .contentType(APPLICATION_JSON_UTF8) - .body(convertErrorAsJson(e.getMessage())); + return convertErrorAsJson(HttpStatus.METHOD_NOT_ALLOWED, e.getMessage()); } /** @@ -154,9 +144,7 @@ public ResponseEntity handleException(final HttpRequestMethodNotSupporte @ResponseBody @ResponseStatus(HttpStatus.FORBIDDEN) public ResponseEntity handleException(final AccessDeniedException cause) { - return ResponseEntity.status(HttpStatus.FORBIDDEN) - .contentType(APPLICATION_JSON_UTF8) - .body(convertErrorAsJson(cause.getMessage())); + return convertErrorAsJson(HttpStatus.FORBIDDEN, cause.getMessage()); } /** @@ -170,9 +158,7 @@ public ResponseEntity handleException(final AccessDeniedException cause) @ResponseBody @ResponseStatus(HttpStatus.FORBIDDEN) public ResponseEntity handleException(final NotAuthorizedException cause) { - return ResponseEntity.status(HttpStatus.FORBIDDEN) - .contentType(APPLICATION_JSON_UTF8) - .body(convertErrorAsJson(cause.getMessage())); + return convertErrorAsJson(HttpStatus.FORBIDDEN, cause.getMessage()); } /** @@ -186,9 +172,7 @@ public ResponseEntity handleException(final NotAuthorizedException cause @ResponseBody @ResponseStatus(HttpStatus.NOT_FOUND) public ResponseEntity handleException(final EntityNotFoundException cause) { - return ResponseEntity.status(HttpStatus.NOT_FOUND) - .contentType(APPLICATION_JSON_UTF8) - .body(convertErrorAsJson(cause.getMessage())); + return convertErrorAsJson(HttpStatus.NOT_FOUND, cause.getMessage()); } /** @@ -211,9 +195,7 @@ public ResponseEntity handleException(final Throwable cause) { logger.debug(stackTrace); } - return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .contentType(APPLICATION_JSON_UTF8) - .body(convertErrorAsJson(cause.getMessage())); + return convertErrorAsJson(HttpStatus.INTERNAL_SERVER_ERROR, cause.getMessage()); } } diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java index 42587d72bb49..a153494d67bd 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java @@ -195,9 +195,7 @@ private ResponseEntity getAllRegionData(String region, String limit) { if (maxLimit < 0) { String errorMessage = String.format("Negative limit param (%1$s) is not valid!", maxLimit); - return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .contentType(APPLICATION_JSON_UTF8) - .body(convertErrorAsJson(errorMessage)); + return convertErrorAsJson(HttpStatus.BAD_REQUEST, errorMessage); } int mapSize = keys.size(); @@ -212,9 +210,7 @@ private ResponseEntity getAllRegionData(String region, String limit) { // limit param is not specified in proper format. set the HTTPHeader // for BAD_REQUEST String errorMessage = String.format("limit param (%1$s) is not valid!", limit); - return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .contentType(APPLICATION_JSON_UTF8) - .body(convertErrorAsJson(errorMessage)); + return convertErrorAsJson(HttpStatus.BAD_REQUEST, errorMessage); } } @@ -253,6 +249,7 @@ private ResponseEntity getRegionKeys(String region, String ignoreMissingKey, logger.debug("Reading data for keys ({}) in Region ({})", ArrayUtils.toString(keys), region); securityService.authorize("READ", region, keys); final HttpHeaders headers = new HttpHeaders(); + headers.setContentType(APPLICATION_JSON_UTF8); if (keys.length == 1) { /* GET op on single key */ Object value = getValue(region, keys[0]); @@ -282,9 +279,7 @@ private ResponseEntity getRegionKeys(String region, String ignoreMissingKey, String errorMessage = String.format( "ignoreMissingKey param (%1$s) is not valid. valid usage is ignoreMissingKey=true!", ignoreMissingKey); - return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .contentType(APPLICATION_JSON_UTF8) - .body(convertErrorAsJson(errorMessage)); + return convertErrorAsJson(HttpStatus.BAD_REQUEST, errorMessage); } final Map valueObjs = getValues(region, keys); @@ -372,9 +367,7 @@ public ResponseEntity update(@PathVariable("region") String region, String errorMessage = String.format( "The op parameter (%1$s) is not valid. Valid values are PUT, REPLACE, or CAS.", opValue); - return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .contentType(APPLICATION_JSON_UTF8) - .body(convertErrorAsJson(errorMessage)); + return convertErrorAsJson(HttpStatus.BAD_REQUEST, errorMessage); } if (keys.length > 1) { updateMultipleKeys(region, keys, json); @@ -444,9 +437,7 @@ public ResponseEntity updateKeys(@PathVariable("region") final String encoded String errorMessage = String.format( "The op parameter (%1$s) is not valid. Valid values are PUT, CREATE, REPLACE, or CAS.", opValue); - return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .contentType(APPLICATION_JSON_UTF8) - .body(convertErrorAsJson(errorMessage)); + return convertErrorAsJson(HttpStatus.BAD_REQUEST, errorMessage); } if (decodedKeys.length > 1) {