Refactor ArtifactsBusinessLogic::handleDelete and ArtifactsBusinessLogic::deleteArtifactByInterface

Signed-off-by: Francis Toth <francis.toth@yoppworks.com>
Change-Id: I89f12c70f388375ed13554edf62bf6a442c9036a
Issue-ID: SDC-2812

6 files changed, 53 insertions, 63 deletions

diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/components/csar/CsarArtifactsAndGroupsBusinessLogic.java b/catalog-be/src/main/java/org/openecomp/sdc/be/components/csar/CsarArtifactsAndGroupsBusinessLogic.java
index b0344007f0..3794343ee4 100644
--- a/catalog-be/src/main/java/org/openecomp/sdc/be/components/csar/CsarArtifactsAndGroupsBusinessLogic.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/be/components/csar/CsarArtifactsAndGroupsBusinessLogic.java
@@ -1247,14 +1247,15 @@ public class CsarArtifactsAndGroupsBusinessLogic extends BaseBusinessLogic {
                 String artifactType = artifact.getArtifactType();
                 final ArtifactTypeEnum artifactTypeEnum = ArtifactTypeEnum.parse(artifactType);
                 if (artifactTypeEnum != ArtifactTypeEnum.HEAT_ENV) {
-                    Either<Either<ArtifactDefinition, Operation>, ResponseFormat> handleDelete = artifactsBusinessLogic
-                            .handleDelete(resourceId, artifact.getUniqueId(), user, AuditingActionEnum.ARTIFACT_DELETE,
-                                    ComponentTypeEnum.RESOURCE, updatedResource, shouldLock, inTransaction);
+                    Either<ArtifactDefinition, ResponseFormat> handleDelete = artifactsBusinessLogic
+                            .handleDelete(resourceId, artifact.getUniqueId(), user,
+                                updatedResource, shouldLock, inTransaction);
+
                     if (handleDelete.isRight()) {
                         return Either.right(handleDelete.right().value());
                     }
 
-                    deletedArtifacts.add(handleDelete.left().value().left().value());
+                    deletedArtifacts.add(handleDelete.left().value());
                 }
 
             }
@@ -1740,7 +1741,9 @@ public class CsarArtifactsAndGroupsBusinessLogic extends BaseBusinessLogic {
                 for(GroupDefinition gr : vfGroupsToDelete){
                     List<String> artifacts = gr.getArtifacts();
                     for (String artifactId : artifacts) {
-                        Either<Either<ArtifactDefinition, Operation>, ResponseFormat> handleDelete = artifactsBusinessLogic.handleDelete(updatedResource.getUniqueId(), artifactId, csarInfo.getModifier(), AuditingActionEnum.ARTIFACT_DELETE, ComponentTypeEnum.RESOURCE,
+                        Either<ArtifactDefinition, ResponseFormat> handleDelete =
+                            artifactsBusinessLogic.handleDelete(
+                                updatedResource.getUniqueId(), artifactId, csarInfo.getModifier(),
                                 updatedResource, shouldLock, inTransaction);
                         if (handleDelete.isRight()) {
                             log.debug("Couldn't delete  artifact {}", artifactId);
diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ArtifactsBusinessLogic.java b/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ArtifactsBusinessLogic.java
index f34f3a6c00..94c379ae29 100644
--- a/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ArtifactsBusinessLogic.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ArtifactsBusinessLogic.java
@@ -1248,26 +1248,31 @@ public class ArtifactsBusinessLogic extends BaseBusinessLogic {
     }
 
     // This method is here for backward compatibility - when other parts of the code are cleaned can change to use the internal version
-    public Either<Either<ArtifactDefinition, Operation>, ResponseFormat> handleDelete(String parentId, String artifactId, User user, AuditingActionEnum auditingAction, ComponentTypeEnum componentType, Component parent,
-                                                                                      boolean shouldLock, boolean inTransaction) {
+    public Either<ArtifactDefinition, ResponseFormat> handleDelete(
+        String parentId, String artifactId, User user, Component parent,
+        boolean shouldLock, boolean inTransaction) {
+
         ResponseFormat responseFormat;
         boolean operationSucceeded = false;
         if (shouldLock) {
-            lockComponent(componentType, artifactId, auditingAction, user, parent);
+            lockComponent(ComponentTypeEnum.RESOURCE, artifactId, AuditingActionEnum.ARTIFACT_DELETE, user, parent);
         }
         try {
-            ArtifactDefinition artifactDefinition = handleDeleteInternal(parentId, artifactId, componentType, parent);
+            ArtifactDefinition artifactDefinition = handleDeleteInternal(parentId, artifactId,
+                ComponentTypeEnum.RESOURCE, parent);
             operationSucceeded = true;
-            return Either.left(Either.left(artifactDefinition));
+            return Either.left(artifactDefinition);
         }
         catch (ComponentException ce) {
             responseFormat = componentsUtils.getResponseFormat(ce);
-            handleAuditing(auditingAction, parent, parentId, user, null, null, artifactId, responseFormat, componentType, null);
+            handleAuditing(AuditingActionEnum.ARTIFACT_DELETE, parent, parentId, user, null, null,
+                artifactId, responseFormat, ComponentTypeEnum.RESOURCE, null);
             return Either.right(responseFormat);
         }
         catch (StorageException se) {
             responseFormat = componentsUtils.getResponseFormat(se);
-            handleAuditing(auditingAction, parent, parentId, user, null, null, artifactId, responseFormat, componentType, null);
+            handleAuditing(AuditingActionEnum.ARTIFACT_DELETE, parent, parentId, user, null, null,
+                artifactId, responseFormat, ComponentTypeEnum.RESOURCE, null);
             return Either.right(responseFormat);
         } finally {
             handleLockingAndCommit(parent, shouldLock, inTransaction, operationSucceeded);
@@ -2670,28 +2675,18 @@ public class ArtifactsBusinessLogic extends BaseBusinessLogic {
         return Either.left(decodedPayload);
     }
 
+    public Either<ArtifactDefinition, ResponseFormat> deleteArtifactByInterface(
+        String resourceId, String userUserId, String artifactId, boolean inTransaction) {
 
-    public Either<Operation, ResponseFormat> deleteArtifactByInterface(String resourceId, String userUserId, String artifactId,
-                                                                       boolean inTransaction) {
-        User user = new User();
-        user.setUserId(userUserId);
-        Either<Resource, StorageOperationStatus> parent = toscaOperationFacade.getToscaElement(resourceId, JsonParseFlagEnum.ParseMetadata);
-        if (parent.isRight()) {
-            ResponseFormat responseFormat = componentsUtils.getResponseFormat(componentsUtils.convertFromStorageResponse(parent
-                    .right()
-                    .value()));
-            return Either.right(responseFormat);
-        }
-        Either<Either<ArtifactDefinition, Operation>, ResponseFormat> handleDelete = handleDelete(resourceId, artifactId, user, AuditingActionEnum.ARTIFACT_DELETE, ComponentTypeEnum.RESOURCE, parent
-                        .left()
-                        .value(),
-                false, inTransaction);
-        if (handleDelete.isRight()) {
-            return Either.right(handleDelete.right().value());
-        }
-        Either<ArtifactDefinition, Operation> result = handleDelete.left().value();
-        return Either.left(result.right().value());
-
+        return toscaOperationFacade
+            .getToscaElement(resourceId, JsonParseFlagEnum.ParseMetadata)
+            .right().map(componentsUtils.toResponseFormat())
+            .left().bind(parentComponent -> {
+                User user = new User(userUserId);
+                return handleDelete(resourceId, artifactId, user,
+                    parentComponent,
+                    false, inTransaction);
+            });
     }
 
     private Operation convertToOperation(ArtifactDefinition artifactInfo, String operationName) {
@@ -4230,7 +4225,6 @@ public class ArtifactsBusinessLogic extends BaseBusinessLogic {
         return Either.left(artifactInfo);
     }
 
-
     /**
      * updates an artifact on a component by UUID
      *
diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ResourceBusinessLogic.java b/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ResourceBusinessLogic.java
index 71707e61f9..114a9c76cc 100644
--- a/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ResourceBusinessLogic.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ResourceBusinessLogic.java
@@ -2319,22 +2319,20 @@ public class ResourceBusinessLogic extends ComponentBusinessLogic {
 		Either<Boolean, ResponseFormat> result = Either.left(true);
 		if (operation.isUpdate() || operation.isDelete()) {
 			if (isArtifactDeletionRequired(artifactId, artifactFileBytes, isFromCsar)) {
-				Either<Either<ArtifactDefinition, Operation>, ResponseFormat> handleDelete = artifactsBusinessLogic
+				Either<ArtifactDefinition, ResponseFormat> handleDelete = artifactsBusinessLogic
 						.handleDelete(resource.getUniqueId(), artifactId, csarInfo.getModifier(),
-								AuditingActionEnum.ARTIFACT_DELETE, ComponentTypeEnum.RESOURCE, resource, shouldLock,
+							resource, shouldLock,
 								inTransaction);
 				if (handleDelete.isRight()) {
 					result = Either.right(handleDelete.right()
 							.value());
 				} else {
-					Either<ArtifactDefinition, Operation> value = handleDelete.left().value();
-					if (value.isLeft()) {
-						String updatedArtifactId = value.left().value().getUniqueId();
-						if (artifactGroupType == ArtifactGroupTypeEnum.DEPLOYMENT) {
-							resource.getDeploymentArtifacts().remove(updatedArtifactId);
-						} else {
-							resource.getArtifacts().remove(updatedArtifactId);
-						}
+					ArtifactDefinition value = handleDelete.left().value();
+					String updatedArtifactId = value.getUniqueId();
+					if (artifactGroupType == ArtifactGroupTypeEnum.DEPLOYMENT) {
+						resource.getDeploymentArtifacts().remove(updatedArtifactId);
+					} else {
+						resource.getArtifacts().remove(updatedArtifactId);
 					}
 				}
 				return result;
@@ -5505,7 +5503,6 @@ public class ResourceBusinessLogic extends ComponentBusinessLogic {
 
 	private Either<Boolean, ResponseFormat> processUpdateOfDerivedFrom(Resource currentResource,
 																	   Resource updatedResource, String userId, boolean inTransaction) {
-		Either<Operation, ResponseFormat> deleteArtifactByInterface;
 		if (updatedResource.getDerivedFrom() != null) {
 			log.debug("Starting derived from update for resource {}", updatedResource.getUniqueId());
 			log.debug("1. Removing interface artifacts from graph");
@@ -5529,12 +5526,12 @@ public class ResourceBusinessLogic extends ComponentBusinessLogic {
 								log.debug("Removing interface artifact definition {}, operation {}, interfaceType {}",
 										uniqueId, operationEntry.getKey(), interfaceType);
 								// only thing that transacts and locks here
-								deleteArtifactByInterface = artifactsBusinessLogic.deleteArtifactByInterface(resourceId,
-										userId, uniqueId, true);
+								Either<ArtifactDefinition, ResponseFormat> deleteArtifactByInterface =
+									artifactsBusinessLogic.deleteArtifactByInterface(resourceId, userId, uniqueId, true);
 								if (deleteArtifactByInterface.isRight()) {
 									log.debug("Couldn't remove artifact definition with id {}", uniqueId);
 									if (!inTransaction) {
-                                        janusGraphDao.rollback();
+										janusGraphDao.rollback();
 									}
 									return Either.right(deleteArtifactByInterface.right()
 											.value());
diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/impl/ComponentsUtils.java b/catalog-be/src/main/java/org/openecomp/sdc/be/impl/ComponentsUtils.java
index d50b91fadc..62a30638e6 100644
--- a/catalog-be/src/main/java/org/openecomp/sdc/be/impl/ComponentsUtils.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/be/impl/ComponentsUtils.java
@@ -28,7 +28,9 @@ import com.fasterxml.jackson.databind.module.SimpleModule;
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
 import com.google.gson.reflect.TypeToken;
+import fj.F;
 import fj.data.Either;
+import java.util.function.Function;
 import org.apache.commons.lang3.StringUtils;
 import org.onap.logging.ref.slf4j.ONAPLogConstants;
 import org.openecomp.sdc.be.auditing.api.AuditEventFactory;
@@ -1667,5 +1669,7 @@ public class ComponentsUtils {
         return uiLeftPaletteComponents;
     }
 
-
+    public F<StorageOperationStatus, ResponseFormat> toResponseFormat() {
+        return sos -> getResponseFormat(convertFromStorageResponse(sos));
+    }
 }
diff --git a/catalog-be/src/test/java/org/openecomp/sdc/be/components/impl/ArtifactBusinessLogicTest.java b/catalog-be/src/test/java/org/openecomp/sdc/be/components/impl/ArtifactBusinessLogicTest.java
index e3e6924b7e..fddb76efe6 100644
--- a/catalog-be/src/test/java/org/openecomp/sdc/be/components/impl/ArtifactBusinessLogicTest.java
+++ b/catalog-be/src/test/java/org/openecomp/sdc/be/components/impl/ArtifactBusinessLogicTest.java
@@ -65,10 +65,7 @@ import org.openecomp.sdc.be.model.jsonjanusgraph.operations.ToscaOperationFacade
 import org.openecomp.sdc.be.model.operations.api.IGraphLockOperation;
 import org.openecomp.sdc.be.model.operations.api.IInterfaceLifecycleOperation;
 import org.openecomp.sdc.be.model.operations.api.StorageOperationStatus;
-import org.openecomp.sdc.be.model.operations.impl.ArtifactOperation;
-import org.openecomp.sdc.be.model.operations.impl.UserAdminOperation;
 import org.openecomp.sdc.be.resources.data.DAOArtifactData;
-import org.openecomp.sdc.be.resources.data.auditing.AuditingActionEnum;
 import org.openecomp.sdc.be.servlets.RepresentationUtils;
 import org.openecomp.sdc.be.tosca.CsarUtils;
 import org.openecomp.sdc.be.tosca.ToscaExportHandler;
@@ -573,7 +570,6 @@ public class ArtifactBusinessLogicTest extends BaseBusinessLogicMock{
 
     @Test
     public void testDeleteComponent_ArtifactNotFound(){
-        Either<Either<ArtifactDefinition, Operation>, ResponseFormat> result;
         Resource resource = new Resource();
         String uniqueId = "uniqueId";
         resource.setUniqueId(uniqueId);
@@ -593,9 +589,9 @@ public class ArtifactBusinessLogicTest extends BaseBusinessLogicMock{
                 .thenReturn(StorageOperationStatus.OK);
         when(graphLockOperation.unlockComponent(uniqueId, NodeTypeEnum.Resource))
                 .thenReturn(StorageOperationStatus.OK);
-        result = artifactBL.handleDelete("parentId", "artifactId", USER, AuditingActionEnum.ARTIFACT_DELETE,
-                ComponentTypeEnum.RESOURCE, resource,
-                true, false);
+        Either<ArtifactDefinition, ResponseFormat>  result = artifactBL.handleDelete(
+            "parentId", "artifactId", USER,
+            resource, true, false);
         assertThat(result.isRight()).isTrue();
     }
 
diff --git a/catalog-be/src/test/java/org/openecomp/sdc/be/components/impl/ArtifactsBusinessLogicTest.java b/catalog-be/src/test/java/org/openecomp/sdc/be/components/impl/ArtifactsBusinessLogicTest.java
index cd171d9b65..6f0b83b0d5 100644
--- a/catalog-be/src/test/java/org/openecomp/sdc/be/components/impl/ArtifactsBusinessLogicTest.java
+++ b/catalog-be/src/test/java/org/openecomp/sdc/be/components/impl/ArtifactsBusinessLogicTest.java
@@ -2373,12 +2373,8 @@ public class ArtifactsBusinessLogicTest extends BaseBusinessLogicMock {
 
         String parentId = "parentId";
         String artifactId = "artifactId";
-        AuditingActionEnum auditingAction = AuditingActionEnum.ARTIFACT_DELETE;
         ArtifactDefinition artifactDefinition = new ArtifactDefinition();
         Resource resource = new Resource();
-        ComponentTypeEnum componentType = ComponentTypeEnum.RESOURCE;
-        boolean shouldUnlock = true;
-        boolean inTransaction = false;
         User user = new User();
 
         artifactDefinition.setArtifactName("test.csar");
@@ -2405,9 +2401,9 @@ public class ArtifactsBusinessLogicTest extends BaseBusinessLogicMock {
         when(artifactCassandraDao.deleteArtifact(any()))
                 .thenReturn(CassandraOperationStatus.OK);
 
-        Either<Either<ArtifactDefinition, Operation>, ResponseFormat> result = artifactBL.handleDelete(parentId, artifactId, user, auditingAction, componentType, resource, shouldUnlock, inTransaction);
-        Either<ArtifactDefinition, Operation> leftValue = result.left().value();
-        Assert.assertEquals(artifactDefinition.getArtifactName(), leftValue.left().value().getArtifactName());
+        Either<ArtifactDefinition, ResponseFormat> result = artifactBL.handleDelete(
+            parentId, artifactId, user, resource, true, false);
+        Assert.assertEquals(artifactDefinition.getArtifactName(), result.left().value().getArtifactName());
     }
 
     @Test