From 861329226e3dffa0e096c3200fb6b96a1613ba83 Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Sat, 25 Nov 2017 13:18:23 -0800 Subject: [PATCH] Improve byte[] loading and consistency. Two significant issues are resolved here: 1. Using Glide.with().load(byte[]) and Glide.with().asDrawable().load(byte[]) used different sets of default options. 2. All byte[] loads default to using the same disk cache key unless override with a signature (which happens inconsistently depending on which variation of load() is called in #1). --- .../GlideRequests.java | 54 +++ .../GlideRequests.java | 54 +++ .../GlideRequests.java | 54 +++ .../com/bumptech/glide/Issue2638Test.java | 80 ---- .../com/bumptech/glide/LoadBytesTest.java | 422 ++++++++++++++++++ .../engine/executor/MockGlideExecutor.java | 116 +++++ .../glide/test/ConcurrencyHelper.java | 6 +- .../java/com/bumptech/glide/ModelTypes.java | 46 ++ .../com/bumptech/glide/RequestBuilder.java | 38 +- .../com/bumptech/glide/RequestManager.java | 98 +++- .../glide/load/model/ByteArrayLoader.java | 9 +- .../java/com/bumptech/glide/GlideTest.java | 17 +- 12 files changed, 879 insertions(+), 115 deletions(-) delete mode 100644 instrumentation/src/androidTest/java/com/bumptech/glide/Issue2638Test.java create mode 100644 instrumentation/src/androidTest/java/com/bumptech/glide/LoadBytesTest.java create mode 100644 instrumentation/src/androidTest/java/com/bumptech/glide/load/engine/executor/MockGlideExecutor.java create mode 100644 library/src/main/java/com/bumptech/glide/ModelTypes.java diff --git a/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideRequests.java b/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideRequests.java index 5a2c92d2fd..9a658eb842 100644 --- a/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideRequests.java +++ b/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideRequests.java @@ -3,6 +3,7 @@ import android.content.Context; import android.graphics.Bitmap; import android.graphics.drawable.Drawable; +import android.net.Uri; import android.support.annotation.CheckResult; import android.support.annotation.NonNull; import android.support.annotation.Nullable; @@ -14,9 +15,13 @@ import com.bumptech.glide.request.RequestOptions; import java.io.File; import java.lang.Class; +import java.lang.Deprecated; +import java.lang.Integer; import java.lang.Object; import java.lang.Override; +import java.lang.String; import java.lang.SuppressWarnings; +import java.net.URL; /** * Includes all additions from methods in {@link com.bumptech.glide.annotation.GlideExtension}s @@ -65,6 +70,55 @@ public GlideRequest asDrawable() { return (GlideRequest) super.asDrawable(); } + @Override + @CheckResult + public GlideRequest load(@Nullable Bitmap arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @CheckResult + public GlideRequest load(@Nullable Drawable arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @CheckResult + public GlideRequest load(@Nullable String arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @CheckResult + public GlideRequest load(@Nullable Uri arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @CheckResult + public GlideRequest load(@Nullable File arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @CheckResult + public GlideRequest load(@Nullable Integer arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @Deprecated + @CheckResult + public GlideRequest load(@Nullable URL arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @CheckResult + public GlideRequest load(@Nullable byte[] arg0) { + return (GlideRequest) super.load(arg0); + } + @Override @CheckResult public GlideRequest load(@Nullable Object arg0) { diff --git a/annotation/compiler/test/src/test/resources/GlideExtensionWithTypeTest/GlideRequests.java b/annotation/compiler/test/src/test/resources/GlideExtensionWithTypeTest/GlideRequests.java index 99aa810a16..7d02b4b5a9 100644 --- a/annotation/compiler/test/src/test/resources/GlideExtensionWithTypeTest/GlideRequests.java +++ b/annotation/compiler/test/src/test/resources/GlideExtensionWithTypeTest/GlideRequests.java @@ -3,6 +3,7 @@ import android.content.Context; import android.graphics.Bitmap; import android.graphics.drawable.Drawable; +import android.net.Uri; import android.support.annotation.CheckResult; import android.support.annotation.NonNull; import android.support.annotation.Nullable; @@ -14,10 +15,14 @@ import com.bumptech.glide.request.RequestOptions; import java.io.File; import java.lang.Class; +import java.lang.Deprecated; +import java.lang.Integer; import java.lang.Number; import java.lang.Object; import java.lang.Override; +import java.lang.String; import java.lang.SuppressWarnings; +import java.net.URL; /** * Includes all additions from methods in {@link com.bumptech.glide.annotation.GlideExtension}s @@ -73,6 +78,55 @@ public GlideRequest asDrawable() { return (GlideRequest) super.asDrawable(); } + @Override + @CheckResult + public GlideRequest load(@Nullable Bitmap arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @CheckResult + public GlideRequest load(@Nullable Drawable arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @CheckResult + public GlideRequest load(@Nullable String arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @CheckResult + public GlideRequest load(@Nullable Uri arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @CheckResult + public GlideRequest load(@Nullable File arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @CheckResult + public GlideRequest load(@Nullable Integer arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @Deprecated + @CheckResult + public GlideRequest load(@Nullable URL arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @CheckResult + public GlideRequest load(@Nullable byte[] arg0) { + return (GlideRequest) super.load(arg0); + } + @Override @CheckResult public GlideRequest load(@Nullable Object arg0) { diff --git a/annotation/compiler/test/src/test/resources/LegacyGlideExtensionWithTypeTest/GlideRequests.java b/annotation/compiler/test/src/test/resources/LegacyGlideExtensionWithTypeTest/GlideRequests.java index e8cf9ea524..287ab61715 100644 --- a/annotation/compiler/test/src/test/resources/LegacyGlideExtensionWithTypeTest/GlideRequests.java +++ b/annotation/compiler/test/src/test/resources/LegacyGlideExtensionWithTypeTest/GlideRequests.java @@ -3,6 +3,7 @@ import android.content.Context; import android.graphics.Bitmap; import android.graphics.drawable.Drawable; +import android.net.Uri; import android.support.annotation.CheckResult; import android.support.annotation.NonNull; import android.support.annotation.Nullable; @@ -14,10 +15,14 @@ import com.bumptech.glide.request.RequestOptions; import java.io.File; import java.lang.Class; +import java.lang.Deprecated; +import java.lang.Integer; import java.lang.Number; import java.lang.Object; import java.lang.Override; +import java.lang.String; import java.lang.SuppressWarnings; +import java.net.URL; /** * Includes all additions from methods in {@link com.bumptech.glide.annotation.GlideExtension}s @@ -75,6 +80,55 @@ public GlideRequest asDrawable() { return (GlideRequest) super.asDrawable(); } + @Override + @CheckResult + public GlideRequest load(@Nullable Bitmap arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @CheckResult + public GlideRequest load(@Nullable Drawable arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @CheckResult + public GlideRequest load(@Nullable String arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @CheckResult + public GlideRequest load(@Nullable Uri arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @CheckResult + public GlideRequest load(@Nullable File arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @CheckResult + public GlideRequest load(@Nullable Integer arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @Deprecated + @CheckResult + public GlideRequest load(@Nullable URL arg0) { + return (GlideRequest) super.load(arg0); + } + + @Override + @CheckResult + public GlideRequest load(@Nullable byte[] arg0) { + return (GlideRequest) super.load(arg0); + } + @Override @CheckResult public GlideRequest load(@Nullable Object arg0) { diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/Issue2638Test.java b/instrumentation/src/androidTest/java/com/bumptech/glide/Issue2638Test.java deleted file mode 100644 index 2696fc73bc..0000000000 --- a/instrumentation/src/androidTest/java/com/bumptech/glide/Issue2638Test.java +++ /dev/null @@ -1,80 +0,0 @@ -package com.bumptech.glide; - -import android.content.Context; -import android.content.res.Resources; -import android.graphics.Bitmap; -import android.graphics.Bitmap.CompressFormat; -import android.graphics.BitmapFactory; -import android.graphics.Color; -import android.graphics.drawable.BitmapDrawable; -import android.support.test.InstrumentationRegistry; -import android.support.test.runner.AndroidJUnit4; -import android.widget.AbsListView.LayoutParams; -import android.widget.ImageView; -import com.bumptech.glide.test.BitmapSubject; -import com.bumptech.glide.test.ConcurrencyHelper; -import com.bumptech.glide.test.ResourceIds; -import com.bumptech.glide.test.TearDownGlide; -import com.google.common.io.ByteStreams; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.util.concurrent.ExecutionException; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.runner.RunWith; - -@RunWith(AndroidJUnit4.class) -public class Issue2638Test { - @Rule public final TearDownGlide tearDownGlide = new TearDownGlide(); - private final ConcurrencyHelper concurrency = new ConcurrencyHelper(); - private Context context; - - @Before - public void setUp() { - context = InstrumentationRegistry.getTargetContext(); - } - - @Test - public void intoImageView_withDifferentByteArrays_loadsDifferentImages() - throws IOException, ExecutionException, InterruptedException { - final ImageView imageView = new ImageView(context); - imageView.setLayoutParams(new LayoutParams(/*w=*/ 100, /*h=*/ 100)); - - final byte[] canonicalBytes = getCanonicalBytes(); - final byte[] modifiedBytes = getModifiedBytes(); - - Glide.with(context) - .load(canonicalBytes) - .submit() - .get(); - - concurrency.loadOnMainThread(Glide.with(context).load(canonicalBytes), imageView); - Bitmap firstBitmap = ((BitmapDrawable) imageView.getDrawable()).getBitmap(); - - concurrency.loadOnMainThread(Glide.with(context).load(modifiedBytes), imageView); - Bitmap secondBitmap = ((BitmapDrawable) imageView.getDrawable()).getBitmap(); - - BitmapSubject.assertThat(firstBitmap).isNotSameAs(secondBitmap); - } - - private byte[] getModifiedBytes() throws IOException { - byte[] canonicalBytes = getCanonicalBytes(); - BitmapFactory.Options options = new BitmapFactory.Options(); - options.inMutable = true; - Bitmap bitmap = - BitmapFactory.decodeByteArray(canonicalBytes, 0, canonicalBytes.length, options); - bitmap.setPixel(0, 0, Color.TRANSPARENT); - ByteArrayOutputStream os = new ByteArrayOutputStream(); - bitmap.compress(CompressFormat.PNG, /*quality=*/ 100, os); - return os.toByteArray(); - } - - private byte[] getCanonicalBytes() throws IOException { - int resourceId = ResourceIds.raw.canonical; - Resources resources = context.getResources(); - InputStream is = resources.openRawResource(resourceId); - return ByteStreams.toByteArray(is); - } -} diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/LoadBytesTest.java b/instrumentation/src/androidTest/java/com/bumptech/glide/LoadBytesTest.java new file mode 100644 index 0000000000..06289e239f --- /dev/null +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/LoadBytesTest.java @@ -0,0 +1,422 @@ +package com.bumptech.glide; + +import static com.bumptech.glide.test.GlideOptions.skipMemoryCacheOf; +import static com.bumptech.glide.test.Matchers.anyDrawable; +import static com.bumptech.glide.test.Matchers.anyTarget; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyBoolean; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.verify; + +import android.content.Context; +import android.content.res.Resources; +import android.graphics.Bitmap; +import android.graphics.Bitmap.CompressFormat; +import android.graphics.Bitmap.Config; +import android.graphics.BitmapFactory; +import android.graphics.drawable.BitmapDrawable; +import android.graphics.drawable.Drawable; +import android.support.test.InstrumentationRegistry; +import android.support.test.runner.AndroidJUnit4; +import android.widget.AbsListView.LayoutParams; +import android.widget.ImageView; +import com.bumptech.glide.load.DataSource; +import com.bumptech.glide.load.engine.DiskCacheStrategy; +import com.bumptech.glide.load.engine.executor.GlideExecutor; +import com.bumptech.glide.load.engine.executor.MockGlideExecutor; +import com.bumptech.glide.request.FutureTarget; +import com.bumptech.glide.request.RequestListener; +import com.bumptech.glide.test.BitmapSubject; +import com.bumptech.glide.test.ConcurrencyHelper; +import com.bumptech.glide.test.GlideApp; +import com.bumptech.glide.test.ResourceIds; +import com.bumptech.glide.test.TearDownGlide; +import com.google.common.io.ByteStreams; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +@RunWith(AndroidJUnit4.class) +public class LoadBytesTest { + @Rule public final TearDownGlide tearDownGlide = new TearDownGlide(); + private final ConcurrencyHelper concurrency = new ConcurrencyHelper(); + + @Mock private RequestListener requestListener; + + private Context context; + private ImageView imageView; + + @Before + public void setUp() throws IOException { + MockitoAnnotations.initMocks(this); + context = InstrumentationRegistry.getTargetContext(); + + imageView = new ImageView(context); + int[] dimensions = getCanonicalDimensions(); + imageView.setLayoutParams(new LayoutParams(/*w=*/ dimensions[0], /*h=*/ dimensions[1])); + + // Writes to the resource disk cache run in a non-blocking manner after the Target is notified. + // Unless we enforce a single threaded executor, the encode task races with our second decode + // task, causing the test to sometimes fail (when the second resource is started after the + // encode and loaded from the disk cache) and sometimes succeed (when the second resource is + // started before the encode and loads from source). + ExecutorService executor = Executors.newSingleThreadExecutor(); + GlideExecutor glideExecutor = MockGlideExecutor.newTestExecutor(executor); + Glide.init(context, new GlideBuilder() + .setAnimationExecutor(glideExecutor) + .setDiskCacheExecutor(glideExecutor) + .setSourceExecutor(glideExecutor)); + } + + @Test + public void loadFromRequestManager_intoImageView_withDifferentByteArrays_loadsDifferentImages() + throws IOException, ExecutionException, InterruptedException { + final byte[] canonicalBytes = getCanonicalBytes(); + final byte[] modifiedBytes = getModifiedBytes(); + + concurrency.loadOnMainThread( + Glide.with(context).load(canonicalBytes), imageView); + Bitmap firstBitmap = copyFromImageViewDrawable(imageView); + + concurrency.loadOnMainThread( + Glide.with(context).load(modifiedBytes), imageView); + Bitmap secondBitmap = copyFromImageViewDrawable(imageView); + + // This assertion alone doesn't catch the case where the second Bitmap is loaded from the result + // cache of the data from the first Bitmap. + BitmapSubject.assertThat(firstBitmap).isNotSameAs(secondBitmap); + + Bitmap expectedCanonicalBitmap = + BitmapFactory.decodeByteArray(canonicalBytes, /*offset=*/ 0, canonicalBytes.length); + BitmapSubject.assertThat(firstBitmap).sameAs(expectedCanonicalBitmap); + + Bitmap expectedModifiedBitmap = + BitmapFactory.decodeByteArray(modifiedBytes, /*offset=*/ 0, modifiedBytes.length); + BitmapSubject.assertThat(secondBitmap).sameAs(expectedModifiedBitmap); + } + + @Test + public void loadFromRequestBuilder_intoImageView_withDifferentByteArrays_loadsDifferentImages() + throws IOException, ExecutionException, InterruptedException { + final byte[] canonicalBytes = getCanonicalBytes(); + final byte[] modifiedBytes = getModifiedBytes(); + + concurrency.loadOnMainThread( + GlideApp.with(context) + .asDrawable() + .load(canonicalBytes), + imageView); + Bitmap firstBitmap = copyFromImageViewDrawable(imageView); + + concurrency.loadOnMainThread( + GlideApp.with(context) + .asDrawable() + .load(modifiedBytes), + imageView); + Bitmap secondBitmap = copyFromImageViewDrawable(imageView); + + // This assertion alone doesn't catch the case where the second Bitmap is loaded from the result + // cache of the data from the first Bitmap. + BitmapSubject.assertThat(firstBitmap).isNotSameAs(secondBitmap); + + Bitmap expectedCanonicalBitmap = + BitmapFactory.decodeByteArray(canonicalBytes, /*offset=*/ 0, canonicalBytes.length); + BitmapSubject.assertThat(firstBitmap).sameAs(expectedCanonicalBitmap); + + Bitmap expectedModifiedBitmap = + BitmapFactory.decodeByteArray(modifiedBytes, /*offset=*/ 0, modifiedBytes.length); + BitmapSubject.assertThat(secondBitmap).sameAs(expectedModifiedBitmap); + } + + @Test + public void requestManager_intoImageView_withSameByteArrayAndMemoryCacheEnabled_loadsFromMemory() + throws IOException { + final byte[] canonicalBytes = getCanonicalBytes(); + concurrency.loadOnMainThread( + Glide.with(context) + .load(canonicalBytes) + .apply(skipMemoryCacheOf(false)), + imageView); + + Glide.with(context).clear(imageView); + + concurrency.loadOnMainThread( + Glide.with(context) + .load(canonicalBytes) + .listener(requestListener) + .apply(skipMemoryCacheOf(false)), + imageView); + + verify(requestListener).onResourceReady( + anyDrawable(), any(), anyTarget(), eq(DataSource.MEMORY_CACHE), anyBoolean()); + } + + @Test + public void requestBuilder_intoImageView_withSameByteArrayAndMemoryCacheEnabled_loadsFromMemory() + throws IOException { + final byte[] canonicalBytes = getCanonicalBytes(); + concurrency.loadOnMainThread( + Glide.with(context) + .asDrawable() + .load(canonicalBytes) + .apply(skipMemoryCacheOf(false)), + imageView); + + Glide.with(context).clear(imageView); + + concurrency.loadOnMainThread( + Glide.with(context) + .asDrawable() + .load(canonicalBytes) + .listener(requestListener) + .apply(skipMemoryCacheOf(false)), + imageView); + + verify(requestListener).onResourceReady( + anyDrawable(), any(), anyTarget(), eq(DataSource.MEMORY_CACHE), anyBoolean()); + } + + @Test + public void loadFromRequestManager_withSameByteArray_validDiskCacheStrategy_returnsFromDiskCache() + throws IOException { + byte[] data = getCanonicalBytes(); + FutureTarget target = concurrency.wait( + GlideApp.with(context) + .load(data) + .diskCacheStrategy(DiskCacheStrategy.RESOURCE) + .submit()); + GlideApp.with(context).clear(target); + + concurrency.runOnMainThread(new Runnable() { + @Override + public void run() { + GlideApp.get(context).clearMemory(); + } + }); + + concurrency.wait( + GlideApp.with(context) + .load(data) + .diskCacheStrategy(DiskCacheStrategy.RESOURCE) + .listener(requestListener) + .submit()); + + verify(requestListener).onResourceReady( + anyDrawable(), any(), anyTarget(), eq(DataSource.RESOURCE_DISK_CACHE), anyBoolean()); + } + + @Test + public void loadFromRequestBuilder_withSameByteArray_validDiskCacheStrategy_returnsFromDiskCache() + throws IOException { + byte[] data = getCanonicalBytes(); + FutureTarget target = concurrency.wait( + GlideApp.with(context) + .asDrawable() + .load(data) + .diskCacheStrategy(DiskCacheStrategy.RESOURCE) + .submit()); + GlideApp.with(context).clear(target); + + concurrency.runOnMainThread(new Runnable() { + @Override + public void run() { + GlideApp.get(context).clearMemory(); + } + }); + + concurrency.wait( + GlideApp.with(context) + .asDrawable() + .load(data) + .diskCacheStrategy(DiskCacheStrategy.RESOURCE) + .listener(requestListener) + .submit()); + + verify(requestListener).onResourceReady( + anyDrawable(), any(), anyTarget(), eq(DataSource.RESOURCE_DISK_CACHE), anyBoolean()); + } + + @Test + public void loadFromRequestManager_withSameByteArray_memoryCacheEnabled_returnsFromCache() + throws IOException { + byte[] data = getCanonicalBytes(); + FutureTarget target = concurrency.wait( + GlideApp.with(context) + .load(data) + .skipMemoryCache(false) + .submit()); + GlideApp.with(context).clear(target); + + concurrency.wait( + GlideApp.with(context) + .load(data) + .skipMemoryCache(false) + .listener(requestListener) + .submit()); + + verify(requestListener).onResourceReady( + anyDrawable(), any(), anyTarget(), eq(DataSource.MEMORY_CACHE), anyBoolean()); + } + + @Test + public void loadFromRequestBuilder_withSameByteArray_memoryCacheEnabled_returnsFromCache() + throws IOException { + byte[] data = getCanonicalBytes(); + FutureTarget target = concurrency.wait( + GlideApp.with(context) + .asDrawable() + .load(data) + .skipMemoryCache(false) + .submit()); + GlideApp.with(context).clear(target); + + concurrency.wait( + GlideApp.with(context) + .asDrawable() + .load(data) + .skipMemoryCache(false) + .listener(requestListener) + .submit()); + + verify(requestListener).onResourceReady( + anyDrawable(), any(), anyTarget(), eq(DataSource.MEMORY_CACHE), anyBoolean()); + } + + @Test + public void loadFromRequestManager_withSameByteArray_returnsFromLocal() throws IOException { + byte[] data = getCanonicalBytes(); + FutureTarget target = concurrency.wait( + GlideApp.with(context) + .load(data) + .submit()); + GlideApp.with(context).clear(target); + + concurrency.wait( + GlideApp.with(context) + .load(data) + .listener(requestListener) + .submit()); + + verify(requestListener).onResourceReady( + anyDrawable(), any(), anyTarget(), eq(DataSource.LOCAL), anyBoolean()); + } + + @Test + public void loadFromRequestBuilder_withSameByteArray_returnsFromLocal() throws IOException { + byte[] data = getCanonicalBytes(); + FutureTarget target = concurrency.wait( + GlideApp.with(context) + .asDrawable() + .load(data) + .submit()); + GlideApp.with(context).clear(target); + + concurrency.wait( + GlideApp.with(context) + .asDrawable() + .load(data) + .listener(requestListener) + .submit()); + + verify(requestListener).onResourceReady( + anyDrawable(), any(), anyTarget(), eq(DataSource.LOCAL), anyBoolean()); + } + + @Test + public void loadFromRequestManager_withSameByteArrayAndMissingFromMemory_returnsFromLocal() + throws IOException { + byte[] data = getCanonicalBytes(); + FutureTarget target = concurrency.wait( + GlideApp.with(context) + .load(data) + .submit()); + GlideApp.with(context).clear(target); + + concurrency.runOnMainThread(new Runnable() { + @Override + public void run() { + GlideApp.get(context).clearMemory(); + } + }); + + concurrency.wait( + GlideApp.with(context) + .load(data) + .listener(requestListener) + .submit()); + + verify(requestListener).onResourceReady( + anyDrawable(), any(), anyTarget(), eq(DataSource.LOCAL), anyBoolean()); + } + + @Test + public void loadFromRequestBuilder_withSameByteArrayAndMissingFromMemory_returnsFromLocal() + throws IOException { + byte[] data = getCanonicalBytes(); + FutureTarget target = concurrency.wait( + GlideApp.with(context) + .asDrawable() + .load(data) + .submit()); + GlideApp.with(context).clear(target); + + concurrency.runOnMainThread(new Runnable() { + @Override + public void run() { + GlideApp.get(context).clearMemory(); + } + }); + + concurrency.wait( + GlideApp.with(context) + .asDrawable() + .load(data) + .listener(requestListener) + .submit()); + + verify(requestListener).onResourceReady( + anyDrawable(), any(), anyTarget(), eq(DataSource.LOCAL), anyBoolean()); + } + + private Bitmap copyFromImageViewDrawable(ImageView imageView) { + if (imageView.getDrawable() == null) { + fail("Drawable unexpectedly null"); + } + + // Glide mutates Bitmaps, so it's possible that a Bitmap loaded into a View in one place may + // be re-used to load a different image later. Create a defensive copy just in case. + return Bitmap.createBitmap(((BitmapDrawable) imageView.getDrawable()).getBitmap()); + } + + private int[] getCanonicalDimensions() throws IOException { + byte[] canonicalBytes = getCanonicalBytes(); + Bitmap bitmap = + BitmapFactory.decodeByteArray(canonicalBytes, /*offset=*/ 0, canonicalBytes.length); + return new int[] { bitmap.getWidth(), bitmap.getHeight() }; + } + + private byte[] getModifiedBytes() throws IOException { + int[] dimensions = getCanonicalDimensions(); + Bitmap bitmap = Bitmap.createBitmap(dimensions[0], dimensions[1], Config.ARGB_8888); + ByteArrayOutputStream os = new ByteArrayOutputStream(); + bitmap.compress(CompressFormat.PNG, /*quality=*/ 100, os); + return os.toByteArray(); + } + + private byte[] getCanonicalBytes() throws IOException { + int resourceId = ResourceIds.raw.canonical; + Resources resources = context.getResources(); + InputStream is = resources.openRawResource(resourceId); + return ByteStreams.toByteArray(is); + } +} diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/load/engine/executor/MockGlideExecutor.java b/instrumentation/src/androidTest/java/com/bumptech/glide/load/engine/executor/MockGlideExecutor.java new file mode 100644 index 0000000000..94ef1782c7 --- /dev/null +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/load/engine/executor/MockGlideExecutor.java @@ -0,0 +1,116 @@ +package com.bumptech.glide.load.engine.executor; + +import android.os.StrictMode; +import android.support.annotation.NonNull; +import android.support.annotation.VisibleForTesting; +import com.google.common.util.concurrent.ForwardingExecutorService; +import com.google.common.util.concurrent.MoreExecutors; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; + +/** + * Creates mock {@link GlideExecutor}s. + */ +@VisibleForTesting +public final class MockGlideExecutor { + private MockGlideExecutor() { + // Utility class. + } + + // Public API. + @SuppressWarnings("WeakerAccess") + public static GlideExecutor newTestExecutor(ExecutorService executorService) { + return new GlideExecutor(executorService); + } + + public static GlideExecutor newMainThreadExecutor() { + return newTestExecutor(new DirectExecutorService()); + } + + /** + * @deprecated Use {@link #newMainThreadExecutor} instead. + */ + @Deprecated + public static GlideExecutor newMainThreadUnlimitedExecutor() { + return newMainThreadExecutor(); + } + + /** + * DirectExecutorService that enforces StrictMode and converts ExecutionExceptions into + * RuntimeExceptions. + */ + private static final class DirectExecutorService extends ForwardingExecutorService { + private static final StrictMode.ThreadPolicy THREAD_POLICY = + new StrictMode.ThreadPolicy.Builder() + .detectNetwork() + .penaltyDeath() + .build(); + + private final ExecutorService delegate; + + DirectExecutorService() { + delegate = MoreExecutors.newDirectExecutorService(); + } + + @Override + protected ExecutorService delegate() { + return delegate; + } + + @NonNull + @Override + public Future submit(@NonNull Runnable task, @NonNull T result) { + return getUninterruptibly(super.submit(task, result)); + } + + @NonNull + @Override + public Future submit(@NonNull Callable task) { + return getUninterruptibly(super.submit(task)); + } + + @NonNull + @Override + public Future submit(@NonNull Runnable task) { + return getUninterruptibly(super.submit(task)); + } + + @Override + public void execute(@NonNull final Runnable command) { + delegate.execute(new Runnable() { + @Override + public void run() { + StrictMode.ThreadPolicy oldPolicy = StrictMode.getThreadPolicy(); + StrictMode.setThreadPolicy(THREAD_POLICY); + try { + command.run(); + } finally { + StrictMode.setThreadPolicy(oldPolicy); + } + } + }); + } + + private Future getUninterruptibly(Future future) { + boolean interrupted = false; + try { + while (!future.isDone()) { + try { + future.get(); + } catch (ExecutionException e) { + throw new RuntimeException(e); + } catch (InterruptedException e) { + interrupted = true; + } + } + } finally { + if (interrupted) { + Thread.currentThread().interrupt(); + } + } + return future; + } + } +} diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/test/ConcurrencyHelper.java b/instrumentation/src/androidTest/java/com/bumptech/glide/test/ConcurrencyHelper.java index a44b59056b..df6eb1be13 100644 --- a/instrumentation/src/androidTest/java/com/bumptech/glide/test/ConcurrencyHelper.java +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/test/ConcurrencyHelper.java @@ -21,7 +21,6 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicReference; /** * Helper for running sections of code on the main thread in emulator tests. @@ -249,14 +248,12 @@ public Void call() throws Exception { } private void callOnMainThread(final Callable callable) { - final AtomicReference reference = new AtomicReference<>(); final CountDownLatch latch = new CountDownLatch(1); handler.post(new Runnable() { @Override public void run() { try { - T result = callable.call(); - reference.set(result); + callable.call(); } catch (Exception e) { throw new RuntimeException(e); } @@ -264,7 +261,6 @@ public void run() { } }); waitOnLatch(latch); - reference.get(); } private static void waitOnLatch(CountDownLatch latch) { diff --git a/library/src/main/java/com/bumptech/glide/ModelTypes.java b/library/src/main/java/com/bumptech/glide/ModelTypes.java new file mode 100644 index 0000000000..71c77fb1c8 --- /dev/null +++ b/library/src/main/java/com/bumptech/glide/ModelTypes.java @@ -0,0 +1,46 @@ +package com.bumptech.glide; + +import android.graphics.Bitmap; +import android.graphics.drawable.Drawable; +import android.net.Uri; +import android.support.annotation.CheckResult; +import android.support.annotation.DrawableRes; +import android.support.annotation.Nullable; +import android.support.annotation.RawRes; +import java.io.File; +import java.net.URL; + +/** + * Ensures that the set of explicitly supported model types remains consistent across Glide's + * API surface. + */ +interface ModelTypes { + @CheckResult + T load(@Nullable Bitmap bitmap); + + @CheckResult + T load(@Nullable Drawable drawable); + + @CheckResult + T load(@Nullable String string); + + @CheckResult + T load(@Nullable Uri uri); + + @CheckResult + T load(@Nullable File file); + + @CheckResult + T load(@RawRes @DrawableRes @Nullable Integer resourceId); + + @Deprecated + @CheckResult + T load(@Nullable URL url); + + @CheckResult + T load(@Nullable byte[] model); + + @CheckResult + @SuppressWarnings("unchecked") + T load(@Nullable Object model); +} diff --git a/library/src/main/java/com/bumptech/glide/RequestBuilder.java b/library/src/main/java/com/bumptech/glide/RequestBuilder.java index 7af04eeca8..d887f72a42 100644 --- a/library/src/main/java/com/bumptech/glide/RequestBuilder.java +++ b/library/src/main/java/com/bumptech/glide/RequestBuilder.java @@ -28,12 +28,10 @@ import com.bumptech.glide.request.target.Target; import com.bumptech.glide.request.target.ViewTarget; import com.bumptech.glide.signature.ApplicationVersionSignature; -import com.bumptech.glide.signature.ObjectKey; import com.bumptech.glide.util.Preconditions; import com.bumptech.glide.util.Util; import java.io.File; import java.net.URL; -import java.util.UUID; /** * A generic class that can handle setting options and staring loads for generic resource types. @@ -43,7 +41,8 @@ */ // Public API. @SuppressWarnings({"unused", "WeakerAccess"}) -public class RequestBuilder implements Cloneable { +public class RequestBuilder implements Cloneable, + ModelTypes> { // Used in generated subclasses protected static final RequestOptions DOWNLOAD_ONLY_OPTIONS = new RequestOptions().diskCacheStrategy(DiskCacheStrategy.DATA).priority(Priority.LOW) @@ -303,14 +302,12 @@ public RequestBuilder thumbnail(float sizeMultiplier) { /** * Sets the specific model to load data for. * - *

This method must be called at least once before - * {@link #into(com.bumptech.glide.request.target.Target)} is called.

- * * @param model The model to load data for, or null. * @return This request builder. */ @CheckResult @SuppressWarnings("unchecked") + @Override public RequestBuilder load(@Nullable Object model) { return loadGeneric(model); } @@ -320,9 +317,8 @@ private RequestBuilder loadGeneric(@Nullable Object model) { isModelSet = true; return this; } - /** - * Returns a request builder to load the given {@link Bitmap}. + * Returns an object to load the given {@link Bitmap}. * *

It's almost always better to allow Glide to load {@link Bitmap}s than * pass {@link Bitmap}s into Glide. If you have a custom way to obtain {@link Bitmap}s that is @@ -339,6 +335,7 @@ private RequestBuilder loadGeneric(@Nullable Object model) { * @see #load(Object) */ @CheckResult + @Override public RequestBuilder load(@Nullable Bitmap bitmap) { return loadGeneric(bitmap) .apply(diskCacheStrategyOf(DiskCacheStrategy.NONE)); @@ -362,6 +359,7 @@ public RequestBuilder load(@Nullable Bitmap bitmap) { * @see #load(Object) */ @CheckResult + @Override public RequestBuilder load(@Nullable Drawable drawable) { return loadGeneric(drawable) .apply(diskCacheStrategyOf(DiskCacheStrategy.NONE)); @@ -386,6 +384,7 @@ public RequestBuilder load(@Nullable Drawable drawable) { * @param string A file path, or a uri or url handled by * {@link com.bumptech.glide.load.model.UriLoader}. */ + @Override @CheckResult public RequestBuilder load(@Nullable String string) { return loadGeneric(string); @@ -410,6 +409,7 @@ public RequestBuilder load(@Nullable String string) { * {@link com.bumptech.glide.load.model.UriLoader}. */ @CheckResult + @Override public RequestBuilder load(@Nullable Uri uri) { return loadGeneric(uri); } @@ -417,7 +417,7 @@ public RequestBuilder load(@Nullable Uri uri) { /** * Returns a request builder to load the given {@link File}. * - *

Note - this method caches data for Files using only the file path itself as the cache key. + *

Note - this method caches data for Files using only the file path itself as the cache key. * The data in the File can change so using this method can lead to displaying stale data. If you * expect the data in the File to change, Consider using * {@link com.bumptech.glide.request.RequestOptions#signature(com.bumptech.glide.load.Key)} @@ -426,13 +426,13 @@ public RequestBuilder load(@Nullable Uri uri) { * {@link com.bumptech.glide.load.engine.DiskCacheStrategy#NONE} and/or * {@link com.bumptech.glide.request.RequestOptions#skipMemoryCache(boolean)} may be * appropriate. - *

* * @see #load(Object) * * @param file The File containing the image */ @CheckResult + @Override public RequestBuilder load(@Nullable File file) { return loadGeneric(file); } @@ -470,6 +470,7 @@ public RequestBuilder load(@Nullable File file) { * @see com.bumptech.glide.signature.ApplicationVersionSignature */ @CheckResult + @Override public RequestBuilder load(@RawRes @DrawableRes @Nullable Integer resourceId) { return loadGeneric(resourceId).apply(signatureOf(ApplicationVersionSignature.obtain(context))); } @@ -479,12 +480,13 @@ public RequestBuilder load(@RawRes @DrawableRes @Nullable Integer * * @param url The URL representing the image. * @see #load(Object) - * @deprecated The {@link java.net.URL} class has a number of - * performance problems and should generally be avoided when possible. Prefer - * {@link #load(android.net.Uri)} or {@link #load(String)}. + * @deprecated The {@link java.net.URL} class has + * a number of performance problems and should generally be + * avoided when possible. Prefer {@link #load(android.net.Uri)} or {@link #load(String)}. */ @Deprecated @CheckResult + @Override public RequestBuilder load(@Nullable URL url) { return loadGeneric(url); } @@ -492,16 +494,18 @@ public RequestBuilder load(@Nullable URL url) { /** * Returns a request to load the given byte array. * - *

Note - by default loads for bytes are not cached in either the memory or the disk cache. - *

+ *

Note - by default loads for bytes are not cached in either the memory or the disk cache. * * @param model the data to load. * @see #load(Object) */ @CheckResult + @Override public RequestBuilder load(@Nullable byte[] model) { - return loadGeneric(model).apply(signatureOf(new ObjectKey(UUID.randomUUID().toString())) - .diskCacheStrategy(DiskCacheStrategy.NONE).skipMemoryCache(true /*skipMemoryCache*/)); + return + loadGeneric(model) + .apply(diskCacheStrategyOf(DiskCacheStrategy.NONE) + .skipMemoryCache(true /*skipMemoryCache*/)); } /** diff --git a/library/src/main/java/com/bumptech/glide/RequestManager.java b/library/src/main/java/com/bumptech/glide/RequestManager.java index 0475262f79..51f3a3ff4e 100644 --- a/library/src/main/java/com/bumptech/glide/RequestManager.java +++ b/library/src/main/java/com/bumptech/glide/RequestManager.java @@ -7,6 +7,7 @@ import android.content.Context; import android.graphics.Bitmap; import android.graphics.drawable.Drawable; +import android.net.Uri; import android.os.Handler; import android.os.Looper; import android.support.annotation.CheckResult; @@ -30,6 +31,7 @@ import com.bumptech.glide.util.Synthetic; import com.bumptech.glide.util.Util; import java.io.File; +import java.net.URL; /** * A class for managing and starting requests for Glide. Can use activity, fragment and connectivity @@ -43,7 +45,8 @@ * @see Glide#with(android.support.v4.app.Fragment) * @see Glide#with(Context) */ -public class RequestManager implements LifecycleListener { +public class RequestManager implements LifecycleListener, + ModelTypes> { private static final RequestOptions DECODE_TYPE_BITMAP = decodeTypeOf(Bitmap.class).lock(); private static final RequestOptions DECODE_TYPE_GIF = decodeTypeOf(GifDrawable.class).lock(); private static final RequestOptions DOWNLOAD_ONLY_OPTIONS = @@ -343,6 +346,98 @@ public RequestBuilder asDrawable() { return as(Drawable.class); } + /** + * Equivalent to calling {@link #asDrawable()} and then {@link RequestBuilder#load(Bitmap)}. + * + * @return A new request builder for loading a {@link Drawable} using the given model. + */ + @CheckResult + @Override + public RequestBuilder load(@Nullable Bitmap bitmap) { + return asDrawable().load(bitmap); + } + + /** + * Equivalent to calling {@link #asDrawable()} and then {@link RequestBuilder#load(Drawable)}. + * + * @return A new request builder for loading a {@link Drawable} using the given model. + */ + @CheckResult + @Override + public RequestBuilder load(@Nullable Drawable drawable) { + return asDrawable().load(drawable); + } + + /** + * Equivalent to calling {@link #asDrawable()} and then {@link RequestBuilder#load(String)}. + * + * @return A new request builder for loading a {@link Drawable} using the given model. + */ + @CheckResult + @Override + public RequestBuilder load(@Nullable String string) { + return asDrawable().load(string); + } + + /** + * Equivalent to calling {@link #asDrawable()} and then {@link RequestBuilder#load(Uri)}. + * + * @return A new request builder for loading a {@link Drawable} using the given model. + */ + @CheckResult + @Override + public RequestBuilder load(@Nullable Uri uri) { + return asDrawable().load(uri); + } + + /** + * Equivalent to calling {@link #asDrawable()} and then {@link RequestBuilder#load(File)}. + * + * @return A new request builder for loading a {@link Drawable} using the given model. + */ + @CheckResult + @Override + public RequestBuilder load(@Nullable File file) { + return asDrawable().load(file); + } + + /** + * Equivalent to calling {@link #asDrawable()} and then {@link RequestBuilder#load(Integer)}. + * + * @return A new request builder for loading a {@link Drawable} using the given model. + */ + @SuppressWarnings("deprecation") + @CheckResult + @Override + public RequestBuilder load(@Nullable Integer resourceId) { + return asDrawable().load(resourceId); + } + + /** + * Equivalent to calling {@link #asDrawable()} and then {@link RequestBuilder#load(URL). + * + * @return A new request builder for loading a {@link Drawable} using the given model. + */ + @SuppressWarnings("deprecation") + @CheckResult + @Override + @Deprecated + public RequestBuilder load(@Nullable URL url) { + return asDrawable().load(url); + } + + + /** + * Equivalent to calling {@link #asDrawable()} and then {@link RequestBuilder#load(byte[])}. + * + * @return A new request builder for loading a {@link Drawable} using the given model. + */ + @CheckResult + @Override + public RequestBuilder load(@Nullable byte[] model) { + return asDrawable().load(model); + } + /** * A helper method equivalent to calling {@link #asDrawable()} and then {@link * RequestBuilder#load(Object)} with the given model. @@ -350,6 +445,7 @@ public RequestBuilder asDrawable() { * @return A new request builder for loading a {@link Drawable} using the given model. */ @CheckResult + @Override public RequestBuilder load(@Nullable Object model) { return asDrawable().load(model); } diff --git a/library/src/main/java/com/bumptech/glide/load/model/ByteArrayLoader.java b/library/src/main/java/com/bumptech/glide/load/model/ByteArrayLoader.java index 9f55c71c3f..c0264e40ba 100644 --- a/library/src/main/java/com/bumptech/glide/load/model/ByteArrayLoader.java +++ b/library/src/main/java/com/bumptech/glide/load/model/ByteArrayLoader.java @@ -5,7 +5,7 @@ import com.bumptech.glide.load.DataSource; import com.bumptech.glide.load.Options; import com.bumptech.glide.load.data.DataFetcher; -import com.bumptech.glide.signature.EmptySignature; +import com.bumptech.glide.signature.ObjectKey; import java.io.ByteArrayInputStream; import java.io.InputStream; import java.nio.ByteBuffer; @@ -27,10 +27,9 @@ public ByteArrayLoader(Converter converter) { } @Override - public LoadData buildLoadData(byte[] model, int width, int height, - Options options) { - // TODO: compare the actual bytes? - return new LoadData<>(EmptySignature.obtain(), new Fetcher<>(model, converter)); + public LoadData buildLoadData( + byte[] model, int width, int height, Options options) { + return new LoadData<>(new ObjectKey(model), new Fetcher<>(model, converter)); } @Override diff --git a/library/src/test/java/com/bumptech/glide/GlideTest.java b/library/src/test/java/com/bumptech/glide/GlideTest.java index 64d8df1106..f85f1692f6 100644 --- a/library/src/test/java/com/bumptech/glide/GlideTest.java +++ b/library/src/test/java/com/bumptech/glide/GlideTest.java @@ -98,6 +98,8 @@ GlideTest.MutableShadowBitmap.class }) @SuppressWarnings("unchecked") public class GlideTest { + // Fixes method overload confusion. + private static final Object NULL = null; @Rule public TearDownGlide tearDownGlide = new TearDownGlide(); @@ -276,6 +278,7 @@ private void runTestFileDefaultLoader() { assertNotNull(imageView.getDrawable()); } + @SuppressWarnings("deprecation") @Test public void testUrlDefaultLoader() throws MalformedURLException { URL url = new URL("http://www.google.com"); @@ -573,24 +576,24 @@ public void testReceivesRecursiveThumbnailWithPercentage() { @Test public void testNullModelInGenericImageLoadDoesNotThrow() { - requestManager.load(null).into(target); + requestManager.load(NULL).into(target); } @Test public void testNullModelInGenericVideoLoadDoesNotThrow() { - requestManager.load(null).into(target); + requestManager.load(NULL).into(target); } @Test public void testNullModelInGenericLoadDoesNotThrow() { - requestManager.load(null).into(target); + requestManager.load(NULL).into(target); } @Test public void testNullModelDoesNotThrow() { Drawable drawable = new ColorDrawable(Color.RED); requestManager - .load(null) + .load(NULL) .apply(errorOf(drawable)) .into(target); @@ -603,7 +606,7 @@ public void testNullModelPrefersErrorDrawable() { Drawable error = new ColorDrawable(Color.RED); requestManager - .load(null) + .load(NULL) .apply(placeholderOf(placeholder) .error(error)) .into(target); @@ -652,7 +655,7 @@ public void testNullModelPrefersFallbackDrawable() { Drawable fallback = new ColorDrawable(Color.BLUE); requestManager - .load(null) + .load(NULL) .apply(placeholderOf(placeholder) .error(error) .fallback(fallback)) @@ -666,7 +669,7 @@ public void testNullModelResolvesToUsePlaceholder() { Drawable placeholder = new ColorDrawable(Color.GREEN); requestManager - .load(null) + .load(NULL) .apply(placeholderOf(placeholder)) .into(target);