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 3d9e8442c..609f7ba92 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 @@ -85,7 +85,7 @@ public class FileUtils { static String getGeolocationOfFile(String filePath) { try { - ExifInterface exifInterface=new ExifInterface(filePath); + ExifInterface exifInterface = new ExifInterface(filePath); GPSExtractor imageObj = new GPSExtractor(exifInterface); if (imageObj.imageCoordsExists) { // If image has geolocation information in its EXIF return imageObj.getCoords(); @@ -98,14 +98,27 @@ public class FileUtils { } } + static String createCopyPathAndCopy(boolean useExternalStorage, + Uri uri, + ContentResolver contentResolver, + Context context) throws IOException { + return useExternalStorage ? createExternalCopyPathAndCopy(uri, contentResolver) : + createCopyPathAndCopy(uri, context); + } + /** * In older devices getPath() may fail depending on the source URI. Creating and using a copy of the file seems to work instead. * * @return path of copy */ - @NonNull - static String createExternalCopyPathAndCopy(Uri uri, ContentResolver contentResolver) throws IOException { - FileDescriptor fileDescriptor = contentResolver.openFileDescriptor(uri, "r").getFileDescriptor(); + @Nullable + private static String createExternalCopyPathAndCopy(Uri uri, ContentResolver contentResolver) throws IOException { + ParcelFileDescriptor parcelFileDescriptor = contentResolver.openFileDescriptor(uri, "r"); + if (parcelFileDescriptor == null) { + return null; + } + + FileDescriptor fileDescriptor = parcelFileDescriptor.getFileDescriptor(); String copyPath = Environment.getExternalStorageDirectory().toString() + "/CommonsApp/" + new Date().getTime() + "." + getFileExt(uri, contentResolver); File newFile = new File(Environment.getExternalStorageDirectory().toString() + "/CommonsApp"); newFile.mkdir(); @@ -119,8 +132,8 @@ public class FileUtils { * * @return path of copy */ - @NonNull - static String createCopyPathAndCopy(Uri uri, Context context) throws IOException { + @Nullable + private static String createCopyPathAndCopy(Uri uri, Context context) throws IOException { FileDescriptor fileDescriptor = context.getContentResolver().openFileDescriptor(uri, "r").getFileDescriptor(); String copyPath = context.getCacheDir().getAbsolutePath() + "/" + new Date().getTime() + "." + getFileExt(uri, context.getContentResolver()); FileUtils.copy(fileDescriptor, copyPath); @@ -435,13 +448,13 @@ public class FileUtils { return result; } - static String getFileExt(String fileName){ + static String getFileExt(String fileName) { //Default file extension - String extension=".jpg"; + String extension = ".jpg"; int i = fileName.lastIndexOf('.'); if (i > 0) { - extension = fileName.substring(i+1); + extension = fileName.substring(i + 1); } return extension; } 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 e5119c095..496d74d8d 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 @@ -20,12 +20,14 @@ public class FileUtilsWrapper { } - public String createExternalCopyPathAndCopy(Uri uri, ContentResolver contentResolver) throws IOException { - return FileUtils.createExternalCopyPathAndCopy(uri, contentResolver); - } - - public String createCopyPathAndCopy(Uri uri, Context context) throws IOException { - return FileUtils.createCopyPathAndCopy(uri, context); + public String createCopyPathAndCopy(boolean useExtStorage, + Uri uri, + ContentResolver contentResolver, + Context context) throws IOException { + return FileUtils.createCopyPathAndCopy(useExtStorage, + uri, + contentResolver, + context); } public String getFileExt(String fileName) { 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 6ef528470..cccf4de93 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 @@ -71,7 +71,6 @@ import static fr.free.nrw.commons.wikidata.WikidataConstants.WIKIDATA_ENTITY_ID_ import static fr.free.nrw.commons.wikidata.WikidataConstants.WIKIDATA_ITEM_LOCATION; public class UploadActivity extends AuthenticatedActivity implements UploadView, SimilarImageInterface { - @Inject InputMethodManager inputMethodManager; @Inject MediaWikiApi mwApi; @Inject @Named("direct_nearby_upload_prefs") SharedPreferences directPrefs; @Inject UploadPresenter presenter; @@ -613,6 +612,10 @@ public class UploadActivity extends AuthenticatedActivity implements UploadView, if (Intent.ACTION_SEND.equals(intent.getAction())) { Uri mediaUri = intent.getParcelableExtra(Intent.EXTRA_STREAM); + if(mediaUri == null) { + handleNullMedia(); + return; + } if (intent.getBooleanExtra("isDirectUpload", false)) { String imageTitle = directPrefs.getString("Title", ""); String imageDesc = directPrefs.getString("Desc", ""); @@ -627,10 +630,24 @@ public class UploadActivity extends AuthenticatedActivity implements UploadView, } else if (Intent.ACTION_SEND_MULTIPLE.equals(intent.getAction())) { ArrayList urisList = intent.getParcelableArrayListExtra(Intent.EXTRA_STREAM); Timber.i("Received multiple upload %s", urisList.size()); + + if(urisList.isEmpty()) { + handleNullMedia(); + return; + } presenter.receive(urisList, mimeType, source); } } + /** + * Handle null URI from the received intent. + * Current implementation will simply show a toast and finish the upload activity. + */ + private void handleNullMedia() { + ViewUtil.showLongToast(this, R.string.error_processing_image); + finish(); + } + private void updateCardState(boolean state, ImageView button, View... content) { button.animate().rotation(button.getRotation() + (state ? 180 : -180)).start(); if (content != null) { 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 86f81bac5..a3eabede0 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 @@ -27,6 +27,7 @@ import fr.free.nrw.commons.settings.Prefs; import fr.free.nrw.commons.utils.BitmapRegionDecoderWrapper; import fr.free.nrw.commons.utils.ImageUtils; import fr.free.nrw.commons.utils.ImageUtilsWrapper; +import fr.free.nrw.commons.utils.StringUtils; import io.reactivex.Observable; import io.reactivex.Single; import io.reactivex.disposables.Disposable; @@ -99,7 +100,7 @@ public class UploadModel { initDefaultValues(); Observable itemObservable = Observable.fromIterable(mediaUri) .map(media -> { - currentMediaUri=media; + currentMediaUri = media; return cacheFileUpload(media); }) .map(filePath -> { @@ -107,7 +108,7 @@ public class UploadModel { Uri uri = Uri.fromFile(new File(filePath)); fileProcessor.initFileDetails(filePath, context.getContentResolver()); UploadItem item = new UploadItem(uri, mimeType, source, fileProcessor.processFileCoordinates(similarImageInterface), - fileUtilsWrapper.getFileExt(filePath), null,fileCreatedDate); + fileUtilsWrapper.getFileExt(filePath), null, fileCreatedDate); Single.zip( Single.fromCallable(() -> fileUtilsWrapper.getFileInputStream(filePath)) @@ -136,7 +137,7 @@ public class UploadModel { Uri uri = Uri.fromFile(new File(filePath)); fileProcessor.initFileDetails(filePath, context.getContentResolver()); UploadItem item = new UploadItem(uri, mimeType, source, fileProcessor.processFileCoordinates(similarImageInterface), - fileUtilsWrapper.getFileExt(filePath), wikidataEntityIdPref,fileCreatedDate); + fileUtilsWrapper.getFileExt(filePath), wikidataEntityIdPref, fileCreatedDate); item.title.setTitleText(title); item.descriptions.get(0).setDescriptionText(desc); //TODO figure out if default descriptions in other languages exist @@ -149,7 +150,7 @@ public class UploadModel { .map(b -> b ? ImageUtils.IMAGE_DUPLICATE : ImageUtils.IMAGE_OK), Single.fromCallable(() -> filePath) .map(fileUtilsWrapper::getGeolocationOfFile) - .map(geoLocation -> imageUtilsWrapper.checkImageGeolocationIsDifferent(geoLocation,wikidataItemLocation)) + .map(geoLocation -> imageUtilsWrapper.checkImageGeolocationIsDifferent(geoLocation, wikidataItemLocation)) .map(r -> r ? ImageUtils.IMAGE_GEOLOCATION_DIFFERENT : ImageUtils.IMAGE_OK), Single.fromCallable(() -> fileUtilsWrapper.getFileInputStream(filePath)) @@ -171,6 +172,7 @@ public class UploadModel { /** * Get file creation date from uri from all possible content providers + * * @param media * @return */ @@ -354,23 +356,25 @@ public class UploadModel { /** * Copy files into local storage and return file path - * + * If somehow copy fails, it returns the original path * @param media Uri of the file * @return path of the enw file */ private String cacheFileUpload(Uri media) { + String finalFilePath; try { - String copyPath; - if (useExtStorage) - copyPath = fileUtilsWrapper.createExternalCopyPathAndCopy(media, contentResolver); - else - copyPath = fileUtilsWrapper.createCopyPathAndCopy(media, context); - Timber.i("File path is " + copyPath); - return copyPath; - } catch (IOException e) { - Timber.w(e, "Error in copying URI " + media.getPath()); - return null; + String copyFilePath = fileUtilsWrapper.createCopyPathAndCopy(useExtStorage, media, contentResolver, context); + Timber.i("Copied file path is %s", copyFilePath); + finalFilePath = copyFilePath; + } catch (Exception e) { + Timber.w(e, "Error in copying URI %s. Using original file path instead", media.getPath()); + finalFilePath = media.getPath(); } + + if (StringUtils.isNullOrWhiteSpace(finalFilePath)) { + finalFilePath = media.getPath(); + } + return finalFilePath; } void keepPicture() { @@ -421,7 +425,7 @@ public class UploadModel { this.gpsCoords = gpsCoords; this.fileExt = fileExt; imageQuality = BehaviorSubject.createDefault(ImageUtils.IMAGE_WAIT); - this.createdTimestamp=createdTimestamp; + this.createdTimestamp = createdTimestamp; } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 4737bcc10..2fd4d99da 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -450,6 +450,7 @@ Upload your first media by touching the camera or gallery icon above. This function requires network connection, please check your connection settings. Upload failed due to issues with edit token. Please try logging out and in again. + Error occurred while processing the image. Please try again! Receiving shared content. Processing the image might take some time depending on the size of the image and your device diff --git a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadModelTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadModelTest.kt index ba26f9cba..9b3308817 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadModelTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/upload/UploadModelTest.kt @@ -64,10 +64,8 @@ class UploadModelTest { `when`(context!!.applicationContext) .thenReturn(mock(Application::class.java)) - `when`(fileUtilsWrapper!!.createCopyPathAndCopy(any(Uri::class.java), any(Context::class.java))) + `when`(fileUtilsWrapper!!.createCopyPathAndCopy(anyBoolean(), any(Uri::class.java), nullable(ContentResolver::class.java), any(Context::class.java))) .thenReturn("file.jpg") - `when`(fileUtilsWrapper!!.createExternalCopyPathAndCopy(any(Uri::class.java), any(ContentResolver::class.java))) - .thenReturn("extFile.jpg") `when`(fileUtilsWrapper!!.getFileExt(anyString())) .thenReturn("jpg") `when`(fileUtilsWrapper!!.getSHA1(any(InputStream::class.java))) @@ -111,7 +109,7 @@ class UploadModelTest { fun verifyPreviousNotAvailableForDirectUpload() { val element = mock(Uri::class.java) uploadModel!!.receiveDirect(element, "image/jpeg", "external", "Q1", "Test", "Test", { _, _ -> } - , "") + , "") assertFalse(uploadModel!!.isPreviousAvailable) }