Check our own mailbox periodically while we're online.

bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientFactoryImpl.java

@@ -1,7 +1,11 @@
 package org.briarproject.bramble.mailbox;
 
+import org.briarproject.bramble.api.lifecycle.IoExecutor;
 import org.briarproject.bramble.api.mailbox.MailboxProperties;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+import org.briarproject.bramble.api.system.TaskScheduler;
+
+import java.util.concurrent.Executor;
 
 import javax.inject.Inject;
 import javax.inject.Provider;
@@ -10,15 +14,21 @@ import javax.inject.Provider;
 class MailboxClientFactoryImpl implements MailboxClientFactory {
 
 	private final MailboxWorkerFactory workerFactory;
+	private final TaskScheduler taskScheduler;
+	private final Executor ioExecutor;
 	private final Provider<ContactMailboxConnectivityChecker>
 			contactCheckerProvider;
 	private final Provider<OwnMailboxConnectivityChecker> ownCheckerProvider;
 
 	@Inject
 	MailboxClientFactoryImpl(MailboxWorkerFactory workerFactory,
+			TaskScheduler taskScheduler,
+			@IoExecutor Executor ioExecutor,
 			Provider<ContactMailboxConnectivityChecker> contactCheckerProvider,
 			Provider<OwnMailboxConnectivityChecker> ownCheckerProvider) {
 		this.workerFactory = workerFactory;
+		this.taskScheduler = taskScheduler;
+		this.ioExecutor = ioExecutor;
 		this.contactCheckerProvider = contactCheckerProvider;
 		this.ownCheckerProvider = ownCheckerProvider;
 	}
@@ -37,6 +47,6 @@ class MailboxClientFactoryImpl implements MailboxClientFactory {
 			MailboxProperties properties) {
 		ConnectivityChecker connectivityChecker = ownCheckerProvider.get();
 		return new OwnMailboxClient(workerFactory, connectivityChecker,
-				reachabilityMonitor, properties);
+				reachabilityMonitor, taskScheduler, ioExecutor, properties);
 	}
 }

bramble-core/src/main/java/org/briarproject/bramble/mailbox/OwnMailboxClient.java

@@ -1,9 +1,13 @@
 package org.briarproject.bramble.mailbox;
 
+import org.briarproject.bramble.api.Cancellable;
 import org.briarproject.bramble.api.contact.ContactId;
+import org.briarproject.bramble.api.lifecycle.IoExecutor;
 import org.briarproject.bramble.api.mailbox.MailboxFolderId;
 import org.briarproject.bramble.api.mailbox.MailboxProperties;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+import org.briarproject.bramble.api.system.TaskScheduler;
+import org.briarproject.bramble.mailbox.ConnectivityChecker.ConnectivityObserver;
 
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -11,17 +15,27 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.Executor;
 import java.util.logging.Logger;
 
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.GuardedBy;
 import javax.annotation.concurrent.ThreadSafe;
 
+import static java.util.concurrent.TimeUnit.HOURS;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.logging.Logger.getLogger;
 
 @ThreadSafe
 @NotNullByDefault
-class OwnMailboxClient implements MailboxClient {
+class OwnMailboxClient implements MailboxClient, ConnectivityObserver {
+
+	/**
+	 * How often to check our own mailbox's connectivity.
+	 * <p>
+	 * Package access for testing.
+	 */
+	static final long CONNECTIVITY_CHECK_INTERVAL_MS = HOURS.toMillis(1);
 
 	private static final Logger LOG =
 			getLogger(OwnMailboxClient.class.getName());
@@ -29,6 +43,9 @@ class OwnMailboxClient implements MailboxClient {
 	private final MailboxWorkerFactory workerFactory;
 	private final ConnectivityChecker connectivityChecker;
 	private final TorReachabilityMonitor reachabilityMonitor;
+	private final TaskScheduler taskScheduler;
+	private final Executor ioExecutor;
+	private final MailboxProperties properties;
 	private final MailboxWorker contactListWorker;
 	private final Object lock = new Object();
 
@@ -53,14 +70,30 @@ class OwnMailboxClient implements MailboxClient {
 	@GuardedBy("lock")
 	private final Set<ContactId> assignedForDownload = new HashSet<>();
 
+	/**
+	 * Scheduled task for periodically checking whether the mailbox is
+	 * reachable.
+	 */
+	@GuardedBy("lock")
+	@Nullable
+	private Cancellable connectivityTask = null;
+
+	@GuardedBy("lock")
+	private boolean destroyed = false;
+
 	OwnMailboxClient(MailboxWorkerFactory workerFactory,
 			ConnectivityChecker connectivityChecker,
 			TorReachabilityMonitor reachabilityMonitor,
+			TaskScheduler taskScheduler,
+			@IoExecutor Executor ioExecutor,
 			MailboxProperties properties) {
 		if (!properties.isOwner()) throw new IllegalArgumentException();
 		this.workerFactory = workerFactory;
 		this.connectivityChecker = connectivityChecker;
 		this.reachabilityMonitor = reachabilityMonitor;
+		this.taskScheduler = taskScheduler;
+		this.ioExecutor = ioExecutor;
+		this.properties = properties;
 		contactListWorker = workerFactory.createContactListWorkerForOwnMailbox(
 				connectivityChecker, properties);
 	}
@@ -68,6 +101,7 @@ class OwnMailboxClient implements MailboxClient {
 	@Override
 	public void start() {
 		LOG.info("Started");
+		checkConnectivity();
 		contactListWorker.start();
 	}
 
@@ -76,15 +110,21 @@ class OwnMailboxClient implements MailboxClient {
 		LOG.info("Destroyed");
 		List<MailboxWorker> uploadWorkers;
 		MailboxWorker downloadWorker;
+		Cancellable connectivityTask;
 		synchronized (lock) {
 			uploadWorkers = new ArrayList<>(this.uploadWorkers.values());
 			this.uploadWorkers.clear();
 			downloadWorker = this.downloadWorker;
 			this.downloadWorker = null;
+			connectivityTask = this.connectivityTask;
+			this.connectivityTask = null;
+			destroyed = true;
 		}
 		// Destroy the workers (with apologies to Mr Marx and Mr Engels)
 		for (MailboxWorker worker : uploadWorkers) worker.destroy();
 		if (downloadWorker != null) downloadWorker.destroy();
+		// If a connectivity check is scheduled, cancel it
+		if (connectivityTask != null) connectivityTask.cancel();
 		contactListWorker.destroy();
 		// The connectivity checker belongs to the client, so it should be
 		// destroyed. The Tor reachability monitor is shared between clients,
@@ -155,4 +195,28 @@ class OwnMailboxClient implements MailboxClient {
 		}
 		if (toDestroy != null) toDestroy.destroy();
 	}
+
+	private void checkConnectivity() {
+		synchronized (lock) {
+			if (destroyed) return;
+			connectivityTask = null;
+		}
+		connectivityChecker.checkConnectivity(properties, this);
+		// Avoid leaking observer in case destroy() is called concurrently
+		// before observer is added
+		boolean removeObserver;
+		synchronized (lock) {
+			removeObserver = destroyed;
+		}
+		if (removeObserver) connectivityChecker.removeObserver(this);
+	}
+
+	@Override
+	public void onConnectivityCheckSucceeded() {
+		synchronized (lock) {
+			if (destroyed) return;
+			connectivityTask = taskScheduler.schedule(this::checkConnectivity,
+					ioExecutor, CONNECTIVITY_CHECK_INTERVAL_MS, MILLISECONDS);
+		}
+	}
 }

bramble-core/src/test/java/org/briarproject/bramble/mailbox/OwnMailboxClientTest.java

@@ -1,13 +1,22 @@
 package org.briarproject.bramble.mailbox;
 
+import org.briarproject.bramble.api.Cancellable;
 import org.briarproject.bramble.api.contact.ContactId;
 import org.briarproject.bramble.api.mailbox.MailboxFolderId;
 import org.briarproject.bramble.api.mailbox.MailboxProperties;
+import org.briarproject.bramble.api.system.TaskScheduler;
 import org.briarproject.bramble.test.BrambleMockTestCase;
+import org.briarproject.bramble.test.CaptureArgumentAction;
 import org.jmock.Expectations;
+import org.jmock.lib.action.DoAllAction;
 import org.junit.Test;
 
+import java.util.concurrent.Executor;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+
 import static org.briarproject.bramble.api.mailbox.MailboxConstants.CLIENT_SUPPORTS;
+import static org.briarproject.bramble.mailbox.OwnMailboxClient.CONNECTIVITY_CHECK_INTERVAL_MS;
 import static org.briarproject.bramble.test.TestUtils.getContactId;
 import static org.briarproject.bramble.test.TestUtils.getMailboxProperties;
 import static org.briarproject.bramble.test.TestUtils.getRandomId;
@@ -20,6 +29,9 @@ public class OwnMailboxClientTest extends BrambleMockTestCase {
 			context.mock(ConnectivityChecker.class);
 	private final TorReachabilityMonitor reachabilityMonitor =
 			context.mock(TorReachabilityMonitor.class);
+	private final TaskScheduler taskScheduler =
+			context.mock(TaskScheduler.class);
+	private final Executor ioExecutor = context.mock(Executor.class);
 	private final MailboxWorker contactListWorker =
 			context.mock(MailboxWorker.class, "contactListWorker");
 	private final MailboxWorker uploadWorker1 =
@@ -28,6 +40,8 @@ public class OwnMailboxClientTest extends BrambleMockTestCase {
 			context.mock(MailboxWorker.class, "uploadWorker2");
 	private final MailboxWorker downloadWorker =
 			context.mock(MailboxWorker.class, "downloadWorker");
+	private final Cancellable connectivityCheck =
+			context.mock(Cancellable.class);
 
 	private final MailboxProperties properties =
 			getMailboxProperties(true, CLIENT_SUPPORTS);
@@ -40,12 +54,13 @@ public class OwnMailboxClientTest extends BrambleMockTestCase {
 	public OwnMailboxClientTest() {
 		expectCreateContactListWorker();
 		client = new OwnMailboxClient(workerFactory, connectivityChecker,
-				reachabilityMonitor, properties);
+				reachabilityMonitor, taskScheduler, ioExecutor, properties);
 		context.assertIsSatisfied();
 	}
 
 	@Test
 	public void testStartAndDestroyWithNoContactsAssigned() {
+		expectStartConnectivityCheck();
 		expectStartWorker(contactListWorker);
 		client.start();
 
@@ -56,6 +71,7 @@ public class OwnMailboxClientTest extends BrambleMockTestCase {
 
 	@Test
 	public void testAssignContactForUploadAndDestroyClient() {
+		expectStartConnectivityCheck();
 		expectStartWorker(contactListWorker);
 		client.start();
 
@@ -74,6 +90,7 @@ public class OwnMailboxClientTest extends BrambleMockTestCase {
 
 	@Test
 	public void testAssignAndDeassignContactForUpload() {
+		expectStartConnectivityCheck();
 		expectStartWorker(contactListWorker);
 		client.start();
 
@@ -95,6 +112,7 @@ public class OwnMailboxClientTest extends BrambleMockTestCase {
 
 	@Test
 	public void testAssignAndDeassignTwoContactsForUpload() {
+		expectStartConnectivityCheck();
 		expectStartWorker(contactListWorker);
 		client.start();
 
@@ -129,6 +147,7 @@ public class OwnMailboxClientTest extends BrambleMockTestCase {
 
 	@Test
 	public void testAssignContactForDownloadAndDestroyClient() {
+		expectStartConnectivityCheck();
 		expectStartWorker(contactListWorker);
 		client.start();
 
@@ -147,6 +166,7 @@ public class OwnMailboxClientTest extends BrambleMockTestCase {
 
 	@Test
 	public void testAssignAndDeassignTwoContactsForDownload() {
+		expectStartConnectivityCheck();
 		expectStartWorker(contactListWorker);
 		client.start();
 
@@ -175,6 +195,68 @@ public class OwnMailboxClientTest extends BrambleMockTestCase {
 		client.destroy();
 	}
 
+	@Test
+	public void testCancelsConnectivityCheckWhenClientIsDestroyed() {
+		expectStartConnectivityCheck();
+		expectStartWorker(contactListWorker);
+		client.start();
+
+		// When the first connectivity check succeeds, the worker should
+		// schedule a second check
+		AtomicReference<Runnable> task = new AtomicReference<>();
+		expectScheduleConnectivityCheck(task);
+		client.onConnectivityCheckSucceeded();
+
+		// When the task runs, the worker should check the mailbox's
+		// connectivity
+		expectStartConnectivityCheck();
+		task.get().run();
+
+		// When the second connectivity check succeeds, the worker should
+		// schedule a third check
+		expectScheduleConnectivityCheck(task);
+		client.onConnectivityCheckSucceeded();
+
+		// When the client is destroyed, the scheduled check should be cancelled
+		expectDestroyWorker(contactListWorker);
+		expectDestroyConnectivityChecker();
+		context.checking(new Expectations() {{
+			oneOf(connectivityCheck).cancel();
+		}});
+		client.destroy();
+
+		// If the task runs anyway (cancellation came too late), it should
+		// return when it finds that the client has been destroyed
+		task.get().run();
+	}
+
+	@Test
+	public void testIgnoresConnectivityResultWhenClientIsDestroyed() {
+		expectStartConnectivityCheck();
+		expectStartWorker(contactListWorker);
+		client.start();
+
+		// When the first connectivity check succeeds, the worker should
+		// schedule a second check
+		AtomicReference<Runnable> task = new AtomicReference<>();
+		expectScheduleConnectivityCheck(task);
+		client.onConnectivityCheckSucceeded();
+
+		// When the task runs, the worker should check the mailbox's
+		// connectivity
+		expectStartConnectivityCheck();
+		task.get().run();
+
+		// Before the connectivity check succeeds, the client is destroyed
+		expectDestroyWorker(contactListWorker);
+		expectDestroyConnectivityChecker();
+		client.destroy();
+
+		// If the connectivity check succeeds despite the connectivity checker
+		// having been destroyed, the client should not schedule another check
+		client.onConnectivityCheckSucceeded();
+	}
+
 	private void expectCreateContactListWorker() {
 		context.checking(new Expectations() {{
 			oneOf(workerFactory).createContactListWorkerForOwnMailbox(
@@ -206,6 +288,25 @@ public class OwnMailboxClientTest extends BrambleMockTestCase {
 		}});
 	}
 
+	private void expectStartConnectivityCheck() {
+		context.checking(new Expectations() {{
+			oneOf(connectivityChecker).checkConnectivity(properties, client);
+		}});
+	}
+
+	private void expectScheduleConnectivityCheck(
+			AtomicReference<Runnable> task) {
+		context.checking(new Expectations() {{
+			oneOf(taskScheduler).schedule(with(any(Runnable.class)),
+					with(ioExecutor), with(CONNECTIVITY_CHECK_INTERVAL_MS),
+					with(TimeUnit.MILLISECONDS));
+			will(new DoAllAction(
+					new CaptureArgumentAction<>(task, Runnable.class, 0),
+					returnValue(connectivityCheck)
+			));
+		}});
+	}
+
 	private void expectDestroyWorker(MailboxWorker worker) {
 		context.checking(new Expectations() {{
 			oneOf(worker).destroy();