[Issue 119] User-agent repeatedly added on retry.

http://codereview.appspot.com/6215076/

Author: rmistry

google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java

@@ -782,13 +782,16 @@ public HttpResponse execute() throws IOException {
         logbuf.append(method).append(' ').append(urlString).append(StringUtils.LINE_SEPARATOR);
       }
       // add to user agent
-      if (headers.getUserAgent() == null) {
+      String originalUserAgent = headers.getUserAgent();
+      if (originalUserAgent == null) {
         headers.setUserAgent(USER_AGENT_SUFFIX);
       } else {
-        headers.setUserAgent(headers.getUserAgent() + " " + USER_AGENT_SUFFIX);
+        headers.setUserAgent(originalUserAgent + " " + USER_AGENT_SUFFIX);
       }
       // headers
       HttpHeaders.serializeHeaders(headers, logbuf, logger, lowLevelHttpRequest);
+      // set the original user agent back to the headers so that retries do not keep appending to it
+      headers.setUserAgent(originalUserAgent);
 
       // content
       HttpContent content = this.content;

google-http-client/src/test/java/com/google/api/client/http/HttpRequestTest.java

@@ -24,6 +24,7 @@
 import com.google.api.client.util.Value;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Lists;
 
 import junit.framework.Assert;
 import junit.framework.TestCase;
@@ -314,17 +315,20 @@ static private class FailThenSuccessConnectionErrorTransport extends MockHttpTra
 
     public int lowLevelExecCalls;
     int callsBeforeSuccess;
+    List<String> userAgentHeader = Lists.newArrayList();
 
     protected FailThenSuccessConnectionErrorTransport(int callsBeforeSuccess) {
       this.callsBeforeSuccess = callsBeforeSuccess;
     }
 
-    public LowLevelHttpRequest retryableGetRequest = new MockLowLevelHttpRequest() {
+    public MockLowLevelHttpRequest retryableGetRequest = new MockLowLevelHttpRequest() {
 
       @Override
       public LowLevelHttpResponse execute() throws IOException {
         lowLevelExecCalls++;
 
+        userAgentHeader = getHeaders().get("User-Agent");
+
         if (lowLevelExecCalls <= callsBeforeSuccess) {
           throw new IOException();
         }
@@ -337,6 +341,7 @@ public LowLevelHttpResponse execute() throws IOException {
 
     @Override
     public LowLevelHttpRequest buildGetRequest(String url) {
+      retryableGetRequest.getHeaders().clear();
       return retryableGetRequest;
     }
   }
@@ -389,6 +394,22 @@ public void testExecuteErrorWithRetryDisabled() throws IOException {
     Assert.assertEquals(1, fakeTransport.lowLevelExecCalls);
   }
 
+  public void testUserAgentWithExecuteErrorAndRetryEnabled() throws IOException {
+    int callsBeforeSuccess = 3;
+    FailThenSuccessConnectionErrorTransport fakeTransport =
+        new FailThenSuccessConnectionErrorTransport(callsBeforeSuccess);
+    HttpRequest req =
+        fakeTransport.createRequestFactory().buildGetRequest(new GenericUrl("http://not/used"));
+    req.setRetryOnExecuteIOException(true);
+    req.setNumberOfRetries(callsBeforeSuccess + 1);
+    HttpResponse resp = req.execute();
+
+    Assert.assertEquals(1, fakeTransport.userAgentHeader.size());
+    Assert.assertEquals(HttpRequest.USER_AGENT_SUFFIX, fakeTransport.userAgentHeader.get(0));
+    Assert.assertEquals(200, resp.getStatusCode());
+    Assert.assertEquals(4, fakeTransport.lowLevelExecCalls);
+  }
+
   public void testAbnormalResponseHandlerWithNoBackOff() throws IOException {
     FailThenSuccessBackoffTransport fakeTransport =
         new FailThenSuccessBackoffTransport(HttpStatusCodes.STATUS_CODE_UNAUTHORIZED, 1);