Skip to content
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

Jump to bottom

BugFix - Don't Use File Download Worker for Two Way Sync #13993

Merged
merged 9 commits into from
Nov 11, 2024
Merged

BugFix - Don't Use File Download Worker for Two Way Sync #13993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
190d0a9
add mCancellationRequested call
alperozturk96 Nov 11, 2024
f15c017
dont use FileDownloadWorker in SynchronizeFolderOperation
alperozturk96 Nov 11, 2024
e48cb5a
dont use FileDownloadWorker in SynchronizeFileOperation
alperozturk96 Nov 11, 2024
db54742
fix usage of mTransferWasRequested
alperozturk96 Nov 11, 2024
dbcfcca
only use FileDownloadWorker via manual sync
alperozturk96 Nov 11, 2024
d8e951e
add missing constructor usages
alperozturk96 Nov 11, 2024
846b701
rename boolean flag
alperozturk96 Nov 11, 2024
c45d839
add curly bracket for condition checks
alperozturk96 Nov 11, 2024
b85affb
check mCancellationRequested in for loop
alperozturk96 Nov 11, 2024
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
13 changes: 9 additions & 4 deletions app/src/main/java/com/nextcloud/client/jobs/InternalTwoWaySyncWork.kt
View file
Open in desktop
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
Open in desktop
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
alperozturk96 marked this conversation as resolved.
Show resolved Hide resolved
)
synchronizeFileOperation.execute(context)
}
Expand Down
44 changes: 33 additions & 11 deletions app/src/main/java/com/owncloud/android/operations/SynchronizeFileOperation.java
View file
Open in desktop
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) {
alperozturk96 marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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) {
alperozturk96 marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Open in desktop
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
Loading