From f897af028a06f533f43e023846edf19de9729772 Mon Sep 17 00:00:00 2001 From: Vivek Maskara Date: Sun, 16 Dec 2018 21:04:49 +0530 Subject: [PATCH] Fix upload tests (#2130) --- .../nrw/commons/upload/FileUtilsWrapper.java | 4 +++ .../free/nrw/commons/upload/UploadModel.java | 25 +++++++++++-------- .../utils/BitmapRegionDecoderWrapper.java | 22 ++++++++++++++++ .../nrw/commons/utils/ImageUtilsWrapper.java | 25 +++++++++++++++++++ .../nrw/commons/upload/UploadModelTest.kt | 19 ++++++++++++-- 5 files changed, 83 insertions(+), 12 deletions(-) create mode 100644 app/src/main/java/fr/free/nrw/commons/utils/BitmapRegionDecoderWrapper.java create mode 100644 app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.java diff --git a/app/src/main/java/fr/free/nrw/commons/upload/FileUtilsWrapper.java b/app/src/main/java/fr/free/nrw/commons/upload/FileUtilsWrapper.java index 894afb9f1..e5119c095 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/FileUtilsWrapper.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/FileUtilsWrapper.java @@ -39,4 +39,8 @@ public class FileUtilsWrapper { public FileInputStream getFileInputStream(String filePath) throws FileNotFoundException { return FileUtils.getFileInputStream(filePath); } + + public String getGeolocationOfFile(String filePath) { + return FileUtils.getGeolocationOfFile(filePath); + } } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java index 126cc4fe5..f4e17543e 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java @@ -8,10 +8,8 @@ import android.database.Cursor; import android.graphics.BitmapRegionDecoder; import android.net.Uri; import android.support.annotation.Nullable; -import android.util.Log; import java.io.File; -import java.io.FileInputStream; import java.io.IOException; import java.util.ArrayList; import java.util.Date; @@ -26,7 +24,9 @@ import fr.free.nrw.commons.auth.SessionManager; import fr.free.nrw.commons.contributions.Contribution; import fr.free.nrw.commons.mwapi.MediaWikiApi; import fr.free.nrw.commons.settings.Prefs; +import fr.free.nrw.commons.utils.BitmapRegionDecoderWrapper; import fr.free.nrw.commons.utils.ImageUtils; +import fr.free.nrw.commons.utils.ImageUtilsWrapper; import io.reactivex.Observable; import io.reactivex.Single; import io.reactivex.disposables.Disposable; @@ -57,6 +57,8 @@ public class UploadModel { private SessionManager sessionManager; private Uri currentMediaUri; private FileUtilsWrapper fileUtilsWrapper; + private ImageUtilsWrapper imageUtilsWrapper; + private BitmapRegionDecoderWrapper bitmapRegionDecoderWrapper; private FileProcessor fileProcessor; @Inject @@ -67,9 +69,12 @@ public class UploadModel { MediaWikiApi mwApi, SessionManager sessionManager, FileUtilsWrapper fileUtilsWrapper, + ImageUtilsWrapper imageUtilsWrapper, + BitmapRegionDecoderWrapper bitmapRegionDecoderWrapper, FileProcessor fileProcessor) { this.licenses = licenses; this.prefs = prefs; + this.bitmapRegionDecoderWrapper = bitmapRegionDecoderWrapper; this.license = Prefs.Licenses.CC_BY_SA_3; this.licensesByName = licensesByName; this.context = context; @@ -78,6 +83,7 @@ public class UploadModel { this.sessionManager = sessionManager; this.fileUtilsWrapper = fileUtilsWrapper; this.fileProcessor = fileProcessor; + this.imageUtilsWrapper = imageUtilsWrapper; useExtStorage = this.prefs.getBoolean("useExternalStorage", false); } @@ -103,8 +109,8 @@ public class UploadModel { .map(b -> b ? ImageUtils.IMAGE_DUPLICATE : ImageUtils.IMAGE_OK), Single.fromCallable(() -> fileUtilsWrapper.getFileInputStream(filePath)) - .map(file -> BitmapRegionDecoder.newInstance(file, false)) - .map(ImageUtils::checkIfImageIsTooDark), //Returns IMAGE_DARK or IMAGE_OK + .map(file -> bitmapRegionDecoderWrapper.newInstance(file, false)) + .map(imageUtilsWrapper::checkIfImageIsTooDark), //Returns IMAGE_DARK or IMAGE_OK (dupe, dark) -> dupe | dark) .observeOn(Schedulers.io()) .subscribe(item.imageQuality::onNext, Timber::e); @@ -134,15 +140,14 @@ public class UploadModel { .map(fileUtilsWrapper::getSHA1) .map(mwApi::existingFile) .map(b -> b ? ImageUtils.IMAGE_DUPLICATE : ImageUtils.IMAGE_OK), - Single.fromCallable(() -> - filePath) - .map(FileUtils::getGeolocationOfFile) - .map(geoLocation -> ImageUtils.checkImageGeolocationIsDifferent(geoLocation,wikidataItemLocation)) + Single.fromCallable(() -> filePath) + .map(fileUtilsWrapper::getGeolocationOfFile) + .map(geoLocation -> imageUtilsWrapper.checkImageGeolocationIsDifferent(geoLocation,wikidataItemLocation)) .map(r -> r ? ImageUtils.IMAGE_GEOLOCATION_DIFFERENT : ImageUtils.IMAGE_OK), Single.fromCallable(() -> fileUtilsWrapper.getFileInputStream(filePath)) - .map(file -> BitmapRegionDecoder.newInstance(file, false)) - .map(ImageUtils::checkIfImageIsTooDark), //Returns IMAGE_DARK or IMAGE_OK + .map(file -> bitmapRegionDecoderWrapper.newInstance(file, false)) + .map(imageUtilsWrapper::checkIfImageIsTooDark), //Returns IMAGE_DARK or IMAGE_OK (dupe, wrongGeo, dark) -> dupe | wrongGeo | dark).subscribe(item.imageQuality::onNext); items.add(item); items.get(0).selected = true; diff --git a/app/src/main/java/fr/free/nrw/commons/utils/BitmapRegionDecoderWrapper.java b/app/src/main/java/fr/free/nrw/commons/utils/BitmapRegionDecoderWrapper.java new file mode 100644 index 000000000..21d411908 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/utils/BitmapRegionDecoderWrapper.java @@ -0,0 +1,22 @@ +package fr.free.nrw.commons.utils; + +import android.graphics.BitmapRegionDecoder; + +import java.io.FileInputStream; +import java.io.IOException; + +import javax.inject.Inject; +import javax.inject.Singleton; + +@Singleton +public class BitmapRegionDecoderWrapper { + + @Inject + public BitmapRegionDecoderWrapper() { + + } + + public BitmapRegionDecoder newInstance(FileInputStream file, boolean isSharable) throws IOException { + return BitmapRegionDecoder.newInstance(file, isSharable); + } +} diff --git a/app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.java b/app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.java new file mode 100644 index 000000000..d5b905e9d --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.java @@ -0,0 +1,25 @@ +package fr.free.nrw.commons.utils; + +import android.graphics.BitmapRegionDecoder; + +import javax.inject.Inject; +import javax.inject.Singleton; + +import static fr.free.nrw.commons.utils.ImageUtils.*; + +@Singleton +public class ImageUtilsWrapper { + + @Inject + public ImageUtilsWrapper() { + + } + + public @Result int checkIfImageIsTooDark(BitmapRegionDecoder bitmapRegionDecoder) { + return ImageUtils.checkIfImageIsTooDark(bitmapRegionDecoder); + } + + public boolean checkImageGeolocationIsDifferent(String geolocationOfFileString, String wikidataItemLocationString) { + return ImageUtils.checkImageGeolocationIsDifferent(geolocationOfFileString, wikidataItemLocationString); + } +} diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadModelTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadModelTest.kt index 34311acd0..ba26f9cba 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadModelTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadModelTest.kt @@ -4,16 +4,19 @@ import android.app.Application import android.content.ContentResolver import android.content.Context import android.content.SharedPreferences +import android.graphics.BitmapRegionDecoder import android.net.Uri import fr.free.nrw.commons.auth.SessionManager import fr.free.nrw.commons.mwapi.MediaWikiApi +import fr.free.nrw.commons.utils.BitmapRegionDecoderWrapper +import fr.free.nrw.commons.utils.ImageUtils.IMAGE_OK +import fr.free.nrw.commons.utils.ImageUtilsWrapper import org.junit.After import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test -import org.mockito.ArgumentMatchers.any -import org.mockito.ArgumentMatchers.anyString +import org.mockito.ArgumentMatchers.* import org.mockito.InjectMocks import org.mockito.Mock import org.mockito.Mockito.`when` @@ -45,6 +48,10 @@ class UploadModelTest { @Mock internal var fileUtilsWrapper: FileUtilsWrapper? = null @Mock + internal var imageUtilsWrapper: ImageUtilsWrapper? = null + @Mock + internal var bitmapRegionDecoderWrapper: BitmapRegionDecoderWrapper? = null + @Mock internal var fileProcessor: FileProcessor? = null @InjectMocks @@ -67,6 +74,14 @@ class UploadModelTest { .thenReturn("sha") `when`(fileUtilsWrapper!!.getFileInputStream(anyString())) .thenReturn(mock(FileInputStream::class.java)) + `when`(fileUtilsWrapper!!.getGeolocationOfFile(anyString())) + .thenReturn("") + `when`(imageUtilsWrapper!!.checkIfImageIsTooDark(any(BitmapRegionDecoder::class.java))) + .thenReturn(IMAGE_OK) + `when`(imageUtilsWrapper!!.checkImageGeolocationIsDifferent(anyString(), anyString())) + .thenReturn(false) + `when`(bitmapRegionDecoderWrapper!!.newInstance(any(FileInputStream::class.java), anyBoolean())) + .thenReturn(mock(BitmapRegionDecoder::class.java)) }