Repository: square/retrofit
Issue: Support for Java8 interface static methods
Labels: Needs Info

Retrofit 2.1.0 currently throws an exception building a client implementation proxy if the interface has static methods.


Issue comments:
Should be easy to fix. Can you paste the exception?

On Wed, Sep 28, 2016 at 1:30 PM mdeluisi notifications@github.com wrote:

> Retrofit 2.1.0 currently throws an exception building a client
> implementation proxy if the interface has static methods.
> 
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> https://github.com/square/retrofit/issues/2032, or mute the thread
> https://github.com/notifications/unsubscribe-auth/AAEEEWF0EQ4Jqo4NpmX2h_F2R-6gYDqQks5quqQqgaJpZM4KJGFG
> .

static method is declared as the following in the interface:

`
static TalendClientBuilder builder() {
   return TalendClientBuilder.create();
}
`
Exception:
Caused by: java.lang.IllegalArgumentException: Could not locate call adapter for class com.corp.talend.client.v1.api.TalendClientBuilder.
  Tried:
- retrofit2.DefaultCallAdapterFactory
  at retrofit2.Retrofit.nextCallAdapter(Retrofit.java:237) ~[retrofit-2.1.0.jar:?]
  at retrofit2.Retrofit.callAdapter(Retrofit.java:201) ~[retrofit-2.1.0.jar:?]
  at retrofit2.ServiceMethod$Builder.createCallAdapter(ServiceMethod.java:232) ~[retrofit-2.1.0.jar:?]
  at retrofit2.ServiceMethod$Builder.build(ServiceMethod.java:160) ~[retrofit-2.1.0.jar:?]
  at retrofit2.Retrofit.loadServiceMethod(Retrofit.java:166) ~[retrofit-2.1.0.jar:?]
  at retrofit2.Retrofit.eagerlyValidateMethods(Retrofit.java:156) ~[retrofit-2.1.0.jar:?]
  at retrofit2.Retrofit.create(Retrofit.java:130) ~[retrofit-2.1.0.jar:?]
  ...

Looks like the issue is in Retrofit.eagerlyValidateMethods method.  It should probably just bypass the validation if the method is static.  If I disable eager validation the create() call works fine and a proxy is created.

I don't see this failing locally. Are you sure you defined the method as static?

Did you enable eager validation in your test?  Yea, I declared it exactly like my example posted above.

I was able to reproduce an issue related to this: https://github.com/bnorm/retrofit/blob/bnorm.1030.static_method/retrofit-adapters/java8/src/test/java/retrofit2/adapter/java8/Java8StaticMethodsTest.java

I can open a pull-request with this failing unit test if that is helpful.

Also, I would be happy to contribute a fix. Should just be adding `&& (method.getModifiers() & Modifier.STATIC) != Modifier.STATIC` to the `Retrofit.eagerlyValidateMethods` method either directly or through `Platform`.

Pull request review comments:
Let me know if you want this removed. It's weird to be calling a static method on an instance, but technically legal. This at least keeps that behavior
Is this line reachable in test cases? I assume it won’t be dispatched this way
Yeah I don't think this can happen. The only case I can think of is if a virtual method was made static without recompiling the caller. But even then I would expect the VM to intervene.
done f1fbbda
Can you move this to a separate test class? I don't want to conflate the two things being tested here together.
fbf8d3c

Left the var name fix in the original though as an opportunistic fix
Is the condition reachable?
Ahh, this happens during reflection, not invocation. Works for me.

Patch that resolved the issue:
diff --git a/retrofit/src/main/java/retrofit2/Retrofit.java b/retrofit/src/main/java/retrofit2/Retrofit.java
index a9040f8b3a..5d0ad8957a 100644
--- a/retrofit/src/main/java/retrofit2/Retrofit.java
+++ b/retrofit/src/main/java/retrofit2/Retrofit.java
@@ -18,6 +18,7 @@
 import java.lang.annotation.Annotation;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.lang.reflect.Proxy;
 import java.lang.reflect.Type;
 import java.util.ArrayList;
@@ -152,7 +153,7 @@ public <T> T create(final Class<T> service) {
   private void eagerlyValidateMethods(Class<?> service) {
     Platform platform = Platform.get();
     for (Method method : service.getDeclaredMethods()) {
-      if (!platform.isDefaultMethod(method)) {
+      if (!platform.isDefaultMethod(method) && !Modifier.isStatic(method.getModifiers())) {
         loadServiceMethod(method);
       }
     }


Test patch:
diff --git a/retrofit/src/test/java/retrofit2/Java8DefaultMethodsTest.java b/retrofit/src/test/java/retrofit2/Java8DefaultMethodsTest.java
index 8333969c65..501c909e48 100644
--- a/retrofit/src/test/java/retrofit2/Java8DefaultMethodsTest.java
+++ b/retrofit/src/test/java/retrofit2/Java8DefaultMethodsTest.java
@@ -39,7 +39,7 @@ public final class Java8DefaultMethodsTest {
   //
   //  Response<String> response = example.user().execute();
   //  assertThat(response.body()).isEqualTo("Hi");
-  //  Response<String> response = example.user("hi").execute();
-  //  assertThat(response.body()).isEqualTo("Hi");
+  //  Response<String> response2 = example.user("Hi").execute();
+  //  assertThat(response2.body()).isEqualTo("Hi");
   //}
 }
diff --git a/retrofit/src/test/java/retrofit2/Java8DefaultStaticMethodsInValidationTest.java b/retrofit/src/test/java/retrofit2/Java8DefaultStaticMethodsInValidationTest.java
new file mode 100644
index 0000000000..bc2521d5a3
--- /dev/null
+++ b/retrofit/src/test/java/retrofit2/Java8DefaultStaticMethodsInValidationTest.java
@@ -0,0 +1,42 @@
+/*
+ * 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;
+
+// TODO this test doesn't play nice in the IDE because it relies on Java 8 language features.
+public final class Java8DefaultStaticMethodsInValidationTest {
+  //@Rule public final MockWebServer server = new MockWebServer();
+  //
+  //interface Example {
+  //  @GET("/") Call<String> user(@Query("name") String name);
+  //
+  //  default Call<String> user() {
+  //    return user("hey");
+  //  }
+  //
+  //  static String staticMethod() {
+  //    return "Hi";
+  //  }
+  //}
+  //
+  //@Test public void test() throws IOException {
+  //  Retrofit retrofit = new Retrofit.Builder()
+  //      .baseUrl(server.url("/"))
+  //      .addConverterFactory(new ToStringConverterFactory())
+  //      .validateEagerly(true)
+  //      .build();
+  //  Example example = retrofit.create(Example.class);
+  //}
+}