Merge branch '2326-fetch-versions' into 'master'

Use /versions for mailbox connectivity check

Closes #2326

See merge request briar/briar!1665

bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxSettingsManager.java

@@ -35,6 +35,9 @@ public interface MailboxSettingsManager {
 	void recordSuccessfulConnection(Transaction txn, long now)
 			throws DbException;
 
+	void recordSuccessfulConnection(Transaction txn, long now,
+			List<MailboxVersion> versions) throws DbException;
+
 	void recordFailedConnectionAttempt(Transaction txn, long now)
 			throws DbException;
 

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

@@ -9,10 +9,12 @@ import org.briarproject.bramble.api.mailbox.MailboxPairingTask;
 import org.briarproject.bramble.api.mailbox.MailboxProperties;
 import org.briarproject.bramble.api.mailbox.MailboxSettingsManager;
 import org.briarproject.bramble.api.mailbox.MailboxStatus;
+import org.briarproject.bramble.api.mailbox.MailboxVersion;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.bramble.api.system.Clock;
 
 import java.io.IOException;
+import java.util.List;
 import java.util.concurrent.Executor;
 import java.util.logging.Logger;
 
@@ -98,34 +100,35 @@ class MailboxManagerImpl implements MailboxManager {
 
 	@Override
 	public boolean checkConnection() {
-		boolean success;
+		List<MailboxVersion> versions = null;
 		try {
 			MailboxProperties props = db.transactionWithNullableResult(true,
 					mailboxSettingsManager::getOwnMailboxProperties);
 			if (props == null) throw new DbException();
-			success = api.checkStatus(props);
+			versions = api.getServerSupports(props);
 		} catch (DbException e) {
 			logException(LOG, WARNING, e);
 			// we don't treat this is a failure to record
 			return false;
 		} catch (IOException | MailboxApi.ApiException e) {
 			// we record this as a failure
-			success = false;
 			logException(LOG, WARNING, e);
 		}
 		try {
-			recordCheckResult(success);
+			recordCheckResult(versions);
 		} catch (DbException e) {
 			logException(LOG, WARNING, e);
 		}
-		return success;
+		return versions != null;
 	}
 
-	private void recordCheckResult(boolean success) throws DbException {
+	private void recordCheckResult(@Nullable List<MailboxVersion> versions)
+			throws DbException {
 		long now = clock.currentTimeMillis();
 		db.transaction(false, txn -> {
-			if (success) {
+			if (versions != null) {
-				mailboxSettingsManager.recordSuccessfulConnection(txn, now);
+				mailboxSettingsManager
+						.recordSuccessfulConnection(txn, now, versions);
 			} else {
 				mailboxSettingsManager.recordFailedConnectionAttempt(txn, now);
 			}

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

@@ -77,13 +77,7 @@ class MailboxSettingsManagerImpl implements MailboxSettingsManager {
 		s.put(SETTINGS_KEY_ONION, p.getBaseUrl());
 		s.put(SETTINGS_KEY_TOKEN, p.getAuthToken().toString());
 		List<MailboxVersion> serverSupports = p.getServerSupports();
-		int[] ints = new int[serverSupports.size() * 2];
+		encodeServerSupports(serverSupports, s);
-		int i = 0;
-		for (MailboxVersion v : serverSupports) {
-			ints[i++] = v.getMajor();
-			ints[i++] = v.getMinor();
-		}
-		s.putIntArray(SETTINGS_KEY_SERVER_SUPPORTS, ints);
 		settingsManager.mergeSettings(txn, s, SETTINGS_NAMESPACE);
 		for (MailboxHook hook : hooks) {
 			hook.mailboxPaired(txn, p.getOnion(), p.getServerSupports());
@@ -121,14 +115,30 @@ class MailboxSettingsManagerImpl implements MailboxSettingsManager {
 	@Override
 	public void recordSuccessfulConnection(Transaction txn, long now)
 			throws DbException {
-		Settings oldSettings =
+		recordSuccessfulConnection(txn, now, null);
-				settingsManager.getSettings(txn, SETTINGS_NAMESPACE);
+	}
+
+	@Override
+	public void recordSuccessfulConnection(Transaction txn, long now,
+			@Nullable List<MailboxVersion> versions) throws DbException {
 		Settings s = new Settings();
+		// fetch version that the server supports first
+		List<MailboxVersion> serverSupports;
+		if (versions == null) {
+			Settings oldSettings =
+					settingsManager.getSettings(txn, SETTINGS_NAMESPACE);
+			serverSupports = parseServerSupports(oldSettings);
+		} else {
+			serverSupports = versions;
+			// store new versions
+			encodeServerSupports(serverSupports, s);
+		}
+		// now record the successful connection
 		s.putLong(SETTINGS_KEY_LAST_ATTEMPT, now);
 		s.putLong(SETTINGS_KEY_LAST_SUCCESS, now);
 		s.putInt(SETTINGS_KEY_ATTEMPTS, 0);
 		settingsManager.mergeSettings(txn, s, SETTINGS_NAMESPACE);
-		List<MailboxVersion> serverSupports = parseServerSupports(oldSettings);
+		// broadcast status event
 		MailboxStatus status = new MailboxStatus(now, now, 0, serverSupports);
 		txn.attach(new OwnMailboxConnectionStatusEvent(status));
 	}
@@ -171,6 +181,17 @@ class MailboxSettingsManagerImpl implements MailboxSettingsManager {
 		return filename;
 	}
 
+	private void encodeServerSupports(List<MailboxVersion> serverSupports,
+			Settings s) {
+		int[] ints = new int[serverSupports.size() * 2];
+		int i = 0;
+		for (MailboxVersion v : serverSupports) {
+			ints[i++] = v.getMajor();
+			ints[i++] = v.getMinor();
+		}
+		s.putIntArray(SETTINGS_KEY_SERVER_SUPPORTS, ints);
+	}
+
 	private List<MailboxVersion> parseServerSupports(Settings s)
 			throws DbException {
 		if (!s.containsKey(SETTINGS_KEY_SERVER_SUPPORTS)) return emptyList();

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

@@ -0,0 +1,128 @@
+package org.briarproject.bramble.mailbox;
+
+import org.briarproject.bramble.api.db.DbException;
+import org.briarproject.bramble.api.db.Transaction;
+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.mailbox.event.OwnMailboxConnectionStatusEvent;
+import org.briarproject.bramble.api.system.Clock;
+import org.briarproject.bramble.test.BrambleMockTestCase;
+import org.briarproject.bramble.test.DbExpectations;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.Random;
+import java.util.concurrent.Executor;
+
+import static org.briarproject.bramble.api.mailbox.MailboxConstants.CLIENT_SUPPORTS;
+import static org.briarproject.bramble.test.TestUtils.getMailboxProperties;
+import static org.briarproject.bramble.test.TestUtils.hasEvent;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class MailboxManagerImplTest extends BrambleMockTestCase {
+
+	private final Executor ioExecutor = context.mock(Executor.class);
+	private final MailboxApi api = context.mock(MailboxApi.class);
+	private final TransactionManager db =
+			context.mock(TransactionManager.class);
+	private final MailboxSettingsManager mailboxSettingsManager =
+			context.mock(MailboxSettingsManager.class);
+	private final MailboxPairingTaskFactory pairingTaskFactory =
+			context.mock(MailboxPairingTaskFactory.class);
+	private final Clock clock =
+			context.mock(Clock.class);
+
+	private final MailboxManagerImpl manager = new MailboxManagerImpl(
+			ioExecutor, api, db, mailboxSettingsManager, pairingTaskFactory,
+			clock);
+
+	@Test
+	public void testDbExceptionDoesNotRecordFailure() throws Exception {
+		Transaction txn = new Transaction(null, true);
+
+		context.checking(new DbExpectations() {{
+			oneOf(db).transactionWithNullableResult(with(true),
+					withNullableDbCallable(txn));
+			oneOf(mailboxSettingsManager).getOwnMailboxProperties(txn);
+			will(throwException(new DbException()));
+		}});
+
+		assertFalse(manager.checkConnection());
+		assertFalse(hasEvent(txn, OwnMailboxConnectionStatusEvent.class));
+	}
+
+	@Test
+	public void testIOExceptionDoesRecordFailure() throws Exception {
+		Transaction txn = new Transaction(null, true);
+		Transaction txn2 = new Transaction(null, false);
+		MailboxProperties props = getMailboxProperties(true, CLIENT_SUPPORTS);
+		long now = new Random().nextLong();
+
+		context.checking(new DbExpectations() {{
+			oneOf(db).transactionWithNullableResult(with(true),
+					withNullableDbCallable(txn));
+			oneOf(mailboxSettingsManager).getOwnMailboxProperties(txn);
+			will(returnValue(props));
+			oneOf(api).getServerSupports(props);
+			will(throwException(new IOException()));
+			oneOf(clock).currentTimeMillis();
+			will(returnValue(now));
+			oneOf(db).transaction(with(false), withDbRunnable(txn2));
+			oneOf(mailboxSettingsManager)
+					.recordFailedConnectionAttempt(txn2, now);
+		}});
+
+		assertFalse(manager.checkConnection());
+	}
+
+	@Test
+	public void testApiExceptionDoesRecordFailure() throws Exception {
+		Transaction txn = new Transaction(null, true);
+		Transaction txn2 = new Transaction(null, false);
+		MailboxProperties props = getMailboxProperties(true, CLIENT_SUPPORTS);
+		long now = new Random().nextLong();
+
+		context.checking(new DbExpectations() {{
+			oneOf(db).transactionWithNullableResult(with(true),
+					withNullableDbCallable(txn));
+			oneOf(mailboxSettingsManager).getOwnMailboxProperties(txn);
+			will(returnValue(props));
+			oneOf(api).getServerSupports(props);
+			will(throwException(new MailboxApi.ApiException()));
+			oneOf(clock).currentTimeMillis();
+			will(returnValue(now));
+			oneOf(db).transaction(with(false), withDbRunnable(txn2));
+			oneOf(mailboxSettingsManager)
+					.recordFailedConnectionAttempt(txn2, now);
+		}});
+
+		assertFalse(manager.checkConnection());
+	}
+
+	@Test
+	public void testConnectionSuccess() throws Exception {
+		Transaction txn = new Transaction(null, true);
+		Transaction txn2 = new Transaction(null, false);
+		MailboxProperties props = getMailboxProperties(true, CLIENT_SUPPORTS);
+		long now = new Random().nextLong();
+
+		context.checking(new DbExpectations() {{
+			oneOf(db).transactionWithNullableResult(with(true),
+					withNullableDbCallable(txn));
+			oneOf(mailboxSettingsManager).getOwnMailboxProperties(txn);
+			will(returnValue(props));
+			oneOf(api).getServerSupports(props);
+			will(returnValue(CLIENT_SUPPORTS));
+			oneOf(clock).currentTimeMillis();
+			will(returnValue(now));
+			oneOf(db).transaction(with(false), withDbRunnable(txn2));
+			oneOf(mailboxSettingsManager)
+					.recordSuccessfulConnection(txn2, now, CLIENT_SUPPORTS);
+		}});
+
+		assertTrue(manager.checkConnection());
+	}
+
+}

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

@@ -18,6 +18,7 @@ import java.util.List;
 import java.util.Random;
 
 import static java.util.Arrays.asList;
+import static java.util.Collections.singletonList;
 import static org.briarproject.bramble.mailbox.MailboxSettingsManagerImpl.SETTINGS_KEY_ATTEMPTS;
 import static org.briarproject.bramble.mailbox.MailboxSettingsManagerImpl.SETTINGS_KEY_LAST_ATTEMPT;
 import static org.briarproject.bramble.mailbox.MailboxSettingsManagerImpl.SETTINGS_KEY_LAST_SUCCESS;
@@ -165,6 +166,28 @@ public class MailboxSettingsManagerImplTest extends BrambleMockTestCase {
 		hasEvent(txn, OwnMailboxConnectionStatusEvent.class);
 	}
 
+	@Test
+	public void testRecordsSuccessWithVersions() throws Exception {
+		Transaction txn = new Transaction(null, false);
+		List<MailboxVersion> versions = singletonList(new MailboxVersion(2, 1));
+		Settings expectedSettings = new Settings();
+		expectedSettings.putLong(SETTINGS_KEY_LAST_ATTEMPT, now);
+		expectedSettings.putLong(SETTINGS_KEY_LAST_SUCCESS, now);
+		expectedSettings.putInt(SETTINGS_KEY_ATTEMPTS, 0);
+		expectedSettings.putInt(SETTINGS_KEY_SERVER_SUPPORTS, 0);
+		int[] newVersionsInts = {2, 1};
+		expectedSettings
+				.putIntArray(SETTINGS_KEY_SERVER_SUPPORTS, newVersionsInts);
+
+		context.checking(new Expectations() {{
+			oneOf(settingsManager).mergeSettings(txn, expectedSettings,
+					SETTINGS_NAMESPACE);
+		}});
+
+		manager.recordSuccessfulConnection(txn, now, versions);
+		hasEvent(txn, OwnMailboxConnectionStatusEvent.class);
+	}
+
 	@Test
 	public void testRecordsFailureOnFirstAttempt() throws Exception {
 		testRecordsFailure(new Settings(), 0);