Reviewed error handling and notifications in downloads

diff --git a/AndroidManifest.xml b/AndroidManifest.xml
index 7f3a9ef..8121b85 100644 (file)
--- a/AndroidManifest.xml
+++ b/AndroidManifest.xml
@@ -18,7 +18,7 @@
  -->\r
 <manifest package="eu.alefzero.owncloud"\r
     android:versionCode="1"\r
  -->\r
 <manifest package="eu.alefzero.owncloud"\r
     android:versionCode="1"\r

-    android:versionName="0.1.179B" xmlns:android="http://schemas.android.com/apk/res/android">\r
+    android:versionName="0.1.180B" xmlns:android="http://schemas.android.com/apk/res/android">\r

 \r
     <uses-permission android:name="android.permission.GET_ACCOUNTS" />\r
     <uses-permission android:name="android.permission.USE_CREDENTIALS" />\r
 \r
     <uses-permission android:name="android.permission.GET_ACCOUNTS" />\r
     <uses-permission android:name="android.permission.USE_CREDENTIALS" />\r

diff --git a/res/values/strings.xml b/res/values/strings.xml
index bafbc26..e1af1e5 100644 (file)
--- a/res/values/strings.xml
+++ b/res/values/strings.xml
@@ -72,8 +72,12 @@
     <string name="uploader_info_dirname">Directory name</string>
     <string name="uploader_upload_succeed">Upload was successful</string>
     <string name="uploader_upload_failed">Upload failed: %1$d/%2$d files uploaded</string>
     <string name="uploader_info_dirname">Directory name</string>
     <string name="uploader_upload_succeed">Upload was successful</string>
     <string name="uploader_upload_failed">Upload failed: %1$d/%2$d files uploaded</string>

-    <string name="downloader_download_succeed">Download was successful</string>
-    <string name="downloader_download_failed">Download could not be completed</string>
+    <string name="downloader_download_in_progress_ticker">Downloading &#8230;</string>
+    <string name="downloader_download_in_progress_content">%1$d%% Downloading %2$s</string>
+    <string name="downloader_download_succeed_ticker">Download suceeded</string>
+    <string name="downloader_download_succeed_content">%1$s was successfully download</string>
+    <string name="downloader_download_failed_ticker">Download failed</string>
+    <string name="downloader_download_failed_content">Download of %1$s could not be completed</string>

     <string name="common_choose_account">Choose account</string>
     <string name="sync_string_contacts">Contacts</string>
     <string name="use_ssl">Use Secure Connection</string>
     <string name="common_choose_account">Choose account</string>
     <string name="sync_string_contacts">Contacts</string>
     <string name="use_ssl">Use Secure Connection</string>

diff --git a/src/eu/alefzero/owncloud/files/services/FileDownloader.java b/src/eu/alefzero/owncloud/files/services/FileDownloader.java
index c147c9d..3ee316f 100644 (file)
--- a/src/eu/alefzero/owncloud/files/services/FileDownloader.java
+++ b/src/eu/alefzero/owncloud/files/services/FileDownloader.java
@@ -8,6 +8,8 @@ import java.util.Map;
 \r
 import android.accounts.Account;\r
 import android.accounts.AccountManager;\r
 \r
 import android.accounts.Account;\r
 import android.accounts.AccountManager;\r

+import android.accounts.AuthenticatorException;\r
+import android.accounts.OperationCanceledException;\r

 import android.app.Notification;\r
 import android.app.NotificationManager;\r
 import android.app.PendingIntent;\r
 import android.app.Notification;\r
 import android.app.NotificationManager;\r
 import android.app.PendingIntent;\r


@@ -22,6 +24,7 @@ import android.os.Looper;
 import android.os.Message;\r
 import android.os.Process;\r
 import android.util.Log;\r
 import android.os.Message;\r
 import android.os.Process;\r
 import android.util.Log;\r

+import android.util.LogPrinter;\r

 import android.widget.RemoteViews;\r
 import android.widget.Toast;\r
 import eu.alefzero.owncloud.R;\r
 import android.widget.RemoteViews;\r
 import android.widget.Toast;\r
 import eu.alefzero.owncloud.R;\r


@@ -49,7 +52,7 @@ public class FileDownloader extends Service implements OnDatatransferProgressLis
     private String mRemotePath;\r
     private int mLastPercent;\r
     private long mTotalDownloadSize;\r
     private String mRemotePath;\r
     private int mLastPercent;\r
     private long mTotalDownloadSize;\r

-    private long mCurrentDownlodSize;\r
+    private long mCurrentDownloadSize;\r

     private Notification mNotification;\r
     \r
     /**\r
     private Notification mNotification;\r
     \r
     /**\r


@@ -112,61 +115,67 @@ public class FileDownloader extends Service implements OnDatatransferProgressLis
 \r
     @Override\r
     public int onStartCommand(Intent intent, int flags, int startId) {\r
 \r
     @Override\r
     public int onStartCommand(Intent intent, int flags, int startId) {\r

-        if (!intent.hasExtra(EXTRA_ACCOUNT)\r
-                && !intent.hasExtra(EXTRA_FILE_PATH)) {\r
+        if (    !intent.hasExtra(EXTRA_ACCOUNT) ||\r
+                !intent.hasExtra(EXTRA_FILE_PATH) ||\r
+                !intent.hasExtra(EXTRA_REMOTE_PATH)\r
+           ) {\r

             Log.e(TAG, "Not enough information provided in intent");\r
             return START_STICKY;\r
         }\r
         mAccount = intent.getParcelableExtra(EXTRA_ACCOUNT);\r
         mFilePath = intent.getStringExtra(EXTRA_FILE_PATH);\r
         mRemotePath = intent.getStringExtra(EXTRA_REMOTE_PATH);\r
             Log.e(TAG, "Not enough information provided in intent");\r
             return START_STICKY;\r
         }\r
         mAccount = intent.getParcelableExtra(EXTRA_ACCOUNT);\r
         mFilePath = intent.getStringExtra(EXTRA_FILE_PATH);\r
         mRemotePath = intent.getStringExtra(EXTRA_REMOTE_PATH);\r

+        mTotalDownloadSize = intent.getLongExtra(EXTRA_FILE_SIZE, -1);\r
+        mCurrentDownloadSize = mLastPercent = 0;\r
+\r

         Message msg = mServiceHandler.obtainMessage();\r
         msg.arg1 = startId;\r
         mServiceHandler.sendMessage(msg);\r
         Message msg = mServiceHandler.obtainMessage();\r
         msg.arg1 = startId;\r
         mServiceHandler.sendMessage(msg);\r

-        mCurrentDownlodSize = mLastPercent = 0;\r
-        mTotalDownloadSize = intent.getLongExtra(EXTRA_FILE_SIZE, -1);\r

 \r
         return START_NOT_STICKY;\r
     }\r
 \r
     void downloadFile() {\r
 \r
         return START_NOT_STICKY;\r
     }\r
 \r
     void downloadFile() {\r

-        AccountManager am = (AccountManager) getSystemService(ACCOUNT_SERVICE);\r
-\r
+        boolean downloadResult = false;\r

 \r
 \r

+        /// prepare client object to send the request to the ownCloud server\r
+        AccountManager am = (AccountManager) getSystemService(ACCOUNT_SERVICE);\r

         WebdavClient wdc = new WebdavClient(mAccount, getApplicationContext());\r
         WebdavClient wdc = new WebdavClient(mAccount, getApplicationContext());\r

-        \r

         String username = mAccount.name.split("@")[0];\r
         String username = mAccount.name.split("@")[0];\r

-        String password = "";\r
+        String password = null;\r

         try {\r
             password = am.blockingGetAuthToken(mAccount,\r
                     AccountAuthenticator.AUTH_TOKEN_TYPE, true);\r
         } catch (Exception e) {\r
         try {\r
             password = am.blockingGetAuthToken(mAccount,\r
                     AccountAuthenticator.AUTH_TOKEN_TYPE, true);\r
         } catch (Exception e) {\r

-            e.printStackTrace();\r
+            Log.e(TAG, "Access to account credentials failed", e);\r
+            // TODO - check if that log prints the stack trace\r
+            sendFinalBroadcast(downloadResult, null);\r

             return;\r
         }\r
             return;\r
         }\r

-\r

         wdc.setCredentials(username, password);\r
         wdc.allowSelfsignedCertificates();\r
         wdc.setDataTransferProgressListener(this);\r
 \r
         wdc.setCredentials(username, password);\r
         wdc.allowSelfsignedCertificates();\r
         wdc.setDataTransferProgressListener(this);\r
 \r

-        mNotification = new Notification(R.drawable.icon, "Downloading file", System.currentTimeMillis());\r
-\r
+        \r
+        /// download will be in a temporal file\r
+        File tmpFile = new File(getTemporalPath() + mAccount.name + mFilePath);\r
+        \r
+        /// create status notification to show the download progress\r
+        mNotification = new Notification(R.drawable.icon, getString(R.string.downloader_download_in_progress_ticker), System.currentTimeMillis());\r

         mNotification.flags |= Notification.FLAG_ONGOING_EVENT;\r
         mNotification.contentView = new RemoteViews(getApplicationContext().getPackageName(), R.layout.progressbar_layout);\r
         mNotification.contentView.setProgressBar(R.id.status_progress, 100, 0, mTotalDownloadSize == -1);\r
         mNotification.flags |= Notification.FLAG_ONGOING_EVENT;\r
         mNotification.contentView = new RemoteViews(getApplicationContext().getPackageName(), R.layout.progressbar_layout);\r
         mNotification.contentView.setProgressBar(R.id.status_progress, 100, 0, mTotalDownloadSize == -1);\r

+        mNotification.contentView.setTextViewText(R.id.status_text, String.format(getString(R.string.downloader_download_in_progress_content), 0, tmpFile.getName()));\r

         mNotification.contentView.setImageViewResource(R.id.status_icon, R.drawable.icon);\r
         // dvelasco ; contentIntent MUST be assigned to avoid app crashes in versions previous to Android 4.x ;\r
         //              BUT an empty Intent is not a very elegant solution; something smart should happen when a user 'clicks' on a download in the notification bar\r
         mNotification.contentIntent = PendingIntent.getActivity(getApplicationContext(), 0, new Intent(), PendingIntent.FLAG_UPDATE_CURRENT);\r
         mNotification.contentView.setImageViewResource(R.id.status_icon, R.drawable.icon);\r
         // dvelasco ; contentIntent MUST be assigned to avoid app crashes in versions previous to Android 4.x ;\r
         //              BUT an empty Intent is not a very elegant solution; something smart should happen when a user 'clicks' on a download in the notification bar\r
         mNotification.contentIntent = PendingIntent.getActivity(getApplicationContext(), 0, new Intent(), PendingIntent.FLAG_UPDATE_CURRENT);\r

+        mNotificationMngr.notify(R.string.downloader_download_in_progress_ticker, mNotification);\r

         \r
         \r

-        mNotificationMngr.notify(1, mNotification);\r

 \r
 \r

-        // download in a temporal file\r
-        File tmpFile = new File(getTemporalPath() + mAccount.name + mFilePath);\r
+        /// perform the download\r

         tmpFile.getParentFile().mkdirs();\r
         mDownloadsInProgress.put(buildRemoteName(mAccount.name, mRemotePath), tmpFile.getAbsolutePath());\r
         tmpFile.getParentFile().mkdirs();\r
         mDownloadsInProgress.put(buildRemoteName(mAccount.name, mRemotePath), tmpFile.getAbsolutePath());\r

-\r
-        boolean download_result = false;\r

         File newFile = null;\r
         if (wdc.downloadFile(mRemotePath, tmpFile)) {\r
             newFile = new File(getSavePath() + mAccount.name + mFilePath);\r
         File newFile = null;\r
         if (wdc.downloadFile(mRemotePath, tmpFile)) {\r
             newFile = new File(getSavePath() + mAccount.name + mFilePath);\r


@@ -184,42 +193,57 @@ public class FileDownloader extends Service implements OnDatatransferProgressLis
                     new String[] {\r
                             mFilePath.substring(mFilePath.lastIndexOf('/') + 1),\r
                             mAccount.name });\r
                     new String[] {\r
                             mFilePath.substring(mFilePath.lastIndexOf('/') + 1),\r
                             mAccount.name });\r

-                download_result = true;\r
+                downloadResult = true;\r

             }\r
         }\r
             }\r
         }\r

-        \r

         mDownloadsInProgress.remove(buildRemoteName(mAccount.name, mRemotePath));\r
         mDownloadsInProgress.remove(buildRemoteName(mAccount.name, mRemotePath));\r

-        \r
-        mNotificationMngr.cancel(1);\r
-        Intent end = new Intent(DOWNLOAD_FINISH_MESSAGE);\r
-        end.putExtra(EXTRA_DOWNLOAD_RESULT, download_result);\r
-        end.putExtra(ACCOUNT_NAME, mAccount.name);\r
-        end.putExtra(EXTRA_REMOTE_PATH, mRemotePath);\r
-        if (download_result) {\r
-            end.putExtra(EXTRA_FILE_PATH, newFile.getAbsolutePath());\r
-        }\r
-        sendBroadcast(end);\r

 \r
 \r

-        if (download_result) {\r
-            Toast.makeText(this, R.string.downloader_download_succeed , Toast.LENGTH_SHORT).show();\r
-        } else {\r
-            Toast.makeText(this, R.string.downloader_download_failed , Toast.LENGTH_SHORT).show();\r
-        }\r

         \r
         \r

+        /// notify result\r
+        mNotificationMngr.cancel(R.string.downloader_download_in_progress_ticker);\r
+        int tickerId = (downloadResult) ? R.string.downloader_download_succeed_ticker : R.string.downloader_download_failed_ticker;\r
+        int contentId = (downloadResult) ? R.string.downloader_download_succeed_content : R.string.downloader_download_failed_content;\r
+        Notification finalNotification = new Notification(R.drawable.icon, getString(tickerId), System.currentTimeMillis());\r
+        finalNotification.flags |= Notification.FLAG_AUTO_CANCEL;\r
+        // dvelasco ; contentIntent MUST be assigned to avoid app crashes in versions previous to Android 4.x ;\r
+        //              BUT an empty Intent is not a very elegant solution; something smart should happen when a user 'clicks' on a download in the notification bar\r
+        finalNotification.contentIntent = PendingIntent.getActivity(getApplicationContext(), 0, new Intent(), PendingIntent.FLAG_UPDATE_CURRENT);\r
+        finalNotification.setLatestEventInfo(getApplicationContext(), getString(tickerId), String.format(getString(contentId), tmpFile.getName()), finalNotification.contentIntent);\r
+        mNotificationMngr.notify(tickerId, finalNotification);\r
+            \r
+        sendFinalBroadcast(downloadResult, (downloadResult)?newFile.getAbsolutePath():null);\r

     }\r
 \r
     }\r
 \r

+    \r

     @Override\r
     public void transferProgress(long progressRate) {\r
     @Override\r
     public void transferProgress(long progressRate) {\r

-        mCurrentDownlodSize += progressRate;\r
-        int percent = (int)(100.0*((double)mCurrentDownlodSize)/((double)mTotalDownloadSize));\r
+        mCurrentDownloadSize += progressRate;\r
+        int percent = (int)(100.0*((double)mCurrentDownloadSize)/((double)mTotalDownloadSize));\r

         if (percent != mLastPercent) {\r
         if (percent != mLastPercent) {\r

-          mNotification.contentView.setProgressBar(R.id.status_progress, 100, (int)(100*mCurrentDownlodSize/mTotalDownloadSize), mTotalDownloadSize == -1);\r
-          mNotification.contentView.setTextViewText(R.id.status_text, percent+"%");\r
-          mNotificationMngr.notify(1, mNotification);\r
+          mNotification.contentView.setProgressBar(R.id.status_progress, 100, (int)(100*mCurrentDownloadSize/mTotalDownloadSize), mTotalDownloadSize == -1);\r
+          mNotification.contentView.setTextViewText(R.id.status_text, String.format(getString(R.string.downloader_download_in_progress_content), percent, new File(mFilePath).getName()));\r
+          mNotificationMngr.notify(R.string.downloader_download_in_progress_ticker, mNotification);\r

         }\r
         \r
         mLastPercent = percent;\r
     }\r
     \r
         }\r
         \r
         mLastPercent = percent;\r
     }\r
     \r

-    \r
+\r
+    /**\r
+     * Sends a broadcast in order to the interested activities can update their view\r
+     * \r
+     * @param downloadResult        'True' if the download was successful\r
+     * @param newFilePath           Absolute path to the download file\r
+     */\r
+    private void sendFinalBroadcast(boolean downloadResult, String newFilePath) {\r
+        Intent end = new Intent(DOWNLOAD_FINISH_MESSAGE);\r
+        end.putExtra(EXTRA_DOWNLOAD_RESULT, downloadResult);\r
+        end.putExtra(ACCOUNT_NAME, mAccount.name);\r
+        end.putExtra(EXTRA_REMOTE_PATH, mRemotePath);\r
+        if (downloadResult) {\r
+            end.putExtra(EXTRA_FILE_PATH, newFilePath);\r
+        }\r
+        sendBroadcast(end);\r
+    }\r
+\r

 }\r
 }\r

diff --git a/src/eu/alefzero/owncloud/ui/fragment/FileDetailFragment.java b/src/eu/alefzero/owncloud/ui/fragment/FileDetailFragment.java
index acc07bf..5d69455 100644 (file)
--- a/src/eu/alefzero/owncloud/ui/fragment/FileDetailFragment.java
+++ b/src/eu/alefzero/owncloud/ui/fragment/FileDetailFragment.java
@@ -218,7 +218,6 @@ public class FileDetailFragment extends SherlockFragment implements
                 i.putExtra(FileDownloader.EXTRA_FILE_SIZE, mFile.getFileLength());\r
                 \r
                 // update ui \r
                 i.putExtra(FileDownloader.EXTRA_FILE_SIZE, mFile.getFileLength());\r
                 \r
                 // update ui \r

-                Toast.makeText(getActivity(), "Downloading", Toast.LENGTH_LONG).show();\r

                 setButtonsForDownloading();\r
                 \r
                 getActivity().startService(i);\r
                 setButtonsForDownloading();\r
                 \r
                 getActivity().startService(i);\r

diff --git a/src/eu/alefzero/webdav/WebdavClient.java b/src/eu/alefzero/webdav/WebdavClient.java
index 7194195..1af39b4 100644 (file)
--- a/src/eu/alefzero/webdav/WebdavClient.java
+++ b/src/eu/alefzero/webdav/WebdavClient.java
@@ -137,24 +137,25 @@ public class WebdavClient extends HttpClient {
      * Downloads a file in remoteFilepath to the local targetPath.\r
      * \r
      * @param remoteFilepath    Path to the file in the remote server, URL DECODED. \r
      * Downloads a file in remoteFilepath to the local targetPath.\r
      * \r
      * @param remoteFilepath    Path to the file in the remote server, URL DECODED. \r

-     * @param targetPath        Local path to save the downloaded file.\r
+     * @param targetFile        Local path to save the downloaded file.\r

      * @return                  'True' when the file is successfully downloaded.\r
      */\r
      * @return                  'True' when the file is successfully downloaded.\r
      */\r

-    public boolean downloadFile(String remoteFilepath, File targetPath) {\r
+    public boolean downloadFile(String remoteFilepath, File targetFile) {\r

         boolean ret = false;\r
         boolean ret = false;\r

+        boolean caughtException = false;\r

         GetMethod get = new GetMethod(mUri.toString() + WebdavUtils.encodePath(remoteFilepath));\r
 \r
         // get.setHeader("Host", mUri.getHost());\r
         // get.setHeader("User-Agent", "Android-ownCloud");\r
 \r
         GetMethod get = new GetMethod(mUri.toString() + WebdavUtils.encodePath(remoteFilepath));\r
 \r
         // get.setHeader("Host", mUri.getHost());\r
         // get.setHeader("User-Agent", "Android-ownCloud");\r
 \r

+        int status = -1;\r

         try {\r
         try {\r

-            int status = executeMethod(get, 0);\r
-            Log.e(TAG, "status return: " + status);\r
+            status = executeMethod(get);\r

             if (status == HttpStatus.SC_OK) {\r
             if (status == HttpStatus.SC_OK) {\r

-                targetPath.createNewFile();\r
+                targetFile.createNewFile();\r

                 BufferedInputStream bis = new BufferedInputStream(\r
                         get.getResponseBodyAsStream());\r
                 BufferedInputStream bis = new BufferedInputStream(\r
                         get.getResponseBodyAsStream());\r

-                FileOutputStream fos = new FileOutputStream(targetPath);\r
+                FileOutputStream fos = new FileOutputStream(targetFile);\r

 \r
                 byte[] bytes = new byte[4096];\r
                 int readResult;\r
 \r
                 byte[] bytes = new byte[4096];\r
                 int readResult;\r


@@ -166,11 +167,28 @@ public class WebdavClient extends HttpClient {
                 ret = true;\r
             }\r
             \r
                 ret = true;\r
             }\r
             \r

-        } catch (Throwable e) {\r
-            e.printStackTrace();\r
-            targetPath.delete();\r
+        } catch (HttpException e) {\r
+            Log.e(TAG, "HTTP exception downloading " + remoteFilepath, e);\r
+            caughtException = true;\r
+\r
+        } catch (IOException e) {\r
+            Log.e(TAG, "I/O exception downloading " + remoteFilepath, e);\r
+            caughtException = true;\r
+\r
+        } catch (Exception e) {\r
+            Log.e(TAG, "Unexpected exception downloading " + remoteFilepath, e);\r
+            caughtException = true;\r
+            \r
+        } finally {\r
+            if (!ret) {\r
+                if (!caughtException) {\r
+                    Log.e(TAG, "Download of " + remoteFilepath + " to " + targetFile + " failed with HTTP status " + status);\r
+                }\r
+                if (targetFile.exists()) {\r
+                    targetFile.delete();\r
+                }\r
+            }\r

         }\r
         }\r

-        \r

         return ret;\r
     }\r
     \r
         return ret;\r
     }\r
     \r