Merge pull request square#3040 from square/jakew/skip-callback-execut…

…or/2019-02-28

Back suspend methods with the CallAdapter for Call.

retrofit/src/main/java/retrofit2/HttpServiceMethod.java

@@ -21,7 +21,6 @@
 import java.lang.reflect.Type;
 import javax.annotation.Nullable;
 import kotlin.coroutines.Continuation;
-import okhttp3.Call;
 import okhttp3.ResponseBody;
 
 import static retrofit2.Utils.getRawType;
@@ -36,14 +35,16 @@ abstract class HttpServiceMethod<ResponseT, ReturnT> extends ServiceMethod<Retur
    */
   static <ResponseT, ReturnT> HttpServiceMethod<ResponseT, ReturnT> parseAnnotations(
       Retrofit retrofit, Method method, RequestFactory requestFactory) {
-    CallAdapter<ResponseT, ReturnT> callAdapter = null;
+    boolean isKotlinSuspendFunction = requestFactory.isKotlinSuspendFunction;
     boolean continuationWantsResponse = false;
     boolean continuationBodyNullable = false;
-    Type responseType;
-    if (requestFactory.isKotlinSuspendFunction) {
+
+    Annotation[] annotations = method.getAnnotations();
+    Type adapterType;
+    if (isKotlinSuspendFunction) {
       Type[] parameterTypes = method.getGenericParameterTypes();
-      Type continuationType = parameterTypes[parameterTypes.length - 1];
-      responseType = Utils.getParameterLowerBound(0, (ParameterizedType) continuationType);
+      Type responseType = Utils.getParameterLowerBound(0,
+          (ParameterizedType) parameterTypes[parameterTypes.length - 1]);
       if (getRawType(responseType) == Response.class && responseType instanceof ParameterizedType) {
         // Unwrap the actual body type from Response<T>.
         responseType = Utils.getParameterUpperBound(0, (ParameterizedType) responseType);
@@ -54,11 +55,16 @@ static <ResponseT, ReturnT> HttpServiceMethod<ResponseT, ReturnT> parseAnnotatio
         // Find the entry for method
         // Determine if return type is nullable or not
       }
+
+      adapterType = new Utils.ParameterizedTypeImpl(null, Call.class, responseType);
+      annotations = SkipCallbackExecutorImpl.ensurePresent(annotations);
     } else {
-      callAdapter = createCallAdapter(retrofit, method);
-      responseType = callAdapter.responseType();
+      adapterType = method.getGenericReturnType();
     }
 
+    CallAdapter<ResponseT, ReturnT> callAdapter =
+        createCallAdapter(retrofit, method, adapterType, annotations);
+    Type responseType = callAdapter.responseType();
     if (responseType == okhttp3.Response.class) {
       throw methodError(method, "'"
           + getRawType(responseType).getName()
@@ -76,23 +82,22 @@ static <ResponseT, ReturnT> HttpServiceMethod<ResponseT, ReturnT> parseAnnotatio
         createResponseConverter(retrofit, method, responseType);
 
     okhttp3.Call.Factory callFactory = retrofit.callFactory;
-    if (callAdapter != null) {
-      return new CallAdapted<>(requestFactory, callFactory, callAdapter, responseConverter);
+    if (!isKotlinSuspendFunction) {
+      return new CallAdapted<>(requestFactory, callFactory, responseConverter, callAdapter);
     } else if (continuationWantsResponse) {
       //noinspection unchecked Kotlin compiler guarantees ReturnT to be Object.
       return (HttpServiceMethod<ResponseT, ReturnT>) new SuspendForResponse<>(requestFactory,
-          callFactory, responseConverter);
+          callFactory, responseConverter, (CallAdapter<ResponseT, Call<ResponseT>>) callAdapter);
     } else {
       //noinspection unchecked Kotlin compiler guarantees ReturnT to be Object.
       return (HttpServiceMethod<ResponseT, ReturnT>) new SuspendForBody<>(requestFactory,
-          callFactory, responseConverter, continuationBodyNullable);
+          callFactory, responseConverter, (CallAdapter<ResponseT, Call<ResponseT>>) callAdapter,
+          continuationBodyNullable);
     }
   }
 
   private static <ResponseT, ReturnT> CallAdapter<ResponseT, ReturnT> createCallAdapter(
-      Retrofit retrofit, Method method) {
-    Type returnType = method.getGenericReturnType();
-    Annotation[] annotations = method.getAnnotations();
+      Retrofit retrofit, Method method, Type returnType, Annotation[] annotations) {
     try {
       //noinspection unchecked
       return (CallAdapter<ResponseT, ReturnT>) retrofit.callAdapter(returnType, annotations);
@@ -115,57 +120,71 @@ private static <ResponseT> Converter<ResponseBody, ResponseT> createResponseConv
   private final okhttp3.Call.Factory callFactory;
   private final Converter<ResponseBody, ResponseT> responseConverter;
 
-  HttpServiceMethod(RequestFactory requestFactory, Call.Factory callFactory,
+  HttpServiceMethod(RequestFactory requestFactory, okhttp3.Call.Factory callFactory,
       Converter<ResponseBody, ResponseT> responseConverter) {
     this.requestFactory = requestFactory;
     this.callFactory = callFactory;
     this.responseConverter = responseConverter;
   }
 
   @Override final @Nullable ReturnT invoke(Object[] args) {
-    return adapt(new OkHttpCall<>(requestFactory, args, callFactory, responseConverter), args);
+    Call<ResponseT> call = new OkHttpCall<>(requestFactory, args, callFactory, responseConverter);
+    return adapt(call, args);
   }
 
-  protected abstract @Nullable ReturnT adapt(OkHttpCall<ResponseT> call, Object[] args);
+  protected abstract @Nullable ReturnT adapt(Call<ResponseT> call, Object[] args);
 
   static final class CallAdapted<ResponseT, ReturnT> extends HttpServiceMethod<ResponseT, ReturnT> {
     private final CallAdapter<ResponseT, ReturnT> callAdapter;
 
-    CallAdapted(RequestFactory requestFactory, Call.Factory callFactory,
-        CallAdapter<ResponseT, ReturnT> callAdapter,
-        Converter<ResponseBody, ResponseT> responseConverter) {
+    CallAdapted(RequestFactory requestFactory, okhttp3.Call.Factory callFactory,
+        Converter<ResponseBody, ResponseT> responseConverter,
+        CallAdapter<ResponseT, ReturnT> callAdapter) {
       super(requestFactory, callFactory, responseConverter);
       this.callAdapter = callAdapter;
     }
 
-    @Override protected ReturnT adapt(OkHttpCall<ResponseT> call, Object[] args) {
+    @Override protected ReturnT adapt(Call<ResponseT> call, Object[] args) {
       return callAdapter.adapt(call);
     }
   }
 
   static final class SuspendForResponse<ResponseT> extends HttpServiceMethod<ResponseT, Object> {
-    SuspendForResponse(RequestFactory requestFactory, Call.Factory callFactory,
-        Converter<ResponseBody, ResponseT> responseConverter) {
+    private final CallAdapter<ResponseT, Call<ResponseT>> callAdapter;
+
+    SuspendForResponse(RequestFactory requestFactory, okhttp3.Call.Factory callFactory,
+        Converter<ResponseBody, ResponseT> responseConverter,
+        CallAdapter<ResponseT, Call<ResponseT>> callAdapter) {
       super(requestFactory, callFactory, responseConverter);
+      this.callAdapter = callAdapter;
     }
 
-    @Override protected Object adapt(OkHttpCall<ResponseT> call, Object[] args) {
+    @Override protected Object adapt(Call<ResponseT> call, Object[] args) {
+      call = callAdapter.adapt(call);
+
+      //noinspection unchecked Checked by reflection inside RequestFactory.
       Continuation<Response<ResponseT>> continuation =
           (Continuation<Response<ResponseT>>) args[args.length - 1];
       return KotlinExtensions.awaitResponse(call, continuation);
     }
   }
 
   static final class SuspendForBody<ResponseT> extends HttpServiceMethod<ResponseT, Object> {
+    private final CallAdapter<ResponseT, Call<ResponseT>> callAdapter;
     private final boolean isNullable;
 
-    SuspendForBody(RequestFactory requestFactory, Call.Factory callFactory,
-        Converter<ResponseBody, ResponseT> responseConverter, boolean isNullable) {
+    SuspendForBody(RequestFactory requestFactory, okhttp3.Call.Factory callFactory,
+        Converter<ResponseBody, ResponseT> responseConverter,
+        CallAdapter<ResponseT, Call<ResponseT>> callAdapter, boolean isNullable) {
       super(requestFactory, callFactory, responseConverter);
+      this.callAdapter = callAdapter;
       this.isNullable = isNullable;
     }
 
-    @Override protected Object adapt(OkHttpCall<ResponseT> call, Object[] args) {
+    @Override protected Object adapt(Call<ResponseT> call, Object[] args) {
+      call = callAdapter.adapt(call);
+
+      //noinspection unchecked Checked by reflection inside RequestFactory.
       Continuation<ResponseT> continuation = (Continuation<ResponseT>) args[args.length - 1];
       return isNullable
           ? KotlinExtensions.awaitNullable(call, continuation)

retrofit/src/main/java/retrofit2/SkipCallbackExecutorImpl.java

@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2019 Square, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package retrofit2;
+
+import java.lang.annotation.Annotation;
+
+// This class conforms to the annotation requirements documented on Annotation.
+final class SkipCallbackExecutorImpl implements SkipCallbackExecutor {
+  private static final SkipCallbackExecutor INSTANCE = new SkipCallbackExecutorImpl();
+
+  static Annotation[] ensurePresent(Annotation[] annotations) {
+    if (Utils.isAnnotationPresent(annotations, SkipCallbackExecutor.class)) {
+      return annotations;
+    }
+
+    Annotation[] newAnnotations = new Annotation[annotations.length + 1];
+    // Place the skip annotation first since we're guaranteed to check for it in the call adapter.
+    newAnnotations[0] = SkipCallbackExecutorImpl.INSTANCE;
+    System.arraycopy(annotations, 0, newAnnotations, 1, annotations.length);
+    return newAnnotations;
+  }
+
+  @Override public Class<? extends Annotation> annotationType() {
+    return SkipCallbackExecutor.class;
+  }
+
+  @Override public boolean equals(Object obj) {
+    return obj instanceof SkipCallbackExecutor;
+  }
+
+  @Override public int hashCode() {
+    return 0;
+  }
+
+  @Override public String toString() {
+    return "@" + SkipCallbackExecutor.class.getName() + "()";
+  }
+}

retrofit/src/main/java/retrofit2/Utils.java

@@ -383,7 +383,7 @@ static boolean hasUnresolvableType(@Nullable Type type) {
         + "GenericArrayType, but <" + type + "> is of type " + className);
   }
 
-  private static final class ParameterizedTypeImpl implements ParameterizedType {
+  static final class ParameterizedTypeImpl implements ParameterizedType {
     private final Type ownerType;
     private final Type rawType;
     private final Type[] typeArguments;

retrofit/src/test/java/retrofit2/KotlinSuspendTest.kt

@@ -33,6 +33,8 @@ import retrofit2.helpers.ToStringConverterFactory
 import retrofit2.http.GET
 import retrofit2.http.Path
 import java.io.IOException
+import java.lang.reflect.ParameterizedType
+import java.lang.reflect.Type
 
 class KotlinSuspendTest {
   @get:Rule val server = MockWebServer()
@@ -213,4 +215,62 @@ class KotlinSuspendTest {
     deferred.cancel()
     assertTrue(call.isCanceled)
   }
+
+  @Test fun doesNotUseCallbackExecutor() {
+    val retrofit = Retrofit.Builder()
+        .baseUrl(server.url("/"))
+        .callbackExecutor { fail() }
+        .addConverterFactory(ToStringConverterFactory())
+        .build()
+    val example = retrofit.create(Service::class.java)
+
+    server.enqueue(MockResponse().setBody("Hi"))
+
+    val body = runBlocking { example.body() }
+    assertThat(body).isEqualTo("Hi")
+  }
+
+  @Test fun usesCallAdapterForCall() {
+    val callAdapterFactory = object : CallAdapter.Factory() {
+      override fun get(returnType: Type, annotations: Array<Annotation>,
+          retrofit: Retrofit): CallAdapter<*, *>? {
+        if (getRawType(returnType) != Call::class.java) {
+          return null
+        }
+        if (getParameterUpperBound(0, returnType as ParameterizedType) != String::class.java) {
+          return null
+        }
+        return object : CallAdapter<String, Call<String>> {
+          override fun responseType() = String::class.java
+          override fun adapt(call: Call<String>): Call<String> {
+            return object : Call<String> by call {
+              override fun enqueue(callback: Callback<String>) {
+                call.enqueue(object : Callback<String> by callback {
+                  override fun onResponse(call: Call<String>, response: Response<String>) {
+                    if (response.isSuccessful) {
+                      callback.onResponse(call, Response.success(response.body()?.repeat(5)))
+                    } else {
+                      callback.onResponse(call, response)
+                    }
+                  }
+                })
+              }
+            }
+          }
+        }
+      }
+    }
+
+    val retrofit = Retrofit.Builder()
+        .baseUrl(server.url("/"))
+        .addCallAdapterFactory(callAdapterFactory)
+        .addConverterFactory(ToStringConverterFactory())
+        .build()
+    val example = retrofit.create(Service::class.java)
+
+    server.enqueue(MockResponse().setBody("Hi"))
+
+    val body = runBlocking { example.body() }
+    assertThat(body).isEqualTo("HiHiHiHiHi")
+  }
 }