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

[Image_Picker] Use cache instance on Android #1877

Merged
cyanglaz merged 8 commits into from
Jul 29, 2019
Merged

[Image_Picker] Use cache instance on Android #1877

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e62447d
migrated cache implementation from static method to use instance methods
juliocbcotta Jul 19, 2019
5b2ff58
format code
juliocbcotta Jul 19, 2019
f965d14
updated CHANGELOG.md and pubspec.yaml
juliocbcotta Jul 19, 2019
6329b62
removed unused method
juliocbcotta Jul 20, 2019
c170614
Merge remote-tracking branch 'flutter/master' into plugin/image_pick_…
juliocbcotta Jul 23, 2019
717f677
fix build version
juliocbcotta Jul 23, 2019
9de1394
Merge remote-tracking branch 'flutter/master' into plugin/image_pick_…
juliocbcotta Jul 26, 2019
a7a289f
Merge branch 'master' into plugin/image_pick_use_cache_instance
Jul 29, 2019
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
4 changes: 4 additions & 0 deletions packages/image_picker/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.6.0+20

* Android: Migrated information cache methods to use instance methods.

## 0.6.0+19

* Android: Fix memory leak due not unregistering ActivityLifecycleCallbacks.
Expand Down
112 changes: 43 additions & 69 deletions ...s/image_picker/android/src/main/java/io/flutter/plugins/imagepicker/ImagePickerCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

package io.flutter.plugins.imagepicker;

import android.app.Activity;
import android.content.Context;
import android.content.SharedPreferences;
import android.net.Uri;
Expand Down Expand Up @@ -34,42 +33,34 @@ class ImagePickerCache {
"flutter_image_picker_pending_image_uri";
private static final String SHARED_PREFERENCES_NAME = "flutter_image_picker_shared_preference";

private static SharedPreferences getFilePref;
private SharedPreferences prefs;
Copy link
Contributor

@cyanglaz cyanglaz Jul 19, 2019

Choose a reason for hiding this comment

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

I see all the null check are removed. I am not a Java expert but how do we ensure the prefs is not null to prevent NPE?

Copy link
Contributor Author

@juliocbcotta juliocbcotta Jul 20, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

.static methods and variables are associated with the class they are declared.
Non static methods and variables are associated with an instance of an object of a given type.
I removed static declaration for prefs to make it part of an instance of ImagePickerCache. To access prefs, I need an instance of ImagePickerCache.
So we know prefs is never null because it is being instantiated in the constructor of ImagePickerCache.

As prefs is no longer static, the static methods were converted to instance methods as well. Static methods can't access non static variables/methods after all.

As a rule of thumb: Avoid static methods/variables. They are hard to write unit tests and they are memory leak and error prone.


static void setUpWithActivity(Activity activity) {
getFilePref =
activity
.getApplicationContext()
.getSharedPreferences(SHARED_PREFERENCES_NAME, Context.MODE_PRIVATE);
ImagePickerCache(Context context) {
prefs = context.getSharedPreferences(SHARED_PREFERENCES_NAME, Context.MODE_PRIVATE);
}

static void saveTypeWithMethodCallName(String methodCallName) {
void saveTypeWithMethodCallName(String methodCallName) {
if (methodCallName.equals(ImagePickerPlugin.METHOD_CALL_IMAGE)) {
setType("image");
} else if (methodCallName.equals(ImagePickerPlugin.METHOD_CALL_VIDEO)) {
setType("video");
}
}

private static void setType(String type) {
if (getFilePref == null) {
return;
}
getFilePref.edit().putString(SHARED_PREFERENCE_TYPE_KEY, type).apply();
private void setType(String type) {

prefs.edit().putString(SHARED_PREFERENCE_TYPE_KEY, type).apply();
}

static void saveDemensionWithMethodCall(MethodCall methodCall) {
Double maxWidth = methodCall.argument("maxWidth");
Double maxHeight = methodCall.argument("maxHeight");
void saveDimensionWithMethodCall(MethodCall methodCall) {
Double maxWidth = methodCall.argument(MAP_KEY_MAX_WIDTH);
Double maxHeight = methodCall.argument(MAP_KEY_MAX_HEIGHT);
setMaxDimension(maxWidth, maxHeight);
}

private static void setMaxDimension(Double maxWidth, Double maxHeight) {
if (getFilePref == null) {
return;
}
private void setMaxDimension(Double maxWidth, Double maxHeight) {

SharedPreferences.Editor editor = getFilePref.edit();
SharedPreferences.Editor editor = prefs.edit();
if (maxWidth != null) {
editor.putLong(SHARED_PREFERENCE_MAX_WIDTH_KEY, Double.doubleToRawLongBits(maxWidth));
}
Expand All @@ -79,29 +70,19 @@ private static void setMaxDimension(Double maxWidth, Double maxHeight) {
editor.apply();
}

static void savePendingCameraMediaUriPath(Uri uri) {
if (getFilePref == null) {
return;
}
getFilePref
.edit()
.putString(SHARED_PREFERENCE_PENDING_IMAGE_URI_PATH_KEY, uri.getPath())
.apply();
void savePendingCameraMediaUriPath(Uri uri) {
prefs.edit().putString(SHARED_PREFERENCE_PENDING_IMAGE_URI_PATH_KEY, uri.getPath()).apply();
}

static String retrievePendingCameraMediaUriPath() {
if (getFilePref == null) {
return null;
}
return getFilePref.getString(SHARED_PREFERENCE_PENDING_IMAGE_URI_PATH_KEY, "");
String retrievePendingCameraMediaUriPath() {

return prefs.getString(SHARED_PREFERENCE_PENDING_IMAGE_URI_PATH_KEY, "");
}

static void saveResult(
void saveResult(
@Nullable String path, @Nullable String errorCode, @Nullable String errorMessage) {
if (getFilePref == null) {
return;
}
SharedPreferences.Editor editor = getFilePref.edit();

SharedPreferences.Editor editor = prefs.edit();
if (path != null) {
editor.putString(FLUTTER_IMAGE_PICKER_IMAGE_PATH_KEY, path);
}
Expand All @@ -114,50 +95,43 @@ static void saveResult(
editor.apply();
}

static void clear() {
if (getFilePref == null) {
return;
}
getFilePref.edit().clear().apply();
void clear() {
prefs.edit().clear().apply();
}

static Map<String, Object> getCacheMap() {
if (getFilePref == null) {
return new HashMap<>();
}
Map<String, Object> getCacheMap() {

Map<String, Object> resultMap = new HashMap<>();
Boolean hasData = false;
boolean hasData = false;

if (getFilePref.contains(FLUTTER_IMAGE_PICKER_IMAGE_PATH_KEY)) {
resultMap.put(MAP_KEY_PATH, getFilePref.getString(FLUTTER_IMAGE_PICKER_IMAGE_PATH_KEY, ""));
if (prefs.contains(FLUTTER_IMAGE_PICKER_IMAGE_PATH_KEY)) {
final String imagePathValue = prefs.getString(FLUTTER_IMAGE_PICKER_IMAGE_PATH_KEY, "");
resultMap.put(MAP_KEY_PATH, imagePathValue);
hasData = true;
}

if (getFilePref.contains(SHARED_PREFERENCE_ERROR_CODE_KEY)) {
resultMap.put(
MAP_KEY_ERROR_CODE, getFilePref.getString(SHARED_PREFERENCE_ERROR_CODE_KEY, ""));
if (prefs.contains(SHARED_PREFERENCE_ERROR_CODE_KEY)) {
final String errorCodeValue = prefs.getString(SHARED_PREFERENCE_ERROR_CODE_KEY, "");
resultMap.put(MAP_KEY_ERROR_CODE, errorCodeValue);
hasData = true;
if (getFilePref.contains(SHARED_PREFERENCE_ERROR_MESSAGE_KEY)) {
resultMap.put(
MAP_KEY_ERROR_MESSAGE, getFilePref.getString(SHARED_PREFERENCE_ERROR_MESSAGE_KEY, ""));
if (prefs.contains(SHARED_PREFERENCE_ERROR_MESSAGE_KEY)) {
final String errorMessageValue = prefs.getString(SHARED_PREFERENCE_ERROR_MESSAGE_KEY, "");
resultMap.put(MAP_KEY_ERROR_MESSAGE, errorMessageValue);
}
}

if (hasData) {
if (getFilePref.contains(SHARED_PREFERENCE_TYPE_KEY)) {
resultMap.put(MAP_KEY_TYPE, getFilePref.getString(SHARED_PREFERENCE_TYPE_KEY, ""));
if (prefs.contains(SHARED_PREFERENCE_TYPE_KEY)) {
final String typeValue = prefs.getString(SHARED_PREFERENCE_TYPE_KEY, "");
resultMap.put(MAP_KEY_TYPE, typeValue);
}

if (getFilePref.contains(SHARED_PREFERENCE_MAX_WIDTH_KEY)) {
resultMap.put(
MAP_KEY_MAX_WIDTH,
Double.longBitsToDouble(getFilePref.getLong(SHARED_PREFERENCE_MAX_WIDTH_KEY, 0)));
if (prefs.contains(SHARED_PREFERENCE_MAX_WIDTH_KEY)) {
final long maxWidthValue = prefs.getLong(SHARED_PREFERENCE_MAX_WIDTH_KEY, 0);
resultMap.put(MAP_KEY_MAX_WIDTH, Double.longBitsToDouble(maxWidthValue));
}

if (getFilePref.contains(SHARED_PREFERENCE_MAX_HEIGHT_KEY)) {
resultMap.put(
MAP_KEY_MAX_HEIGHT,
Double.longBitsToDouble(getFilePref.getLong(SHARED_PREFERENCE_MAX_HEIGHT_KEY, 0)));
if (prefs.contains(SHARED_PREFERENCE_MAX_HEIGHT_KEY)) {
final long maxHeighValue = prefs.getLong(SHARED_PREFERENCE_MAX_HEIGHT_KEY, 0);
resultMap.put(MAP_KEY_MAX_HEIGHT, Double.longBitsToDouble(maxHeighValue));
}
}

Expand Down
55 changes: 31 additions & 24 deletions ...mage_picker/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 @@ -80,6 +80,7 @@ public class ImagePickerDelegate
private final Activity activity;
private final File externalFilesDirectory;
private final ImageResizer imageResizer;
private final ImagePickerCache cache;
private final PermissionManager permissionManager;
private final IntentResolver intentResolver;
private final FileUriResolver fileUriResolver;
Expand Down Expand Up @@ -112,13 +113,17 @@ interface OnPathReadyListener {
private MethodCall methodCall;

public ImagePickerDelegate(
final Activity activity, File externalFilesDirectory, ImageResizer imageResizer) {
final Activity activity,
final File externalFilesDirectory,
final ImageResizer imageResizer,
final ImagePickerCache cache) {
this(
activity,
externalFilesDirectory,
imageResizer,
null,
null,
cache,
new PermissionManager() {
@Override
public boolean isPermissionGranted(String permissionName) {
Expand Down Expand Up @@ -171,15 +176,16 @@ public void onScanCompleted(String path, Uri uri) {
*/
@VisibleForTesting
ImagePickerDelegate(
Activity activity,
File externalFilesDirectory,
ImageResizer imageResizer,
MethodChannel.Result result,
MethodCall methodCall,
PermissionManager permissionManager,
IntentResolver intentResolver,
FileUriResolver fileUriResolver,
FileUtils fileUtils) {
final Activity activity,
final File externalFilesDirectory,
final ImageResizer imageResizer,
final MethodChannel.Result result,
final MethodCall methodCall,
final ImagePickerCache cache,
final PermissionManager permissionManager,
final IntentResolver intentResolver,
final FileUriResolver fileUriResolver,
final FileUtils fileUtils) {
this.activity = activity;
this.externalFilesDirectory = externalFilesDirectory;
this.imageResizer = imageResizer;
Expand All @@ -190,35 +196,36 @@ public void onScanCompleted(String path, Uri uri) {
this.intentResolver = intentResolver;
this.fileUriResolver = fileUriResolver;
this.fileUtils = fileUtils;
this.cache = cache;
}

void saveStateBeforeResult() {
if (methodCall == null) {
return;
}

ImagePickerCache.saveTypeWithMethodCallName(methodCall.method);
ImagePickerCache.saveDemensionWithMethodCall(methodCall);
cache.saveTypeWithMethodCallName(methodCall.method);
cache.saveDimensionWithMethodCall(methodCall);
if (pendingCameraMediaUri != null) {
ImagePickerCache.savePendingCameraMediaUriPath(pendingCameraMediaUri);
cache.savePendingCameraMediaUriPath(pendingCameraMediaUri);
}
}

void retrieveLostImage(MethodChannel.Result result) {
Map<String, Object> resultMap = ImagePickerCache.getCacheMap();
String path = (String) resultMap.get(ImagePickerCache.MAP_KEY_PATH);
Map<String, Object> resultMap = cache.getCacheMap();
String path = (String) resultMap.get(cache.MAP_KEY_PATH);
if (path != null) {
Double maxWidth = (Double) resultMap.get(ImagePickerCache.MAP_KEY_MAX_WIDTH);
Double maxHeight = (Double) resultMap.get(ImagePickerCache.MAP_KEY_MAX_HEIGHT);
Double maxWidth = (Double) resultMap.get(cache.MAP_KEY_MAX_WIDTH);
Double maxHeight = (Double) resultMap.get(cache.MAP_KEY_MAX_HEIGHT);
String newPath = imageResizer.resizeImageIfNeeded(path, maxWidth, maxHeight);
resultMap.put(ImagePickerCache.MAP_KEY_PATH, newPath);
resultMap.put(cache.MAP_KEY_PATH, newPath);
}
if (resultMap.isEmpty()) {
result.success(null);
} else {
result.success(resultMap);
}
ImagePickerCache.clear();
cache.clear();
}

public void chooseVideoFromGallery(MethodCall methodCall, MethodChannel.Result result) {
Expand Down Expand Up @@ -472,7 +479,7 @@ private void handleCaptureImageResult(int resultCode) {
fileUriResolver.getFullImagePath(
pendingCameraMediaUri != null
? pendingCameraMediaUri
: Uri.parse(ImagePickerCache.retrievePendingCameraMediaUriPath()),
: Uri.parse(cache.retrievePendingCameraMediaUriPath()),
new OnPathReadyListener() {
@Override
public void onPathReady(String path) {
Expand All @@ -491,7 +498,7 @@ private void handleCaptureVideoResult(int resultCode) {
fileUriResolver.getFullImagePath(
pendingCameraMediaUri != null
? pendingCameraMediaUri
: Uri.parse(ImagePickerCache.retrievePendingCameraMediaUriPath()),
: Uri.parse(cache.retrievePendingCameraMediaUriPath()),
new OnPathReadyListener() {
@Override
public void onPathReady(String path) {
Expand Down Expand Up @@ -536,14 +543,14 @@ private boolean setPendingMethodCallAndResult(
pendingResult = result;

// Clean up cache if a new image picker is launched.
ImagePickerCache.clear();
cache.clear();

return true;
}

private void finishWithSuccess(String imagePath) {
if (pendingResult == null) {
ImagePickerCache.saveResult(imagePath, null, null);
cache.saveResult(imagePath, null, null);
return;
}
pendingResult.success(imagePath);
Expand All @@ -556,7 +563,7 @@ private void finishWithAlreadyActiveError(MethodChannel.Result result) {

private void finishWithError(String errorCode, String errorMessage) {
if (pendingResult == null) {
ImagePickerCache.saveResult(null, errorCode, errorMessage);
cache.saveResult(null, errorCode, errorMessage);
return;
}
pendingResult.error(errorCode, errorMessage, null);
Expand Down
4 changes: 2 additions & 2 deletions .../image_picker/android/src/main/java/io/flutter/plugins/imagepicker/ImagePickerPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public static void registerWith(PluginRegistry.Registrar registrar) {
// we stop the registering process immediately because the ImagePicker requires an activity.
return;
}
ImagePickerCache.setUpWithActivity(registrar.activity());
final ImagePickerCache cache = new ImagePickerCache(registrar.activity());

final MethodChannel channel = new MethodChannel(registrar.messenger(), CHANNEL);

Expand All @@ -46,7 +46,7 @@ public static void registerWith(PluginRegistry.Registrar registrar) {
final ExifDataCopier exifDataCopier = new ExifDataCopier();
final ImageResizer imageResizer = new ImageResizer(externalFilesDirectory, exifDataCopier);
final ImagePickerDelegate delegate =
new ImagePickerDelegate(registrar.activity(), externalFilesDirectory, imageResizer);
new ImagePickerDelegate(registrar.activity(), externalFilesDirectory, imageResizer, cache);

registrar.addActivityResultListener(delegate);
registrar.addRequestPermissionsResultListener(delegate);
Expand Down
3 changes: 3 additions & 0 deletions ...ple/android/app/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 @@ -35,6 +35,7 @@ public class ImagePickerDelegateTest {
@Mock ImagePickerDelegate.IntentResolver mockIntentResolver;
@Mock FileUtils mockFileUtils;
@Mock Intent mockIntent;
@Mock ImagePickerCache cache;

ImagePickerDelegate.FileUriResolver mockFileUriResolver;

Expand Down Expand Up @@ -375,6 +376,7 @@ private ImagePickerDelegate createDelegate() {
mockImageResizer,
null,
null,
cache,
mockPermissionManager,
mockIntentResolver,
mockFileUriResolver,
Expand All @@ -388,6 +390,7 @@ private ImagePickerDelegate createDelegateWithPendingResultAndMethodCall() {
mockImageResizer,
mockResult,
mockMethodCall,
cache,
mockPermissionManager,
mockIntentResolver,
mockFileUriResolver,
Expand Down
2 changes: 1 addition & 1 deletion packages/image_picker/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ authors:
- Rhodes Davis Jr. <[email protected]>
homepage: https://github.com/flutter/plugins/tree/master/packages/image_picker

version: 0.6.0+19
version: 0.6.0+20

flutter:
plugin:
Expand Down