From 87938e2fbb141e7f80b9800b5f6a5d513bc104f0 Mon Sep 17 00:00:00 2001 From: Sean Mac Gillicuddy Date: Fri, 20 Mar 2020 10:29:03 +0000 Subject: [PATCH] #3408 Refactoring the FileProcessor and GPSExtractor classes - set coordinates for upload item if user chooses it --- .../repository/UploadRemoteDataSource.java | 4 +- .../commons/repository/UploadRepository.java | 4 +- .../free/nrw/commons/upload/FileProcessor.kt | 46 +++++++++---------- .../commons/upload/SimilarImageInterface.java | 2 +- .../free/nrw/commons/upload/UploadModel.java | 13 +++--- .../UploadMediaDetailFragment.java | 5 +- .../UploadMediaDetailsContract.java | 2 +- .../mediaDetails/UploadMediaPresenter.java | 9 ++-- .../upload/UploadMediaPresenterTest.kt | 27 ++++++----- 9 files changed, 53 insertions(+), 59 deletions(-) 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 35eca57b03..cf5aa9c052 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 @@ -204,7 +204,7 @@ public Place getNearbyPlaces(double latitude, double longitude) { } } - public void usePictureCoordinatesFrom(ImageCoordinates imageCoordinates) { - uploadModel.usePictureCoordinatesFrom(imageCoordinates); + public void useSimilarPictureCoordinates(ImageCoordinates imageCoordinates, int uploadItemIndex) { + uploadModel.useSimilarPictureCoordinates(imageCoordinates, uploadItemIndex); } } 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 4b0502f83c..3d08a48bb5 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 @@ -273,7 +273,7 @@ public Place checkNearbyPlaces(double decLatitude, double decLongitude) { return remoteDataSource.getNearbyPlaces(decLatitude, decLongitude); } - public void usePictureCoordinatesFrom(ImageCoordinates imageCoordinates) { - remoteDataSource.usePictureCoordinatesFrom(imageCoordinates); + public void useSimilarPictureCoordinates(ImageCoordinates imageCoordinates, int uploadItemIndex) { + remoteDataSource.useSimilarPictureCoordinates(imageCoordinates, uploadItemIndex); } } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.kt b/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.kt index 1dcdd2eb27..1d99317015 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.kt @@ -53,7 +53,6 @@ class FileProcessor @Inject constructor( if (originalImageCoordinates.decimalCoords == null) { //Find other photos taken around the same time which has gps coordinates findOtherImages( - originalImageCoordinates, File(filePath), similarImageInterface ) @@ -121,7 +120,6 @@ class FileProcessor @Inject constructor( * @param similarImageInterface */ private fun findOtherImages( - originalImageCoordinates: ImageCoordinates, fileBeingProcessed: File, similarImageInterface: SimilarImageInterface ) { @@ -141,7 +139,6 @@ class FileProcessor @Inject constructor( similarImageInterface.showSimilarImageFragment( fileBeingProcessed.path, fileCoordinatesPair.first.absolutePath, - originalImageCoordinates, fileCoordinatesPair.second ) } @@ -167,29 +164,28 @@ class FileProcessor @Inject constructor( * @param imageCoordinates */ fun useImageCoords(imageCoordinates: ImageCoordinates) { - if (imageCoordinates.decimalCoords != null) { - cacheController.setQtPoint(imageCoordinates.decLongitude, imageCoordinates.decLatitude) - val displayCatList = cacheController.findCategory() + requireNotNull(imageCoordinates.decimalCoords) + cacheController.setQtPoint(imageCoordinates.decLongitude, imageCoordinates.decLatitude) + val displayCatList = cacheController.findCategory() - // If no categories found in cache, call MediaWiki API to match image coords with nearby Commons categories - if (displayCatList.isEmpty()) { - compositeDisposable.add( - apiCall.request(imageCoordinates.decimalCoords) - .subscribeOn(Schedulers.io()) - .observeOn(Schedulers.io()) - .subscribe( - { gpsCategoryModel.categoryList = it }, - { - Timber.e(it) - gpsCategoryModel.clear() - } - ) - ) - Timber.d("displayCatList size 0, calling MWAPI %s", displayCatList) - } else { - Timber.d("Cache found, setting categoryList in model to %s", displayCatList) - gpsCategoryModel.categoryList = displayCatList - } + // If no categories found in cache, call MediaWiki API to match image coords with nearby Commons categories + if (displayCatList.isEmpty()) { + compositeDisposable.add( + apiCall.request(imageCoordinates.decimalCoords) + .subscribeOn(Schedulers.io()) + .observeOn(Schedulers.io()) + .subscribe( + { gpsCategoryModel.categoryList = it }, + { + Timber.e(it) + gpsCategoryModel.clear() + } + ) + ) + Timber.d("displayCatList size 0, calling MWAPI %s", displayCatList) + } else { + Timber.d("Cache found, setting categoryList in model to %s", displayCatList) + gpsCategoryModel.categoryList = displayCatList } } } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/SimilarImageInterface.java b/app/src/main/java/fr/free/nrw/commons/upload/SimilarImageInterface.java index 23a9543536..50e6e5f27a 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/SimilarImageInterface.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/SimilarImageInterface.java @@ -2,5 +2,5 @@ public interface SimilarImageInterface { void showSimilarImageFragment(String originalFilePath, String possibleFilePath, - ImageCoordinates originalImageCoordinates, ImageCoordinates similarImageCoordinates); + ImageCoordinates similarImageCoordinates); } 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 12fe21c678..e17436101e 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 @@ -129,10 +129,6 @@ private UploadItem getUploadItem(UploadableFile uploadableFile, return uploadItem; } - int getStepCount() { - return items.size() + 2; - } - public int getCount() { return items.size(); } @@ -211,8 +207,9 @@ public void updateUploadItem(int index, UploadItem uploadItem) { uploadItem1.setTitle(uploadItem.title); } - public void usePictureCoordinatesFrom(ImageCoordinates imageCoordinates) { + public void useSimilarPictureCoordinates(ImageCoordinates imageCoordinates, int uploadItemIndex) { fileProcessor.useImageCoords(imageCoordinates); + items.get(uploadItemIndex).setGpsCoords(imageCoordinates); } @SuppressWarnings("WeakerAccess") @@ -222,7 +219,11 @@ public static class UploadItem { private final Uri mediaUri; private final String mimeType; private final String source; - private final ImageCoordinates gpsCoords; + private ImageCoordinates gpsCoords; + + public void setGpsCoords(ImageCoordinates gpsCoords) { + this.gpsCoords = gpsCoords; + } private Title title; private List descriptions; 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 c32f305026..67a1314bbe 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 @@ -255,19 +255,18 @@ public void onButtonAddDescriptionClicked() { @Override public void showSimilarImageFragment(String originalFilePath, String possibleFilePath, - ImageCoordinates originalImageCoordinates, ImageCoordinates similarImageCoordinates) { + ImageCoordinates similarImageCoordinates) { SimilarImageDialogFragment newFragment = new SimilarImageDialogFragment(); newFragment.setCallback(new SimilarImageDialogFragment.Callback() { @Override public void onPositiveResponse() { Timber.d("positive response from similar image fragment"); - presenter.usePictureCoordinatesFrom(similarImageCoordinates); + presenter.useSimilarPictureCoordinates(similarImageCoordinates, callback.getIndexInViewFlipper(UploadMediaDetailFragment.this)); } @Override public void onNegativeResponse() { Timber.d("negative response from similar image fragment"); - presenter.usePictureCoordinatesFrom(originalImageCoordinates); } }); Bundle args = new Bundle(); 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 518d91de95..a04bcae329 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 @@ -50,7 +50,7 @@ void receiveImage(UploadableFile uploadableFile, @Contribution.FileSource String void fetchPreviousTitleAndDescription(int indexInViewFlipper); - void usePictureCoordinatesFrom(ImageCoordinates imageCoordinates); + void useSimilarPictureCoordinates(ImageCoordinates imageCoordinates, int uploadItemIndex); } 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 e1ac04d618..c36d1b781f 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 @@ -158,8 +158,8 @@ public void fetchPreviousTitleAndDescription(int indexInViewFlipper) { } @Override - public void usePictureCoordinatesFrom(ImageCoordinates imageCoordinates) { - repository.usePictureCoordinatesFrom(imageCoordinates); + public void useSimilarPictureCoordinates(ImageCoordinates imageCoordinates, int uploadItemIndex) { + repository.useSimilarPictureCoordinates(imageCoordinates, uploadItemIndex); } /** @@ -205,13 +205,12 @@ public void handleBadImage(Integer errorCode) { * notifies the user that a similar image exists * @param originalFilePath * @param possibleFilePath - * @param originalImageCoordinates * @param similarImageCoordinates */ @Override public void showSimilarImageFragment(String originalFilePath, String possibleFilePath, - ImageCoordinates originalImageCoordinates, ImageCoordinates similarImageCoordinates) { - view.showSimilarImageFragment(originalFilePath, possibleFilePath, originalImageCoordinates, + ImageCoordinates similarImageCoordinates) { + view.showSimilarImageFragment(originalFilePath, possibleFilePath, similarImageCoordinates ); } 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 accd4d0045..68bef84fd8 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 @@ -1,6 +1,7 @@ package fr.free.nrw.commons.upload import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.whenever import fr.free.nrw.commons.filepicker.UploadableFile import fr.free.nrw.commons.nearby.Place import fr.free.nrw.commons.repository.UploadRepository @@ -15,7 +16,6 @@ import org.junit.Test import org.mockito.ArgumentMatchers import org.mockito.ArgumentMatchers.eq import org.mockito.Mock -import org.mockito.Mockito import org.mockito.Mockito.verify import org.mockito.MockitoAnnotations @@ -71,7 +71,7 @@ class UploadMediaPresenterTest { */ @Test fun receiveImageTest() { - Mockito.`when`( + whenever( repository.preProcessImage( ArgumentMatchers.any(UploadableFile::class.java), ArgumentMatchers.any(Place::class.java), @@ -94,9 +94,9 @@ class UploadMediaPresenterTest { */ @Test fun verifyImageQualityTest() { - Mockito.`when`(repository.getImageQuality(ArgumentMatchers.any(UploadModel.UploadItem::class.java))) + whenever(repository.getImageQuality(ArgumentMatchers.any(UploadModel.UploadItem::class.java))) .thenReturn(testSingleImageResult) - Mockito.`when`(uploadItem.imageQuality).thenReturn(ArgumentMatchers.anyInt()) + whenever(uploadItem.imageQuality).thenReturn(ArgumentMatchers.anyInt()) uploadMediaPresenter.verifyImageQuality(uploadItem) verify(view).showProgress(true) testScheduler.triggerActions() @@ -132,11 +132,11 @@ class UploadMediaPresenterTest { */ @Test fun fetchPreviousImageAndTitleTestPositive() { - Mockito.`when`(repository.getPreviousUploadItem(ArgumentMatchers.anyInt())) + whenever(repository.getPreviousUploadItem(ArgumentMatchers.anyInt())) .thenReturn(uploadItem) - Mockito.`when`(uploadItem.descriptions).thenReturn(descriptions) - Mockito.`when`(uploadItem.title).thenReturn(title) - Mockito.`when`(title.getTitleText()).thenReturn(ArgumentMatchers.anyString()) + whenever(uploadItem.descriptions).thenReturn(descriptions) + whenever(uploadItem.title).thenReturn(title) + whenever(title.getTitleText()).thenReturn(ArgumentMatchers.anyString()) uploadMediaPresenter.fetchPreviousTitleAndDescription(0) verify(view).setTitleAndDescription(ArgumentMatchers.anyString(), ArgumentMatchers.any()) @@ -147,7 +147,7 @@ class UploadMediaPresenterTest { */ @Test fun fetchPreviousImageAndTitleTestNegative() { - Mockito.`when`(repository.getPreviousUploadItem(ArgumentMatchers.anyInt())) + whenever(repository.getPreviousUploadItem(ArgumentMatchers.anyInt())) .thenReturn(null) uploadMediaPresenter.fetchPreviousTitleAndDescription(0) verify(view).showMessage(ArgumentMatchers.anyInt(), ArgumentMatchers.anyInt()) @@ -159,7 +159,7 @@ class UploadMediaPresenterTest { @Test fun handleBadImageBaseTestInvalidLocation() { uploadMediaPresenter.handleBadImage(8) - verify(repository)?.saveValue(ArgumentMatchers.anyString(), eq(false)) + verify(repository).saveValue(ArgumentMatchers.anyString(), eq(false)) verify(view).showBadImagePopup(8) } @@ -187,10 +187,9 @@ class UploadMediaPresenterTest { */ @Test fun showSimilarImageFragmentTest() { - val original: ImageCoordinates = mock() val similar: ImageCoordinates = mock() - uploadMediaPresenter.showSimilarImageFragment("original", "possible", original, similar) - verify(view).showSimilarImageFragment("original", "possible", original, similar) + uploadMediaPresenter.showSimilarImageFragment("original", "possible", similar) + verify(view).showSimilarImageFragment("original", "possible", similar) } /** @@ -199,7 +198,7 @@ class UploadMediaPresenterTest { @Test fun setUploadItemTest() { uploadMediaPresenter.setUploadItem(0, uploadItem) - verify(repository)?.updateUploadItem(0, uploadItem) + verify(repository).updateUploadItem(0, uploadItem) } }