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.

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

[New arch] Fixes for QA reports in uploads rearchitecture #3735

Merged
merged 5 commits into from
Aug 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,8 @@ import com.owncloud.android.domain.sharing.shares.usecases.EditPublicShareAsyncU
import com.owncloud.android.domain.sharing.shares.usecases.GetShareAsLiveDataUseCase
import com.owncloud.android.domain.sharing.shares.usecases.GetSharesAsLiveDataUseCase
import com.owncloud.android.domain.sharing.shares.usecases.RefreshSharesFromServerAsyncUseCase
import com.owncloud.android.domain.transfers.usecases.ClearFailedTransfersUseCase
import com.owncloud.android.usecases.transfers.uploads.ClearFailedTransfersUseCase
import com.owncloud.android.domain.transfers.usecases.ClearSuccessfulTransfersUseCase
import com.owncloud.android.domain.transfers.usecases.DeleteTransferWithIdUseCase
import com.owncloud.android.domain.transfers.usecases.GetAllTransfersAsLiveDataUseCase
import com.owncloud.android.domain.transfers.usecases.UpdatePendingUploadsPathUseCase
import com.owncloud.android.domain.user.usecases.GetStoredQuotaUseCase
Expand Down Expand Up @@ -168,8 +167,7 @@ val useCaseModule = module {
factory { RetryUploadFromContentUriUseCase(get(), get(), get()) }
factory { GetAllTransfersAsLiveDataUseCase(get()) }
factory { CancelUploadWithIdUseCase(get(), get()) }
factory { DeleteTransferWithIdUseCase(get()) }
factory { ClearFailedTransfersUseCase(get()) }
factory { ClearFailedTransfersUseCase(get(), get()) }
factory { RetryFailedUploadsUseCase(get(), get(), get(), get()) }
factory { ClearSuccessfulTransfersUseCase(get()) }
factory { CancelUploadsFromAccountUseCase(get(), get()) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,5 +89,5 @@ val viewModelModule = module {
viewModel { FileDetailsViewModel(get(), get(), get(), get(), get()) }
viewModel { FileOperationsViewModel(get(), get(), get(), get(), get(), get(), get(), get(), get(), get()) }
viewModel { (accountName: String, initialFolderToDisplay: OCFile) -> MainFileListViewModel(get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), accountName, initialFolderToDisplay) }
viewModel { TransfersViewModel(get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get()) }
viewModel { TransfersViewModel(get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get()) }
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ import java.io.File

class TransfersAdapter(
val cancel: (Long) -> Unit,
val delete: (Long) -> Unit,
val retry: (OCTransfer) -> Unit,
val clearFailed: () -> Unit,
val retryFailed: () -> Unit,
Expand Down Expand Up @@ -141,7 +140,7 @@ class TransfersAdapter(
uploadRightButton.apply {
setImageResource(R.drawable.ic_action_delete_grey)
setOnClickListener {
delete(transferItem.transfer.id!!)
cancel(transferItem.transfer.id!!)
}
}
holder.itemView.setOnClickListener {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,6 @@ class TransferListFragment : Fragment() {
cancel = { id ->
transfersViewModel.cancelTransferWithId(id)
},
delete = { id ->
transfersViewModel.deleteTransferWithId(id)
},
retry = { transfer: OCTransfer ->
if (transfer.lastResult == TransferResult.CREDENTIAL_ERROR) {
val parentActivity = requireActivity() as FileActivity
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,8 @@ import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import androidx.work.WorkInfo
import com.owncloud.android.domain.transfers.model.OCTransfer
import com.owncloud.android.domain.transfers.usecases.ClearFailedTransfersUseCase
import com.owncloud.android.usecases.transfers.uploads.ClearFailedTransfersUseCase
import com.owncloud.android.domain.transfers.usecases.ClearSuccessfulTransfersUseCase
import com.owncloud.android.domain.transfers.usecases.DeleteTransferWithIdUseCase
import com.owncloud.android.domain.transfers.usecases.GetAllTransfersAsLiveDataUseCase
import com.owncloud.android.providers.CoroutinesDispatcherProvider
import com.owncloud.android.providers.WorkManagerProvider
Expand All @@ -48,7 +47,6 @@ class TransfersViewModel(
private val uploadFilesFromContentUriUseCase: UploadFilesFromContentUriUseCase,
private val uploadFilesFromSystemUseCase: UploadFilesFromSystemUseCase,
private val cancelUploadWithIdUseCase: CancelUploadWithIdUseCase,
private val deleteTransferWithIdUseCase: DeleteTransferWithIdUseCase,
private val retryUploadFromSystemUseCase: RetryUploadFromSystemUseCase,
private val retryUploadFromContentUriUseCase: RetryUploadFromContentUriUseCase,
private val clearFailedTransfersUseCase: ClearFailedTransfersUseCase,
Expand Down Expand Up @@ -130,14 +128,6 @@ class TransfersViewModel(
}
}

fun deleteTransferWithId(id: Long) {
viewModelScope.launch(coroutinesDispatcherProvider.io) {
deleteTransferWithIdUseCase.execute(
DeleteTransferWithIdUseCase.Params(id = id)
)
}
}

fun retryUploadFromSystem(id: Long) {
viewModelScope.launch(coroutinesDispatcherProvider.io) {
retryUploadFromSystemUseCase.execute(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ import com.owncloud.android.usecases.transfers.downloads.DownloadFileUseCase
import com.owncloud.android.usecases.transfers.uploads.UploadFilesFromSystemUseCase
import com.owncloud.android.utils.FileStorageUtils
import com.owncloud.android.utils.NotificationUtils
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import org.koin.android.ext.android.inject
import timber.log.Timber
import java.io.File
Expand Down Expand Up @@ -136,7 +139,9 @@ class DocumentsStorageProvider : DocumentsProvider() {
listOfLocalPaths = listOf(fileToOpen.path),
uploadFolderPath = ocFile.remotePath.substringBeforeLast(PATH_SEPARATOR)
)
uploadFilesUseCase.execute(uploadFilesUseCaseParams)
CoroutineScope(Dispatchers.IO).launch {
uploadFilesUseCase.execute(uploadFilesUseCaseParams)
}
} else {
Thread {
val synchronizeFileUseCase: SynchronizeFileUseCase by inject()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
import com.owncloud.android.presentation.ui.files.createfolder.CreateFolderDialogFragment;
import com.owncloud.android.presentation.ui.files.operations.FileOperation;
import com.owncloud.android.presentation.ui.files.operations.FileOperationsViewModel;
import com.owncloud.android.presentation.viewmodels.transfers.TransfersViewModel;
import com.owncloud.android.syncadapter.FileSyncAdapter;
import com.owncloud.android.ui.adapter.ReceiveExternalFilesAdapter;
import com.owncloud.android.ui.asynctasks.CopyAndUploadContentUrisTask;
Expand All @@ -97,6 +98,7 @@
import com.owncloud.android.utils.DisplayUtils;
import com.owncloud.android.utils.FileStorageUtils;
import com.owncloud.android.utils.SortFilesUtils;
import kotlin.Lazy;
import kotlin.Unit;
import org.jetbrains.annotations.NotNull;
import timber.log.Timber;
Expand All @@ -111,6 +113,7 @@
import java.util.Vector;

import static org.koin.java.KoinJavaComponent.get;
import static org.koin.java.KoinJavaComponent.inject;

/**
* This can be used to upload things to an ownCloud instance.
Expand Down Expand Up @@ -950,10 +953,13 @@ public void afterTextChanged(Editable editable) {
error = getString(R.string.uploader_upload_text_dialog_filename_error_empty);
} else {
fileName += ".txt";
Uri fileUri = savePlainTextToFile(fileName);
mStreamsToUpload.clear();
mStreamsToUpload.add(fileUri);
uploadFiles();
String filePath = savePlainTextToFile(fileName);
ArrayList<String> fileToUpload = new ArrayList<>();
fileToUpload.add(filePath);
@NotNull Lazy<TransfersViewModel> transfersViewModelLazy = inject(TransfersViewModel.class);
TransfersViewModel transfersViewModel = transfersViewModelLazy.getValue();
transfersViewModel.uploadFilesFromSystem(getAccount().name, fileToUpload, mUploadPath);
finish();
}
inputLayout.setErrorEnabled(error != null);
inputLayout.setError(error);
Expand All @@ -966,22 +972,21 @@ public void afterTextChanged(Editable editable) {
* Store plain text from intent to a new file in cache dir.
*
* @param fileName The name of the file.
* @return Uri from created file.
* @return Path of the created file.
*/
private Uri savePlainTextToFile(String fileName) {
Uri uri = null;
private String savePlainTextToFile(String fileName) {
File tmpFile = null;
String content = getIntent().getStringExtra(Intent.EXTRA_TEXT);
try {
File tmpFile = new File(getCacheDir(), fileName);
tmpFile = new File(getCacheDir(), fileName);
FileOutputStream outputStream = new FileOutputStream(tmpFile);
outputStream.write(content.getBytes());
outputStream.close();
uri = Uri.fromFile(tmpFile);

} catch (IOException e) {
Timber.w(e, "Failed to create temp file for uploading plain text: %s", e.getMessage());
}
return uri;
return tmpFile.getAbsolutePath();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,21 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

package com.owncloud.android.domain.transfers.usecases
package com.owncloud.android.usecases.transfers.uploads

import androidx.work.WorkManager
import com.owncloud.android.domain.BaseUseCase
import com.owncloud.android.domain.transfers.TransferRepository

class ClearFailedTransfersUseCase(
private val workManager: WorkManager,
private val transferRepository: TransferRepository,
) : BaseUseCase<Unit, Unit>() {
override fun run(params: Unit): Unit =
override fun run(params: Unit) {
val failedTransfers = transferRepository.getFailedTransfers()
failedTransfers.forEach { failedTransfer ->
workManager.cancelAllWorkByTag(failedTransfer.id.toString())
}
transferRepository.clearFailedTransfers()

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import com.owncloud.android.domain.BaseUseCase
import com.owncloud.android.domain.transfers.TransferRepository
import com.owncloud.android.extensions.getWorkInfoByTags
import com.owncloud.android.workers.UploadFileFromContentUriWorker
import timber.log.Timber

class RetryUploadFromContentUriUseCase(
private val workManager: WorkManager,
Expand All @@ -40,17 +41,17 @@ class RetryUploadFromContentUriUseCase(

uploadToRetry ?: return

transferRepository.updateTransferStatusToEnqueuedById(params.uploadIdInStorageManager)

val workInfo = workManager.getWorkInfoByTags(
val workInfos = workManager.getWorkInfoByTags(
listOf(
params.uploadIdInStorageManager.toString(),
uploadToRetry.accountName,
UploadFileFromContentUriWorker::class.java.name
)
)

if (workInfo.firstOrNull()?.state == WorkInfo.State.FAILED) {
if (workInfos.isEmpty() || workInfos.firstOrNull()?.state == WorkInfo.State.FAILED) {
transferRepository.updateTransferStatusToEnqueuedById(params.uploadIdInStorageManager)

uploadFileFromContentUriUseCase.execute(
UploadFileFromContentUriUseCase.Params(
accountName = uploadToRetry.accountName,
Expand All @@ -63,6 +64,8 @@ class RetryUploadFromContentUriUseCase(
chargingOnly = false
)
)
} else {
Timber.w("Upload $uploadToRetry is already in state ${workInfos.firstOrNull()?.state}. Won't be retried")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import com.owncloud.android.domain.BaseUseCase
import com.owncloud.android.domain.transfers.TransferRepository
import com.owncloud.android.extensions.getWorkInfoByTags
import com.owncloud.android.workers.UploadFileFromFileSystemWorker
import timber.log.Timber

class RetryUploadFromSystemUseCase(
private val workManager: WorkManager,
Expand All @@ -39,17 +40,17 @@ class RetryUploadFromSystemUseCase(

uploadToRetry ?: return

transferRepository.updateTransferStatusToEnqueuedById(params.uploadIdInStorageManager)

val workInfo = workManager.getWorkInfoByTags(
val workInfos = workManager.getWorkInfoByTags(
listOf(
params.uploadIdInStorageManager.toString(),
uploadToRetry.accountName,
UploadFileFromFileSystemWorker::class.java.name
)
)

if (workInfo.firstOrNull()?.state == WorkInfo.State.FAILED) {
if (workInfos.isEmpty() || workInfos.firstOrNull()?.state == WorkInfo.State.FAILED) {
transferRepository.updateTransferStatusToEnqueuedById(params.uploadIdInStorageManager)

uploadFileFromSystemUseCase.execute(
UploadFileFromSystemUseCase.Params(
accountName = uploadToRetry.accountName,
Expand All @@ -60,6 +61,8 @@ class RetryUploadFromSystemUseCase(
uploadIdInStorageManager = params.uploadIdInStorageManager
)
)
} else {
Timber.w("Upload $uploadToRetry is already in state ${workInfos.firstOrNull()?.state}. Won't be retried")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import com.owncloud.android.domain.exceptions.NoConnectionWithServerException
import com.owncloud.android.domain.exceptions.QuotaExceededException
import com.owncloud.android.domain.exceptions.SSLRecoverablePeerUnverifiedException
import com.owncloud.android.domain.exceptions.ServiceUnavailableException
import com.owncloud.android.domain.exceptions.SpecificServiceUnavailableException
import com.owncloud.android.domain.exceptions.SpecificUnsupportedMediaTypeException
import com.owncloud.android.domain.exceptions.UnauthorizedException

Expand Down Expand Up @@ -85,6 +86,7 @@ enum class TransferResult constructor(val value: Int) {
is ConflictException -> CONFLICT_ERROR
is ForbiddenException -> PRIVILEGES_ERROR
is ServiceUnavailableException -> SERVICE_UNAVAILABLE
is SpecificServiceUnavailableException -> SPECIFIC_SERVICE_UNAVAILABLE
is QuotaExceededException -> QUOTA_EXCEEDED
is SpecificUnsupportedMediaTypeException -> SPECIFIC_UNSUPPORTED_MEDIA_TYPE
is SSLRecoverablePeerUnverifiedException -> SSL_RECOVERABLE_PEER_UNVERIFIED
Expand Down

This file was deleted.