diff options
author | sheetalm <sheetal.mudholkar@amdocs.com> | 2018-04-02 18:12:54 +0530 |
---|---|---|
committer | Vitaly Emporopulo <Vitaliy.Emporopulo@amdocs.com> | 2018-04-10 14:37:17 +0000 |
commit | 40bd5b855df62e257e4cc2810a95248bc3426302 (patch) | |
tree | 518cff1b6aaf972a3f06066f8e47ac1a5e65bfee | |
parent | 7a9073719da3db83024e66a1da9ba14546cfd175 (diff) |
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 <sheetal.mudholkar@amdocs.com>
8 files changed, 85 insertions, 78 deletions
diff --git a/openecomp-bdd/features/ActivitySpec/TestCreate.feature b/openecomp-bdd/features/ActivitySpec/TestCreate.feature index accae6ad11..c4e77a7c1b 100644 --- a/openecomp-bdd/features/ActivitySpec/TestCreate.feature +++ b/openecomp-bdd/features/ActivitySpec/TestCreate.feature @@ -35,9 +35,9 @@ # And I want to get the ActivitySpec for the current item # And I want to check property "status" for value "Deleted" # -# #Pass Invalid Id to Get and verify error code +# #Pass Invalid Id to Get and verify error message # Then I want to set property "item.id" to value "invalidId" -# Then I want the following to fail with error code "ACTIVITYSPEC_NOT_FOUND" +# Then I want the following to fail with error message "No Activity Spec found for the given identifiers" # And I want to get the ActivitySpec for the current item # # # SDC-6353 @@ -49,22 +49,22 @@ # Then I want to check property "id" exists # And I want to check property "versionId" exists # -# #Again Create ActivitySpec with name "test" and verify error code +# #Again Create ActivitySpec with name "test" and verify error message # When I want to set the input data to file "resources/json/createActivitySpec.json" # Then I want to update the input property "name" with value "test" -# Then I want the following to fail with error code "UNIQUE_VALUE_VIOLATION" +# Then I want the following to fail with error message "name already in use" # When I want to create an ActivitySpec # # # SDC-6354 # Scenario: Test Create Activity Spec With Invalid Name Format # When I want to set the input data to file "resources/json/createActivitySpec.json" # Then I want to update the input property "name" with value "test!@" -# Then I want the following to fail with error code "FIELD_VALIDATION_ERROR_ERR_ID" +# Then I want the following to fail with error message "name should match with \"^[a-zA-Z0-9-]*$\" pattern" # When I want to create an ActivitySpec # # # SDC-6355 # Scenario: Test Create Activity Spec With Null/Blank Name # When I want to set the input data to file "resources/json/createActivitySpec.json" # Then I want to update the input property "name" with value "" -# Then I want the following to fail with error code "FIELD_VALIDATION_ERROR_ERR_ID" +# Then I want the following to fail with error message "Mandatory name field is missing/null" # When I want to create an ActivitySpec
\ No newline at end of file diff --git a/openecomp-bdd/features/ActivitySpec/TestInvalidStatusTransition.feature b/openecomp-bdd/features/ActivitySpec/TestInvalidStatusTransition.feature index 1a94635a49..452570e421 100644 --- a/openecomp-bdd/features/ActivitySpec/TestInvalidStatusTransition.feature +++ b/openecomp-bdd/features/ActivitySpec/TestInvalidStatusTransition.feature @@ -14,16 +14,16 @@ # When I want to get the ActivitySpec for the current item # Then I want to check property "status" for value "Draft" # -# #Deprecate "Draft" activity status and verify error code -# Then I want the following to fail with error code "STATUS_NOT_CERTIFIED" +# #Deprecate "Draft" activity status and verify error message +# Then I want the following to fail with error message "Activity Spec is in an invalid state" # When I want to call action "DEPRECATE" on this ActivitySpec item # # #Delete "Draft" activity spec and verify error code -# Then I want the following to fail with error code "STATUS_NOT_DEPRECATED" +# Then I want the following to fail with error message "Activity Spec is in an invalid state" # When I want to call action "DELETE" on this ActivitySpec item # # #Certify activity spec # When I want to call action "CERTIFY" on this ActivitySpec item # #Certify "certified" activity spec and verify error code -# Then I want the following to fail with error code "STATUS_NOT_DRAFT" -# When I want to call action "CERTIFY" on this ActivitySpec item +# Then I want the following to fail with error message "Activity Spec is in an invalid state" +# When I want to call action "CERTIFY" on this ActivitySpec item
\ No newline at end of file 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<Version> list = null; + List<Version> 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<Exception> { 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<Exception> { 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<Exception> { } 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); } } |