Refactor backoff logic into separate class

Calculating the backoff delay has been moved into a new BackoffCounter
class. Sleeping the current thread was also abstracted into a Time class
so that unit tests don't actually have to wait for a delay.

Author: gcurtis

build.gradle

@@ -12,6 +12,7 @@ dependencies {
     testCompile 'junit:junit:4.11'
     testCompile 'org.hamcrest:hamcrest-library:1.3'
     testCompile 'com.github.tomakehurst:wiremock:1.47'
+    testCompile 'org.mockito:mockito-core:1.9.5'
 }
 
 tasks.withType(JavaCompile) {

src/main/java/com/box/sdk/BackoffCounter.java

@@ -0,0 +1,59 @@
+package com.box.sdk;
+
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+class BackoffCounter {
+    private static final Logger LOGGER = Logger.getLogger(BackoffCounter.class.getName());
+    private static final int MIN_EXPONENT = 10;
+    private static final int MAX_EXPONENT = 16;
+
+    private final Time time;
+
+    private int maxAttempts;
+    private int attemptsRemaining;
+
+    public BackoffCounter() {
+        this.time = Time.getInstance();
+    }
+
+    public BackoffCounter(Time time) {
+        this.time = time;
+    }
+
+    public int getAttemptsRemaining() {
+        return this.attemptsRemaining;
+    }
+
+    public void waitBackoff() throws InterruptedException {
+        int delay = this.calculateDelay();
+        if (this.attemptsRemaining > 1) {
+            LOGGER.log(Level.WARNING, String.format("Backing off for %d seconds before retrying %d more times.",
+                (delay / 1000), this.attemptsRemaining));
+        } else {
+            LOGGER.log(Level.WARNING, String.format("Backing off for %d seconds before retrying %d more time.",
+                (delay / 1000), this.attemptsRemaining));
+        }
+
+        this.time.waitDuration(delay);
+    }
+
+    public boolean decrement() {
+        this.attemptsRemaining--;
+        return (this.attemptsRemaining > 0);
+    }
+
+    public void reset(int maxAttempts) {
+        this.maxAttempts = maxAttempts;
+        this.attemptsRemaining = maxAttempts;
+    }
+
+    private int calculateDelay() {
+        int exponent = (MIN_EXPONENT + (this.maxAttempts - (this.attemptsRemaining + 1)));
+        if (exponent > MAX_EXPONENT) {
+            exponent = MAX_EXPONENT;
+        }
+
+        return (2 << exponent);
+    }
+}

src/main/java/com/box/sdk/BoxAPIRequest.java

@@ -15,15 +15,14 @@
 import java.util.logging.Logger;
 
 public class BoxAPIRequest {
-    private static final Logger LOGGER = Logger.getLogger(BoxFolder.class.getName());
-    private static final int MAX_BACKOFF_MILLISECONDS = 60000;
-    private static final int INITIAL_BACKOFF_EXPONENT = 10;
+    private static final Logger LOGGER = Logger.getLogger(BoxAPIRequest.class.getName());
 
     private final BoxAPIConnection api;
     private final URL url;
     private final List<RequestHeader> headers;
     private final String method;
 
+    private BackoffCounter backoffCounter;
     private int timeout;
     private InputStream body;
     private long bodyLength;
@@ -38,6 +37,7 @@ public BoxAPIRequest(BoxAPIConnection api, URL url, String method) {
         this.url = url;
         this.method = method;
         this.headers = new ArrayList<RequestHeader>();
+        this.backoffCounter = new BackoffCounter(new Time());
 
         this.addHeader("Accept-Encoding", "gzip");
         this.addHeader("Accept-Charset", "utf-8");
@@ -66,10 +66,35 @@ public void setBody(String body) {
 
     public BoxAPIResponse send() {
         if (this.api == null) {
-            return this.send(BoxAPIConnection.DEFAULT_MAX_ATTEMPTS, INITIAL_BACKOFF_EXPONENT);
+            this.backoffCounter.reset(BoxAPIConnection.DEFAULT_MAX_ATTEMPTS);
         } else {
-            return this.send(this.api.getMaxAttempts(), INITIAL_BACKOFF_EXPONENT);
+            this.backoffCounter.reset(this.api.getMaxAttempts());
         }
+
+        while (this.backoffCounter.getAttemptsRemaining() > 0) {
+            try {
+                return this.trySend();
+            } catch (BoxAPIException apiException) {
+                if (!this.backoffCounter.decrement() || !isResponseRetryable(apiException.getResponseCode())) {
+                    throw apiException;
+                }
+
+                try {
+                    this.resetBody();
+                } catch (IOException ioException) {
+                    throw apiException;
+                }
+
+                try {
+                    this.backoffCounter.waitBackoff();
+                } catch (InterruptedException interruptedException) {
+                    Thread.currentThread().interrupt();
+                    throw apiException;
+                }
+            }
+        }
+
+        throw new RuntimeException();
     }
 
     @Override
@@ -101,6 +126,10 @@ public String toString() {
         return builder.toString();
     }
 
+    void setBackoffCounter(BackoffCounter counter) {
+        this.backoffCounter = counter;
+    }
+
     protected String bodyToString() {
         return null;
     }
@@ -130,46 +159,6 @@ protected void resetBody() throws IOException {
         }
     }
 
-    private BoxAPIResponse send(int maxAttempts, int backoffExp) {
-        BoxAPIResponse response = null;
-        try {
-            response = this.trySend();
-        } catch (BoxAPIException apiException) {
-            int remainingAttempts = (maxAttempts - 1);
-            if (remainingAttempts <= 0 || !isResponseRetryable(apiException.getResponseCode())) {
-                throw apiException;
-            }
-
-            int backoffMilliseconds = 2 << backoffExp;
-            int nextBackoffExp;
-            if (MAX_BACKOFF_MILLISECONDS < backoffMilliseconds) {
-                backoffMilliseconds = MAX_BACKOFF_MILLISECONDS;
-                nextBackoffExp = backoffExp;
-            } else {
-                nextBackoffExp = backoffExp + 1;
-            }
-
-            try {
-                this.resetBody();
-            } catch (IOException ioException) {
-                throw apiException;
-            }
-
-            LOGGER.log(Level.WARNING, String.format("Waiting %d seconds before retrying %d more time(s).",
-                (backoffMilliseconds / 1000), remainingAttempts));
-            try {
-                Thread.sleep(backoffMilliseconds);
-            } catch (InterruptedException interruptedException) {
-                Thread.currentThread().interrupt();
-                throw apiException;
-            }
-
-            this.send(remainingAttempts, nextBackoffExp);
-        }
-
-        return response;
-    }
-
     private BoxAPIResponse trySend() {
         HttpURLConnection connection = this.createConnection();
         if (this.bodyLength > 0) {

src/main/java/com/box/sdk/Time.java

@@ -0,0 +1,18 @@
+package com.box.sdk;
+
+class Time {
+    private static final ThreadLocal<Time> THREAD_LOCAL_INSTANCE = new ThreadLocal<Time>() {
+        @Override
+        protected Time initialValue() {
+            return new Time();
+        }
+    };
+
+    static Time getInstance() {
+        return THREAD_LOCAL_INSTANCE.get();
+    }
+
+    void waitDuration(int milliseconds) throws InterruptedException {
+        Thread.sleep(milliseconds);
+    }
+}

src/test/java/com/box/sdk/BoxAPIRequestTest.java

@@ -5,6 +5,7 @@
 
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
 
 import org.junit.Rule;
 import org.junit.Test;
@@ -18,15 +19,21 @@ public class BoxAPIRequestTest {
     @Rule
     public WireMockRule wireMockRule = new WireMockRule(8080);
 
-    @Test(expected = BoxAPIException.class)
+    @Test
     @Category(UnitTest.class)
     public void requestRetriesTheDefaultNumberOfTimesWhenServerReturns500() throws MalformedURLException {
         stubFor(get(urlEqualTo("/")).willReturn(aResponse().withStatus(500)));
+        Time mockTime = mock(Time.class);
+        BackoffCounter backoffCounter = new BackoffCounter(mockTime);
 
         URL url = new URL("http://localhost:8080/");
         BoxAPIRequest request = new BoxAPIRequest(url, "GET");
-        request.send();
+        request.setBackoffCounter(backoffCounter);
 
-        verify(BoxAPIConnection.DEFAULT_MAX_ATTEMPTS, getRequestedFor(urlEqualTo("/")));
+        try {
+            request.send();
+        } catch (BoxAPIException e) {
+            verify(BoxAPIConnection.DEFAULT_MAX_ATTEMPTS, getRequestedFor(urlEqualTo("/")));
+        }
     }
 }