Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[image_picker] Change storage location for camera captures to internal cache on Android, to comply with new Google Play storage requirements. #3956

Merged
merged 20 commits into from
Jun 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
7be1c7c
Switch to app-specific folder for writing camera captures.
BeMacized May 21, 2021
d79bac3
Merge branch 'master' into issue/80502
BeMacized May 21, 2021
df306e2
Update documentation
BeMacized May 21, 2021
467bb5c
Update targetSdkVersion for example. Update tests.
BeMacized May 21, 2021
b2e902e
Updated changelog and pubspec version
BeMacized May 21, 2021
84a5ad1
Update packages/image_picker/image_picker/android/src/main/AndroidMan…
BeMacized May 21, 2021
46df81f
Update packages/image_picker/image_picker/example/android/app/src/tes…
BeMacized May 21, 2021
15d841d
Update packages/image_picker/image_picker/android/src/main/res/xml/fl…
BeMacized May 21, 2021
e1cb24e
Implemented PR feedback
BeMacized May 21, 2021
eafa023
Add test to check if file is created in cache directory
BeMacized May 26, 2021
aa3c24d
Upgrade PR to breaking change
BeMacized May 26, 2021
a3c737c
Update packages/image_picker/image_picker/CHANGELOG.md
BeMacized May 26, 2021
bc3e4d5
Update CHANGELOG.md
BeMacized May 26, 2021
121a7ca
Merge branch 'master' into issue/80502
BeMacized May 26, 2021
1143108
Removed redundant robolectric.properties
BeMacized May 26, 2021
bfb2b6a
Fix merge issue
BeMacized May 26, 2021
e973353
Removed WRITE_EXTERNAL_STORAGE permission completely
BeMacized Jun 1, 2021
ca846ba
Remove READ_EXTERNAL_STORAGE permission checks
BeMacized Jun 1, 2021
fa3c577
Ran format and removed redundant unit tests
BeMacized Jun 1, 2021
80906ca
Merge branch 'master' into issue/80502
BeMacized Jun 1, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions packages/image_picker/image_picker/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
## 0.8.0

* BREAKING CHANGE: Changed storage location for captured images and videos to internal cache on Android,
to comply with new Google Play storage requirements. This means developers are responsible for moving
the image or video to a different location in case more permanent storage is required. Other applications
will no longer be able to access images or videos captured unless they are moved to a publicly accessible location.
* Updated Mockito to fix Android tests.

## 0.7.5+4
* Migrate maven repo from jcenter to mavenCentral.

Expand Down
6 changes: 3 additions & 3 deletions packages/image_picker/image_picker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ Add the following keys to your _Info.plist_ file, located in `<project root>/ios

### Android

#### API < 29
No configuration required - the plugin should work out of the box.

#### API 29+
It is no longer required to add `android:requestLegacyExternalStorage="true"` as an attribute to the `<application>` tag in AndroidManifest.xml, as `image_picker` has been updated to make use of scoped storage.

Add `android:requestLegacyExternalStorage="true"` as an attribute to the `<application>` tag in AndroidManifest.xml. The [attribute](https://developer.android.com/training/data-storage/compatibility) is `false` by default on apps targeting Android Q.
**Note:** Images and videos picked using the camera are saved to your application's local cache, and should therefore be expected to only be around temporarily.
If you require your picked image to be stored permanently, it is your responsibility to move it to a more permanent location.

### Example

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="io.flutter.plugins.imagepicker">
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE"/>
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/>
package="io.flutter.plugins.imagepicker">

<application>
<provider
Expand All @@ -11,7 +9,7 @@
android:grantUriPermissions="true">
<meta-data
android:name="android.support.FILE_PROVIDER_PATHS"
android:resource="@xml/flutter_image_picker_file_paths"/>
android:resource="@xml/flutter_image_picker_file_paths" />
</provider>
</application>
</manifest>
</manifest>
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,8 @@ enum CameraDevice {
* means that the chooseImageFromGallery() or takeImageWithCamera() method was called at least
* twice. In this case, stop executing and finish with an error.
*
* <p>2. Check that a required runtime permission has been granted. The chooseImageFromGallery()
* method checks if the {@link Manifest.permission#READ_EXTERNAL_STORAGE} permission has been
* granted. Similarly, the takeImageWithCamera() method checks that {@link
* Manifest.permission#CAMERA} has been granted.
* <p>2. Check that a required runtime permission has been granted. The takeImageWithCamera() method
* checks that {@link Manifest.permission#CAMERA} has been granted.
*
* <p>The permission check can end up in two different outcomes:
*
Expand Down Expand Up @@ -76,17 +74,15 @@ public class ImagePickerDelegate
PluginRegistry.RequestPermissionsResultListener {
@VisibleForTesting static final int REQUEST_CODE_CHOOSE_IMAGE_FROM_GALLERY = 2342;
@VisibleForTesting static final int REQUEST_CODE_TAKE_IMAGE_WITH_CAMERA = 2343;
@VisibleForTesting static final int REQUEST_EXTERNAL_IMAGE_STORAGE_PERMISSION = 2344;
@VisibleForTesting static final int REQUEST_CAMERA_IMAGE_PERMISSION = 2345;
@VisibleForTesting static final int REQUEST_CODE_CHOOSE_VIDEO_FROM_GALLERY = 2352;
@VisibleForTesting static final int REQUEST_CODE_TAKE_VIDEO_WITH_CAMERA = 2353;
@VisibleForTesting static final int REQUEST_EXTERNAL_VIDEO_STORAGE_PERMISSION = 2354;
@VisibleForTesting static final int REQUEST_CAMERA_VIDEO_PERMISSION = 2355;

@VisibleForTesting final String fileProviderName;

private final Activity activity;
private final File externalFilesDirectory;
@VisibleForTesting final File externalFilesDirectory;
private final ImageResizer imageResizer;
private final ImagePickerCache cache;
private final PermissionManager permissionManager;
Expand Down Expand Up @@ -257,12 +253,6 @@ public void chooseVideoFromGallery(MethodCall methodCall, MethodChannel.Result r
return;
}

if (!permissionManager.isPermissionGranted(Manifest.permission.READ_EXTERNAL_STORAGE)) {
permissionManager.askForPermission(
Manifest.permission.READ_EXTERNAL_STORAGE, REQUEST_EXTERNAL_VIDEO_STORAGE_PERMISSION);
return;
}

launchPickVideoFromGalleryIntent();
}

Expand Down Expand Up @@ -322,12 +312,6 @@ public void chooseImageFromGallery(MethodCall methodCall, MethodChannel.Result r
return;
}

if (!permissionManager.isPermissionGranted(Manifest.permission.READ_EXTERNAL_STORAGE)) {
permissionManager.askForPermission(
Manifest.permission.READ_EXTERNAL_STORAGE, REQUEST_EXTERNAL_IMAGE_STORAGE_PERMISSION);
return;
}

launchPickImageFromGalleryIntent();
}

Expand Down Expand Up @@ -424,16 +408,6 @@ public boolean onRequestPermissionsResult(
grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED;

switch (requestCode) {
case REQUEST_EXTERNAL_IMAGE_STORAGE_PERMISSION:
if (permissionGranted) {
launchPickImageFromGalleryIntent();
}
break;
case REQUEST_EXTERNAL_VIDEO_STORAGE_PERMISSION:
if (permissionGranted) {
launchPickVideoFromGalleryIntent();
}
break;
case REQUEST_CAMERA_IMAGE_PERMISSION:
if (permissionGranted) {
launchTakeImageWithCameraIntent();
Expand All @@ -450,10 +424,6 @@ public boolean onRequestPermissionsResult(

if (!permissionGranted) {
switch (requestCode) {
case REQUEST_EXTERNAL_IMAGE_STORAGE_PERMISSION:
case REQUEST_EXTERNAL_VIDEO_STORAGE_PERMISSION:
finishWithError("photo_access_denied", "The user did not allow photo access.");
break;
case REQUEST_CAMERA_IMAGE_PERMISSION:
case REQUEST_CAMERA_VIDEO_PERMISSION:
finishWithError("camera_access_denied", "The user did not allow camera access.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import android.app.Activity;
import android.app.Application;
import android.os.Bundle;
import android.os.Environment;
import android.os.Handler;
import android.os.Looper;
import androidx.annotation.NonNull;
Expand Down Expand Up @@ -216,11 +215,11 @@ private void tearDown() {
application = null;
}

private final ImagePickerDelegate constructDelegate(final Activity setupActivity) {
@VisibleForTesting
final ImagePickerDelegate constructDelegate(final Activity setupActivity) {
final ImagePickerCache cache = new ImagePickerCache(setupActivity);

final File externalFilesDirectory =
setupActivity.getExternalFilesDir(Environment.DIRECTORY_PICTURES);
final File externalFilesDirectory = setupActivity.getCacheDir();
final ExifDataCopier exifDataCopier = new ExifDataCopier();
final ImageResizer imageResizer = new ImageResizer(externalFilesDirectory, exifDataCopier);
return new ImagePickerDelegate(setupActivity, externalFilesDirectory, imageResizer, cache);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<paths>
<external-path name="external_files" path="."/>
</paths>
<cache-path name="cached_files" path="."/>
</paths>
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ flutter {

dependencies {
testImplementation 'junit:junit:4.12'
testImplementation 'org.mockito:mockito-core:2.17.0'
testImplementation 'org.mockito:mockito-core:3.10.0'
androidTestImplementation 'androidx.test:runner:1.1.1'
androidTestImplementation 'androidx.test.espresso:espresso-core:3.1.1'
testImplementation 'androidx.test:core:1.2.0'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
Expand All @@ -25,6 +26,8 @@
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;

public class ImagePickerDelegateTest {
Expand Down Expand Up @@ -100,20 +103,6 @@ public void chooseImageFromGallery_WhenPendingResultExists_FinishesWithAlreadyAc
verifyNoMoreInteractions(mockResult);
}

@Test
public void chooseImageFromGallery_WhenHasNoExternalStoragePermission_RequestsForPermission() {
when(mockPermissionManager.isPermissionGranted(Manifest.permission.READ_EXTERNAL_STORAGE))
.thenReturn(false);

ImagePickerDelegate delegate = createDelegate();
delegate.chooseImageFromGallery(mockMethodCall, mockResult);

verify(mockPermissionManager)
.askForPermission(
Manifest.permission.READ_EXTERNAL_STORAGE,
ImagePickerDelegate.REQUEST_EXTERNAL_IMAGE_STORAGE_PERMISSION);
}

@Test
public void
chooseImageFromGallery_WhenHasExternalStoragePermission_LaunchesChooseFromGalleryIntent() {
Expand Down Expand Up @@ -193,47 +182,21 @@ public void takeImageWithCamera_WhenCameraPermissionNotPresent_RequestsForPermis
}

@Test
public void
onRequestPermissionsResult_WhenReadExternalStoragePermissionDenied_FinishesWithError() {
ImagePickerDelegate delegate = createDelegateWithPendingResultAndMethodCall();

delegate.onRequestPermissionsResult(
ImagePickerDelegate.REQUEST_EXTERNAL_IMAGE_STORAGE_PERMISSION,
new String[] {Manifest.permission.READ_EXTERNAL_STORAGE},
new int[] {PackageManager.PERMISSION_DENIED});

verify(mockResult).error("photo_access_denied", "The user did not allow photo access.", null);
verifyNoMoreInteractions(mockResult);
}

@Test
public void
onRequestChooseImagePermissionsResult_WhenReadExternalStorageGranted_LaunchesChooseImageFromGalleryIntent() {
ImagePickerDelegate delegate = createDelegateWithPendingResultAndMethodCall();
public void takeImageWithCamera_WritesImageToCacheDirectory() {
when(mockPermissionManager.isPermissionGranted(Manifest.permission.CAMERA)).thenReturn(true);
when(mockIntentResolver.resolveActivity(any(Intent.class))).thenReturn(true);

delegate.onRequestPermissionsResult(
ImagePickerDelegate.REQUEST_EXTERNAL_IMAGE_STORAGE_PERMISSION,
new String[] {Manifest.permission.READ_EXTERNAL_STORAGE},
new int[] {PackageManager.PERMISSION_GRANTED});
MockedStatic<File> mockStaticFile = Mockito.mockStatic(File.class);
mockStaticFile
.when(() -> File.createTempFile(any(), any(), any()))
.thenReturn(new File("/tmpfile"));

verify(mockActivity)
.startActivityForResult(
any(Intent.class), eq(ImagePickerDelegate.REQUEST_CODE_CHOOSE_IMAGE_FROM_GALLERY));
}

@Test
public void
onRequestChooseVideoPermissionsResult_WhenReadExternalStorageGranted_LaunchesChooseVideoFromGalleryIntent() {
ImagePickerDelegate delegate = createDelegateWithPendingResultAndMethodCall();

delegate.onRequestPermissionsResult(
ImagePickerDelegate.REQUEST_EXTERNAL_VIDEO_STORAGE_PERMISSION,
new String[] {Manifest.permission.READ_EXTERNAL_STORAGE},
new int[] {PackageManager.PERMISSION_GRANTED});
ImagePickerDelegate delegate = createDelegate();
delegate.takeImageWithCamera(mockMethodCall, mockResult);

verify(mockActivity)
.startActivityForResult(
any(Intent.class), eq(ImagePickerDelegate.REQUEST_CODE_CHOOSE_VIDEO_FROM_GALLERY));
mockStaticFile.verify(
() -> File.createTempFile(any(), eq(".jpg"), eq(new File("/image_picker_cache"))),
times(1));
}

@Test
Expand Down Expand Up @@ -394,7 +357,7 @@ public void onActivityResult_WhenImageTakenWithCamera_AndNoResizeNeeded_Finishes
private ImagePickerDelegate createDelegate() {
return new ImagePickerDelegate(
mockActivity,
null,
new File("/image_picker_cache"),
mockImageResizer,
null,
null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@

package io.flutter.plugins.imagepicker;

import static org.hamcrest.core.IsEqual.equalTo;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.mockito.Mockito.when;
Expand All @@ -15,6 +18,7 @@
import android.app.Application;
import io.flutter.plugin.common.MethodCall;
import io.flutter.plugin.common.MethodChannel;
import java.io.File;
import java.util.HashMap;
import java.util.Map;
import org.junit.Before;
Expand Down Expand Up @@ -149,6 +153,20 @@ public void onConstructor_WhenContextTypeIsActivity_ShouldNotCrash() {
"No exception thrown when ImagePickerPlugin() ran with context instanceof Activity", true);
}

@Test
public void constructDelegate_ShouldUseInternalCacheDirectory() {
File mockDirectory = new File("/mockpath");
when(mockActivity.getCacheDir()).thenReturn(mockDirectory);

ImagePickerDelegate delegate = plugin.constructDelegate(mockActivity);

verify(mockActivity, times(1)).getCacheDir();
assertThat(
"Delegate uses cache directory for storing camera captures",
delegate.externalFilesDirectory,
equalTo(mockDirectory));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great if we also check (in another unit test) that calling delegate.takeImageWithCamera writes a file to the mock directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point! A test for this has been added.

}

private MethodCall buildMethodCall(String method, final int source) {
final Map<String, Object> arguments = new HashMap<>();
arguments.put("source", source);
Expand Down
2 changes: 1 addition & 1 deletion packages/image_picker/image_picker/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ description: Flutter plugin for selecting images from the Android and iOS image
library, and taking new pictures with the camera.
repository: https://github.com/flutter/plugins/tree/master/packages/image_picker/image_picker
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+image_picker%22
version: 0.7.5+4
version: 0.8.0

environment:
sdk: ">=2.12.0 <3.0.0"
Expand Down