Skip to content

Commit

Permalink
Distinguish between missing and non-handling ModelLoaders in exceptions.
Browse files Browse the repository at this point in the history
Fixes b/141615023.
  • Loading branch information
sjudd committed Oct 1, 2019
1 parent 291af10 commit 83ba102
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 6 deletions.
18 changes: 12 additions & 6 deletions library/src/main/java/com/bumptech/glide/Registry.java
Original file line number Diff line number Diff line change
Expand Up @@ -582,11 +582,7 @@ public <X> DataRewinder<X> getRewinder(@NonNull X data) {

@NonNull
public <Model> List<ModelLoader<Model, ?>> getModelLoaders(@NonNull Model model) {
List<ModelLoader<Model, ?>> result = modelLoaderRegistry.getModelLoaders(model);
if (result.isEmpty()) {
throw new NoModelLoaderAvailableException(model);
}
return result;
return modelLoaderRegistry.getModelLoaders(model);
}

@NonNull
Expand All @@ -605,8 +601,18 @@ public List<ImageHeaderParser> getImageHeaderParsers() {
// Never serialized by Glide.
@SuppressWarnings("serial")
public static class NoModelLoaderAvailableException extends MissingComponentException {

public NoModelLoaderAvailableException(@NonNull Object model) {
super("Failed to find any ModelLoaders for model: " + model);
super("Failed to find any ModelLoaders registered for model class: " + model.getClass());
}

public <M> NoModelLoaderAvailableException(
@NonNull M model, @NonNull List<ModelLoader<M, ?>> matchingButNotHandlingModelLoaders) {
super(
"Found ModelLoaders for model class: "
+ matchingButNotHandlingModelLoaders
+ ", but none that handle this specific model instance: "
+ model);
}

public NoModelLoaderAvailableException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.core.util.Pools.Pool;
import com.bumptech.glide.Registry.NoModelLoaderAvailableException;
import com.bumptech.glide.util.Synthetic;
import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -72,6 +73,9 @@ private <Model, Data> void tearDown(
@NonNull
public <A> List<ModelLoader<A, ?>> getModelLoaders(@NonNull A model) {
List<ModelLoader<A, ?>> modelLoaders = getModelLoadersForClass(getClass(model));
if (modelLoaders.isEmpty()) {
throw new NoModelLoaderAvailableException(model);
}
int size = modelLoaders.size();
boolean isEmpty = true;
List<ModelLoader<A, ?>> filteredLoaders = Collections.emptyList();
Expand All @@ -86,6 +90,9 @@ private <Model, Data> void tearDown(
filteredLoaders.add(loader);
}
}
if (filteredLoaders.isEmpty()) {
throw new NoModelLoaderAvailableException(model, modelLoaders);
}
return filteredLoaders;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package com.bumptech.glide.load.model;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import androidx.annotation.NonNull;
import com.bumptech.glide.Registry.NoModelLoaderAvailableException;
import com.bumptech.glide.util.pool.FactoryPools;
import org.junit.Before;
import org.junit.Test;
import org.junit.function.ThrowingRunnable;
import org.junit.runner.RunWith;
import org.robolectric.RobolectricTestRunner;

@RunWith(RobolectricTestRunner.class)
public class ModelLoaderRegistryTest {
private static final String MOCK_MODEL_LOADER_NAME = "MockModelLoader";

private ModelLoaderRegistry registry;

@Before
public void setUp() {
registry = new ModelLoaderRegistry(FactoryPools.<Throwable>threadSafeList());
}

@Test
public void getModelLoaders_withNoRegisteredModelLoader_throws() {
final Object model = new Object();
NoModelLoaderAvailableException thrown =
assertThrows(
NoModelLoaderAvailableException.class,
new ThrowingRunnable() {
@Override
public void run() {
registry.getModelLoaders(model);
}
});

assertThat(thrown)
.hasMessageThat()
.contains(
"Failed to find any ModelLoaders registered for model class: " + model.getClass());
}

@Test
public void getModelLoaders_withRegisteredModelLoader_thatDoesNotHandleModelInstance_throws() {
final Object model = new Object();
final ModelLoader<Object, Object> modelLoader = mockModelLoader();
when(modelLoader.handles(model)).thenReturn(false);
appendModelLoader(modelLoader);

NoModelLoaderAvailableException thrown =
assertThrows(
NoModelLoaderAvailableException.class,
new ThrowingRunnable() {
@Override
public void run() {
registry.getModelLoaders(model);
}
});

assertThat(thrown)
.hasMessageThat()
.contains(
"Found ModelLoaders for model class: [MockModelLoader], but none that handle this specific model instance: java.lang.Object");
}

@Test
public void getModelLoaders_withRegisteredModelLoader_handlesModel_returnsModelLoader() {
final Object model = new Object();
final ModelLoader<Object, Object> modelLoader = mockModelLoader();
when(modelLoader.handles(model)).thenReturn(true);
appendModelLoader(modelLoader);

assertThat(registry.getModelLoaders(model)).containsExactly(modelLoader);
}

@Test
public void
getModelLoaders_withRegisteredModelLoaders_onlyOneHandlesModel_returnsHandlingModelLoader() {
final Object model = new Object();

ModelLoader<Object, Object> handlingModelLoader = mockModelLoader();
when(handlingModelLoader.handles(model)).thenReturn(true);
appendModelLoader(handlingModelLoader);

ModelLoader<Object, Object> notHandlingModelLoader = mockModelLoader();
when(notHandlingModelLoader.handles(model)).thenReturn(false);
appendModelLoader(notHandlingModelLoader);

assertThat(registry.getModelLoaders(model)).containsExactly(handlingModelLoader);
}

private void appendModelLoader(final ModelLoader<Object, Object> modelLoader) {
registry.append(
Object.class,
Object.class,
new ModelLoaderFactory<Object, Object>() {
@NonNull
@Override
public ModelLoader<Object, Object> build(@NonNull MultiModelLoaderFactory multiFactory) {
return modelLoader;
}

@Override
public void teardown() {}
});
}

@SuppressWarnings("unchecked")
private static ModelLoader<Object, Object> mockModelLoader() {
return mock(ModelLoader.class, MOCK_MODEL_LOADER_NAME);
}
}

0 comments on commit 83ba102

Please sign in to comment.