Include Transcode class in cache key for cached resource classes.

We've avoided returning resource classes that can't possible be
transcoded to our expected transcode class to avoid uselessly decoding
and then throwing out an unusable resource. Unfortunately when that
optimization was implemented we didn't change the cache key used to keep
track of the set of available resource classes, so the set of resources
for two different transcode class requests could have the same cache key
but different contents. As a result we introduced a race where a smaller
than accurate (or even empty) set of resource classes could be returned
if the first call to getRegisteredResourceClasses used the same Model
and Resource class, but a different and less available trascode class.

This change includes the transcode class in the cache key so that we
stop incorrectly sharing cache keys across different resource class
sets.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=210131660

library/src/main/java/com/bumptech/glide/Registry.java

@@ -523,9 +523,11 @@ private <Data, TResource, Transcode> List<DecodePath<Data, TResource, Transcode>
 
   @NonNull
   public <Model, TResource, Transcode> List<Class<?>> getRegisteredResourceClasses(
-      @NonNull Class<Model> modelClass, @NonNull Class<TResource> resourceClass,
+      @NonNull Class<Model> modelClass,
+      @NonNull Class<TResource> resourceClass,
       @NonNull Class<Transcode> transcodeClass) {
-    List<Class<?>> result = modelToResourceClassCache.get(modelClass, resourceClass);
+    List<Class<?>> result =
+        modelToResourceClassCache.get(modelClass, resourceClass, transcodeClass);
 
     if (result == null) {
       result = new ArrayList<>();
@@ -541,8 +543,8 @@ public <Model, TResource, Transcode> List<Class<?>> getRegisteredResourceClasses
           }
         }
       }
-      modelToResourceClassCache.put(modelClass, resourceClass,
-          Collections.unmodifiableList(result));
+      modelToResourceClassCache.put(
+          modelClass, resourceClass, transcodeClass, Collections.unmodifiableList(result));
     }
 
     return result;

library/src/main/java/com/bumptech/glide/provider/ModelToResourceClassCache.java

@@ -17,12 +17,15 @@ public class ModelToResourceClassCache {
       new ArrayMap<>();
 
   @Nullable
-  public List<Class<?>> get(@NonNull Class<?> modelClass, @NonNull Class<?> resourceClass) {
+  public List<Class<?>> get(
+      @NonNull Class<?> modelClass,
+      @NonNull Class<?> resourceClass,
+      @NonNull Class<?> transcodeClass) {
     MultiClassKey key = resourceClassKeyRef.getAndSet(null);
     if (key == null) {
-      key = new MultiClassKey(modelClass, resourceClass);
+      key = new MultiClassKey(modelClass, resourceClass, transcodeClass);
     } else {
-      key.set(modelClass, resourceClass);
+      key.set(modelClass, resourceClass, transcodeClass);
     }
     final List<Class<?>> result;
     synchronized (registeredResourceClassCache) {
@@ -32,11 +35,14 @@ public List<Class<?>> get(@NonNull Class<?> modelClass, @NonNull Class<?> resour
     return result;
   }
 
-  public void put(@NonNull Class<?> modelClass, @NonNull Class<?> resourceClass,
+  public void put(
+      @NonNull Class<?> modelClass,
+      @NonNull Class<?> resourceClass,
+      @NonNull Class<?> transcodeClass,
       @NonNull List<Class<?>> resourceClasses) {
     synchronized (registeredResourceClassCache) {
       registeredResourceClassCache
-          .put(new MultiClassKey(modelClass, resourceClass), resourceClasses);
+          .put(new MultiClassKey(modelClass, resourceClass, transcodeClass), resourceClasses);
     }
   }
 

library/test/src/test/java/com/bumptech/glide/RegistryTest.java

@@ -0,0 +1,162 @@
+package com.bumptech.glide;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.bumptech.glide.load.ResourceDecoder;
+import com.bumptech.glide.load.model.ModelLoaderFactory;
+import com.bumptech.glide.load.resource.transcode.ResourceTranscoder;
+import java.util.List;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.annotation.Config;
+
+@RunWith(RobolectricTestRunner.class)
+@Config(manifest = Config.NONE)
+public class RegistryTest {
+
+  @Mock private ModelLoaderFactory<Model, Data> modelLoaderFactory;
+  @Mock private ResourceDecoder<Data, ResourceOne> resourceOneDecoder;
+  @Mock private ResourceDecoder<Data, ResourceTwo> resourceTwoDecoder;
+  @Mock private ResourceTranscoder<ResourceOne, TranscodeOne> resourceOneTranscodeOneTranscoder;
+  private Registry registry;
+
+  @Before
+  public void setUp() {
+    MockitoAnnotations.initMocks(this);
+    registry = new Registry();
+  }
+
+  @Test
+  public void getRegisteredResourceClasses_withNoResources_isEmpty() {
+    assertThat(getRegisteredResourceClasses()).isEmpty();
+  }
+
+  @Test
+  public void getRegisteredResourceClasses_withOneDataClass_noResourceClasses_isEmpty() {
+    registry.append(Model.class, Data.class, modelLoaderFactory);
+    assertThat(getRegisteredResourceClasses()).isEmpty();
+  }
+
+  @Test
+  public void getRegisteredResourceClasses_withOneDataAndResourceClass_noTranscodeClass_isEmpty() {
+    registry.append(Model.class, Data.class, modelLoaderFactory);
+    registry.append(Data.class, ResourceOne.class, resourceOneDecoder);
+    assertThat(getRegisteredResourceClasses()).isEmpty();
+  }
+
+  @Test
+  public void getRegisteredResourceClasses_withOneDataAndResourceAndTranscodeClass_isNotEmpty() {
+    registry.append(Model.class, Data.class, modelLoaderFactory);
+    registry.append(Data.class, ResourceOne.class, resourceOneDecoder);
+    registry.register(ResourceOne.class, TranscodeOne.class, resourceOneTranscodeOneTranscoder);
+    assertThat(getRegisteredResourceClasses()).containsExactly(ResourceOne.class);
+  }
+
+  @Test
+  public void getRegisteredResourceClasses_withMissingTranscodeForOneOfTwoResources_isNotEmpty() {
+    // The loop allows us to make sure that the order in which we call getRegisteredResourceClasses
+    // doesn't affect the output.
+    for (int i = 0; i < 2; i++) {
+      Registry registry = new Registry();
+      registry.append(Model.class, Data.class, modelLoaderFactory);
+
+      registry.append(Data.class, ResourceOne.class, resourceOneDecoder);
+      registry.append(Data.class, ResourceTwo.class, resourceTwoDecoder);
+
+      registry.register(ResourceOne.class, TranscodeOne.class, resourceOneTranscodeOneTranscoder);
+
+      List<Class<?>> resourceOneClasses;
+      List<Class<?>> resourceTwoClasses;
+      if (i == 0) {
+        resourceOneClasses =
+            registry.getRegisteredResourceClasses(
+                Model.class, ResourceOne.class, TranscodeOne.class);
+        resourceTwoClasses =
+            registry.getRegisteredResourceClasses(
+                Model.class, ResourceTwo.class, TranscodeOne.class);
+      } else {
+        resourceTwoClasses =
+            registry.getRegisteredResourceClasses(
+                Model.class, ResourceTwo.class, TranscodeOne.class);
+        resourceOneClasses =
+            registry.getRegisteredResourceClasses(
+                Model.class, ResourceOne.class, TranscodeOne.class);
+      }
+      // ResourceOne has a corresponding transcode class, so we should return it.
+      assertThat(resourceOneClasses).containsExactly(ResourceOne.class);
+      // ResourceTwo has no matching transcode class, so we shouldn't return it.
+      assertThat(resourceTwoClasses).isEmpty();
+    }
+  }
+
+  @Test
+  public void getRegisteredResourceClasses_withOneOfTwoMissingTranscoders_isNotEmpty() {
+    // The loop allows us to make sure that the order in which we call getRegisteredResourceClasses
+    // doesn't affect the output.
+    for (int i = 0; i < 2; i++) {
+      Registry registry = new Registry();
+      registry.append(Model.class, Data.class, modelLoaderFactory);
+
+      registry.append(Data.class, ResourceOne.class, resourceOneDecoder);
+
+      registry.register(ResourceOne.class, TranscodeOne.class, resourceOneTranscodeOneTranscoder);
+
+      List<Class<?>> transcodeOneClasses;
+      List<Class<?>> transcodeTwoClasses;
+      if (i == 0) {
+        transcodeOneClasses =
+            registry.getRegisteredResourceClasses(
+                Model.class, ResourceOne.class, TranscodeOne.class);
+        transcodeTwoClasses =
+            registry.getRegisteredResourceClasses(
+                Model.class, ResourceOne.class, TranscodeTwo.class);
+      } else {
+        transcodeTwoClasses =
+            registry.getRegisteredResourceClasses(
+                Model.class, ResourceOne.class, TranscodeTwo.class);
+        transcodeOneClasses =
+            registry.getRegisteredResourceClasses(
+                Model.class, ResourceOne.class, TranscodeOne.class);
+      }
+      // TranscodeOne has a corresponding ResourceTranscoder, so we expect to see the resource
+      // class.
+      assertThat(transcodeOneClasses).containsExactly(ResourceOne.class);
+      // TranscodeTwo has no corresponding ResourceTranscoder class, so we shouldn't return the
+      // resource class.
+      assertThat(transcodeTwoClasses).isEmpty();
+    }
+  }
+
+  private List<Class<?>> getRegisteredResourceClasses() {
+    return registry.getRegisteredResourceClasses(
+        Model.class, ResourceOne.class, TranscodeOne.class);
+  }
+
+  private static final class Model {
+    // Empty class to represent model classes for readability.
+  }
+
+  private static final class Data {
+    // Empty class to represent data classes for readability.
+  }
+
+  private static final class ResourceOne {
+    // Empty class to represent resource classes for readability.
+  }
+
+  private static final class ResourceTwo {
+    // Empty class to represent another resource class for readability.
+  }
+
+  private static final class TranscodeOne {
+    // Empty class to represent transcode classes for readability.
+  }
+
+  private static final class TranscodeTwo {
+    // Empty class to represent transcode classes for readability.
+  }
+}