Make RequestBuilder consistently support lock and autoClone

sjudd · glide-copybara-robot · commit 11530fd76903 · 2020-12-16T12:54:56.000-08:00
Previously these methods worked, but only for the methods in RequestBuilder that came from BaseRequestOptions. This changes makes sure the remaining methods in RequestBuilder (and the generated equivalent) obey those two methods in a consistent manner.

PiperOrigin-RevId: 347879132
diff --git a/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestBuilderGenerator.java b/annotation/compiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestBuilderGenerator.java
@@ -104,7 +104,7 @@ final class RequestBuilderGenerator {
   private static final String TRANSCODE_TYPE_NAME = "TranscodeType";
   /** A set of method names to avoid overriding from RequestOptions. */
   private static final ImmutableSet<String> EXCLUDED_METHODS_FROM_BASE_REQUEST_OPTIONS =
-      ImmutableSet.of("clone", "apply", "autoLock", "lock", "autoClone");
+      ImmutableSet.of("clone", "apply");
 
   private final ProcessingEnvironment processingEnv;
   private final ProcessorUtil processorUtil;
diff --git a/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideRequest.java b/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideRequest.java
@@ -450,6 +450,22 @@ public GlideRequest<TranscodeType> dontAnimate() {
     return (GlideRequest<TranscodeType>) super.dontAnimate();
   }
 
+  /**
+   * @see GlideOptions#lock()
+   */
+  @NonNull
+  public GlideRequest<TranscodeType> lock() {
+    return (GlideRequest<TranscodeType>) super.lock();
+  }
+
+  /**
+   * @see GlideOptions#autoClone()
+   */
+  @NonNull
+  public GlideRequest<TranscodeType> autoClone() {
+    return (GlideRequest<TranscodeType>) super.autoClone();
+  }
+
   @Override
   @NonNull
   @CheckResult
diff --git a/annotation/compiler/test/src/test/resources/GlideExtensionOptionsTest/MemoizeStaticMethod/GlideRequest.java b/annotation/compiler/test/src/test/resources/GlideExtensionOptionsTest/MemoizeStaticMethod/GlideRequest.java
@@ -450,6 +450,22 @@ public GlideRequest<TranscodeType> dontAnimate() {
     return (GlideRequest<TranscodeType>) super.dontAnimate();
   }
 
+  /**
+   * @see GlideOptions#lock()
+   */
+  @NonNull
+  public GlideRequest<TranscodeType> lock() {
+    return (GlideRequest<TranscodeType>) super.lock();
+  }
+
+  /**
+   * @see GlideOptions#autoClone()
+   */
+  @NonNull
+  public GlideRequest<TranscodeType> autoClone() {
+    return (GlideRequest<TranscodeType>) super.autoClone();
+  }
+
   @Override
   @NonNull
   @CheckResult
diff --git a/annotation/compiler/test/src/test/resources/GlideExtensionOptionsTest/OverrideExtend/GlideRequest.java b/annotation/compiler/test/src/test/resources/GlideExtensionOptionsTest/OverrideExtend/GlideRequest.java
@@ -441,6 +441,22 @@ public GlideRequest<TranscodeType> dontAnimate() {
     return (GlideRequest<TranscodeType>) super.dontAnimate();
   }
 
+  /**
+   * @see GlideOptions#lock()
+   */
+  @NonNull
+  public GlideRequest<TranscodeType> lock() {
+    return (GlideRequest<TranscodeType>) super.lock();
+  }
+
+  /**
+   * @see GlideOptions#autoClone()
+   */
+  @NonNull
+  public GlideRequest<TranscodeType> autoClone() {
+    return (GlideRequest<TranscodeType>) super.autoClone();
+  }
+
   @Override
   @NonNull
   @CheckResult
diff --git a/annotation/compiler/test/src/test/resources/GlideExtensionOptionsTest/OverrideExtendMultipleArguments/GlideRequest.java b/annotation/compiler/test/src/test/resources/GlideExtensionOptionsTest/OverrideExtendMultipleArguments/GlideRequest.java
@@ -441,6 +441,22 @@ public GlideRequest<TranscodeType> dontAnimate() {
     return (GlideRequest<TranscodeType>) super.dontAnimate();
   }
 
+  /**
+   * @see GlideOptions#lock()
+   */
+  @NonNull
+  public GlideRequest<TranscodeType> lock() {
+    return (GlideRequest<TranscodeType>) super.lock();
+  }
+
+  /**
+   * @see GlideOptions#autoClone()
+   */
+  @NonNull
+  public GlideRequest<TranscodeType> autoClone() {
+    return (GlideRequest<TranscodeType>) super.autoClone();
+  }
+
   @Override
   @NonNull
   @CheckResult
diff --git a/annotation/compiler/test/src/test/resources/GlideExtensionOptionsTest/OverrideReplace/GlideRequest.java b/annotation/compiler/test/src/test/resources/GlideExtensionOptionsTest/OverrideReplace/GlideRequest.java
@@ -441,6 +441,22 @@ public GlideRequest<TranscodeType> dontAnimate() {
     return (GlideRequest<TranscodeType>) super.dontAnimate();
   }
 
+  /**
+   * @see GlideOptions#lock()
+   */
+  @NonNull
+  public GlideRequest<TranscodeType> lock() {
+    return (GlideRequest<TranscodeType>) super.lock();
+  }
+
+  /**
+   * @see GlideOptions#autoClone()
+   */
+  @NonNull
+  public GlideRequest<TranscodeType> autoClone() {
+    return (GlideRequest<TranscodeType>) super.autoClone();
+  }
+
   @Override
   @NonNull
   @CheckResult
diff --git a/annotation/compiler/test/src/test/resources/GlideExtensionOptionsTest/SkipStaticMethod/GlideRequest.java b/annotation/compiler/test/src/test/resources/GlideExtensionOptionsTest/SkipStaticMethod/GlideRequest.java
@@ -450,6 +450,22 @@ public GlideRequest<TranscodeType> dontAnimate() {
     return (GlideRequest<TranscodeType>) super.dontAnimate();
   }
 
+  /**
+   * @see GlideOptions#lock()
+   */
+  @NonNull
+  public GlideRequest<TranscodeType> lock() {
+    return (GlideRequest<TranscodeType>) super.lock();
+  }
+
+  /**
+   * @see GlideOptions#autoClone()
+   */
+  @NonNull
+  public GlideRequest<TranscodeType> autoClone() {
+    return (GlideRequest<TranscodeType>) super.autoClone();
+  }
+
   @Override
   @NonNull
   @CheckResult
diff --git a/annotation/compiler/test/src/test/resources/GlideExtensionOptionsTest/StaticMethodName/GlideRequest.java b/annotation/compiler/test/src/test/resources/GlideExtensionOptionsTest/StaticMethodName/GlideRequest.java
@@ -450,6 +450,22 @@ public GlideRequest<TranscodeType> dontAnimate() {
     return (GlideRequest<TranscodeType>) super.dontAnimate();
   }
 
+  /**
+   * @see GlideOptions#lock()
+   */
+  @NonNull
+  public GlideRequest<TranscodeType> lock() {
+    return (GlideRequest<TranscodeType>) super.lock();
+  }
+
+  /**
+   * @see GlideOptions#autoClone()
+   */
+  @NonNull
+  public GlideRequest<TranscodeType> autoClone() {
+    return (GlideRequest<TranscodeType>) super.autoClone();
+  }
+
   @Override
   @NonNull
   @CheckResult
diff --git a/annotation/compiler/test/src/test/resources/GlideExtensionWithOptionTest/GlideRequest.java b/annotation/compiler/test/src/test/resources/GlideExtensionWithOptionTest/GlideRequest.java
@@ -450,6 +450,22 @@ public GlideRequest<TranscodeType> dontAnimate() {
     return (GlideRequest<TranscodeType>) super.dontAnimate();
   }
 
+  /**
+   * @see GlideOptions#lock()
+   */
+  @NonNull
+  public GlideRequest<TranscodeType> lock() {
+    return (GlideRequest<TranscodeType>) super.lock();
+  }
+
+  /**
+   * @see GlideOptions#autoClone()
+   */
+  @NonNull
+  public GlideRequest<TranscodeType> autoClone() {
+    return (GlideRequest<TranscodeType>) super.autoClone();
+  }
+
   @Override
   @NonNull
   @CheckResult
diff --git a/library/src/main/java/com/bumptech/glide/RequestBuilder.java b/library/src/main/java/com/bumptech/glide/RequestBuilder.java
@@ -152,9 +152,12 @@ public RequestBuilder<TranscodeType> apply(@NonNull BaseRequestOptions<?> reques
   @CheckResult
   public RequestBuilder<TranscodeType> transition(
       @NonNull TransitionOptions<?, ? super TranscodeType> transitionOptions) {
+    if (isAutoCloneEnabled()) {
+      return clone().transition(transitionOptions);
+    }
     this.transitionOptions = Preconditions.checkNotNull(transitionOptions);
     isDefaultTransitionOptionsSet = false;
-    return this;
+    return selfOrThrowIfLocked();
   }
 
   /**
@@ -173,6 +176,9 @@ public RequestBuilder<TranscodeType> transition(
   @SuppressWarnings("unchecked")
   public RequestBuilder<TranscodeType> listener(
       @Nullable RequestListener<TranscodeType> requestListener) {
+    if (isAutoCloneEnabled()) {
+      return clone().listener(requestListener);
+    }
     this.requestListeners = null;
     return addListener(requestListener);
   }
@@ -188,13 +194,16 @@ public RequestBuilder<TranscodeType> listener(
   @CheckResult
   public RequestBuilder<TranscodeType> addListener(
       @Nullable RequestListener<TranscodeType> requestListener) {
+    if (isAutoCloneEnabled()) {
+      return clone().addListener(requestListener);
+    }
     if (requestListener != null) {
       if (this.requestListeners == null) {
         this.requestListeners = new ArrayList<>();
       }
       this.requestListeners.add(requestListener);
     }
-    return this;
+    return selfOrThrowIfLocked();
   }
 
   /**
@@ -221,8 +230,11 @@ public RequestBuilder<TranscodeType> addListener(
    */
   @NonNull
   public RequestBuilder<TranscodeType> error(@Nullable RequestBuilder<TranscodeType> errorBuilder) {
+    if (isAutoCloneEnabled()) {
+      return clone().error(errorBuilder);
+    }
     this.errorBuilder = errorBuilder;
-    return this;
+    return selfOrThrowIfLocked();
   }
 
   /**
@@ -279,9 +291,12 @@ private RequestBuilder<TranscodeType> cloneWithNullErrorAndThumbnail() {
   @SuppressWarnings("unchecked")
   public RequestBuilder<TranscodeType> thumbnail(
       @Nullable RequestBuilder<TranscodeType> thumbnailRequest) {
+    if (isAutoCloneEnabled()) {
+      return clone().thumbnail(thumbnailRequest);
+    }
     this.thumbnailBuilder = thumbnailRequest;
 
-    return this;
+    return selfOrThrowIfLocked();
   }
 
   /**
@@ -413,12 +428,15 @@ public RequestBuilder<TranscodeType> thumbnail(
   @CheckResult
   @SuppressWarnings("unchecked")
   public RequestBuilder<TranscodeType> thumbnail(float sizeMultiplier) {
+    if (isAutoCloneEnabled()) {
+      return clone().thumbnail(sizeMultiplier);
+    }
     if (sizeMultiplier < 0f || sizeMultiplier > 1f) {
       throw new IllegalArgumentException("sizeMultiplier must be between 0 and 1");
     }
     this.thumbSizeMultiplier = sizeMultiplier;
 
-    return this;
+    return selfOrThrowIfLocked();
   }
 
   /**
@@ -437,9 +455,12 @@ public RequestBuilder<TranscodeType> load(@Nullable Object model) {
 
   @NonNull
   private RequestBuilder<TranscodeType> loadGeneric(@Nullable Object model) {
+    if (isAutoCloneEnabled()) {
+      return clone().loadGeneric(model);
+    }
     this.model = model;
     isModelSet = true;
-    return this;
+    return selfOrThrowIfLocked();
   }
   /**
    * Returns an object to load the given {@link Bitmap}.
diff --git a/library/src/main/java/com/bumptech/glide/request/BaseRequestOptions.java b/library/src/main/java/com/bumptech/glide/request/BaseRequestOptions.java
@@ -1283,14 +1283,14 @@ public T autoClone() {
 
   @NonNull
   @SuppressWarnings("unchecked")
-  private T selfOrThrowIfLocked() {
+  protected final T selfOrThrowIfLocked() {
     if (isLocked) {
       throw new IllegalStateException("You cannot modify locked T, consider clone()");
     }
     return self();
   }
 
-  protected boolean isAutoCloneEnabled() {
+  protected final boolean isAutoCloneEnabled() {
     return isAutoCloneEnabled;
   }
 

Original file line number	Diff line number	Diff line change
`@@ -1283,14 +1283,14 @@ public T autoClone() {`
`1283`	`1283`
`1284`	`1284`	`@NonNull`
`1285`	`1285`	`@SuppressWarnings("unchecked")`
`1286`		`- private T selfOrThrowIfLocked() {`
	`1286`	`+ protected final T selfOrThrowIfLocked() {`
`1287`	`1287`	`if (isLocked) {`
`1288`	`1288`	`throw new IllegalStateException("You cannot modify locked T, consider clone()");`
`1289`	`1289`	`}`
`1290`	`1290`	`return self();`
`1291`	`1291`	`}`
`1292`	`1292`
`1293`		`- protected boolean isAutoCloneEnabled() {`
	`1293`	`+ protected final boolean isAutoCloneEnabled() {`
`1294`	`1294`	`return isAutoCloneEnabled;`
`1295`	`1295`	`}`
`1296`	`1296`