Merge branch '4.20' of https://github.com/apache/cloudstack into ghi11438-errorprone-fixes

.github/workflows/merge-conflict-checker.yml

@@ -18,8 +18,8 @@
 name: "PR Merge Conflict Check"
 on:
   push:
-  pull_request_target:
-    types: [synchronize]
+  pull_request:
+    types: [opened, synchronize, reopened]
 
 permissions:  # added using https://github.com/step-security/secure-workflows
   contents: read

api/src/main/java/com/cloud/host/Host.java

@@ -59,6 +59,9 @@ public interface Host extends StateObject<Status>, Identity, Partition, HAResour
     String HOST_INSTANCE_CONVERSION = "host.instance.conversion";
     String HOST_OVFTOOL_VERSION = "host.ovftool.version";
     String HOST_VIRTV2V_VERSION = "host.virtv2v.version";
+    String HOST_SSH_PORT = "host.ssh.port";
+
+    int DEFAULT_SSH_PORT = 22;
 
     /**
      * @return name of the machine.

api/src/main/java/org/apache/cloudstack/api/ApiServerService.java

@@ -48,4 +48,6 @@ public interface ApiServerService {
     boolean forgotPassword(UserAccount userAccount, Domain domain);
 
     boolean resetPassword(UserAccount userAccount, String token, String password);
+
+    String getDomainId(Map<String, Object[]> params);
 }

api/src/main/java/org/apache/cloudstack/api/BaseUpdateTemplateOrIsoCmd.java

@@ -145,8 +145,8 @@ public abstract class BaseUpdateTemplateOrIsoCmd extends BaseCmd {
         return (Map) (paramsCollection.toArray())[0];
     }
 
-    public boolean isCleanupDetails(){
-        return cleanupDetails == null ? false : cleanupDetails.booleanValue();
+    public boolean isCleanupDetails() {
+        return cleanupDetails != null && cleanupDetails;
     }
 
     public CPU.CPUArch getCPUArch() {

api/src/main/java/org/apache/cloudstack/api/command/admin/host/AddHostCmd.java

@@ -60,7 +60,8 @@ public class AddHostCmd extends BaseCmd {
     @Parameter(name = ApiConstants.POD_ID, type = CommandType.UUID, entityType = PodResponse.class, required = true, description = "The Pod ID for the host")
     private Long podId;
 
-    @Parameter(name = ApiConstants.URL, type = CommandType.STRING, required = true, description = "The host URL")
+    @Parameter(name = ApiConstants.URL, type = CommandType.STRING, required = true, description = "The host URL, optionally add ssh port (format: 'host:port') for KVM hosts," +
+            " otherwise falls back to the port defined at the config 'kvm.host.discovery.ssh.port'")
     private String url;
 
     @Parameter(name = ApiConstants.ZONE_ID, type = CommandType.UUID, entityType = ZoneResponse.class, required = true, description = "The Zone ID for the host")

api/src/main/java/org/apache/cloudstack/backup/BackupManager.java

@@ -26,6 +26,7 @@ import org.apache.cloudstack.api.command.user.backup.DeleteBackupScheduleCmd;
 import org.apache.cloudstack.api.command.user.backup.ListBackupOfferingsCmd;
 import org.apache.cloudstack.api.command.user.backup.ListBackupsCmd;
 import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.ValidatedConfigKey;
 import org.apache.cloudstack.framework.config.Configurable;
 
 import com.cloud.utils.Pair;
@@ -42,10 +43,11 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer
             "false",
             "Is backup and recovery framework enabled.", false, ConfigKey.Scope.Zone);
 
-    ConfigKey<String> BackupProviderPlugin = new ConfigKey<>("Advanced", String.class,
+    ConfigKey<String> BackupProviderPlugin = new ValidatedConfigKey<>("Advanced", String.class,
             "backup.framework.provider.plugin",
             "dummy",
-            "The backup and recovery provider plugin.", true, ConfigKey.Scope.Zone, BackupFrameworkEnabled.key());
+            "The backup and recovery provider plugin. Valid plugin values: dummy, veeam, networker and nas",
+            true, ConfigKey.Scope.Zone, BackupFrameworkEnabled.key(), value -> validateBackupProviderConfig((String)value));
 
     ConfigKey<Long> BackupSyncPollingInterval = new ConfigKey<>("Advanced", Long.class,
             "backup.framework.sync.interval",
@@ -148,4 +150,14 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer
     boolean deleteBackup(final Long backupId, final Boolean forced);
 
     BackupOffering updateBackupOffering(UpdateBackupOfferingCmd updateBackupOfferingCmd);
+
+    static void validateBackupProviderConfig(String value) {
+        if (value != null && (value.contains(",") || value.trim().contains(" "))) {
+            throw new IllegalArgumentException("Multiple backup provider plugins are not supported. Please provide a single plugin value.");
+        }
+        List<String> validPlugins = List.of("dummy", "veeam", "networker", "nas");
+        if (value != null && !validPlugins.contains(value)) {
+            throw new IllegalArgumentException("Invalid backup provider plugin: " + value + ". Valid plugin values are: " + String.join(", ", validPlugins));
+        }
+    }
 }

core/src/main/java/com/cloud/agent/api/ModifyStoragePoolAnswer.java

@@ -46,6 +46,10 @@ public class ModifyStoragePoolAnswer extends Answer {
         templateInfo = tInfo;
     }
 
+    public ModifyStoragePoolAnswer(final Command command, final boolean success, final String details) {
+        super(command, success, details);
+    }
+
     public void setPoolInfo(StoragePoolInfo poolInfo) {
         this.poolInfo = poolInfo;
     }

core/src/main/java/org/apache/cloudstack/backup/RestoreBackupCommand.java

@@ -31,9 +31,9 @@ public class RestoreBackupCommand extends Command  {
     private String backupRepoType;
     private String backupRepoAddress;
     private List<String> volumePaths;
+    private List<String> backupFiles;
     private String diskType;
     private Boolean vmExists;
-    private String restoreVolumeUUID;
     private VirtualMachine.State vmState;
 
     protected RestoreBackupCommand() {
@@ -80,6 +80,14 @@ public class RestoreBackupCommand extends Command  {
         this.volumePaths = volumePaths;
     }
 
+    public List<String> getBackupFiles() {
+        return backupFiles;
+    }
+
+    public void setBackupFiles(List<String> backupFiles) {
+        this.backupFiles = backupFiles;
+    }
+
     public Boolean isVmExists() {
         return vmExists;
     }
@@ -104,14 +112,6 @@ public class RestoreBackupCommand extends Command  {
         this.mountOptions = mountOptions;
     }
 
-    public String getRestoreVolumeUUID() {
-        return restoreVolumeUUID;
-    }
-
-    public void setRestoreVolumeUUID(String restoreVolumeUUID) {
-        this.restoreVolumeUUID = restoreVolumeUUID;
-    }
-
     public VirtualMachine.State getVmState() {
         return vmState;
     }

engine/components-api/src/main/java/com/cloud/agent/AgentManager.java

@@ -54,6 +54,10 @@ public interface AgentManager {
             "This timeout overrides the wait global config. This holds a comma separated key value pairs containing timeout (in seconds) for specific commands. " +
                     "For example: DhcpEntryCommand=600, SavePasswordCommand=300, VmDataCommand=300", false);
 
+    ConfigKey<Integer> KVMHostDiscoverySshPort = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Integer.class,
+            "kvm.host.discovery.ssh.port", String.valueOf(Host.DEFAULT_SSH_PORT), "SSH port used for KVM host discovery and any other operations on host (using SSH)." +
+                " Please note that this is applicable when port is not defined through host url while adding the KVM host.", true, ConfigKey.Scope.Cluster);
+
     enum TapAgentsAction {
         Add, Del, Contains,
     }
@@ -170,4 +174,6 @@ public interface AgentManager {
     void notifyMonitorsOfRemovedHost(long hostId, long clusterId);
 
     void propagateChangeToAgents(Map<String, String> params);
+
+    int getHostSshPort(HostVO host);
 }

engine/components-api/src/main/java/com/cloud/resource/ResourceManager.java

@@ -154,6 +154,8 @@ public interface ResourceManager extends ResourceService, Configurable {
 
     public HostVO findHostByGuid(String guid);
 
+    HostVO findHostByGuidPrefix(String guid);
+
     public HostVO findHostByName(String name);
 
     HostStats getHostStatistics(Host host);

engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java

@@ -40,6 +40,7 @@ import java.util.stream.Collectors;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.utils.StringUtils;
 import org.apache.cloudstack.agent.lb.IndirectAgentLB;
 import org.apache.cloudstack.ca.CAManager;
 import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
@@ -55,7 +56,6 @@ import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToSt
 import org.apache.commons.collections.MapUtils;
 import org.apache.commons.lang3.BooleanUtils;
 import org.apache.commons.lang3.ObjectUtils;
-import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.ThreadContext;
 
 import com.cloud.agent.AgentManager;
@@ -1977,7 +1977,7 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl
         return new ConfigKey<?>[] { CheckTxnBeforeSending, Workers, Port, Wait, AlertWait, DirectAgentLoadSize,
                 DirectAgentPoolSize, DirectAgentThreadCap, EnableKVMAutoEnableDisable, ReadyCommandWait,
                 GranularWaitTimeForCommands, RemoteAgentSslHandshakeTimeout, RemoteAgentMaxConcurrentNewConnections,
-                RemoteAgentNewConnectionsMonitorInterval };
+                RemoteAgentNewConnectionsMonitorInterval, KVMHostDiscoverySshPort };
     }
 
     protected class SetHostParamsListener implements Listener {
@@ -2093,6 +2093,25 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl
         }
     }
 
+    @Override
+    public int getHostSshPort(HostVO host) {
+        if (host == null) {
+            return KVMHostDiscoverySshPort.value();
+        }
+
+        if (host.getHypervisorType() != HypervisorType.KVM) {
+            return Host.DEFAULT_SSH_PORT;
+        }
+
+        _hostDao.loadDetails(host);
+        String hostPort = host.getDetail(Host.HOST_SSH_PORT);
+        if (StringUtils.isBlank(hostPort)) {
+            return KVMHostDiscoverySshPort.valueIn(host.getClusterId());
+        }
+
+        return Integer.parseInt(hostPort);
+    }
+
     private GlobalLock getHostJoinLock(Long hostId) {
         return GlobalLock.getInternLock(String.format("%s-%s", "Host-Join", hostId));
     }

engine/orchestration/src/test/java/com/cloud/agent/manager/AgentManagerImplTest.java

@@ -22,9 +22,11 @@ import com.cloud.agent.api.ReadyCommand;
 import com.cloud.agent.api.StartupCommand;
 import com.cloud.agent.api.StartupRoutingCommand;
 import com.cloud.exception.ConnectionException;
+import com.cloud.host.Host;
 import com.cloud.host.HostVO;
 import com.cloud.host.Status;
 import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor;
 import com.cloud.utils.Pair;
 import org.junit.Assert;
 import org.junit.Before;
@@ -103,4 +105,36 @@ public class AgentManagerImplTest {
 
         Assert.assertEquals(50, result);
     }
+
+    @Test
+    public void testGetHostSshPortWithHostNull() {
+        int hostSshPort = mgr.getHostSshPort(null);
+        Assert.assertEquals(22, hostSshPort);
+    }
+
+    @Test
+    public void testGetHostSshPortWithNonKVMHost() {
+        HostVO host = Mockito.mock(HostVO.class);
+        Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.XenServer);
+        int hostSshPort = mgr.getHostSshPort(host);
+        Assert.assertEquals(22, hostSshPort);
+    }
+
+    @Test
+    public void testGetHostSshPortWithKVMHostDefaultPort() {
+        HostVO host = Mockito.mock(HostVO.class);
+        Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM);
+        Mockito.when(host.getClusterId()).thenReturn(1L);
+        int hostSshPort = mgr.getHostSshPort(host);
+        Assert.assertEquals(22, hostSshPort);
+    }
+
+    @Test
+    public void testGetHostSshPortWithKVMHostCustomPort() {
+        HostVO host = Mockito.mock(HostVO.class);
+        Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM);
+        Mockito.when(host.getDetail(Host.HOST_SSH_PORT)).thenReturn(String.valueOf(3922));
+        int hostSshPort = mgr.getHostSshPort(host);
+        Assert.assertEquals(3922, hostSshPort);
+    }
 }

engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java

@@ -163,7 +163,7 @@ public class ClusterDetailsDaoImpl extends ResourceDetailsDaoBase<ClusterDetails
     @Override
     public String getConfigValue(long id, String key) {
         ClusterDetailsVO vo = findDetail(id, key);
-        return vo == null ? null : vo.getValue();
+        return vo == null ? null : getActualValue(vo);
     }
 
     @Override

engine/schema/src/main/java/com/cloud/dc/dao/DataCenterDetailsDaoImpl.java

@@ -46,7 +46,7 @@ public class DataCenterDetailsDaoImpl extends ResourceDetailsDaoBase<DataCenterD
     @Override
     public String getConfigValue(long id, String key) {
         ResourceDetail vo = findDetail(id, key);
-        return vo == null ? null : vo.getValue();
+        return vo == null ? null : getActualValue(vo);
     }
 
     @Override

engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java

@@ -1348,7 +1348,7 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, Long> implements HostDao
         SearchCriteria<HostVO> sc = TypeClusterStatusSearch.create();
         sc.setParameters("type", Host.Type.Routing);
         sc.setParameters("cluster", clusterId);
-        List<HostVO> list = listBy(sc, new Filter(1));
+        List<HostVO> list = listBy(sc, new Filter(1, true));
         return list.isEmpty() ? null : list.get(0);
     }
 

engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java

@@ -459,7 +459,7 @@ public class VMTemplateDaoImpl extends GenericDaoBase<VMTemplateVO, Long> implem
         if (detailsStr == null) {
             return;
         }
-        List<VMTemplateDetailVO> details = new ArrayList<VMTemplateDetailVO>();
+        List<VMTemplateDetailVO> details = new ArrayList<>();
         for (String key : detailsStr.keySet()) {
             VMTemplateDetailVO detail = new VMTemplateDetailVO(tmpl.getId(), key, detailsStr.get(key), true);
             details.add(detail);
@@ -481,7 +481,7 @@ public class VMTemplateDaoImpl extends GenericDaoBase<VMTemplateVO, Long> implem
             }
 
             if (tmplt.getDetails() != null) {
-                List<VMTemplateDetailVO> details = new ArrayList<VMTemplateDetailVO>();
+                List<VMTemplateDetailVO> details = new ArrayList<>();
                 for (String key : tmplt.getDetails().keySet()) {
                     details.add(new VMTemplateDetailVO(tmplt.getId(), key, tmplt.getDetails().get(key), true));
                 }

engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java

@@ -382,7 +382,7 @@ public class VolumeDaoImpl extends GenericDaoBase<VolumeVO, Long> implements Vol
 
     public VolumeDaoImpl() {
         AllFieldsSearch = createSearchBuilder();
-        AllFieldsSearch.and("state", AllFieldsSearch.entity().getState(), Op.EQ);
+        AllFieldsSearch.and("state", AllFieldsSearch.entity().getState(), Op.IN);
         AllFieldsSearch.and("accountId", AllFieldsSearch.entity().getAccountId(), Op.EQ);
         AllFieldsSearch.and("dcId", AllFieldsSearch.entity().getDataCenterId(), Op.EQ);
         AllFieldsSearch.and("pod", AllFieldsSearch.entity().getPodId(), Op.EQ);
@@ -579,17 +579,16 @@ public class VolumeDaoImpl extends GenericDaoBase<VolumeVO, Long> implements Vol
 
     @Override
     public List<VolumeVO> listVolumesToBeDestroyed() {
-        SearchCriteria<VolumeVO> sc = AllFieldsSearch.create();
-        sc.setParameters("state", Volume.State.Destroy);
-
-        return listBy(sc);
+        return listVolumesToBeDestroyed(null);
     }
 
     @Override
     public List<VolumeVO> listVolumesToBeDestroyed(Date date) {
         SearchCriteria<VolumeVO> sc = AllFieldsSearch.create();
-        sc.setParameters("state", Volume.State.Destroy);
-        sc.setParameters("updateTime", date);
+        sc.setParameters("state", Volume.State.Destroy, Volume.State.Expunging);
+        if (date != null) {
+            sc.setParameters("updateTime", date);
+        }
 
         return listBy(sc);
     }

engine/schema/src/main/java/com/cloud/user/UserVO.java

@@ -123,8 +123,8 @@ public class UserVO implements User, Identity, InternalIdentity {
     }
 
     public UserVO(long id) {
+        this();
         this.id = id;
-        this.uuid = UUID.randomUUID().toString();
     }
 
     public UserVO(long accountId, String username, String password, String firstName, String lastName, String email, String timezone, String uuid, Source source) {

engine/schema/src/main/java/com/cloud/user/dao/AccountDao.java

@@ -21,13 +21,11 @@ import java.util.List;
 
 import com.cloud.user.Account;
 import com.cloud.user.AccountVO;
-import com.cloud.user.User;
 import com.cloud.utils.Pair;
 import com.cloud.utils.db.Filter;
 import com.cloud.utils.db.GenericDao;
 
 public interface AccountDao extends GenericDao<AccountVO, Long> {
-    Pair<User, Account> findUserAccountByApiKey(String apiKey);
 
     List<AccountVO> findAccountsLike(String accountName);
 

engine/schema/src/main/java/com/cloud/user/dao/AccountDaoImpl.java

@@ -16,8 +16,6 @@
 // under the License.
 package com.cloud.user.dao;
 
-import java.sql.PreparedStatement;
-import java.sql.ResultSet;
 import java.util.Date;
 import java.util.List;
 
@@ -27,10 +25,7 @@ import org.springframework.stereotype.Component;
 import com.cloud.user.Account;
 import com.cloud.user.Account.State;
 import com.cloud.user.AccountVO;
-import com.cloud.user.User;
-import com.cloud.user.UserVO;
 import com.cloud.utils.Pair;
-import com.cloud.utils.crypt.DBEncryptionUtil;
 import com.cloud.utils.db.Filter;
 import com.cloud.utils.db.GenericDaoBase;
 import com.cloud.utils.db.GenericSearchBuilder;
@@ -38,13 +33,9 @@ import com.cloud.utils.db.SearchBuilder;
 import com.cloud.utils.db.SearchCriteria;
 import com.cloud.utils.db.SearchCriteria.Func;
 import com.cloud.utils.db.SearchCriteria.Op;
-import com.cloud.utils.db.TransactionLegacy;
 
 @Component
 public class AccountDaoImpl extends GenericDaoBase<AccountVO, Long> implements AccountDao {
-    private static final String FIND_USER_ACCOUNT_BY_API_KEY = "SELECT u.id, u.username, u.account_id, u.secret_key, u.state, u.api_key_access, "
-        + "a.id, a.account_name, a.type, a.role_id, a.domain_id, a.state, a.api_key_access " + "FROM `cloud`.`user` u, `cloud`.`account` a "
-        + "WHERE u.account_id = a.id AND u.api_key = ? and u.removed IS NULL";
 
     protected final SearchBuilder<AccountVO> AllFieldsSearch;
     protected final SearchBuilder<AccountVO> AccountTypeSearch;
@@ -132,51 +123,6 @@ public class AccountDaoImpl extends GenericDaoBase<AccountVO, Long> implements A
         return listBy(sc);
     }
 
-    @Override
-    public Pair<User, Account> findUserAccountByApiKey(String apiKey) {
-        TransactionLegacy txn = TransactionLegacy.currentTxn();
-        PreparedStatement pstmt = null;
-        Pair<User, Account> userAcctPair = null;
-        try {
-            String sql = FIND_USER_ACCOUNT_BY_API_KEY;
-            pstmt = txn.prepareAutoCloseStatement(sql);
-            pstmt.setString(1, apiKey);
-            ResultSet rs = pstmt.executeQuery();
-            // TODO:  make sure we don't have more than 1 result?  ApiKey had better be unique
-            if (rs.next()) {
-                User u = new UserVO(rs.getLong(1));
-                u.setUsername(rs.getString(2));
-                u.setAccountId(rs.getLong(3));
-                u.setSecretKey(DBEncryptionUtil.decrypt(rs.getString(4)));
-                u.setState(State.getValueOf(rs.getString(5)));
-                boolean apiKeyAccess = rs.getBoolean(6);
-                if (rs.wasNull()) {
-                    u.setApiKeyAccess(null);
-                } else {
-                    u.setApiKeyAccess(apiKeyAccess);
-                }
-
-                AccountVO a = new AccountVO(rs.getLong(7));
-                a.setAccountName(rs.getString(8));
-                a.setType(Account.Type.getFromValue(rs.getInt(9)));
-                a.setRoleId(rs.getLong(10));
-                a.setDomainId(rs.getLong(11));
-                a.setState(State.getValueOf(rs.getString(12)));
-                apiKeyAccess = rs.getBoolean(13);
-                if (rs.wasNull()) {
-                    a.setApiKeyAccess(null);
-                } else {
-                    a.setApiKeyAccess(apiKeyAccess);
-                }
-
-                userAcctPair = new Pair<User, Account>(u, a);
-            }
-        } catch (Exception e) {
-            logger.warn("Exception finding user/acct by api key: " + apiKey, e);
-        }
-        return userAcctPair;
-    }
-
     @Override
     public List<AccountVO> findAccountsLike(String accountName) {
         return findAccountsLike(accountName, null).first();
@@ -341,11 +287,9 @@ public class AccountDaoImpl extends GenericDaoBase<AccountVO, Long> implements A
             domain_id = account_vo.getDomainId();
         }
         catch (Exception e) {
-            logger.warn("getDomainIdForGivenAccountId: Exception :" + e.getMessage());
-        }
-        finally {
-            return domain_id;
+            logger.warn("Can not get DomainId for the given AccountId; exception message : {}", e.getMessage());
         }
+        return domain_id;
     }
 
     @Override

engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java

@@ -202,7 +202,7 @@ public class ImageStoreDaoImpl extends GenericDaoBase<ImageStoreVO, Long> implem
         sc.setParameters("dataCenterId", dataCenterId);
         sc.setParameters("protocol", protocol);
         sc.setParameters("role", DataStoreRole.Image);
-        Filter filter = new Filter(1);
+        Filter filter = new Filter(1, true);
         List<ImageStoreVO> results =  listBy(sc, filter);
         return results.size() == 0 ? null : results.get(0);
     }

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

@@ -434,6 +434,9 @@ public class VolumeServiceImpl implements VolumeService {
             // no need to change state in volumes table
             volume.processEventOnly(Event.DestroyRequested);
         } else if (volume.getDataStore().getRole() == DataStoreRole.Primary) {
+            if (vol.getState() == Volume.State.Expunging) {
+                logger.info("Volume {} is already in Expunging, retrying", volume);
+            }
             volume.processEvent(Event.ExpungeRequested);
         }
 

framework/config/src/main/java/org/apache/cloudstack/framework/config/ValidatedConfigKey.java

@@ -14,27 +14,25 @@
 // KIND, either express or implied.  See the License for the
 // specific language governing permissions and limitations
 // under the License.
+package org.apache.cloudstack.framework.config;
 
-package org.apache.cloudstack.quota.activationrule.presetvariables;
+import java.util.function.Consumer;
 
-import org.apache.commons.lang3.builder.ToStringBuilder;
-import org.apache.commons.lang3.builder.ToStringStyle;
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
+public class ValidatedConfigKey<T> extends ConfigKey<T> {
+    private final Consumer<T> validator;
 
-@RunWith(MockitoJUnitRunner.class)
-public class ResourceTest {
-
-    @Test
-    public void toStringTestReturnAJson() {
-        Resource variable = new Resource();
-
-        String expected = ToStringBuilder.reflectionToString(variable, ToStringStyle.JSON_STYLE);
-        String result = variable.toString();
-
-        Assert.assertEquals(expected, result);
+    public ValidatedConfigKey(String category, Class<T> type, String name, String defaultValue, String description, boolean dynamic, Scope scope, String parent, Consumer<T> validator) {
+        super(category, type, name, defaultValue, description, dynamic, scope, parent);
+        this.validator = validator;
     }
 
+    public Consumer<T> getValidator() {
+        return validator;
+    }
+
+    public void validateValue(String value) {
+        if (validator != null) {
+            validator.accept((T) value);
+        }
+    }
 }

framework/db/src/main/java/com/cloud/utils/db/Filter.java

@@ -57,7 +57,18 @@ public class Filter {
     }
 
     public Filter(long limit) {
-        _orderBy = " ORDER BY RAND()";
+        this(limit, false);
+    }
+
+    /**
+     * Constructor for creating a filter with random ordering
+     * @param limit the maximum number of results to return
+     * @param randomize if true, orders results randomly
+     */
+    public Filter(long limit, boolean randomize) {
+        if (randomize) {
+            _orderBy = " ORDER BY RAND()" ;
+        }
         _limit = limit;
     }
 

framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java

@@ -347,7 +347,7 @@ public abstract class GenericDaoBase<T, ID extends Serializable> extends Compone
     @Override
     @DB()
     public T lockOneRandomRow(final SearchCriteria<T> sc, final boolean exclusive) {
-        final Filter filter = new Filter(1);
+        final Filter filter = new Filter(1, true);
         final List<T> beans = search(sc, filter, exclusive, true);
         return beans.isEmpty() ? null : beans.get(0);
     }
@@ -927,7 +927,7 @@ public abstract class GenericDaoBase<T, ID extends Serializable> extends Compone
 
     @DB()
     protected T findOneIncludingRemovedBy(final SearchCriteria<T> sc) {
-        Filter filter = new Filter(1);
+        Filter filter = new Filter(1, true);
         List<T> results = searchIncludingRemoved(sc, filter, null, false);
         assert results.size() <= 1 : "Didn't the limiting worked?";
         return results.size() == 0 ? null : results.get(0);
@@ -1324,7 +1324,7 @@ public abstract class GenericDaoBase<T, ID extends Serializable> extends Compone
         Filter filter = null;
         final long batchSizeFinal = ObjectUtils.defaultIfNull(batchSize, 0L);
         if (batchSizeFinal > 0) {
-            filter = new Filter(null, batchSizeFinal);
+            filter = new Filter(batchSizeFinal);
         }
         int expunged = 0;
         int currentExpunged = 0;

framework/db/src/test/java/com/cloud/utils/db/FilterTest.java

@@ -41,4 +41,62 @@ public class FilterTest {
         Assert.assertTrue(filter.getOrderBy().split(",").length == 3);
         Assert.assertTrue(filter.getOrderBy().split(",")[2].trim().toLowerCase().equals("test.fld_int asc"));
     }
+
+
+    @Test
+    public void testFilterWithLimitOnly() {
+        Filter filter = new Filter(5);
+
+        Assert.assertEquals(Long.valueOf(5), filter.getLimit());
+        Assert.assertNull(filter.getOrderBy());
+        Assert.assertNull(filter.getOffset());
+    }
+
+    @Test
+    public void testFilterWithLimitAndRandomizeFalse() {
+        Filter filter = new Filter(10, false);
+
+        Assert.assertEquals(Long.valueOf(10), filter.getLimit());
+        Assert.assertNull(filter.getOrderBy());
+        Assert.assertNull(filter.getOffset());
+    }
+
+    @Test
+    public void testFilterWithLimitAndRandomizeTrue() {
+        Filter filter = new Filter(3, true);
+
+        Assert.assertNull(filter.getLimit());
+        Assert.assertNotNull(filter.getOrderBy());
+        Assert.assertTrue(filter.getOrderBy().contains("ORDER BY RAND()"));
+        Assert.assertTrue(filter.getOrderBy().contains("LIMIT 3"));
+        Assert.assertEquals(" ORDER BY RAND() LIMIT 3", filter.getOrderBy());
+    }
+
+    @Test
+    public void testFilterRandomizeWithDifferentLimits() {
+        Filter filter1 = new Filter(1, true);
+        Filter filter10 = new Filter(10, true);
+        Filter filter100 = new Filter(100, true);
+
+        Assert.assertEquals(" ORDER BY RAND() LIMIT 1", filter1.getOrderBy());
+        Assert.assertEquals(" ORDER BY RAND() LIMIT 10", filter10.getOrderBy());
+        Assert.assertEquals(" ORDER BY RAND() LIMIT 100", filter100.getOrderBy());
+    }
+
+    @Test
+    public void testFilterConstructorBackwardsCompatibility() {
+        // Test that Filter(long) behaves differently now (no ORDER BY RAND())
+        // compared to Filter(long, true) which preserves old behavior
+        Filter simpleLimitFilter = new Filter(1);
+        Filter randomFilter = new Filter(1, true);
+
+        // Simple limit filter should just set limit
+        Assert.assertEquals(Long.valueOf(1), simpleLimitFilter.getLimit());
+        Assert.assertNull(simpleLimitFilter.getOrderBy());
+
+        // Random filter should set orderBy with RAND()
+        Assert.assertNull(randomFilter.getLimit());
+        Assert.assertNotNull(randomFilter.getOrderBy());
+        Assert.assertTrue(randomFilter.getOrderBy().contains("RAND()"));
+    }
 }

framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java

@@ -20,6 +20,7 @@ import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.List;
 
 import org.junit.Assert;
 import org.junit.Before;
@@ -263,4 +264,71 @@ public class GenericDaoBaseTest {
                 " INNER JOIN tableA tableA2Alias ON tableC.column3=tableA2Alias.column2 " +
                 " INNER JOIN tableA tableA3Alias ON tableD.column4=tableA3Alias.column3 AND tableD.column5=? ", joinString.toString());
     }
+
+
+    @Test
+    public void testLockOneRandomRowUsesRandomFilter() {
+        // Create a mock DAO to test lockOneRandomRow behavior
+        GenericDaoBase<DbTestVO, Long> testDao = Mockito.mock(GenericDaoBase.class);
+
+        // Capture the filter passed to the search method
+        final Filter[] capturedFilter = new Filter[1];
+
+        Mockito.when(testDao.lockOneRandomRow(Mockito.any(SearchCriteria.class), Mockito.anyBoolean()))
+                .thenCallRealMethod();
+
+        Mockito.when(testDao.search(Mockito.any(SearchCriteria.class), Mockito.any(Filter.class),
+                        Mockito.anyBoolean(), Mockito.anyBoolean()))
+                .thenAnswer(invocation -> {
+                    capturedFilter[0] = invocation.getArgument(1);
+                    return new ArrayList<DbTestVO>();
+                });
+
+        SearchCriteria<DbTestVO> sc = Mockito.mock(SearchCriteria.class);
+        testDao.lockOneRandomRow(sc, true);
+
+        // Verify that the filter uses random ordering
+        Assert.assertNotNull(capturedFilter[0]);
+        Assert.assertNotNull(capturedFilter[0].getOrderBy());
+        Assert.assertTrue(capturedFilter[0].getOrderBy().contains("ORDER BY RAND()"));
+        Assert.assertTrue(capturedFilter[0].getOrderBy().contains("LIMIT 1"));
+    }
+
+    @Test
+    public void testLockOneRandomRowReturnsNullOnEmptyResult() {
+        GenericDaoBase<DbTestVO, Long> testDao = Mockito.mock(GenericDaoBase.class);
+
+        Mockito.when(testDao.lockOneRandomRow(Mockito.any(SearchCriteria.class), Mockito.anyBoolean()))
+                .thenCallRealMethod();
+
+        Mockito.when(testDao.search(Mockito.any(SearchCriteria.class), Mockito.any(Filter.class),
+                        Mockito.anyBoolean(), Mockito.anyBoolean()))
+                .thenReturn(new ArrayList<DbTestVO>());
+
+        SearchCriteria<DbTestVO> sc = Mockito.mock(SearchCriteria.class);
+        DbTestVO result = testDao.lockOneRandomRow(sc, true);
+
+        Assert.assertNull(result);
+    }
+
+    @Test
+    public void testLockOneRandomRowReturnsFirstElement() {
+        GenericDaoBase<DbTestVO, Long> testDao = Mockito.mock(GenericDaoBase.class);
+        DbTestVO expectedResult = new DbTestVO();
+        List<DbTestVO> resultList = new ArrayList<>();
+        resultList.add(expectedResult);
+
+        Mockito.when(testDao.lockOneRandomRow(Mockito.any(SearchCriteria.class), Mockito.anyBoolean()))
+                .thenCallRealMethod();
+
+        Mockito.when(testDao.search(Mockito.any(SearchCriteria.class), Mockito.any(Filter.class),
+                        Mockito.anyBoolean(), Mockito.anyBoolean()))
+                .thenReturn(resultList);
+
+        SearchCriteria<DbTestVO> sc = Mockito.mock(SearchCriteria.class);
+        DbTestVO result = testDao.lockOneRandomRow(sc, true);
+
+        Assert.assertNotNull(result);
+        Assert.assertEquals(expectedResult, result);
+    }
 }

framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java

@@ -428,7 +428,7 @@ public class QuotaManagerImpl extends ManagerBase implements QuotaManager {
         }
 
         injectPresetVariablesIntoJsInterpreter(jsInterpreter, presetVariables);
-        jsInterpreter.injectVariable("lastTariffs", lastAppliedTariffsList.toString());
+        jsInterpreter.injectVariable("lastTariffs", lastAppliedTariffsList);
 
         String scriptResult = jsInterpreter.executeScript(activationRule).toString();
 
@@ -458,18 +458,18 @@ public class QuotaManagerImpl extends ManagerBase implements QuotaManager {
     protected void injectPresetVariablesIntoJsInterpreter(JsInterpreter jsInterpreter, PresetVariables presetVariables) {
         jsInterpreter.discardCurrentVariables();
 
-        jsInterpreter.injectVariable("account", presetVariables.getAccount().toString());
-        jsInterpreter.injectVariable("domain", presetVariables.getDomain().toString());
+        jsInterpreter.injectVariable("account", presetVariables.getAccount());
+        jsInterpreter.injectVariable("domain", presetVariables.getDomain());
 
         GenericPresetVariable project = presetVariables.getProject();
         if (project != null) {
-            jsInterpreter.injectVariable("project", project.toString());
+            jsInterpreter.injectVariable("project", project);
 
         }
 
         jsInterpreter.injectVariable("resourceType", presetVariables.getResourceType());
-        jsInterpreter.injectVariable("value", presetVariables.getValue().toString());
-        jsInterpreter.injectVariable("zone", presetVariables.getZone().toString());
+        jsInterpreter.injectVariable("value", presetVariables.getValue());
+        jsInterpreter.injectVariable("zone", presetVariables.getZone());
     }
 
     /**

framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Account.java

@@ -28,7 +28,6 @@ public class Account extends GenericPresetVariable {
 
     public void setRole(Role role) {
         this.role = role;
-        fieldNamesToIncludeInToString.add("role");
     }
 
 }

framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/BackupOffering.java

@@ -29,6 +29,5 @@ public class BackupOffering extends GenericPresetVariable {
 
     public void setExternalId(String externalId) {
         this.externalId = externalId;
-        fieldNamesToIncludeInToString.add("externalId");
     }
 }

framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/ComputeOffering.java

@@ -27,7 +27,6 @@ public class ComputeOffering extends GenericPresetVariable {
 
     public void setCustomized(boolean customized) {
         this.customized = customized;
-        fieldNamesToIncludeInToString.add("customized");
     }
 
 }

framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Domain.java

@@ -27,7 +27,6 @@ public class Domain extends GenericPresetVariable {
 
     public void setPath(String path) {
         this.path = path;
-        fieldNamesToIncludeInToString.add("path");
     }
 
 }

framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/GenericPresetVariable.java

@@ -17,10 +17,8 @@
 
 package org.apache.cloudstack.quota.activationrule.presetvariables;
 
-import java.util.HashSet;
-import java.util.Set;
-
-import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
+import org.apache.commons.lang3.builder.ToStringBuilder;
+import org.apache.commons.lang3.builder.ToStringStyle;
 
 public class GenericPresetVariable {
     @PresetVariableDefinition(description = "ID of the resource.")
@@ -29,15 +27,12 @@ public class GenericPresetVariable {
     @PresetVariableDefinition(description = "Name of the resource.")
     private String name;
 
-    protected transient Set<String> fieldNamesToIncludeInToString = new HashSet<>();
-
     public String getId() {
         return id;
     }
 
     public void setId(String id) {
         this.id = id;
-        fieldNamesToIncludeInToString.add("id");
     }
 
     public String getName() {
@@ -46,15 +41,10 @@ public class GenericPresetVariable {
 
     public void setName(String name) {
         this.name = name;
-        fieldNamesToIncludeInToString.add("name");
     }
 
-    /***
-     * Converts the preset variable into a valid JSON object that will be injected into the JS interpreter.
-     * This method should not be overridden or changed.
-     */
     @Override
-    public final String toString() {
-        return ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, fieldNamesToIncludeInToString.toArray(new String[0]));
+    public String toString() {
+        return ToStringBuilder.reflectionToString(this, ToStringStyle.JSON_STYLE);
     }
 }

framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Host.java

@@ -32,7 +32,6 @@ public class Host extends GenericPresetVariable {
 
     public void setTags(List<String> tags) {
         this.tags = tags;
-        fieldNamesToIncludeInToString.add("tags");
     }
 
     public Boolean getIsTagARule() {
@@ -41,6 +40,5 @@ public class Host extends GenericPresetVariable {
 
     public void setIsTagARule(Boolean isTagARule) {
         this.isTagARule = isTagARule;
-        fieldNamesToIncludeInToString.add("isTagARule");
     }
 }

framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelper.java

@@ -243,7 +243,7 @@ public class PresetVariableHelper {
         Role role = new Role();
         role.setId(roleVo.getUuid());
         role.setName(roleVo.getName());
-        role.setType(roleVo.getRoleType());
+        role.setType(roleVo.getRoleType().toString());
 
         return role;
     }
@@ -490,7 +490,7 @@ public class PresetVariableHelper {
         value.setDiskOffering(getPresetVariableValueDiskOffering(volumeVo.getDiskOfferingId()));
         value.setId(volumeVo.getUuid());
         value.setName(volumeVo.getName());
-        value.setProvisioningType(volumeVo.getProvisioningType());
+        value.setProvisioningType(volumeVo.getProvisioningType().toString());
 
         Long poolId = volumeVo.getPoolId();
         if (poolId == null) {
@@ -533,7 +533,7 @@ public class PresetVariableHelper {
         storage = new Storage();
         storage.setId(storagePoolVo.getUuid());
         storage.setName(storagePoolVo.getName());
-        storage.setScope(storagePoolVo.getScope());
+        storage.setScope(storagePoolVo.getScope().toString());
         List<StoragePoolTagVO> storagePoolTagVOList = storagePoolTagsDao.findStoragePoolTags(storageId);
         List<String> storageTags = new ArrayList<>();
         boolean isTagARule = false;
@@ -602,7 +602,7 @@ public class PresetVariableHelper {
         value.setId(snapshotVo.getUuid());
         value.setName(snapshotVo.getName());
         value.setSize(ByteScaleUtils.bytesToMebibytes(snapshotVo.getSize()));
-        value.setSnapshotType(Snapshot.Type.values()[snapshotVo.getSnapshotType()]);
+        value.setSnapshotType(Snapshot.Type.values()[snapshotVo.getSnapshotType()].toString());
         value.setStorage(getPresetVariableValueStorage(getSnapshotDataStoreId(snapshotId, usageRecord.getZoneId()), usageType));
         value.setTags(getPresetVariableValueResourceTags(snapshotId, ResourceObjectType.Snapshot));
         Hypervisor.HypervisorType hypervisorType = snapshotVo.getHypervisorType();
@@ -671,7 +671,7 @@ public class PresetVariableHelper {
         value.setId(vmSnapshotVo.getUuid());
         value.setName(vmSnapshotVo.getName());
         value.setTags(getPresetVariableValueResourceTags(vmSnapshotId, ResourceObjectType.VMSnapshot));
-        value.setVmSnapshotType(vmSnapshotVo.getType());
+        value.setVmSnapshotType(vmSnapshotVo.getType().toString());
 
         VMInstanceVO vmVo = vmInstanceDao.findByIdIncludingRemoved(vmSnapshotVo.getVmId());
         if (vmVo != null && vmVo.getHypervisorType() != null) {

framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Role.java

@@ -17,19 +17,16 @@
 
 package org.apache.cloudstack.quota.activationrule.presetvariables;
 
-import org.apache.cloudstack.acl.RoleType;
-
 public class Role extends GenericPresetVariable {
     @PresetVariableDefinition(description = "Role type of the resource's owner.")
-    private RoleType type;
+    private String type;
 
-    public RoleType getType() {
+    public String getType() {
         return type;
     }
 
-    public void setType(RoleType type) {
+    public void setType(String type) {
         this.type = type;
-        fieldNamesToIncludeInToString.add("type");
     }
 
 }

framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Storage.java

@@ -19,8 +19,6 @@ package org.apache.cloudstack.quota.activationrule.presetvariables;
 
 import java.util.List;
 
-import com.cloud.storage.ScopeType;
-
 public class Storage extends GenericPresetVariable {
     @PresetVariableDefinition(description = "List of string representing the tags of the storage where the volume is (i.e.: [\"a\", \"b\"]).")
     private List<String> tags;
@@ -29,7 +27,7 @@ public class Storage extends GenericPresetVariable {
     private Boolean isTagARule;
 
     @PresetVariableDefinition(description = "Scope of the storage where the volume is. Values can be: ZONE, CLUSTER or HOST. Applicable only for primary storages.")
-    private ScopeType scope;
+    private String scope;
 
     public List<String> getTags() {
         return tags;
@@ -37,7 +35,6 @@ public class Storage extends GenericPresetVariable {
 
     public void setTags(List<String> tags) {
         this.tags = tags;
-        fieldNamesToIncludeInToString.add("tags");
     }
 
     public Boolean getIsTagARule() {
@@ -46,16 +43,14 @@ public class Storage extends GenericPresetVariable {
 
     public void setIsTagARule(Boolean isTagARule) {
         this.isTagARule = isTagARule;
-        fieldNamesToIncludeInToString.add("isTagARule");
     }
 
-    public ScopeType getScope() {
+    public String getScope() {
         return scope;
     }
 
-    public void setScope(ScopeType scope) {
+    public void setScope(String scope) {
         this.scope = scope;
-        fieldNamesToIncludeInToString.add("scope");
     }
 
 }

framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Tariff.java

@@ -28,6 +28,5 @@ public class Tariff extends GenericPresetVariable {
 
     public void setValue(BigDecimal value) {
         this.value = value;
-        fieldNamesToIncludeInToString.add("value");
     }
 }

framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Value.java

@@ -20,9 +20,6 @@ package org.apache.cloudstack.quota.activationrule.presetvariables;
 import java.util.List;
 import java.util.Map;
 
-import com.cloud.storage.Snapshot;
-import com.cloud.storage.Storage.ProvisioningType;
-import com.cloud.vm.snapshot.VMSnapshot;
 import org.apache.cloudstack.quota.constant.QuotaTypes;
 
 public class Value extends GenericPresetVariable {
@@ -60,13 +57,13 @@ public class Value extends GenericPresetVariable {
     private Long virtualSize;
 
     @PresetVariableDefinition(description = "Provisioning type of the resource. Values can be: thin, sparse or fat.", supportedTypes = {QuotaTypes.VOLUME})
-    private ProvisioningType provisioningType;
+    private String provisioningType;
 
     @PresetVariableDefinition(description = "Type of the snapshot. Values can be: MANUAL, RECURRING, HOURLY, DAILY, WEEKLY and MONTHLY.", supportedTypes = {QuotaTypes.SNAPSHOT})
-    private Snapshot.Type snapshotType;
+    private String snapshotType;
 
     @PresetVariableDefinition(description = "Type of the VM snapshot. Values can be: Disk or DiskAndMemory.", supportedTypes = {QuotaTypes.VM_SNAPSHOT})
-    private VMSnapshot.Type vmSnapshotType;
+    private String vmSnapshotType;
 
     @PresetVariableDefinition(description = "Computing offering of the VM.", supportedTypes = {QuotaTypes.RUNNING_VM, QuotaTypes.ALLOCATED_VM})
     private ComputeOffering computeOffering;
@@ -101,7 +98,6 @@ public class Value extends GenericPresetVariable {
 
     public void setHost(Host host) {
         this.host = host;
-        fieldNamesToIncludeInToString.add("host");
     }
 
     public String getOsName() {
@@ -110,7 +106,6 @@ public class Value extends GenericPresetVariable {
 
     public void setOsName(String osName) {
         this.osName = osName;
-        fieldNamesToIncludeInToString.add("osName");
     }
 
     public List<Resource> getAccountResources() {
@@ -119,7 +114,6 @@ public class Value extends GenericPresetVariable {
 
     public void setAccountResources(List<Resource> accountResources) {
         this.accountResources = accountResources;
-        fieldNamesToIncludeInToString.add("accountResources");
     }
 
     public Map<String, String> getTags() {
@@ -128,7 +122,6 @@ public class Value extends GenericPresetVariable {
 
     public void setTags(Map<String, String> tags) {
         this.tags = tags;
-        fieldNamesToIncludeInToString.add("tags");
     }
 
     public String getTag() {
@@ -137,7 +130,6 @@ public class Value extends GenericPresetVariable {
 
     public void setTag(String tag) {
         this.tag = tag;
-        fieldNamesToIncludeInToString.add("tag");
     }
 
     public Long getSize() {
@@ -146,34 +138,30 @@ public class Value extends GenericPresetVariable {
 
     public void setSize(Long size) {
         this.size = size;
-        fieldNamesToIncludeInToString.add("size");
     }
 
-    public ProvisioningType getProvisioningType() {
+    public String getProvisioningType() {
         return provisioningType;
     }
 
-    public void setProvisioningType(ProvisioningType provisioningType) {
+    public void setProvisioningType(String provisioningType) {
         this.provisioningType = provisioningType;
-        fieldNamesToIncludeInToString.add("provisioningType");
     }
 
-    public Snapshot.Type getSnapshotType() {
+    public String getSnapshotType() {
         return snapshotType;
     }
 
-    public void setSnapshotType(Snapshot.Type snapshotType) {
+    public void setSnapshotType(String snapshotType) {
         this.snapshotType = snapshotType;
-        fieldNamesToIncludeInToString.add("snapshotType");
     }
 
-    public VMSnapshot.Type getVmSnapshotType() {
+    public String getVmSnapshotType() {
         return vmSnapshotType;
     }
 
-    public void setVmSnapshotType(VMSnapshot.Type vmSnapshotType) {
+    public void setVmSnapshotType(String vmSnapshotType) {
         this.vmSnapshotType = vmSnapshotType;
-        fieldNamesToIncludeInToString.add("vmSnapshotType");
     }
 
     public ComputeOffering getComputeOffering() {
@@ -182,7 +170,6 @@ public class Value extends GenericPresetVariable {
 
     public void setComputeOffering(ComputeOffering computeOffering) {
         this.computeOffering = computeOffering;
-        fieldNamesToIncludeInToString.add("computeOffering");
     }
 
     public GenericPresetVariable getTemplate() {
@@ -191,7 +178,6 @@ public class Value extends GenericPresetVariable {
 
     public void setTemplate(GenericPresetVariable template) {
         this.template = template;
-        fieldNamesToIncludeInToString.add("template");
     }
 
     public GenericPresetVariable getDiskOffering() {
@@ -200,7 +186,6 @@ public class Value extends GenericPresetVariable {
 
     public void setDiskOffering(GenericPresetVariable diskOffering) {
         this.diskOffering = diskOffering;
-        fieldNamesToIncludeInToString.add("diskOffering");
     }
 
     public Storage getStorage() {
@@ -209,7 +194,6 @@ public class Value extends GenericPresetVariable {
 
     public void setStorage(Storage storage) {
         this.storage = storage;
-        fieldNamesToIncludeInToString.add("storage");
     }
 
     public ComputingResources getComputingResources() {
@@ -218,7 +202,6 @@ public class Value extends GenericPresetVariable {
 
     public void setComputingResources(ComputingResources computingResources) {
         this.computingResources = computingResources;
-        fieldNamesToIncludeInToString.add("computingResources");
     }
 
     public Long getVirtualSize() {
@@ -227,7 +210,6 @@ public class Value extends GenericPresetVariable {
 
     public void setVirtualSize(Long virtualSize) {
         this.virtualSize = virtualSize;
-        fieldNamesToIncludeInToString.add("virtualSize");
     }
 
     public BackupOffering getBackupOffering() {
@@ -236,12 +218,10 @@ public class Value extends GenericPresetVariable {
 
     public void setBackupOffering(BackupOffering backupOffering) {
         this.backupOffering = backupOffering;
-        fieldNamesToIncludeInToString.add("backupOffering");
     }
 
     public void setHypervisorType(String hypervisorType) {
         this.hypervisorType = hypervisorType;
-        fieldNamesToIncludeInToString.add("hypervisorType");
     }
 
     public String getHypervisorType() {
@@ -250,7 +230,6 @@ public class Value extends GenericPresetVariable {
 
     public void setVolumeFormat(String volumeFormat) {
         this.volumeFormat = volumeFormat;
-        fieldNamesToIncludeInToString.add("volumeFormat");
     }
 
     public String getVolumeFormat() {
@@ -263,6 +242,5 @@ public class Value extends GenericPresetVariable {
 
     public void setState(String state) {
         this.state = state;
-        fieldNamesToIncludeInToString.add("state");
     }
 }

framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java

@@ -267,12 +267,12 @@ public class QuotaManagerImplTest {
 
         quotaManagerImplSpy.injectPresetVariablesIntoJsInterpreter(jsInterpreterMock, presetVariablesMock);
 
-        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("account"), Mockito.anyString());
-        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("domain"), Mockito.anyString());
-        Mockito.verify(jsInterpreterMock, Mockito.never()).injectVariable(Mockito.eq("project"), Mockito.anyString());
-        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("resourceType"), Mockito.anyString());
-        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("value"), Mockito.anyString());
-        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("zone"), Mockito.anyString());
+        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("account"), Mockito.any());
+        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("domain"), Mockito.any());
+        Mockito.verify(jsInterpreterMock, Mockito.never()).injectVariable(Mockito.eq("project"), Mockito.any());
+        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("resourceType"), Mockito.any());
+        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("value"), Mockito.any());
+        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("zone"), Mockito.any());
     }
 
     @Test
@@ -288,12 +288,12 @@ public class QuotaManagerImplTest {
 
         quotaManagerImplSpy.injectPresetVariablesIntoJsInterpreter(jsInterpreterMock, presetVariablesMock);
 
-        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("account"), Mockito.anyString());
-        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("domain"), Mockito.anyString());
-        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("project"), Mockito.anyString());
-        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("resourceType"), Mockito.anyString());
-        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("value"), Mockito.anyString());
-        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("zone"), Mockito.anyString());
+        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("account"), Mockito.any());
+        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("domain"), Mockito.any());
+        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("project"), Mockito.any());
+        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("resourceType"), Mockito.any());
+        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("value"), Mockito.any());
+        Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("zone"), Mockito.any());
     }
 
     @Test

framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/AccountTest.java

@@ -1,34 +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.quota.activationrule.presetvariables;
-
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class AccountTest {
-
-    @Test
-    public void setRoleTestAddFieldRoleToCollection() {
-        Account variable = new Account();
-        variable.setRole(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("role"));
-    }
-}

framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/BackupOfferingTest.java

@@ -1,36 +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.quota.activationrule.presetvariables;
-
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class BackupOfferingTest {
-    @Test
-    public void setExternalIdTestAddFieldExternalIdToCollection() {
-        BackupOffering backupOffering = new BackupOffering();
-        backupOffering.setExternalId("any-external-id");
-        Assert.assertTrue(backupOffering.fieldNamesToIncludeInToString.contains("externalId"));
-    }
-
-}

framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ComputeOfferingTest.java

@@ -1,35 +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.quota.activationrule.presetvariables;
-
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class ComputeOfferingTest {
-
-    @Test
-    public void setCustomizedTestAddFieldCustomizedToCollection() {
-        ComputeOffering variable = new ComputeOffering();
-        variable.setCustomized(true);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("customized"));
-    }
-
-}

framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ComputingResourcesTest.java

@@ -1,40 +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.quota.activationrule.presetvariables;
-
-import org.apache.commons.lang3.builder.ToStringBuilder;
-import org.apache.commons.lang3.builder.ToStringStyle;
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class ComputingResourcesTest {
-
-    @Test
-    public void toStringTestReturnAJson() {
-        ComputingResources variable = new ComputingResources();
-
-        String expected = ToStringBuilder.reflectionToString(variable, ToStringStyle.JSON_STYLE);
-        String result = variable.toString();
-
-        Assert.assertEquals(expected, result);
-    }
-
-}

framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/DomainTest.java

@@ -1,35 +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.quota.activationrule.presetvariables;
-
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class DomainTest {
-
-    @Test
-    public void setPathTestAddFieldPathToCollection() {
-        Domain variable = new Domain();
-        variable.setPath("test path");
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("path"));
-    }
-
-}

framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/GenericPresetVariableTest.java

@@ -1,73 +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.quota.activationrule.presetvariables;
-
-import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class GenericPresetVariableTest {
-
-    @Test
-    public void setIdTestAddFieldIdToCollection() {
-        GenericPresetVariable variable = new GenericPresetVariable();
-        variable.setId("test");
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("id"));
-    }
-
-    @Test
-    public void setNameTestAddFieldNameToCollection() {
-        GenericPresetVariable variable = new GenericPresetVariable();
-        variable.setName("test");
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("name"));
-    }
-
-    @Test
-    public void toStringTestSetAllFieldsAndReturnAJson() {
-        GenericPresetVariable variable = new GenericPresetVariable();
-        variable.setId("test id");
-        variable.setName("test name");
-
-        String expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "id", "name");
-        String result = variable.toString();
-
-        Assert.assertEquals(expected, result);
-    }
-
-    @Test
-    public void toStringTestSetSomeFieldsAndReturnAJson() {
-        GenericPresetVariable variable = new GenericPresetVariable();
-        variable.setId("test id");
-
-        String expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "id");
-        String result = variable.toString();
-
-        Assert.assertEquals(expected, result);
-
-        variable = new GenericPresetVariable();
-        variable.setName("test name");
-
-        expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "name");
-        result = variable.toString();
-
-        Assert.assertEquals(expected, result);
-    }
-}

framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/HostTest.java

@@ -1,34 +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.quota.activationrule.presetvariables;
-
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class HostTest {
-
-    @Test
-    public void setTagsTestAddFieldTagsToCollection() {
-        Host variable = new Host();
-        variable.setTags(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("tags"));
-    }
-}

framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelperTest.java

@@ -209,12 +209,12 @@ public class PresetVariableHelperTest {
         value.setTags(Collections.singletonMap("tag1", "value1"));
         value.setTemplate(getGenericPresetVariableForTests());
         value.setDiskOffering(getGenericPresetVariableForTests());
-        value.setProvisioningType(ProvisioningType.THIN);
+        value.setProvisioningType(ProvisioningType.THIN.toString());
         value.setStorage(getStorageForTests());
         value.setSize(ByteScaleUtils.GiB);
-        value.setSnapshotType(Snapshot.Type.HOURLY);
+        value.setSnapshotType(Snapshot.Type.HOURLY.toString());
         value.setTag("tag_test");
-        value.setVmSnapshotType(VMSnapshot.Type.Disk);
+        value.setVmSnapshotType(VMSnapshot.Type.Disk.toString());
         value.setComputingResources(getComputingResourcesForTests());
         return value;
     }
@@ -256,7 +256,7 @@ public class PresetVariableHelperTest {
         storage.setId("storage_id");
         storage.setName("storage_name");
         storage.setTags(Arrays.asList("tag1", "tag2"));
-        storage.setScope(ScopeType.ZONE);
+        storage.setScope(ScopeType.ZONE.toString());
         return storage;
     }
 
@@ -293,13 +293,6 @@ public class PresetVariableHelperTest {
         Assert.assertEquals(expected.getName(), result.getName());
     }
 
-    private void validateFieldNamesToIncludeInToString(List<String> expected, GenericPresetVariable resultObject) {
-        List<String> result = new ArrayList<>(resultObject.fieldNamesToIncludeInToString);
-        Collections.sort(expected);
-        Collections.sort(result);
-        Assert.assertEquals(expected, result);
-    }
-
     private BackupOffering getBackupOfferingForTests() {
         BackupOffering backupOffering = new BackupOffering();
         backupOffering.setId("backup_offering_id");
@@ -362,7 +355,6 @@ public class PresetVariableHelperTest {
 
         Assert.assertNotNull(result.getProject());
         assertPresetVariableIdAndName(account, result.getProject());
-        validateFieldNamesToIncludeInToString(Arrays.asList("id", "name"), result.getProject());
     }
 
     @Test
@@ -379,7 +371,6 @@ public class PresetVariableHelperTest {
         Account result = presetVariableHelperSpy.getPresetVariableAccount(1l);
 
         assertPresetVariableIdAndName(account, result);
-        validateFieldNamesToIncludeInToString(Arrays.asList("id", "name"), result);
     }
 
     @Test
@@ -409,18 +400,16 @@ public class PresetVariableHelperTest {
             Role role = new Role();
             role.setId("test_id");
             role.setName("test_name");
-            role.setType(roleType);
+            role.setType(roleType.toString());
 
             Mockito.doReturn(role.getId()).when(roleVoMock).getUuid();
             Mockito.doReturn(role.getName()).when(roleVoMock).getName();
-            Mockito.doReturn(role.getType()).when(roleVoMock).getRoleType();
+            Mockito.doReturn(RoleType.fromString(role.getType())).when(roleVoMock).getRoleType();
 
             Role result = presetVariableHelperSpy.getPresetVariableRole(1l);
 
             assertPresetVariableIdAndName(role, result);
             Assert.assertEquals(role.getType(), result.getType());
-
-            validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "type"), result);
         });
     }
 
@@ -439,8 +428,6 @@ public class PresetVariableHelperTest {
 
         assertPresetVariableIdAndName(domain, result);
         Assert.assertEquals(domain.getPath(), result.getPath());
-
-        validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "path"), result);
     }
 
     @Test
@@ -456,7 +443,6 @@ public class PresetVariableHelperTest {
         GenericPresetVariable result = presetVariableHelperSpy.getPresetVariableZone(1l);
 
         assertPresetVariableIdAndName(expected, result);
-        validateFieldNamesToIncludeInToString(Arrays.asList("id", "name"), result);
     }
 
     @Test
@@ -477,7 +463,6 @@ public class PresetVariableHelperTest {
         Value result = presetVariableHelperSpy.getPresetVariableValue(usageVoMock);
 
         Assert.assertEquals(resources, result.getAccountResources());
-        validateFieldNamesToIncludeInToString(Arrays.asList("accountResources"), result);
     }
 
     @Test
@@ -536,8 +521,6 @@ public class PresetVariableHelperTest {
                 Assert.assertEquals(expected.getTags(), result.getTags());
                 Assert.assertEquals(expected.getTemplate(), result.getTemplate());
                 Assert.assertEquals(hypervisorType.name(), result.getHypervisorType());
-
-                validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "osName", "tags", "template", "hypervisorType"), result);
             });
         }
 
@@ -569,7 +552,6 @@ public class PresetVariableHelperTest {
 
         assertPresetVariableIdAndName(expectedHost, result.getHost());
         Assert.assertEquals(expectedHost.getTags(), result.getHost().getTags());
-        validateFieldNamesToIncludeInToString(Arrays.asList("host"), result);
     }
 
     @Test
@@ -588,7 +570,6 @@ public class PresetVariableHelperTest {
 
         assertPresetVariableIdAndName(expected, result);
         Assert.assertEquals(expected.getTags(), result.getTags());
-        validateFieldNamesToIncludeInToString(Arrays.asList("id", "isTagARule", "name", "tags"), result);
     }
 
     @Test
@@ -608,7 +589,6 @@ public class PresetVariableHelperTest {
         assertPresetVariableIdAndName(expected, result);
         Assert.assertEquals(new ArrayList<>(), result.getTags());
         Assert.assertTrue(result.getIsTagARule());
-        validateFieldNamesToIncludeInToString(Arrays.asList("id", "isTagARule", "name", "tags"), result);
     }
 
     @Test
@@ -636,7 +616,6 @@ public class PresetVariableHelperTest {
 
         assertPresetVariableIdAndName(expected, result);
         Assert.assertEquals(expected.isCustomized(), result.isCustomized());
-        validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "customized"), result);
     }
 
     @Test
@@ -652,7 +631,6 @@ public class PresetVariableHelperTest {
         GenericPresetVariable result = presetVariableHelperSpy.getPresetVariableValueTemplate(1l);
 
         assertPresetVariableIdAndName(expected, result);
-        validateFieldNamesToIncludeInToString(Arrays.asList("id", "name"), result);
     }
 
     @Test
@@ -696,7 +674,7 @@ public class PresetVariableHelperTest {
             Mockito.doReturn(expected.getId()).when(volumeVoMock).getUuid();
             Mockito.doReturn(expected.getName()).when(volumeVoMock).getName();
             Mockito.doReturn(expected.getDiskOffering()).when(presetVariableHelperSpy).getPresetVariableValueDiskOffering(Mockito.anyLong());
-            Mockito.doReturn(expected.getProvisioningType()).when(volumeVoMock).getProvisioningType();
+            Mockito.doReturn(ProvisioningType.getProvisioningType(expected.getProvisioningType())).when(volumeVoMock).getProvisioningType();
             Mockito.doReturn(expected.getStorage()).when(presetVariableHelperSpy).getPresetVariableValueStorage(Mockito.anyLong(), Mockito.anyInt());
             Mockito.doReturn(expected.getTags()).when(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.any(ResourceObjectType.class));
             Mockito.doReturn(expected.getSize()).when(volumeVoMock).getSize();
@@ -716,8 +694,6 @@ public class PresetVariableHelperTest {
             Assert.assertEquals(expected.getTags(), result.getTags());
             Assert.assertEquals(expectedSize, result.getSize());
             Assert.assertEquals(imageFormat.name(), result.getVolumeFormat());
-
-            validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "diskOffering", "provisioningType", "storage", "tags", "size", "volumeFormat"), result);
         }
 
         Mockito.verify(presetVariableHelperSpy, Mockito.times(ImageFormat.values().length)).getPresetVariableValueResourceTags(Mockito.anyLong(),
@@ -738,7 +714,7 @@ public class PresetVariableHelperTest {
             Mockito.doReturn(expected.getId()).when(volumeVoMock).getUuid();
             Mockito.doReturn(expected.getName()).when(volumeVoMock).getName();
             Mockito.doReturn(expected.getDiskOffering()).when(presetVariableHelperSpy).getPresetVariableValueDiskOffering(Mockito.anyLong());
-            Mockito.doReturn(expected.getProvisioningType()).when(volumeVoMock).getProvisioningType();
+            Mockito.doReturn(ProvisioningType.getProvisioningType(expected.getProvisioningType())).when(volumeVoMock).getProvisioningType();
             Mockito.doReturn(expected.getTags()).when(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.any(ResourceObjectType.class));
             Mockito.doReturn(expected.getSize()).when(volumeVoMock).getSize();
             Mockito.doReturn(imageFormat).when(volumeVoMock).getFormat();
@@ -757,8 +733,6 @@ public class PresetVariableHelperTest {
             Assert.assertEquals(expected.getTags(), result.getTags());
             Assert.assertEquals(expectedSize, result.getSize());
             Assert.assertEquals(imageFormat.name(), result.getVolumeFormat());
-
-            validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "diskOffering", "provisioningType", "tags", "size", "volumeFormat"), result);
         }
 
         Mockito.verify(presetVariableHelperSpy, Mockito.times(ImageFormat.values().length)).getPresetVariableValueResourceTags(Mockito.anyLong(),
@@ -778,7 +752,6 @@ public class PresetVariableHelperTest {
         GenericPresetVariable result = presetVariableHelperSpy.getPresetVariableValueDiskOffering(1l);
 
         assertPresetVariableIdAndName(expected, result);
-        validateFieldNamesToIncludeInToString(Arrays.asList("id", "name"), result);
     }
 
     @Test
@@ -804,7 +777,7 @@ public class PresetVariableHelperTest {
 
         Mockito.doReturn(expected.getId()).when(storagePoolVoMock).getUuid();
         Mockito.doReturn(expected.getName()).when(storagePoolVoMock).getName();
-        Mockito.doReturn(expected.getScope()).when(storagePoolVoMock).getScope();
+        Mockito.doReturn(ScopeType.validateAndGetScopeType(expected.getScope())).when(storagePoolVoMock).getScope();
         Mockito.doReturn(storageTagVOListMock).when(storagePoolTagsDaoMock).findStoragePoolTags(Mockito.anyLong());
 
         Storage result = presetVariableHelperSpy.getPresetVariableValueStorage(1l, 2);
@@ -812,8 +785,6 @@ public class PresetVariableHelperTest {
         assertPresetVariableIdAndName(expected, result);
         Assert.assertEquals(expected.getScope(), result.getScope());
         Assert.assertEquals(expected.getTags(), result.getTags());
-
-        validateFieldNamesToIncludeInToString(Arrays.asList("id", "isTagARule",  "name", "scope", "tags"), result);
     }
 
     @Test
@@ -828,7 +799,7 @@ public class PresetVariableHelperTest {
 
         Mockito.doReturn(expected.getId()).when(storagePoolVoMock).getUuid();
         Mockito.doReturn(expected.getName()).when(storagePoolVoMock).getName();
-        Mockito.doReturn(expected.getScope()).when(storagePoolVoMock).getScope();
+        Mockito.doReturn(ScopeType.validateAndGetScopeType(expected.getScope())).when(storagePoolVoMock).getScope();
         Mockito.doReturn(storageTagVOListMock).when(storagePoolTagsDaoMock).findStoragePoolTags(Mockito.anyLong());
 
         Storage result = presetVariableHelperSpy.getPresetVariableValueStorage(1l, 2);
@@ -837,8 +808,6 @@ public class PresetVariableHelperTest {
         Assert.assertEquals(expected.getScope(), result.getScope());
         Assert.assertEquals(new ArrayList<>(), result.getTags());
         Assert.assertTrue(result.getIsTagARule());
-
-        validateFieldNamesToIncludeInToString(Arrays.asList("id", "isTagARule",  "name", "scope", "tags"), result);
     }
 
     @Test
@@ -874,7 +843,6 @@ public class PresetVariableHelperTest {
         Storage result = presetVariableHelperSpy.getSecondaryStorageForSnapshot(1l, UsageTypes.SNAPSHOT);
 
         assertPresetVariableIdAndName(expected, result);
-        validateFieldNamesToIncludeInToString(Arrays.asList("id", "name"), result);
     }
 
     @Test
@@ -915,8 +883,6 @@ public class PresetVariableHelperTest {
             Assert.assertEquals(expected.getOsName(), result.getOsName());
             Assert.assertEquals(expected.getTags(), result.getTags());
             Assert.assertEquals(expectedSize, result.getSize());
-
-            validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "osName", "tags", "size"), result);
         });
 
         Mockito.verify(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.eq(ResourceObjectType.Template));
@@ -967,8 +933,6 @@ public class PresetVariableHelperTest {
             Assert.assertEquals(expected.getTags(), result.getTags());
             Assert.assertEquals(expectedSize, result.getSize());
             Assert.assertEquals(hypervisorType.name(), result.getHypervisorType());
-
-            validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "snapshotType", "storage", "tags", "size", "hypervisorType"), result);
         }
 
         Mockito.verify(presetVariableHelperSpy, Mockito.times(Hypervisor.HypervisorType.values().length)).getPresetVariableValueResourceTags(Mockito.anyLong(),
@@ -1053,8 +1017,6 @@ public class PresetVariableHelperTest {
 
         assertPresetVariableIdAndName(expected, result);
         Assert.assertEquals(expected.getTag(), result.getTag());
-
-        validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "tag"), result);
     }
 
     @Test
@@ -1079,7 +1041,7 @@ public class PresetVariableHelperTest {
         Mockito.doReturn(expected.getId()).when(vmSnapshotVoMock).getUuid();
         Mockito.doReturn(expected.getName()).when(vmSnapshotVoMock).getName();
         Mockito.doReturn(expected.getTags()).when(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.any(ResourceObjectType.class));
-        Mockito.doReturn(expected.getVmSnapshotType()).when(vmSnapshotVoMock).getType();
+        Mockito.doReturn(VMSnapshot.Type.valueOf(expected.getVmSnapshotType())).when(vmSnapshotVoMock).getType();
 
         Mockito.doReturn(UsageTypes.VM_SNAPSHOT).when(usageVoMock).getUsageType();
 
@@ -1090,8 +1052,6 @@ public class PresetVariableHelperTest {
         Assert.assertEquals(expected.getTags(), result.getTags());
         Assert.assertEquals(expected.getVmSnapshotType(), result.getVmSnapshotType());
 
-        validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "tags", "vmSnapshotType"), result);
-
         Mockito.verify(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.eq(ResourceObjectType.VMSnapshot));
     }
 
@@ -1124,9 +1084,6 @@ public class PresetVariableHelperTest {
 
             if (typeInt == UsageTypes.RUNNING_VM) {
                 Assert.assertEquals(expected.getComputingResources(), result.getComputingResources());
-                validateFieldNamesToIncludeInToString(Arrays.asList("computeOffering", "computingResources"), result);
-            } else {
-                validateFieldNamesToIncludeInToString(Arrays.asList("computeOffering"), result);
             }
         });
     }
@@ -1219,8 +1176,6 @@ public class PresetVariableHelperTest {
         Assert.assertEquals(expected.getVirtualSize(), result.getVirtualSize());
         Assert.assertEquals(expected.getBackupOffering(), result.getBackupOffering());
 
-        validateFieldNamesToIncludeInToString(Arrays.asList("size", "virtualSize", "backupOffering"), result);
-
         Mockito.verify(presetVariableHelperSpy).getPresetVariableValueBackupOffering(Mockito.anyLong());
     }
 
@@ -1239,7 +1194,6 @@ public class PresetVariableHelperTest {
 
         assertPresetVariableIdAndName(expected, result);
         Assert.assertEquals(expected.getExternalId(), result.getExternalId());
-        validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "externalId"), result);
     }
 
     @Test

framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/RoleTest.java

@@ -1,34 +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.quota.activationrule.presetvariables;
-
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class RoleTest {
-
-    @Test
-    public void setTagsTestAddFieldTagsToCollection() {
-        Role variable = new Role();
-        variable.setType(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("type"));
-    }
-}

framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/StorageTest.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.quota.activationrule.presetvariables;
-
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class StorageTest {
-
-    @Test
-    public void setTagsTestAddFieldTagsToCollection() {
-        Storage variable = new Storage();
-        variable.setTags(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("tags"));
-    }
-
-    @Test
-    public void setScopeTestAddFieldScopeToCollection() {
-        Storage variable = new Storage();
-        variable.setScope(null);;
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("scope"));
-    }
-}

framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ValueTest.java

@@ -1,175 +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.quota.activationrule.presetvariables;
-
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class ValueTest {
-
-    @Test
-    public void setIdTestAddFieldIdToCollection() {
-        Value variable = new Value();
-        variable.setId(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("id"));
-    }
-
-    @Test
-    public void setNameTestAddFieldNameToCollection() {
-        Value variable = new Value();
-        variable.setName(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("name"));
-    }
-
-    @Test
-    public void setHostTestAddFieldHostToCollection() {
-        Value variable = new Value();
-        variable.setHost(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("host"));
-    }
-
-    @Test
-    public void setOsNameTestAddFieldOsNameToCollection() {
-        Value variable = new Value();
-        variable.setOsName(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("osName"));
-    }
-
-    @Test
-    public void setAccountResourcesTestAddFieldAccountResourcesToCollection() {
-        Value variable = new Value();
-        variable.setAccountResources(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("accountResources"));
-    }
-
-    @Test
-    public void setTagsTestAddFieldTagsToCollection() {
-        Value variable = new Value();
-        variable.setTags(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("tags"));
-    }
-
-    @Test
-    public void setTagTestAddFieldTagToCollection() {
-        Value variable = new Value();
-        variable.setTag(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("tag"));
-    }
-
-    @Test
-    public void setSizeTestAddFieldSizeToCollection() {
-        Value variable = new Value();
-        variable.setSize(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("size"));
-    }
-
-    @Test
-    public void setProvisioningTypeTestAddFieldProvisioningTypeToCollection() {
-        Value variable = new Value();
-        variable.setProvisioningType(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("provisioningType"));
-    }
-
-    @Test
-    public void setSnapshotTypeTestAddFieldSnapshotTypeToCollection() {
-        Value variable = new Value();
-        variable.setSnapshotType(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("snapshotType"));
-    }
-
-    @Test
-    public void setVmSnapshotTypeTestAddFieldVmSnapshotTypeToCollection() {
-        Value variable = new Value();
-        variable.setVmSnapshotType(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("vmSnapshotType"));
-    }
-
-    @Test
-    public void setComputeOfferingTestAddFieldComputeOfferingToCollection() {
-        Value variable = new Value();
-        variable.setComputeOffering(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("computeOffering"));
-    }
-
-    @Test
-    public void setTemplateTestAddFieldTemplateToCollection() {
-        Value variable = new Value();
-        variable.setTemplate(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("template"));
-    }
-
-    @Test
-    public void setDiskOfferingTestAddFieldDiskOfferingToCollection() {
-        Value variable = new Value();
-        variable.setDiskOffering(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("diskOffering"));
-    }
-
-    @Test
-    public void setStorageTestAddFieldStorageToCollection() {
-        Value variable = new Value();
-        variable.setStorage(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("storage"));
-    }
-
-    @Test
-    public void setComputingResourcesTestAddFieldComputingResourcesToCollection() {
-        Value variable = new Value();
-        variable.setComputingResources(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("computingResources"));
-    }
-
-    @Test
-    public void setVirtualSizeTestAddFieldVirtualSizeToCollection() {
-        Value variable = new Value();
-        variable.setVirtualSize(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("virtualSize"));
-    }
-
-    @Test
-    public void setBackupOfferingTestAddFieldBackupOfferingToCollection() {
-        Value variable = new Value();
-        variable.setBackupOffering(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("backupOffering"));
-    }
-
-    @Test
-    public void setHypervisorTypeTestAddFieldHypervisorTypeToCollection() {
-        Value variable = new Value();
-        variable.setHypervisorType(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("hypervisorType"));
-    }
-
-    @Test
-    public void setVolumeFormatTestAddFieldVolumeFormatToCollection() {
-        Value variable = new Value();
-        variable.setVolumeFormat(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("volumeFormat"));
-    }
-
-    @Test
-    public void setStateTestAddFieldStateToCollection() {
-        Value variable = new Value();
-        variable.setState(null);
-        Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("state"));
-    }
-
-}

plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java

@@ -58,7 +58,6 @@ import java.util.Locale;
 import java.util.Map;
 import java.util.HashMap;
 import java.util.Objects;
-import java.util.Optional;
 import java.util.UUID;
 import java.util.stream.Collectors;
 
@@ -230,6 +229,7 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
         restoreCommand.setMountOptions(backupRepository.getMountOptions());
         restoreCommand.setVmName(vm.getName());
         restoreCommand.setVolumePaths(getVolumePaths(volumes));
+        restoreCommand.setBackupFiles(getBackupFiles(backedVolumes));
         restoreCommand.setVmExists(vm.getRemoved() == null);
         restoreCommand.setVmState(vm.getState());
 
@@ -244,6 +244,14 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
         return answer.getResult();
     }
 
+    private List<String> getBackupFiles(List<Backup.VolumeInfo> backedVolumes) {
+        List<String> backupFiles = new ArrayList<>();
+        for (Backup.VolumeInfo backedVolume : backedVolumes) {
+            backupFiles.add(backedVolume.getPath());
+        }
+        return backupFiles;
+    }
+
     private List<String> getVolumePaths(List<VolumeVO> volumes) {
         List<String> volumePaths = new ArrayList<>();
         for (VolumeVO volume : volumes) {
@@ -271,8 +279,11 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
         final StoragePoolHostVO dataStore = storagePoolHostDao.findByUuid(dataStoreUuid);
         final HostVO hostVO = hostDao.findByIp(hostIp);
 
-        Optional<Backup.VolumeInfo> matchingVolume = getBackedUpVolumeInfo(backupSourceVm.getBackupVolumeList(), volumeUuid);
-        Long backedUpVolumeSize = matchingVolume.isPresent() ? matchingVolume.get().getSize() : 0L;
+        Backup.VolumeInfo matchingVolume = getBackedUpVolumeInfo(backup.getBackedUpVolumes(), volumeUuid);
+        if (matchingVolume == null) {
+            throw new CloudRuntimeException(String.format("Unable to find volume %s in the list of backed up volumes for backup %s, cannot proceed with restore", volumeUuid, backup));
+        }
+        Long backedUpVolumeSize = matchingVolume.getSize();
 
         LOG.debug("Restoring vm volume {} from backup {} on the NAS Backup Provider", volume, backup);
         BackupRepository backupRepository = getBackupRepository(backupSourceVm, backup);
@@ -300,11 +311,11 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
         restoreCommand.setBackupRepoAddress(backupRepository.getAddress());
         restoreCommand.setVmName(vmNameAndState.first());
         restoreCommand.setVolumePaths(Collections.singletonList(String.format("%s/%s", dataStore.getLocalPath(), volumeUUID)));
+        restoreCommand.setBackupFiles(Collections.singletonList(matchingVolume.getPath()));
         restoreCommand.setDiskType(volume.getVolumeType().name().toLowerCase(Locale.ROOT));
         restoreCommand.setMountOptions(backupRepository.getMountOptions());
         restoreCommand.setVmExists(null);
         restoreCommand.setVmState(vmNameAndState.second());
-        restoreCommand.setRestoreVolumeUUID(volumeUuid);
 
         BackupAnswer answer = null;
         try {
@@ -339,10 +350,11 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
         return backupRepository;
     }
 
-    private Optional<Backup.VolumeInfo> getBackedUpVolumeInfo(List<Backup.VolumeInfo> backedUpVolumes, String volumeUuid) {
+    private Backup.VolumeInfo getBackedUpVolumeInfo(List<Backup.VolumeInfo> backedUpVolumes, String volumeUuid) {
         return backedUpVolumes.stream()
                 .filter(v -> v.getUuid().equals(volumeUuid))
-                .findFirst();
+                .findFirst()
+                .orElse(null);
     }
 
     @Override

plugins/backup/networker/src/main/java/org/apache/cloudstack/backup/NetworkerBackupProvider.java

@@ -16,6 +16,7 @@
 // under the License.
 package org.apache.cloudstack.backup;
 
+import com.cloud.agent.AgentManager;
 import com.cloud.dc.dao.ClusterDao;
 import com.cloud.host.HostVO;
 import com.cloud.host.Status;
@@ -117,6 +118,9 @@ public class NetworkerBackupProvider extends AdapterBase implements BackupProvid
     @Inject
     private VMInstanceDao vmInstanceDao;
 
+    @Inject
+    private AgentManager agentMgr;
+
     private static String getUrlDomain(String url) throws URISyntaxException {
         URI uri;
         try {
@@ -229,8 +233,13 @@ public class NetworkerBackupProvider extends AdapterBase implements BackupProvid
         String nstRegex = "\\bcompleted savetime=([0-9]{10})";
         Pattern saveTimePattern = Pattern.compile(nstRegex);
 
+        if (host == null) {
+            LOG.warn("Unable to take backup, host is null");
+            return null;
+        }
+
         try {
-            Pair<Boolean, String> response = SshHelper.sshExecute(host.getPrivateIpAddress(), 22,
+            Pair<Boolean, String> response = SshHelper.sshExecute(host.getPrivateIpAddress(), agentMgr.getHostSshPort(host),
                     username, null, password, command, 120000, 120000, 3600000);
             if (!response.first()) {
                 LOG.error(String.format("Backup Script failed on HYPERVISOR %s due to: %s", host, response.second()));
@@ -249,9 +258,13 @@ public class NetworkerBackupProvider extends AdapterBase implements BackupProvid
         return null;
     }
     private boolean executeRestoreCommand(HostVO host, String username, String password, String command) {
+        if (host == null) {
+            LOG.warn("Unable to restore backup, host is null");
+            return false;
+        }
 
         try {
-            Pair<Boolean, String> response = SshHelper.sshExecute(host.getPrivateIpAddress(), 22,
+            Pair<Boolean, String> response = SshHelper.sshExecute(host.getPrivateIpAddress(), agentMgr.getHostSshPort(host),
                 username, null, password, command, 120000, 120000, 3600000);
 
             if (!response.first()) {

plugins/event-bus/rabbitmq/src/main/java/org/apache/cloudstack/mom/rabbitmq/RabbitMQEventBus.java

@@ -492,7 +492,7 @@ public class RabbitMQEventBus extends ManagerBase implements EventBus {
 
     @Override
     public synchronized boolean stop() {
-        if (s_connection.isOpen()) {
+        if (s_connection != null && s_connection.isOpen()) {
             for (String subscriberId : s_subscribers.keySet()) {
                 Ternary<String, Channel, EventSubscriber> subscriberDetails = s_subscribers.get(subscriberId);
                 Channel channel = subscriberDetails.second();

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java

@@ -31,7 +31,6 @@ import org.apache.cloudstack.backup.BackupAnswer;
 import org.apache.cloudstack.backup.RestoreBackupCommand;
 import org.apache.commons.lang3.RandomStringUtils;
 
-import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Paths;
@@ -59,20 +58,21 @@ public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBa
         Boolean vmExists = command.isVmExists();
         String diskType = command.getDiskType();
         List<String> volumePaths = command.getVolumePaths();
-        String restoreVolumeUuid = command.getRestoreVolumeUUID();
+        List<String> backupFiles = command.getBackupFiles();
 
         String newVolumeId = null;
         try {
             if (Objects.isNull(vmExists)) {
                 String volumePath = volumePaths.get(0);
+                String backupFile = backupFiles.get(0);
                 int lastIndex = volumePath.lastIndexOf("/");
                 newVolumeId = volumePath.substring(lastIndex + 1);
-                restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, restoreVolumeUuid,
+                restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, backupFile,
                         new Pair<>(vmName, command.getVmState()), mountOptions);
             } else if (Boolean.TRUE.equals(vmExists)) {
-                restoreVolumesOfExistingVM(volumePaths, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+                restoreVolumesOfExistingVM(volumePaths, backupPath, backupFiles, backupRepoType, backupRepoAddress, mountOptions);
             } else {
-                restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+                restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupFiles, backupRepoType, backupRepoAddress, mountOptions);
             }
         } catch (CloudRuntimeException e) {
             String errorMessage = "Failed to restore backup for VM: " + vmName + ".";
@@ -86,17 +86,18 @@ public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBa
         return new BackupAnswer(command, true, newVolumeId);
     }
 
-    private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupPath,
+    private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupPath, List<String> backupFiles,
                                              String backupRepoType, String backupRepoAddress, String mountOptions) {
         String diskType = "root";
         String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType, mountOptions);
         try {
             for (int idx = 0; idx < volumePaths.size(); idx++) {
                 String volumePath = volumePaths.get(idx);
-                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, null);
+                String backupFile = backupFiles.get(idx);
+                String bkpPath = getBackupPath(mountDirectory, backupPath, backupFile, diskType);
                 diskType = "datadisk";
-                if (!replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first())) {
-                    throw new CloudRuntimeException(String.format("Unable to restore backup for volume [%s].", bkpPathAndVolUuid.second()));
+                if (!replaceVolumeWithBackup(volumePath, bkpPath)) {
+                    throw new CloudRuntimeException(String.format("Unable to restore backup from volume [%s].", volumePath));
                 }
             }
         } finally {
@@ -106,17 +107,18 @@ public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBa
 
     }
 
-    private void restoreVolumesOfDestroyedVMs(List<String> volumePaths, String vmName, String backupPath,
+    private void restoreVolumesOfDestroyedVMs(List<String> volumePaths, String vmName, String backupPath, List<String> backupFiles,
                                               String backupRepoType, String backupRepoAddress, String mountOptions) {
         String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType, mountOptions);
         String diskType = "root";
         try {
-            for (int i = 0; i < volumePaths.size(); i++) {
-                String volumePath = volumePaths.get(i);
-                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, null);
+            for (int idx = 0; idx < volumePaths.size(); idx++) {
+                String volumePath = volumePaths.get(idx);
+                String backupFile = backupFiles.get(idx);
+                String bkpPath = getBackupPath(mountDirectory, backupPath, backupFile, diskType);
                 diskType = "datadisk";
-                if (!replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first())) {
-                    throw new CloudRuntimeException(String.format("Unable to restore backup for volume [%s].", bkpPathAndVolUuid.second()));
+                if (!replaceVolumeWithBackup(volumePath, bkpPath)) {
+                    throw new CloudRuntimeException(String.format("Unable to restore backup from volume [%s].", volumePath));
                 }
             }
         } finally {
@@ -126,13 +128,13 @@ public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBa
     }
 
     private void restoreVolume(String backupPath, String backupRepoType, String backupRepoAddress, String volumePath,
-                               String diskType, String volumeUUID, Pair<String, VirtualMachine.State> vmNameAndState, String mountOptions) {
+                               String diskType, String backupFile, Pair<String, VirtualMachine.State> vmNameAndState, String mountOptions) {
         String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType, mountOptions);
-        Pair<String, String> bkpPathAndVolUuid;
+        String bkpPath;
         try {
-            bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, volumeUUID);
-            if (!replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first())) {
-                throw new CloudRuntimeException(String.format("Unable to restore backup for volume [%s].", bkpPathAndVolUuid.second()));
+            bkpPath = getBackupPath(mountDirectory, backupPath, backupFile, diskType);
+            if (!replaceVolumeWithBackup(volumePath, bkpPath)) {
+                throw new CloudRuntimeException(String.format("Unable to restore backup from volume [%s].", volumePath));
             }
             if (VirtualMachine.State.Running.equals(vmNameAndState.second())) {
                 if (!attachVolumeToVm(vmNameAndState.first(), volumePath)) {
@@ -188,13 +190,11 @@ public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBa
         }
     }
 
-    private Pair<String, String> getBackupPath(String mountDirectory, String volumePath, String backupPath, String diskType, String volumeUuid) {
+    private String getBackupPath(String mountDirectory, String backupPath, String backupFile, String diskType) {
         String bkpPath = String.format(FILE_PATH_PLACEHOLDER, mountDirectory, backupPath);
-        int lastIndex = volumePath.lastIndexOf(File.separator);
-        String volUuid = Objects.isNull(volumeUuid) ? volumePath.substring(lastIndex + 1) : volumeUuid;
-        String backupFileName = String.format("%s.%s.qcow2", diskType.toLowerCase(Locale.ROOT), volUuid);
+        String backupFileName = String.format("%s.%s.qcow2", diskType.toLowerCase(Locale.ROOT), backupFile);
         bkpPath = String.format(FILE_PATH_PLACEHOLDER, bkpPath, backupFileName);
-        return new Pair<>(bkpPath, volUuid);
+        return bkpPath;
     }
 
     private boolean replaceVolumeWithBackup(String volumePath, String backupPath) {

plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixModifyStoragePoolCommandWrapper.java

@@ -59,7 +59,7 @@ public final class CitrixModifyStoragePoolCommandWrapper extends CommandWrapper<
                 if (capacity == -1) {
                     final String msg = "Pool capacity is -1! pool: " + pool.getHost() + pool.getPath();
                     logger.warn(msg);
-                    return new Answer(command, false, msg);
+                    return new ModifyStoragePoolAnswer(command, false, msg);
                 }
                 final Map<String, TemplateProp> tInfo = new HashMap<String, TemplateProp>();
                 final ModifyStoragePoolAnswer answer = new ModifyStoragePoolAnswer(command, capacity, available, tInfo);
@@ -68,12 +68,12 @@ public final class CitrixModifyStoragePoolCommandWrapper extends CommandWrapper<
                 final String msg = "ModifyStoragePoolCommand add XenAPIException:" + e.toString() + " host:" + citrixResourceBase.getHost().getUuid() + " pool: " + pool.getHost()
                         + pool.getPath();
                 logger.warn(msg, e);
-                return new Answer(command, false, msg);
+                return new ModifyStoragePoolAnswer(command, false, msg);
             } catch (final Exception e) {
                 final String msg = "ModifyStoragePoolCommand add XenAPIException:" + e.getMessage() + " host:" + citrixResourceBase.getHost().getUuid() + " pool: "
                         + pool.getHost() + pool.getPath();
                 logger.warn(msg, e);
-                return new Answer(command, false, msg);
+                return new ModifyStoragePoolAnswer(command, false, msg);
             }
         } else {
             try {
@@ -85,17 +85,17 @@ public final class CitrixModifyStoragePoolCommandWrapper extends CommandWrapper<
                 if (result == null || !result.split("#")[1].equals("0")) {
                     throw new CloudRuntimeException("Unable to remove heartbeat file entry for SR " + srUuid + " due to " + result);
                 }
-                return new Answer(command, true, "success");
+                return new ModifyStoragePoolAnswer(command, true, "success");
             } catch (final XenAPIException e) {
                 final String msg = "ModifyStoragePoolCommand remove XenAPIException:" + e.toString() + " host:" + citrixResourceBase.getHost().getUuid() + " pool: "
                         + pool.getHost() + pool.getPath();
                 logger.warn(msg, e);
-                return new Answer(command, false, msg);
+                return new ModifyStoragePoolAnswer(command, false, msg);
             } catch (final Exception e) {
                 final String msg = "ModifyStoragePoolCommand remove XenAPIException:" + e.getMessage() + " host:" + citrixResourceBase.getHost().getUuid() + " pool: "
                         + pool.getHost() + pool.getPath();
                 logger.warn(msg, e);
-                return new Answer(command, false, msg);
+                return new ModifyStoragePoolAnswer(command, false, msg);
             }
         }
     }

plugins/integrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterImpl.java

@@ -17,6 +17,7 @@
 package org.apache.cloudstack.metrics;
 
 import java.math.BigDecimal;
+import java.security.cert.X509Certificate;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -26,6 +27,7 @@ import java.util.stream.Collectors;
 
 import javax.inject.Inject;
 
+import org.apache.cloudstack.ca.CAManager;
 import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
 import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
 import org.apache.commons.lang3.StringUtils;
@@ -133,6 +135,8 @@ public class PrometheusExporterImpl extends ManagerBase implements PrometheusExp
     private ResourceCountDao _resourceCountDao;
     @Inject
     private HostTagsDao _hostTagsDao;
+    @Inject
+    private CAManager caManager;
 
     public PrometheusExporterImpl() {
         super();
@@ -216,6 +220,9 @@ public class PrometheusExporterImpl extends ManagerBase implements PrometheusExp
             }
 
             metricsList.add(new ItemHostVM(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), vmDao.listByHostId(host.getId()).size()));
+
+            addSSLCertificateExpirationMetrics(metricsList, zoneName, zoneUuid, host);
+
             final CapacityVO coreCapacity = capacityDao.findByHostIdType(host.getId(), Capacity.CAPACITY_TYPE_CPU_CORE);
 
             if (coreCapacity == null && !host.isInMaintenanceStates()){
@@ -253,6 +260,18 @@ public class PrometheusExporterImpl extends ManagerBase implements PrometheusExp
         addHostTagsMetrics(metricsList, dcId, zoneName, zoneUuid, totalHosts, upHosts, downHosts, total, up, down);
     }
 
+    private void addSSLCertificateExpirationMetrics(List<Item> metricsList, String zoneName, String zoneUuid, HostVO host) {
+        if (caManager == null || caManager.getActiveCertificatesMap() == null) {
+            return;
+        }
+        X509Certificate cert = caManager.getActiveCertificatesMap().getOrDefault(host.getPrivateIpAddress(), null);
+        if (cert == null) {
+            return;
+        }
+        long certExpiryEpoch = cert.getNotAfter().getTime() / 1000; // Convert to epoch seconds
+        metricsList.add(new ItemHostCertExpiry(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), certExpiryEpoch));
+    }
+
     private String markTagMaps(HostVO host, Map<String, Integer> totalHosts, Map<String, Integer> upHosts, Map<String, Integer> downHosts) {
         List<HostTagVO> hostTagVOS = _hostTagsDao.getHostTags(host.getId());
         List<String> hostTags = new ArrayList<>();
@@ -1049,4 +1068,28 @@ public class PrometheusExporterImpl extends ManagerBase implements PrometheusExp
             return String.format("%s{zone=\"%s\",cpu=\"%d\",memory=\"%d\"} %d", name, zoneName, cpu, memory, total);
         }
     }
+
+    class ItemHostCertExpiry extends Item {
+        String zoneName;
+        String zoneUuid;
+        String hostName;
+        String hostUuid;
+        String hostIp;
+        long expiryTimestamp;
+
+        public ItemHostCertExpiry(final String zoneName, final String zoneUuid, final String hostName, final String hostUuid, final String hostIp, final long expiry) {
+            super("cloudstack_host_cert_expiry_timestamp");
+            this.zoneName = zoneName;
+            this.zoneUuid = zoneUuid;
+            this.hostName = hostName;
+            this.hostUuid = hostUuid;
+            this.hostIp = hostIp;
+            this.expiryTimestamp = expiry;
+        }
+
+        @Override
+        public String toMetricsString() {
+            return String.format("%s{zone=\"%s\",hostname=\"%s\",ip=\"%s\"} %d", name, zoneName, hostName, hostIp, expiryTimestamp);
+        }
+    }
 }

plugins/integrations/prometheus/src/test/java/org/apache/cloudstack/metrics/PrometheusExporterImplTest.java

@@ -0,0 +1,108 @@
+// 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.metrics;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import org.junit.Test;
+
+public class PrometheusExporterImplTest {
+
+    private static final String TEST_ZONE_NAME = "zone1";
+    private static final String TEST_ZONE_UUID = "zone-uuid-1";
+    private static final String TEST_HOST_NAME = "host1";
+    private static final String TEST_HOST_UUID = "host-uuid-1";
+    private static final String TEST_HOST_IP = "192.168.1.10";
+    private static final long CERT_EXPIRY_TIME = 1735689600000L; // 2025-01-01 00:00:00 UTC
+    private static final long CERT_EXPIRY_EPOCH = CERT_EXPIRY_TIME / 1000;
+
+    @Test
+    public void testItemHostCertExpiryFormat() {
+        PrometheusExporterImpl exporter = new PrometheusExporterImpl();
+        PrometheusExporterImpl.ItemHostCertExpiry item = exporter.new ItemHostCertExpiry(
+                TEST_ZONE_NAME,
+                TEST_ZONE_UUID,
+                TEST_HOST_NAME,
+                TEST_HOST_UUID,
+                TEST_HOST_IP,
+                CERT_EXPIRY_EPOCH
+        );
+
+        String metricsString = item.toMetricsString();
+        String expected = String.format(
+                "cloudstack_host_cert_expiry_timestamp{zone=\"%s\",hostname=\"%s\",ip=\"%s\"} %d",
+                TEST_ZONE_NAME,
+                TEST_HOST_NAME,
+                TEST_HOST_IP,
+                CERT_EXPIRY_EPOCH
+        );
+        assertEquals("Certificate expiry metric format should match expected format", expected, metricsString);
+    }
+
+    @Test
+    public void testItemHostCertExpiryContainsCorrectMetricName() {
+        PrometheusExporterImpl exporter = new PrometheusExporterImpl();
+        PrometheusExporterImpl.ItemHostCertExpiry item = exporter.new ItemHostCertExpiry(
+                TEST_ZONE_NAME,
+                TEST_ZONE_UUID,
+                TEST_HOST_NAME,
+                TEST_HOST_UUID,
+                TEST_HOST_IP,
+                CERT_EXPIRY_EPOCH
+        );
+
+        String metricsString = item.toMetricsString();
+        assertTrue("Metric should contain correct metric name",
+                metricsString.contains("cloudstack_host_cert_expiry_timestamp"));
+    }
+
+    @Test
+    public void testItemHostCertExpiryContainsAllLabels() {
+        PrometheusExporterImpl exporter = new PrometheusExporterImpl();
+        PrometheusExporterImpl.ItemHostCertExpiry item = exporter.new ItemHostCertExpiry(
+                TEST_ZONE_NAME,
+                TEST_ZONE_UUID,
+                TEST_HOST_NAME,
+                TEST_HOST_UUID,
+                TEST_HOST_IP,
+                CERT_EXPIRY_EPOCH
+        );
+
+        String metricsString = item.toMetricsString();
+        assertTrue("Metric should contain zone label", metricsString.contains("zone=\"" + TEST_ZONE_NAME + "\""));
+        assertTrue("Metric should contain hostname label", metricsString.contains("hostname=\"" + TEST_HOST_NAME + "\""));
+        assertTrue("Metric should contain ip label", metricsString.contains("ip=\"" + TEST_HOST_IP + "\""));
+    }
+
+    @Test
+    public void testItemHostCertExpiryContainsTimestampValue() {
+        PrometheusExporterImpl exporter = new PrometheusExporterImpl();
+        PrometheusExporterImpl.ItemHostCertExpiry item = exporter.new ItemHostCertExpiry(
+                TEST_ZONE_NAME,
+                TEST_ZONE_UUID,
+                TEST_HOST_NAME,
+                TEST_HOST_UUID,
+                TEST_HOST_IP,
+                CERT_EXPIRY_EPOCH
+        );
+
+        String metricsString = item.toMetricsString();
+        assertTrue("Metric should contain correct timestamp value",
+                metricsString.endsWith(" " + CERT_EXPIRY_EPOCH));
+    }
+}

plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/OauthLoginAPIAuthenticatorCmd.java

@@ -176,18 +176,14 @@ public class OauthLoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent
     }
 
     protected Long getDomainIdFromParams(Map<String, Object[]> params, StringBuilder auditTrailSb, String responseType) {
-        String[] domainIdArr = (String[])params.get(ApiConstants.DOMAIN_ID);
-
-        if (domainIdArr == null) {
-            domainIdArr = (String[])params.get(ApiConstants.DOMAIN__ID);
-        }
+        String domainIdStr = _apiServer.getDomainId(params);
         Long domainId = null;
-        if ((domainIdArr != null) && (domainIdArr.length > 0)) {
+        if (StringUtils.isNotEmpty(domainIdStr)) {
             try {
                 //check if UUID is passed in for domain
-                domainId = _apiServer.fetchDomainId(domainIdArr[0]);
+                domainId = _apiServer.fetchDomainId(domainIdStr);
                 if (domainId == null) {
-                    domainId = Long.parseLong(domainIdArr[0]);
+                    domainId = Long.parseLong(domainIdStr);
                 }
                 auditTrailSb.append(" domainid=" + domainId);// building the params for POST call
             } catch (final NumberFormatException e) {

plugins/user-authenticators/oauth2/src/test/java/org/apache/cloudstack/oauth2/api/command/OauthLoginAPIAuthenticatorCmdTest.java

@@ -85,10 +85,29 @@ public class OauthLoginAPIAuthenticatorCmdTest {
         ApiServer apiServer = mock(ApiServer.class);
         cmd._apiServer = apiServer;
         when(apiServer.fetchDomainId("1234")).thenReturn(5678L);
+        when(apiServer.getDomainId(params)).thenCallRealMethod();
 
         Long domainId = cmd.getDomainIdFromParams(params, auditTrailSb, responseType);
 
         assertEquals(Long.valueOf(5678), domainId);
         assertEquals(" domainid=5678", auditTrailSb.toString());
     }
+
+    @Test
+    public void testGetDomainIdFromCamelCaseParam() {
+        StringBuilder auditTrailSb = new StringBuilder();
+        String responseType = "json";
+        Map<String, Object[]> params = new HashMap<>();
+        params.put(ApiConstants.DOMAIN_ID, null);
+        params.put(ApiConstants.DOMAIN__ID, new String[]{"5678"});
+        ApiServer apiServer = mock(ApiServer.class);
+        cmd._apiServer = apiServer;
+        when(apiServer.fetchDomainId("5678")).thenReturn(1234L);
+        when(apiServer.getDomainId(params)).thenCallRealMethod();
+
+        Long domainId = cmd.getDomainIdFromParams(params, auditTrailSb, responseType);
+
+        assertEquals(Long.valueOf(1234), domainId);
+        assertEquals(" domainid=1234", auditTrailSb.toString());
+    }
 }

plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java

@@ -158,11 +158,17 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent
                 String domainPath = null;
 
                 if (params.containsKey(ApiConstants.IDP_ID)) {
-                    idpId = ((String[])params.get(ApiConstants.IDP_ID))[0];
+                    String[] idpIds = (String[])params.get(ApiConstants.IDP_ID);
+                    if (idpIds != null && idpIds.length > 0) {
+                        idpId = idpIds[0];
+                    }
                 }
 
                 if (params.containsKey(ApiConstants.DOMAIN)) {
-                    domainPath = ((String[])params.get(ApiConstants.DOMAIN))[0];
+                    String[] domainPaths = (String[])params.get(ApiConstants.DOMAIN);
+                    if (domainPaths != null && domainPaths.length > 0) {
+                        domainPath = domainPaths[0];
+                    }
                 }
 
                 if (domainPath != null && !domainPath.isEmpty()) {

pom.xml

@@ -63,20 +63,20 @@
         <!-- Plugins versions -->
         <cs.antrun-plugin.version>1.8</cs.antrun-plugin.version>
         <cs.builder-helper-plugin.version>3.0.0</cs.builder-helper-plugin.version>
-        <cs.checkstyle-plugin.version>3.1.0</cs.checkstyle-plugin.version>
+        <cs.checkstyle-plugin.version>3.6.0</cs.checkstyle-plugin.version>
         <cs.jacoco-plugin.version>0.8.11</cs.jacoco-plugin.version>
         <cs.compiler-plugin.version>3.8.1</cs.compiler-plugin.version>
-        <cs.dependency-plugin.version>3.1.1</cs.dependency-plugin.version>
+        <cs.dependency-plugin.version>3.9.0</cs.dependency-plugin.version>
         <cs.failsafe-plugin.version>2.22.2</cs.failsafe-plugin.version>
         <cs.spotbugs.version>3.1.12</cs.spotbugs.version>
         <cs.spotbugs-maven-plugin.version>3.1.12.2</cs.spotbugs-maven-plugin.version>
         <cs.jar-plugin.version>3.2.0</cs.jar-plugin.version>
-        <cs.pmd-plugin.version>3.12.0</cs.pmd-plugin.version>
+        <cs.pmd-plugin.version>3.28.0</cs.pmd-plugin.version>
         <cs.project-info-plugin.version>3.0.0</cs.project-info-plugin.version>
         <cs.owasp.dependency-checker-plugin.version>7.4.4</cs.owasp.dependency-checker-plugin.version>
         <cs.release-plugin.version>2.5.3</cs.release-plugin.version>
         <cs.resources-plugin.version>3.1.0</cs.resources-plugin.version>
-        <cs.site-plugin.version>3.8.2</cs.site-plugin.version>
+        <cs.site-plugin.version>3.21.0</cs.site-plugin.version>
         <cs.surefire-plugin.version>2.22.2</cs.surefire-plugin.version>
         <cs.clover-maven-plugin.version>4.4.1</cs.clover-maven-plugin.version>
         <cs.exec-maven-plugin.version>3.2.0</cs.exec-maven-plugin.version>

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

@@ -4364,7 +4364,7 @@ public class ApiResponseHelper implements ResponseGenerator {
         } else if (usageRecord.getUsageType() == UsageTypes.BACKUP) {
             resourceType = ResourceObjectType.Backup;
             final StringBuilder builder = new StringBuilder();
-            builder.append("Backup usage of size ").append(usageRecord.getUsageDisplay());
+            builder.append("Backup usage");
             if (vmInstance != null) {
                 resourceId = vmInstance.getId();
                 usageRecResponse.setResourceName(vmInstance.getInstanceName());
@@ -4377,9 +4377,12 @@ public class ApiResponseHelper implements ResponseGenerator {
                             .append(" (").append(backupOffering.getUuid()).append(", user ad-hoc/scheduled backup allowed: ")
                             .append(backupOffering.isUserDrivenBackupAllowed()).append(")");
                 }
-
             }
+            builder.append(" with size ").append(toHumanReadableSize(usageRecord.getSize()));
+            builder.append(" and with virtual size ").append(toHumanReadableSize(usageRecord.getVirtualSize()));
             usageRecResponse.setDescription(builder.toString());
+            usageRecResponse.setSize(usageRecord.getSize());
+            usageRecResponse.setVirtualSize(usageRecord.getVirtualSize());
         } else if (usageRecord.getUsageType() == UsageTypes.VM_SNAPSHOT) {
             resourceType = ResourceObjectType.VMSnapshot;
             VMSnapshotVO vmSnapshotVO = null;

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

@@ -115,6 +115,7 @@ import org.apache.cloudstack.framework.messagebus.MessageHandler;
 import org.apache.cloudstack.managed.context.ManagedContextRunnable;
 import org.apache.cloudstack.user.UserPasswordResetManager;
 import org.apache.commons.codec.binary.Base64;
+import org.apache.commons.collections.MapUtils;
 import org.apache.commons.lang3.EnumUtils;
 import org.apache.http.ConnectionClosedException;
 import org.apache.http.HttpException;
@@ -1354,6 +1355,25 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
         return userPasswordResetManager.validateAndResetPassword(userAccount, token, password);
     }
 
+    @Override
+    public String getDomainId(Map<String, Object[]> params) {
+        if (MapUtils.isEmpty(params)) {
+            return null;
+        }
+
+        String[] domainIdArr = (String[])params.get(ApiConstants.DOMAIN_ID);
+        if (domainIdArr == null) {
+            // Fallback to support clients using the camelCase parameter name "domainId"
+            domainIdArr = (String[])params.get(ApiConstants.DOMAIN__ID);
+        }
+
+        if (domainIdArr == null || domainIdArr.length == 0) {
+            return null;
+        }
+
+        return domainIdArr[0];
+    }
+
     private void checkCommandAvailable(final User user, final String commandName, final InetAddress remoteAddress) throws PermissionDeniedException {
         if (user == null) {
             throw new PermissionDeniedException("User is null for role based API access check for command" + commandName);

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

@@ -34,6 +34,7 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
 
+import com.cloud.api.auth.DefaultForgotPasswordAPIAuthenticatorCmd;
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.ApiErrorCode;
 import org.apache.cloudstack.api.ApiServerService;
@@ -164,7 +165,6 @@ public class ApiServlet extends HttpServlet {
                 LOGGER.warn(message);
             }
         });
-
     }
 
     void processRequestInContext(final HttpServletRequest req, final HttpServletResponse resp) {
@@ -226,7 +226,6 @@ public class ApiServlet extends HttpServlet {
             }
 
             if (command != null && !command.equals(ValidateUserTwoFactorAuthenticationCodeCmd.APINAME)) {
-
                 APIAuthenticator apiAuthenticator = authManager.getAPIAuthenticator(command);
                 if (apiAuthenticator != null) {
                     auditTrailSb.append("command=");
@@ -262,7 +261,9 @@ public class ApiServlet extends HttpServlet {
                     } catch (ServerApiException e) {
                         httpResponseCode = e.getErrorCode().getHttpCode();
                         responseString = e.getMessage();
-                        LOGGER.debug("Authentication failure: " + e.getMessage());
+                        if (!DefaultForgotPasswordAPIAuthenticatorCmd.APINAME.equalsIgnoreCase(command) || StringUtils.isNotBlank(username)) {
+                            LOGGER.debug("Authentication failure: {}", e.getMessage());
+                        }
                     }
 
                     if (apiAuthenticator.getAPIType() == APIAuthenticationType.LOGOUT_API) {
@@ -330,7 +331,7 @@ public class ApiServlet extends HttpServlet {
                     }
                 }
 
-                if (! requestChecksoutAsSane(resp, auditTrailSb, responseType, params, session, command, userId, account, accountObj))
+                if (!requestChecksoutAsSane(resp, auditTrailSb, responseType, params, session, command, userId, account, accountObj))
                     return;
             } else {
                 CallContext.register(accountMgr.getSystemUser(), accountMgr.getSystemAccount());
@@ -360,7 +361,6 @@ public class ApiServlet extends HttpServlet {
                         apiServer.getSerializedApiError(HttpServletResponse.SC_UNAUTHORIZED, "unable to verify user credentials and/or request signature", params,
                                 responseType);
                 HttpUtils.writeHttpResponse(resp, serializedResponse, HttpServletResponse.SC_UNAUTHORIZED, responseType, ApiServer.JSONcontentType.value());
-
             }
         } catch (final ServerApiException se) {
             final String serializedResponseText = apiServer.getSerializedApiError(se, params, responseType);
@@ -550,6 +550,9 @@ public class ApiServlet extends HttpServlet {
             if (LOGGER.isTraceEnabled()) {
                 LOGGER.trace(msg);
             }
+            if (session == null) {
+                return;
+            }
             session.invalidate();
         } catch (final IllegalStateException ise) {
             if (LOGGER.isTraceEnabled()) {

server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java

@@ -44,13 +44,13 @@ import java.net.InetAddress;
 import java.util.List;
 import java.util.Map;
 
-@APICommand(name = "forgotPassword",
+@APICommand(name = DefaultForgotPasswordAPIAuthenticatorCmd.APINAME,
         description = "Sends an email to the user with a token to reset the password using resetPassword command.",
         since = "4.20.0.0",
         requestHasSensitiveInfo = true,
         responseObject = SuccessResponse.class)
 public class DefaultForgotPasswordAPIAuthenticatorCmd extends BaseCmd implements APIAuthenticator {
-
+    public static final String APINAME = "forgotPassword";
 
     /////////////////////////////////////////////////////
     //////////////// API parameters /////////////////////
@@ -108,10 +108,12 @@ public class DefaultForgotPasswordAPIAuthenticatorCmd extends BaseCmd implements
                 if (userDomain != null) {
                     domainId = userDomain.getId();
                 } else {
+                    logger.debug("Unable to find the domain from the path {}", domain);
                     throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Unable to find the domain from the path %s", domain));
                 }
                 final UserAccount userAccount = _accountService.getActiveUserAccount(username[0], domainId);
                 if (userAccount != null && List.of(User.Source.SAML2, User.Source.OAUTH2, User.Source.LDAP).contains(userAccount.getSource())) {
+                    logger.debug("Forgot Password is not allowed for the user {} from source {}", username[0], userAccount.getSource());
                     throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Forgot Password is not allowed for this user");
                 }
                 boolean success = _apiServer.forgotPassword(userAccount, userDomain);

server/src/main/java/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java

@@ -20,6 +20,7 @@ import com.cloud.api.ApiServlet;
 import com.cloud.domain.Domain;
 import com.cloud.user.User;
 import com.cloud.user.UserAccount;
+import com.cloud.utils.StringUtils;
 import org.apache.cloudstack.api.ApiServerService;
 import com.cloud.api.response.ApiResponseSerializer;
 import com.cloud.exception.CloudAuthenticationException;
@@ -47,7 +48,6 @@ import java.net.InetAddress;
 @APICommand(name = "login", description = "Logs a user into the CloudStack. A successful login attempt will generate a JSESSIONID cookie value that can be passed in subsequent Query command calls until the \"logout\" command has been issued or the session has expired.", requestHasSensitiveInfo = true, responseObject = LoginCmdResponse.class, entityType = {})
 public class DefaultLoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthenticator {
 
-
     /////////////////////////////////////////////////////
     //////////////// API parameters /////////////////////
     /////////////////////////////////////////////////////
@@ -107,22 +107,18 @@ public class DefaultLoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthe
         if (HTTPMethod.valueOf(req.getMethod()) != HTTPMethod.POST) {
             throw new ServerApiException(ApiErrorCode.METHOD_NOT_ALLOWED, "Please use HTTP POST to authenticate using this API");
         }
+
         // FIXME: ported from ApiServlet, refactor and cleanup
         final String[] username = (String[])params.get(ApiConstants.USERNAME);
         final String[] password = (String[])params.get(ApiConstants.PASSWORD);
-        String[] domainIdArr = (String[])params.get(ApiConstants.DOMAIN_ID);
-
-        if (domainIdArr == null) {
-            domainIdArr = (String[])params.get(ApiConstants.DOMAIN__ID);
-        }
-        final String[] domainName = (String[])params.get(ApiConstants.DOMAIN);
+        String domainIdStr = _apiServer.getDomainId(params);
         Long domainId = null;
-        if ((domainIdArr != null) && (domainIdArr.length > 0)) {
+        if (StringUtils.isNotEmpty(domainIdStr)) {
             try {
                 //check if UUID is passed in for domain
-                domainId = _apiServer.fetchDomainId(domainIdArr[0]);
+                domainId = _apiServer.fetchDomainId(domainIdStr);
                 if (domainId == null) {
-                    domainId = Long.parseLong(domainIdArr[0]);
+                    domainId = Long.parseLong(domainIdStr);
                 }
                 auditTrailSb.append(" domainid=" + domainId);// building the params for POST call
             } catch (final NumberFormatException e) {
@@ -135,6 +131,7 @@ public class DefaultLoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthe
         }
 
         String domain = null;
+        final String[] domainName = (String[])params.get(ApiConstants.DOMAIN);
         domain = getDomainName(auditTrailSb, domainName, domain);
 
         String serializedResponse = null;

server/src/main/java/com/cloud/api/auth/DefaultResetPasswordAPIAuthenticatorCmd.java

@@ -53,7 +53,6 @@ import java.util.Map;
         responseObject = SuccessResponse.class)
 public class DefaultResetPasswordAPIAuthenticatorCmd extends BaseCmd implements APIAuthenticator {
 
-
     /////////////////////////////////////////////////////
     //////////////// API parameters /////////////////////
     /////////////////////////////////////////////////////

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

@@ -107,6 +107,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
 import org.apache.cloudstack.framework.config.ConfigDepot;
 import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.framework.config.ValidatedConfigKey;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.cloudstack.framework.config.dao.ConfigurationGroupDao;
 import org.apache.cloudstack.framework.config.dao.ConfigurationSubGroupDao;
@@ -716,6 +717,12 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
             throw new InvalidParameterValueException(validationMsg);
         }
 
+        ConfigKey<?> configKey = _configDepot.get(name);
+        if (configKey instanceof ValidatedConfigKey) {
+            ValidatedConfigKey<?> validatedConfigKey = (ValidatedConfigKey<?>) configKey;
+            validatedConfigKey.validateValue(value);
+        }
+
         // If scope of the parameter is given then it needs to be updated in the
         // corresponding details table,
         // if scope is mentioned as global or not mentioned then it is normal

server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java

@@ -272,7 +272,12 @@ public abstract class LibvirtServerDiscoverer extends DiscovererBase implements
                 }
             }
 
-            sshConnection = new Connection(agentIp, 22);
+            int port = uri.getPort();
+            if (port <= 0) {
+                port = AgentManager.KVMHostDiscoverySshPort.valueIn(clusterId);
+            }
+
+            sshConnection = new Connection(agentIp, port);
 
             sshConnection.connect(null, 60000, 60000);
 
@@ -380,6 +385,9 @@ public abstract class LibvirtServerDiscoverer extends DiscovererBase implements
             Map<String, String> hostDetails = connectedHost.getDetails();
             hostDetails.put("password", password);
             hostDetails.put("username", username);
+            if (uri.getPort() > 0) {
+                hostDetails.put(Host.HOST_SSH_PORT, String.valueOf(uri.getPort()));
+            }
             _hostDao.saveDetails(connectedHost);
             return resources;
         } catch (DiscoveredWithErrorException e) {

server/src/main/java/com/cloud/resource/ResourceManagerImpl.java

@@ -776,7 +776,6 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
                 _clusterDetailsDao.persist(cluster_cpu_detail);
                 _clusterDetailsDao.persist(cluster_memory_detail);
             }
-
         }
 
         try {
@@ -871,7 +870,6 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
                         hosts.add(host);
                     }
                     discoverer.postDiscovery(hosts, _nodeId);
-
                 }
                 logger.info("server resources successfully discovered by " + discoverer.getName());
                 return hosts;
@@ -2261,15 +2259,26 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
     private HostVO getNewHost(StartupCommand[] startupCommands) {
         StartupCommand startupCommand = startupCommands[0];
 
-        HostVO host = findHostByGuid(startupCommand.getGuid());
+        String fullGuid = startupCommand.getGuid();
+        logger.debug(String.format("Trying to find Host by guid %s", fullGuid));
+        HostVO host = findHostByGuid(fullGuid);
 
         if (host != null) {
+            logger.debug(String.format("Found Host by guid %s: %s", fullGuid, host));
             return host;
         }
 
-        host = findHostByGuid(startupCommand.getGuidWithoutResource());
+        String guidPrefix = startupCommand.getGuidWithoutResource();
+        logger.debug(String.format("Trying to find Host by guid prefix %s", guidPrefix));
+        host = findHostByGuidPrefix(guidPrefix);
 
-        return host; // even when host == null!
+        if (host != null) {
+            logger.debug(String.format("Found Host by guid prefix %s: %s", guidPrefix, host));
+            return host;
+        }
+
+        logger.debug(String.format("Could not find Host by guid %s", fullGuid));
+        return null;
     }
 
     protected HostVO createHostVO(final StartupCommand[] cmds, final ServerResource resource, final Map<String, String> details, List<String> hostTags,
@@ -2949,7 +2958,7 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
      */
     protected void connectAndRestartAgentOnHost(HostVO host, String username, String password, String privateKey) {
         final com.trilead.ssh2.Connection connection = SSHCmdHelper.acquireAuthorizedConnection(
-                host.getPrivateIpAddress(), 22, username, password, privateKey);
+                host.getPrivateIpAddress(), _agentMgr.getHostSshPort(host), username, password, privateKey);
         if (connection == null) {
             throw new CloudRuntimeException(String.format("SSH to agent is enabled, but failed to connect to %s via IP address [%s].", host, host.getPrivateIpAddress()));
         }
@@ -3296,6 +3305,15 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
     public HostVO findHostByGuid(final String guid) {
         final QueryBuilder<HostVO> sc = QueryBuilder.create(HostVO.class);
         sc.and(sc.entity().getGuid(), Op.EQ, guid);
+        sc.and(sc.entity().getRemoved(), Op.NULL);
+        return sc.find();
+    }
+
+    @Override
+    public HostVO findHostByGuidPrefix(String guid) {
+        final QueryBuilder<HostVO> sc = QueryBuilder.create(HostVO.class);
+        sc.and(sc.entity().getGuid(), Op.LIKE, guid + "%");
+        sc.and(sc.entity().getRemoved(), Op.NULL);
         return sc.find();
     }
 
@@ -3303,6 +3321,7 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
     public HostVO findHostByName(final String name) {
         final QueryBuilder<HostVO> sc = QueryBuilder.create(HostVO.class);
         sc.and(sc.entity().getName(), Op.EQ, name);
+        sc.and(sc.entity().getRemoved(), Op.NULL);
         return sc.find();
     }
 

server/src/main/java/com/cloud/server/ManagementServerImpl.java

@@ -44,6 +44,7 @@ import javax.crypto.spec.SecretKeySpec;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.network.vpc.VpcVO;
 import org.apache.cloudstack.acl.ControlledEntity;
 import org.apache.cloudstack.acl.SecurityChecker;
 import org.apache.cloudstack.affinity.AffinityGroupProcessor;
@@ -2580,12 +2581,21 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
         }
 
         if (associatedNetworkId != null) {
-            _accountMgr.checkAccess(caller, null, false, networkDao.findById(associatedNetworkId));
-            sc.setParameters("associatedNetworkIdEq", associatedNetworkId);
+            NetworkVO associatedNetwork = networkDao.findById(associatedNetworkId);
+
+            if (associatedNetwork != null) {
+                _accountMgr.checkAccess(caller, null, false, associatedNetwork);
+                sc.setParameters("associatedNetworkIdEq", associatedNetworkId);
+            }
         }
+
         if (vpcId != null) {
-            _accountMgr.checkAccess(caller, null, false, _vpcDao.findById(vpcId));
-            sc.setParameters("vpcId", vpcId);
+            VpcVO vpc = _vpcDao.findById(vpcId);
+
+            if (vpc != null) {
+                _accountMgr.checkAccess(caller, null, false, vpc);
+                sc.setParameters("vpcId", vpcId);
+            }
         }
 
         addrs = _publicIpAddressDao.search(sc, searchFilter); // Allocated
@@ -2602,13 +2612,16 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
             }
             if (associatedNetworkId != null) {
                 NetworkVO guestNetwork = networkDao.findById(associatedNetworkId);
-                if (zoneId == null) {
-                    zoneId = guestNetwork.getDataCenterId();
-                } else if (zoneId != guestNetwork.getDataCenterId()) {
-                    InvalidParameterValueException ex = new InvalidParameterValueException("Please specify a valid associated network id in the specified zone.");
-                    throw ex;
+
+                if (guestNetwork != null) {
+                    if (zoneId == null) {
+                        zoneId = guestNetwork.getDataCenterId();
+                    } else if (zoneId != guestNetwork.getDataCenterId()) {
+                        InvalidParameterValueException ex = new InvalidParameterValueException("Please specify a valid associated network id in the specified zone.");
+                        throw ex;
+                    }
+                    owner = _accountDao.findById(guestNetwork.getAccountId());
                 }
-                owner = _accountDao.findById(guestNetwork.getAccountId());
             }
             List<DataCenterVO> dcList = new ArrayList<>();
             if (zoneId == null){

server/src/main/java/com/cloud/template/TemplateManagerImpl.java

@@ -219,7 +219,6 @@ import com.cloud.vm.VirtualMachineProfileImpl;
 import com.cloud.vm.VmDetailConstants;
 import com.cloud.vm.dao.UserVmDao;
 import com.cloud.vm.dao.VMInstanceDao;
-import com.google.common.base.Joiner;
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
 
@@ -1375,9 +1374,16 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
         else {
             vmInstanceVOList = _vmInstanceDao.listNonExpungedByTemplate(templateId);
         }
-        if(!cmd.isForced() && CollectionUtils.isNotEmpty(vmInstanceVOList)) {
-            final String message = String.format("Unable to delete Template: %s because Instance: [%s] are using it.",  template, Joiner.on(",").join(vmInstanceVOList));
-            logger.warn(message);
+        if (!cmd.isForced() && CollectionUtils.isNotEmpty(vmInstanceVOList)) {
+            String message = String.format("Unable to delete template [%s] because there are [%d] VM instances using it.", template, vmInstanceVOList.size());
+            String instancesListMessage = String.format(" Instances list: [%s].", StringUtils.join(vmInstanceVOList, ","));
+
+            logger.warn("{}{}", message, instancesListMessage);
+
+            if (_accountMgr.isRootAdmin(caller.getAccountId())) {
+                message += instancesListMessage;
+            }
+
             throw new InvalidParameterValueException(message);
         }
 
@@ -2210,7 +2216,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
                   templateType == null &&
                   templateTag == null &&
                   arch == null &&
-                  (! cleanupDetails && details == null) //update details in every case except this one
+                  (!cleanupDetails && details == null) //update details in every case except this one
                   );
         if (!updateNeeded) {
             return template;
@@ -2308,8 +2314,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
         if (cleanupDetails) {
             template.setDetails(null);
             _tmpltDetailsDao.removeDetails(id);
-        }
-        else if (details != null && !details.isEmpty()) {
+        } else if (details != null && !details.isEmpty()) {
             template.setDetails(details);
             _tmpltDao.saveDetails(template);
         }

server/src/main/java/com/cloud/user/AccountManagerImpl.java

@@ -85,7 +85,6 @@ import org.apache.cloudstack.webhook.WebhookHelper;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.lang3.BooleanUtils;
-import org.apache.commons.lang3.StringUtils;
 import org.jetbrains.annotations.NotNull;
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 
@@ -176,6 +175,7 @@ import com.cloud.utils.ConstantTimeComparator;
 import com.cloud.utils.NumbersUtil;
 import com.cloud.utils.Pair;
 import com.cloud.utils.Ternary;
+import com.cloud.utils.StringUtils;
 import com.cloud.utils.component.ComponentContext;
 import com.cloud.utils.component.Manager;
 import com.cloud.utils.component.ManagerBase;
@@ -223,7 +223,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
     @Inject
     private InstanceGroupDao _vmGroupDao;
     @Inject
-    private UserAccountDao _userAccountDao;
+    private UserAccountDao userAccountDao;
     @Inject
     private VolumeDao _volumeDao;
     @Inject
@@ -585,11 +585,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
             if (acct == null) {
                 return false;  //account is deleted or does not exist
             }
-            if ((isRootAdmin(accountId)) || (isDomainAdmin(accountId)) || (isResourceDomainAdmin(accountId))) {
-                return true;
-            } else if (acct.getType() == Account.Type.READ_ONLY_ADMIN) {
-                return true;
-            }
+            return (isRootAdmin(accountId)) || (isDomainAdmin(accountId)) || (isResourceDomainAdmin(accountId)) || (acct.getType() == Account.Type.READ_ONLY_ADMIN);
 
         }
         return false;
@@ -644,10 +640,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
     @Override
     public boolean isNormalUser(long accountId) {
         AccountVO acct = _accountDao.findById(accountId);
-        if (acct != null && acct.getType() == Account.Type.NORMAL) {
-            return true;
-        }
-        return false;
+        return acct != null && acct.getType() == Account.Type.NORMAL;
     }
 
     @Override
@@ -678,10 +671,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
         if (account == null) {
             return false;  //account is deleted or does not exist
         }
-        if (isRootAdmin(accountId) || (account.getType() == Account.Type.ADMIN)) {
-            return true;
-        }
-        return false;
+        return isRootAdmin(accountId) || (account.getType() == Account.Type.ADMIN);
     }
 
     @Override
@@ -712,7 +702,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
             for (ControlledEntity entity : entities) {
                 if (ownerId == null) {
                     ownerId = entity.getAccountId();
-                } else if (ownerId.longValue() != entity.getAccountId()) {
+                } else if (! ownerId.equals(entity.getAccountId())) {
                     throw new PermissionDeniedException("Entity " + entity + " and entity " + prevEntity + " belong to different accounts");
                 }
                 prevEntity = entity;
@@ -738,7 +728,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
                 domainId = account != null ? account.getDomainId() : -1;
             }
             if (entity.getAccountId() != -1 && domainId != -1 && !(entity instanceof VirtualMachineTemplate)
-                    && !(entity instanceof Network && accessType != null && (accessType == AccessType.UseEntry || accessType == AccessType.OperateEntry))
+                    && !(entity instanceof Network && (accessType == AccessType.UseEntry || accessType == AccessType.OperateEntry))
                     && !(entity instanceof AffinityGroup) && !(entity instanceof VirtualRouter)) {
                 List<ControlledEntity> toBeChecked = domains.get(entity.getDomainId());
                 // for templates, we don't have to do cross domains check
@@ -821,7 +811,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
 
         // Currently just for resource domain admin
         List<DataCenterVO> dcList = _dcDao.findZonesByDomainId(account.getDomainId());
-        if (dcList != null && dcList.size() != 0) {
+        if (CollectionUtils.isNotEmpty(dcList)) {
             return dcList.get(0).getId();
         } else {
             throw new CloudRuntimeException("Failed to find any private zone for Resource domain admin.");
@@ -836,23 +826,23 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
                 @Override
                 public void doInTransactionWithoutResult(TransactionStatus status) {
                     UserAccountVO user = null;
-                    user = _userAccountDao.lockRow(id, true);
+                    user = userAccountDao.lockRow(id, true);
                     user.setLoginAttempts(attempts);
                     if (toDisable) {
                         user.setState(State.DISABLED.toString());
                     }
-                    _userAccountDao.update(id, user);
+                    userAccountDao.update(id, user);
                 }
             });
         } catch (Exception e) {
-            logger.error("Failed to update login attempts for user {}", () -> _userAccountDao.findById(id));
+            logger.error("Failed to update login attempts for user {}", () -> userAccountDao.findById(id));
         }
     }
 
     private boolean doSetUserStatus(long userId, State state) {
         UserVO userForUpdate = _userDao.createForUpdate();
         userForUpdate.setState(state);
-        return _userDao.update(Long.valueOf(userId), userForUpdate);
+        return _userDao.update(userId, userForUpdate);
     }
 
     @Override
@@ -861,7 +851,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
         AccountVO acctForUpdate = _accountDao.createForUpdate();
         acctForUpdate.setState(State.ENABLED);
         acctForUpdate.setNeedsCleanup(false);
-        success = _accountDao.update(Long.valueOf(accountId), acctForUpdate);
+        success = _accountDao.update(accountId, acctForUpdate);
         return success;
     }
 
@@ -874,7 +864,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
             } else if (account.getState().equals(State.ENABLED)) {
                 AccountVO acctForUpdate = _accountDao.createForUpdate();
                 acctForUpdate.setState(State.LOCKED);
-                success = _accountDao.update(Long.valueOf(accountId), acctForUpdate);
+                success = _accountDao.update(accountId, acctForUpdate);
             } else {
                 if (logger.isInfoEnabled()) {
                     logger.info("Attempting to lock a non-enabled account {}, current state is {}, locking failed.", account, account.getState());
@@ -988,7 +978,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
             }
 
             // Destroy VM Snapshots
-            List<VMSnapshotVO> vmSnapshots = _vmSnapshotDao.listByAccountId(Long.valueOf(accountId));
+            List<VMSnapshotVO> vmSnapshots = _vmSnapshotDao.listByAccountId(accountId);
             for (VMSnapshot vmSnapshot : vmSnapshots) {
                 try {
                     _vmSnapshotMgr.deleteVMSnapshot(vmSnapshot.getId());
@@ -1010,8 +1000,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
                     try {
                         _vmMgr.destroyVm(vm.getId(), false);
                     } catch (Exception e) {
-                        e.printStackTrace();
-                        logger.warn("Failed destroying instance {} as part of account deletion.", vm);
+                        logger.warn("Failed destroying instance {} as part of account deletion.", vm, e);
                     }
                 }
                 // no need to catch exception at this place as expunging vm
@@ -1069,7 +1058,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
             logger.debug("Deleting networks for account {}", account);
             List<NetworkVO> networks = _networkDao.listByOwner(accountId);
             if (networks != null) {
-                Collections.sort(networks, new Comparator<>() {
+                networks.sort(new Comparator<>() {
                     @Override
                     public int compare(NetworkVO network1, NetworkVO network2) {
                         if (network1.getGuestType() != network2.getGuestType() && Network.GuestType.Isolated.equals(network2.getGuestType())) {
@@ -1237,7 +1226,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
         } else {
             AccountVO acctForUpdate = _accountDao.createForUpdate();
             acctForUpdate.setState(State.DISABLED);
-            success = _accountDao.update(Long.valueOf(accountId), acctForUpdate);
+            success = _accountDao.update(accountId, acctForUpdate);
 
             if (success) {
                 boolean disableAccountResult = false;
@@ -1331,11 +1320,11 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
         // Check permissions
         checkAccess(getCurrentCallingAccount(), domain);
 
-        if (!userAllowMultipleAccounts.valueInDomain(domainId) && !_userAccountDao.validateUsernameInDomain(userName, domainId)) {
+        if (!userAllowMultipleAccounts.valueInDomain(domainId) && !userAccountDao.validateUsernameInDomain(userName, domainId)) {
             throw new InvalidParameterValueException(String.format("The user %s already exists in domain %s", userName, domain));
         }
 
-        if (networkDomain != null && networkDomain.length() > 0) {
+        if (StringUtils.isNotEmpty(networkDomain)) {
             if (!NetUtils.verifyDomainName(networkDomain)) {
                 throw new InvalidParameterValueException(
                         "Invalid network domain. Total length shouldn't exceed 190 chars. Each domain label must be between 1 and 63 characters long, can contain ASCII letters 'a' through 'z', the digits '0' through '9', "
@@ -1387,7 +1376,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
         CallContext.current().putContextParameter(User.class, userId);
 
         // check success
-        return _userAccountDao.findById(userId);
+        return userAccountDao.findById(userId);
     }
 
     /*
@@ -1525,7 +1514,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
             throw new PermissionDeniedException(String.format("Account: %s is a system account, can't add a user to it", account));
         }
 
-        if (!userAllowMultipleAccounts.valueInDomain(domainId) && !_userAccountDao.validateUsernameInDomain(userName, domainId)) {
+        if (!userAllowMultipleAccounts.valueInDomain(domainId) && !userAccountDao.validateUsernameInDomain(userName, domainId)) {
             throw new CloudRuntimeException("The user " + userName + " already exists in domain " + domainId);
         }
         List<UserVO> duplicatedUsers = _userDao.findUsersByName(userName);
@@ -1579,7 +1568,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
             user.setUser2faEnabled(true);
         }
         _userDao.update(user.getId(), user);
-        return _userAccountDao.findById(user.getId());
+        return userAccountDao.findById(user.getId());
     }
 
     @Override
@@ -1861,10 +1850,9 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
         if (isApiKeyBlank && isSecretKeyBlank) {
             return;
         }
-        Pair<User, Account> apiKeyOwner = _accountDao.findUserAccountByApiKey(apiKey);
+        UserAccount apiKeyOwner = userAccountDao.getUserByApiKey(apiKey);
         if (apiKeyOwner != null) {
-            User userThatHasTheProvidedApiKey = apiKeyOwner.first();
-            if (userThatHasTheProvidedApiKey.getId() != user.getId()) {
+            if (apiKeyOwner.getId() != user.getId()) {
                 throw new InvalidParameterValueException(String.format("The API key [%s] already exists in the system. Please provide a unique key.", apiKey));
             }
         }
@@ -1952,7 +1940,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
             CallContext.current().putContextParameter(User.class, user.getUuid());
 
             // user successfully disabled
-            return _userAccountDao.findById(userId);
+            return userAccountDao.findById(userId);
         } else {
             throw new CloudRuntimeException(String.format("Unable to disable user %s", user));
         }
@@ -2006,7 +1994,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
 
             CallContext.current().putContextParameter(User.class, user.getUuid());
 
-            return _userAccountDao.findById(userId);
+            return userAccountDao.findById(userId);
         } else {
             throw new CloudRuntimeException(String.format("Unable to enable user %s", user));
         }
@@ -2047,7 +2035,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
         boolean success;
         if (user.getState().equals(State.LOCKED)) {
             // already locked...no-op
-            return _userAccountDao.findById(userId);
+            return userAccountDao.findById(userId);
         } else if (user.getState().equals(State.ENABLED)) {
             success = doSetUserStatus(user.getId(), State.LOCKED);
 
@@ -2074,7 +2062,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
 
             CallContext.current().putContextParameter(User.class, user.getUuid());
 
-            return _userAccountDao.findById(userId);
+            return userAccountDao.findById(userId);
         } else {
             throw new CloudRuntimeException(String.format("Unable to lock user %s", user));
         }
@@ -2602,7 +2590,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
 
             return owner;
         } else if (!isAdmin(caller.getId()) && accountName != null && domainId != null) {
-            if (!accountName.equals(caller.getAccountName()) || domainId.longValue() != caller.getDomainId()) {
+            if (!accountName.equals(caller.getAccountName()) || domainId != caller.getDomainId()) {
                 throw new PermissionDeniedException("Can't create/list resources for account " + accountName + " in domain " + domainId + ", permission denied");
             } else {
                 return caller;
@@ -2627,12 +2615,12 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
 
     @Override
     public UserAccount getActiveUserAccount(String username, Long domainId) {
-        return _userAccountDao.getUserAccount(username, domainId);
+        return userAccountDao.getUserAccount(username, domainId);
     }
 
     @Override
     public List<UserAccount> getActiveUserAccountByEmail(String email, Long domainId) {
-        List<UserAccountVO> userAccountByEmail = _userAccountDao.getUserAccountByEmail(email, domainId);
+        List<UserAccountVO> userAccountByEmail = userAccountDao.getUserAccountByEmail(email, domainId);
         List<UserAccount> userAccounts = userAccountByEmail.stream()
                 .map(userAccountVO -> (UserAccount) userAccountVO)
                 .collect(Collectors.toList());
@@ -2676,7 +2664,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
     public void markUserRegistered(long userId) {
         UserVO userForUpdate = _userDao.createForUpdate();
         userForUpdate.setRegistered(true);
-        _userDao.update(Long.valueOf(userId), userForUpdate);
+        _userDao.update(userId, userForUpdate);
     }
 
     @Override
@@ -2731,7 +2719,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
                     throw new CloudRuntimeException(String.format("Failed to create account name %s in domain id=%s", accountName, _domainMgr.getDomain(domainId)));
                 }
 
-                Long accountId = account.getId();
+                long accountId = account.getId();
 
                 if (details != null) {
                     _accountDetailsDao.persist(accountId, details);
@@ -2780,7 +2768,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
 
     @Override
     public void logoutUser(long userId) {
-        UserAccount userAcct = _userAccountDao.findById(userId);
+        UserAccount userAcct = userAccountDao.findById(userId);
         if (userAcct != null) {
             ActionEventUtils.onActionEvent(userId, userAcct.getAccountId(), userAcct.getDomainId(), EventTypes.EVENT_USER_LOGOUT, "user has logged out", userId, ApiCommandResourceType.User.toString());
         } // else log some kind of error event? This likely means the user doesn't exist, or has been deleted...
@@ -2822,11 +2810,11 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
             final Boolean ApiSourceCidrChecksEnabled = ApiServiceConfiguration.ApiSourceCidrChecksEnabled.value();
 
             if (ApiSourceCidrChecksEnabled) {
-                logger.debug("CIDRs from which account '" + account.toString() + "' is allowed to perform API calls: " + accessAllowedCidrs);
+                logger.debug("CIDRs from which account '{}' is allowed to perform API calls: {}", account.toString(), accessAllowedCidrs);
 
                 // Block when is not in the list of allowed IPs
                 if (!NetUtils.isIpInCidrList(loginIpAddress, accessAllowedCidrs.split(","))) {
-                    logger.warn("Request by account '" + account.toString() + "' was denied since " + loginIpAddress.toString().replace("/", "") + " does not match " + accessAllowedCidrs);
+                    logger.warn("Request by account '{}' was denied since {} does not match {}", account.toString(), loginIpAddress.toString().replace("/", ""), accessAllowedCidrs);
                     throw new CloudAuthenticationException("Failed to authenticate user '" + username + "' in domain '" + domain.getPath() + "' from ip "
                             + loginIpAddress.toString().replace("/", "") + "; please provide valid credentials");
                 }
@@ -2858,6 +2846,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
                 try {
                     Thread.sleep(waitTimeDurationInMs);
                 } catch (final InterruptedException e) {
+                    // ignored
                 }
             }
 
@@ -2869,7 +2858,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
         if (logger.isDebugEnabled()) {
             logger.debug("Attempting to log in user: " + username + " in domain " + domainId);
         }
-        UserAccount userAccount = _userAccountDao.getUserAccount(username, domainId);
+        UserAccount userAccount = userAccountDao.getUserAccount(username, domainId);
 
         boolean authenticated = false;
         HashSet<ActionOnFailedAuthentication> actionsOnFailedAuthenticaion = new HashSet<>();
@@ -2899,11 +2888,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
 
         if (authenticated) {
             Domain domain = _domainMgr.getDomain(domainId);
-            String domainName = null;
-            if (domain != null) {
-                domainName = domain.getName();
-            }
-            userAccount = _userAccountDao.getUserAccount(username, domainId);
+            userAccount = userAccountDao.getUserAccount(username, domainId);
 
             if (!userAccount.getState().equalsIgnoreCase(Account.State.ENABLED.toString()) || !userAccount.getAccountState().equalsIgnoreCase(Account.State.ENABLED.toString())) {
                 if (logger.isInfoEnabled()) {
@@ -2963,11 +2948,9 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
 
         // - build a request string with sorted params, make sure it's all lowercase
         // - sign the request, verify the signature is the same
-        List<String> parameterNames = new ArrayList<>();
 
-        for (Object paramNameObj : requestParameters.keySet()) {
-            parameterNames.add((String)paramNameObj); // put the name in a list that we'll sort later
-        }
+        // put the name in a list that we'll sort later
+        List<String> parameterNames = new ArrayList<>(requestParameters.keySet());
 
         Collections.sort(parameterNames);
 
@@ -2999,7 +2982,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
                     if (unsignedRequestBuffer.length() != 0) {
                         unsignedRequestBuffer.append("&");
                     }
-                    unsignedRequestBuffer.append(paramName).append("=").append(URLEncoder.encode(paramValue, "UTF-8"));
+                    unsignedRequestBuffer.append(paramName).append("=").append(URLEncoder.encode(paramValue, com.cloud.utils.StringUtils.getPreferredCharset()));
                 }
             }
 
@@ -3022,7 +3005,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
             if (!equalSig) {
                 logger.info("User signature: " + signature + " is not equaled to computed signature: " + computedSignature);
             } else {
-                user = _userAccountDao.getUserAccount(username, domainId);
+                user = userAccountDao.getUserAccount(username, domainId);
             }
         } catch (Exception ex) {
             logger.error("Exception authenticating user", ex);
@@ -3050,7 +3033,14 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
 
     @Override
     public Pair<User, Account> findUserByApiKey(String apiKey) {
-        return _accountDao.findUserAccountByApiKey(apiKey);
+        UserAccount userAccount = userAccountDao.getUserByApiKey(apiKey);
+        if (userAccount != null) {
+            User user = _userDao.getUser(userAccount.getId());
+            Account account = _accountDao.findById(userAccount.getAccountId());
+            return new Pair<>(user, account);
+        } else {
+            return null;
+        }
     }
 
     @Override
@@ -3184,14 +3174,14 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
             UserVO updatedUser = _userDao.createForUpdate();
 
             String encodedKey;
-            Pair<User, Account> userAcct;
+            UserAccount userAcct;
             int retryLimit = 10;
             do {
                 // FIXME: what algorithm should we use for API keys?
                 KeyGenerator generator = KeyGenerator.getInstance("HmacSHA1");
                 SecretKey key = generator.generateKey();
                 encodedKey = Base64.encodeBase64URLSafeString(key.getEncoded());
-                userAcct = _accountDao.findUserAccountByApiKey(encodedKey);
+                userAcct = userAccountDao.getUserByApiKey(encodedKey);
                 retryLimit--;
             } while ((userAcct != null) && (retryLimit >= 0));
 
@@ -3202,7 +3192,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
             _userDao.update(userId, updatedUser);
             return encodedKey;
         } catch (NoSuchAlgorithmException ex) {
-            logger.error("error generating secret key for user {}", _userAccountDao.findById(userId), ex);
+            logger.error("error generating secret key for user {}", userAccountDao.findById(userId), ex);
         }
         return null;
     }
@@ -3229,7 +3219,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
             _userDao.update(userId, updatedUser);
             return encodedKey;
         } catch (NoSuchAlgorithmException ex) {
-            logger.error("error generating secret key for user {}", _userAccountDao.findById(userId), ex);
+            logger.error("error generating secret key for user {}", userAccountDao.findById(userId), ex);
         }
         return null;
     }
@@ -3440,12 +3430,12 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
 
     @Override
     public UserAccount getUserByApiKey(String apiKey) {
-        return _userAccountDao.getUserByApiKey(apiKey);
+        return userAccountDao.getUserByApiKey(apiKey);
     }
 
     @Override
     public List<String> listAclGroupsByAccount(Long accountId) {
-        if (_querySelectors == null || _querySelectors.size() == 0) {
+        if (CollectionUtils.isEmpty(_querySelectors)) {
             return new ArrayList<>();
         }
 
@@ -3500,7 +3490,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
 
     @Override
     public UserAccount getUserAccountById(Long userId) {
-        UserAccount userAccount = _userAccountDao.findById(userId);
+        UserAccount userAccount = userAccountDao.findById(userId);
         Map<String, String> details = _userDetailsDao.listDetailsKeyPairs(userId);
         userAccount.setDetails(details);
 
@@ -3674,7 +3664,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
     }
 
     protected UserTwoFactorAuthenticationSetupResponse enableTwoFactorAuthentication(Long userId, String providerName) {
-        UserAccountVO userAccount = _userAccountDao.findById(userId);
+        UserAccountVO userAccount = userAccountDao.findById(userId);
         UserVO userVO = _userDao.findById(userId);
         Long domainId = userAccount.getDomainId();
         if (Boolean.FALSE.equals(enableUserTwoFactorAuthentication.valueIn(domainId)) && Boolean.FALSE.equals(mandateUserTwoFactorAuthentication.valueIn(domainId))) {
@@ -3766,11 +3756,11 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
             if (userDetailVO != null) {
                 _userDetailsDao.remove(userDetailVO.getId());
             }
-            UserAccountVO userAccountVO = _userAccountDao.findById(user.getId());
+            UserAccountVO userAccountVO = userAccountDao.findById(user.getId());
             userAccountVO.setUser2faEnabled(false);
             userAccountVO.setUser2faProvider(null);
             userAccountVO.setKeyFor2fa(null);
-            _userAccountDao.update(user.getId(), userAccountVO);
+            userAccountDao.update(user.getId(), userAccountVO);
             return userAccountVO;
         });
     }

server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java

@@ -23,6 +23,7 @@ import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
@@ -182,6 +183,11 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme
             Long.class, "vm.job.check.interval", "3000",
             "Interval in milliseconds to check if the job is complete", false);
 
+    private static final Set<String> VM_SNAPSHOT_CUSTOM_SERVICE_OFFERING_DETAILS = Set.of(
+            VmDetailConstants.CPU_NUMBER.toLowerCase(),
+            VmDetailConstants.CPU_SPEED.toLowerCase(),
+            VmDetailConstants.MEMORY.toLowerCase());
+
     @Override
     public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
         _name = name;
@@ -473,7 +479,8 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme
     }
 
     /**
-     * Add entries on vm_snapshot_details if service offering is dynamic. This will allow setting details when revert to vm snapshot
+     * Add entries about cpu, cpu_speed and memory in vm_snapshot_details if service offering is dynamic.
+     * This will allow setting details when revert to vm snapshot.
      * @param vmId vm id
      * @param serviceOfferingId service offering id
      * @param vmSnapshotId vm snapshot id
@@ -484,7 +491,7 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme
             List<UserVmDetailVO> vmDetails = _userVmDetailsDao.listDetails(vmId);
             List<VMSnapshotDetailsVO> vmSnapshotDetails = new ArrayList<VMSnapshotDetailsVO>();
             for (UserVmDetailVO detail : vmDetails) {
-                if(detail.getName().equalsIgnoreCase(VmDetailConstants.CPU_NUMBER) || detail.getName().equalsIgnoreCase(VmDetailConstants.CPU_SPEED) || detail.getName().equalsIgnoreCase(VmDetailConstants.MEMORY)) {
+                if (VM_SNAPSHOT_CUSTOM_SERVICE_OFFERING_DETAILS.contains(detail.getName().toLowerCase())) {
                     vmSnapshotDetails.add(new VMSnapshotDetailsVO(vmSnapshotId, detail.getName(), detail.getValue(), detail.isDisplay()));
                 }
             }
@@ -931,7 +938,7 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme
             Transaction.execute(new TransactionCallbackWithExceptionNoReturn<CloudRuntimeException>() {
                 @Override
                 public void doInTransactionWithoutResult(TransactionStatus status) throws CloudRuntimeException {
-                    revertUserVmDetailsFromVmSnapshot(userVm, vmSnapshotVo);
+                    revertCustomServiceOfferingDetailsFromVmSnapshot(userVm, vmSnapshotVo);
                     updateUserVmServiceOffering(userVm, vmSnapshotVo);
                 }
             });
@@ -943,19 +950,19 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme
     }
 
     /**
-     * Update or add user vm details from vm snapshot for vms with custom service offerings
+     * Update or add user vm details (cpu, cpu_speed and memory) from vm snapshot for vms with custom service offerings
      * @param userVm user vm
      * @param vmSnapshotVo vm snapshot
      */
-    protected void revertUserVmDetailsFromVmSnapshot(UserVmVO userVm, VMSnapshotVO vmSnapshotVo) {
+    protected void revertCustomServiceOfferingDetailsFromVmSnapshot(UserVmVO userVm, VMSnapshotVO vmSnapshotVo) {
         ServiceOfferingVO serviceOfferingVO = _serviceOfferingDao.findById(vmSnapshotVo.getServiceOfferingId());
         if (serviceOfferingVO.isDynamic()) {
             List<VMSnapshotDetailsVO> vmSnapshotDetails = _vmSnapshotDetailsDao.listDetails(vmSnapshotVo.getId());
-            List<UserVmDetailVO> userVmDetails = new ArrayList<UserVmDetailVO>();
             for (VMSnapshotDetailsVO detail : vmSnapshotDetails) {
-                userVmDetails.add(new UserVmDetailVO(userVm.getId(), detail.getName(), detail.getValue(), detail.isDisplay()));
+                if (VM_SNAPSHOT_CUSTOM_SERVICE_OFFERING_DETAILS.contains(detail.getName().toLowerCase())) {
+                    _userVmDetailsDao.addDetail(userVm.getId(), detail.getName(), detail.getValue(), detail.isDisplay());
+                }
             }
-            _userVmDetailsDao.saveDetails(userVmDetails);
         }
     }
 

server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java

@@ -826,7 +826,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager {
             throw new CloudRuntimeException(String.format("Error restoring volume [%s] of VM [%s] to host [%s] using backup provider [%s] due to: [%s].",
                     backedUpVolumeUuid, vm.getUuid(), host.getUuid(), backupProvider.getName(), result.second()));
         }
-        if (!attachVolumeToVM(vm.getDataCenterId(), result.second(), vmFromBackup.getBackupVolumeList(),
+        if (!attachVolumeToVM(vm.getDataCenterId(), result.second(), backup.getBackedUpVolumes(),
                             backedUpVolumeUuid, vm, datastore.getUuid(), backup)) {
             throw new CloudRuntimeException(String.format("Error attaching volume [%s] to VM [%s]." + backedUpVolumeUuid, vm.getUuid()));
         }
@@ -972,10 +972,10 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager {
         if (StringUtils.isEmpty(name)) {
             throw new CloudRuntimeException("Invalid backup provider name provided");
         }
-        if (!backupProvidersMap.containsKey(name)) {
-            throw new CloudRuntimeException("Failed to find backup provider by the name: " + name);
-        }
-        return backupProvidersMap.get(name);
+       if (!backupProvidersMap.containsKey(name)) {
+           throw new CloudRuntimeException("Failed to find backup provider by the name: " + name);
+       }
+       return backupProvidersMap.get(name);
     }
 
     @Override

server/src/main/java/org/apache/cloudstack/storage/heuristics/HeuristicRuleHelper.java

@@ -139,23 +139,23 @@ public class HeuristicRuleHelper {
      * @param presetVariables used for injecting in the JS interpreter.
      */
     protected void injectPresetVariables(JsInterpreter jsInterpreter, PresetVariables presetVariables) {
-        jsInterpreter.injectVariable("secondaryStorages", presetVariables.getSecondaryStorages().toString());
+        jsInterpreter.injectVariable("secondaryStorages", presetVariables.getSecondaryStorages());
 
         if (presetVariables.getTemplate() != null) {
-            jsInterpreter.injectVariable("template", presetVariables.getTemplate().toString());
-            jsInterpreter.injectVariable("iso", presetVariables.getTemplate().toString());
+            jsInterpreter.injectVariable("template", presetVariables.getTemplate());
+            jsInterpreter.injectVariable("iso", presetVariables.getTemplate());
         }
 
         if (presetVariables.getSnapshot() != null) {
-            jsInterpreter.injectVariable("snapshot", presetVariables.getSnapshot().toString());
+            jsInterpreter.injectVariable("snapshot", presetVariables.getSnapshot());
         }
 
         if (presetVariables.getVolume() != null) {
-            jsInterpreter.injectVariable("volume", presetVariables.getVolume().toString());
+            jsInterpreter.injectVariable("volume", presetVariables.getVolume());
         }
 
         if (presetVariables.getAccount() != null) {
-            jsInterpreter.injectVariable("account", presetVariables.getAccount().toString());
+            jsInterpreter.injectVariable("account", presetVariables.getAccount());
         }
     }
 
@@ -185,8 +185,8 @@ public class HeuristicRuleHelper {
         Template template = new Template();
 
         template.setName(templateVO.getName());
-        template.setFormat(templateVO.getFormat());
-        template.setHypervisorType(templateVO.getHypervisorType());
+        template.setFormat(templateVO.getFormat().toString());
+        template.setHypervisorType(templateVO.getHypervisorType().toString());
 
         return template;
     }
@@ -195,7 +195,7 @@ public class HeuristicRuleHelper {
         Volume volumePresetVariable = new Volume();
 
         volumePresetVariable.setName(volumeVO.getName());
-        volumePresetVariable.setFormat(volumeVO.getFormat());
+        volumePresetVariable.setFormat(volumeVO.getFormat().toString());
         volumePresetVariable.setSize(volumeVO.getSize());
 
         return volumePresetVariable;
@@ -206,7 +206,7 @@ public class HeuristicRuleHelper {
 
         snapshot.setName(snapshotInfo.getName());
         snapshot.setSize(snapshotInfo.getSize());
-        snapshot.setHypervisorType(snapshotInfo.getHypervisorType());
+        snapshot.setHypervisorType(snapshotInfo.getHypervisorType().toString());
 
         return snapshot;
     }

server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Account.java

@@ -27,7 +27,6 @@ public class Account extends GenericHeuristicPresetVariable {
 
     public void setId(String id) {
         this.id = id;
-        fieldNamesToIncludeInToString.add("id");
     }
 
     public Domain getDomain() {
@@ -36,6 +35,5 @@ public class Account extends GenericHeuristicPresetVariable {
 
     public void setDomain(Domain domain) {
         this.domain = domain;
-        fieldNamesToIncludeInToString.add("domain");
     }
 }

server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Domain.java

@@ -25,6 +25,5 @@ public class Domain extends GenericHeuristicPresetVariable {
 
     public void setId(String id) {
         this.id = id;
-        fieldNamesToIncludeInToString.add("id");
     }
 }

server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/GenericHeuristicPresetVariable.java

@@ -16,15 +16,11 @@
 // under the License.
 package org.apache.cloudstack.storage.heuristics.presetvariables;
 
-import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
-
-import java.util.HashSet;
-import java.util.Set;
+import org.apache.commons.lang3.builder.ToStringBuilder;
+import org.apache.commons.lang3.builder.ToStringStyle;
 
 public class GenericHeuristicPresetVariable {
 
-    protected transient Set<String> fieldNamesToIncludeInToString = new HashSet<>();
-
     private String name;
 
     public String getName() {
@@ -33,15 +29,10 @@ public class GenericHeuristicPresetVariable {
 
     public void setName(String name) {
         this.name = name;
-        fieldNamesToIncludeInToString.add("name");
     }
 
-    /***
-     * Converts the preset variable into a valid JSON object that will be injected into the JS interpreter.
-     * This method should not be overridden or changed.
-     */
     @Override
-    public final String toString() {
-        return ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, fieldNamesToIncludeInToString.toArray(new String[0]));
+    public String toString() {
+        return ToStringBuilder.reflectionToString(this, ToStringStyle.JSON_STYLE);
     }
 }

server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/SecondaryStorage.java

@@ -32,7 +32,6 @@ public class SecondaryStorage extends GenericHeuristicPresetVariable {
 
     public void setId(String id) {
         this.id = id;
-        fieldNamesToIncludeInToString.add("id");
     }
 
     public Long getUsedDiskSize() {
@@ -41,7 +40,6 @@ public class SecondaryStorage extends GenericHeuristicPresetVariable {
 
     public void setUsedDiskSize(Long usedDiskSize) {
         this.usedDiskSize = usedDiskSize;
-        fieldNamesToIncludeInToString.add("usedDiskSize");
     }
 
     public Long getTotalDiskSize() {
@@ -50,7 +48,6 @@ public class SecondaryStorage extends GenericHeuristicPresetVariable {
 
     public void setTotalDiskSize(Long totalDiskSize) {
         this.totalDiskSize = totalDiskSize;
-        fieldNamesToIncludeInToString.add("totalDiskSize");
     }
 
     public String getProtocol() {
@@ -59,6 +56,5 @@ public class SecondaryStorage extends GenericHeuristicPresetVariable {
 
     public void setProtocol(String protocol) {
         this.protocol = protocol;
-        fieldNamesToIncludeInToString.add("protocol");
     }
 }

server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Snapshot.java

@@ -16,13 +16,11 @@
 // under the License.
 package org.apache.cloudstack.storage.heuristics.presetvariables;
 
-import com.cloud.hypervisor.Hypervisor;
-
 public class Snapshot extends GenericHeuristicPresetVariable {
 
     private Long size;
 
-    private Hypervisor.HypervisorType hypervisorType;
+    private String hypervisorType;
 
     public Long getSize() {
         return size;
@@ -30,15 +28,13 @@ public class Snapshot extends GenericHeuristicPresetVariable {
 
     public void setSize(Long size) {
         this.size = size;
-        fieldNamesToIncludeInToString.add("size");
     }
 
-    public Hypervisor.HypervisorType getHypervisorType() {
+    public String getHypervisorType() {
         return hypervisorType;
     }
 
-    public void setHypervisorType(Hypervisor.HypervisorType hypervisorType) {
+    public void setHypervisorType(String hypervisorType) {
         this.hypervisorType = hypervisorType;
-        fieldNamesToIncludeInToString.add("hypervisorType");
     }
 }

server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Template.java

@@ -16,41 +16,35 @@
 // under the License.
 package org.apache.cloudstack.storage.heuristics.presetvariables;
 
-import com.cloud.hypervisor.Hypervisor;
-import com.cloud.storage.Storage;
-
 public class Template extends GenericHeuristicPresetVariable {
 
-    private Hypervisor.HypervisorType hypervisorType;
+    private String hypervisorType;
 
-    private Storage.ImageFormat format;
+    private String format;
 
-    private Storage.TemplateType templateType;
+    private String templateType;
 
-    public Hypervisor.HypervisorType getHypervisorType() {
+    public String getHypervisorType() {
         return hypervisorType;
     }
 
-    public void setHypervisorType(Hypervisor.HypervisorType hypervisorType) {
+    public void setHypervisorType(String hypervisorType) {
         this.hypervisorType = hypervisorType;
-        fieldNamesToIncludeInToString.add("hypervisorType");
     }
 
-    public Storage.ImageFormat getFormat() {
+    public String getFormat() {
         return format;
     }
 
-    public void setFormat(Storage.ImageFormat format) {
+    public void setFormat(String format) {
         this.format = format;
-        fieldNamesToIncludeInToString.add("format");
     }
 
-    public Storage.TemplateType getTemplateType() {
+    public String getTemplateType() {
         return templateType;
     }
 
-    public void setTemplateType(Storage.TemplateType templateType) {
+    public void setTemplateType(String templateType) {
         this.templateType = templateType;
-        fieldNamesToIncludeInToString.add("templateType");
     }
 }

server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Volume.java

@@ -16,13 +16,11 @@
 // under the License.
 package org.apache.cloudstack.storage.heuristics.presetvariables;
 
-import com.cloud.storage.Storage;
-
 public class Volume extends GenericHeuristicPresetVariable {
 
     private Long size;
 
-    private Storage.ImageFormat format;
+    private String format;
 
     public Long getSize() {
         return size;
@@ -30,15 +28,13 @@ public class Volume extends GenericHeuristicPresetVariable {
 
     public void setSize(Long size) {
         this.size = size;
-        fieldNamesToIncludeInToString.add("size");
     }
 
-    public Storage.ImageFormat getFormat() {
+    public String getFormat() {
         return format;
     }
 
-    public void setFormat(Storage.ImageFormat format) {
+    public void setFormat(String format) {
         this.format = format;
-        fieldNamesToIncludeInToString.add("format");
     }
 }

server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java

@@ -460,6 +460,11 @@ public class MockResourceManagerImpl extends ManagerBase implements ResourceMana
         return null;
     }
 
+    @Override
+    public HostVO findHostByGuidPrefix(String guid) {
+        return null;
+    }
+
     /* (non-Javadoc)
      * @see com.cloud.resource.ResourceManager#findHostByName(java.lang.String)
      */

server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java

@@ -360,12 +360,14 @@ public class ResourceManagerImplTest {
 
     @Test
     public void testConnectAndRestartAgentOnHost() {
+        when(agentManager.getHostSshPort(any())).thenReturn(22);
         resourceManager.connectAndRestartAgentOnHost(host, hostUsername, hostPassword, hostPrivateKey);
     }
 
     @Test
     public void testHandleAgentSSHEnabledNotConnectedAgent() {
         when(host.getStatus()).thenReturn(Status.Disconnected);
+        when(agentManager.getHostSshPort(any())).thenReturn(22);
         resourceManager.handleAgentIfNotConnected(host, false);
         verify(resourceManager).getHostCredentials(eq(host));
         verify(resourceManager).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword), eq(hostPrivateKey));

server/src/test/java/com/cloud/user/AccountManagentImplTestBase.java

@@ -84,7 +84,7 @@ import java.util.HashMap;
 import java.util.Map;
 
 @RunWith(MockitoJUnitRunner.class)
-public class AccountManagetImplTestBase {
+public class AccountManagentImplTestBase {
 
     @Mock
     AccountDao _accountDao;
@@ -99,7 +99,7 @@ public class AccountManagetImplTestBase {
     @Mock
     InstanceGroupDao _vmGroupDao;
     @Mock
-    UserAccountDao userAccountDaoMock;
+    UserAccountDao userAccountDao;
     @Mock
     VolumeDao _volumeDao;
     @Mock
@@ -210,9 +210,6 @@ public class AccountManagetImplTestBase {
     @Mock
     RoutedIpv4Manager routedIpv4Manager;
 
-    @Mock
-    Account accountMock;
-
     @Before
     public void setup() {
         accountManagerImpl.setUserAuthenticators(Arrays.asList(userAuthenticator));
@@ -228,7 +225,6 @@ public class AccountManagetImplTestBase {
     @Test
     public void test()
     {
-        return;
     }
 
     public static Map<String, Field> getInheritedFields(Class<?> type) {

server/src/test/java/com/cloud/user/AccountManagerImplTest.java

@@ -47,13 +47,11 @@ import org.apache.cloudstack.webhook.WebhookHelper;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
-import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
 import org.mockito.InOrder;
 import org.mockito.Mock;
 import org.mockito.MockedStatic;
 import org.mockito.Mockito;
-import org.mockito.junit.MockitoJUnitRunner;
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 
 import com.cloud.acl.DomainChecker;
@@ -76,8 +74,7 @@ import com.cloud.vm.UserVmVO;
 import com.cloud.vm.VMInstanceVO;
 import com.cloud.vm.snapshot.VMSnapshotVO;
 
-@RunWith(MockitoJUnitRunner.class)
-public class AccountManagerImplTest extends AccountManagetImplTestBase {
+public class AccountManagerImplTest extends AccountManagentImplTestBase {
 
     @Mock
     private UserVmManagerImpl _vmMgr;
@@ -100,11 +97,11 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
     @Mock
     private UpdateAccountCmd UpdateAccountCmdMock;
 
-    private long userVoIdMock = 111l;
+    private final long userVoIdMock = 111L;
     @Mock
     private UserVO userVoMock;
 
-    private long accountMockId = 100l;
+    private final long accountMockId = 100L;
 
     @Mock
     private Account accountMock;
@@ -154,7 +151,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
 
     @Test
     public void disableAccountNotexisting() throws ConcurrentOperationException, ResourceUnavailableException {
-        Mockito.when(_accountDao.findById(42l)).thenReturn(null);
+        Mockito.when(_accountDao.findById(42L)).thenReturn(null);
         Assert.assertTrue(accountManagerImpl.disableAccount(42));
     }
 
@@ -162,7 +159,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
     public void disableAccountDisabled() throws ConcurrentOperationException, ResourceUnavailableException {
         AccountVO disabledAccount = new AccountVO();
         disabledAccount.setState(State.DISABLED);
-        Mockito.when(_accountDao.findById(42l)).thenReturn(disabledAccount);
+        Mockito.when(_accountDao.findById(42L)).thenReturn(disabledAccount);
         Assert.assertTrue(accountManagerImpl.disableAccount(42));
     }
 
@@ -170,22 +167,22 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
     public void disableAccount() throws ConcurrentOperationException, ResourceUnavailableException {
         AccountVO account = new AccountVO();
         account.setState(State.ENABLED);
-        Mockito.when(_accountDao.findById(42l)).thenReturn(account);
+        Mockito.when(_accountDao.findById(42L)).thenReturn(account);
         Mockito.when(_accountDao.createForUpdate()).thenReturn(new AccountVO());
-        Mockito.when(_accountDao.update(Mockito.eq(42l), Mockito.any(AccountVO.class))).thenReturn(true);
-        Mockito.when(_vmDao.listByAccountId(42l)).thenReturn(Arrays.asList(Mockito.mock(VMInstanceVO.class)));
+        Mockito.when(_accountDao.update(Mockito.eq(42L), Mockito.any(AccountVO.class))).thenReturn(true);
+        Mockito.when(_vmDao.listByAccountId(42L)).thenReturn(Arrays.asList(Mockito.mock(VMInstanceVO.class)));
         Assert.assertTrue(accountManagerImpl.disableAccount(42));
-        Mockito.verify(_accountDao, Mockito.atLeastOnce()).update(Mockito.eq(42l), Mockito.any(AccountVO.class));
+        Mockito.verify(_accountDao, Mockito.atLeastOnce()).update(Mockito.eq(42L), Mockito.any(AccountVO.class));
     }
 
     @Test
     public void deleteUserAccount() {
         AccountVO account = new AccountVO();
-        account.setId(42l);
+        account.setId(42L);
         DomainVO domain = new DomainVO();
-        Mockito.when(_accountDao.findById(42l)).thenReturn(account);
+        Mockito.when(_accountDao.findById(42L)).thenReturn(account);
         Mockito.doNothing().when(accountManagerImpl).checkAccess(Mockito.any(Account.class), Mockito.isNull(), Mockito.anyBoolean(), Mockito.any(Account.class));
-        Mockito.when(_accountDao.remove(42l)).thenReturn(true);
+        Mockito.when(_accountDao.remove(42L)).thenReturn(true);
         Mockito.when(_configMgr.releaseAccountSpecificVirtualRanges(account)).thenReturn(true);
         Mockito.lenient().when(_domainMgr.getDomain(Mockito.anyLong())).thenReturn(domain);
         Mockito.lenient().when(securityChecker.checkAccess(Mockito.any(Account.class), Mockito.any(Domain.class))).thenReturn(true);
@@ -194,7 +191,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
 
         List<SSHKeyPairVO> sshkeyList = new ArrayList<SSHKeyPairVO>();
         SSHKeyPairVO sshkey = new SSHKeyPairVO();
-        sshkey.setId(1l);
+        sshkey.setId(1L);
         sshkeyList.add(sshkey);
         Mockito.when(_sshKeyPairDao.listKeyPairs(Mockito.anyLong(), Mockito.anyLong())).thenReturn(sshkeyList);
         Mockito.when(_sshKeyPairDao.remove(Mockito.anyLong())).thenReturn(true);
@@ -202,30 +199,30 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
         Mockito.doNothing().when(accountManagerImpl).deleteWebhooksForAccount(Mockito.anyLong());
         Mockito.doNothing().when(accountManagerImpl).verifyCallerPrivilegeForUserOrAccountOperations((Account) any());
 
-        Assert.assertTrue(accountManagerImpl.deleteUserAccount(42l));
+        Assert.assertTrue(accountManagerImpl.deleteUserAccount(42L));
         // assert that this was a clean delete
-        Mockito.verify(_accountDao, Mockito.never()).markForCleanup(Mockito.eq(42l));
+        Mockito.verify(_accountDao, Mockito.never()).markForCleanup(Mockito.eq(42L));
     }
 
     @Test
     public void deleteUserAccountCleanup() {
         AccountVO account = new AccountVO();
-        account.setId(42l);
+        account.setId(42L);
         DomainVO domain = new DomainVO();
-        Mockito.when(_accountDao.findById(42l)).thenReturn(account);
+        Mockito.when(_accountDao.findById(42L)).thenReturn(account);
         Mockito.doNothing().when(accountManagerImpl).checkAccess(Mockito.any(Account.class), Mockito.isNull(), Mockito.anyBoolean(), Mockito.any(Account.class));
-        Mockito.when(_accountDao.remove(42l)).thenReturn(true);
+        Mockito.when(_accountDao.remove(42L)).thenReturn(true);
         Mockito.when(_configMgr.releaseAccountSpecificVirtualRanges(account)).thenReturn(true);
-        Mockito.when(_userVmDao.listByAccountId(42l)).thenReturn(Arrays.asList(Mockito.mock(UserVmVO.class)));
+        Mockito.when(_userVmDao.listByAccountId(42L)).thenReturn(Arrays.asList(Mockito.mock(UserVmVO.class)));
         Mockito.when(_vmMgr.expunge(Mockito.any(UserVmVO.class))).thenReturn(false);
         Mockito.lenient().when(_domainMgr.getDomain(Mockito.anyLong())).thenReturn(domain);
         Mockito.lenient().when(securityChecker.checkAccess(Mockito.any(Account.class), Mockito.any(Domain.class))).thenReturn(true);
         Mockito.doNothing().when(accountManagerImpl).deleteWebhooksForAccount(Mockito.anyLong());
         Mockito.doNothing().when(accountManagerImpl).verifyCallerPrivilegeForUserOrAccountOperations((Account) any());
 
-        Assert.assertTrue(accountManagerImpl.deleteUserAccount(42l));
+        Assert.assertTrue(accountManagerImpl.deleteUserAccount(42L));
         // assert that this was NOT a clean delete
-        Mockito.verify(_accountDao, Mockito.atLeastOnce()).markForCleanup(Mockito.eq(42l));
+        Mockito.verify(_accountDao, Mockito.atLeastOnce()).markForCleanup(Mockito.eq(42L));
     }
 
     @Test (expected = InvalidParameterValueException.class)
@@ -308,7 +305,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
         UserAccountVO userAccountVO = new UserAccountVO();
         userAccountVO.setSource(User.Source.UNKNOWN);
         userAccountVO.setState(Account.State.DISABLED.toString());
-        Mockito.when(userAccountDaoMock.getUserAccount("test", 1L)).thenReturn(userAccountVO);
+        Mockito.when(userAccountDao.getUserAccount("test", 1L)).thenReturn(userAccountVO);
         Mockito.when(userAuthenticator.authenticate("test", "fail", 1L, new HashMap<>())).thenReturn(failureAuthenticationPair);
         Mockito.lenient().when(userAuthenticator.authenticate("test", null, 1L, new HashMap<>())).thenReturn(successAuthenticationPair);
         Mockito.lenient().when(userAuthenticator.authenticate("test", "", 1L, new HashMap<>())).thenReturn(successAuthenticationPair);
@@ -337,7 +334,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
         CallContext.register(callingUser, callingAccount); // Calling account is user account i.e normal account
         Mockito.when(_listkeyscmd.getID()).thenReturn(1L);
         Mockito.when(accountManagerImpl.getActiveUser(1L)).thenReturn(userVoMock);
-        Mockito.when(userAccountDaoMock.findById(1L)).thenReturn(userAccountVO);
+        Mockito.when(userAccountDao.findById(1L)).thenReturn(userAccountVO);
         Mockito.when(userAccountVO.getAccountId()).thenReturn(1L);
         Mockito.lenient().when(accountManagerImpl.getAccount(Mockito.anyLong())).thenReturn(accountMock); // Queried account - admin account
 
@@ -355,7 +352,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
         CallContext.register(callingUser, callingAccount);
         Mockito.when(_listkeyscmd.getID()).thenReturn(2L);
         Mockito.when(accountManagerImpl.getActiveUser(2L)).thenReturn(userVoMock);
-        Mockito.when(userAccountDaoMock.findById(2L)).thenReturn(userAccountVO);
+        Mockito.when(userAccountDao.findById(2L)).thenReturn(userAccountVO);
         Mockito.when(userAccountVO.getAccountId()).thenReturn(2L);
         Mockito.when(userDetailsDaoMock.listDetailsKeyPairs(Mockito.anyLong())).thenReturn(null);
 
@@ -442,14 +439,14 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
         Mockito.doNothing().when(accountManagerImpl).validateUserPasswordAndUpdateIfNeeded(Mockito.anyString(), Mockito.eq(userVoMock), Mockito.anyString(), Mockito.eq(false));
 
         Mockito.doReturn(true).when(userDaoMock).update(Mockito.anyLong(), Mockito.eq(userVoMock));
-        Mockito.doReturn(Mockito.mock(UserAccountVO.class)).when(userAccountDaoMock).findById(Mockito.anyLong());
+        Mockito.doReturn(Mockito.mock(UserAccountVO.class)).when(userAccountDao).findById(Mockito.anyLong());
         Mockito.doNothing().when(accountManagerImpl).checkAccess(nullable(User.class), nullable(Account.class));
 
         accountManagerImpl.updateUser(UpdateUserCmdMock);
 
         Mockito.lenient().doNothing().when(accountManagerImpl).checkRoleEscalation(accountMock, accountMock);
 
-        InOrder inOrder = Mockito.inOrder(userVoMock, accountManagerImpl, userDaoMock, userAccountDaoMock);
+        InOrder inOrder = Mockito.inOrder(userVoMock, accountManagerImpl, userDaoMock, userAccountDao);
 
         inOrder.verify(accountManagerImpl).retrieveAndValidateUser(UpdateUserCmdMock);
         inOrder.verify(accountManagerImpl).retrieveAndValidateAccount(userVoMock);
@@ -464,7 +461,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
         inOrder.verify(userVoMock, Mockito.times(numberOfExpectedCallsForSetEmailAndSetTimeZone)).setTimezone(Mockito.anyString());
 
         inOrder.verify(userDaoMock).update(Mockito.anyLong(), Mockito.eq(userVoMock));
-        inOrder.verify(userAccountDaoMock).findById(Mockito.anyLong());
+        inOrder.verify(userAccountDao).findById(Mockito.anyLong());
     }
 
     @Test(expected = InvalidParameterValueException.class)
@@ -487,7 +484,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
     public void validateAndUpdatApiAndSecretKeyIfNeededTestNoKeys() {
         accountManagerImpl.validateAndUpdateApiAndSecretKeyIfNeeded(UpdateUserCmdMock, userVoMock);
 
-        Mockito.verify(_accountDao, Mockito.times(0)).findUserAccountByApiKey(Mockito.anyString());
+        Mockito.verify(userAccountDao, Mockito.times(0)).getUserByApiKey(Mockito.anyString());
     }
 
     @Test(expected = InvalidParameterValueException.class)
@@ -513,10 +510,9 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
         Mockito.doReturn(1L).when(userVoMock).getId();
 
         User otherUserMock = Mockito.mock(User.class);
-        Mockito.doReturn(2L).when(otherUserMock).getId();
 
-        Pair<User, Account> pairUserAccountMock = new Pair<User, Account>(otherUserMock, Mockito.mock(Account.class));
-        Mockito.doReturn(pairUserAccountMock).when(_accountDao).findUserAccountByApiKey(apiKey);
+        UserAccount UserAccountMock = Mockito.mock(UserAccount.class);
+        Mockito.doReturn(UserAccountMock).when(userAccountDao).getUserByApiKey(apiKey);
 
         accountManagerImpl.validateAndUpdateApiAndSecretKeyIfNeeded(UpdateUserCmdMock, userVoMock);
     }
@@ -529,17 +525,13 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
         String secretKey = "secretKey";
         Mockito.doReturn(secretKey).when(UpdateUserCmdMock).getSecretKey();
 
-        Mockito.doReturn(1L).when(userVoMock).getId();
-
         User otherUserMock = Mockito.mock(User.class);
-        Mockito.doReturn(1L).when(otherUserMock).getId();
 
-        Pair<User, Account> pairUserAccountMock = new Pair<User, Account>(otherUserMock, Mockito.mock(Account.class));
-        Mockito.doReturn(pairUserAccountMock).when(_accountDao).findUserAccountByApiKey(apiKey);
+        Mockito.doReturn(null).when(userAccountDao).getUserByApiKey(apiKey);
 
         accountManagerImpl.validateAndUpdateApiAndSecretKeyIfNeeded(UpdateUserCmdMock, userVoMock);
 
-        Mockito.verify(_accountDao).findUserAccountByApiKey(apiKey);
+        Mockito.verify(userAccountDao).getUserByApiKey(apiKey);
         Mockito.verify(userVoMock).setApiKey(apiKey);
         Mockito.verify(userVoMock).setSecretKey(secretKey);
     }
@@ -693,18 +685,18 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
 
     @Test(expected = InvalidParameterValueException.class)
     public void validateAndUpdateUsernameIfNeededTestDuplicatedUserSameDomainThisUser() {
-        long domanIdCurrentUser = 22l;
+        long domanIdCurrentUser = 22L;
 
         String userName = "username";
         Mockito.doReturn(userName).when(UpdateUserCmdMock).getUsername();
         Mockito.lenient().doReturn(userName).when(userVoMock).getUsername();
         Mockito.doReturn(domanIdCurrentUser).when(accountMock).getDomainId();
 
-        long userVoDuplicatedMockId = 67l;
+        long userVoDuplicatedMockId = 67L;
         UserVO userVoDuplicatedMock = Mockito.mock(UserVO.class);
         Mockito.doReturn(userVoDuplicatedMockId).when(userVoDuplicatedMock).getId();
 
-        long accountIdUserDuplicated = 98l;
+        long accountIdUserDuplicated = 98L;
 
         Mockito.doReturn(accountIdUserDuplicated).when(userVoDuplicatedMock).getAccountId();
 
@@ -728,24 +720,24 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
 
     @Test
     public void validateAndUpdateUsernameIfNeededTestDuplicatedUserButInDifferentDomains() {
-        long domanIdCurrentUser = 22l;
+        long domanIdCurrentUser = 22L;
 
         String userName = "username";
         Mockito.doReturn(userName).when(UpdateUserCmdMock).getUsername();
         Mockito.lenient().doReturn(userName).when(userVoMock).getUsername();
         Mockito.doReturn(domanIdCurrentUser).when(accountMock).getDomainId();
 
-        long userVoDuplicatedMockId = 67l;
+        long userVoDuplicatedMockId = 67L;
         UserVO userVoDuplicatedMock = Mockito.mock(UserVO.class);
         Mockito.lenient().doReturn(userName).when(userVoDuplicatedMock).getUsername();
         Mockito.doReturn(userVoDuplicatedMockId).when(userVoDuplicatedMock).getId();
 
-        long accountIdUserDuplicated = 98l;
+        long accountIdUserDuplicated = 98L;
         Mockito.doReturn(accountIdUserDuplicated).when(userVoDuplicatedMock).getAccountId();
 
         Account accountUserDuplicatedMock = Mockito.mock(AccountVO.class);
         Mockito.lenient().doReturn(accountIdUserDuplicated).when(accountUserDuplicatedMock).getId();
-        Mockito.doReturn(45l).when(accountUserDuplicatedMock).getDomainId();
+        Mockito.doReturn(45L).when(accountUserDuplicatedMock).getDomainId();
 
         List<UserVO> usersWithSameUserName = new ArrayList<>();
         usersWithSameUserName.add(userVoMock);
@@ -763,7 +755,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
 
     @Test
     public void validateAndUpdateUsernameIfNeededTestNoDuplicatedUserNames() {
-        long domanIdCurrentUser = 22l;
+        long domanIdCurrentUser = 22L;
 
         String userName = "username";
         Mockito.doReturn(userName).when(UpdateUserCmdMock).getUsername();
@@ -961,7 +953,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
     @Test
     public void validateCurrentPasswordTestUserAuthenticatedWithProvidedCurrentPasswordViaFirstAuthenticator() {
         AccountVO accountVoMock = Mockito.mock(AccountVO.class);
-        long domainId = 14l;
+        long domainId = 14L;
         Mockito.doReturn(domainId).when(accountVoMock).getDomainId();
 
         Mockito.doReturn(accountVoMock).when(_accountDao).findById(accountMockId);
@@ -990,7 +982,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
     @Test
     public void validateCurrentPasswordTestUserAuthenticatedWithProvidedCurrentPasswordViaSecondAuthenticator() {
         AccountVO accountVoMock = Mockito.mock(AccountVO.class);
-        long domainId = 14l;
+        long domainId = 14L;
         Mockito.doReturn(domainId).when(accountVoMock).getDomainId();
 
         Mockito.doReturn(accountVoMock).when(_accountDao).findById(accountMockId);
@@ -1051,7 +1043,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
         UserAccountVO userAccount = Mockito.mock(UserAccountVO.class);
         UserVO userVO = Mockito.mock(UserVO.class);
 
-        Mockito.when(userAccountDaoMock.findById(userId)).thenReturn(userAccount);
+        Mockito.when(userAccountDao.findById(userId)).thenReturn(userAccount);
         Mockito.when(userDaoMock.findById(userId)).thenReturn(userVO);
         Mockito.when(userAccount.getDomainId()).thenReturn(1L);
 
@@ -1070,7 +1062,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
         UserAccountVO userAccount = Mockito.mock(UserAccountVO.class);
         UserVO userVO = Mockito.mock(UserVO.class);
 
-        Mockito.when(userAccountDaoMock.findById(userId)).thenReturn(userAccount);
+        Mockito.when(userAccountDao.findById(userId)).thenReturn(userAccount);
         Mockito.when(userDaoMock.findById(userId)).thenReturn(userVO);
         Mockito.when(userAccount.getDomainId()).thenReturn(1L);
 
@@ -1099,7 +1091,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
         UserAccountVO userAccount = Mockito.mock(UserAccountVO.class);
         UserVO userVO = Mockito.mock(UserVO.class);
 
-        Mockito.when(userAccountDaoMock.findById(userId)).thenReturn(userAccount);
+        Mockito.when(userAccountDao.findById(userId)).thenReturn(userAccount);
         Mockito.when(userDaoMock.findById(userId)).thenReturn(userVO);
         Mockito.when(userAccount.getDomainId()).thenReturn(1L);
 
@@ -1205,7 +1197,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
         Mockito.when(callingUser.getId()).thenReturn(1L);
         CallContext.register(callingUser, callingAccount); // Calling account is user account i.e normal account
         Mockito.lenient().when(_accountService.getActiveAccountById(1L)).thenReturn(accountMock);
-        Mockito.when(userAccountDaoMock.findById(1L)).thenReturn(userAccountVO);
+        Mockito.when(userAccountDao.findById(1L)).thenReturn(userAccountVO);
         Mockito.when(userDaoMock.findById(1L)).thenReturn(userVoMock);
         Mockito.when(userAccountVO.getDomainId()).thenReturn(1L);
         Mockito.when(enableUserTwoFactorAuthenticationMock.valueIn(1L)).thenReturn(true);
@@ -1231,7 +1223,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
         List<UserAccountVO> userAccountVOList = new ArrayList<>();
         UserAccountVO userAccountVO = new UserAccountVO();
         userAccountVOList.add(userAccountVO);
-        Mockito.when(userAccountDaoMock.getUserAccountByEmail(email, domainId)).thenReturn(userAccountVOList);
+        Mockito.when(userAccountDao.getUserAccountByEmail(email, domainId)).thenReturn(userAccountVOList);
         List<UserAccount> userAccounts = accountManagerImpl.getActiveUserAccountByEmail(email, domainId);
         Assert.assertEquals(userAccountVOList.size(), userAccounts.size());
         Assert.assertEquals(userAccountVOList.get(0), userAccounts.get(0));
@@ -1406,7 +1398,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
         Mockito.when(user.getUser2faProvider()).thenReturn(null);
         UserAccount result = accountManagerImpl.clearUserTwoFactorAuthenticationInSetupStateOnLogin(user);
         Assert.assertSame(user, result);
-        Mockito.verifyNoInteractions(userDetailsDaoMock, userAccountDaoMock);
+        Mockito.verifyNoInteractions(userDetailsDaoMock, userAccountDao);
     }
 
     @Test
@@ -1420,7 +1412,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
         UserAccount result = accountManagerImpl.clearUserTwoFactorAuthenticationInSetupStateOnLogin(user);
         Assert.assertSame(user, result);
         Mockito.verify(userDetailsDaoMock).findDetail(1L, UserDetailVO.Setup2FADetail);
-        Mockito.verifyNoMoreInteractions(userDetailsDaoMock, userAccountDaoMock);
+        Mockito.verifyNoMoreInteractions(userDetailsDaoMock, userAccountDao);
     }
 
     @Test
@@ -1433,16 +1425,16 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase {
         UserAccountVO userAccountVO = new UserAccountVO();
         userAccountVO.setId(1L);
         Mockito.when(userDetailsDaoMock.findDetail(1L, UserDetailVO.Setup2FADetail)).thenReturn(userDetail);
-        Mockito.when(userAccountDaoMock.findById(1L)).thenReturn(userAccountVO);
+        Mockito.when(userAccountDao.findById(any())).thenReturn(userAccountVO);
         UserAccount result = accountManagerImpl.clearUserTwoFactorAuthenticationInSetupStateOnLogin(user);
         Assert.assertNotNull(result);
         Assert.assertFalse(result.isUser2faEnabled());
         Assert.assertNull(result.getUser2faProvider());
         Mockito.verify(userDetailsDaoMock).findDetail(1L, UserDetailVO.Setup2FADetail);
         Mockito.verify(userDetailsDaoMock).remove(Mockito.anyLong());
-        Mockito.verify(userAccountDaoMock).findById(1L);
+        Mockito.verify(userAccountDao).findById(1L);
         ArgumentCaptor<UserAccountVO> captor = ArgumentCaptor.forClass(UserAccountVO.class);
-        Mockito.verify(userAccountDaoMock).update(Mockito.eq(1L), captor.capture());
+        Mockito.verify(userAccountDao).update(Mockito.eq(1L), captor.capture());
         UserAccountVO updatedUser = captor.getValue();
         Assert.assertFalse(updatedUser.isUser2faEnabled());
         Assert.assertNull(updatedUser.getUser2faProvider());

server/src/test/java/com/cloud/user/AccountManagerImplVolumeDeleteEventTest.java

@@ -63,7 +63,7 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 @RunWith(MockitoJUnitRunner.class)
-public class AccountManagerImplVolumeDeleteEventTest extends AccountManagetImplTestBase {
+public class AccountManagerImplVolumeDeleteEventTest extends AccountManagentImplTestBase {
 
     private static final Long ACCOUNT_ID = 1l;
     private static final String VOLUME_UUID = "vol-111111";

server/src/test/java/com/cloud/vm/snapshot/VMSnapshotManagerTest.java

@@ -51,6 +51,7 @@ import com.cloud.vm.UserVmManager;
 import com.cloud.vm.UserVmVO;
 import com.cloud.vm.VirtualMachine.State;
 import com.cloud.vm.VirtualMachineManager;
+import com.cloud.vm.VmDetailConstants;
 import com.cloud.vm.dao.UserVmDao;
 import com.cloud.vm.dao.UserVmDetailsDao;
 import com.cloud.vm.dao.VMInstanceDao;
@@ -67,7 +68,6 @@ import org.junit.Before;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Captor;
-import org.mockito.ArgumentMatchers;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.mockito.Spy;
@@ -79,13 +79,18 @@ import java.util.List;
 import java.util.Map;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -225,13 +230,13 @@ public class VMSnapshotManagerTest {
         when(_serviceOfferingDao.findById(SERVICE_OFFERING_ID)).thenReturn(serviceOffering);
 
         for (ResourceDetail detail : Arrays.asList(userVmDetailCpuNumber, vmSnapshotDetailCpuNumber)) {
-            when(detail.getName()).thenReturn("cpuNumber");
+            when(detail.getName()).thenReturn(VmDetailConstants.CPU_NUMBER);
             when(detail.getValue()).thenReturn("2");
             when(detail.isDisplay()).thenReturn(true);
         }
 
         for (ResourceDetail detail : Arrays.asList(userVmDetailMemory, vmSnapshotDetailMemory)) {
-            when(detail.getName()).thenReturn("memory");
+            when(detail.getName()).thenReturn(VmDetailConstants.MEMORY);
             when(detail.getValue()).thenReturn("2048");
             when(detail.isDisplay()).thenReturn(true);
         }
@@ -348,12 +353,12 @@ public class VMSnapshotManagerTest {
     @Test
     public void testUpdateUserVmServiceOfferingDifferentServiceOffering() throws ConcurrentOperationException, ResourceUnavailableException, ManagementServerException, VirtualMachineMigrationException {
         when(userVm.getServiceOfferingId()).thenReturn(SERVICE_OFFERING_DIFFERENT_ID);
-        when(_userVmManager.upgradeVirtualMachine(ArgumentMatchers.eq(TEST_VM_ID), ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(true);
+        when(_userVmManager.upgradeVirtualMachine(eq(TEST_VM_ID), eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(true);
         _vmSnapshotMgr.updateUserVmServiceOffering(userVm, vmSnapshotVO);
 
         verify(_vmSnapshotMgr).changeUserVmServiceOffering(userVm, vmSnapshotVO);
         verify(_vmSnapshotMgr).getVmMapDetails(userVm);
-        verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(ArgumentMatchers.eq(userVm), ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture());
+        verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(eq(userVm), eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture());
     }
 
     @Test
@@ -368,18 +373,18 @@ public class VMSnapshotManagerTest {
 
     @Test
     public void testChangeUserVmServiceOffering() throws ConcurrentOperationException, ResourceUnavailableException, ManagementServerException, VirtualMachineMigrationException {
-        when(_userVmManager.upgradeVirtualMachine(ArgumentMatchers.eq(TEST_VM_ID), ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(true);
+        when(_userVmManager.upgradeVirtualMachine(eq(TEST_VM_ID), eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(true);
         _vmSnapshotMgr.changeUserVmServiceOffering(userVm, vmSnapshotVO);
         verify(_vmSnapshotMgr).getVmMapDetails(userVm);
-        verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(ArgumentMatchers.eq(userVm), ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture());
+        verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(eq(userVm), eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture());
     }
 
     @Test(expected=CloudRuntimeException.class)
     public void testChangeUserVmServiceOfferingFailOnUpgradeVMServiceOffering() throws ConcurrentOperationException, ResourceUnavailableException, ManagementServerException, VirtualMachineMigrationException {
-        when(_userVmManager.upgradeVirtualMachine(ArgumentMatchers.eq(TEST_VM_ID), ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(false);
+        when(_userVmManager.upgradeVirtualMachine(eq(TEST_VM_ID), eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(false);
         _vmSnapshotMgr.changeUserVmServiceOffering(userVm, vmSnapshotVO);
         verify(_vmSnapshotMgr).getVmMapDetails(userVm);
-        verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(ArgumentMatchers.eq(userVm), ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture());
+        verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(eq(userVm), eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture());
     }
 
     @Test
@@ -395,16 +400,27 @@ public class VMSnapshotManagerTest {
 
     @Test
     public void testRevertUserVmDetailsFromVmSnapshotNotDynamicServiceOffering() {
-        _vmSnapshotMgr.revertUserVmDetailsFromVmSnapshot(vmMock, vmSnapshotVO);
+        _vmSnapshotMgr.revertCustomServiceOfferingDetailsFromVmSnapshot(vmMock, vmSnapshotVO);
         verify(_vmSnapshotDetailsDao, never()).listDetails(anyLong());
     }
 
     @Test
     public void testRevertUserVmDetailsFromVmSnapshotDynamicServiceOffering() {
         when(serviceOffering.isDynamic()).thenReturn(true);
-        _vmSnapshotMgr.revertUserVmDetailsFromVmSnapshot(vmMock, vmSnapshotVO);
-        verify(_vmSnapshotDetailsDao).listDetails(VM_SNAPSHOT_ID);
-        verify(_userVmDetailsDao).saveDetails(listUserVmDetailsCaptor.capture());
-    }
+        VMSnapshotDetailsVO uefiSnapshotDetail = new VMSnapshotDetailsVO(VM_SNAPSHOT_ID, "UEFI", "SECURE", true);
+        List<VMSnapshotDetailsVO> snapshotDetailsWithUefi = Arrays.asList(
+                vmSnapshotDetailCpuNumber, vmSnapshotDetailMemory, uefiSnapshotDetail);
+        when(_vmSnapshotDetailsDao.listDetails(VM_SNAPSHOT_ID)).thenReturn(snapshotDetailsWithUefi);
 
+        _vmSnapshotMgr.revertCustomServiceOfferingDetailsFromVmSnapshot(vmMock, vmSnapshotVO);
+
+        verify(_vmSnapshotDetailsDao).listDetails(VM_SNAPSHOT_ID);
+        verify(_userVmDetailsDao, never()).saveDetails(any());
+        ArgumentCaptor<String> detailNameCaptor = ArgumentCaptor.forClass(String.class);
+        verify(_userVmDetailsDao, times(2)).addDetail(eq(TEST_VM_ID), detailNameCaptor.capture(), anyString(), anyBoolean());
+        List<String> appliedNames = detailNameCaptor.getAllValues();
+        assertTrue(appliedNames.contains(VmDetailConstants.CPU_NUMBER));
+        assertTrue(appliedNames.contains(VmDetailConstants.MEMORY));
+        assertFalse("UEFI must not be applied from snapshot so that existing UEFI setting is preserved", appliedNames.contains("UEFI"));
+    }
 }

server/src/test/java/org/apache/cloudstack/storage/heuristics/HeuristicRuleHelperTest.java

@@ -16,6 +16,8 @@
 // under the License.
 package org.apache.cloudstack.storage.heuristics;
 
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.storage.Storage;
 import com.cloud.storage.VMTemplateVO;
 import com.cloud.storage.VolumeVO;
 import com.cloud.utils.exception.CloudRuntimeException;
@@ -29,6 +31,7 @@ import org.apache.cloudstack.storage.heuristics.presetvariables.PresetVariables;
 import org.apache.cloudstack.utils.jsinterpreter.JsInterpreter;
 import org.apache.logging.log4j.Logger;
 import org.junit.Assert;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
@@ -68,6 +71,19 @@ public class HeuristicRuleHelperTest {
     @InjectMocks
     HeuristicRuleHelper heuristicRuleHelperSpy = new HeuristicRuleHelper();
 
+    @Before
+    public void setUp() {
+        Mockito.doReturn("template-name").when(vmTemplateVOMock).getName();
+        Mockito.doReturn(Storage.ImageFormat.QCOW2).when(vmTemplateVOMock).getFormat();
+        Mockito.doReturn(Hypervisor.HypervisorType.KVM).when(vmTemplateVOMock).getHypervisorType();
+        Mockito.doReturn("snapshot-name").when(snapshotInfoMock).getName();
+        Mockito.doReturn(1024L).when(snapshotInfoMock).getSize();
+        Mockito.doReturn(Hypervisor.HypervisorType.VMware).when(snapshotInfoMock).getHypervisorType();
+        Mockito.doReturn("volume-name").when(volumeVOMock).getName();
+        Mockito.doReturn(Storage.ImageFormat.RAW).when(volumeVOMock).getFormat();
+        Mockito.doReturn(2048L).when(volumeVOMock).getSize();
+    }
+
     @Test
     public void getImageStoreIfThereIsHeuristicRuleTestZoneDoesNotHaveHeuristicRuleShouldReturnNull() {
         Long zoneId = 1L;

server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/AccountTest.java

@@ -1,46 +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.heuristics.presetvariables;
-
-import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class AccountTest {
-
-    @Test
-    public void toStringTestReturnsValidJson() {
-        Account variable = new Account();
-        variable.setName("test name");
-        variable.setId("test id");
-
-        Domain domainVariable = new Domain();
-        domainVariable.setId("domain id");
-        domainVariable.setName("domain name");
-        variable.setDomain(domainVariable);
-
-        String expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "name", "id", "domain");
-        String result = variable.toString();
-
-        Assert.assertEquals(expected, result);
-    }
-
-}

server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/DomainTest.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.heuristics.presetvariables;
-
-import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class DomainTest {
-
-    @Test
-    public void toStringTestReturnsValidJson() {
-        Domain variable = new Domain();
-        variable.setName("test name");
-        variable.setId("test id");
-
-        String expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "name", "id");
-        String result = variable.toString();
-
-        Assert.assertEquals(expected, result);
-    }
-
-}

server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/GenericHeuristicPresetVariableTest.java

@@ -1,40 +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.heuristics.presetvariables;
-
-import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class GenericHeuristicPresetVariableTest {
-
-    @Test
-    public void toStringTestReturnsValidJson() {
-        GenericHeuristicPresetVariable variable = new GenericHeuristicPresetVariable();
-        variable.setName("test name");
-
-        String expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "name");
-        String result = variable.toString();
-
-        Assert.assertEquals(expected, result);
-    }
-
-}

server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/SecondaryStorageTest.java

@@ -1,45 +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.heuristics.presetvariables;
-
-import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class SecondaryStorageTest {
-
-    @Test
-    public void toStringTestReturnsValidJson() {
-        SecondaryStorage variable = new SecondaryStorage();
-        variable.setName("test name");
-        variable.setId("test id");
-        variable.setProtocol("test protocol");
-        variable.setUsedDiskSize(1L);
-        variable.setTotalDiskSize(2L);
-
-        String expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "name", "id",
-                "protocol", "usedDiskSize", "totalDiskSize");
-        String result = variable.toString();
-
-        Assert.assertEquals(expected, result);
-    }
-
-}

server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/SnapshotTest.java

@@ -1,44 +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.heuristics.presetvariables;
-
-import com.cloud.hypervisor.Hypervisor;
-import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class SnapshotTest {
-
-    @Test
-    public void toStringTestReturnsValidJson() {
-        Snapshot variable = new Snapshot();
-        variable.setName("test name");
-        variable.setSize(1L);
-        variable.setHypervisorType(Hypervisor.HypervisorType.KVM);
-
-        String expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "name", "size",
-                "hypervisorType");
-        String result = variable.toString();
-
-        Assert.assertEquals(expected, result);
-    }
-
-}

server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/TemplateTest.java

@@ -1,46 +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.heuristics.presetvariables;
-
-import com.cloud.hypervisor.Hypervisor;
-import com.cloud.storage.Storage;
-import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class TemplateTest {
-
-    @Test
-    public void toStringTestReturnsValidJson() {
-        Template variable = new Template();
-        variable.setName("test name");
-        variable.setTemplateType(Storage.TemplateType.USER);
-        variable.setHypervisorType(Hypervisor.HypervisorType.KVM);
-        variable.setFormat(Storage.ImageFormat.QCOW2);
-
-        String expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "name", "templateType",
-                "hypervisorType", "format");
-        String result = variable.toString();
-
-        Assert.assertEquals(expected, result);
-    }
-
-}

server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/VolumeTest.java

@@ -1,44 +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.heuristics.presetvariables;
-
-import com.cloud.storage.Storage;
-import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class VolumeTest {
-
-    @Test
-    public void toStringTestReturnsValidJson() {
-        Volume variable = new Volume();
-        variable.setName("test name");
-        variable.setFormat(Storage.ImageFormat.QCOW2);
-        variable.setSize(1L);
-
-        String expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "name", "format",
-                "size");
-        String result = variable.toString();
-
-        Assert.assertEquals(expected, result);
-    }
-
-}

services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadManagerImpl.java

@@ -16,7 +16,10 @@
 // under the License.
 package org.apache.cloudstack.storage.template;
 
+import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
+
 import java.io.File;
+import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.nio.file.Path;
@@ -30,10 +33,10 @@ import java.util.concurrent.Executors;
 
 import javax.naming.ConfigurationException;
 
-import com.cloud.agent.api.Answer;
-
 import org.apache.cloudstack.storage.resource.SecondaryStorageResource;
+import org.apache.commons.lang3.StringUtils;
 
+import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.storage.CreateEntityDownloadURLAnswer;
 import com.cloud.agent.api.storage.CreateEntityDownloadURLCommand;
 import com.cloud.agent.api.storage.DeleteEntityDownloadURLCommand;
@@ -48,15 +51,18 @@ import com.cloud.storage.template.FtpTemplateUploader;
 import com.cloud.storage.template.TemplateUploader;
 import com.cloud.storage.template.TemplateUploader.Status;
 import com.cloud.storage.template.TemplateUploader.UploadCompleteCallback;
+import com.cloud.utils.FileUtil;
 import com.cloud.utils.NumbersUtil;
+import com.cloud.utils.UuidUtils;
 import com.cloud.utils.component.ManagerBase;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.script.Script;
 
-import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
-
 public class UploadManagerImpl extends ManagerBase implements UploadManager {
 
+    protected static final String EXTRACT_USERDATA_DIR = "userdata";
+    protected static final String BASE_EXTRACT_PATH = String.format("/var/www/html/%s/", EXTRACT_USERDATA_DIR);
+
     public class Completion implements UploadCompleteCallback {
         private final String jobId;
 
@@ -266,7 +272,7 @@ public class UploadManagerImpl extends ManagerBase implements UploadManager {
             return new CreateEntityDownloadURLAnswer(errorString, CreateEntityDownloadURLAnswer.RESULT_FAILURE);
         }
         // Create the directory structure so that its visible under apache server root
-        String extractDir = "/var/www/html/userdata/";
+        String extractDir = BASE_EXTRACT_PATH;
         extractDir = extractDir + cmd.getFilepathInExtractURL() + File.separator;
         Script command = new Script("/bin/su", logger);
         command.add("-s");
@@ -330,12 +336,20 @@ public class UploadManagerImpl extends ManagerBase implements UploadManager {
         String extractUrl = cmd.getExtractUrl();
         String result;
         if (extractUrl != null) {
-            command.add("unlink /var/www/html/userdata/" + extractUrl.substring(extractUrl.lastIndexOf(File.separator) + 1));
+            URI uri = URI.create(extractUrl);
+            String uriPath = uri.getPath();
+            String marker = String.format("/%s/", EXTRACT_USERDATA_DIR);
+            String linkPath = uriPath.startsWith(marker)
+                    ? uriPath.substring(marker.length())
+                    : uriPath.substring(uriPath.indexOf(marker) + marker.length());
+            command.add("unlink " + BASE_EXTRACT_PATH + linkPath);
             result = command.execute();
             if (result != null) {
                 // FIXME - Ideally should bail out if you can't delete symlink. Not doing it right now.
                 // This is because the ssvm might already be destroyed and the symlinks do not exist.
                 logger.warn("Error in deleting symlink :" + result);
+            } else {
+                deleteEntitySymlinkRootDirectoryIfNeeded(cmd, linkPath);
             }
         }
 
@@ -356,6 +370,30 @@ public class UploadManagerImpl extends ManagerBase implements UploadManager {
         return new Answer(cmd, true, "");
     }
 
+    protected void deleteEntitySymlinkRootDirectoryIfNeeded(DeleteEntityDownloadURLCommand cmd, String linkPath) {
+        if (StringUtils.isEmpty(linkPath)) {
+            return;
+        }
+        String[] parts = linkPath.split("/");
+        if (parts.length == 0) {
+            return;
+        }
+        String rootDir = parts[0];
+        if (StringUtils.isEmpty(rootDir) || !UuidUtils.isUuid(rootDir)) {
+            return;
+        }
+        logger.info("Deleting symlink root directory: {} for {}", rootDir, cmd.getExtractUrl());
+        Path rootDirPath = Path.of(BASE_EXTRACT_PATH, rootDir);
+        String failMsg = "Failed to delete symlink root directory: {} for {}";
+        try {
+            if (!FileUtil.deleteRecursively(rootDirPath)) {
+                logger.warn(failMsg, rootDir, cmd.getExtractUrl());
+            }
+        } catch (IOException e) {
+            logger.warn(failMsg, rootDir, cmd.getExtractUrl(), e);
+        }
+    }
+
     private String getInstallPath(String jobId) {
         // TODO Auto-generated method stub
         return null;

services/secondary-storage/server/src/test/java/org/apache/cloudstack/storage/template/UploadManagerImplTest.java

@@ -0,0 +1,85 @@
+// 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.template;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+
+import java.nio.file.Path;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import com.cloud.agent.api.storage.DeleteEntityDownloadURLCommand;
+import com.cloud.utils.FileUtil;
+
+@RunWith(MockitoJUnitRunner.class)
+public class UploadManagerImplTest {
+
+    @InjectMocks
+    UploadManagerImpl uploadManager;
+
+    MockedStatic<FileUtil> fileUtilMock;
+
+    @Before
+    public void setup() {
+        fileUtilMock = mockStatic(FileUtil.class, Mockito.CALLS_REAL_METHODS);
+        fileUtilMock.when(() -> FileUtil.deleteRecursively(any(Path.class))).thenReturn(true);
+    }
+
+    @After
+    public void tearDown() {
+        fileUtilMock.close();
+    }
+
+    @Test
+    public void doesNotDeleteWhenLinkPathIsEmpty() {
+        String emptyLinkPath = "";
+        uploadManager.deleteEntitySymlinkRootDirectoryIfNeeded(mock(DeleteEntityDownloadURLCommand.class), emptyLinkPath);
+        fileUtilMock.verify(() -> FileUtil.deleteRecursively(any(Path.class)), never());
+    }
+
+    @Test
+    public void doesNotDeleteWhenRootDirIsNotUuid() {
+        String invalidLinkPath = "invalidRootDir/file";
+        uploadManager.deleteEntitySymlinkRootDirectoryIfNeeded(mock(DeleteEntityDownloadURLCommand.class), invalidLinkPath);
+        fileUtilMock.verify(() -> FileUtil.deleteRecursively(any(Path.class)), never());
+    }
+
+    @Test
+    public void deletesSymlinkRootDirectoryWhenValidUuid() {
+        String validLinkPath = "123e4567-e89b-12d3-a456-426614174000/file";
+        uploadManager.deleteEntitySymlinkRootDirectoryIfNeeded(mock(DeleteEntityDownloadURLCommand.class), validLinkPath);
+        fileUtilMock.verify(() -> FileUtil.deleteRecursively(any(Path.class)), times(1));
+    }
+
+    @Test
+    public void deletesSymlinkRootDirectoryWhenNoFile() {
+        String validLinkPath = "123e4567-e89b-12d3-a456-426614174000";
+        uploadManager.deleteEntitySymlinkRootDirectoryIfNeeded(mock(DeleteEntityDownloadURLCommand.class), validLinkPath);
+        fileUtilMock.verify(() -> FileUtil.deleteRecursively(any(Path.class)), times(1));
+    }
+}