diff --git a/library/src/main/java/com/bumptech/glide/Registry.java b/library/src/main/java/com/bumptech/glide/Registry.java index 40b6cde678..8889321043 100644 --- a/library/src/main/java/com/bumptech/glide/Registry.java +++ b/library/src/main/java/com/bumptech/glide/Registry.java @@ -582,11 +582,7 @@ public DataRewinder getRewinder(@NonNull X data) { @NonNull public List> getModelLoaders(@NonNull Model model) { - List> result = modelLoaderRegistry.getModelLoaders(model); - if (result.isEmpty()) { - throw new NoModelLoaderAvailableException(model); - } - return result; + return modelLoaderRegistry.getModelLoaders(model); } @NonNull @@ -605,8 +601,18 @@ public List 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 NoModelLoaderAvailableException( + @NonNull M model, @NonNull List> matchingButNotHandlingModelLoaders) { + super( + "Found ModelLoaders for model class: " + + matchingButNotHandlingModelLoaders + + ", but none that handle this specific model instance: " + + model); } public NoModelLoaderAvailableException( diff --git a/library/src/main/java/com/bumptech/glide/load/model/ModelLoaderRegistry.java b/library/src/main/java/com/bumptech/glide/load/model/ModelLoaderRegistry.java index be288cab70..e3b613866d 100644 --- a/library/src/main/java/com/bumptech/glide/load/model/ModelLoaderRegistry.java +++ b/library/src/main/java/com/bumptech/glide/load/model/ModelLoaderRegistry.java @@ -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; @@ -72,6 +73,9 @@ private void tearDown( @NonNull public List> getModelLoaders(@NonNull A model) { List> modelLoaders = getModelLoadersForClass(getClass(model)); + if (modelLoaders.isEmpty()) { + throw new NoModelLoaderAvailableException(model); + } int size = modelLoaders.size(); boolean isEmpty = true; List> filteredLoaders = Collections.emptyList(); @@ -86,6 +90,9 @@ private void tearDown( filteredLoaders.add(loader); } } + if (filteredLoaders.isEmpty()) { + throw new NoModelLoaderAvailableException(model, modelLoaders); + } return filteredLoaders; } diff --git a/library/test/src/test/java/com/bumptech/glide/load/model/ModelLoaderRegistryTest.java b/library/test/src/test/java/com/bumptech/glide/load/model/ModelLoaderRegistryTest.java new file mode 100644 index 0000000000..7d47f0227e --- /dev/null +++ b/library/test/src/test/java/com/bumptech/glide/load/model/ModelLoaderRegistryTest.java @@ -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.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 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 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 handlingModelLoader = mockModelLoader(); + when(handlingModelLoader.handles(model)).thenReturn(true); + appendModelLoader(handlingModelLoader); + + ModelLoader notHandlingModelLoader = mockModelLoader(); + when(notHandlingModelLoader.handles(model)).thenReturn(false); + appendModelLoader(notHandlingModelLoader); + + assertThat(registry.getModelLoaders(model)).containsExactly(handlingModelLoader); + } + + private void appendModelLoader(final ModelLoader modelLoader) { + registry.append( + Object.class, + Object.class, + new ModelLoaderFactory() { + @NonNull + @Override + public ModelLoader build(@NonNull MultiModelLoaderFactory multiFactory) { + return modelLoader; + } + + @Override + public void teardown() {} + }); + } + + @SuppressWarnings("unchecked") + private static ModelLoader mockModelLoader() { + return mock(ModelLoader.class, MOCK_MODEL_LOADER_NAME); + } +}