From 0a3b625f442683c66b30dcaafc19939edd15e97a Mon Sep 17 00:00:00 2001 From: Thibault Deckers Date: Fri, 29 May 2020 15:42:34 +0900 Subject: [PATCH] android: reviewed storage access --- .../aves/model/provider/ImageProvider.java | 41 +++++-------------- .../provider/MediaStoreImageProvider.java | 4 +- .../java/deckers/thibault/aves/utils/Env.java | 13 ++++-- .../thibault/aves/utils/StorageUtils.java | 2 +- 4 files changed, 24 insertions(+), 36 deletions(-) diff --git a/android/app/src/main/java/deckers/thibault/aves/model/provider/ImageProvider.java b/android/app/src/main/java/deckers/thibault/aves/model/provider/ImageProvider.java index 1cc5bebd3..091d5ef41 100644 --- a/android/app/src/main/java/deckers/thibault/aves/model/provider/ImageProvider.java +++ b/android/app/src/main/java/deckers/thibault/aves/model/provider/ImageProvider.java @@ -83,8 +83,7 @@ public abstract class ImageProvider { return; } - if (Env.isOnSdCard(activity, oldPath)) { - // rename with DocumentFile + if (Env.requireAccessPermission(oldPath)) { Uri sdCardTreeUri = PermissionManager.getSdCardTreeUri(activity); if (sdCardTreeUri == null) { Runnable runnable = () -> rename(activity, oldPath, oldMediaUri, mimeType, newFilename, callback); @@ -173,21 +172,14 @@ public abstract class ImageProvider { private void rotateJpeg(final Activity activity, final String path, final Uri uri, boolean clockwise, final ImageOpCallback callback) { final String mimeType = MimeTypes.JPEG; - String editablePath = path; - boolean onSdCard = Env.isOnSdCard(activity, path); - if (onSdCard) { + // copy original file to a temporary file for editing + final String editablePath = StorageUtils.copyFileToTemp(path); + if (Env.requireAccessPermission(path)) { if (PermissionManager.getSdCardTreeUri(activity) == null) { Runnable runnable = () -> rotate(activity, path, uri, mimeType, clockwise, callback); new Handler(Looper.getMainLooper()).post(() -> PermissionManager.showSdCardAccessDialog(activity, runnable)); return; } - // copy original file to a temporary file for editing - editablePath = StorageUtils.copyFileToTemp(path); - } - - if (editablePath == null) { - callback.onFailure(); - return; } boolean rotated = false; @@ -211,10 +203,8 @@ public abstract class ImageProvider { exif.setAttribute(ExifInterface.TAG_ORIENTATION, Integer.toString(newOrientationCode)); exif.saveAttributes(); - // if the image is on the SD card, copy the edited temporary file to the original DocumentFile - if (onSdCard) { - DocumentFileCompat.fromFile(new File(editablePath)).copyTo(DocumentFileCompat.fromSingleUri(activity, uri)); - } + // copy the edited temporary file to the original DocumentFile + DocumentFileCompat.fromFile(new File(editablePath)).copyTo(DocumentFileCompat.fromSingleUri(activity, uri)); rotated = true; } catch (IOException e) { Log.w(LOG_TAG, "failed to edit EXIF to rotate image at path=" + path, e); @@ -251,21 +241,14 @@ public abstract class ImageProvider { private void rotatePng(final Activity activity, final String path, final Uri uri, boolean clockwise, final ImageOpCallback callback) { final String mimeType = MimeTypes.PNG; - String editablePath = path; - boolean onSdCard = Env.isOnSdCard(activity, path); - if (onSdCard) { + // copy original file to a temporary file for editing + final String editablePath = StorageUtils.copyFileToTemp(path); + if (Env.requireAccessPermission(path)) { if (PermissionManager.getSdCardTreeUri(activity) == null) { Runnable runnable = () -> rotate(activity, path, uri, mimeType, clockwise, callback); new Handler(Looper.getMainLooper()).post(() -> PermissionManager.showSdCardAccessDialog(activity, runnable)); return; } - // copy original file to a temporary file for editing - editablePath = StorageUtils.copyFileToTemp(path); - } - - if (editablePath == null) { - callback.onFailure(); - return; } Bitmap originalImage = BitmapFactory.decodeFile(path); @@ -284,10 +267,8 @@ public abstract class ImageProvider { try (FileOutputStream fos = new FileOutputStream(editablePath)) { rotatedImage.compress(Bitmap.CompressFormat.PNG, 100, fos); - // if the image is on the SD card, copy the edited temporary file to the original DocumentFile - if (onSdCard) { - DocumentFileCompat.fromFile(new File(editablePath)).copyTo(DocumentFileCompat.fromSingleUri(activity, uri)); - } + // copy the edited temporary file to the original DocumentFile + DocumentFileCompat.fromFile(new File(editablePath)).copyTo(DocumentFileCompat.fromSingleUri(activity, uri)); rotated = true; } catch (IOException e) { Log.e(LOG_TAG, "failed to save rotated image to path=" + path, e); diff --git a/android/app/src/main/java/deckers/thibault/aves/model/provider/MediaStoreImageProvider.java b/android/app/src/main/java/deckers/thibault/aves/model/provider/MediaStoreImageProvider.java index 0769c8d40..f01ef4fb5 100644 --- a/android/app/src/main/java/deckers/thibault/aves/model/provider/MediaStoreImageProvider.java +++ b/android/app/src/main/java/deckers/thibault/aves/model/provider/MediaStoreImageProvider.java @@ -180,7 +180,7 @@ public class MediaStoreImageProvider extends ImageProvider { public ListenableFuture delete(final Activity activity, final String path, final Uri mediaUri) { SettableFuture future = SettableFuture.create(); - if (Env.isOnSdCard(activity, path)) { + if (Env.requireAccessPermission(path)) { Uri sdCardTreeUri = PermissionManager.getSdCardTreeUri(activity); if (sdCardTreeUri == null) { Runnable runnable = () -> { @@ -235,7 +235,7 @@ public class MediaStoreImageProvider extends ImageProvider { if (uuid != null) { // the UUID returned may be uppercase // but it should be lowercase to work with the MediaStore - volumeName = volume.getUuid().toLowerCase(); + volumeName = uuid.toLowerCase(); } } } diff --git a/android/app/src/main/java/deckers/thibault/aves/utils/Env.java b/android/app/src/main/java/deckers/thibault/aves/utils/Env.java index 9b0fdb034..2bad2cbf4 100644 --- a/android/app/src/main/java/deckers/thibault/aves/utils/Env.java +++ b/android/app/src/main/java/deckers/thibault/aves/utils/Env.java @@ -5,6 +5,8 @@ import android.content.Context; import android.content.SharedPreferences; import android.os.Environment; +import androidx.annotation.NonNull; + public class Env { private static String[] mStorageVolumeRoots; private static String mExternalStorage; @@ -38,12 +40,17 @@ public class Env { private static String getExternalStorage() { if (mExternalStorage == null) { - mExternalStorage = Environment.getExternalStorageDirectory().toString(); + mExternalStorage = Environment.getExternalStorageDirectory().getAbsolutePath(); + if (!mExternalStorage.endsWith("/")) { + mExternalStorage += "/"; + } } return mExternalStorage; } - public static boolean isOnSdCard(final Activity activity, String path) { - return path != null && !getExternalStorage().equals(new PathSegments(path, getStorageVolumeRoots(activity)).getStorage()); + public static boolean requireAccessPermission(@NonNull String path) { + boolean onPrimaryVolume = path.startsWith(getExternalStorage()); + // TODO TLAD on Android R, we should require access permission even on primary + return !onPrimaryVolume; } } diff --git a/android/app/src/main/java/deckers/thibault/aves/utils/StorageUtils.java b/android/app/src/main/java/deckers/thibault/aves/utils/StorageUtils.java index 3a2a77c93..801812076 100644 --- a/android/app/src/main/java/deckers/thibault/aves/utils/StorageUtils.java +++ b/android/app/src/main/java/deckers/thibault/aves/utils/StorageUtils.java @@ -212,7 +212,7 @@ public class StorageUtils { @Nullable public static DocumentFileCompat getDocumentFile(Activity activity, @NonNull String path, @NonNull Uri mediaUri) { - if (Env.isOnSdCard(activity, path)) { + if (Env.requireAccessPermission(path)) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { Uri docUri = MediaStore.getDocumentUri(activity, mediaUri); return DocumentFileCompat.fromSingleUri(activity, docUri);