Address some review comments.

This commit is contained in:
akwizgran
2022-08-16 13:36:15 +01:00
parent 62883b4bde
commit ab360e1e25
2 changed files with 10 additions and 11 deletions

View File

@@ -95,7 +95,8 @@ class MailboxClientManager implements Service, EventListener {
private final TorReachabilityMonitor reachabilityMonitor; private final TorReachabilityMonitor reachabilityMonitor;
private final AtomicBoolean used = new AtomicBoolean(false); 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<ContactId, Updates> contactUpdates = new HashMap<>(); private final Map<ContactId, Updates> contactUpdates = new HashMap<>();
private final Map<ContactId, MailboxClient> contactClients = private final Map<ContactId, MailboxClient> contactClients =
new HashMap<>(); new HashMap<>();
@@ -343,13 +344,12 @@ class MailboxClientManager implements Service, EventListener {
MailboxClient contactClient = MailboxClient contactClient =
requireNonNull(contactClients.get(c)); requireNonNull(contactClients.get(c));
contactClient.deassignContactForDownload(c); contactClient.deassignContactForDownload(c);
assignContactToOwnMailboxForDownload(c, u);
} else { } else {
// The contact doesn't have a usable mailbox, so assign the // 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); assignContactToOwnMailboxForUpload(c, u);
assignContactToOwnMailboxForDownload(c, u);
} }
assignContactToOwnMailboxForDownload(c, u);
} }
} }

View File

@@ -281,12 +281,11 @@ public class MailboxClientManagerTest extends BrambleMockTestCase {
manager.eventOccurred(new TransportActiveEvent(ID)); manager.eventOccurred(new TransportActiveEvent(ID));
// When we go offline, the manager should destroy the client for our // When we go offline, the manager should destroy the client for our
// own mailbox // own mailbox.
expectDestroyClientForOwnMailbox(); expectDestroyClientForOwnMailbox();
manager.eventOccurred(new TransportInactiveEvent(ID)); manager.eventOccurred(new TransportInactiveEvent(ID));
// At shutdown the manager should destroy the client for our own // At shutdown the manager should destroy the reachability monitor.
// mailbox and the reachability monitor.
expectRunTaskOnEventExecutor(); expectRunTaskOnEventExecutor();
expectDestroyReachabilityMonitor(); expectDestroyReachabilityMonitor();
manager.stopService(); manager.stopService();
@@ -322,8 +321,8 @@ public class MailboxClientManagerTest extends BrambleMockTestCase {
@Test @Test
public void testReassignsContactToOurMailboxWhenPaired() throws Exception { public void testReassignsContactToOurMailboxWhenPaired() throws Exception {
// At startup the manager should load the local and remote updates // At startup the manager should load the local and remote updates
// and our own mailbox properties. We have a mailbox but the contact // and our own mailbox properties. The contact has a mailbox but we
// doesn't. // don't.
// //
// We're online, so the manager should create a client for the // We're online, so the manager should create a client for the
// contact's mailbox and assign the contact to it for upload and // contact's mailbox and assign the contact to it for upload and
@@ -358,8 +357,8 @@ public class MailboxClientManagerTest extends BrambleMockTestCase {
public void testDoesNotAssignContactWhenPairedIfContactHasNotSentUpdate() public void testDoesNotAssignContactWhenPairedIfContactHasNotSentUpdate()
throws Exception { throws Exception {
// At startup the manager should load the local and remote updates // At startup the manager should load the local and remote updates
// and our own mailbox properties. We have a mailbox but the contact // and our own mailbox properties. We don't have a mailbox and the
// has never sent us an update, so the remote update is null. // contact has never sent us an update, so the remote update is null.
expectLoadUpdates(localUpdateWithoutMailbox, null, null); expectLoadUpdates(localUpdateWithoutMailbox, null, null);
expectCheckPluginState(ACTIVE); expectCheckPluginState(ACTIVE);
manager.startService(); manager.startService();