From 4d7a3bca6244a0d3ade4f5b5b2e4437871550ec9 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 20 Jun 2022 10:41:13 +0100 Subject: [PATCH] Address review feedback. --- .../mailbox/MailboxFileManagerImpl.java | 10 +--- .../mailbox/MailboxUploadWorkerTest.java | 56 ++++++------------- 2 files changed, 20 insertions(+), 46 deletions(-) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxFileManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxFileManagerImpl.java index 58b5a56cf..82c7c47cb 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxFileManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxFileManagerImpl.java @@ -176,11 +176,7 @@ class MailboxFileManagerImpl implements MailboxFileManager, EventListener { File uploadDir = createDirectoryIfNeeded(UPLOAD_DIR_NAME); File[] orphanedUploads = uploadDir.listFiles(); if (orphanedUploads != null) { - for (File f : orphanedUploads) { - if (!f.delete()) { - LOG.warning("Failed to delete orphaned upload"); - } - } + for (File f : orphanedUploads) delete(f); } File downloadDir = createDirectoryIfNeeded(DOWNLOAD_DIR_NAME); File[] orphanedDownloads = downloadDir.listFiles(); @@ -217,9 +213,7 @@ class MailboxFileManagerImpl implements MailboxFileManager, EventListener { delegate.dispose(exception, recognised); if (isHandlingComplete(exception, recognised)) { LOG.info("Deleting downloaded file"); - if (!file.delete()) { - LOG.warning("Failed to delete downloaded file"); - } + delete(file); } } } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxUploadWorkerTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxUploadWorkerTest.java index 150b92f29..048194983 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxUploadWorkerTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxUploadWorkerTest.java @@ -94,10 +94,7 @@ public class MailboxUploadWorkerTest extends BrambleMockTestCase { public void testChecksForDataWhenStartedAndRemovesObserverWhenDestroyed() throws Exception { // When the worker is started it should check for data to send - context.checking(new DbExpectations() {{ - oneOf(ioExecutor).execute(with(any(Runnable.class))); - will(new RunAction()); - }}); + expectRunTaskOnIoExecutor(); expectCheckForDataToSendNoDataWaiting(); worker.start(); @@ -117,10 +114,7 @@ public class MailboxUploadWorkerTest extends BrambleMockTestCase { // When the worker is started it should check for data to send. As // there's data ready to send immediately, the worker should start a // connectivity check - context.checking(new DbExpectations() {{ - oneOf(ioExecutor).execute(with(any(Runnable.class))); - will(new RunAction()); - }}); + expectRunTaskOnIoExecutor(); expectCheckForDataToSendAndStartConnectivityCheck(); worker.start(); @@ -131,10 +125,9 @@ public class MailboxUploadWorkerTest extends BrambleMockTestCase { // When the connectivity check succeeds, the worker should write a file // and start an upload task + expectRunTaskOnIoExecutor(); AtomicReference upload = new AtomicReference<>(); context.checking(new Expectations() {{ - oneOf(ioExecutor).execute(with(any(Runnable.class))); - will(new RunAction()); oneOf(mailboxFileManager).createAndWriteTempFileForUpload( with(contactId), with(any(OutgoingSessionRecord.class))); will(new DoAllAction( @@ -183,10 +176,7 @@ public class MailboxUploadWorkerTest extends BrambleMockTestCase { // When the worker is started it should check for data to send. As // there's data ready to send immediately, the worker should start a // connectivity check - context.checking(new DbExpectations() {{ - oneOf(ioExecutor).execute(with(any(Runnable.class))); - will(new RunAction()); - }}); + expectRunTaskOnIoExecutor(); expectCheckForDataToSendAndStartConnectivityCheck(); worker.start(); @@ -197,10 +187,9 @@ public class MailboxUploadWorkerTest extends BrambleMockTestCase { // When the connectivity check succeeds, the worker should write a file // and start an upload task + expectRunTaskOnIoExecutor(); AtomicReference upload = new AtomicReference<>(); context.checking(new Expectations() {{ - oneOf(ioExecutor).execute(with(any(Runnable.class))); - will(new RunAction()); oneOf(mailboxFileManager).createAndWriteTempFileForUpload( with(contactId), with(any(OutgoingSessionRecord.class))); will(new DoAllAction( @@ -244,11 +233,8 @@ public class MailboxUploadWorkerTest extends BrambleMockTestCase { // When the worker is started it should check for data to send. As // the data isn't ready to send immediately, the worker should // schedule a wakeup + expectRunTaskOnIoExecutor(); AtomicReference wakeup = new AtomicReference<>(); - context.checking(new DbExpectations() {{ - oneOf(ioExecutor).execute(with(any(Runnable.class))); - will(new RunAction()); - }}); expectCheckForDataToSendAndScheduleWakeup(wakeup); worker.start(); @@ -270,11 +256,8 @@ public class MailboxUploadWorkerTest extends BrambleMockTestCase { // When the worker is started it should check for data to send. As // the data isn't ready to send immediately, the worker should // schedule a wakeup + expectRunTaskOnIoExecutor(); AtomicReference wakeup = new AtomicReference<>(); - context.checking(new DbExpectations() {{ - oneOf(ioExecutor).execute(with(any(Runnable.class))); - will(new RunAction()); - }}); expectCheckForDataToSendAndScheduleWakeup(wakeup); worker.start(); @@ -299,11 +282,8 @@ public class MailboxUploadWorkerTest extends BrambleMockTestCase { // When the worker is started it should check for data to send. As // the data isn't ready to send immediately, the worker should // schedule a wakeup + expectRunTaskOnIoExecutor(); AtomicReference wakeup = new AtomicReference<>(); - context.checking(new DbExpectations() {{ - oneOf(ioExecutor).execute(with(any(Runnable.class))); - will(new RunAction()); - }}); expectCheckForDataToSendAndScheduleWakeup(wakeup); worker.start(); @@ -343,10 +323,7 @@ public class MailboxUploadWorkerTest extends BrambleMockTestCase { @Test public void testCancelsCheckWhenDestroyed() throws Exception { // When the worker is started it should check for data to send - context.checking(new DbExpectations() {{ - oneOf(ioExecutor).execute(with(any(Runnable.class))); - will(new RunAction()); - }}); + expectRunTaskOnIoExecutor(); expectCheckForDataToSendNoDataWaiting(); worker.start(); @@ -379,10 +356,7 @@ public class MailboxUploadWorkerTest extends BrambleMockTestCase { // When the worker is started it should check for data to send. As // there's data ready to send immediately, the worker should start a // connectivity check - context.checking(new DbExpectations() {{ - oneOf(ioExecutor).execute(with(any(Runnable.class))); - will(new RunAction()); - }}); + expectRunTaskOnIoExecutor(); expectCheckForDataToSendAndStartConnectivityCheck(); worker.start(); @@ -390,10 +364,9 @@ public class MailboxUploadWorkerTest extends BrambleMockTestCase { // When the connectivity check succeeds, the worker should try to // write a file. This fails with an exception, so the worker should // retry by scheduling a check for new data after a short delay + expectRunTaskOnIoExecutor(); AtomicReference check = new AtomicReference<>(); context.checking(new Expectations() {{ - oneOf(ioExecutor).execute(with(any(Runnable.class))); - will(new RunAction()); oneOf(mailboxFileManager).createAndWriteTempFileForUpload( with(contactId), with(any(OutgoingSessionRecord.class))); will(throwException(new IOException())); // Oh noes! @@ -414,6 +387,13 @@ public class MailboxUploadWorkerTest extends BrambleMockTestCase { worker.destroy(); } + private void expectRunTaskOnIoExecutor() { + context.checking(new Expectations() {{ + oneOf(ioExecutor).execute(with(any(Runnable.class))); + will(new RunAction()); + }}); + } + private void expectCheckForDataToSendNoDataWaiting() throws Exception { Transaction txn = new Transaction(null, true);