diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsPresenter.java b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsPresenter.java index 495a4bc647..4d05711f38 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsPresenter.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsPresenter.java @@ -62,7 +62,10 @@ public Contribution getContributionsWithTitle(String title) { */ public void checkDuplicateImageAndRestartContribution(Contribution contribution) { compositeDisposable.add(uploadRepository - .checkDuplicateImage(contribution.getLocalUriPath().getPath()) + .checkDuplicateImage( + contribution.getContentUri(), + contribution.getLocalUri() + ) .subscribeOn(ioThreadScheduler) .subscribe(imageCheckResult -> { if (imageCheckResult == IMAGE_OK) { diff --git a/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.kt b/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.kt index 9acdde32c5..a27993f9e9 100644 --- a/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.kt +++ b/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.kt @@ -1,5 +1,6 @@ package fr.free.nrw.commons.repository +import android.net.Uri import fr.free.nrw.commons.Media import fr.free.nrw.commons.category.CategoriesModel import fr.free.nrw.commons.category.CategoryItem @@ -203,8 +204,8 @@ class UploadRepository @Inject constructor( * @param filePath file to be checked * @return IMAGE_DUPLICATE or IMAGE_OK */ - fun checkDuplicateImage(filePath: String): Single { - return uploadModel.checkDuplicateImage(filePath) + fun checkDuplicateImage(originalFilePath: Uri, modifiedFilePath: Uri): Single { + return uploadModel.checkDuplicateImage(originalFilePath, modifiedFilePath) } /** diff --git a/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.kt b/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.kt index fca10be1e4..3acd13c656 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.kt @@ -1,5 +1,7 @@ package fr.free.nrw.commons.upload +import android.content.Context +import android.net.Uri import fr.free.nrw.commons.location.LatLng import fr.free.nrw.commons.media.MediaClient import fr.free.nrw.commons.nearby.Place @@ -13,7 +15,7 @@ import io.reactivex.Single import io.reactivex.schedulers.Schedulers import org.apache.commons.lang3.StringUtils import timber.log.Timber -import java.io.FileInputStream +import java.io.InputStream import javax.inject.Inject import javax.inject.Singleton @@ -26,7 +28,8 @@ class ImageProcessingService @Inject constructor( private val imageUtilsWrapper: ImageUtilsWrapper, private val readFBMD: ReadFBMD, private val exifReader: EXIFReader, - private val mediaClient: MediaClient + private val mediaClient: MediaClient, + private val appContext: Context ) { /** * Check image quality before upload - checks duplicate image - checks dark image - checks @@ -47,7 +50,10 @@ class ImageProcessingService @Inject constructor( val filePath = uploadItem.mediaUri?.path return Single.zip( - checkDuplicateImage(filePath), + checkIfFileAlreadyExists( + originalFilePath = uploadItem.contentUri!!, + modifiedFilePath = uploadItem.mediaUri!! + ), checkImageGeoLocation(uploadItem.place, filePath, inAppPictureLocation), checkDarkImage(filePath!!), checkFBMD(filePath), @@ -114,18 +120,31 @@ class ImageProcessingService @Inject constructor( .subscribeOn(Schedulers.io()) } + /** + * Checks if file already exists using original & modified file's SHA + * + * @param originalFilePath original file to be checked + * @param modifiedFilePath modified (after exif modifications) file to be checked + * @return IMAGE_DUPLICATE or IMAGE_OK + */ + fun checkIfFileAlreadyExists(originalFilePath: Uri, modifiedFilePath: Uri): Single { + return Single.zip( + checkDuplicateImage(inputStream = appContext.contentResolver.openInputStream(originalFilePath)!!), + checkDuplicateImage(inputStream = fileUtilsWrapper.getFileInputStream(modifiedFilePath.path)) + ) { resultForOriginal, resultForDuplicate -> + return@zip if (resultForOriginal == IMAGE_DUPLICATE || resultForDuplicate == IMAGE_DUPLICATE) + IMAGE_DUPLICATE else IMAGE_OK + } + } + /** * Checks for duplicate image * * @param filePath file to be checked * @return IMAGE_DUPLICATE or IMAGE_OK */ - fun checkDuplicateImage(filePath: String?): Single { - Timber.d("Checking for duplicate image %s", filePath) - return Single.fromCallable { fileUtilsWrapper.getFileInputStream(filePath) } - .map { stream: FileInputStream? -> - fileUtilsWrapper.getSHA1(stream) - } + private fun checkDuplicateImage(inputStream: InputStream): Single { + return Single.fromCallable { fileUtilsWrapper.getSHA1(inputStream) } .flatMap { fileSha: String? -> mediaClient.checkFileExistsUsingSha(fileSha) } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsPresenter.kt b/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsPresenter.kt index 324f988d46..0b2c47ddc8 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsPresenter.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsPresenter.kt @@ -149,7 +149,10 @@ class PendingUploadsPresenter @Inject internal constructor( } compositeDisposable.add( uploadRepository - .checkDuplicateImage(contribution.localUriPath!!.path) + .checkDuplicateImage( + originalFilePath = contribution.contentUri!!, + modifiedFilePath = contribution.localUri!! + ) .subscribeOn(ioThreadScheduler) .subscribe({ imageCheckResult: Int -> if (imageCheckResult == IMAGE_OK) { @@ -218,7 +221,10 @@ class PendingUploadsPresenter @Inject internal constructor( } compositeDisposable.add( uploadRepository - .checkDuplicateImage(contribution.localUriPath!!.path) + .checkDuplicateImage( + originalFilePath = contribution.contentUri!!, + modifiedFilePath = contribution.localUri!! + ) .subscribeOn(ioThreadScheduler) .subscribe { imageCheckResult: Int -> if (imageCheckResult == IMAGE_OK) { diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.kt b/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.kt index c7ffe06f70..2cda7a8903 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.kt @@ -103,8 +103,11 @@ class UploadModel @Inject internal constructor( * @param filePath file to be checked * @return IMAGE_DUPLICATE or IMAGE_OK */ - fun checkDuplicateImage(filePath: String?): Single = - imageProcessingService.checkDuplicateImage(filePath) + fun checkDuplicateImage(originalFilePath: Uri?, modifiedFilePath: Uri?): Single = + imageProcessingService.checkIfFileAlreadyExists( + originalFilePath = originalFilePath!!, + modifiedFilePath = modifiedFilePath!! + ) /** * Calls validateCaption() of ImageProcessingService to check caption of image diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/ImageProcessingServiceTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/ImageProcessingServiceTest.kt index 90d2f20e65..9a5e20b255 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/ImageProcessingServiceTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/upload/ImageProcessingServiceTest.kt @@ -1,5 +1,7 @@ package fr.free.nrw.commons.upload +import android.content.ContentResolver +import android.content.Context import android.net.Uri import fr.free.nrw.commons.location.LatLng import fr.free.nrw.commons.media.MediaClient @@ -18,6 +20,7 @@ import org.mockito.Mockito.mock import org.mockito.Mockito.`when` import org.mockito.MockitoAnnotations import java.io.FileInputStream +import java.io.InputStream class ImageProcessingServiceTest { @Mock @@ -38,6 +41,9 @@ class ImageProcessingServiceTest { @Mock internal var location: LatLng? = null + @Mock + internal lateinit var appContext: Context + @InjectMocks var imageProcessingService: ImageProcessingService? = null @@ -49,8 +55,10 @@ class ImageProcessingServiceTest { fun setUp() { MockitoAnnotations.openMocks(this) val mediaUri = mock(Uri::class.java) + val contentUri = mock(Uri::class.java) val mockPlace = mock(Place::class.java) val mockTitle = mock(List::class.java) + val contentResolver = mock(ContentResolver::class.java) `when`(mockPlace.wikiDataEntityId).thenReturn("Q1") `when`(mockPlace.getLocation()).thenReturn(mock(LatLng::class.java)) @@ -59,6 +67,8 @@ class ImageProcessingServiceTest { `when`(mockTitle.isSet).thenReturn(true)*/ `when`(uploadItem.mediaUri).thenReturn(mediaUri) + `when`(uploadItem.contentUri).thenReturn(contentUri) + `when`(appContext.contentResolver).thenReturn(contentResolver) `when`(uploadItem.imageQuality).thenReturn(ImageUtils.IMAGE_WAIT) `when`(uploadItem.uploadMediaDetails).thenReturn(mockTitle as MutableList?) @@ -68,7 +78,9 @@ class ImageProcessingServiceTest { `when`(fileUtilsWrapper!!.getFileInputStream(ArgumentMatchers.anyString())) .thenReturn(mock(FileInputStream::class.java)) - `when`(fileUtilsWrapper!!.getSHA1(any(FileInputStream::class.java))) + `when`(appContext.contentResolver.openInputStream(ArgumentMatchers.any())) + .thenReturn(mock(InputStream::class.java)) + `when`(fileUtilsWrapper!!.getSHA1(any(InputStream::class.java))) .thenReturn("fileSha") `when`(fileUtilsWrapper!!.getGeolocationOfFile(ArgumentMatchers.anyString(), any(LatLng::class.java)))