feedback-to-code/mozzarella huggingface dataset:
Repository: square/retrofit
Issue: Change Thread Names Based On Current Request

To make thread dump debugging a lot more clear.


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/retrofit/src/main/java/retrofit/http/Platform.java b/retrofit/src/main/java/retrofit/http/Platform.java
index 377cdcc596..fa72a470c4 100644
--- a/retrofit/src/main/java/retrofit/http/Platform.java
+++ b/retrofit/src/main/java/retrofit/http/Platform.java
@@ -14,6 +14,7 @@
 
 import static android.os.Process.THREAD_PRIORITY_BACKGROUND;
 import static retrofit.http.RestAdapter.SynchronousExecutor;
+import static retrofit.http.RestAdapter.THREAD_PREFIX;
 
 abstract class Platform {
   private static final Platform PLATFORM = findPlatform();
@@ -58,7 +59,7 @@ private static class Base extends Platform {
               Thread.currentThread().setPriority(THREAD_PRIORITY_BACKGROUND);
               r.run();
             }
-          }, "Retrofit-" + threadCounter.getAndIncrement());
+          }, THREAD_PREFIX + threadCounter.getAndIncrement());
         }
       });
     }
@@ -89,7 +90,7 @@ private static class Android extends Platform {
               Process.setThreadPriority(THREAD_PRIORITY_BACKGROUND);
               r.run();
             }
-          }, "Retrofit-" + threadCounter.getAndIncrement());
+          }, THREAD_PREFIX + threadCounter.getAndIncrement());
         }
       });
     }
diff --git a/retrofit/src/main/java/retrofit/http/RestAdapter.java b/retrofit/src/main/java/retrofit/http/RestAdapter.java
index fd3fa2e5d8..8f605c273b 100644
--- a/retrofit/src/main/java/retrofit/http/RestAdapter.java
+++ b/retrofit/src/main/java/retrofit/http/RestAdapter.java
@@ -44,6 +44,7 @@
 public class RestAdapter {
   private static final Logger LOGGER = Logger.getLogger(RestAdapter.class.getName());
   private static final int LOG_CHUNK_SIZE = 4000;
+  static final String THREAD_PREFIX = "Retrofit-";
 
   private final Server server;
   private final Provider<HttpClient> httpClientProvider;
@@ -141,6 +142,11 @@ private Object invokeRequest(Method method, Object[] args, boolean isSynchronous
             .build();
         url = request.getURI().toString();
 
+        if (!isSynchronousInvocation) {
+          // If we are executing asynchronously then update the current thread with a useful name.
+          Thread.currentThread().setName(THREAD_PREFIX + url);
+        }
+
         // Determine deserialization type by method return type or generic parameter to Callback argument.
         Type type = responseTypeCache.get(method);
         if (type == null) {


Test patch:
Repository: square/retrofit
Issue: Calling Non-endpoint Methods Triggers Logic

We need to only proceed trying to make an HTTP request if the method invoked on the `InvocationHandler` is annotated with one of our HTTP verb annotations.


Issue comments:
Fixed by #132 .


Pull request review comments:
use `Class<?>` here. Otherwise somebody might accidentally pass a parameterized type, and that'll never work in comparison against `getDeclaringClass()`.


Patch that resolved the issue:
diff --git a/retrofit/src/main/java/retrofit/http/RestAdapter.java b/retrofit/src/main/java/retrofit/http/RestAdapter.java
index 37c9647df5..4e3fc72464 100644
--- a/retrofit/src/main/java/retrofit/http/RestAdapter.java
+++ b/retrofit/src/main/java/retrofit/http/RestAdapter.java
@@ -5,6 +5,7 @@
 import java.io.UnsupportedEncodingException;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Proxy;
@@ -100,15 +101,26 @@ private RestAdapter(Server server, Provider<HttpClient> httpClientProvider, Exec
   @SuppressWarnings("unchecked")
   public <T> T create(Class<T> type) {
     return (T) Proxy.newProxyInstance(type.getClassLoader(), new Class<?>[] { type },
-        new RestHandler());
+        new RestHandler(type));
   }
 
   private class RestHandler implements InvocationHandler {
-    final Map<Method, MethodDetails> methodDetailsCache =
+    private final Class<?> declaringType;
+    private final Map<Method, MethodDetails> methodDetailsCache =
         new LinkedHashMap<Method, MethodDetails>();
 
+    RestHandler(Class<?> declaringType) {
+      this.declaringType = declaringType;
+    }
+
     @SuppressWarnings("unchecked")
-    @Override public Object invoke(Object proxy, Method method, final Object[] args) {
+    @Override public Object invoke(Object proxy, Method method, final Object[] args)
+        throws InvocationTargetException, IllegalAccessException {
+      // If the method is not a direct member of the interface then defer to normal invocation.
+      if (method.getDeclaringClass() != declaringType) {
+        return method.invoke(this, args);
+      }
+
       // Load or create the details cache for the current method.
       final MethodDetails methodDetails;
       synchronized (methodDetailsCache) {


Test patch:
diff --git a/retrofit/src/test/java/retrofit/http/RestAdapterTest.java b/retrofit/src/test/java/retrofit/http/RestAdapterTest.java
index 068ec3da3b..9b3a6f03ae 100644
--- a/retrofit/src/test/java/retrofit/http/RestAdapterTest.java
+++ b/retrofit/src/test/java/retrofit/http/RestAdapterTest.java
@@ -539,7 +539,6 @@ public void testSynchronousWithAsyncCallback() {
     method.init();
   }
 
-  @Ignore // TODO Issue #130
   @Test public void testNonEndpointMethodsSucceed() {
     TypeTestService service = restAdapter.create(TypeTestService.class);
     assertThat(service.equals(new Object())).isFalse();
Repository: square/retrofit
Issue: Named path params ending with digits should be valid

The following test should pass: https://github.com/matthewmichihara/retrofit/commit/abfade69c7ab213fd07dbcc2604f5942c3324114


Issue comments:
Ah so it looks like the regular expression that matches path params does not accept digits -- `private static final Pattern PATH_PARAMETERS = Pattern.compile("\\{([a-z_-]+)\\}");` at https://github.com/square/retrofit/blob/master/retrofit/src/main/java/retrofit/http/RestMethodInfo.java#L18.

Was it intentional that path params such as `/foo/{bar1}` not be allowed?


Pull request review comments:

Patch that resolved the issue:
diff --git a/retrofit/src/main/java/retrofit/http/RestMethodInfo.java b/retrofit/src/main/java/retrofit/http/RestMethodInfo.java
index e7ac42b56e..8591265de1 100644
--- a/retrofit/src/main/java/retrofit/http/RestMethodInfo.java
+++ b/retrofit/src/main/java/retrofit/http/RestMethodInfo.java
@@ -15,7 +15,10 @@
 /** Cached details about an interface method. */
 final class RestMethodInfo {
   static final int NO_SINGLE_ENTITY = -1;
-  private static final Pattern PATH_PARAMETERS = Pattern.compile("\\{([a-z_-]+)\\}");
+
+  // Matches strings containing lowercase characters, digits, underscores, or hyphens that start
+  // with a lowercase character in between '{' and '}'.
+  private static final Pattern PATH_PARAMETERS = Pattern.compile("\\{([a-z][a-z0-9_-]*)\\}");
 
   final Method method;
   final boolean isSynchronous;


Test patch:
diff --git a/retrofit/src/test/java/retrofit/http/RestMethodInfoTest.java b/retrofit/src/test/java/retrofit/http/RestMethodInfoTest.java
index 80255d253c..7591ecfd38 100644
--- a/retrofit/src/test/java/retrofit/http/RestMethodInfoTest.java
+++ b/retrofit/src/test/java/retrofit/http/RestMethodInfoTest.java
@@ -31,6 +31,8 @@ public class RestMethodInfoTest {
     expectParams("foo/bar/{taco}/or/{taco}", "taco");
     expectParams("foo/bar/{taco-shell}", "taco-shell");
     expectParams("foo/bar/{taco_shell}", "taco_shell");
+    expectParams("foo/bar/{sha256}", "sha256");
+    expectParams("foo/bar/{1}"); // Invalid parameter, name cannot start with digit.
   }
 
   private static void expectParams(String path, String... expected) {
Repository: square/retrofit
Issue: Missing RestAdapter Documentation
Labels: Website

The primary documentation for Retrofit appears to be http://square.github.io/retrofit/. However, that page is missing how to actually instantiate the `RestAdapter` (only documented on [the blog post announcing Retrofit](http://corner.squareup.com/2013/05/retrofit-one-dot-oh.html)). Basically, the page just starts talking about `restAdapter` with no indication of where it came from.


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/website/index.html b/website/index.html
index 2b46e883fc..326933e8b0 100644
--- a/website/index.html
+++ b/website/index.html
@@ -49,7 +49,11 @@ <h3 id="introduction">Introduction</h3>
   List&lt;Repo> listRepos(@Path("user") String user);
 }</pre>
             <p>The <code>RestAdapter</code> class generates an implementation of the <code>GitHubService</code> interface.</p>
-            <pre class="prettyprint">GitHubService service = restAdapter.create(GitHubService.class);</pre>
+            <pre class="prettyprint">RestAdapter restAdapter = new RestAdapter.Builder()
+    .setServer("https://api.github.com")
+    .build();
+
+GitHubService service = restAdapter.create(GitHubService.class);</pre>
             <p>Each call on the generated <code>GitHubService</code> makes an HTTP request to the remote webserver.</p>
             <pre class="prettyprint">List&lt;Repo> repos = service.listRepos("octocat");</pre>
             <p>Use annotations to describe the HTTP request:</p>


Test patch:
Repository: square/retrofit
Issue: When developing in Android Studio, Android Platform is detected

I'm using Android Studio for developing retrofit.  When running unit tests, `Platform.Android` is used as `android.os.Build` is in my classpath incidentally.  Is there an alternate way to tell if we are running inside Android? Or could we expose Platform so that folks can use different means to detect the platform without c&p what's in retrofit?


Issue comments:
This will always happen with the current detection method because the platform jar will be on the classpath. I didn't know of anything better at the time. Maybe a system property or something could be used.


Pull request review comments:

Patch that resolved the issue:
diff --git a/retrofit/src/main/java/retrofit/Platform.java b/retrofit/src/main/java/retrofit/Platform.java
index 8721a097df..aeead9acee 100644
--- a/retrofit/src/main/java/retrofit/Platform.java
+++ b/retrofit/src/main/java/retrofit/Platform.java
@@ -43,10 +43,12 @@ static Platform get() {
   private static Platform findPlatform() {
     try {
       Class.forName("android.os.Build");
-      return new Android();
-    } catch (ClassNotFoundException e) {
-      return new Base();
+      if (Build.VERSION.SDK_INT != 0) {
+        return new Android();
+      }
+    } catch (ClassNotFoundException ignored) {
     }
+    return new Base();
   }
 
   abstract Converter defaultConverter();


Test patch:
Repository: square/retrofit
Issue: NPE trying to get the body of a RetrofitError without response

In `RetrofitError` we have

``` java
public Object getBody() {
  TypedInput body = response.getBody();
  if (body == null) {
    return null;
  }
  try {
    return converter.fromBody(body, successType);
  } catch (ConversionException e) {
    throw new RuntimeException(e);
  }
}
```

and something similar for `getBodyAs(Type)`.

if `response` is null, as is the case for a network error for example (`UnknownHostException` etc), an NPE is thrown.

Now I didn't submit a fix yet because maybe you want to explicitly fail in this case ("don't try to get the body of an error without a response"), not sure, but in any case I usually consider NPEs as bug.

I think adding

``` java
if (response == null) {
  return null;
}
```

before would be consistent, but in case where we want to fail we should at least throw a more explicit exception.


Issue comments:
The `null` check is good. Want to submit a PR?

I should be able to do that :p
I'll send a PR later then.

Thanks.

Does it make sense to add tests for this ?
I guess that would just be createError / check accessor results tests, not sure it'd be very very useful.

No, you can skip that.


Pull request review comments:

Patch that resolved the issue:
diff --git a/retrofit/src/main/java/retrofit/RetrofitError.java b/retrofit/src/main/java/retrofit/RetrofitError.java
index 3ee113207d..39438fc26b 100644
--- a/retrofit/src/main/java/retrofit/RetrofitError.java
+++ b/retrofit/src/main/java/retrofit/RetrofitError.java
@@ -77,6 +77,9 @@ public boolean isNetworkError() {
    * the generic type of the supplied {@link Callback} parameter.
    */
   public Object getBody() {
+    if (response == null) {
+      return null;
+    }
     TypedInput body = response.getBody();
     if (body == null) {
       return null;
@@ -90,6 +93,9 @@ public Object getBody() {
 
   /** HTTP response body converted to specified {@code type}. */
   public Object getBodyAs(Type type) {
+    if (response == null) {
+      return null;
+    }
     TypedInput body = response.getBody();
     if (body == null) {
       return null;


Test patch:
Repository: square/retrofit
Issue: @Path/Query exception message update

> URL query string must not have replace block.

Should read something like:

> URL query string must not have replace block. For dynamic query parameters use @Query.

http://stackoverflow.com/q/25658885/132047


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/retrofit/src/main/java/retrofit/RestMethodInfo.java b/retrofit/src/main/java/retrofit/RestMethodInfo.java
index c14b00ff9c..5905f25771 100644
--- a/retrofit/src/main/java/retrofit/RestMethodInfo.java
+++ b/retrofit/src/main/java/retrofit/RestMethodInfo.java
@@ -223,7 +223,8 @@ private void parsePath(String path) {
       // Ensure the query string does not have any named parameters.
       Matcher queryParamMatcher = PARAM_URL_REGEX.matcher(query);
       if (queryParamMatcher.find()) {
-        throw methodError("URL query string \"%s\" must not have replace block.", query);
+        throw methodError("URL query string \"%s\" must not have replace block. For dynamic query"
+            + " parameters use @Query.", query);
       }
     }
 


Test patch:
diff --git a/retrofit/src/test/java/retrofit/RestMethodInfoTest.java b/retrofit/src/test/java/retrofit/RestMethodInfoTest.java
index 1a118eba7c..696fee28f6 100644
--- a/retrofit/src/test/java/retrofit/RestMethodInfoTest.java
+++ b/retrofit/src/test/java/retrofit/RestMethodInfoTest.java
@@ -1307,7 +1307,8 @@ class Example {
       fail();
     } catch (IllegalArgumentException e) {
       assertThat(e).hasMessage(
-          "Example.a: URL query string \"bar={bar}\" must not have replace block.");
+          "Example.a: URL query string \"bar={bar}\" must not have replace block. For dynamic query"
+              + " parameters use @Query.");
     }
   }
Repository: square/retrofit
Issue: Update to Moshi 0.10.0 / 1.0.0

Waiting for its release (October / November), blocks our 2.0.0 release.


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/pom.xml b/pom.xml
index 3c3ecd9fba..5638a73c28 100644
--- a/pom.xml
+++ b/pom.xml
@@ -62,7 +62,7 @@
     <jackson.version>2.4.3</jackson.version>
     <wire.version>1.7.0</wire.version>
     <simplexml.version>2.7.1</simplexml.version>
-    <moshi.version>0.9.0</moshi.version>
+    <moshi.version>1.0.0</moshi.version>
 
     <!-- Test Dependencies -->
     <junit.version>4.12</junit.version>
diff --git a/retrofit-converters/moshi/README.md b/retrofit-converters/moshi/README.md
index 492e426015..9968ccd385 100644
--- a/retrofit-converters/moshi/README.md
+++ b/retrofit-converters/moshi/README.md
@@ -3,8 +3,8 @@ Moshi Converter
 
 A `Converter` which uses [Moshi][1] for serialization to and from JSON.
 
-A default `Moshi` instance will be created or one can be configured and passed to the
-`MoshiConverter` construction to further control the serialization.
+A default `Moshi` instance will be created or one can be configured and passed to
+`MoshiConverterFactory.create()` to further control the serialization.
 
 
  [1]: https://github.com/square/moshi


Test patch:
Repository: square/retrofit
Issue: Fix documentation of @Path encoded parameter

Documentation of `@Path` mentions `encode` parameter but the actual annotation has the `encoded` which has the opposite meaning.

You should also mention that setting `encoded = true` still doesn’t turn of the validation so `/` still gets encoded.


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/retrofit/src/main/java/retrofit2/http/Path.java b/retrofit/src/main/java/retrofit2/http/Path.java
index 22efed8344..feaf34b273 100644
--- a/retrofit/src/main/java/retrofit2/http/Path.java
+++ b/retrofit/src/main/java/retrofit2/http/Path.java
@@ -23,7 +23,7 @@
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
 
 /**
- * Named replacement in the URL path. Values are converted to string using
+ * Named replacement in a URL path segment. Values are converted to string using
  * {@link String#valueOf(Object)} and URL encoded.
  * <p>
  * Simple example:
@@ -33,13 +33,13 @@
  * }</pre>
  * Calling with {@code foo.example(1)} yields {@code /image/1}.
  * <p>
- * Values are URL encoded by default. Disable with {@code encode=false}.
+ * Values are URL encoded by default. Disable with {@code encoded=true}.
  * <pre>{@code
  * &#64;GET("/user/{name}")
  * Call&lt;ResponseBody> encoded(@Path("name") String name);
  *
  * &#64;GET("/user/{name}")
- * Call&lt;ResponseBody> notEncoded(@Path(value="name", encode=false) String name);
+ * Call&lt;ResponseBody> notEncoded(@Path(value="name", encoded=true) String name);
  * }</pre>
  * Calling {@code foo.encoded("John+Doe")} yields {@code /user/John%2BDoe} whereas
  * {@code foo.notEncoded("John+Doe")} yields {@code /user/John+Doe}.


Test patch:
Repository: square/retrofit
Issue: changelog

not 2015，it`s 2016


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d0745e7988..a8fef71ff6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,7 +1,7 @@
 Change Log
 ==========
 
-Version 2.0.0-beta3 *(2015-01-05)*
+Version 2.0.0-beta3 *(2016-01-05)*
 ----------------------------------
 
  * New: All classes have been migrated to the `retrofit2.*` package name. The Maven groupId is now


Test patch:
Repository: square/retrofit
Issue: Update To OkHttp 3.1.2

Update Retrofit to use OkHttp 3.1.2 to address a [Certificate Pinning Vulnerability that was discovered](https://publicobject.com/2016/02/11/okhttp-certificate-pinning-vulnerability/).


Issue comments:
The fix was in 3.1.0, actually. 3.1.2 is a bugfix for something completely
unrelated .

On Sun, Feb 14, 2016, 10:18 PM Brad Leege notifications@github.com wrote:

> Update Retrofit to use OkHttp 3.1.2 to address a Certificate Pinning
> Vulnerability that was discovered
> https://publicobject.com/2016/02/11/okhttp-certificate-pinning-vulnerability/
> .
> 
> —
> Reply to this email directly or view it on GitHub
> https://github.com/square/retrofit/issues/1600.


Pull request review comments:

Patch that resolved the issue:
diff --git a/pom.xml b/pom.xml
index 846b9c6c7a..3b28746998 100644
--- a/pom.xml
+++ b/pom.xml
@@ -49,7 +49,7 @@
 
     <!-- Dependencies -->
     <android.version>4.1.1.4</android.version>
-    <okhttp.version>3.1.1</okhttp.version>
+    <okhttp.version>3.1.2</okhttp.version>
     <animal.sniffer.version>1.14</animal.sniffer.version>
 
     <!-- Adapter Dependencies -->


Test patch:
Repository: square/retrofit
Issue: Call.enqueue with null parameter throws NullPointerException

`code Call.enqueue` should add a null check. If it can't accepts `null`parameter then it should be annotated with `@NotNull`

``` java
java.lang.NullPointerException: Attempt to invoke interface method 'void retrofit2.Callback.onResponse(retrofit2.Call, retrofit2.Response)' on a null object reference
at retrofit2.ExecutorCallAdapterFactory$ExecutorCallbackCall$1$1.run(ExecutorCallAdapterFactory.java:66)
at android.os.Handler.handleCallback(Handler.java:739)
at android.os.Handler.dispatchMessage(Handler.java:95)
at android.os.Looper.loop(Looper.java:145)
at android.app.ActivityThread.main(ActivityThread.java:6872)
at java.lang.reflect.Method.invoke(Native Method)
at java.lang.reflect.Method.invoke(Method.java:372)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1404)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1199)
```


Issue comments:
There are no nullability annotations that are acceptable.


Pull request review comments:

Patch that resolved the issue:
diff --git a/retrofit-mock/src/main/java/retrofit2/mock/BehaviorCall.java b/retrofit-mock/src/main/java/retrofit2/mock/BehaviorCall.java
index ac10878fa9..cfc92c5f79 100644
--- a/retrofit-mock/src/main/java/retrofit2/mock/BehaviorCall.java
+++ b/retrofit-mock/src/main/java/retrofit2/mock/BehaviorCall.java
@@ -52,6 +52,8 @@ final class BehaviorCall<T> implements Call<T> {
   }
 
   @Override public void enqueue(final Callback<T> callback) {
+    if (callback == null) throw new NullPointerException("callback == null");
+
     synchronized (this) {
       if (executed) throw new IllegalStateException("Already executed");
       executed = true;
diff --git a/retrofit/src/main/java/retrofit2/ExecutorCallAdapterFactory.java b/retrofit/src/main/java/retrofit2/ExecutorCallAdapterFactory.java
index 155dc528c5..95ecbf4b7e 100644
--- a/retrofit/src/main/java/retrofit2/ExecutorCallAdapterFactory.java
+++ b/retrofit/src/main/java/retrofit2/ExecutorCallAdapterFactory.java
@@ -55,6 +55,8 @@ static final class ExecutorCallbackCall<T> implements Call<T> {
     }
 
     @Override public void enqueue(final Callback<T> callback) {
+      if (callback == null) throw new NullPointerException("callback == null");
+
       delegate.enqueue(new Callback<T>() {
         @Override public void onResponse(final Call<T> call, final Response<T> response) {
           callbackExecutor.execute(new Runnable() {
diff --git a/retrofit/src/main/java/retrofit2/OkHttpCall.java b/retrofit/src/main/java/retrofit2/OkHttpCall.java
index c793fecd55..5aa102db25 100644
--- a/retrofit/src/main/java/retrofit2/OkHttpCall.java
+++ b/retrofit/src/main/java/retrofit2/OkHttpCall.java
@@ -69,6 +69,8 @@ final class OkHttpCall<T> implements Call<T> {
   }
 
   @Override public void enqueue(final Callback<T> callback) {
+    if (callback == null) throw new NullPointerException("callback == null");
+
     okhttp3.Call call;
     Throwable failure;
 


Test patch: