From 5073ca08c30d347fb8750527b8de501a7f9b123c Mon Sep 17 00:00:00 2001 From: Ritika Pahwa <83745993+RitikaPahwa4444@users.noreply.github.com> Date: Fri, 1 Sep 2023 12:15:50 +0530 Subject: [PATCH] 5196: Fix in-app camera location loss (#5249) Merging as this is a great improvement, additional issues/bugs can be filed as GitHub issues. * fix in-app camera location loss * fix failing unit tests * UploadMediaDetailFragmentUnitTest: modify testOnActivityResultAddLocationDialog to have null location * reintroduce removed variable * enable prePopulateCategoriesAndDepictionsBy for current user location * add relevant comment and fix failing test * modify dialog and disable location tag redaction from EXIF * modify in-app camera dialog flow and change location to inAppPictureLocation * change location to inAppPictureLocation * fix location flow * preferences.xml: remove redundant default value * inform users about location loss happening for first upload * FileProcessor.kt: remove commented-out code * prevent user location from getting attached to images with no EXIF location in normal and custom selector * handle onPermissionDenied for location permission * remove last location when the user turns the GPS off * disable photo picker and in app camera preferences in settings for logged-out users * remove debug statements and add toast inside runnables --- .../contributions/ContributionController.java | 118 ++++++++++++++- .../commons/contributions/MainActivity.java | 1 + .../location/LocationPermissionsHelper.java | 137 ++++++++++++++++++ .../commons/repository/UploadRepository.java | 8 +- .../commons/settings/SettingsFragment.java | 56 ++++++- .../free/nrw/commons/upload/FileProcessor.kt | 12 +- .../fr/free/nrw/commons/upload/FileUtils.java | 5 +- .../nrw/commons/upload/FileUtilsWrapper.java | 5 +- .../nrw/commons/upload/ImageCoordinates.kt | 16 +- .../upload/ImageProcessingService.java | 9 +- .../nrw/commons/upload/UploadActivity.java | 78 +++++++++- .../free/nrw/commons/upload/UploadModel.java | 16 +- .../UploadMediaDetailFragment.java | 17 ++- .../UploadMediaDetailsContract.java | 5 +- .../mediaDetails/UploadMediaPresenter.java | 13 +- app/src/main/res/values/strings.xml | 9 ++ app/src/main/res/xml/preferences.xml | 63 ++++---- .../upload/ImageProcessingServiceTest.kt | 16 +- .../upload/UploadMediaPresenterTest.kt | 25 ++-- .../upload/UploadRepositoryUnitTest.kt | 11 +- .../UploadMediaDetailFragmentUnitTest.kt | 9 +- 21 files changed, 537 insertions(+), 92 deletions(-) create mode 100644 app/src/main/java/fr/free/nrw/commons/location/LocationPermissionsHelper.java diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionController.java b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionController.java index 18f307984..e4cc0e55f 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionController.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionController.java @@ -6,6 +6,7 @@ import android.Manifest; import android.app.Activity; import android.content.Context; import android.content.Intent; +import android.widget.Toast; import androidx.annotation.NonNull; import fr.free.nrw.commons.R; import fr.free.nrw.commons.filepicker.DefaultCallback; @@ -13,8 +14,14 @@ import fr.free.nrw.commons.filepicker.FilePicker; import fr.free.nrw.commons.filepicker.FilePicker.ImageSource; import fr.free.nrw.commons.filepicker.UploadableFile; import fr.free.nrw.commons.kvstore.JsonKvStore; +import fr.free.nrw.commons.location.LatLng; +import fr.free.nrw.commons.location.LocationPermissionsHelper; +import fr.free.nrw.commons.location.LocationPermissionsHelper.Dialog; +import fr.free.nrw.commons.location.LocationPermissionsHelper.LocationPermissionCallback; +import fr.free.nrw.commons.location.LocationServiceManager; import fr.free.nrw.commons.nearby.Place; import fr.free.nrw.commons.upload.UploadActivity; +import fr.free.nrw.commons.utils.DialogUtil; import fr.free.nrw.commons.utils.PermissionUtils; import fr.free.nrw.commons.utils.ViewUtil; import java.util.ArrayList; @@ -28,7 +35,11 @@ public class ContributionController { public static final String ACTION_INTERNAL_UPLOADS = "internalImageUploads"; private final JsonKvStore defaultKvStore; + private LatLng locationBeforeImageCapture; + private boolean isInAppCameraUpload; + @Inject + LocationServiceManager locationManager; @Inject public ContributionController(@Named("default_preferences") JsonKvStore defaultKvStore) { this.defaultKvStore = defaultKvStore; @@ -46,11 +57,94 @@ public class ContributionController { PermissionUtils.checkPermissionsAndPerformAction(activity, Manifest.permission.WRITE_EXTERNAL_STORAGE, - () -> initiateCameraUpload(activity), + () -> { + if (defaultKvStore.getBoolean("inAppCameraFirstRun")) { + defaultKvStore.putBoolean("inAppCameraFirstRun", false); + askUserToAllowLocationAccess(activity); + } else if(defaultKvStore.getBoolean("inAppCameraLocationPref")) { + createDialogsAndHandleLocationPermissions(activity); + } else { + initiateCameraUpload(activity); + } + }, R.string.storage_permission_title, R.string.write_storage_permission_rationale); } + /** + * Asks users to provide location access + * + * @param activity + */ + private void createDialogsAndHandleLocationPermissions(Activity activity) { + LocationPermissionsHelper.Dialog locationAccessDialog = new Dialog( + R.string.location_permission_title, + R.string.in_app_camera_location_permission_rationale + ); + + LocationPermissionsHelper.Dialog locationOffDialog = new Dialog( + R.string.ask_to_turn_location_on, + R.string.in_app_camera_needs_location + ); + LocationPermissionsHelper locationPermissionsHelper = new LocationPermissionsHelper( + activity, locationManager, + new LocationPermissionCallback() { + @Override + public void onLocationPermissionDenied() { + initiateCameraUpload(activity); + } + + @Override + public void onLocationPermissionGranted() { + initiateCameraUpload(activity); + } + } + ); + locationPermissionsHelper.handleLocationPermissions( + locationAccessDialog, + locationOffDialog + ); + } + + /** + * Suggest user to attach location information with pictures. + * If the user selects "Yes", then: + * + * Location is taken from the EXIF if the default camera application + * does not redact location tags. + * + * Otherwise, if the EXIF metadata does not have location information, + * then location captured by the app is used + * + * @param activity + */ + private void askUserToAllowLocationAccess(Activity activity) { + DialogUtil.showAlertDialog(activity, + activity.getString(R.string.in_app_camera_location_permission_title), + activity.getString(R.string.in_app_camera_location_access_explanation), + activity.getString(R.string.option_allow), + activity.getString(R.string.option_dismiss), + ()-> { + defaultKvStore.putBoolean("inAppCameraLocationPref", true); + createDialogsAndHandleLocationPermissions(activity); + }, + () -> { + defaultKvStore.putBoolean("inAppCameraLocationPref", false); + initiateCameraUpload(activity); + }, + null, + true); + } + + /** + * Check if apps have access to location even after having individual access + * + * @return + */ + private boolean isLocationAccessToAppsTurnedOn() { + return (locationManager.isNetworkProviderEnabled() || locationManager.isGPSProviderEnabled()); + } + /** * Initiate gallery picker */ @@ -66,9 +160,7 @@ public class ContributionController { PermissionUtils.checkPermissionsAndPerformAction(activity, Manifest.permission.WRITE_EXTERNAL_STORAGE, - () -> { - FilePicker.openCustomSelector(activity, 0); - }, + () -> FilePicker.openCustomSelector(activity, 0), R.string.storage_permission_title, R.string.write_storage_permission_rationale); } @@ -99,6 +191,10 @@ public class ContributionController { */ private void initiateCameraUpload(Activity activity) { setPickerConfiguration(activity, false); + if (defaultKvStore.getBoolean("inAppCameraLocationPref", false)) { + locationBeforeImageCapture = locationManager.getLastLocation(); + } + isInAppCameraUpload = true; FilePicker.openCameraForImage(activity, 0); } @@ -134,7 +230,8 @@ public class ContributionController { /** * Returns intent to be passed to upload activity - * Attaches place object for nearby uploads + * Attaches place object for nearby uploads and + * location before image capture if in-app camera is used */ private Intent handleImagesPicked(Context context, List imagesFiles) { @@ -148,6 +245,17 @@ public class ContributionController { shareIntent.putExtra(PLACE_OBJECT, place); } + if (locationBeforeImageCapture != null) { + shareIntent.putExtra( + UploadActivity.LOCATION_BEFORE_IMAGE_CAPTURE, + locationBeforeImageCapture); + } + + shareIntent.putExtra( + UploadActivity.IN_APP_CAMERA_UPLOAD, + isInAppCameraUpload + ); + isInAppCameraUpload = false; // reset the flag for next use return shareIntent; } diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java b/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java index a96f1f37b..740952664 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java @@ -403,6 +403,7 @@ public class MainActivity extends BaseActivity if ((applicationKvStore.getBoolean("firstrun", true)) && (!applicationKvStore.getBoolean("login_skipped"))) { + defaultKvStore.putBoolean("inAppCameraFirstRun", true); WelcomeActivity.startYourself(this); } } diff --git a/app/src/main/java/fr/free/nrw/commons/location/LocationPermissionsHelper.java b/app/src/main/java/fr/free/nrw/commons/location/LocationPermissionsHelper.java new file mode 100644 index 000000000..d618fa62e --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/location/LocationPermissionsHelper.java @@ -0,0 +1,137 @@ +package fr.free.nrw.commons.location; + +import android.Manifest.permission; +import android.app.Activity; +import android.content.Intent; +import android.content.pm.PackageManager; +import android.provider.Settings; +import android.widget.Toast; +import fr.free.nrw.commons.R; +import fr.free.nrw.commons.utils.DialogUtil; +import fr.free.nrw.commons.utils.PermissionUtils; + +/** + * Helper class to handle location permissions + */ +public class LocationPermissionsHelper { + Activity activity; + LocationServiceManager locationManager; + LocationPermissionCallback callback; + public LocationPermissionsHelper(Activity activity, LocationServiceManager locationManager, + LocationPermissionCallback callback) { + this.activity = activity; + this.locationManager = locationManager; + this.callback = callback; + } + public static class Dialog { + int dialogTitleResource; + int dialogTextResource; + + public Dialog(int dialogTitle, int dialogText) { + dialogTitleResource = dialogTitle; + dialogTextResource = dialogText; + } + } + + /** + * Handles the entire location permissions flow + * + * @param locationAccessDialog + * @param locationOffDialog + */ + public void handleLocationPermissions(Dialog locationAccessDialog, + Dialog locationOffDialog) { + requestForLocationAccess(locationAccessDialog, locationOffDialog); + } + + /** + * Ask for location permission if the user agrees on attaching location with pictures + * and the app does not have the access to location + * + * @param locationAccessDialog + * @param locationOffDialog + */ + private void requestForLocationAccess( + Dialog locationAccessDialog, + Dialog locationOffDialog + ) { + PermissionUtils.checkPermissionsAndPerformAction(activity, + permission.ACCESS_FINE_LOCATION, + () -> { + if(!isLocationAccessToAppsTurnedOn()) { + showLocationOffDialog(locationOffDialog); + } else { + if (callback != null) { + callback.onLocationPermissionGranted(); + } + } + }, + () -> { + if (callback != null) { + Toast.makeText( + activity, + R.string.in_app_camera_location_permission_denied, + Toast.LENGTH_LONG + ).show(); + callback.onLocationPermissionDenied(); + } + }, + locationAccessDialog.dialogTitleResource, + locationAccessDialog.dialogTextResource); + } + + /** + * Check if apps have access to location even after having individual access + * + * @return + */ + public boolean isLocationAccessToAppsTurnedOn() { + return (locationManager.isNetworkProviderEnabled() || locationManager.isGPSProviderEnabled()); + } + + /** + * Ask user to grant location access to apps + * + */ + + private void showLocationOffDialog(Dialog locationOffDialog) { + DialogUtil + .showAlertDialog(activity, + activity.getString(locationOffDialog.dialogTitleResource), + activity.getString(locationOffDialog.dialogTextResource), + activity.getString(R.string.title_app_shortcut_setting), + activity.getString(R.string.cancel), + () -> openLocationSettings(), + () -> { + Toast.makeText( + activity, + R.string.in_app_camera_location_unavailable, + Toast.LENGTH_LONG + ).show(); + callback.onLocationPermissionDenied(); + }); + } + + /** + * Open location source settings so that apps with location access can access it + * + * TODO: modify it to fix https://github.com/commons-app/apps-android-commons/issues/5255 + */ + + private void openLocationSettings() { + final Intent intent = new Intent(Settings.ACTION_LOCATION_SOURCE_SETTINGS); + final PackageManager packageManager = activity.getPackageManager(); + + if (intent.resolveActivity(packageManager)!= null) { + activity.startActivity(intent); + } + } + + /** + * Handle onPermissionDenied within individual classes based on the requirements + */ + public interface LocationPermissionCallback { + void onLocationPermissionDenied(); + void onLocationPermissionGranted(); + } +} 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 007e89cb2..919c8d2cf 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 @@ -188,9 +188,9 @@ public class UploadRepository { * @return */ public Observable preProcessImage(UploadableFile uploadableFile, Place place, - SimilarImageInterface similarImageInterface) { + SimilarImageInterface similarImageInterface, LatLng inAppPictureLocation) { return uploadModel.preProcessImage(uploadableFile, place, - similarImageInterface); + similarImageInterface, inAppPictureLocation); } /** @@ -199,8 +199,8 @@ public class UploadRepository { * @param uploadItem * @return */ - public Single getImageQuality(UploadItem uploadItem) { - return uploadModel.getImageQuality(uploadItem); + public Single getImageQuality(UploadItem uploadItem, LatLng location) { + return uploadModel.getImageQuality(uploadItem, location); } /** diff --git a/app/src/main/java/fr/free/nrw/commons/settings/SettingsFragment.java b/app/src/main/java/fr/free/nrw/commons/settings/SettingsFragment.java index c7654c5fe..fddb91df8 100644 --- a/app/src/main/java/fr/free/nrw/commons/settings/SettingsFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/settings/SettingsFragment.java @@ -18,6 +18,7 @@ import android.widget.AdapterView.OnItemClickListener; import android.widget.EditText; import android.widget.ListView; import android.widget.TextView; +import android.widget.Toast; import androidx.preference.ListPreference; import androidx.preference.MultiSelectListPreference; import androidx.preference.Preference; @@ -30,13 +31,15 @@ import androidx.recyclerview.widget.RecyclerView.Adapter; import com.karumi.dexter.Dexter; import com.karumi.dexter.listener.PermissionGrantedResponse; import com.karumi.dexter.listener.single.BasePermissionListener; -import com.mapbox.mapboxsdk.Mapbox; import fr.free.nrw.commons.R; import fr.free.nrw.commons.Utils; import fr.free.nrw.commons.campaigns.CampaignView; import fr.free.nrw.commons.contributions.MainActivity; import fr.free.nrw.commons.di.ApplicationlessInjection; import fr.free.nrw.commons.kvstore.JsonKvStore; +import fr.free.nrw.commons.location.LocationPermissionsHelper; +import fr.free.nrw.commons.location.LocationPermissionsHelper.LocationPermissionCallback; +import fr.free.nrw.commons.location.LocationServiceManager; import fr.free.nrw.commons.logging.CommonsLogSender; import fr.free.nrw.commons.recentlanguages.Language; import fr.free.nrw.commons.recentlanguages.RecentLanguagesAdapter; @@ -65,6 +68,9 @@ public class SettingsFragment extends PreferenceFragmentCompat { @Inject RecentLanguagesDao recentLanguagesDao; + @Inject + LocationServiceManager locationManager; + private ListPreference themeListPreference; private Preference descriptionLanguageListPreference; private Preference appUiLanguageListPreference; @@ -97,6 +103,18 @@ public class SettingsFragment extends PreferenceFragmentCompat { }); } + Preference inAppCameraLocationPref = findPreference("inAppCameraLocationPref"); + + inAppCameraLocationPref.setOnPreferenceChangeListener( + (preference, newValue) -> { + boolean isInAppCameraLocationTurnedOn = (boolean) newValue; + if (isInAppCameraLocationTurnedOn) { + createDialogsAndHandleLocationPermissions(getActivity()); + } + return true; + } + ); + // Gets current language code from shared preferences String languageCode; @@ -172,9 +190,45 @@ public class SettingsFragment extends PreferenceFragmentCompat { findPreference("displayLocationPermissionForCardView").setEnabled(false); findPreference(CampaignView.CAMPAIGNS_DEFAULT_PREFERENCE).setEnabled(false); findPreference("managed_exif_tags").setEnabled(false); + findPreference("openDocumentPhotoPickerPref").setEnabled(false); + findPreference("inAppCameraLocationPref").setEnabled(false); } } + /** + * Asks users to provide location access + * + * @param activity + */ + private void createDialogsAndHandleLocationPermissions(Activity activity) { + LocationPermissionsHelper.Dialog locationAccessDialog = new LocationPermissionsHelper.Dialog( + R.string.location_permission_title, + R.string.in_app_camera_location_permission_rationale + ); + + LocationPermissionsHelper.Dialog locationOffDialog = new LocationPermissionsHelper.Dialog( + R.string.ask_to_turn_location_on, + R.string.in_app_camera_needs_location + ); + + LocationPermissionsHelper locationPermissionsHelper = new LocationPermissionsHelper( + activity, locationManager, new LocationPermissionCallback() { + @Override + public void onLocationPermissionDenied() { + // dismiss the dialog + } + + @Override + public void onLocationPermissionGranted() { + // dismiss the dialog + } + }); + locationPermissionsHelper.handleLocationPermissions( + locationAccessDialog, + locationOffDialog + ); + } + /** * On some devices, the new Photo Picker with GET_CONTENT takeover * redacts location tags from EXIF metadata 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 3d0c62236..4bd4797c4 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 @@ -6,6 +6,7 @@ import android.net.Uri import androidx.exifinterface.media.ExifInterface import fr.free.nrw.commons.R import fr.free.nrw.commons.kvstore.JsonKvStore +import fr.free.nrw.commons.location.LatLng import fr.free.nrw.commons.mwapi.CategoryApi import fr.free.nrw.commons.mwapi.OkHttpJsonApiClient import fr.free.nrw.commons.settings.Prefs @@ -48,7 +49,8 @@ class FileProcessor @Inject constructor( /** * Processes filePath coordinates, either from EXIF data or user location */ - fun processFileCoordinates(similarImageInterface: SimilarImageInterface, filePath: String?) + fun processFileCoordinates(similarImageInterface: SimilarImageInterface, + filePath: String?, inAppPictureLocation: LatLng?) : ImageCoordinates { val exifInterface: ExifInterface? = try { ExifInterface(filePath!!) @@ -59,7 +61,7 @@ class FileProcessor @Inject constructor( // Redact EXIF data as indicated in preferences. redactExifTags(exifInterface, getExifTagsToRedact()) Timber.d("Calling GPSExtractor") - val originalImageCoordinates = ImageCoordinates(exifInterface) + val originalImageCoordinates = ImageCoordinates(exifInterface, inAppPictureLocation) if (originalImageCoordinates.decimalCoords == null) { //Find other photos taken around the same time which has gps coordinates findOtherImages( @@ -156,11 +158,13 @@ class FileProcessor @Inject constructor( private fun readImageCoordinates(file: File) = try { - ImageCoordinates(contentResolver.openInputStream(Uri.fromFile(file))!!) + /* Used null location as location for similar images captured before is not available + in case it is not present in the EXIF. */ + ImageCoordinates(contentResolver.openInputStream(Uri.fromFile(file))!!, null) } catch (e: IOException) { Timber.e(e) try { - ImageCoordinates(file.absolutePath) + ImageCoordinates(file.absolutePath, null) } catch (ex: IOException) { Timber.e(ex) null 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 10e157705..f2c22fa9b 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 @@ -7,6 +7,7 @@ import android.webkit.MimeTypeMap; import androidx.exifinterface.media.ExifInterface; +import fr.free.nrw.commons.location.LatLng; import java.io.BufferedReader; import java.io.File; import java.io.FileInputStream; @@ -64,11 +65,11 @@ public class FileUtils { /** * Get Geolocation of filePath from input filePath path */ - static String getGeolocationOfFile(String filePath) { + static String getGeolocationOfFile(String filePath, LatLng inAppPictureLocation) { try { ExifInterface exifInterface = new ExifInterface(filePath); - ImageCoordinates imageObj = new ImageCoordinates(exifInterface); + ImageCoordinates imageObj = new ImageCoordinates(exifInterface, inAppPictureLocation); if (imageObj.getDecimalCoords() != null) { // If image has geolocation information in its EXIF return imageObj.getDecimalCoords(); } else { diff --git a/app/src/main/java/fr/free/nrw/commons/upload/FileUtilsWrapper.java b/app/src/main/java/fr/free/nrw/commons/upload/FileUtilsWrapper.java index 51bbd02bf..14154a66d 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/FileUtilsWrapper.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/FileUtilsWrapper.java @@ -1,6 +1,7 @@ package fr.free.nrw.commons.upload; import android.content.Context; +import fr.free.nrw.commons.location.LatLng; import io.reactivex.Observable; import java.io.BufferedInputStream; import java.io.File; @@ -37,8 +38,8 @@ public class FileUtilsWrapper { return FileUtils.getFileInputStream(filePath); } - public String getGeolocationOfFile(String filePath) { - return FileUtils.getGeolocationOfFile(filePath); + public String getGeolocationOfFile(String filePath, LatLng inAppPictureLocation) { + return FileUtils.getGeolocationOfFile(filePath, inAppPictureLocation); } 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 7486bb235..3f2ab36a6 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 @@ -9,8 +9,10 @@ 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. + * Otherwise, if current user location is available while using the in-app camera, + * use it to set image coordinates */ -class ImageCoordinates internal constructor(exif: ExifInterface?) { +class ImageCoordinates internal constructor(exif: ExifInterface?, inAppPictureLocation: LatLng?) { var decLatitude = 0.0 var decLongitude = 0.0 var imageCoordsExists = false @@ -26,13 +28,13 @@ class ImageCoordinates internal constructor(exif: ExifInterface?) { /** * Construct from a stream. */ - internal constructor(stream: InputStream) : this(ExifInterface(stream)) + internal constructor(stream: InputStream, inAppPictureLocation: LatLng?) : this(ExifInterface(stream), inAppPictureLocation) /** * 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)) + internal constructor(path: String, inAppPictureLocation: LatLng?) : this(ExifInterface(path), inAppPictureLocation) init { //If image has no EXIF data and user has enabled GPS setting, get user's location @@ -61,6 +63,14 @@ class ImageCoordinates internal constructor(exif: ExifInterface?) { //If image has EXIF data, extract image coords imageCoordsExists = true Timber.d("EXIF data has location info") + } else if (inAppPictureLocation != null) { + decLatitude = inAppPictureLocation.latitude + decLongitude = inAppPictureLocation.longitude + if (!(decLatitude == 0.0 && decLongitude == 0.0)) { + decimalCoords = "$decLatitude|$decLongitude" + imageCoordsExists = true + Timber.d("Image coordinates recorded while using in-app camera") + } } } } diff --git a/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.java b/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.java index 1f31c1690..6aa637ee5 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/ImageProcessingService.java @@ -5,6 +5,7 @@ import static fr.free.nrw.commons.utils.ImageUtils.FILE_NAME_EXISTS; import static fr.free.nrw.commons.utils.ImageUtils.IMAGE_OK; import android.content.Context; +import fr.free.nrw.commons.location.LatLng; import fr.free.nrw.commons.media.MediaClient; import fr.free.nrw.commons.nearby.Place; import fr.free.nrw.commons.utils.ImageUtils; @@ -46,7 +47,7 @@ public class ImageProcessingService { * Check image quality before upload - checks duplicate image - checks dark image - checks * geolocation for image - check for valid title */ - Single validateImage(UploadItem uploadItem) { + Single validateImage(UploadItem uploadItem, LatLng inAppPictureLocation) { int currentImageQuality = uploadItem.getImageQuality(); Timber.d("Current image quality is %d", currentImageQuality); if (currentImageQuality == ImageUtils.IMAGE_KEEP) { @@ -57,7 +58,7 @@ public class ImageProcessingService { return Single.zip( checkDuplicateImage(filePath), - checkImageGeoLocation(uploadItem.getPlace(), filePath), + checkImageGeoLocation(uploadItem.getPlace(), filePath, inAppPictureLocation), checkDarkImage(filePath), validateItemTitle(uploadItem), checkFBMD(filePath), @@ -148,13 +149,13 @@ public class ImageProcessingService { * @param filePath file to be checked * @return IMAGE_GEOLOCATION_DIFFERENT or IMAGE_OK */ - private Single checkImageGeoLocation(Place place, String filePath) { + private Single checkImageGeoLocation(Place place, String filePath, LatLng inAppPictureLocation) { Timber.d("Checking for image geolocation %s", filePath); if (place == null || StringUtils.isBlank(place.getWikiDataEntityId())) { return Single.just(ImageUtils.IMAGE_OK); } return Single.fromCallable(() -> filePath) - .map(fileUtilsWrapper::getGeolocationOfFile) + .flatMap(path -> Single.just(fileUtilsWrapper.getGeolocationOfFile(path, inAppPictureLocation))) .flatMap(geoLocation -> { if (StringUtils.isBlank(geoLocation)) { return Single.just(ImageUtils.IMAGE_OK); diff --git a/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java b/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java index 7fd4ecce7..9770e1ff4 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java @@ -8,6 +8,8 @@ import android.Manifest; import android.annotation.SuppressLint; import android.app.ProgressDialog; import android.content.Intent; +import android.location.Location; +import android.location.LocationManager; import android.os.Bundle; import android.util.DisplayMetrics; import android.view.View; @@ -38,8 +40,12 @@ import fr.free.nrw.commons.contributions.ContributionController; import fr.free.nrw.commons.contributions.MainActivity; import fr.free.nrw.commons.filepicker.UploadableFile; import fr.free.nrw.commons.kvstore.JsonKvStore; +import fr.free.nrw.commons.location.LatLng; +import fr.free.nrw.commons.location.LocationPermissionsHelper; +import fr.free.nrw.commons.location.LocationServiceManager; import fr.free.nrw.commons.mwapi.UserClient; import fr.free.nrw.commons.nearby.Place; +import fr.free.nrw.commons.settings.Prefs; import fr.free.nrw.commons.theme.BaseActivity; import fr.free.nrw.commons.upload.categories.UploadCategoriesFragment; import fr.free.nrw.commons.upload.depicts.DepictsFragment; @@ -57,6 +63,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; +import java.util.Set; import javax.inject.Inject; import javax.inject.Named; import timber.log.Timber; @@ -74,6 +81,8 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, SessionManager sessionManager; @Inject UserClient userClient; + @Inject + LocationServiceManager locationManager; @BindView(R.id.cv_container_top_card) @@ -109,6 +118,9 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, private ThumbnailsAdapter thumbnailsAdapter; private Place place; + private LatLng prevLocation; + private LatLng currLocation; + private boolean isInAppCameraUpload; private List uploadableFiles = Collections.emptyList(); private int currentSelectedPosition = 0; /* @@ -117,6 +129,8 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, private boolean isMultipleFilesSelected = false; public static final String EXTRA_FILES = "commons_image_exta"; + public static final String LOCATION_BEFORE_IMAGE_CAPTURE = "user_location_before_image_capture"; + public static final String IN_APP_CAMERA_UPLOAD = "in_app_camera_upload"; /** * Stores all nearby places found and related users response for @@ -148,6 +162,11 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, if (dpi<=321) { onRlContainerTitleClicked(); } + if (PermissionUtils.hasPermission(this, Manifest.permission.ACCESS_FINE_LOCATION)) { + locationManager.registerLocationManager(); + } + locationManager.requestLocationUpdatesFromProvider(LocationManager.GPS_PROVIDER); + locationManager.requestLocationUpdatesFromProvider(LocationManager.NETWORK_PROVIDER); } private void init() { @@ -366,7 +385,29 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, fragments = new ArrayList<>(); for (UploadableFile uploadableFile : uploadableFiles) { UploadMediaDetailFragment uploadMediaDetailFragment = new UploadMediaDetailFragment(); - uploadMediaDetailFragment.setImageTobeUploaded(uploadableFile, place); + + LocationPermissionsHelper locationPermissionsHelper = new LocationPermissionsHelper( + this, locationManager, null); + if (locationPermissionsHelper.isLocationAccessToAppsTurnedOn()) { + currLocation = locationManager.getLastLocation(); + } + + if (currLocation != null) { + float locationDifference = getLocationDifference(currLocation, prevLocation); + boolean isLocationTagUnchecked = isLocationTagUncheckedInTheSettings(); + /* Remove location if the user has unchecked the Location EXIF tag in the + Manage EXIF Tags setting or turned "Record location for in-app shots" off. + Also, location information is discarded if the difference between + current location and location recorded just before capturing the image + is greater than 100 meters */ + if (isLocationTagUnchecked || locationDifference > 100 + || !defaultKvStore.getBoolean("inAppCameraLocationPref") + || !isInAppCameraUpload) { + currLocation = null; + } + } + uploadMediaDetailFragment.setImageTobeUploaded(uploadableFile, place, currLocation); + locationManager.unregisterLocationManager(); uploadMediaDetailFragment.setCallback(new UploadMediaDetailFragmentCallback() { @Override public void deletePictureAtIndex(int index) { @@ -424,6 +465,39 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, } } + /** + * Users may uncheck Location tag from the Manage EXIF tags setting any time. + * So, their location must not be shared in this case. + * + * @return + */ + private boolean isLocationTagUncheckedInTheSettings() { + Set prefExifTags = defaultKvStore.getStringSet(Prefs.MANAGED_EXIF_TAGS); + if (prefExifTags.contains(getString(R.string.exif_tag_location))) { + return false; + } + return true; + } + + /** + * Calculate the difference between current location and + * location recorded before capturing the image + * + * @param currLocation + * @param prevLocation + * @return + */ + private float getLocationDifference(LatLng currLocation, LatLng prevLocation) { + if (prevLocation == null) { + return 0.0f; + } + float[] distance = new float[2]; + Location.distanceBetween( + currLocation.getLatitude(), currLocation.getLongitude(), + prevLocation.getLatitude(), prevLocation.getLongitude(), distance); + return distance[0]; + } + private void receiveExternalSharedItems() { uploadableFiles = contributionController.handleExternalImagesPicked(this, getIntent()); } @@ -438,6 +512,8 @@ public class UploadActivity extends BaseActivity implements UploadContract.View, Timber.i("Received multiple upload %s", uploadableFiles.size()); place = intent.getParcelableExtra(PLACE_OBJECT); + prevLocation = intent.getParcelableExtra(LOCATION_BEFORE_IMAGE_CAPTURE); + isInAppCameraUpload = intent.getBooleanExtra(IN_APP_CAMERA_UPLOAD, false); resetDirectPrefs(); } 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 369ce0244..9617e95c7 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 @@ -7,6 +7,7 @@ import fr.free.nrw.commons.auth.SessionManager; import fr.free.nrw.commons.contributions.Contribution; import fr.free.nrw.commons.filepicker.UploadableFile; import fr.free.nrw.commons.kvstore.JsonKvStore; +import fr.free.nrw.commons.location.LatLng; import fr.free.nrw.commons.nearby.Place; import fr.free.nrw.commons.settings.Prefs; import fr.free.nrw.commons.upload.depicts.DepictsFragment; @@ -86,18 +87,20 @@ public class UploadModel { */ public Observable preProcessImage(final UploadableFile uploadableFile, final Place place, - final SimilarImageInterface similarImageInterface) { + final SimilarImageInterface similarImageInterface, + LatLng inAppPictureLocation) { return Observable.just( - createAndAddUploadItem(uploadableFile, place, similarImageInterface)); + createAndAddUploadItem(uploadableFile, place, similarImageInterface, inAppPictureLocation)); } - public Single getImageQuality(final UploadItem uploadItem) { - return imageProcessingService.validateImage(uploadItem); + public Single getImageQuality(final UploadItem uploadItem, LatLng inAppPictureLocation) { + return imageProcessingService.validateImage(uploadItem, inAppPictureLocation); } private UploadItem createAndAddUploadItem(final UploadableFile uploadableFile, final Place place, - final SimilarImageInterface similarImageInterface) { + final SimilarImageInterface similarImageInterface, + LatLng inAppPictureLocation) { final UploadableFile.DateTimeWithSource dateTimeWithSource = uploadableFile .getFileCreatedDate(context); long fileCreatedDate = -1; @@ -110,7 +113,8 @@ public class UploadModel { } Timber.d("File created date is %d", fileCreatedDate); final ImageCoordinates imageCoordinates = fileProcessor - .processFileCoordinates(similarImageInterface, uploadableFile.getFilePath()); + .processFileCoordinates(similarImageInterface, uploadableFile.getFilePath(), + inAppPictureLocation); final UploadItem uploadItem = new UploadItem( Uri.parse(uploadableFile.getFilePath()), uploadableFile.getMimeType(context), imageCoordinates, place, fileCreatedDate, 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 09599667d..6f7c7c72b 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 @@ -31,6 +31,8 @@ import fr.free.nrw.commons.LocationPicker.LocationPicker; import fr.free.nrw.commons.R; import fr.free.nrw.commons.filepicker.UploadableFile; import fr.free.nrw.commons.kvstore.JsonKvStore; +import fr.free.nrw.commons.location.LatLng; +import fr.free.nrw.commons.location.LocationServiceManager; import fr.free.nrw.commons.nearby.Place; import fr.free.nrw.commons.recentlanguages.RecentLanguagesDao; import fr.free.nrw.commons.settings.Prefs; @@ -117,6 +119,11 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements */ private Place nearbyPlace; private UploadItem uploadItem; + /** + * inAppPictureLocation: use location recorded while using the in-app camera if + * device camera does not record it in the EXIF + */ + private LatLng inAppPictureLocation; /** * editableUploadItem : Storing the upload item before going to update the coordinates */ @@ -133,9 +140,10 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements super.onCreate(savedInstanceState); } - public void setImageTobeUploaded(UploadableFile uploadableFile, Place place) { + public void setImageTobeUploaded(UploadableFile uploadableFile, Place place, LatLng inAppPictureLocation) { this.uploadableFile = uploadableFile; this.place = place; + this.inAppPictureLocation = inAppPictureLocation; } @Nullable @@ -160,7 +168,7 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements tooltip.setOnClickListener( v -> showInfoAlert(R.string.media_detail_step_title, R.string.media_details_tooltip)); initPresenter(); - presenter.receiveImage(uploadableFile, place); + presenter.receiveImage(uploadableFile, place, inAppPictureLocation); initRecyclerView(); if (callback.getIndexInViewFlipper(this) == 0) { @@ -222,7 +230,7 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements @OnClick(R.id.btn_next) public void onNextButtonClicked() { - presenter.verifyImageQuality(callback.getIndexInViewFlipper(this)); + presenter.verifyImageQuality(callback.getIndexInViewFlipper(this), inAppPictureLocation); } @OnClick(R.id.btn_previous) @@ -448,6 +456,9 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements double defaultLongitude = -122.431297; double defaultZoom = 16.0; + /* Retrieve image location from EXIF if present or + check if user has provided location while using the in-app camera. + Use location of last UploadItem if none of them is available */ if (uploadItem.getGpsCoords() != null && uploadItem.getGpsCoords() .getDecLatitude() != 0.0 && uploadItem.getGpsCoords().getDecLongitude() != 0.0) { defaultLatitude = uploadItem.getGpsCoords() 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 e17a6f4da..99299e2a9 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 @@ -2,6 +2,7 @@ package fr.free.nrw.commons.upload.mediaDetails; import fr.free.nrw.commons.BasePresenter; import fr.free.nrw.commons.filepicker.UploadableFile; +import fr.free.nrw.commons.location.LatLng; import fr.free.nrw.commons.nearby.Place; import fr.free.nrw.commons.upload.ImageCoordinates; import fr.free.nrw.commons.upload.SimilarImageInterface; @@ -43,9 +44,9 @@ public interface UploadMediaDetailsContract { interface UserActionListener extends BasePresenter { - void receiveImage(UploadableFile uploadableFile, Place place); + void receiveImage(UploadableFile uploadableFile, Place place, LatLng inAppPictureLocation); - void verifyImageQuality(int uploadItemIndex); + void verifyImageQuality(int uploadItemIndex, LatLng inAppPictureLocation); void copyTitleAndDescriptionToSubsequentMedia(int indexInViewFlipper); 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 d8e2538cf..04d69e825 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 @@ -87,11 +87,12 @@ public class UploadMediaPresenter implements UserActionListener, SimilarImageInt * @param place */ @Override - public void receiveImage(final UploadableFile uploadableFile, final Place place) { + public void receiveImage(final UploadableFile uploadableFile, final Place place, + LatLng inAppPictureLocation) { view.showProgress(true); compositeDisposable.add( repository - .preProcessImage(uploadableFile, place, this) + .preProcessImage(uploadableFile, place, this, inAppPictureLocation) .map(uploadItem -> { if(place!=null && place.isMonument()){ if (place.location != null) { @@ -177,15 +178,15 @@ public class UploadMediaPresenter implements UserActionListener, SimilarImageInt * @param uploadItemIndex */ @Override - public void verifyImageQuality(int uploadItemIndex) { + public void verifyImageQuality(int uploadItemIndex, LatLng inAppPictureLocation) { final UploadItem uploadItem = repository.getUploads().get(uploadItemIndex); - if (uploadItem.getGpsCoords().getDecimalCoords() == null) { + if (uploadItem.getGpsCoords().getDecimalCoords() == null && inAppPictureLocation == null) { final Runnable onSkipClicked = () -> { view.showProgress(true); compositeDisposable.add( repository - .getImageQuality(uploadItem) + .getImageQuality(uploadItem, inAppPictureLocation) .observeOn(mainThreadScheduler) .subscribe(imageResult -> { view.showProgress(false); @@ -208,7 +209,7 @@ public class UploadMediaPresenter implements UserActionListener, SimilarImageInt view.showProgress(true); compositeDisposable.add( repository - .getImageQuality(uploadItem) + .getImageQuality(uploadItem, inAppPictureLocation) .observeOn(mainThreadScheduler) .subscribe(imageResult -> { view.showProgress(false); diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index e402e0c1c..42a1b70dc 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -190,6 +190,8 @@ Required permission: Read external storage. App cannot access your gallery without this. Required permission: Write external storage. App cannot access your camera/gallery without this. Requesting Location Permission + Record location for in-app shots + Enable this to record location with in-app shots in case the device camera does not record it OK Warning Duplicate File Name found @@ -440,6 +442,13 @@ Upload your first media by tapping on the add button. Ends on: Display campaigns See the ongoing campaigns + Allow the app to fetch location in case the camera does not record it. Some device cameras do not record location. In such cases, letting the app fetch and attach location to it makes your contribution more useful. You may change this any time from the Settings + Allow + Dismiss + Please turn on location access from the Settings and try again. \n\nNote: The upload may not have location if app is unable to retrieve location from device within a short interval. + In-app camera needs location permission to attach it to your images in case location is not available in EXIF. Please allow the app to access your location and try again.\n\nNote: The upload may not have location if app is unable to retrieve location from device within a short interval. + The app would not record location along with in-shots due to lack of location permission + The app would not record location along with in-shots as the GPS is turned off Use document based photo picker The new Android photo picker risks losing location information. Enable if you seem to be using it. Turning this off could trigger the new Android photo picker. It risks losing location information.\n\nTap on \'Read more\' for more information. diff --git a/app/src/main/res/xml/preferences.xml b/app/src/main/res/xml/preferences.xml index f8e786df2..8ac890545 100644 --- a/app/src/main/res/xml/preferences.xml +++ b/app/src/main/res/xml/preferences.xml @@ -19,13 +19,6 @@ - - - - - - + + + + + + android:defaultValue="true" + android:key="useExternalStorage" + app:singleLineTitle="false" + android:summary="@string/use_external_storage_summary" + android:title="@string/use_external_storage" /> + + + + + + + + + +