[GitHub] [cloudstack] GutoVeronezi commented on a diff in pull request #6589: [Veeam] disable jobs but keep backups

[GitBox <gi...@apache.org>]

GutoVeronezi commented on code in PR #6589:
URL: https://github.com/apache/cloudstack/pull/6589#discussion_r966237797


##########
api/src/main/java/org/apache/cloudstack/backup/Backup.java:
##########
@@ -125,9 +139,17 @@ public Long getSize() {
             return size;
         }
 
+        public void setDeviceId(Long deviceId) {
+            this.deviceId = deviceId;
+        }
+
+        public Long getDeviceId() {
+            return deviceId;
+        }
+
         @Override
         public String toString() {
-            return StringUtils.join(":", uuid, path, type, size);
+            return StringUtils.join(":", uuid, path, type, size, deviceId);

Review Comment:
   This `toString` is used in many places? If not, we could rewrite it to use the `ReflectionToStringBuilderUtils`.



##########
engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupDaoImpl.java:
##########
@@ -141,32 +143,37 @@ public List<Backup> syncBackups(Long zoneId, Long vmId, List<Backup> externalBac
 
     @Override
     public BackupResponse newBackupResponse(Backup backup) {
-        VMInstanceVO vm = vmInstanceDao.findByIdIncludingRemoved(backup.getVmId());
-        AccountVO account = accountDao.findByIdIncludingRemoved(vm.getAccountId());
-        DomainVO domain = domainDao.findByIdIncludingRemoved(vm.getDomainId());
-        DataCenterVO zone = dataCenterDao.findByIdIncludingRemoved(vm.getDataCenterId());
-        BackupOffering offering = backupOfferingDao.findByIdIncludingRemoved(vm.getBackupOfferingId());
-
-        BackupResponse response = new BackupResponse();
-        response.setId(backup.getUuid());
-        response.setVmId(vm.getUuid());
-        response.setVmName(vm.getHostName());
-        response.setExternalId(backup.getExternalId());
-        response.setType(backup.getType());
-        response.setDate(backup.getDate());
-        response.setSize(backup.getSize());
-        response.setProtectedSize(backup.getProtectedSize());
-        response.setStatus(backup.getStatus());
-        response.setVolumes(new Gson().toJson(vm.getBackupVolumeList().toArray(), Backup.VolumeInfo[].class));
-        response.setBackupOfferingId(offering.getUuid());
-        response.setBackupOffering(offering.getName());
-        response.setAccountId(account.getUuid());
-        response.setAccount(account.getAccountName());
-        response.setDomainId(domain.getUuid());
-        response.setDomain(domain.getName());
-        response.setZoneId(zone.getUuid());
-        response.setZone(zone.getName());
-        response.setObjectName("backup");
-        return response;
+        try {
+            VMInstanceVO vm = vmInstanceDao.findByIdIncludingRemoved(backup.getVmId());
+            AccountVO account = accountDao.findByIdIncludingRemoved(vm.getAccountId());
+            DomainVO domain = domainDao.findByIdIncludingRemoved(vm.getDomainId());
+            DataCenterVO zone = dataCenterDao.findByIdIncludingRemoved(vm.getDataCenterId());
+            BackupOffering offering = backupOfferingDao.findByIdIncludingRemoved(backup.getBackupOfferingId());
+
+            BackupResponse response = new BackupResponse();
+            response.setId(backup.getUuid());
+            response.setVmId(vm.getUuid());
+            response.setVmName(vm.getHostName());
+            response.setExternalId(backup.getExternalId());
+            response.setType(backup.getType());
+            response.setDate(backup.getDate());
+            response.setSize(backup.getSize());
+            response.setProtectedSize(backup.getProtectedSize());
+            response.setStatus(backup.getStatus());
+            response.setVolumes(new Gson().toJson(backup.getBackupVolumeList().toArray(), Backup.VolumeInfo[].class));

Review Comment:
    We can use `GsonHelper` instead of instantiating `Gson` every time.



##########
engine/schema/src/main/java/org/apache/cloudstack/backup/BackupVO.java:
##########
@@ -192,4 +201,16 @@ public Class<?> getEntityType() {
     public String getName() {
         return null;
     }
+
+    @Override
+    public List<Backup.VolumeInfo> getBackupVolumeList() {
+        if (StringUtils.isEmpty(this.backupVolumes)) {
+            return Collections.emptyList();
+        }
+        return Arrays.asList(new Gson().fromJson(this.backupVolumes, Backup.VolumeInfo[].class));

Review Comment:
    We can use `GsonHelper` instead of instantiating `Gson` every time.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org