From 8ec998f64544e7c647955591fd046b6ccabbda66 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 26 May 2022 12:52:29 +0100 Subject: [PATCH] Replace Supplier with more legible ApiCall interface. --- .../briarproject/bramble/api/Supplier.java | 9 ------ .../briarproject/bramble/mailbox/ApiCall.java | 21 ++++++++++++++ .../bramble/mailbox/MailboxApiCaller.java | 17 ++++------- .../bramble/mailbox/MailboxApiCallerImpl.java | 15 +++++----- .../bramble/mailbox/SimpleApiCall.java | 9 +++--- .../mailbox/MailboxApiCallerImplTest.java | 28 ++++++++----------- 6 files changed, 49 insertions(+), 50 deletions(-) delete mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/Supplier.java create mode 100644 bramble-core/src/main/java/org/briarproject/bramble/mailbox/ApiCall.java diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/Supplier.java b/bramble-api/src/main/java/org/briarproject/bramble/api/Supplier.java deleted file mode 100644 index 7a83bddb6..000000000 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/Supplier.java +++ /dev/null @@ -1,9 +0,0 @@ -package org.briarproject.bramble.api; - -import org.briarproject.bramble.api.nullsafety.NotNullByDefault; - -@NotNullByDefault -public interface Supplier { - - T get(); -} diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/ApiCall.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/ApiCall.java new file mode 100644 index 000000000..9c47161ab --- /dev/null +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/ApiCall.java @@ -0,0 +1,21 @@ +package org.briarproject.bramble.mailbox; + +import org.briarproject.bramble.api.lifecycle.IoExecutor; +import org.briarproject.bramble.mailbox.MailboxApi.TolerableFailureException; + +/** + * An interface for calling an API endpoint with the option to retry the call. + */ +interface ApiCall { + + /** + * This method makes a synchronous call to an API endpoint and returns + * true if the call should be retried, in which case the method may be + * called again on the same {@link ApiCall} instance after a delay. + * + * @return True if the API call needs to be retried, or false if the API + * call succeeded or {@link TolerableFailureException failed tolerably}. + */ + @IoExecutor + boolean callApi(); +} diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiCaller.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiCaller.java index 7280eb0a1..a3888e01c 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiCaller.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiCaller.java @@ -1,10 +1,8 @@ package org.briarproject.bramble.mailbox; import org.briarproject.bramble.api.Cancellable; -import org.briarproject.bramble.api.Supplier; import org.briarproject.bramble.api.lifecycle.IoExecutor; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; -import org.briarproject.bramble.mailbox.MailboxApi.TolerableFailureException; import static java.util.concurrent.TimeUnit.DAYS; import static java.util.concurrent.TimeUnit.MINUTES; @@ -23,15 +21,12 @@ interface MailboxApiCaller { long MAX_RETRY_INTERVAL_MS = DAYS.toMillis(1); /** - * Asynchronously calls the given supplier, automatically retrying at - * increasing intervals until the supplier returns false. The returned - * {@link Cancellable} can be used to cancel any future retries. + * Asynchronously calls the given API call on the {@link IoExecutor}, + * automatically retrying at increasing intervals until the API call + * returns false or retries are cancelled. * - * @param supplier A wrapper for an API call. The supplier's - * {@link Supplier#get() get()} method will be called on the - * {@link IoExecutor}. It should return true if the API call needs to be - * retried, or false if the API call succeeded or - * {@link TolerableFailureException failed tolerably}. + * @return A {@link Cancellable} that can be used to cancel any future + * retries. */ - Cancellable retryWithBackoff(Supplier supplier); + Cancellable retryWithBackoff(ApiCall apiCall); } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiCallerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiCallerImpl.java index 9b476eb98..fd625b084 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiCallerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiCallerImpl.java @@ -1,7 +1,6 @@ package org.briarproject.bramble.mailbox; import org.briarproject.bramble.api.Cancellable; -import org.briarproject.bramble.api.Supplier; import org.briarproject.bramble.api.lifecycle.IoExecutor; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.system.TaskScheduler; @@ -31,15 +30,15 @@ class MailboxApiCallerImpl implements MailboxApiCaller { } @Override - public Cancellable retryWithBackoff(Supplier supplier) { - Task task = new Task(supplier); + public Cancellable retryWithBackoff(ApiCall apiCall) { + Task task = new Task(apiCall); task.start(); return task; } private class Task implements Cancellable { - private final Supplier supplier; + private final ApiCall apiCall; private final Object lock = new Object(); @GuardedBy("lock") @@ -52,8 +51,8 @@ class MailboxApiCallerImpl implements MailboxApiCaller { @GuardedBy("lock") private long retryIntervalMs = MIN_RETRY_INTERVAL_MS; - private Task(Supplier supplier) { - this.supplier = supplier; + private Task(ApiCall apiCall) { + this.apiCall = apiCall; } private void start() { @@ -68,8 +67,8 @@ class MailboxApiCallerImpl implements MailboxApiCaller { synchronized (lock) { if (cancelled) return; } - // The supplier returns true if we should retry - if (supplier.get()) { + // The call returns true if we should retry + if (apiCall.callApi()) { synchronized (lock) { if (cancelled) return; scheduledTask = taskScheduler.schedule(this::callApi, diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/SimpleApiCall.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/SimpleApiCall.java index 14c508b33..7c3e6f4b0 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/SimpleApiCall.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/SimpleApiCall.java @@ -1,6 +1,5 @@ package org.briarproject.bramble.mailbox; -import org.briarproject.bramble.api.Supplier; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.mailbox.MailboxApi.ApiException; import org.briarproject.bramble.mailbox.MailboxApi.TolerableFailureException; @@ -17,17 +16,17 @@ import static org.briarproject.bramble.util.LogUtils.logException; * Convenience class for making simple API calls that don't return values. */ @NotNullByDefault -public abstract class SimpleApiCall implements Supplier { +public abstract class SimpleApiCall implements ApiCall { private static final Logger LOG = getLogger(SimpleApiCall.class.getName()); - abstract void callApi() + abstract void tryToCallApi() throws IOException, ApiException, TolerableFailureException; @Override - public Boolean get() { + public boolean callApi() { try { - callApi(); + tryToCallApi(); return false; // Succeeded, don't retry } catch (IOException | ApiException e) { logException(LOG, WARNING, e); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiCallerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiCallerImplTest.java index 10bb91745..2612fe16a 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiCallerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiCallerImplTest.java @@ -1,7 +1,6 @@ package org.briarproject.bramble.mailbox; import org.briarproject.bramble.api.Cancellable; -import org.briarproject.bramble.api.Supplier; import org.briarproject.bramble.api.system.TaskScheduler; import org.briarproject.bramble.test.BrambleMockTestCase; import org.briarproject.bramble.test.CaptureArgumentAction; @@ -23,8 +22,7 @@ public class MailboxApiCallerImplTest extends BrambleMockTestCase { private final TaskScheduler taskScheduler = context.mock(TaskScheduler.class); private final Executor ioExecutor = context.mock(Executor.class); - private final BooleanSupplier supplier = - context.mock(BooleanSupplier.class); + private final ApiCall apiCall = context.mock(ApiCall.class); private final Cancellable scheduledTask = context.mock(Cancellable.class); private final MailboxApiCallerImpl caller = @@ -39,12 +37,12 @@ public class MailboxApiCallerImplTest extends BrambleMockTestCase { will(new CaptureArgumentAction<>(runnable, Runnable.class, 0)); }}); - caller.retryWithBackoff(supplier); + caller.retryWithBackoff(apiCall); - // When the task runs, the supplier should be called. The supplier + // When the task runs, the API call should be called. The call // returns false, so no retries should be scheduled context.checking(new Expectations() {{ - oneOf(supplier).get(); + oneOf(apiCall).callApi(); will(returnValue(false)); }}); @@ -60,12 +58,12 @@ public class MailboxApiCallerImplTest extends BrambleMockTestCase { will(new CaptureArgumentAction<>(runnable, Runnable.class, 0)); }}); - Cancellable returned = caller.retryWithBackoff(supplier); + Cancellable returned = caller.retryWithBackoff(apiCall); - // When the task runs, the supplier should be called. The supplier + // When the task runs, the API call should be called. The call // returns true, so a retry should be scheduled context.checking(new Expectations() {{ - oneOf(supplier).get(); + oneOf(apiCall).callApi(); will(returnValue(true)); oneOf(taskScheduler).schedule(with(any(Runnable.class)), with(ioExecutor), with(MIN_RETRY_INTERVAL_MS), @@ -90,7 +88,7 @@ public class MailboxApiCallerImplTest extends BrambleMockTestCase { returned.cancel(); // If the scheduled task runs anyway (cancellation came too late), - // the supplier should not be called and no further tries should be + // the API call should not be called and no further tries should be // scheduled runnable.get().run(); } @@ -114,13 +112,13 @@ public class MailboxApiCallerImplTest extends BrambleMockTestCase { will(new CaptureArgumentAction<>(runnable, Runnable.class, 0)); }}); - caller.retryWithBackoff(supplier); + caller.retryWithBackoff(apiCall); - // Each time the task runs, the supplier returns true, so a retry + // Each time the task runs, the API call returns true, so a retry // should be scheduled with a longer interval for (long interval : expectedIntervals) { context.checking(new Expectations() {{ - oneOf(supplier).get(); + oneOf(apiCall).callApi(); will(returnValue(true)); oneOf(taskScheduler).schedule(with(any(Runnable.class)), with(ioExecutor), with(interval), with(MILLISECONDS)); @@ -134,8 +132,4 @@ public class MailboxApiCallerImplTest extends BrambleMockTestCase { runnable.get().run(); } } - - // Reify the generic type to mollify jMock - private interface BooleanSupplier extends Supplier { - } }