From 3a657c12f0aec56b9965d33f83332ba8100bf3aa Mon Sep 17 00:00:00 2001 From: Thibault Deckers Date: Fri, 29 May 2020 11:39:05 +0900 Subject: [PATCH] android: reviewed storage access --- .../aves/channelhandlers/MetadataHandler.java | 4 + .../aves/channelhandlers/StorageHandler.java | 2 +- .../aves/model/provider/ImageProvider.java | 96 +++++++++--------- .../provider/MediaStoreImageProvider.java | 29 +++--- .../java/deckers/thibault/aves/utils/Env.java | 12 +-- ...{PathComponents.java => PathSegments.java} | 18 ++-- .../thibault/aves/utils/StorageUtils.java | 99 +++++++------------ 7 files changed, 122 insertions(+), 138 deletions(-) rename android/app/src/main/java/deckers/thibault/aves/utils/{PathComponents.java => PathSegments.java} (51%) diff --git a/android/app/src/main/java/deckers/thibault/aves/channelhandlers/MetadataHandler.java b/android/app/src/main/java/deckers/thibault/aves/channelhandlers/MetadataHandler.java index dfd950e67..4e4588db3 100644 --- a/android/app/src/main/java/deckers/thibault/aves/channelhandlers/MetadataHandler.java +++ b/android/app/src/main/java/deckers/thibault/aves/channelhandlers/MetadataHandler.java @@ -5,6 +5,7 @@ import android.content.Context; import android.database.Cursor; import android.media.MediaMetadataRetriever; import android.net.Uri; +import android.os.Build; import android.provider.MediaStore; import android.text.format.Formatter; @@ -351,6 +352,9 @@ public class MetadataHandler implements MethodChannel.MethodCallHandler { } else if (mimeType.startsWith(MimeTypes.VIDEO)) { contentUri = ContentUris.withAppendedId(MediaStore.Video.Media.EXTERNAL_CONTENT_URI, id); } + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + contentUri = MediaStore.setRequireOriginal(contentUri); + } Cursor cursor = context.getContentResolver().query(contentUri, null, null, null, null); if (cursor != null && cursor.moveToFirst()) { diff --git a/android/app/src/main/java/deckers/thibault/aves/channelhandlers/StorageHandler.java b/android/app/src/main/java/deckers/thibault/aves/channelhandlers/StorageHandler.java index 8f283ba98..b321fbcbe 100644 --- a/android/app/src/main/java/deckers/thibault/aves/channelhandlers/StorageHandler.java +++ b/android/app/src/main/java/deckers/thibault/aves/channelhandlers/StorageHandler.java @@ -59,7 +59,7 @@ public class StorageHandler implements MethodChannel.MethodCallHandler { List> volumes = new ArrayList<>(); StorageManager sm = activity.getSystemService(StorageManager.class); if (sm != null) { - for (String path : Env.getStorageVolumes(activity)) { + for (String path : Env.getStorageVolumeRoots(activity)) { try { File file = new File(path); StorageVolume volume = sm.getStorageVolume(file); 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 3b4716959..1cc5bebd3 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 @@ -13,7 +13,6 @@ import android.net.Uri; import android.os.Build; import android.os.Handler; import android.os.Looper; -import android.os.ParcelFileDescriptor; import android.provider.MediaStore; import android.util.Log; @@ -21,6 +20,7 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.exifinterface.media.ExifInterface; +import com.commonsware.cwac.document.DocumentFileCompat; import com.drew.imaging.ImageMetadataReader; import com.drew.imaging.ImageProcessingException; import com.drew.metadata.Metadata; @@ -29,7 +29,6 @@ import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import java.io.File; -import java.io.FileDescriptor; import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; @@ -44,6 +43,15 @@ import deckers.thibault.aves.utils.PermissionManager; import deckers.thibault.aves.utils.StorageUtils; import deckers.thibault.aves.utils.Utils; +// *** about file access to write/rename/delete +// * primary volume +// until 28/Pie, use `File` +// on 29/Q, use `File` after setting `requestLegacyExternalStorage` flag in the manifest +// from 30/R, use `DocumentFile` (not `File`) after requesting permission to the volume root??? +// * non primary volumes +// on 19/KitKat, use `DocumentFile` (not `File`) after getting permission for each file +// from 21/Lollipop, use `DocumentFile` (not `File`) after getting permission to the volume root + public abstract class ImageProvider { private static final String LOG_TAG = Utils.createLogTag(ImageProvider.class); @@ -59,9 +67,9 @@ public abstract class ImageProvider { return Futures.immediateFailedFuture(new UnsupportedOperationException()); } - public void rename(final Activity activity, final String oldPath, final Uri oldUri, final String mimeType, final String newFilename, final ImageOpCallback callback) { + public void rename(final Activity activity, final String oldPath, final Uri oldMediaUri, final String mimeType, final String newFilename, final ImageOpCallback callback) { if (oldPath == null) { - Log.w(LOG_TAG, "entry does not have a path, uri=" + oldUri); + Log.w(LOG_TAG, "entry does not have a path, uri=" + oldMediaUri); callback.onFailure(); return; } @@ -75,26 +83,26 @@ public abstract class ImageProvider { return; } - // Before KitKat, we do whatever we want on the SD card. - // From KitKat, we need access permission from the Document Provider, at the file level. - // From Lollipop, we can request the permission at the SD card root level. - boolean renamed; if (Env.isOnSdCard(activity, oldPath)) { // rename with DocumentFile Uri sdCardTreeUri = PermissionManager.getSdCardTreeUri(activity); if (sdCardTreeUri == null) { - Runnable runnable = () -> rename(activity, oldPath, oldUri, mimeType, newFilename, callback); + Runnable runnable = () -> rename(activity, oldPath, oldMediaUri, mimeType, newFilename, callback); new Handler(Looper.getMainLooper()).post(() -> PermissionManager.showSdCardAccessDialog(activity, runnable)); return; } - renamed = StorageUtils.renameOnSdCard(activity, sdCardTreeUri, Env.getStorageVolumes(activity), oldPath, newFilename); - } else { - // rename with File - renamed = oldFile.renameTo(newFile); } - if (!renamed) { - Log.w(LOG_TAG, "failed to rename entry at path=" + oldPath); + DocumentFileCompat df = StorageUtils.getDocumentFile(activity, oldPath, oldMediaUri); + try { + boolean renamed = df != null && df.renameTo(newFilename); + if (!renamed) { + Log.w(LOG_TAG, "failed to rename entry at path=" + oldPath); + callback.onFailure(); + return; + } + } catch (FileNotFoundException e) { + Log.w(LOG_TAG, "failed to rename entry at path=" + oldPath, e); callback.onFailure(); return; } @@ -204,7 +212,10 @@ public abstract class ImageProvider { exif.saveAttributes(); // if the image is on the SD card, copy the edited temporary file to the original DocumentFile - rotated = !onSdCard || StorageUtils.writeToDocumentFile(activity, editablePath, uri); + if (onSdCard) { + 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); } @@ -227,7 +238,8 @@ public abstract class ImageProvider { values.clear(); values.put(MediaStore.MediaColumns.IS_PENDING, 0); } - values.put(MediaStore.MediaColumns.ORIENTATION, orientationDegrees); + // uses MediaStore.Images.Media instead of MediaStore.MediaColumns for APIs < Q + values.put(MediaStore.Images.Media.ORIENTATION, orientationDegrees); int updatedRowCount = contentResolver.update(uri, values, null, null); if (updatedRowCount > 0) { MediaScannerConnection.scanFile(activity, new String[]{path}, new String[]{mimeType}, (p, u) -> callback.onSuccess(newFields)); @@ -239,15 +251,20 @@ 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; - if (path == null) { - callback.onFailure(); - return; + String editablePath = path; + boolean onSdCard = Env.isOnSdCard(activity, path); + if (onSdCard) { + 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); } - boolean onSdCard = Env.isOnSdCard(activity, path); - if (onSdCard && PermissionManager.getSdCardTreeUri(activity) == null) { - Runnable runnable = () -> rotate(activity, path, uri, mimeType, clockwise, callback); - new Handler(Looper.getMainLooper()).post(() -> PermissionManager.showSdCardAccessDialog(activity, runnable)); + if (editablePath == null) { + callback.onFailure(); return; } @@ -264,29 +281,16 @@ public abstract class ImageProvider { Bitmap rotatedImage = Bitmap.createBitmap(originalImage, 0, 0, originalWidth, originalHeight, matrix, true); boolean rotated = false; - if (onSdCard) { - FileDescriptor fd = null; - try { - ParcelFileDescriptor pfd = activity.getContentResolver().openFileDescriptor(uri, "rw"); - if (pfd != null) fd = pfd.getFileDescriptor(); - } catch (FileNotFoundException e) { - Log.e(LOG_TAG, "failed to get file descriptor for document at uri=" + path, e); - } - if (fd != null) { - try (FileOutputStream fos = new FileOutputStream(fd)) { - rotatedImage.compress(Bitmap.CompressFormat.PNG, 100, fos); - rotated = true; - } catch (IOException e) { - Log.e(LOG_TAG, "failed to save rotated image to document at uri=" + path, e); - } - } - } else { - try (FileOutputStream fos = new FileOutputStream(path)) { - rotatedImage.compress(Bitmap.CompressFormat.PNG, 100, fos); - rotated = true; - } catch (IOException e) { - Log.e(LOG_TAG, "failed to save rotated image to path=" + path, e); + 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)); } + rotated = true; + } catch (IOException e) { + Log.e(LOG_TAG, "failed to save rotated image to path=" + path, e); } if (!rotated) { callback.onFailure(); 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 acc01d19e..0769c8d40 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 @@ -22,6 +22,7 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.SettableFuture; import java.io.File; +import java.io.FileNotFoundException; import java.util.HashMap; import java.util.Map; import java.util.stream.Stream; @@ -29,7 +30,6 @@ import java.util.stream.Stream; import deckers.thibault.aves.model.ImageEntry; import deckers.thibault.aves.utils.Env; import deckers.thibault.aves.utils.MimeTypes; -import deckers.thibault.aves.utils.PathComponents; import deckers.thibault.aves.utils.PermissionManager; import deckers.thibault.aves.utils.StorageUtils; import deckers.thibault.aves.utils.Utils; @@ -176,13 +176,8 @@ public class MediaStoreImageProvider extends ImageProvider { return !MimeTypes.SVG.equals(mimeType); } - // check write access permission to SD card - // Before KitKat, we do whatever we want on the SD card. - // From KitKat, we need access permission from the Document Provider, at the file level. - // From Lollipop, we can request the permission at the SD card root level. - @Override - public ListenableFuture delete(final Activity activity, final String path, final Uri uri) { + public ListenableFuture delete(final Activity activity, final String path, final Uri mediaUri) { SettableFuture future = SettableFuture.create(); if (Env.isOnSdCard(activity, path)) { @@ -190,7 +185,7 @@ public class MediaStoreImageProvider extends ImageProvider { if (sdCardTreeUri == null) { Runnable runnable = () -> { try { - future.set(delete(activity, path, uri).get()); + future.set(delete(activity, path, mediaUri).get()); } catch (Exception e) { future.setException(e); } @@ -201,15 +196,20 @@ public class MediaStoreImageProvider extends ImageProvider { // if the file is on SD card, calling the content resolver delete() removes the entry from the Media Store // but it doesn't delete the file, even if the app has the permission - StorageUtils.deleteFromSdCard(activity, sdCardTreeUri, Env.getStorageVolumes(activity), path); - Log.d(LOG_TAG, "deleted from SD card at path=" + uri); - future.set(null); + try { + DocumentFileCompat df = StorageUtils.getDocumentFile(activity, path, mediaUri); + if (df != null && df.delete()) { + future.set(null); + future.setException(new Exception("failed to delete file with df=" + df)); + } + } catch (FileNotFoundException e) { + future.setException(e); + } return future; } try { - if (activity.getContentResolver().delete(uri, null, null) > 0) { - Log.d(LOG_TAG, "deleted from content resolver uri=" + uri); + if (activity.getContentResolver().delete(mediaUri, null, null) > 0) { future.set(null); } else { future.setException(new Exception("failed to delete row from content provider")); @@ -247,8 +247,7 @@ public class MediaStoreImageProvider extends ImageProvider { // DocumentFile.getParentFile() is null without picking a tree first // DocumentsContract.copyDocument() and moveDocument() need parent doc uri - PathComponents sourcePathComponents = new PathComponents(sourcePath, Env.getStorageVolumes(activity)); - String destinationPath = destinationDir + File.separator + sourcePathComponents.getFilename(); + String destinationPath = destinationDir + File.separator + new File(sourcePath).getName(); ContentValues contentValues = new ContentValues(); contentValues.put(MediaStore.MediaColumns.DATA, destinationPath); 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 f44f075d2..9b0fdb034 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 @@ -6,7 +6,7 @@ import android.content.SharedPreferences; import android.os.Environment; public class Env { - private static String[] mStorageVolumes; + private static String[] mStorageVolumeRoots; private static String mExternalStorage; // SD card path as a content URI from the Documents Provider // e.g. content://com.android.externalstorage.documents/tree/12A9-8B42%3A @@ -29,11 +29,11 @@ public class Env { return mSdCardDocumentUri; } - public static String[] getStorageVolumes(final Activity activity) { - if (mStorageVolumes == null) { - mStorageVolumes = StorageUtils.getStorageVolumes(activity); + public static String[] getStorageVolumeRoots(final Activity activity) { + if (mStorageVolumeRoots == null) { + mStorageVolumeRoots = StorageUtils.getStorageVolumeRoots(activity); } - return mStorageVolumes; + return mStorageVolumeRoots; } private static String getExternalStorage() { @@ -44,6 +44,6 @@ public class Env { } public static boolean isOnSdCard(final Activity activity, String path) { - return path != null && !getExternalStorage().equals(new PathComponents(path, getStorageVolumes(activity)).getStorage()); + return path != null && !getExternalStorage().equals(new PathSegments(path, getStorageVolumeRoots(activity)).getStorage()); } } diff --git a/android/app/src/main/java/deckers/thibault/aves/utils/PathComponents.java b/android/app/src/main/java/deckers/thibault/aves/utils/PathSegments.java similarity index 51% rename from android/app/src/main/java/deckers/thibault/aves/utils/PathComponents.java rename to android/app/src/main/java/deckers/thibault/aves/utils/PathSegments.java index b1fca3ce8..2567c2f31 100644 --- a/android/app/src/main/java/deckers/thibault/aves/utils/PathComponents.java +++ b/android/app/src/main/java/deckers/thibault/aves/utils/PathSegments.java @@ -4,22 +4,22 @@ import androidx.annotation.NonNull; import java.io.File; -public class PathComponents { +public class PathSegments { private String storage; - private String folder; + private String relativePath; private String filename; - public PathComponents(@NonNull String path, @NonNull String[] storageVolumes) { - for (int i = 0; i < storageVolumes.length && storage == null; i++) { - if (path.startsWith(storageVolumes[i])) { - storage = storageVolumes[i]; + public PathSegments(@NonNull String path, @NonNull String[] storageVolumePaths) { + for (int i = 0; i < storageVolumePaths.length && storage == null; i++) { + if (path.startsWith(storageVolumePaths[i])) { + storage = storageVolumePaths[i]; } } int lastSeparatorIndex = path.lastIndexOf(File.separator) + 1; if (lastSeparatorIndex > storage.length()) { filename = path.substring(lastSeparatorIndex); - folder = path.substring(storage.length(), lastSeparatorIndex); + relativePath = path.substring(storage.length(), lastSeparatorIndex); } } @@ -27,8 +27,8 @@ public class PathComponents { return storage; } - public String getFolder() { - return folder; + public String getRelativePath() { + return relativePath; } public String getFilename() { 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 e795a1fb3..3a2a77c93 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 @@ -1,20 +1,22 @@ package deckers.thibault.aves.utils; import android.annotation.SuppressLint; +import android.app.Activity; import android.content.ContentResolver; import android.content.Context; import android.media.MediaMetadataRetriever; import android.net.Uri; import android.os.Build; import android.os.Environment; -import android.os.ParcelFileDescriptor; import android.provider.MediaStore; import android.text.TextUtils; import android.util.Log; import android.webkit.MimeTypeMap; -import androidx.documentfile.provider.DocumentFile; +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import com.commonsware.cwac.document.DocumentFileCompat; import com.google.common.base.Splitter; import com.google.common.collect.Lists; @@ -85,7 +87,7 @@ public class StorageUtils { * @return paths to all available SD-Cards in the system (include emulated) */ @SuppressLint("ObsoleteSdkInt") - public static String[] getStorageVolumes(Context context) { + public static String[] getStorageVolumeRoots(Context context) { // Final set of paths final Set rv = new HashSet<>(); @@ -182,36 +184,47 @@ public class StorageUtils { }; } - private static Optional getSdCardDocumentFile(Context context, Uri sdCardTreeUri, String[] storageVolumes, String path) { - if (sdCardTreeUri == null || storageVolumes == null || path == null) { + private static Optional getSdCardDocumentFile(Context context, Uri rootTreeUri, String[] storageVolumeRoots, String path) { + if (rootTreeUri == null || storageVolumeRoots == null || path == null) { return Optional.empty(); } - PathComponents pathComponents = new PathComponents(path, storageVolumes); - ArrayList pathSegments = Lists.newArrayList(Splitter.on(File.separatorChar) - .trimResults().omitEmptyStrings().split(pathComponents.getFolder())); - pathSegments.add(pathComponents.getFilename()); - Iterator pathIterator = pathSegments.iterator(); + PathSegments pathSegments = new PathSegments(path, storageVolumeRoots); + ArrayList pathSteps = Lists.newArrayList(Splitter.on(File.separatorChar) + .trimResults().omitEmptyStrings().split(pathSegments.getRelativePath())); + pathSteps.add(pathSegments.getFilename()); + Iterator pathIterator = pathSteps.iterator(); + DocumentFileCompat documentFile = DocumentFileCompat.fromTreeUri(context, rootTreeUri); + if (documentFile == null) { + return Optional.empty(); + } // follow the entry path down the document tree - boolean found = true; - DocumentFile documentFile = DocumentFile.fromTreeUri(context, sdCardTreeUri); - while (pathIterator.hasNext() && found) { - String segment = pathIterator.next(); - found = false; - if (documentFile != null) { - DocumentFile[] children = documentFile.listFiles(); - for (int i = children.length - 1; i >= 0 && !found; i--) { - DocumentFile child = children[i]; - if (segment.equals(child.getName())) { - found = true; - documentFile = child; - } - } + while (pathIterator.hasNext()) { + documentFile = documentFile.findFile(pathIterator.next()); + if (documentFile == null) { + return Optional.empty(); } } + return Optional.of(documentFile); + } - return found && documentFile != null ? Optional.of(documentFile) : Optional.empty(); + + @Nullable + public static DocumentFileCompat getDocumentFile(Activity activity, @NonNull String path, @NonNull Uri mediaUri) { + if (Env.isOnSdCard(activity, path)) { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + Uri docUri = MediaStore.getDocumentUri(activity, mediaUri); + return DocumentFileCompat.fromSingleUri(activity, docUri); + } else { + Uri sdCardTreeUri = PermissionManager.getSdCardTreeUri(activity); + String[] storageVolumeRoots = Env.getStorageVolumeRoots(activity); + Optional docFile = StorageUtils.getSdCardDocumentFile(activity, sdCardTreeUri, storageVolumeRoots, path); + return docFile.orElse(null); + } + } else { + return DocumentFileCompat.fromFile(new File(path)); + } } public static String copyFileToTemp(String path) { @@ -226,40 +239,4 @@ public class StorageUtils { } return null; } - - public static boolean writeToDocumentFile(Context context, String from, Uri documentUri) { - try { - ParcelFileDescriptor pfd = context.getContentResolver().openFileDescriptor(documentUri, "rw"); - if (pfd == null) { - Log.w(LOG_TAG, "failed to get file descriptor for documentUri=" + documentUri); - return false; - } - Utils.copyFile(new File(from), pfd.getFileDescriptor()); - return true; - } catch (IOException e) { - Log.w(LOG_TAG, "failed to write to DocumentFile at documentUri=" + documentUri); - } - return false; - } - - /** - * Delete the specified file on SD card - * Note that it does not update related content providers such as the Media Store. - */ - public static boolean deleteFromSdCard(Context context, Uri sdCardTreeUri, String[] storageVolumes, String path) { - Optional documentFile = getSdCardDocumentFile(context, sdCardTreeUri, storageVolumes, path); - boolean success = documentFile.isPresent() && documentFile.get().delete(); - Log.d(LOG_TAG, "deleteFromSdCard success=" + success + " for sdCardTreeUri=" + sdCardTreeUri + ", path=" + path); - return success; - } - - /** - * Rename the specified file on SD card - * Note that it does not update related content providers such as the Media Store. - */ - public static boolean renameOnSdCard(Context context, Uri sdCardTreeUri, String[] storageVolumes, String path, String newFilename) { - Log.d(LOG_TAG, "renameOnSdCard with path=" + path + ", newFilename=" + newFilename); - Optional documentFile = getSdCardDocumentFile(context, sdCardTreeUri, storageVolumes, path); - return documentFile.isPresent() && documentFile.get().renameTo(newFilename); - } }