From 11530fd76903ea4c6b63d5b0697b42af6393476b Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Wed, 16 Dec 2020 12:52:53 -0800 Subject: [PATCH] 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 --- .../compiler/RequestBuilderGenerator.java | 2 +- .../EmptyAppGlideModuleTest/GlideRequest.java | 16 +++++++++ .../MemoizeStaticMethod/GlideRequest.java | 16 +++++++++ .../OverrideExtend/GlideRequest.java | 16 +++++++++ .../GlideRequest.java | 16 +++++++++ .../OverrideReplace/GlideRequest.java | 16 +++++++++ .../SkipStaticMethod/GlideRequest.java | 16 +++++++++ .../StaticMethodName/GlideRequest.java | 16 +++++++++ .../GlideRequest.java | 16 +++++++++ .../com/bumptech/glide/RequestBuilder.java | 33 +++++++++++++++---- .../glide/request/BaseRequestOptions.java | 4 +-- 11 files changed, 158 insertions(+), 9 deletions(-) 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 index fc2ba2ac85..374dcc84b7 100644 --- 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 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 index 4dfbc05b4b..5227b83d8c 100644 --- 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 dontAnimate() { return (GlideRequest) super.dontAnimate(); } + /** + * @see GlideOptions#lock() + */ + @NonNull + public GlideRequest lock() { + return (GlideRequest) super.lock(); + } + + /** + * @see GlideOptions#autoClone() + */ + @NonNull + public GlideRequest autoClone() { + return (GlideRequest) 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 index ecf5fa19c0..9477d1a6b3 100644 --- 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 dontAnimate() { return (GlideRequest) super.dontAnimate(); } + /** + * @see GlideOptions#lock() + */ + @NonNull + public GlideRequest lock() { + return (GlideRequest) super.lock(); + } + + /** + * @see GlideOptions#autoClone() + */ + @NonNull + public GlideRequest autoClone() { + return (GlideRequest) 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 index 7c77d801ea..82e614850a 100644 --- 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 dontAnimate() { return (GlideRequest) super.dontAnimate(); } + /** + * @see GlideOptions#lock() + */ + @NonNull + public GlideRequest lock() { + return (GlideRequest) super.lock(); + } + + /** + * @see GlideOptions#autoClone() + */ + @NonNull + public GlideRequest autoClone() { + return (GlideRequest) 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 index 12f34f6506..7659a49885 100644 --- 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 dontAnimate() { return (GlideRequest) super.dontAnimate(); } + /** + * @see GlideOptions#lock() + */ + @NonNull + public GlideRequest lock() { + return (GlideRequest) super.lock(); + } + + /** + * @see GlideOptions#autoClone() + */ + @NonNull + public GlideRequest autoClone() { + return (GlideRequest) 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 index 891a44ef46..ad0f3581eb 100644 --- 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 dontAnimate() { return (GlideRequest) super.dontAnimate(); } + /** + * @see GlideOptions#lock() + */ + @NonNull + public GlideRequest lock() { + return (GlideRequest) super.lock(); + } + + /** + * @see GlideOptions#autoClone() + */ + @NonNull + public GlideRequest autoClone() { + return (GlideRequest) 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 index ecf5fa19c0..9477d1a6b3 100644 --- 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 dontAnimate() { return (GlideRequest) super.dontAnimate(); } + /** + * @see GlideOptions#lock() + */ + @NonNull + public GlideRequest lock() { + return (GlideRequest) super.lock(); + } + + /** + * @see GlideOptions#autoClone() + */ + @NonNull + public GlideRequest autoClone() { + return (GlideRequest) 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 index ecf5fa19c0..9477d1a6b3 100644 --- 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 dontAnimate() { return (GlideRequest) super.dontAnimate(); } + /** + * @see GlideOptions#lock() + */ + @NonNull + public GlideRequest lock() { + return (GlideRequest) super.lock(); + } + + /** + * @see GlideOptions#autoClone() + */ + @NonNull + public GlideRequest autoClone() { + return (GlideRequest) 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 index 2013c6f6cd..b0921ad37c 100644 --- 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 dontAnimate() { return (GlideRequest) super.dontAnimate(); } + /** + * @see GlideOptions#lock() + */ + @NonNull + public GlideRequest lock() { + return (GlideRequest) super.lock(); + } + + /** + * @see GlideOptions#autoClone() + */ + @NonNull + public GlideRequest autoClone() { + return (GlideRequest) 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 index 9a4e2d3c15..366da0bbec 100644 --- 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 apply(@NonNull BaseRequestOptions reques @CheckResult public RequestBuilder transition( @NonNull TransitionOptions transitionOptions) { + if (isAutoCloneEnabled()) { + return clone().transition(transitionOptions); + } this.transitionOptions = Preconditions.checkNotNull(transitionOptions); isDefaultTransitionOptionsSet = false; - return this; + return selfOrThrowIfLocked(); } /** @@ -173,6 +176,9 @@ public RequestBuilder transition( @SuppressWarnings("unchecked") public RequestBuilder listener( @Nullable RequestListener requestListener) { + if (isAutoCloneEnabled()) { + return clone().listener(requestListener); + } this.requestListeners = null; return addListener(requestListener); } @@ -188,13 +194,16 @@ public RequestBuilder listener( @CheckResult public RequestBuilder addListener( @Nullable RequestListener 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 addListener( */ @NonNull public RequestBuilder error(@Nullable RequestBuilder errorBuilder) { + if (isAutoCloneEnabled()) { + return clone().error(errorBuilder); + } this.errorBuilder = errorBuilder; - return this; + return selfOrThrowIfLocked(); } /** @@ -279,9 +291,12 @@ private RequestBuilder cloneWithNullErrorAndThumbnail() { @SuppressWarnings("unchecked") public RequestBuilder thumbnail( @Nullable RequestBuilder thumbnailRequest) { + if (isAutoCloneEnabled()) { + return clone().thumbnail(thumbnailRequest); + } this.thumbnailBuilder = thumbnailRequest; - return this; + return selfOrThrowIfLocked(); } /** @@ -413,12 +428,15 @@ public RequestBuilder thumbnail( @CheckResult @SuppressWarnings("unchecked") public RequestBuilder 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 load(@Nullable Object model) { @NonNull private RequestBuilder 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 index 6472b80600..dd50ca1490 100644 --- 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; }