Skip to content

[Bug fix] Check if duplicate exist using both original and modified file's checksum #6169

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

Merged
merged 2 commits into from
Feb 4, 2025
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 @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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)
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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),
Expand Down Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
7 changes: 5 additions & 2 deletions app/src/main/java/fr/free/nrw/commons/upload/UploadModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -38,6 +41,9 @@ class ImageProcessingServiceTest {
@Mock
internal var location: LatLng? = null

@Mock
internal lateinit var appContext: Context

@InjectMocks
var imageProcessingService: ImageProcessingService? = null

Expand All @@ -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))
Expand All @@ -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>?)
Expand All @@ -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)))
Expand Down