From deffbf64bd6f48fef477fa84d04ebb078bf00006 Mon Sep 17 00:00:00 2001 From: Rashmi Pujar Date: Wed, 2 Mar 2022 22:12:02 -0500 Subject: [PATCH] Handle exceptions around DB transcations, data integrity Issue-ID: POLICY-3976 Signed-off-by: Rashmi Pujar Change-Id: I9cdfff788d901824904bc74fdb7e2d482d28d562 --- .../policy/api/main/config/PolicyApiConfig.java | 2 +- .../main/exception/ServiceExceptionHandler.java | 73 ++++++++++++++++++++++ .../policy/api/main/rest/ApiRestController.java | 6 +- .../policy/api/main/rest/CommonRestController.java | 53 +++++++++------- .../{ => healthcheck}/HealthCheckProvider.java | 4 +- .../statistics}/ApiStatisticsManager.java | 2 +- .../{ => statistics}/StatisticsProvider.java | 3 +- .../statistics}/StatisticsReport.java | 4 +- .../policy/api/main/rest/TestApiRestServer.java | 2 + .../api/main/rest/TestApiStatisticsManager.java | 2 +- .../policy/api/main/rest/TestStatisticsReport.java | 3 +- 11 files changed, 117 insertions(+), 37 deletions(-) create mode 100644 main/src/main/java/org/onap/policy/api/main/exception/ServiceExceptionHandler.java rename main/src/main/java/org/onap/policy/api/main/rest/provider/{ => healthcheck}/HealthCheckProvider.java (95%) rename main/src/main/java/org/onap/policy/api/main/rest/{ => provider/statistics}/ApiStatisticsManager.java (99%) rename main/src/main/java/org/onap/policy/api/main/rest/provider/{ => statistics}/StatisticsProvider.java (94%) rename main/src/main/java/org/onap/policy/api/main/rest/{ => provider/statistics}/StatisticsReport.java (97%) diff --git a/main/src/main/java/org/onap/policy/api/main/config/PolicyApiConfig.java b/main/src/main/java/org/onap/policy/api/main/config/PolicyApiConfig.java index 48202a8a..0510d65f 100644 --- a/main/src/main/java/org/onap/policy/api/main/config/PolicyApiConfig.java +++ b/main/src/main/java/org/onap/policy/api/main/config/PolicyApiConfig.java @@ -21,7 +21,7 @@ package org.onap.policy.api.main.config; -import org.onap.policy.api.main.rest.StatisticsReport; +import org.onap.policy.api.main.rest.provider.statistics.StatisticsReport; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; diff --git a/main/src/main/java/org/onap/policy/api/main/exception/ServiceExceptionHandler.java b/main/src/main/java/org/onap/policy/api/main/exception/ServiceExceptionHandler.java new file mode 100644 index 00000000..f75013c9 --- /dev/null +++ b/main/src/main/java/org/onap/policy/api/main/exception/ServiceExceptionHandler.java @@ -0,0 +1,73 @@ +/*- + * ============LICENSE_START======================================================= + * Copyright (C) 2022 Bell Canada. All rights reserved. + * ================================================================================ + * 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.policy.api.main.exception; + +import javax.ws.rs.core.Response; +import org.aspectj.lang.JoinPoint; +import org.aspectj.lang.annotation.AfterThrowing; +import org.aspectj.lang.annotation.Aspect; +import org.onap.policy.models.base.PfModelRuntimeException; +import org.onap.policy.models.errors.concepts.ErrorResponse; +import org.springframework.http.ResponseEntity; +import org.springframework.stereotype.Component; +import org.springframework.transaction.TransactionException; + +@Aspect +@Component +public class ServiceExceptionHandler { + + /** + * Handle any exceptions that are not already handled. + * For e.g., runtime exceptions that could happen during SQL query execution related to data integrity etc. + * + * @param joinPoint the point of execution + * @param exception the exception + */ + @AfterThrowing(pointcut = "execution(* org.onap.policy.api.main.service.*.*(..))", throwing = "exception") + public ResponseEntity handleServiceException(JoinPoint joinPoint, RuntimeException exception) { + if (exception instanceof PolicyApiRuntimeException || exception instanceof PfModelRuntimeException) { + throw exception; + } else { + final var errorResponse = new ErrorResponse(); + errorResponse.setResponseCode(Response.Status.INTERNAL_SERVER_ERROR); + errorResponse.setErrorMessage(exception.getMessage()); + throw new PolicyApiRuntimeException(exception.getMessage(), exception.getCause(), errorResponse, null); + } + } + + /** + * Handle DB Transaction related exceptions. + * All service classes in org.onap.policy.api.main.service are transactional and autowiring these service classes + * can cause TransactionException. + * For e.g., JDBC connection failure occurs and failed to open transaction at service level + * + * @param joinPoint the point of execution + * @param exception the exception + */ + @AfterThrowing(pointcut = "execution(* org.onap.policy.api.main..*.*(..))" + + " && !execution(* org.onap.policy.api.main.rest.provider.statistics.*.*(..))", throwing = "exception") + public ResponseEntity handleTransactionException(JoinPoint joinPoint, TransactionException exception) { + final var errorResponse = new ErrorResponse(); + errorResponse.setResponseCode(Response.Status.INTERNAL_SERVER_ERROR); + errorResponse.setErrorMessage(exception.getMessage()); + throw new PolicyApiRuntimeException(exception.getMessage(), exception.getCause(), errorResponse, null); + } +} \ No newline at end of file diff --git a/main/src/main/java/org/onap/policy/api/main/rest/ApiRestController.java b/main/src/main/java/org/onap/policy/api/main/rest/ApiRestController.java index 84b26c88..d6f6d6af 100644 --- a/main/src/main/java/org/onap/policy/api/main/rest/ApiRestController.java +++ b/main/src/main/java/org/onap/policy/api/main/rest/ApiRestController.java @@ -44,8 +44,10 @@ import java.util.UUID; import javax.ws.rs.core.Response.Status; import lombok.RequiredArgsConstructor; import org.onap.policy.api.main.exception.PolicyApiRuntimeException; -import org.onap.policy.api.main.rest.provider.HealthCheckProvider; -import org.onap.policy.api.main.rest.provider.StatisticsProvider; +import org.onap.policy.api.main.rest.provider.healthcheck.HealthCheckProvider; +import org.onap.policy.api.main.rest.provider.statistics.ApiStatisticsManager; +import org.onap.policy.api.main.rest.provider.statistics.StatisticsProvider; +import org.onap.policy.api.main.rest.provider.statistics.StatisticsReport; import org.onap.policy.api.main.service.ToscaServiceTemplateService; import org.onap.policy.common.endpoints.event.comm.Topic.CommInfrastructure; import org.onap.policy.common.endpoints.report.HealthCheckReport; diff --git a/main/src/main/java/org/onap/policy/api/main/rest/CommonRestController.java b/main/src/main/java/org/onap/policy/api/main/rest/CommonRestController.java index 5e4c3ee5..ce479bc0 100644 --- a/main/src/main/java/org/onap/policy/api/main/rest/CommonRestController.java +++ b/main/src/main/java/org/onap/policy/api/main/rest/CommonRestController.java @@ -31,9 +31,9 @@ import org.onap.policy.common.utils.coder.CoderException; import org.onap.policy.common.utils.coder.StandardCoder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.http.HttpHeaders; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.context.request.WebRequest; /** * Super class from which REST controllers are derived. @@ -63,7 +63,7 @@ public class CommonRestController { protected static final String VERSION_LATEST_NAME = "X-LatestVersion"; protected static final String VERSION_LATEST_DESCRIPTION = "Used only to communicate an API's latest version"; - protected static final String REQUEST_ID_NAME = "X-ONAP-RequestID"; + public static final String REQUEST_ID_NAME = "X-ONAP-RequestID"; protected static final String REQUEST_ID_HDR_DESCRIPTION = "Used to track REST transactions for logging purpose"; protected static final String REQUEST_ID_PARAM_DESCRIPTION = "RequestID for http transaction"; @@ -79,32 +79,34 @@ public class CommonRestController { protected final Coder coder = new StandardCoder(); protected ResponseEntity makeOkResponse(UUID requestId, T respEntity) { - HttpHeaders headers = new HttpHeaders(); - addVersionControlHeaders(headers); - addLoggingHeaders(headers, requestId); - return ResponseEntity.ok().headers(headers).body(respEntity); + return CommonRestController.addLoggingHeaders(addVersionControlHeaders(ResponseEntity.ok()), requestId) + .body(respEntity); } - protected ResponseEntity makeErrorResponse(UUID requestId, T respEntity, int status) { - HttpHeaders headers = new HttpHeaders(); - addVersionControlHeaders(headers); - addLoggingHeaders(headers, requestId); - return ResponseEntity.status(status).headers(headers).body(respEntity); - } - - private void addVersionControlHeaders(HttpHeaders headers) { - headers.add("X-MinorVersion", "0"); - headers.add("X-PatchVersion", "0"); - headers.add("X-LatestVersion", "1.0.0"); + /** + * Adds version headers to the response. + * + * @param respBuilder response builder + * @return the response builder, with version headers + */ + public static ResponseEntity.BodyBuilder addVersionControlHeaders(ResponseEntity.BodyBuilder respBuilder) { + return respBuilder.header(VERSION_MINOR_NAME, "0").header(VERSION_PATCH_NAME, "0").header(VERSION_LATEST_NAME, + API_VERSION); } - private void addLoggingHeaders(HttpHeaders headers, UUID requestId) { + /** + * Adds logging headers to the response. + * + * @param respBuilder response builder + * @return the response builder, with version logging + */ + public static ResponseEntity.BodyBuilder addLoggingHeaders(ResponseEntity.BodyBuilder respBuilder, UUID requestId) { if (requestId == null) { // Generate a random uuid if client does not embed requestId in rest request - headers.add("X-ONAP-RequestID", UUID.randomUUID().toString()); - } else { - headers.add("X-ONAP-RequestID", requestId.toString()); + return respBuilder.header(REQUEST_ID_NAME, UUID.randomUUID().toString()); } + + return respBuilder.header(REQUEST_ID_NAME, requestId.toString()); } /** @@ -128,9 +130,12 @@ public class CommonRestController { } @ExceptionHandler(value = {PolicyApiRuntimeException.class}) - protected ResponseEntity handleException(PolicyApiRuntimeException ex) { + protected ResponseEntity handleException(PolicyApiRuntimeException ex, WebRequest req) { LOGGER.warn(ex.getMessage(), ex.getCause()); - return makeErrorResponse(ex.getRequestId(), ex.getErrorResponse(), - ex.getErrorResponse().getResponseCode().getStatusCode()); + final var requestId = req.getHeader(CommonRestController.REQUEST_ID_NAME); + final var status = ex.getErrorResponse().getResponseCode().getStatusCode(); + return CommonRestController.addLoggingHeaders( + CommonRestController.addVersionControlHeaders(ResponseEntity.status(status)), + requestId != null ? UUID.fromString(requestId) : ex.getRequestId()).body(ex.getErrorResponse()); } } \ No newline at end of file diff --git a/main/src/main/java/org/onap/policy/api/main/rest/provider/HealthCheckProvider.java b/main/src/main/java/org/onap/policy/api/main/rest/provider/healthcheck/HealthCheckProvider.java similarity index 95% rename from main/src/main/java/org/onap/policy/api/main/rest/provider/HealthCheckProvider.java rename to main/src/main/java/org/onap/policy/api/main/rest/provider/healthcheck/HealthCheckProvider.java index 139e5282..88bb1c93 100644 --- a/main/src/main/java/org/onap/policy/api/main/rest/provider/HealthCheckProvider.java +++ b/main/src/main/java/org/onap/policy/api/main/rest/provider/healthcheck/HealthCheckProvider.java @@ -23,14 +23,12 @@ * ============LICENSE_END========================================================= */ -package org.onap.policy.api.main.rest.provider; +package org.onap.policy.api.main.rest.provider.healthcheck; import lombok.RequiredArgsConstructor; -import org.onap.policy.api.main.rest.PolicyFetchMode; import org.onap.policy.api.main.service.ToscaServiceTemplateService; import org.onap.policy.common.endpoints.report.HealthCheckReport; import org.onap.policy.common.utils.network.NetworkUtil; -import org.onap.policy.models.base.PfModelException; import org.onap.policy.models.base.PfModelRuntimeException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/main/src/main/java/org/onap/policy/api/main/rest/ApiStatisticsManager.java b/main/src/main/java/org/onap/policy/api/main/rest/provider/statistics/ApiStatisticsManager.java similarity index 99% rename from main/src/main/java/org/onap/policy/api/main/rest/ApiStatisticsManager.java rename to main/src/main/java/org/onap/policy/api/main/rest/provider/statistics/ApiStatisticsManager.java index 91097425..c0d95e27 100644 --- a/main/src/main/java/org/onap/policy/api/main/rest/ApiStatisticsManager.java +++ b/main/src/main/java/org/onap/policy/api/main/rest/provider/statistics/ApiStatisticsManager.java @@ -21,7 +21,7 @@ * ============LICENSE_END========================================================= */ -package org.onap.policy.api.main.rest; +package org.onap.policy.api.main.rest.provider.statistics; import lombok.RequiredArgsConstructor; import org.springframework.stereotype.Component; diff --git a/main/src/main/java/org/onap/policy/api/main/rest/provider/StatisticsProvider.java b/main/src/main/java/org/onap/policy/api/main/rest/provider/statistics/StatisticsProvider.java similarity index 94% rename from main/src/main/java/org/onap/policy/api/main/rest/provider/StatisticsProvider.java rename to main/src/main/java/org/onap/policy/api/main/rest/provider/statistics/StatisticsProvider.java index b7089ad9..7016ed40 100644 --- a/main/src/main/java/org/onap/policy/api/main/rest/provider/StatisticsProvider.java +++ b/main/src/main/java/org/onap/policy/api/main/rest/provider/statistics/StatisticsProvider.java @@ -22,10 +22,9 @@ * ============LICENSE_END========================================================= */ -package org.onap.policy.api.main.rest.provider; +package org.onap.policy.api.main.rest.provider.statistics; import lombok.RequiredArgsConstructor; -import org.onap.policy.api.main.rest.StatisticsReport; import org.springframework.stereotype.Service; /** diff --git a/main/src/main/java/org/onap/policy/api/main/rest/StatisticsReport.java b/main/src/main/java/org/onap/policy/api/main/rest/provider/statistics/StatisticsReport.java similarity index 97% rename from main/src/main/java/org/onap/policy/api/main/rest/StatisticsReport.java rename to main/src/main/java/org/onap/policy/api/main/rest/provider/statistics/StatisticsReport.java index bf12c7f5..3236392a 100644 --- a/main/src/main/java/org/onap/policy/api/main/rest/StatisticsReport.java +++ b/main/src/main/java/org/onap/policy/api/main/rest/provider/statistics/StatisticsReport.java @@ -21,7 +21,7 @@ * ============LICENSE_END========================================================= */ -package org.onap.policy.api.main.rest; +package org.onap.policy.api.main.rest.provider.statistics; import lombok.Getter; import lombok.Setter; @@ -52,4 +52,4 @@ public class StatisticsReport { protected long policyTypeGetFailureCount; protected long policyTypePostSuccessCount; protected long policyTypePostFailureCount; -} +} \ No newline at end of file diff --git a/main/src/test/java/org/onap/policy/api/main/rest/TestApiRestServer.java b/main/src/test/java/org/onap/policy/api/main/rest/TestApiRestServer.java index a22a0a07..bf35d27b 100644 --- a/main/src/test/java/org/onap/policy/api/main/rest/TestApiRestServer.java +++ b/main/src/test/java/org/onap/policy/api/main/rest/TestApiRestServer.java @@ -40,6 +40,8 @@ import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; import org.onap.policy.api.main.PolicyApiApplication; +import org.onap.policy.api.main.rest.provider.statistics.ApiStatisticsManager; +import org.onap.policy.api.main.rest.provider.statistics.StatisticsReport; import org.onap.policy.api.main.rest.utils.CommonTestRestController; import org.onap.policy.common.endpoints.report.HealthCheckReport; import org.onap.policy.common.utils.coder.StandardCoder; diff --git a/main/src/test/java/org/onap/policy/api/main/rest/TestApiStatisticsManager.java b/main/src/test/java/org/onap/policy/api/main/rest/TestApiStatisticsManager.java index 48484d6e..f343e135 100644 --- a/main/src/test/java/org/onap/policy/api/main/rest/TestApiStatisticsManager.java +++ b/main/src/test/java/org/onap/policy/api/main/rest/TestApiStatisticsManager.java @@ -26,10 +26,10 @@ package org.onap.policy.api.main.rest; import static org.junit.Assert.assertEquals; -import lombok.RequiredArgsConstructor; import org.junit.Test; import org.junit.runner.RunWith; import org.onap.policy.api.main.PolicyApiApplication; +import org.onap.policy.api.main.rest.provider.statistics.ApiStatisticsManager; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.test.context.ActiveProfiles; diff --git a/main/src/test/java/org/onap/policy/api/main/rest/TestStatisticsReport.java b/main/src/test/java/org/onap/policy/api/main/rest/TestStatisticsReport.java index 01d5a9c5..664c9591 100644 --- a/main/src/test/java/org/onap/policy/api/main/rest/TestStatisticsReport.java +++ b/main/src/test/java/org/onap/policy/api/main/rest/TestStatisticsReport.java @@ -29,6 +29,7 @@ import com.openpojo.validation.rule.impl.SetterMustExistRule; import com.openpojo.validation.test.impl.GetterTester; import com.openpojo.validation.test.impl.SetterTester; import org.junit.Test; +import org.onap.policy.api.main.rest.provider.statistics.StatisticsReport; import org.onap.policy.common.utils.test.ToStringTester; /** @@ -45,4 +46,4 @@ public class TestStatisticsReport { validator.validate(StatisticsReport.class.getPackage().getName(), new FilterClassName(StatisticsReport.class.getName())); } -} +} \ No newline at end of file -- 2.16.6