From 0917ef365ea35ca3e0f1eb12a2b7c843c84042f3 Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Tue, 30 Jan 2018 07:54:14 -0800 Subject: [PATCH] Avoid throwing when decoding Files with DiskCacheStrategy.AUTOMATIC. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AUTOMATIC allows decoding items from the resource cache in case the item being loaded is local. If the item is remote and the user is requesting a File however, the resource cache generator will throw an exception because it’s unable to find any path that will obtain a File from any Model. I’ve fixed this here by special casing Files and ignoring missing paths for them in ResourceCacheGenerator so that loads for File types won’t fail with automatic if they would have otherwise succeed with DiskCacheStrategy.DATA. I’ve also updated a few of the error messages to try to make it clearer why a load has failed if it fails for a similar reason. Fixes #2824. --- .../java/com/bumptech/glide/AsFileTest.java | 124 ++++++++++++++++++ .../bumptech/glide/test/MockModelLoader.java | 97 ++++++++++++++ .../java/com/bumptech/glide/Registry.java | 5 +- .../glide/load/engine/DecodeHelper.java | 8 ++ .../load/engine/ResourceCacheGenerator.java | 8 ++ 5 files changed, 241 insertions(+), 1 deletion(-) create mode 100644 instrumentation/src/androidTest/java/com/bumptech/glide/AsFileTest.java create mode 100644 instrumentation/src/androidTest/java/com/bumptech/glide/test/MockModelLoader.java diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/AsFileTest.java b/instrumentation/src/androidTest/java/com/bumptech/glide/AsFileTest.java new file mode 100644 index 0000000000..057c95c5a5 --- /dev/null +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/AsFileTest.java @@ -0,0 +1,124 @@ +package com.bumptech.glide; + + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.fail; + +import android.content.Context; +import android.support.test.InstrumentationRegistry; +import android.support.test.runner.AndroidJUnit4; +import com.bumptech.glide.load.engine.DiskCacheStrategy; +import com.bumptech.glide.test.ConcurrencyHelper; +import com.bumptech.glide.test.GlideApp; +import com.bumptech.glide.test.MockModelLoader; +import com.bumptech.glide.test.ResourceIds; +import com.bumptech.glide.test.TearDownGlide; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(AndroidJUnit4.class) +public class AsFileTest { + @Rule public final TearDownGlide tearDownGlide = new TearDownGlide(); + private final ConcurrencyHelper concurrency = new ConcurrencyHelper(); + private final Context context = InstrumentationRegistry.getTargetContext(); + + @Test + public void asFile_withUrl_succeeds() { + String url = "https://www.w3schools.com/howto/img_fjords.jpg"; + + MockModelLoader.mock(url, getData()); + + File file = + concurrency.get( + GlideApp.with(context) + .asFile() + .load("https://www.w3schools.com/howto/img_fjords.jpg") + .submit()); + assertThat(file).isNotNull(); + } + + @Test + public void asFile_withUrlAndDiskCacheStrategyData_succeeds() { + String url = "https://www.w3schools.com/howto/img_fjords.jpg"; + + MockModelLoader.mock(url, getData()); + + File file = + concurrency.get( + GlideApp.with(context) + .asFile() + .diskCacheStrategy(DiskCacheStrategy.DATA) + .load("https://www.w3schools.com/howto/img_fjords.jpg") + .submit()); + assertThat(file).isNotNull(); + } + + @Test + public void asFile_withUrlAndDiskCacheStrategyResource_fails() { + String url = "https://www.w3schools.com/howto/img_fjords.jpg"; + + MockModelLoader.mock(url, getData()); + + try { + concurrency.get( + GlideApp.with(context) + .asFile() + .diskCacheStrategy(DiskCacheStrategy.RESOURCE) + .load("https://www.w3schools.com/howto/img_fjords.jpg") + .submit()); + fail(); + } catch (RuntimeException e) { + // expected. + } + } + + @Test + public void asFile_withUrlAndDiskCacheStrategyAll_fails() { + String url = "https://www.w3schools.com/howto/img_fjords.jpg"; + + MockModelLoader.mock(url, getData()); + + try { + concurrency.get( + GlideApp.with(context) + .asFile() + .diskCacheStrategy(DiskCacheStrategy.ALL) + .load("https://www.w3schools.com/howto/img_fjords.jpg") + .submit()); + fail(); + } catch (RuntimeException e) { + // Expected. + } + } + + private InputStream getData() { + InputStream is = null; + try { + is = context.getResources().openRawResource(ResourceIds.raw.canonical); + byte[] buffer = new byte[1024 * 1024]; + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + int read; + while ((read = is.read(buffer)) != -1) { + outputStream.write(buffer, 0, read); + } + byte[] data = outputStream.toByteArray(); + return new ByteArrayInputStream(data); + } catch (IOException e) { + throw new RuntimeException(e); + } finally { + if (is != null) { + try { + is.close(); + } catch (IOException e) { + // Ignored. + } + } + } + } +} diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/test/MockModelLoader.java b/instrumentation/src/androidTest/java/com/bumptech/glide/test/MockModelLoader.java new file mode 100644 index 0000000000..c1f14110d6 --- /dev/null +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/test/MockModelLoader.java @@ -0,0 +1,97 @@ +package com.bumptech.glide.test; + +import android.content.Context; +import android.support.annotation.NonNull; +import android.support.test.InstrumentationRegistry; +import com.bumptech.glide.Glide; +import com.bumptech.glide.Priority; +import com.bumptech.glide.load.DataSource; +import com.bumptech.glide.load.Options; +import com.bumptech.glide.load.data.DataFetcher; +import com.bumptech.glide.load.model.ModelLoader; +import com.bumptech.glide.load.model.ModelLoaderFactory; +import com.bumptech.glide.load.model.MultiModelLoaderFactory; +import com.bumptech.glide.signature.ObjectKey; + +public final class MockModelLoader implements ModelLoader { + private final ModelT model; + private final DataT data; + + @SuppressWarnings("unchecked") + public static void mock(final ModelT model, final DataT data) { + Context context = InstrumentationRegistry.getTargetContext(); + + Glide.get(context) + .getRegistry() + .replace( + (Class) model.getClass(), + (Class) data.getClass(), + new ModelLoaderFactory() { + @NonNull + @Override + public ModelLoader build( + @NonNull MultiModelLoaderFactory multiFactory) { + return new MockModelLoader<>(model, data); + } + + @Override + public void teardown() { + // Do nothing. + } + }); + } + + private MockModelLoader(ModelT model, DataT data) { + this.model = model; + this.data = data; + } + + @Override + public LoadData buildLoadData(@NonNull ModelT modelT, int width, int height, + @NonNull Options options) { + return new LoadData<>(new ObjectKey(modelT), new MockDataFetcher<>(data)); + } + + @Override + public boolean handles(@NonNull ModelT model) { + return this.model.equals(model); + } + + private static final class MockDataFetcher implements DataFetcher { + + private final DataT data; + + MockDataFetcher(DataT data) { + this.data = data; + } + + @Override + public void loadData( + @NonNull Priority priority, @NonNull DataCallback callback) { + callback.onDataReady(data); + } + + @Override + public void cleanup() { + // Do nothing. + } + + @Override + public void cancel() { + // Do nothing. + } + + @NonNull + @Override + @SuppressWarnings("unchecked") + public Class getDataClass() { + return (Class) data.getClass(); + } + + @NonNull + @Override + public DataSource getDataSource() { + return DataSource.REMOTE; + } + } +} diff --git a/library/src/main/java/com/bumptech/glide/Registry.java b/library/src/main/java/com/bumptech/glide/Registry.java index cce4a86928..62f135d7d1 100644 --- a/library/src/main/java/com/bumptech/glide/Registry.java +++ b/library/src/main/java/com/bumptech/glide/Registry.java @@ -619,7 +619,10 @@ public NoModelLoaderAvailableException(@NonNull Class modelClass, @SuppressWarnings("serial") public static class NoResultEncoderAvailableException extends MissingComponentException { public NoResultEncoderAvailableException(@NonNull Class resourceClass) { - super("Failed to find result encoder for resource class: " + resourceClass); + super("Failed to find result encoder for resource class: " + resourceClass + + ", you may need to consider registering a new Encoder for the requested type or" + + " DiskCacheStrategy.DATA/DiskCacheStrategy.NONE if caching your transformed resource is" + + " unnecessary."); } } diff --git a/library/src/main/java/com/bumptech/glide/load/engine/DecodeHelper.java b/library/src/main/java/com/bumptech/glide/load/engine/DecodeHelper.java index 02e80e41fe..6a4283f1c3 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/DecodeHelper.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/DecodeHelper.java @@ -124,6 +124,14 @@ ArrayPool getArrayPool() { return glideContext.getArrayPool(); } + Class getTranscodeClass() { + return transcodeClass; + } + + Class getModelClass() { + return model.getClass(); + } + List> getRegisteredResourceClasses() { return glideContext.getRegistry() .getRegisteredResourceClasses(model.getClass(), resourceClass, transcodeClass); diff --git a/library/src/main/java/com/bumptech/glide/load/engine/ResourceCacheGenerator.java b/library/src/main/java/com/bumptech/glide/load/engine/ResourceCacheGenerator.java index c30722bf82..4e427b920a 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/ResourceCacheGenerator.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/ResourceCacheGenerator.java @@ -44,6 +44,14 @@ public boolean startNext() { return false; } List> resourceClasses = helper.getRegisteredResourceClasses(); + if (resourceClasses.isEmpty()) { + if (File.class.equals(helper.getTranscodeClass())) { + return false; + } + throw new IllegalStateException( + "Failed to find any load path from " + helper.getModelClass() + " to " + + helper.getTranscodeClass()); + } while (modelLoaders == null || !hasNextModelLoader()) { resourceClassIndex++; if (resourceClassIndex >= resourceClasses.size()) {