Merge branch '2229-mailbox-client' into 'master'

Add connectivity check tasks, refactor mailbox properties

See merge request briar/briar!1650

bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java

@@ -25,6 +25,7 @@ import org.briarproject.bramble.api.identity.Author;
 import org.briarproject.bramble.api.identity.AuthorFactory;
 import org.briarproject.bramble.api.mailbox.MailboxAuthToken;
 import org.briarproject.bramble.api.mailbox.MailboxFolderId;
+import org.briarproject.bramble.api.mailbox.MailboxProperties;
 import org.briarproject.bramble.api.mailbox.MailboxUpdate;
 import org.briarproject.bramble.api.mailbox.MailboxUpdateWithMailbox;
 import org.briarproject.bramble.api.mailbox.MailboxVersion;
@@ -455,9 +456,11 @@ class ClientHelperImpl implements ClientHelper {
 		checkLength(inboxId, UniqueId.LENGTH);
 		byte[] outboxId = properties.getRaw(PROP_KEY_OUTBOXID);
 		checkLength(outboxId, UniqueId.LENGTH);
-		return new MailboxUpdateWithMailbox(clientSupportsList,
-				serverSupportsList, onion, new MailboxAuthToken(authToken),
+		String baseUrl = "http://" + onion + ".onion"; // TODO
+		MailboxProperties props = new MailboxProperties(baseUrl,
+				new MailboxAuthToken(authToken), serverSupportsList,
 				new MailboxFolderId(inboxId), new MailboxFolderId(outboxId));
+		return new MailboxUpdateWithMailbox(clientSupportsList, props);
 	}
 
 	@Override

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();
+}

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

@@ -0,0 +1,32 @@
+package org.briarproject.bramble.mailbox;
+
+import org.briarproject.bramble.api.mailbox.MailboxProperties;
+import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+
+import javax.annotation.concurrent.ThreadSafe;
+
+/**
+ * An interface for checking whether a mailbox is reachable.
+ */
+@ThreadSafe
+@NotNullByDefault
+interface ConnectivityChecker {
+
+	/**
+	 * Destroys the checker. Any current connectivity check is cancelled.
+	 */
+	void destroy();
+
+	/**
+	 * Starts a connectivity check if needed and calls the given observer when
+	 * the check succeeds. If a check is already running then the observer is
+	 * called when the check succeeds. If a connectivity check has recently
+	 * succeeded then the observer is called immediately.
+	 */
+	void checkConnectivity(MailboxProperties properties,
+			ConnectivityObserver o);
+
+	interface ConnectivityObserver {
+		void onConnectivityCheckSucceeded();
+	}
+}

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

@@ -0,0 +1,111 @@
+package org.briarproject.bramble.mailbox;
+
+import org.briarproject.bramble.api.Cancellable;
+import org.briarproject.bramble.api.mailbox.MailboxProperties;
+import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+import org.briarproject.bramble.api.system.Clock;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.GuardedBy;
+import javax.annotation.concurrent.ThreadSafe;
+
+@ThreadSafe
+@NotNullByDefault
+abstract class ConnectivityCheckerImpl implements ConnectivityChecker {
+
+	/**
+	 * If no more than this much time has elapsed since the last connectivity
+	 * check succeeded, consider the result to be fresh and don't check again.
+	 * <p>
+	 * Package access for testing.
+	 */
+	static final long CONNECTIVITY_CHECK_FRESHNESS_MS = 10_000;
+
+	private final Object lock = new Object();
+
+	protected final Clock clock;
+	private final MailboxApiCaller mailboxApiCaller;
+
+	@GuardedBy("lock")
+	private boolean destroyed = false;
+
+	@GuardedBy("lock")
+	@Nullable
+	private Cancellable connectivityCheck = null;
+
+	@GuardedBy("lock")
+	private long lastConnectivityCheckSucceeded = 0;
+
+	@GuardedBy("lock")
+	private final List<ConnectivityObserver> connectivityObservers =
+			new ArrayList<>();
+
+	/**
+	 * Creates an {@link ApiCall} for checking whether the mailbox is
+	 * reachable. The {@link ApiCall} should call
+	 * {@link #onConnectivityCheckSucceeded(long)} if the check succeeds.
+	 */
+	abstract ApiCall createConnectivityCheckTask(MailboxProperties properties);
+
+	ConnectivityCheckerImpl(Clock clock, MailboxApiCaller mailboxApiCaller) {
+		this.clock = clock;
+		this.mailboxApiCaller = mailboxApiCaller;
+	}
+
+	@Override
+	public void destroy() {
+		synchronized (lock) {
+			destroyed = true;
+			connectivityObservers.clear();
+			if (connectivityCheck != null) {
+				connectivityCheck.cancel();
+				connectivityCheck = null;
+			}
+		}
+	}
+
+	@Override
+	public void checkConnectivity(MailboxProperties properties,
+			ConnectivityObserver o) {
+		boolean callNow = false;
+		synchronized (lock) {
+			if (destroyed) return;
+			if (connectivityCheck == null) {
+				// No connectivity check is running
+				long now = clock.currentTimeMillis();
+				if (now - lastConnectivityCheckSucceeded
+						> CONNECTIVITY_CHECK_FRESHNESS_MS) {
+					// The last connectivity check is stale, start a new one
+					connectivityObservers.add(o);
+					ApiCall task =
+							createConnectivityCheckTask(properties);
+					connectivityCheck = mailboxApiCaller.retryWithBackoff(task);
+				} else {
+					// The last connectivity check is fresh
+					callNow = true;
+				}
+			} else {
+				// A connectivity check is running, wait for it to succeed
+				connectivityObservers.add(o);
+			}
+		}
+		if (callNow) o.onConnectivityCheckSucceeded();
+	}
+
+	protected void onConnectivityCheckSucceeded(long now) {
+		List<ConnectivityObserver> observers;
+		synchronized (lock) {
+			if (destroyed) return;
+			connectivityCheck = null;
+			lastConnectivityCheckSucceeded = now;
+			observers = new ArrayList<>(connectivityObservers);
+			connectivityObservers.clear();
+		}
+		for (ConnectivityObserver o : observers) {
+			o.onConnectivityCheckSucceeded();
+		}
+	}
+}

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

@@ -0,0 +1,39 @@
+package org.briarproject.bramble.mailbox;
+
+import org.briarproject.bramble.api.mailbox.MailboxProperties;
+import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+import org.briarproject.bramble.api.system.Clock;
+import org.briarproject.bramble.mailbox.MailboxApi.ApiException;
+
+import java.io.IOException;
+
+import javax.annotation.concurrent.ThreadSafe;
+
+@ThreadSafe
+@NotNullByDefault
+class ContactMailboxConnectivityChecker extends ConnectivityCheckerImpl {
+
+	private final MailboxApi mailboxApi;
+
+	ContactMailboxConnectivityChecker(Clock clock,
+			MailboxApiCaller mailboxApiCaller, MailboxApi mailboxApi) {
+		super(clock, mailboxApiCaller);
+		this.mailboxApi = mailboxApi;
+	}
+
+	@Override
+	ApiCall createConnectivityCheckTask(MailboxProperties properties) {
+		if (properties.isOwner()) throw new IllegalArgumentException();
+		return new SimpleApiCall() {
+			@Override
+			void tryToCallApi() throws IOException, ApiException {
+				if (!mailboxApi.checkStatus(properties)) {
+					throw new ApiException();
+				}
+				// Call the observers and cache the result
+				onConnectivityCheckSucceeded(clock.currentTimeMillis());
+			}
+		};
+	}
+
+}

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<Boolean> supplier);
+	Cancellable retryWithBackoff(ApiCall apiCall);
 }

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<Boolean> 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<Boolean> 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<Boolean> 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,

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

@@ -95,7 +95,7 @@ class MailboxApiImpl implements MailboxApi {
 			}
 			return new MailboxProperties(properties.getBaseUrl(),
 					MailboxAuthToken.fromString(tokenNode.textValue()),
-					true, parseServerSupports(node));
+					parseServerSupports(node));
 		} catch (JacksonException | InvalidMailboxIdException e) {
 			throw new ApiException();
 		}
@@ -127,7 +127,6 @@ class MailboxApiImpl implements MailboxApi {
 	@Override
 	public boolean checkStatus(MailboxProperties properties)
 			throws IOException, ApiException {
-		if (!properties.isOwner()) throw new IllegalArgumentException();
 		Response response = sendGetRequest(properties, "/status");
 		if (response.code() == 401) throw new ApiException();
 		return response.isSuccessful();

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

@@ -177,11 +177,10 @@ class MailboxPairingTaskImpl implements MailboxPairingTask {
 		LOG.info("QR code is valid");
 		byte[] onionPubKey = Arrays.copyOfRange(bytes, 1, 33);
 		String onion = crypto.encodeOnion(onionPubKey);
-		String baseUrl = "http://" + onion + ".onion";
+		String baseUrl = "http://" + onion + ".onion"; // TODO
 		byte[] tokenBytes = Arrays.copyOfRange(bytes, 33, 65);
 		MailboxAuthToken setupToken = new MailboxAuthToken(tokenBytes);
-		return new MailboxProperties(baseUrl, setupToken, true,
-				new ArrayList<>());
+		return new MailboxProperties(baseUrl, setupToken, new ArrayList<>());
 	}
 
 }

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

@@ -31,6 +31,7 @@ class MailboxSettingsManagerImpl implements MailboxSettingsManager {
 
 	// Package access for testing
 	static final String SETTINGS_NAMESPACE = "mailbox";
+	// TODO: This currently stores the base URL, not the 56-char onion address
 	static final String SETTINGS_KEY_ONION = "onion";
 	static final String SETTINGS_KEY_TOKEN = "token";
 	static final String SETTINGS_KEY_SERVER_SUPPORTS = "serverSupports";
@@ -70,7 +71,7 @@ class MailboxSettingsManagerImpl implements MailboxSettingsManager {
 		}
 		try {
 			MailboxAuthToken tokenId = MailboxAuthToken.fromString(token);
-			return new MailboxProperties(onion, tokenId, true, serverSupports);
+			return new MailboxProperties(onion, tokenId, serverSupports);
 		} catch (InvalidMailboxIdException e) {
 			throw new DbException(e);
 		}

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

@@ -242,11 +242,14 @@ class MailboxUpdateManagerImpl implements MailboxUpdateManager,
 	private void createAndSendUpdateWithMailbox(Transaction txn, Contact c,
 			List<MailboxVersion> serverSupports, String ownOnion)
 			throws DbException {
-		MailboxUpdate u = new MailboxUpdateWithMailbox(
-				clientSupports, serverSupports, ownOnion,
+		String baseUrl = "http://" + ownOnion + ".onion"; // TODO
+		MailboxProperties properties = new MailboxProperties(baseUrl,
 				new MailboxAuthToken(crypto.generateUniqueId().getBytes()),
+				serverSupports,
 				new MailboxFolderId(crypto.generateUniqueId().getBytes()),
 				new MailboxFolderId(crypto.generateUniqueId().getBytes()));
+		MailboxUpdate u =
+				new MailboxUpdateWithMailbox(clientSupports, properties);
 		Group g = getContactGroup(c);
 		storeMessageReplaceLatest(txn, g.getId(), u);
 	}
@@ -325,11 +328,12 @@ class MailboxUpdateManagerImpl implements MailboxUpdateManager,
 		BdfList serverSupports = new BdfList();
 		if (u.hasMailbox()) {
 			MailboxUpdateWithMailbox um = (MailboxUpdateWithMailbox) u;
-			serverSupports = encodeSupportsList(um.getServerSupports());
-			dict.put(PROP_KEY_ONION, um.getOnion());
-			dict.put(PROP_KEY_AUTHTOKEN, um.getAuthToken().getBytes());
-			dict.put(PROP_KEY_INBOXID, um.getInboxId().getBytes());
-			dict.put(PROP_KEY_OUTBOXID, um.getOutboxId().getBytes());
+			MailboxProperties properties = um.getMailboxProperties();
+			serverSupports = encodeSupportsList(properties.getServerSupports());
+			dict.put(PROP_KEY_ONION, properties.getOnion());
+			dict.put(PROP_KEY_AUTHTOKEN, properties.getAuthToken());
+			dict.put(PROP_KEY_INBOXID, properties.getInboxId());
+			dict.put(PROP_KEY_OUTBOXID, properties.getOutboxId());
 		}
 		return BdfList.of(version, encodeSupportsList(u.getClientSupports()),
 				serverSupports, dict);

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

@@ -0,0 +1,75 @@
+package org.briarproject.bramble.mailbox;
+
+import org.briarproject.bramble.api.db.DbException;
+import org.briarproject.bramble.api.db.TransactionManager;
+import org.briarproject.bramble.api.mailbox.MailboxProperties;
+import org.briarproject.bramble.api.mailbox.MailboxSettingsManager;
+import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+import org.briarproject.bramble.api.system.Clock;
+import org.briarproject.bramble.mailbox.MailboxApi.ApiException;
+
+import java.io.IOException;
+import java.util.logging.Logger;
+
+import javax.annotation.concurrent.ThreadSafe;
+
+import static java.util.logging.Level.WARNING;
+import static java.util.logging.Logger.getLogger;
+import static org.briarproject.bramble.util.LogUtils.logException;
+
+@ThreadSafe
+@NotNullByDefault
+class OwnMailboxConnectivityChecker extends ConnectivityCheckerImpl {
+
+	private static final Logger LOG =
+			getLogger(OwnMailboxConnectivityChecker.class.getName());
+
+	private final MailboxApi mailboxApi;
+	private final TransactionManager db;
+	private final MailboxSettingsManager mailboxSettingsManager;
+
+	OwnMailboxConnectivityChecker(Clock clock,
+			MailboxApiCaller mailboxApiCaller,
+			MailboxApi mailboxApi,
+			TransactionManager db,
+			MailboxSettingsManager mailboxSettingsManager) {
+		super(clock, mailboxApiCaller);
+		this.mailboxApi = mailboxApi;
+		this.db = db;
+		this.mailboxSettingsManager = mailboxSettingsManager;
+	}
+
+	@Override
+	ApiCall createConnectivityCheckTask(MailboxProperties properties) {
+		if (!properties.isOwner()) throw new IllegalArgumentException();
+		return () -> {
+			try {
+				return checkConnectivityAndStoreResult(properties);
+			} catch (DbException e) {
+				logException(LOG, WARNING, e);
+				return true; // Retry
+			}
+		};
+	}
+
+	private boolean checkConnectivityAndStoreResult(
+			MailboxProperties properties) throws DbException {
+		try {
+			if (!mailboxApi.checkStatus(properties)) throw new ApiException();
+			LOG.info("Own mailbox is reachable");
+			long now = clock.currentTimeMillis();
+			db.transaction(false, txn -> mailboxSettingsManager
+					.recordSuccessfulConnection(txn, now));
+			// Call the observers and cache the result
+			onConnectivityCheckSucceeded(now);
+			return false; // Don't retry
+		} catch (IOException | ApiException e) {
+			LOG.warning("Own mailbox is unreachable");
+			logException(LOG, WARNING, e);
+			long now = clock.currentTimeMillis();
+			db.transaction(false, txn -> mailboxSettingsManager
+					.recordFailedConnectionAttempt(txn, now));
+		}
+		return true; // Retry
+	}
+}

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<Boolean> {
+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);