Skip to content

[image_picker] Move I/O operations to a separate thread #3506

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
auto-submit merged 10 commits into from
Apr 28, 2023
Merged

[image_picker] Move I/O operations to a separate thread #3506

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f1892ea
Offloads result handling to separate thread
JeroenWeener Mar 21, 2023
aeff575
Add tests
JeroenWeener Mar 21, 2023
7cfc713
Update version and changelog
JeroenWeener Mar 21, 2023
06983b4
Format files
JeroenWeener Mar 21, 2023
2cba265
Restructure onActivityResult logic
JeroenWeener Apr 3, 2023
7fa1101
Synchronize `setPendingOptionsAndResult`
JeroenWeener Apr 5, 2023
b3e32b1
Remove unused imports
JeroenWeener Apr 5, 2023
38e67c1
Synchronize thread-unsafe code
JeroenWeener Apr 12, 2023
f42d307
Format `ImagePickerDelegate.java`
JeroenWeener Apr 12, 2023
557bf02
Re-bump version
stuartmorgan-g Apr 28, 2023
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
Prev Previous commit
Next Next commit
Add tests
  • Loading branch information
@JeroenWeener
JeroenWeener committed Apr 28, 2023
commit aeff575ec55bb63c0621cd37b7718752a54afee1
8 changes: 5 additions & 3 deletions ...ker_android/android/src/main/java/io/flutter/plugins/imagepicker/ImagePickerDelegate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,8 @@ public void onScanCompleted(String path, Uri uri) {
});
}
},
new FileUtils());
new FileUtils(),
new ThreadPoolExecutor(0, 1, 1L, TimeUnit.SECONDS, new LinkedBlockingQueue<>()));
}

/**
Expand All @@ -208,7 +209,8 @@ public void onScanCompleted(String path, Uri uri) {
final ImagePickerCache cache,
final PermissionManager permissionManager,
final FileUriResolver fileUriResolver,
final FileUtils fileUtils) {
final FileUtils fileUtils,
final ExecutorService executor) {
this.activity = activity;
this.externalFilesDirectory = externalFilesDirectory;
this.imageResizer = imageResizer;
Expand All @@ -221,7 +223,7 @@ public void onScanCompleted(String path, Uri uri) {
this.fileUriResolver = fileUriResolver;
this.fileUtils = fileUtils;
this.cache = cache;
this.executor = new ThreadPoolExecutor(0, 1, 1L, TimeUnit.SECONDS, new LinkedBlockingQueue<>());
this.executor = executor;
}

void setCameraDevice(CameraDevice device) {
Expand Down
147 changes: 127 additions & 20 deletions ...android/android/src/test/java/io/flutter/plugins/imagepicker/ImagePickerDelegateTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.IsEqual.equalTo;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
Expand All @@ -32,6 +34,8 @@
import java.io.File;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.ExecutorService;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -64,6 +68,7 @@ public class ImagePickerDelegateTest {
@Mock FileUtils mockFileUtils;
@Mock Intent mockIntent;
@Mock ImagePickerCache cache;
@Mock ExecutorService mockExecutor;

ImagePickerDelegate.FileUriResolver mockFileUriResolver;
MockedStatic<File> mockStaticFile;
Expand Down Expand Up @@ -349,11 +354,15 @@ public void onRequestPermissionsResult_whenCameraPermissionDenied_finishesWithEr

@Test
public void onActivityResult_whenPickFromGalleryCanceled_finishesWithEmptyList() {
Mockito.doAnswer(invocation -> {
((Runnable) invocation.getArgument(0)).run();
return null;
}).when(mockExecutor).execute(any(Runnable.class));
ImagePickerDelegate delegate =
createDelegateWithPendingResultAndOptions(DEFAULT_IMAGE_OPTIONS, null);
createDelegateWithPendingResultAndOptions(DEFAULT_IMAGE_OPTIONS, null);

delegate.onActivityResult(
ImagePickerDelegate.REQUEST_CODE_CHOOSE_IMAGE_FROM_GALLERY, Activity.RESULT_CANCELED, null);
ImagePickerDelegate.REQUEST_CODE_CHOOSE_IMAGE_FROM_GALLERY, Activity.RESULT_CANCELED, null);

@SuppressWarnings("unchecked")
ArgumentCaptor<List<String>> pathListCapture = ArgumentCaptor.forClass(List.class);
Expand All @@ -364,19 +373,27 @@ public void onActivityResult_whenPickFromGalleryCanceled_finishesWithEmptyList()

@Test
public void onActivityResult_whenPickFromGalleryCanceled_storesNothingInCache() {
Mockito.doAnswer(invocation -> {
((Runnable) invocation.getArgument(0)).run();
return null;
}).when(mockExecutor).execute(any(Runnable.class));
ImagePickerDelegate delegate = createDelegate();

delegate.onActivityResult(
ImagePickerDelegate.REQUEST_CODE_CHOOSE_IMAGE_FROM_GALLERY, Activity.RESULT_CANCELED, null);
ImagePickerDelegate.REQUEST_CODE_CHOOSE_IMAGE_FROM_GALLERY, Activity.RESULT_CANCELED, null);

verify(cache, never()).saveResult(any(), any(), any());
}

@Test
public void
onActivityResult_whenImagePickedFromGallery_andNoResizeNeeded_finishesWithImagePath() {
Mockito.doAnswer(invocation -> {
((Runnable) invocation.getArgument(0)).run();
return null;
}).when(mockExecutor).execute(any(Runnable.class));
ImagePickerDelegate delegate =
createDelegateWithPendingResultAndOptions(DEFAULT_IMAGE_OPTIONS, null);
createDelegateWithPendingResultAndOptions(DEFAULT_IMAGE_OPTIONS, null);

delegate.onActivityResult(
ImagePickerDelegate.REQUEST_CODE_CHOOSE_IMAGE_FROM_GALLERY, Activity.RESULT_OK, mockIntent);
Expand All @@ -390,10 +407,14 @@ public void onActivityResult_whenPickFromGalleryCanceled_storesNothingInCache()

@Test
public void onActivityResult_whenImagePickedFromGallery_andNoResizeNeeded_storesImageInCache() {
Mockito.doAnswer(invocation -> {
((Runnable) invocation.getArgument(0)).run();
return null;
}).when(mockExecutor).execute(any(Runnable.class));
ImagePickerDelegate delegate = createDelegate();

delegate.onActivityResult(
ImagePickerDelegate.REQUEST_CODE_CHOOSE_IMAGE_FROM_GALLERY, Activity.RESULT_OK, mockIntent);
ImagePickerDelegate.REQUEST_CODE_CHOOSE_IMAGE_FROM_GALLERY, Activity.RESULT_OK, mockIntent);

@SuppressWarnings("unchecked")
ArgumentCaptor<ArrayList<String>> pathListCapture = ArgumentCaptor.forClass(ArrayList.class);
Expand All @@ -404,8 +425,13 @@ public void onActivityResult_whenImagePickedFromGallery_andNoResizeNeeded_stores
@Test
public void
onActivityResult_whenImagePickedFromGallery_andResizeNeeded_finishesWithScaledImagePath() {
Mockito.doAnswer(invocation -> {
((Runnable) invocation.getArgument(0)).run();
return null;
}).when(mockExecutor).execute(any(Runnable.class));
ImagePickerDelegate delegate =
createDelegateWithPendingResultAndOptions(RESIZE_TRIGGERING_IMAGE_OPTIONS, null);
createDelegateWithPendingResultAndOptions(RESIZE_TRIGGERING_IMAGE_OPTIONS, null);

delegate.onActivityResult(
ImagePickerDelegate.REQUEST_CODE_CHOOSE_IMAGE_FROM_GALLERY, Activity.RESULT_OK, mockIntent);

Expand All @@ -418,11 +444,16 @@ public void onActivityResult_whenImagePickedFromGallery_andNoResizeNeeded_stores

@Test
public void
onActivityResult_whenVideoPickedFromGallery_andResizeParametersSupplied_finishesWithFilePath() {
onActivityResult_whenVideoPickedFromGallery_andResizeParametersSupplied_finishesWithFilePath() {
Mockito.doAnswer(invocation -> {
((Runnable) invocation.getArgument(0)).run();
return null;
}).when(mockExecutor).execute(any(Runnable.class));
ImagePickerDelegate delegate =
createDelegateWithPendingResultAndOptions(RESIZE_TRIGGERING_IMAGE_OPTIONS, null);
createDelegateWithPendingResultAndOptions(RESIZE_TRIGGERING_IMAGE_OPTIONS, null);

delegate.onActivityResult(
ImagePickerDelegate.REQUEST_CODE_CHOOSE_VIDEO_FROM_GALLERY, Activity.RESULT_OK, mockIntent);
ImagePickerDelegate.REQUEST_CODE_CHOOSE_VIDEO_FROM_GALLERY, Activity.RESULT_OK, mockIntent);

@SuppressWarnings("unchecked")
ArgumentCaptor<List<String>> pathListCapture = ArgumentCaptor.forClass(List.class);
Expand All @@ -433,11 +464,15 @@ public void onActivityResult_whenImagePickedFromGallery_andNoResizeNeeded_stores

@Test
public void onActivityResult_whenTakeImageWithCameraCanceled_finishesWithEmptyList() {
Mockito.doAnswer(invocation -> {
((Runnable) invocation.getArgument(0)).run();
return null;
}).when(mockExecutor).execute(any(Runnable.class));
ImagePickerDelegate delegate =
createDelegateWithPendingResultAndOptions(DEFAULT_IMAGE_OPTIONS, null);
createDelegateWithPendingResultAndOptions(DEFAULT_IMAGE_OPTIONS, null);

delegate.onActivityResult(
ImagePickerDelegate.REQUEST_CODE_TAKE_IMAGE_WITH_CAMERA, Activity.RESULT_CANCELED, null);
ImagePickerDelegate.REQUEST_CODE_TAKE_IMAGE_WITH_CAMERA, Activity.RESULT_CANCELED, null);

@SuppressWarnings("unchecked")
ArgumentCaptor<List<String>> pathListCapture = ArgumentCaptor.forClass(List.class);
Expand All @@ -448,9 +483,13 @@ public void onActivityResult_whenTakeImageWithCameraCanceled_finishesWithEmptyLi

@Test
public void onActivityResult_whenImageTakenWithCamera_andNoResizeNeeded_finishesWithImagePath() {
when(cache.retrievePendingCameraMediaUriPath()).thenReturn("testString");
Mockito.doAnswer(invocation -> {
((Runnable) invocation.getArgument(0)).run();
return null;
}).when(mockExecutor).execute(any(Runnable.class));
ImagePickerDelegate delegate =
createDelegateWithPendingResultAndOptions(DEFAULT_IMAGE_OPTIONS, null);
when(cache.retrievePendingCameraMediaUriPath()).thenReturn("testString");

delegate.onActivityResult(
ImagePickerDelegate.REQUEST_CODE_TAKE_IMAGE_WITH_CAMERA, Activity.RESULT_OK, mockIntent);
Expand All @@ -466,9 +505,13 @@ public void onActivityResult_whenImageTakenWithCamera_andNoResizeNeeded_finishes
public void
onActivityResult_whenImageTakenWithCamera_andResizeNeeded_finishesWithScaledImagePath() {
when(cache.retrievePendingCameraMediaUriPath()).thenReturn("testString");

Mockito.doAnswer(invocation -> {
((Runnable) invocation.getArgument(0)).run();
return null;
}).when(mockExecutor).execute(any(Runnable.class));
ImagePickerDelegate delegate =
createDelegateWithPendingResultAndOptions(RESIZE_TRIGGERING_IMAGE_OPTIONS, null);

delegate.onActivityResult(
ImagePickerDelegate.REQUEST_CODE_TAKE_IMAGE_WITH_CAMERA, Activity.RESULT_OK, mockIntent);

Expand All @@ -481,13 +524,17 @@ public void onActivityResult_whenImageTakenWithCamera_andNoResizeNeeded_finishes

@Test
public void
onActivityResult_whenVideoTakenWithCamera_andResizeParametersSupplied_finishesWithFilePath() {
onActivityResult_whenVideoTakenWithCamera_andResizeParametersSupplied_finishesWithFilePath() {
when(cache.retrievePendingCameraMediaUriPath()).thenReturn("testString");

Mockito.doAnswer(invocation -> {
((Runnable) invocation.getArgument(0)).run();
return null;
}).when(mockExecutor).execute(any(Runnable.class));
ImagePickerDelegate delegate =
createDelegateWithPendingResultAndOptions(RESIZE_TRIGGERING_IMAGE_OPTIONS, null);
createDelegateWithPendingResultAndOptions(RESIZE_TRIGGERING_IMAGE_OPTIONS, null);

delegate.onActivityResult(
ImagePickerDelegate.REQUEST_CODE_TAKE_VIDEO_WITH_CAMERA, Activity.RESULT_OK, mockIntent);
ImagePickerDelegate.REQUEST_CODE_TAKE_VIDEO_WITH_CAMERA, Activity.RESULT_OK, mockIntent);

@SuppressWarnings("unchecked")
ArgumentCaptor<List<String>> pathListCapture = ArgumentCaptor.forClass(List.class);
Expand All @@ -500,10 +547,14 @@ public void onActivityResult_whenImageTakenWithCamera_andNoResizeNeeded_finishes
public void
onActivityResult_whenVideoTakenWithCamera_andMaxDurationParametersSupplied_finishesWithFilePath() {
when(cache.retrievePendingCameraMediaUriPath()).thenReturn("testString");

Mockito.doAnswer(invocation -> {
((Runnable) invocation.getArgument(0)).run();
return null;
}).when(mockExecutor).execute(any(Runnable.class));
ImagePickerDelegate delegate =
createDelegateWithPendingResultAndOptions(
null, new VideoSelectionOptions.Builder().setMaxDurationSeconds(MAX_DURATION).build());

delegate.onActivityResult(
ImagePickerDelegate.REQUEST_CODE_TAKE_VIDEO_WITH_CAMERA, Activity.RESULT_OK, mockIntent);

Expand All @@ -514,6 +565,60 @@ public void onActivityResult_whenImageTakenWithCamera_andNoResizeNeeded_finishes
verifyNoMoreInteractions(mockResult);
}

@Test
public void onActivityResult_whenImagePickedFromGallery_returnsTrue() {
ImagePickerDelegate delegate = createDelegate();

boolean isHandled = delegate.onActivityResult(ImagePickerDelegate.REQUEST_CODE_CHOOSE_IMAGE_FROM_GALLERY, Activity.RESULT_OK, mockIntent);

assertTrue(isHandled);
}

@Test
public void onActivityResult_whenMultipleImagesPickedFromGallery_returnsTrue() {
ImagePickerDelegate delegate = createDelegate();

boolean isHandled = delegate.onActivityResult(ImagePickerDelegate.REQUEST_CODE_CHOOSE_MULTI_IMAGE_FROM_GALLERY, Activity.RESULT_OK, mockIntent);

assertTrue(isHandled);
}

@Test
public void onActivityResult_whenVideoPickerFromGallery_returnsTrue() {
ImagePickerDelegate delegate = createDelegate();

boolean isHandled = delegate.onActivityResult(ImagePickerDelegate.REQUEST_CODE_CHOOSE_VIDEO_FROM_GALLERY, Activity.RESULT_OK, mockIntent);

assertTrue(isHandled);
}

@Test
public void onActivityResult_whenImageTakenWithCamera_returnsTrue() {
ImagePickerDelegate delegate = createDelegate();

boolean isHandled = delegate.onActivityResult(ImagePickerDelegate.REQUEST_CODE_TAKE_IMAGE_WITH_CAMERA, Activity.RESULT_OK, mockIntent);

assertTrue(isHandled);
}

@Test
public void onActivityResult_whenVideoTakenWithCamera_returnsTrue() {
ImagePickerDelegate delegate = createDelegate();

boolean isHandled = delegate.onActivityResult(ImagePickerDelegate.REQUEST_CODE_TAKE_VIDEO_WITH_CAMERA, Activity.RESULT_OK, mockIntent);

assertTrue(isHandled);
}

@Test
public void onActivityResult_withUnknownRequest_returnsFalse() {
ImagePickerDelegate delegate = createDelegate();

boolean isHandled = delegate.onActivityResult(314, Activity.RESULT_OK, mockIntent);

assertFalse(isHandled);
}

private ImagePickerDelegate createDelegate() {
return new ImagePickerDelegate(
mockActivity,
Expand All @@ -525,7 +630,8 @@ private ImagePickerDelegate createDelegate() {
cache,
mockPermissionManager,
mockFileUriResolver,
mockFileUtils);
mockFileUtils,
mockExecutor);
}

private ImagePickerDelegate createDelegateWithPendingResultAndOptions(
Expand All @@ -540,7 +646,8 @@ private ImagePickerDelegate createDelegateWithPendingResultAndOptions(
cache,
mockPermissionManager,
mockFileUriResolver,
mockFileUtils);
mockFileUtils,
mockExecutor);
}

private void verifyFinishedWithAlreadyActiveError() {
Expand Down