From 6813476c86d87ee83be58cf63e74c10ebc76b1e7 Mon Sep 17 00:00:00 2001 From: Ashish Date: Mon, 22 Mar 2021 00:33:13 +0530 Subject: [PATCH] Updated JavaDocs in UploadWorker, Fixed Test cases --- .../nrw/commons/upload/worker/UploadWorker.kt | 79 +++++++++++++++++-- .../pictures/BookmarkPictureDaoTest.kt | 2 +- .../recentsearches/RecentSearchesDaoTest.kt | 2 +- .../commons/upload/UploadControllerTest.kt | 12 +-- 4 files changed, 80 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/upload/worker/UploadWorker.kt b/app/src/main/java/fr/free/nrw/commons/upload/worker/UploadWorker.kt index 3fc78a71c..af3cf18c1 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/worker/UploadWorker.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/worker/UploadWorker.kt @@ -21,7 +21,6 @@ import fr.free.nrw.commons.media.MediaClient import fr.free.nrw.commons.upload.StashUploadState import fr.free.nrw.commons.upload.UploadClient import fr.free.nrw.commons.upload.UploadResult -import fr.free.nrw.commons.utils.ViewUtil import fr.free.nrw.commons.wikidata.WikidataEditService import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.asFlow @@ -29,8 +28,8 @@ import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.map import kotlinx.coroutines.withContext import timber.log.Timber -import java.lang.Exception import java.util.* +import java.util.regex.Pattern import javax.inject.Inject import kotlin.collections.ArrayList @@ -184,23 +183,34 @@ class UploadWorker(var appContext: Context, workerParams: WorkerParameters) : } }.collect() + //Dismiss the global notification notificationManager?.cancel( PROCESSING_UPLOADS_NOTIFICATION_TAG, PROCESSING_UPLOADS_NOTIFICATION_ID ) + //No need to keep looking if the limited connection mode is on, + //If the user toggles it, the work manager will be started again if(isLimitedConnectionModeEnabled()){ break; } } } + //TODO make this smart, think of handling retries in the future return Result.success() } + /** + * Returns true is the limited connection mode is enabled + */ private fun isLimitedConnectionModeEnabled(): Boolean { return sessionManager.getPreference(CommonsApplication.IS_LIMITED_CONNECTION_MODE_ENABLED) } + /** + * Upload the contribution + * @param contribution + */ @SuppressLint("StringFormatInvalid") private suspend fun uploadContribution(contribution: Contribution) { if (contribution.localUri == null || contribution.localUri.path == null) { @@ -211,7 +221,8 @@ class UploadWorker(var appContext: Context, workerParams: WorkerParameters) : val displayTitle = contribution.media.displayTitle currentNotificationTag = contribution.localUri.toString() - currentNotificationID = contribution.hashCode() + currentNotificationID = + (contribution.localUri.toString() + contribution.media.filename).hashCode() curentNotification getNotificationBuilder(CommonsApplication.NOTIFICATION_CHANNEL_ID_ALL)!! @@ -239,18 +250,21 @@ class UploadWorker(var appContext: Context, workerParams: WorkerParameters) : ) try { + //Upload the file to stash val stashUploadResult = uploadClient.uploadFileToStash( appContext, filename, contribution, notificationProgressUpdater ).blockingSingle() when (stashUploadResult.state) { StashUploadState.SUCCESS -> { + //If the stash upload succeeds Timber.d("Upload to stash success for fileName: $filename") Timber.d("Ensure uniqueness of filename"); val uniqueFileName = findUniqueFileName(filename!!) try { + //Upload the file from stash val uploadResult = uploadClient.uploadFileFromStash( contribution, uniqueFileName, stashUploadResult.fileKey ).blockingSingle() @@ -306,6 +320,9 @@ class UploadWorker(var appContext: Context, workerParams: WorkerParameters) : } } + /** + * Make the WikiData Edit, if applicable + */ private suspend fun makeWikiDataEdit(uploadResult: UploadResult, contribution: Contribution) { wikidataEditService.addDepictionsAndCaptions(uploadResult, contribution) val wikiDataPlace = contribution.wikidataPlace @@ -351,10 +368,54 @@ class UploadWorker(var appContext: Context, workerParams: WorkerParameters) : .blockingAwait() } - private fun findUniqueFileName(filename: String): String { - return filename; + private fun findUniqueFileName(fileName: String): String { + var sequenceFileName: String? + var sequenceNumber = 1 + while (true) { + sequenceFileName = if (sequenceNumber == 1) { + fileName + } else { + if (fileName.indexOf('.') == -1) { + "$fileName $sequenceNumber" + } else { + val regex = + Pattern.compile("^(.*)(\\..+?)$") + val regexMatcher = regex.matcher(fileName) + regexMatcher.replaceAll("$1 $sequenceNumber$2") + } + } + if (!mediaClient.checkPageExistsUsingTitle( + String.format( + "File:%s", + sequenceFileName + ) + ) + .blockingGet() + && !isThereAnUnfinishedUploadWithFileName(sequenceFileName!!) + ) { + break + } + sequenceNumber++ + } + return sequenceFileName!! } + /** + * To remove possibilities of overriding already uploaded files partially/fully, lets + * make sure we don't already have a file with the same same + */ + private fun isThereAnUnfinishedUploadWithFileName(fileName: String): Boolean { + val contributionWithTitle = contributionDao.getContributionWithTitle(fileName) + if (contributionWithTitle.isEmpty()) { + return true + } + return false; + } + + /** + * Notify that the current upload has succeeded + * @param contribution + */ @SuppressLint("StringFormatInvalid") private fun showSuccessNotification(contribution: Contribution) { val displayTitle = contribution.media.displayTitle @@ -375,6 +436,10 @@ class UploadWorker(var appContext: Context, workerParams: WorkerParameters) : contributionDao.save(contribution).blockingAwait() } + /** + * Notify that the current upload has failed + * @param contribution + */ @SuppressLint("StringFormatInvalid") private fun showFailedNotification(contribution: Contribution) { val displayTitle = contribution.media.displayTitle @@ -393,6 +458,10 @@ class UploadWorker(var appContext: Context, workerParams: WorkerParameters) : ) } + /** + * Notify that the current upload is paused + * @param contribution + */ private fun showPausedNotification(contribution: Contribution) { val displayTitle = contribution.media.displayTitle curentNotification.setContentTitle( diff --git a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/pictures/BookmarkPictureDaoTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/pictures/BookmarkPictureDaoTest.kt index 4894aebba..010a00a43 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/pictures/BookmarkPictureDaoTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/bookmarks/pictures/BookmarkPictureDaoTest.kt @@ -124,7 +124,7 @@ class BookmarkPictureDaoTest { whenever(client.query(any(), any(), any(), any(), anyOrNull())).thenReturn(createCursor(1)) assertFalse(testObject.updateBookmark(exampleBookmark)) - verify(client).delete(eq(exampleBookmark.contentUri), isNull(), isNull()) + verify(client).delete(eq(exampleBookmark.contentUri!!), isNull(), isNull()) } @Test diff --git a/app/src/test/kotlin/fr/free/nrw/commons/explore/recentsearches/RecentSearchesDaoTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/explore/recentsearches/RecentSearchesDaoTest.kt index 6752ca7fe..71d464c9f 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/explore/recentsearches/RecentSearchesDaoTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/explore/recentsearches/RecentSearchesDaoTest.kt @@ -152,7 +152,7 @@ class RecentSearchesDaoTest { testObject.save(recentSearch) - verify(client).update(eq(recentSearch.contentUri), captor.capture(), isNull(), isNull()) + verify(client).update(eq(recentSearch.contentUri!!), captor.capture(), isNull(), isNull()) captor.firstValue.let { cv -> assertEquals(2, cv.size()) assertEquals(recentSearch.query, cv.getAsString(COLUMN_NAME)) diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadControllerTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadControllerTest.kt index b05e0f280..ecae412ae 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadControllerTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadControllerTest.kt @@ -1,29 +1,24 @@ package fr.free.nrw.commons.upload -import android.content.ComponentName +import android.content.ContentResolver import android.content.Context import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.whenever import fr.free.nrw.commons.Media -import fr.free.nrw.commons.auth.SessionManager import fr.free.nrw.commons.contributions.Contribution -import fr.free.nrw.commons.kvstore.JsonKvStore import org.junit.Before import org.junit.Test import org.mockito.InjectMocks import org.mockito.Mock -import org.mockito.Mockito.`when` import org.mockito.Mockito.mock import org.mockito.MockitoAnnotations class UploadControllerTest { - - @Mock - internal var sessionManager: SessionManager? = null @Mock internal var context: Context? = null + @Mock - internal var prefs: JsonKvStore? = null + internal lateinit var contentResolver: ContentResolver @InjectMocks var uploadController: UploadController? = null @@ -47,6 +42,7 @@ class UploadControllerTest { val media = mock() whenever(contribution.media).thenReturn(media) whenever(media.author).thenReturn("Creator") + whenever(context?.contentResolver).thenReturn(contentResolver) uploadController!!.prepareMedia(contribution) } }