From 76b475fde72b703efe3c0c4c4a94500457922669 Mon Sep 17 00:00:00 2001 From: Sean Mac Gillicuddy Date: Wed, 18 Mar 2020 15:47:14 +0000 Subject: [PATCH 1/8] #3408 Refactoring the FileProcessor and GPSExtractor classes - refactor FileProcessor --- .../commons/di/CommonsApplicationModule.java | 38 +- .../repository/UploadRemoteDataSource.java | 5 + .../commons/repository/UploadRepository.java | 5 + .../nrw/commons/upload/FileProcessor.java | 411 +++++++++--------- .../upload/SimilarImageDialogFragment.java | 10 +- .../commons/upload/SimilarImageInterface.java | 3 +- .../free/nrw/commons/upload/UploadModel.java | 82 +--- .../UploadMediaDetailFragment.java | 14 +- .../UploadMediaDetailsContract.java | 4 + .../mediaDetails/UploadMediaPresenter.java | 17 +- .../upload/UploadMediaPresenterTest.kt | 137 +++--- .../nrw/commons/upload/UploadModelTest.kt | 132 ------ 12 files changed, 347 insertions(+), 511 deletions(-) delete mode 100644 app/src/test/kotlin/fr/free/nrw/commons/upload/UploadModelTest.kt diff --git a/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.java b/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.java index 4e8e50fe11..8ed13d4df1 100644 --- a/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.java +++ b/app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.java @@ -2,26 +2,13 @@ import android.app.Activity; import android.content.ContentProviderClient; +import android.content.ContentResolver; import android.content.Context; import android.view.inputmethod.InputMethodManager; - import androidx.collection.LruCache; import androidx.room.Room; - import com.github.varunpant.quadtree.QuadTree; import com.google.gson.Gson; - -import org.wikipedia.AppAdapter; - -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Objects; - -import javax.inject.Named; -import javax.inject.Singleton; - import dagger.Module; import dagger.Provides; import fr.free.nrw.commons.BuildConfig; @@ -41,10 +28,18 @@ import io.reactivex.Scheduler; import io.reactivex.android.schedulers.AndroidSchedulers; import io.reactivex.schedulers.Schedulers; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import javax.inject.Named; +import javax.inject.Singleton; +import org.wikipedia.AppAdapter; /** - * The Dependency Provider class for Commons Android. - * + * The Dependency Provider class for Commons Android. + * * Provides all sorts of ContentProviderClients used by the app * along with the Liscences, AccountUtility, UploadController, Logged User, * Location manager etc @@ -101,7 +96,7 @@ public AccountUtil providesAccountUtil(Context context) { } /** - * Provides an instance of CategoryContentProviderClient i.e. the categories + * Provides an instance of CategoryContentProviderClient i.e. the categories * that are there in local storage */ @Provides @@ -203,7 +198,7 @@ public boolean provideIsBetaVariant() { /** * Provide JavaRx IO scheduler which manages IO operations - * across various Threads + * across various Threads */ @Named(IO_THREAD) @Provides @@ -244,4 +239,9 @@ public AppDatabase provideAppDataBase() { public ContributionDao providesContributionsDao() { return appDatabase.getContributionDao(); } -} \ No newline at end of file + + @Provides + public ContentResolver providesContentResolver(Context context){ + return context.getContentResolver(); + } +} 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 2263eb5f3e..3930b7ba86 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 @@ -1,5 +1,6 @@ package fr.free.nrw.commons.repository; +import fr.free.nrw.commons.upload.GPSExtractor; import java.io.IOException; import java.util.Comparator; import java.util.List; @@ -202,4 +203,8 @@ public Place getNearbyPlaces(double latitude, double longitude) { return null; } } + + public void usePictureCoordinatesFrom(GPSExtractor gpsExtractor) { + uploadModel.usePictureCoordinatesFrom(gpsExtractor); + } } 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 877327c148..99af4e31b4 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 @@ -1,5 +1,6 @@ package fr.free.nrw.commons.repository; +import fr.free.nrw.commons.upload.GPSExtractor; import java.util.Comparator; import java.util.List; @@ -271,4 +272,8 @@ public void setSelectedLicense(String licenseName) { public Place checkNearbyPlaces(double decLatitude, double decLongitude) { return remoteDataSource.getNearbyPlaces(decLatitude, decLongitude); } + + public void usePictureCoordinatesFrom(GPSExtractor gpsExtractor) { + remoteDataSource.usePictureCoordinatesFrom(gpsExtractor); + } } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.java b/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.java index 361ecf0ad8..629a7dd6bc 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.java @@ -4,242 +4,231 @@ import android.content.ContentResolver; import android.content.Context; import android.net.Uri; - -import androidx.annotation.NonNull; import androidx.exifinterface.media.ExifInterface; - -import com.google.gson.reflect.TypeToken; - -import java.io.File; -import java.io.IOException; -import java.lang.reflect.Type; -import java.util.Arrays; -import java.util.HashSet; -import java.util.List; -import java.util.Set; - -import javax.inject.Inject; -import javax.inject.Named; -import javax.inject.Singleton; - import fr.free.nrw.commons.R; import fr.free.nrw.commons.caching.CacheController; import fr.free.nrw.commons.kvstore.JsonKvStore; import fr.free.nrw.commons.mwapi.CategoryApi; import fr.free.nrw.commons.settings.Prefs; -import fr.free.nrw.commons.upload.SimilarImageDialogFragment.Callback; import io.reactivex.Observable; import io.reactivex.disposables.CompositeDisposable; -import io.reactivex.disposables.Disposable; import io.reactivex.schedulers.Schedulers; +import java.io.File; +import java.io.IOException; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import javax.inject.Inject; +import javax.inject.Named; +import org.jetbrains.annotations.NotNull; import timber.log.Timber; /** * Processing of the image filePath that is about to be uploaded via ShareActivity is done here */ -@Singleton -public class FileProcessor implements Callback { - - @Inject - CacheController cacheController; - @Inject - GpsCategoryModel gpsCategoryModel; - @Inject - CategoryApi apiCall; - @Inject - @Named("default_preferences") - JsonKvStore defaultKvStore; - private String filePath; - private ContentResolver contentResolver; - private GPSExtractor imageObj; - private String decimalCoords; - private ExifInterface exifInterface; - private boolean haveCheckedForOtherImages = false; - private GPSExtractor tempImageObj; - private CompositeDisposable compositeDisposable = new CompositeDisposable(); - - @Inject - public FileProcessor() { - } - - public void cleanup() { - compositeDisposable.clear(); - } - - void initFileDetails(@NonNull String filePath, ContentResolver contentResolver) { - this.filePath = filePath; - this.contentResolver = contentResolver; - try { - exifInterface = new ExifInterface(filePath); - } catch (IOException e) { - Timber.e(e); - } +public class FileProcessor { + + private final Context context; + private final ContentResolver contentResolver; + private final CacheController cacheController; + private final GpsCategoryModel gpsCategoryModel; + private final JsonKvStore defaultKvStore; + private final CategoryApi apiCall; + + private final CompositeDisposable compositeDisposable = new CompositeDisposable(); + + @Inject + public FileProcessor(Context context, ContentResolver contentResolver, + CacheController cacheController, GpsCategoryModel gpsCategoryModel, + @Named("default_preferences") JsonKvStore defaultKvStore, CategoryApi apiCall) { + this.context = context; + this.contentResolver = contentResolver; + this.cacheController = cacheController; + this.gpsCategoryModel = gpsCategoryModel; + this.defaultKvStore = defaultKvStore; + this.apiCall = apiCall; + } + + public void cleanup() { + compositeDisposable.clear(); + } + + /** + * Processes filePath coordinates, either from EXIF data or user location + */ + GPSExtractor processFileCoordinates(SimilarImageInterface similarImageInterface, + String filePath) { + ExifInterface exifInterface = null; + try { + exifInterface = new ExifInterface(filePath); + } catch (IOException e) { + Timber.e(e); } - - /** - * Processes filePath coordinates, either from EXIF data or user location - */ - GPSExtractor processFileCoordinates(SimilarImageInterface similarImageInterface, Context context) { - // Redact EXIF data as indicated in preferences. - redactExifTags(exifInterface, getExifTagsToRedact(context)); - - Timber.d("Calling GPSExtractor"); - imageObj = new GPSExtractor(exifInterface); - decimalCoords = imageObj.getCoords(); - if (decimalCoords == null || !imageObj.imageCoordsExists) { - //Find other photos taken around the same time which has gps coordinates - if (!haveCheckedForOtherImages) - findOtherImages(similarImageInterface);// Do not do repeat the process - } else { - useImageCoords(); - } - - return imageObj; + // Redact EXIF data as indicated in preferences. + redactExifTags(exifInterface, getExifTagsToRedact()); + + Timber.d("Calling GPSExtractor"); + GPSExtractor originalImageExtractor = new GPSExtractor(exifInterface); + String decimalCoords = originalImageExtractor.getCoords(); + if (decimalCoords == null || !originalImageExtractor.imageCoordsExists) { + //Find other photos taken around the same time which has gps coordinates + findOtherImages(originalImageExtractor, new File(filePath), + similarImageInterface);// Do not do repeat the process + } else { + useImageCoords(originalImageExtractor); } - /** - * Gets EXIF Tags from preferences to be redacted. - * - * @param context application context - * @return tags to be redacted - */ - private Set getExifTagsToRedact(Context context) { - Type setType = new TypeToken>() {}.getType(); - Set prefManageEXIFTags = defaultKvStore.getStringSet(Prefs.MANAGED_EXIF_TAGS); + return originalImageExtractor; + } - Set redactTags = new HashSet<>(Arrays.asList( - context.getResources().getStringArray(R.array.pref_exifTag_values))); - Timber.d(redactTags.toString()); + /** + * Gets EXIF Tags from preferences to be redacted. + * + * @return tags to be redacted + */ + private Set getExifTagsToRedact() { + Set prefManageEXIFTags = defaultKvStore.getStringSet(Prefs.MANAGED_EXIF_TAGS); - if (prefManageEXIFTags != null) redactTags.removeAll(prefManageEXIFTags); + Set redactTags = new HashSet<>(Arrays.asList( + context.getResources().getStringArray(R.array.pref_exifTag_values))); + Timber.d(redactTags.toString()); - return redactTags; + if (prefManageEXIFTags != null) { + redactTags.removeAll(prefManageEXIFTags); } - /** - * Redacts EXIF metadata as indicated in preferences. - * - * @param exifInterface ExifInterface object - * @param redactTags tags to be redacted - */ - public static void redactExifTags(ExifInterface exifInterface, Set redactTags) { - if(redactTags.isEmpty()) return; - - Disposable disposable = Observable.fromIterable(redactTags) - .flatMap(tag -> Observable.fromArray(FileMetadataUtils.getTagsFromPref(tag))) - .forEach(tag -> { - Timber.d("Checking for tag: %s", tag); - String oldValue = exifInterface.getAttribute(tag); - if (oldValue != null && !oldValue.isEmpty()) { - Timber.d("Exif tag %s with value %s redacted.", tag, oldValue); - exifInterface.setAttribute(tag, null); - } - }); - CompositeDisposable disposables = new CompositeDisposable(); - disposables.add(disposable); - disposables.clear(); - - try { - exifInterface.saveAttributes(); - } catch (IOException e) { - Timber.w("EXIF redaction failed: %s", e.toString()); - } + return redactTags; + } + + /** + * Redacts EXIF metadata as indicated in preferences. + * + * @param exifInterface ExifInterface object + * @param redactTags tags to be redacted + */ + private void redactExifTags(ExifInterface exifInterface, Set redactTags) { + compositeDisposable.add( + Observable.fromIterable(redactTags) + .flatMap(tag -> Observable.fromArray(FileMetadataUtils.getTagsFromPref(tag))) + .subscribe( + (tag) -> redactTag(exifInterface, tag), + Timber::d, + () -> save(exifInterface) + )); + } + + private void save(ExifInterface exifInterface) { + try { + exifInterface.saveAttributes(); + } catch (IOException e) { + Timber.w("EXIF redaction failed: %s", e.toString()); } - - /** - * Find other images around the same location that were taken within the last 20 sec - * @param similarImageInterface - */ - private void findOtherImages(SimilarImageInterface similarImageInterface) { - Timber.d("filePath" + filePath); - - long timeOfCreation = new File(filePath).lastModified();//Time when the original image was created - File folder = new File(filePath.substring(0, filePath.lastIndexOf('/'))); - File[] files = folder.listFiles(); - Timber.d("folderTime Number:" + files.length); - - - for (File file : files) { - if (file.lastModified() - timeOfCreation <= (120 * 1000) && file.lastModified() - timeOfCreation >= -(120 * 1000)) { - //Make sure the photos were taken within 20seconds - Timber.d("fild date:" + file.lastModified() + " time of creation" + timeOfCreation); - tempImageObj = null;//Temporary GPSExtractor to extract coords from these photos - try { - tempImageObj = new GPSExtractor(contentResolver.openInputStream(Uri.fromFile(file))); - } catch (Exception e) { - e.printStackTrace(); - } - if (tempImageObj != null) { - tempImageObj = new GPSExtractor(file.getAbsolutePath()); - } - if (tempImageObj != null) { - Timber.d("not null fild EXIF" + tempImageObj.imageCoordsExists + " coords" + tempImageObj.getCoords()); - if (tempImageObj.getCoords() != null && tempImageObj.imageCoordsExists) { - // Current image has gps coordinates and it's not current gps locaiton - Timber.d("This filePath has image coords:" + file.getAbsolutePath()); - similarImageInterface.showSimilarImageFragment(filePath, file.getAbsolutePath()); - break; - } - } - } - } - haveCheckedForOtherImages = true; //Finished checking for other images + } + + private void redactTag(ExifInterface exifInterface, String tag) { + Timber.d("Checking for tag: %s", tag); + String oldValue = exifInterface.getAttribute(tag); + if (oldValue != null && !oldValue.isEmpty()) { + Timber.d("Exif tag %s with value %s redacted.", tag, oldValue); + exifInterface.setAttribute(tag, null); } - - /** - * Initiates retrieval of image coordinates or user coordinates, and caching of coordinates. - * Then initiates the calls to MediaWiki API through an instance of CategoryApi. - */ - @SuppressLint("CheckResult") - private void useImageCoords() { - if (decimalCoords != null) { - Timber.d("Decimal coords of image: %s", decimalCoords); - Timber.d("is EXIF data present:" + imageObj.imageCoordsExists + " from findOther image"); - - // Only set cache for this point if image has coords - if (imageObj.imageCoordsExists) { - double decLongitude = imageObj.getDecLongitude(); - double decLatitude = imageObj.getDecLatitude(); - cacheController.setQtPoint(decLongitude, decLatitude); - } - - List displayCatList = cacheController.findCategory(); - boolean catListEmpty = displayCatList.isEmpty(); - - - // If no categories found in cache, call MediaWiki API to match image coords with nearby Commons categories - if (catListEmpty) { - compositeDisposable.add(apiCall.request(decimalCoords) - .subscribeOn(Schedulers.io()) - .observeOn(Schedulers.io()) - .subscribe( - gpsCategoryModel::setCategoryList, - throwable -> { - Timber.e(throwable); - gpsCategoryModel.clear(); - } - )); - Timber.d("displayCatList size 0, calling MWAPI %s", displayCatList); - } else { - Timber.d("Cache found, setting categoryList in model to %s", displayCatList); - gpsCategoryModel.setCategoryList(displayCatList); - } - } else { - Timber.d("EXIF: no coords"); + } + + /** + * Find other images around the same location that were taken within the last 20 sec + * + * @param originalImageExtractor + * @param fileBeingProcessed + * @param similarImageInterface + */ + private void findOtherImages( + GPSExtractor originalImageExtractor, + File fileBeingProcessed, + SimilarImageInterface similarImageInterface) { + //Time when the original image was created + long timeOfCreation = fileBeingProcessed.lastModified(); + File[] files = fileBeingProcessed.getParentFile().listFiles(); + Timber.d("folderTime Number:" + files.length); + + for (File file : files) { + if (file.lastModified() - timeOfCreation <= (120 * 1000) + && file.lastModified() - timeOfCreation >= -(120 * 1000)) { + //Make sure the photos were taken within 20seconds + Timber.d("fild date:" + file.lastModified() + " time of creation" + timeOfCreation); + GPSExtractor similarPictureExtractor = createGpsExtractor(file); + Timber.d("not null fild EXIF" + similarPictureExtractor.imageCoordsExists + " coords" + + similarPictureExtractor.getCoords()); + if (similarPictureExtractor.getCoords() != null + && similarPictureExtractor.imageCoordsExists) { + // Current image has gps coordinates and it's not current gps locaiton + Timber.d("This filePath has image coords:" + file.getAbsolutePath()); + similarImageInterface.showSimilarImageFragment( + fileBeingProcessed.getPath(), + file.getAbsolutePath(), + originalImageExtractor, + similarPictureExtractor + ); + break; } + } } - - @Override - public void onPositiveResponse() { - imageObj = tempImageObj; - decimalCoords = imageObj.getCoords();// Not necessary to use gps as image already ha EXIF data - Timber.d("EXIF from tempImageObj"); - useImageCoords(); + } + + @NotNull + private GPSExtractor createGpsExtractor(File file) { + try { + return new GPSExtractor(contentResolver.openInputStream(Uri.fromFile(file))); + } catch (IOException e) { + e.printStackTrace(); + return new GPSExtractor(file.getAbsolutePath()); } - - @Override - public void onNegativeResponse() { - Timber.d("EXIF from imageObj"); - useImageCoords(); + } + + /** + * Initiates retrieval of image coordinates or user coordinates, and caching of coordinates. Then + * initiates the calls to MediaWiki API through an instance of CategoryApi. + * + * @param gpsExtractor + */ + @SuppressLint("CheckResult") + public void useImageCoords(GPSExtractor gpsExtractor) { + useImageCoords(gpsExtractor, gpsExtractor.getCoords()); + } + + private void useImageCoords(GPSExtractor gpsExtractor, String decimalCoords) { + if (decimalCoords != null) { + Timber.d("Decimal coords of image: %s", decimalCoords); + Timber.d("is EXIF data present:" + gpsExtractor.imageCoordsExists + + " from findOther image"); + + // Only set cache for this point if image has coords + if (gpsExtractor.imageCoordsExists) { + cacheController.setQtPoint(gpsExtractor.getDecLongitude(), gpsExtractor.getDecLatitude()); + } + + List displayCatList = cacheController.findCategory(); + boolean catListEmpty = displayCatList.isEmpty(); + + // If no categories found in cache, call MediaWiki API to match image coords with nearby Commons categories + if (catListEmpty) { + compositeDisposable.add(apiCall.request(decimalCoords) + .subscribeOn(Schedulers.io()) + .observeOn(Schedulers.io()) + .subscribe( + gpsCategoryModel::setCategoryList, + throwable -> { + Timber.e(throwable); + gpsCategoryModel.clear(); + } + )); + Timber.d("displayCatList size 0, calling MWAPI %s", displayCatList); + } else { + Timber.d("Cache found, setting categoryList in model to %s", displayCatList); + gpsCategoryModel.setCategoryList(displayCatList); + } + } else { + Timber.d("EXIF: no coords"); } -} \ No newline at end of file + } +} diff --git a/app/src/main/java/fr/free/nrw/commons/upload/SimilarImageDialogFragment.java b/app/src/main/java/fr/free/nrw/commons/upload/SimilarImageDialogFragment.java index 84b5b1081e..2491fad2be 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/SimilarImageDialogFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/SimilarImageDialogFragment.java @@ -9,20 +9,16 @@ import android.view.ViewGroup; import android.view.Window; import android.widget.Button; - import androidx.annotation.Nullable; import androidx.fragment.app.DialogFragment; import androidx.vectordrawable.graphics.drawable.VectorDrawableCompat; - -import com.facebook.drawee.generic.GenericDraweeHierarchyBuilder; -import com.facebook.drawee.view.SimpleDraweeView; - -import java.io.File; - import butterknife.BindView; import butterknife.ButterKnife; import butterknife.OnClick; +import com.facebook.drawee.generic.GenericDraweeHierarchyBuilder; +import com.facebook.drawee.view.SimpleDraweeView; import fr.free.nrw.commons.R; +import java.io.File; /** * Created by harisanker on 14/2/18. 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 6436f621bb..08bf299f1f 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 @@ -1,5 +1,6 @@ package fr.free.nrw.commons.upload; public interface SimilarImageInterface { - void showSimilarImageFragment(String originalFilePath, String possibleFilePath); + void showSimilarImageFragment(String originalFilePath, String possibleFilePath, + GPSExtractor originalPictureExtractor, GPSExtractor similarPictureExtractor); } 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 f92f5bfa6a..2b7b5e470b 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 @@ -3,20 +3,7 @@ import android.annotation.SuppressLint; import android.content.Context; import android.net.Uri; - import androidx.annotation.Nullable; - -import java.util.ArrayList; -import java.util.Date; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Objects; - -import javax.inject.Inject; -import javax.inject.Named; -import javax.inject.Singleton; - import fr.free.nrw.commons.CommonsApplication; import fr.free.nrw.commons.Utils; import fr.free.nrw.commons.auth.SessionManager; @@ -31,26 +18,25 @@ import io.reactivex.Single; import io.reactivex.disposables.CompositeDisposable; import io.reactivex.subjects.BehaviorSubject; +import java.util.ArrayList; +import java.util.Date; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import javax.inject.Inject; +import javax.inject.Named; +import javax.inject.Singleton; import timber.log.Timber; @Singleton public class UploadModel { - private static UploadItem DUMMY = new UploadItem( - Uri.EMPTY, Uri.EMPTY, - "", - "", - GPSExtractor.DUMMY, - null, - -1L, "") { - }; private final JsonKvStore store; private final List licenses; private final Context context; private String license; private final Map licensesByName; private List items = new ArrayList<>(); - private int currentStepIndex = 0; private CompositeDisposable compositeDisposable = new CompositeDisposable(); private SessionManager sessionManager; @@ -95,19 +81,6 @@ public void setSelectedCategories(List selectedCategories) { this.selectedCategories = selectedCategories; } - /** - * pre process a list of items - */ - @SuppressLint("CheckResult") - Observable preProcessImages(List uploadableFiles, - Place place, - String source, - SimilarImageInterface similarImageInterface) { - return Observable.fromIterable(uploadableFiles) - .map(uploadableFile -> getUploadItem(uploadableFile, place, source, - similarImageInterface)); - } - /** * pre process a one item at a time @@ -127,8 +100,6 @@ private UploadItem getUploadItem(UploadableFile uploadableFile, Place place, String source, SimilarImageInterface similarImageInterface) { - fileProcessor.initFileDetails(Objects.requireNonNull(uploadableFile.getFilePath()), - context.getContentResolver()); UploadableFile.DateTimeWithSource dateTimeWithSource = uploadableFile .getFileCreatedDate(context); long fileCreatedDate = -1; @@ -139,7 +110,7 @@ private UploadItem getUploadItem(UploadableFile uploadableFile, } Timber.d("File created date is %d", fileCreatedDate); GPSExtractor gpsExtractor = fileProcessor - .processFileCoordinates(similarImageInterface, context); + .processFileCoordinates(similarImageInterface, uploadableFile.getFilePath()); UploadItem uploadItem = new UploadItem(uploadableFile.getContentUri(), Uri.parse(uploadableFile.getFilePath()), uploadableFile.getMimeType(context), source, gpsExtractor, place, fileCreatedDate, @@ -158,10 +129,6 @@ private UploadItem getUploadItem(UploadableFile uploadableFile, return uploadItem; } - int getCurrentStep() { - return currentStepIndex + 1; - } - int getStepCount() { return items.size() + 2; } @@ -244,6 +211,10 @@ public void updateUploadItem(int index, UploadItem uploadItem) { uploadItem1.setTitle(uploadItem.title); } + public void usePictureCoordinatesFrom(GPSExtractor originalPictureExtractor) { + fileProcessor.useImageCoords(originalPictureExtractor); + } + @SuppressWarnings("WeakerAccess") public static class UploadItem { @@ -253,13 +224,9 @@ public static class UploadItem { private final String source; private final GPSExtractor gpsCoords; - private boolean selected = false; - private boolean first = false; private Title title; private List descriptions; private Place place; - private boolean visited; - private boolean error; private long createdTimestamp; private String createdTimestampSource; private BehaviorSubject imageQuality; @@ -287,10 +254,6 @@ public String getCreatedTimestampSource() { return createdTimestampSource; } - public String getMimeType() { - return mimeType; - } - public String getSource() { return source; } @@ -299,26 +262,10 @@ public GPSExtractor getGpsCoords() { return gpsCoords; } - public boolean isSelected() { - return selected; - } - - public boolean isFirst() { - return first; - } - public List getDescriptions() { return descriptions; } - public boolean isVisited() { - return visited; - } - - public boolean isError() { - return error; - } - public long getCreatedTimestamp() { return createdTimestamp; } @@ -373,10 +320,9 @@ public boolean equals(@Nullable Object obj) { } - //Travis is complaining :P @Override public int hashCode() { - return super.hashCode(); + return mediaUri.hashCode(); } } 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 8b0b8c39f7..58ebc4f834 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 @@ -23,6 +23,7 @@ import com.github.chrisbanes.photoview.PhotoView; import com.jakewharton.rxbinding2.widget.RxTextView; +import fr.free.nrw.commons.upload.GPSExtractor; import org.apache.commons.lang3.StringUtils; import java.util.ArrayList; @@ -231,14 +232,6 @@ private void initRecyclerView() { rvDescriptions.setAdapter(descriptionsAdapter); } - /** - * returns the default locale value of the user's device - * @return - */ - private String getUserDefaultLocale() { - return getContext().getResources().getConfiguration().locale.getLanguage(); - } - /** * show dialog with info * @param titleStringID @@ -267,17 +260,20 @@ public void onButtonAddDescriptionClicked() { } @Override - public void showSimilarImageFragment(String originalFilePath, String possibleFilePath) { + public void showSimilarImageFragment(String originalFilePath, String possibleFilePath, + GPSExtractor originalPictureExtractor, GPSExtractor similarPictureExtractor) { SimilarImageDialogFragment newFragment = new SimilarImageDialogFragment(); newFragment.setCallback(new SimilarImageDialogFragment.Callback() { @Override public void onPositiveResponse() { Timber.d("positive response from similar image fragment"); + presenter.usePictureCoordinatesFrom(similarPictureExtractor); } @Override public void onNegativeResponse() { Timber.d("negative response from similar image fragment"); + presenter.usePictureCoordinatesFrom(originalPictureExtractor); } }); 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 280128999d..0b497375eb 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 @@ -1,5 +1,6 @@ package fr.free.nrw.commons.upload.mediaDetails; +import fr.free.nrw.commons.upload.GPSExtractor; import java.util.List; import fr.free.nrw.commons.BasePresenter; @@ -48,6 +49,9 @@ void receiveImage(UploadableFile uploadableFile, @Contribution.FileSource String void setUploadItem(int index, UploadItem uploadItem); void fetchPreviousTitleAndDescription(int indexInViewFlipper); + + void usePictureCoordinatesFrom(GPSExtractor gpsExtractor); + } } 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 a468ee15e3..5543251c25 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 @@ -159,7 +159,12 @@ public void fetchPreviousTitleAndDescription(int indexInViewFlipper) { } } - /** + @Override + public void usePictureCoordinatesFrom(GPSExtractor gpsExtractor) { + repository.usePictureCoordinatesFrom(gpsExtractor); + } + + /** * handles image quality verifications * * @param imageResult @@ -200,12 +205,16 @@ public void handleBadImage(Integer errorCode) { /** * notifies the user that a similar image exists - * * @param originalFilePath * @param possibleFilePath + * @param originalPictureExtractor + * @param similarPictureExtractor */ @Override - public void showSimilarImageFragment(String originalFilePath, String possibleFilePath) { - view.showSimilarImageFragment(originalFilePath, possibleFilePath); + public void showSimilarImageFragment(String originalFilePath, String possibleFilePath, + GPSExtractor originalPictureExtractor, GPSExtractor similarPictureExtractor) { + view.showSimilarImageFragment(originalFilePath, possibleFilePath, originalPictureExtractor, + similarPictureExtractor + ); } } 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 a16af59bbb..7dd8f5575d 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,5 +1,6 @@ package fr.free.nrw.commons.upload +import com.nhaarman.mockitokotlin2.mock import fr.free.nrw.commons.filepicker.UploadableFile import fr.free.nrw.commons.nearby.Place import fr.free.nrw.commons.repository.UploadRepository @@ -24,31 +25,32 @@ import org.mockito.MockitoAnnotations */ class UploadMediaPresenterTest { @Mock - internal var repository: UploadRepository? = null + internal lateinit var repository: UploadRepository + @Mock - internal var view: UploadMediaDetailsContract.View? = null + internal lateinit var view: UploadMediaDetailsContract.View - private var uploadMediaPresenter: UploadMediaPresenter? = null + private lateinit var uploadMediaPresenter: UploadMediaPresenter @Mock - private var uploadableFile: UploadableFile? = null + private lateinit var uploadableFile: UploadableFile @Mock - private var place: Place? = null + private lateinit var place: Place @Mock - private var uploadItem: UploadModel.UploadItem? = null + private lateinit var uploadItem: UploadModel.UploadItem @Mock - private var title: Title? = null + private lateinit var title: Title @Mock - private var descriptions: List? = null + private lateinit var descriptions: List - private var testObservableUploadItem: Observable? = null - private var testSingleImageResult: Single? = null + private lateinit var testObservableUploadItem: Observable + private lateinit var testSingleImageResult: Single - private var testScheduler: TestScheduler? = null + private lateinit var testScheduler: TestScheduler /** * initial setup unit test environment @@ -61,7 +63,7 @@ class UploadMediaPresenterTest { testSingleImageResult = Single.just(1) testScheduler = TestScheduler() uploadMediaPresenter = UploadMediaPresenter(repository, testScheduler, testScheduler) - uploadMediaPresenter?.onAttachView(view) + uploadMediaPresenter.onAttachView(view) } /** @@ -69,12 +71,22 @@ class UploadMediaPresenterTest { */ @Test fun receiveImageTest() { - Mockito.`when`(repository?.preProcessImage(ArgumentMatchers.any(UploadableFile::class.java), ArgumentMatchers.any(Place::class.java), ArgumentMatchers.anyString(), ArgumentMatchers.any(UploadMediaPresenter::class.java))).thenReturn(testObservableUploadItem) - uploadMediaPresenter?.receiveImage(uploadableFile, ArgumentMatchers.anyString(), place) - verify(view)?.showProgress(true) - testScheduler?.triggerActions() - verify(view)?.onImageProcessed(ArgumentMatchers.any(UploadModel.UploadItem::class.java), ArgumentMatchers.any(Place::class.java)) - verify(view)?.showProgress(false) + Mockito.`when`( + repository.preProcessImage( + ArgumentMatchers.any(UploadableFile::class.java), + ArgumentMatchers.any(Place::class.java), + ArgumentMatchers.anyString(), + ArgumentMatchers.any(UploadMediaPresenter::class.java) + ) + ).thenReturn(testObservableUploadItem) + uploadMediaPresenter.receiveImage(uploadableFile, ArgumentMatchers.anyString(), place) + verify(view).showProgress(true) + testScheduler.triggerActions() + verify(view).onImageProcessed( + ArgumentMatchers.any(UploadModel.UploadItem::class.java), + ArgumentMatchers.any(Place::class.java) + ) + verify(view).showProgress(false) } /** @@ -82,12 +94,13 @@ class UploadMediaPresenterTest { */ @Test fun verifyImageQualityTest() { - Mockito.`when`(repository?.getImageQuality(ArgumentMatchers.any(UploadModel.UploadItem::class.java))).thenReturn(testSingleImageResult) - Mockito.`when`(uploadItem?.imageQuality).thenReturn(ArgumentMatchers.anyInt()) - uploadMediaPresenter?.verifyImageQuality(uploadItem) - verify(view)?.showProgress(true) - testScheduler?.triggerActions() - verify(view)?.showProgress(false) + Mockito.`when`(repository.getImageQuality(ArgumentMatchers.any(UploadModel.UploadItem::class.java))) + .thenReturn(testSingleImageResult) + Mockito.`when`(uploadItem.imageQuality).thenReturn(ArgumentMatchers.anyInt()) + uploadMediaPresenter.verifyImageQuality(uploadItem) + verify(view).showProgress(true) + testScheduler.triggerActions() + verify(view).showProgress(false) } /** @@ -96,21 +109,21 @@ class UploadMediaPresenterTest { @Test fun handleImageResult() { //Positive case test - uploadMediaPresenter?.handleImageResult(IMAGE_KEEP) - verify(view)?.onImageValidationSuccess() + uploadMediaPresenter.handleImageResult(IMAGE_KEEP) + verify(view).onImageValidationSuccess() //Duplicate file name - uploadMediaPresenter?.handleImageResult(FILE_NAME_EXISTS) - verify(view)?.showDuplicatePicturePopup() + uploadMediaPresenter.handleImageResult(FILE_NAME_EXISTS) + verify(view).showDuplicatePicturePopup() //Empty Title test - uploadMediaPresenter?.handleImageResult(EMPTY_TITLE) - verify(view)?.showMessage(ArgumentMatchers.anyInt(), ArgumentMatchers.anyInt()) + uploadMediaPresenter.handleImageResult(EMPTY_TITLE) + verify(view).showMessage(ArgumentMatchers.anyInt(), ArgumentMatchers.anyInt()) //Bad Picture test //Empty Title test - uploadMediaPresenter?.handleImageResult(-7) - verify(view)?.showBadImagePopup(ArgumentMatchers.anyInt()) + uploadMediaPresenter.handleImageResult(-7) + verify(view).showBadImagePopup(ArgumentMatchers.anyInt()) } @@ -118,52 +131,54 @@ class UploadMediaPresenterTest { * Test fetch previous image title when there was one */ @Test - fun fetchPreviousImageAndTitleTestPositive(){ - Mockito.`when`(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()) - - uploadMediaPresenter?.fetchPreviousTitleAndDescription(0) - verify(view)?.setTitleAndDescription(ArgumentMatchers.anyString(),ArgumentMatchers.any()) + fun fetchPreviousImageAndTitleTestPositive() { + Mockito.`when`(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()) + + uploadMediaPresenter.fetchPreviousTitleAndDescription(0) + verify(view).setTitleAndDescription(ArgumentMatchers.anyString(), ArgumentMatchers.any()) } /** * Test fetch previous image title when there was none */ @Test - fun fetchPreviousImageAndTitleTestNegative(){ - Mockito.`when`(repository?.getPreviousUploadItem(ArgumentMatchers.anyInt())).thenReturn(null) - uploadMediaPresenter?.fetchPreviousTitleAndDescription(0) - verify(view)?.showMessage(ArgumentMatchers.anyInt(),ArgumentMatchers.anyInt()) + fun fetchPreviousImageAndTitleTestNegative() { + Mockito.`when`(repository.getPreviousUploadItem(ArgumentMatchers.anyInt())) + .thenReturn(null) + uploadMediaPresenter.fetchPreviousTitleAndDescription(0) + verify(view).showMessage(ArgumentMatchers.anyInt(), ArgumentMatchers.anyInt()) } /** * Test bad image invalid location */ @Test - fun handleBadImageBaseTestInvalidLocation(){ - uploadMediaPresenter?.handleBadImage(8) - verify(repository)?.saveValue(ArgumentMatchers.anyString(),eq(false)) - verify(view)?.showBadImagePopup(8) + fun handleBadImageBaseTestInvalidLocation() { + uploadMediaPresenter.handleBadImage(8) + verify(repository)?.saveValue(ArgumentMatchers.anyString(), eq(false)) + verify(view).showBadImagePopup(8) } /** * Test bad image empty title */ @Test - fun handleBadImageBaseTestEmptyTitle(){ - uploadMediaPresenter?.handleBadImage(-3) - verify(view)?.showMessage(ArgumentMatchers.anyInt(),ArgumentMatchers.anyInt()) + fun handleBadImageBaseTestEmptyTitle() { + uploadMediaPresenter.handleBadImage(-3) + verify(view).showMessage(ArgumentMatchers.anyInt(), ArgumentMatchers.anyInt()) } /** * Teste show file already exists */ @Test - fun handleBadImageBaseTestFileNameExists(){ - uploadMediaPresenter?.handleBadImage(-4) - verify(view)?.showDuplicatePicturePopup() + fun handleBadImageBaseTestFileNameExists() { + uploadMediaPresenter.handleBadImage(-4) + verify(view).showDuplicatePicturePopup() } @@ -171,18 +186,20 @@ class UploadMediaPresenterTest { * Test show SimilarImageFragment */ @Test - fun showSimilarImageFragmentTest(){ - uploadMediaPresenter?.showSimilarImageFragment(ArgumentMatchers.anyString(),ArgumentMatchers.anyString()) - verify(view)?.showSimilarImageFragment(ArgumentMatchers.anyString(),ArgumentMatchers.anyString()) + fun showSimilarImageFragmentTest() { + val original: GPSExtractor = mock() + val similar: GPSExtractor = mock() + uploadMediaPresenter.showSimilarImageFragment("original", "possible", original, similar) + verify(view).showSimilarImageFragment("original", "possible", original, similar) } /** * Test set upload item */ @Test - fun setUploadItemTest(){ - uploadMediaPresenter?.setUploadItem(0,uploadItem) - verify(repository)?.updateUploadItem(0,uploadItem) + fun setUploadItemTest() { + uploadMediaPresenter.setUploadItem(0, uploadItem) + verify(repository)?.updateUploadItem(0, uploadItem) } } 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 deleted file mode 100644 index 764c0a447f..0000000000 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadModelTest.kt +++ /dev/null @@ -1,132 +0,0 @@ -package fr.free.nrw.commons.upload - -import android.app.Application -import android.content.Context -import fr.free.nrw.commons.auth.SessionManager -import fr.free.nrw.commons.filepicker.UploadableFile -import fr.free.nrw.commons.kvstore.JsonKvStore -import fr.free.nrw.commons.nearby.Place -import fr.free.nrw.commons.utils.ImageUtils.IMAGE_OK -import io.reactivex.Single -import org.junit.After -import org.junit.Assert.assertTrue -import org.junit.Before -import org.junit.Test -import org.mockito.ArgumentMatchers.* -import org.mockito.InjectMocks -import org.mockito.Mock -import org.mockito.Mockito.`when` -import org.mockito.Mockito.mock -import org.mockito.MockitoAnnotations -import java.io.FileInputStream -import java.io.InputStream -import java.util.* -import javax.inject.Inject -import javax.inject.Named - - -class UploadModelTest { - - @Mock - @field:[Inject Named("licenses")] - internal var licenses: List? = null - @Mock - @field:[Inject Named("default_preferences")] - internal var prefs: JsonKvStore? = null - @Mock - @field:[Inject Named("licenses_by_name")] - internal var licensesByName: Map? = null - @Mock - internal var context: Context? = null - @Mock - internal var sessionManage: SessionManager? = null - @Mock - internal var fileUtilsWrapper: FileUtilsWrapper? = null - @Mock - internal var fileProcessor: FileProcessor? = null - @Mock - internal var imageProcessingService: ImageProcessingService? = null - - @InjectMocks - var uploadModel: UploadModel? = null - - @Before - @Throws(Exception::class) - fun setUp() { - MockitoAnnotations.initMocks(this) - - `when`(context!!.applicationContext) - .thenReturn(mock(Application::class.java)) - `when`(fileUtilsWrapper!!.getFileExt(anyString())) - .thenReturn("jpg") - `when`(fileUtilsWrapper!!.getSHA1(any(InputStream::class.java))) - .thenReturn("sha") - `when`(fileUtilsWrapper!!.getFileInputStream(anyString())) - .thenReturn(mock(FileInputStream::class.java)) - `when`(fileUtilsWrapper!!.getGeolocationOfFile(anyString())) - .thenReturn("") - `when`(imageProcessingService!!.validateImage(any(UploadModel.UploadItem::class.java))) - .thenReturn(Single.just(IMAGE_OK)) - - } - - @After - @Throws(Exception::class) - fun tearDown() { - } - - @Test - fun receive() { - val preProcessImages = uploadModel!!.preProcessImages(getMediaList(), mock(Place::class.java), "external") { _, _ -> } - preProcessImages.doOnComplete { - assertTrue(uploadModel!!.items.size == 2) - } - } - - @Test - fun getCurrentStep() { - uploadModel!!.preProcessImages(getMediaList(), mock(Place::class.java), "external") { _, _ -> } - assertTrue(uploadModel!!.currentStep == 1) - } - - @Test - fun getStepCount() { - val preProcessImages = uploadModel!!.preProcessImages(getMediaList(), mock(Place::class.java), "external") { _, _ -> } - preProcessImages.doOnComplete { - assertTrue(uploadModel!!.stepCount == 4) - } - } - - @Test - fun getCount() { - val preProcessImages = uploadModel!!.preProcessImages(getMediaList(), mock(Place::class.java), "external") { _, _ -> } - preProcessImages.doOnComplete { - assertTrue(uploadModel!!.count == 2) - } - } - - @Test - fun getUploads() { - val preProcessImages = uploadModel!!.preProcessImages(getMediaList(), mock(Place::class.java), "external") { _, _ -> } - preProcessImages.doOnComplete { - assertTrue(uploadModel!!.uploads.size == 2) - } - } - - private fun getMediaList(): List { - val element = getElement() - val element2 = getElement() - var uriList: List = mutableListOf(element, element2) - return uriList - } - - private fun getElement(): UploadableFile { - val mock = mock(UploadableFile::class.java) - `when`(mock.filePath).thenReturn(UUID.randomUUID().toString() + "/filePath.jpg") - return mock - } - - @Test - fun buildContributions() { - } -} From e8c1cd1a24de97d02049720be9b7ebcab517198a Mon Sep 17 00:00:00 2001 From: Sean Mac Gillicuddy Date: Thu, 19 Mar 2020 10:27:50 +0000 Subject: [PATCH 2/8] #3408 Refactoring the FileProcessor and GPSExtractor classes - refactor and rename GpsExtractor --- .../repository/UploadRemoteDataSource.java | 6 +- .../commons/repository/UploadRepository.java | 6 +- .../nrw/commons/upload/FileProcessor.java | 42 ++--- .../fr/free/nrw/commons/upload/FileUtils.java | 6 +- .../free/nrw/commons/upload/GPSExtractor.java | 152 ------------------ .../nrw/commons/upload/ImageCoordinates.java | 92 +++++++++++ .../commons/upload/SimilarImageInterface.java | 2 +- .../free/nrw/commons/upload/UploadModel.java | 14 +- .../UploadMediaDetailFragment.java | 4 +- .../UploadMediaDetailsContract.java | 4 +- .../mediaDetails/UploadMediaPresenter.java | 10 +- .../upload/UploadMediaPresenterTest.kt | 4 +- 12 files changed, 144 insertions(+), 198 deletions(-) delete mode 100644 app/src/main/java/fr/free/nrw/commons/upload/GPSExtractor.java create mode 100644 app/src/main/java/fr/free/nrw/commons/upload/ImageCoordinates.java 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 3930b7ba86..35eca57b03 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 @@ -1,6 +1,6 @@ package fr.free.nrw.commons.repository; -import fr.free.nrw.commons.upload.GPSExtractor; +import fr.free.nrw.commons.upload.ImageCoordinates; import java.io.IOException; import java.util.Comparator; import java.util.List; @@ -204,7 +204,7 @@ public Place getNearbyPlaces(double latitude, double longitude) { } } - public void usePictureCoordinatesFrom(GPSExtractor gpsExtractor) { - uploadModel.usePictureCoordinatesFrom(gpsExtractor); + public void usePictureCoordinatesFrom(ImageCoordinates imageCoordinates) { + uploadModel.usePictureCoordinatesFrom(imageCoordinates); } } 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 99af4e31b4..4b0502f83c 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 @@ -1,6 +1,6 @@ package fr.free.nrw.commons.repository; -import fr.free.nrw.commons.upload.GPSExtractor; +import fr.free.nrw.commons.upload.ImageCoordinates; import java.util.Comparator; import java.util.List; @@ -273,7 +273,7 @@ public Place checkNearbyPlaces(double decLatitude, double decLongitude) { return remoteDataSource.getNearbyPlaces(decLatitude, decLongitude); } - public void usePictureCoordinatesFrom(GPSExtractor gpsExtractor) { - remoteDataSource.usePictureCoordinatesFrom(gpsExtractor); + public void usePictureCoordinatesFrom(ImageCoordinates imageCoordinates) { + remoteDataSource.usePictureCoordinatesFrom(imageCoordinates); } } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.java b/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.java index 629a7dd6bc..28fe666111 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.java @@ -57,7 +57,7 @@ public void cleanup() { /** * Processes filePath coordinates, either from EXIF data or user location */ - GPSExtractor processFileCoordinates(SimilarImageInterface similarImageInterface, + ImageCoordinates processFileCoordinates(SimilarImageInterface similarImageInterface, String filePath) { ExifInterface exifInterface = null; try { @@ -69,8 +69,8 @@ GPSExtractor processFileCoordinates(SimilarImageInterface similarImageInterface, redactExifTags(exifInterface, getExifTagsToRedact()); Timber.d("Calling GPSExtractor"); - GPSExtractor originalImageExtractor = new GPSExtractor(exifInterface); - String decimalCoords = originalImageExtractor.getCoords(); + ImageCoordinates originalImageExtractor = new ImageCoordinates(exifInterface); + String decimalCoords = originalImageExtractor.getDecimalCoords(); if (decimalCoords == null || !originalImageExtractor.imageCoordsExists) { //Find other photos taken around the same time which has gps coordinates findOtherImages(originalImageExtractor, new File(filePath), @@ -143,7 +143,7 @@ private void redactTag(ExifInterface exifInterface, String tag) { * @param similarImageInterface */ private void findOtherImages( - GPSExtractor originalImageExtractor, + ImageCoordinates originalImageExtractor, File fileBeingProcessed, SimilarImageInterface similarImageInterface) { //Time when the original image was created @@ -156,10 +156,10 @@ private void findOtherImages( && file.lastModified() - timeOfCreation >= -(120 * 1000)) { //Make sure the photos were taken within 20seconds Timber.d("fild date:" + file.lastModified() + " time of creation" + timeOfCreation); - GPSExtractor similarPictureExtractor = createGpsExtractor(file); + ImageCoordinates similarPictureExtractor = createGpsExtractor(file); Timber.d("not null fild EXIF" + similarPictureExtractor.imageCoordsExists + " coords" - + similarPictureExtractor.getCoords()); - if (similarPictureExtractor.getCoords() != null + + similarPictureExtractor.getDecimalCoords()); + if (similarPictureExtractor.getDecimalCoords() != null && similarPictureExtractor.imageCoordsExists) { // Current image has gps coordinates and it's not current gps locaiton Timber.d("This filePath has image coords:" + file.getAbsolutePath()); @@ -176,12 +176,17 @@ private void findOtherImages( } @NotNull - private GPSExtractor createGpsExtractor(File file) { + private ImageCoordinates createGpsExtractor(File file) { try { - return new GPSExtractor(contentResolver.openInputStream(Uri.fromFile(file))); + return new ImageCoordinates(contentResolver.openInputStream(Uri.fromFile(file))); } catch (IOException e) { - e.printStackTrace(); - return new GPSExtractor(file.getAbsolutePath()); + Timber.e(e); + try { + return new ImageCoordinates(file.getAbsolutePath()); + } catch (IOException ex) { + Timber.e(ex); + return null; + } } } @@ -189,22 +194,23 @@ private GPSExtractor createGpsExtractor(File file) { * Initiates retrieval of image coordinates or user coordinates, and caching of coordinates. Then * initiates the calls to MediaWiki API through an instance of CategoryApi. * - * @param gpsExtractor + * @param imageCoordinates */ @SuppressLint("CheckResult") - public void useImageCoords(GPSExtractor gpsExtractor) { - useImageCoords(gpsExtractor, gpsExtractor.getCoords()); + public void useImageCoords(ImageCoordinates imageCoordinates) { + useImageCoords(imageCoordinates, imageCoordinates.getDecimalCoords()); } - private void useImageCoords(GPSExtractor gpsExtractor, String decimalCoords) { + private void useImageCoords(ImageCoordinates imageCoordinates, String decimalCoords) { if (decimalCoords != null) { Timber.d("Decimal coords of image: %s", decimalCoords); - Timber.d("is EXIF data present:" + gpsExtractor.imageCoordsExists + + Timber.d("is EXIF data present:" + imageCoordinates.imageCoordsExists + " from findOther image"); // Only set cache for this point if image has coords - if (gpsExtractor.imageCoordsExists) { - cacheController.setQtPoint(gpsExtractor.getDecLongitude(), gpsExtractor.getDecLatitude()); + if (imageCoordinates.imageCoordsExists) { + cacheController + .setQtPoint(imageCoordinates.getDecLongitude(), imageCoordinates.getDecLatitude()); } List displayCatList = cacheController.findCategory(); diff --git a/app/src/main/java/fr/free/nrw/commons/upload/FileUtils.java b/app/src/main/java/fr/free/nrw/commons/upload/FileUtils.java index 72158f2213..098c032f31 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/FileUtils.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/FileUtils.java @@ -68,9 +68,9 @@ static String getGeolocationOfFile(String filePath) { try { ExifInterface exifInterface = new ExifInterface(filePath); - GPSExtractor imageObj = new GPSExtractor(exifInterface); + ImageCoordinates imageObj = new ImageCoordinates(exifInterface); if (imageObj.imageCoordsExists) { // If image has geolocation information in its EXIF - return imageObj.getCoords(); + return imageObj.getDecimalCoords(); } else { return ""; } @@ -178,4 +178,4 @@ public static boolean fileExists(Uri localUri) { return false; } } -} \ No newline at end of file +} diff --git a/app/src/main/java/fr/free/nrw/commons/upload/GPSExtractor.java b/app/src/main/java/fr/free/nrw/commons/upload/GPSExtractor.java deleted file mode 100644 index f602f4ff7c..0000000000 --- a/app/src/main/java/fr/free/nrw/commons/upload/GPSExtractor.java +++ /dev/null @@ -1,152 +0,0 @@ -package fr.free.nrw.commons.upload; - -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; -import androidx.exifinterface.media.ExifInterface; - -import java.io.IOException; -import java.io.InputStream; - -import timber.log.Timber; - -/** - * Extracts geolocation to be passed to API for category suggestions. If a picture with geolocation - * is uploaded, extract latitude and longitude from EXIF data of image. - */ -public class GPSExtractor { - - static final GPSExtractor DUMMY= new GPSExtractor(); - private double decLatitude; - private double decLongitude; - public boolean imageCoordsExists; - private String latitude; - private String longitude; - private String latitudeRef; - private String longitudeRef; - private String decimalCoords; - - /** - * Dummy constructor. - */ - private GPSExtractor(){ - - } - /** - * Construct from a stream. - */ - GPSExtractor(@NonNull InputStream stream) throws IOException { - ExifInterface exif = new ExifInterface(stream); - processCoords(exif); - } - - /** - * Construct from the file path of the image. - * @param path file path of the image - * - */ - GPSExtractor(@NonNull String path) { - try { - ExifInterface exif = new ExifInterface(path); - processCoords(exif); - } catch (IOException | IllegalArgumentException e) { - Timber.w(e); - } - } - - /** - * Construct from the file path of the image. - * @param exif exif interface of the image - * - */ - GPSExtractor(@NonNull ExifInterface exif){ - processCoords(exif); - } - - private void processCoords(ExifInterface exif){ - //If image has no EXIF data and user has enabled GPS setting, get user's location - //Always return null as a temporary fix for #1599 - if (exif != null && exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE) != null) { - //If image has EXIF data, extract image coords - imageCoordsExists = true; - Timber.d("EXIF data has location info"); - - latitude = exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE); - latitudeRef = exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE_REF); - longitude = exif.getAttribute(ExifInterface.TAG_GPS_LONGITUDE); - longitudeRef = exif.getAttribute(ExifInterface.TAG_GPS_LONGITUDE_REF); - } - } - - /** - * Extracts geolocation (either of image from EXIF data, or of user) - * @return coordinates as string (needs to be passed as a String in API query) - */ - @Nullable - String getCoords() { - if(decimalCoords!=null){ - return decimalCoords; - }else if (latitude!=null && latitudeRef!=null && longitude!=null && longitudeRef!=null) { - Timber.d("Latitude: %s %s", latitude, latitudeRef); - Timber.d("Longitude: %s %s", longitude, longitudeRef); - - decimalCoords = getDecimalCoords(latitude, latitudeRef, longitude, longitudeRef); - return decimalCoords; - } else { - return null; - } - } - - public double getDecLatitude() { - return decLatitude; - } - - public double getDecLongitude() { - return decLongitude; - } - - /** - * Converts format of geolocation into decimal coordinates as required by MediaWiki API - * @return the coordinates in decimals - */ - private String getDecimalCoords(String latitude, String latitude_ref, String longitude, String longitude_ref) { - - if (latitude_ref.equals("N")) { - decLatitude = convertToDegree(latitude); - } else { - decLatitude = 0 - convertToDegree(latitude); - } - - if (longitude_ref.equals("E")) { - decLongitude = convertToDegree(longitude); - } else { - decLongitude = 0 - convertToDegree(longitude); - } - - String decimalCoords = decLatitude + "|" + decLongitude; - Timber.d("Latitude and Longitude are %s", decimalCoords); - return decimalCoords; - } - - private double convertToDegree(String stringDMS) { - double result; - String[] DMS = stringDMS.split(",", 3); - - String[] stringD = DMS[0].split("/", 2); - double d0 = Double.parseDouble(stringD[0]); - double d1 = Double.parseDouble(stringD[1]); - double degrees = d0/d1; - - String[] stringM = DMS[1].split("/", 2); - double m0 = Double.parseDouble(stringM[0]); - double m1 = Double.parseDouble(stringM[1]); - double minutes = m0/m1; - - String[] stringS = DMS[2].split("/", 2); - double s0 = Double.parseDouble(stringS[0]); - double s1 = Double.parseDouble(stringS[1]); - double seconds = s0/s1; - - result = degrees + (minutes/60) + (seconds/3600); - return result; - } -} diff --git a/app/src/main/java/fr/free/nrw/commons/upload/ImageCoordinates.java b/app/src/main/java/fr/free/nrw/commons/upload/ImageCoordinates.java new file mode 100644 index 0000000000..64c418f19c --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/upload/ImageCoordinates.java @@ -0,0 +1,92 @@ +package fr.free.nrw.commons.upload; + +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import androidx.exifinterface.media.ExifInterface; +import java.io.IOException; +import java.io.InputStream; +import timber.log.Timber; + +/** + * Extracts geolocation to be passed to API for category suggestions. If a picture with geolocation + * is uploaded, extract latitude and longitude from EXIF data of image. + */ +public class ImageCoordinates { + + private double decLatitude; + private double decLongitude; + public boolean imageCoordsExists; + private String decimalCoords; + + /** + * Construct from the file path of the image. + * @param exif exif interface of the image + * + */ + ImageCoordinates(ExifInterface exif){ + //If image has no EXIF data and user has enabled GPS setting, get user's location + //Always return null as a temporary fix for #1599 + if (exif != null && exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE) != null) { + //If image has EXIF data, extract image coords + imageCoordsExists = true; + Timber.d("EXIF data has location info"); + + String latitude = exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE); + String latitudeRef = exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE_REF); + String longitude = exif.getAttribute(ExifInterface.TAG_GPS_LONGITUDE); + String longitudeRef = exif.getAttribute(ExifInterface.TAG_GPS_LONGITUDE_REF); + decLatitude = "N".equals(latitudeRef) ? convertToDegree(latitude) : + 0 - convertToDegree(latitude); + decLongitude = "E".equals(longitudeRef) ? convertToDegree(longitude) + : 0 - convertToDegree(longitude); + + decimalCoords = decLatitude + "|" + decLongitude; + } + } + + /** + * Construct from a stream. + */ + ImageCoordinates(@NonNull InputStream stream) throws IOException { + this(new ExifInterface(stream)); + } + + /** + * Construct from the file path of the image. + * @param path file path of the image + * + */ + ImageCoordinates(@NonNull String path) throws IOException{ + this(new ExifInterface(path)); + } + + /** + * Extracts geolocation (either of image from EXIF data, or of user) + * @return coordinates as string (needs to be passed as a String in API query) + */ + @Nullable + String getDecimalCoords() { + return decimalCoords; + } + + public double getDecLatitude() { + return decLatitude; + } + + public double getDecLongitude() { + return decLongitude; + } + + private double convertToDegree(String stringDMS) { + String[] DMS = stringDMS.split(",", 3); + double degrees = divideComponents(DMS[0]); + double minutes = divideComponents(DMS[1]); + double seconds = divideComponents(DMS[2]); + return degrees + (minutes/60) + (seconds/3600); + } + + private double divideComponents(String dm) { + String[] stringD = dm.split("/", 2); + return Double.parseDouble(stringD[0]) / Double.parseDouble(stringD[1]); + } +} 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 08bf299f1f..200899f42c 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, - GPSExtractor originalPictureExtractor, GPSExtractor similarPictureExtractor); + ImageCoordinates originalPictureExtractor, ImageCoordinates similarPictureExtractor); } 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 2b7b5e470b..5cc7ab369a 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 @@ -109,11 +109,11 @@ private UploadItem getUploadItem(UploadableFile uploadableFile, createdTimestampSource = dateTimeWithSource.getSource(); } Timber.d("File created date is %d", fileCreatedDate); - GPSExtractor gpsExtractor = fileProcessor + ImageCoordinates imageCoordinates = fileProcessor .processFileCoordinates(similarImageInterface, uploadableFile.getFilePath()); UploadItem uploadItem = new UploadItem(uploadableFile.getContentUri(), Uri.parse(uploadableFile.getFilePath()), - uploadableFile.getMimeType(context), source, gpsExtractor, place, fileCreatedDate, + uploadableFile.getMimeType(context), source, imageCoordinates, place, fileCreatedDate, createdTimestampSource); if (place != null) { uploadItem.title.setTitleText(place.name); @@ -161,7 +161,7 @@ public Observable buildContributions() { item.getFileName(), Description.formatList(item.descriptions), -1, null, null, sessionManager.getAuthorName(), - CommonsApplication.DEFAULT_EDIT_SUMMARY, item.gpsCoords.getCoords()); + CommonsApplication.DEFAULT_EDIT_SUMMARY, item.gpsCoords.getDecimalCoords()); if (item.place != null) { contribution.setWikiDataEntityId(item.place.getWikiDataEntityId()); // If item already has an image, we need to know it. We don't want to override existing image later @@ -211,7 +211,7 @@ public void updateUploadItem(int index, UploadItem uploadItem) { uploadItem1.setTitle(uploadItem.title); } - public void usePictureCoordinatesFrom(GPSExtractor originalPictureExtractor) { + public void usePictureCoordinatesFrom(ImageCoordinates originalPictureExtractor) { fileProcessor.useImageCoords(originalPictureExtractor); } @@ -222,7 +222,7 @@ public static class UploadItem { private final Uri mediaUri; private final String mimeType; private final String source; - private final GPSExtractor gpsCoords; + private final ImageCoordinates gpsCoords; private Title title; private List descriptions; @@ -233,7 +233,7 @@ public static class UploadItem { @SuppressLint("CheckResult") UploadItem(Uri originalContentUri, - Uri mediaUri, String mimeType, String source, GPSExtractor gpsCoords, + Uri mediaUri, String mimeType, String source, ImageCoordinates gpsCoords, Place place, long createdTimestamp, String createdTimestampSource) { @@ -258,7 +258,7 @@ public String getSource() { return source; } - public GPSExtractor getGpsCoords() { + public ImageCoordinates getGpsCoords() { return gpsCoords; } 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 58ebc4f834..b69567924d 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 @@ -23,7 +23,7 @@ import com.github.chrisbanes.photoview.PhotoView; import com.jakewharton.rxbinding2.widget.RxTextView; -import fr.free.nrw.commons.upload.GPSExtractor; +import fr.free.nrw.commons.upload.ImageCoordinates; import org.apache.commons.lang3.StringUtils; import java.util.ArrayList; @@ -261,7 +261,7 @@ public void onButtonAddDescriptionClicked() { @Override public void showSimilarImageFragment(String originalFilePath, String possibleFilePath, - GPSExtractor originalPictureExtractor, GPSExtractor similarPictureExtractor) { + ImageCoordinates originalPictureExtractor, ImageCoordinates similarPictureExtractor) { SimilarImageDialogFragment newFragment = new SimilarImageDialogFragment(); newFragment.setCallback(new SimilarImageDialogFragment.Callback() { @Override 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 0b497375eb..518d91de95 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 @@ -1,6 +1,6 @@ package fr.free.nrw.commons.upload.mediaDetails; -import fr.free.nrw.commons.upload.GPSExtractor; +import fr.free.nrw.commons.upload.ImageCoordinates; import java.util.List; import fr.free.nrw.commons.BasePresenter; @@ -50,7 +50,7 @@ void receiveImage(UploadableFile uploadableFile, @Contribution.FileSource String void fetchPreviousTitleAndDescription(int indexInViewFlipper); - void usePictureCoordinatesFrom(GPSExtractor gpsExtractor); + void usePictureCoordinatesFrom(ImageCoordinates imageCoordinates); } 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 5543251c25..cd97074eef 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 @@ -9,7 +9,7 @@ import fr.free.nrw.commons.filepicker.UploadableFile; import fr.free.nrw.commons.nearby.Place; import fr.free.nrw.commons.repository.UploadRepository; -import fr.free.nrw.commons.upload.GPSExtractor; +import fr.free.nrw.commons.upload.ImageCoordinates; import fr.free.nrw.commons.upload.SimilarImageInterface; import fr.free.nrw.commons.upload.UploadModel.UploadItem; import fr.free.nrw.commons.upload.mediaDetails.UploadMediaDetailsContract.UserActionListener; @@ -81,7 +81,7 @@ public void receiveImage(UploadableFile uploadableFile, String source, Place pla .subscribe(uploadItem -> { view.onImageProcessed(uploadItem, place); - GPSExtractor gpsCoords = uploadItem.getGpsCoords(); + ImageCoordinates gpsCoords = uploadItem.getGpsCoords(); view.showMapWithImageCoordinates(gpsCoords != null && gpsCoords.imageCoordsExists); view.showProgress(false); if (gpsCoords != null && gpsCoords.imageCoordsExists) { @@ -160,8 +160,8 @@ public void fetchPreviousTitleAndDescription(int indexInViewFlipper) { } @Override - public void usePictureCoordinatesFrom(GPSExtractor gpsExtractor) { - repository.usePictureCoordinatesFrom(gpsExtractor); + public void usePictureCoordinatesFrom(ImageCoordinates imageCoordinates) { + repository.usePictureCoordinatesFrom(imageCoordinates); } /** @@ -212,7 +212,7 @@ public void handleBadImage(Integer errorCode) { */ @Override public void showSimilarImageFragment(String originalFilePath, String possibleFilePath, - GPSExtractor originalPictureExtractor, GPSExtractor similarPictureExtractor) { + ImageCoordinates originalPictureExtractor, ImageCoordinates similarPictureExtractor) { view.showSimilarImageFragment(originalFilePath, possibleFilePath, originalPictureExtractor, similarPictureExtractor ); 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 7dd8f5575d..accd4d0045 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 @@ -187,8 +187,8 @@ class UploadMediaPresenterTest { */ @Test fun showSimilarImageFragmentTest() { - val original: GPSExtractor = mock() - val similar: GPSExtractor = mock() + val original: ImageCoordinates = mock() + val similar: ImageCoordinates = mock() uploadMediaPresenter.showSimilarImageFragment("original", "possible", original, similar) verify(view).showSimilarImageFragment("original", "possible", original, similar) } From 297f63756bb849eb8747922358084d756531500a Mon Sep 17 00:00:00 2001 From: Sean Mac Gillicuddy Date: Thu, 19 Mar 2020 10:55:53 +0000 Subject: [PATCH 3/8] #3408 Refactoring the FileProcessor and GPSExtractor classes - convert ImageCoordinates to kotlin --- .../nrw/commons/upload/ImageCoordinates.java | 92 ------------------- .../nrw/commons/upload/ImageCoordinates.kt | 72 +++++++++++++++ 2 files changed, 72 insertions(+), 92 deletions(-) delete mode 100644 app/src/main/java/fr/free/nrw/commons/upload/ImageCoordinates.java create mode 100644 app/src/main/java/fr/free/nrw/commons/upload/ImageCoordinates.kt diff --git a/app/src/main/java/fr/free/nrw/commons/upload/ImageCoordinates.java b/app/src/main/java/fr/free/nrw/commons/upload/ImageCoordinates.java deleted file mode 100644 index 64c418f19c..0000000000 --- a/app/src/main/java/fr/free/nrw/commons/upload/ImageCoordinates.java +++ /dev/null @@ -1,92 +0,0 @@ -package fr.free.nrw.commons.upload; - -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; -import androidx.exifinterface.media.ExifInterface; -import java.io.IOException; -import java.io.InputStream; -import timber.log.Timber; - -/** - * Extracts geolocation to be passed to API for category suggestions. If a picture with geolocation - * is uploaded, extract latitude and longitude from EXIF data of image. - */ -public class ImageCoordinates { - - private double decLatitude; - private double decLongitude; - public boolean imageCoordsExists; - private String decimalCoords; - - /** - * Construct from the file path of the image. - * @param exif exif interface of the image - * - */ - ImageCoordinates(ExifInterface exif){ - //If image has no EXIF data and user has enabled GPS setting, get user's location - //Always return null as a temporary fix for #1599 - if (exif != null && exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE) != null) { - //If image has EXIF data, extract image coords - imageCoordsExists = true; - Timber.d("EXIF data has location info"); - - String latitude = exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE); - String latitudeRef = exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE_REF); - String longitude = exif.getAttribute(ExifInterface.TAG_GPS_LONGITUDE); - String longitudeRef = exif.getAttribute(ExifInterface.TAG_GPS_LONGITUDE_REF); - decLatitude = "N".equals(latitudeRef) ? convertToDegree(latitude) : - 0 - convertToDegree(latitude); - decLongitude = "E".equals(longitudeRef) ? convertToDegree(longitude) - : 0 - convertToDegree(longitude); - - decimalCoords = decLatitude + "|" + decLongitude; - } - } - - /** - * Construct from a stream. - */ - ImageCoordinates(@NonNull InputStream stream) throws IOException { - this(new ExifInterface(stream)); - } - - /** - * Construct from the file path of the image. - * @param path file path of the image - * - */ - ImageCoordinates(@NonNull String path) throws IOException{ - this(new ExifInterface(path)); - } - - /** - * Extracts geolocation (either of image from EXIF data, or of user) - * @return coordinates as string (needs to be passed as a String in API query) - */ - @Nullable - String getDecimalCoords() { - return decimalCoords; - } - - public double getDecLatitude() { - return decLatitude; - } - - public double getDecLongitude() { - return decLongitude; - } - - private double convertToDegree(String stringDMS) { - String[] DMS = stringDMS.split(",", 3); - double degrees = divideComponents(DMS[0]); - double minutes = divideComponents(DMS[1]); - double seconds = divideComponents(DMS[2]); - return degrees + (minutes/60) + (seconds/3600); - } - - private double divideComponents(String dm) { - String[] stringD = dm.split("/", 2); - return Double.parseDouble(stringD[0]) / Double.parseDouble(stringD[1]); - } -} diff --git a/app/src/main/java/fr/free/nrw/commons/upload/ImageCoordinates.kt b/app/src/main/java/fr/free/nrw/commons/upload/ImageCoordinates.kt new file mode 100644 index 0000000000..7fb6ce5775 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/upload/ImageCoordinates.kt @@ -0,0 +1,72 @@ +package fr.free.nrw.commons.upload + +import androidx.exifinterface.media.ExifInterface +import timber.log.Timber +import java.io.InputStream + +/** + * Extracts geolocation to be passed to API for category suggestions. If a picture with geolocation + * is uploaded, extract latitude and longitude from EXIF data of image. + */ +class ImageCoordinates internal constructor(exif: ExifInterface?) { + var decLatitude = 0.0 + var decLongitude = 0.0 + var imageCoordsExists = false + /** + * @return string of `"[decLatitude]|[decLongitude]"` or null if coordinates do not exist + */ + var decimalCoords: String? = null + + /** + * Construct from a stream. + */ + internal constructor(stream: InputStream) : this(ExifInterface(stream)) + + /** + * Construct from the file path of the image. + * @param path file path of the image + */ + internal constructor(path: String) : this(ExifInterface(path)) + + + + init { + //If image has no EXIF data and user has enabled GPS setting, get user's location + //Always return null as a temporary fix for #1599 + if (exif != null) { + val latitude = exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE) + val latitudeRef = exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE_REF) + val longitude = exif.getAttribute(ExifInterface.TAG_GPS_LONGITUDE) + val longitudeRef = exif.getAttribute(ExifInterface.TAG_GPS_LONGITUDE_REF) + if (latitude != null && longitude != null && latitudeRef != null && longitudeRef != null) { + //If image has EXIF data, extract image coords + imageCoordsExists = true + Timber.d("EXIF data has location info") + decLatitude = + if (ExifInterface.LATITUDE_NORTH == latitudeRef) convertToDegree(latitude) + else 0 - convertToDegree(latitude) + decLongitude = + if (ExifInterface.LONGITUDE_EAST == longitudeRef) convertToDegree(longitude) + else 0 - convertToDegree(longitude) + decimalCoords = "$decLatitude|$decLongitude" + } + } + } + + /** + * Convert a string to an accurate Degree + * + * @param degreeMinuteSecondString - template string "a/b,c/d,e/f" where the letters represent numbers + * @return the degree accurate to the second + */ + private fun convertToDegree(degreeMinuteSecondString: String) = + degreeMinuteSecondString.split(",").let { + val degrees = evaluateExpression(it[0]) + val minutes = evaluateExpression(it[1]) + val seconds = evaluateExpression(it[2]) + degrees + minutes / 60 + seconds / 3600 + } + + private fun evaluateExpression(dm: String) = + dm.split("/").let { it[0].toDouble() / it[1].toDouble() } +} From 4a287d3d9fd2d2f1788ebbe88a147e9834fac260 Mon Sep 17 00:00:00 2001 From: Sean Mac Gillicuddy Date: Thu, 19 Mar 2020 12:03:49 +0000 Subject: [PATCH 4/8] #3408 Refactoring the FileProcessor and GPSExtractor classes - convert FileProcessor to kotlin --- .../nrw/commons/upload/FileProcessor.java | 240 ------------------ .../free/nrw/commons/upload/FileProcessor.kt | 198 +++++++++++++++ .../nrw/commons/upload/ImageCoordinates.kt | 2 + 3 files changed, 200 insertions(+), 240 deletions(-) delete mode 100644 app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.java create mode 100644 app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.kt diff --git a/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.java b/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.java deleted file mode 100644 index 28fe666111..0000000000 --- a/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.java +++ /dev/null @@ -1,240 +0,0 @@ -package fr.free.nrw.commons.upload; - -import android.annotation.SuppressLint; -import android.content.ContentResolver; -import android.content.Context; -import android.net.Uri; -import androidx.exifinterface.media.ExifInterface; -import fr.free.nrw.commons.R; -import fr.free.nrw.commons.caching.CacheController; -import fr.free.nrw.commons.kvstore.JsonKvStore; -import fr.free.nrw.commons.mwapi.CategoryApi; -import fr.free.nrw.commons.settings.Prefs; -import io.reactivex.Observable; -import io.reactivex.disposables.CompositeDisposable; -import io.reactivex.schedulers.Schedulers; -import java.io.File; -import java.io.IOException; -import java.util.Arrays; -import java.util.HashSet; -import java.util.List; -import java.util.Set; -import javax.inject.Inject; -import javax.inject.Named; -import org.jetbrains.annotations.NotNull; -import timber.log.Timber; - -/** - * Processing of the image filePath that is about to be uploaded via ShareActivity is done here - */ -public class FileProcessor { - - private final Context context; - private final ContentResolver contentResolver; - private final CacheController cacheController; - private final GpsCategoryModel gpsCategoryModel; - private final JsonKvStore defaultKvStore; - private final CategoryApi apiCall; - - private final CompositeDisposable compositeDisposable = new CompositeDisposable(); - - @Inject - public FileProcessor(Context context, ContentResolver contentResolver, - CacheController cacheController, GpsCategoryModel gpsCategoryModel, - @Named("default_preferences") JsonKvStore defaultKvStore, CategoryApi apiCall) { - this.context = context; - this.contentResolver = contentResolver; - this.cacheController = cacheController; - this.gpsCategoryModel = gpsCategoryModel; - this.defaultKvStore = defaultKvStore; - this.apiCall = apiCall; - } - - public void cleanup() { - compositeDisposable.clear(); - } - - /** - * Processes filePath coordinates, either from EXIF data or user location - */ - ImageCoordinates processFileCoordinates(SimilarImageInterface similarImageInterface, - String filePath) { - ExifInterface exifInterface = null; - try { - exifInterface = new ExifInterface(filePath); - } catch (IOException e) { - Timber.e(e); - } - // Redact EXIF data as indicated in preferences. - redactExifTags(exifInterface, getExifTagsToRedact()); - - Timber.d("Calling GPSExtractor"); - ImageCoordinates originalImageExtractor = new ImageCoordinates(exifInterface); - String decimalCoords = originalImageExtractor.getDecimalCoords(); - if (decimalCoords == null || !originalImageExtractor.imageCoordsExists) { - //Find other photos taken around the same time which has gps coordinates - findOtherImages(originalImageExtractor, new File(filePath), - similarImageInterface);// Do not do repeat the process - } else { - useImageCoords(originalImageExtractor); - } - - return originalImageExtractor; - } - - /** - * Gets EXIF Tags from preferences to be redacted. - * - * @return tags to be redacted - */ - private Set getExifTagsToRedact() { - Set prefManageEXIFTags = defaultKvStore.getStringSet(Prefs.MANAGED_EXIF_TAGS); - - Set redactTags = new HashSet<>(Arrays.asList( - context.getResources().getStringArray(R.array.pref_exifTag_values))); - Timber.d(redactTags.toString()); - - if (prefManageEXIFTags != null) { - redactTags.removeAll(prefManageEXIFTags); - } - - return redactTags; - } - - /** - * Redacts EXIF metadata as indicated in preferences. - * - * @param exifInterface ExifInterface object - * @param redactTags tags to be redacted - */ - private void redactExifTags(ExifInterface exifInterface, Set redactTags) { - compositeDisposable.add( - Observable.fromIterable(redactTags) - .flatMap(tag -> Observable.fromArray(FileMetadataUtils.getTagsFromPref(tag))) - .subscribe( - (tag) -> redactTag(exifInterface, tag), - Timber::d, - () -> save(exifInterface) - )); - } - - private void save(ExifInterface exifInterface) { - try { - exifInterface.saveAttributes(); - } catch (IOException e) { - Timber.w("EXIF redaction failed: %s", e.toString()); - } - } - - private void redactTag(ExifInterface exifInterface, String tag) { - Timber.d("Checking for tag: %s", tag); - String oldValue = exifInterface.getAttribute(tag); - if (oldValue != null && !oldValue.isEmpty()) { - Timber.d("Exif tag %s with value %s redacted.", tag, oldValue); - exifInterface.setAttribute(tag, null); - } - } - - /** - * Find other images around the same location that were taken within the last 20 sec - * - * @param originalImageExtractor - * @param fileBeingProcessed - * @param similarImageInterface - */ - private void findOtherImages( - ImageCoordinates originalImageExtractor, - File fileBeingProcessed, - SimilarImageInterface similarImageInterface) { - //Time when the original image was created - long timeOfCreation = fileBeingProcessed.lastModified(); - File[] files = fileBeingProcessed.getParentFile().listFiles(); - Timber.d("folderTime Number:" + files.length); - - for (File file : files) { - if (file.lastModified() - timeOfCreation <= (120 * 1000) - && file.lastModified() - timeOfCreation >= -(120 * 1000)) { - //Make sure the photos were taken within 20seconds - Timber.d("fild date:" + file.lastModified() + " time of creation" + timeOfCreation); - ImageCoordinates similarPictureExtractor = createGpsExtractor(file); - Timber.d("not null fild EXIF" + similarPictureExtractor.imageCoordsExists + " coords" - + similarPictureExtractor.getDecimalCoords()); - if (similarPictureExtractor.getDecimalCoords() != null - && similarPictureExtractor.imageCoordsExists) { - // Current image has gps coordinates and it's not current gps locaiton - Timber.d("This filePath has image coords:" + file.getAbsolutePath()); - similarImageInterface.showSimilarImageFragment( - fileBeingProcessed.getPath(), - file.getAbsolutePath(), - originalImageExtractor, - similarPictureExtractor - ); - break; - } - } - } - } - - @NotNull - private ImageCoordinates createGpsExtractor(File file) { - try { - return new ImageCoordinates(contentResolver.openInputStream(Uri.fromFile(file))); - } catch (IOException e) { - Timber.e(e); - try { - return new ImageCoordinates(file.getAbsolutePath()); - } catch (IOException ex) { - Timber.e(ex); - return null; - } - } - } - - /** - * Initiates retrieval of image coordinates or user coordinates, and caching of coordinates. Then - * initiates the calls to MediaWiki API through an instance of CategoryApi. - * - * @param imageCoordinates - */ - @SuppressLint("CheckResult") - public void useImageCoords(ImageCoordinates imageCoordinates) { - useImageCoords(imageCoordinates, imageCoordinates.getDecimalCoords()); - } - - private void useImageCoords(ImageCoordinates imageCoordinates, String decimalCoords) { - if (decimalCoords != null) { - Timber.d("Decimal coords of image: %s", decimalCoords); - Timber.d("is EXIF data present:" + imageCoordinates.imageCoordsExists + - " from findOther image"); - - // Only set cache for this point if image has coords - if (imageCoordinates.imageCoordsExists) { - cacheController - .setQtPoint(imageCoordinates.getDecLongitude(), imageCoordinates.getDecLatitude()); - } - - List displayCatList = cacheController.findCategory(); - boolean catListEmpty = displayCatList.isEmpty(); - - // If no categories found in cache, call MediaWiki API to match image coords with nearby Commons categories - if (catListEmpty) { - compositeDisposable.add(apiCall.request(decimalCoords) - .subscribeOn(Schedulers.io()) - .observeOn(Schedulers.io()) - .subscribe( - gpsCategoryModel::setCategoryList, - throwable -> { - Timber.e(throwable); - gpsCategoryModel.clear(); - } - )); - Timber.d("displayCatList size 0, calling MWAPI %s", displayCatList); - } else { - Timber.d("Cache found, setting categoryList in model to %s", displayCatList); - gpsCategoryModel.setCategoryList(displayCatList); - } - } else { - Timber.d("EXIF: no coords"); - } - } -} 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 new file mode 100644 index 0000000000..ec67d055b3 --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.kt @@ -0,0 +1,198 @@ +package fr.free.nrw.commons.upload + +import android.content.ContentResolver +import android.content.Context +import android.net.Uri +import androidx.exifinterface.media.ExifInterface +import fr.free.nrw.commons.R +import fr.free.nrw.commons.caching.CacheController +import fr.free.nrw.commons.kvstore.JsonKvStore +import fr.free.nrw.commons.mwapi.CategoryApi +import fr.free.nrw.commons.settings.Prefs +import io.reactivex.Observable +import io.reactivex.disposables.CompositeDisposable +import io.reactivex.schedulers.Schedulers +import timber.log.Timber +import java.io.File +import java.io.IOException +import javax.inject.Inject +import javax.inject.Named + +/** + * Processing of the image filePath that is about to be uploaded via ShareActivity is done here + */ +class FileProcessor @Inject constructor( + private val context: Context, + private val contentResolver: ContentResolver, + private val cacheController: CacheController, + private val gpsCategoryModel: GpsCategoryModel, + @param:Named("default_preferences") private val defaultKvStore: JsonKvStore, + private val apiCall: CategoryApi +) { + private val compositeDisposable = CompositeDisposable() + fun cleanup() { + compositeDisposable.clear() + } + + /** + * Processes filePath coordinates, either from EXIF data or user location + */ + fun processFileCoordinates( + similarImageInterface: SimilarImageInterface, + filePath: String? + ): ImageCoordinates { + val exifInterface: ExifInterface? = try { + ExifInterface(filePath!!) + } catch (e: IOException) { + Timber.e(e) + null + } + // Redact EXIF data as indicated in preferences. + redactExifTags(exifInterface, getExifTagsToRedact()) + Timber.d("Calling GPSExtractor") + val originalImageCoordinates = ImageCoordinates(exifInterface) + if (originalImageCoordinates.decimalCoords == null ) { + //Find other photos taken around the same time which has gps coordinates + findOtherImages( + originalImageCoordinates, + File(filePath), + similarImageInterface + ) + } else { + useImageCoords(originalImageCoordinates) + } + return originalImageCoordinates + } + + /** + * Gets EXIF Tags from preferences to be redacted. + * + * @return tags to be redacted + */ + private fun getExifTagsToRedact(): Set { + val prefManageEXIFTags = + defaultKvStore.getStringSet(Prefs.MANAGED_EXIF_TAGS) ?: emptySet() + val redactTags: Set = + context.resources.getStringArray(R.array.pref_exifTag_values).toSet() + return redactTags - prefManageEXIFTags + } + + /** + * Redacts EXIF metadata as indicated in preferences. + * + * @param exifInterface ExifInterface object + * @param redactTags tags to be redacted + */ + private fun redactExifTags( + exifInterface: ExifInterface?, + redactTags: Set + ) { + compositeDisposable.add( + Observable.fromIterable(redactTags) + .flatMap { Observable.fromArray(*FileMetadataUtils.getTagsFromPref(it)) } + .subscribe( + { redactTag(exifInterface, it) }, + { Timber.d(it) }, + { save(exifInterface) } + ) + ) + } + + private fun save(exifInterface: ExifInterface?) { + try { + exifInterface?.saveAttributes() + } catch (e: IOException) { + Timber.w("EXIF redaction failed: %s", e.toString()) + } + } + + private fun redactTag(exifInterface: ExifInterface?, tag: String) { + Timber.d("Checking for tag: %s", tag) + exifInterface?.getAttribute(tag) + ?.takeIf { it.isNotEmpty() } + ?.let { + exifInterface.setAttribute(tag, null).also { + Timber.d("Exif tag $tag with value $it redacted.") + } + } + } + + /** + * Find other images around the same location that were taken within the last 20 sec + * + * @param originalImageCoordinates + * @param fileBeingProcessed + * @param similarImageInterface + */ + private fun findOtherImages( + originalImageCoordinates: ImageCoordinates, + fileBeingProcessed: File, + similarImageInterface: SimilarImageInterface + ) { + val oneHundredAndTwentySeconds = 120 * 1000 + //Time when the original image was created + val timeOfCreation = fileBeingProcessed.lastModified() + LongRange + val timeOfCreationRange = + timeOfCreation - oneHundredAndTwentySeconds..timeOfCreation + oneHundredAndTwentySeconds + fileBeingProcessed.parentFile + .listFiles() + .asSequence() + .filter { it.lastModified() in timeOfCreationRange } + .map { Pair(it, readimageCoordinates(it)) } + .firstOrNull { it.second?.decimalCoords != null } + ?.let { fileCoordinatesPair -> + similarImageInterface.showSimilarImageFragment( + fileBeingProcessed.path, + fileCoordinatesPair.first.absolutePath, + originalImageCoordinates, + fileCoordinatesPair.second + ) + } + } + + private fun readimageCoordinates(file: File) = try { + ImageCoordinates(contentResolver.openInputStream(Uri.fromFile(file))) + } catch (e: IOException) { + Timber.e(e) + try { + ImageCoordinates(file.absolutePath) + } catch (ex: IOException) { + Timber.e(ex) + null + } + } + + /** + * Initiates retrieval of image coordinates or user coordinates, and caching of coordinates. Then + * initiates the calls to MediaWiki API through an instance of CategoryApi. + * + * @param imageCoordinates + */ + fun useImageCoords(imageCoordinates: ImageCoordinates) { + if (imageCoordinates.decimalCoords != null) { + 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 + } + } + } +} diff --git a/app/src/main/java/fr/free/nrw/commons/upload/ImageCoordinates.kt b/app/src/main/java/fr/free/nrw/commons/upload/ImageCoordinates.kt index 7fb6ce5775..dc79f1673c 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/ImageCoordinates.kt +++ b/app/src/main/java/fr/free/nrw/commons/upload/ImageCoordinates.kt @@ -2,6 +2,7 @@ package fr.free.nrw.commons.upload import androidx.exifinterface.media.ExifInterface import timber.log.Timber +import java.io.IOException import java.io.InputStream /** @@ -26,6 +27,7 @@ class ImageCoordinates internal constructor(exif: ExifInterface?) { * Construct from the file path of the image. * @param path file path of the image */ + @Throws(IOException::class) internal constructor(path: String) : this(ExifInterface(path)) From 95f27022f0e36f0cf3427b1c20e9b9e1c3d62e84 Mon Sep 17 00:00:00 2001 From: Sean Mac Gillicuddy Date: Thu, 19 Mar 2020 12:04:13 +0000 Subject: [PATCH 5/8] #3408 Refactoring the FileProcessor and GPSExtractor classes - minor reformatting --- .../free/nrw/commons/upload/FileProcessor.kt | 35 +++++++++---------- 1 file changed, 16 insertions(+), 19 deletions(-) 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 ec67d055b3..d456581d92 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 @@ -30,6 +30,7 @@ class FileProcessor @Inject constructor( private val apiCall: CategoryApi ) { private val compositeDisposable = CompositeDisposable() + fun cleanup() { compositeDisposable.clear() } @@ -37,10 +38,8 @@ class FileProcessor @Inject constructor( /** * Processes filePath coordinates, either from EXIF data or user location */ - fun processFileCoordinates( - similarImageInterface: SimilarImageInterface, - filePath: String? - ): ImageCoordinates { + fun processFileCoordinates(similarImageInterface: SimilarImageInterface, filePath: String?) + : ImageCoordinates { val exifInterface: ExifInterface? = try { ExifInterface(filePath!!) } catch (e: IOException) { @@ -51,7 +50,7 @@ class FileProcessor @Inject constructor( redactExifTags(exifInterface, getExifTagsToRedact()) Timber.d("Calling GPSExtractor") val originalImageCoordinates = ImageCoordinates(exifInterface) - if (originalImageCoordinates.decimalCoords == null ) { + if (originalImageCoordinates.decimalCoords == null) { //Find other photos taken around the same time which has gps coordinates findOtherImages( originalImageCoordinates, @@ -83,10 +82,7 @@ class FileProcessor @Inject constructor( * @param exifInterface ExifInterface object * @param redactTags tags to be redacted */ - private fun redactExifTags( - exifInterface: ExifInterface?, - redactTags: Set - ) { + private fun redactExifTags(exifInterface: ExifInterface?, redactTags: Set) { compositeDisposable.add( Observable.fromIterable(redactTags) .flatMap { Observable.fromArray(*FileMetadataUtils.getTagsFromPref(it)) } @@ -139,7 +135,7 @@ class FileProcessor @Inject constructor( .listFiles() .asSequence() .filter { it.lastModified() in timeOfCreationRange } - .map { Pair(it, readimageCoordinates(it)) } + .map { Pair(it, readImageCoordinates(it)) } .firstOrNull { it.second?.decimalCoords != null } ?.let { fileCoordinatesPair -> similarImageInterface.showSimilarImageFragment( @@ -151,17 +147,18 @@ class FileProcessor @Inject constructor( } } - private fun readimageCoordinates(file: File) = try { - ImageCoordinates(contentResolver.openInputStream(Uri.fromFile(file))) - } catch (e: IOException) { - Timber.e(e) + private fun readImageCoordinates(file: File) = try { - ImageCoordinates(file.absolutePath) - } catch (ex: IOException) { - Timber.e(ex) - null + ImageCoordinates(contentResolver.openInputStream(Uri.fromFile(file))) + } catch (e: IOException) { + Timber.e(e) + try { + ImageCoordinates(file.absolutePath) + } catch (ex: IOException) { + Timber.e(ex) + null + } } - } /** * Initiates retrieval of image coordinates or user coordinates, and caching of coordinates. Then From 7b56dee9850757933065f7c637f845a9cf341596 Mon Sep 17 00:00:00 2001 From: Sean Mac Gillicuddy Date: Thu, 19 Mar 2020 12:16:14 +0000 Subject: [PATCH 6/8] #3408 Refactoring the FileProcessor and GPSExtractor classes - fix compilation and naming issues --- .../free/nrw/commons/upload/FileProcessor.kt | 2 +- .../fr/free/nrw/commons/upload/FileUtils.java | 2 +- .../commons/upload/SimilarImageInterface.java | 2 +- .../free/nrw/commons/upload/UploadModel.java | 4 +-- .../UploadMediaDetailFragment.java | 36 ++++++++----------- .../mediaDetails/UploadMediaPresenter.java | 34 +++++++++--------- 6 files changed, 36 insertions(+), 44 deletions(-) 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 d456581d92..1dcdd2eb27 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 @@ -125,7 +125,7 @@ class FileProcessor @Inject constructor( fileBeingProcessed: File, similarImageInterface: SimilarImageInterface ) { - val oneHundredAndTwentySeconds = 120 * 1000 + val oneHundredAndTwentySeconds = 120 * 1000L //Time when the original image was created val timeOfCreation = fileBeingProcessed.lastModified() LongRange diff --git a/app/src/main/java/fr/free/nrw/commons/upload/FileUtils.java b/app/src/main/java/fr/free/nrw/commons/upload/FileUtils.java index 098c032f31..10e1577055 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/FileUtils.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/FileUtils.java @@ -69,7 +69,7 @@ static String getGeolocationOfFile(String filePath) { try { ExifInterface exifInterface = new ExifInterface(filePath); ImageCoordinates imageObj = new ImageCoordinates(exifInterface); - if (imageObj.imageCoordsExists) { // If image has geolocation information in its EXIF + if (imageObj.getDecimalCoords() != null) { // If image has geolocation information in its EXIF return imageObj.getDecimalCoords(); } else { return ""; 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 200899f42c..23a9543536 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 originalPictureExtractor, ImageCoordinates similarPictureExtractor); + ImageCoordinates originalImageCoordinates, 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 5cc7ab369a..12fe21c678 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 @@ -211,8 +211,8 @@ public void updateUploadItem(int index, UploadItem uploadItem) { uploadItem1.setTitle(uploadItem.title); } - public void usePictureCoordinatesFrom(ImageCoordinates originalPictureExtractor) { - fileProcessor.useImageCoords(originalPictureExtractor); + public void usePictureCoordinatesFrom(ImageCoordinates imageCoordinates) { + fileProcessor.useImageCoords(imageCoordinates); } @SuppressWarnings("WeakerAccess") 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 b69567924d..c32f305026 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 @@ -1,5 +1,7 @@ package fr.free.nrw.commons.upload.mediaDetails; +import static fr.free.nrw.commons.utils.ImageUtils.getErrorMessageForResult; + import android.annotation.SuppressLint; import android.content.Context; import android.os.Bundle; @@ -12,31 +14,17 @@ import android.widget.EditText; import android.widget.LinearLayout; import android.widget.TextView; - import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.appcompat.widget.AppCompatButton; import androidx.appcompat.widget.AppCompatImageButton; import androidx.recyclerview.widget.LinearLayoutManager; import androidx.recyclerview.widget.RecyclerView; - -import com.github.chrisbanes.photoview.PhotoView; -import com.jakewharton.rxbinding2.widget.RxTextView; - -import fr.free.nrw.commons.upload.ImageCoordinates; -import org.apache.commons.lang3.StringUtils; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.Locale; - -import javax.inject.Inject; -import javax.inject.Named; - import butterknife.BindView; import butterknife.ButterKnife; import butterknife.OnClick; +import com.github.chrisbanes.photoview.PhotoView; +import com.jakewharton.rxbinding2.widget.RxTextView; import fr.free.nrw.commons.R; import fr.free.nrw.commons.Utils; import fr.free.nrw.commons.filepicker.UploadableFile; @@ -46,6 +34,7 @@ import fr.free.nrw.commons.settings.Prefs; import fr.free.nrw.commons.upload.Description; import fr.free.nrw.commons.upload.DescriptionsAdapter; +import fr.free.nrw.commons.upload.ImageCoordinates; import fr.free.nrw.commons.upload.SimilarImageDialogFragment; import fr.free.nrw.commons.upload.Title; import fr.free.nrw.commons.upload.UploadBaseFragment; @@ -55,10 +44,15 @@ import fr.free.nrw.commons.utils.ImageUtils; import fr.free.nrw.commons.utils.ViewUtil; import io.reactivex.disposables.Disposable; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Locale; +import javax.inject.Inject; +import javax.inject.Named; +import org.apache.commons.lang3.StringUtils; import timber.log.Timber; -import static fr.free.nrw.commons.utils.ImageUtils.getErrorMessageForResult; - public class UploadMediaDetailFragment extends UploadBaseFragment implements UploadMediaDetailsContract.View { @@ -261,19 +255,19 @@ public void onButtonAddDescriptionClicked() { @Override public void showSimilarImageFragment(String originalFilePath, String possibleFilePath, - ImageCoordinates originalPictureExtractor, ImageCoordinates similarPictureExtractor) { + ImageCoordinates originalImageCoordinates, 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(similarPictureExtractor); + presenter.usePictureCoordinatesFrom(similarImageCoordinates); } @Override public void onNegativeResponse() { Timber.d("negative response from similar image fragment"); - presenter.usePictureCoordinatesFrom(originalPictureExtractor); + presenter.usePictureCoordinatesFrom(originalImageCoordinates); } }); Bundle args = new Bundle(); 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 cd97074eef..e1ac04d618 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 @@ -1,9 +1,11 @@ package fr.free.nrw.commons.upload.mediaDetails; -import java.lang.reflect.Proxy; - -import javax.inject.Inject; -import javax.inject.Named; +import static fr.free.nrw.commons.di.CommonsApplicationModule.IO_THREAD; +import static fr.free.nrw.commons.di.CommonsApplicationModule.MAIN_THREAD; +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_KEEP; +import static fr.free.nrw.commons.utils.ImageUtils.IMAGE_OK; import fr.free.nrw.commons.R; import fr.free.nrw.commons.filepicker.UploadableFile; @@ -18,15 +20,11 @@ import io.reactivex.Scheduler; import io.reactivex.disposables.CompositeDisposable; import io.reactivex.disposables.Disposable; +import java.lang.reflect.Proxy; +import javax.inject.Inject; +import javax.inject.Named; import timber.log.Timber; -import static fr.free.nrw.commons.di.CommonsApplicationModule.IO_THREAD; -import static fr.free.nrw.commons.di.CommonsApplicationModule.MAIN_THREAD; -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_KEEP; -import static fr.free.nrw.commons.utils.ImageUtils.IMAGE_OK; - public class UploadMediaPresenter implements UserActionListener, SimilarImageInterface { private static final UploadMediaDetailsContract.View DUMMY = (UploadMediaDetailsContract.View) Proxy @@ -82,9 +80,9 @@ public void receiveImage(UploadableFile uploadableFile, String source, Place pla { view.onImageProcessed(uploadItem, place); ImageCoordinates gpsCoords = uploadItem.getGpsCoords(); - view.showMapWithImageCoordinates(gpsCoords != null && gpsCoords.imageCoordsExists); + view.showMapWithImageCoordinates(gpsCoords != null && gpsCoords.getImageCoordsExists()); view.showProgress(false); - if (gpsCoords != null && gpsCoords.imageCoordsExists) { + if (gpsCoords != null && gpsCoords.getImageCoordsExists()) { checkNearbyPlaces(uploadItem); } }, @@ -207,14 +205,14 @@ public void handleBadImage(Integer errorCode) { * notifies the user that a similar image exists * @param originalFilePath * @param possibleFilePath - * @param originalPictureExtractor - * @param similarPictureExtractor + * @param originalImageCoordinates + * @param similarImageCoordinates */ @Override public void showSimilarImageFragment(String originalFilePath, String possibleFilePath, - ImageCoordinates originalPictureExtractor, ImageCoordinates similarPictureExtractor) { - view.showSimilarImageFragment(originalFilePath, possibleFilePath, originalPictureExtractor, - similarPictureExtractor + ImageCoordinates originalImageCoordinates, ImageCoordinates similarImageCoordinates) { + view.showSimilarImageFragment(originalFilePath, possibleFilePath, originalImageCoordinates, + similarImageCoordinates ); } } From e4b8cbee915aaa7c00e2d1e1584251a5a0236370 Mon Sep 17 00:00:00 2001 From: Sean Mac Gillicuddy Date: Thu, 19 Mar 2020 12:29:17 +0000 Subject: [PATCH 7/8] #3408 Refactoring the FileProcessor and GPSExtractor classes - remove empty test --- .../nrw/commons/upload/FileProcessorTest.kt | 89 ------------------- 1 file changed, 89 deletions(-) delete mode 100644 app/src/test/kotlin/fr/free/nrw/commons/upload/FileProcessorTest.kt diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/FileProcessorTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/FileProcessorTest.kt deleted file mode 100644 index 98f6a2e226..0000000000 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/FileProcessorTest.kt +++ /dev/null @@ -1,89 +0,0 @@ -package fr.free.nrw.commons.upload - -import android.content.SharedPreferences -import androidx.exifinterface.media.ExifInterface -import fr.free.nrw.commons.caching.CacheController -import fr.free.nrw.commons.mwapi.CategoryApi -import org.junit.Before -import org.junit.Test -import org.mockito.InjectMocks -import org.mockito.Mock -import org.mockito.MockitoAnnotations -import javax.inject.Inject -import javax.inject.Named - -import java.io.FileInputStream -import java.io.FileOutputStream - -class FileProcessorTest { - - @Mock - internal var cacheController: CacheController? = null - @Mock - internal var gpsCategoryModel: GpsCategoryModel? = null - @Mock - internal var apiCall: CategoryApi? = null - @Mock - @field:[Inject Named("default_preferences")] - internal var prefs: SharedPreferences? = null - - @InjectMocks - var fileProcessor: FileProcessor? = null - - @Before - fun setup() { - MockitoAnnotations.initMocks(this) - } - - @Test - fun processFileCoordinates() { - - } - - /** - * Test method to verify redaction Exif metadata - */ - @Test - fun redactExifTags() { - /* - val filePathRef: String? = "src/test/data/exif_redact_sample.jpg" - val filePathTmp: String? = "" + System.getProperty("java.io.tmpdir") + "exif_redact_sample_tmp.jpg" - - val inStream = FileInputStream(filePathRef) - val outStream = FileOutputStream(filePathTmp) - val inChannel = inStream.getChannel() - val outChannel = outStream.getChannel() - inChannel.transferTo(0, inChannel.size(), outChannel) - inStream.close() - outStream.close() - - val redactTags = mutableSetOf("Author", "Copyright", "Location", "Camera Model", - "Lens Model", "Serial Numbers", "Software") - - val exifInterface : ExifInterface? = ExifInterface(filePathTmp.toString()) - - var nonEmptyTag = false - for (redactTag in redactTags) { - for (tag in FileMetadataUtils.getTagsFromPref(redactTag)) { - val tagValue = exifInterface?.getAttribute(tag) - if(tagValue != null) { - nonEmptyTag = true - break - } - } - if (nonEmptyTag) break - } - // all tags are empty, can't test redaction - assert(nonEmptyTag) - - FileProcessor.redactExifTags(exifInterface, redactTags) - - for (redactTag in redactTags) { - for (tag in FileMetadataUtils.getTagsFromPref(redactTag)) { - val oldValue = exifInterface?.getAttribute(tag) - assert(oldValue == null) - } - } - */ - } -} \ No newline at end of file From 87938e2fbb141e7f80b9800b5f6a5d513bc104f0 Mon Sep 17 00:00:00 2001 From: Sean Mac Gillicuddy Date: Fri, 20 Mar 2020 10:29:03 +0000 Subject: [PATCH 8/8] #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) } }