Bit of work on uploads

Signed-off-by: Mario Danic <mario@lovelyhq.com>

build.gradle

@@ -182,7 +182,7 @@ dependencies {
     /// dependencies for app building
     compile name: 'touch-image-view'
     compile 'com.android.support:multidex:1.0.1'
-    compile 'com.github.nextcloud:android-library:1.0.22'
+    compile 'com.github.nextcloud:android-library:add-lock-failure-status-SNAPSHOT'
     compile "com.android.support:support-v4:${supportLibraryVersion}"
     compile "com.android.support:design:${supportLibraryVersion}"
     compile 'com.jakewharton:disklrucache:2.0.2'

src/main/java/com/owncloud/android/datamodel/UploadsStorageManager.java

@@ -35,15 +35,12 @@ import com.owncloud.android.db.OCUpload;
 import com.owncloud.android.db.ProviderMeta.ProviderTableMeta;
 import com.owncloud.android.db.UploadResult;
 import com.owncloud.android.files.services.FileUploader;
+import com.owncloud.android.jobs.AutoUploadJob;
 import com.owncloud.android.lib.common.operations.RemoteOperationResult;
 import com.owncloud.android.lib.common.utils.Log_OC;
 import com.owncloud.android.operations.UploadFileOperation;
-import com.owncloud.android.jobs.AutoUploadJob;
 
-import java.util.ArrayList;
 import java.util.Calendar;
-import java.util.Collections;
-import java.util.List;
 import java.util.Observable;
 import java.util.Set;
 
@@ -386,48 +383,8 @@ public class UploadsStorageManager extends Observable {
                 new String[]{account.name}
         );
 
-        //return uploads;
-
-        List<OCUpload> result = getPendingJobs();
-        Collections.addAll(result, uploads);
-        return result.toArray(uploads);
-    }
-
-    /**
-     * Get all uploads which are currently being uploaded or waiting in the queue to be uploaded.
-     */
-    public OCUpload[] getCurrentAndPendingUploads() {
-
-
-        OCUpload[] uploads = getUploads(
-            ProviderTableMeta.UPLOADS_STATUS + "==" + UploadStatus.UPLOAD_IN_PROGRESS.value + " OR " +
-            ProviderTableMeta.UPLOADS_LAST_RESULT + "==" + UploadResult.DELAYED_FOR_WIFI.getValue() + " OR " +
-            ProviderTableMeta.UPLOADS_LAST_RESULT + "==" + UploadResult.DELAYED_FOR_CHARGING.getValue(),
-            null
-        );
-
-        //return uploads;
-
-        List<OCUpload> result = getPendingJobs();
-        Collections.addAll(result, uploads);
-        return result.toArray(uploads);
-    }
-
-    private List<OCUpload> getPendingJobs() {
-        Set<JobRequest> jobRequests = JobManager.create(mContext).getAllJobRequestsForTag(AutoUploadJob.TAG);
-
-        ArrayList<OCUpload> list = new ArrayList<>();
+        return uploads;
 
-        for (JobRequest ji : jobRequests) {
-            PersistableBundleCompat extras = ji.getExtras();
-            OCUpload upload = new OCUpload(extras.getString("filePath", ""),
-                    extras.getString("remotePath", ""),
-                    extras.getString("account", ""));
-
-            list.add(upload);
-        }
-
-        return list;
     }
 
     public void cancelPendingAutoUploadJobsForAccount(Account account) {

src/main/java/com/owncloud/android/db/UploadResult.java

@@ -35,7 +35,8 @@ public enum UploadResult {
     DELAYED_FOR_WIFI(9),
     SERVICE_INTERRUPTED(10),
     DELAYED_FOR_CHARGING(11),
-    MAINTENANCE_MODE(12);
+    MAINTENANCE_MODE(12),
+    LOCK_FAILED(13);
 
     private final int value;
 
@@ -77,6 +78,8 @@ public enum UploadResult {
                 return DELAYED_FOR_CHARGING;
             case 12:
                 return MAINTENANCE_MODE;
+            case 13:
+                return LOCK_FAILED;
         }
         return null;
     }
@@ -120,6 +123,8 @@ public enum UploadResult {
                 return UNKNOWN;
             case MAINTENANCE_MODE:
                 return MAINTENANCE_MODE;
+            case LOCK_FAILED:
+                return LOCK_FAILED;
             default:
                 return UNKNOWN;
         }

src/main/java/com/owncloud/android/files/services/FileUploader.java

@@ -374,7 +374,8 @@ public class FileUploader extends Service
             boolean accountMatch;
             for ( OCUpload failedUpload: failedUploads) {
                 accountMatch = (account == null || account.name.equals(failedUpload.getAccountName()));
-                resultMatch = (uploadResult == null || uploadResult.equals(failedUpload.getLastResult()));
+                resultMatch = ((uploadResult == null || uploadResult.equals(failedUpload.getLastResult())) &&
+                        !uploadResult.equals(UploadResult.FILE_NOT_FOUND));
                 if (accountMatch && resultMatch) {
                     if (currentAccount == null ||
                             !currentAccount.name.equals(failedUpload.getAccountName())) {

src/main/java/com/owncloud/android/operations/UploadFileOperation.java

@@ -1,21 +1,20 @@
 /**
- *   ownCloud Android client application
+ * ownCloud Android client application
  *
- *   @author David A. Velasco
- *   Copyright (C) 2016 ownCloud GmbH.
+ * @author David A. Velasco
+ * Copyright (C) 2016 ownCloud GmbH.
  *
- *   This program is free software: you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License version 2,
- *   as published by the Free Software Foundation.
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2,
+ * as published by the Free Software Foundation.
  *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY; without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *   GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
  *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 package com.owncloud.android.operations;
@@ -55,11 +54,15 @@ import org.apache.commons.httpclient.methods.RequestEntity;
 
 import java.io.File;
 import java.io.FileInputStream;
+import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.io.RandomAccessFile;
 import java.nio.channels.FileChannel;
+import java.nio.channels.FileLock;
+import java.nio.channels.OverlappingFileLockException;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Set;
@@ -192,7 +195,9 @@ public class UploadFileOperation extends SyncOperation {
         return mOnWifiOnly;
     }
 
-    public boolean getIsChargingRequired() {return mWhileChargingOnly;}
+    public boolean getIsChargingRequired() {
+        return mWhileChargingOnly;
+    }
 
     public Account getAccount() {
         return mAccount;
@@ -249,7 +254,7 @@ public class UploadFileOperation extends SyncOperation {
         }
     }
 
-    public int getCreatedBy () {
+    public int getCreatedBy() {
         return mCreatedBy;
     }
 
@@ -261,9 +266,10 @@ public class UploadFileOperation extends SyncOperation {
         return mCreatedBy == CREATED_AS_INSTANT_VIDEO;
     }
 
-    public void setOCUploadId(long id){
+    public void setOCUploadId(long id) {
         mOCUploadId = id;
     }
+
     public long getOCUploadId() {
         return mOCUploadId;
     }
@@ -272,14 +278,14 @@ public class UploadFileOperation extends SyncOperation {
         return mDataTransferListeners;
     }
 
-    public void addDatatransferProgressListener (OnDatatransferProgressListener listener) {
+    public void addDatatransferProgressListener(OnDatatransferProgressListener listener) {
         synchronized (mDataTransferListeners) {
             mDataTransferListeners.add(listener);
         }
         if (mEntity != null) {
-            ((ProgressiveDataTransferer)mEntity).addDatatransferProgressListener(listener);
+            ((ProgressiveDataTransferer) mEntity).addDatatransferProgressListener(listener);
         }
-        if(mUploadOperation != null){
+        if (mUploadOperation != null) {
             mUploadOperation.addDatatransferProgressListener(listener);
         }
     }
@@ -289,14 +295,14 @@ public class UploadFileOperation extends SyncOperation {
             mDataTransferListeners.remove(listener);
         }
         if (mEntity != null) {
-            ((ProgressiveDataTransferer)mEntity).removeDatatransferProgressListener(listener);
+            ((ProgressiveDataTransferer) mEntity).removeDatatransferProgressListener(listener);
         }
-        if(mUploadOperation != null){
+        if (mUploadOperation != null) {
             mUploadOperation.removeDatatransferProgressListener(listener);
         }
     }
 
-    public void addRenameUploadListener (OnRenameListener listener) {
+    public void addRenameUploadListener(OnRenameListener listener) {
         mRenameUploadListener = listener;
     }
 
@@ -309,17 +315,18 @@ public class UploadFileOperation extends SyncOperation {
         File temporalFile = null;
         File originalFile = new File(mOriginalStoragePath);
         File expectedFile = null;
+        FileLock fileLock = null;
 
         try {
 
             /// Check that connectivity conditions are met and delays the upload otherwise
-            if (mOnWifiOnly && !Device.getNetworkType(mContext).equals(JobRequest.NetworkType.UNMETERED)){
+            if (mOnWifiOnly && !Device.getNetworkType(mContext).equals(JobRequest.NetworkType.UNMETERED)) {
                 Log_OC.d(TAG, "Upload delayed until WiFi is available: " + getRemotePath());
                 return new RemoteOperationResult(ResultCode.DELAYED_FOR_WIFI);
             }
 
             // Check if charging conditions are met and delays the upload otherwise
-            if (mWhileChargingOnly && !Device.isCharging(mContext)){
+            if (mWhileChargingOnly && !Device.isCharging(mContext)) {
                 Log_OC.d(TAG, "Upload delayed until the device is charging: " + getRemotePath());
                 return new RemoteOperationResult(ResultCode.DELAYED_FOR_CHARGING);
             }
@@ -384,20 +391,21 @@ public class UploadFileOperation extends SyncOperation {
             }
 
             // Get the last modification date of the file from the file system
-            Long timeStampLong = originalFile.lastModified()/1000;
+            Long timeStampLong = originalFile.lastModified() / 1000;
             String timeStamp = timeStampLong.toString();
 
             /// perform the upload
-            if ( mChunked &&
+            if (mChunked &&
                     (new File(mFile.getStoragePath())).length() >
-                            ChunkedUploadRemoteFileOperation.CHUNK_SIZE ) {
+                            ChunkedUploadRemoteFileOperation.CHUNK_SIZE) {
                 mUploadOperation = new ChunkedUploadRemoteFileOperation(mContext, mFile.getStoragePath(),
                         mFile.getRemotePath(), mFile.getMimetype(), mFile.getEtagInConflict(), timeStamp);
             } else {
                 mUploadOperation = new UploadRemoteFileOperation(mFile.getStoragePath(),
                         mFile.getRemotePath(), mFile.getMimetype(), mFile.getEtagInConflict(), timeStamp);
             }
-            Iterator <OnDatatransferProgressListener> listener = mDataTransferListeners.iterator();
+
+            Iterator<OnDatatransferProgressListener> listener = mDataTransferListeners.iterator();
             while (listener.hasNext()) {
                 mUploadOperation.addDatatransferProgressListener(listener.next());
             }
@@ -406,23 +414,41 @@ public class UploadFileOperation extends SyncOperation {
                 throw new OperationCancelledException();
             }
 
+            FileChannel channel = new RandomAccessFile(mFile.getStoragePath(), "rw").getChannel();
+            fileLock = channel.tryLock();
+
             result = mUploadOperation.execute(client);
 
             /// move local temporal file or original file to its corresponding
             // location in the ownCloud local folder
-            if (!result.isSuccess() && result.getHttpCode() == HttpStatus.SC_PRECONDITION_FAILED ) {
+            if (!result.isSuccess() && result.getHttpCode() == HttpStatus.SC_PRECONDITION_FAILED) {
                 result = new RemoteOperationResult(ResultCode.SYNC_CONFLICT);
             }
 
+        } catch (FileNotFoundException e) {
+            Log_OC.d(TAG, mOriginalStoragePath + " not exists anymore");
+            return new RemoteOperationResult(ResultCode.LOCAL_FILE_NOT_FOUND);
+        } catch (OverlappingFileLockException e) {
+            Log_OC.d(TAG, "Overlapping file lock exception");
+            return new RemoteOperationResult(ResultCode.LOCK_FAILED);
         } catch (Exception e) {
             result = new RemoteOperationResult(e);
 
         } finally {
             mUploadStarted.set(false);
+
+            if (fileLock != null) {
+                try {
+                    fileLock.release();
+                } catch (IOException e) {
+                    Log_OC.d(TAG, "Failed to unlock file with path " + mOriginalStoragePath);
+                }
+            }
+
             if (temporalFile != null && !originalFile.equals(temporalFile)) {
                 temporalFile.delete();
             }
-            if (result == null){
+            if (result == null) {
                 result = new RemoteOperationResult(ResultCode.UNKNOWN_ERROR);
             }
             if (result.isSuccess()) {
@@ -430,7 +456,7 @@ public class UploadFileOperation extends SyncOperation {
                         result.getLogMessage());
             } else {
                 if (result.getException() != null) {
-                    if(result.isCancelled()){
+                    if (result.isCancelled()) {
                         Log_OC.w(TAG, "Upload of " + mOriginalStoragePath + " to " + mRemotePath +
                                 ": " + result.getLogMessage());
                     } else {
@@ -543,7 +569,8 @@ public class UploadFileOperation extends SyncOperation {
 
     /**
      * Create a new OCFile mFile with new remote path. This is required if forceOverwrite==false.
-     * New file is stored as mFile, original as mOldFile. 
+     * New file is stored as mFile, original as mOldFile.
+     *
      * @param newRemotePath new remote path
      */
     private void createNewOCFile(String newRemotePath) {
@@ -592,8 +619,7 @@ public class UploadFileOperation extends SyncOperation {
             suffix = " (" + count + ")";
             if (pos >= 0) {
                 check = existsFile(wc, remotePath + suffix + "." + extension);
-            }
-            else {
+            } else {
                 check = existsFile(wc, remotePath + suffix);
             }
             count++;
@@ -606,7 +632,7 @@ public class UploadFileOperation extends SyncOperation {
         }
     }
 
-    private boolean existsFile(OwnCloudClient client, String remotePath){
+    private boolean existsFile(OwnCloudClient client, String remotePath) {
         ExistenceCheckRemoteOperation existsOperation =
                 new ExistenceCheckRemoteOperation(remotePath, mContext, false);
         RemoteOperationResult result = existsOperation.execute(client);
@@ -644,10 +670,10 @@ public class UploadFileOperation extends SyncOperation {
      * TODO rewrite with homogeneous fail handling, remove dependency on {@link RemoteOperationResult},
      * TODO     use Exceptions instead
      *
-     * @param   sourceFile      Source file to copy.
-     * @param   targetFile      Target location to copy the file.
-     * @return  {@link RemoteOperationResult}
-     * @throws  IOException
+     * @param sourceFile Source file to copy.
+     * @param targetFile Target location to copy the file.
+     * @return {@link RemoteOperationResult}
+     * @throws IOException
      */
     private RemoteOperationResult copy(File sourceFile, File targetFile) throws IOException {
         Log_OC.d(TAG, "Copying local file");
@@ -735,10 +761,10 @@ public class UploadFileOperation extends SyncOperation {
      *
      * TODO refactor both this and 'copy' in a single method
      *
-     * @param   sourceFile      Source file to move.
-     * @param   targetFile      Target location to move the file.
-     * @return  {@link RemoteOperationResult}
-     * @throws  IOException
+     * @param sourceFile Source file to move.
+     * @param targetFile Target location to move the file.
+     * @return {@link RemoteOperationResult}
+     * @throws IOException
      */
     private void move(File sourceFile, File targetFile) throws IOException {
 
@@ -746,8 +772,8 @@ public class UploadFileOperation extends SyncOperation {
             File expectedFolder = targetFile.getParentFile();
             expectedFolder.mkdirs();
 
-            if (expectedFolder.isDirectory()){
-                if (!sourceFile.renameTo(targetFile)){
+            if (expectedFolder.isDirectory()) {
+                if (!sourceFile.renameTo(targetFile)) {
                     // try to copy and then delete
                     targetFile.createNewFile();
                     FileChannel inChannel = new FileInputStream(sourceFile).getChannel();
@@ -755,12 +781,11 @@ public class UploadFileOperation extends SyncOperation {
                     try {
                         inChannel.transferTo(0, inChannel.size(), outChannel);
                         sourceFile.delete();
-                    } catch (Exception e){
+                    } catch (Exception e) {
                         mFile.setStoragePath(""); // forget the local file
                         // by now, treat this as a success; the file was uploaded
                         // the best option could be show a warning message
-                    }
-                    finally {
+                    } finally {
                         if (inChannel != null) {
                             inChannel.close();
                         }
@@ -824,7 +849,7 @@ public class UploadFileOperation extends SyncOperation {
 
         // generate new Thumbnail
         final ThumbnailsCacheManager.ThumbnailGenerationTask task =
-            new ThumbnailsCacheManager.ThumbnailGenerationTask(getStorageManager(), mAccount);
+                new ThumbnailsCacheManager.ThumbnailGenerationTask(getStorageManager(), mAccount);
         task.execute(file);
     }
 

src/main/java/com/owncloud/android/ui/adapter/ExpandableUploadListAdapter.java

@@ -528,6 +528,9 @@ public class ExpandableUploadListAdapter extends BaseExpandableListAdapter imple
                     case MAINTENANCE_MODE:
                         status = mParentActivity.getString(R.string.maintenance_mode);
                         break;
+                    case LOCK_FAILED:
+                        status = mParentActivity.getString(R.string.lock_failed);
+                        break;
                     default:
                         status = "Naughty devs added a new fail result but no description for the user";
                         break;

src/main/res/values/strings.xml

@@ -557,6 +557,7 @@
     <string name="confirmation_remove_files_alert">Do you really want to remove the selected items?</string>
     <string name="confirmation_remove_folders_alert">Do you really want to remove the selected items and their contents?</string>
     <string name="maintenance_mode">Server in maintenance mode</string>
+    <string name="lock_failed">Failed to lock the file</string>
 
     <string name="uploads_view_upload_status_waiting_for_charging">Awaiting charge</string>
     <string name="actionbar_search">Search</string>