Skip to content

Commit

Permalink
Avoid throwing when decoding Files with DiskCacheStrategy.AUTOMATIC.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sjudd committed Jan 30, 2018
1 parent c99a530 commit 0917ef3
Show file tree
Hide file tree
Showing 5 changed files with 241 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -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.
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -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<ModelT, DataT> implements ModelLoader<ModelT, DataT> {
private final ModelT model;
private final DataT data;

@SuppressWarnings("unchecked")
public static <ModelT, DataT> void mock(final ModelT model, final DataT data) {
Context context = InstrumentationRegistry.getTargetContext();

Glide.get(context)
.getRegistry()
.replace(
(Class<ModelT>) model.getClass(),
(Class<DataT>) data.getClass(),
new ModelLoaderFactory<ModelT, DataT>() {
@NonNull
@Override
public ModelLoader<ModelT, DataT> 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<DataT> 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<DataT> implements DataFetcher<DataT> {

private final DataT data;

MockDataFetcher(DataT data) {
this.data = data;
}

@Override
public void loadData(
@NonNull Priority priority, @NonNull DataCallback<? super DataT> callback) {
callback.onDataReady(data);
}

@Override
public void cleanup() {
// Do nothing.
}

@Override
public void cancel() {
// Do nothing.
}

@NonNull
@Override
@SuppressWarnings("unchecked")
public Class<DataT> getDataClass() {
return (Class<DataT>) data.getClass();
}

@NonNull
@Override
public DataSource getDataSource() {
return DataSource.REMOTE;
}
}
}
5 changes: 4 additions & 1 deletion library/src/main/java/com/bumptech/glide/Registry.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,14 @@ ArrayPool getArrayPool() {
return glideContext.getArrayPool();
}

Class<?> getTranscodeClass() {
return transcodeClass;
}

Class<?> getModelClass() {
return model.getClass();
}

List<Class<?>> getRegisteredResourceClasses() {
return glideContext.getRegistry()
.getRegisteredResourceClasses(model.getClass(), resourceClass, transcodeClass);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ public boolean startNext() {
return false;
}
List<Class<?>> 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()) {
Expand Down

0 comments on commit 0917ef3

Please sign in to comment.