Skip to content

Commit

Permalink
Merge pull request #13160 from nextcloud/make-upload-restart-more-eff…
Browse files Browse the repository at this point in the history 
…icient

Remove unnecessary checks and calls from restartFailedUploads
  • Loading branch information
@alperozturk96
alperozturk96 authored Jul 4, 2024
2 parents f07ebd7 + 3657923 commit 6bed2ff
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 104 deletions.
2 changes: 1 addition & 1 deletion app/src/main/java/com/nextcloud/client/jobs/FilesSyncWork.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ class FilesSyncWork(
} else {
// Check every file in synced folder for changes and update
// filesystemDataProvider database (potentially needs a long time)
FilesSyncHelper.insertAllDBEntries(syncedFolder, powerManagementService)
FilesSyncHelper.insertAllDBEntriesForSyncedFolder(syncedFolder)
}
}

Expand Down
101 changes: 76 additions & 25 deletions app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadHelper.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ import android.content.Context
import android.content.Intent
import com.nextcloud.client.account.User
import com.nextcloud.client.account.UserAccountManager
import com.nextcloud.client.device.BatteryStatus
import com.nextcloud.client.device.PowerManagementService
import com.nextcloud.client.jobs.BackgroundJobManager
import com.nextcloud.client.jobs.upload.FileUploadWorker.Companion.currentUploadFileOperation
import com.nextcloud.client.network.Connectivity
import com.nextcloud.client.network.ConnectivityService
import com.owncloud.android.MainApp
import com.owncloud.android.datamodel.OCFile
Expand All @@ -28,9 +30,9 @@ import com.owncloud.android.lib.common.operations.RemoteOperationResult
import com.owncloud.android.lib.common.utils.Log_OC
import com.owncloud.android.lib.resources.files.ReadFileRemoteOperation
import com.owncloud.android.lib.resources.files.model.RemoteFile
import com.owncloud.android.operations.UploadFileOperation
import com.owncloud.android.utils.FileUtil
import java.io.File
import java.util.Optional
import javax.inject.Inject

@Suppress("TooManyFunctions")
Expand Down Expand Up @@ -117,34 +119,44 @@ class FileUploadHelper {
failedUploads: Array<OCUpload>
): Boolean {
var showNotExistMessage = false
val (gotNetwork, _, gotWifi) = connectivityService.connectivity
val isOnline = checkConnectivity(connectivityService)
val connectivity = connectivityService.connectivity
val batteryStatus = powerManagementService.battery
val charging = batteryStatus.isCharging || batteryStatus.isFull
val isPowerSaving = powerManagementService.isPowerSavingEnabled
var uploadUser = Optional.empty<User>()
val accountNames = accountManager.accounts.filter { account ->
accountManager.getUser(account.name).isPresent
}.map { account ->
account.name
}.toHashSet()

for (failedUpload in failedUploads) {
val isDeleted = !File(failedUpload.localPath).exists()
if (isDeleted) {
showNotExistMessage = true
if (!accountNames.contains(failedUpload.accountName)) {
uploadsStorageManager.removeUpload(failedUpload)
continue
}

val uploadResult =
checkUploadConditions(failedUpload, connectivity, batteryStatus, powerManagementService, isOnline)

// 2A. for deleted files, mark as permanently failed
if (failedUpload.lastResult != UploadResult.FILE_NOT_FOUND) {
failedUpload.lastResult = UploadResult.FILE_NOT_FOUND
if (uploadResult != UploadResult.UPLOADED) {
if (failedUpload.lastResult != uploadResult) {
failedUpload.lastResult = uploadResult
uploadsStorageManager.updateUpload(failedUpload)
}
} else if (!isPowerSaving && gotNetwork &&
canUploadBeRetried(failedUpload, gotWifi, charging) && !connectivityService.isInternetWalled
) {
// 2B. for existing local files, try restarting it if possible
failedUpload.uploadStatus = UploadStatus.UPLOAD_IN_PROGRESS
uploadsStorageManager.updateUpload(failedUpload)
if (uploadResult == UploadResult.FILE_NOT_FOUND) {
showNotExistMessage = true
}
continue
}

failedUpload.uploadStatus = UploadStatus.UPLOAD_IN_PROGRESS
uploadsStorageManager.updateUpload(failedUpload)
}

accountManager.accounts.forEach {
val user = accountManager.getUser(it.name)
if (user.isPresent) backgroundJobManager.startFilesUploadJob(user.get())
accountNames.forEach { accountName ->
val user = accountManager.getUser(accountName)
if (user.isPresent) {
backgroundJobManager.startFilesUploadJob(user.get())
}
}

return showNotExistMessage
Expand Down Expand Up @@ -216,11 +228,50 @@ class FileUploadHelper {
return upload.uploadStatus == UploadStatus.UPLOAD_IN_PROGRESS
}

private fun canUploadBeRetried(upload: OCUpload, gotWifi: Boolean, isCharging: Boolean): Boolean {
val file = File(upload.localPath)
val needsWifi = upload.isUseWifiOnly
val needsCharging = upload.isWhileChargingOnly
return file.exists() && (!needsWifi || gotWifi) && (!needsCharging || isCharging)
private fun checkConnectivity(connectivityService: ConnectivityService): Boolean {
// check that connection isn't walled off and that the server is reachable
return connectivityService.getConnectivity().isConnected && !connectivityService.isInternetWalled()
}

/**
* Dupe of [UploadFileOperation.checkConditions], needed to check if the upload should even be scheduled
* @return [UploadResult.UPLOADED] if the upload should be scheduled, otherwise the reason why it shouldn't
*/
private fun checkUploadConditions(
upload: OCUpload,
connectivity: Connectivity,
battery: BatteryStatus,
powerManagementService: PowerManagementService,
hasGeneralConnection: Boolean
): UploadResult {
var conditions = UploadResult.UPLOADED

// check that internet is available
if (!hasGeneralConnection) {
conditions = UploadResult.NETWORK_CONNECTION
}

// check that local file exists; skip the upload otherwise
if (!File(upload.localPath).exists()) {
conditions = UploadResult.FILE_NOT_FOUND
}

// check that connectivity conditions are met; delay upload otherwise
if (upload.isUseWifiOnly && (!connectivity.isWifi || connectivity.isMetered)) {
conditions = UploadResult.DELAYED_FOR_WIFI
}

// check if charging conditions are met; delay upload otherwise
if (upload.isWhileChargingOnly && !battery.isCharging && !battery.isFull) {
conditions = UploadResult.DELAYED_FOR_CHARGING
}

// check that device is not in power save mode; delay upload otherwise
if (powerManagementService.isPowerSavingEnabled) {
conditions = UploadResult.DELAYED_IN_POWER_SAVE_MODE
}

return conditions
}

@Suppress("ReturnCount")
Expand Down
4 changes: 2 additions & 2 deletions app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadWorker.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ class FileUploadWorker(
@Suppress("ReturnCount")
private fun retrievePagesBySortingUploadsByID(): Result {
val accountName = inputData.getString(ACCOUNT) ?: return Result.failure()
var uploadsPerPage = uploadsStorageManager.getCurrentAndPendingUploadsForAccountPageAscById(-1, accountName)
var uploadsPerPage = uploadsStorageManager.getCurrentUploadsForAccountPageAscById(-1, accountName)
val totalUploadSize = uploadsStorageManager.getTotalUploadSize(accountName)

Log_OC.d(TAG, "Total upload size: $totalUploadSize")
Expand All @@ -148,7 +148,7 @@ class FileUploadWorker(
val lastId = uploadsPerPage.last().uploadId
uploadFiles(totalUploadSize, uploadsPerPage, accountName)
uploadsPerPage =
uploadsStorageManager.getCurrentAndPendingUploadsForAccountPageAscById(lastId, accountName)
uploadsStorageManager.getCurrentUploadsForAccountPageAscById(lastId, accountName)
}

if (isStopped) {
Expand Down
21 changes: 17 additions & 4 deletions app/src/main/java/com/owncloud/android/datamodel/UploadsStorageManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ private List<OCUpload> getUploadPage(final long afterId, @Nullable String select
return getUploadPage(QUERY_PAGE_SIZE, afterId, true, selection, selectionArgs);
}

private String getInProgressUploadsSelection() {
private String getInProgressAndDelayedUploadsSelection() {
return "( " + ProviderTableMeta.UPLOADS_STATUS + EQUAL + UploadStatus.UPLOAD_IN_PROGRESS.value +
OR + ProviderTableMeta.UPLOADS_LAST_RESULT +
EQUAL + UploadResult.DELAYED_FOR_WIFI.getValue() +
Expand All @@ -485,7 +485,8 @@ private String getInProgressUploadsSelection() {
}

public int getTotalUploadSize(@Nullable String... selectionArgs) {
final String selection = getInProgressUploadsSelection();
final String selection = ProviderTableMeta.UPLOADS_STATUS + EQUAL + UploadStatus.UPLOAD_IN_PROGRESS.value +
AND + ProviderTableMeta.UPLOADS_ACCOUNT_NAME + IS_EQUAL;
int totalSize = 0;

Cursor cursor = getDB().query(
Expand Down Expand Up @@ -605,17 +606,29 @@ public OCUpload[] getCurrentAndPendingUploadsForCurrentAccount() {
}

public OCUpload[] getCurrentAndPendingUploadsForAccount(final @NonNull String accountName) {
String inProgressUploadsSelection = getInProgressUploadsSelection();
String inProgressUploadsSelection = getInProgressAndDelayedUploadsSelection();
return getUploads(inProgressUploadsSelection, accountName);
}

public OCUpload[] getCurrentUploadsForAccount(final @NonNull String accountName) {
return getUploads(ProviderTableMeta.UPLOADS_STATUS + EQUAL + UploadStatus.UPLOAD_IN_PROGRESS.value + AND +
ProviderTableMeta.UPLOADS_ACCOUNT_NAME + IS_EQUAL, accountName);
}

public List<OCUpload> getCurrentUploadsForAccountPageAscById(final long afterId, final @NonNull String accountName) {
final String selection = ProviderTableMeta.UPLOADS_STATUS + EQUAL + UploadStatus.UPLOAD_IN_PROGRESS.value +
AND + ProviderTableMeta.UPLOADS_ACCOUNT_NAME + IS_EQUAL;
return getUploadPage(QUERY_PAGE_SIZE, afterId, false, selection, accountName);
}


/**
* Gets a page of uploads after <code>afterId</code>, where uploads are sorted by ascending upload id.
* <p>
* If <code>afterId</code> is -1, returns the first page
*/
public List<OCUpload> getCurrentAndPendingUploadsForAccountPageAscById(final long afterId, final @NonNull String accountName) {
final String selection = getInProgressUploadsSelection();
final String selection = getInProgressAndDelayedUploadsSelection();
return getUploadPage(QUERY_PAGE_SIZE, afterId, false, selection, accountName);
}

Expand Down
79 changes: 7 additions & 72 deletions app/src/main/java/com/owncloud/android/utils/FilesSyncHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public FileVisitResult visitFileFailed(Path file, IOException exc) {
}
}

private static void insertAllDBEntriesForSyncedFolder(SyncedFolder syncedFolder) {
public static void insertAllDBEntriesForSyncedFolder(SyncedFolder syncedFolder) {
final Context context = MainApp.getAppContext();
final ContentResolver contentResolver = context.getContentResolver();

Expand Down Expand Up @@ -146,18 +146,6 @@ private static void insertAllDBEntriesForSyncedFolder(SyncedFolder syncedFolder)
}
}

public static void insertAllDBEntries(SyncedFolder syncedFolder,
PowerManagementService powerManagementService) {
if (syncedFolder.isEnabled() &&
!(syncedFolder.isChargingOnly() &&
!powerManagementService.getBattery().isCharging() &&
!powerManagementService.getBattery().isFull()
)
) {
insertAllDBEntriesForSyncedFolder(syncedFolder);
}
}

public static void insertChangedEntries(SyncedFolder syncedFolder,
String[] changedFiles) {
final ContentResolver contentResolver = MainApp.getAppContext().getContentResolver();
Expand Down Expand Up @@ -248,66 +236,13 @@ public static void restartUploadsIfNeeded(final UploadsStorageManager uploadsSto
final UserAccountManager accountManager,
final ConnectivityService connectivityService,
final PowerManagementService powerManagementService) {
boolean accountExists;

boolean whileChargingOnly = true;
boolean useWifiOnly = true;

// Make all in progress downloads failed to restart upload worker
uploadsStorageManager.failInProgressUploads(UploadResult.SERVICE_INTERRUPTED);

OCUpload[] failedUploads = uploadsStorageManager.getFailedUploads();

for (OCUpload failedUpload : failedUploads) {
accountExists = false;
if (!failedUpload.isWhileChargingOnly()) {
whileChargingOnly = false;
}
if (!failedUpload.isUseWifiOnly()) {
useWifiOnly = false;
}

// check if accounts still exists
for (Account account : accountManager.getAccounts()) {
if (account.name.equals(failedUpload.getAccountName())) {
accountExists = true;
break;
}
}

if (!accountExists) {
uploadsStorageManager.removeUpload(failedUpload);
}
}

failedUploads = uploadsStorageManager.getFailedUploads();
if (failedUploads.length == 0) {
//nothing to do
return;
}

if (whileChargingOnly) {
final BatteryStatus batteryStatus = powerManagementService.getBattery();
final boolean charging = batteryStatus.isCharging() || batteryStatus.isFull();
if (!charging) {
//all uploads requires charging
return;
}
}

if (useWifiOnly && !connectivityService.getConnectivity().isWifi()) {
//all uploads requires wifi
return;
}


new Thread(() -> {
if (connectivityService.getConnectivity().isConnected()) {
FileUploadHelper.Companion.instance().retryFailedUploads(
uploadsStorageManager,
connectivityService,
accountManager,
powerManagementService);
}
FileUploadHelper.Companion.instance().retryFailedUploads(
uploadsStorageManager,
connectivityService,
accountManager,
powerManagementService);
}).start();
}

Expand Down

0 comments on commit 6bed2ff

Please sign in to comment.