Make RequestBuilder consistently support lock and autoClone

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
bumptech · Dec 16, 2020 · 11530fd · 11530fd
1 parent 332807a
commit 11530fd
Show file tree

Hide file tree

Showing 11 changed files with 158 additions and 9 deletions.
diff --git a/...ompiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestBuilderGenerator.java b/...ompiler/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/...r/test/src/test/resources/GlideExtensionOptionsTest/MemoizeStaticMethod/GlideRequest.java b/...r/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/...mpiler/test/src/test/resources/GlideExtensionOptionsTest/OverrideExtend/GlideRequest.java b/...mpiler/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/...est/resources/GlideExtensionOptionsTest/OverrideExtendMultipleArguments/GlideRequest.java b/...est/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/...piler/test/src/test/resources/GlideExtensionOptionsTest/OverrideReplace/GlideRequest.java b/...piler/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/...iler/test/src/test/resources/GlideExtensionOptionsTest/SkipStaticMethod/GlideRequest.java b/...iler/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/...iler/test/src/test/resources/GlideExtensionOptionsTest/StaticMethodName/GlideRequest.java b/...iler/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;
   }