mirror of
https://github.com/commons-app/apps-android-commons.git
synced 2025-10-26 12:23:58 +01:00
[Bug fix] Check if duplicate exist using both original and modified file's checksum (#6169)
* check original file's SHA too along with modified one Signed-off-by: parneet-guraya <gurayaparneet@gmail.com> * fix tests Signed-off-by: parneet-guraya <gurayaparneet@gmail.com> --------- Signed-off-by: parneet-guraya <gurayaparneet@gmail.com>
This commit is contained in:
Parneet Singh
Nicolas Raoul
parent
9038de2d6a
commit
6d104c556e
6 changed files with 61 additions and 17 deletions
|
|
@ -62,7 +62,10 @@ public class ContributionsPresenter implements UserActionListener {
|
|||
*/
|
||||
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) {
|
||||
|
|
|
|||
|
|
@ -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<Int> {
|
||||
return uploadModel.checkDuplicateImage(filePath)
|
||||
fun checkDuplicateImage(originalFilePath: Uri, modifiedFilePath: Uri): Single<Int> {
|
||||
return uploadModel.checkDuplicateImage(originalFilePath, modifiedFilePath)
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -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<Int> {
|
||||
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<Int> {
|
||||
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<Int> {
|
||||
return Single.fromCallable { fileUtilsWrapper.getSHA1(inputStream) }
|
||||
.flatMap { fileSha: String? ->
|
||||
mediaClient.checkFileExistsUsingSha(fileSha)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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<Int> =
|
||||
imageProcessingService.checkDuplicateImage(filePath)
|
||||
fun checkDuplicateImage(originalFilePath: Uri?, modifiedFilePath: Uri?): Single<Int> =
|
||||
imageProcessingService.checkIfFileAlreadyExists(
|
||||
originalFilePath = originalFilePath!!,
|
||||
modifiedFilePath = modifiedFilePath!!
|
||||
)
|
||||
|
||||
/**
|
||||
* Calls validateCaption() of ImageProcessingService to check caption of image
|
||||
|
|
|
|||
|
|
@ -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<UploadMediaDetail>?)
|
||||
|
|
@ -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)))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue