Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 6 additions & 1 deletion packages/image_picker/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
## 0.6.2+1

* Android: Fix a crash when a non-image file is picked.
* Android: Fix unwanted bitmap scaling.

## 0.6.2

* iOS: Fixes an issue where picking conent from Gallery would result in a crash on iOS 13.
* iOS: Fixes an issue where picking content from Gallery would result in a crash on iOS 13.

## 0.6.1+11

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -521,10 +521,7 @@ private void handleImageResult(String path, boolean shouldDeleteOriginalIfScaled
if (methodCall != null) {
Double maxWidth = methodCall.argument("maxWidth");
Double maxHeight = methodCall.argument("maxHeight");
int imageQuality =
methodCall.argument("imageQuality") == null
? 100
: (int) methodCall.argument("imageQuality");
Integer imageQuality = methodCall.argument("imageQuality");

String finalImagePath =
imageResizer.resizeImageIfNeeded(path, maxWidth, maxHeight, imageQuality);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
import android.util.Log;
import androidx.annotation.Nullable;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.FileOutputStream;
Expand All @@ -28,31 +29,39 @@ class ImageResizer {
* <p>If no resizing is needed, returns the path for the original image.
*/
String resizeImageIfNeeded(
String imagePath, Double maxWidth, Double maxHeight, int imageQuality) {
String imagePath,
@Nullable Double maxWidth,
@Nullable Double maxHeight,
@Nullable Integer imageQuality) {
boolean shouldScale =
maxWidth != null || maxHeight != null || (imageQuality > -1 && imageQuality < 101);

if (!shouldScale) {
return imagePath;
maxWidth != null || maxHeight != null || isImageQualityValid(imageQuality);
String[] pathParts = imagePath.split("/");
String imageName = pathParts[pathParts.length - 1];
File file;
Bitmap bmp = decodeFile(imagePath);
if (bmp == null) {
return null;
}

try {
File scaledImage = resizedImage(imagePath, maxWidth, maxHeight, imageQuality);
exifDataCopier.copyExif(imagePath, scaledImage.getPath());

return scaledImage.getPath();
if (!shouldScale) {
file = createImageOnExternalDirectory(imageName, bmp, 100);
} else {
file = resizedImage(bmp, maxWidth, maxHeight, imageQuality, imageName);
}
copyExif(imagePath, file.getPath());
return file.getPath();
} catch (IOException e) {
throw new RuntimeException(e);
}
}

private File resizedImage(String path, Double maxWidth, Double maxHeight, int imageQuality)
private File resizedImage(
Bitmap bmp, Double maxWidth, Double maxHeight, Integer imageQuality, String outputImageName)
throws IOException {
Bitmap bmp = BitmapFactory.decodeFile(path);
double originalWidth = bmp.getWidth() * 1.0;
double originalHeight = bmp.getHeight() * 1.0;

if (imageQuality < 0 || imageQuality > 100) {
if (!isImageQualityValid(imageQuality)) {
imageQuality = 100;
}

Expand Down Expand Up @@ -91,24 +100,51 @@ private File resizedImage(String path, Double maxWidth, Double maxHeight, int im
}
}

Bitmap scaledBmp = Bitmap.createScaledBitmap(bmp, width.intValue(), height.intValue(), false);
Bitmap scaledBmp = createScaledBitmap(bmp, width.intValue(), height.intValue(), false);
File file =
createImageOnExternalDirectory("/scaled_" + outputImageName, scaledBmp, imageQuality);
return file;
}

private File createFile(File externalFilesDirectory, String child) {
return new File(externalFilesDirectory, child);
}

private FileOutputStream createOutputStream(File imageFile) throws IOException {
return new FileOutputStream(imageFile);
}

private void copyExif(String filePathOri, String filePathDest) {
exifDataCopier.copyExif(filePathOri, filePathDest);
}

private Bitmap decodeFile(String path) {
return BitmapFactory.decodeFile(path);
}

private Bitmap createScaledBitmap(Bitmap bmp, int width, int height, boolean filter) {
return Bitmap.createScaledBitmap(bmp, width, height, filter);
}

private boolean isImageQualityValid(Integer imageQuality) {
return imageQuality != null && imageQuality > 0 && imageQuality < 100;
}

private File createImageOnExternalDirectory(String name, Bitmap bitmap, int imageQuality)
throws IOException {
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
boolean saveAsPNG = bmp.hasAlpha();
boolean saveAsPNG = bitmap.hasAlpha();
if (saveAsPNG) {
Log.d(
"ImageResizer",
"image_picker: compressing is not supported for type PNG. Returning the image with original quality");
}
scaledBmp.compress(
bitmap.compress(
saveAsPNG ? Bitmap.CompressFormat.PNG : Bitmap.CompressFormat.JPEG,
imageQuality,
outputStream);

String[] pathParts = path.split("/");
String imageName = pathParts[pathParts.length - 1];

File imageFile = new File(externalFilesDirectory, "/scaled_" + imageName);
FileOutputStream fileOutput = new FileOutputStream(imageFile);
File imageFile = createFile(externalFilesDirectory, name);
FileOutputStream fileOutput = createOutputStream(imageFile);
fileOutput.write(outputStream.toByteArray());
fileOutput.close();
return imageFile;
Expand Down
1 change: 1 addition & 0 deletions packages/image_picker/example/android/app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,5 @@ dependencies {
testImplementation 'org.mockito:mockito-core:2.17.0'
androidTestImplementation 'androidx.test:runner:1.1.1'
androidTestImplementation 'androidx.test.espresso:espresso-core:3.1.1'
testImplementation "org.robolectric:robolectric:3.3.2"
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,7 @@
import org.mockito.MockitoAnnotations;

public class ImagePickerCacheTest {
private static final double WIDTH = 10.0;
private static final double HEIGHT = 10.0;
private static final int IMAGE_QUALITY = 90;
private static final String PATH = "a_mock_path";

@Mock Activity mockActivity;
@Mock SharedPreferences mockPreference;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@
import org.mockito.MockitoAnnotations;

public class ImagePickerDelegateTest {
private static final double WIDTH = 10.0;
private static final double HEIGHT = 10.0;
private static final int IMAGE_QUALITY = 100;
private static final Double WIDTH = 10.0;
private static final Double HEIGHT = 10.0;
private static final Integer IMAGE_QUALITY = 90;

@Mock Activity mockActivity;
@Mock ImageResizer mockImageResizer;
Expand Down Expand Up @@ -62,13 +62,15 @@ public void setUp() {
when(mockFileUtils.getPathFromUri(any(Context.class), any(Uri.class)))
.thenReturn("pathFromUri");

when(mockImageResizer.resizeImageIfNeeded("pathFromUri", null, null, null))
.thenReturn("originalPath");
when(mockImageResizer.resizeImageIfNeeded("pathFromUri", null, null, IMAGE_QUALITY))
.thenReturn("originalPath");
when(mockImageResizer.resizeImageIfNeeded("pathFromUri", WIDTH, HEIGHT, IMAGE_QUALITY))
when(mockImageResizer.resizeImageIfNeeded("pathFromUri", WIDTH, HEIGHT, null))
.thenReturn("scaledPath");
when(mockImageResizer.resizeImageIfNeeded("pathFromUri", WIDTH, null, IMAGE_QUALITY))
when(mockImageResizer.resizeImageIfNeeded("pathFromUri", WIDTH, null, null))
.thenReturn("scaledPath");
when(mockImageResizer.resizeImageIfNeeded("pathFromUri", null, HEIGHT, IMAGE_QUALITY))
when(mockImageResizer.resizeImageIfNeeded("pathFromUri", null, HEIGHT, null))
.thenReturn("scaledPath");

mockFileUriResolver = new MockFileUriResolver();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// Copyright 2019 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package io.flutter.plugins.imagepicker;

import static org.hamcrest.core.IsEqual.equalTo;
import static org.junit.Assert.assertThat;

import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
import java.io.File;
import java.io.IOException;
import org.junit.Before;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
import org.mockito.MockitoAnnotations;
import org.robolectric.RobolectricTestRunner;

// RobolectricTestRunner always creates a default mock bitmap when reading from file. So we cannot actually test the scaling.
// But we can still test whether the original or scaled file is created.
@RunWith(RobolectricTestRunner.class)
public class ImageResizerTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we have some time to improve this, it would be better to change the style of testing here so that it's really testing the behavior of ImageResizer's public API instead of the interactions of a bunch of its private methods. The way this test is written right now it's really just verifying that when certain methods are called, other methods also get triggered. This isn't really testing the logic of the class, and it could be broken by trivial method renames. It also exposes a bunch of methods with @VisibleForTesting that would be better kept private.

https://testing.googleblog.com/2013/08/testing-on-toilet-test-behavior-not.html

Here's the approach I would probably take:

  1. Use TemporaryFolder to create a file directory for the unit tests, and pass it into ImageResizer as externalFilesDirectory as its constructed in the tests.
  2. Write a sample bitmap to TemporaryFolder.
  3. Call ImageResizer.resizeImageIfNeeded with a combination of settings. Verify that the bitmap at the returned path exists and has been resized correctly for each one.

What do you think, does that sound workable to you? The catch here is I'm not sure how creating a Bitmap in a test goes, and I'm not sure if it's easy to check a Bitmap's image quality or not. I don't remember that API off the top of my head.

This is still an improvement over what we have so I am happy to merge this if needed, thank you for writing tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will take a look


ImageResizer resizer;
File imageFile;
File externalDirectory;
Bitmap originalImageBitmap;

@Before
public void setUp() throws IOException {
MockitoAnnotations.initMocks(this);
imageFile = new File(getClass().getClassLoader().getResource("pngImage.png").getFile());
originalImageBitmap = BitmapFactory.decodeFile(imageFile.getPath());
TemporaryFolder temporaryFolder = new TemporaryFolder();
temporaryFolder.create();
externalDirectory = temporaryFolder.newFolder("image_picker_testing_path");
resizer = new ImageResizer(externalDirectory, new ExifDataCopier());
}

@Test
public void onResizeImageIfNeeded_WhenQualityIsNull_ShoultNotResize_ReturnTheUnscaledFile() {
String outoutFile = resizer.resizeImageIfNeeded(imageFile.getPath(), null, null, null);
assertThat(outoutFile, equalTo(externalDirectory.getPath() + "/pngImage.png"));
}

@Test
public void onResizeImageIfNeeded_WhenQualityIsNotNull_ShoulResize_ReturnResizedFile() {
String outoutFile = resizer.resizeImageIfNeeded(imageFile.getPath(), null, null, 50);
assertThat(outoutFile, equalTo(externalDirectory.getPath() + "/scaled_pngImage.png"));
}

@Test
public void onResizeImageIfNeeded_WhenWidthIsNotNull_ShoulResize_ReturnResizedFile() {
String outoutFile = resizer.resizeImageIfNeeded(imageFile.getPath(), 50.0, null, null);
assertThat(outoutFile, equalTo(externalDirectory.getPath() + "/scaled_pngImage.png"));
}

@Test
public void onResizeImageIfNeeded_WhenHeightIsNotNull_ShoulResize_ReturnResizedFile() {
String outoutFile = resizer.resizeImageIfNeeded(imageFile.getPath(), null, 50.0, null);
assertThat(outoutFile, equalTo(externalDirectory.getPath() + "/scaled_pngImage.png"));
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion packages/image_picker/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ authors:
- Flutter Team <flutter-dev@googlegroups.com>
- Rhodes Davis Jr. <rody.davis.jr@gmail.com>
homepage: https://github.com/flutter/plugins/tree/master/packages/image_picker
version: 0.6.2
version: 0.6.2+1

flutter:
plugin:
Expand Down