Revert "Refactoring error handling"

This reverts commit d37a35b.

Author: PeterBackroll

plugins/backup/backroll/src/main/java/org/apache/cloudstack/backup/BackrollBackupProvider.java

@@ -20,6 +20,7 @@
 import java.net.URISyntaxException;
 import java.security.KeyManagementException;
 import java.security.NoSuchAlgorithmException;
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -38,6 +39,7 @@
 import org.apache.cloudstack.framework.config.Configurable;
 
 import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.http.client.ClientProtocolException;
 import org.joda.time.DateTime;
 
@@ -203,10 +205,7 @@ public boolean takeBackup(VirtualMachine vm) {
             backup.setAccountId(vm.getAccountId());
             backup.setDomainId(vm.getDomainId());
             backup.setZoneId(vm.getDataCenterId());
-            if (backupDao.persist(backup) == null) {// TODO is null a failure ?
-                throw new Exception("Failed to persist backup.");
-            }
-            ;
+            assert backupDao.persist(backup) != null;
         } catch (Exception e) {
             throw new CloudRuntimeException(String.format("Failed to take a backup of VM %s.", vm.getName()), e);
         }
@@ -360,31 +359,33 @@ public boolean deleteBackup(Backup backup, boolean forced) {
         logger.info("backroll delete backup id: {}", backup.getExternalId());
         if (backup.getStatus().equals(Backup.Status.BackingUp)) {
             throw new CloudRuntimeException("You can't delete a backup while it still backing up.");
-        }
+        } else {
+            logger.debug("backroll - try delete backup");
+            VMInstanceVO vm = vmInstanceDao.findByIdIncludingRemoved(backup.getVmId());
 
-        logger.debug("backroll - try delete backup");
-        VMInstanceVO vm = vmInstanceDao.findByIdIncludingRemoved(backup.getVmId());
+            if (backup.getStatus().equals(Backup.Status.Removed) || backup.getStatus().equals(Backup.Status.Failed)) {
+                return deleteBackupInDb(backup);
+            }
 
-        if (backup.getStatus().equals(Backup.Status.Removed) || backup.getStatus().equals(Backup.Status.Failed)) {
-            deleteBackupInDb(backup);
-            return true;
-        }
+            try {
+                getClient(backup.getZoneId()).deleteBackup(vm.getUuid(), getBackupName(backup));
+            } catch (Exception e) {
+                throw new CloudRuntimeException(String.format("Failed to delete backup %s", backup.getName()));
+            }
 
-        try {
-            getClient(backup.getZoneId()).deleteBackup(vm.getUuid(), getBackupName(backup));
-        } catch (Exception e) {
-            throw new CloudRuntimeException(String.format("Failed to delete backup %s", backup.getName()));
+            logger.debug("Backup deletion for backup {} complete on backroll side.", backup.getUuid());
+            return deleteBackupInDb(backup);
         }
-
-        logger.debug("Backup deletion for backup {} complete on backroll side.", backup.getUuid());
-        deleteBackupInDb(backup);
-        return true;
     }
 
-    private void deleteBackupInDb(Backup backup) {
+    private boolean deleteBackupInDb(Backup backup) {
         BackupVO backupToUpdate = ((BackupVO) backup);
         backupToUpdate.setStatus(Backup.Status.Removed);
-        backupDao.persist(backupToUpdate); // TODO is null a failure ?
+        if (backupDao.persist(backupToUpdate) != null) {
+            logger.debug("Backroll backup {} deleted in database.", backup.getUuid());
+            return true;
+        }
+        return false;
     }
 
     protected BackrollClient getClient(final Long zoneId) throws ClientProtocolException, IOException {

plugins/backup/backroll/src/main/java/org/apache/cloudstack/backup/backroll/BackrollClient.java

@@ -80,6 +80,7 @@
 
 import org.joda.time.DateTime;
 
+import org.json.JSONException;
 import org.json.JSONObject;
 
 import com.cloud.utils.exception.CloudRuntimeException;
@@ -139,22 +140,25 @@ public BackrollClient(final String url, final String appname, final String passw
         }
     }
 
-    public String startBackupJob(final String jobId)
-            throws ClientProtocolException, IOException, ParseException, NotOkBodyException {
+    public String startBackupJob(final String jobId) {
         logger.info("Trying to start backup for Backroll job: {}", jobId);
 
-        ensureLoggedIn();
-
-        return this.<BackrollTaskRequestResponse>parse(okBody(
-                post(String.format("/tasks/singlebackup/%s", jobId), null))).location
-                .replace("/api/v1/status/", "");
+        try {
+            loginIfAuthenticationFailed();
+            return this.<BackrollTaskRequestResponse>parse(okBody(
+                    post(String.format("/tasks/singlebackup/%s", jobId), null))).location
+                    .replace("/api/v1/status/", "");
+        } catch (final Exception e) {
+            logger.error("Failed to start Backroll backup job due to: {}", e.getMessage());
+        }
+        return null;
     }
 
     public String getBackupOfferingUrl()
             throws ClientProtocolException, IOException, ParseException, NotOkBodyException {
         logger.info("Trying to list backroll backup policies");
 
-        ensureLoggedIn();
+        loginIfAuthenticationFailed();
 
         return this.<BackrollTaskRequestResponse>parse(okBody(get("/backup_policies"))).location.replace("/api/v1",
                 "");
@@ -164,7 +168,7 @@ public List<BackupOffering> getBackupOfferings(String idTask)
             throws ParseException, NotOkBodyException, ClientProtocolException, IOException, InterruptedException {
         logger.info("Trying to list backroll backup policies");
 
-        ensureLoggedIn();
+        loginIfAuthenticationFailed();
 
         BackupPoliciesResponse backupPoliciesResponse = waitGet(idTask);
 
@@ -178,7 +182,7 @@ public List<BackupOffering> getBackupOfferings(String idTask)
     public void restoreVMFromBackup(final String vmId, final String backupName) throws Exception {
         logger.info("Start restore backup with backroll with backup {} for vm {}", backupName, vmId);
 
-        ensureLoggedIn();
+        loginIfAuthenticationFailed();
 
         JSONObject jsonBody = new JSONObject();
         jsonBody.put("virtual_machine_id", vmId);
@@ -203,7 +207,7 @@ public BackrollTaskStatus checkBackupTaskStatus(String taskId)
             throws ParseException, IOException, NotOkBodyException {
         logger.info("Trying to get backup status for Backroll task: {}", taskId);
 
-        ensureLoggedIn();
+        loginIfAuthenticationFailed();
 
         BackrollTaskStatus status = new BackrollTaskStatus();
 
@@ -221,27 +225,32 @@ public BackrollTaskStatus checkBackupTaskStatus(String taskId)
         return status;
     }
 
-    public void deleteBackup(final String vmId, final String backupName)
-            throws Exception {
+    public boolean deleteBackup(final String vmId, final String backupName)
+            throws ClientProtocolException, IOException {
         logger.info("Trying to delete backup {} for vm {} using Backroll", vmId, backupName);
 
-        ensureLoggedIn();
+        loginIfAuthenticationFailed();
 
-        String urlToRequest = this.<BackrollTaskRequestResponse>parse(okBody(delete(
-                String.format("/virtualmachines/%s/backups/%s", vmId, backupName)))).location
-                .replace("/api/v1", "");
+        try {
+            String urlToRequest = this.<BackrollTaskRequestResponse>parse(okBody(delete(
+                    String.format("/virtualmachines/%s/backups/%s", vmId, backupName)))).location
+                    .replace("/api/v1", "");
 
-        BackrollBackupsFromVMResponse backrollBackupsFromVMResponse = waitGet(urlToRequest);
-        logger.debug(backrollBackupsFromVMResponse.state);
-        if (!backrollBackupsFromVMResponse.state.equals(TaskState.SUCCESS)) {
-            throw new Exception("Backroll task failed.");
+            BackrollBackupsFromVMResponse backrollBackupsFromVMResponse = waitGet(urlToRequest);
+            logger.debug(backrollBackupsFromVMResponse.state);
+            return backrollBackupsFromVMResponse.state.equals(TaskState.SUCCESS);
+        } catch (final NotOkBodyException e) {
+            return false;
+        } catch (final Exception e) {
+            logger.error("Failed to delete backup using Backroll due to: {}", e.getMessage());
         }
+        return false;
     }
 
     public Metric getVirtualMachineMetrics(final String vmId) throws ClientProtocolException, IOException {
         logger.info("Trying to retrieve virtual machine metric from Backroll for vm {}", vmId);
 
-        ensureLoggedIn();
+        loginIfAuthenticationFailed();
 
         Metric metric = new Metric(0L, 0L);
 
@@ -276,7 +285,7 @@ public BackrollBackupMetrics getBackupMetrics(String vmId, String backupId) thro
             JsonProcessingException, ParseException, IOException, NotOkBodyException, InterruptedException {
         logger.info("Trying to get backup metrics for VM: {}, and backup: {}", vmId, backupId);
 
-        ensureLoggedIn();
+        loginIfAuthenticationFailed();
 
         String urlToRequest = this.<BackrollTaskRequestResponse>parse(okBody(get(
                 String.format("/virtualmachines/%s/backups/%s", vmId, backupId)))).location.replace("/api/v1", "");
@@ -323,20 +332,26 @@ public List<BackrollVmBackup> getAllBackupsfromVirtualMachine(String vmId) {
     }
 
     private HttpResponse post(final String path, final JSONObject json) throws IOException {
+        String xml = null;
+        StringEntity params = null;
+        if (json != null) {
+            logger.debug("JSON {}", json.toString());
+            params = new StringEntity(json.toString(), ContentType.APPLICATION_JSON);
+        }
+
         String url = apiURI.toString() + path;
         final HttpPost request = new HttpPost(url);
 
+        if (params != null) {
+            request.setEntity(params);
+        }
+
         request.setHeader(HttpHeaders.AUTHORIZATION, "Bearer " + backrollToken);
         request.setHeader("Content-type", "application/json");
 
-        if (json != null) {
-            logger.debug("JSON {}", json.toString());
-            request.setEntity(new StringEntity(json.toString(), ContentType.APPLICATION_JSON));
-        }
-
         final HttpResponse response = httpClient.execute(request);
 
-        logger.debug("Response received in POST request with body {} is: {} for URL {}.", json, response.toString(),
+        logger.debug("Response received in POST request with body {} is: {} for URL {}.", xml, response.toString(),
                 url);
 
         return response;
@@ -363,14 +378,9 @@ protected HttpResponse delete(final String path) throws IOException {
         return response;
     }
 
-    private boolean isResponseOk(final HttpResponse response) {
-        switch (response.getStatusLine().getStatusCode()) {
-            case HttpStatus.SC_OK:
-            case HttpStatus.SC_ACCEPTED:
-                return true;
-            default:
-                return false;
-        }
+    private boolean isResponseAuthorized(final HttpResponse response) {
+        return response.getStatusLine().getStatusCode() == HttpStatus.SC_OK
+                || response.getStatusLine().getStatusCode() == HttpStatus.SC_ACCEPTED;
     }
 
     public class NotOkBodyException extends Exception {
@@ -388,15 +398,18 @@ public HttpResponse getResponse() {
 
     private String okBody(final HttpResponse response)
             throws ParseException, IOException, NotOkBodyException {
-        if (isResponseOk(response)) {
-            HttpEntity bodyEntity = response.getEntity();
-            try {
-                return EntityUtils.toString(bodyEntity);
-            } finally {
-                EntityUtils.consumeQuietly(bodyEntity);
-            }
+        switch (response.getStatusLine().getStatusCode()) {
+            case HttpStatus.SC_OK:
+            case HttpStatus.SC_ACCEPTED:
+                HttpEntity bodyEntity = response.getEntity();
+                try {
+                    return EntityUtils.toString(bodyEntity);
+                } finally {
+                    EntityUtils.consumeQuietly(bodyEntity);
+                }
+            default:
+                throw new NotOkBodyException(response);
         }
-        throw new NotOkBodyException(response);
     }
 
     private <T> T parse(final String json)
@@ -428,15 +441,15 @@ private boolean isAuthenticated() {
         boolean result = false;
         try {
             final HttpResponse response = post("/auth", null);
-            result = isResponseOk(response);
-            EntityUtils.consumeQuietly(response.getEntity()); // TODO Move to a finally clause ?
+            result = isResponseAuthorized(response);
+            EntityUtils.consumeQuietly(response.getEntity());
         } catch (IOException e) {
             logger.error("Failed to authenticate to Backroll due to: {}", e.getMessage());
         }
         return result;
     }
 
-    private void ensureLoggedIn() throws ClientProtocolException, IOException {
+    private void loginIfAuthenticationFailed() throws ClientProtocolException, IOException {
         if (!isAuthenticated()) {
             login(appname, password);
         }