Merge pull request #2960 from square/jakew/optional-android/2018-11-16

Enable built-in Optional converter on Android API 24+

pom.xml

@@ -75,7 +75,7 @@
     <junit.version>4.12</junit.version>
     <assertj.version>1.7.0</assertj.version>
     <mockito.version>1.9.5</mockito.version>
-    <robolectric.version>3.0</robolectric.version>
+    <robolectric.version>3.2</robolectric.version>
   </properties>
 
   <scm>

retrofit/src/main/java/retrofit2/OptionalConverterFactory.java

@@ -24,11 +24,7 @@
 import okhttp3.ResponseBody;
 import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement;
 
-/**
- * A {@linkplain Converter.Factory converter} for {@code Optional<T>} which delegates to another
- * converter to deserialize {@code T} and then wraps it into {@link Optional}.
- */
-@IgnoreJRERequirement
+@IgnoreJRERequirement // Only added when Optional is available (Java 8+ / Android API 24+).
 final class OptionalConverterFactory extends Converter.Factory {
   static final Converter.Factory INSTANCE = new OptionalConverterFactory();
 

retrofit/src/main/java/retrofit2/Platform.java

@@ -22,6 +22,7 @@
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.Executor;
 import javax.annotation.Nullable;
@@ -149,6 +150,16 @@ static class Android extends Platform {
       return singletonList(new ExecutorCallAdapterFactory(callbackExecutor));
     }
 
+    @Override List<? extends Converter.Factory> defaultConverterFactories() {
+      return Build.VERSION.SDK_INT >= 24
+          ? singletonList(OptionalConverterFactory.INSTANCE)
+          : Collections.<Converter.Factory>emptyList();
+    }
+
+    @Override int defaultConverterFactoriesSize() {
+      return Build.VERSION.SDK_INT >= 24 ? 1 : 0;
+    }
+
     static class MainThreadExecutor implements Executor {
       private final Handler handler = new Handler(Looper.getMainLooper());
 

retrofit/src/test/java/retrofit2/OptionalConverterFactoryAndroidTest.java

@@ -0,0 +1,82 @@
+/*
+ * Copyright (C) 2017 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.io.IOException;
+import java.util.Optional;
+import okhttp3.mockwebserver.MockResponse;
+import okhttp3.mockwebserver.MockWebServer;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.annotation.Config;
+import retrofit2.helpers.ObjectInstanceConverterFactory;
+import retrofit2.http.GET;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.fail;
+import static org.robolectric.annotation.Config.NEWEST_SDK;
+
+@RunWith(RobolectricTestRunner.class)
+@Config(sdk = NEWEST_SDK)
+public final class OptionalConverterFactoryAndroidTest {
+  interface Service {
+    @GET("/") Call<Optional<Object>> optional();
+    @GET("/") Call<Object> object();
+  }
+
+  @Rule public final MockWebServer server = new MockWebServer();
+
+  private Service service;
+
+  @Before public void setUp() {
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl(server.url("/"))
+        .addConverterFactory(new ObjectInstanceConverterFactory())
+        .build();
+    service = retrofit.create(Service.class);
+  }
+
+  @Config(sdk = 24)
+  @Test public void optionalApi24() throws IOException {
+    server.enqueue(new MockResponse());
+
+    Optional<Object> optional = service.optional().execute().body();
+    assertThat(optional).isNotNull();
+    assertThat(optional.get()).isSameAs(ObjectInstanceConverterFactory.VALUE);
+  }
+
+  @Config(sdk = 21)
+  @Test public void optionalPreApi24() {
+    try {
+      service.optional();
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage(
+          "Unable to create converter for java.util.Optional<java.lang.Object>\n"
+              + "    for method Service.optional");
+    }
+  }
+
+  @Test public void onlyMatchesOptional() throws IOException {
+    server.enqueue(new MockResponse());
+
+    Object body = service.object().execute().body();
+    assertThat(body).isSameAs(ObjectInstanceConverterFactory.VALUE);
+  }
+}

retrofit/src/test/java/retrofit2/OptionalConverterFactoryTest.java

@@ -16,16 +16,13 @@
 package retrofit2;
 
 import java.io.IOException;
-import java.lang.annotation.Annotation;
-import java.lang.reflect.Type;
 import java.util.Optional;
-import javax.annotation.Nullable;
-import okhttp3.ResponseBody;
 import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import retrofit2.helpers.ObjectInstanceConverterFactory;
 import retrofit2.http.GET;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -43,7 +40,7 @@ interface Service {
   @Before public void setUp() {
     Retrofit retrofit = new Retrofit.Builder()
         .baseUrl(server.url("/"))
-        .addConverterFactory(new ObjectToNullConverterFactory())
+        .addConverterFactory(new ObjectInstanceConverterFactory())
         .build();
     service = retrofit.create(Service.class);
   }
@@ -53,27 +50,13 @@ interface Service {
 
     Optional<Object> optional = service.optional().execute().body();
     assertThat(optional).isNotNull();
-    assertThat(optional.isPresent()).isFalse();
+    assertThat(optional.get()).isSameAs(ObjectInstanceConverterFactory.VALUE);
   }
 
   @Test public void onlyMatchesOptional() throws IOException {
     server.enqueue(new MockResponse());
 
     Object body = service.object().execute().body();
-    assertThat(body).isNull();
-  }
-
-  static final class ObjectToNullConverterFactory extends Converter.Factory {
-    @Override public @Nullable Converter<ResponseBody, ?> responseBodyConverter(
-        Type type, Annotation[] annotations, Retrofit retrofit) {
-      if (type != Object.class) {
-        return null;
-      }
-      return new Converter<ResponseBody, Object>() {
-        @Override public Object convert(ResponseBody value) {
-          return null;
-        }
-      };
-    }
+    assertThat(body).isSameAs(ObjectInstanceConverterFactory.VALUE);
   }
 }

retrofit/src/test/java/retrofit2/RequestFactoryAndroidTest.java

@@ -21,13 +21,16 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.robolectric.RobolectricTestRunner;
+import org.robolectric.annotation.Config;
 import retrofit2.http.GET;
 import retrofit2.http.Url;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.robolectric.annotation.Config.NEWEST_SDK;
 import static retrofit2.RequestFactoryTest.buildRequest;
 
 @RunWith(RobolectricTestRunner.class)
+@Config(sdk = NEWEST_SDK)
 @SuppressWarnings({"UnusedParameters", "unused"}) // Parameters inspected reflectively.
 public final class RequestFactoryAndroidTest {
   @Test public void getWithAndroidUriUrl() {

retrofit/src/test/java/retrofit2/helpers/ObjectInstanceConverterFactory.java

@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2017 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.helpers;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Type;
+import javax.annotation.Nullable;
+import okhttp3.ResponseBody;
+import retrofit2.Converter;
+import retrofit2.Retrofit;
+
+public final class ObjectInstanceConverterFactory extends Converter.Factory {
+  public static final Object VALUE = new Object();
+
+  @Override public @Nullable Converter<ResponseBody, ?> responseBodyConverter(
+      Type type, Annotation[] annotations, Retrofit retrofit) {
+    if (type != Object.class) {
+      return null;
+    }
+    return new Converter<ResponseBody, Object>() {
+      @Override public Object convert(ResponseBody value) {
+        return VALUE;
+      }
+    };
+  }
+}