diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeJsonResource.java b/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeJsonResource.java index 48300fc4..5a589034 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeJsonResource.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeJsonResource.java @@ -14,7 +14,9 @@ package org.shredzone.acme4j; import java.net.URL; +import java.time.Instant; import java.util.Objects; +import java.util.Optional; import edu.umd.cs.findbugs.annotations.Nullable; import org.shredzone.acme4j.exception.AcmeException; @@ -34,6 +36,7 @@ public abstract class AcmeJsonResource extends AcmeResource { private static final Logger LOG = LoggerFactory.getLogger(AcmeJsonResource.class); private @Nullable JSON data = null; + private @Nullable Instant retryAfter = null; /** * Create a new {@link AcmeJsonResource}. @@ -62,10 +65,7 @@ public abstract class AcmeJsonResource extends AcmeResource { public JSON getJSON() { if (data == null) { try { - update(); - } catch (AcmeRetryAfterException ex) { - // ignore... The object was still updated. - LOG.debug("Retry-After", ex); + fetch(); } catch (AcmeException ex) { throw new AcmeLazyLoadingException(this, ex); } @@ -88,7 +88,7 @@ public abstract class AcmeJsonResource extends AcmeResource { * Checks if this resource is valid. * * @return {@code true} if the resource state has been loaded from the server. If - * {@code false}, {@link #getJSON()} would implicitly call {@link #update()} + * {@code false}, {@link #getJSON()} would implicitly call {@link #fetch()} * to fetch the current state from the server. */ protected boolean isValid() { @@ -96,7 +96,7 @@ public abstract class AcmeJsonResource extends AcmeResource { } /** - * Invalidates the state of this resource. Enforces an {@link #update()} when + * Invalidates the state of this resource. Enforces a {@link #fetch()} when * {@link #getJSON()} is invoked. *

* Subclasses can override this method to purge internal caches that are based on the @@ -104,28 +104,78 @@ public abstract class AcmeJsonResource extends AcmeResource { */ protected void invalidate() { data = null; + retryAfter = null; + } + + /** + * Updates this resource, by fetching the current resource data from the server. + *

+ * Note: Prefer to use {@link #fetch()} instead. It is working the same way, but + * returns the Retry-After instant instead of throwing an exception. This method will + * become deprecated in a future release. + * + * @throws AcmeException + * if the resource could not be fetched. + * @throws AcmeRetryAfterException + * the resource is still being processed, and the server returned an estimated + * date when the process will be completed. If you are polling for the + * resource to complete, you should wait for the date given in + * {@link AcmeRetryAfterException#getRetryAfter()}. Note that the status of + * the resource is updated even if this exception was thrown. + * @see #fetch() + */ + public void update() throws AcmeException { + var retryAfter = fetch(); + if (retryAfter.isPresent()) { + throw new AcmeRetryAfterException(getClass().getSimpleName() + " is not completed yet", retryAfter.get()); + } } /** * Updates this resource, by fetching the current resource data from the server. * + * @return An {@link Optional} estimation when the resource status will change. If you + * are polling for the resource to complete, you should wait for the given instant + * before trying again. Empty if the server did not return a "Retry-After" header. * @throws AcmeException - * if the resource could not be fetched. - * @throws AcmeRetryAfterException - * the resource is still being processed, and the server returned an - * estimated date when the process will be completed. If you are polling - * for the resource to complete, you should wait for the date given in - * {@link AcmeRetryAfterException#getRetryAfter()}. Note that the status - * of the resource is updated even if this exception was thrown. + * if the resource could not be fetched. + * @see #update() + * @since 3.2.0 */ - public void update() throws AcmeException { + public Optional fetch() throws AcmeException { var resourceType = getClass().getSimpleName(); LOG.debug("update {}", resourceType); try (var conn = getSession().connect()) { conn.sendSignedPostAsGetRequest(getLocation(), getLogin()); setJSON(conn.readJsonResponse()); - conn.handleRetryAfter(resourceType + " is not completed yet"); + var retryAfterOpt = conn.getRetryAfter(); + retryAfterOpt.ifPresent(instant -> LOG.debug("Retry-After: {}", instant)); + setRetryAfter(retryAfterOpt.orElse(null)); + return retryAfterOpt; } } + /** + * Sets a Retry-After instant. + * + * @since 3.2.0 + */ + protected void setRetryAfter(@Nullable Instant retryAfter) { + this.retryAfter = retryAfter; + } + + /** + * Gets an estimation when the resource status will change. If you are polling for + * the resource to complete, you should wait for the given instant before trying + * a status refresh. + *

+ * This instant was sent with the Retry-After header at the last update. + * + * @return Retry-after {@link Instant}, or empty if there was no such header. + * @since 3.2.0 + */ + public Optional getRetryAfter() { + return Optional.ofNullable(retryAfter); + } + } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/RenewalInfo.java b/acme4j-client/src/main/java/org/shredzone/acme4j/RenewalInfo.java index 721baf16..378b1c02 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/RenewalInfo.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/RenewalInfo.java @@ -38,8 +38,6 @@ import org.slf4j.LoggerFactory; public class RenewalInfo extends AcmeJsonResource { private static final Logger LOG = LoggerFactory.getLogger(RenewalInfo.class); - private @Nullable Instant recheckAfter = null; - protected RenewalInfo(Login login, URL location) { super(login, location); } @@ -68,15 +66,6 @@ public class RenewalInfo extends AcmeJsonResource { return getJSON().get("explanationURL").optional().map(Value::asURL); } - /** - * An optional {@link Instant} that serves as recommendation when to re-check the - * renewal information of a certificate. - */ - public Optional getRecheckAfter() { - getJSON(); // make sure resource is loaded - return Optional.ofNullable(recheckAfter); - } - /** * Checks if the given {@link Instant} is before the suggested time window, so a * certificate renewal is not required yet. @@ -175,16 +164,6 @@ public class RenewalInfo extends AcmeJsonResource { end.toEpochMilli()))); } - @Override - public void update() throws AcmeException { - LOG.debug("update RenewalInfo"); - try (Connection conn = getSession().connect()) { - conn.sendRequest(getLocation(), getSession(), null); - setJSON(conn.readJsonResponse()); - recheckAfter = conn.getRetryAfter().orElse(null); - } - } - /** * Asserts that the end of the suggested time window is after the start. */ @@ -194,4 +173,17 @@ public class RenewalInfo extends AcmeJsonResource { } } + @Override + public Optional fetch() throws AcmeException { + LOG.debug("update RenewalInfo"); + try (Connection conn = getSession().connect()) { + conn.sendRequest(getLocation(), getSession(), null); + setJSON(conn.readJsonResponse()); + var retryAfterOpt = conn.getRetryAfter(); + retryAfterOpt.ifPresent(instant -> LOG.debug("Retry-After: {}", instant)); + setRetryAfter(retryAfterOpt.orElse(null)); + return retryAfterOpt; + } + } + } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/Connection.java b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/Connection.java index 10b1b032..8299b0e1 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/Connection.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/Connection.java @@ -162,8 +162,15 @@ public interface Connection extends AutoCloseable { * * @param message * Message to be sent along with the {@link AcmeRetryAfterException} + * @deprecated Prefer to use {@link #getRetryAfter()}. */ - void handleRetryAfter(String message) throws AcmeException; + @Deprecated + default void handleRetryAfter(String message) throws AcmeException { + var retryAfter = getRetryAfter(); + if (retryAfter.isPresent()) { + throw new AcmeRetryAfterException(message, retryAfter.get()); + } + } /** * Gets the nonce from the nonce header. diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java index 57b14087..8e4fdcdc 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java @@ -51,7 +51,6 @@ import org.shredzone.acme4j.exception.AcmeException; import org.shredzone.acme4j.exception.AcmeNetworkException; import org.shredzone.acme4j.exception.AcmeProtocolException; import org.shredzone.acme4j.exception.AcmeRateLimitedException; -import org.shredzone.acme4j.exception.AcmeRetryAfterException; import org.shredzone.acme4j.exception.AcmeServerException; import org.shredzone.acme4j.exception.AcmeUnauthorizedException; import org.shredzone.acme4j.exception.AcmeUserActionRequiredException; @@ -231,14 +230,6 @@ public class DefaultConnection implements Connection { } } - @Override - public void handleRetryAfter(String message) throws AcmeException { - var retryAfter = getRetryAfter(); - if (retryAfter.isPresent()) { - throw new AcmeRetryAfterException(message, retryAfter.get()); - } - } - @Override public Optional getNonce() { var nonceHeaderOpt = getResponse().headers() diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/exception/AcmeRetryAfterException.java b/acme4j-client/src/main/java/org/shredzone/acme4j/exception/AcmeRetryAfterException.java index b805d603..fd7f8931 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/exception/AcmeRetryAfterException.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/exception/AcmeRetryAfterException.java @@ -16,9 +16,15 @@ package org.shredzone.acme4j.exception; import java.time.Instant; import java.util.Objects; +import org.shredzone.acme4j.AcmeJsonResource; + /** * A server side process has not been completed yet. The server also provides an estimate * of when the process is expected to complete. + *

+ * Note: Prefer to use {@link AcmeJsonResource#fetch()}. Invoking + * {@link AcmeJsonResource#update()} and catching this exception is unnecessary + * complicated and a legacy from acme4j v2 which will disappear in a future release. */ public class AcmeRetryAfterException extends AcmeException { private static final long serialVersionUID = 4461979121063649905L; diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/AccountBuilderTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/AccountBuilderTest.java index a7cd2fd8..8a52cbe1 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/AccountBuilderTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/AccountBuilderTest.java @@ -155,6 +155,7 @@ public class AccountBuilderTest { }; provider.putTestResource(Resource.NEW_ACCOUNT, resourceUrl); + provider.putMetadata("externalAccountRequired", true); var builder = new AccountBuilder(); builder.useKeyPair(accountKey); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/AccountTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/AccountTest.java index 1c2854da..c36640fc 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/AccountTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/AccountTest.java @@ -96,11 +96,6 @@ public class AccountTest { public Collection getLinks(String relation) { return emptyList(); } - - @Override - public void handleRetryAfter(String message) { - // do nothing - } }; var login = provider.createLogin(); @@ -156,11 +151,6 @@ public class AccountTest { default: return emptyList(); } } - - @Override - public void handleRetryAfter(String message) { - // do nothing - } }; var account = new Account(provider.createLogin()); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/AcmeJsonResourceTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/AcmeJsonResourceTest.java index de4934c7..fd534d96 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/AcmeJsonResourceTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/AcmeJsonResourceTest.java @@ -18,8 +18,12 @@ import static org.shredzone.acme4j.toolbox.TestUtils.getJSON; import static org.shredzone.acme4j.toolbox.TestUtils.url; import java.net.URL; +import java.time.Instant; +import java.util.Optional; +import edu.umd.cs.findbugs.annotations.Nullable; import org.junit.jupiter.api.Test; +import org.shredzone.acme4j.exception.AcmeException; import org.shredzone.acme4j.toolbox.JSON; import org.shredzone.acme4j.toolbox.TestUtils; @@ -43,6 +47,7 @@ public class AcmeJsonResourceTest { assertThat(resource.getSession()).isEqualTo(login.getSession()); assertThat(resource.getLocation()).isEqualTo(LOCATION_URL); assertThat(resource.isValid()).isFalse(); + assertThat(resource.getRetryAfter()).isEmpty(); assertUpdateInvoked(resource, 0); assertThat(resource.getJSON()).isEqualTo(JSON_DATA); @@ -74,6 +79,25 @@ public class AcmeJsonResourceTest { assertUpdateInvoked(resource, 0); } + /** + * Test Retry-After + */ + @Test + public void testRetryAfter() { + var login = TestUtils.login(); + var retryAfter = Instant.now().plusSeconds(30L); + var jsonData = getJSON("requestOrderResponse"); + + var resource = new DummyJsonResource(login, LOCATION_URL, jsonData, retryAfter); + assertThat(resource.isValid()).isTrue(); + assertThat(resource.getJSON()).isEqualTo(jsonData); + assertThat(resource.getRetryAfter()).hasValue(retryAfter); + assertUpdateInvoked(resource, 0); + + resource.setRetryAfter(null); + assertThat(resource.getRetryAfter()).isEmpty(); + } + /** * Test {@link AcmeJsonResource#invalidate()}. */ @@ -124,17 +148,19 @@ public class AcmeJsonResourceTest { super(login, location); } - public DummyJsonResource(Login login, URL location, JSON json) { + public DummyJsonResource(Login login, URL location, JSON json, @Nullable Instant retryAfter) { super(login, location); setJSON(json); + setRetryAfter(retryAfter); } @Override - public void update() { - // update() is tested individually in all AcmeJsonResource subclasses. + public Optional fetch() throws AcmeException { + // fetch() is tested individually in all AcmeJsonResource subclasses. // Here we just simulate the update, by setting a JSON. updateCount++; setJSON(JSON_DATA); + return Optional.empty(); } } diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/AuthorizationTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/AuthorizationTest.java index f4aa4e95..9b35ccd5 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/AuthorizationTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/AuthorizationTest.java @@ -25,6 +25,7 @@ import java.net.URL; import java.time.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; +import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; import org.junit.jupiter.api.Test; @@ -32,9 +33,7 @@ import org.shredzone.acme4j.challenge.Challenge; import org.shredzone.acme4j.challenge.Dns01Challenge; import org.shredzone.acme4j.challenge.Http01Challenge; import org.shredzone.acme4j.challenge.TlsAlpn01Challenge; -import org.shredzone.acme4j.exception.AcmeException; import org.shredzone.acme4j.exception.AcmeProtocolException; -import org.shredzone.acme4j.exception.AcmeRetryAfterException; import org.shredzone.acme4j.provider.TestableConnectionProvider; import org.shredzone.acme4j.toolbox.JSON; import org.shredzone.acme4j.toolbox.JSONBuilder; @@ -128,11 +127,6 @@ public class AuthorizationTest { public JSON readJsonResponse() { return getJSON("updateAuthorizationResponse"); } - - @Override - public void handleRetryAfter(String message) { - // Just do nothing - } }; var login = provider.createLogin(); @@ -174,11 +168,6 @@ public class AuthorizationTest { public JSON readJsonResponse() { return getJSON("updateAuthorizationWildcardResponse"); } - - @Override - public void handleRetryAfter(String message) { - // Just do nothing - } }; var login = provider.createLogin(); @@ -219,11 +208,6 @@ public class AuthorizationTest { public JSON readJsonResponse() { return getJSON("updateAuthorizationResponse"); } - - @Override - public void handleRetryAfter(String message) { - // Just do nothing - } }; var login = provider.createLogin(); @@ -270,8 +254,8 @@ public class AuthorizationTest { } @Override - public void handleRetryAfter(String message) throws AcmeException { - throw new AcmeRetryAfterException(message, retryAfter); + public Optional getRetryAfter() { + return Optional.of(retryAfter); } }; @@ -282,8 +266,8 @@ public class AuthorizationTest { provider.putTestChallenge("tls-alpn-01", TlsAlpn01Challenge::new); var auth = new Authorization(login, locationUrl); - var ex = assertThrows(AcmeRetryAfterException.class, auth::update); - assertThat(ex.getRetryAfter()).isEqualTo(retryAfter); + var returnedRetryAfter = auth.fetch(); + assertThat(returnedRetryAfter).hasValue(retryAfter); assertThat(auth.getIdentifier().getDomain()).isEqualTo("example.org"); assertThat(auth.getStatus()).isEqualTo(Status.VALID); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/CertificateTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/CertificateTest.java index de794548..23df6e93 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/CertificateTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/CertificateTest.java @@ -340,13 +340,11 @@ public class CertificateTest { assertThat(cert.getCertID()).isEqualTo("MFgwCwYJYIZIAWUDBAIBBCCeWLRusNLb--vmWOkxm34qDjTMWkc3utIhOMoMwKDqbgQg2iiKWySZrD-6c88HMZ6vhIHZPamChLlzGHeZ7pTS8jYCBQCHZUMh"); assertThat(cert.hasRenewalInfo()).isTrue(); assertThat(cert.getRenewalInfoLocation()) - .isNotEmpty() - .contains(certResourceUrl); + .hasValue(certResourceUrl); var renewalInfo = cert.getRenewalInfo(); - assertThat(renewalInfo.getRecheckAfter()) - .isNotEmpty() - .contains(retryAfterInstant); + assertThat(renewalInfo.getRetryAfter()) + .isEmpty(); assertThat(renewalInfo.getSuggestedWindowStart()) .isEqualTo("2021-01-03T00:00:00Z"); assertThat(renewalInfo.getSuggestedWindowEnd()) @@ -355,6 +353,9 @@ public class CertificateTest { .isNotEmpty() .contains(url("https://example.com/docs/example-mass-reissuance-event")); + assertThat(renewalInfo.fetch()).hasValue(retryAfterInstant); + assertThat(renewalInfo.getRetryAfter()).hasValue(retryAfterInstant); + provider.close(); } @@ -404,9 +405,7 @@ public class CertificateTest { var cert = new Certificate(provider.createLogin(), locationUrl); assertThat(cert.hasRenewalInfo()).isTrue(); - assertThat(cert.getRenewalInfoLocation()) - .isNotEmpty() - .contains(certResourceUrl); + assertThat(cert.getRenewalInfoLocation()).hasValue(certResourceUrl); provider.close(); } diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/OrderTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/OrderTest.java index 76d8e488..ca4962cc 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/OrderTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/OrderTest.java @@ -56,11 +56,6 @@ public class OrderTest { public JSON readJsonResponse() { return getJSON("updateOrderResponse"); } - - @Override - public void handleRetryAfter(String message) { - assertThat(message).isNotNull(); - } }; var login = provider.createLogin(); @@ -133,11 +128,6 @@ public class OrderTest { public JSON readJsonResponse() { return getJSON("updateOrderResponse"); } - - @Override - public void handleRetryAfter(String message) { - assertThat(message).isNotNull(); - } }; var login = provider.createLogin(); @@ -192,11 +182,6 @@ public class OrderTest { public JSON readJsonResponse() { return getJSON(isFinalized ? "finalizeResponse" : "updateOrderResponse"); } - - @Override - public void handleRetryAfter(String message) { - assertThat(message).isNotNull(); - } }; var login = provider.createLogin(); @@ -250,11 +235,6 @@ public class OrderTest { public JSON readJsonResponse() { return getJSON("updateAutoRenewOrderResponse"); } - - @Override - public void handleRetryAfter(String message) { - assertThat(message).isNotNull(); - } }; provider.putMetadata("auto-renewal", JSON.empty()); @@ -298,11 +278,6 @@ public class OrderTest { public JSON readJsonResponse() { return getJSON("finalizeAutoRenewResponse"); } - - @Override - public void handleRetryAfter(String message) { - assertThat(message).isNotNull(); - } }; var login = provider.createLogin(); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/RenewalInfoTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/RenewalInfoTest.java index d64adfd8..c4b31d56 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/RenewalInfoTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/RenewalInfoTest.java @@ -68,14 +68,12 @@ public class RenewalInfoTest { var login = provider.createLogin(); var renewalInfo = new RenewalInfo(login, locationUrl); - renewalInfo.update(); + var recheckAfter = renewalInfo.fetch(); + assertThat(recheckAfter).hasValue(retryAfterInstant); // Check getters try (var softly = new AutoCloseableSoftAssertions()) { softly.assertThat(renewalInfo.getLocation()).isEqualTo(locationUrl); - softly.assertThat(renewalInfo.getRecheckAfter()) - .isNotEmpty() - .contains(retryAfterInstant); softly.assertThat(renewalInfo.getSuggestedWindowStart()) .isEqualTo(startWindow); softly.assertThat(renewalInfo.getSuggestedWindowEnd()) diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/ChallengeTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/ChallengeTest.java index 37662511..512efd48 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/ChallengeTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/challenge/ChallengeTest.java @@ -26,14 +26,13 @@ import java.net.URL; import java.time.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; +import java.util.Optional; import org.assertj.core.api.AutoCloseableSoftAssertions; import org.junit.jupiter.api.Test; import org.shredzone.acme4j.Login; import org.shredzone.acme4j.Status; -import org.shredzone.acme4j.exception.AcmeException; import org.shredzone.acme4j.exception.AcmeProtocolException; -import org.shredzone.acme4j.exception.AcmeRetryAfterException; import org.shredzone.acme4j.provider.TestableConnectionProvider; import org.shredzone.acme4j.toolbox.JSON; import org.shredzone.acme4j.toolbox.JSONBuilder; @@ -141,11 +140,6 @@ public class ChallengeTest { public JSON readJsonResponse() { return getJSON("updateHttpChallengeResponse"); } - - @Override - public void handleRetryAfter(String message) { - // Just do nothing - } }; var login = provider.createLogin(); @@ -179,17 +173,17 @@ public class ChallengeTest { return getJSON("updateHttpChallengeResponse"); } - @Override - public void handleRetryAfter(String message) throws AcmeException { - throw new AcmeRetryAfterException(message, retryAfter); + public Optional getRetryAfter() { + return Optional.of(retryAfter); } }; var login = provider.createLogin(); var challenge = new Http01Challenge(login, getJSON("triggerHttpChallengeResponse")); - assertThrows(AcmeRetryAfterException.class, challenge::update); + var returnedRetryAfter = challenge.fetch(); + assertThat(returnedRetryAfter).hasValue(retryAfter); assertThat(challenge.getStatus()).isEqualTo(Status.VALID); assertThat(challenge.getLocation()).isEqualTo(locationUrl); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DefaultConnectionTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DefaultConnectionTest.java index 8ea4f69b..57659158 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DefaultConnectionTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DefaultConnectionTest.java @@ -52,7 +52,6 @@ import org.shredzone.acme4j.Session; import org.shredzone.acme4j.exception.AcmeException; import org.shredzone.acme4j.exception.AcmeProtocolException; import org.shredzone.acme4j.exception.AcmeRateLimitedException; -import org.shredzone.acme4j.exception.AcmeRetryAfterException; import org.shredzone.acme4j.exception.AcmeServerException; import org.shredzone.acme4j.exception.AcmeUnauthorizedException; import org.shredzone.acme4j.exception.AcmeUserActionRequiredException; @@ -310,30 +309,24 @@ public class DefaultConnectionTest { * Test if Retry-After header with absolute date is correctly parsed. */ @Test - public void testHandleRetryAfterHeaderDate() { + public void testHandleRetryAfterHeaderDate() throws AcmeException { var retryDate = Instant.now().plus(Duration.ofHours(10)).truncatedTo(SECONDS); - var retryMsg = "absolute date"; stubFor(get(urlEqualTo(REQUEST_PATH)).willReturn(ok() .withHeader("Retry-After", DATE_FORMATTER.format(retryDate)) )); - var ex = assertThrows(AcmeRetryAfterException.class, () -> { - try (var conn = session.connect()) { - conn.sendRequest(requestUrl, session, null); - conn.handleRetryAfter(retryMsg); - } - }); - - assertThat(ex.getRetryAfter()).isEqualTo(retryDate); - assertThat(ex.getMessage()).isEqualTo(retryMsg); + try (var conn = session.connect()) { + conn.sendRequest(requestUrl, session, null); + assertThat(conn.getRetryAfter()).hasValue(retryDate); + } } /** * Test if Retry-After header with relative timespan is correctly parsed. */ @Test - public void testHandleRetryAfterHeaderDelta() { + public void testHandleRetryAfterHeaderDelta() throws AcmeException { var delta = 10 * 60 * 60; var now = Instant.now().truncatedTo(SECONDS); var retryMsg = "relative time"; @@ -343,15 +336,10 @@ public class DefaultConnectionTest { .withHeader("Date", DATE_FORMATTER.format(now)) )); - var ex = assertThrows(AcmeRetryAfterException.class, () -> { - try (var conn = session.connect()) { - conn.sendRequest(requestUrl, session, null); - conn.handleRetryAfter(retryMsg); - } - }); - - assertThat(ex.getRetryAfter()).isEqualTo(now.plusSeconds(delta)); - assertThat(ex.getMessage()).isEqualTo(retryMsg); + try (var conn = session.connect()) { + conn.sendRequest(requestUrl, session, null); + assertThat(conn.getRetryAfter()).hasValue(now.plusSeconds(delta)); + } } /** @@ -365,7 +353,7 @@ public class DefaultConnectionTest { try (var conn = session.connect()) { conn.sendRequest(requestUrl, session, null); - conn.handleRetryAfter("no header"); + assertThat(conn.getRetryAfter()).isEmpty(); } verify(getRequestedFor(urlEqualTo(REQUEST_PATH))); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DummyConnection.java b/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DummyConnection.java index 299758ad..9c206212 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DummyConnection.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DummyConnection.java @@ -80,11 +80,6 @@ public class DummyConnection implements Connection { throw new UnsupportedOperationException(); } - @Override - public void handleRetryAfter(String message) throws AcmeException { - throw new UnsupportedOperationException(); - } - @Override public Optional getNonce() { throw new UnsupportedOperationException(); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/provider/TestableConnectionProvider.java b/acme4j-client/src/test/java/org/shredzone/acme4j/provider/TestableConnectionProvider.java index 1203e009..505e92c7 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/provider/TestableConnectionProvider.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/provider/TestableConnectionProvider.java @@ -16,8 +16,10 @@ package org.shredzone.acme4j.provider; import java.io.IOException; import java.net.URI; import java.net.URL; +import java.time.Instant; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import java.util.function.BiFunction; import org.shredzone.acme4j.Login; @@ -109,6 +111,11 @@ public class TestableConnectionProvider extends DummyConnection implements AcmeP return session.login(new URL(TestUtils.ACCOUNT_URL), TestUtils.createKeyPair()); } + @Override + public Optional getRetryAfter() { + return Optional.empty(); + } + @Override public boolean accepts(URI serverUri) { throw new UnsupportedOperationException(); diff --git a/acme4j-example/src/main/java/org/shredzone/acme4j/example/ClientTest.java b/acme4j-example/src/main/java/org/shredzone/acme4j/example/ClientTest.java index 0e32ec5a..14a40e2c 100644 --- a/acme4j-example/src/main/java/org/shredzone/acme4j/example/ClientTest.java +++ b/acme4j-example/src/main/java/org/shredzone/acme4j/example/ClientTest.java @@ -45,7 +45,6 @@ import org.shredzone.acme4j.challenge.Challenge; import org.shredzone.acme4j.challenge.Dns01Challenge; import org.shredzone.acme4j.challenge.Http01Challenge; import org.shredzone.acme4j.exception.AcmeException; -import org.shredzone.acme4j.exception.AcmeRetryAfterException; import org.shredzone.acme4j.util.KeyPairUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -142,7 +141,7 @@ public class ClientTest { order.execute(domainKeyPair); // Wait for the order to complete - Status status = waitForCompletion(order::getStatus, order::update); + Status status = waitForCompletion(order::getStatus, order::fetch); if (status != Status.VALID) { LOG.error("Order has failed, reason: {}", order.getError() .map(Problem::toString) @@ -293,7 +292,7 @@ public class ClientTest { challenge.trigger(); // Poll for the challenge to complete. - Status status = waitForCompletion(challenge::getStatus, challenge::update); + Status status = waitForCompletion(challenge::getStatus, challenge::fetch); if (status != Status.VALID) { LOG.error("Challenge has failed, reason: {}", challenge.getError() .map(Problem::toString) @@ -397,7 +396,8 @@ public class ClientTest { * Method of the resource that returns the current status * @param statusUpdater * Method of the resource that updates the internal state and fetches the - * current status from the server + * current status from the server. It returns the instant of an optional + * retry-after header. * @return The final status, either {@link Status#VALID} or {@link Status#INVALID} * @throws AcmeException * If an error occured, or if the status did not reach one of the accepted @@ -412,18 +412,11 @@ public class ClientTest { for (int attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) { LOG.info("Checking current status, attempt {} of {}", attempt, MAX_ATTEMPTS); - // A reasonable default retry-after delay Instant now = Instant.now(); - Instant retryAfter = now.plusSeconds(3L); // Update the status property - try { - statusUpdater.update(); - } catch (AcmeRetryAfterException ex) { - // Server sent a retry-after header, use this instant instead - LOG.info("Server asks to try again at: {}", ex.getRetryAfter()); - retryAfter = ex.getRetryAfter(); - } + Instant retryAfter = statusUpdater.updateAndGetRetryAfter() + .orElse(now.plusSeconds(3L)); // Check the status Status currentStatus = statusSupplier.get(); @@ -445,12 +438,12 @@ public class ClientTest { } /** - * Functional interface that refers to a resource update method that is able to - * throw an {@link AcmeException}. + * Functional interface that refers to a resource update method that returns an + * optional retry-after instant and is able to throw an {@link AcmeException}. */ @FunctionalInterface private interface UpdateMethod { - void update() throws AcmeException; + Optional updateAndGetRetryAfter() throws AcmeException; } /** diff --git a/src/doc/docs/ca/zerossl.md b/src/doc/docs/ca/zerossl.md index 6d03edda..b15a375c 100644 --- a/src/doc/docs/ca/zerossl.md +++ b/src/doc/docs/ca/zerossl.md @@ -13,7 +13,7 @@ ZeroSSL does not provide a staging server (as of Feburary 2024). ## Note * ZeroSSL requires account creation with [key identifier](../usage/account.md#external-account-binding). -* ZeroSSL makes use of the retry-after header, so expect [AcmeRetryAfterException](../usage/exceptions.md#acmeretryafterexception)s to be thrown, and handle them accordingly (see example). +* ZeroSSL makes use of the retry-after header, so expect that the `fetch()` methods return an `Instant`, and wait until this moment has passed (see [example](../example.md)). * Certificate creation can take a considerable amount of time (up to 24h). The retry-after header still gives a short retry period, resulting in a very high number of status update reattempts. * Server response can be very slow sometimes. It is recommended to set a timeout of 30 seconds or higher in the [network settings](../usage/advanced.md#network-settings). diff --git a/src/doc/docs/example.md b/src/doc/docs/example.md index 41b712c0..a77833e8 100644 --- a/src/doc/docs/example.md +++ b/src/doc/docs/example.md @@ -108,7 +108,7 @@ public void fetchCertificate(Collection domains) order.execute(domainKeyPair); // Wait for the order to complete - Status status = waitForCompletion(order::getStatus, order::update); + Status status = waitForCompletion(order::getStatus, order::fetch); if (status != Status.VALID) { LOG.error("Order has failed, reason: {}", order.getError() .map(Problem::toString) @@ -265,7 +265,7 @@ private void authorize(Authorization auth) challenge.trigger(); // Poll for the challenge to complete. - Status status = waitForCompletion(challenge::getStatus, challenge::update); + Status status = waitForCompletion(challenge::getStatus, challenge::fetch); if (status != Status.VALID) { LOG.error("Challenge has failed, reason: {}", challenge.getError() .map(Problem::toString) @@ -364,11 +364,11 @@ The ACME protocol does not specify the sending of events. For this reason, resou This example does a very simple polling in a synchronous busy loop. It updates the local copy of the resource and checks if the status is either `VALID` or `INVALID`. If it is not, it just sleeps for a certain amount of time, and then rechecks the current status. -Some CAs respond with a `Retry-After` HTTP header, which provides a recommendation when to check for a status change again. If this header is present, _acme4j_ will throw an `AcmeRetryAfterException` (and will still update the resource state). If this header is not present, just wait a reasonable amount of time before checking again. +Some CAs respond with a `Retry-After` HTTP header, which provides a recommendation when to check for a status change again. If this header is present, the updater function will return the given instant. If this header is not present, we will just wait a reasonable amount of time before checking again. An enterprise level implementation would do an asynchronous polling by storing the recheck time in a database or a queue with scheduled delivery. -The following method will check if a resource reaches completion (by reaching either `VALID` or `INVALID` status). The first parameter provides the method that fetches the current status (e.g. `Order::getStatus`). The second parameter provides the method that updates the resource status (e.g. `Order::update()`). It returned the terminating status once it has been reached, or will throw an exception if something went wrong. +The following method will check if a resource reaches completion (by reaching either `VALID` or `INVALID` status). The first parameter provides the method that fetches the current status (e.g. `Order::getStatus`). The second parameter provides the method that updates the resource status (e.g. `Order::fetch`). It returned the terminating status once it has been reached, or will throw an exception if something went wrong. ```java private Status waitForCompletion(Supplier statusSupplier, @@ -380,18 +380,11 @@ private Status waitForCompletion(Supplier statusSupplier, for (int attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) { LOG.info("Checking current status, attempt {} of {}", attempt, MAX_ATTEMPTS); - // A reasonable default retry-after delay Instant now = Instant.now(); - Instant retryAfter = now.plusSeconds(3L); // Update the status property - try { - statusUpdater.update(); - } catch (AcmeRetryAfterException ex) { - // Server sent a retry-after header, use this instant instead - LOG.info("Server asks to try again at: {}", ex.getRetryAfter()); - retryAfter = ex.getRetryAfter(); - } + Instant retryAfter = statusUpdater.updateAndGetRetryAfter() + .orElse(now.plusSeconds(3L)); // Check the status Status currentStatus = statusSupplier.get(); @@ -414,7 +407,7 @@ private Status waitForCompletion(Supplier statusSupplier, @FunctionalInterface private interface UpdateMethod { - void update() throws AcmeException; + Optional updateAndGetRetryAfter() throws AcmeException; } ``` diff --git a/src/doc/docs/migration.md b/src/doc/docs/migration.md index aef9b385..a847264c 100644 --- a/src/doc/docs/migration.md +++ b/src/doc/docs/migration.md @@ -8,6 +8,7 @@ This document will help you migrate your code to the latest _acme4j_ version. - acme4j was updated to support the latest [draft-ietf-acme-ari-03](https://www.ietf.org/archive/id/draft-ietf-acme-ari-03.html) now. It is a breaking change! If you use ARI, make sure your server supports the latest draft before updating to this version of acme4j. - `Certificate.markAsReplace()` has been removed, because this method is not supported by [draft-ietf-acme-ari-03](https://www.ietf.org/archive/id/draft-ietf-acme-ari-03.html) anymore. To mark an existing certificate as replaced, use the new method `OrderBuilder.replaces()` now. - `Certificate.getCertID()` is not needed in the ACME context anymore. This method has been marked as deprecated. In a future version of acme4j, it will be removed without replacement. Refer to the source code to see how the certificate ID is computed. +- Instead of invoking `update()` and then handling an `AcmeRetryAfterException`, you should now prefer to invoke `fetch()`. It gives an optional retry-after `Instant` as return value, which makes the retry-after handling less complex. In a future version, `update()` will be fully replaced by `fetch()`. ## Migration to Version 3.0.0 diff --git a/src/doc/docs/usage/exceptions.md b/src/doc/docs/usage/exceptions.md index e28dc65e..a113bac6 100644 --- a/src/doc/docs/usage/exceptions.md +++ b/src/doc/docs/usage/exceptions.md @@ -31,7 +31,7 @@ The exception provides the causing `IOException`. This `AcmeException` shows that a server-side process has not been completed yet, and gives an estimation when the process might be completed. -It can only be thrown when invoking `update()` or one of the getters. +It can only be thrown when invoking `update()`. However, it is preferred to invoke `fetch()`, which returns the retry-after instant directly, instead of throwing this exception. !!! note The internal state of the resource is still updated. diff --git a/src/doc/docs/usage/order.md b/src/doc/docs/usage/order.md index f049e106..ec538ba3 100644 --- a/src/doc/docs/usage/order.md +++ b/src/doc/docs/usage/order.md @@ -72,7 +72,7 @@ Now you have to wait for the server to check your response. If the checks are co ```java while (!EnumSet.of(Status.VALID, Status.INVALID).contains(auth.getStatus())) { Thread.sleep(3000L); - auth.update(); + auth.fetch(); } ``` @@ -81,7 +81,7 @@ This is a very simple example which can be improved in many ways: * Limit the number of checks, to avoid endless loops if an authorization is stuck on server side. * Wait with the status checks until the CA has accessed the response for the first time (e.g. after an incoming HTTP request to the response file). * Use an asynchronous architecture instead of a blocking `Thread.sleep()`. -* Check if `auth.update()` throws an `AcmeRetryAfterException`, and wait for the next update until `AcmeRetryAfterException.getRetryAfter()`. See the [example](../example.md) for a simple way to do that. +* Check if `auth.fetch()` returns a retry-after `Instant`, and wait for the next update at least until this moment is reached. See the [example](../example.md) for a simple way to do that. The CA server may start with the validation immediately after `trigger()` is invoked, so make sure your server is ready to respond to requests before invoking `trigger()`. Otherwise the challenge might fail instantly. @@ -135,7 +135,7 @@ Order order = ... // your Order object from the previous step while (!EnumSet.of(Status.VALID, Status.INVALID).contains(order.getStatus())) { Thread.sleep(3000L); - order.update(); + order.fetch(); } ``` @@ -143,7 +143,7 @@ This is a very simple example which can be improved in many ways: * Limit the number of checks, to avoid endless loops if the order is stuck on server side. * Use an asynchronous architecture instead of a blocking `Thread.sleep()`. -* Check if `order.update()` throws an `AcmeRetryAfterException`, and wait for the next update until `AcmeRetryAfterException.getRetryAfter()`. See the [example](../example.md) for a simple way to do that. +* Check if `order.fetch()` returns a retry-after `Instant`, and wait for the next update at least until this moment is reached. See the [example](../example.md) for a simple way to do that. !!! tip If the status is `PENDING`, you have not completed all authorizations yet.