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>

6 files changed, 74 insertions, 67 deletions

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);
     }
   }