From 098a93c84aae8d8d74e7589518a576b84c38ca27 Mon Sep 17 00:00:00 2001 From: ToineSiebelink Date: Tue, 8 Dec 2020 12:08:31 +0000 Subject: Improve CPS Exception handling - Removing unnecessary level of CPS(Validation)Exception - Moved exception inside spi package - Rename exceptions are per agreed new exception hierarchy - Use new exceptions - Added SchemaSetAlreadDefined Exception - Added AnchorAlreadyDefined Exception - Ensure unique serialVersionUIDs - Fixed CpsAdmin(Persistence)ServiceImplSpec tests (there was a mix-up about what this class was testing) - Replaced exception builder with dedicated exception (constructors) Issue-ID: CPS-8 Change-Id: I9cb80af4951e69ee7b0e99303853955de9d4399f Signed-off-by: ToineSiebelink --- .../cps/rest/controller/CpsRestController.java | 14 ++-- .../rest/exceptions/CpsRestExceptionHandler.java | 34 +++++----- .../exceptions/CpsRestExceptionHandlerSpec.groovy | 75 ++++++++++++++++------ 3 files changed, 80 insertions(+), 43 deletions(-) (limited to 'cps-rest/src') diff --git a/cps-rest/src/main/java/org/onap/cps/rest/controller/CpsRestController.java b/cps-rest/src/main/java/org/onap/cps/rest/controller/CpsRestController.java index 0821dca2b6..30d3e24bb9 100755 --- a/cps-rest/src/main/java/org/onap/cps/rest/controller/CpsRestController.java +++ b/cps-rest/src/main/java/org/onap/cps/rest/controller/CpsRestController.java @@ -31,9 +31,9 @@ import org.modelmapper.ModelMapper; import org.onap.cps.api.CpService; import org.onap.cps.api.CpsAdminService; import org.onap.cps.api.CpsModuleService; -import org.onap.cps.exceptions.CpsException; -import org.onap.cps.exceptions.CpsValidationException; import org.onap.cps.rest.api.CpsRestApi; +import org.onap.cps.spi.exceptions.CpsException; +import org.onap.cps.spi.exceptions.DataValidationException; import org.onap.cps.spi.model.Anchor; import org.opendaylight.yangtools.yang.model.api.SchemaContext; import org.springframework.beans.factory.annotation.Autowired; @@ -75,7 +75,7 @@ public class CpsRestController implements CpsRestApi { final Anchor anchorDetails = modelMapper.map(anchor, Anchor.class); anchorDetails.setDataspaceName(dataspaceName); final String anchorName = cpsAdminService.createAnchor(anchorDetails); - return new ResponseEntity(anchorName, HttpStatus.CREATED); + return new ResponseEntity<>(anchorName, HttpStatus.CREATED); } @Override @@ -154,7 +154,7 @@ public class CpsRestController implements CpsRestApi { @GetMapping("/json-object/{id}") public final ResponseEntity getJsonObjectById( @PathVariable("id") final int jsonObjectId) { - return new ResponseEntity(cpService.getJsonById(jsonObjectId), HttpStatus.OK); + return new ResponseEntity<>(cpService.getJsonById(jsonObjectId), HttpStatus.OK); } /** @@ -175,7 +175,7 @@ public class CpsRestController implements CpsRestApi { final Gson gson = new Gson(); gson.fromJson(getJsonString(multipartFile), Object.class); } catch (final JsonSyntaxException e) { - throw new CpsValidationException("Not a valid JSON file.", e); + throw new DataValidationException("Not a valid JSON file.", e.getMessage(), e); } } @@ -183,7 +183,7 @@ public class CpsRestController implements CpsRestApi { try { final File file = File.createTempFile("tempFile", ".yang"); file.deleteOnExit(); - try (OutputStream outputStream = new FileOutputStream(file)) { + try (final OutputStream outputStream = new FileOutputStream(file)) { outputStream.write(multipartFile.getBytes()); } return file; @@ -200,4 +200,4 @@ public class CpsRestController implements CpsRestApi { throw new CpsException(e); } } -} \ No newline at end of file +} diff --git a/cps-rest/src/main/java/org/onap/cps/rest/exceptions/CpsRestExceptionHandler.java b/cps-rest/src/main/java/org/onap/cps/rest/exceptions/CpsRestExceptionHandler.java index 94226b78c0..fc0164f5e0 100644 --- a/cps-rest/src/main/java/org/onap/cps/rest/exceptions/CpsRestExceptionHandler.java +++ b/cps-rest/src/main/java/org/onap/cps/rest/exceptions/CpsRestExceptionHandler.java @@ -20,11 +20,14 @@ package org.onap.cps.rest.exceptions; import org.apache.commons.lang3.exception.ExceptionUtils; -import org.onap.cps.exceptions.CpsException; -import org.onap.cps.exceptions.CpsNotFoundException; -import org.onap.cps.exceptions.CpsValidationException; import org.onap.cps.rest.controller.CpsRestController; import org.onap.cps.rest.model.ErrorMessage; +import org.onap.cps.spi.exceptions.AnchorAlreadyDefinedException; +import org.onap.cps.spi.exceptions.CpsException; +import org.onap.cps.spi.exceptions.DataValidationException; +import org.onap.cps.spi.exceptions.ModelValidationException; +import org.onap.cps.spi.exceptions.NotFoundInDataspaceException; +import org.onap.cps.spi.exceptions.SchemaSetAlreadyDefinedException; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.ExceptionHandler; @@ -39,32 +42,33 @@ public class CpsRestExceptionHandler { * @param exception the exception to handle * @return response with response code 500. */ - @ExceptionHandler - public ResponseEntity handleInternalErrorException(final Exception exception) { + @ExceptionHandler public static ResponseEntity handleInternalServerErrorExceptions( + final Exception exception) { return buildErrorResponse(HttpStatus.INTERNAL_SERVER_ERROR, exception); } - @ExceptionHandler({CpsException.class}) - public ResponseEntity handleCpsException(final CpsException exception) { - return buildErrorResponse(HttpStatus.INTERNAL_SERVER_ERROR, exception.getMessage(), extractDetails(exception)); - } - - @ExceptionHandler({CpsValidationException.class}) - public ResponseEntity handleCpsValidationException(final CpsException exception) { + @ExceptionHandler({ModelValidationException.class, DataValidationException.class, + SchemaSetAlreadyDefinedException.class, AnchorAlreadyDefinedException.class}) + public static ResponseEntity handleBadRequestExceptions(final CpsException exception) { return buildErrorResponse(HttpStatus.BAD_REQUEST, exception.getMessage(), extractDetails(exception)); } - @ExceptionHandler({CpsNotFoundException.class}) - public ResponseEntity handleCpsNotFoundException(final CpsException exception) { + @ExceptionHandler({NotFoundInDataspaceException.class}) + public static ResponseEntity handleNotFoundExceptions(final CpsException exception) { return buildErrorResponse(HttpStatus.NOT_FOUND, exception.getMessage(), extractDetails(exception)); } + @ExceptionHandler({CpsException.class}) + public static ResponseEntity handleAnyOtherCpsExceptions(final CpsException exception) { + return buildErrorResponse(HttpStatus.INTERNAL_SERVER_ERROR, exception.getMessage(), extractDetails(exception)); + } + private static ResponseEntity buildErrorResponse(final HttpStatus status, final Exception exception) { return buildErrorResponse(status, exception.getMessage(), ExceptionUtils.getStackTrace(exception)); } private static ResponseEntity buildErrorResponse(final HttpStatus status, final String message, - final String details) { + final String details) { final ErrorMessage errorMessage = new ErrorMessage(); errorMessage.setStatus(status.toString()); errorMessage.setMessage(message); diff --git a/cps-rest/src/test/groovy/org/onap/cps/rest/exceptions/CpsRestExceptionHandlerSpec.groovy b/cps-rest/src/test/groovy/org/onap/cps/rest/exceptions/CpsRestExceptionHandlerSpec.groovy index dd1b956d8d..d951cbeb69 100644 --- a/cps-rest/src/test/groovy/org/onap/cps/rest/exceptions/CpsRestExceptionHandlerSpec.groovy +++ b/cps-rest/src/test/groovy/org/onap/cps/rest/exceptions/CpsRestExceptionHandlerSpec.groovy @@ -21,11 +21,16 @@ package org.onap.cps.rest.exceptions import groovy.json.JsonSlurper import org.onap.cps.api.CpService -import org.onap.cps.exceptions.CpsException -import org.onap.cps.exceptions.CpsNotFoundException -import org.onap.cps.exceptions.CpsValidationException +import org.onap.cps.spi.exceptions.AnchorAlreadyDefinedException +import org.onap.cps.spi.exceptions.CpsException +import org.onap.cps.spi.exceptions.DataValidationException +import org.onap.cps.spi.exceptions.NotFoundInDataspaceException +import org.onap.cps.spi.exceptions.ModelValidationException import org.onap.cps.rest.controller.CpsRestController +import org.onap.cps.spi.exceptions.SchemaSetAlreadyDefinedException +import spock.lang.Shared import spock.lang.Specification +import spock.lang.Unroll import static org.springframework.http.HttpStatus.BAD_REQUEST import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR @@ -35,9 +40,18 @@ import static org.springframework.test.web.servlet.setup.MockMvcBuilders.standal class CpsRestExceptionHandlerSpec extends Specification { - def cpsRestController = new CpsRestController(); + @Shared + def errorMessage = 'some error message' + @Shared + def errorDetails = 'some error details' + @Shared + def dataspaceName = 'MyDataSpace' + @Shared + def existingObjectName = 'MyAdminObject' + + def cpsRestController = new CpsRestController() def mockCpService = Mock(CpService.class) - def objectUnderTest = new CpsRestExceptionHandler(); + def objectUnderTest = new CpsRestExceptionHandler() def mockMvc = standaloneSetup(cpsRestController).setControllerAdvice(objectUnderTest).build() def setup() { @@ -47,48 +61,67 @@ class CpsRestExceptionHandlerSpec extends Specification { def 'Get request with runtime exception returns HTTP Status Internal Server Error'() { when: 'runtime exception is thrown by the service' - def errorMessage = 'runtime error message' setupTestException(new IllegalStateException(errorMessage)) def response = performTestRequest() - then: 'an HTTP Internal Server Error response is returned with the correct message' + then: 'an HTTP Internal Server Error response is returned with correct message and details' assertTestResponse(response, INTERNAL_SERVER_ERROR, errorMessage, null) } def 'Get request with generic CPS exception returns HTTP Status Internal Server Error'() { when: 'generic CPS exception is thrown by the service' - def errorMessage = 'cps generic error message' - def errorDetails = 'cps generic error details' setupTestException(new CpsException(errorMessage, errorDetails)) def response = performTestRequest() - then: 'an HTTP Internal Server Error response is returned with the correct message' + then: 'an HTTP Internal Server Error response is returned with correct message and details' assertTestResponse(response, INTERNAL_SERVER_ERROR, errorMessage, errorDetails) } def 'Get request with no data found CPS exception returns HTTP Status Not Found'() { when: 'no data found CPS exception is thrown by the service' - def errorMessage = 'cps no data error message' - def errorDetails = 'cps no data error details' - setupTestException(new CpsNotFoundException(errorMessage, errorDetails)) + def dataspaceName = 'MyDataSpace' + def descriptionOfObject = 'Description' + setupTestException(new NotFoundInDataspaceException(dataspaceName, descriptionOfObject)) def response = performTestRequest() - then: 'an HTTP Not Found response is returned with the correct message' - assertTestResponse(response, NOT_FOUND, errorMessage, errorDetails) + then: 'an HTTP Not Found response is returned with correct message and details' + assertTestResponse(response, NOT_FOUND, 'Object not found', + 'Description does not exist in dataspace MyDataSpace.') } - def 'Get request with CPS validation exception returns HTTP Status Bad Request'() { + @Unroll + def 'request with an expectedObjectTypeInMessage object already defined exception returns HTTP Status Bad Request'() { + + when: 'no data found CPS exception is thrown by the service' + setupTestException(exceptionThrown) + def response = performTestRequest() + + then: 'an HTTP Bad Request response is returned with correct message an details' + assertTestResponse(response, BAD_REQUEST, + "Duplicate ${expectedObjectTypeInMessage}", + "${expectedObjectTypeInMessage} with name ${existingObjectName} " + + 'already exists for dataspace MyDataSpace.') + where: 'the following exceptions are thrown' + exceptionThrown || expectedObjectTypeInMessage + new SchemaSetAlreadyDefinedException(dataspaceName, existingObjectName, null) || 'Schema Set' + new AnchorAlreadyDefinedException(dataspaceName, existingObjectName, null) || 'Anchor' + } + + @Unroll + def 'Get request with a #exceptionThrown.class.simpleName returns HTTP Status Bad Request'() { when: 'CPS validation exception is thrown by the service' - def errorMessage = 'cps validation error message' - def errorDetails = 'cps validation error details' - setupTestException(new CpsValidationException(errorMessage, errorDetails)) + setupTestException(exceptionThrown) def response = performTestRequest() - then: 'an HTTP Bad Request response is returned with the correct message' + then: 'an HTTP Bad Request response is returned with correct message and details' assertTestResponse(response, BAD_REQUEST, errorMessage, errorDetails) + + where: 'the following exceptions are thrown' + exceptionThrown << [new ModelValidationException(errorMessage, errorDetails, null), + new DataValidationException(errorMessage, errorDetails, null)] } /* @@ -111,5 +144,5 @@ class CpsRestExceptionHandlerSpec extends Specification { assert content['message'] == expectedErrorMessage assert expectedErrorDetails == null || content['details'] == expectedErrorDetails } - + } -- cgit 1.2.3-korg