Call clone() before calling extension methods if auto clone is enabled.

Otherwise when our extension methods mutate the RequestOptions we pass them, the mutations create a new RequestOptions object that’s dropped at the end of the extension method, which causes any options applied in the extension method to be ignored. Cloning first matches the behavior of other RequestOptions methods and ensures that the RequestOptions object passed into the extension method is mutable.
bumptech · Aug 29, 2017 · b39a9db · b39a9db
1 parent 48754e6
commit b39a9db
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 13 deletions.
diff --git a/...ompiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestOptionsGenerator.java b/...ompiler/src/main/java/com/bumptech/glide/annotation/compiler/RequestOptionsGenerator.java
@@ -231,21 +231,28 @@ private List<MethodAndStaticVar> generateMethodsForRequestOptionsExtension(
     List<? extends VariableElement> parameters =
         element.getParameters().subList(1, element.getParameters().size());
 
-    // Add the correct super() call.
-    if (overrideType == OVERRIDE_EXTEND) {
-      String callSuper = "super.$L(";
-      List<Object> args = new ArrayList<>();
-      args.add(element.getSimpleName().toString());
-      if (!parameters.isEmpty()) {
-        for (VariableElement variable : parameters) {
-          callSuper += "$L, ";
-          args.add(variable.getSimpleName().toString());
-        }
-        callSuper = callSuper.substring(0, callSuper.length() - 2);
+    // Generates the String and list of arguments to pass in when calling this method or super.
+    // IE centerCrop(context) creates methodLiterals="%L" and methodArgs=[centerCrop, context].
+    List<Object> methodArgs = new ArrayList<>();
+    methodArgs.add(element.getSimpleName().toString());
+    String methodLiterals = "";
+    if (!parameters.isEmpty()) {
+      for (VariableElement variable : parameters) {
+        methodLiterals += "$L, ";
+        methodArgs.add(variable.getSimpleName().toString());
       }
-      callSuper += ")";
+      methodLiterals = methodLiterals.substring(0, methodLiterals.length() - 2);
+    }
+
+    builder.beginControlFlow("if (isAutoCloneEnabled())")
+        .addStatement(
+            "return clone().$N(" + methodLiterals + ")", methodArgs.toArray(new Object[0]))
+        .endControlFlow();
 
-      builder.addStatement(callSuper, args.toArray(new Object[0]))
+    // Add the correct super() call.
+    if (overrideType == OVERRIDE_EXTEND) {
+      String callSuper = "super.$L(" + methodLiterals + ")";
+      builder.addStatement(callSuper, methodArgs.toArray(new Object[0]))
           .addJavadoc(processorUtil.generateSeeMethodJavadoc(
               requestOptionsName, methodName, parameters))
           .addAnnotation(Override.class);

diff --git a/library/src/main/java/com/bumptech/glide/request/RequestOptions.java b/library/src/main/java/com/bumptech/glide/request/RequestOptions.java
@@ -1288,6 +1288,10 @@ private RequestOptions selfOrThrowIfLocked() {
     return this;
   }
 
+  protected boolean isAutoCloneEnabled() {
+    return isAutoCloneEnabled;
+  }
+
   @NonNull
   public final Map<Class<?>, Transformation<?>> getTransformations() {
     return transformations;