Refactor ArtifactsBusinessLogic::handleDelete and ArtifactsBusinessLogic::deleteArtif... 86/107386/3
authorFrancis Toth <francis.toth@yoppworks.com>
Fri, 8 May 2020 12:17:33 +0000 (08:17 -0400)
committerOfir Sonsino <ofir.sonsino@intl.att.com>
Sun, 10 May 2020 06:52:51 +0000 (06:52 +0000)
Signed-off-by: Francis Toth <francis.toth@yoppworks.com>
Change-Id: I89f12c70f388375ed13554edf62bf6a442c9036a
Issue-ID: SDC-2812

catalog-be/src/main/java/org/openecomp/sdc/be/components/csar/CsarArtifactsAndGroupsBusinessLogic.java
catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ArtifactsBusinessLogic.java
catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ResourceBusinessLogic.java
catalog-be/src/main/java/org/openecomp/sdc/be/impl/ComponentsUtils.java
catalog-be/src/test/java/org/openecomp/sdc/be/components/impl/ArtifactBusinessLogicTest.java
catalog-be/src/test/java/org/openecomp/sdc/be/components/impl/ArtifactsBusinessLogicTest.java

index b034400..3794343 100644 (file)
@@ -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);
index f34f3a6..94c379a 100644 (file)
@@ -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
      *
index 71707e6..114a9c7 100644 (file)
@@ -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());
index d50b91f..62a3063 100644 (file)
@@ -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));
+    }
 }
index e3e6924..fddb76e 100644 (file)
@@ -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();
     }
 
index cd171d9..6f0b83b 100644 (file)
@@ -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