From 6a2a456f2ee242fd1da7279a1b4c7da13504e36d Mon Sep 17 00:00:00 2001 From: Sean Mac Gillicuddy Date: Thu, 12 Mar 2020 10:51:01 +0000 Subject: [PATCH 1/3] #3493 App freezes for 15 seconds when you press Next in UploadMediaDetailsFragment - add apropriate schedulers and convert justs to fromCallable --- .../repository/UploadRemoteDataSource.java | 5 +- .../commons/repository/UploadRepository.java | 5 +- .../upload/ImageProcessingService.java | 87 +++++++++---------- .../fr/free/nrw/commons/upload/ReadFBMD.java | 48 ---------- .../fr/free/nrw/commons/upload/ReadFBMD.kt | 35 ++++++++ .../free/nrw/commons/upload/UploadModel.java | 6 +- .../UploadMediaDetailFragment.java | 2 +- .../UploadMediaDetailsContract.java | 2 +- .../mediaDetails/UploadMediaPresenter.java | 15 ++-- .../nrw/commons/utils/ImageUtilsWrapper.java | 31 ------- .../nrw/commons/utils/ImageUtilsWrapper.kt | 19 ++++ .../upload/ImageProcessingServiceTest.kt | 16 ++-- .../upload/UploadMediaPresenterTest.kt | 6 +- .../nrw/commons/upload/UploadModelTest.kt | 4 +- 14 files changed, 124 insertions(+), 157 deletions(-) delete mode 100644 app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.java create mode 100644 app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.kt delete mode 100644 app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.java create mode 100644 app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.kt diff --git a/app/src/main/java/fr/free/nrw/commons/repository/UploadRemoteDataSource.java b/app/src/main/java/fr/free/nrw/commons/repository/UploadRemoteDataSource.java index 047574a1a7..2263eb5f3e 100644 --- a/app/src/main/java/fr/free/nrw/commons/repository/UploadRemoteDataSource.java +++ b/app/src/main/java/fr/free/nrw/commons/repository/UploadRemoteDataSource.java @@ -179,11 +179,10 @@ public Observable preProcessImage(UploadableFile uploadableFile, Pla * ask the UplaodModel for the image quality of the UploadItem * * @param uploadItem - * @param shouldValidateTitle * @return */ - public Single getImageQuality(UploadItem uploadItem, boolean shouldValidateTitle) { - return uploadModel.getImageQuality(uploadItem, shouldValidateTitle); + public Single getImageQuality(UploadItem uploadItem) { + return uploadModel.getImageQuality(uploadItem); } /** diff --git a/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java b/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java index 0e84f9f379..877327c148 100644 --- a/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java +++ b/app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java @@ -187,11 +187,10 @@ public Observable preProcessImage(UploadableFile uploadableFile, Pla * query the RemoteDataSource for image quality * * @param uploadItem - * @param shouldValidateTitle * @return */ - public Single getImageQuality(UploadItem uploadItem, boolean shouldValidateTitle) { - return remoteDataSource.getImageQuality(uploadItem, shouldValidateTitle); + public Single getImageQuality(UploadItem uploadItem) { + return remoteDataSource.getImageQuality(uploadItem); } /** diff --git a/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.java b/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.java index 31386d9d65..d9fd8a0346 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.java @@ -1,23 +1,21 @@ package fr.free.nrw.commons.upload; -import android.content.Context; - -import org.apache.commons.lang3.StringUtils; - -import javax.inject.Inject; -import javax.inject.Singleton; +import static fr.free.nrw.commons.utils.ImageUtils.EMPTY_TITLE; +import static fr.free.nrw.commons.utils.ImageUtils.FILE_NAME_EXISTS; +import static fr.free.nrw.commons.utils.ImageUtils.IMAGE_OK; +import android.content.Context; import fr.free.nrw.commons.media.MediaClient; import fr.free.nrw.commons.nearby.Place; import fr.free.nrw.commons.utils.ImageUtils; import fr.free.nrw.commons.utils.ImageUtilsWrapper; import io.reactivex.Single; +import io.reactivex.schedulers.Schedulers; +import javax.inject.Inject; +import javax.inject.Singleton; +import org.apache.commons.lang3.StringUtils; import timber.log.Timber; -import static fr.free.nrw.commons.utils.ImageUtils.EMPTY_TITLE; -import static fr.free.nrw.commons.utils.ImageUtils.FILE_NAME_EXISTS; -import static fr.free.nrw.commons.utils.ImageUtils.IMAGE_OK; - /** * Methods for pre-processing images to be uploaded */ @@ -41,38 +39,33 @@ public ImageProcessingService(FileUtilsWrapper fileUtilsWrapper, this.mediaClient = mediaClient; } - /** - * Check image quality before upload - * - checks duplicate image - * - checks dark image - * - checks geolocation for image - * - check for valid title - */ - Single validateImage(UploadModel.UploadItem uploadItem, boolean checkTitle) { - int currentImageQuality = uploadItem.getImageQuality(); - Timber.d("Current image quality is %d", currentImageQuality); - if (currentImageQuality == ImageUtils.IMAGE_KEEP) { - return Single.just(ImageUtils.IMAGE_OK); - } - Timber.d("Checking the validity of image"); - String filePath = uploadItem.getMediaUri().getPath(); - Single duplicateImage = checkDuplicateImage(filePath); - Single wrongGeoLocation = checkImageGeoLocation(uploadItem.getPlace(), filePath); - Single darkImage = checkDarkImage(filePath); - Single itemTitle = checkTitle ? validateItemTitle(uploadItem) : Single.just(ImageUtils.IMAGE_OK); - Single checkFBMD = checkFBMD(filePath); - Single checkEXIF = checkEXIF(filePath); - - Single zipResult = Single.zip(duplicateImage, wrongGeoLocation, darkImage, itemTitle, - (duplicate, wrongGeo, dark, title) -> { - Timber.d("Result for duplicate: %d, geo: %d, dark: %d, title: %d", duplicate, wrongGeo, dark, title); - return duplicate | wrongGeo | dark | title; - }); - return Single.zip(zipResult, checkFBMD , checkEXIF , (zip , fbmd , exif)->{ - Timber.d("zip:" + zip + "fbmd:" + fbmd + "exif:" + exif); - return zip | fbmd | exif; - }); + /** + * Check image quality before upload - checks duplicate image - checks dark image - checks + * geolocation for image - check for valid title + */ + Single validateImage(UploadModel.UploadItem uploadItem) { + int currentImageQuality = uploadItem.getImageQuality(); + Timber.d("Current image quality is %d", currentImageQuality); + if (currentImageQuality == ImageUtils.IMAGE_KEEP) { + return Single.just(ImageUtils.IMAGE_OK); } + Timber.d("Checking the validity of image"); + String filePath = uploadItem.getMediaUri().getPath(); + + return Single.zip( + checkDuplicateImage(filePath), + checkImageGeoLocation(uploadItem.getPlace(), filePath), + checkDarkImage(filePath), + validateItemTitle(uploadItem), + checkFBMD(filePath), + checkEXIF(filePath), + (duplicateImage, wrongGeoLocation, darkImage, itemTitle, fbmd, exif) -> { + Timber.d("duplicate: %d, geo: %d, dark: %d, title: %d" + "fbmd:" + fbmd + "exif:" + exif, + duplicateImage, wrongGeoLocation, darkImage, itemTitle); + return duplicateImage | wrongGeoLocation | darkImage | itemTitle | fbmd | exif; + } + ); + } /** * We want to discourage users from uploading images to Commons that were taken from Facebook. @@ -113,7 +106,8 @@ private Single validateItemTitle(UploadModel.UploadItem uploadItem) { .map(doesFileExist -> { Timber.d("Result for valid title is %s", doesFileExist); return doesFileExist ? FILE_NAME_EXISTS : IMAGE_OK; - }); + }) + .subscribeOn(Schedulers.io()); } /** @@ -124,14 +118,14 @@ private Single validateItemTitle(UploadModel.UploadItem uploadItem) { */ private Single checkDuplicateImage(String filePath) { Timber.d("Checking for duplicate image %s", filePath); - return Single.fromCallable(() -> - fileUtilsWrapper.getFileInputStream(filePath)) + return Single.fromCallable(() -> fileUtilsWrapper.getFileInputStream(filePath)) .map(fileUtilsWrapper::getSHA1) .flatMap(mediaClient::checkFileExistsUsingSha) .map(b -> { Timber.d("Result for duplicate image %s", b); return b ? ImageUtils.IMAGE_DUPLICATE : ImageUtils.IMAGE_OK; - }); + }) + .subscribeOn(Schedulers.io()); } /** @@ -164,7 +158,8 @@ private Single checkImageGeoLocation(Place place, String filePath) { return Single.just(ImageUtils.IMAGE_OK); } return imageUtilsWrapper.checkImageGeolocationIsDifferent(geoLocation, place.getLocation()); - }); + }) + .subscribeOn(Schedulers.io()); } } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.java b/app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.java deleted file mode 100644 index 99b2a5a5e1..0000000000 --- a/app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.java +++ /dev/null @@ -1,48 +0,0 @@ -package fr.free.nrw.commons.upload; - -import java.io.FileInputStream; -import java.io.IOException; - -import javax.inject.Inject; -import javax.inject.Singleton; - -import fr.free.nrw.commons.utils.ImageUtils; -import io.reactivex.Single; - -/** - * We want to discourage users from uploading images to Commons that were taken from Facebook. - * This attempts to detect whether an image was downloaded from Facebook by heuristically - * searching for metadata that is specific to images that come from Facebook. - */ -@Singleton -public class ReadFBMD { - - @Inject - public ReadFBMD() { - } - - public Single processMetadata(String path) { - try { - int psBlockOffset; - int fbmdOffset; - - try (FileInputStream fs = new FileInputStream(path)) { - byte[] bytes = new byte[4096]; - fs.read(bytes); - fs.close(); - String fileStr = new String(bytes); - psBlockOffset = fileStr.indexOf("8BIM"); - fbmdOffset = fileStr.indexOf("FBMD"); - } - - if (psBlockOffset > 0 && fbmdOffset > 0 - && fbmdOffset > psBlockOffset && fbmdOffset - psBlockOffset < 0x80) { - return Single.just(ImageUtils.FILE_FBMD); - } - } catch (IOException e) { - e.printStackTrace(); - } - return Single.just(ImageUtils.IMAGE_OK); - } -} - diff --git a/app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.kt b/app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.kt new file mode 100644 index 0000000000..9e9ea8e423 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.kt @@ -0,0 +1,35 @@ +package fr.free.nrw.commons.upload + +import fr.free.nrw.commons.utils.ImageUtils +import io.reactivex.Single +import io.reactivex.schedulers.Schedulers +import java.io.FileInputStream +import java.io.IOException +import javax.inject.Inject +import javax.inject.Singleton + +/** + * We want to discourage users from uploading images to Commons that were taken from Facebook. This + * attempts to detect whether an image was downloaded from Facebook by heuristically searching for + * metadata that is specific to images that come from Facebook. + */ +@Singleton +class ReadFBMD @Inject constructor() { + fun processMetadata(path: String?) = Single.fromCallable { + try { + val fileStr = FileInputStream(path).use { + ByteArray(4096).apply { it.read(this) }.toString() + } + if (isFbmd(fileStr.indexOf("8BIM"), fileStr.indexOf("FBMD"))) + return@fromCallable ImageUtils.FILE_FBMD + } catch (e: IOException) { + e.printStackTrace() + } + ImageUtils.IMAGE_OK + }.subscribeOn(Schedulers.io()) + + private fun isFbmd(psBlockOffset: Int, fbmdOffset: Int) = + psBlockOffset > 0 && fbmdOffset > 0 && + fbmdOffset > psBlockOffset && + fbmdOffset - psBlockOffset < 0x80 +} 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 9630d83e99..172d7a360c 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 @@ -119,8 +119,8 @@ public Observable preProcessImage(UploadableFile uploadableFile, return Observable.just(getUploadItem(uploadableFile, place, source, similarImageInterface)); } - public Single getImageQuality(UploadItem uploadItem, boolean checkTitle) { - return imageProcessingService.validateImage(uploadItem, checkTitle); + public Single getImageQuality(UploadItem uploadItem) { + return imageProcessingService.validateImage(uploadItem); } private UploadItem getUploadItem(UploadableFile uploadableFile, @@ -378,4 +378,4 @@ public int hashCode() { } } -} \ No newline at end of file +} diff --git a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.java b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.java index 1c6140b408..8b0b8c39f7 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.java @@ -251,7 +251,7 @@ private void showInfoAlert(int titleStringID, int messageStringId) { @OnClick(R.id.btn_next) public void onNextButtonClicked() { uploadItem.setDescriptions(descriptionsAdapter.getDescriptions()); - presenter.verifyImageQuality(uploadItem, true); + presenter.verifyImageQuality(uploadItem); } @OnClick(R.id.btn_previous) diff --git a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailsContract.java b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailsContract.java index b442d79396..280128999d 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailsContract.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailsContract.java @@ -43,7 +43,7 @@ interface UserActionListener extends BasePresenter { void receiveImage(UploadableFile uploadableFile, @Contribution.FileSource String source, Place place); - void verifyImageQuality(UploadItem uploadItem, boolean validateTitle); + void verifyImageQuality(UploadItem uploadItem); void setUploadItem(int index, UploadItem uploadItem); diff --git a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.java b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.java index 3cab3c7537..a468ee15e3 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaPresenter.java @@ -111,14 +111,14 @@ private void checkNearbyPlaces(UploadItem uploadItem) { * asks the repository to verify image quality * * @param uploadItem - * @param validateTitle */ @Override - public void verifyImageQuality(UploadItem uploadItem, boolean validateTitle) { + public void verifyImageQuality(UploadItem uploadItem) { view.showProgress(true); - Disposable imageQualityDisposable = repository - .getImageQuality(uploadItem, true) - .subscribeOn(ioScheduler) + + compositeDisposable.add( + repository + .getImageQuality(uploadItem) .observeOn(mainThreadScheduler) .subscribe(imageResult -> { view.showProgress(false); @@ -129,9 +129,8 @@ public void verifyImageQuality(UploadItem uploadItem, boolean validateTitle) { view.showMessage("" + throwable.getLocalizedMessage(), R.color.color_error); Timber.e(throwable, "Error occurred while handling image"); - }); - - compositeDisposable.add(imageQualityDisposable); + }) + ); } /** 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 deleted file mode 100644 index 5cfaf08ec5..0000000000 --- a/app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.java +++ /dev/null @@ -1,31 +0,0 @@ -package fr.free.nrw.commons.utils; - -import javax.inject.Inject; -import javax.inject.Singleton; - -import fr.free.nrw.commons.location.LatLng; -import io.reactivex.Single; -import io.reactivex.schedulers.Schedulers; - -@Singleton -public class ImageUtilsWrapper { - - @Inject - public ImageUtilsWrapper() { - - } - - public Single checkIfImageIsTooDark(String bitmapPath) { - return Single.just(ImageUtils.checkIfImageIsTooDark(bitmapPath)) - .subscribeOn(Schedulers.computation()) - .observeOn(Schedulers.computation()); - } - - public Single checkImageGeolocationIsDifferent(String geolocationOfFileString, LatLng latLng) { - boolean isImageGeoLocationDifferent = ImageUtils.checkImageGeolocationIsDifferent(geolocationOfFileString, latLng); - return Single.just(isImageGeoLocationDifferent) - .subscribeOn(Schedulers.computation()) - .observeOn(Schedulers.computation()) - .map(isDifferent -> isDifferent ? ImageUtils.IMAGE_GEOLOCATION_DIFFERENT : ImageUtils.IMAGE_OK); - } -} diff --git a/app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.kt b/app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.kt new file mode 100644 index 0000000000..93134c5eae --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.kt @@ -0,0 +1,19 @@ +package fr.free.nrw.commons.utils + +import fr.free.nrw.commons.location.LatLng +import io.reactivex.Single +import io.reactivex.schedulers.Schedulers +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +class ImageUtilsWrapper @Inject constructor() { + fun checkIfImageIsTooDark(bitmapPath: String?) = + Single.fromCallable { ImageUtils.checkIfImageIsTooDark(bitmapPath) } + .subscribeOn(Schedulers.computation()) + + fun checkImageGeolocationIsDifferent(geolocationOfFileString: String?, latLng: LatLng?) = + Single.just(ImageUtils.checkImageGeolocationIsDifferent(geolocationOfFileString, latLng)) + .subscribeOn(Schedulers.computation()) + .map { isDifferent: Boolean -> if (isDifferent) ImageUtils.IMAGE_GEOLOCATION_DIFFERENT else ImageUtils.IMAGE_OK } +} 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 cb42067cff..bba47a1934 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 @@ -88,7 +88,7 @@ class u { @Test fun validateImageForKeepImage() { `when`(uploadItem.imageQuality).thenReturn(ImageUtils.IMAGE_KEEP) - val validateImage = imageProcessingService!!.validateImage(uploadItem, false) + val validateImage = imageProcessingService!!.validateImage(uploadItem) assertEquals(ImageUtils.IMAGE_OK, validateImage.blockingGet()) } @@ -96,13 +96,13 @@ class u { fun validateImageForDuplicateImage() { `when`(mediaClient!!.checkFileExistsUsingSha(ArgumentMatchers.anyString())) .thenReturn(Single.just(true)) - val validateImage = imageProcessingService!!.validateImage(uploadItem, false) + val validateImage = imageProcessingService!!.validateImage(uploadItem) assertEquals(ImageUtils.IMAGE_DUPLICATE, validateImage.blockingGet()) } @Test fun validateImageForOkImage() { - val validateImage = imageProcessingService!!.validateImage(uploadItem, false) + val validateImage = imageProcessingService!!.validateImage(uploadItem) assertEquals(ImageUtils.IMAGE_OK, validateImage.blockingGet()) } @@ -110,7 +110,7 @@ class u { fun validateImageForDarkImage() { `when`(imageUtilsWrapper?.checkIfImageIsTooDark(ArgumentMatchers.anyString())) .thenReturn(Single.just(ImageUtils.IMAGE_DARK)) - val validateImage = imageProcessingService!!.validateImage(uploadItem, false) + val validateImage = imageProcessingService!!.validateImage(uploadItem) assertEquals(ImageUtils.IMAGE_DARK, validateImage.blockingGet()) } @@ -118,7 +118,7 @@ class u { fun validateImageForWrongGeoLocation() { `when`(imageUtilsWrapper!!.checkImageGeolocationIsDifferent(ArgumentMatchers.anyString(), any(LatLng::class.java))) .thenReturn(Single.just(ImageUtils.IMAGE_GEOLOCATION_DIFFERENT)) - val validateImage = imageProcessingService!!.validateImage(uploadItem, false) + val validateImage = imageProcessingService!!.validateImage(uploadItem) assertEquals(ImageUtils.IMAGE_GEOLOCATION_DIFFERENT, validateImage.blockingGet()) } @@ -126,7 +126,7 @@ class u { fun validateImageForFileNameExistsWithCheckTitleOff() { `when`(mediaClient?.checkPageExistsUsingTitle(ArgumentMatchers.anyString())) .thenReturn(Single.just(true)) - val validateImage = imageProcessingService!!.validateImage(uploadItem, false) + val validateImage = imageProcessingService!!.validateImage(uploadItem) assertEquals(ImageUtils.IMAGE_OK, validateImage.blockingGet()) } @@ -134,7 +134,7 @@ class u { fun validateImageForFileNameExistsWithCheckTitleOn() { `when`(mediaClient?.checkPageExistsUsingTitle(ArgumentMatchers.anyString())) .thenReturn(Single.just(true)) - val validateImage = imageProcessingService!!.validateImage(uploadItem, true) + val validateImage = imageProcessingService!!.validateImage(uploadItem) assertEquals(ImageUtils.FILE_NAME_EXISTS, validateImage.blockingGet()) } -} \ No newline at end of file +} diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaPresenterTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaPresenterTest.kt index eb7b2322bd..a16af59bbb 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaPresenterTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadMediaPresenterTest.kt @@ -82,9 +82,9 @@ class UploadMediaPresenterTest { */ @Test fun verifyImageQualityTest() { - Mockito.`when`(repository?.getImageQuality(ArgumentMatchers.any(UploadModel.UploadItem::class.java), ArgumentMatchers.any(Boolean::class.java))).thenReturn(testSingleImageResult) + Mockito.`when`(repository?.getImageQuality(ArgumentMatchers.any(UploadModel.UploadItem::class.java))).thenReturn(testSingleImageResult) Mockito.`when`(uploadItem?.imageQuality).thenReturn(ArgumentMatchers.anyInt()) - uploadMediaPresenter?.verifyImageQuality(uploadItem, true) + uploadMediaPresenter?.verifyImageQuality(uploadItem) verify(view)?.showProgress(true) testScheduler?.triggerActions() verify(view)?.showProgress(false) @@ -185,4 +185,4 @@ class UploadMediaPresenterTest { verify(repository)?.updateUploadItem(0,uploadItem) } -} \ No newline at end of file +} 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 c6f8518897..764c0a447f 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 @@ -65,7 +65,7 @@ class UploadModelTest { .thenReturn(mock(FileInputStream::class.java)) `when`(fileUtilsWrapper!!.getGeolocationOfFile(anyString())) .thenReturn("") - `when`(imageProcessingService!!.validateImage(any(UploadModel.UploadItem::class.java), anyBoolean())) + `when`(imageProcessingService!!.validateImage(any(UploadModel.UploadItem::class.java))) .thenReturn(Single.just(IMAGE_OK)) } @@ -129,4 +129,4 @@ class UploadModelTest { @Test fun buildContributions() { } -} \ No newline at end of file +} From c5e18f96a227fd603aa49a273a14c5abc189a687 Mon Sep 17 00:00:00 2001 From: Sean Mac Gillicuddy Date: Thu, 12 Mar 2020 10:55:00 +0000 Subject: [PATCH 2/3] #3493 App freezes for 15 seconds when you press Next in UploadMediaDetailsFragment - remove test for removed functionality --- .../free/nrw/commons/upload/ImageProcessingServiceTest.kt | 8 -------- 1 file changed, 8 deletions(-) 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 bba47a1934..d64905f79f 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 @@ -122,14 +122,6 @@ class u { assertEquals(ImageUtils.IMAGE_GEOLOCATION_DIFFERENT, validateImage.blockingGet()) } - @Test - fun validateImageForFileNameExistsWithCheckTitleOff() { - `when`(mediaClient?.checkPageExistsUsingTitle(ArgumentMatchers.anyString())) - .thenReturn(Single.just(true)) - val validateImage = imageProcessingService!!.validateImage(uploadItem) - assertEquals(ImageUtils.IMAGE_OK, validateImage.blockingGet()) - } - @Test fun validateImageForFileNameExistsWithCheckTitleOn() { `when`(mediaClient?.checkPageExistsUsingTitle(ArgumentMatchers.anyString())) From ef7e476976e6e3e22f7fc35a5df876f28c1faf11 Mon Sep 17 00:00:00 2001 From: Sean Mac Gillicuddy Date: Fri, 13 Mar 2020 10:37:14 +0000 Subject: [PATCH 3/3] #3493 App freezes for 15 seconds when you press Next in UploadMediaDetailsFragment - replace kotlin with java --- .../fr/free/nrw/commons/upload/ReadFBMD.java | 48 +++++++++++++++++++ .../fr/free/nrw/commons/upload/ReadFBMD.kt | 35 -------------- .../nrw/commons/utils/ImageUtilsWrapper.java | 30 ++++++++++++ .../nrw/commons/utils/ImageUtilsWrapper.kt | 19 -------- 4 files changed, 78 insertions(+), 54 deletions(-) create mode 100644 app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.java delete mode 100644 app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.kt create mode 100644 app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.java delete mode 100644 app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.kt diff --git a/app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.java b/app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.java new file mode 100644 index 0000000000..e98ab9ec5f --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.java @@ -0,0 +1,48 @@ +package fr.free.nrw.commons.upload; + +import fr.free.nrw.commons.utils.ImageUtils; +import io.reactivex.Single; +import java.io.FileInputStream; +import java.io.IOException; +import javax.inject.Inject; +import javax.inject.Singleton; + +/** + * We want to discourage users from uploading images to Commons that were taken from Facebook. This + * attempts to detect whether an image was downloaded from Facebook by heuristically searching for + * metadata that is specific to images that come from Facebook. + */ +@Singleton +public class ReadFBMD { + + @Inject + public ReadFBMD() { + } + + public Single processMetadata(String path) { + return Single.fromCallable(() -> { + try { + int psBlockOffset; + int fbmdOffset; + + try (FileInputStream fs = new FileInputStream(path)) { + byte[] bytes = new byte[4096]; + fs.read(bytes); + fs.close(); + String fileStr = new String(bytes); + psBlockOffset = fileStr.indexOf("8BIM"); + fbmdOffset = fileStr.indexOf("FBMD"); + } + + if (psBlockOffset > 0 && fbmdOffset > 0 + && fbmdOffset > psBlockOffset && fbmdOffset - psBlockOffset < 0x80) { + return ImageUtils.FILE_FBMD; + } + } catch (IOException e) { + e.printStackTrace(); + } + return ImageUtils.IMAGE_OK; + }); + } +} + diff --git a/app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.kt b/app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.kt deleted file mode 100644 index 9e9ea8e423..0000000000 --- a/app/src/main/java/fr/free/nrw/commons/upload/ReadFBMD.kt +++ /dev/null @@ -1,35 +0,0 @@ -package fr.free.nrw.commons.upload - -import fr.free.nrw.commons.utils.ImageUtils -import io.reactivex.Single -import io.reactivex.schedulers.Schedulers -import java.io.FileInputStream -import java.io.IOException -import javax.inject.Inject -import javax.inject.Singleton - -/** - * We want to discourage users from uploading images to Commons that were taken from Facebook. This - * attempts to detect whether an image was downloaded from Facebook by heuristically searching for - * metadata that is specific to images that come from Facebook. - */ -@Singleton -class ReadFBMD @Inject constructor() { - fun processMetadata(path: String?) = Single.fromCallable { - try { - val fileStr = FileInputStream(path).use { - ByteArray(4096).apply { it.read(this) }.toString() - } - if (isFbmd(fileStr.indexOf("8BIM"), fileStr.indexOf("FBMD"))) - return@fromCallable ImageUtils.FILE_FBMD - } catch (e: IOException) { - e.printStackTrace() - } - ImageUtils.IMAGE_OK - }.subscribeOn(Schedulers.io()) - - private fun isFbmd(psBlockOffset: Int, fbmdOffset: Int) = - psBlockOffset > 0 && fbmdOffset > 0 && - fbmdOffset > psBlockOffset && - fbmdOffset - psBlockOffset < 0x80 -} 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 0000000000..bb40ee1cf4 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.java @@ -0,0 +1,30 @@ +package fr.free.nrw.commons.utils; + +import fr.free.nrw.commons.location.LatLng; +import io.reactivex.Single; +import io.reactivex.schedulers.Schedulers; +import javax.inject.Inject; +import javax.inject.Singleton; + +@Singleton +public class ImageUtilsWrapper { + + @Inject + public ImageUtilsWrapper() { + + } + + public Single checkIfImageIsTooDark(String bitmapPath) { + return Single.fromCallable(() -> ImageUtils.checkIfImageIsTooDark(bitmapPath)) + .subscribeOn(Schedulers.computation()); + } + + public Single checkImageGeolocationIsDifferent(String geolocationOfFileString, + LatLng latLng) { + return Single.fromCallable( + () -> ImageUtils.checkImageGeolocationIsDifferent(geolocationOfFileString, latLng)) + .subscribeOn(Schedulers.computation()) + .map(isDifferent -> isDifferent ? ImageUtils.IMAGE_GEOLOCATION_DIFFERENT + : ImageUtils.IMAGE_OK); + } +} diff --git a/app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.kt b/app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.kt deleted file mode 100644 index 93134c5eae..0000000000 --- a/app/src/main/java/fr/free/nrw/commons/utils/ImageUtilsWrapper.kt +++ /dev/null @@ -1,19 +0,0 @@ -package fr.free.nrw.commons.utils - -import fr.free.nrw.commons.location.LatLng -import io.reactivex.Single -import io.reactivex.schedulers.Schedulers -import javax.inject.Inject -import javax.inject.Singleton - -@Singleton -class ImageUtilsWrapper @Inject constructor() { - fun checkIfImageIsTooDark(bitmapPath: String?) = - Single.fromCallable { ImageUtils.checkIfImageIsTooDark(bitmapPath) } - .subscribeOn(Schedulers.computation()) - - fun checkImageGeolocationIsDifferent(geolocationOfFileString: String?, latLng: LatLng?) = - Single.just(ImageUtils.checkImageGeolocationIsDifferent(geolocationOfFileString, latLng)) - .subscribeOn(Schedulers.computation()) - .map { isDifferent: Boolean -> if (isDifferent) ImageUtils.IMAGE_GEOLOCATION_DIFFERENT else ImageUtils.IMAGE_OK } -}