Skip to content

Commit

Permalink
Merge pull request #13993 from nextcloud/bugfix/dont-use-file-downloa…
Browse files Browse the repository at this point in the history 
…d-worker-for-two-way-sync

BugFix - Don't Use File Download Worker for Two Way Sync
  • Loading branch information
@tobiasKaminsky
tobiasKaminsky authored Nov 11, 2024
2 parents e2dfea7 + b85affb commit ca29ea9
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 26 deletions.
13 changes: 9 additions & 4 deletions app/src/main/java/com/nextcloud/client/jobs/InternalTwoWaySyncWork.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class InternalTwoWaySyncWork(
private val appPreferences: AppPreferences
) : Worker(context, params) {
private var shouldRun = true
private var operation: SynchronizeFolderOperation? = null

override fun doWork(): Result {
Log_OC.d(TAG, "Worker started!")
Expand Down Expand Up @@ -66,18 +67,21 @@ class InternalTwoWaySyncWork(
}

Log_OC.d(TAG, "Folder ${folder.remotePath}: started!")
val operation = SynchronizeFolderOperation(context, folder.remotePath, user, fileDataStorageManager)
.execute(context)
operation = SynchronizeFolderOperation(context, folder.remotePath, user, fileDataStorageManager, true)
val operationResult = operation?.execute(context)

if (operation.isSuccess) {
if (operationResult?.isSuccess == true) {
Log_OC.d(TAG, "Folder ${folder.remotePath}: finished!")
} else {
Log_OC.d(TAG, "Folder ${folder.remotePath} failed!")
result = false
}

folder.apply {
internalFolderSyncResult = operation.code.toString()
operationResult?.let {
internalFolderSyncResult = it.code.toString()
}

internalFolderSyncTimestamp = System.currentTimeMillis()
}

Expand All @@ -96,6 +100,7 @@ class InternalTwoWaySyncWork(

override fun onStopped() {
Log_OC.d(TAG, "OnStopped of worker called!")
operation?.cancel()
shouldRun = false
super.onStopped()
}
Expand Down
3 changes: 2 additions & 1 deletion app/src/main/java/com/nextcloud/client/jobs/OfflineSyncWork.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ class OfflineSyncWork constructor(
user,
true,
context,
storageManager
storageManager,
true
)
synchronizeFileOperation.execute(context)
}
Expand Down
44 changes: 33 additions & 11 deletions app/src/main/java/com/owncloud/android/operations/SynchronizeFileOperation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ public class SynchronizeFileOperation extends SyncOperation {
private boolean mSyncFileContents;
private Context mContext;
private boolean mTransferWasRequested;
private final boolean syncInBackgroundWorker;


/**
* When 'false', uploads to the server are not done; only downloads or conflict detection. This is a temporal
Expand Down Expand Up @@ -74,7 +76,8 @@ public SynchronizeFileOperation(
User user,
boolean syncFileContents,
Context context,
FileDataStorageManager storageManager) {
FileDataStorageManager storageManager,
boolean syncInBackgroundWorker) {
super(storageManager);

mRemotePath = remotePath;
Expand All @@ -84,6 +87,7 @@ public SynchronizeFileOperation(
mSyncFileContents = syncFileContents;
mContext = context;
mAllowUploads = true;
this.syncInBackgroundWorker = syncInBackgroundWorker;
}


Expand All @@ -110,7 +114,8 @@ public SynchronizeFileOperation(
User user,
boolean syncFileContents,
Context context,
FileDataStorageManager storageManager) {
FileDataStorageManager storageManager,
boolean syncInBackgroundWorker) {
super(storageManager);

mLocalFile = localFile;
Expand All @@ -130,6 +135,7 @@ public SynchronizeFileOperation(
mSyncFileContents = syncFileContents;
mContext = context;
mAllowUploads = true;
this.syncInBackgroundWorker = syncInBackgroundWorker;
}


Expand Down Expand Up @@ -159,9 +165,9 @@ public SynchronizeFileOperation(
boolean syncFileContents,
boolean allowUploads,
Context context,
FileDataStorageManager storageManager) {

this(localFile, serverFile, user, syncFileContents, context, storageManager);
FileDataStorageManager storageManager,
boolean syncInBackgroundWorker) {
this(localFile, serverFile, user, syncFileContents, context, storageManager, syncInBackgroundWorker);
mAllowUploads = allowUploads;
}

Expand Down Expand Up @@ -295,13 +301,29 @@ private void requestForUpload(OCFile file) {
}

private void requestForDownload(OCFile file) {
Log_OC.d("InternalTwoWaySyncWork", "download file: " + file.getFileName());

FileDownloadHelper.Companion.instance().downloadFile(
mUser,
file);
if (syncInBackgroundWorker) {
Log_OC.d("InternalTwoWaySyncWork", "download file: " + file.getFileName());

mTransferWasRequested = true;
try {
final var operation = new DownloadFileOperation(mUser, file, mContext);
var result = operation.execute(getClient());

mTransferWasRequested = true;

String filename = file.getFileName();
if (filename != null) {
if (result.isSuccess()) {
Log_OC.d(TAG, "requestForDownload completed for: " + file.getFileName());
} else {
Log_OC.d(TAG, "requestForDownload failed for: " + file.getFileName());
}
}
} catch (Exception e) {
Log_OC.d(TAG, "Exception caught at requestForDownload" + e);
}
} else {
FileDownloadHelper.Companion.instance().downloadFile(mUser, file);
}
}

public boolean transferWasRequested() {
Expand Down
49 changes: 43 additions & 6 deletions app/src/main/java/com/owncloud/android/operations/SynchronizeFolderOperation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import java.util.Map;
import java.util.Vector;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;

Expand Down Expand Up @@ -87,6 +86,8 @@ public class SynchronizeFolderOperation extends SyncOperation {

private final AtomicBoolean mCancellationRequested;

private final boolean syncInBackgroundWorker;

/**
* Creates a new instance of {@link SynchronizeFolderOperation}.
*
Expand All @@ -97,7 +98,8 @@ public class SynchronizeFolderOperation extends SyncOperation {
public SynchronizeFolderOperation(Context context,
String remotePath,
User user,
FileDataStorageManager storageManager) {
FileDataStorageManager storageManager,
boolean syncInBackgroundWorker) {
super(storageManager);

mRemotePath = remotePath;
Expand All @@ -107,6 +109,7 @@ public SynchronizeFolderOperation(Context context,
mFilesForDirectDownload = new Vector<>();
mFilesToSyncContents = new Vector<>();
mCancellationRequested = new AtomicBoolean(false);
this.syncInBackgroundWorker = syncInBackgroundWorker;
}


Expand Down Expand Up @@ -406,7 +409,8 @@ private void classifyFileForLaterSyncOrDownload(OCFile remoteFile, OCFile localF
user,
true,
mContext,
getStorageManager()
getStorageManager(),
syncInBackgroundWorker
);
mFilesToSyncContents.add(operation);
}
Expand All @@ -427,7 +431,8 @@ private void prepareOpsFromLocalKnowledge() throws OperationCancelledException {
user,
true,
mContext,
getStorageManager()
getStorageManager(),
syncInBackgroundWorker
);
mFilesToSyncContents.add(operation);
}
Expand All @@ -441,8 +446,40 @@ private void syncContents() throws OperationCancelledException {
}

private void startDirectDownloads() {
final var fileDownloadHelper = FileDownloadHelper.Companion.instance();
mFilesForDirectDownload.forEach(file -> fileDownloadHelper.downloadFile(user, file));
if (syncInBackgroundWorker) {
try {
for (OCFile file: mFilesForDirectDownload) {
synchronized (mCancellationRequested) {
if (mCancellationRequested.get()) {
break;
}
}

if (file == null) {
continue;
}

final var operation = new DownloadFileOperation(user, file, mContext);
var result = operation.execute(getClient());

String filename = file.getFileName();
if (filename == null) {
continue;
}

if (result.isSuccess()) {
Log_OC.d(TAG, "startDirectDownloads completed for: " + file.getFileName());
} else {
Log_OC.d(TAG, "startDirectDownloads failed for: " + file.getFileName());
}
}
} catch (Exception e) {
Log_OC.d(TAG, "Exception caught at startDirectDownloads" + e);
}
} else {
final var fileDownloadHelper = FileDownloadHelper.Companion.instance();
mFilesForDirectDownload.forEach(file -> fileDownloadHelper.downloadFile(user, file));
}
}

/**
Expand Down
6 changes: 4 additions & 2 deletions app/src/main/java/com/owncloud/android/services/OperationsService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,8 @@ private Pair<Target, RemoteOperation> newOperation(Intent operationIntent) {
user,
syncFileContents,
getApplicationContext(),
fileDataStorageManager);
fileDataStorageManager,
false);
break;

case ACTION_SYNC_FOLDER:
Expand All @@ -707,7 +708,8 @@ private Pair<Target, RemoteOperation> newOperation(Intent operationIntent) {
this, // TODO remove this dependency from construction time
remotePath,
user,
fileDataStorageManager
fileDataStorageManager,
false
);
break;

Expand Down
6 changes: 4 additions & 2 deletions app/src/main/java/com/owncloud/android/ui/helpers/FileOperationsHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ private void syncFile(OCFile file, User user, FileDataStorageManager storageMana
user,
true,
fileActivity,
storageManager);
storageManager,
false);
RemoteOperationResult result = sfo.execute(fileActivity);

if (result.getCode() == RemoteOperationResult.ResultCode.SYNC_CONFLICT) {
Expand Down Expand Up @@ -303,7 +304,8 @@ public void run() {
user,
true,
fileActivity,
storageManager);
storageManager,
false);
RemoteOperationResult result = sfo.execute(fileActivity);
fileActivity.dismissLoadingDialog();
if (result.getCode() == RemoteOperationResult.ResultCode.SYNC_CONFLICT) {
Expand Down

0 comments on commit ca29ea9

Please sign in to comment.