Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 376e5213 authored by Vincent Bourgmayer's avatar Vincent Bourgmayer
Browse files

Refactor DownloadFileOperation.java

- Extract code from run() into onDownloadSuccess(String tmpTargetFodlerPath);
- Extract code from run() into moveFileToRealLocation(File tmpFile, File targetFile);
and update code to avoid NPE and SecurityException
- Increase patch version number
parent 60d10b1f
Loading
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -5,7 +5,7 @@ plugins {

def versionMajor = 1
def versionMinor = 2
def versionPatch = 4
def versionPatch = 5



+62 −32
Original line number Diff line number Diff line
@@ -9,6 +9,11 @@

package foundation.e.drive.operations;

import static com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode.FILE_NOT_FOUND;
import static com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode.FORBIDDEN;
import static com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode.INVALID_OVERWRITE;
import static com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode.OK;

import android.content.Context;
import com.owncloud.android.lib.common.OwnCloudClient;
import com.owncloud.android.lib.common.operations.RemoteOperation;
@@ -72,40 +77,13 @@ public class DownloadFileOperation extends RemoteOperation {
        boolean mustRestart = true;

        if (downloadResult.isSuccess()) {
            final String tmpFilePath = tmpTargetFolderPath+remoteFile.getRemotePath();
            final File tmpFile = new File(tmpFilePath);

            if (!tmpFile.exists()) {
                Timber.d( "Missing downloaded temporary file for %s", remoteFile.getRemotePath());
                resultCode = RemoteOperationResult.ResultCode.FILE_NOT_FOUND;

            } else if (tmpFile.length() != remoteFile.getLength()) {
                Timber.d( "Local and remote version of %s doesn't match", remoteFile.getRemotePath());
                resultCode = RemoteOperationResult.ResultCode.INVALID_OVERWRITE;
                tmpFile.delete();
            } else { //file has been correctly download.

                final File localFile = new File(targetPath);
                if (!localFile.getParentFile().exists()) {
                    localFile.getParentFile().mkdirs();
                } else if (localFile.exists()) {
                    localFile.delete();
                }

                if (!tmpFile.renameTo(localFile)) {
                    Timber.d("failed to move %s to %s", tmpFile.getAbsolutePath(), targetPath);
                    return new RemoteOperationResult(RemoteOperationResult.ResultCode.FORBIDDEN);
                }

                syncedFileState.setLocalLastModified(localFile.lastModified())
                        .setLastETAG(remoteFile.getEtag());
            resultCode = onDownloadSuccess(tmpTargetFolderPath);

            if (resultCode == OK || resultCode == FORBIDDEN) {
                mustRestart = false;
                resultCode = RemoteOperationResult.ResultCode.OK;
                //needed to make Gallery show new image
                CommonUtils.doActionMediaScannerConnexionScanFile(context, syncedFileState.getLocalPath());
            }
        } else { //If download failed

        } else {
            Timber.d("Download failed: %s, %s", downloadResult.getCode(), downloadResult.getLogMessage());
            resultCode = RemoteOperationResult.ResultCode.UNKNOWN_ERROR;
        }
@@ -117,13 +95,65 @@ public class DownloadFileOperation extends RemoteOperation {
                this.restartCounter += 1;
                return this.run(ownCloudClient);
            } else {
                resultCode = RemoteOperationResult.ResultCode.INVALID_OVERWRITE;
                resultCode = INVALID_OVERWRITE;
            }
        }

        //So now, we can update instance of SyncedState and save it to DB
        if (DbHelper.manageSyncedFileStateDB(syncedFileState, "UPDATE", context) <= 0) {
            //todo : define what to do in this case. Is this test even relevant ?
        }
        return new RemoteOperationResult(resultCode);
    }


    private RemoteOperationResult.ResultCode onDownloadSuccess(String tmpTargetFolderPath) {
        final String tmpFilePath = tmpTargetFolderPath + remoteFile.getRemotePath();
        final File tmpFile = new File(tmpFilePath);

        if (!tmpFile.exists()) {
            Timber.d("Missing downloaded temporary file for %s", remoteFile.getRemotePath());
            return FILE_NOT_FOUND;
        }

        if (tmpFile.length() != remoteFile.getLength()) {
            Timber.d("Local and remote version of %s doesn't match", remoteFile.getRemotePath());
            tmpFile.delete();
            return INVALID_OVERWRITE;
        }

        final File targetFile = new File(targetPath);
        if (!moveFileToRealLocation(tmpFile, targetFile)) {
            Timber.d("Failed to move %s to %s", tmpFile.getAbsolutePath(), targetPath);
            return FORBIDDEN;
        }

        syncedFileState.setLocalLastModified(targetFile.lastModified())
                .setLastETAG(remoteFile.getEtag());

        CommonUtils.doActionMediaScannerConnexionScanFile(context, syncedFileState.getLocalPath()); //required to make Gallery show new image
        return OK;
    }

    /**
     * Move the temporary file downloaded to its final location
     * @param tmpFile Temporary file that has just been downloaded
     * @param targetFile real location of the file
     * @return true if success, false otherwise
     */
    private boolean moveFileToRealLocation(File tmpFile, File targetFile) {
        final File targetFolder = targetFile.getParentFile();

        try {
            if (!targetFolder.exists()) {
                targetFolder.mkdirs();
            } else if (targetFile.exists()) {
                targetFile.delete();
            }
            return tmpFile.renameTo(targetFile);
        } catch (SecurityException | NullPointerException exception) {
            Timber.e(exception);
        }
        return false;
    }
}