Repository: square/retrofit
Issue: Proposal: Completable API support in rxjava adapter
Labels: Enhancement
Blocked

There's a new `Completable` API coming in RxJava soon, would this be something that you'd want to support? This would basically just notify if the request failed or finished and you don't care to or want to process the response body (firing off analytics events comes to mind).

http://akarnokd.blogspot.com/2015/12/the-new-completable-api-part-1.html

Presumably would look something like this in use:

``` java
@POST('myendpoint')
Completable sendAnalyticsEvent(@Body AnalyticsEvent event)
```


Issue comments:
I don't really care. So, sure! Its implementation would be just returning `Void` as the response type and calling `toCompletable()` on the `Observable<Void>`.

:+1: cool, I can look into contributing a PR for it once it's released. Would you prefer to wait until it's in the public API or are Beta/Experimental ok when it's released in 1.1.1?

Beta or Experimental is fine, but it needs to be done in a way that if the
class is renamed or deleted it doesn't break Retrofit. Look to the current
Single behavior for how to do that.

On Mon, Jan 4, 2016 at 1:19 AM Zac Sweers notifications@github.com wrote:

> [image: :+1:] cool, I can look into contributing a PR for it once it's
> released. Would you prefer to wait until it's in the public API or are
> Beta/Incubating ok?
> 
> —
> Reply to this email directly or view it on GitHub
> https://github.com/square/retrofit/issues/1432#issuecomment-168592256.

Kicked the tires a bit and have a working POC here if you want to take a look. Will wait until it's released before PR-ing.

https://github.com/hzsweers/retrofit/tree/z/completable

d80d65c902648b4f1402fb85f784d57504efe1ec


Pull request review comments:
This goes in the root pom.

You can delete the try/catch and `assertThat`, they don't add value

Whoops, fixed in d4a8b7c525bc4fc9a2b634d164aeb2ee02af928a

6970b1d

Why the wrapper? Just return `callAdapter` here.

Actually, this whole thing is stateless so there can be a singleton instance in a static field.

Why `CompositeSubscription` here? We should just be able to use the `Subscription` returned from `Subscriptions.create` directly.

uber nit: you don't need `else` after a `return`. Helps save indentation (and it'll make the diff look a lot nicer)

Good point! Was going by SingleHelper's implementation at first and didn't realize this could all be simplified a lot. 6f011ac

Good point, was just following what convention I saw internally in `Completable` since I wasn't sure how to test this portion.  bd46587

f0b95cd

nit: remove `private` to avoid the need for a synthetic accessor method to be generated

e0f8cf0


Patch that resolved the issue:
diff --git a/pom.xml b/pom.xml
index 846b9c6c7a..1339bf0c5e 100644
--- a/pom.xml
+++ b/pom.xml
@@ -53,7 +53,7 @@
     <animal.sniffer.version>1.14</animal.sniffer.version>
 
     <!-- Adapter Dependencies -->
-    <rxjava.version>1.1.0</rxjava.version>
+    <rxjava.version>1.1.1</rxjava.version>
 
     <!-- Converter Dependencies -->
     <gson.version>2.4</gson.version>
diff --git a/retrofit-adapters/rxjava/src/main/java/retrofit2/adapter/rxjava/CompletableHelper.java b/retrofit-adapters/rxjava/src/main/java/retrofit2/adapter/rxjava/CompletableHelper.java
new file mode 100644
index 0000000000..7f5980bde8
--- /dev/null
+++ b/retrofit-adapters/rxjava/src/main/java/retrofit2/adapter/rxjava/CompletableHelper.java
@@ -0,0 +1,84 @@
+/*
+ * Copyright (C) 2016 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.adapter.rxjava;
+
+import retrofit2.Call;
+import retrofit2.CallAdapter;
+import retrofit2.Response;
+import rx.Completable;
+import rx.Subscription;
+import rx.exceptions.Exceptions;
+import rx.functions.Action0;
+import rx.subscriptions.Subscriptions;
+
+import java.lang.reflect.Type;
+
+final class CompletableHelper {
+
+  private static final CallAdapter<Completable> COMPLETABLE_CALL_ADAPTER
+          = new CallAdapter<Completable>() {
+            @Override public Type responseType() {
+              return Void.class;
+            }
+
+            @Override public Completable adapt(Call call) {
+              return Completable.create(new CompletableCallOnSubscribe(call));
+            }
+          };
+
+  static CallAdapter<Completable> makeCompletable() {
+    return COMPLETABLE_CALL_ADAPTER;
+  }
+
+  private static final class CompletableCallOnSubscribe
+      implements Completable.CompletableOnSubscribe {
+    private final Call originalCall;
+
+    CompletableCallOnSubscribe(Call originalCall) {
+      this.originalCall = originalCall;
+    }
+
+    @Override
+    public void call(final Completable.CompletableSubscriber subscriber) {
+      // Since Call is a one-shot type, clone it for each new subscriber.
+      final Call call = originalCall.clone();
+
+      // Attempt to cancel the call if it is still in-flight on unsubscription.
+      Subscription subscription = Subscriptions.create(new Action0() {
+        @Override public void call() {
+          call.cancel();
+        }
+      });
+      subscriber.onSubscribe(subscription);
+
+      try {
+        Response response = call.execute();
+        if (!subscription.isUnsubscribed()) {
+          if (response.isSuccess()) {
+            subscriber.onCompleted();
+          } else {
+            subscriber.onError(new HttpException(response));
+          }
+        }
+      } catch (Throwable t) {
+        Exceptions.throwIfFatal(t);
+        if (!subscription.isUnsubscribed()) {
+          subscriber.onError(t);
+        }
+      }
+    }
+  }
+}
diff --git a/retrofit-adapters/rxjava/src/main/java/retrofit2/adapter/rxjava/RxJavaCallAdapterFactory.java b/retrofit-adapters/rxjava/src/main/java/retrofit2/adapter/rxjava/RxJavaCallAdapterFactory.java
index f4b60209a7..ecbdc8a533 100644
--- a/retrofit-adapters/rxjava/src/main/java/retrofit2/adapter/rxjava/RxJavaCallAdapterFactory.java
+++ b/retrofit-adapters/rxjava/src/main/java/retrofit2/adapter/rxjava/RxJavaCallAdapterFactory.java
@@ -15,9 +15,6 @@
  */
 package retrofit2.adapter.rxjava;
 
-import java.lang.annotation.Annotation;
-import java.lang.reflect.ParameterizedType;
-import java.lang.reflect.Type;
 import retrofit2.Call;
 import retrofit2.CallAdapter;
 import retrofit2.Response;
@@ -29,6 +26,10 @@
 import rx.functions.Func1;
 import rx.subscriptions.Subscriptions;
 
+import java.lang.annotation.Annotation;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
+
 /**
  * TODO docs
  */
@@ -46,20 +47,31 @@ private RxJavaCallAdapterFactory() {
   @Override
   public CallAdapter<?> get(Type returnType, Annotation[] annotations, Retrofit retrofit) {
     Class<?> rawType = getRawType(returnType);
-    boolean isSingle = "rx.Single".equals(rawType.getCanonicalName());
-    if (rawType != Observable.class && !isSingle) {
+    final String canonicalName = rawType.getCanonicalName();
+    boolean isSingle = "rx.Single".equals(canonicalName);
+    boolean isCompletable = "rx.Completable".equals(canonicalName);
+    if (rawType != Observable.class && !isSingle && !isCompletable) {
       return null;
     }
-    if (!(returnType instanceof ParameterizedType)) {
+    if (!isCompletable && !(returnType instanceof ParameterizedType)) {
       String name = isSingle ? "Single" : "Observable";
       throw new IllegalStateException(name + " return type must be parameterized"
           + " as " + name + "<Foo> or " + name + "<? extends Foo>");
     }
 
+    if (isCompletable) {
+      // Add Completable-converter wrapper from a separate class. This defers classloading such that
+      // regular Observable operation can be leveraged without relying on this unstable RxJava API.
+      // Note that this has to be done separately since Completable doesn't have a parametrized
+      // type.
+      return CompletableHelper.makeCompletable();
+    }
+
     CallAdapter<Observable<?>> callAdapter = getCallAdapter(returnType);
     if (isSingle) {
       // Add Single-converter wrapper from a separate class. This defers classloading such that
-      // regular Observable operation can be leveraged without relying on this unstable RxJava API.
+      // regular Observable operation can be leveraged without relying on this unstable RxJava
+      // API.
       return SingleHelper.makeSingle(callAdapter);
     }
     return callAdapter;


Test patch:
diff --git a/retrofit-adapters/rxjava/src/test/java/retrofit2/adapter/rxjava/CompletableTest.java b/retrofit-adapters/rxjava/src/test/java/retrofit2/adapter/rxjava/CompletableTest.java
new file mode 100644
index 0000000000..f52b92ee33
--- /dev/null
+++ b/retrofit-adapters/rxjava/src/test/java/retrofit2/adapter/rxjava/CompletableTest.java
@@ -0,0 +1,80 @@
+/*
+ * Copyright (C) 2016 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.adapter.rxjava;
+
+import okhttp3.mockwebserver.MockResponse;
+import okhttp3.mockwebserver.MockWebServer;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import retrofit2.Retrofit;
+import retrofit2.http.GET;
+import rx.Completable;
+
+import java.io.IOException;
+
+import static okhttp3.mockwebserver.SocketPolicy.DISCONNECT_AFTER_REQUEST;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.fail;
+
+public final class CompletableTest {
+  @Rule
+  public final MockWebServer server = new MockWebServer();
+
+  interface Service {
+    @GET("/") Completable completable();
+  }
+
+  private Service service;
+
+  @Before public void setUp() {
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl(server.url("/"))
+        .addConverterFactory(new StringConverterFactory())
+        .addCallAdapterFactory(RxJavaCallAdapterFactory.create())
+        .build();
+    service = retrofit.create(Service.class);
+  }
+
+  @Test public void completableSuccess200() {
+    server.enqueue(new MockResponse().setBody("Hi"));
+
+    service.completable().await();
+  }
+
+  @Test public void completableSuccess404() {
+    server.enqueue(new MockResponse().setResponseCode(404));
+
+    try {
+      service.completable().await();
+      fail();
+    } catch (RuntimeException e) {
+      Throwable cause = e.getCause();
+      assertThat(cause).isInstanceOf(HttpException.class).hasMessage("HTTP 404 Client Error");
+    }
+  }
+
+  @Test public void completableFailure() {
+    server.enqueue(new MockResponse().setSocketPolicy(DISCONNECT_AFTER_REQUEST));
+
+    try {
+      service.completable().await();
+      fail();
+    } catch (RuntimeException e) {
+      assertThat(e.getCause()).isInstanceOf(IOException.class);
+    }
+  }
+}