Keep volume policies after migrating it to another primary storage (#5067)

* Add commons-lang3 to Utils

* Create an util to provide methods that ReflectionToStringBuilder does not have yet

* Create method to retrieve map of tags from resource

* Enable tests on volume components and remove useless tests

* Refactor VolumeObject and add unit tests

* Extract createPolicy in several methods

* Create method to copy policies between volumes and add unit tests

* Copy policies to new volume before removing old volume on volume migration

* Extract "destroySourceVolumeAfterMigration" to a method and test it

* Remove javadoc @param with no sensible information

* Rename method name to a generic name

Co-authored-by: Daniel Augusto Veronezi Salvador <daniel@scclouds.com.br>

api/src/main/java/com/cloud/server/TaggedResourceService.java

@@ -54,4 +54,12 @@ public interface TaggedResourceService {
     String getUuid(String resourceId, ResourceObjectType resourceType);
 
     public long getResourceId(String resourceId, ResourceObjectType resourceType);
+
+    /**
+     * Retrieves tags from resource.
+     * @param type
+     * @param resourceId
+     * @return If the list of tags is not null, returns a map with the tags, otherwise, returns null.
+     */
+    public Map<String, String> getTagsFromResource(ResourceObjectType type, long resourceId);
 }

engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java

@@ -18,6 +18,7 @@
  */
 package org.apache.cloudstack.engine.subsystem.api.storage;
 
+import com.cloud.agent.api.Answer;
 import java.util.Map;
 
 import org.apache.cloudstack.engine.cloud.entity.api.VolumeEntity;
@@ -100,4 +101,11 @@ public interface VolumeService {
     VolumeInfo updateHypervisorSnapshotReserveForVolume(DiskOffering diskOffering, long volumeId, HypervisorType hyperType);
 
     void unmanageVolume(long volumeId);
+
+    /**
+     * After volume migration, copies snapshot policies from the source volume to destination volume; then, it destroys and expunges the source volume.
+     * @return If no exception happens, it will return false, otherwise true.
+     */
+    boolean copyPoliciesBetweenVolumesAndDestroySourceVolumeAfterMigration(ObjectInDataStoreStateMachine.Event destinationEvent, Answer destinationEventAnswer,
+      VolumeInfo sourceVolume, VolumeInfo destinationVolume, boolean retryExpungeVolumeAsync);
 }

engine/schema/src/main/java/com/cloud/storage/VolumeVO.java

@@ -36,6 +36,7 @@ import com.cloud.storage.Storage.ProvisioningType;
 import com.cloud.storage.Storage.StoragePoolType;
 import com.cloud.utils.NumbersUtil;
 import com.cloud.utils.db.GenericDao;
+import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
 
 @Entity
 @Table(name = "volumes")
@@ -643,4 +644,8 @@ public class VolumeVO implements Volume {
     public Class<?> getEntityType() {
         return Volume.class;
     }
+
+    public String getVolumeDescription(){
+        return ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, "name", "uuid");
+    }
 }

engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java

@@ -2062,30 +2062,12 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
             handleQualityOfServiceForVolumeMigration(destVolumeInfo, PrimaryDataStoreDriver.QualityOfServiceState.NO_MIGRATION);
 
             if (success) {
-                srcVolumeInfo.processEvent(Event.OperationSuccessed);
-                destVolumeInfo.processEvent(Event.OperationSuccessed);
-
-                _volumeDao.updateUuid(srcVolumeInfo.getId(), destVolumeInfo.getId());
-
                 VolumeVO volumeVO = _volumeDao.findById(destVolumeInfo.getId());
-
                 volumeVO.setFormat(ImageFormat.QCOW2);
-
                 _volumeDao.update(volumeVO.getId(), volumeVO);
 
-                try {
+                _volumeService.copyPoliciesBetweenVolumesAndDestroySourceVolumeAfterMigration(Event.OperationSuccessed, null, srcVolumeInfo, destVolumeInfo, false);
-                    _volumeService.destroyVolume(srcVolumeInfo.getId());
 
-                    srcVolumeInfo = _volumeDataFactory.getVolume(srcVolumeInfo.getId());
-
-                    AsyncCallFuture<VolumeApiResult> destroyFuture = _volumeService.expungeVolumeAsync(srcVolumeInfo);
-
-                    if (destroyFuture.get().isFailed()) {
-                        LOGGER.debug("Failed to clean up source volume on storage");
-                    }
-                } catch (Exception e) {
-                    LOGGER.debug("Failed to clean up source volume on storage", e);
-                }
 
                 // Update the volume ID for snapshots on secondary storage
                 if (!_snapshotDao.listByVolumeId(srcVolumeInfo.getId()).isEmpty()) {

engine/storage/volume/pom.xml

@@ -43,9 +43,6 @@
         <plugins>
             <plugin>
                 <artifactId>maven-surefire-plugin</artifactId>
-                <configuration>
-                    <skipTests>true</skipTests>
-                </configuration>
                 <executions>
                     <execution>
                         <phase>integration-test</phase>

engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java

@@ -49,6 +49,7 @@ import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.storage.DownloadAnswer;
 import com.cloud.agent.api.to.DataObjectType;
 import com.cloud.agent.api.to.DataTO;
+import com.cloud.exception.ConcurrentOperationException;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.offering.DiskOffering.DiskCacheMode;
 import com.cloud.storage.DataStoreRole;
@@ -68,6 +69,12 @@ import com.cloud.utils.storage.encoding.EncodingType;
 import com.cloud.vm.VMInstanceVO;
 import com.cloud.vm.VirtualMachine;
 import com.cloud.vm.dao.VMInstanceDao;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Function;
+import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
 
 public class VolumeObject implements VolumeInfo {
     private static final Logger s_logger = Logger.getLogger(VolumeObject.class);
@@ -100,6 +107,13 @@ public class VolumeObject implements VolumeInfo {
     private boolean directDownload;
     private String vSphereStoragePolicyId;
 
+    private final List<Volume.State> volumeStatesThatShouldNotTransitWhenDataStoreRoleIsImage = Arrays.asList(Volume.State.Migrating, Volume.State.Uploaded, Volume.State.Copying,
+      Volume.State.Expunged);
+
+    private final List<Volume.State> volumeStatesThatShouldNotDeleteEntry = Arrays.asList(Volume.State.UploadError, Volume.State.Uploaded, Volume.State.Copying);
+
+    private final List<DataStoreRole> imageAndImageCacheRoles = Arrays.asList(DataStoreRole.Image, DataStoreRole.ImageCache);
+
     public VolumeObject() {
         _volStateMachine = Volume.State.getStateMachine();
     }
@@ -118,26 +132,21 @@ public class VolumeObject implements VolumeInfo {
     @Override
     public String getAttachedVmName() {
         Long vmId = volumeVO.getInstanceId();
-        if (vmId != null) {
+        VMInstanceVO vm = null;
-            VMInstanceVO vm = vmInstanceDao.findById(vmId);
 
-            if (vm == null) {
+        if (vmId != null) {
-                return null;
+            vm = vmInstanceDao.findById(vmId);
-            }
-            return vm.getInstanceName();
         }
-        return null;
+
+        return vm == null ? null : vm.getInstanceName();
     }
 
     @Override
     public VirtualMachine getAttachedVM() {
         Long vmId = volumeVO.getInstanceId();
-        if (vmId != null) {
+        return vmId == null ? null : vmInstanceDao.findById(vmId);
-            VMInstanceVO vm = vmInstanceDao.findById(vmId);
-            return vm;
-        }
-        return null;
     }
+
     @Override
     public String getUuid() {
         return volumeVO.getUuid();
@@ -210,125 +219,82 @@ public class VolumeObject implements VolumeInfo {
                 volumeVO = volumeDao.findById(volumeVO.getId());
             }
         } catch (NoTransitionException e) {
-            String errorMessage = "Failed to transit volume: " + getVolumeId() + ", due to: " + e.toString();
+            String errorMessage = String.format("Failed to transit volume %s to [%s] due to [%s].", volumeVO.getVolumeDescription(), event, e.getMessage());
-            s_logger.debug(errorMessage);
+            s_logger.warn(errorMessage, e);
-            throw new CloudRuntimeException(errorMessage);
+            throw new CloudRuntimeException(errorMessage, e);
         }
         return result;
     }
 
-    private DiskOfferingVO getDiskOfferingVO() {
+    protected DiskOfferingVO getDiskOfferingVO() {
-        if (getDiskOfferingId() != null) {
+        Long diskOfferingId = getDiskOfferingId();
-            DiskOfferingVO diskOfferingVO = diskOfferingDao.findById(getDiskOfferingId());
+        return diskOfferingId == null ? null : diskOfferingDao.findById(diskOfferingId);
-            return diskOfferingVO;
-        }
-        return null;
     }
 
     @Override
     public Long getBytesReadRate() {
-        DiskOfferingVO diskOfferingVO = getDiskOfferingVO();
+        return getLongValueFromDiskOfferingVoMethod(DiskOfferingVO::getBytesReadRate);
-        if (diskOfferingVO != null) {
-            return diskOfferingVO.getBytesReadRate();
-        }
-        return null;
     }
 
     @Override
     public Long getBytesReadRateMax() {
-        DiskOfferingVO diskOfferingVO = getDiskOfferingVO();
+        return getLongValueFromDiskOfferingVoMethod(DiskOfferingVO::getBytesReadRateMax);
-        if (diskOfferingVO != null) {
-            return diskOfferingVO.getBytesReadRateMax();
-        }
-        return null;
     }
 
     @Override
     public Long getBytesReadRateMaxLength() {
-        DiskOfferingVO diskOfferingVO = getDiskOfferingVO();
+        return getLongValueFromDiskOfferingVoMethod(DiskOfferingVO::getBytesReadRateMaxLength);
-        if (diskOfferingVO != null) {
-            return diskOfferingVO.getBytesReadRateMaxLength();
-        }
-        return null;
     }
 
     @Override
     public Long getBytesWriteRate() {
-        DiskOfferingVO diskOfferingVO = getDiskOfferingVO();
+        return getLongValueFromDiskOfferingVoMethod(DiskOfferingVO::getBytesWriteRate);
-        if (diskOfferingVO != null) {
-            return diskOfferingVO.getBytesWriteRate();
-        }
-        return null;
     }
 
     @Override
     public Long getBytesWriteRateMax() {
-        DiskOfferingVO diskOfferingVO = getDiskOfferingVO();
+        return getLongValueFromDiskOfferingVoMethod(DiskOfferingVO::getBytesWriteRateMax);
-        if (diskOfferingVO != null) {
-            return diskOfferingVO.getBytesWriteRateMax();
-        }
-        return null;
     }
 
     @Override
     public Long getBytesWriteRateMaxLength() {
-        DiskOfferingVO diskOfferingVO = getDiskOfferingVO();
+        return getLongValueFromDiskOfferingVoMethod(DiskOfferingVO::getBytesWriteRateMaxLength);
-        if (diskOfferingVO != null) {
-            return diskOfferingVO.getBytesWriteRateMaxLength();
-        }
-        return null;
     }
 
     @Override
     public Long getIopsReadRate() {
-        DiskOfferingVO diskOfferingVO = getDiskOfferingVO();
+        return getLongValueFromDiskOfferingVoMethod(DiskOfferingVO::getIopsReadRate);
-        if (diskOfferingVO != null) {
-            return diskOfferingVO.getIopsReadRate();
-        }
-        return null;
     }
 
     @Override
     public Long getIopsReadRateMax() {
-        DiskOfferingVO diskOfferingVO = getDiskOfferingVO();
+        return getLongValueFromDiskOfferingVoMethod(DiskOfferingVO::getIopsReadRateMax);
-        if (diskOfferingVO != null) {
-            return diskOfferingVO.getIopsReadRateMax();
-        }
-        return null;
     }
 
     @Override
     public Long getIopsReadRateMaxLength() {
-        DiskOfferingVO diskOfferingVO = getDiskOfferingVO();
+        return getLongValueFromDiskOfferingVoMethod(DiskOfferingVO::getIopsReadRateMaxLength);
-        if (diskOfferingVO != null) {
-            return diskOfferingVO.getIopsReadRateMaxLength();
-        }
-        return null;
     }
 
     @Override
     public Long getIopsWriteRate() {
-        DiskOfferingVO diskOfferingVO = getDiskOfferingVO();
+        return getLongValueFromDiskOfferingVoMethod(DiskOfferingVO::getIopsWriteRate);
-        if (diskOfferingVO != null) {
-            return diskOfferingVO.getIopsWriteRate();
-        }
-        return null;
     }
 
     @Override
     public Long getIopsWriteRateMax() {
-        DiskOfferingVO diskOfferingVO = getDiskOfferingVO();
+        return getLongValueFromDiskOfferingVoMethod(DiskOfferingVO::getIopsWriteRateMax);
-        if (diskOfferingVO != null) {
-            return diskOfferingVO.getIopsWriteRateMax();
-        }
-        return null;
     }
 
     @Override
     public Long getIopsWriteRateMaxLength() {
+        return getLongValueFromDiskOfferingVoMethod(DiskOfferingVO::getIopsWriteRateMaxLength);
+    }
+
+    protected Long getLongValueFromDiskOfferingVoMethod(Function<DiskOfferingVO, Long> method){
         DiskOfferingVO diskOfferingVO = getDiskOfferingVO();
         if (diskOfferingVO != null) {
-            return diskOfferingVO.getIopsWriteRateMaxLength();
+            return method.apply(diskOfferingVO);
         }
         return null;
     }
@@ -336,10 +302,7 @@ public class VolumeObject implements VolumeInfo {
     @Override
     public DiskCacheMode getCacheMode() {
         DiskOfferingVO diskOfferingVO = getDiskOfferingVO();
-        if (diskOfferingVO != null) {
+        return diskOfferingVO == null ? null : diskOfferingVO.getCacheMode();
-            return diskOfferingVO.getCacheMode();
-        }
-        return null;
     }
 
     @Override
@@ -374,7 +337,7 @@ public class VolumeObject implements VolumeInfo {
 
     @Override
     public boolean isAttachedVM() {
-        return (volumeVO.getInstanceId() == null) ? false : true;
+        return volumeVO.getInstanceId() != null;
     }
 
     @Override
@@ -401,64 +364,39 @@ public class VolumeObject implements VolumeInfo {
         if (dataStore == null) {
             return;
         }
-        try {
+
-            Volume.Event volEvent = null;
+        if (imageAndImageCacheRoles.contains(dataStore.getRole())) {
-            if (dataStore.getRole() == DataStoreRole.ImageCache) {
+            updateObjectInDataStoreManager(event, volumeVO != null && !volumeStatesThatShouldNotDeleteEntry.contains(volumeVO.getState()));
-                objectInStoreMgr.update(this, event);
+
+            if (dataStore.getRole() == DataStoreRole.ImageCache || volumeStatesThatShouldNotTransitWhenDataStoreRoleIsImage.contains(volumeVO.getState())
+              || event == ObjectInDataStoreStateMachine.Event.MigrateDataRequested) {
                 return;
             }
-            if (dataStore.getRole() == DataStoreRole.Image) {
-                objectInStoreMgr.update(this, event);
-                if (volumeVO.getState() == Volume.State.Migrating || volumeVO.getState() == Volume.State.Copying ||
-                    volumeVO.getState() == Volume.State.Uploaded || volumeVO.getState() == Volume.State.Expunged) {
-                    return;
-                }
-                if (event == ObjectInDataStoreStateMachine.Event.CreateOnlyRequested) {
-                    volEvent = Volume.Event.UploadRequested;
-                } else if (event == ObjectInDataStoreStateMachine.Event.MigrationRequested) {
-                    volEvent = Volume.Event.CopyRequested;
-                } else if (event == ObjectInDataStoreStateMachine.Event.MigrateDataRequested) {
-                    return;
-                }
-            } else {
-                if (event == ObjectInDataStoreStateMachine.Event.CreateRequested || event == ObjectInDataStoreStateMachine.Event.CreateOnlyRequested) {
-                    volEvent = Volume.Event.CreateRequested;
-                } else if (event == ObjectInDataStoreStateMachine.Event.CopyingRequested) {
-                    volEvent = Volume.Event.CopyRequested;
-                } else if (event == ObjectInDataStoreStateMachine.Event.MigrationRequested) {
-                    volEvent = Volume.Event.MigrationRequested;
-                } else if (event == ObjectInDataStoreStateMachine.Event.MigrationCopyRequested) {
-                    volEvent = Event.MigrationCopyRequested;
-                }
-            }
-
-            if (event == ObjectInDataStoreStateMachine.Event.DestroyRequested) {
-                volEvent = Volume.Event.DestroyRequested;
-            } else if (event == ObjectInDataStoreStateMachine.Event.ExpungeRequested) {
-                volEvent = Volume.Event.ExpungingRequested;
-            } else if (event == ObjectInDataStoreStateMachine.Event.OperationSuccessed) {
-                volEvent = Volume.Event.OperationSucceeded;
-            } else if (event == ObjectInDataStoreStateMachine.Event.MigrationCopySucceeded) {
-              volEvent = Event.MigrationCopySucceeded;
-            } else if (event == ObjectInDataStoreStateMachine.Event.OperationFailed) {
-                volEvent = Volume.Event.OperationFailed;
-            } else if (event == ObjectInDataStoreStateMachine.Event.MigrationCopyFailed) {
-              volEvent = Event.MigrationCopyFailed;
-            } else if (event == ObjectInDataStoreStateMachine.Event.ResizeRequested) {
-                volEvent = Volume.Event.ResizeRequested;
-            }
-            stateTransit(volEvent);
-        } catch (Exception e) {
-            s_logger.debug("Failed to update state", e);
-            throw new CloudRuntimeException("Failed to update state:" + e.toString());
-        } finally {
-            // in case of OperationFailed, expunge the entry
-            // state transit call reloads the volume from DB and so check for null as well
-            if (event == ObjectInDataStoreStateMachine.Event.OperationFailed &&
-                (volumeVO != null && volumeVO.getState() != Volume.State.Copying && volumeVO.getState() != Volume.State.Uploaded && volumeVO.getState() != Volume.State.UploadError)) {
-                objectInStoreMgr.deleteIfNotReady(this);
-            }
         }
+
+        stateTransit(getMapOfEvents().get(event));
+    }
+
+    protected Map<ObjectInDataStoreStateMachine.Event, Volume.Event> getMapOfEvents() {
+        Map<ObjectInDataStoreStateMachine.Event, Volume.Event> mapOfEvents = new HashMap<>();
+        if (dataStore.getRole() == DataStoreRole.Image) {
+            mapOfEvents.put(ObjectInDataStoreStateMachine.Event.CreateOnlyRequested, Volume.Event.UploadRequested);
+            mapOfEvents.put(ObjectInDataStoreStateMachine.Event.MigrationRequested, Volume.Event.CopyRequested);
+        } else {
+            mapOfEvents.put(ObjectInDataStoreStateMachine.Event.CreateRequested, Volume.Event.CreateRequested);
+            mapOfEvents.put(ObjectInDataStoreStateMachine.Event.CreateOnlyRequested, Volume.Event.CreateRequested);
+            mapOfEvents.put(ObjectInDataStoreStateMachine.Event.CopyingRequested, Volume.Event.CopyRequested);
+            mapOfEvents.put(ObjectInDataStoreStateMachine.Event.MigrationRequested, Volume.Event.MigrationRequested);
+            mapOfEvents.put(ObjectInDataStoreStateMachine.Event.MigrationCopyRequested, Volume.Event.MigrationCopyRequested);
+        }
+        mapOfEvents.put(ObjectInDataStoreStateMachine.Event.DestroyRequested, Volume.Event.DestroyRequested);
+        mapOfEvents.put(ObjectInDataStoreStateMachine.Event.ExpungeRequested, Volume.Event.ExpungingRequested);
+        mapOfEvents.put(ObjectInDataStoreStateMachine.Event.OperationSuccessed, Volume.Event.OperationSucceeded);
+        mapOfEvents.put(ObjectInDataStoreStateMachine.Event.MigrationCopySucceeded, Volume.Event.MigrationCopySucceeded);
+        mapOfEvents.put(ObjectInDataStoreStateMachine.Event.OperationFailed, Volume.Event.OperationFailed);
+        mapOfEvents.put(ObjectInDataStoreStateMachine.Event.MigrationCopyFailed, Volume.Event.MigrationCopyFailed);
+        mapOfEvents.put(ObjectInDataStoreStateMachine.Event.ResizeRequested, Volume.Event.ResizeRequested);
+        return mapOfEvents;
     }
 
     @Override
@@ -475,16 +413,29 @@ public class VolumeObject implements VolumeInfo {
 
     @Override
     public void processEventOnly(ObjectInDataStoreStateMachine.Event event) {
+        updateObjectInDataStoreManager(event, true);
+    }
+
+    protected void updateObjectInDataStoreManager(ObjectInDataStoreStateMachine.Event event, boolean callExpungeEntry){
         try {
             objectInStoreMgr.update(this, event);
-        } catch (Exception e) {
+        } catch (ConcurrentOperationException | NoTransitionException e) {
-            s_logger.debug("Failed to update state", e);
+            String message = String.format("Failed to update %sto state [%s] due to [%s].", volumeVO == null ? "" : String.format("volume %s ", volumeVO.getVolumeDescription()),
-            throw new CloudRuntimeException("Failed to update state:" + e.toString());
+              getMapOfEvents().get(event), e.getMessage());
+            s_logger.warn(message, e);
+            throw new CloudRuntimeException(message, e);
         } finally {
-            // in case of OperationFailed, expunge the entry
+            expungeEntryOnOperationFailed(event, callExpungeEntry);
-            if (event == ObjectInDataStoreStateMachine.Event.OperationFailed) {
+        }
-                objectInStoreMgr.deleteIfNotReady(this);
+    }
-            }
+
+    protected void expungeEntryOnOperationFailed(ObjectInDataStoreStateMachine.Event event) {
+        expungeEntryOnOperationFailed(event, true);
+    }
+
+    protected void expungeEntryOnOperationFailed(ObjectInDataStoreStateMachine.Event event, boolean callExpungeEntry) {
+        if (event == ObjectInDataStoreStateMachine.Event.OperationFailed && callExpungeEntry) {
+            objectInStoreMgr.deleteIfNotReady(this);
         }
     }
 
@@ -647,90 +598,150 @@ public class VolumeObject implements VolumeInfo {
 
     @Override
     public void processEvent(ObjectInDataStoreStateMachine.Event event, Answer answer) {
-        try {
+        if (answer != null) {
-            if (dataStore.getRole() == DataStoreRole.Primary) {
+            handleProcessEventAnswer(event, answer);
-                if (answer instanceof CopyCmdAnswer) {
-                    CopyCmdAnswer cpyAnswer = (CopyCmdAnswer)answer;
-                    VolumeVO vol = volumeDao.findById(getId());
-                    VolumeObjectTO newVol = (VolumeObjectTO)cpyAnswer.getNewData();
-                    vol.setPath(newVol.getPath());
-                    if (newVol.getSize() != null) {
-                        // Root disk resize may be requested where the original
-                        // template size is less than the requested root disk size
-                        if (vol.getSize() == null || vol.getSize() < newVol.getSize()) {
-                            vol.setSize(newVol.getSize());
-                        }
-                    }
-                    if (newVol.getFormat() != null) {
-                        vol.setFormat(newVol.getFormat());
-                    }
-                    vol.setPoolId(getDataStore().getId());
-                    volumeDao.update(vol.getId(), vol);
-                } else if (answer instanceof CreateObjectAnswer) {
-                    CreateObjectAnswer createAnswer = (CreateObjectAnswer)answer;
-                    VolumeObjectTO newVol = (VolumeObjectTO)createAnswer.getData();
-                    VolumeVO vol = volumeDao.findById(getId());
-                    vol.setPath(newVol.getPath());
-                    if (newVol.getSize() != null) {
-                        vol.setSize(newVol.getSize());
-                    }
-                    vol.setPoolId(getDataStore().getId());
-                    if (newVol.getFormat() != null) {
-                        vol.setFormat(newVol.getFormat());
-                    }
-                    volumeDao.update(vol.getId(), vol);
-                }
-            } else {
-                // image store or imageCache store
-                if (answer instanceof DownloadAnswer) {
-                    DownloadAnswer dwdAnswer = (DownloadAnswer)answer;
-                    VolumeDataStoreVO volStore = volumeStoreDao.findByStoreVolume(dataStore.getId(), getId());
-                    volStore.setInstallPath(dwdAnswer.getInstallPath());
-                    volStore.setChecksum(dwdAnswer.getCheckSum());
-                    volumeStoreDao.update(volStore.getId(), volStore);
-                } else if (answer instanceof CopyCmdAnswer) {
-                    CopyCmdAnswer cpyAnswer = (CopyCmdAnswer)answer;
-                    VolumeDataStoreVO volStore = volumeStoreDao.findByStoreVolume(dataStore.getId(), getId());
-                    VolumeObjectTO newVol = (VolumeObjectTO)cpyAnswer.getNewData();
-                    volStore.setInstallPath(newVol.getPath());
-                    if (newVol.getSize() != null) {
-                        volStore.setSize(newVol.getSize());
-                    }
-                    volumeStoreDao.update(volStore.getId(), volStore);
-                }
-            }
-        } catch (RuntimeException ex) {
-            if (event == ObjectInDataStoreStateMachine.Event.OperationFailed) {
-                objectInStoreMgr.deleteIfNotReady(this);
-            }
-            throw ex;
         }
-        this.processEvent(event);
 
+        this.processEvent(event);
     }
 
-    @Override
+    protected void handleProcessEventAnswer(ObjectInDataStoreStateMachine.Event event, Answer answer) throws RuntimeException {
-    public void incRefCount() {
+        try {
-        if (dataStore == null) {
+            if (answer instanceof CopyCmdAnswer) {
+                handleProcessEventAnswer((CopyCmdAnswer)answer);
+            } else if (answer instanceof CreateObjectAnswer) {
+                handleProcessEventAnswer((CreateObjectAnswer)answer);
+            } else if (answer instanceof DownloadAnswer) {
+                handleProcessEventAnswer((DownloadAnswer) answer);
+            }
+        } catch (RuntimeException ex) {
+            expungeEntryOnOperationFailed(event);
+            throw ex;
+        }
+    }
+
+    protected boolean isPrimaryDataStore(){
+        return dataStore.getRole() == DataStoreRole.Primary;
+    }
+
+    protected void setVolumeFormat(VolumeObjectTO newVolume, boolean setFormat, VolumeVO volumeVo) {
+        if (newVolume.getFormat() != null && setFormat) {
+            volumeVo.setFormat(newVolume.getFormat());
+        }
+    }
+
+    protected void handleProcessEventAnswer(CopyCmdAnswer copyAnswer) {
+        handleProcessEventAnswer(copyAnswer, true, true);
+    }
+
+    protected void handleProcessEventAnswer(CopyCmdAnswer copyAnswer, boolean validateVolumeSize, boolean setFormat) {
+        VolumeObjectTO newVolume = (VolumeObjectTO)copyAnswer.getNewData();
+
+        if (this.isPrimaryDataStore()) {
+            handleProcessEventCopyCmdAnswerPrimaryStore(newVolume, validateVolumeSize, setFormat);
+        } else {
+            handleProcessEventCopyCmdAnswerNotPrimaryStore(newVolume);
+        }
+    }
+
+    protected void handleProcessEventCopyCmdAnswerPrimaryStore(VolumeObjectTO newVolume, boolean validateVolumeSize, boolean setFormat) {
+        VolumeVO volumeVo = volumeDao.findById(getId());
+        updateVolumeInfo(newVolume, volumeVo, (!validateVolumeSize || newVolume.getSize() == null || volumeVo.getSize() == null || volumeVo.getSize() < newVolume.getSize()),
+          setFormat);
+    }
+
+    protected void updateVolumeInfo(VolumeObjectTO newVolume, VolumeVO volumeVo, boolean setVolumeSize, boolean setFormat) {
+        String previousValues = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(volumeVo, "path", "size", "format", "poolId");
+
+        volumeVo.setPath(newVolume.getPath());
+        Long newVolumeSize = newVolume.getSize();
+
+        if (newVolumeSize != null && setVolumeSize) {
+            volumeVo.setSize(newVolumeSize);
+        }
+
+        setVolumeFormat(newVolume, setFormat, volumeVo);
+
+        volumeVo.setPoolId(getDataStore().getId());
+        volumeDao.update(volumeVo.getId(), volumeVo);
+
+        String newValues = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(volumeVo, "path", "size", "format", "poolId");
+        s_logger.debug(String.format("Updated %s from %s to %s ", volumeVo.getVolumeDescription(), previousValues, newValues));
+    }
+
+   protected void handleProcessEventCopyCmdAnswerNotPrimaryStore(VolumeObjectTO newVolume) {
+        VolumeDataStoreVO volStore = volumeStoreDao.findByStoreVolume(dataStore.getId(), getId());
+
+        String previousValues = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(volStore, "installPath", "size");
+
+        volStore.setInstallPath(newVolume.getPath());
+        Long newVolumeSize = newVolume.getSize();
+
+        if (newVolumeSize != null) {
+            volStore.setSize(newVolumeSize);
+        }
+
+        volumeStoreDao.update(volStore.getId(), volStore);
+
+        String newValues = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(volStore, "installPath", "size");
+        s_logger.debug(String.format("Updated volume_store_ref %s from %s to %s.", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(volStore, "id", "volumeId"),
+          previousValues, newValues));
+    }
+
+    protected void handleProcessEventAnswer(CreateObjectAnswer createObjectAnswer) {
+        handleProcessEventAnswer(createObjectAnswer, true);
+    }
+
+    protected void handleProcessEventAnswer(CreateObjectAnswer createObjectAnswer, boolean setFormat) {
+        if (!isPrimaryDataStore()) {
             return;
         }
 
-        if (dataStore.getRole() == DataStoreRole.Image || dataStore.getRole() == DataStoreRole.ImageCache) {
+        VolumeObjectTO newVolume = (VolumeObjectTO)createObjectAnswer.getData();
-            VolumeDataStoreVO store = volumeStoreDao.findByStoreVolume(dataStore.getId(), getId());
+        VolumeVO volumeVo = volumeDao.findById(getId());
-            store.incrRefCnt();
+        updateVolumeInfo(newVolume, volumeVo, true, setFormat);
-            store.setLastUpdated(new Date());
+    }
-            volumeStoreDao.update(store.getId(), store);
+
+    protected void handleProcessEventAnswer(DownloadAnswer downloadAnswer) {
+        if (isPrimaryDataStore()) {
+            return;
         }
+
+        VolumeDataStoreVO volumeDataStoreVo = volumeStoreDao.findByStoreVolume(dataStore.getId(), getId());
+        String previousValues = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(volumeDataStoreVo, "installPath", "checksum");
+
+        volumeDataStoreVo.setInstallPath(downloadAnswer.getInstallPath());
+        volumeDataStoreVo.setChecksum(downloadAnswer.getCheckSum());
+        volumeStoreDao.update(volumeDataStoreVo.getId(), volumeDataStoreVo);
+
+        String newValues = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(volumeDataStoreVo, "installPath", "checksum");
+        s_logger.debug(String.format("Updated volume_store_ref %s from %s to %s.", ReflectionToStringBuilderUtils.
+          reflectOnlySelectedFields(volumeDataStoreVo, "id", "volumeId"), previousValues, newValues));
+    }
+    @Override
+    public void incRefCount() {
+        updateRefCount(true);
     }
 
     @Override
     public void decRefCount() {
+        updateRefCount(false);
+    }
+
+    protected void updateRefCount(boolean increase){
         if (dataStore == null) {
             return;
         }
-        if (dataStore.getRole() == DataStoreRole.Image || dataStore.getRole() == DataStoreRole.ImageCache) {
+
+        if (imageAndImageCacheRoles.contains(dataStore.getRole())) {
             VolumeDataStoreVO store = volumeStoreDao.findByStoreVolume(dataStore.getId(), getId());
-            store.decrRefCnt();
+
+            if (increase) {
+                store.incrRefCnt();
+            } else {
+                store.decrRefCnt();
+            }
+
             store.setLastUpdated(new Date());
             volumeStoreDao.update(store.getId(), store);
         }
@@ -741,7 +752,8 @@ public class VolumeObject implements VolumeInfo {
         if (dataStore == null) {
             return null;
         }
-        if (dataStore.getRole() == DataStoreRole.Image || dataStore.getRole() == DataStoreRole.ImageCache) {
+
+        if (imageAndImageCacheRoles.contains(dataStore.getRole())) {
             VolumeDataStoreVO store = volumeStoreDao.findByStoreVolume(dataStore.getId(), getId());
             return store.getRefCnt();
         }
@@ -751,55 +763,19 @@ public class VolumeObject implements VolumeInfo {
     @Override
     public void processEventOnly(ObjectInDataStoreStateMachine.Event event, Answer answer) {
         try {
-            if (dataStore.getRole() == DataStoreRole.Primary) {
+            if (answer instanceof CopyCmdAnswer){
-                if (answer instanceof CopyCmdAnswer) {
+                handleProcessEventAnswer((CopyCmdAnswer) answer, false, false);
-                    CopyCmdAnswer cpyAnswer = (CopyCmdAnswer)answer;
+            } else if (answer instanceof CreateObjectAnswer) {
-                    VolumeVO vol = volumeDao.findById(getId());
+                handleProcessEventAnswer((CreateObjectAnswer) answer, false);
-                    VolumeObjectTO newVol = (VolumeObjectTO)cpyAnswer.getNewData();
+            } else if (answer instanceof DownloadAnswer) {
-                    vol.setPath(newVol.getPath());
+                handleProcessEventAnswer((DownloadAnswer) answer);
-                    if (newVol.getSize() != null) {
-                        vol.setSize(newVol.getSize());
-                    }
-                    vol.setPoolId(getDataStore().getId());
-                    volumeDao.update(vol.getId(), vol);
-                } else if (answer instanceof CreateObjectAnswer) {
-                    CreateObjectAnswer createAnswer = (CreateObjectAnswer)answer;
-                    VolumeObjectTO newVol = (VolumeObjectTO)createAnswer.getData();
-                    VolumeVO vol = volumeDao.findById(getId());
-                    vol.setPath(newVol.getPath());
-                    if (newVol.getSize() != null) {
-                        vol.setSize(newVol.getSize());
-                    }
-                    vol.setPoolId(getDataStore().getId());
-                    volumeDao.update(vol.getId(), vol);
-                }
-            } else {
-                // image store or imageCache store
-                if (answer instanceof DownloadAnswer) {
-                    DownloadAnswer dwdAnswer = (DownloadAnswer)answer;
-                    VolumeDataStoreVO volStore = volumeStoreDao.findByStoreVolume(dataStore.getId(), getId());
-                    volStore.setInstallPath(dwdAnswer.getInstallPath());
-                    volStore.setChecksum(dwdAnswer.getCheckSum());
-                    volumeStoreDao.update(volStore.getId(), volStore);
-                } else if (answer instanceof CopyCmdAnswer) {
-                    CopyCmdAnswer cpyAnswer = (CopyCmdAnswer)answer;
-                    VolumeDataStoreVO volStore = volumeStoreDao.findByStoreVolume(dataStore.getId(), getId());
-                    VolumeObjectTO newVol = (VolumeObjectTO)cpyAnswer.getNewData();
-                    volStore.setInstallPath(newVol.getPath());
-                    if (newVol.getSize() != null) {
-                        volStore.setSize(newVol.getSize());
-                    }
-                    volumeStoreDao.update(volStore.getId(), volStore);
-                }
             }
         } catch (RuntimeException ex) {
-            if (event == ObjectInDataStoreStateMachine.Event.OperationFailed) {
+            expungeEntryOnOperationFailed(event);
-                objectInStoreMgr.deleteIfNotReady(this);
-            }
             throw ex;
         }
-        this.processEventOnly(event);
 
+        this.processEventOnly(event);
     }
 
     public String getvSphereStoragePolicyId() {
@@ -836,10 +812,7 @@ public class VolumeObject implements VolumeInfo {
 
     @Override
     public boolean delete() {
-        if (dataStore != null) {
+        return dataStore == null ? true : dataStore.delete(this);
-            return dataStore.delete(this);
-        }
-        return true;
     }
 
     @Override

engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java

@@ -135,6 +135,7 @@ import com.cloud.vm.VirtualMachine;
 import com.google.common.base.Strings;
 
 import static com.cloud.storage.resource.StorageProcessor.REQUEST_TEMPLATE_RELOAD;
+import java.util.concurrent.ExecutionException;
 
 @Component
 public class VolumeServiceImpl implements VolumeService {
@@ -1838,30 +1839,8 @@ public class VolumeServiceImpl implements VolumeService {
                 destroyFuture.get();
                 future.complete(res);
             } else {
-                srcVolume.processEvent(Event.OperationSuccessed);
+                if (copyPoliciesBetweenVolumesAndDestroySourceVolumeAfterMigration(Event.MigrationCopySucceeded, result.getAnswer(), srcVolume, destVolume, true)) {
-                destVolume.processEvent(Event.MigrationCopySucceeded, result.getAnswer());
-                volDao.updateUuid(srcVolume.getId(), destVolume.getId());
-                try {
-                    destroyVolume(srcVolume.getId());
-                    if (srcVolume.getStoragePoolType() == StoragePoolType.PowerFlex) {
-                        s_logger.info("Src volume " + srcVolume.getId() + " can be removed");
-                        srcVolume.processEvent(Event.ExpungeRequested);
-                        srcVolume.processEvent(Event.OperationSuccessed);
-                        volDao.remove(srcVolume.getId());
-                        future.complete(res);
-                        return null;
-                    }
-                    srcVolume = volFactory.getVolume(srcVolume.getId());
-                    AsyncCallFuture<VolumeApiResult> destroyFuture = expungeVolumeAsync(srcVolume);
-                    // If volume destroy fails, this could be because of vdi is still in use state, so wait and retry.
-                    if (destroyFuture.get().isFailed()) {
-                        Thread.sleep(5 * 1000);
-                        destroyFuture = expungeVolumeAsync(srcVolume);
-                        destroyFuture.get();
-                    }
                     future.complete(res);
-                } catch (Exception e) {
-                    s_logger.debug("failed to clean up volume on storage", e);
                 }
             }
         } catch (Exception e) {
@@ -1873,6 +1852,70 @@ public class VolumeServiceImpl implements VolumeService {
         return null;
     }
 
+    @Override
+    public boolean copyPoliciesBetweenVolumesAndDestroySourceVolumeAfterMigration(Event destinationEvent, Answer destinationEventAnswer, VolumeInfo sourceVolume,
+      VolumeInfo destinationVolume, boolean retryExpungeVolumeAsync) {
+        VolumeVO sourceVolumeVo = ((VolumeObject) sourceVolume).getVolume();
+        snapshotMgr.copySnapshotPoliciesBetweenVolumes(sourceVolumeVo, ((VolumeObject) destinationVolume).getVolume());
+        return destroySourceVolumeAfterMigration(destinationEvent, destinationEventAnswer, sourceVolume, destinationVolume, retryExpungeVolumeAsync);
+    }
+
+    protected boolean destroySourceVolumeAfterMigration(Event destinationEvent, Answer destinationEventAnswer, VolumeInfo sourceVolume,
+      VolumeInfo destinationVolume, boolean retryExpungeVolumeAsync) {
+        sourceVolume.processEvent(Event.OperationSuccessed);
+        destinationVolume.processEvent(destinationEvent, destinationEventAnswer);
+
+        VolumeVO sourceVolumeVo = ((VolumeObject) sourceVolume).getVolume();
+
+        long sourceVolumeId = sourceVolume.getId();
+        volDao.updateUuid(sourceVolumeId, destinationVolume.getId());
+
+        s_logger.info(String.format("Cleaning up %s on storage [%s].", sourceVolumeVo.getVolumeDescription(), sourceVolumeVo.getPoolId()));
+        destroyVolume(sourceVolumeId);
+
+        try {
+            if (sourceVolume.getStoragePoolType() == StoragePoolType.PowerFlex) {
+                s_logger.info(String.format("Source volume %s can be removed.", sourceVolumeVo.getVolumeDescription()));
+                sourceVolume.processEvent(Event.ExpungeRequested);
+                sourceVolume.processEvent(Event.OperationSuccessed);
+                volDao.remove(sourceVolume.getId());
+                return true;
+            }
+            expungeSourceVolumeAfterMigration(sourceVolumeVo, retryExpungeVolumeAsync);
+            return true;
+        } catch (InterruptedException | ExecutionException e) {
+            s_logger.error(String.format("Failed to clean up %s on storage [%s].", sourceVolumeVo.getVolumeDescription(), sourceVolumeVo.getPoolId()), e);
+            return false;
+        }
+    }
+
+    protected void expungeSourceVolumeAfterMigration(VolumeVO sourceVolumeVo, boolean retryExpungeVolumeAsync) throws
+      ExecutionException, InterruptedException {
+        VolumeInfo sourceVolume = volFactory.getVolume(sourceVolumeVo.getId());
+
+        AsyncCallFuture<VolumeApiResult> destroyFuture = expungeVolumeAsync(sourceVolume);
+        VolumeApiResult volumeApiResult = destroyFuture.get();
+
+        if (volumeApiResult.isSuccess()) {
+            s_logger.debug(String.format("%s on storage [%s] was cleaned up successfully.", sourceVolumeVo.getVolumeDescription(), sourceVolumeVo.getPoolId()));
+            return;
+        }
+
+        String message = String.format("Failed to clean up %s on storage [%s] due to [%s].", sourceVolumeVo.getVolumeDescription(), sourceVolumeVo.getPoolId(),
+          volumeApiResult.getResult());
+
+        if (!retryExpungeVolumeAsync) {
+            s_logger.warn(message);
+        } else {
+            int intervalBetweenExpungeVolumeAsyncTriesInSeconds = 5;
+            s_logger.info(String.format("%s Trying again in [%s] seconds.", message, intervalBetweenExpungeVolumeAsyncTriesInSeconds));
+
+            Thread.sleep(intervalBetweenExpungeVolumeAsyncTriesInSeconds * 1000);
+            destroyFuture = expungeVolumeAsync(sourceVolume);
+            destroyFuture.get();
+        }
+    }
+
     private class CopyManagedVolumeContext<T> extends AsyncRpcContext<T> {
         final VolumeInfo srcVolume;
         final VolumeInfo destVolume;

engine/storage/volume/src/test/java/org/apache/cloudstack/storage/volume/VolumeObjectTest.java

@@ -19,59 +19,558 @@
 
 package org.apache.cloudstack.storage.volume;
 
+import com.cloud.agent.api.storage.DownloadAnswer;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.storage.DataStoreRole;
+import com.cloud.storage.DiskOfferingVO;
+import com.cloud.storage.Storage;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.fsm.NoTransitionException;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import junit.framework.TestCase;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
+import org.apache.cloudstack.storage.command.CopyCmdAnswer;
+import org.apache.cloudstack.storage.command.CreateObjectAnswer;
 import org.apache.cloudstack.storage.datastore.ObjectInDataStoreManager;
 import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.Mockito;
-import org.mockito.runners.MockitoJUnitRunner;
+import org.mockito.Spy;
-
+import org.mockito.junit.MockitoJUnitRunner;
-import com.cloud.storage.Storage;
-import com.cloud.storage.Volume;
-import com.cloud.storage.VolumeVO;
-import com.cloud.storage.dao.DiskOfferingDao;
-import com.cloud.storage.dao.VolumeDao;
-import com.cloud.vm.dao.VMInstanceDao;
 
 @RunWith(MockitoJUnitRunner.class)
-public class VolumeObjectTest {
+public class VolumeObjectTest extends TestCase{
+
+    @Spy
+    VolumeObject volumeObjectSpy;
 
     @Mock
-    VolumeDao volumeDao;
+    DataStore dataStoreMock;
 
     @Mock
-    VolumeDataStoreDao volumeStoreDao;
+    VolumeVO volumeVoMock;
 
     @Mock
-    ObjectInDataStoreManager objectInStoreMgr;
+    VolumeDataStoreDao volumeDataStoreDaoMock;
 
     @Mock
-    VMInstanceDao vmInstanceDao;
+    VolumeDataStoreVO volumeDataStoreVoMock;
 
     @Mock
-    DiskOfferingDao diskOfferingDao;
+    VolumeObjectTO volumeObjectToMock;
 
-    @InjectMocks
+    @Mock
-    VolumeObject volumeObject;
+    VolumeDao volumeDaoMock;
+
+    @Mock
+    ObjectInDataStoreManager objectInDataStoreManagerMock;
+
+    Set<Function<DiskOfferingVO, Long>> diskOfferingVoMethodsWithLongReturn = new HashSet<>();
+
+    List<ObjectInDataStoreStateMachine.Event> objectInDataStoreStateMachineEvents = Arrays.asList(ObjectInDataStoreStateMachine.Event.values());
+
+    List<DataStoreRole> dataStoreRolesExceptImageAndImageCache = new LinkedList<>(Arrays.asList(DataStoreRole.values()));
 
     @Before
-    public void setUp() throws Exception {
+    public void setup(){
-        volumeObject.configure(Mockito.mock(DataStore.class), new VolumeVO("name", 1l, 1l, 1l, 1l, 1l, "folder", "path", Storage.ProvisioningType.THIN, 1l, Volume.Type.DATADISK));
+        volumeObjectSpy.configure(dataStoreMock, volumeVoMock);
+        volumeObjectSpy.volumeStoreDao = volumeDataStoreDaoMock;
+        volumeObjectSpy.volumeDao = volumeDaoMock;
+        volumeObjectSpy.objectInStoreMgr = objectInDataStoreManagerMock;
+
+        diskOfferingVoMethodsWithLongReturn.add(DiskOfferingVO::getBytesReadRate);
+        diskOfferingVoMethodsWithLongReturn.add(DiskOfferingVO::getBytesReadRateMax);
+        diskOfferingVoMethodsWithLongReturn.add(DiskOfferingVO::getBytesReadRateMaxLength);
+        diskOfferingVoMethodsWithLongReturn.add(DiskOfferingVO::getBytesWriteRate);
+        diskOfferingVoMethodsWithLongReturn.add(DiskOfferingVO::getBytesWriteRateMax);
+        diskOfferingVoMethodsWithLongReturn.add(DiskOfferingVO::getBytesWriteRateMaxLength);
+        diskOfferingVoMethodsWithLongReturn.add(DiskOfferingVO::getIopsReadRate);
+        diskOfferingVoMethodsWithLongReturn.add(DiskOfferingVO::getIopsReadRateMax);
+        diskOfferingVoMethodsWithLongReturn.add(DiskOfferingVO::getIopsReadRateMaxLength);
+        diskOfferingVoMethodsWithLongReturn.add(DiskOfferingVO::getIopsWriteRate);
+        diskOfferingVoMethodsWithLongReturn.add(DiskOfferingVO::getIopsWriteRateMax);
+        diskOfferingVoMethodsWithLongReturn.add(DiskOfferingVO::getIopsWriteRateMaxLength);
+
+        dataStoreRolesExceptImageAndImageCache.remove(DataStoreRole.Image);
+        dataStoreRolesExceptImageAndImageCache.remove(DataStoreRole.ImageCache);
     }
 
-    /**
-     * Tests the following scenario:
-     * If the volume gets deleted by another thread (cleanup) and the cleanup is attempted again, the volume isnt found in DB and hence NPE occurs
-     * during transition
-     */
     @Test
-    public void testStateTransit() {
+    public void validateGetLongValueFromDiskOfferingVoMethodNullDiskOfferingMustReturnNull(){
-        boolean result = volumeObject.stateTransit(Volume.Event.OperationFailed);
+        Mockito.doReturn(null).when(volumeObjectSpy).getDiskOfferingVO();
-        Assert.assertFalse("since the volume doesnt exist in the db, the operation should fail but, should not throw any exception", result);
+
+        diskOfferingVoMethodsWithLongReturn.forEach(method -> Assert.assertNull(volumeObjectSpy.getLongValueFromDiskOfferingVoMethod(method)));
     }
-}
+
+    @Test
+    public void validateGetLongValueFromDiskOfferingVoMethodNotNullNullDiskOfferingMustReturnValues(){
+        DiskOfferingVO diskOfferingVO = new DiskOfferingVO();
+        Mockito.doReturn(diskOfferingVO).when(volumeObjectSpy).getDiskOfferingVO();
+
+        diskOfferingVO.setBytesReadRate(1l);
+        diskOfferingVO.setBytesReadRateMax(2l);
+        diskOfferingVO.setBytesReadRateMaxLength(3l);
+        diskOfferingVO.setBytesWriteRate(4l);
+        diskOfferingVO.setBytesWriteRateMax(5l);
+        diskOfferingVO.setBytesWriteRateMaxLength(6l);
+        diskOfferingVO.setIopsReadRate(7l);
+        diskOfferingVO.setIopsReadRateMax(8l);
+        diskOfferingVO.setIopsReadRateMaxLength(9l);
+        diskOfferingVO.setIopsWriteRate(10l);
+        diskOfferingVO.setIopsWriteRateMax(11l);
+        diskOfferingVO.setIopsWriteRateMaxLength(12l);
+
+        diskOfferingVoMethodsWithLongReturn.forEach(method -> Assert.assertEquals(method.apply(diskOfferingVO), volumeObjectSpy.getLongValueFromDiskOfferingVoMethod(method)));
+    }
+
+    @Test
+    public void validateGetMapOfEventsDataStoreIsImage(){
+        Map<ObjectInDataStoreStateMachine.Event, Volume.Event> expectedResult = new HashMap<>();
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.CreateOnlyRequested, Volume.Event.UploadRequested);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.MigrationRequested, Volume.Event.CopyRequested);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.DestroyRequested, Volume.Event.DestroyRequested);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.ExpungeRequested, Volume.Event.ExpungingRequested);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.OperationSuccessed, Volume.Event.OperationSucceeded);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.MigrationCopySucceeded, Volume.Event.MigrationCopySucceeded);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.OperationFailed, Volume.Event.OperationFailed);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.MigrationCopyFailed, Volume.Event.MigrationCopyFailed);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.ResizeRequested, Volume.Event.ResizeRequested);
+
+        Mockito.doReturn(DataStoreRole.Image).when(dataStoreMock).getRole();
+
+        Assert.assertEquals(expectedResult, volumeObjectSpy.getMapOfEvents());
+    }
+
+    @Test
+    public void validateGetMapOfEventsDataStoreIsNotImage(){
+        Map<ObjectInDataStoreStateMachine.Event, Volume.Event> expectedResult = new HashMap<>();
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.CreateRequested, Volume.Event.CreateRequested);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.CreateOnlyRequested, Volume.Event.CreateRequested);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.CopyingRequested, Volume.Event.CopyRequested);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.MigrationRequested, Volume.Event.MigrationRequested);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.MigrationCopyRequested, Volume.Event.MigrationCopyRequested);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.DestroyRequested, Volume.Event.DestroyRequested);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.ExpungeRequested, Volume.Event.ExpungingRequested);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.OperationSuccessed, Volume.Event.OperationSucceeded);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.MigrationCopySucceeded, Volume.Event.MigrationCopySucceeded);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.OperationFailed, Volume.Event.OperationFailed);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.MigrationCopyFailed, Volume.Event.MigrationCopyFailed);
+        expectedResult.put(ObjectInDataStoreStateMachine.Event.ResizeRequested, Volume.Event.ResizeRequested);
+
+        List<DataStoreRole> roles = new LinkedList<>(Arrays.asList(DataStoreRole.values()));
+        roles.remove(DataStoreRole.Image);
+
+        roles.forEach(role -> {
+            Mockito.doReturn(role).when(dataStoreMock).getRole();
+            Assert.assertEquals(expectedResult, volumeObjectSpy.getMapOfEvents());
+        });
+    }
+
+    @Test
+    public void validateUpdateObjectInDataStoreManagerConcurrentOperationExceptionThrowsCloudRuntimeException() throws NoTransitionException{
+        Mockito.doThrow(new ConcurrentOperationException("")).when(objectInDataStoreManagerMock).update(Mockito.any(), Mockito.any());
+        Mockito.doNothing().when(volumeObjectSpy).expungeEntryOnOperationFailed(Mockito.any(), Mockito.anyBoolean());
+
+        objectInDataStoreStateMachineEvents.forEach(event -> {
+            boolean threwException = false;
+
+            try {
+                volumeObjectSpy.updateObjectInDataStoreManager(event, true);
+            } catch (CloudRuntimeException e) {
+                threwException = true;
+            }
+
+            Assert.assertTrue(threwException);
+        });
+
+        Mockito.verify(volumeObjectSpy, Mockito.times(objectInDataStoreStateMachineEvents.size())).expungeEntryOnOperationFailed(Mockito.any(), Mockito.anyBoolean());
+    }
+
+    @Test
+    public void validateUpdateObjectInDataStoreManagerNoTransitionExceptionThrowsCloudRuntimeException() throws NoTransitionException{
+        Mockito.doThrow(new NoTransitionException("")).when(objectInDataStoreManagerMock).update(Mockito.any(), Mockito.any());
+        Mockito.doNothing().when(volumeObjectSpy).expungeEntryOnOperationFailed(Mockito.any(), Mockito.anyBoolean());
+
+        objectInDataStoreStateMachineEvents.forEach(event -> {
+            boolean threwException = false;
+
+            try {
+                volumeObjectSpy.updateObjectInDataStoreManager(event, true);
+            } catch (CloudRuntimeException e) {
+                threwException = true;
+            }
+
+            Assert.assertTrue(threwException);
+        });
+
+        Mockito.verify(volumeObjectSpy, Mockito.times(objectInDataStoreStateMachineEvents.size())).expungeEntryOnOperationFailed(Mockito.any(), Mockito.anyBoolean());
+    }
+
+    @Test
+    public void validateUpdateObjectInDataStoreManagerThrowsAnyOtherExceptionDoNotCatch() throws NoTransitionException{
+        Mockito.doThrow(new RuntimeException("")).when(objectInDataStoreManagerMock).update(Mockito.any(), Mockito.any());
+        Mockito.doNothing().when(volumeObjectSpy).expungeEntryOnOperationFailed(Mockito.any(), Mockito.anyBoolean());
+
+        objectInDataStoreStateMachineEvents.forEach(event -> {
+            boolean threwCloudRuntimeException = false;
+
+            try {
+                volumeObjectSpy.updateObjectInDataStoreManager(event, true);
+            } catch (CloudRuntimeException e) {
+                threwCloudRuntimeException = true;
+            } catch (RuntimeException e) {
+            }
+
+            Assert.assertFalse(threwCloudRuntimeException);
+        });
+
+        Mockito.verify(volumeObjectSpy, Mockito.times(objectInDataStoreStateMachineEvents.size())).expungeEntryOnOperationFailed(Mockito.any(), Mockito.anyBoolean());
+    }
+
+    @Test
+    public void validateUpdateObjectInDataStoreManagerUpdateSuccessfully() throws NoTransitionException{
+        Mockito.doReturn(true).when(objectInDataStoreManagerMock).update(Mockito.any(), Mockito.any());
+        Mockito.doNothing().when(volumeObjectSpy).expungeEntryOnOperationFailed(Mockito.any(), Mockito.anyBoolean());
+
+        objectInDataStoreStateMachineEvents.forEach(event -> {
+            boolean threwCloudRuntimeException = false;
+
+            try {
+                volumeObjectSpy.updateObjectInDataStoreManager(event, true);
+            } catch (RuntimeException e) {
+                threwCloudRuntimeException = true;
+            }
+
+            Assert.assertFalse(threwCloudRuntimeException);
+        });
+
+        Mockito.verify(volumeObjectSpy, Mockito.times(objectInDataStoreStateMachineEvents.size())).expungeEntryOnOperationFailed(Mockito.any(), Mockito.anyBoolean());
+    }
+
+    @Test
+    public void validateExpungeEntryOnOperationFailedCallExpungeEntryFalse() {
+        objectInDataStoreStateMachineEvents.forEach(event -> {
+            volumeObjectSpy.expungeEntryOnOperationFailed(event, false);
+        });
+
+        Mockito.verify(objectInDataStoreManagerMock, Mockito.never()).deleteIfNotReady(Mockito.any());
+    }
+
+    @Test
+    public void validateExpungeEntryOnOperationFailedCallExpungeEntryTrue() {
+        Mockito.doReturn(true).when(objectInDataStoreManagerMock).deleteIfNotReady(Mockito.any());
+
+        objectInDataStoreStateMachineEvents.forEach(event -> {
+            volumeObjectSpy.expungeEntryOnOperationFailed(event, true);
+        });
+
+        Mockito.verify(objectInDataStoreManagerMock, Mockito.times(1)).deleteIfNotReady(Mockito.any());
+    }
+
+    @Test
+    public void validateExpungeEntryOnOperationFailed() {
+        Mockito.doNothing().when(volumeObjectSpy).expungeEntryOnOperationFailed(Mockito.any(), Mockito.anyBoolean());
+
+        objectInDataStoreStateMachineEvents.forEach(event -> {
+            volumeObjectSpy.expungeEntryOnOperationFailed(event);
+            Mockito.verify(volumeObjectSpy, Mockito.times(1)).expungeEntryOnOperationFailed(event, true);
+        });
+
+    }
+
+    @Test
+    public void validateUpdateRefCountDataStoreNullReturn(){
+        volumeObjectSpy.dataStore = null;
+
+        volumeObjectSpy.updateRefCount(true);
+        volumeObjectSpy.updateRefCount(false);
+
+        Mockito.verify(volumeDataStoreDaoMock, Mockito.never()).findByStoreVolume(Mockito.anyLong(), Mockito.anyLong());
+    }
+
+    @Test
+    public void validateUpdateRefCountDataStoreIsNotImage(){
+        dataStoreRolesExceptImageAndImageCache.forEach(role -> {
+            Mockito.doReturn(role).when(dataStoreMock).getRole();
+            volumeObjectSpy.updateRefCount(true);
+            volumeObjectSpy.updateRefCount(false);
+        });
+
+        Mockito.verify(volumeDataStoreDaoMock, Mockito.never()).findByStoreVolume(Mockito.anyLong(), Mockito.anyLong());
+    }
+
+    @Test
+    public void validateUpdateRefCountDataStoreIsImagerOrImageCacheIncreasingCount(){
+        Mockito.doReturn(volumeDataStoreVoMock).when(volumeDataStoreDaoMock).findByStoreVolume(Mockito.anyLong(), Mockito.anyLong());
+        Mockito.doNothing().when(volumeDataStoreVoMock).incrRefCnt();
+        Mockito.doReturn(true).when(volumeDataStoreDaoMock).update(Mockito.anyLong(), Mockito.any());
+
+        Arrays.asList(DataStoreRole.Image, DataStoreRole.ImageCache).forEach(role -> {
+            Mockito.doReturn(role).when(dataStoreMock).getRole();
+            volumeObjectSpy.updateRefCount(true);
+        });
+
+        Mockito.verify(volumeDataStoreDaoMock, Mockito.times(2)).findByStoreVolume(Mockito.anyLong(), Mockito.anyLong());
+    }
+
+    @Test
+    public void validateUpdateRefCountDataStoreIsImagerOrImageCacheDecreasingCount(){
+        Mockito.doReturn(volumeDataStoreVoMock).when(volumeDataStoreDaoMock).findByStoreVolume(Mockito.anyLong(), Mockito.anyLong());
+        Mockito.doNothing().when(volumeDataStoreVoMock).decrRefCnt();
+        Mockito.doReturn(true).when(volumeDataStoreDaoMock).update(Mockito.anyLong(), Mockito.any());
+
+        Arrays.asList(DataStoreRole.Image, DataStoreRole.ImageCache).forEach(role -> {
+            Mockito.doReturn(role).when(dataStoreMock).getRole();
+            volumeObjectSpy.updateRefCount(false);
+        });
+
+        Mockito.verify(volumeDataStoreDaoMock, Mockito.times(2)).findByStoreVolume(Mockito.anyLong(), Mockito.anyLong());
+    }
+
+    @Test
+    public void validateIsPrimaryDataStore(){
+        List<DataStoreRole> dataStoreRoles = Arrays.asList(DataStoreRole.values());
+        dataStoreRoles.forEach(dataStoreRole -> {
+            boolean expectedResult = dataStoreRole == DataStoreRole.Primary;
+            Mockito.doReturn(dataStoreRole).when(dataStoreMock).getRole();
+
+            boolean result = volumeObjectSpy.isPrimaryDataStore();
+            Assert.assertEquals(expectedResult, result);
+        });
+    }
+
+    @Test
+    public void validateSetVolumeFormatNullFormatAndSetFormatFalseDoNothing(){
+        VolumeObjectTO volumeObjectTo = new VolumeObjectTO();
+        volumeObjectTo.setFormat(null);
+
+        volumeObjectSpy.setVolumeFormat(volumeObjectTo, false, volumeVoMock);
+        Mockito.verifyNoInteractions(volumeVoMock);
+    }
+
+    @Test
+    public void validateSetVolumeFormatNullFormatAndSetFormatTrueDoNothing(){
+        VolumeObjectTO volumeObjectTo = new VolumeObjectTO();
+        volumeObjectTo.setFormat(null);
+
+        volumeObjectSpy.setVolumeFormat(volumeObjectTo, true, volumeVoMock);
+        Mockito.verifyNoInteractions(volumeVoMock);
+    }
+
+    @Test
+    public void validateSetVolumeFormatValidFormatAndSetFormatFalseDoNothing(){
+        VolumeObjectTO volumeObjectTo = new VolumeObjectTO();
+        List<Storage.ImageFormat> storageImageFormats = Arrays.asList(Storage.ImageFormat.values());
+
+        storageImageFormats.forEach(imageFormat -> {
+            volumeObjectTo.setFormat(Storage.ImageFormat.QCOW2);
+
+            volumeObjectSpy.setVolumeFormat(volumeObjectTo, false, volumeVoMock);
+        });
+
+        Mockito.verifyNoInteractions(volumeVoMock);
+    }
+
+    @Test
+    public void validateSetVolumeFormatValidFormatAndSetFormatTrueSetFormat(){
+        VolumeObjectTO volumeObjectTo = new VolumeObjectTO();
+        VolumeVO volumeVo = new VolumeVO() {};
+        List<Storage.ImageFormat> storageImageFormats = Arrays.asList(Storage.ImageFormat.values());
+
+        storageImageFormats.forEach(imageFormat -> {
+            volumeObjectTo.setFormat(imageFormat);
+
+            volumeObjectSpy.setVolumeFormat(volumeObjectTo, true, volumeVo);
+            Assert.assertEquals(imageFormat, volumeVo.getFormat());
+        });
+    }
+
+    @Test
+    public void validateHandleProcessEventAnswerDownloadAnswerIsPrimaryDataStore(){
+        Mockito.doReturn(true).when(volumeObjectSpy).isPrimaryDataStore();
+        volumeObjectSpy.handleProcessEventAnswer(new DownloadAnswer() {});
+        Mockito.verifyNoInteractions(dataStoreMock, volumeDataStoreDaoMock);
+    }
+
+    @Test
+    public void validateHandleProcessEventAnswerDownloadAnswerIsNotPrimaryDataStore(){
+        Mockito.doReturn(false).when(volumeObjectSpy).isPrimaryDataStore();
+        Mockito.doReturn(1l).when(volumeObjectSpy).getId();
+        Mockito.doReturn(1l).when(dataStoreMock).getId();
+        Mockito.doReturn(volumeDataStoreVoMock).when(volumeDataStoreDaoMock).findByStoreVolume(Mockito.anyLong(), Mockito.anyLong());
+        Mockito.doReturn(true).when(volumeDataStoreDaoMock).update(Mockito.anyLong(), Mockito.any());
+
+        volumeObjectSpy.handleProcessEventAnswer(new DownloadAnswer() {});
+        Mockito.verify(volumeDataStoreDaoMock, Mockito.times(1)).findByStoreVolume(Mockito.anyLong(), Mockito.anyLong());
+        Mockito.verify(volumeDataStoreDaoMock, Mockito.times(1)).update(Mockito.anyLong(), Mockito.any());
+    }
+
+    @Test
+    public void validateUpdateVolumeInfoSetSetVolumeSizeFalseAndVolumeSizeNullDoNotSetVolumeSize(){
+        Mockito.doReturn(null).when(volumeObjectToMock).getSize();
+        Mockito.doNothing().when(volumeObjectSpy).setVolumeFormat(Mockito.any(), Mockito.anyBoolean(), Mockito.any());
+        Mockito.doReturn(true).when(volumeDaoMock).update(Mockito.anyLong(), Mockito.any());
+
+        volumeObjectSpy.updateVolumeInfo(volumeObjectToMock, volumeVoMock, false, false);
+
+        Mockito.verify(volumeVoMock, Mockito.never()).setSize(Mockito.anyLong());
+        Mockito.verify(volumeDaoMock, Mockito.times(1)).update(Mockito.anyLong(), Mockito.any());
+    }
+
+    @Test
+    public void validateUpdateVolumeInfoSetSetVolumeSizeTrueAndVolumeSizeNullDoNotSetVolumeSize(){
+        Mockito.doReturn(null).when(volumeObjectToMock).getSize();
+        Mockito.doNothing().when(volumeObjectSpy).setVolumeFormat(Mockito.any(), Mockito.anyBoolean(), Mockito.any());
+        Mockito.doReturn(true).when(volumeDaoMock).update(Mockito.anyLong(), Mockito.any());
+
+        volumeObjectSpy.updateVolumeInfo(volumeObjectToMock, volumeVoMock, true, false);
+
+        Mockito.verify(volumeVoMock, Mockito.never()).setSize(Mockito.anyLong());
+        Mockito.verify(volumeDaoMock, Mockito.times(1)).update(Mockito.anyLong(), Mockito.any());
+    }
+
+    @Test
+    public void validateUpdateVolumeInfoSetSetVolumeSizeFalseAndVolumeSizeNotNullDoNotSetVolumeSize(){
+        Mockito.doReturn(1l).when(volumeObjectToMock).getSize();
+        Mockito.doNothing().when(volumeObjectSpy).setVolumeFormat(Mockito.any(), Mockito.anyBoolean(), Mockito.any());
+        Mockito.doReturn(true).when(volumeDaoMock).update(Mockito.anyLong(), Mockito.any());
+
+        volumeObjectSpy.updateVolumeInfo(volumeObjectToMock, volumeVoMock, false, false);
+
+        Mockito.verify(volumeVoMock, Mockito.never()).setSize(Mockito.anyLong());
+        Mockito.verify(volumeDaoMock, Mockito.times(1)).update(Mockito.anyLong(), Mockito.any());
+    }
+
+    @Test
+    public void validateUpdateVolumeInfoSetSetVolumeSizeTrueAndVolumeSizeNotNullVolumeSize(){
+        Mockito.doReturn(1l).when(volumeObjectToMock).getSize();
+        Mockito.doNothing().when(volumeObjectSpy).setVolumeFormat(Mockito.any(), Mockito.anyBoolean(), Mockito.any());
+        Mockito.doReturn(true).when(volumeDaoMock).update(Mockito.anyLong(), Mockito.any());
+
+        volumeObjectSpy.updateVolumeInfo(volumeObjectToMock, volumeVoMock, true, false);
+
+        Mockito.verify(volumeVoMock, Mockito.times(1)).setSize(Mockito.anyLong());
+        Mockito.verify(volumeDaoMock, Mockito.times(1)).update(Mockito.anyLong(), Mockito.any());
+    }
+
+    @Test
+    public void validateHandleProcessEventAnswerCreateObjectAnswerIsNotPrimaryDataStore(){
+        Mockito.doReturn(false).when(volumeObjectSpy).isPrimaryDataStore();
+        volumeObjectSpy.handleProcessEventAnswer(new CreateObjectAnswer(volumeObjectToMock), false);
+        Mockito.verifyNoInteractions(volumeObjectToMock, volumeDaoMock);
+    }
+
+    @Test
+    public void validateHandleProcessEventAnswerCreateObjectAnswerPrimaryDataStore(){
+        Mockito.doReturn(true).when(volumeObjectSpy).isPrimaryDataStore();
+        Mockito.doReturn(1l).when(volumeObjectSpy).getId();
+        Mockito.doReturn(volumeVoMock).when(volumeDaoMock).findById(Mockito.anyLong());
+        Mockito.doNothing().when(volumeObjectSpy).updateVolumeInfo(Mockito.any(), Mockito.any(), Mockito.anyBoolean(), Mockito.anyBoolean());
+
+        volumeObjectSpy.handleProcessEventAnswer(new CreateObjectAnswer(volumeObjectToMock), false);
+
+        Mockito.verify(volumeDaoMock, Mockito.times(1)).findById(Mockito.anyLong());
+        Mockito.verify(volumeObjectSpy, Mockito.times(1)).updateVolumeInfo(Mockito.any(), Mockito.any(), Mockito.anyBoolean(), Mockito.anyBoolean());
+    }
+
+    @Test
+    public void validateHandleProcessEventAnswerCreateObjectAnswer(){
+        CreateObjectAnswer createObjectAnswer = new CreateObjectAnswer(volumeObjectToMock);
+        Mockito.doNothing().when(volumeObjectSpy).handleProcessEventAnswer(Mockito.any(), Mockito.anyBoolean());
+
+        volumeObjectSpy.handleProcessEventAnswer(createObjectAnswer);
+
+        Mockito.verify(volumeObjectSpy, Mockito.times(1)).handleProcessEventAnswer(createObjectAnswer, true);
+    }
+
+    @Test
+    public void validateHandleProcessEventCopyCmdAnswerNotPrimaryStoreDoNotSetSize(){
+        Mockito.doReturn(1l).when(volumeObjectSpy).getId();
+        Mockito.doReturn(1l).when(dataStoreMock).getId();
+        Mockito.doReturn(volumeDataStoreVoMock).when(volumeDataStoreDaoMock).findByStoreVolume(Mockito.anyLong(), Mockito.anyLong());
+        Mockito.doReturn(null).when(volumeObjectToMock).getSize();
+        Mockito.doReturn(true).when(volumeDataStoreDaoMock).update(Mockito.anyLong(), Mockito.any());
+
+        volumeObjectSpy.handleProcessEventAnswer(new CopyCmdAnswer(volumeObjectToMock) {});
+
+        Mockito.verify(volumeDataStoreDaoMock, Mockito.times(1)).findByStoreVolume(Mockito.anyLong(), Mockito.anyLong());
+        Mockito.verify(volumeDataStoreVoMock, Mockito.never()).setSize(Mockito.anyLong());
+        Mockito.verify(volumeDataStoreDaoMock, Mockito.times(1)).update(Mockito.anyLong(), Mockito.any());
+    }
+
+    @Test
+    public void validateHandleProcessEventCopyCmdAnswerNotPrimaryStoreSetSize(){
+        Mockito.doReturn(1l).when(volumeObjectSpy).getId();
+        Mockito.doReturn(1l).when(dataStoreMock).getId();
+        Mockito.doReturn(volumeDataStoreVoMock).when(volumeDataStoreDaoMock).findByStoreVolume(Mockito.anyLong(), Mockito.anyLong());
+        Mockito.doReturn(1l).when(volumeObjectToMock).getSize();
+        Mockito.doReturn(true).when(volumeDataStoreDaoMock).update(Mockito.anyLong(), Mockito.any());
+
+        volumeObjectSpy.handleProcessEventAnswer(new CopyCmdAnswer(volumeObjectToMock) {});
+
+        Mockito.verify(volumeDataStoreDaoMock, Mockito.times(1)).findByStoreVolume(Mockito.anyLong(), Mockito.anyLong());
+        Mockito.verify(volumeDataStoreVoMock, Mockito.times(1)).setSize(Mockito.anyLong());
+        Mockito.verify(volumeDataStoreDaoMock, Mockito.times(1)).update(Mockito.anyLong(), Mockito.any());
+    }
+
+    @Test
+    public void validateHandleProcessEventCopyCmdAnswerPrimaryStore(){
+        Mockito.doReturn(1l).when(volumeObjectSpy).getId();
+        Mockito.doReturn(volumeVoMock).when(volumeDaoMock).findById(Mockito.anyLong());
+        Mockito.doNothing().when(volumeObjectSpy).updateVolumeInfo(Mockito.any(), Mockito.any(), Mockito.anyBoolean(), Mockito.anyBoolean());
+
+        volumeObjectSpy.handleProcessEventCopyCmdAnswerPrimaryStore(volumeObjectToMock, true, true);
+
+        Mockito.verify(volumeDaoMock, Mockito.times(1)).findById(Mockito.anyLong());
+        Mockito.verify(volumeObjectSpy, Mockito.times(1)).updateVolumeInfo(Mockito.any(), Mockito.any(), Mockito.anyBoolean(), Mockito.anyBoolean());
+    }
+
+    @Test
+    public void validateHandleProcessEventAnswerCopyCmdAnswerIsPrimaryStore(){
+        Mockito.doReturn(true).when(volumeObjectSpy).isPrimaryDataStore();
+        Mockito.doNothing().when(volumeObjectSpy).handleProcessEventCopyCmdAnswerPrimaryStore(Mockito.any(), Mockito.anyBoolean(), Mockito.anyBoolean());
+
+        volumeObjectSpy.handleProcessEventAnswer(new CopyCmdAnswer(volumeObjectToMock), true, false);
+
+        Mockito.verify(volumeObjectSpy, Mockito.times(1)).handleProcessEventCopyCmdAnswerPrimaryStore(Mockito.any(), Mockito.anyBoolean(), Mockito.anyBoolean());
+        Mockito.verify(volumeObjectSpy, Mockito.never()).handleProcessEventCopyCmdAnswerNotPrimaryStore(Mockito.any());
+    }
+
+    @Test
+    public void validateHandleProcessEventAnswerCopyCmdAnswerIsNotPrimaryStore(){
+        Mockito.doReturn(false).when(volumeObjectSpy).isPrimaryDataStore();
+        Mockito.doNothing().when(volumeObjectSpy).handleProcessEventCopyCmdAnswerNotPrimaryStore(Mockito.any());
+
+        volumeObjectSpy.handleProcessEventAnswer(new CopyCmdAnswer(volumeObjectToMock), true, false);
+
+        Mockito.verify(volumeObjectSpy, Mockito.never()).handleProcessEventCopyCmdAnswerPrimaryStore(Mockito.any(), Mockito.anyBoolean(), Mockito.anyBoolean());
+        Mockito.verify(volumeObjectSpy, Mockito.times(1)).handleProcessEventCopyCmdAnswerNotPrimaryStore(Mockito.any());
+    }
+
+    @Test
+    public void validateHandleProcessEventAnswerCopyCmdAnswer(){
+        CopyCmdAnswer copyCmdAnswer = new CopyCmdAnswer(volumeObjectToMock);
+        Mockito.doNothing().when(volumeObjectSpy).handleProcessEventAnswer(Mockito.any(), Mockito.anyBoolean(), Mockito.anyBoolean());
+
+        volumeObjectSpy.handleProcessEventAnswer(copyCmdAnswer);
+
+        Mockito.verify(volumeObjectSpy, Mockito.times(1)).handleProcessEventAnswer(copyCmdAnswer, true, true);
+    }
+}

engine/storage/volume/src/test/java/org/apache/cloudstack/storage/volume/VolumeServiceTest.java

@@ -0,0 +1,213 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.apache.cloudstack.storage.volume;
+
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.storage.snapshot.SnapshotManager;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import junit.framework.TestCase;
+import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService;
+import org.apache.cloudstack.framework.async.AsyncCallFuture;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class VolumeServiceTest extends TestCase{
+
+    @Spy
+    VolumeServiceImpl volumeServiceImplSpy;
+
+    @Mock
+    VolumeDataFactory volumeDataFactoryMock;
+
+    @Mock
+    VolumeInfo volumeInfoMock;
+
+    @Mock
+    AsyncCallFuture<VolumeService.VolumeApiResult> asyncCallFutureVolumeApiResultMock;
+
+    @Mock
+    VolumeService.VolumeApiResult volumeApiResultMock;
+
+    @Mock
+    VolumeDao volumeDaoMock;
+
+    @Mock
+    SnapshotManager snapshotManagerMock;
+
+    @Mock
+    VolumeVO volumeVoMock;
+
+    @Before
+    public void setup(){
+        volumeServiceImplSpy = Mockito.spy(new VolumeServiceImpl());
+        volumeServiceImplSpy.volFactory = volumeDataFactoryMock;
+        volumeServiceImplSpy.volDao = volumeDaoMock;
+        volumeServiceImplSpy.snapshotMgr = snapshotManagerMock;
+    }
+
+    @Test(expected = InterruptedException.class)
+    public void validateExpungeSourceVolumeAfterMigrationThrowInterruptedExceptionOnFirstFutureGetCall() throws InterruptedException, ExecutionException{
+        Mockito.doReturn(volumeInfoMock).when(volumeDataFactoryMock).getVolume(Mockito.anyLong());
+        Mockito.doReturn(asyncCallFutureVolumeApiResultMock).when(volumeServiceImplSpy).expungeVolumeAsync(Mockito.any());
+        Mockito.doThrow(new InterruptedException()).when(asyncCallFutureVolumeApiResultMock).get();
+
+        volumeServiceImplSpy.expungeSourceVolumeAfterMigration(new VolumeVO() {}, false);
+    }
+
+    @Test(expected = ExecutionException.class)
+    public void validateExpungeSourceVolumeAfterMigrationThrowExecutionExceptionOnFirstFutureGetCall() throws InterruptedException, ExecutionException{
+        Mockito.doReturn(volumeInfoMock).when(volumeDataFactoryMock).getVolume(Mockito.anyLong());
+        Mockito.doReturn(asyncCallFutureVolumeApiResultMock).when(volumeServiceImplSpy).expungeVolumeAsync(Mockito.any());
+        Mockito.doThrow(new ExecutionException() {}).when(asyncCallFutureVolumeApiResultMock).get();
+
+        volumeServiceImplSpy.expungeSourceVolumeAfterMigration(new VolumeVO() {}, false);
+    }
+
+    @Test
+    public void validateExpungeSourceVolumeAfterMigrationVolumeApiResultSucceedDoNoMoreInteractions() throws InterruptedException, ExecutionException{
+        Mockito.doReturn(volumeInfoMock).when(volumeDataFactoryMock).getVolume(Mockito.anyLong());
+        Mockito.doReturn(asyncCallFutureVolumeApiResultMock).when(volumeServiceImplSpy).expungeVolumeAsync(Mockito.any());
+        Mockito.doReturn(volumeApiResultMock).when(asyncCallFutureVolumeApiResultMock).get();
+        Mockito.doReturn(true).when(volumeApiResultMock).isSuccess();
+
+        volumeServiceImplSpy.expungeSourceVolumeAfterMigration(new VolumeVO() {}, false);
+        Mockito.verify(volumeApiResultMock, Mockito.never()).getResult();
+    }
+
+    @Test
+    public void validateExpungeSourceVolumeAfterMigrationVolumeApiResultFailedDoNotRetryExpungeVolume() throws InterruptedException, ExecutionException{
+        Mockito.doReturn(volumeInfoMock).when(volumeDataFactoryMock).getVolume(Mockito.anyLong());
+        Mockito.doReturn(asyncCallFutureVolumeApiResultMock).when(volumeServiceImplSpy).expungeVolumeAsync(Mockito.any());
+        Mockito.doReturn(volumeApiResultMock).when(asyncCallFutureVolumeApiResultMock).get();
+        Mockito.doReturn(false).when(volumeApiResultMock).isSuccess();
+        boolean retryExpungeVolume = false;
+
+        volumeServiceImplSpy.expungeSourceVolumeAfterMigration(new VolumeVO() {}, retryExpungeVolume);
+        Mockito.verify(volumeServiceImplSpy, Mockito.times(1)).expungeVolumeAsync(volumeInfoMock);
+    }
+
+    @Test (expected = InterruptedException.class)
+    public void validateExpungeSourceVolumeAfterMigrationVolumeApiResultFailedRetryExpungeVolumeThrowInterruptedException() throws InterruptedException, ExecutionException{
+        Mockito.doReturn(volumeInfoMock).when(volumeDataFactoryMock).getVolume(Mockito.anyLong());
+        Mockito.doReturn(asyncCallFutureVolumeApiResultMock).when(volumeServiceImplSpy).expungeVolumeAsync(Mockito.any());
+        Mockito.doReturn(volumeApiResultMock).doThrow(new InterruptedException()).when(asyncCallFutureVolumeApiResultMock).get();
+        Mockito.doReturn(false).when(volumeApiResultMock).isSuccess();
+        boolean retryExpungeVolume = true;
+
+        volumeServiceImplSpy.expungeSourceVolumeAfterMigration(new VolumeVO() {}, retryExpungeVolume);
+    }
+
+    @Test (expected = ExecutionException.class)
+    public void validateExpungeSourceVolumeAfterMigrationVolumeApiResultFailedRetryExpungeVolumeThrowExecutionException() throws InterruptedException, ExecutionException{
+        Mockito.doReturn(volumeInfoMock).when(volumeDataFactoryMock).getVolume(Mockito.anyLong());
+        Mockito.doReturn(asyncCallFutureVolumeApiResultMock).when(volumeServiceImplSpy).expungeVolumeAsync(Mockito.any());
+        Mockito.doReturn(volumeApiResultMock).doThrow(new ExecutionException(){}).when(asyncCallFutureVolumeApiResultMock).get();
+        Mockito.doReturn(false).when(volumeApiResultMock).isSuccess();
+        boolean retryExpungeVolume = true;
+
+        volumeServiceImplSpy.expungeSourceVolumeAfterMigration(new VolumeVO() {}, retryExpungeVolume);
+    }
+
+    @Test
+    public void validateCopyPoliciesBetweenVolumesAndDestroySourceVolumeAfterMigrationReturnTrueOrFalse() throws ExecutionException, InterruptedException{
+        VolumeObject volumeObject = new VolumeObject();
+        volumeObject.configure(null, new VolumeVO() {});
+
+        Mockito.doNothing().when(snapshotManagerMock).copySnapshotPoliciesBetweenVolumes(Mockito.any(), Mockito.any());
+        Mockito.doReturn(true, false).when(volumeServiceImplSpy).destroySourceVolumeAfterMigration(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyBoolean());
+
+        boolean result = volumeServiceImplSpy.copyPoliciesBetweenVolumesAndDestroySourceVolumeAfterMigration(ObjectInDataStoreStateMachine.Event.DestroyRequested, null,
+          volumeObject, volumeObject, true);
+        boolean result2 = volumeServiceImplSpy.copyPoliciesBetweenVolumesAndDestroySourceVolumeAfterMigration(ObjectInDataStoreStateMachine.Event.DestroyRequested, null,
+          volumeObject, volumeObject, true);
+
+        Assert.assertTrue(result);
+        Assert.assertFalse(result2);
+    }
+
+    @Test (expected = Exception.class)
+    public void validateCopyPoliciesBetweenVolumesAndDestroySourceVolumeAfterMigrationThrowAnyOtherException() throws
+      ExecutionException, InterruptedException{
+        VolumeObject volumeObject = new VolumeObject();
+        volumeObject.configure(null, new VolumeVO() {});
+
+        volumeServiceImplSpy.copyPoliciesBetweenVolumesAndDestroySourceVolumeAfterMigration(ObjectInDataStoreStateMachine.Event.DestroyRequested, null, volumeObject,
+          volumeObject, true);
+    }
+
+    @Test
+    public void validateDestroySourceVolumeAfterMigrationReturnTrue() throws ExecutionException, InterruptedException{
+        VolumeObject volumeObject = new VolumeObject();
+        volumeObject.configure(null, new VolumeVO() {});
+
+        Mockito.doReturn(true).when(volumeDaoMock).updateUuid(Mockito.anyLong(), Mockito.anyLong());
+        Mockito.doNothing().when(volumeServiceImplSpy).destroyVolume(Mockito.anyLong());
+        Mockito.doNothing().when(volumeServiceImplSpy).expungeSourceVolumeAfterMigration(Mockito.any(), Mockito.anyBoolean());
+
+        boolean result = volumeServiceImplSpy.destroySourceVolumeAfterMigration(ObjectInDataStoreStateMachine.Event.DestroyRequested, null, volumeObject,
+          volumeObject, true);
+
+        Assert.assertTrue(result);
+    }
+
+    @Test
+    public void validateDestroySourceVolumeAfterMigrationExpungeSourceVolumeAfterMigrationThrowExceptionReturnFalse() throws
+      ExecutionException, InterruptedException{
+        VolumeObject volumeObject = new VolumeObject();
+        volumeObject.configure(null, new VolumeVO() {});
+
+        List<Exception> exceptions = new ArrayList<>(Arrays.asList(new InterruptedException(), new ExecutionException() {}));
+
+        for (Exception exception : exceptions) {
+            Mockito.doReturn(true).when(volumeDaoMock).updateUuid(Mockito.anyLong(), Mockito.anyLong());
+            Mockito.doNothing().when(volumeServiceImplSpy).destroyVolume(Mockito.anyLong());
+            Mockito.doThrow(exception).when(volumeServiceImplSpy).expungeSourceVolumeAfterMigration(Mockito.any(), Mockito.anyBoolean());
+
+            boolean result = volumeServiceImplSpy.destroySourceVolumeAfterMigration(ObjectInDataStoreStateMachine.Event.DestroyRequested, null,
+              volumeObject, volumeObject, true);
+
+            Assert.assertFalse(result);
+        }
+    }
+
+    @Test (expected = Exception.class)
+    public void validateDestroySourceVolumeAfterMigrationThrowAnyOtherException() throws
+      ExecutionException, InterruptedException{
+        VolumeObject volumeObject = new VolumeObject();
+        volumeObject.configure(null, new VolumeVO() {});
+
+        volumeServiceImplSpy.destroySourceVolumeAfterMigration(ObjectInDataStoreStateMachine.Event.DestroyRequested, null, volumeObject,
+          volumeObject, true);
+    }
+}

engine/storage/volume/src/test/java/org/apache/cloudstack/storage/volume/test/ConfiguratorTest.java

@@ -1,93 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you 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.apache.cloudstack.storage.volume.test;
-
-import static org.junit.Assert.assertTrue;
-
-import java.util.List;
-
-import javax.inject.Inject;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.springframework.test.context.ContextConfiguration;
-import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
-
-import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreProvider;
-
-import com.cloud.dc.dao.ClusterDao;
-
-@RunWith(SpringJUnit4ClassRunner.class)
-@ContextConfiguration(locations = "classpath:/testContext.xml")
-public class ConfiguratorTest {
-
-    @Inject
-    List<PrimaryDataStoreProvider> providers;
-
-    @Inject
-    ClusterDao clusterDao;
-
-    @Before
-    public void setup() {
-        /*
-         * ClusterVO cluster = new ClusterVO();
-         * cluster.setHypervisorType(HypervisorType.XenServer.toString());
-         * Mockito
-         * .when(clusterDao.findById(Mockito.anyLong())).thenReturn(cluster);
-         * try { providerMgr.configure("manager", null); } catch
-         * (ConfigurationException e) { // TODO Auto-generated catch block
-         * e.printStackTrace(); }
-         */
-    }
-
-    @Test
-    public void testLoadConfigurator() {
-        /*
-         * for (PrimaryDataStoreConfigurator configurator : configurators) {
-         * System.out.println(configurator.getClass().getName()); }
-         */
-    }
-
-    @Test
-    public void testProvider() {
-        for (PrimaryDataStoreProvider provider : providers) {
-            if (provider.getName().startsWith("default")) {
-                assertTrue(true);
-            }
-        }
-    }
-
-    @Test
-    public void getProvider() {
-        // assertNotNull(providerMgr.getDataStoreProvider("sample primary data store provider"));
-    }
-
-    @Test
-    public void createDataStore() {
-        /*
-         * PrimaryDataStoreProvider provider =
-         * providerMgr.getDataStoreProvider("sample primary data store provider"
-         * ); Map<String, String> params = new HashMap<String, String>();
-         * params.put("url", "nfs://localhost/mnt"); params.put("clusterId",
-         * "1"); params.put("name", "nfsprimary");
-         * assertNotNull(provider.registerDataStore(params));
-         */
-    }
-}

engine/storage/volume/src/test/java/org/apache/cloudstack/storage/volume/test/Server.java

@@ -1,43 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you 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.apache.cloudstack.storage.volume.test;
-
-import org.apache.cloudstack.framework.async.AsyncCallbackDispatcher;
-
-public class Server {
-    Server1 svr;
-
-    public Server() {
-        svr = new Server1();
-    }
-
-    void foo() {
-        // svr.foo1("foo", new
-        // AsyncCallbackDispatcher(this).setOperationName("callback").setContextParam("name",
-        // "foo"));
-    }
-
-    void foocallback(AsyncCallbackDispatcher callback) {
-        /*
-         * System.out.println(callback.getContextParam("name")); String result =
-         * callback.getResult(); System.out.println(result);
-         */
-    }
-
-}

engine/storage/volume/src/test/java/org/apache/cloudstack/storage/volume/test/Server1.java

@@ -1,27 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you 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.apache.cloudstack.storage.volume.test;
-
-import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
-
-public class Server1 {
-    public void foo1(String name, AsyncCompletionCallback<String> callback) {
-        callback.complete("success");
-    }
-}

engine/storage/volume/src/test/java/org/apache/cloudstack/storage/volume/test/TestConfiguration.java

@@ -1,41 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you 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.apache.cloudstack.storage.volume.test;
-
-import org.mockito.Mockito;
-import org.springframework.context.annotation.Bean;
-import org.springframework.context.annotation.Configuration;
-
-import org.apache.cloudstack.storage.image.motion.ImageMotionService;
-
-import com.cloud.dc.dao.ClusterDao;
-import com.cloud.dc.dao.ClusterDaoImpl;
-
-@Configuration
-public class TestConfiguration {
-    @Bean
-    public ImageMotionService imageMotion() {
-        return Mockito.mock(ImageMotionService.class);
-    }
-
-    @Bean
-    public ClusterDao clusterDao() {
-        return Mockito.mock(ClusterDaoImpl.class);
-    }
-}

engine/storage/volume/src/test/java/org/apache/cloudstack/storage/volume/test/TestInProcessAsync.java

@@ -1,41 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you 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.apache.cloudstack.storage.volume.test;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.springframework.test.context.ContextConfiguration;
-import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
-
-@RunWith(SpringJUnit4ClassRunner.class)
-@ContextConfiguration(locations = "classpath:/resource/testContext.xml")
-public class TestInProcessAsync {
-    Server svr;
-
-    @Before
-    public void setup() {
-        svr = new Server();
-    }
-
-    @Test
-    public void testRpc() {
-        svr.foo();
-    }
-}

server/src/main/java/com/cloud/storage/snapshot/SnapshotManager.java

@@ -26,6 +26,7 @@ import com.cloud.agent.api.Command;
 import com.cloud.exception.ResourceAllocationException;
 import com.cloud.storage.SnapshotVO;
 import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
 
 /**
  *
@@ -85,4 +86,11 @@ public interface SnapshotManager extends Configurable {
     SnapshotVO getParentSnapshot(VolumeInfo volume);
 
     SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationException;
+
+    /**
+     * Copy the snapshot policies from a volume to another.
+     * @param srcVolume source volume.
+     * @param destVolume destination volume.
+     */
+    void copySnapshotPoliciesBetweenVolumes(VolumeVO srcVolume, VolumeVO destVolume);
 }

server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java

@@ -142,6 +142,9 @@ import com.cloud.vm.dao.UserVmDao;
 import com.cloud.vm.snapshot.VMSnapshot;
 import com.cloud.vm.snapshot.VMSnapshotVO;
 import com.cloud.vm.snapshot.dao.VMSnapshotDao;
+import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
+import org.apache.commons.lang3.builder.ReflectionToStringBuilder;
+import org.apache.commons.lang3.builder.ToStringStyle;
 
 @Component
 public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implements SnapshotManager, SnapshotApiService, Configurable {
@@ -825,12 +828,13 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement
     public SnapshotPolicyVO createPolicy(CreateSnapshotPolicyCmd cmd, Account policyOwner) {
         Long volumeId = cmd.getVolumeId();
         boolean display = cmd.isDisplay();
-        SnapshotPolicyVO policy = null;
         VolumeVO volume = _volsDao.findById(cmd.getVolumeId());
         if (volume == null) {
             throw new InvalidParameterValueException("Failed to create snapshot policy, unable to find a volume with id " + volumeId);
         }
 
+        String volumeDescription = volume.getVolumeDescription();
+
         _accountMgr.checkAccess(CallContext.current().getCallingAccount(), null, true, volume);
 
         // If display is false we don't actually schedule snapshots.
@@ -847,45 +851,55 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement
 
         AccountVO owner = _accountDao.findById(volume.getAccountId());
         Long instanceId = volume.getInstanceId();
+        String intervalType = cmd.getIntervalType();
         if (instanceId != null) {
             // It is not detached, but attached to a VM
             if (_vmDao.findById(instanceId) == null) {
                 // It is not a UserVM but a SystemVM or DomR
-                throw new InvalidParameterValueException("Failed to create snapshot policy, snapshots of volumes attached to System or router VM are not allowed");
+                throw new InvalidParameterValueException(String.format("Failed to create snapshot policy [%s] for volume %s; Snapshots of volumes attached to System or router VM are not allowed.", intervalType, volumeDescription));
             }
         }
-        IntervalType intvType = DateUtil.IntervalType.getIntervalType(cmd.getIntervalType());
+
+        IntervalType intvType = DateUtil.IntervalType.getIntervalType(intervalType);
         if (intvType == null) {
-            throw new InvalidParameterValueException("Unsupported interval type " + cmd.getIntervalType());
+            throw new InvalidParameterValueException("Unsupported interval type " + intervalType);
         }
         Type type = getSnapshotType(intvType);
+        String cmdTimezone = cmd.getTimezone();
 
-        TimeZone timeZone = TimeZone.getTimeZone(cmd.getTimezone());
+        TimeZone timeZone = TimeZone.getTimeZone(cmdTimezone);
         String timezoneId = timeZone.getID();
-        if (!timezoneId.equals(cmd.getTimezone())) {
+        if (!timezoneId.equals(cmdTimezone)) {
-            s_logger.warn("Using timezone: " + timezoneId + " for running this snapshot policy as an equivalent of " + cmd.getTimezone());
+            s_logger.warn(String.format("Using timezone [%s] for running the snapshot policy [%s] for volume %s, as an equivalent of [%s].", timezoneId, intervalType, volumeDescription,
-        }
+              cmdTimezone));
-        try {
-            DateUtil.getNextRunTime(intvType, cmd.getSchedule(), timezoneId, null);
-        } catch (Exception e) {
-            throw new InvalidParameterValueException("Invalid schedule: " + cmd.getSchedule() + " for interval type: " + cmd.getIntervalType());
         }
 
-        if (cmd.getMaxSnaps() <= 0) {
+        String schedule = cmd.getSchedule();
-            throw new InvalidParameterValueException("maxSnaps should be greater than 0");
+
+        try {
+            DateUtil.getNextRunTime(intvType, schedule, timezoneId, null);
+        } catch (Exception e) {
+            throw new InvalidParameterValueException(String.format("%s has an invalid schedule [%s] for interval type [%s].",
+              volumeDescription, schedule, intervalType));
+        }
+
+        int maxSnaps = cmd.getMaxSnaps();
+
+        if (maxSnaps <= 0) {
+            throw new InvalidParameterValueException(String.format("maxSnaps [%s] for volume %s should be greater than 0.", maxSnaps, volumeDescription));
         }
 
         int intervalMaxSnaps = type.getMax();
-        if (cmd.getMaxSnaps() > intervalMaxSnaps) {
+        if (maxSnaps > intervalMaxSnaps) {
-            throw new InvalidParameterValueException("maxSnaps exceeds limit: " + intervalMaxSnaps + " for interval type: " + cmd.getIntervalType());
+            throw new InvalidParameterValueException(String.format("maxSnaps [%s] for volume %s exceeds limit [%s] for interval type [%s].", maxSnaps, volumeDescription,
+              intervalMaxSnaps, intervalType));
         }
 
         // Verify that max doesn't exceed domain and account snapshot limits in case display is on
         if (display) {
             long accountLimit = _resourceLimitMgr.findCorrectResourceLimitForAccount(owner, ResourceType.snapshot);
             long domainLimit = _resourceLimitMgr.findCorrectResourceLimitForDomain(_domainMgr.getDomain(owner.getDomainId()), ResourceType.snapshot);
-            int max = cmd.getMaxSnaps().intValue();
+            if (!_accountMgr.isRootAdmin(owner.getId()) && ((accountLimit != -1 && maxSnaps > accountLimit) || (domainLimit != -1 && maxSnaps > domainLimit))) {
-            if (!_accountMgr.isRootAdmin(owner.getId()) && ((accountLimit != -1 && max > accountLimit) || (domainLimit != -1 && max > domainLimit))) {
                 String message = "domain/account";
                 if (owner.getType() == Account.ACCOUNT_TYPE_PROJECT) {
                     message = "domain/project";
@@ -895,45 +909,87 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement
             }
         }
 
-        final GlobalLock createSnapshotPolicyLock = GlobalLock.getInternLock("createSnapshotPolicy_" + volumeId);
+        Map<String, String> tags = cmd.getTags();
+        boolean active = true;
+
+        return persistSnapshotPolicy(volume, schedule, timezoneId, intvType, maxSnaps, display, active, tags);
+    }
+
+    protected SnapshotPolicyVO persistSnapshotPolicy(VolumeVO volume, String schedule, String timezone, IntervalType intervalType, int maxSnaps, boolean display, boolean active, Map<String, String> tags) {
+        long volumeId = volume.getId();
+        String volumeDescription = volume.getVolumeDescription();
+
+        GlobalLock createSnapshotPolicyLock = GlobalLock.getInternLock("createSnapshotPolicy_" + volumeId);
         boolean isLockAcquired = createSnapshotPolicyLock.lock(5);
-        if (isLockAcquired) {
+
-            s_logger.debug("Acquired lock for creating snapshot policy of volume : " + volume.getName());
+        if (!isLockAcquired) {
-            try {
+            throw new CloudRuntimeException(String.format("Unable to aquire lock for creating snapshot policy [%s] for %s.", intervalType, volumeDescription));
-                policy = _snapshotPolicyDao.findOneByVolumeInterval(volumeId, intvType);
+        }
-                if (policy == null) {
+
-                    policy = new SnapshotPolicyVO(volumeId, cmd.getSchedule(), timezoneId, intvType, cmd.getMaxSnaps(), display);
+        s_logger.debug(String.format("Acquired lock for creating snapshot policy [%s] for volume %s.", intervalType, volumeDescription));
-                    policy = _snapshotPolicyDao.persist(policy);
+
-                    _snapSchedMgr.scheduleNextSnapshotJob(policy);
+        try {
-                } else {
+            SnapshotPolicyVO policy = _snapshotPolicyDao.findOneByVolumeInterval(volumeId, intervalType);
-                    boolean previousDisplay = policy.isDisplay();
+
-                    policy.setSchedule(cmd.getSchedule());
+            if (policy == null) {
-                    policy.setTimezone(timezoneId);
+                policy = createSnapshotPolicy(volumeId, schedule, timezone, intervalType, maxSnaps, display);
-                    policy.setInterval((short)intvType.ordinal());
+            } else {
-                    policy.setMaxSnaps(cmd.getMaxSnaps());
+                updateSnapshotPolicy(policy, schedule, timezone, intervalType, maxSnaps, active, display);
-                    policy.setActive(true);
-                    policy.setDisplay(display);
-                    _snapshotPolicyDao.update(policy.getId(), policy);
-                    _snapSchedMgr.scheduleOrCancelNextSnapshotJobOnDisplayChange(policy, previousDisplay);
-                    taggedResourceService.deleteTags(Collections.singletonList(policy.getUuid()), ResourceObjectType.SnapshotPolicy, null);
-                }
-                final Map<String, String> tags = cmd.getTags();
-                if (MapUtils.isNotEmpty(tags)) {
-                    taggedResourceService.createTags(Collections.singletonList(policy.getUuid()), ResourceObjectType.SnapshotPolicy, tags, null);
-                }
-            } finally {
-                createSnapshotPolicyLock.unlock();
             }
 
-            // TODO - Make createSnapshotPolicy - BaseAsyncCreate and remove this.
+            createTagsForSnapshotPolicy(tags, policy);
             CallContext.current().putContextParameter(SnapshotPolicy.class, policy.getUuid());
+
             return policy;
-        } else {
+        } finally {
-            s_logger.warn("Unable to acquire lock for creating snapshot policy of volume : " + volume.getName());
+            createSnapshotPolicyLock.unlock();
-            return null;
         }
     }
 
+    protected SnapshotPolicyVO createSnapshotPolicy(long volumeId, String schedule, String timezone, IntervalType intervalType, int maxSnaps, boolean display) {
+        SnapshotPolicyVO policy = new SnapshotPolicyVO(volumeId, schedule, timezone, intervalType, maxSnaps, display);
+        policy = _snapshotPolicyDao.persist(policy);
+        _snapSchedMgr.scheduleNextSnapshotJob(policy);
+        s_logger.debug(String.format("Created snapshot policy %s.", new ReflectionToStringBuilder(policy, ToStringStyle.JSON_STYLE).setExcludeFieldNames("id", "uuid", "active")));
+        return policy;
+    }
+
+    protected void updateSnapshotPolicy(SnapshotPolicyVO policy, String schedule, String timezone, IntervalType intervalType, int maxSnaps, boolean active, boolean display) {
+        String previousPolicy = new ReflectionToStringBuilder(policy, ToStringStyle.JSON_STYLE).setExcludeFieldNames("id", "uuid").toString();
+        boolean previousDisplay = policy.isDisplay();
+        policy.setSchedule(schedule);
+        policy.setTimezone(timezone);
+        policy.setInterval((short) intervalType.ordinal());
+        policy.setMaxSnaps(maxSnaps);
+        policy.setActive(active);
+        policy.setDisplay(display);
+        _snapshotPolicyDao.update(policy.getId(), policy);
+        _snapSchedMgr.scheduleOrCancelNextSnapshotJobOnDisplayChange(policy, previousDisplay);
+        taggedResourceService.deleteTags(Collections.singletonList(policy.getUuid()), ResourceObjectType.SnapshotPolicy, null);
+        s_logger.debug(String.format("Updated snapshot policy %s to %s.", previousPolicy, new ReflectionToStringBuilder(policy, ToStringStyle.JSON_STYLE)
+          .setExcludeFieldNames("id", "uuid")));
+    }
+
+    protected void createTagsForSnapshotPolicy(Map<String, String> tags, SnapshotPolicyVO policy) {
+        if (MapUtils.isNotEmpty(tags)) {
+            taggedResourceService.createTags(Collections.singletonList(policy.getUuid()), ResourceObjectType.SnapshotPolicy, tags, null);
+        }
+    }
+
+    @Override
+    public void copySnapshotPoliciesBetweenVolumes(VolumeVO srcVolume, VolumeVO destVolume){
+        IntervalType[] intervalTypes = IntervalType.values();
+        List<SnapshotPolicyVO> policies = listPoliciesforVolume(srcVolume.getId());
+
+        s_logger.debug(String.format("Copying snapshot policies %s from volume %s to volume %s.", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(policies,
+          "id", "uuid"), srcVolume.getVolumeDescription(), destVolume.getVolumeDescription()));
+
+        policies.forEach(policy ->
+          persistSnapshotPolicy(destVolume, policy.getSchedule(), policy.getTimezone(), intervalTypes[policy.getInterval()], policy.getMaxSnaps(),
+            policy.isDisplay(), policy.isActive(), taggedResourceService.getTagsFromResource(ResourceObjectType.SnapshotPolicy, policy.getId()))
+        );
+    }
+
     protected boolean deletePolicy(Long policyId) {
         SnapshotPolicyVO snapshotPolicy = _snapshotPolicyDao.findById(policyId);
         _snapSchedMgr.removeSchedule(snapshotPolicy.getVolumeId(), snapshotPolicy.getId());

server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java

@@ -416,4 +416,10 @@ public class TaggedResourceManagerImpl extends ManagerBase implements TaggedReso
     public List<? extends ResourceTag> listByResourceTypeAndId(ResourceObjectType resourceType, long resourceId) {
         return _resourceTagDao.listBy(resourceId, resourceType);
     }
+
+    @Override
+    public Map<String, String> getTagsFromResource(ResourceObjectType type, long resourceId) {
+        List<? extends ResourceTag> listResourceTags = listByResourceTypeAndId(type, resourceId);
+        return listResourceTags == null ? null : listResourceTags.stream().collect(Collectors.toMap(ResourceTag::getKey, ResourceTag::getValue));
+    }
 }

server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerTest.java

@@ -58,14 +58,17 @@ import com.cloud.exception.ResourceAllocationException;
 import com.cloud.hypervisor.Hypervisor;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.resource.ResourceManager;
+import com.cloud.server.TaggedResourceService;
 import com.cloud.storage.DataStoreRole;
 import com.cloud.storage.ScopeType;
 import com.cloud.storage.Snapshot;
+import com.cloud.storage.SnapshotPolicyVO;
 import com.cloud.storage.SnapshotVO;
 import com.cloud.storage.Storage.ImageFormat;
 import com.cloud.storage.Volume;
 import com.cloud.storage.VolumeVO;
 import com.cloud.storage.dao.SnapshotDao;
+import com.cloud.storage.dao.SnapshotPolicyDao;
 import com.cloud.storage.dao.VolumeDao;
 import com.cloud.user.Account;
 import com.cloud.user.AccountManager;
@@ -73,6 +76,9 @@ import com.cloud.user.AccountVO;
 import com.cloud.user.ResourceLimitService;
 import com.cloud.user.User;
 import com.cloud.user.UserVO;
+import com.cloud.utils.DateUtil;
+import com.cloud.utils.DateUtil.IntervalType;
+import com.cloud.utils.db.GlobalLock;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.vm.UserVmVO;
 import com.cloud.vm.VirtualMachine.State;
@@ -80,7 +86,18 @@ import com.cloud.vm.dao.UserVmDao;
 import com.cloud.vm.snapshot.VMSnapshot;
 import com.cloud.vm.snapshot.VMSnapshotVO;
 import com.cloud.vm.snapshot.dao.VMSnapshotDao;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import org.junit.runner.RunWith;
+import org.mockito.BDDMockito;
+import org.mockito.verification.VerificationMode;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
 
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(GlobalLock.class)
 public class SnapshotManagerTest {
     @Spy
     SnapshotManagerImpl _snapshotMgr = new SnapshotManagerImpl();
@@ -133,12 +150,39 @@ public class SnapshotManagerTest {
     @Mock
     SnapshotService snapshotSrv;
 
+    @Mock
+    GlobalLock globalLockMock;
+
+    @Mock
+    SnapshotPolicyDao snapshotPolicyDaoMock;
+
+    @Mock
+    SnapshotPolicyVO snapshotPolicyVoMock;
+
+    @Mock
+    HashMap<String,String> mapStringStringMock;
+
+    @Mock
+    SnapshotScheduler snapshotSchedulerMock;
+
+    @Mock
+    TaggedResourceService taggedResourceServiceMock;
+
+    SnapshotPolicyVO snapshotPolicyVoInstance;
+
+    List<DateUtil.IntervalType> listIntervalTypes = Arrays.asList(DateUtil.IntervalType.values());
 
     private static final long TEST_SNAPSHOT_ID = 3L;
     private static final long TEST_VOLUME_ID = 4L;
     private static final long TEST_VM_ID = 5L;
     private static final long TEST_STORAGE_POOL_ID = 6L;
     private static final long TEST_VM_SNAPSHOT_ID = 6L;
+    private static final String TEST_SNAPSHOT_POLICY_SCHEDULE = "";
+    private static final String TEST_SNAPSHOT_POLICY_TIMEZONE = "";
+    private static final IntervalType TEST_SNAPSHOT_POLICY_INTERVAL = IntervalType.MONTHLY;
+    private static final int TEST_SNAPSHOT_POLICY_MAX_SNAPS = 1;
+    private static final boolean TEST_SNAPSHOT_POLICY_DISPLAY = true;
+    private static final boolean TEST_SNAPSHOT_POLICY_ACTIVE = true;
 
     @Before
     public void setup() throws ResourceAllocationException {
@@ -155,6 +199,9 @@ public class SnapshotManagerTest {
         _snapshotMgr._resourceMgr = _resourceMgr;
         _snapshotMgr._vmSnapshotDao = _vmSnapshotDao;
         _snapshotMgr._snapshotStoreDao = snapshotStoreDao;
+        _snapshotMgr._snapshotPolicyDao = snapshotPolicyDaoMock;
+        _snapshotMgr._snapSchedMgr = snapshotSchedulerMock;
+        _snapshotMgr.taggedResourceService = taggedResourceServiceMock;
 
         when(_snapshotDao.findById(anyLong())).thenReturn(snapshotMock);
         when(snapshotMock.getVolumeId()).thenReturn(TEST_VOLUME_ID);
@@ -188,6 +235,9 @@ public class SnapshotManagerTest {
         when(poolMock.getScope()).thenReturn(ScopeType.ZONE);
         when(poolMock.getHypervisor()).thenReturn(HypervisorType.KVM);
         when(_resourceMgr.listAllUpAndEnabledHostsInOneZoneByHypervisor(any(HypervisorType.class), anyLong())).thenReturn(null);
+
+        snapshotPolicyVoInstance = new SnapshotPolicyVO(TEST_VOLUME_ID, TEST_SNAPSHOT_POLICY_SCHEDULE, TEST_SNAPSHOT_POLICY_TIMEZONE, TEST_SNAPSHOT_POLICY_INTERVAL,
+          TEST_SNAPSHOT_POLICY_MAX_SNAPS, TEST_SNAPSHOT_POLICY_DISPLAY);
     }
 
     @After
@@ -352,4 +402,128 @@ public class SnapshotManagerTest {
         when(snapshotInfoMock.getStatus()).thenReturn(ObjectInDataStoreStateMachine.State.Destroyed);
         _snapshotMgr.archiveSnapshot(TEST_SNAPSHOT_ID);
     }
+
+    public void assertSnapshotPolicyResultAgainstPreBuiltInstance(SnapshotPolicyVO snapshotPolicyVo){
+        Assert.assertEquals(snapshotPolicyVoInstance.getVolumeId(), snapshotPolicyVo.getVolumeId());
+        Assert.assertEquals(snapshotPolicyVoInstance.getSchedule(), snapshotPolicyVo.getSchedule());
+        Assert.assertEquals(snapshotPolicyVoInstance.getTimezone(), snapshotPolicyVo.getTimezone());
+        Assert.assertEquals(snapshotPolicyVoInstance.getInterval(), snapshotPolicyVo.getInterval());
+        Assert.assertEquals(snapshotPolicyVoInstance.getMaxSnaps(), snapshotPolicyVo.getMaxSnaps());
+        Assert.assertEquals(snapshotPolicyVoInstance.isDisplay(), snapshotPolicyVo.isDisplay());
+        Assert.assertEquals(snapshotPolicyVoInstance.isActive(), snapshotPolicyVo.isActive());
+    }
+
+    @Test
+    public void validateCreateSnapshotPolicy(){
+        Mockito.doReturn(snapshotPolicyVoInstance).when(snapshotPolicyDaoMock).persist(Mockito.any());
+        Mockito.doReturn(null).when(snapshotSchedulerMock).scheduleNextSnapshotJob(Mockito.any());
+
+        SnapshotPolicyVO result = _snapshotMgr.createSnapshotPolicy(TEST_VOLUME_ID, TEST_SNAPSHOT_POLICY_SCHEDULE, TEST_SNAPSHOT_POLICY_TIMEZONE, TEST_SNAPSHOT_POLICY_INTERVAL,
+          TEST_SNAPSHOT_POLICY_MAX_SNAPS, TEST_SNAPSHOT_POLICY_DISPLAY);
+
+        assertSnapshotPolicyResultAgainstPreBuiltInstance(result);
+    }
+
+    @Test
+    public void validateUpdateSnapshotPolicy(){
+        Mockito.doReturn(true).when(snapshotPolicyDaoMock).update(Mockito.anyLong(), Mockito.any());
+        Mockito.doNothing().when(snapshotSchedulerMock).scheduleOrCancelNextSnapshotJobOnDisplayChange(Mockito.any(), Mockito.anyBoolean());
+        Mockito.doReturn(true).when(taggedResourceServiceMock).deleteTags(Mockito.any(), Mockito.any(), Mockito.any());
+
+        SnapshotPolicyVO snapshotPolicyVo = new SnapshotPolicyVO(TEST_VOLUME_ID, TEST_SNAPSHOT_POLICY_SCHEDULE, TEST_SNAPSHOT_POLICY_TIMEZONE, TEST_SNAPSHOT_POLICY_INTERVAL,
+          TEST_SNAPSHOT_POLICY_MAX_SNAPS, TEST_SNAPSHOT_POLICY_DISPLAY);
+
+        _snapshotMgr.updateSnapshotPolicy(snapshotPolicyVo, TEST_SNAPSHOT_POLICY_SCHEDULE, TEST_SNAPSHOT_POLICY_TIMEZONE,
+          TEST_SNAPSHOT_POLICY_INTERVAL, TEST_SNAPSHOT_POLICY_MAX_SNAPS, TEST_SNAPSHOT_POLICY_DISPLAY, TEST_SNAPSHOT_POLICY_ACTIVE);
+
+        assertSnapshotPolicyResultAgainstPreBuiltInstance(snapshotPolicyVo);
+    }
+
+    @Test
+    public void validateCreateTagsForSnapshotPolicyWithNullTags(){
+        _snapshotMgr.createTagsForSnapshotPolicy(null, snapshotPolicyVoMock);
+        Mockito.verify(taggedResourceServiceMock, Mockito.never()).createTags(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
+    }
+
+    @Test
+    public void validateCreateTagsForSnapshotPolicyWithEmptyTags(){
+        _snapshotMgr.createTagsForSnapshotPolicy(new HashMap<>(), snapshotPolicyVoMock);
+        Mockito.verify(taggedResourceServiceMock, Mockito.never()).createTags(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
+    }
+
+    @Test
+    public void validateCreateTagsForSnapshotPolicyWithValidTags(){
+        Mockito.doReturn(null).when(taggedResourceServiceMock).createTags(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
+
+        Map map = new HashMap<>();
+        map.put("test", "test");
+
+        _snapshotMgr.createTagsForSnapshotPolicy(map, snapshotPolicyVoMock);
+        Mockito.verify(taggedResourceServiceMock, Mockito.times(1)).createTags(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void validatePersistSnapshotPolicyLockIsNotAquiredMustThrowException() {
+        PowerMockito.mockStatic(GlobalLock.class);
+        BDDMockito.given(GlobalLock.getInternLock(Mockito.anyString())).willReturn(globalLockMock);
+        Mockito.doReturn(false).when(globalLockMock).lock(Mockito.anyInt());
+
+        _snapshotMgr.persistSnapshotPolicy(volumeMock, TEST_SNAPSHOT_POLICY_SCHEDULE, TEST_SNAPSHOT_POLICY_TIMEZONE, TEST_SNAPSHOT_POLICY_INTERVAL, TEST_SNAPSHOT_POLICY_MAX_SNAPS,
+                TEST_SNAPSHOT_POLICY_DISPLAY, TEST_SNAPSHOT_POLICY_ACTIVE, mapStringStringMock);
+    }
+
+    @Test
+    public void validatePersistSnapshotPolicyLockAquiredCreateSnapshotPolicy() {
+        PowerMockito.mockStatic(GlobalLock.class);
+
+        BDDMockito.given(GlobalLock.getInternLock(Mockito.anyString())).willReturn(globalLockMock);
+        Mockito.doReturn(true).when(globalLockMock).lock(Mockito.anyInt());
+
+        for (IntervalType intervalType : listIntervalTypes) {
+
+            Mockito.doReturn(null).when(snapshotPolicyDaoMock).findOneByVolumeInterval(Mockito.anyLong(), Mockito.eq(intervalType));
+            Mockito.doReturn(snapshotPolicyVoInstance).when(_snapshotMgr).createSnapshotPolicy(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.eq(intervalType),
+              Mockito.anyInt(), Mockito.anyBoolean());
+            Mockito.doNothing().when(_snapshotMgr).createTagsForSnapshotPolicy(mapStringStringMock, snapshotPolicyVoMock);
+
+            SnapshotPolicyVO result = _snapshotMgr.persistSnapshotPolicy(volumeMock, TEST_SNAPSHOT_POLICY_SCHEDULE, TEST_SNAPSHOT_POLICY_TIMEZONE, intervalType,
+              TEST_SNAPSHOT_POLICY_MAX_SNAPS, TEST_SNAPSHOT_POLICY_DISPLAY, TEST_SNAPSHOT_POLICY_ACTIVE, null);
+
+            assertSnapshotPolicyResultAgainstPreBuiltInstance(result);
+        }
+
+        VerificationMode timesVerification = Mockito.times(listIntervalTypes.size());
+        Mockito.verify(_snapshotMgr, timesVerification).createSnapshotPolicy(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.any(DateUtil.IntervalType.class),
+            Mockito.anyInt(), Mockito.anyBoolean());
+        Mockito.verify(_snapshotMgr, Mockito.never()).updateSnapshotPolicy(Mockito.any(SnapshotPolicyVO.class), Mockito.anyString(), Mockito.anyString(),
+          Mockito.any(DateUtil.IntervalType.class), Mockito.anyInt(), Mockito.anyBoolean(), Mockito.anyBoolean());
+        Mockito.verify(_snapshotMgr, timesVerification).createTagsForSnapshotPolicy(Mockito.any(), Mockito.any());
+    }
+
+    @Test
+    public void validatePersistSnapshotPolicyLockAquiredUpdateSnapshotPolicy() {
+        PowerMockito.mockStatic(GlobalLock.class);
+
+        BDDMockito.given(GlobalLock.getInternLock(Mockito.anyString())).willReturn(globalLockMock);
+        Mockito.doReturn(true).when(globalLockMock).lock(Mockito.anyInt());
+
+        for (IntervalType intervalType : listIntervalTypes) {
+            Mockito.doReturn(snapshotPolicyVoInstance).when(snapshotPolicyDaoMock).findOneByVolumeInterval(Mockito.anyLong(), Mockito.eq(intervalType));
+            Mockito.doNothing().when(_snapshotMgr).updateSnapshotPolicy(Mockito.any(SnapshotPolicyVO.class), Mockito.anyString(), Mockito.anyString(),
+              Mockito.any(DateUtil.IntervalType.class), Mockito.anyInt(), Mockito.anyBoolean(), Mockito.anyBoolean());
+            Mockito.doNothing().when(_snapshotMgr).createTagsForSnapshotPolicy(mapStringStringMock, snapshotPolicyVoMock);
+
+            SnapshotPolicyVO result = _snapshotMgr.persistSnapshotPolicy(volumeMock, TEST_SNAPSHOT_POLICY_SCHEDULE, TEST_SNAPSHOT_POLICY_TIMEZONE, intervalType,
+              TEST_SNAPSHOT_POLICY_MAX_SNAPS, TEST_SNAPSHOT_POLICY_DISPLAY, TEST_SNAPSHOT_POLICY_ACTIVE, null);
+
+            assertSnapshotPolicyResultAgainstPreBuiltInstance(result);
+        }
+
+        VerificationMode timesVerification = Mockito.times(listIntervalTypes.size());
+        Mockito.verify(_snapshotMgr, Mockito.never()).createSnapshotPolicy(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.any(DateUtil.IntervalType.class),
+            Mockito.anyInt(), Mockito.anyBoolean());
+        Mockito.verify(_snapshotMgr, timesVerification).updateSnapshotPolicy(Mockito.any(SnapshotPolicyVO.class), Mockito.anyString(), Mockito.anyString(),
+          Mockito.any(DateUtil.IntervalType.class), Mockito.anyInt(), Mockito.anyBoolean(), Mockito.anyBoolean());
+        Mockito.verify(_snapshotMgr, timesVerification).createTagsForSnapshotPolicy(Mockito.any(), Mockito.any());
+    }
 }

server/src/test/java/com/cloud/tags/TaggedResourceManagerImplTest.java

@@ -0,0 +1,87 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 com.cloud.tags;
+
+import com.cloud.server.ResourceTag;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import junit.framework.TestCase;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class TaggedResourceManagerImplTest extends TestCase{
+
+    @Spy
+    private final TaggedResourceManagerImpl taggedResourceManagerImplSpy = new TaggedResourceManagerImpl();
+
+    private final List<ResourceTag.ResourceObjectType> listResourceObjectTypes = Arrays.asList(ResourceTag.ResourceObjectType.values());
+
+    @Test
+    public void validateGetTagsFromResourceMustReturnValues(){
+        Map<String, String> expectedResult = new HashMap<>();
+        expectedResult.put("test1", "test1");
+        expectedResult.put("test2", "test2");
+
+        listResourceObjectTypes.forEach(resourceObjectType -> {
+            List<ResourceTag> resourceTags = new ArrayList<>();
+            expectedResult.entrySet().forEach(entry -> {
+                resourceTags.add(new ResourceTagVO(entry.getKey(), entry.getValue(), 0, 0, 0, resourceObjectType, "test", "test"));
+            });
+
+            Mockito.doReturn(resourceTags).when(taggedResourceManagerImplSpy).listByResourceTypeAndId(Mockito.eq(resourceObjectType), Mockito.anyLong());
+            Map<String, String> result = taggedResourceManagerImplSpy.getTagsFromResource(resourceObjectType, 0l);
+            Assert.assertEquals(expectedResult, result);
+        });
+    }
+
+    @Test
+    public void validateGetTagsFromResourceMustReturnNull(){
+        Map<String, String> expectedResult = null;
+
+        listResourceObjectTypes.forEach(resourceObjectType -> {
+            List<ResourceTag> resourceTags = null;
+
+            Mockito.doReturn(resourceTags).when(taggedResourceManagerImplSpy).listByResourceTypeAndId(Mockito.eq(resourceObjectType), Mockito.anyLong());
+            Map<String, String> result = taggedResourceManagerImplSpy.getTagsFromResource(resourceObjectType, 0l);
+            Assert.assertEquals(expectedResult, result);
+        });
+    }
+
+    @Test
+    public void validateGetTagsFromResourceMustReturnEmpty(){
+        Map<String, String> expectedResult = new HashMap<>();
+
+        listResourceObjectTypes.forEach(resourceObjectType -> {
+            List<ResourceTag> resourceTags = new ArrayList<>();
+
+            Mockito.doReturn(resourceTags).when(taggedResourceManagerImplSpy).listByResourceTypeAndId(Mockito.eq(resourceObjectType), Mockito.anyLong());
+            Map<String, String> result = taggedResourceManagerImplSpy.getTagsFromResource(resourceObjectType, 0l);
+            Assert.assertEquals(expectedResult, result);
+        });
+    }
+}

utils/pom.xml

@@ -125,6 +125,10 @@
             <artifactId>commons-io</artifactId>
             <scope>provided</scope>
         </dependency>
+        <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-lang3</artifactId>
+        </dependency>
         <dependency>
             <groupId>org.reflections</groupId>
             <artifactId>reflections</artifactId>

utils/src/main/java/org/apache/cloudstack/utils/reflectiontostringbuilderutils/ReflectionToStringBuilderUtils.java

@@ -0,0 +1,201 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you 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.apache.cloudstack.utils.reflectiontostringbuilderutils;
+
+import java.lang.reflect.Field;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.lang3.builder.ReflectionToStringBuilder;
+import org.apache.commons.lang3.builder.ToStringStyle;
+import org.apache.log4j.Logger;
+import org.reflections.ReflectionUtils;
+
+/**
+ * Provides methods that ReflectionToStringBuilder does not have yet, as:
+ * <br><br>
+ * - Reflect a collection of objects, according to object data structure; ReflectionToStringBuilder will execute a toString based on the collection itself,
+ * and not on the objects contained in it.<br>
+ * - Reflect only selected fields (ReflectionToStringBuilder just has methods to exclude fields).
+ */
+public class ReflectionToStringBuilderUtils {
+    protected static final Logger LOGGER = Logger.getLogger(ReflectionToStringBuilderUtils.class);
+    private static final ToStringStyle DEFAULT_STYLE = ToStringStyle.JSON_STYLE;
+
+    /**
+    * Default separator to join objects when the parameter <b>object</b> is a Collection.
+    */
+    private static final String DEFAULT_MULTIPLE_VALUES_SEPARATOR = ",";
+
+    /**
+     * Reflect only selected fields of the object into a JSON formatted String.
+     * @param object Object to be reflected.
+     * @param selectedFields Fields names that must return in JSON.
+     * @return If <b>object</b> is null, returns null.<br>
+     * If <b>selectedFields</b> is null, returns an empty String.<br>
+     * If <b>object</b> is a Collection, returns a JSON array containing the elements, else, returns the object as JSON.<br>
+     */
+    public static String reflectOnlySelectedFields(Object object, String... selectedFields) {
+        String reflection = reflectOnlySelectedFields(object, DEFAULT_STYLE, ",", selectedFields);
+
+        if (reflection == null) {
+            return null;
+        }
+
+        if (isCollection(object)) {
+            return String.format("[%s]", reflection);
+        }
+
+        return reflection;
+    }
+
+    /**
+     * Reflect only selected fields of the object into a formatted String.
+     * @param object Object to be reflected.
+     * @param style Style to format the string.
+     * @param multipleValuesSeparator Separator, in case the object is a Collection.
+     * @param selectedFields Fields names that must be reflected.
+     * @return If <b>object</b> is null, returns null.<br>
+     * If <b>selectedFields</b> is null, returns an empty String.<br>
+     * If <b>object</b> is a Collection, returns a <b>style</b> formatted string containing the elements, else, returns the object as the <b>style</b> parameter.<br>
+     */
+    public static String reflectOnlySelectedFields(Object object, ToStringStyle style, String multipleValuesSeparator, String... selectedFields) {
+        String[] nonSelectedFields = getNonSelectedFields(object, selectedFields);
+        if (nonSelectedFields == null) {
+            return null;
+        }
+
+        if (ArrayUtils.isEmpty(nonSelectedFields)) {
+            return "";
+        }
+
+        String collectionReflection = reflectCollection(object, style, multipleValuesSeparator, nonSelectedFields);
+        return collectionReflection != null ? collectionReflection : getReflectedObject(object, style, nonSelectedFields);
+    }
+
+    /**
+     * Validate if object is not null.
+     * @param object
+     * @return <b>true</b> if it is not null (valid) or <b>false</b> if it is null (invalid).
+     */
+    protected static boolean isValidObject(Object object) {
+        if (object != null) {
+            return true;
+        }
+
+        LOGGER.debug("Object is null, not reflecting it.");
+        return false;
+    }
+
+    /**
+     * Similar to {@link ReflectionToStringBuilderUtils#reflectOnlySelectedFields(Object, ToStringStyle, String, String...)}, but excluding the fields instead of reflecting which
+     * were selected.<br><br>
+     * This method must be called only to {@link Collection}, as it will reflect the objects contained in it.<br>
+     * To reflect the Collection itself or other objects, see {@link ReflectionToStringBuilder}.
+     * @param object Collection to be reflected.
+     * @param style Style to format the string.
+     * @param multipleValuesSeparator Separator when joining the objects.
+     * @param fieldsToExclude Fields names that must not be reflected.
+     * @return If <b>object</b> is null or is not a Collection, returns null.<br>
+     * If <b>selectedFields</b> is null, returns an empty String.<br>
+     * If <b>object</b> is a Collection, returns a <b>style</b> formatted string containing the not null elements, else, returns the object as the <b>style</b> parameter.<br>
+     */
+    public static String reflectCollection(Object object, ToStringStyle style, String multipleValuesSeparator, String... fieldsToExclude){
+        if (!isCollection(object) || !isValidObject(object)) {
+            return null;
+        }
+
+        return String.valueOf(((Collection) object).stream()
+          .filter(obj -> obj != null)
+          .map(obj -> getReflectedObject(obj, style, fieldsToExclude))
+          .collect(Collectors.joining(multipleValuesSeparator == null ? DEFAULT_MULTIPLE_VALUES_SEPARATOR : multipleValuesSeparator)));
+    }
+
+    /**
+     * Verify if object is a Collection.
+     * @param object
+     * @return <b>true</b> if it is a Collection or <b>false</b> if not.
+     */
+    protected static boolean isCollection(Object object) {
+        return object instanceof Collection;
+    }
+
+    /**
+     * Create a new ReflectionToStringBuilder according to parameters.
+     * @param object Object to be reflected.
+     * @param style Style to format the string.
+     * @param fieldsToExclude Fields names to be removed from the reflection.
+     * @return A ReflectionToStringBuilder according to parameters
+     */
+    protected static String getReflectedObject(Object object, ToStringStyle style, String... fieldsToExclude) {
+        return new ReflectionToStringBuilder(object, style).setExcludeFieldNames(fieldsToExclude).toString();
+    }
+
+    /**
+     * Method to retrieve all fields declared in class, except selected fields. If the object is a collection, it will search for any not null object and reflect it.
+     * @param object Object to getReflectionObject.
+     * @param selectedFields Fields names that must no return.
+     * @return Retrieve all fields declared in class, except selected fields. If the object is a collection, it will search for any not null object and reflect it.
+     * @throws SecurityException
+     */
+    protected static String[] getNonSelectedFields(Object object, String... selectedFields) throws SecurityException {
+        if (ArrayUtils.isEmpty(selectedFields)) {
+            return new String[0];
+        }
+
+        Class<?> classToReflect = getObjectClass(object);
+        if (classToReflect == null) {
+             return null;
+        }
+
+        Set<Field> objectFields = ReflectionUtils.getAllFields(classToReflect);
+        List<String> objectFieldsNames = objectFields.stream().map(objectField -> objectField.getName()).collect(Collectors.toList());
+
+        objectFieldsNames.removeAll(Arrays.asList(selectedFields));
+        return objectFieldsNames.toArray(new String[objectFieldsNames.size()]);
+    }
+
+    /**
+     * Get class from object.
+     * @param object
+     * @return If <b>object</b> is not a Collection, returns its class.<br>
+     * if it is a Collection, <b>null</b> if it is an empty Collection or has only null values, else, it will return the class of the Collection data object.
+     */
+    protected static Class<?> getObjectClass(Object object){
+        if (!isValidObject(object)){
+            return null;
+        }
+
+        if (!isCollection(object)) {
+            return object.getClass();
+        }
+
+        Collection objectAsCollection = (Collection)object;
+
+        Optional<Object> anyNotNullObject = objectAsCollection.stream().filter(obj -> obj != null).findAny();
+        if (anyNotNullObject.isEmpty()) {
+            LOGGER.info(String.format("Collection [%s] is empty or has only null values, not reflecting it.", objectAsCollection));
+            return null;
+        }
+
+        return anyNotNullObject.get().getClass();
+    }
+}

utils/src/test/java/org/apache/cloudstack/utils/reflectiontostringbuilderutils/ReflectionToStringBuilderUtilsTest.java

@@ -0,0 +1,332 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.apache.cloudstack.utils.reflectiontostringbuilderutils;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.PriorityQueue;
+import java.util.Set;
+import java.util.stream.Collectors;
+import junit.framework.TestCase;
+import org.apache.commons.lang3.builder.ReflectionToStringBuilder;
+import org.apache.commons.lang3.builder.ToStringStyle;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mockito;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PowerMockIgnore;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+import org.reflections.ReflectionUtils;
+
+@RunWith(PowerMockRunner.class)
+@PowerMockIgnore({"org.w3c.*", "javax.xml.*", "org.xml.*"})
+@PrepareForTest(ReflectionToStringBuilderUtils.class)
+public class ReflectionToStringBuilderUtilsTest extends TestCase {
+
+    private static final Set<ToStringStyle> TO_STRING_STYLES = new HashSet<>(Arrays.asList(ToStringStyle.DEFAULT_STYLE, ToStringStyle.JSON_STYLE, ToStringStyle.MULTI_LINE_STYLE,
+          ToStringStyle.NO_CLASS_NAME_STYLE, ToStringStyle.NO_FIELD_NAMES_STYLE, ToStringStyle.SHORT_PREFIX_STYLE, ToStringStyle.SIMPLE_STYLE));
+
+    private Class<?> classToReflect;
+    private List<String> classToReflectFieldsNamesList;
+    private String classToReflectRemovedField;
+    private String[] classToReflectFieldsNamesArray;
+
+    @Before
+    public void setup(){
+        classToReflect = String.class;
+        classToReflectFieldsNamesList = ReflectionUtils.getAllFields(classToReflect).stream().map(objectField -> objectField.getName()).collect(Collectors.toList());
+        classToReflectRemovedField = classToReflectFieldsNamesList.remove(0);
+        classToReflectFieldsNamesArray = classToReflectFieldsNamesList.toArray(new String[classToReflectFieldsNamesList.size()]);
+    }
+
+    @Test
+    public void validateIsValidObjectNullObjectMustReturnFalse(){
+        boolean result = ReflectionToStringBuilderUtils.isValidObject(null);
+        Assert.assertFalse(result);
+    }
+
+    @Test
+    public void validateIsValidObjectNotNullObjectMustReturnTrue(){
+        boolean result = ReflectionToStringBuilderUtils.isValidObject(new Object());
+        Assert.assertTrue(result);
+    }
+
+    @Test
+    public void validateIsCollectionObjectIsNotACollectionMustReturnFalse(){
+        boolean result = ReflectionToStringBuilderUtils.isCollection(new Object());
+        Assert.assertFalse(result);
+    }
+
+    @Test
+    public void validateIsCollectionObjectIsACollectionMustReturnTrue(){
+        boolean resultSet = ReflectionToStringBuilderUtils.isCollection(new HashSet<>());
+        boolean resultList = ReflectionToStringBuilderUtils.isCollection(new ArrayList<>());
+        boolean resultQueue = ReflectionToStringBuilderUtils.isCollection(new PriorityQueue<>());
+
+        Assert.assertTrue(resultSet);
+        Assert.assertTrue(resultList);
+        Assert.assertTrue(resultQueue);
+    }
+
+    @Test
+    public void validateGetObjectClassInvalidObjectMustReturnNull(){
+        PowerMockito.spy(ReflectionToStringBuilderUtils.class);
+        PowerMockito.when(ReflectionToStringBuilderUtils.isValidObject(Mockito.any())).thenReturn(false);
+
+        Class<?> result = ReflectionToStringBuilderUtils.getObjectClass("test");
+
+        Assert.assertNull(result);
+    }
+
+    @Test
+    public void validateGetObjectClassObjectIsNotACollectionMustReturnObjectClass(){
+        Class<?> expectedResult = classToReflect;
+
+        PowerMockito.spy(ReflectionToStringBuilderUtils.class);
+        PowerMockito.when(ReflectionToStringBuilderUtils.isValidObject(Mockito.any())).thenReturn(true);
+        PowerMockito.when(ReflectionToStringBuilderUtils.isCollection(Mockito.any())).thenReturn(false);
+
+        Class<?> result = ReflectionToStringBuilderUtils.getObjectClass("test");
+
+        Assert.assertEquals(expectedResult, result);
+    }
+
+    @Test
+    public void validateGetObjectClassObjectIsAnEmptyCollectionMustReturnNull(){
+        PowerMockito.spy(ReflectionToStringBuilderUtils.class);
+        PowerMockito.when(ReflectionToStringBuilderUtils.isValidObject(Mockito.any())).thenReturn(true);
+        PowerMockito.when(ReflectionToStringBuilderUtils.isCollection(Mockito.any())).thenReturn(true);
+
+        Class<?> result = ReflectionToStringBuilderUtils.getObjectClass(new ArrayList<String>());
+
+        Assert.assertNull(result);
+    }
+
+    @Test
+    public void validateGetObjectClassObjectIsACollectionWithOnlyNullValuesMustReturnNull(){
+        PowerMockito.spy(ReflectionToStringBuilderUtils.class);
+        PowerMockito.when(ReflectionToStringBuilderUtils.isValidObject(Mockito.any())).thenReturn(true);
+        PowerMockito.when(ReflectionToStringBuilderUtils.isCollection(Mockito.any())).thenReturn(true);
+
+        Class<?> result = ReflectionToStringBuilderUtils.getObjectClass(new ArrayList<String>(Arrays.asList(null, null)));
+
+        Assert.assertNull(result);
+    }
+
+    @Test
+    public void validateGetObjectClassObjectIsACollectionWithAtLeastOneObjectsMustReturnObjectClass(){
+        Class<?> expectedResult = classToReflect;
+
+        PowerMockito.spy(ReflectionToStringBuilderUtils.class);
+        PowerMockito.when(ReflectionToStringBuilderUtils.isValidObject(Mockito.any())).thenReturn(true);
+        PowerMockito.when(ReflectionToStringBuilderUtils.isCollection(Mockito.any())).thenReturn(true);
+
+        Class<?> result = ReflectionToStringBuilderUtils.getObjectClass(new ArrayList<>(Arrays.asList(null, "test1")));
+
+        Assert.assertEquals(expectedResult, result);
+    }
+
+    @Test
+    public void validateGetNonSelectedFieldsEmptyOrNullSelectedFieldsMustReturnEmptyArray(){
+        String[] expectedResult = new String[0];
+        String[] resultEmpty = ReflectionToStringBuilderUtils.getNonSelectedFields(new String(), new String[0]);
+        String[] resultNull = ReflectionToStringBuilderUtils.getNonSelectedFields(new String(), (String[]) null);
+
+        Assert.assertArrayEquals(expectedResult, resultEmpty);
+        Assert.assertArrayEquals(expectedResult, resultNull);
+    }
+
+    @Test
+    public void validateGetNonSelectedFieldsNullObjectClassMustReturnNull(){
+        PowerMockito.spy(ReflectionToStringBuilderUtils.class);
+        PowerMockito.when(ReflectionToStringBuilderUtils.getObjectClass(Mockito.any())).thenReturn(null);
+
+        String[] result = ReflectionToStringBuilderUtils.getNonSelectedFields(null, "test1", "test2");
+
+        Assert.assertNull(result);
+    }
+
+    @Test
+    public void validateGetNonSelectedFieldsObjectIsNotACollectionAndValidSelectedFieldsMustReturnNonSelectedFields(){
+        String fieldToRemove = classToReflectRemovedField;
+        String[] expectedResult = classToReflectFieldsNamesArray;
+
+        String[] result = ReflectionToStringBuilderUtils.getNonSelectedFields("test", fieldToRemove);
+        Assert.assertArrayEquals(expectedResult, result);
+    }
+
+    @Test
+    public void validateGetNonSelectedFieldsObjectIsACollectionAndValidSelectedFieldsMustReturnNonSelectedFields(){
+        String fieldToRemove = classToReflectRemovedField;
+        String[] expectedResult = classToReflectFieldsNamesArray;
+
+        String[] result = ReflectionToStringBuilderUtils.getNonSelectedFields(Arrays.asList("test1", "test2"), fieldToRemove);
+        Assert.assertArrayEquals(expectedResult, result);
+    }
+
+    @Test
+    public void validateGetReflectedObject(){
+        String fieldToRemove = classToReflectRemovedField;
+
+        TO_STRING_STYLES.forEach(style -> {
+            String objectToReflect = "test";
+            String expectedResult = new ReflectionToStringBuilder(objectToReflect, style).setExcludeFieldNames(fieldToRemove).toString();
+            String result = ReflectionToStringBuilderUtils.getReflectedObject(objectToReflect, style, fieldToRemove);
+            Assert.assertEquals(expectedResult, result);
+        });
+    }
+
+    @Test
+    public void validateReflectCollectionInvalidObjectNorACollectionMustReturnNull() throws Exception{
+        String fieldToRemove = classToReflectRemovedField;
+        TO_STRING_STYLES.forEach(style -> {
+            String resultNull = ReflectionToStringBuilderUtils.reflectCollection(null, style, "-", fieldToRemove);
+            String resultNotACollection = ReflectionToStringBuilderUtils.reflectCollection(new Object(), style, "-", fieldToRemove);
+
+            Assert.assertNull(resultNull);
+            Assert.assertNull(resultNotACollection);
+        });
+    }
+
+    @Test
+    public void validateReflectCollectionWithOnlyNullValuesMustReturnEmptyString() throws Exception{
+        String fieldToRemove = classToReflectRemovedField;
+        Set<String> objectToReflect = new HashSet<>(Arrays.asList(null, null));
+
+        TO_STRING_STYLES.forEach(style -> {
+            String expectedResult = "";
+            String result = ReflectionToStringBuilderUtils.reflectCollection(objectToReflect, style, "-", fieldToRemove);
+
+            Assert.assertEquals(expectedResult, result);
+        });
+    }
+
+    @Test
+    public void validateReflectCollectionValuesMustReturnReflection() throws Exception{
+        String fieldToRemove = classToReflectRemovedField;
+        Set<String> objectToReflect = new HashSet<>(Arrays.asList(null, "test1", null, "test2"));
+
+        TO_STRING_STYLES.forEach(style -> {
+            String expectedResult = String.valueOf(objectToReflect
+              .stream()
+              .filter(obj -> obj != null)
+              .map(obj -> ReflectionToStringBuilderUtils.getReflectedObject(obj, style, "-", fieldToRemove))
+              .collect(Collectors.joining("-")));
+
+            String result = ReflectionToStringBuilderUtils.reflectCollection(objectToReflect, style, "-", fieldToRemove);
+
+            Assert.assertEquals(expectedResult, result);
+        });
+    }
+
+    @Test
+    public void validateReflectOnlySelectedFieldsNullNonSelectedFieldsMustReturnNull() throws Exception{
+        PowerMockito.spy(ReflectionToStringBuilderUtils.class);
+        PowerMockito.when(ReflectionToStringBuilderUtils.getNonSelectedFields(Mockito.any(), Mockito.any())).thenReturn(null);
+
+        TO_STRING_STYLES.forEach(style -> {
+            String result = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(null, style, "-");
+            Assert.assertNull(result);
+        });
+    }
+
+    @Test
+    public void validateReflectOnlySelectedFieldsEmptyNonSelectedFieldsMustReturnEmptyString() throws Exception{
+        String expectedResult = "";
+
+        PowerMockito.spy(ReflectionToStringBuilderUtils.class);
+        PowerMockito.when(ReflectionToStringBuilderUtils.getNonSelectedFields(Mockito.any(), Mockito.any())).thenReturn(new String[0]);
+
+        TO_STRING_STYLES.forEach(style -> {
+            String result = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(null, style, "-");
+            Assert.assertEquals(expectedResult, result);
+        });
+    }
+
+    @Test
+    public void validateReflectOnlySelectedFieldsObjectIsACollectionMustReflectCollection() throws Exception{
+        String fieldToRemove = classToReflectRemovedField;
+        String expectedResult = "test";
+
+        PowerMockito.spy(ReflectionToStringBuilderUtils.class);
+        PowerMockito.when(ReflectionToStringBuilderUtils.getNonSelectedFields(Mockito.any(), Mockito.any())).thenReturn(classToReflectFieldsNamesArray);
+        PowerMockito.when(ReflectionToStringBuilderUtils.reflectCollection(Mockito.any(), Mockito.any(), Mockito.anyString(), Mockito.any())).thenReturn(expectedResult);
+
+        TO_STRING_STYLES.forEach(style -> {
+            String result = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(new Object(), style, "-", fieldToRemove);
+            Assert.assertEquals(expectedResult, result);
+        });
+    }
+
+    @Test
+    public void validateReflectOnlySelectedFieldsObjectIsNotACollectionMustReflectObject() throws Exception{
+        String expectedResult = "test";
+
+        PowerMockito.spy(ReflectionToStringBuilderUtils.class);
+        PowerMockito.when(ReflectionToStringBuilderUtils.getNonSelectedFields(Mockito.any(), Mockito.any())).thenReturn(classToReflectFieldsNamesArray);
+        PowerMockito.when(ReflectionToStringBuilderUtils.reflectCollection(Mockito.any(), Mockito.any(), Mockito.anyString(), Mockito.any())).thenReturn(null);
+
+        for (ToStringStyle style : TO_STRING_STYLES){
+            PowerMockito.doReturn(expectedResult).when(ReflectionToStringBuilderUtils.class, "getReflectedObject", Mockito.any(), Mockito.any(), Mockito.any());
+            String result = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(expectedResult, style, "-", classToReflectFieldsNamesArray);
+            Assert.assertEquals(expectedResult, result);
+        }
+    }
+
+    @Test
+    public void validateReflectOnlySelectedFieldsDefaultStyleReflectionNullMustReturnNull(){
+        String expectedResult = null;
+
+        PowerMockito.spy(ReflectionToStringBuilderUtils.class);
+        PowerMockito.when(ReflectionToStringBuilderUtils.reflectOnlySelectedFields(Mockito.any(), Mockito.any(), Mockito.anyString(), Mockito.any())).thenReturn(null);
+
+        String result = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(new Object(), (String[]) null);
+        Assert.assertEquals(expectedResult, result);
+    }
+
+    @Test
+    public void validateReflectOnlySelectedFieldsDefaultStyleReflectCollectionMustReturnValue(){
+        String expectedResult = "[test]";
+
+        PowerMockito.spy(ReflectionToStringBuilderUtils.class);
+        PowerMockito.when(ReflectionToStringBuilderUtils.reflectOnlySelectedFields(Mockito.any(), Mockito.any(), Mockito.anyString(), Mockito.any())).thenReturn("test");
+        PowerMockito.when(ReflectionToStringBuilderUtils.isCollection(Mockito.any())).thenReturn(true);
+
+        String result = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(new Object());
+        Assert.assertEquals(expectedResult, result);
+    }
+
+    @Test
+    public void validateReflectOnlySelectedFieldsDefaultStyleReflectMustReturnValue(){
+        String expectedResult = "test";
+
+        PowerMockito.spy(ReflectionToStringBuilderUtils.class);
+        PowerMockito.when(ReflectionToStringBuilderUtils.reflectOnlySelectedFields(Mockito.any(), Mockito.any(), Mockito.anyString(), Mockito.any())).thenReturn(expectedResult);
+        PowerMockito.when(ReflectionToStringBuilderUtils.isCollection(Mockito.any())).thenReturn(false);
+
+        String result = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(new Object());
+        Assert.assertEquals(expectedResult, result);
+    }
+}