From 1c6fb6491a5f6f35d1dece68e0beb36db333c161 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 10 Jun 2022 11:38:52 -0300 Subject: [PATCH] Use /versions for mailbox connectivity check Briar's mailbox status screen used the status API endpoint for its connectivity check. Now, it uses the versions endpoint instead, so that if we've warned the user that Briar and the Mailbox are using incompatible API versions, and the user has upgraded one of the apps to fix the issue, the user can use the "check connection" button in the status screen to check that the issue has been fixed. (This is specifically needed for the case where the user has upgraded the Mailbox, because in the case where the user has upgraded Briar, Briar should automatically check the mailbox's API versions when it comes back online after upgrading.) --- .../api/mailbox/MailboxSettingsManager.java | 3 + .../bramble/mailbox/MailboxManagerImpl.java | 19 +-- .../mailbox/MailboxSettingsManagerImpl.java | 41 ++++-- .../mailbox/MailboxManagerImplTest.java | 128 ++++++++++++++++++ .../MailboxSettingsManagerImplTest.java | 23 ++++ 5 files changed, 196 insertions(+), 18 deletions(-) create mode 100644 bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxManagerImplTest.java diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxSettingsManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxSettingsManager.java index 3bd50d6c6..e2c8580bd 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxSettingsManager.java +++ b/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 versions) throws DbException; + void recordFailedConnectionAttempt(Transaction txn, long now) throws DbException; diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java index da8d8d47d..753e7cfef 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java +++ b/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 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 versions) + throws DbException { long now = clock.currentTimeMillis(); db.transaction(false, txn -> { - if (success) { - mailboxSettingsManager.recordSuccessfulConnection(txn, now); + if (versions != null) { + mailboxSettingsManager + .recordSuccessfulConnection(txn, now, versions); } else { mailboxSettingsManager.recordFailedConnectionAttempt(txn, now); } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImpl.java index 23f9ca486..2a0aed3c6 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImpl.java +++ b/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 serverSupports = p.getServerSupports(); - 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); + encodeServerSupports(serverSupports, s); 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 = - settingsManager.getSettings(txn, SETTINGS_NAMESPACE); + recordSuccessfulConnection(txn, now, null); + } + + @Override + public void recordSuccessfulConnection(Transaction txn, long now, + @Nullable List versions) throws DbException { Settings s = new Settings(); + // fetch version that the server supports first + List 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 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 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 parseServerSupports(Settings s) throws DbException { if (!s.containsKey(SETTINGS_KEY_SERVER_SUPPORTS)) return emptyList(); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxManagerImplTest.java new file mode 100644 index 000000000..ae1ee4666 --- /dev/null +++ b/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()); + } + +} diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImplTest.java index 17a9330b6..0cdf08cdb 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImplTest.java +++ b/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 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);