Fix bug in change item owner

Change-Id: I200387d41f95d61ad0f6be6663b8d8fac198aa90
Issue-ID: SDC-1355
Signed-off-by: talig <talig@amdocs.com>

5 files changed, 70 insertions, 56 deletions

diff --git a/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-api/src/main/java/org/openecomp/sdc/itempermissions/PermissionsServices.java b/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-api/src/main/java/org/openecomp/sdc/itempermissions/PermissionsServices.java
index 49d72bbcbf..72ae535b20 100644
--- a/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-api/src/main/java/org/openecomp/sdc/itempermissions/PermissionsServices.java
+++ b/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-api/src/main/java/org/openecomp/sdc/itempermissions/PermissionsServices.java
@@ -18,6 +18,7 @@ package org.openecomp.sdc.itempermissions;
 import org.openecomp.sdc.itempermissions.type.ItemPermissionsEntity;
 
 import java.util.Collection;
+import java.util.Optional;
 import java.util.Set;
 
 /**
@@ -32,12 +33,11 @@ public interface PermissionsServices {
   void updateItemPermissions(String itemId, String permission, Set<String> addedUsersIds,
                              Set<String> removedUsersIds);
 
-  boolean isAllowed(String itemId,String userId,String action);
+  boolean isAllowed(String itemId, String userId, String action);
 
-  void execute(String itemId,String userId,String action);
+  void execute(String itemId, String userId, String action);
 
-  String getUserItemPermiission(String itemId, String userId);
+  Optional<String> getUserItemPermission(String itemId, String userId);
 
-
-    void deleteItemPermissions(String itemId);
+  void deleteItemPermissions(String itemId);
 }
diff --git a/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-api/src/main/java/org/openecomp/sdc/itempermissions/dao/ItemPermissionsDao.java b/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-api/src/main/java/org/openecomp/sdc/itempermissions/dao/ItemPermissionsDao.java
index b8b7ddd7a0..ea2211df85 100644
--- a/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-api/src/main/java/org/openecomp/sdc/itempermissions/dao/ItemPermissionsDao.java
+++ b/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-api/src/main/java/org/openecomp/sdc/itempermissions/dao/ItemPermissionsDao.java
@@ -18,6 +18,7 @@ package org.openecomp.sdc.itempermissions.dao;
 import org.openecomp.sdc.itempermissions.type.ItemPermissionsEntity;
 
 import java.util.Collection;
+import java.util.Optional;
 import java.util.Set;
 
 /**
@@ -30,7 +31,7 @@ public interface ItemPermissionsDao {
   void updateItemPermissions(String itemId, String permission, Set<String> addedUsersIds,
                              Set<String> removedUsersIds);
 
-  String getUserItemPermission(String itemId, String userId);
+  Optional<String> getUserItemPermission(String itemId, String userId);
 
   void deleteItemPermissions(String itemId);
 }
diff --git a/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-core/src/main/java/org/openecomp/sdc/itempermissions/dao/impl/ItemPermissionsDaoImpl.java b/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-core/src/main/java/org/openecomp/sdc/itempermissions/dao/impl/ItemPermissionsDaoImpl.java
index 87a9949468..6421c283f3 100644
--- a/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-core/src/main/java/org/openecomp/sdc/itempermissions/dao/impl/ItemPermissionsDaoImpl.java
+++ b/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-core/src/main/java/org/openecomp/sdc/itempermissions/dao/impl/ItemPermissionsDaoImpl.java
@@ -25,6 +25,7 @@ import org.openecomp.sdc.itempermissions.dao.ItemPermissionsDao;
 import org.openecomp.sdc.itempermissions.type.ItemPermissionsEntity;
 
 import java.util.Collection;
+import java.util.Optional;
 import java.util.Set;
 
 /**
@@ -43,23 +44,25 @@ public class ItemPermissionsDaoImpl implements ItemPermissionsDao {
   @Override
   public void updateItemPermissions(String itemId, String permission, Set<String> addedUsersIds,
                                     Set<String> removedUsersIds) {
-    addedUsersIds.forEach(userId ->  accessor.addPermission(itemId,userId,permission));
-    removedUsersIds.forEach(userId -> accessor.deletePermission(itemId,userId));
+    addedUsersIds.forEach(userId -> accessor.addPermission(itemId, userId, permission));
+    removedUsersIds.stream()
+        .filter(userId -> getUserItemPermission(itemId, userId)
+            .map(userPermissionOnItem -> userPermissionOnItem.equals(permission))
+            .orElse(false))
+        .forEach(userId -> accessor.deletePermission(itemId, userId));
   }
 
   @Override
-  public String getUserItemPermission(String itemId, String userId) {
-
-    ResultSet result =  accessor.getUserItemPermission(itemId,userId);
-    if (result.getAvailableWithoutFetching() < 1) {
-      return null;
-    }
-    return result.one().getString(0);
+  public Optional<String> getUserItemPermission(String itemId, String userId) {
+    ResultSet result = accessor.getUserItemPermission(itemId, userId);
+    return result.getAvailableWithoutFetching() < 1
+        ? Optional.empty()
+        : Optional.of(result.one().getString(0));
   }
 
   @Override
   public void deleteItemPermissions(String itemId) {
-   accessor.deleteItemPermissions(itemId);
+    accessor.deleteItemPermissions(itemId);
   }
 
 
@@ -69,13 +72,13 @@ public class ItemPermissionsDaoImpl implements ItemPermissionsDao {
     Result<ItemPermissionsEntity> getItemPermissions(String itemId);
 
     @Query("select permission from dox.item_permissions WHERE item_id = ? AND user_id=?")
-    ResultSet getUserItemPermission(String itemId,String userId);
+    ResultSet getUserItemPermission(String itemId, String userId);
 
     @Query("delete from dox.item_permissions where item_id = ? and user_id = ?")
     void deletePermission(String itemId, String userId);
 
     @Query("insert into dox.item_permissions (item_id,user_id,permission) values (?,?,?)")
-    void addPermission(String itemId,String userId, String permission);
+    void addPermission(String itemId, String userId, String permission);
 
     @Query("delete from dox.item_permissions where item_id=?")
     void deleteItemPermissions(String itemId);
diff --git a/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-core/src/main/java/org/openecomp/sdc/itempermissions/dao/impl/PermissionsServicesImpl.java b/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-core/src/main/java/org/openecomp/sdc/itempermissions/dao/impl/PermissionsServicesImpl.java
index 608102916e..ec914b1530 100644
--- a/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-core/src/main/java/org/openecomp/sdc/itempermissions/dao/impl/PermissionsServicesImpl.java
+++ b/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-core/src/main/java/org/openecomp/sdc/itempermissions/dao/impl/PermissionsServicesImpl.java
@@ -23,6 +23,7 @@ import org.openecomp.sdc.itempermissions.dao.UserPermissionsDao;
 import org.openecomp.sdc.itempermissions.type.ItemPermissionsEntity;
 
 import java.util.Collection;
+import java.util.Optional;
 import java.util.Set;
 
 
@@ -40,7 +41,8 @@ public class PermissionsServicesImpl implements PermissionsServices {
   private static final String CHANGE_PERMISSIONS = "Change_Item_Permissions";
 
   public PermissionsServicesImpl(PermissionsRules permissionsRules,
-                                 ItemPermissionsDao itemPermissionsDao,UserPermissionsDao userPermissionsDao) {
+                                 ItemPermissionsDao itemPermissionsDao,
+                                 UserPermissionsDao userPermissionsDao) {
     this.itemPermissionsDao = itemPermissionsDao;
     this.permissionsRules = permissionsRules;
     this.userPermissionsDao = userPermissionsDao;
@@ -54,7 +56,7 @@ public class PermissionsServicesImpl implements PermissionsServices {
 
   @Override
   public Set<String> listUserPermittedItems(String userId, String permission) {
-    return userPermissionsDao.listUserPermittedItems(userId,permission);
+    return userPermissionsDao.listUserPermittedItems(userId, permission);
   }
 
   @Override
@@ -62,36 +64,36 @@ public class PermissionsServicesImpl implements PermissionsServices {
                                     Set<String> removedUsersIds) {
 
     String currentUserId = SessionContextProviderFactory.getInstance()
-          .createInterface().get().getUser().getUserId();
+        .createInterface().get().getUser().getUserId();
 
-    permissionsRules.executeAction(itemId,currentUserId,CHANGE_PERMISSIONS);
+    permissionsRules.executeAction(itemId, currentUserId, CHANGE_PERMISSIONS);
 
-    permissionsRules.updatePermission(itemId,currentUserId,permission,addedUsersIds,
-          removedUsersIds);
+    permissionsRules.updatePermission(itemId, currentUserId, permission, addedUsersIds,
+        removedUsersIds);
 
     itemPermissionsDao.updateItemPermissions(itemId, permission,
-          addedUsersIds, removedUsersIds);
+        addedUsersIds, removedUsersIds);
 
     userPermissionsDao.updatePermissions(itemId, permission,
-            addedUsersIds, removedUsersIds);
+        addedUsersIds, removedUsersIds);
 
   }
 
   @Override
-  public boolean isAllowed(String itemId,String userId,String action) {
-
-    String userPermission = itemPermissionsDao.getUserItemPermission(itemId,userId);
-    return permissionsRules.isAllowed(userPermission,action);
+  public boolean isAllowed(String itemId, String userId, String action) {
+    return itemPermissionsDao.getUserItemPermission(itemId, userId)
+        .map(permission -> permissionsRules.isAllowed(permission, action))
+        .orElse(false);
   }
 
   @Override
-  public void execute(String itemId,String userId,String action) {
+  public void execute(String itemId, String userId, String action) {
     permissionsRules.executeAction(itemId, userId, action);
   }
 
   @Override
-  public String getUserItemPermiission(String itemId, String userId) {
-    return itemPermissionsDao.getUserItemPermission(itemId,userId);
+  public Optional<String> getUserItemPermission(String itemId, String userId) {
+    return itemPermissionsDao.getUserItemPermission(itemId, userId);
   }
 
   @Override
diff --git a/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-impl/src/main/java/org/openecomp/sdc/itempermissions/impl/PermissionsRulesImpl.java b/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-impl/src/main/java/org/openecomp/sdc/itempermissions/impl/PermissionsRulesImpl.java
index feb3d59f7f..2ab9326f6a 100644
--- a/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-impl/src/main/java/org/openecomp/sdc/itempermissions/impl/PermissionsRulesImpl.java
+++ b/openecomp-be/lib/openecomp-item-permissions-lib/openecomp-item-permissions-impl/src/main/java/org/openecomp/sdc/itempermissions/impl/PermissionsRulesImpl.java
@@ -15,8 +15,10 @@
  */
 package org.openecomp.sdc.itempermissions.impl;
 
+import org.apache.commons.collections.CollectionUtils;
 import org.openecomp.sdc.common.errors.CoreException;
 import org.openecomp.sdc.itempermissions.PermissionsRules;
+import org.openecomp.sdc.itempermissions.PermissionsServices;
 import org.openecomp.sdc.itempermissions.PermissionsServicesFactory;
 import org.openecomp.sdc.itempermissions.errors.PermissionsErrorMessagesBuilder;
 import org.openecomp.sdc.itempermissions.impl.types.PermissionActionTypes;
@@ -118,33 +120,39 @@ public class PermissionsRulesImpl implements PermissionsRules {
         }
     }
 
-    @Override
-    public void updatePermission(String itemId, String currentUserId, String permission, Set<String>
-            addedUsersIds, Set<String> removedUsersIds) {
-        try {
-            PermissionTypes.valueOf(permission);
-        } catch (IllegalArgumentException ex) {
-            throw new CoreException(new PermissionsErrorMessagesBuilder(INVALID_PERMISSION_TYPE).build());
-        }
-
-        if (permission.equals(PermissionTypes.Owner.name())) {
-          makeCurrentUserContributor(itemId,currentUserId);
-        }
+  @Override
+  public void updatePermission(String itemId, String currentUserId, String permission, Set<String>
+      addedUsersIds, Set<String> removedUsersIds) {
+    try {
+      PermissionTypes.valueOf(permission);
+    } catch (IllegalArgumentException ex) {
+      throw new CoreException(new PermissionsErrorMessagesBuilder(INVALID_PERMISSION_TYPE).build());
     }
 
-    private void makeCurrentUserContributor(String itemId, String currentUserId) {
-
-        String currentPermission = PermissionsServicesFactory.getInstance().createInterface().
-                getUserItemPermiission(itemId,currentUserId);
-
-        if(currentPermission != null) {
-
-            PermissionsServicesFactory.getInstance().createInterface()
-                    .updateItemPermissions(itemId, PermissionTypes.Contributor.name(),
-                            Collections.singleton(currentUserId), new HashSet<>());
+    if (isOwnerAdded(permission, addedUsersIds)) {
+      handleCurrentOwner(itemId, currentUserId);
+    }
+  }
+
+  private boolean isOwnerAdded(String permission, Set<String> addedUsersIds) {
+    return permission.equals(PermissionTypes.Owner.name()) &&
+        CollectionUtils.isNotEmpty(addedUsersIds);
+  }
+
+  private void handleCurrentOwner(String itemId, String currentUserId) {
+    PermissionsServices permissionsServices =
+        PermissionsServicesFactory.getInstance().createInterface();
+    if (!permissionsServices.getUserItemPermission(itemId, currentUserId).isPresent()) {
+      return; // no current owner - first owner addition
     }
 
-}
+    Set<String> currentUserSet = Collections.singleton(currentUserId);
+    permissionsServices
+        .updateItemPermissions(itemId, PermissionTypes.Contributor.name(), currentUserSet,
+            new HashSet<>());
+    permissionsServices.updateItemPermissions(itemId, PermissionTypes.Owner.name(), new HashSet<>(),
+        currentUserSet);
+  }
 
     private void caseCreateItem(String userId, String itemId) {
         HashSet<String> ownerId = new HashSet<>();