Move deletion of unsent attachments into task.

briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java

@@ -2,6 +2,7 @@ package org.briarproject.briar.android.attachment;
 
 import android.arch.lifecycle.LiveData;
 import android.arch.lifecycle.MutableLiveData;
+import android.arch.lifecycle.Observer;
 import android.content.ContentResolver;
 import android.net.Uri;
 
@@ -19,10 +20,14 @@ import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.concurrent.Executor;
 import java.util.logging.Logger;
 
 import javax.annotation.Nullable;
+import javax.annotation.concurrent.ThreadSafe;
 
+import static java.util.Objects.requireNonNull;
+import static java.util.logging.Level.INFO;
 import static java.util.logging.Level.WARNING;
 import static java.util.logging.Logger.getLogger;
 import static org.briarproject.bramble.util.IoUtils.tryToClose;
@@ -31,12 +36,14 @@ import static org.briarproject.bramble.util.LogUtils.logException;
 import static org.briarproject.bramble.util.LogUtils.now;
 import static org.briarproject.briar.api.messaging.MessagingConstants.IMAGE_MIME_TYPES;
 
+@ThreadSafe
 @NotNullByDefault
 class AttachmentCreationTask {
 
 	private static Logger LOG =
 			getLogger(AttachmentCreationTask.class.getName());
 
+	private final Executor ioExecutor;
 	private final MessagingManager messagingManager;
 	private final ContentResolver contentResolver;
 	private final AttachmentRetriever retriever;
@@ -47,9 +54,11 @@ class AttachmentCreationTask {
 
 	private volatile boolean canceled = false;
 
-	AttachmentCreationTask(MessagingManager messagingManager,
+	AttachmentCreationTask(Executor ioExecutor,
-			ContentResolver contentResolver, AttachmentRetriever retriever,
+			MessagingManager messagingManager, ContentResolver contentResolver,
-			GroupId groupId, Collection<Uri> uris, boolean needsSize) {
+			AttachmentRetriever retriever, GroupId groupId,
+			Collection<Uri> uris, boolean needsSize) {
+		this.ioExecutor = ioExecutor;
 		this.messagingManager = messagingManager;
 		this.contentResolver = contentResolver;
 		this.retriever = retriever;
@@ -63,21 +72,42 @@ class AttachmentCreationTask {
 		return result;
 	}
 
+	/**
+	 * Cancels the task, asynchronously waits for it to finish, and deletes any
+	 * created attachments.
+	 */
 	void cancel() {
 		canceled = true;
+		// Observe the task until it finishes (which may already have happened)
+		result.observeForever(new Observer<AttachmentResult>() {
+			@Override
+			public void onChanged(@Nullable AttachmentResult attachmentResult) {
+				requireNonNull(attachmentResult);
+				if (attachmentResult.isFinished()) {
+					deleteUnsentAttachments(attachmentResult.getItemResults());
+					result.removeObserver(this);
+				}
+			}
+		});
 	}
 
-	@IoExecutor
+	/**
+	 * Asynchronously creates and stores the attachments.
+	 */
 	void storeAttachments() {
-		List<AttachmentItemResult> results = new ArrayList<>();
+		ioExecutor.execute(() -> {
-		for (Uri uri : uris) {
+			if (LOG.isLoggable(INFO))
-			if (canceled) break;
+				LOG.info("Storing " + uris.size() + " attachments");
-			results.add(processUri(uri));
+			List<AttachmentItemResult> results = new ArrayList<>();
+			for (Uri uri : uris) {
+				if (canceled) break;
+				results.add(processUri(uri));
+				result.postValue(new AttachmentResult(new ArrayList<>(results),
+						false, false));
+			}
 			result.postValue(new AttachmentResult(new ArrayList<>(results),
-					false, false));
+					true, !canceled));
-		}
+		});
-		result.postValue(new AttachmentResult(new ArrayList<>(results), true,
-				!canceled));
 	}
 
 	@IoExecutor
@@ -134,4 +164,24 @@ class AttachmentCreationTask {
 			logException(LOG, WARNING, e1);
 		}
 	}
+
+	private void deleteUnsentAttachments(
+			Collection<AttachmentItemResult> itemResults) {
+		List<AttachmentHeader> headers = new ArrayList<>(itemResults.size());
+		for (AttachmentItemResult itemResult : itemResults) {
+			AttachmentItem item = itemResult.getItem();
+			if (item != null) headers.add(item.getHeader());
+		}
+		if (LOG.isLoggable(INFO))
+			LOG.info("Deleting " + headers.size() + " unsent attachments");
+		ioExecutor.execute(() -> {
+			for (AttachmentHeader header : headers) {
+				try {
+					messagingManager.removeAttachment(header);
+				} catch (DbException e) {
+					logException(LOG, WARNING, e);
+				}
+			}
+		});
+	}
 }

briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java

@@ -1,14 +1,11 @@
 package org.briarproject.briar.android.attachment;
 
-
 import android.app.Application;
 import android.arch.lifecycle.LiveData;
-import android.arch.lifecycle.Observer;
 import android.net.Uri;
 import android.support.annotation.Nullable;
 import android.support.annotation.UiThread;
 
-import org.briarproject.bramble.api.db.DbException;
 import org.briarproject.bramble.api.lifecycle.IoExecutor;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.bramble.api.sync.GroupId;
@@ -19,19 +16,12 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.Executor;
-import java.util.logging.Logger;
 
 import static java.util.Collections.emptyList;
-import static java.util.Objects.requireNonNull;
-import static java.util.logging.Level.WARNING;
-import static java.util.logging.Logger.getLogger;
-import static org.briarproject.bramble.util.LogUtils.logException;
 
 @NotNullByDefault
 public class AttachmentCreator {
 
-	private static Logger LOG = getLogger(AttachmentCreator.class.getName());
-
 	private final Application app;
 	@IoExecutor
 	private final Executor ioExecutor;
@@ -58,9 +48,9 @@ public class AttachmentCreator {
 			Collection<Uri> uris) {
 		if (task != null) throw new IllegalStateException();
 		boolean needsSize = uris.size() == 1;
-		task = new AttachmentCreationTask(messagingManager,
+		task = new AttachmentCreationTask(ioExecutor, messagingManager,
 				app.getContentResolver(), retriever, groupId, uris, needsSize);
-		ioExecutor.execute(() -> task.storeAttachments());
+		task.storeAttachments();
 		return task.getResult();
 	}
 
@@ -79,7 +69,8 @@ public class AttachmentCreator {
 
 	/**
 	 * Returns the headers of any attachments created by
-	 * {@link #storeAttachments(GroupId, Collection)}.
+	 * {@link #storeAttachments(GroupId, Collection)}, unless
+	 * {@link #onAttachmentsSent()} or {@link #cancel()} has been called.
 	 */
 	@UiThread
 	public List<AttachmentHeader> getAttachmentHeadersForSending() {
@@ -100,50 +91,19 @@ public class AttachmentCreator {
 	 */
 	@UiThread
 	public void onAttachmentsSent() {
-		task = null;
+		task = null; // Prevent cancel() from cancelling the task
 	}
 
 	/**
 	 * Cancels the task started by
-	 * {@link #storeAttachments(GroupId, Collection)} and deletes any
+	 * {@link #storeAttachments(GroupId, Collection)}, if any, unless
-	 * created attachments, unless {@link #onAttachmentsSent()} has
+	 * {@link #onAttachmentsSent()} has been called.
-	 * been called.
 	 */
 	@UiThread
 	public void cancel() {
-		if (task == null) return; // Already sent or cancelled
+		if (task != null) {
-		task.cancel();
+			task.cancel();
-		// Observe the task until it finishes (which may already have
+			task = null;
-		// happened) and delete any created attachments
-		LiveData<AttachmentResult> taskResult = task.getResult();
-		taskResult.observeForever(new Observer<AttachmentResult>() {
-			@Override
-			public void onChanged(@Nullable AttachmentResult result) {
-				requireNonNull(result);
-				if (result.isFinished()) {
-					deleteUnsentAttachments(result.getItemResults());
-					taskResult.removeObserver(this);
-				}
-			}
-		});
-		task = null;
-	}
-
-	private void deleteUnsentAttachments(
-			Collection<AttachmentItemResult> itemResults) {
-		List<AttachmentHeader> headers = new ArrayList<>(itemResults.size());
-		for (AttachmentItemResult itemResult : itemResults) {
-			AttachmentItem item = itemResult.getItem();
-			if (item != null) headers.add(item.getHeader());
 		}
-		ioExecutor.execute(() -> {
-			for (AttachmentHeader header : headers) {
-				try {
-					messagingManager.removeAttachment(header);
-				} catch (DbException e) {
-					logException(LOG, WARNING, e);
-				}
-			}
-		});
 	}
 }