From f338b922dbd40df98aa94312ab0a13e9581f3b26 Mon Sep 17 00:00:00 2001 From: Ruslan Kashapov Date: Wed, 28 Oct 2020 16:53:58 +0200 Subject: [PATCH] Exception handling on REST interface - customisable error message - unified approach for exception handling - unified approach for error message delivery Change-Id: Iecb46a119008fdae284fc730f459e729168e92a7 Issue-ID: CPS-41 JIRA: https://jira.onap.org/browse/CPS-41 Signed-off-by: Ruslan Kashapov --- cps/cps-rest/pom.xml | 16 ++++ .../cps/rest/controller/CpsRestController.java | 88 +++++++++---------- .../rest/exceptions/CpsRestExceptionHandler.java | 98 ++++++++++++++++++++++ .../org/onap/cps/rest/exceptions/ErrorMessage.java | 37 ++++++++ .../src/main/java/org/onap/cps/api/CpService.java | 4 +- .../java/org/onap/cps/api/impl/CpServiceImpl.java | 28 +++++-- .../java/org/onap/cps/exceptions/CpsException.java | 62 ++++++++++++++ .../onap/cps/exceptions/CpsNotFoundException.java | 57 +++++++++++++ .../cps/exceptions/CpsValidationException.java | 55 ++++++++++++ .../org/onap/cps/api/impl/CpServiceImplSpec.groovy | 7 +- 10 files changed, 389 insertions(+), 63 deletions(-) create mode 100644 cps/cps-rest/src/main/java/org/onap/cps/rest/exceptions/CpsRestExceptionHandler.java create mode 100644 cps/cps-rest/src/main/java/org/onap/cps/rest/exceptions/ErrorMessage.java create mode 100644 cps/cps-service/src/main/java/org/onap/cps/exceptions/CpsException.java create mode 100644 cps/cps-service/src/main/java/org/onap/cps/exceptions/CpsNotFoundException.java create mode 100644 cps/cps-service/src/main/java/org/onap/cps/exceptions/CpsValidationException.java diff --git a/cps/cps-rest/pom.xml b/cps/cps-rest/pom.xml index fcd97dcf7..c1f08eaba 100644 --- a/cps/cps-rest/pom.xml +++ b/cps/cps-rest/pom.xml @@ -61,6 +61,22 @@ + + + org.codehaus.groovy + groovy + test + + + org.spockframework + spock-core + test + + + cglib + cglib-nodep + test + diff --git a/cps/cps-rest/src/main/java/org/onap/cps/rest/controller/CpsRestController.java b/cps/cps-rest/src/main/java/org/onap/cps/rest/controller/CpsRestController.java index 9a31b6d8e..90c287cda 100644 --- a/cps/cps-rest/src/main/java/org/onap/cps/rest/controller/CpsRestController.java +++ b/cps/cps-rest/src/main/java/org/onap/cps/rest/controller/CpsRestController.java @@ -26,15 +26,13 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.OutputStream; -import javax.persistence.PersistenceException; import javax.validation.Valid; -import org.hibernate.exception.ConstraintViolationException; import org.onap.cps.api.CpService; +import org.onap.cps.exceptions.CpsException; +import org.onap.cps.exceptions.CpsValidationException; import org.onap.cps.rest.api.CpsRestApi; import org.opendaylight.yangtools.yang.model.api.SchemaContext; -import org.opendaylight.yangtools.yang.model.parser.api.YangParserException; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.DeleteMapping; @@ -58,16 +56,10 @@ public class CpsRestController implements CpsRestApi { @Override public ResponseEntity createModules(@Valid MultipartFile multipartFile, String dataspaceName) { - try { - final File fileToParse = saveToFile(multipartFile); - final SchemaContext schemaContext = cpService.parseAndValidateModel(fileToParse); - cpService.storeSchemaContext(schemaContext, dataspaceName); - return new ResponseEntity<>("Resource successfully created", HttpStatus.CREATED); - } catch (final YangParserException | ConstraintViolationException e) { - return new ResponseEntity<>(e.getMessage(), HttpStatus.BAD_REQUEST); - } catch (final Exception e) { - return new ResponseEntity<>(e.getMessage(), HttpStatus.INTERNAL_SERVER_ERROR); - } + final File fileToParse = saveToFile(multipartFile); + final SchemaContext schemaContext = cpService.parseAndValidateModel(fileToParse); + cpService.storeSchemaContext(schemaContext, dataspaceName); + return new ResponseEntity<>("Resource successfully created", HttpStatus.CREATED); } @Override @@ -124,16 +116,11 @@ public class CpsRestController implements CpsRestApi { @PostMapping("/upload-yang-json-data-file") public final ResponseEntity uploadYangJsonDataFile( @RequestParam("file") MultipartFile uploadedFile) { - try { - validateJsonStructure(uploadedFile); - final int persistenceObjectId = cpService.storeJsonStructure(new String(uploadedFile.getBytes())); - return new ResponseEntity( - "Object stored in CPS with identity: " + persistenceObjectId, HttpStatus.OK); - } catch (final JsonSyntaxException e) { - return new ResponseEntity(e.getMessage(), HttpStatus.BAD_REQUEST); - } catch (final Exception e) { - return new ResponseEntity(e.getMessage(), HttpStatus.INTERNAL_SERVER_ERROR); - } + validateJsonStructure(uploadedFile); + final int persistenceObjectId = cpService.storeJsonStructure(getJsonString(uploadedFile)); + return new ResponseEntity( + "Object stored in CPS with identity: " + persistenceObjectId, HttpStatus.OK); + } /** @@ -145,13 +132,7 @@ public class CpsRestController implements CpsRestApi { @GetMapping("/json-object/{id}") public final ResponseEntity getJsonObjectById( @PathVariable("id") final int jsonObjectId) { - try { - return new ResponseEntity(cpService.getJsonById(jsonObjectId), HttpStatus.OK); - } catch (final PersistenceException e) { - return new ResponseEntity(e.getMessage(), HttpStatus.NOT_FOUND); - } catch (final Exception e) { - return new ResponseEntity(e.getMessage(), HttpStatus.INTERNAL_SERVER_ERROR); - } + return new ResponseEntity(cpService.getJsonById(jsonObjectId), HttpStatus.OK); } /** @@ -163,30 +144,39 @@ public class CpsRestController implements CpsRestApi { @DeleteMapping("json-object/{id}") public final ResponseEntity deleteJsonObjectById( @PathVariable("id") final int jsonObjectId) { + cpService.deleteJsonById(jsonObjectId); + return new ResponseEntity<>(HttpStatus.NO_CONTENT); + } + + private static void validateJsonStructure(final MultipartFile multipartFile) { try { - cpService.deleteJsonById(jsonObjectId); - return new ResponseEntity<>(HttpStatus.NO_CONTENT); - } catch (final EmptyResultDataAccessException e) { - return new ResponseEntity<>(HttpStatus.NOT_FOUND.toString(), HttpStatus.NOT_FOUND); - } catch (final Exception e) { - return new ResponseEntity<>(e.getMessage(), HttpStatus.INTERNAL_SERVER_ERROR); + final Gson gson = new Gson(); + gson.fromJson(getJsonString(multipartFile), Object.class); + } catch (JsonSyntaxException e) { + throw new CpsValidationException("Not a valid JSON file.", e); } } - private static final void validateJsonStructure(final MultipartFile jsonFile) - throws JsonSyntaxException, IOException { - final Gson gson = new Gson(); - gson.fromJson(new String(jsonFile.getBytes()), Object.class); - } + private static File saveToFile(final MultipartFile multipartFile) { + try { + final File file = File.createTempFile("tempFile", ".yang"); + file.deleteOnExit(); - private static final File saveToFile(final MultipartFile multipartFile) - throws IOException { - final File file = File.createTempFile("tempFile", ".yang"); - file.deleteOnExit(); + try (OutputStream outputStream = new FileOutputStream(file)) { + outputStream.write(multipartFile.getBytes()); + } + return file; - try (OutputStream outputStream = new FileOutputStream(file)) { - outputStream.write(multipartFile.getBytes()); + } catch (IOException e) { + throw new CpsException(e); + } + } + + private static String getJsonString(final MultipartFile multipartFile) { + try { + return new String(multipartFile.getBytes()); + } catch (IOException e) { + throw new CpsException(e); } - return file; } } \ No newline at end of file diff --git a/cps/cps-rest/src/main/java/org/onap/cps/rest/exceptions/CpsRestExceptionHandler.java b/cps/cps-rest/src/main/java/org/onap/cps/rest/exceptions/CpsRestExceptionHandler.java new file mode 100644 index 000000000..9cf4935d4 --- /dev/null +++ b/cps/cps-rest/src/main/java/org/onap/cps/rest/exceptions/CpsRestExceptionHandler.java @@ -0,0 +1,98 @@ +/* + * ============LICENSE_START======================================================= + * Copyright (C) 2020 Pantheon.tech + * ================================================================================ + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + * ============LICENSE_END========================================================= + */ + +package org.onap.cps.rest.exceptions; + +import org.apache.commons.lang3.exception.ExceptionUtils; +import org.hibernate.exception.ConstraintViolationException; +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.springframework.dao.EmptyResultDataAccessException; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.RestControllerAdvice; + +@RestControllerAdvice(assignableTypes = {CpsRestController.class}) +public class CpsRestExceptionHandler { + + /** + * Default exception handler. + * + * @param exception the exception to handle + * @return response with response code 500. + */ + @ExceptionHandler + public ResponseEntity handleInternalErrorException(Exception exception) { + return buildErrorResponse(HttpStatus.INTERNAL_SERVER_ERROR, exception); + } + + /* + TODO: Rid off extra dependencies. + + Generic exception handler and CpsException (incl. children) are the only + exceptions to be handled here. All the other exceptions which require a special + treatment should be rethrown as CpsException in the place of occurrence -> + e.g. persistence exceptions are to be handled in cps-ri module. + */ + + @ExceptionHandler({ConstraintViolationException.class}) + public ResponseEntity handleBadRequestException(Exception exception) { + return buildErrorResponse(HttpStatus.BAD_REQUEST, exception); + } + + @ExceptionHandler({EmptyResultDataAccessException.class}) + public ResponseEntity handleNotFoundException(Exception exception) { + return buildErrorResponse(HttpStatus.NOT_FOUND, exception); + } + + @ExceptionHandler({CpsException.class}) + public ResponseEntity handleCpsException(CpsException exception) { + return buildErrorResponse(HttpStatus.INTERNAL_SERVER_ERROR, exception.getMessage(), extractDetails(exception)); + } + + @ExceptionHandler({CpsValidationException.class}) + public ResponseEntity handleCpsValidationException(CpsException exception) { + return buildErrorResponse(HttpStatus.BAD_REQUEST, exception.getMessage(), extractDetails(exception)); + } + + @ExceptionHandler({CpsNotFoundException.class}) + public ResponseEntity handleCpsNotFoundException(CpsException exception) { + return buildErrorResponse(HttpStatus.NOT_FOUND, exception.getMessage(), extractDetails(exception)); + } + + private static ResponseEntity buildErrorResponse(HttpStatus status, Exception exception) { + return buildErrorResponse(status, exception.getMessage(), ExceptionUtils.getStackTrace(exception)); + } + + private static ResponseEntity buildErrorResponse(HttpStatus status, String message, String details) { + return new ResponseEntity<>( + ErrorMessage.builder().status(status.toString()).message(message).details(details).build(), + status + ); + } + + private static String extractDetails(CpsException exception) { + return exception.getCause() == null + ? exception.getDetails() + : ExceptionUtils.getStackTrace(exception.getCause()); + } +} diff --git a/cps/cps-rest/src/main/java/org/onap/cps/rest/exceptions/ErrorMessage.java b/cps/cps-rest/src/main/java/org/onap/cps/rest/exceptions/ErrorMessage.java new file mode 100644 index 000000000..1f2a0b786 --- /dev/null +++ b/cps/cps-rest/src/main/java/org/onap/cps/rest/exceptions/ErrorMessage.java @@ -0,0 +1,37 @@ +/* + * ============LICENSE_START======================================================= + * Copyright (C) 2020 Pantheon.tech + * ================================================================================ + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + * ============LICENSE_END========================================================= + */ + +package org.onap.cps.rest.exceptions; + +import lombok.Builder; +import lombok.Data; + +/** + * Temporary POJO class used as error response model. + * TODO: To replace with model class generated from Open API specification. + * + */ +@Builder +@Data +public class ErrorMessage { + private String status; + private String message; + private String details; +} + diff --git a/cps/cps-service/src/main/java/org/onap/cps/api/CpService.java b/cps/cps-service/src/main/java/org/onap/cps/api/CpService.java index 2f735abc7..4d94a4654 100644 --- a/cps/cps-service/src/main/java/org/onap/cps/api/CpService.java +++ b/cps/cps-service/src/main/java/org/onap/cps/api/CpService.java @@ -36,7 +36,7 @@ public interface CpService { * @param yangModelContent the input stream * @return the schema context */ - SchemaContext parseAndValidateModel(final String yangModelContent) throws IOException, YangParserException; + SchemaContext parseAndValidateModel(final String yangModelContent); /** * Parse and validate a file representing a yang model to generate a schema context. @@ -44,7 +44,7 @@ public interface CpService { * @param yangModelFile the yang file * @return the schema context */ - SchemaContext parseAndValidateModel(final File yangModelFile) throws IOException, YangParserException; + SchemaContext parseAndValidateModel(final File yangModelFile); /** * Store schema context for a yang model. diff --git a/cps/cps-service/src/main/java/org/onap/cps/api/impl/CpServiceImpl.java b/cps/cps-service/src/main/java/org/onap/cps/api/impl/CpServiceImpl.java index 45aad3e44..c33746e86 100644 --- a/cps/cps-service/src/main/java/org/onap/cps/api/impl/CpServiceImpl.java +++ b/cps/cps-service/src/main/java/org/onap/cps/api/impl/CpServiceImpl.java @@ -26,6 +26,8 @@ import java.io.FileWriter; import java.io.IOException; import java.util.Optional; import org.onap.cps.api.CpService; +import org.onap.cps.exceptions.CpsException; +import org.onap.cps.exceptions.CpsValidationException; import org.onap.cps.spi.DataPersistencyService; import org.onap.cps.spi.ModelPersistencyService; import org.onap.cps.utils.YangUtils; @@ -47,18 +49,28 @@ public class CpServiceImpl implements CpService { private DataPersistencyService dataPersistencyService; @Override - public final SchemaContext parseAndValidateModel(final String yangModelContent) throws IOException, - YangParserException { - final File tempFile = File.createTempFile("yang", ".yang"); - try (BufferedWriter writer = new BufferedWriter(new FileWriter(tempFile))) { - writer.write(yangModelContent); + public final SchemaContext parseAndValidateModel(final String yangModelContent) { + + try { + final File tempFile = File.createTempFile("yang", ".yang"); + try (BufferedWriter writer = new BufferedWriter(new FileWriter(tempFile))) { + writer.write(yangModelContent); + } + return parseAndValidateModel(tempFile); + } catch (IOException e) { + throw new CpsException(e); } - return parseAndValidateModel(tempFile); } @Override - public final SchemaContext parseAndValidateModel(final File yangModelFile) throws IOException, YangParserException { - return YangUtils.parseYangModelFile(yangModelFile); + public final SchemaContext parseAndValidateModel(final File yangModelFile) { + try { + return YangUtils.parseYangModelFile(yangModelFile); + } catch (YangParserException e) { + throw new CpsValidationException("Yang file validation failed", e.getMessage()); + } catch (IOException e) { + throw new CpsException(e); + } } @Override diff --git a/cps/cps-service/src/main/java/org/onap/cps/exceptions/CpsException.java b/cps/cps-service/src/main/java/org/onap/cps/exceptions/CpsException.java new file mode 100644 index 000000000..b54453cd9 --- /dev/null +++ b/cps/cps-service/src/main/java/org/onap/cps/exceptions/CpsException.java @@ -0,0 +1,62 @@ +/* + * ============LICENSE_START======================================================= + * Copyright (C) 2020 Pantheon.tech + * ================================================================================ + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + * ============LICENSE_END========================================================= + */ + +package org.onap.cps.exceptions; + +import lombok.Getter; +import org.springframework.core.NestedExceptionUtils; + +/** + * CP Service exception. + */ +public class CpsException extends RuntimeException { + + @Getter + String details; + + /** + * Constructor. + * + * @param cause the cause of the exception + */ + public CpsException(Throwable cause) { + super(cause.getMessage(), cause); + } + + /** + * Constructor. + * + * @param message the error message + * @param cause the cause of the exception + */ + public CpsException(String message, Throwable cause) { + super(message, cause); + } + + /** + * Constructor. + * + * @param message the error message + * @param details the error details + */ + public CpsException(String message, String details) { + super(message); + this.details = details; + } +} diff --git a/cps/cps-service/src/main/java/org/onap/cps/exceptions/CpsNotFoundException.java b/cps/cps-service/src/main/java/org/onap/cps/exceptions/CpsNotFoundException.java new file mode 100644 index 000000000..f44fe806c --- /dev/null +++ b/cps/cps-service/src/main/java/org/onap/cps/exceptions/CpsNotFoundException.java @@ -0,0 +1,57 @@ +/* + * ============LICENSE_START======================================================= + * Copyright (C) 2020 Pantheon.tech + * ================================================================================ + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + * ============LICENSE_END========================================================= + */ + +package org.onap.cps.exceptions; + +import lombok.Getter; + +/** + * CP Service exception. Indicates the requested data being absent. + */ +public class CpsNotFoundException extends CpsException { + + /** + * Constructor. + * + * @param cause the cause of the exception + */ + public CpsNotFoundException(Throwable cause) { + super(cause.getMessage(), cause); + } + + /** + * Constructor. + * + * @param message the error message + * @param cause the cause of the exception + */ + public CpsNotFoundException(String message, Throwable cause) { + super(message, cause); + } + + /** + * Constructor. + * + * @param message the error message + * @param details the error details + */ + public CpsNotFoundException(String message, String details) { + super(message, details); + } +} diff --git a/cps/cps-service/src/main/java/org/onap/cps/exceptions/CpsValidationException.java b/cps/cps-service/src/main/java/org/onap/cps/exceptions/CpsValidationException.java new file mode 100644 index 000000000..dba9c165b --- /dev/null +++ b/cps/cps-service/src/main/java/org/onap/cps/exceptions/CpsValidationException.java @@ -0,0 +1,55 @@ +/* + * ============LICENSE_START======================================================= + * Copyright (C) 2020 Pantheon.tech + * ================================================================================ + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + * ============LICENSE_END========================================================= + */ + +package org.onap.cps.exceptions; + +/** + * CP Service exception. Indicates the parameter validation failure. + */ +public class CpsValidationException extends CpsException { + + /** + * Constructor. + * + * @param cause the cause of the exception + */ + public CpsValidationException(Throwable cause) { + super(cause.getMessage(), cause); + } + + /** + * Constructor. + * + * @param message the error message + * @param cause the cause of the exception + */ + public CpsValidationException(String message, Throwable cause) { + super(message, cause); + } + + /** + * Constructor. + * + * @param message the error message + * @param details the error details + */ + public CpsValidationException(String message, String details) { + super(message, details); + } +} diff --git a/cps/cps-service/src/test/groovy/org/onap/cps/api/impl/CpServiceImplSpec.groovy b/cps/cps-service/src/test/groovy/org/onap/cps/api/impl/CpServiceImplSpec.groovy index 5e6fccb7a..5f42810bd 100644 --- a/cps/cps-service/src/test/groovy/org/onap/cps/api/impl/CpServiceImplSpec.groovy +++ b/cps/cps-service/src/test/groovy/org/onap/cps/api/impl/CpServiceImplSpec.groovy @@ -21,11 +21,10 @@ package org.onap.cps.api.impl import org.onap.cps.TestUtils +import org.onap.cps.exceptions.CpsValidationException import org.onap.cps.spi.DataPersistencyService - import org.opendaylight.yangtools.yang.common.Revision import org.opendaylight.yangtools.yang.model.api.SchemaContext -import org.opendaylight.yangtools.yang.model.parser.api.YangParserException import spock.lang.Specification class CpServiceImplSpec extends Specification { @@ -73,8 +72,8 @@ class CpServiceImplSpec extends Specification { File file = new File(ClassLoader.getSystemClassLoader().getResource('invalid.yang').getFile()) when: 'the model is parsed and validated' objectUnderTest.parseAndValidateModel(file) - then: 'a YangParserException is thrown' - thrown(YangParserException) + then: 'a CpsValidationException is thrown' + thrown(CpsValidationException) } def 'Store a SchemaContext'() { -- 2.16.6