From a2a2da0260c6cc806e7fa8966f022b555cab043e Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 20 Jun 2022 11:23:26 +0100 Subject: [PATCH 1/7] Make MailboxSettingsManager a singleton, now that it accepts hooks. --- .../java/org/briarproject/bramble/mailbox/MailboxModule.java | 1 + .../bramble/mailbox/MailboxSettingsManagerImpl.java | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) 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 7fe5d0824..2141851ee 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 @@ -52,6 +52,7 @@ public class MailboxModule { } @Provides + @Singleton MailboxSettingsManager provideMailboxSettingsManager( MailboxSettingsManagerImpl mailboxSettingsManager) { return mailboxSettingsManager; diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImpl.java index 23f9ca486..3e285d930 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSettingsManagerImpl.java @@ -20,13 +20,13 @@ import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import javax.annotation.Nullable; -import javax.annotation.concurrent.Immutable; +import javax.annotation.concurrent.ThreadSafe; import javax.inject.Inject; import static java.util.Collections.emptyList; import static org.briarproject.bramble.util.StringUtils.isNullOrEmpty; -@Immutable +@ThreadSafe @NotNullByDefault class MailboxSettingsManagerImpl implements MailboxSettingsManager { From 0aff23a067a4230b40cf46a5844bf498c704329d Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 20 Jun 2022 11:27:47 +0100 Subject: [PATCH 2/7] Add MailboxWorkerFactory. --- .../bramble/mailbox/MailboxModule.java | 6 ++ .../bramble/mailbox/MailboxWorkerFactory.java | 27 +++++++ .../mailbox/MailboxWorkerFactoryImpl.java | 81 +++++++++++++++++++ 3 files changed, 114 insertions(+) create mode 100644 bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxWorkerFactory.java create mode 100644 bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxWorkerFactoryImpl.java 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 2141851ee..3c62ce3a6 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 @@ -115,4 +115,10 @@ public class MailboxModule { } return mailboxFileManager; } + + @Provides + MailboxWorkerFactory provideMailboxWorkerFactory( + MailboxWorkerFactoryImpl mailboxWorkerFactory) { + return mailboxWorkerFactory; + } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxWorkerFactory.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxWorkerFactory.java new file mode 100644 index 000000000..2f6f827e8 --- /dev/null +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxWorkerFactory.java @@ -0,0 +1,27 @@ +package org.briarproject.bramble.mailbox; + +import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.mailbox.MailboxFolderId; +import org.briarproject.bramble.api.mailbox.MailboxProperties; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import javax.annotation.concurrent.ThreadSafe; + +@ThreadSafe +@NotNullByDefault +interface MailboxWorkerFactory { + + MailboxWorker createUploadWorker(ConnectivityChecker connectivityChecker, + MailboxProperties properties, MailboxFolderId folderId, + ContactId contactId); + + MailboxWorker createDownloadWorkerForContactMailbox( + ConnectivityChecker connectivityChecker, + TorReachabilityMonitor reachabilityMonitor, + MailboxProperties properties); + + MailboxWorker createDownloadWorkerForOwnMailbox( + ConnectivityChecker connectivityChecker, + TorReachabilityMonitor reachabilityMonitor, + MailboxProperties properties); +} diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxWorkerFactoryImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxWorkerFactoryImpl.java new file mode 100644 index 000000000..950cfd749 --- /dev/null +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxWorkerFactoryImpl.java @@ -0,0 +1,81 @@ +package org.briarproject.bramble.mailbox; + +import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.db.DatabaseComponent; +import org.briarproject.bramble.api.event.EventBus; +import org.briarproject.bramble.api.lifecycle.IoExecutor; +import org.briarproject.bramble.api.mailbox.MailboxFolderId; +import org.briarproject.bramble.api.mailbox.MailboxProperties; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.system.Clock; +import org.briarproject.bramble.api.system.TaskScheduler; + +import java.util.concurrent.Executor; + +import javax.annotation.concurrent.Immutable; +import javax.inject.Inject; + +@Immutable +@NotNullByDefault +class MailboxWorkerFactoryImpl implements MailboxWorkerFactory { + + private final Executor ioExecutor; + private final DatabaseComponent db; + private final Clock clock; + private final TaskScheduler taskScheduler; + private final EventBus eventBus; + private final MailboxApiCaller mailboxApiCaller; + private final MailboxApi mailboxApi; + private final MailboxFileManager mailboxFileManager; + + @Inject + MailboxWorkerFactoryImpl(@IoExecutor Executor ioExecutor, + DatabaseComponent db, + Clock clock, + TaskScheduler taskScheduler, + EventBus eventBus, + MailboxApiCaller mailboxApiCaller, + MailboxApi mailboxApi, + MailboxFileManager mailboxFileManager) { + this.ioExecutor = ioExecutor; + this.db = db; + this.clock = clock; + this.taskScheduler = taskScheduler; + this.eventBus = eventBus; + this.mailboxApiCaller = mailboxApiCaller; + this.mailboxApi = mailboxApi; + this.mailboxFileManager = mailboxFileManager; + } + + @Override + public MailboxWorker createUploadWorker( + ConnectivityChecker connectivityChecker, + MailboxProperties properties, MailboxFolderId folderId, + ContactId contactId) { + MailboxUploadWorker worker = new MailboxUploadWorker(ioExecutor, db, + clock, taskScheduler, eventBus, connectivityChecker, + mailboxApiCaller, mailboxApi, mailboxFileManager, + properties, folderId, contactId); + eventBus.addListener(worker); + return worker; + } + + @Override + public MailboxWorker createDownloadWorkerForContactMailbox( + ConnectivityChecker connectivityChecker, + TorReachabilityMonitor reachabilityMonitor, + MailboxProperties properties) { + return new ContactMailboxDownloadWorker(connectivityChecker, + reachabilityMonitor, mailboxApiCaller, mailboxApi, + mailboxFileManager, properties); + } + + @Override + public MailboxWorker createDownloadWorkerForOwnMailbox( + ConnectivityChecker connectivityChecker, + TorReachabilityMonitor reachabilityMonitor, + MailboxProperties properties) { + // TODO + throw new UnsupportedOperationException(); + } +} From aeb2a370e100767e877dfa75aab56518d0c0b16a Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 20 Jun 2022 12:20:15 +0100 Subject: [PATCH 3/7] Return safely if destroy() is called before start(). --- .../bramble/mailbox/ContactMailboxDownloadWorker.java | 2 +- .../org/briarproject/bramble/mailbox/MailboxUploadWorker.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/ContactMailboxDownloadWorker.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/ContactMailboxDownloadWorker.java index 2994dbbc4..a70f18d69 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/ContactMailboxDownloadWorker.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/ContactMailboxDownloadWorker.java @@ -90,7 +90,7 @@ class ContactMailboxDownloadWorker implements MailboxWorker, LOG.info("Started"); synchronized (lock) { // Don't allow the worker to be reused - if (state != State.CREATED) throw new IllegalStateException(); + if (state != State.CREATED) return; state = State.CONNECTIVITY_CHECK; } // Avoid leaking observer in case destroy() is called concurrently diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxUploadWorker.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxUploadWorker.java index e0f3496a1..5c6462c6d 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxUploadWorker.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxUploadWorker.java @@ -147,7 +147,7 @@ class MailboxUploadWorker implements MailboxWorker, ConnectivityObserver, LOG.info("Started"); synchronized (lock) { // Don't allow the worker to be reused - if (state != State.CREATED) throw new IllegalStateException(); + if (state != State.CREATED) return; state = State.CHECKING_FOR_DATA; } ioExecutor.execute(this::checkForDataToSend); From 13c3974f73f03cca94815e836df53a293dddd6de Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 20 Jun 2022 12:24:21 +0100 Subject: [PATCH 4/7] Implement client for a contact's mailbox. --- .../bramble/mailbox/ContactMailboxClient.java | 109 ++++++++++++++++++ .../bramble/mailbox/MailboxClient.java | 46 ++++++++ 2 files changed, 155 insertions(+) create mode 100644 bramble-core/src/main/java/org/briarproject/bramble/mailbox/ContactMailboxClient.java create mode 100644 bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClient.java diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/ContactMailboxClient.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/ContactMailboxClient.java new file mode 100644 index 000000000..35e0f0d4f --- /dev/null +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/ContactMailboxClient.java @@ -0,0 +1,109 @@ +package org.briarproject.bramble.mailbox; + +import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.mailbox.MailboxFolderId; +import org.briarproject.bramble.api.mailbox.MailboxProperties; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import javax.annotation.Nullable; +import javax.annotation.concurrent.GuardedBy; +import javax.annotation.concurrent.ThreadSafe; +import javax.inject.Inject; + +@ThreadSafe +@NotNullByDefault +class ContactMailboxClient implements MailboxClient { + + private final MailboxWorkerFactory workerFactory; + private final ConnectivityChecker connectivityChecker; + private final TorReachabilityMonitor reachabilityMonitor; + private final Object lock = new Object(); + + @GuardedBy("lock") + @Nullable + private MailboxWorker uploadWorker = null, downloadWorker = null; + + @Inject + ContactMailboxClient(MailboxWorkerFactory workerFactory, + ConnectivityChecker connectivityChecker, + TorReachabilityMonitor reachabilityMonitor) { + this.workerFactory = workerFactory; + this.connectivityChecker = connectivityChecker; + this.reachabilityMonitor = reachabilityMonitor; + } + + @Override + public void start() { + // Nothing to do until contact is assigned + } + + @Override + public void destroy() { + MailboxWorker uploadWorker, downloadWorker; + synchronized (lock) { + uploadWorker = this.uploadWorker; + this.uploadWorker = null; + downloadWorker = this.downloadWorker; + this.downloadWorker = null; + } + if (uploadWorker != null) uploadWorker.destroy(); + if (downloadWorker != null) downloadWorker.destroy(); + } + + @Override + public void assignContactForUpload(ContactId contactId, + MailboxProperties properties, MailboxFolderId folderId) { + if (properties.isOwner()) throw new IllegalArgumentException(); + // For a contact's mailbox we should always be uploading to the outbox + // assigned to us by the contact + if (!folderId.equals(properties.getOutboxId())) { + throw new IllegalArgumentException(); + } + MailboxWorker uploadWorker = workerFactory.createUploadWorker( + connectivityChecker, properties, folderId, contactId); + synchronized (lock) { + if (this.uploadWorker != null) throw new IllegalStateException(); + this.uploadWorker = uploadWorker; + } + uploadWorker.start(); + } + + @Override + public void deassignContactForUpload(ContactId contactId) { + MailboxWorker uploadWorker; + synchronized (lock) { + uploadWorker = this.uploadWorker; + this.uploadWorker = null; + } + if (uploadWorker != null) uploadWorker.destroy(); + } + + @Override + public void assignContactForDownload(ContactId contactId, + MailboxProperties properties, MailboxFolderId folderId) { + if (properties.isOwner()) throw new IllegalArgumentException(); + // For a contact's mailbox we should always be downloading from the + // inbox assigned to us by the contact + if (!folderId.equals(properties.getInboxId())) { + throw new IllegalArgumentException(); + } + MailboxWorker downloadWorker = + workerFactory.createDownloadWorkerForContactMailbox( + connectivityChecker, reachabilityMonitor, properties); + synchronized (lock) { + if (this.downloadWorker != null) throw new IllegalStateException(); + this.downloadWorker = downloadWorker; + } + downloadWorker.start(); + } + + @Override + public void deassignContactForDownload(ContactId contactId) { + MailboxWorker downloadWorker; + synchronized (lock) { + downloadWorker = this.downloadWorker; + this.downloadWorker = null; + } + if (downloadWorker != null) downloadWorker.destroy(); + } +} diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClient.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClient.java new file mode 100644 index 000000000..8dc4ab4e1 --- /dev/null +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxClient.java @@ -0,0 +1,46 @@ +package org.briarproject.bramble.mailbox; + +import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.mailbox.MailboxFolderId; +import org.briarproject.bramble.api.mailbox.MailboxProperties; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import javax.annotation.concurrent.ThreadSafe; + +@ThreadSafe +@NotNullByDefault +interface MailboxClient { + + /** + * Asynchronously starts the client. + */ + void start(); + + /** + * Destroys the client and its workers, cancelling any pending tasks or + * retries. + */ + void destroy(); + + /** + * Assigns a contact to the client for upload. + */ + void assignContactForUpload(ContactId c, MailboxProperties properties, + MailboxFolderId folderId); + + /** + * Deassigns a contact from the client for upload. + */ + void deassignContactForUpload(ContactId c); + + /** + * Assigns a contact to the client for download. + */ + void assignContactForDownload(ContactId c, MailboxProperties properties, + MailboxFolderId folderId); + + /** + * Deassigns a contact from the client for download. + */ + void deassignContactForDownload(ContactId c); +} From f8e3579a92099a7e201117d6c93da27c94f715a5 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 20 Jun 2022 13:33:32 +0100 Subject: [PATCH 5/7] Add tests for ContactMailboxClient. --- .../mailbox/ContactMailboxClientTest.java | 136 ++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 bramble-core/src/test/java/org/briarproject/bramble/mailbox/ContactMailboxClientTest.java diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/ContactMailboxClientTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/ContactMailboxClientTest.java new file mode 100644 index 000000000..a264b7650 --- /dev/null +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/ContactMailboxClientTest.java @@ -0,0 +1,136 @@ +package org.briarproject.bramble.mailbox; + +import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.mailbox.MailboxFolderId; +import org.briarproject.bramble.api.mailbox.MailboxProperties; +import org.briarproject.bramble.test.BrambleMockTestCase; +import org.jmock.Expectations; +import org.junit.Before; +import org.junit.Test; + +import static org.briarproject.bramble.api.mailbox.MailboxConstants.CLIENT_SUPPORTS; +import static org.briarproject.bramble.api.nullsafety.NullSafety.requireNonNull; +import static org.briarproject.bramble.test.TestUtils.getContactId; +import static org.briarproject.bramble.test.TestUtils.getMailboxProperties; + +public class ContactMailboxClientTest extends BrambleMockTestCase { + + private final MailboxWorkerFactory workerFactory = + context.mock(MailboxWorkerFactory.class); + private final ConnectivityChecker connectivityChecker = + context.mock(ConnectivityChecker.class); + private final TorReachabilityMonitor reachabilityMonitor = + context.mock(TorReachabilityMonitor.class); + private final MailboxWorker uploadWorker = + context.mock(MailboxWorker.class, "uploadWorker"); + private final MailboxWorker downloadWorker = + context.mock(MailboxWorker.class, "downloadWorker"); + + private final MailboxProperties properties = + getMailboxProperties(false, CLIENT_SUPPORTS); + private final MailboxFolderId inboxId = + requireNonNull(properties.getInboxId()); + private final MailboxFolderId outboxId = + requireNonNull(properties.getOutboxId()); + private final ContactId contactId = getContactId(); + + private ContactMailboxClient client; + + @Before + public void setUp() { + client = new ContactMailboxClient(workerFactory, connectivityChecker, + reachabilityMonitor); + } + + @Test + public void testStartAndDestroyWithNoContactsAssigned() { + client.start(); + client.destroy(); + } + + @Test + public void testAssignContactForUploadAndDestroyClient() { + client.start(); + + // When the contact is assigned, the worker should be created and + // started + expectCreateAndStartUploadWorker(); + client.assignContactForUpload(contactId, properties, outboxId); + + // When the client is destroyed, the worker should be destroyed + expectDestroyWorker(uploadWorker); + client.destroy(); + } + + @Test + public void testAssignAndDeassignContactForUpload() { + client.start(); + + // When the contact is assigned, the worker should be created and + // started + expectCreateAndStartUploadWorker(); + client.assignContactForUpload(contactId, properties, outboxId); + + // When the contact is deassigned, the worker should be destroyed + expectDestroyWorker(uploadWorker); + client.deassignContactForUpload(contactId); + context.assertIsSatisfied(); + + client.destroy(); + } + + @Test + public void assignContactForDownloadAndDestroyClient() { + client.start(); + + // When the contact is assigned, the worker should be created and + // started + expectCreateAndStartDownloadWorker(); + client.assignContactForDownload(contactId, properties, inboxId); + + // When the client is destroyed, the worker should be destroyed + expectDestroyWorker(downloadWorker); + client.destroy(); + } + + @Test + public void assignAndDeassignContactForDownload() { + client.start(); + + // When the contact is assigned, the worker should be created and + // started + expectCreateAndStartDownloadWorker(); + client.assignContactForDownload(contactId, properties, inboxId); + + // When the contact is deassigned, the worker should be destroyed + expectDestroyWorker(downloadWorker); + client.deassignContactForDownload(contactId); + context.assertIsSatisfied(); + + client.destroy(); + } + + private void expectCreateAndStartUploadWorker() { + context.checking(new Expectations() {{ + oneOf(workerFactory).createUploadWorker(connectivityChecker, + properties, outboxId, contactId); + will(returnValue(uploadWorker)); + oneOf(uploadWorker).start(); + }}); + } + + private void expectCreateAndStartDownloadWorker() { + context.checking(new Expectations() {{ + oneOf(workerFactory).createDownloadWorkerForContactMailbox( + connectivityChecker, reachabilityMonitor, properties); + will(returnValue(downloadWorker)); + oneOf(downloadWorker).start(); + }}); + } + + private void expectDestroyWorker(MailboxWorker worker) { + context.checking(new Expectations() {{ + oneOf(worker).destroy(); + }}); + } +} From 0b93af5d7105e506a71c220b0529f25669ca272a Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 20 Jun 2022 13:46:09 +0100 Subject: [PATCH 6/7] Add some logging. --- .../bramble/mailbox/ContactMailboxClient.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/ContactMailboxClient.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/ContactMailboxClient.java index 35e0f0d4f..c27fe977a 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/ContactMailboxClient.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/ContactMailboxClient.java @@ -5,15 +5,22 @@ import org.briarproject.bramble.api.mailbox.MailboxFolderId; import org.briarproject.bramble.api.mailbox.MailboxProperties; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import java.util.logging.Logger; + import javax.annotation.Nullable; import javax.annotation.concurrent.GuardedBy; import javax.annotation.concurrent.ThreadSafe; import javax.inject.Inject; +import static java.util.logging.Logger.getLogger; + @ThreadSafe @NotNullByDefault class ContactMailboxClient implements MailboxClient { + private static final Logger LOG = + getLogger(ContactMailboxClient.class.getName()); + private final MailboxWorkerFactory workerFactory; private final ConnectivityChecker connectivityChecker; private final TorReachabilityMonitor reachabilityMonitor; @@ -34,11 +41,13 @@ class ContactMailboxClient implements MailboxClient { @Override public void start() { + LOG.info("Started"); // Nothing to do until contact is assigned } @Override public void destroy() { + LOG.info("Destroyed"); MailboxWorker uploadWorker, downloadWorker; synchronized (lock) { uploadWorker = this.uploadWorker; @@ -53,6 +62,7 @@ class ContactMailboxClient implements MailboxClient { @Override public void assignContactForUpload(ContactId contactId, MailboxProperties properties, MailboxFolderId folderId) { + LOG.info("Contact assigned for upload"); if (properties.isOwner()) throw new IllegalArgumentException(); // For a contact's mailbox we should always be uploading to the outbox // assigned to us by the contact @@ -70,6 +80,7 @@ class ContactMailboxClient implements MailboxClient { @Override public void deassignContactForUpload(ContactId contactId) { + LOG.info("Contact deassigned for upload"); MailboxWorker uploadWorker; synchronized (lock) { uploadWorker = this.uploadWorker; @@ -81,6 +92,7 @@ class ContactMailboxClient implements MailboxClient { @Override public void assignContactForDownload(ContactId contactId, MailboxProperties properties, MailboxFolderId folderId) { + LOG.info("Contact assigned for download"); if (properties.isOwner()) throw new IllegalArgumentException(); // For a contact's mailbox we should always be downloading from the // inbox assigned to us by the contact @@ -99,6 +111,7 @@ class ContactMailboxClient implements MailboxClient { @Override public void deassignContactForDownload(ContactId contactId) { + LOG.info("Contact deassigned for download"); MailboxWorker downloadWorker; synchronized (lock) { downloadWorker = this.downloadWorker; From 05bf3833cf8f24b2518c3cf06b2031ddf6f66912 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 20 Jun 2022 16:24:55 +0100 Subject: [PATCH 7/7] No need to use @Before to create stateful test objects. --- .../bramble/contact/ContactManagerImplTest.java | 11 +++-------- .../bramble/data/BdfWriterImplTest.java | 11 ++--------- .../bramble/identity/IdentityManagerImplTest.java | 10 ++-------- .../bramble/lifecycle/LifecycleManagerImplTest.java | 9 ++------- .../bramble/mailbox/ContactMailboxClientTest.java | 11 +++-------- .../mailbox/TorReachabilityMonitorImplTest.java | 11 +++-------- .../briarproject/bramble/plugin/PollerImplTest.java | 9 ++------- .../bramble/plugin/tcp/LanTcpPluginTest.java | 9 +++------ .../rendezvous/RendezvousPollerImplTest.java | 13 ++++--------- .../bramble/sync/SyncRecordReaderImplTest.java | 9 ++------- .../sync/validation/ValidationManagerImplTest.java | 8 +++----- .../transport/TransportKeyManagerImplTest.java | 11 +++-------- .../bramble/plugin/modem/ModemPluginTest.java | 9 ++------- 13 files changed, 34 insertions(+), 97 deletions(-) diff --git a/bramble-core/src/test/java/org/briarproject/bramble/contact/ContactManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/contact/ContactManagerImplTest.java index bb4631dc1..bb4bcb67d 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/contact/ContactManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/contact/ContactManagerImplTest.java @@ -17,7 +17,6 @@ import org.briarproject.bramble.api.identity.LocalAuthor; import org.briarproject.bramble.api.transport.KeyManager; import org.briarproject.bramble.test.BrambleMockTestCase; import org.briarproject.bramble.test.DbExpectations; -import org.junit.Before; import org.junit.Test; import java.util.Collection; @@ -63,13 +62,9 @@ public class ContactManagerImplTest extends BrambleMockTestCase { private final long timestamp = System.currentTimeMillis(); private final boolean alice = new Random().nextBoolean(); - private ContactManagerImpl contactManager; - - @Before - public void setUp() { - contactManager = new ContactManagerImpl(db, keyManager, - identityManager, pendingContactFactory); - } + private final ContactManagerImpl contactManager = + new ContactManagerImpl(db, keyManager, identityManager, + pendingContactFactory); @Test public void testAddContact() throws Exception { diff --git a/bramble-core/src/test/java/org/briarproject/bramble/data/BdfWriterImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/data/BdfWriterImplTest.java index 22d328166..d643a9b62 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/data/BdfWriterImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/data/BdfWriterImplTest.java @@ -2,7 +2,6 @@ package org.briarproject.bramble.data; import org.briarproject.bramble.test.BrambleTestCase; import org.briarproject.bramble.util.StringUtils; -import org.junit.Before; import org.junit.Test; import java.io.ByteArrayOutputStream; @@ -17,14 +16,8 @@ import static org.junit.Assert.assertArrayEquals; public class BdfWriterImplTest extends BrambleTestCase { - private ByteArrayOutputStream out = null; - private BdfWriterImpl w = null; - - @Before - public void setUp() { - out = new ByteArrayOutputStream(); - w = new BdfWriterImpl(out); - } + private final ByteArrayOutputStream out = new ByteArrayOutputStream(); + private final BdfWriterImpl w = new BdfWriterImpl(out); @Test public void testWriteNull() throws IOException { diff --git a/bramble-core/src/test/java/org/briarproject/bramble/identity/IdentityManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/identity/IdentityManagerImplTest.java index 16d2f8135..b729d6aeb 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/identity/IdentityManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/identity/IdentityManagerImplTest.java @@ -14,7 +14,6 @@ import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.test.BrambleMockTestCase; import org.briarproject.bramble.test.DbExpectations; import org.jmock.Expectations; -import org.junit.Before; import org.junit.Test; import static java.util.Collections.singletonList; @@ -41,13 +40,8 @@ public class IdentityManagerImplTest extends BrambleMockTestCase { private final KeyPair handshakeKeyPair = new KeyPair(handshakePublicKey, handshakePrivateKey); - private IdentityManagerImpl identityManager; - - @Before - public void setUp() { - identityManager = - new IdentityManagerImpl(db, crypto, authorFactory, clock); - } + private final IdentityManagerImpl identityManager = + new IdentityManagerImpl(db, crypto, authorFactory, clock); @Test public void testOpenDatabaseIdentityRegistered() throws Exception { diff --git a/bramble-core/src/test/java/org/briarproject/bramble/lifecycle/LifecycleManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/lifecycle/LifecycleManagerImplTest.java index e6c7ac708..0b4bfbcef 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/lifecycle/LifecycleManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/lifecycle/LifecycleManagerImplTest.java @@ -10,7 +10,6 @@ import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.test.BrambleMockTestCase; import org.briarproject.bramble.test.DbExpectations; import org.jmock.Expectations; -import org.junit.Before; import org.junit.Test; import java.util.concurrent.atomic.AtomicBoolean; @@ -34,12 +33,8 @@ public class LifecycleManagerImplTest extends BrambleMockTestCase { private final SecretKey dbKey = getSecretKey(); - private LifecycleManagerImpl lifecycleManager; - - @Before - public void setUp() { - lifecycleManager = new LifecycleManagerImpl(db, eventBus, clock); - } + private final LifecycleManagerImpl lifecycleManager = + new LifecycleManagerImpl(db, eventBus, clock); @Test public void testOpenDatabaseHooksAreCalledAtStartup() throws Exception { diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/ContactMailboxClientTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/ContactMailboxClientTest.java index a264b7650..0ec96d0a5 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/ContactMailboxClientTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/ContactMailboxClientTest.java @@ -5,7 +5,6 @@ import org.briarproject.bramble.api.mailbox.MailboxFolderId; import org.briarproject.bramble.api.mailbox.MailboxProperties; import org.briarproject.bramble.test.BrambleMockTestCase; import org.jmock.Expectations; -import org.junit.Before; import org.junit.Test; import static org.briarproject.bramble.api.mailbox.MailboxConstants.CLIENT_SUPPORTS; @@ -34,13 +33,9 @@ public class ContactMailboxClientTest extends BrambleMockTestCase { requireNonNull(properties.getOutboxId()); private final ContactId contactId = getContactId(); - private ContactMailboxClient client; - - @Before - public void setUp() { - client = new ContactMailboxClient(workerFactory, connectivityChecker, - reachabilityMonitor); - } + private final ContactMailboxClient client = + new ContactMailboxClient(workerFactory, connectivityChecker, + reachabilityMonitor); @Test public void testStartAndDestroyWithNoContactsAssigned() { diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/TorReachabilityMonitorImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/TorReachabilityMonitorImplTest.java index 54e30db85..032095cd6 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/TorReachabilityMonitorImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/TorReachabilityMonitorImplTest.java @@ -12,7 +12,6 @@ import org.briarproject.bramble.test.BrambleMockTestCase; import org.briarproject.bramble.test.CaptureArgumentAction; import org.jmock.Expectations; import org.jmock.lib.action.DoAllAction; -import org.junit.Before; import org.junit.Test; import java.util.concurrent.Executor; @@ -38,13 +37,9 @@ public class TorReachabilityMonitorImplTest extends BrambleMockTestCase { private final TorReachabilityObserver observer = context.mock(TorReachabilityObserver.class); - private TorReachabilityMonitorImpl monitor; - - @Before - public void setUp() { - monitor = new TorReachabilityMonitorImpl(ioExecutor, taskScheduler, - pluginManager, eventBus); - } + private final TorReachabilityMonitorImpl monitor = + new TorReachabilityMonitorImpl(ioExecutor, taskScheduler, + pluginManager, eventBus); @Test public void testSchedulesTaskWhenStartedIfTorIsActive() { diff --git a/bramble-core/src/test/java/org/briarproject/bramble/plugin/PollerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/plugin/PollerImplTest.java index b06539c59..2e4930b21 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/plugin/PollerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/plugin/PollerImplTest.java @@ -26,7 +26,6 @@ import org.briarproject.bramble.test.ImmediateExecutor; import org.briarproject.bramble.test.RunAction; import org.jmock.Expectations; import org.jmock.imposters.ByteBuddyClassImposteriser; -import org.junit.Before; import org.junit.Test; import java.security.SecureRandom; @@ -59,22 +58,18 @@ public class PollerImplTest extends BrambleMockTestCase { private final SecureRandom random; private final Executor ioExecutor = new ImmediateExecutor(); - private final Executor wakefulIoExecutor = new ImmediateExecutor(); private final TransportId transportId = getTransportId(); private final ContactId contactId = getContactId(); private final TransportProperties properties = new TransportProperties(); private final int pollingInterval = 60 * 1000; private final long now = System.currentTimeMillis(); - private PollerImpl poller; + private final PollerImpl poller; public PollerImplTest() { context.setImposteriser(ByteBuddyClassImposteriser.INSTANCE); random = context.mock(SecureRandom.class); - } - - @Before - public void setUp() { + Executor wakefulIoExecutor = new ImmediateExecutor(); poller = new PollerImpl(ioExecutor, wakefulIoExecutor, scheduler, connectionManager, connectionRegistry, pluginManager, transportPropertyManager, random, clock); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/plugin/tcp/LanTcpPluginTest.java b/bramble-core/src/test/java/org/briarproject/bramble/plugin/tcp/LanTcpPluginTest.java index cd560e3a6..6b2cd7101 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/plugin/tcp/LanTcpPluginTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/plugin/tcp/LanTcpPluginTest.java @@ -12,7 +12,6 @@ import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; import org.briarproject.bramble.api.properties.TransportProperties; import org.briarproject.bramble.api.settings.Settings; import org.briarproject.bramble.test.BrambleTestCase; -import org.junit.Before; import org.junit.Test; import java.io.IOException; @@ -46,13 +45,11 @@ public class LanTcpPluginTest extends BrambleTestCase { private final Backoff backoff = new TestBackoff(); private final ExecutorService ioExecutor = newCachedThreadPool(); + private final Callback callback = new Callback(); - private Callback callback = null; - private LanTcpPlugin plugin = null; + private final LanTcpPlugin plugin; - @Before - public void setUp() { - callback = new Callback(); + public LanTcpPluginTest() { plugin = new LanTcpPlugin(ioExecutor, ioExecutor, backoff, callback, 0, 0, 1000) { @Override diff --git a/bramble-core/src/test/java/org/briarproject/bramble/rendezvous/RendezvousPollerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/rendezvous/RendezvousPollerImplTest.java index 76c7bda5a..9f5892e07 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/rendezvous/RendezvousPollerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/rendezvous/RendezvousPollerImplTest.java @@ -34,7 +34,6 @@ import org.briarproject.bramble.test.DbExpectations; import org.briarproject.bramble.test.ImmediateExecutor; import org.briarproject.bramble.test.PredicateMatcher; import org.jmock.Expectations; -import org.junit.Before; import org.junit.Test; import java.util.Random; @@ -93,14 +92,10 @@ public class RendezvousPollerImplTest extends BrambleMockTestCase { getTransportProperties(3); private final boolean alice = new Random().nextBoolean(); - private RendezvousPollerImpl rendezvousPoller; - - @Before - public void setUp() { - rendezvousPoller = new RendezvousPollerImpl(ioExecutor, scheduler, db, - identityManager, transportCrypto, rendezvousCrypto, - pluginManager, connectionManager, eventBus, clock); - } + private final RendezvousPollerImpl rendezvousPoller = + new RendezvousPollerImpl(ioExecutor, scheduler, db, + identityManager, transportCrypto, rendezvousCrypto, + pluginManager, connectionManager, eventBus, clock); @Test public void testAddsPendingContactsAndSchedulesPollingAtStartup() diff --git a/bramble-core/src/test/java/org/briarproject/bramble/sync/SyncRecordReaderImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/sync/SyncRecordReaderImplTest.java index 740451837..2c6b0002b 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/sync/SyncRecordReaderImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/sync/SyncRecordReaderImplTest.java @@ -14,7 +14,6 @@ import org.briarproject.bramble.api.sync.SyncRecordReader; import org.briarproject.bramble.api.sync.Versions; import org.briarproject.bramble.test.BrambleMockTestCase; import org.jmock.Expectations; -import org.junit.Before; import org.junit.Test; import java.io.ByteArrayOutputStream; @@ -44,12 +43,8 @@ public class SyncRecordReaderImplTest extends BrambleMockTestCase { context.mock(MessageFactory.class); private final RecordReader recordReader = context.mock(RecordReader.class); - private SyncRecordReader reader; - - @Before - public void setUp() { - reader = new SyncRecordReaderImpl(messageFactory, recordReader); - } + private final SyncRecordReader reader = + new SyncRecordReaderImpl(messageFactory, recordReader); @Test public void testNoFormatExceptionIfAckIsMaximumSize() throws Exception { diff --git a/bramble-core/src/test/java/org/briarproject/bramble/sync/validation/ValidationManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/sync/validation/ValidationManagerImplTest.java index fea01c120..694ee125d 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/sync/validation/ValidationManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/sync/validation/ValidationManagerImplTest.java @@ -20,7 +20,6 @@ import org.briarproject.bramble.api.sync.validation.MessageValidator; import org.briarproject.bramble.test.BrambleMockTestCase; import org.briarproject.bramble.test.DbExpectations; import org.briarproject.bramble.test.ImmediateExecutor; -import org.junit.Before; import org.junit.Test; import java.util.LinkedHashMap; @@ -70,11 +69,10 @@ public class ValidationManagerImplTest extends BrambleMockTestCase { private final MessageContext validResultWithDependencies = new MessageContext(metadata, singletonList(messageId1)); - private ValidationManagerImpl vm; + private final ValidationManagerImpl vm = + new ValidationManagerImpl(db, dbExecutor, validationExecutor); - @Before - public void setUp() { - vm = new ValidationManagerImpl(db, dbExecutor, validationExecutor); + public ValidationManagerImplTest() { vm.registerMessageValidator(clientId, majorVersion, validator); vm.registerIncomingMessageHook(clientId, majorVersion, hook); } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/transport/TransportKeyManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/transport/TransportKeyManagerImplTest.java index 0676e5b4b..8dff76ea9 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/transport/TransportKeyManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/transport/TransportKeyManagerImplTest.java @@ -22,7 +22,6 @@ import org.hamcrest.Description; import org.jmock.Expectations; import org.jmock.api.Action; import org.jmock.api.Invocation; -import org.junit.Before; import org.junit.Test; import java.util.ArrayList; @@ -72,13 +71,9 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { private final SecretKey rootKey = getSecretKey(); private final Random random = new Random(); - private TransportKeyManager transportKeyManager; - - @Before - public void setUp() { - transportKeyManager = new TransportKeyManagerImpl(db, transportCrypto, - dbExecutor, scheduler, clock, transportId, maxLatency); - } + private final TransportKeyManager transportKeyManager = + new TransportKeyManagerImpl(db, transportCrypto, dbExecutor, + scheduler, clock, transportId, maxLatency); @Test public void testKeysAreUpdatedAtStartup() throws Exception { diff --git a/bramble-java/src/test/java/org/briarproject/bramble/plugin/modem/ModemPluginTest.java b/bramble-java/src/test/java/org/briarproject/bramble/plugin/modem/ModemPluginTest.java index 4e3ae0684..672779455 100644 --- a/bramble-java/src/test/java/org/briarproject/bramble/plugin/modem/ModemPluginTest.java +++ b/bramble-java/src/test/java/org/briarproject/bramble/plugin/modem/ModemPluginTest.java @@ -4,7 +4,6 @@ import org.briarproject.bramble.api.plugin.PluginCallback; import org.briarproject.bramble.api.properties.TransportProperties; import org.briarproject.bramble.test.BrambleMockTestCase; import org.jmock.Expectations; -import org.junit.Before; import org.junit.Test; import java.io.IOException; @@ -25,12 +24,8 @@ public class ModemPluginTest extends BrambleMockTestCase { private final PluginCallback callback = context.mock(PluginCallback.class); private final Modem modem = context.mock(Modem.class); - private ModemPlugin plugin; - - @Before - public void setUp() { - plugin = new ModemPlugin(modemFactory, serialPortList, callback, 0); - } + private final ModemPlugin plugin = + new ModemPlugin(modemFactory, serialPortList, callback, 0); @Test public void testModemCreation() throws Exception {