Simplify handling of Retry-After header

acme4j-client/src/main/java/org/shredzone/acme4j/Authorization.java

@@ -178,19 +178,11 @@ public class Authorization extends AcmeResource {
         LOG.debug("update");
         try (Connection conn = getSession().provider().connect()) {
             conn.sendRequest(getLocation(), getSession());
-            int rc = conn.accept(HttpURLConnection.HTTP_OK, HttpURLConnection.HTTP_ACCEPTED);
+            conn.accept(HttpURLConnection.HTTP_OK, HttpURLConnection.HTTP_ACCEPTED);
 
-            JSON result = conn.readJsonResponse();
+            unmarshalAuthorization(conn.readJsonResponse());
-            unmarshalAuthorization(result);
 
-            if (rc == HttpURLConnection.HTTP_ACCEPTED) {
+            conn.handleRetryAfter("authorization is not completed yet");
-                Date retryAfter = conn.getRetryAfterHeader();
-                if (retryAfter != null) {
-                    throw new AcmeRetryAfterException(
-                                    "authorization is not completed yet",
-                                    retryAfter);
-                }
-            }
         }
     }
 

acme4j-client/src/main/java/org/shredzone/acme4j/Certificate.java

@@ -18,7 +18,6 @@ import java.net.URI;
 import java.security.cert.CertificateEncodingException;
 import java.security.cert.X509Certificate;
 import java.util.ArrayList;
-import java.util.Date;
 import java.util.List;
 
 import org.shredzone.acme4j.connector.Connection;
@@ -89,15 +88,8 @@ public class Certificate extends AcmeResource {
             LOG.debug("download");
             try (Connection conn = getSession().provider().connect()) {
                 conn.sendRequest(getLocation(), getSession());
-                int rc = conn.accept(HttpURLConnection.HTTP_OK, HttpURLConnection.HTTP_ACCEPTED);
+                conn.accept(HttpURLConnection.HTTP_OK, HttpURLConnection.HTTP_ACCEPTED);
-                if (rc == HttpURLConnection.HTTP_ACCEPTED) {
+                conn.handleRetryAfter("certificate is not available for download yet");
-                    Date retryAfter = conn.getRetryAfterHeader();
-                    if (retryAfter != null) {
-                        throw new AcmeRetryAfterException(
-                                        "certificate is not available for download yet",
-                                        retryAfter);
-                    }
-                }
 
                 chainCertUri = conn.getLink("up");
                 cert = conn.readCertificate();

acme4j-client/src/main/java/org/shredzone/acme4j/challenge/Challenge.java

@@ -204,17 +204,11 @@ public class Challenge extends AcmeResource {
         LOG.debug("update");
         try (Connection conn = getSession().provider().connect()) {
             conn.sendRequest(getLocation(), getSession());
-            int rc = conn.accept(HttpURLConnection.HTTP_OK, HttpURLConnection.HTTP_ACCEPTED);
+            conn.accept(HttpURLConnection.HTTP_OK, HttpURLConnection.HTTP_ACCEPTED);
 
             unmarshall(conn.readJsonResponse());
 
-            if (rc == HttpURLConnection.HTTP_ACCEPTED) {
+            conn.handleRetryAfter("challenge is not completed yet");
-                Date retryAfter = conn.getRetryAfterHeader();
-                if (retryAfter != null) {
-                    throw new AcmeRetryAfterException("challenge is not completed yet",
-                                    retryAfter);
-                }
-            }
         }
     }
 

acme4j-client/src/main/java/org/shredzone/acme4j/connector/Connection.java

@@ -16,12 +16,12 @@ package org.shredzone.acme4j.connector;
 import java.net.URI;
 import java.security.cert.X509Certificate;
 import java.util.Collection;
-import java.util.Date;
 
 import org.shredzone.acme4j.Session;
 import org.shredzone.acme4j.exception.AcmeException;
-import org.shredzone.acme4j.util.JSONBuilder;
+import org.shredzone.acme4j.exception.AcmeRetryAfterException;
 import org.shredzone.acme4j.util.JSON;
+import org.shredzone.acme4j.util.JSONBuilder;
 
 /**
  * Connects to the ACME server and offers different methods for invoking the API.
@@ -74,6 +74,15 @@ public interface Connection extends AutoCloseable {
      */
     X509Certificate readCertificate() throws AcmeException;
 
+    /**
+     * Throws an {@link AcmeRetryAfterException} if the last status was HTTP Accepted and
+     * a Retry-After header was received.
+     *
+     * @param message
+     *            Message to be sent along with the {@link AcmeRetryAfterException}
+     */
+    void handleRetryAfter(String message) throws AcmeException;
+
     /**
      * Updates a {@link Session} by evaluating the HTTP response header.
      *
@@ -114,13 +123,6 @@ public interface Connection extends AutoCloseable {
      */
     Collection<URI> getLinks(String relation);
 
-    /**
-     * Returns the moment returned in a "Retry-After" header.
-     *
-     * @return Moment, or {@code null} if no "Retry-After" header was set.
-     */
-    Date getRetryAfterHeader();
-
     /**
      * Closes the {@link Connection}, releasing all resources.
      */

acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java

@@ -47,6 +47,7 @@ import org.shredzone.acme4j.exception.AcmeException;
 import org.shredzone.acme4j.exception.AcmeNetworkException;
 import org.shredzone.acme4j.exception.AcmeProtocolException;
 import org.shredzone.acme4j.exception.AcmeRateLimitExceededException;
+import org.shredzone.acme4j.exception.AcmeRetryAfterException;
 import org.shredzone.acme4j.exception.AcmeServerException;
 import org.shredzone.acme4j.exception.AcmeUnauthorizedException;
 import org.shredzone.acme4j.util.JSON;
@@ -228,6 +229,22 @@ public class DefaultConnection implements Connection {
         }
     }
 
+    @Override
+    public void handleRetryAfter(String message) throws AcmeException {
+        assertConnectionIsOpen();
+
+        try {
+            if (conn.getResponseCode() == HttpURLConnection.HTTP_ACCEPTED) {
+                Date retryAfter = getRetryAfterHeader();
+                if (retryAfter != null) {
+                    throw new AcmeRetryAfterException(message, retryAfter);
+                }
+            }
+        } catch (IOException ex) {
+            throw new AcmeNetworkException(ex);
+        }
+    }
+
     @Override
     public void updateSession(Session session) {
         assertConnectionIsOpen();
@@ -296,9 +313,14 @@ public class DefaultConnection implements Connection {
     }
 
     @Override
-    public Date getRetryAfterHeader() {
+    public void close() {
-        assertConnectionIsOpen();
+        conn = null;
+    }
 
+    /**
+     * Gets the instant sent with the Retry-After header.
+     */
+    private Date getRetryAfterHeader() {
         // See RFC 2616 section 14.37
         String header = conn.getHeaderField("Retry-After");
         if (header == null) {
@@ -321,11 +343,6 @@ public class DefaultConnection implements Connection {
         }
     }
 
-    @Override
-    public void close() {
-        conn = null;
-    }
-
     /**
      * Handles a problem by throwing an exception. If a JSON problem was returned, an
      * {@link AcmeServerException} will be thrown. Otherwise a generic

acme4j-client/src/test/java/org/shredzone/acme4j/AuthorizationTest.java

@@ -135,6 +135,11 @@ public class AuthorizationTest {
             public JSON readJsonResponse() {
                 return getJsonAsObject("updateAuthorizationResponse");
             }
+
+            @Override
+            public void handleRetryAfter(String message) throws AcmeException {
+                // Just do nothing
+            }
         };
 
         Session session = provider.createSession();
@@ -189,6 +194,11 @@ public class AuthorizationTest {
             public JSON readJsonResponse() {
                 return getJsonAsObject("updateAuthorizationResponse");
             }
+
+            @Override
+            public void handleRetryAfter(String message) throws AcmeException {
+                // Just do nothing
+            }
         };
 
         Session session = provider.createSession();
@@ -239,8 +249,8 @@ public class AuthorizationTest {
             }
 
             @Override
-            public Date getRetryAfterHeader() {
+            public void handleRetryAfter(String message) throws AcmeException {
-                return new Date(retryAfter);
+                throw new AcmeRetryAfterException(message, new Date(retryAfter));
             }
         };
 

acme4j-client/src/test/java/org/shredzone/acme4j/CertificateTest.java

@@ -76,6 +76,11 @@ public class CertificateTest {
                 return originalCert;
             }
 
+            @Override
+            public void handleRetryAfter(String message) throws AcmeException {
+                // Just do nothing
+            }
+
             @Override
             public URI getLink(String relation) {
                 switch(relation) {
@@ -117,9 +122,10 @@ public class CertificateTest {
                 return HttpURLConnection.HTTP_ACCEPTED;
             }
 
+
             @Override
-            public Date getRetryAfterHeader() {
+            public void handleRetryAfter(String message) throws AcmeException {
-                return new Date(retryAfter);
+                throw new AcmeRetryAfterException(message, new Date(retryAfter));
             }
         };
 

acme4j-client/src/test/java/org/shredzone/acme4j/challenge/ChallengeTest.java

@@ -204,6 +204,11 @@ public class ChallengeTest {
             public JSON readJsonResponse() {
                 return getJsonAsObject("updateHttpChallengeResponse");
             }
+
+            @Override
+            public void handleRetryAfter(String message) throws AcmeException {
+                // Just do nothing
+            }
         };
 
         Session session = provider.createSession();
@@ -244,9 +249,10 @@ public class ChallengeTest {
                 return getJsonAsObject("updateHttpChallengeResponse");
             }
 
+
             @Override
-            public Date getRetryAfterHeader() {
+            public void handleRetryAfter(String message) throws AcmeException {
-                return new Date(retryAfter);
+                throw new AcmeRetryAfterException(message, new Date(retryAfter));
             }
         };
 

acme4j-client/src/test/java/org/shredzone/acme4j/connector/DefaultConnectionTest.java

@@ -43,6 +43,7 @@ import org.shredzone.acme4j.Session;
 import org.shredzone.acme4j.exception.AcmeException;
 import org.shredzone.acme4j.exception.AcmeNetworkException;
 import org.shredzone.acme4j.exception.AcmeProtocolException;
+import org.shredzone.acme4j.exception.AcmeRetryAfterException;
 import org.shredzone.acme4j.exception.AcmeServerException;
 import org.shredzone.acme4j.util.JSON;
 import org.shredzone.acme4j.util.JSONBuilder;
@@ -252,17 +253,24 @@ public class DefaultConnectionTest {
      * Test if Retry-After header with absolute date is correctly parsed.
      */
     @Test
-    public void testGetRetryAfterHeaderDate() {
+    public void testHandleRetryAfterHeaderDate() throws AcmeException, IOException {
         Date retryDate = new Date(System.currentTimeMillis() + 10 * 60 * 60 * 1000L);
+        String retryMsg = "absolute date";
 
+        when(mockUrlConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_ACCEPTED);
         when(mockUrlConnection.getHeaderField("Retry-After")).thenReturn(retryDate.toString());
         when(mockUrlConnection.getHeaderFieldDate("Retry-After", 0L)).thenReturn(retryDate.getTime());
 
         try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) {
             conn.conn = mockUrlConnection;
-            assertThat(conn.getRetryAfterHeader(), is(retryDate));
+            conn.handleRetryAfter(retryMsg);
+            fail("no AcmeRetryAfterException was thrown");
+        } catch (AcmeRetryAfterException ex) {
+            assertThat(ex.getRetryAfter(), is(retryDate));
+            assertThat(ex.getMessage(), is(retryMsg));
         }
 
+        verify(mockUrlConnection, atLeastOnce()).getResponseCode();
         verify(mockUrlConnection, atLeastOnce()).getHeaderField("Retry-After");
     }
 
@@ -270,10 +278,13 @@ public class DefaultConnectionTest {
      * Test if Retry-After header with relative timespan is correctly parsed.
      */
     @Test
-    public void testGetRetryAfterHeaderDelta() {
+    public void testHandleRetryAfterHeaderDelta() throws AcmeException, IOException {
         int delta = 10 * 60 * 60;
         long now = System.currentTimeMillis();
+        String retryMsg = "relative time";
 
+        when(mockUrlConnection.getResponseCode())
+                .thenReturn(HttpURLConnection.HTTP_ACCEPTED);
         when(mockUrlConnection.getHeaderField("Retry-After"))
                 .thenReturn(String.valueOf(delta));
         when(mockUrlConnection.getHeaderFieldDate(
@@ -283,9 +294,14 @@ public class DefaultConnectionTest {
 
         try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) {
             conn.conn = mockUrlConnection;
-            assertThat(conn.getRetryAfterHeader(), is(new Date(now + delta * 1000L)));
+            conn.handleRetryAfter(retryMsg);
+            fail("no AcmeRetryAfterException was thrown");
+        } catch (AcmeRetryAfterException ex) {
+            assertThat(ex.getRetryAfter(), is(new Date(now + delta * 1000L)));
+            assertThat(ex.getMessage(), is(retryMsg));
         }
 
+        verify(mockUrlConnection, atLeastOnce()).getResponseCode();
         verify(mockUrlConnection, atLeastOnce()).getHeaderField("Retry-After");
     }
 
@@ -293,18 +309,40 @@ public class DefaultConnectionTest {
      * Test if no Retry-After header is correctly handled.
      */
     @Test
-    public void testGetRetryAfterHeaderNull() {
+    public void testHandleRetryAfterHeaderNull() throws AcmeException, IOException {
+        when(mockUrlConnection.getResponseCode())
+                .thenReturn(HttpURLConnection.HTTP_ACCEPTED);
         when(mockUrlConnection.getHeaderField("Retry-After"))
                 .thenReturn(null);
 
         try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) {
             conn.conn = mockUrlConnection;
-            assertThat(conn.getRetryAfterHeader(), is(nullValue()));
+            conn.handleRetryAfter("no header");
+        } catch (AcmeRetryAfterException ex) {
+            fail("an AcmeRetryAfterException was thrown");
         }
 
+        verify(mockUrlConnection, atLeastOnce()).getResponseCode();
         verify(mockUrlConnection, atLeastOnce()).getHeaderField("Retry-After");
     }
 
+    /**
+     * Test if no HTTP_ACCEPTED status is correctly handled.
+     */
+    @Test
+    public void testHandleRetryAfterNotAccepted() throws AcmeException, IOException {
+        when(mockUrlConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_OK);
+
+        try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) {
+            conn.conn = mockUrlConnection;
+            conn.handleRetryAfter("http ok");
+        } catch (AcmeRetryAfterException ex) {
+            fail("an AcmeRetryAfterException was thrown");
+        }
+
+        verify(mockUrlConnection, atLeastOnce()).getResponseCode();
+    }
+
     /**
      * Test if an {@link AcmeServerException} is thrown on an acme problem.
      */

acme4j-client/src/test/java/org/shredzone/acme4j/connector/DummyConnection.java

@@ -16,12 +16,11 @@ package org.shredzone.acme4j.connector;
 import java.net.URI;
 import java.security.cert.X509Certificate;
 import java.util.Collection;
-import java.util.Date;
 
 import org.shredzone.acme4j.Session;
 import org.shredzone.acme4j.exception.AcmeException;
-import org.shredzone.acme4j.util.JSONBuilder;
 import org.shredzone.acme4j.util.JSON;
+import org.shredzone.acme4j.util.JSONBuilder;
 
 /**
  * Dummy implementation of {@link Connection} that always fails. Single methods are
@@ -54,6 +53,11 @@ public class DummyConnection implements Connection {
         throw new UnsupportedOperationException();
     }
 
+    @Override
+    public void handleRetryAfter(String message) throws AcmeException {
+        throw new UnsupportedOperationException();
+    }
+
     @Override
     public void updateSession(Session session) {
         throw new UnsupportedOperationException();
@@ -74,11 +78,6 @@ public class DummyConnection implements Connection {
         throw new UnsupportedOperationException();
     }
 
-    @Override
-    public Date getRetryAfterHeader() {
-        throw new UnsupportedOperationException();
-    }
-
     @Override
     public void close() {
         // closing is always safe