From 46352f664cfd1776e524c5c4e247a231fd003d47 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Wed, 20 Jul 2022 16:49:05 +0100 Subject: [PATCH 1/8] Add mailbox client manager. --- .../bramble/api/mailbox/MailboxHelper.java | 7 +- .../bramble/mailbox/MailboxClientManager.java | 622 ++++++++++++++++++ 2 files changed, 627 insertions(+), 2 deletions(-) create mode 100644 bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxHelper.java b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxHelper.java index c57eea29b..42740156a 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxHelper.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxHelper.java @@ -1,19 +1,22 @@ package org.briarproject.bramble.api.mailbox; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + import java.util.List; import java.util.TreeSet; import static org.briarproject.bramble.api.mailbox.MailboxConstants.API_CLIENT_TOO_OLD; import static org.briarproject.bramble.api.mailbox.MailboxConstants.API_SERVER_TOO_OLD; -class MailboxHelper { +@NotNullByDefault +public class MailboxHelper { /** * Returns the highest major version that both client and server support * or {@link MailboxConstants#API_SERVER_TOO_OLD} if the server is too old * or {@link MailboxConstants#API_CLIENT_TOO_OLD} if the client is too old. */ - static int getHighestCommonMajorVersion( + public static int getHighestCommonMajorVersion( List client, List server) { TreeSet clientVersions = new TreeSet<>(); for (MailboxVersion version : client) { diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java new file mode 100644 index 000000000..1750c49ca --- /dev/null +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java @@ -0,0 +1,622 @@ +package org.briarproject.bramble.mailbox; + +import org.briarproject.bramble.api.contact.Contact; +import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.contact.ContactManager; +import org.briarproject.bramble.api.contact.event.ContactRemovedEvent; +import org.briarproject.bramble.api.db.DatabaseExecutor; +import org.briarproject.bramble.api.db.DbException; +import org.briarproject.bramble.api.db.TransactionManager; +import org.briarproject.bramble.api.event.Event; +import org.briarproject.bramble.api.event.EventExecutor; +import org.briarproject.bramble.api.event.EventListener; +import org.briarproject.bramble.api.lifecycle.Service; +import org.briarproject.bramble.api.lifecycle.ServiceException; +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.MailboxUpdate; +import org.briarproject.bramble.api.mailbox.MailboxUpdateManager; +import org.briarproject.bramble.api.mailbox.MailboxUpdateWithMailbox; +import org.briarproject.bramble.api.mailbox.MailboxVersion; +import org.briarproject.bramble.api.mailbox.event.MailboxPairedEvent; +import org.briarproject.bramble.api.mailbox.event.MailboxUnpairedEvent; +import org.briarproject.bramble.api.mailbox.event.MailboxUpdateSentEvent; +import org.briarproject.bramble.api.mailbox.event.OwnMailboxConnectionStatusEvent; +import org.briarproject.bramble.api.mailbox.event.RemoteMailboxUpdateEvent; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.plugin.Plugin; +import org.briarproject.bramble.api.plugin.PluginManager; +import org.briarproject.bramble.api.plugin.event.TransportActiveEvent; +import org.briarproject.bramble.api.plugin.event.TransportInactiveEvent; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executor; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.logging.Logger; + +import javax.annotation.Nullable; +import javax.annotation.concurrent.ThreadSafe; +import javax.inject.Inject; + +import static java.util.logging.Level.WARNING; +import static java.util.logging.Logger.getLogger; +import static org.briarproject.bramble.api.mailbox.MailboxConstants.CLIENT_SUPPORTS; +import static org.briarproject.bramble.api.mailbox.MailboxHelper.getHighestCommonMajorVersion; +import static org.briarproject.bramble.api.nullsafety.NullSafety.requireNonNull; +import static org.briarproject.bramble.api.plugin.Plugin.State.ACTIVE; +import static org.briarproject.bramble.api.plugin.TorConstants.ID; +import static org.briarproject.bramble.util.LogUtils.logException; + +@ThreadSafe +@NotNullByDefault +class MailboxClientManager implements Service, EventListener { + + private static final Logger LOG = + getLogger(MailboxClientManager.class.getName()); + + private final Executor eventExecutor, dbExecutor; + private final TransactionManager db; + private final ContactManager contactManager; + private final PluginManager pluginManager; + private final MailboxSettingsManager mailboxSettingsManager; + private final MailboxUpdateManager mailboxUpdateManager; + private final MailboxClientFactory mailboxClientFactory; + private final TorReachabilityMonitor reachabilityMonitor; + private final AtomicBoolean used = new AtomicBoolean(false); + + // All mutable state must only be accessed on the worker thread + private final Map contactUpdates = new HashMap<>(); + private final Map contactClients = + new HashMap<>(); + @Nullable + private MailboxProperties ownProperties = null; + @Nullable + private MailboxClient ownClient = null; + private boolean online = false, handleEvents = false; + + @Inject + MailboxClientManager(@EventExecutor Executor eventExecutor, + @DatabaseExecutor Executor dbExecutor, + TransactionManager db, + ContactManager contactManager, + PluginManager pluginManager, + MailboxSettingsManager mailboxSettingsManager, + MailboxUpdateManager mailboxUpdateManager, + MailboxClientFactory mailboxClientFactory, + TorReachabilityMonitor reachabilityMonitor) { + this.eventExecutor = eventExecutor; + this.dbExecutor = dbExecutor; + this.db = db; + this.contactManager = contactManager; + this.pluginManager = pluginManager; + this.mailboxSettingsManager = mailboxSettingsManager; + this.mailboxUpdateManager = mailboxUpdateManager; + this.mailboxClientFactory = mailboxClientFactory; + this.reachabilityMonitor = reachabilityMonitor; + } + + @Override + public void startService() throws ServiceException { + if (used.getAndSet(true)) throw new IllegalStateException(); + reachabilityMonitor.start(); + dbExecutor.execute(this::loadMailboxProperties); + } + + @DatabaseExecutor + private void loadMailboxProperties() { + LOG.info("Loading mailbox properties"); + try { + db.transaction(true, txn -> { + Map updates = new HashMap<>(); + for (Contact c : contactManager.getContacts(txn)) { + MailboxUpdate local = mailboxUpdateManager + .getLocalUpdate(txn, c.getId()); + MailboxUpdate remote = mailboxUpdateManager + .getRemoteUpdate(txn, c.getId()); + updates.put(c.getId(), new Updates(local, remote)); + } + MailboxProperties ownProps = + mailboxSettingsManager.getOwnMailboxProperties(txn); + // Use a commit action so the state in memory remains + // consistent with the state in the DB + txn.attach(() -> initialiseState(updates, ownProps)); + }); + } catch (DbException e) { + logException(LOG, WARNING, e); + } + } + + @EventExecutor + private void initialiseState(Map updates, + @Nullable MailboxProperties ownProps) { + contactUpdates.putAll(updates); + ownProperties = ownProps; + Plugin tor = pluginManager.getPlugin(ID); + if (tor != null && tor.getState() == ACTIVE) { + LOG.info("Online"); + online = true; + createClients(); + } + handleEvents = true; + } + + @EventExecutor + private void createClients() { + LOG.info("Creating clients"); + for (Entry e : contactUpdates.entrySet()) { + ContactId c = e.getKey(); + Updates u = e.getValue(); + if (isContactMailboxUsable(u.remote)) { + // Create and start a client for the contact's mailbox + MailboxClient contactClient = createAndStartClient(c); + // Assign the contact to the contact's mailbox for upload + assignContactToContactMailboxForUpload(c, contactClient, u); + if (!isOwnMailboxUsable(ownProperties, u.remote)) { + // We don't have a usable mailbox, so assign the contact to + // the contact's mailbox for download too + assignContactToContactMailboxForDownload(c, + contactClient, u); + } + } + } + if (ownProperties == null) return; + if (!isOwnMailboxUsable(ownProperties)) { + LOG.warning("We have a mailbox but we can't use it"); + return; + } + // Create and start a client for our mailbox + createAndStartClientForOwnMailbox(); + // Assign contacts to our mailbox for upload/download + for (Entry e : contactUpdates.entrySet()) { + ContactId c = e.getKey(); + Updates u = e.getValue(); + if (isOwnMailboxUsable(ownProperties, u.remote)) { + // Assign the contact to our mailbox for download + assignContactToOwnMailboxForDownload(c, u); + if (!isContactMailboxUsable(u.remote)) { + // The contact doesn't have a usable mailbox, so assign + // the contact to our mailbox for upload too + assignContactToOwnMailboxForUpload(c, u); + } + } + } + } + + @Override + public void stopService() throws ServiceException { + CountDownLatch latch = new CountDownLatch(1); + eventExecutor.execute(() -> { + handleEvents = false; + destroyClients(); + latch.countDown(); + }); + reachabilityMonitor.destroy(); + try { + latch.await(); + } catch (InterruptedException e) { + throw new ServiceException(e); + } + } + + @EventExecutor + private void destroyClients() { + LOG.info("Destroying clients"); + for (MailboxClient client : contactClients.values()) { + client.destroy(); + } + contactClients.clear(); + destroyOwnClient(); + } + + @EventExecutor + private void destroyOwnClient() { + if (ownClient != null) { + ownClient.destroy(); + ownClient = null; + } + } + + @Override + public void eventOccurred(Event e) { + if (!handleEvents) return; + if (e instanceof TransportActiveEvent) { + TransportActiveEvent t = (TransportActiveEvent) e; + if (t.getTransportId().equals(ID)) onTorActive(); + } else if (e instanceof TransportInactiveEvent) { + TransportInactiveEvent t = (TransportInactiveEvent) e; + if (t.getTransportId().equals(ID)) onTorInactive(); + } else if (e instanceof MailboxPairedEvent) { + LOG.info("Mailbox paired"); + MailboxPairedEvent m = (MailboxPairedEvent) e; + onMailboxPaired(m.getProperties(), m.getLocalUpdates()); + } else if (e instanceof MailboxUnpairedEvent) { + LOG.info("Mailbox unpaired"); + MailboxUnpairedEvent m = (MailboxUnpairedEvent) e; + onMailboxUnpaired(m.getLocalUpdates()); + } else if (e instanceof MailboxUpdateSentEvent) { + LOG.info("Contact added"); + MailboxUpdateSentEvent m = (MailboxUpdateSentEvent) e; + onContactAdded(m.getContactId(), m.getMailboxUpdate()); + } else if (e instanceof ContactRemovedEvent) { + LOG.info("Contact removed"); + ContactRemovedEvent c = (ContactRemovedEvent) e; + onContactRemoved(c.getContactId()); + } else if (e instanceof RemoteMailboxUpdateEvent) { + LOG.info("Remote mailbox update"); + RemoteMailboxUpdateEvent r = (RemoteMailboxUpdateEvent) e; + onRemoteMailboxUpdate(r.getContact(), r.getMailboxUpdate()); + } else if (e instanceof OwnMailboxConnectionStatusEvent) { + OwnMailboxConnectionStatusEvent o = + (OwnMailboxConnectionStatusEvent) e; + onOwnMailboxConnectionStatusChanged(o.getStatus()); + } + } + + @EventExecutor + private void onTorActive() { + // If we checked the plugin at startup concurrently with the plugin + // becoming active then `online` may already be true when we receive + // the first TransportActiveEvent, in which case ignore it + if (online) return; + LOG.info("Online"); + online = true; + createClients(); + } + + @EventExecutor + private void onTorInactive() { + // If we checked the plugin at startup concurrently with the plugin + // becoming inactive then `online` may already be false when we + // receive the first TransportInactiveEvent, in which case ignore it + if (!online) return; + LOG.info("Offline"); + online = false; + destroyClients(); + } + + @EventExecutor + private void onMailboxPaired(MailboxProperties ownProps, + Map localUpdates) { + for (Entry e : + localUpdates.entrySet()) { + ContactId c = e.getKey(); + Updates u = contactUpdates.get(c); + contactUpdates.put(c, new Updates(e.getValue(), u.remote)); + } + ownProperties = ownProps; + if (!online) return; + if (!isOwnMailboxUsable(ownProperties)) { + LOG.warning("We have a mailbox but we can't use it"); + return; + } + // Create and start a client for our mailbox + createAndStartClientForOwnMailbox(); + // Assign contacts to our mailbox for upload/download + for (Entry e : contactUpdates.entrySet()) { + ContactId c = e.getKey(); + Updates u = e.getValue(); + boolean isOwnMailboxUsable = + isOwnMailboxUsable(ownProperties, u.remote); + if (isContactMailboxUsable(u.remote)) { + // The contact has a usable mailbox, so the contact should + // currently be assigned to the contact's mailbox for upload + // and download + if (isOwnMailboxUsable) { + // Reassign the contact to our mailbox for download + MailboxClient contactClient = + requireNonNull(contactClients.get(c)); + contactClient.deassignContactForDownload(c); + assignContactToOwnMailboxForDownload(c, u); + } + // Else the contact remains assigned to the contact's mailbox + // for download + } else if (isOwnMailboxUsable) { + // The contact doesn't have a usable mailbox, so assign the + // contact to our mailbox for upload and download + assignContactToOwnMailboxForUpload(c, u); + assignContactToOwnMailboxForDownload(c, u); + } + // Else the contact doesn't have a usable mailbox and neither do we, + // so the contact remains unassigned for upload and download + } + } + + @EventExecutor + private void onMailboxUnpaired(Map localUpdates) { + for (Entry e : localUpdates.entrySet()) { + ContactId c = e.getKey(); + Updates updates = contactUpdates.get(c); + contactUpdates.put(c, new Updates(e.getValue(), updates.remote)); + } + MailboxProperties oldOwnProperties = ownProperties; + ownProperties = null; + if (!online) return; + // Destroy the client for our own mailbox, if any + destroyOwnClient(); + // Reassign contacts to their own mailboxes for download where possible + for (Entry e : contactUpdates.entrySet()) { + ContactId c = e.getKey(); + Updates u = e.getValue(); + if (isContactMailboxUsable(u.remote) && + isOwnMailboxUsable(oldOwnProperties, u.remote)) { + // The contact should currently be assigned to our mailbox + // for download. Reassign the contact to the contact's + // mailbox for download + MailboxClient contactClient = + requireNonNull(contactClients.get(c)); + assignContactToContactMailboxForDownload(c, contactClient, u); + } + } + } + + @EventExecutor + private void onContactAdded(ContactId c, MailboxUpdate u) { + Updates old = contactUpdates.put(c, new Updates(u, null)); + if (old != null) throw new IllegalStateException(); + // We haven't yet received an update from the newly added contact, + // so at this stage we don't need to assign the contact to any + // mailboxes for upload or download + } + + @EventExecutor + private void onContactRemoved(ContactId c) { + Updates updates = requireNonNull(contactUpdates.remove(c)); + if (!online) return; + // Destroy the client for the contact's mailbox, if any + MailboxClient client = contactClients.remove(c); + if (client != null) client.destroy(); + // If we have a mailbox and the contact is assigned to it for upload + // and/or download, deassign the contact + if (ownProperties == null) return; + if (isOwnMailboxUsable(ownProperties, updates.remote)) { + // We have a usable mailbox, so the contact should currently be + // assigned to our mailbox for download. Deassign the contact from + // our mailbox for download + requireNonNull(ownClient).deassignContactForDownload(c); + if (!isContactMailboxUsable(updates.remote)) { + // The contact doesn't have a usable mailbox, so the contact + // should currently be assigned to our mailbox for upload. + // Deassign the contact from our mailbox for upload + requireNonNull(ownClient).deassignContactForUpload(c); + } + } + } + + @EventExecutor + private void onRemoteMailboxUpdate(ContactId c, MailboxUpdate remote) { + Updates old = contactUpdates.get(c); + MailboxUpdate oldRemote = old.remote; + Updates u = new Updates(old.local, remote); + contactUpdates.put(c, u); + if (!online) return; + // What may have changed? + // * Contact's mailbox may be usable now, was unusable before + // * Contact's mailbox may be unusable now, was usable before + // * Contact's mailbox may have been replaced + // * Contact's mailbox may have changed its API versions + // * Contact may be able to use our mailbox now, was unable before + // * Contact may be unable to use our mailbox now, was able before + boolean wasContactMailboxUsable = isContactMailboxUsable(oldRemote); + boolean isContactMailboxUsable = isContactMailboxUsable(remote); + boolean wasOwnMailboxUsable = + isOwnMailboxUsable(ownProperties, oldRemote); + boolean isOwnMailboxUsable = isOwnMailboxUsable(ownProperties, remote); + + // Create/destroy/replace the client for the contact's mailbox if needed + MailboxClient contactClient = null; + boolean clientReplaced = false; + if (isContactMailboxUsable) { + if (wasContactMailboxUsable) { + MailboxProperties oldProps = getMailboxProperties(oldRemote); + MailboxProperties newProps = getMailboxProperties(remote); + if (oldProps.equals(newProps)) { + // The contact previously had a usable mailbox, now has + // a usable mailbox, it's the same mailbox, and its API + // versions haven't changed. Keep using the existing client + contactClient = requireNonNull(contactClients.get(c)); + } else { + // The contact previously had a usable mailbox and now has + // a usable mailbox, but either it's a new mailbox or its + // API versions have changed. Replace the client + requireNonNull(contactClients.remove(c)).destroy(); + contactClient = createAndStartClient(c); + clientReplaced = true; + } + } else { + // The client didn't previously have a usable mailbox but now + // has one. Create and start a client + contactClient = createAndStartClient(c); + } + } else if (wasContactMailboxUsable) { + // The client previously had a usable mailbox but no longer does. + // Destroy the existing client + requireNonNull(contactClients.remove(c)).destroy(); + } + + boolean wasAssignedToOwnMailboxForUpload = + wasOwnMailboxUsable && !wasContactMailboxUsable; + boolean willBeAssignedToOwnMailboxForUpload = + isOwnMailboxUsable && !isContactMailboxUsable; + + boolean wasAssignedToContactMailboxForDownload = + !wasOwnMailboxUsable && wasContactMailboxUsable; + boolean willBeAssignedToContactMailboxForDownload = + !isOwnMailboxUsable && isContactMailboxUsable; + + // Deassign the contact for upload/download if needed + if (wasAssignedToOwnMailboxForUpload && + !willBeAssignedToOwnMailboxForUpload) { + requireNonNull(ownClient).deassignContactForUpload(c); + } + if (wasOwnMailboxUsable && !isOwnMailboxUsable) { + requireNonNull(ownClient).deassignContactForDownload(c); + } + // If the client for the contact's mailbox was replaced or destroyed + // above then we don't need to deassign the contact for download + if (wasAssignedToContactMailboxForDownload && + !willBeAssignedToContactMailboxForDownload && + !clientReplaced && isContactMailboxUsable) { + requireNonNull(contactClient).deassignContactForDownload(c); + } + // We never need to deassign the contact from the contact's mailbox for + // upload: this would only be needed if the contact's mailbox were no + // longer usable, in which case the client would already have been + // destroyed above. Thanks to the linter for spotting this + + // Assign the contact for upload/download if needed + if (!wasAssignedToOwnMailboxForUpload && + willBeAssignedToOwnMailboxForUpload) { + assignContactToOwnMailboxForUpload(c, u); + } + if (!wasOwnMailboxUsable && isOwnMailboxUsable) { + assignContactToOwnMailboxForDownload(c, u); + } + if ((!wasContactMailboxUsable || clientReplaced) && + isContactMailboxUsable) { + assignContactToContactMailboxForUpload(c, contactClient, u); + } + if ((!wasAssignedToContactMailboxForDownload || clientReplaced) && + willBeAssignedToContactMailboxForDownload) { + assignContactToContactMailboxForDownload(c, contactClient, u); + } + } + + @EventExecutor + private void onOwnMailboxConnectionStatusChanged(MailboxStatus status) { + if (!online || ownProperties == null) return; + List oldServerSupports = + ownProperties.getServerSupports(); + List newServerSupports = status.getServerSupports(); + if (!oldServerSupports.equals(newServerSupports)) { + LOG.info("Our mailbox's supported API versions have changed"); + // This potentially affects every assignment of contacts to + // mailboxes for upload and download, so just rebuild the clients + // and assignments from scratch + destroyClients(); + createClients(); + } + } + + @EventExecutor + private void createAndStartClientForOwnMailbox() { + if (ownClient != null) throw new IllegalStateException(); + ownClient = mailboxClientFactory.createOwnMailboxClient( + reachabilityMonitor, requireNonNull(ownProperties)); + ownClient.start(); + } + + @EventExecutor + private MailboxClient createAndStartClient(ContactId c) { + MailboxClient client = mailboxClientFactory + .createContactMailboxClient(reachabilityMonitor); + MailboxClient old = contactClients.put(c, client); + if (old != null) throw new IllegalStateException(); + client.start(); + return client; + } + + @EventExecutor + private void assignContactToOwnMailboxForDownload(ContactId c, Updates u) { + MailboxProperties localProps = getMailboxProperties(u.local); + requireNonNull(ownClient).assignContactForDownload(c, + requireNonNull(ownProperties), + requireNonNull(localProps.getOutboxId())); + } + + @EventExecutor + private void assignContactToOwnMailboxForUpload(ContactId c, Updates u) { + MailboxProperties localProps = getMailboxProperties(u.local); + requireNonNull(ownClient).assignContactForUpload(c, + requireNonNull(ownProperties), + requireNonNull(localProps.getInboxId())); + } + + @EventExecutor + private void assignContactToContactMailboxForDownload(ContactId c, + MailboxClient contactClient, Updates u) { + MailboxProperties remoteProps = + getMailboxProperties(requireNonNull(u.remote)); + contactClient.assignContactForDownload(c, remoteProps, + requireNonNull(remoteProps.getInboxId())); + } + + @EventExecutor + private void assignContactToContactMailboxForUpload(ContactId c, + MailboxClient contactClient, Updates u) { + MailboxProperties remoteProps = + getMailboxProperties(requireNonNull(u.remote)); + contactClient.assignContactForUpload(c, remoteProps, + requireNonNull(remoteProps.getOutboxId())); + } + + private MailboxProperties getMailboxProperties(MailboxUpdate update) { + if (!(update instanceof MailboxUpdateWithMailbox)) { + throw new IllegalArgumentException(); + } + MailboxUpdateWithMailbox mailbox = (MailboxUpdateWithMailbox) update; + return mailbox.getMailboxProperties(); + } + + /** + * Returns true if we can use our own mailbox to communicate with the + * contact that sent the given update. + */ + private boolean isOwnMailboxUsable( + @Nullable MailboxProperties ownProperties, + @Nullable MailboxUpdate remote) { + if (ownProperties == null || remote == null) return false; + return isMailboxUsable(remote.getClientSupports(), + ownProperties.getServerSupports()); + } + + /** + * Returns true if we can use the contact's mailbox to communicate with + * the contact that sent the given update. + */ + private boolean isContactMailboxUsable(@Nullable MailboxUpdate remote) { + if (remote instanceof MailboxUpdateWithMailbox) { + MailboxUpdateWithMailbox remoteMailbox = + (MailboxUpdateWithMailbox) remote; + return isMailboxUsable(remoteMailbox.getClientSupports(), + remoteMailbox.getMailboxProperties().getServerSupports()); + } + return false; + } + + /** + * Returns true if we can communicate with a contact that has the given + * client-supported API versions via a mailbox with the given + * server-supported API versions. + */ + private boolean isMailboxUsable(List contactClient, + List server) { + return getHighestCommonMajorVersion(contactClient, server) >= 0 + && getHighestCommonMajorVersion(CLIENT_SUPPORTS, server) >= 0; + } + + /** + * Returns true if we're compatible with our own mailbox. + */ + @SuppressWarnings("BooleanMethodIsAlwaysInverted") + private boolean isOwnMailboxUsable(MailboxProperties ownProperties) { + return getHighestCommonMajorVersion(CLIENT_SUPPORTS, + ownProperties.getServerSupports()) >= 0; + } + + private static class Updates { + + private final MailboxUpdate local; + @Nullable + private final MailboxUpdate remote; + + private Updates(MailboxUpdate local, @Nullable MailboxUpdate remote) { + this.local = local; + this.remote = remote; + } + } +} From 339e4dadedde5cd5dc13d233c419cf6ee082caeb Mon Sep 17 00:00:00 2001 From: akwizgran Date: Wed, 3 Aug 2022 15:22:11 +0100 Subject: [PATCH 2/8] Update Dagger modules. --- .../bramble/BrambleAndroidModule.java | 2 + .../briarproject/bramble/io}/DnsModule.java | 2 +- .../org/briarproject/bramble/io}/NoDns.java | 5 +- .../bramble/mailbox/MailboxModule.java | 52 +++++++++++++++++++ ...ntactExchangeIntegrationTestComponent.java | 6 ++- ...emovableDriveIntegrationTestComponent.java | 6 ++- .../sync/SyncIntegrationTestComponent.java | 6 ++- .../test/BrambleIntegrationTestComponent.java | 4 +- .../bramble}/test/TestDnsModule.java | 2 +- .../TransportKeyAgreementTestComponent.java | 6 ++- .../bramble/BrambleJavaModule.java | 2 + .../bramble/plugin/tor/BridgeTest.java | 17 +++--- .../BrambleJavaIntegrationTestComponent.java | 3 +- .../bramble/test/TestTorPortsModule.java | 26 ++++++++++ .../briarproject/briar/BriarCoreModule.java | 2 - .../FeedManagerIntegrationTestComponent.java | 4 +- .../IntroductionIntegrationTestComponent.java | 6 ++- .../MessageSizeIntegrationTestComponent.java | 6 ++- ...plexMessagingIntegrationTestComponent.java | 6 ++- .../test/BriarIntegrationTestComponent.java | 6 ++- .../briar/headless/BriarHeadlessApp.kt | 2 + .../briar/headless/HeadlessModule.kt | 10 +--- .../briar/headless/BriarHeadlessTestApp.kt | 2 + .../briar/headless/HeadlessTestModule.kt | 8 --- 24 files changed, 150 insertions(+), 41 deletions(-) rename {briar-core/src/main/java/org/briarproject/briar/feed => bramble-core/src/main/java/org/briarproject/bramble/io}/DnsModule.java (86%) rename {briar-core/src/main/java/org/briarproject/briar/feed => bramble-core/src/main/java/org/briarproject/bramble/io}/NoDns.java (81%) rename {briar-core/src/test/java/org/briarproject/briar => bramble-core/src/test/java/org/briarproject/bramble}/test/TestDnsModule.java (80%) create mode 100644 bramble-java/src/test/java/org/briarproject/bramble/test/TestTorPortsModule.java diff --git a/bramble-android/src/main/java/org/briarproject/bramble/BrambleAndroidModule.java b/bramble-android/src/main/java/org/briarproject/bramble/BrambleAndroidModule.java index 917926ed2..5be846f61 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/BrambleAndroidModule.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/BrambleAndroidModule.java @@ -1,6 +1,7 @@ package org.briarproject.bramble; import org.briarproject.bramble.battery.AndroidBatteryModule; +import org.briarproject.bramble.io.DnsModule; import org.briarproject.bramble.network.AndroidNetworkModule; import org.briarproject.bramble.plugin.tor.CircumventionModule; import org.briarproject.bramble.reporting.ReportingModule; @@ -18,6 +19,7 @@ import dagger.Module; AndroidTaskSchedulerModule.class, AndroidWakefulIoExecutorModule.class, CircumventionModule.class, + DnsModule.class, ReportingModule.class, SocksModule.class }) diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/DnsModule.java b/bramble-core/src/main/java/org/briarproject/bramble/io/DnsModule.java similarity index 86% rename from briar-core/src/main/java/org/briarproject/briar/feed/DnsModule.java rename to bramble-core/src/main/java/org/briarproject/bramble/io/DnsModule.java index dc6cd8a88..b489d11f9 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/DnsModule.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/io/DnsModule.java @@ -1,4 +1,4 @@ -package org.briarproject.briar.feed; +package org.briarproject.bramble.io; import dagger.Module; import dagger.Provides; diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/NoDns.java b/bramble-core/src/main/java/org/briarproject/bramble/io/NoDns.java similarity index 81% rename from briar-core/src/main/java/org/briarproject/briar/feed/NoDns.java rename to bramble-core/src/main/java/org/briarproject/bramble/io/NoDns.java index 5f5cc8c44..91a8616ed 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/NoDns.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/io/NoDns.java @@ -1,4 +1,6 @@ -package org.briarproject.briar.feed; +package org.briarproject.bramble.io; + +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import java.net.InetAddress; import java.net.UnknownHostException; @@ -9,6 +11,7 @@ import javax.inject.Inject; import okhttp3.Dns; +@NotNullByDefault class NoDns implements Dns { private static final byte[] UNSPECIFIED_ADDRESS = new byte[4]; diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxModule.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxModule.java index 73e418ed4..3d7845799 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxModule.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxModule.java @@ -4,17 +4,22 @@ import org.briarproject.bramble.api.FeatureFlags; import org.briarproject.bramble.api.client.ClientHelper; import org.briarproject.bramble.api.contact.ContactManager; import org.briarproject.bramble.api.data.MetadataEncoder; +import org.briarproject.bramble.api.db.DatabaseExecutor; +import org.briarproject.bramble.api.db.TransactionManager; import org.briarproject.bramble.api.event.EventBus; +import org.briarproject.bramble.api.event.EventExecutor; import org.briarproject.bramble.api.lifecycle.LifecycleManager; import org.briarproject.bramble.api.mailbox.MailboxManager; import org.briarproject.bramble.api.mailbox.MailboxSettingsManager; import org.briarproject.bramble.api.mailbox.MailboxUpdateManager; import org.briarproject.bramble.api.mailbox.MailboxVersion; +import org.briarproject.bramble.api.plugin.PluginManager; import org.briarproject.bramble.api.sync.validation.ValidationManager; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.api.versioning.ClientVersioningManager; import java.util.List; +import java.util.concurrent.Executor; import javax.inject.Inject; import javax.inject.Singleton; @@ -37,6 +42,8 @@ public class MailboxModule { MailboxUpdateManager mailboxUpdateManager; @Inject MailboxFileManager mailboxFileManager; + @Inject + MailboxClientManager mailboxClientManager; } @Provides @@ -126,4 +133,49 @@ public class MailboxModule { MailboxWorkerFactoryImpl mailboxWorkerFactory) { return mailboxWorkerFactory; } + + @Provides + MailboxClientFactory provideMailboxClientFactory( + MailboxClientFactoryImpl mailboxClientFactory) { + return mailboxClientFactory; + } + + @Provides + MailboxApiCaller provideMailboxApiCaller( + MailboxApiCallerImpl mailboxApiCaller) { + return mailboxApiCaller; + } + + @Provides + @Singleton + TorReachabilityMonitor provideTorReachabilityMonitor( + TorReachabilityMonitorImpl reachabilityMonitor) { + return reachabilityMonitor; + } + + @Provides + @Singleton + MailboxClientManager provideMailboxClientManager( + @EventExecutor Executor eventExecutor, + @DatabaseExecutor Executor dbExecutor, + TransactionManager db, + ContactManager contactManager, + PluginManager pluginManager, + MailboxSettingsManager mailboxSettingsManager, + MailboxUpdateManager mailboxUpdateManager, + MailboxClientFactory mailboxClientFactory, + TorReachabilityMonitor reachabilityMonitor, + FeatureFlags featureFlags, + LifecycleManager lifecycleManager, + EventBus eventBus) { + MailboxClientManager manager = new MailboxClientManager(eventExecutor, + dbExecutor, db, contactManager, pluginManager, + mailboxSettingsManager, mailboxUpdateManager, + mailboxClientFactory, reachabilityMonitor); + if (featureFlags.shouldEnableMailbox()) { + lifecycleManager.registerService(manager); + eventBus.addListener(manager); + } + return manager; + } } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/contact/ContactExchangeIntegrationTestComponent.java b/bramble-core/src/test/java/org/briarproject/bramble/contact/ContactExchangeIntegrationTestComponent.java index 99afc4005..97f1eeaf7 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/contact/ContactExchangeIntegrationTestComponent.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/contact/ContactExchangeIntegrationTestComponent.java @@ -10,6 +10,8 @@ import org.briarproject.bramble.api.identity.IdentityManager; import org.briarproject.bramble.api.lifecycle.IoExecutor; import org.briarproject.bramble.api.lifecycle.LifecycleManager; import org.briarproject.bramble.test.BrambleCoreIntegrationTestModule; +import org.briarproject.bramble.test.TestDnsModule; +import org.briarproject.bramble.test.TestSocksModule; import java.util.concurrent.Executor; @@ -20,7 +22,9 @@ import dagger.Component; @Singleton @Component(modules = { BrambleCoreIntegrationTestModule.class, - BrambleCoreModule.class + BrambleCoreModule.class, + TestDnsModule.class, + TestSocksModule.class }) interface ContactExchangeIntegrationTestComponent extends BrambleCoreIntegrationTestEagerSingletons { diff --git a/bramble-core/src/test/java/org/briarproject/bramble/plugin/file/RemovableDriveIntegrationTestComponent.java b/bramble-core/src/test/java/org/briarproject/bramble/plugin/file/RemovableDriveIntegrationTestComponent.java index 32c0471aa..6c5080dc6 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/plugin/file/RemovableDriveIntegrationTestComponent.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/plugin/file/RemovableDriveIntegrationTestComponent.java @@ -12,9 +12,11 @@ import org.briarproject.bramble.event.DefaultEventExecutorModule; import org.briarproject.bramble.system.DefaultWakefulIoExecutorModule; import org.briarproject.bramble.system.TimeTravelModule; import org.briarproject.bramble.test.TestDatabaseConfigModule; +import org.briarproject.bramble.test.TestDnsModule; import org.briarproject.bramble.test.TestFeatureFlagModule; import org.briarproject.bramble.test.TestMailboxDirectoryModule; import org.briarproject.bramble.test.TestSecureRandomModule; +import org.briarproject.bramble.test.TestSocksModule; import javax.inject.Singleton; @@ -27,12 +29,14 @@ import dagger.Component; DefaultEventExecutorModule.class, DefaultWakefulIoExecutorModule.class, TestDatabaseConfigModule.class, + TestDnsModule.class, TestFeatureFlagModule.class, TestMailboxDirectoryModule.class, RemovableDriveIntegrationTestModule.class, RemovableDriveModule.class, TestSecureRandomModule.class, - TimeTravelModule.class + TimeTravelModule.class, + TestSocksModule.class }) interface RemovableDriveIntegrationTestComponent extends BrambleCoreEagerSingletons { diff --git a/bramble-core/src/test/java/org/briarproject/bramble/sync/SyncIntegrationTestComponent.java b/bramble-core/src/test/java/org/briarproject/bramble/sync/SyncIntegrationTestComponent.java index 701d706ad..4a0a8a83a 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/sync/SyncIntegrationTestComponent.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/sync/SyncIntegrationTestComponent.java @@ -3,6 +3,8 @@ package org.briarproject.bramble.sync; import org.briarproject.bramble.BrambleCoreIntegrationTestEagerSingletons; import org.briarproject.bramble.BrambleCoreModule; import org.briarproject.bramble.test.BrambleCoreIntegrationTestModule; +import org.briarproject.bramble.test.TestDnsModule; +import org.briarproject.bramble.test.TestSocksModule; import javax.inject.Singleton; @@ -11,7 +13,9 @@ import dagger.Component; @Singleton @Component(modules = { BrambleCoreIntegrationTestModule.class, - BrambleCoreModule.class + BrambleCoreModule.class, + TestDnsModule.class, + TestSocksModule.class }) interface SyncIntegrationTestComponent extends BrambleCoreIntegrationTestEagerSingletons { diff --git a/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTestComponent.java b/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTestComponent.java index c7b0d80c5..444da32a3 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTestComponent.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTestComponent.java @@ -14,7 +14,9 @@ import dagger.Component; @Singleton @Component(modules = { BrambleCoreIntegrationTestModule.class, - BrambleCoreModule.class + BrambleCoreModule.class, + TestDnsModule.class, + TestSocksModule.class }) public interface BrambleIntegrationTestComponent extends BrambleCoreIntegrationTestEagerSingletons { diff --git a/briar-core/src/test/java/org/briarproject/briar/test/TestDnsModule.java b/bramble-core/src/test/java/org/briarproject/bramble/test/TestDnsModule.java similarity index 80% rename from briar-core/src/test/java/org/briarproject/briar/test/TestDnsModule.java rename to bramble-core/src/test/java/org/briarproject/bramble/test/TestDnsModule.java index 13323b346..f18aac8d5 100644 --- a/briar-core/src/test/java/org/briarproject/briar/test/TestDnsModule.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/test/TestDnsModule.java @@ -1,4 +1,4 @@ -package org.briarproject.briar.test; +package org.briarproject.bramble.test; import dagger.Module; import dagger.Provides; diff --git a/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementTestComponent.java b/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementTestComponent.java index a0de3c5e4..a54852b17 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementTestComponent.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementTestComponent.java @@ -9,6 +9,8 @@ import org.briarproject.bramble.api.properties.TransportPropertyManager; import org.briarproject.bramble.api.transport.KeyManager; import org.briarproject.bramble.test.BrambleCoreIntegrationTestModule; import org.briarproject.bramble.test.BrambleIntegrationTestComponent; +import org.briarproject.bramble.test.TestDnsModule; +import org.briarproject.bramble.test.TestSocksModule; import javax.inject.Singleton; @@ -17,7 +19,9 @@ import dagger.Component; @Singleton @Component(modules = { BrambleCoreIntegrationTestModule.class, - BrambleCoreModule.class + BrambleCoreModule.class, + TestDnsModule.class, + TestSocksModule.class }) interface TransportKeyAgreementTestComponent extends BrambleIntegrationTestComponent { diff --git a/bramble-java/src/main/java/org/briarproject/bramble/BrambleJavaModule.java b/bramble-java/src/main/java/org/briarproject/bramble/BrambleJavaModule.java index f50ea15ae..cdb764909 100644 --- a/bramble-java/src/main/java/org/briarproject/bramble/BrambleJavaModule.java +++ b/bramble-java/src/main/java/org/briarproject/bramble/BrambleJavaModule.java @@ -1,5 +1,6 @@ package org.briarproject.bramble; +import org.briarproject.bramble.io.DnsModule; import org.briarproject.bramble.network.JavaNetworkModule; import org.briarproject.bramble.plugin.tor.CircumventionModule; import org.briarproject.bramble.socks.SocksModule; @@ -9,6 +10,7 @@ import dagger.Module; @Module(includes = { CircumventionModule.class, + DnsModule.class, JavaNetworkModule.class, JavaSystemModule.class, SocksModule.class diff --git a/bramble-java/src/test/java/org/briarproject/bramble/plugin/tor/BridgeTest.java b/bramble-java/src/test/java/org/briarproject/bramble/plugin/tor/BridgeTest.java index 4e4b0524f..79818a349 100644 --- a/bramble-java/src/test/java/org/briarproject/bramble/plugin/tor/BridgeTest.java +++ b/bramble-java/src/test/java/org/briarproject/bramble/plugin/tor/BridgeTest.java @@ -9,6 +9,8 @@ import org.briarproject.bramble.api.lifecycle.IoExecutor; import org.briarproject.bramble.api.network.NetworkManager; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.BackoffFactory; +import org.briarproject.bramble.api.plugin.TorControlPort; +import org.briarproject.bramble.api.plugin.TorSocksPort; import org.briarproject.bramble.api.plugin.duplex.DuplexPlugin; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.api.system.LocationUtils; @@ -42,8 +44,6 @@ import static java.util.Collections.singletonList; import static java.util.concurrent.TimeUnit.MINUTES; import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.api.plugin.Plugin.State.ACTIVE; -import static org.briarproject.bramble.api.plugin.TorConstants.DEFAULT_CONTROL_PORT; -import static org.briarproject.bramble.api.plugin.TorConstants.DEFAULT_SOCKS_PORT; import static org.briarproject.bramble.plugin.tor.CircumventionProvider.BridgeType.DEFAULT_OBFS4; import static org.briarproject.bramble.plugin.tor.CircumventionProvider.BridgeType.MEEK; import static org.briarproject.bramble.plugin.tor.CircumventionProvider.BridgeType.NON_DEFAULT_OBFS4; @@ -89,9 +89,6 @@ public class BridgeTest extends BrambleTestCase { private final static long MEEK_TIMEOUT = MINUTES.toMillis(6); private final static int UNREACHABLE_BRIDGES_ALLOWED = 6; private final static int ATTEMPTS_PER_BRIDGE = 5; - // Use different ports from Briar Desktop to avoid conflicts - private final static int SOCKS_PORT = DEFAULT_SOCKS_PORT + 10; - private final static int CONTROL_PORT = DEFAULT_CONTROL_PORT + 10; private final static Logger LOG = getLogger(BridgeTest.class.getName()); @@ -115,6 +112,12 @@ public class BridgeTest extends BrambleTestCase { Clock clock; @Inject CryptoComponent crypto; + @Inject + @TorSocksPort + int torSocksPort; + @Inject + @TorControlPort + int torControlPort; private final File torDir = getTestDirectory(); private final Params params; @@ -167,8 +170,8 @@ public class BridgeTest extends BrambleTestCase { factory = new UnixTorPluginFactory(ioExecutor, wakefulIoExecutor, networkManager, locationUtils, eventBus, torSocketFactory, backoffFactory, resourceProvider, bridgeProvider, - batteryManager, clock, crypto, torDir, - SOCKS_PORT, CONTROL_PORT); + batteryManager, clock, crypto, torDir, torSocksPort, + torControlPort); } @After diff --git a/bramble-java/src/test/java/org/briarproject/bramble/test/BrambleJavaIntegrationTestComponent.java b/bramble-java/src/test/java/org/briarproject/bramble/test/BrambleJavaIntegrationTestComponent.java index 489f83408..857f408e6 100644 --- a/bramble-java/src/test/java/org/briarproject/bramble/test/BrambleJavaIntegrationTestComponent.java +++ b/bramble-java/src/test/java/org/briarproject/bramble/test/BrambleJavaIntegrationTestComponent.java @@ -14,7 +14,8 @@ import dagger.Component; @Component(modules = { BrambleCoreIntegrationTestModule.class, BrambleCoreModule.class, - BrambleJavaModule.class + BrambleJavaModule.class, + TestTorPortsModule.class }) public interface BrambleJavaIntegrationTestComponent extends BrambleCoreIntegrationTestEagerSingletons { diff --git a/bramble-java/src/test/java/org/briarproject/bramble/test/TestTorPortsModule.java b/bramble-java/src/test/java/org/briarproject/bramble/test/TestTorPortsModule.java new file mode 100644 index 000000000..a8f03045d --- /dev/null +++ b/bramble-java/src/test/java/org/briarproject/bramble/test/TestTorPortsModule.java @@ -0,0 +1,26 @@ +package org.briarproject.bramble.test; + +import org.briarproject.bramble.api.plugin.TorControlPort; +import org.briarproject.bramble.api.plugin.TorSocksPort; + +import dagger.Module; +import dagger.Provides; + +import static org.briarproject.bramble.api.plugin.TorConstants.DEFAULT_CONTROL_PORT; +import static org.briarproject.bramble.api.plugin.TorConstants.DEFAULT_SOCKS_PORT; + +@Module +class TestTorPortsModule { + + @Provides + @TorSocksPort + int provideTorSocksPort() { + return DEFAULT_SOCKS_PORT + 10; + } + + @Provides + @TorControlPort + int provideTorControlPort() { + return DEFAULT_CONTROL_PORT + 10; + } +} diff --git a/briar-core/src/main/java/org/briarproject/briar/BriarCoreModule.java b/briar-core/src/main/java/org/briarproject/briar/BriarCoreModule.java index 9211d9201..fbd15acfd 100644 --- a/briar-core/src/main/java/org/briarproject/briar/BriarCoreModule.java +++ b/briar-core/src/main/java/org/briarproject/briar/BriarCoreModule.java @@ -6,7 +6,6 @@ import org.briarproject.briar.avatar.AvatarModule; import org.briarproject.briar.blog.BlogModule; import org.briarproject.briar.client.BriarClientModule; import org.briarproject.briar.conversation.ConversationModule; -import org.briarproject.briar.feed.DnsModule; import org.briarproject.briar.feed.FeedModule; import org.briarproject.briar.forum.ForumModule; import org.briarproject.briar.identity.IdentityModule; @@ -26,7 +25,6 @@ import dagger.Module; BlogModule.class, BriarClientModule.class, ConversationModule.class, - DnsModule.class, FeedModule.class, ForumModule.class, GroupInvitationModule.class, diff --git a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerIntegrationTestComponent.java b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerIntegrationTestComponent.java index 78b6c0f1c..8cdaa7bb3 100644 --- a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerIntegrationTestComponent.java +++ b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerIntegrationTestComponent.java @@ -5,6 +5,7 @@ import org.briarproject.bramble.BrambleCoreModule; import org.briarproject.bramble.api.identity.IdentityManager; import org.briarproject.bramble.api.lifecycle.LifecycleManager; import org.briarproject.bramble.test.BrambleCoreIntegrationTestModule; +import org.briarproject.bramble.test.TestDnsModule; import org.briarproject.bramble.test.TestSocksModule; import org.briarproject.briar.api.blog.BlogManager; import org.briarproject.briar.api.feed.FeedManager; @@ -12,7 +13,6 @@ import org.briarproject.briar.avatar.AvatarModule; import org.briarproject.briar.blog.BlogModule; import org.briarproject.briar.client.BriarClientModule; import org.briarproject.briar.identity.IdentityModule; -import org.briarproject.briar.test.TestDnsModule; import javax.inject.Singleton; @@ -28,7 +28,7 @@ import dagger.Component; FeedModule.class, IdentityModule.class, TestDnsModule.class, - TestSocksModule.class, + TestSocksModule.class }) interface FeedManagerIntegrationTestComponent extends BrambleCoreIntegrationTestEagerSingletons { diff --git a/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTestComponent.java b/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTestComponent.java index eb39cf1d6..ec856d553 100644 --- a/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTestComponent.java +++ b/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTestComponent.java @@ -2,6 +2,8 @@ package org.briarproject.briar.introduction; import org.briarproject.bramble.BrambleCoreModule; import org.briarproject.bramble.test.BrambleCoreIntegrationTestModule; +import org.briarproject.bramble.test.TestDnsModule; +import org.briarproject.bramble.test.TestSocksModule; import org.briarproject.briar.attachment.AttachmentModule; import org.briarproject.briar.autodelete.AutoDeleteModule; import org.briarproject.briar.avatar.AvatarModule; @@ -36,7 +38,9 @@ import dagger.Component; IntroductionModule.class, MessagingModule.class, PrivateGroupModule.class, - SharingModule.class + SharingModule.class, + TestDnsModule.class, + TestSocksModule.class }) interface IntroductionIntegrationTestComponent extends BriarIntegrationTestComponent { diff --git a/briar-core/src/test/java/org/briarproject/briar/messaging/MessageSizeIntegrationTestComponent.java b/briar-core/src/test/java/org/briarproject/briar/messaging/MessageSizeIntegrationTestComponent.java index c7068b880..e84f80afe 100644 --- a/briar-core/src/test/java/org/briarproject/briar/messaging/MessageSizeIntegrationTestComponent.java +++ b/briar-core/src/test/java/org/briarproject/briar/messaging/MessageSizeIntegrationTestComponent.java @@ -3,6 +3,8 @@ package org.briarproject.briar.messaging; import org.briarproject.bramble.BrambleCoreIntegrationTestEagerSingletons; import org.briarproject.bramble.BrambleCoreModule; import org.briarproject.bramble.test.BrambleCoreIntegrationTestModule; +import org.briarproject.bramble.test.TestDnsModule; +import org.briarproject.bramble.test.TestSocksModule; import org.briarproject.briar.autodelete.AutoDeleteModule; import org.briarproject.briar.avatar.AvatarModule; import org.briarproject.briar.client.BriarClientModule; @@ -24,7 +26,9 @@ import dagger.Component; ConversationModule.class, ForumModule.class, IdentityModule.class, - MessagingModule.class + MessagingModule.class, + TestDnsModule.class, + TestSocksModule.class }) interface MessageSizeIntegrationTestComponent extends BrambleCoreIntegrationTestEagerSingletons { diff --git a/briar-core/src/test/java/org/briarproject/briar/messaging/SimplexMessagingIntegrationTestComponent.java b/briar-core/src/test/java/org/briarproject/briar/messaging/SimplexMessagingIntegrationTestComponent.java index f0b97ee40..511946edf 100644 --- a/briar-core/src/test/java/org/briarproject/briar/messaging/SimplexMessagingIntegrationTestComponent.java +++ b/briar-core/src/test/java/org/briarproject/briar/messaging/SimplexMessagingIntegrationTestComponent.java @@ -8,6 +8,8 @@ import org.briarproject.bramble.api.event.EventBus; import org.briarproject.bramble.api.identity.IdentityManager; import org.briarproject.bramble.api.lifecycle.LifecycleManager; import org.briarproject.bramble.test.BrambleCoreIntegrationTestModule; +import org.briarproject.bramble.test.TestDnsModule; +import org.briarproject.bramble.test.TestSocksModule; import org.briarproject.briar.api.messaging.MessagingManager; import org.briarproject.briar.api.messaging.PrivateMessageFactory; import org.briarproject.briar.autodelete.AutoDeleteModule; @@ -25,7 +27,9 @@ import dagger.Component; BrambleCoreModule.class, BriarClientModule.class, ConversationModule.class, - MessagingModule.class + MessagingModule.class, + TestDnsModule.class, + TestSocksModule.class }) interface SimplexMessagingIntegrationTestComponent extends BrambleCoreIntegrationTestEagerSingletons { diff --git a/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTestComponent.java b/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTestComponent.java index 7c529d949..494961d9b 100644 --- a/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTestComponent.java +++ b/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTestComponent.java @@ -10,6 +10,8 @@ import org.briarproject.bramble.api.properties.TransportPropertyManager; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.test.BrambleCoreIntegrationTestModule; import org.briarproject.bramble.test.BrambleIntegrationTestComponent; +import org.briarproject.bramble.test.TestDnsModule; +import org.briarproject.bramble.test.TestSocksModule; import org.briarproject.bramble.test.TimeTravel; import org.briarproject.briar.api.attachment.AttachmentReader; import org.briarproject.briar.api.autodelete.AutoDeleteManager; @@ -61,7 +63,9 @@ import dagger.Component; IntroductionModule.class, MessagingModule.class, PrivateGroupModule.class, - SharingModule.class + SharingModule.class, + TestDnsModule.class, + TestSocksModule.class }) public interface BriarIntegrationTestComponent extends BrambleIntegrationTestComponent { diff --git a/briar-headless/src/main/java/org/briarproject/briar/headless/BriarHeadlessApp.kt b/briar-headless/src/main/java/org/briarproject/briar/headless/BriarHeadlessApp.kt index fc5a523ea..b3016d119 100644 --- a/briar-headless/src/main/java/org/briarproject/briar/headless/BriarHeadlessApp.kt +++ b/briar-headless/src/main/java/org/briarproject/briar/headless/BriarHeadlessApp.kt @@ -3,6 +3,7 @@ package org.briarproject.briar.headless import dagger.Component import org.briarproject.bramble.BrambleCoreEagerSingletons import org.briarproject.bramble.BrambleCoreModule +import org.briarproject.bramble.BrambleJavaModule import org.briarproject.briar.BriarCoreEagerSingletons import org.briarproject.briar.BriarCoreModule import java.security.SecureRandom @@ -11,6 +12,7 @@ import javax.inject.Singleton @Component( modules = [ BrambleCoreModule::class, + BrambleJavaModule::class, BriarCoreModule::class, HeadlessModule::class ] diff --git a/briar-headless/src/main/java/org/briarproject/briar/headless/HeadlessModule.kt b/briar-headless/src/main/java/org/briarproject/briar/headless/HeadlessModule.kt index 018045a0a..a841f9913 100644 --- a/briar-headless/src/main/java/org/briarproject/briar/headless/HeadlessModule.kt +++ b/briar-headless/src/main/java/org/briarproject/briar/headless/HeadlessModule.kt @@ -18,16 +18,12 @@ import org.briarproject.bramble.api.plugin.duplex.DuplexPluginFactory import org.briarproject.bramble.api.plugin.simplex.SimplexPluginFactory import org.briarproject.bramble.battery.DefaultBatteryManagerModule import org.briarproject.bramble.event.DefaultEventExecutorModule -import org.briarproject.bramble.network.JavaNetworkModule -import org.briarproject.bramble.plugin.tor.CircumventionModule import org.briarproject.bramble.plugin.tor.UnixTorPluginFactory import org.briarproject.bramble.plugin.tor.WindowsTorPluginFactory -import org.briarproject.bramble.socks.SocksModule import org.briarproject.bramble.system.ClockModule import org.briarproject.bramble.system.DefaultTaskSchedulerModule import org.briarproject.bramble.system.DefaultWakefulIoExecutorModule import org.briarproject.bramble.system.DesktopSecureRandomModule -import org.briarproject.bramble.system.JavaSystemModule import org.briarproject.bramble.util.OsUtils.isLinux import org.briarproject.bramble.util.OsUtils.isMac import org.briarproject.bramble.util.OsUtils.isWindows @@ -43,7 +39,6 @@ import javax.inject.Singleton @Module( includes = [ AccountModule::class, - CircumventionModule::class, ClockModule::class, DefaultBatteryManagerModule::class, DefaultEventExecutorModule::class, @@ -54,10 +49,7 @@ import javax.inject.Singleton HeadlessContactModule::class, HeadlessEventModule::class, HeadlessForumModule::class, - HeadlessMessagingModule::class, - JavaNetworkModule::class, - JavaSystemModule::class, - SocksModule::class + HeadlessMessagingModule::class ] ) internal class HeadlessModule(private val appDir: File) { diff --git a/briar-headless/src/test/java/org/briarproject/briar/headless/BriarHeadlessTestApp.kt b/briar-headless/src/test/java/org/briarproject/briar/headless/BriarHeadlessTestApp.kt index 89e69fae7..ace7d937d 100644 --- a/briar-headless/src/test/java/org/briarproject/briar/headless/BriarHeadlessTestApp.kt +++ b/briar-headless/src/test/java/org/briarproject/briar/headless/BriarHeadlessTestApp.kt @@ -3,6 +3,7 @@ package org.briarproject.briar.headless import dagger.Component import org.briarproject.bramble.BrambleCoreEagerSingletons import org.briarproject.bramble.BrambleCoreModule +import org.briarproject.bramble.BrambleJavaModule import org.briarproject.bramble.api.crypto.CryptoComponent import org.briarproject.briar.BriarCoreEagerSingletons import org.briarproject.briar.BriarCoreModule @@ -12,6 +13,7 @@ import javax.inject.Singleton @Component( modules = [ BrambleCoreModule::class, + BrambleJavaModule::class, BriarCoreModule::class, HeadlessTestModule::class ] diff --git a/briar-headless/src/test/java/org/briarproject/briar/headless/HeadlessTestModule.kt b/briar-headless/src/test/java/org/briarproject/briar/headless/HeadlessTestModule.kt index 3c0da66aa..2725b70a5 100644 --- a/briar-headless/src/test/java/org/briarproject/briar/headless/HeadlessTestModule.kt +++ b/briar-headless/src/test/java/org/briarproject/briar/headless/HeadlessTestModule.kt @@ -15,13 +15,9 @@ import org.briarproject.bramble.api.plugin.TransportId import org.briarproject.bramble.api.plugin.duplex.DuplexPluginFactory import org.briarproject.bramble.api.plugin.simplex.SimplexPluginFactory import org.briarproject.bramble.event.DefaultEventExecutorModule -import org.briarproject.bramble.network.JavaNetworkModule -import org.briarproject.bramble.plugin.tor.CircumventionModule -import org.briarproject.bramble.socks.SocksModule import org.briarproject.bramble.system.ClockModule import org.briarproject.bramble.system.DefaultTaskSchedulerModule import org.briarproject.bramble.system.DefaultWakefulIoExecutorModule -import org.briarproject.bramble.system.JavaSystemModule import org.briarproject.bramble.test.TestFeatureFlagModule import org.briarproject.bramble.test.TestSecureRandomModule import org.briarproject.briar.api.test.TestAvatarCreator @@ -36,15 +32,11 @@ import javax.inject.Singleton @Module( includes = [ - JavaNetworkModule::class, - JavaSystemModule::class, AccountModule::class, - CircumventionModule::class, ClockModule::class, DefaultEventExecutorModule::class, DefaultTaskSchedulerModule::class, DefaultWakefulIoExecutorModule::class, - SocksModule::class, TestFeatureFlagModule::class, TestSecureRandomModule::class, HeadlessBlogModule::class, From d3a06cf2c07643b1d5511e74a075f996b443d8e2 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 5 Aug 2022 15:43:17 +0100 Subject: [PATCH 3/8] Add some javadocs. --- .../bramble/mailbox/MailboxClientManager.java | 53 +++++++++++++++++-- 1 file changed, 50 insertions(+), 3 deletions(-) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java index 1750c49ca..4e68f5815 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java @@ -52,6 +52,32 @@ import static org.briarproject.bramble.api.plugin.Plugin.State.ACTIVE; import static org.briarproject.bramble.api.plugin.TorConstants.ID; import static org.briarproject.bramble.util.LogUtils.logException; +/** + * This component manages a {@link MailboxClient} for each mailbox we know + * about and are able to use (our own mailbox and/or contacts' mailboxes). + * The clients are created when we come online (i.e. when the Tor plugin + * becomes {@link Plugin.State#ACTIVE active}) and destroyed when we go + * offline. + *

+ * The manager keeps track of the latest {@link MailboxUpdate} sent to and + * received from each contact. These updates are used to decide which + * mailboxes the manager needs clients for, and which mailbox (if any) should + * be used for uploading data to and/or downloading data from each contact. + *

+ * The assignments of contacts to mailboxes for upload and/or download may + * change when the following events happen: + *

    + *
  • A mailbox is paired or unpaired
    • + *
  • A contact is added or removed
    • + *
  • A {@link MailboxUpdate} is received from a contact
    • + *
  • We discover that our own mailbox's supported API versions have + * changed
    • + *
+ *

+ * The manager keeps its mutable state consistent with the state in the DB by + * using commit actions and events to update the manager's state on the event + * thread. + */ @ThreadSafe @NotNullByDefault class MailboxClientManager implements Service, EventListener { @@ -69,7 +95,7 @@ class MailboxClientManager implements Service, EventListener { private final TorReachabilityMonitor reachabilityMonitor; private final AtomicBoolean used = new AtomicBoolean(false); - // All mutable state must only be accessed on the worker thread + // The following mutable state must only be accessed on the event thread private final Map contactUpdates = new HashMap<>(); private final Map contactClients = new HashMap<>(); @@ -142,6 +168,10 @@ class MailboxClientManager implements Service, EventListener { online = true; createClients(); } + // Now that the mutable state has been initialised we can start + // handling events. This is done in a commit action so that we don't + // miss any changes to the DB state or handle events for any changes + // that were already reflected in the initial load handleEvents = true; } @@ -192,7 +222,7 @@ class MailboxClientManager implements Service, EventListener { CountDownLatch latch = new CountDownLatch(1); eventExecutor.execute(() -> { handleEvents = false; - destroyClients(); + if (online) destroyClients(); latch.countDown(); }); reachabilityMonitor.destroy(); @@ -554,6 +584,10 @@ class MailboxClientManager implements Service, EventListener { requireNonNull(remoteProps.getOutboxId())); } + /** + * Returns the {@link MailboxProperties} included in the given update, + * which must be a {@link MailboxUpdateWithMailbox}. + */ private MailboxProperties getMailboxProperties(MailboxUpdate update) { if (!(update instanceof MailboxUpdateWithMailbox)) { throw new IllegalArgumentException(); @@ -600,7 +634,8 @@ class MailboxClientManager implements Service, EventListener { } /** - * Returns true if we're compatible with our own mailbox. + * Returns true if our client-supported API versions are compatible with + * our own mailbox's server-supported API versions. */ @SuppressWarnings("BooleanMethodIsAlwaysInverted") private boolean isOwnMailboxUsable(MailboxProperties ownProperties) { @@ -608,9 +643,21 @@ class MailboxClientManager implements Service, EventListener { ownProperties.getServerSupports()) >= 0; } + /** + * A container for the latest {@link MailboxUpdate updates} sent to and + * received from a given contact. + */ private static class Updates { + /** + * The latest update sent to the contact. + */ private final MailboxUpdate local; + + /** + * The latest update received from the contact, or null if no update + * has been received. + */ @Nullable private final MailboxUpdate remote; From 42243f73f4473d53eeba39de632bcb91d5750045 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 8 Aug 2022 12:30:20 +0100 Subject: [PATCH 4/8] Simplify logic. --- .../bramble/mailbox/MailboxClientManager.java | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java index 4e68f5815..3fa1c7046 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java @@ -330,29 +330,26 @@ class MailboxClientManager implements Service, EventListener { for (Entry e : contactUpdates.entrySet()) { ContactId c = e.getKey(); Updates u = e.getValue(); - boolean isOwnMailboxUsable = - isOwnMailboxUsable(ownProperties, u.remote); + if (!isOwnMailboxUsable(ownProperties, u.remote)) { + // Our mailbox isn't usable for communicating with this + // contact, so don't assign/reassign this contact + continue; + } if (isContactMailboxUsable(u.remote)) { // The contact has a usable mailbox, so the contact should // currently be assigned to the contact's mailbox for upload - // and download - if (isOwnMailboxUsable) { - // Reassign the contact to our mailbox for download - MailboxClient contactClient = - requireNonNull(contactClients.get(c)); - contactClient.deassignContactForDownload(c); - assignContactToOwnMailboxForDownload(c, u); - } - // Else the contact remains assigned to the contact's mailbox - // for download - } else if (isOwnMailboxUsable) { + // and download. Reassign the contact to our mailbox for + // download + MailboxClient contactClient = + requireNonNull(contactClients.get(c)); + contactClient.deassignContactForDownload(c); + assignContactToOwnMailboxForDownload(c, u); + } else { // The contact doesn't have a usable mailbox, so assign the // contact to our mailbox for upload and download assignContactToOwnMailboxForUpload(c, u); assignContactToOwnMailboxForDownload(c, u); } - // Else the contact doesn't have a usable mailbox and neither do we, - // so the contact remains unassigned for upload and download } } From 62883b4bde14e19aada7ce446356a3846be2c463 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 5 Aug 2022 18:00:16 +0100 Subject: [PATCH 5/8] Unit tests for mailbox client manager. --- .../mailbox/MailboxClientManagerTest.java | 930 ++++++++++++++++++ 1 file changed, 930 insertions(+) create mode 100644 bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxClientManagerTest.java diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxClientManagerTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxClientManagerTest.java new file mode 100644 index 000000000..655f6952a --- /dev/null +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxClientManagerTest.java @@ -0,0 +1,930 @@ +package org.briarproject.bramble.mailbox; + +import org.briarproject.bramble.api.contact.Contact; +import org.briarproject.bramble.api.contact.ContactManager; +import org.briarproject.bramble.api.contact.event.ContactRemovedEvent; +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.MailboxStatus; +import org.briarproject.bramble.api.mailbox.MailboxUpdate; +import org.briarproject.bramble.api.mailbox.MailboxUpdateManager; +import org.briarproject.bramble.api.mailbox.MailboxUpdateWithMailbox; +import org.briarproject.bramble.api.mailbox.MailboxVersion; +import org.briarproject.bramble.api.mailbox.event.MailboxPairedEvent; +import org.briarproject.bramble.api.mailbox.event.MailboxUnpairedEvent; +import org.briarproject.bramble.api.mailbox.event.OwnMailboxConnectionStatusEvent; +import org.briarproject.bramble.api.mailbox.event.RemoteMailboxUpdateEvent; +import org.briarproject.bramble.api.plugin.Plugin.State; +import org.briarproject.bramble.api.plugin.PluginManager; +import org.briarproject.bramble.api.plugin.duplex.DuplexPlugin; +import org.briarproject.bramble.api.plugin.event.TransportActiveEvent; +import org.briarproject.bramble.api.plugin.event.TransportInactiveEvent; +import org.briarproject.bramble.test.BrambleMockTestCase; +import org.briarproject.bramble.test.DbExpectations; +import org.briarproject.bramble.test.RunAction; +import org.jmock.Expectations; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.Executor; + +import javax.annotation.Nullable; + +import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; +import static org.briarproject.bramble.api.mailbox.MailboxConstants.CLIENT_SUPPORTS; +import static org.briarproject.bramble.api.nullsafety.NullSafety.requireNonNull; +import static org.briarproject.bramble.api.plugin.Plugin.State.ACTIVE; +import static org.briarproject.bramble.api.plugin.Plugin.State.ENABLING; +import static org.briarproject.bramble.api.plugin.TorConstants.ID; +import static org.briarproject.bramble.test.TestUtils.getContact; +import static org.briarproject.bramble.test.TestUtils.getMailboxProperties; + +public class MailboxClientManagerTest extends BrambleMockTestCase { + + private final Executor eventExecutor = + context.mock(Executor.class, "eventExecutor"); + private final Executor dbExecutor = + context.mock(Executor.class, "dbExecutor"); + private final TransactionManager db = + context.mock(TransactionManager.class); + private final ContactManager contactManager = + context.mock(ContactManager.class); + private final PluginManager pluginManager = + context.mock(PluginManager.class); + private final MailboxSettingsManager mailboxSettingsManager = + context.mock(MailboxSettingsManager.class); + private final MailboxUpdateManager mailboxUpdateManager = + context.mock(MailboxUpdateManager.class); + private final MailboxClientFactory mailboxClientFactory = + context.mock(MailboxClientFactory.class); + private final TorReachabilityMonitor reachabilityMonitor = + context.mock(TorReachabilityMonitor.class); + private final DuplexPlugin plugin = context.mock(DuplexPlugin.class); + private final MailboxClient ownClient = + context.mock(MailboxClient.class, "ownClient"); + private final MailboxClient contactClient = + context.mock(MailboxClient.class, "contactClient"); + + private final MailboxClientManager manager = + new MailboxClientManager(eventExecutor, dbExecutor, db, + contactManager, pluginManager, mailboxSettingsManager, + mailboxUpdateManager, mailboxClientFactory, + reachabilityMonitor); + + private final Contact contact = getContact(); + private final List incompatibleVersions = + singletonList(new MailboxVersion(999, 0)); + private final MailboxProperties ownProperties = + getMailboxProperties(true, CLIENT_SUPPORTS); + private final MailboxProperties localProperties = + getMailboxProperties(false, CLIENT_SUPPORTS); + private final MailboxProperties remoteProperties = + getMailboxProperties(false, CLIENT_SUPPORTS); + private final MailboxProperties remotePropertiesForNewMailbox = + getMailboxProperties(false, CLIENT_SUPPORTS); + private final MailboxUpdate localUpdateWithoutMailbox = + new MailboxUpdate(CLIENT_SUPPORTS); + private final MailboxUpdate remoteUpdateWithoutMailbox = + new MailboxUpdate(CLIENT_SUPPORTS); + private final MailboxUpdate remoteUpdateWithIncompatibleClientVersions = + new MailboxUpdate(incompatibleVersions); + private final MailboxUpdateWithMailbox localUpdateWithMailbox = + new MailboxUpdateWithMailbox(CLIENT_SUPPORTS, localProperties); + private final MailboxUpdateWithMailbox remoteUpdateWithMailbox = + new MailboxUpdateWithMailbox(CLIENT_SUPPORTS, remoteProperties); + private final MailboxUpdateWithMailbox remoteUpdateWithNewMailbox = + new MailboxUpdateWithMailbox(CLIENT_SUPPORTS, + remotePropertiesForNewMailbox); + + @Test + public void testLoadsMailboxUpdatesAtStartupWhenOffline() throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We're offline so there's nothing + // else to do. + expectLoadUpdates(localUpdateWithoutMailbox, + remoteUpdateWithoutMailbox, null); + expectCheckPluginState(ENABLING); + manager.startService(); + + // At shutdown there should be no clients to destroy. The manager + // should destroy the reachability monitor. + expectRunTaskOnEventExecutor(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testLoadsMailboxUpdatesAtStartupWhenOnline() throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We're online but we don't have a + // mailbox and neither does the contact, so there's nothing else to do. + expectLoadUpdates(localUpdateWithoutMailbox, + remoteUpdateWithoutMailbox, null); + expectCheckPluginState(ACTIVE); + manager.startService(); + + // At shutdown there should be no clients to destroy. The manager + // should destroy the reachability monitor. + expectRunTaskOnEventExecutor(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testAssignsContactToOurMailboxIfContactHasNoMailbox() + throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We have a mailbox but the contact + // doesn't. + // + // We're online, so the manager should create a client for our own + // mailbox and assign the contact to it for upload and download. + expectLoadUpdates(localUpdateWithMailbox, + remoteUpdateWithoutMailbox, ownProperties); + expectCheckPluginState(ACTIVE); + expectCreateClientForOwnMailbox(); + expectAssignContactToOwnMailboxForDownload(); + expectAssignContactToOwnMailboxForUpload(); + manager.startService(); + + // At shutdown the manager should destroy the client for our own + // mailbox and the reachability monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForOwnMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testDoesNotAssignContactToOurMailboxIfContactHasNotSentUpdate() + throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We have a mailbox but the contact + // has never sent us an update, so the remote update is null. + // + // We're online, so the manager should create a client for our own + // mailbox. We don't know what API versions the contact supports, + // if any, so the contact should not be assigned to our mailbox. + expectLoadUpdates(localUpdateWithMailbox, null, ownProperties); + expectCheckPluginState(ACTIVE); + expectCreateClientForOwnMailbox(); + manager.startService(); + + // At shutdown the manager should destroy the client for our own + // mailbox and the reachability monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForOwnMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testDoesNotAssignContactToOurMailboxIfContactHasIncompatibleClientVersions() + throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We have a mailbox but the contact + // doesn't. The contact's client API versions are incompatible with + // our mailbox. + // + // We're online, so the manager should create a client for our own + // mailbox. The contact's client API versions are incompatible with + // our mailbox, so the contact should not be assigned to our mailbox. + expectLoadUpdates(localUpdateWithMailbox, + remoteUpdateWithIncompatibleClientVersions, ownProperties); + expectCheckPluginState(ACTIVE); + expectCreateClientForOwnMailbox(); + manager.startService(); + + // At shutdown the manager should destroy the client for our own + // mailbox and the reachability monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForOwnMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testAssignsContactToContactMailboxIfWeHaveNoMailbox() + throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We don't have a mailbox but the + // contact does. + // + // We're online, so the manager should create a client for the + // contact's mailbox and assign the contact to it for upload and + // download. + expectLoadUpdates(localUpdateWithoutMailbox, + remoteUpdateWithMailbox, null); + expectCheckPluginState(ACTIVE); + expectCreateClientForContactMailbox(); + expectAssignContactToContactMailboxForDownload(remoteProperties); + expectAssignContactToContactMailboxForUpload(remoteProperties); + manager.startService(); + + // At shutdown the manager should destroy the client for the contact's + // mailbox and the reachability monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForContactMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testAssignsContactToBothMailboxesIfWeBothHaveMailboxes() + throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We have a mailbox and so does the + // contact. + // + // We're online, so the manager should create clients for the + // contact's mailbox and our mailbox. The manager should assign the + // contact to the contact's mailbox for upload and our mailbox for + // download. + expectLoadUpdates(localUpdateWithMailbox, + remoteUpdateWithMailbox, ownProperties); + expectCheckPluginState(ACTIVE); + expectCreateClientForContactMailbox(); + expectAssignContactToContactMailboxForUpload(remoteProperties); + expectCreateClientForOwnMailbox(); + expectAssignContactToOwnMailboxForDownload(); + manager.startService(); + + // At shutdown the manager should destroy the client for the contact's + // mailbox, the client for our own mailbox, and the reachability + // monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForContactMailbox(); + expectDestroyClientForOwnMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testCreatesClientsWhenTorBecomesActive() throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We have a mailbox but the contact + // doesn't. We're offline so there's nothing else to do. + expectLoadUpdates(localUpdateWithMailbox, + remoteUpdateWithoutMailbox, ownProperties); + expectCheckPluginState(ENABLING); + manager.startService(); + + // When we come online, the manager should create a client for our own + // mailbox and assign the contact to it for upload and download. + expectCreateClientForOwnMailbox(); + expectAssignContactToOwnMailboxForDownload(); + expectAssignContactToOwnMailboxForUpload(); + manager.eventOccurred(new TransportActiveEvent(ID)); + + // When we go offline, the manager should destroy the client for our + // own mailbox + expectDestroyClientForOwnMailbox(); + manager.eventOccurred(new TransportInactiveEvent(ID)); + + // At shutdown the manager should destroy the client for our own + // mailbox and the reachability monitor. + expectRunTaskOnEventExecutor(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testAssignsContactToOurMailboxWhenPaired() throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We're online but we don't have a + // mailbox and neither does the contact, so there's nothing else to do. + expectLoadUpdates(localUpdateWithoutMailbox, + remoteUpdateWithoutMailbox, null); + expectCheckPluginState(ACTIVE); + manager.startService(); + + // When we pair a mailbox, the manager should create a client for our + // mailbox and assign the contact to our mailbox for upload and + // download. + expectCreateClientForOwnMailbox(); + expectAssignContactToOwnMailboxForUpload(); + expectAssignContactToOwnMailboxForDownload(); + manager.eventOccurred(new MailboxPairedEvent(ownProperties, + singletonMap(contact.getId(), localUpdateWithMailbox))); + + // At shutdown the manager should destroy the client for our mailbox + // and the reachability monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForOwnMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testReassignsContactToOurMailboxWhenPaired() throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We have a mailbox but the contact + // doesn't. + // + // We're online, so the manager should create a client for the + // contact's mailbox and assign the contact to it for upload and + // download. + expectLoadUpdates(localUpdateWithoutMailbox, + remoteUpdateWithMailbox, null); + expectCheckPluginState(ACTIVE); + expectCreateClientForContactMailbox(); + expectAssignContactToContactMailboxForDownload(remoteProperties); + expectAssignContactToContactMailboxForUpload(remoteProperties); + manager.startService(); + + // When we pair a mailbox, the manager should create a client for our + // mailbox and reassign the contact to our mailbox for download. + expectCreateClientForOwnMailbox(); + expectDeassignContactFromContactMailboxForDownload(); + expectAssignContactToOwnMailboxForDownload(); + manager.eventOccurred(new MailboxPairedEvent(ownProperties, + singletonMap(contact.getId(), localUpdateWithMailbox))); + + // At shutdown the manager should destroy the client for the contact's + // mailbox, the client for our own mailbox, and the reachability + // monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForContactMailbox(); + expectDestroyClientForOwnMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testDoesNotAssignContactWhenPairedIfContactHasNotSentUpdate() + throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We have a mailbox but the contact + // has never sent us an update, so the remote update is null. + expectLoadUpdates(localUpdateWithoutMailbox, null, null); + expectCheckPluginState(ACTIVE); + manager.startService(); + + // When we pair a mailbox, the manager should create a client for our + // mailbox. We don't know whether the contact can use our mailbox, so + // the contact should not be assigned to our mailbox. + expectCreateClientForOwnMailbox(); + manager.eventOccurred(new MailboxPairedEvent(ownProperties, + singletonMap(contact.getId(), localUpdateWithMailbox))); + + // At shutdown the manager should destroy the client for our mailbox + // and the reachability monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForOwnMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testDoesNotAssignContactWhenPairedIfContactHasIncompatibleClientVersions() + throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We don't have a mailbox and neither + // does the contact. The contact's client API versions are + // incompatible with our mailbox. + expectLoadUpdates(localUpdateWithoutMailbox, + remoteUpdateWithIncompatibleClientVersions, null); + expectCheckPluginState(ACTIVE); + manager.startService(); + + // When we pair a mailbox, the manager should create a client for our + // mailbox. The contact's client API versions are incompatible with + // our mailbox, so the contact should not be assigned to our mailbox. + expectCreateClientForOwnMailbox(); + manager.eventOccurred(new MailboxPairedEvent(ownProperties, + singletonMap(contact.getId(), localUpdateWithMailbox))); + + // At shutdown the manager should destroy the client for our mailbox + // and the reachability monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForOwnMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testReassignsContactToContactMailboxWhenUnpaired() + throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We have a mailbox and so does the + // contact. + // + // We're online, so the manager should create clients for the + // contact's mailbox and our mailbox. The manager should assign the + // contact to the contact's mailbox for upload and our mailbox for + // download. + expectLoadUpdates(localUpdateWithMailbox, + remoteUpdateWithMailbox, ownProperties); + expectCheckPluginState(ACTIVE); + expectCreateClientForContactMailbox(); + expectAssignContactToContactMailboxForUpload(remoteProperties); + expectCreateClientForOwnMailbox(); + expectAssignContactToOwnMailboxForDownload(); + manager.startService(); + + // When we unpair our mailbox, the manager should destroy the client + // for our mailbox and reassign the contact to the contact's mailbox + // for download. + expectDestroyClientForOwnMailbox(); + expectAssignContactToContactMailboxForDownload(remoteProperties); + manager.eventOccurred(new MailboxUnpairedEvent( + singletonMap(contact.getId(), localUpdateWithoutMailbox))); + + // At shutdown the manager should destroy the client for the contact's + // mailbox and the reachability monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForContactMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testDeassignsContactForUploadAndDownloadWhenContactRemoved() + throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We have a mailbox but the contact + // doesn't. + // + // We're online, so the manager should create a client for our mailbox. + // The manager should assign the contact to our mailbox for upload and + // download. + expectLoadUpdates(localUpdateWithMailbox, + remoteUpdateWithoutMailbox, ownProperties); + expectCheckPluginState(ACTIVE); + expectCreateClientForOwnMailbox(); + expectAssignContactToOwnMailboxForUpload(); + expectAssignContactToOwnMailboxForDownload(); + manager.startService(); + + // When the contact is removed, the manager should deassign the contact + // from our mailbox for upload and download. + expectDeassignContactFromOwnMailboxForUpload(); + expectDeassignContactFromOwnMailboxForDownload(); + manager.eventOccurred(new ContactRemovedEvent(contact.getId())); + + // At shutdown the manager should destroy the client for our mailbox + // and the reachability monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForOwnMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testDeassignsContactForDownloadWhenContactRemoved() + throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We have a mailbox and so does the + // contact. + // + // We're online, so the manager should create clients for the + // contact's mailbox and our mailbox. The manager should assign the + // contact to the contact's mailbox for upload and our mailbox for + // download. + expectLoadUpdates(localUpdateWithMailbox, + remoteUpdateWithMailbox, ownProperties); + expectCheckPluginState(ACTIVE); + expectCreateClientForContactMailbox(); + expectAssignContactToContactMailboxForUpload(remoteProperties); + expectCreateClientForOwnMailbox(); + expectAssignContactToOwnMailboxForDownload(); + manager.startService(); + + // When the contact is removed, the manager should destroy the client + // for the contact's mailbox and deassign the contact from our mailbox + // for download. + expectDestroyClientForContactMailbox(); + expectDeassignContactFromOwnMailboxForDownload(); + manager.eventOccurred(new ContactRemovedEvent(contact.getId())); + + // At shutdown the manager should destroy the client for our mailbox + // and the reachability monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForOwnMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testAssignsContactToContactMailboxWhenContactPairsMailbox() + throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We're online but we don't have a + // mailbox and neither does the contact, so there's nothing else to do. + expectLoadUpdates(localUpdateWithoutMailbox, + remoteUpdateWithoutMailbox, null); + expectCheckPluginState(ACTIVE); + manager.startService(); + + // When the contact pairs a mailbox, the manager should create a client + // for the contact's mailbox and assign the contact to the contact's + // mailbox for upload and download. + expectCreateClientForContactMailbox(); + expectAssignContactToContactMailboxForUpload(remoteProperties); + expectAssignContactToContactMailboxForDownload(remoteProperties); + manager.eventOccurred(new RemoteMailboxUpdateEvent(contact.getId(), + remoteUpdateWithMailbox)); + + // At shutdown the manager should destroy the client for the contact's + // mailbox and the reachability monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForContactMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testReassignsContactForUploadWhenContactPairsMailbox() + throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We have a mailbox but the contact + // doesn't. + // + // We're online, so the manager should create a client for our mailbox. + // The manager should assign the contact to our mailbox for upload and + // download. + expectLoadUpdates(localUpdateWithMailbox, + remoteUpdateWithoutMailbox, ownProperties); + expectCheckPluginState(ACTIVE); + expectCreateClientForOwnMailbox(); + expectAssignContactToOwnMailboxForUpload(); + expectAssignContactToOwnMailboxForDownload(); + manager.startService(); + + // When the contact pairs a mailbox, the manager should create a client + // for the contact's mailbox and reassign the contact to the contact's + // mailbox for upload. + expectCreateClientForContactMailbox(); + expectDeassignContactFromOwnMailboxForUpload(); + expectAssignContactToContactMailboxForUpload(remoteProperties); + manager.eventOccurred(new RemoteMailboxUpdateEvent(contact.getId(), + remoteUpdateWithMailbox)); + + // At shutdown the manager should destroy the client for the contact's + // mailbox, the client for our own mailbox, and the reachability + // monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForContactMailbox(); + expectDestroyClientForOwnMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testReassignsContactForUploadWhenContactUnpairsMailbox() + throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We have a mailbox and so does the + // contact. + // + // We're online, so the manager should create clients for the + // contact's mailbox and our mailbox. The manager should assign the + // contact to the contact's mailbox for upload and our mailbox for + // download. + expectLoadUpdates(localUpdateWithMailbox, + remoteUpdateWithMailbox, ownProperties); + expectCheckPluginState(ACTIVE); + expectCreateClientForContactMailbox(); + expectAssignContactToContactMailboxForUpload(remoteProperties); + expectCreateClientForOwnMailbox(); + expectAssignContactToOwnMailboxForDownload(); + manager.startService(); + + // When the contact unpairs their mailbox, the manager should destroy + // the client for the contact's mailbox and reassign the contact to + // our mailbox for upload. + expectDestroyClientForContactMailbox(); + expectAssignContactToOwnMailboxForUpload(); + manager.eventOccurred(new RemoteMailboxUpdateEvent(contact.getId(), + remoteUpdateWithoutMailbox)); + + // At shutdown the manager should destroy the client for our mailbox + // and the reachability monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForOwnMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testReassignsContactForUploadWhenContactReplacesMailbox() + throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We have a mailbox and so does the + // contact. + // + // We're online, so the manager should create clients for the + // contact's mailbox and our mailbox. The manager should assign the + // contact to the contact's mailbox for upload and our mailbox for + // download. + expectLoadUpdates(localUpdateWithMailbox, + remoteUpdateWithMailbox, ownProperties); + expectCheckPluginState(ACTIVE); + expectCreateClientForContactMailbox(); + expectAssignContactToContactMailboxForUpload(remoteProperties); + expectCreateClientForOwnMailbox(); + expectAssignContactToOwnMailboxForDownload(); + manager.startService(); + + // When the contact replaces their mailbox, the manager should replace + // the client for the contact's mailbox and assign the contact to + // the contact's new mailbox for upload. + expectDestroyClientForContactMailbox(); + expectCreateClientForContactMailbox(); + expectAssignContactToContactMailboxForUpload( + remotePropertiesForNewMailbox); + manager.eventOccurred(new RemoteMailboxUpdateEvent(contact.getId(), + remoteUpdateWithNewMailbox)); + + // At shutdown the manager should destroy the client for the contact's + // mailbox, the client for our own mailbox, and the reachability + // monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForContactMailbox(); + expectDestroyClientForOwnMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testReassignsContactWhenContactReplacesMailbox() + throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We don't have a mailbox but the + // contact does. + // + // We're online, so the manager should create a client for the + // contact's mailbox and assign the contact to the contact's mailbox + // for upload and download. + expectLoadUpdates(localUpdateWithoutMailbox, + remoteUpdateWithMailbox, null); + expectCheckPluginState(ACTIVE); + expectCreateClientForContactMailbox(); + expectAssignContactToContactMailboxForUpload(remoteProperties); + expectAssignContactToContactMailboxForDownload(remoteProperties); + manager.startService(); + + // When the contact replaces their mailbox, the manager should replace + // the client for the contact's mailbox and assign the contact to + // the contact's new mailbox for upload and download. + expectDestroyClientForContactMailbox(); + expectCreateClientForContactMailbox(); + expectAssignContactToContactMailboxForUpload( + remotePropertiesForNewMailbox); + expectAssignContactToContactMailboxForDownload( + remotePropertiesForNewMailbox); + manager.eventOccurred(new RemoteMailboxUpdateEvent(contact.getId(), + remoteUpdateWithNewMailbox)); + + // At shutdown the manager should destroy the client for the contact's + // mailbox and the reachability monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForContactMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testDoesNotReassignContactWhenRemotePropertiesAreUnchanged() + throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We don't have a mailbox but the + // contact does. + // + // We're online, so the manager should create a client for the + // contact's mailbox and assign the contact to the contact's mailbox + // for upload and download. + expectLoadUpdates(localUpdateWithoutMailbox, + remoteUpdateWithMailbox, null); + expectCheckPluginState(ACTIVE); + expectCreateClientForContactMailbox(); + expectAssignContactToContactMailboxForUpload(remoteProperties); + expectAssignContactToContactMailboxForDownload(remoteProperties); + manager.startService(); + + // When the contact sends an update with unchanged properties, the + // clients and assignments should not be affected. + manager.eventOccurred(new RemoteMailboxUpdateEvent(contact.getId(), + remoteUpdateWithMailbox)); + + // At shutdown the manager should destroy the client for the contact's + // mailbox and the reachability monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForContactMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testAssignsContactToOurMailboxWhenClientVersionsBecomeCompatible() + throws Exception { + // At startup the manager should load the local and remote updates + // and our own mailbox properties. We have a mailbox but the contact + // doesn't. The contact's client API versions are incompatible with + // our mailbox. + // + // We're online, so the manager should create a client for our own + // mailbox. The contact's client API versions are incompatible with + // our mailbox, so the contact should not be assigned to our mailbox. + expectLoadUpdates(localUpdateWithMailbox, + remoteUpdateWithIncompatibleClientVersions, ownProperties); + expectCheckPluginState(ACTIVE); + expectCreateClientForOwnMailbox(); + manager.startService(); + + // When the contact sends an update indicating that their client API + // versions are now compatible with our mailbox, the manager should + // assign the contact to our mailbox for upload and download. + expectAssignContactToOwnMailboxForUpload(); + expectAssignContactToOwnMailboxForDownload(); + manager.eventOccurred(new RemoteMailboxUpdateEvent(contact.getId(), + remoteUpdateWithoutMailbox)); + + // At shutdown the manager should destroy the client for our own + // mailbox and the reachability monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForOwnMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + @Test + public void testRecreatesClientsWhenOwnMailboxServerVersionsChange() + throws Exception { + long now = System.currentTimeMillis(); + List compatibleVersions = + new ArrayList<>(CLIENT_SUPPORTS); + compatibleVersions.add(new MailboxVersion(999, 0)); + MailboxStatus mailboxStatus = + new MailboxStatus(now, now, 0, compatibleVersions); + + // At startup the manager should load the local and remote updates + // and our own mailbox properties. The contact has a mailbox, so the + // remote update contains the properties received from the contact. + // We also have a mailbox, so the local update contains the properties + // we sent to the contact. + // + // We're online, so the manager should create clients for the + // contact's mailbox and our mailbox. The manager should assign the + // contact to the contact's mailbox for upload and our mailbox for + // download. + expectLoadUpdates(localUpdateWithMailbox, + remoteUpdateWithMailbox, ownProperties); + expectCheckPluginState(ACTIVE); + expectCreateClientForContactMailbox(); + expectAssignContactToContactMailboxForUpload(remoteProperties); + expectCreateClientForOwnMailbox(); + expectAssignContactToOwnMailboxForDownload(); + manager.startService(); + + // When we learn that our mailbox's API versions have changed, the + // manager should destroy and recreate the clients for our own mailbox + // and the contact's mailbox and assign the contact to the new clients. + expectDestroyClientForContactMailbox(); + expectDestroyClientForOwnMailbox(); + expectCreateClientForContactMailbox(); + expectAssignContactToContactMailboxForUpload(remoteProperties); + expectCreateClientForOwnMailbox(); + expectAssignContactToOwnMailboxForDownload(); + manager.eventOccurred( + new OwnMailboxConnectionStatusEvent(mailboxStatus)); + + // At shutdown the manager should destroy the client for the contact's + // mailbox, the client for our own mailbox, and the reachability + // monitor. + expectRunTaskOnEventExecutor(); + expectDestroyClientForContactMailbox(); + expectDestroyClientForOwnMailbox(); + expectDestroyReachabilityMonitor(); + manager.stopService(); + } + + private void expectLoadUpdates(MailboxUpdate local, + @Nullable MailboxUpdate remote, + @Nullable MailboxProperties own) throws Exception { + Transaction txn = new Transaction(null, true); + + context.checking(new DbExpectations() {{ + oneOf(reachabilityMonitor).start(); + oneOf(dbExecutor).execute(with(any(Runnable.class))); + will(new RunAction()); + oneOf(db).transaction(with(true), withDbRunnable(txn)); + oneOf(contactManager).getContacts(txn); + will(returnValue(singletonList(contact))); + oneOf(mailboxUpdateManager).getLocalUpdate(txn, contact.getId()); + will(returnValue(local)); + oneOf(mailboxUpdateManager).getRemoteUpdate(txn, contact.getId()); + will(returnValue(remote)); + oneOf(mailboxSettingsManager).getOwnMailboxProperties(txn); + will(returnValue(own)); + }}); + } + + private void expectCheckPluginState(State state) { + context.checking(new Expectations() {{ + oneOf(pluginManager).getPlugin(ID); + will(returnValue(plugin)); + oneOf(plugin).getState(); + will(returnValue(state)); + }}); + } + + private void expectCreateClientForOwnMailbox() { + context.checking(new Expectations() {{ + oneOf(mailboxClientFactory).createOwnMailboxClient( + reachabilityMonitor, ownProperties); + will(returnValue(ownClient)); + oneOf(ownClient).start(); + }}); + } + + private void expectCreateClientForContactMailbox() { + context.checking(new Expectations() {{ + oneOf(mailboxClientFactory) + .createContactMailboxClient(reachabilityMonitor); + will(returnValue(contactClient)); + oneOf(contactClient).start(); + }}); + } + + private void expectAssignContactToOwnMailboxForDownload() { + context.checking(new Expectations() {{ + oneOf(ownClient).assignContactForDownload(contact.getId(), + ownProperties, + requireNonNull(localProperties.getOutboxId())); + }}); + } + + private void expectAssignContactToOwnMailboxForUpload() { + context.checking(new Expectations() {{ + oneOf(ownClient).assignContactForUpload(contact.getId(), + ownProperties, + requireNonNull(localProperties.getInboxId())); + }}); + } + + private void expectAssignContactToContactMailboxForDownload( + MailboxProperties remoteProperties) { + context.checking(new Expectations() {{ + oneOf(contactClient).assignContactForDownload(contact.getId(), + remoteProperties, + requireNonNull(remoteProperties.getInboxId())); + }}); + } + + private void expectAssignContactToContactMailboxForUpload( + MailboxProperties remoteProperties) { + context.checking(new Expectations() {{ + oneOf(contactClient).assignContactForUpload(contact.getId(), + remoteProperties, + requireNonNull(remoteProperties.getOutboxId())); + }}); + } + + private void expectDeassignContactFromOwnMailboxForUpload() { + context.checking(new Expectations() {{ + oneOf(ownClient).deassignContactForUpload(contact.getId()); + }}); + } + + private void expectDeassignContactFromOwnMailboxForDownload() { + context.checking(new Expectations() {{ + oneOf(ownClient).deassignContactForDownload(contact.getId()); + }}); + } + + private void expectDeassignContactFromContactMailboxForDownload() { + context.checking(new Expectations() {{ + oneOf(contactClient).deassignContactForDownload(contact.getId()); + }}); + } + + private void expectRunTaskOnEventExecutor() { + context.checking(new Expectations() {{ + oneOf(eventExecutor).execute(with(any(Runnable.class))); + will(new RunAction()); + }}); + } + + private void expectDestroyClientForOwnMailbox() { + context.checking(new Expectations() {{ + oneOf(ownClient).destroy(); + }}); + } + + private void expectDestroyClientForContactMailbox() { + context.checking(new Expectations() {{ + oneOf(contactClient).destroy(); + }}); + } + + private void expectDestroyReachabilityMonitor() { + context.checking(new Expectations() {{ + oneOf(reachabilityMonitor).destroy(); + }}); + } +} From ab360e1e250988445d11c714d53be4e74ba94f82 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 16 Aug 2022 13:36:15 +0100 Subject: [PATCH 6/8] Address some review comments. --- .../bramble/mailbox/MailboxClientManager.java | 8 ++++---- .../bramble/mailbox/MailboxClientManagerTest.java | 13 ++++++------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java index 3fa1c7046..7057f0cac 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java @@ -95,7 +95,8 @@ class MailboxClientManager implements Service, EventListener { private final TorReachabilityMonitor reachabilityMonitor; private final AtomicBoolean used = new AtomicBoolean(false); - // The following mutable state must only be accessed on the event thread + // All of the following mutable state must only be accessed on the + // event thread private final Map contactUpdates = new HashMap<>(); private final Map contactClients = new HashMap<>(); @@ -343,13 +344,12 @@ class MailboxClientManager implements Service, EventListener { MailboxClient contactClient = requireNonNull(contactClients.get(c)); contactClient.deassignContactForDownload(c); - assignContactToOwnMailboxForDownload(c, u); } else { // The contact doesn't have a usable mailbox, so assign the - // contact to our mailbox for upload and download + // contact to our mailbox for upload assignContactToOwnMailboxForUpload(c, u); - assignContactToOwnMailboxForDownload(c, u); } + assignContactToOwnMailboxForDownload(c, u); } } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxClientManagerTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxClientManagerTest.java index 655f6952a..c658a6d76 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxClientManagerTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxClientManagerTest.java @@ -281,12 +281,11 @@ public class MailboxClientManagerTest extends BrambleMockTestCase { manager.eventOccurred(new TransportActiveEvent(ID)); // When we go offline, the manager should destroy the client for our - // own mailbox + // own mailbox. expectDestroyClientForOwnMailbox(); manager.eventOccurred(new TransportInactiveEvent(ID)); - // At shutdown the manager should destroy the client for our own - // mailbox and the reachability monitor. + // At shutdown the manager should destroy the reachability monitor. expectRunTaskOnEventExecutor(); expectDestroyReachabilityMonitor(); manager.stopService(); @@ -322,8 +321,8 @@ public class MailboxClientManagerTest extends BrambleMockTestCase { @Test public void testReassignsContactToOurMailboxWhenPaired() throws Exception { // At startup the manager should load the local and remote updates - // and our own mailbox properties. We have a mailbox but the contact - // doesn't. + // and our own mailbox properties. The contact has a mailbox but we + // don't. // // We're online, so the manager should create a client for the // contact's mailbox and assign the contact to it for upload and @@ -358,8 +357,8 @@ public class MailboxClientManagerTest extends BrambleMockTestCase { public void testDoesNotAssignContactWhenPairedIfContactHasNotSentUpdate() throws Exception { // At startup the manager should load the local and remote updates - // and our own mailbox properties. We have a mailbox but the contact - // has never sent us an update, so the remote update is null. + // and our own mailbox properties. We don't have a mailbox and the + // contact has never sent us an update, so the remote update is null. expectLoadUpdates(localUpdateWithoutMailbox, null, null); expectCheckPluginState(ACTIVE); manager.startService(); From de76986ee4f8d1207698b3094842e08d97c6e8aa Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 16 Aug 2022 15:06:38 +0100 Subject: [PATCH 7/8] Rename event, only broadcast it when adding a new contact. --- ...> MailboxUpdateSentToNewContactEvent.java} | 17 +++++---- .../bramble/mailbox/MailboxClientManager.java | 7 ++-- .../mailbox/MailboxUpdateManagerImpl.java | 20 ++++++++-- .../mailbox/MailboxUpdateManagerImplTest.java | 37 +++++++++---------- 4 files changed, 46 insertions(+), 35 deletions(-) rename bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/event/{MailboxUpdateSentEvent.java => MailboxUpdateSentToNewContactEvent.java} (56%) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/event/MailboxUpdateSentEvent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/event/MailboxUpdateSentToNewContactEvent.java similarity index 56% rename from bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/event/MailboxUpdateSentEvent.java rename to bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/event/MailboxUpdateSentToNewContactEvent.java index 4017780bd..efba3afd7 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/event/MailboxUpdateSentEvent.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/event/MailboxUpdateSentToNewContactEvent.java @@ -3,27 +3,28 @@ package org.briarproject.bramble.api.mailbox.event; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.event.Event; import org.briarproject.bramble.api.mailbox.MailboxUpdate; +import org.briarproject.bramble.api.mailbox.MailboxUpdateManager; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import javax.annotation.concurrent.Immutable; /** - * An event that is broadcast when a mailbox update is sent to a contact. + * An event that is broadcast when the first mailbox update is sent to a + * newly added contact, which happens in the same transaction in which the + * contact is added. *

- * Note that this event is not broadcast when a mailbox is paired or - * unpaired, although updates are sent to all contacts in those situations. - * - * @see MailboxPairedEvent - * @see MailboxUnpairedEvent + * This event is not broadcast when the first mailbox update is sent to an + * existing contact when setting up the + * {@link MailboxUpdateManager mailbox update client}. */ @Immutable @NotNullByDefault -public class MailboxUpdateSentEvent extends Event { +public class MailboxUpdateSentToNewContactEvent extends Event { private final ContactId contactId; private final MailboxUpdate mailboxUpdate; - public MailboxUpdateSentEvent(ContactId contactId, + public MailboxUpdateSentToNewContactEvent(ContactId contactId, MailboxUpdate mailboxUpdate) { this.contactId = contactId; this.mailboxUpdate = mailboxUpdate; diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java index 7057f0cac..7f23024c5 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java @@ -21,7 +21,7 @@ import org.briarproject.bramble.api.mailbox.MailboxUpdateWithMailbox; import org.briarproject.bramble.api.mailbox.MailboxVersion; import org.briarproject.bramble.api.mailbox.event.MailboxPairedEvent; import org.briarproject.bramble.api.mailbox.event.MailboxUnpairedEvent; -import org.briarproject.bramble.api.mailbox.event.MailboxUpdateSentEvent; +import org.briarproject.bramble.api.mailbox.event.MailboxUpdateSentToNewContactEvent; import org.briarproject.bramble.api.mailbox.event.OwnMailboxConnectionStatusEvent; import org.briarproject.bramble.api.mailbox.event.RemoteMailboxUpdateEvent; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; @@ -269,9 +269,10 @@ class MailboxClientManager implements Service, EventListener { LOG.info("Mailbox unpaired"); MailboxUnpairedEvent m = (MailboxUnpairedEvent) e; onMailboxUnpaired(m.getLocalUpdates()); - } else if (e instanceof MailboxUpdateSentEvent) { + } else if (e instanceof MailboxUpdateSentToNewContactEvent) { LOG.info("Contact added"); - MailboxUpdateSentEvent m = (MailboxUpdateSentEvent) e; + MailboxUpdateSentToNewContactEvent + m = (MailboxUpdateSentToNewContactEvent) e; onContactAdded(m.getContactId(), m.getMailboxUpdate()); } else if (e instanceof ContactRemovedEvent) { LOG.info("Contact removed"); diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxUpdateManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxUpdateManagerImpl.java index 4a6556c52..12dd1694d 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxUpdateManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxUpdateManagerImpl.java @@ -27,7 +27,7 @@ import org.briarproject.bramble.api.mailbox.MailboxUpdateWithMailbox; import org.briarproject.bramble.api.mailbox.MailboxVersion; import org.briarproject.bramble.api.mailbox.event.MailboxPairedEvent; import org.briarproject.bramble.api.mailbox.event.MailboxUnpairedEvent; -import org.briarproject.bramble.api.mailbox.event.MailboxUpdateSentEvent; +import org.briarproject.bramble.api.mailbox.event.MailboxUpdateSentToNewContactEvent; import org.briarproject.bramble.api.mailbox.event.RemoteMailboxUpdateEvent; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.sync.Group; @@ -115,13 +115,12 @@ class MailboxUpdateManagerImpl implements MailboxUpdateManager, } Group g = getContactGroup(c); storeMessageReplaceLatest(txn, g.getId(), updated); - txn.attach(new MailboxUpdateSentEvent(c.getId(), updated)); } } else { db.addGroup(txn, localGroup); // Set things up for any pre-existing contacts for (Contact c : db.getContacts(txn)) { - addingContact(txn, c); + addingContact(txn, c, false); } } @@ -137,6 +136,17 @@ class MailboxUpdateManagerImpl implements MailboxUpdateManager, @Override public void addingContact(Transaction txn, Contact c) throws DbException { + addingContact(txn, c, true); + } + + /** + * @param attachEvent True if a {@link MailboxUpdateSentToNewContactEvent} + * should be attached to the transaction. We should only do this when + * adding a new contact, not when setting up this client for an existing + * contact. + */ + private void addingContact(Transaction txn, Contact c, boolean attachEvent) + throws DbException { // Create a group to share with the contact Group g = getContactGroup(c); db.addGroup(txn, g); @@ -157,7 +167,9 @@ class MailboxUpdateManagerImpl implements MailboxUpdateManager, // Not paired, but we still want to get our clientSupports sent u = sendUpdateNoMailbox(txn, c); } - txn.attach(new MailboxUpdateSentEvent(c.getId(), u)); + if (attachEvent) { + txn.attach(new MailboxUpdateSentToNewContactEvent(c.getId(), u)); + } } @Override diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxUpdateManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxUpdateManagerImplTest.java index f176dc89b..af0893eb9 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxUpdateManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxUpdateManagerImplTest.java @@ -19,7 +19,7 @@ import org.briarproject.bramble.api.mailbox.MailboxUpdateWithMailbox; import org.briarproject.bramble.api.mailbox.MailboxVersion; import org.briarproject.bramble.api.mailbox.event.MailboxPairedEvent; import org.briarproject.bramble.api.mailbox.event.MailboxUnpairedEvent; -import org.briarproject.bramble.api.mailbox.event.MailboxUpdateSentEvent; +import org.briarproject.bramble.api.mailbox.event.MailboxUpdateSentToNewContactEvent; import org.briarproject.bramble.api.mailbox.event.RemoteMailboxUpdateEvent; import org.briarproject.bramble.api.sync.Group; import org.briarproject.bramble.api.sync.GroupId; @@ -187,8 +187,7 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { MailboxUpdateManagerImpl t = createInstance(someClientSupportsList); t.onDatabaseOpened(txn); - MailboxUpdateSentEvent e = getEvent(txn, MailboxUpdateSentEvent.class); - assertNoMailboxPropertiesSent(e, someClientSupportsList); + assertFalse(hasEvent(txn, MailboxUpdateSentToNewContactEvent.class)); } @Test @@ -243,8 +242,7 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { MailboxUpdateManagerImpl t = createInstance(someClientSupportsList); t.onDatabaseOpened(txn); - MailboxUpdateSentEvent e = getEvent(txn, MailboxUpdateSentEvent.class); - assertMailboxPropertiesSent(e, someClientSupportsList); + assertFalse(hasEvent(txn, MailboxUpdateSentToNewContactEvent.class)); } @Test @@ -295,8 +293,7 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { MailboxUpdateManagerImpl t = createInstance(someClientSupportsList); t.onDatabaseOpened(txn1); - MailboxUpdateSentEvent e = getEvent(txn1, MailboxUpdateSentEvent.class); - assertNoMailboxPropertiesSent(e, someClientSupportsList); + assertFalse(hasEvent(txn1, MailboxUpdateSentToNewContactEvent.class)); context.checking(new Expectations() {{ oneOf(db).containsGroup(txn2, localGroup.getId()); @@ -311,7 +308,7 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { t = createInstance(someClientSupportsList); t.onDatabaseOpened(txn2); - assertFalse(hasEvent(txn2, MailboxUpdateSentEvent.class)); + assertFalse(hasEvent(txn2, MailboxUpdateSentToNewContactEvent.class)); } @Test @@ -363,9 +360,7 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { MailboxUpdateManagerImpl t = createInstance(someClientSupportsList); t.onDatabaseOpened(txn1); - MailboxUpdateSentEvent e1 = - getEvent(txn1, MailboxUpdateSentEvent.class); - assertNoMailboxPropertiesSent(e1, someClientSupportsList); + assertFalse(hasEvent(txn1, MailboxUpdateSentToNewContactEvent.class)); BdfDictionary metaDictionary = BdfDictionary.of( new BdfEntry(MSG_KEY_VERSION, 1), @@ -427,9 +422,7 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { t = createInstance(newerClientSupportsList); t.onDatabaseOpened(txn2); - MailboxUpdateSentEvent e2 = - getEvent(txn2, MailboxUpdateSentEvent.class); - assertNoMailboxPropertiesSent(e2, newerClientSupportsList); + assertFalse(hasEvent(txn2, MailboxUpdateSentToNewContactEvent.class)); } @Test @@ -466,7 +459,8 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { MailboxUpdateManagerImpl t = createInstance(someClientSupportsList); t.addingContact(txn, contact); - MailboxUpdateSentEvent e = getEvent(txn, MailboxUpdateSentEvent.class); + MailboxUpdateSentToNewContactEvent + e = getEvent(txn, MailboxUpdateSentToNewContactEvent.class); assertNoMailboxPropertiesSent(e, someClientSupportsList); } @@ -510,7 +504,8 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { MailboxUpdateManagerImpl t = createInstance(someClientSupportsList); t.addingContact(txn, contact); - MailboxUpdateSentEvent e = getEvent(txn, MailboxUpdateSentEvent.class); + MailboxUpdateSentToNewContactEvent + e = getEvent(txn, MailboxUpdateSentToNewContactEvent.class); assertMailboxPropertiesSent(e, someClientSupportsList); } @@ -722,7 +717,7 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { assertEquals(updateWithMailbox.getMailboxProperties(), u.getMailboxProperties()); - assertFalse(hasEvent(txn, MailboxUpdateSentEvent.class)); + assertFalse(hasEvent(txn, MailboxUpdateSentToNewContactEvent.class)); } @Test @@ -765,7 +760,7 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { MailboxUpdate u = localUpdates.get(contact.getId()); assertFalse(u.hasMailbox()); - assertFalse(hasEvent(txn, MailboxUpdateSentEvent.class)); + assertFalse(hasEvent(txn, MailboxUpdateSentToNewContactEvent.class)); } @Test @@ -948,7 +943,8 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { }}); } - private void assertNoMailboxPropertiesSent(MailboxUpdateSentEvent e, + private void assertNoMailboxPropertiesSent( + MailboxUpdateSentToNewContactEvent e, List clientSupports) { assertEquals(contact.getId(), e.getContactId()); MailboxUpdate u = e.getMailboxUpdate(); @@ -956,7 +952,8 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { assertFalse(u.hasMailbox()); } - private void assertMailboxPropertiesSent(MailboxUpdateSentEvent e, + private void assertMailboxPropertiesSent( + MailboxUpdateSentToNewContactEvent e, List clientSupports) { assertEquals(contact.getId(), e.getContactId()); MailboxUpdate u = e.getMailboxUpdate(); From 079ef5b3c09ad6b97013f890cf79da9906674dc6 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 16 Aug 2022 15:11:53 +0100 Subject: [PATCH 8/8] Add helper method for checking client/server compatibility. --- .../bramble/api/mailbox/MailboxHelper.java | 9 +++++++++ .../bramble/mailbox/MailboxClientManager.java | 10 +++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxHelper.java b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxHelper.java index 42740156a..304fded5e 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxHelper.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxHelper.java @@ -35,4 +35,13 @@ public class MailboxHelper { return API_SERVER_TOO_OLD; } + /** + * Returns true if a client and server with the given API versions can + * communicate with each other (ie, have any major API versions in common). + */ + public static boolean isClientCompatibleWithServer( + List client, List server) { + int common = getHighestCommonMajorVersion(client, server); + return common != API_CLIENT_TOO_OLD && common != API_SERVER_TOO_OLD; + } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java index 7f23024c5..fe91f1b32 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClientManager.java @@ -46,7 +46,7 @@ import javax.inject.Inject; import static java.util.logging.Level.WARNING; import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.api.mailbox.MailboxConstants.CLIENT_SUPPORTS; -import static org.briarproject.bramble.api.mailbox.MailboxHelper.getHighestCommonMajorVersion; +import static org.briarproject.bramble.api.mailbox.MailboxHelper.isClientCompatibleWithServer; import static org.briarproject.bramble.api.nullsafety.NullSafety.requireNonNull; import static org.briarproject.bramble.api.plugin.Plugin.State.ACTIVE; import static org.briarproject.bramble.api.plugin.TorConstants.ID; @@ -627,8 +627,8 @@ class MailboxClientManager implements Service, EventListener { */ private boolean isMailboxUsable(List contactClient, List server) { - return getHighestCommonMajorVersion(contactClient, server) >= 0 - && getHighestCommonMajorVersion(CLIENT_SUPPORTS, server) >= 0; + return isClientCompatibleWithServer(contactClient, server) + && isClientCompatibleWithServer(CLIENT_SUPPORTS, server); } /** @@ -637,8 +637,8 @@ class MailboxClientManager implements Service, EventListener { */ @SuppressWarnings("BooleanMethodIsAlwaysInverted") private boolean isOwnMailboxUsable(MailboxProperties ownProperties) { - return getHighestCommonMajorVersion(CLIENT_SUPPORTS, - ownProperties.getServerSupports()) >= 0; + return isClientCompatibleWithServer(CLIENT_SUPPORTS, + ownProperties.getServerSupports()); } /**