From 40bd5b855df62e257e4cc2810a95248bc3426302 Mon Sep 17 00:00:00 2001 From: sheetalm Date: Mon, 2 Apr 2018 18:12:54 +0530 Subject: Activity Spec - Change Error Response Structure 1 Added ActivitySpecNotFoundException to map to 404 Not Found response code 2 404 Not Found response code will only be returned in case activity spec does not exists irrespective of GET or PUT used in REST 3 Update flow test to check error messages 4 Error Response to have only "message" now 5 Use proper response code instead of EXPECTATION_FAILED Change-Id: I82fb3c0970f4e9416d9fe2577174dcaf0a4fef92 Issue-ID: SDC-1048 Signed-off-by: sheetalm --- .../be/impl/ActivitySpecManagerImpl.java | 58 +++++++++------------- .../errors/ActivitySpecErrorResponse.java | 8 +-- .../errors/ActivitySpecNotFoundException.java | 24 +++++++++ .../errors/DefaultExceptionMapper.java | 23 +++++---- .../activityspec/utils/ActivitySpecConstant.java | 2 + .../be/impl/ActivitySpecManagerImplTest.java | 26 +++++----- 6 files changed, 74 insertions(+), 67 deletions(-) create mode 100644 services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/errors/ActivitySpecNotFoundException.java (limited to 'services/activity-spec') diff --git a/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/be/impl/ActivitySpecManagerImpl.java b/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/be/impl/ActivitySpecManagerImpl.java index b4a735409c..44383734e8 100644 --- a/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/be/impl/ActivitySpecManagerImpl.java +++ b/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/be/impl/ActivitySpecManagerImpl.java @@ -19,6 +19,8 @@ package org.openecomp.activityspec.be.impl; import static org.openecomp.activityspec.api.rest.types.ActivitySpecAction.CERTIFY; import static org.openecomp.activityspec.api.rest.types.ActivitySpecAction.DELETE; import static org.openecomp.activityspec.api.rest.types.ActivitySpecAction.DEPRECATE; +import static org.openecomp.activityspec.utils.ActivitySpecConstant.ACTIVITY_SPEC_NOT_FOUND; +import static org.openecomp.activityspec.utils.ActivitySpecConstant.INVALID_STATE; import static org.openecomp.sdc.versioning.dao.types.VersionStatus.Certified; import static org.openecomp.sdc.versioning.dao.types.VersionStatus.Deleted; import static org.openecomp.sdc.versioning.dao.types.VersionStatus.Deprecated; @@ -36,6 +38,7 @@ import org.openecomp.activityspec.be.ActivitySpecManager; import org.openecomp.activityspec.be.dao.ActivitySpecDao; import org.openecomp.activityspec.be.dao.types.ActivitySpecEntity; import org.openecomp.activityspec.be.datatypes.ItemType; +import org.openecomp.activityspec.errors.ActivitySpecNotFoundException; import org.openecomp.activityspec.utils.ActivitySpecConstant; import org.openecomp.core.dao.UniqueValueDao; import org.openecomp.core.util.UniqueValueUtil; @@ -103,6 +106,7 @@ public class ActivitySpecManagerImpl implements ActivitySpecManager { try { uniqueValueUtil.validateUniqueValue(ACTIVITY_SPEC_NAME, activitySpecEntity.getName()); } catch (CoreException exception) { + LOGGER.debug("Unique value error for activity spec name :" + activitySpecEntity.getName(), exception); throw new CoreException(new ErrorCode.ErrorCodeBuilder() .withCategory(ErrorCategory.APPLICATION) .withId(exception.code().id()) @@ -118,16 +122,14 @@ public class ActivitySpecManagerImpl implements ActivitySpecManager { @Override public ActivitySpecEntity get(ActivitySpecEntity activitySpec) { - activitySpec.setVersion(calculateLatestVersion(activitySpec.getId(),activitySpec.getVersion() - )); - ActivitySpecEntity retrieved = null; + activitySpec.setVersion(calculateLatestVersion(activitySpec.getId(),activitySpec.getVersion())); + ActivitySpecEntity retrieved; try { retrieved = activitySpecDao.get(activitySpec); } catch (SdcRuntimeException runtimeException) { - LOGGER.error("Failed to retrieve activity spec for activitySpecId: " + activitySpec.getId() - + " and version: " - + activitySpec.getVersion().getId(), runtimeException); - validateActivitySpecExistence(activitySpec.getId(), activitySpec.getVersion()); + LOGGER.debug("Failed to retrieve activity spec for activitySpecId: " + activitySpec.getId() + + " and version: " + activitySpec.getVersion().getId(), runtimeException); + throw new ActivitySpecNotFoundException(ACTIVITY_SPEC_NOT_FOUND, runtimeException); } if (retrieved != null) { final Version retrievedVersion = versioningManager.get(activitySpec.getId(), @@ -183,23 +185,27 @@ public class ActivitySpecManagerImpl implements ActivitySpecManager { private void updateVersionStatus(String activitySpecId, ActivitySpecAction action, Version version) { VersionStatus prevVersionStatus = null; - Version retrievedVersion = null; + Version retrievedVersion; try { retrievedVersion = versioningManager.get(activitySpecId, version); } catch (SdcRuntimeException exception) { - LOGGER.error("Failed to get version for activitySpecId: " + activitySpecId + LOGGER.debug("Failed to get version for activitySpecId: " + activitySpecId + " and version: " + version.getId(), exception); - validateActivitySpecExistence(activitySpecId, version); + throw new ActivitySpecNotFoundException(ACTIVITY_SPEC_NOT_FOUND, exception); } VersionStatus status = version.getStatus(); Transition transition = TRANSITIONS.get(status); if (transition != null) { - String errMsg = String.format("Activity Spec is in an invalid state", - transition.action, activitySpecId, transition.prevStatus); - validateStatus(Objects.nonNull(retrievedVersion) ? retrievedVersion.getStatus() : null, - transition.prevStatus, errMsg); + + VersionStatus retrievedStatus = Objects.nonNull(retrievedVersion) ? retrievedVersion.getStatus() : null; + if (retrievedStatus != transition.prevStatus) { + LOGGER.debug("Failed to " + version.getStatus() + " since activity spec is in " + + retrievedStatus); + throw new CoreException(new ErrorCode.ErrorCodeBuilder() + .withMessage(INVALID_STATE).build()); + } prevVersionStatus = transition.prevStatus; } @@ -212,33 +218,15 @@ public class ActivitySpecManagerImpl implements ActivitySpecManager { } } - private void validateActivitySpecExistence(String activitySpecId, Version version) { - throw new CoreException(new ErrorCode.ErrorCodeBuilder() - .withCategory(ErrorCategory.APPLICATION) - .withId("ACTIVITYSPEC_NOT_FOUND") - .withMessage("No Activity Spec found for the given identifiers") - .build()); - } - - private void validateStatus(VersionStatus retrievedVersionStatus, - VersionStatus expectedVersionStatus, String errorMessage) { - if (retrievedVersionStatus != expectedVersionStatus) { - throw new CoreException(new ErrorCode.ErrorCodeBuilder() - .withCategory(ErrorCategory.APPLICATION) - .withId("STATUS_NOT_" + expectedVersionStatus.name().toUpperCase()) - .withMessage(errorMessage).build()); - } - } - private Version calculateLatestVersion(String activitySpecId, Version version) { if (ActivitySpecConstant.VERSION_ID_DEFAULT_VALUE.equalsIgnoreCase(version.getId())) { - List list = null; + List list; try { list = versioningManager.list(activitySpecId); } catch (SdcRuntimeException runtimeException) { - LOGGER.error("Failed to list versions for activitySpecId " + LOGGER.debug("Failed to list versions for activitySpecId " + activitySpecId, runtimeException); - validateActivitySpecExistence(activitySpecId, version); + throw new ActivitySpecNotFoundException(ACTIVITY_SPEC_NOT_FOUND, runtimeException); } if (Objects.nonNull(list) && !list.isEmpty()) { return list.get(0); diff --git a/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/errors/ActivitySpecErrorResponse.java b/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/errors/ActivitySpecErrorResponse.java index 8a7bd76347..18ae886d87 100644 --- a/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/errors/ActivitySpecErrorResponse.java +++ b/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/errors/ActivitySpecErrorResponse.java @@ -16,21 +16,15 @@ package org.openecomp.activityspec.errors; -import javax.ws.rs.core.Response; - @lombok.Data public class ActivitySpecErrorResponse { - private Response.Status status; - private String errorCode; private String message; public ActivitySpecErrorResponse() { //default constructor } - public ActivitySpecErrorResponse(Response.Status status, String errorCode, String message) { - this.status = status; - this.errorCode = errorCode; + public ActivitySpecErrorResponse(String message) { this.message = message; } } diff --git a/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/errors/ActivitySpecNotFoundException.java b/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/errors/ActivitySpecNotFoundException.java new file mode 100644 index 0000000000..8c775f935c --- /dev/null +++ b/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/errors/ActivitySpecNotFoundException.java @@ -0,0 +1,24 @@ +/* + * Copyright © 2016-2018 European Support Limited + * + * 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. + */ + +package org.openecomp.activityspec.errors; + +public class ActivitySpecNotFoundException extends RuntimeException { + + public ActivitySpecNotFoundException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/errors/DefaultExceptionMapper.java b/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/errors/DefaultExceptionMapper.java index e95953bd56..950988a4b1 100644 --- a/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/errors/DefaultExceptionMapper.java +++ b/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/errors/DefaultExceptionMapper.java @@ -38,6 +38,8 @@ public class DefaultExceptionMapper implements ExceptionMapper { Response response; if (exception instanceof CoreException) { response = transform(CoreException.class.cast(exception)); + } else if (exception instanceof ActivitySpecNotFoundException) { + response = transform(ActivitySpecNotFoundException.class.cast(exception)); } else if (exception instanceof ConstraintViolationException) { response = transform(ConstraintViolationException.class.cast(exception)); } else if (exception instanceof JsonMappingException) { @@ -49,10 +51,14 @@ public class DefaultExceptionMapper implements ExceptionMapper { return response; } + private Response transform(ActivitySpecNotFoundException notFoundException) { + LOGGER.error("Transforming ActivitySpecNotFoundException to Error Response :", notFoundException); + return generateResponse(Status.NOT_FOUND, new ActivitySpecErrorResponse(notFoundException.getMessage())); + } + private Response transform(CoreException coreException) { LOGGER.error("Transforming CoreException to Error Response :", coreException); - return generateResponse(Status.EXPECTATION_FAILED, new ActivitySpecErrorResponse(Status.EXPECTATION_FAILED, coreException.code().id(), - coreException.getMessage()) ); + return generateResponse(Status.BAD_REQUEST, new ActivitySpecErrorResponse(coreException.getMessage())); } private Response transform(ConstraintViolationException validationException) { @@ -72,22 +78,17 @@ public class DefaultExceptionMapper implements ExceptionMapper { } else { message = validationException.getMessage(); } - return generateResponse(Status.EXPECTATION_FAILED, new ActivitySpecErrorResponse(Status.EXPECTATION_FAILED, - "FIELD_VALIDATION_ERROR_ERR_ID", - String.format(message,fieldName))); - } + return generateResponse(Status.BAD_REQUEST, new ActivitySpecErrorResponse(String.format(message,fieldName))); + } private Response transform(Exception exception) { LOGGER.error("Transforming Exception to Error Response " + exception); - return generateResponse(Status.INTERNAL_SERVER_ERROR, new ActivitySpecErrorResponse(Status.INTERNAL_SERVER_ERROR, - "GENERAL_ERROR_REST_ID", - "An error has occurred: " + exception.getMessage())); + return generateResponse(Status.INTERNAL_SERVER_ERROR, new ActivitySpecErrorResponse(exception.getMessage())); } private Response transform(JsonMappingException jsonMappingException) { LOGGER.error("Transforming JsonMappingException to Error Response " + jsonMappingException); - return generateResponse(Status.EXPECTATION_FAILED, new ActivitySpecErrorResponse(Status.EXPECTATION_FAILED,"JSON_MAPPING_ERROR_ERR_ID", - "Invalid Json Input")); + return generateResponse(Status.BAD_REQUEST, new ActivitySpecErrorResponse("Invalid Json Input")); } private Response generateResponse(Response.Status status, ActivitySpecErrorResponse diff --git a/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/utils/ActivitySpecConstant.java b/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/utils/ActivitySpecConstant.java index 49752ecaef..f63810d884 100644 --- a/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/utils/ActivitySpecConstant.java +++ b/services/activity-spec/activity-spec-web/activity-spec-service/src/main/java/org/openecomp/activityspec/utils/ActivitySpecConstant.java @@ -23,6 +23,8 @@ public class ActivitySpecConstant { public static final String TENANT = "activity_spec"; public static final String USER = "activity_spec_USER"; public static final String USER_ID_HEADER_PARAM = "USER_ID"; + public static final String ACTIVITY_SPEC_NOT_FOUND = "No Activity Spec found for the given identifiers"; + public static final String INVALID_STATE = "Activity Spec is in an invalid state"; private ActivitySpecConstant(){ //Utility Class declaring constants does not require instantiation. diff --git a/services/activity-spec/activity-spec-web/activity-spec-service/src/test/java/org/openecomp/activityspec/be/impl/ActivitySpecManagerImplTest.java b/services/activity-spec/activity-spec-web/activity-spec-service/src/test/java/org/openecomp/activityspec/be/impl/ActivitySpecManagerImplTest.java index ac44b031bf..e930dd9454 100644 --- a/services/activity-spec/activity-spec-web/activity-spec-service/src/test/java/org/openecomp/activityspec/be/impl/ActivitySpecManagerImplTest.java +++ b/services/activity-spec/activity-spec-web/activity-spec-service/src/test/java/org/openecomp/activityspec/be/impl/ActivitySpecManagerImplTest.java @@ -26,6 +26,7 @@ import org.openecomp.activityspec.api.rest.types.ActivitySpecAction; import org.openecomp.activityspec.be.dao.ActivitySpecDao; import org.openecomp.activityspec.be.dao.types.ActivitySpecEntity; import org.openecomp.activityspec.be.datatypes.ActivitySpecParameter; +import org.openecomp.activityspec.errors.ActivitySpecNotFoundException; import org.openecomp.core.dao.UniqueValueDao; import org.openecomp.sdc.common.errors.CoreException; import org.openecomp.sdc.common.errors.SdcRuntimeException; @@ -47,14 +48,14 @@ import static org.mockito.Matchers.anyObject; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.verify; +import static org.openecomp.activityspec.utils.ActivitySpecConstant.ACTIVITY_SPEC_NOT_FOUND; +import static org.openecomp.activityspec.utils.ActivitySpecConstant.INVALID_STATE; import static org.openecomp.activityspec.utils.ActivitySpecConstant.VERSION_ID_DEFAULT_VALUE; public class ActivitySpecManagerImplTest { private static final String STRING_TYPE = "String"; - private static final String ACTIVITYSPEC_NOT_FOUND = "ACTIVITYSPEC_NOT_FOUND"; private static final String TEST_ERROR_MSG = "Test Error"; - private static final String ERROR_MSG_PREFIX = "STATUS_NOT_"; private ActivitySpecEntity input; private static final Version VERSION01 = new Version("12345"); private static final String ID = "ID1"; @@ -188,8 +189,8 @@ public class ActivitySpecManagerImplTest { try { activitySpecManager.get(input); Assert.fail(); - } catch (CoreException exception) { - Assert.assertEquals(exception.code().id(), ACTIVITYSPEC_NOT_FOUND); + } catch (ActivitySpecNotFoundException exception) { + Assert.assertEquals(exception.getMessage(), ACTIVITY_SPEC_NOT_FOUND); } } @@ -203,8 +204,8 @@ public class ActivitySpecManagerImplTest { try { activitySpecManager.get(input); Assert.fail(); - } catch (CoreException exception) { - Assert.assertEquals(exception.code().id(), ACTIVITYSPEC_NOT_FOUND); + } catch (ActivitySpecNotFoundException exception) { + Assert.assertEquals(exception.getMessage(), ACTIVITY_SPEC_NOT_FOUND); } } @@ -215,8 +216,7 @@ public class ActivitySpecManagerImplTest { VERSION01.getId(), ActivitySpecAction.DEPRECATE); } catch (CoreException exception) { - Assert.assertEquals(exception.code().id(), ERROR_MSG_PREFIX +VersionStatus.Certified.name() - .toUpperCase()); + Assert.assertEquals(exception.getMessage(), INVALID_STATE); } } @@ -227,8 +227,7 @@ public class ActivitySpecManagerImplTest { VERSION01.getId(), ActivitySpecAction.DELETE); } catch (CoreException exception) { - Assert.assertEquals(exception.code().id(), ERROR_MSG_PREFIX+VersionStatus.Deprecated.name() - .toUpperCase()); + Assert.assertEquals(exception.getMessage(), INVALID_STATE); } } @@ -252,8 +251,7 @@ public class ActivitySpecManagerImplTest { VERSION01.getId(), ActivitySpecAction.CERTIFY); } catch (CoreException exception) { - Assert.assertEquals(exception.code().id(), ERROR_MSG_PREFIX+VersionStatus.Draft.name() - .toUpperCase()); + Assert.assertEquals(exception.getMessage(), INVALID_STATE); } } @@ -265,8 +263,8 @@ public class ActivitySpecManagerImplTest { activitySpecManager.actOnAction(ID, VERSION01.getId(), ActivitySpecAction.CERTIFY); Assert.fail(); - } catch (CoreException exception) { - Assert.assertEquals(exception.code().id(), ACTIVITYSPEC_NOT_FOUND); + } catch (ActivitySpecNotFoundException exception) { + Assert.assertEquals(exception.getMessage(), ACTIVITY_SPEC_NOT_FOUND); } } -- cgit 1.2.3-korg