Skip to content

Auto upload can compare local and remote files. #7894

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tomaszduda23 wants to merge 1 commit into from
Closed

Auto upload can compare local and remote files. #7894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/main/java/com/owncloud/android/files/services/FileUploader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1120,7 +1120,8 @@ public enum NameCollisionPolicy {
RENAME, // Ordinal corresponds to old forceOverwrite = false (0 in database)
OVERWRITE, // Ordinal corresponds to old forceOverwrite = true (1 in database)
CANCEL,
ASK_USER;
ASK_USER,
ASK_USER_IF_DIFF;

public static final NameCollisionPolicy DEFAULT = RENAME;

Expand Down
278 changes: 174 additions & 104 deletions src/main/java/com/owncloud/android/operations/UploadFileOperation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import com.owncloud.android.lib.common.utils.Log_OC;
import com.owncloud.android.lib.resources.e2ee.UnlockFileRemoteOperation;
import com.owncloud.android.lib.resources.files.ChunkedFileUploadRemoteOperation;
import com.owncloud.android.lib.resources.files.DownloadFileRemoteOperation;
import com.owncloud.android.lib.resources.files.ExistenceCheckRemoteOperation;
import com.owncloud.android.lib.resources.files.ReadFileRemoteOperation;
import com.owncloud.android.lib.resources.files.UploadFileRemoteOperation;
Expand All @@ -66,6 +67,7 @@

import org.apache.commons.httpclient.HttpStatus;
import org.apache.commons.httpclient.methods.RequestEntity;
import org.apache.commons.io.FileUtils;
import org.lukhnos.nnio.file.Files;
import org.lukhnos.nnio.file.Paths;

Expand All @@ -81,18 +83,21 @@
import java.nio.channels.FileLock;
import java.nio.channels.OverlappingFileLockException;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicBoolean;

import androidx.annotation.CheckResult;

import static com.owncloud.android.files.services.FileUploader.NameCollisionPolicy.ASK_USER_IF_DIFF;


/**
* Operation performing the update in the ownCloud server
* of a file that was modified locally.
*/
public class UploadFileOperation extends SyncOperation {
public class UploadFileOperation extends SyncOperation implements OnDatatransferProgressListener{

private static final String TAG = UploadFileOperation.class.getSimpleName();

Expand Down Expand Up @@ -120,6 +125,7 @@ public class UploadFileOperation extends SyncOperation {
private boolean mWhileChargingOnly;
private boolean mIgnoringPowerSaveMode;
private final boolean mDisableRetries;
private boolean mTheSameFiles;

private boolean mWasRenamed;
private long mOCUploadId;
Expand All @@ -136,8 +142,7 @@ public class UploadFileOperation extends SyncOperation {
private Context mContext;

private UploadFileRemoteOperation mUploadOperation;

private RequestEntity mEntity;
private DownloadFileRemoteOperation mDownloadOperation;

private final User user;
private final OCUpload mUpload;
Expand Down Expand Up @@ -336,24 +341,12 @@ public void addDataTransferProgressListener(OnDatatransferProgressListener liste
synchronized (mDataTransferListeners) {
mDataTransferListeners.add(listener);
}
if (mEntity != null) {
((ProgressiveDataTransfer) mEntity).addDataTransferProgressListener(listener);
}
if (mUploadOperation != null) {
mUploadOperation.addDataTransferProgressListener(listener);
}
}

public void removeDataTransferProgressListener(OnDatatransferProgressListener listener) {
synchronized (mDataTransferListeners) {
mDataTransferListeners.remove(listener);
}
if (mEntity != null) {
((ProgressiveDataTransfer) mEntity).removeDataTransferProgressListener(listener);
}
if (mUploadOperation != null) {
mUploadOperation.removeDataTransferProgressListener(listener);
}
}

public UploadFileOperation addRenameUploadListener(OnRenameListener listener) {
Expand Down Expand Up @@ -582,9 +575,7 @@ private RemoteOperationResult encryptedUpload(OwnCloudClient client, OCFile pare
);
}

for (OnDatatransferProgressListener mDataTransferListener : mDataTransferListeners) {
mUploadOperation.addDataTransferProgressListener(mDataTransferListener);
}
mUploadOperation.addDataTransferProgressListener(this);

if (mCancellationRequested.get()) {
throw new OperationCancelledException();
Expand Down Expand Up @@ -755,84 +746,84 @@ private RemoteOperationResult normalUpload(OwnCloudClient client) {
return result;
}

// Get the last modification date of the file from the file system
Long timeStampLong = originalFile.lastModified() / 1000;
String timeStamp = timeStampLong.toString();
if(!mTheSameFiles) {
// Get the last modification date of the file from the file system
Long timeStampLong = originalFile.lastModified() / 1000;
String timeStamp = timeStampLong.toString();

FileChannel channel = null;
try {
channel = new RandomAccessFile(mFile.getStoragePath(), "rw").getChannel();
fileLock = channel.tryLock();
} catch (FileNotFoundException e) {
// this basically means that the file is on SD card
// try to copy file to temporary dir if it doesn't exist
String temporalPath = FileStorageUtils.getInternalTemporalPath(user.getAccountName(), mContext) +
mFile.getRemotePath();
mFile.setStoragePath(temporalPath);
temporalFile = new File(temporalPath);

Files.deleteIfExists(Paths.get(temporalPath));
result = copy(originalFile, temporalFile);

if (result.isSuccess()) {
if (temporalFile.length() == originalFile.length()) {
channel = new RandomAccessFile(temporalFile.getAbsolutePath(), "rw").getChannel();
fileLock = channel.tryLock();
} else {
result = new RemoteOperationResult(ResultCode.LOCK_FAILED);
FileChannel channel = null;
try {
channel = new RandomAccessFile(mFile.getStoragePath(), "rw").getChannel();
fileLock = channel.tryLock();
} catch (FileNotFoundException e) {
// this basically means that the file is on SD card
// try to copy file to temporary dir if it doesn't exist
String temporalPath = FileStorageUtils.getInternalTemporalPath(user.getAccountName(), mContext) +
mFile.getRemotePath();
mFile.setStoragePath(temporalPath);
temporalFile = new File(temporalPath);

Files.deleteIfExists(Paths.get(temporalPath));
result = copy(originalFile, temporalFile);

if (result.isSuccess()) {
if (temporalFile.length() == originalFile.length()) {
channel = new RandomAccessFile(temporalFile.getAbsolutePath(), "rw").getChannel();
fileLock = channel.tryLock();
} else {
result = new RemoteOperationResult(ResultCode.LOCK_FAILED);
}
}
}
}

try {
size = channel.size();
} catch (Exception e1) {
size = new File(mFile.getStoragePath()).length();
}

for (OCUpload ocUpload : uploadsStorageManager.getAllStoredUploads()) {
if (ocUpload.getUploadId() == getOCUploadId()) {
ocUpload.setFileSize(size);
uploadsStorageManager.updateUpload(ocUpload);
break;
try {
size = channel.size();
} catch (Exception e1) {
size = new File(mFile.getStoragePath()).length();
}
}

// perform the upload
if (size > ChunkedFileUploadRemoteOperation.CHUNK_SIZE_MOBILE) {
boolean onWifiConnection = connectivityService.getConnectivity().isWifi();
for (OCUpload ocUpload : uploadsStorageManager.getAllStoredUploads()) {
if (ocUpload.getUploadId() == getOCUploadId()) {
ocUpload.setFileSize(size);
uploadsStorageManager.updateUpload(ocUpload);
break;
}
}

mUploadOperation = new ChunkedFileUploadRemoteOperation(mFile.getStoragePath(),
mFile.getRemotePath(),
mFile.getMimeType(),
mFile.getEtagInConflict(),
timeStamp,
onWifiConnection,
mDisableRetries);
} else {
mUploadOperation = new UploadFileRemoteOperation(mFile.getStoragePath(),
mFile.getRemotePath(),
mFile.getMimeType(),
mFile.getEtagInConflict(),
timeStamp,
mDisableRetries);
}
// perform the upload
if (size > ChunkedFileUploadRemoteOperation.CHUNK_SIZE_MOBILE) {
boolean onWifiConnection = connectivityService.getConnectivity().isWifi();

mUploadOperation = new ChunkedFileUploadRemoteOperation(mFile.getStoragePath(),
mFile.getRemotePath(),
mFile.getMimeType(),
mFile.getEtagInConflict(),
timeStamp,
onWifiConnection,
mDisableRetries);
} else {
mUploadOperation = new UploadFileRemoteOperation(mFile.getStoragePath(),
mFile.getRemotePath(),
mFile.getMimeType(),
mFile.getEtagInConflict(),
timeStamp,
mDisableRetries);
}

for (OnDatatransferProgressListener mDataTransferListener : mDataTransferListeners) {
mUploadOperation.addDataTransferProgressListener(mDataTransferListener);
}
mUploadOperation.addDataTransferProgressListener(this);

if (mCancellationRequested.get()) {
throw new OperationCancelledException();
}
if (mCancellationRequested.get()) {
throw new OperationCancelledException();
}

if (result.isSuccess() && mUploadOperation != null) {
result = mUploadOperation.execute(client);
if (result.isSuccess() && mUploadOperation != null) {
result = mUploadOperation.execute(client);

/// move local temporal file or original file to its corresponding
// location in the Nextcloud local folder
if (!result.isSuccess() && result.getHttpCode() == HttpStatus.SC_PRECONDITION_FAILED) {
result = new RemoteOperationResult(ResultCode.SYNC_CONFLICT);
/// move local temporal file or original file to its corresponding
// location in the Nextcloud local folder
if (!result.isSuccess() && result.getHttpCode() == HttpStatus.SC_PRECONDITION_FAILED) {
result = new RemoteOperationResult(ResultCode.SYNC_CONFLICT);
}
}
}
} catch (FileNotFoundException e) {
Expand Down Expand Up @@ -923,25 +914,28 @@ private RemoteOperationResult checkNameCollision(OwnCloudClient client,
Log_OC.d(TAG, "Checking name collision in server");

if (existsFile(client, mRemotePath, metadata, encrypted)) {
switch (mNameCollisionPolicy) {
case CANCEL:
Log_OC.d(TAG, "File exists; canceling");
throw new OperationCancelledException();
case RENAME:
mRemotePath = getNewAvailableRemotePath(client, mRemotePath, metadata, encrypted);
mWasRenamed = true;
createNewOCFile(mRemotePath);
Log_OC.d(TAG, "File renamed as " + mRemotePath);
if (mRenameUploadListener != null) {
mRenameUploadListener.onRenameUpload();
}
break;
case OVERWRITE:
Log_OC.d(TAG, "Overwriting file");
break;
case ASK_USER:
Log_OC.d(TAG, "Name collision; asking the user what to do");
return new RemoteOperationResult(ResultCode.SYNC_CONFLICT);
if(mNameCollisionPolicy != ASK_USER_IF_DIFF || !theSameFiles(client, encrypted)) {
switch (mNameCollisionPolicy) {
case CANCEL:
Log_OC.d(TAG, "File exists; canceling");
throw new OperationCancelledException();
case RENAME:
mRemotePath = getNewAvailableRemotePath(client, mRemotePath, metadata, encrypted);
mWasRenamed = true;
createNewOCFile(mRemotePath);
Log_OC.d(TAG, "File renamed as " + mRemotePath);
if (mRenameUploadListener != null) {
mRenameUploadListener.onRenameUpload();
}
break;
case OVERWRITE:
Log_OC.d(TAG, "Overwriting file");
break;
case ASK_USER_IF_DIFF:
case ASK_USER:
Log_OC.d(TAG, "Name collision; asking the user what to do");
return new RemoteOperationResult(ResultCode.SYNC_CONFLICT);
}
}
}

Expand Down Expand Up @@ -1119,6 +1113,57 @@ private String getNewAvailableRemotePath(OwnCloudClient client, String remotePat
return newPath;
}

private boolean theSameFiles(OwnCloudClient client, boolean encrypted) throws OperationCancelledException {
if(encrypted)
{
Log_OC.d(TAG, "Not implemented for encrypted content");
return false;
}

File originalFile = new File(mOriginalStoragePath);
if(mFile.getFileLength() != 0 && originalFile.length() != mFile.getFileLength())
{
return false;
}

String tmpFolder = FileStorageUtils.getInternalTemporalPath(user.getAccountName(), mContext);
String temporalPath = tmpFolder + mFile.getRemotePath();

try {
Files.deleteIfExists(Paths.get(temporalPath));
} catch (IOException e) {
Log_OC.d(TAG, temporalPath + " unable to remove");
return false;
}

if (mCancellationRequested.get()) {
throw new OperationCancelledException();
}

mDownloadOperation = new DownloadFileRemoteOperation(mFile.getRemotePath(),
Copy link
Member

@tobiasKaminsky tobiasKaminsky Feb 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that you download the entire file, or?
What is the purpose of it?

Copy link
Contributor Author

@tomaszduda23 tomaszduda23 Mar 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A file is downloaded only if another file with the same name and the same size exists on a remote server. In that way the app can decided if the file was uploaded already or it is new one. If the file is new notification about conflict is displayed.

It would be better to use some kind of hash for that but it seems that it is impossible with current nextcloud API.

Let me know if you interested in the idea. Than I will fix a few things in this patch.

Copy link
Member

@tobiasKaminsky tobiasKaminsky Mar 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Downloading the file is unfortunately something we should not do.
Imagine you have 50 conflicts, or conflicts with very big files (>2Gb).

Let us ask at server repo to get such an endpoint/info directly:
nextcloud/server#25949

In general I really like the idea, but we always need to be aware of such edge cases.
I would vote for closing this PR for now, and see if / what is discussed on server issue and then work further on this afterwards?

Copy link
Contributor Author

@tomaszduda23 tomaszduda23 Mar 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now during conflict default behavior is "Ask me every time". I added another option "Ask me if files are different". Downloading file just to compare the content is not the best idea. Still it seems to be better than asking an user regarding each file (I have over 7000 photos) after resetting (db) the app.

I though about adding hash to server side but there was already a few attempts and all of them were rejected. E.g. nextcloud/server#22495

For now downloading and comparing files during conflict seems to be the best option. It could be easily adjusted later on when/if server supports hash API.

BTW even if there are many big files most likely it won't use more data transfer than current options. Rename new version, Overwrite remote version - those will re-upload so transfer will be wasted anyway, Skip uploading - only this save the data transfer. Also for now there is no way for user to know if local and remote files are the same or not.

tmpFolder);
mDownloadOperation.addDatatransferProgressListener(this);

RemoteOperationResult result = mDownloadOperation.execute(client);

if (result.isSuccess()) {
File temporalFile = new File(temporalPath);
try {
mTheSameFiles = FileUtils.contentEquals(temporalFile, originalFile);
} catch (IOException e) {
Log_OC.d(TAG, e.getMessage() + " unable to compare files");
}
}

try {
Files.deleteIfExists(Paths.get(temporalPath));
} catch (IOException e) {
Log_OC.d(TAG, temporalPath + " unable to remove");
}

return mTheSameFiles;
}

private boolean existsFile(OwnCloudClient client, String remotePath, DecryptedFolderMetadata metadata,
boolean encrypted) {
if (encrypted) {
Expand Down Expand Up @@ -1155,6 +1200,9 @@ public void cancel(ResultCode cancellationReason) {
Log_OC.d(TAG, "Cancelling upload during actual upload operation.");
mUploadOperation.cancel(cancellationReason);
}
if(mDownloadOperation != null){
mDownloadOperation.cancel();
}
}

/**
Expand Down Expand Up @@ -1362,6 +1410,28 @@ private void updateOCFile(OCFile file, RemoteFile remoteFile) {
file.setRemoteId(remoteFile.getRemoteId());
}

@Override
public void onTransferProgress(long progressRate, long totalTransferredSoFar, long totalToTransfer, String fileAbsoluteName) {
if(mDownloadOperation != null)
{
//the size of remote and local files are the same
//we need to download the file and upload it again if different so we multiply size by 2
totalToTransfer *= 2;
}
if(mUploadOperation != null)
{
//download was completed so size of the file should be added
totalTransferredSoFar += totalToTransfer;
}
Iterator<OnDatatransferProgressListener> it;
synchronized (mDataTransferListeners) {
it = mDataTransferListeners.iterator();
while (it.hasNext()) {
it.next().onTransferProgress(progressRate, totalTransferredSoFar, totalToTransfer, fileAbsoluteName);
}
}
}

public interface OnRenameListener {

void onRenameUpload();
Expand Down
Loading