From e442557fc5ff640e9988c850bfe4d7be73c09b18 Mon Sep 17 00:00:00 2001
From: Sam Judd <judds@google.com>
Date: Tue, 1 Dec 2020 13:19:02 -0800
Subject: [PATCH] Avoid assertions on model type in .error()

It's too complicated to figure out if models are likely compatible or incompatible. The current logic is broken if we have three classes, Parent, Child1 and Child2 where both the Child classes extend parent. We want Child1 and Child2 to be compatible, but they won't be because Child2 is not assignable to Child1. Anything we do that's more sophisticated is likely to be heuristic anyway.

Ideally we'd just unapply the automatically applied options from the original model and apply the correct automatic options (if any) for the new model. Keeping track of which options can be undone and which options should be re-applied is quite difficult, in part because they automatic options could also be deliberately applied.

Rather than do anything intelligent, we'll rely on callers to use the API appropriately and update the javadoc to warn more. In general I expect the common case to be matching models and there's a reasonable fallback if people have trouble. Unfortunately I expect this won't be perfect, there will be some amount of debugging caused by someone expecting or not expecting a default option :/. I think the tradeoff of not having to create a new request builder and duplicate options for that request builder is probably worth the cost.

PiperOrigin-RevId: 345085610
---
 .../com/bumptech/glide/RequestBuilder.java    | 25 ++++++++-----------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/library/src/main/java/com/bumptech/glide/RequestBuilder.java b/library/src/main/java/com/bumptech/glide/RequestBuilder.java
index 880db65049..33a9ebbcfd 100644
--- a/library/src/main/java/com/bumptech/glide/RequestBuilder.java
+++ b/library/src/main/java/com/bumptech/glide/RequestBuilder.java
@@ -228,14 +228,19 @@ public RequestBuilder<TranscodeType> error(@Nullable RequestBuilder<TranscodeTyp
   /**
    * Identical to calling {@link #error(RequestBuilder)} where the {@code RequestBuilder} is the
    * result of calling {@link #clone()} and removing any existing thumbnail and error {@code
-   * RequestBuilders}
-   *
-   * <p>You can only call this method on a {@code RequestBuilder} that has previously had {@code
-   * load()} called on it with a non-null model. .
+   * RequestBuilders}.
    *
    * <p>Other than thumbnail and error {@code RequestBuilder}s, which are removed, all other options
    * are retained from the primary request. However, <b>order matters!</b> Any options applied after
-   * this method is called will not be applied to the error {@code RequestBuilder}. We should move
+   * this method is called will not be applied to the error {@code RequestBuilder}.
+   *
+   * <p>WARNING: Calling this method with a {@code model} whose type does not match the type of the
+   * model passed to {@code load()} may be dangerous! Any options that were applied by the various
+   * type specific {@code load()} methods, like {@link #load(byte[])} will be copied to the error
+   * request here even if the {@code model} you pass to this method doesn't match. Similary, options
+   * that would be normally applied by type specific {@code load()} methods will <em>not</em> be
+   * applied to this request. If this behavior is confusing or unexpected, use {@link
+   * #error(RequestBuilder)} instead.
    */
   @NonNull
   @CheckResult
@@ -243,16 +248,6 @@ public RequestBuilder<TranscodeType> error(Object model) {
     if (model == null) {
       return error((RequestBuilder<TranscodeType>) null);
     }
-    if (this.model == null) {
-      throw new IllegalArgumentException(
-          "Call this method after calling #load() with a non-null" + " model.");
-    }
-    if (!this.model.getClass().isAssignableFrom(model.getClass())) {
-      throw new IllegalArgumentException(
-          "You can only call #error(Object) with the same type of"
-              + " model that you provided to #load(). If you need to load a different type, use the"
-              + " somewhat more verbose #error(RequestBuilder) method instead of this shortcut");
-    }
     return error(cloneWithNullErrorAndThumbnail().load(model));
   }