send attachments one after the other with delay

briar-android/src/androidTestOfficial/java/org/briarproject/briar/android/attachment/AttachmentRetrieverIntegrationTest.java

@@ -54,8 +54,8 @@ public class AttachmentRetrieverIntegrationTest {
 	public void testSmallJpegImage() throws Exception {
 		AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg");
 		InputStream is = getUrlInputStream(smallKitten);
-		Attachment a = new Attachment(is);
-		AttachmentItem item = retriever.getAttachmentItem(h, a, true);
+		Attachment a = new Attachment(h, is);
+		AttachmentItem item = retriever.getAttachmentItem(a, true);
 		assertEquals(msgId, item.getMessageId());
 		assertEquals(160, item.getWidth());
 		assertEquals(240, item.getHeight());
@@ -70,8 +70,8 @@ public class AttachmentRetrieverIntegrationTest {
 	public void testBigJpegImage() throws Exception {
 		AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg");
 		InputStream is = getUrlInputStream(originalKitten);
-		Attachment a = new Attachment(is);
-		AttachmentItem item = retriever.getAttachmentItem(h, a, true);
+		Attachment a = new Attachment(h, is);
+		AttachmentItem item = retriever.getAttachmentItem(a, true);
 		assertEquals(msgId, item.getMessageId());
 		assertEquals(1728, item.getWidth());
 		assertEquals(2592, item.getHeight());
@@ -86,8 +86,8 @@ public class AttachmentRetrieverIntegrationTest {
 	public void testSmallPngImage() throws Exception {
 		AttachmentHeader h = new AttachmentHeader(msgId, "image/png");
 		InputStream is = getUrlInputStream(pngKitten);
-		Attachment a = new Attachment(is);
-		AttachmentItem item = retriever.getAttachmentItem(h, a, true);
+		Attachment a = new Attachment(h, is);
+		AttachmentItem item = retriever.getAttachmentItem(a, true);
 		assertEquals(msgId, item.getMessageId());
 		assertEquals(737, item.getWidth());
 		assertEquals(510, item.getHeight());
@@ -102,8 +102,8 @@ public class AttachmentRetrieverIntegrationTest {
 	public void testUberGif() throws Exception {
 		AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg");
 		InputStream is = getUrlInputStream(uberGif);
-		Attachment a = new Attachment(is);
-		AttachmentItem item = retriever.getAttachmentItem(h, a, true);
+		Attachment a = new Attachment(h, is);
+		AttachmentItem item = retriever.getAttachmentItem(a, true);
 		assertEquals(1, item.getWidth());
 		assertEquals(1, item.getHeight());
 		assertEquals(dimensions.minHeight, item.getThumbnailWidth());
@@ -117,8 +117,8 @@ public class AttachmentRetrieverIntegrationTest {
 	public void testLottaPixels() throws Exception {
 		AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg");
 		InputStream is = getUrlInputStream(lottaPixel);
-		Attachment a = new Attachment(is);
-		AttachmentItem item = retriever.getAttachmentItem(h, a, true);
+		Attachment a = new Attachment(h, is);
+		AttachmentItem item = retriever.getAttachmentItem(a, true);
 		assertEquals(64250, item.getWidth());
 		assertEquals(64250, item.getHeight());
 		assertEquals(dimensions.maxWidth, item.getThumbnailWidth());
@@ -132,8 +132,8 @@ public class AttachmentRetrieverIntegrationTest {
 	public void testImageIoCrash() throws Exception {
 		AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg");
 		InputStream is = getUrlInputStream(imageIoCrash);
-		Attachment a = new Attachment(is);
-		AttachmentItem item = retriever.getAttachmentItem(h, a, true);
+		Attachment a = new Attachment(h, is);
+		AttachmentItem item = retriever.getAttachmentItem(a, true);
 		assertEquals(1184, item.getWidth());
 		assertEquals(448, item.getHeight());
 		assertEquals(dimensions.maxWidth, item.getThumbnailWidth());
@@ -147,8 +147,8 @@ public class AttachmentRetrieverIntegrationTest {
 	public void testGimpCrash() throws Exception {
 		AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg");
 		InputStream is = getUrlInputStream(gimpCrash);
-		Attachment a = new Attachment(is);
-		AttachmentItem item = retriever.getAttachmentItem(h, a, true);
+		Attachment a = new Attachment(h, is);
+		AttachmentItem item = retriever.getAttachmentItem(a, true);
 		assertEquals(1, item.getWidth());
 		assertEquals(1, item.getHeight());
 		assertEquals(dimensions.minHeight, item.getThumbnailWidth());
@@ -162,8 +162,8 @@ public class AttachmentRetrieverIntegrationTest {
 	public void testOptiPngAfl() throws Exception {
 		AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg");
 		InputStream is = getUrlInputStream(optiPngAfl);
-		Attachment a = new Attachment(is);
-		AttachmentItem item = retriever.getAttachmentItem(h, a, true);
+		Attachment a = new Attachment(h, is);
+		AttachmentItem item = retriever.getAttachmentItem(a, true);
 		assertEquals(32, item.getWidth());
 		assertEquals(32, item.getHeight());
 		assertEquals(dimensions.minHeight, item.getThumbnailWidth());
@@ -177,8 +177,8 @@ public class AttachmentRetrieverIntegrationTest {
 	public void testLibrawError() throws Exception {
 		AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg");
 		InputStream is = getUrlInputStream(librawError);
-		Attachment a = new Attachment(is);
-		AttachmentItem item = retriever.getAttachmentItem(h, a, true);
+		Attachment a = new Attachment(h, is);
+		AttachmentItem item = retriever.getAttachmentItem(a, true);
 		assertTrue(item.hasError());
 	}
 
@@ -186,8 +186,8 @@ public class AttachmentRetrieverIntegrationTest {
 	public void testSmallAnimatedGifMaxDimensions() throws Exception {
 		AttachmentHeader h = new AttachmentHeader(msgId, "image/gif");
 		InputStream is = getAssetInputStream("animated.gif");
-		Attachment a = new Attachment(is);
-		AttachmentItem item = retriever.getAttachmentItem(h, a, true);
+		Attachment a = new Attachment(h, is);
+		AttachmentItem item = retriever.getAttachmentItem(a, true);
 		assertEquals(65535, item.getWidth());
 		assertEquals(65535, item.getHeight());
 		assertEquals(dimensions.maxWidth, item.getThumbnailWidth());
@@ -201,8 +201,8 @@ public class AttachmentRetrieverIntegrationTest {
 	public void testSmallAnimatedGifHugeDimensions() throws Exception {
 		AttachmentHeader h = new AttachmentHeader(msgId, "image/gif");
 		InputStream is = getAssetInputStream("animated2.gif");
-		Attachment a = new Attachment(is);
-		AttachmentItem item = retriever.getAttachmentItem(h, a, true);
+		Attachment a = new Attachment(h, is);
+		AttachmentItem item = retriever.getAttachmentItem(a, true);
 		assertEquals(10000, item.getWidth());
 		assertEquals(10000, item.getHeight());
 		assertEquals(dimensions.maxWidth, item.getThumbnailWidth());
@@ -216,8 +216,8 @@ public class AttachmentRetrieverIntegrationTest {
 	public void testSmallGifLargeDimensions() throws Exception {
 		AttachmentHeader h = new AttachmentHeader(msgId, "image/gif");
 		InputStream is = getAssetInputStream("error_large.gif");
-		Attachment a = new Attachment(is);
-		AttachmentItem item = retriever.getAttachmentItem(h, a, true);
+		Attachment a = new Attachment(h, is);
+		AttachmentItem item = retriever.getAttachmentItem(a, true);
 		assertEquals(16384, item.getWidth());
 		assertEquals(16384, item.getHeight());
 		assertEquals(dimensions.maxWidth, item.getThumbnailWidth());
@@ -231,8 +231,8 @@ public class AttachmentRetrieverIntegrationTest {
 	public void testHighError() throws Exception {
 		AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg");
 		InputStream is = getAssetInputStream("error_high.jpg");
-		Attachment a = new Attachment(is);
-		AttachmentItem item = retriever.getAttachmentItem(h, a, true);
+		Attachment a = new Attachment(h, is);
+		AttachmentItem item = retriever.getAttachmentItem(a, true);
 		assertEquals(1, item.getWidth());
 		assertEquals(10000, item.getHeight());
 		assertEquals(dimensions.minWidth, item.getThumbnailWidth());
@@ -246,8 +246,8 @@ public class AttachmentRetrieverIntegrationTest {
 	public void testWideError() throws Exception {
 		AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg");
 		InputStream is = getAssetInputStream("error_wide.jpg");
-		Attachment a = new Attachment(is);
-		AttachmentItem item = retriever.getAttachmentItem(h, a, true);
+		Attachment a = new Attachment(h, is);
+		AttachmentItem item = retriever.getAttachmentItem(a, true);
 		assertEquals(1920, item.getWidth());
 		assertEquals(1, item.getHeight());
 		assertEquals(dimensions.maxWidth, item.getThumbnailWidth());

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

@@ -1,33 +1,22 @@
 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;
+import android.support.annotation.Nullable;
 
 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;
-import org.briarproject.briar.api.messaging.Attachment;
 import org.briarproject.briar.api.messaging.AttachmentHeader;
 import org.briarproject.briar.api.messaging.MessagingManager;
 import org.jsoup.UnsupportedMimeTypeException;
 
 import java.io.IOException;
 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;
@@ -36,97 +25,64 @@ 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;
 	private final GroupId groupId;
 	private final Collection<Uri> uris;
 	private final boolean needsSize;
-	private final MutableLiveData<AttachmentResult> result;
+	@Nullable
+	private volatile AttachmentCreator attachmentCreator;
 
 	private volatile boolean canceled = false;
 
-	AttachmentCreationTask(Executor ioExecutor,
-			MessagingManager messagingManager, ContentResolver contentResolver,
-			AttachmentRetriever retriever, GroupId groupId,
+	AttachmentCreationTask(MessagingManager messagingManager,
+			ContentResolver contentResolver,
+			AttachmentCreator attachmentCreator, GroupId groupId,
 			Collection<Uri> uris, boolean needsSize) {
-		this.ioExecutor = ioExecutor;
 		this.messagingManager = messagingManager;
 		this.contentResolver = contentResolver;
-		this.retriever = retriever;
 		this.groupId = groupId;
 		this.uris = uris;
 		this.needsSize = needsSize;
-		result = new MutableLiveData<>();
+		this.attachmentCreator = attachmentCreator;
 	}
 
-	LiveData<AttachmentResult> getResult() {
-		return result;
-	}
-
-	/**
-	 * Cancels the task, asynchronously waits for it to finish, and deletes any
-	 * created attachments.
-	 */
-	void cancel() {
+	public 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);
-				}
-			}
-		});
-	}
-
-	/**
-	 * Asynchronously creates and stores the attachments.
-	 */
-	void storeAttachments() {
-		ioExecutor.execute(() -> {
-			if (LOG.isLoggable(INFO))
-				LOG.info("Storing " + uris.size() + " attachments");
-			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),
-					true, !canceled));
-		});
+		attachmentCreator = null;
 	}
 
 	@IoExecutor
-	private AttachmentItemResult processUri(Uri uri) {
-		AttachmentHeader header = null;
+	public void storeAttachments() {
+		for (Uri uri: uris) processUri(uri);
+		AttachmentCreator attachmentCreator = this.attachmentCreator;
+		if (!canceled && attachmentCreator != null)
+			attachmentCreator.onAttachmentCreationFinished();
+		this.attachmentCreator = null;
+	}
+
+	@IoExecutor
+	private void processUri(Uri uri) {
+		if (canceled) return;
 		try {
-			header = storeAttachment(uri);
-			Attachment a = retriever.getMessageAttachment(header);
-			AttachmentItem item =
-					retriever.getAttachmentItem(header, a, needsSize);
-			if (item.hasError()) throw new IOException();
-			if (needsSize) retriever.cachePut(item);
-			return new AttachmentItemResult(uri, item);
+			AttachmentHeader h = storeAttachment(uri);
+			AttachmentCreator attachmentCreator = this.attachmentCreator;
+			if (attachmentCreator != null) {
+				attachmentCreator.onAttachmentHeaderReceived(uri, h, needsSize);
+			}
 		} catch (DbException | IOException e) {
 			logException(LOG, WARNING, e);
-			// If the attachment was already stored, delete it
-			tryToRemove(header);
+			AttachmentCreator attachmentCreator = this.attachmentCreator;
+			if (attachmentCreator != null) {
+				attachmentCreator.onAttachmentError(uri, e);
+			}
 			canceled = true;
-			return new AttachmentItemResult(uri, e);
 		}
 	}
 
@@ -157,31 +113,4 @@ class AttachmentCreationTask {
 		return false;
 	}
 
-	private void tryToRemove(@Nullable AttachmentHeader h) {
-		try {
-			if (h != null) messagingManager.removeAttachment(h);
-		} catch (DbException e1) {
-			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,33 +1,56 @@
 package org.briarproject.briar.android.attachment;
 
+
 import android.app.Application;
 import android.arch.lifecycle.LiveData;
+import android.arch.lifecycle.MutableLiveData;
 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;
+import org.briarproject.bramble.api.sync.MessageId;
+import org.briarproject.briar.R;
+import org.briarproject.briar.api.messaging.Attachment;
 import org.briarproject.briar.api.messaging.AttachmentHeader;
+import org.briarproject.briar.api.messaging.FileTooBigException;
 import org.briarproject.briar.api.messaging.MessagingManager;
+import org.jsoup.UnsupportedMimeTypeException;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.Executor;
+import java.util.logging.Logger;
 
-import static java.util.Collections.emptyList;
+import static java.util.logging.Level.WARNING;
+import static java.util.logging.Logger.getLogger;
+import static org.briarproject.bramble.util.LogUtils.logException;
+import static org.briarproject.briar.android.util.UiUtils.observeForeverOnce;
+import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_IMAGE_SIZE;
 
 @NotNullByDefault
 public class AttachmentCreator {
 
+	private static Logger LOG = getLogger(AttachmentCreator.class.getName());
+
 	private final Application app;
 	@IoExecutor
 	private final Executor ioExecutor;
 	private final MessagingManager messagingManager;
 	private final AttachmentRetriever retriever;
 
+	private final CopyOnWriteArrayList<Uri> uris = new CopyOnWriteArrayList<>();
+	private final CopyOnWriteArrayList<AttachmentItemResult> itemResults =
+			new CopyOnWriteArrayList<>();
+
+	private final MutableLiveData<AttachmentResult> result =
+			new MutableLiveData<>();
 	@Nullable
 	private AttachmentCreationTask task;
 
@@ -39,19 +62,20 @@ public class AttachmentCreator {
 		this.retriever = retriever;
 	}
 
-	/**
-	 * Starts a background task to create attachments from the given URIs and
-	 * returns a LiveData to monitor the progress of the task.
-	 */
 	@UiThread
-	public LiveData<AttachmentResult> storeAttachments(GroupId groupId,
-			Collection<Uri> uris) {
-		if (task != null) throw new IllegalStateException();
-		boolean needsSize = uris.size() == 1;
-		task = new AttachmentCreationTask(ioExecutor, messagingManager,
-				app.getContentResolver(), retriever, groupId, uris, needsSize);
-		task.storeAttachments();
-		return task.getResult();
+	public LiveData<AttachmentResult> storeAttachments(
+			LiveData<GroupId> groupId, Collection<Uri> newUris) {
+		if (task != null || !uris.isEmpty())
+			throw new IllegalStateException();
+		uris.addAll(newUris);
+		observeForeverOnce(groupId, id -> {
+			if (id == null) throw new IllegalStateException();
+			boolean needsSize = uris.size() == 1;
+			task = new AttachmentCreationTask(messagingManager,
+					app.getContentResolver(), this, id, uris, needsSize);
+			ioExecutor.execute(() -> task.storeAttachments());
+		});
+		return result;
 	}
 
 	/**
@@ -61,49 +85,134 @@ public class AttachmentCreator {
 	 */
 	@UiThread
 	public LiveData<AttachmentResult> getLiveAttachments() {
-		if (task == null) throw new IllegalStateException();
+		if (task == null || uris.isEmpty())
+			throw new IllegalStateException();
 		// A task is already running. It will update the result LiveData.
 		// So nothing more to do here.
-		return task.getResult();
+		return result;
+	}
+
+	@IoExecutor
+	void onAttachmentHeaderReceived(Uri uri, AttachmentHeader h,
+			boolean needsSize) {
+		// get and cache AttachmentItem for ImagePreview
+		try {
+			Attachment a = retriever.getMessageAttachment(h);
+			AttachmentItem item = retriever.getAttachmentItem(a, needsSize);
+			if (item.hasError()) throw new IOException();
+			AttachmentItemResult itemResult =
+					new AttachmentItemResult(uri, item);
+			itemResults.add(itemResult);
+			result.postValue(getResult(false));
+		} catch (IOException | DbException e) {
+			logException(LOG, WARNING, e);
+			onAttachmentError(uri, e);
+		}
+	}
+
+	@IoExecutor
+	void onAttachmentError(Uri uri, Throwable t) {
+		// get error message
+		String errorMsg;
+		if (t instanceof UnsupportedMimeTypeException) {
+			String mimeType = ((UnsupportedMimeTypeException) t).getMimeType();
+			errorMsg = app.getString(
+					R.string.image_attach_error_invalid_mime_type, mimeType);
+		} else if (t instanceof FileTooBigException) {
+			int mb = MAX_IMAGE_SIZE / 1024 / 1024;
+			errorMsg = app.getString(R.string.image_attach_error_too_big, mb);
+		} else {
+			errorMsg = null; // generic error
+		}
+		AttachmentItemResult itemResult =
+				new AttachmentItemResult(uri, errorMsg);
+		itemResults.add(itemResult);
+		result.postValue(getResult(false));
+		// expect to receive a cancel from the UI
+	}
+
+	@IoExecutor
+	void onAttachmentCreationFinished() {
+		result.postValue(getResult(true));
 	}
 
-	/**
-	 * Returns the headers of any attachments created by
-	 * {@link #storeAttachments(GroupId, Collection)}, unless
-	 * {@link #onAttachmentsSent()} or {@link #cancel()} has been called.
-	 */
 	@UiThread
 	public List<AttachmentHeader> getAttachmentHeadersForSending() {
-		if (task == null) return emptyList();
-		AttachmentResult result = task.getResult().getValue();
-		if (result == null) return emptyList();
-		List<AttachmentHeader> headers = new ArrayList<>();
-		for (AttachmentItemResult itemResult : result.getItemResults()) {
-			AttachmentItem item = itemResult.getItem();
-			if (item != null) headers.add(item.getHeader());
+		List<AttachmentHeader> headers = new ArrayList<>(itemResults.size());
+		for (AttachmentItemResult itemResult : itemResults) {
+			// check if we are trying to send attachment items with errors
+			if (itemResult.getItem() == null) throw new IllegalStateException();
+			headers.add(itemResult.getItem().getHeader());
 		}
 		return headers;
 	}
 
 	/**
-	 * Informs the AttachmentCreator that the attachments created by
-	 * {@link #storeAttachments(GroupId, Collection)} will be sent.
+	 * Marks the attachments as sent and adds the items to the cache for display
+	 *
+	 * @param id The MessageId of the sent message.
 	 */
 	@UiThread
-	public void onAttachmentsSent() {
-		task = null; // Prevent cancel() from cancelling the task
+	public void onAttachmentsSent(MessageId id) {
+		List<AttachmentItem> items = new ArrayList<>(itemResults.size());
+		for (AttachmentItemResult itemResult : itemResults) {
+			// check if we are trying to send attachment items with errors
+			if (itemResult.getItem() == null) throw new IllegalStateException();
+			items.add(itemResult.getItem());
+		}
+		retriever.cachePut(id, items);
+		resetState();
 	}
 
 	/**
-	 * Cancels the task started by
-	 * {@link #storeAttachments(GroupId, Collection)}, if any, unless
-	 * {@link #onAttachmentsSent()} has been called.
+	 * Needs to be called when created attachments will not be sent anymore.
 	 */
 	@UiThread
 	public void cancel() {
-		if (task != null) {
-			task.cancel();
-			task = null;
-		}
+		if (task == null) throw new AssertionError();
+		task.cancel();
+		deleteUnsentAttachments();
+		resetState();
 	}
+
+	@UiThread
+	private void resetState() {
+		task = null;
+		uris.clear();
+		itemResults.clear();
+		result.setValue(null);
+	}
+
+	@UiThread
+	public void deleteUnsentAttachments() {
+		// Make a copy for the IoExecutor as we clear the itemResults soon
+		List<AttachmentHeader> headers = new ArrayList<>(itemResults.size());
+		for (AttachmentItemResult itemResult : itemResults) {
+			// check if we are trying to send attachment items with errors
+			if (itemResult.getItem() != null)
+				headers.add(itemResult.getItem().getHeader());
+		}
+		ioExecutor.execute(() -> {
+			for (AttachmentHeader header : headers) {
+				try {
+					messagingManager.removeAttachment(header);
+				} catch (DbException e) {
+					logException(LOG, WARNING, e);
+				}
+			}
+		});
+	}
+
+	private AttachmentResult getResult(boolean finished) {
+		// Make a copy of the list,
+		// because our copy will continue to change in the background.
+		// (As it's a CopyOnWriteArrayList,
+		//  the code that receives the result can safely do simple things
+		//  like iterating over the list,
+		//  but anything that involves calling more than one list method
+		//  is still unsafe.)
+		Collection<AttachmentItemResult> items = new ArrayList<>(itemResults);
+		return new AttachmentResult(items, finished);
+	}
+
 }

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

@@ -68,7 +68,7 @@ public class AttachmentItem implements Parcelable {
 		header = new AttachmentHeader(messageId, mimeType);
 	}
 
-	AttachmentHeader getHeader() {
+	public AttachmentHeader getHeader() {
 		return header;
 	}
 

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

@@ -15,18 +15,18 @@ public class AttachmentItemResult {
 	@Nullable
 	private final AttachmentItem item;
 	@Nullable
-	private final Exception exception;
+	private final String errorMsg;
 
 	AttachmentItemResult(Uri uri, AttachmentItem item) {
 		this.uri = uri;
 		this.item = item;
-		this.exception = null;
+		this.errorMsg = null;
 	}
 
-	AttachmentItemResult(Uri uri, Exception exception) {
+	AttachmentItemResult(Uri uri, @Nullable String errorMsg) {
 		this.uri = uri;
 		this.item = null;
-		this.exception = exception;
+		this.errorMsg = errorMsg;
 	}
 
 	public Uri getUri() {
@@ -43,8 +43,8 @@ public class AttachmentItemResult {
 	}
 
 	@Nullable
-	public Exception getException() {
-		return exception;
+	public String getErrorMsg() {
+		return errorMsg;
 	}
 
 }

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

@@ -12,13 +12,11 @@ public class AttachmentResult {
 
 	private final Collection<AttachmentItemResult> itemResults;
 	private final boolean finished;
-	private final boolean success;
 
-	AttachmentResult(Collection<AttachmentItemResult> itemResults,
-			boolean finished, boolean success) {
+	public AttachmentResult(Collection<AttachmentItemResult> itemResults,
+			boolean finished) {
 		this.itemResults = itemResults;
 		this.finished = finished;
-		this.success = success;
 	}
 
 	public Collection<AttachmentItemResult> getItemResults() {
@@ -29,7 +27,4 @@ public class AttachmentResult {
 		return finished;
 	}
 
-	public boolean isSuccess() {
-		return success;
-	}
 }

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

@@ -9,8 +9,6 @@ import android.webkit.MimeTypeMap;
 
 import com.bumptech.glide.util.MarkEnforcingInputStream;
 
-import org.briarproject.bramble.api.Pair;
-import org.briarproject.bramble.api.db.DatabaseExecutor;
 import org.briarproject.bramble.api.db.DbException;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.bramble.api.sync.MessageId;
@@ -22,7 +20,6 @@ import org.briarproject.briar.api.messaging.MessagingManager;
 import java.io.BufferedInputStream;
 import java.io.IOException;
 import java.io.InputStream;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
@@ -38,16 +35,13 @@ import static android.support.media.ExifInterface.TAG_ORIENTATION;
 import static java.util.logging.Level.WARNING;
 import static java.util.logging.Logger.getLogger;
 import static org.briarproject.bramble.util.IoUtils.tryToClose;
-import static org.briarproject.bramble.util.LogUtils.logDuration;
 import static org.briarproject.bramble.util.LogUtils.logException;
-import static org.briarproject.bramble.util.LogUtils.now;
 
 @NotNullByDefault
 public class AttachmentRetriever {
 
 	private static final Logger LOG =
 			getLogger(AttachmentRetriever.class.getName());
-
 	private static final int READ_LIMIT = 1024 * 8192;
 
 	private final MessagingManager messagingManager;
@@ -56,7 +50,7 @@ public class AttachmentRetriever {
 	private final int minWidth, maxWidth;
 	private final int minHeight, maxHeight;
 
-	private final Map<MessageId, AttachmentItem> attachmentCache =
+	private final Map<MessageId, List<AttachmentItem>> attachmentCache =
 			new ConcurrentHashMap<>();
 
 	@VisibleForTesting
@@ -94,56 +88,27 @@ public class AttachmentRetriever {
 		});
 	}
 
-	public void cachePut(AttachmentItem item) {
-		attachmentCache.put(item.getMessageId(), item);
+	public void cachePut(MessageId messageId,
+			List<AttachmentItem> attachments) {
+		attachmentCache.put(messageId, attachments);
 	}
 
 	@Nullable
-	public AttachmentItem cacheGet(MessageId attachmentId) {
-		return attachmentCache.get(attachmentId);
+	public List<AttachmentItem> cacheGet(MessageId messageId) {
+		return attachmentCache.get(messageId);
 	}
 
-	@DatabaseExecutor
-	public List<Pair<AttachmentHeader, Attachment>> getMessageAttachments(
-			List<AttachmentHeader> headers) throws DbException {
-		long start = now();
-		List<Pair<AttachmentHeader, Attachment>> attachments =
-				new ArrayList<>(headers.size());
-		for (AttachmentHeader h : headers) {
-			Attachment a = messagingManager.getAttachment(h.getMessageId());
-			attachments.add(new Pair<>(h, a));
-		}
-		logDuration(LOG, "Loading attachments", start);
-		return attachments;
-	}
-
-	Attachment getMessageAttachment(AttachmentHeader h) throws DbException {
-		return messagingManager.getAttachment(h.getMessageId());
-	}
-
-	/**
-	 * Creates {@link AttachmentItem}s from the passed headers and Attachments.
-	 * <p>
-	 * Note: This closes the {@link Attachment}'s {@link InputStream}.
-	 */
-	public List<AttachmentItem> getAttachmentItems(
-			List<Pair<AttachmentHeader, Attachment>> attachments) {
-		boolean needsSize = attachments.size() == 1;
-		List<AttachmentItem> items = new ArrayList<>(attachments.size());
-		for (Pair<AttachmentHeader, Attachment> a : attachments) {
-			AttachmentItem item =
-					getAttachmentItem(a.getFirst(), a.getSecond(), needsSize);
-			items.add(item);
-		}
-		return items;
+	public Attachment getMessageAttachment(AttachmentHeader h)
+			throws DbException {
+		return messagingManager.getAttachment(h);
 	}
 
 	/**
 	 * Creates an {@link AttachmentItem} from the {@link Attachment}'s
 	 * {@link InputStream} which will be closed when this method returns.
 	 */
-	AttachmentItem getAttachmentItem(AttachmentHeader h, Attachment a,
-			boolean needsSize) {
+	public AttachmentItem getAttachmentItem(Attachment a, boolean needsSize) {
+		AttachmentHeader h = a.getHeader();
 		if (!needsSize) {
 			String extension =
 					imageHelper.getExtensionFromMimeType(h.getContentType());

briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java

@@ -37,6 +37,7 @@ 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.NoSuchContactException;
+import org.briarproject.bramble.api.db.NoSuchMessageException;
 import org.briarproject.bramble.api.event.Event;
 import org.briarproject.bramble.api.event.EventBus;
 import org.briarproject.bramble.api.event.EventListener;
@@ -81,6 +82,7 @@ import org.briarproject.briar.api.messaging.Attachment;
 import org.briarproject.briar.api.messaging.AttachmentHeader;
 import org.briarproject.briar.api.messaging.MessagingManager;
 import org.briarproject.briar.api.messaging.PrivateMessageHeader;
+import org.briarproject.briar.api.messaging.event.AttachmentReceivedEvent;
 import org.briarproject.briar.api.privategroup.invitation.GroupInvitationManager;
 
 import java.util.ArrayList;
@@ -107,10 +109,12 @@ import static android.support.v7.util.SortedList.INVALID_POSITION;
 import static android.view.Gravity.RIGHT;
 import static android.widget.Toast.LENGTH_SHORT;
 import static java.util.Collections.emptyList;
+import static java.util.Collections.singletonList;
 import static java.util.Collections.sort;
 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.LogUtils.logDuration;
 import static org.briarproject.bramble.util.LogUtils.logException;
 import static org.briarproject.bramble.util.LogUtils.now;
@@ -138,7 +142,7 @@ public class ConversationActivity extends BriarActivity
 	public static final String CONTACT_ID = "briar.CONTACT_ID";
 
 	private static final Logger LOG =
-			Logger.getLogger(ConversationActivity.class.getName());
+			getLogger(ConversationActivity.class.getName());
 
 	private static final int TRANSITION_DURATION_MS = 500;
 	private static final int ONBOARDING_DELAY_MS = 250;
@@ -171,6 +175,8 @@ public class ConversationActivity extends BriarActivity
 	volatile GroupInvitationManager groupInvitationManager;
 
 	private final Map<MessageId, String> textCache = new ConcurrentHashMap<>();
+	private final Map<MessageId, PrivateMessageHeader> missingAttachments =
+			new ConcurrentHashMap<>();
 	private final Observer<String> contactNameObserver = name -> {
 		requireNonNull(name);
 		loadMessages();
@@ -434,31 +440,40 @@ public class ConversationActivity extends BriarActivity
 		});
 	}
 
-	private void eagerlyLoadMessageSize(PrivateMessageHeader h)
-			throws DbException {
-		MessageId id = h.getId();
-		// If the message has text, load it
-		if (h.hasText()) {
-			String text = textCache.get(id);
-			if (text == null) {
-				LOG.info("Eagerly loading text for latest message");
-				text = messagingManager.getMessageText(id);
-				textCache.put(id, requireNonNull(text));
+	private void eagerlyLoadMessageSize(PrivateMessageHeader h) {
+		try {
+			MessageId id = h.getId();
+			// If the message has text, load it
+			if (h.hasText()) {
+				String text = textCache.get(id);
+				if (text == null) {
+					LOG.info("Eagerly loading text for latest message");
+					text = messagingManager.getMessageText(id);
+					textCache.put(id, requireNonNull(text));
+				}
 			}
-		}
-		// If the message has a single image, load its size - for multiple
-		// images we use a grid so the size is fixed
-		List<AttachmentHeader> headers = h.getAttachmentHeaders();
-		if (headers.size() == 1) {
-			MessageId attachmentId = headers.get(0).getMessageId();
-			AttachmentItem item = attachmentRetriever.cacheGet(attachmentId);
-			if (item == null) {
-				LOG.info("Eagerly loading image size for latest message");
-				item = attachmentRetriever.getAttachmentItems(
-						attachmentRetriever.getMessageAttachments(headers))
-						.get(0);
-				attachmentRetriever.cachePut(item);
+			// If the message has a single image, load its size - for multiple
+			// images we use a grid so the size is fixed
+			List<AttachmentHeader> headers = h.getAttachmentHeaders();
+			if (headers.size() == 1) {
+				List<AttachmentItem> items = attachmentRetriever.cacheGet(id);
+				if (items == null) {
+					LOG.info("Eagerly loading image size for latest message");
+					AttachmentHeader header = headers.get(0);
+					try {
+						Attachment a = attachmentRetriever
+								.getMessageAttachment(header);
+						AttachmentItem item =
+								attachmentRetriever.getAttachmentItem(a, true);
+						attachmentRetriever.cachePut(id, singletonList(item));
+					} catch (NoSuchMessageException e) {
+						LOG.info("Attachment not received yet");
+						missingAttachments.put(header.getMessageId(), h);
+					}
+				}
 			}
+		} catch (DbException e) {
+			logException(LOG, WARNING, e);
 		}
 	}
 
@@ -536,18 +551,30 @@ public class ConversationActivity extends BriarActivity
 				&& adapter.isScrolledToBottom(layoutManager);
 	}
 
-	private void loadMessageAttachments(MessageId messageId,
-			List<AttachmentHeader> headers) {
+	private void loadMessageAttachments(PrivateMessageHeader h) {
+		// TODO: Use placeholders for missing/invalid attachments
 		runOnDbThread(() -> {
 			try {
-				List<Pair<AttachmentHeader, Attachment>> attachments =
-						attachmentRetriever.getMessageAttachments(headers);
 				// TODO move getting the items off to IoExecutor, if size == 1
-				List<AttachmentItem> items =
-						attachmentRetriever.getAttachmentItems(attachments);
-				if (items.size() == 1)
-					attachmentRetriever.cachePut(items.get(0));
-				displayMessageAttachments(messageId, items);
+				List<AttachmentHeader> headers = h.getAttachmentHeaders();
+				boolean needsSize = headers.size() == 1;
+				List<AttachmentItem> items = new ArrayList<>(headers.size());
+				for (AttachmentHeader header : headers) {
+					try {
+						Attachment a = attachmentRetriever
+								.getMessageAttachment(header);
+						AttachmentItem item = attachmentRetriever
+								.getAttachmentItem(a, needsSize);
+						items.add(item);
+					} catch (NoSuchMessageException e) {
+						LOG.info("Attachment not received yet");
+						missingAttachments.put(header.getMessageId(), h);
+						return;
+					}
+				}
+				// Don't cache items unless all are present and valid
+				attachmentRetriever.cachePut(h.getId(), items);
+				displayMessageAttachments(h.getId(), items);
 			} catch (DbException e) {
 				logException(LOG, WARNING, e);
 			}
@@ -570,6 +597,13 @@ public class ConversationActivity extends BriarActivity
 
 	@Override
 	public void eventOccurred(Event e) {
+		if (e instanceof AttachmentReceivedEvent) {
+			AttachmentReceivedEvent a = (AttachmentReceivedEvent) e;
+			if (a.getContactId().equals(contactId)) {
+				LOG.info("Attachment received");
+				onAttachmentReceived(a.getMessageId());
+			}
+		}
 		if (e instanceof ContactRemovedEvent) {
 			ContactRemovedEvent c = (ContactRemovedEvent) e;
 			if (c.getContactId().equals(contactId)) {
@@ -620,6 +654,15 @@ public class ConversationActivity extends BriarActivity
 			scrollToBottom();
 	}
 
+	@UiThread
+	private void onAttachmentReceived(MessageId attachmentId) {
+		PrivateMessageHeader h = missingAttachments.remove(attachmentId);
+		if (h != null) {
+			LOG.info("Missing attachment received");
+			loadMessageAttachments(h);
+		}
+	}
+
 	@UiThread
 	private void onNewConversationMessage(ConversationMessageHeader h) {
 		if (h instanceof ConversationRequest ||
@@ -906,19 +949,14 @@ public class ConversationActivity extends BriarActivity
 	}
 
 	@Override
-	public List<AttachmentItem> getAttachmentItems(MessageId m,
-			List<AttachmentHeader> headers) {
-		List<AttachmentItem> items = new ArrayList<>(headers.size());
-		for (AttachmentHeader header : headers) {
-			AttachmentItem item =
-					attachmentRetriever.cacheGet(header.getMessageId());
-			if (item == null) {
-				loadMessageAttachments(m, headers);
-				return emptyList();
-			}
-			items.add(item);
+	public List<AttachmentItem> getAttachmentItems(PrivateMessageHeader h) {
+		List<AttachmentItem> attachments =
+				attachmentRetriever.cacheGet(h.getId());
+		if (attachments == null) {
+			loadMessageAttachments(h);
+			return emptyList();
 		}
-		return items;
+		return attachments;
 	}
 
 }

briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java

@@ -4,7 +4,6 @@ import android.app.Application;
 import android.arch.lifecycle.AndroidViewModel;
 import android.arch.lifecycle.LiveData;
 import android.arch.lifecycle.MutableLiveData;
-import android.arch.lifecycle.Observer;
 import android.arch.lifecycle.Transformations;
 import android.net.Uri;
 import android.support.annotation.Nullable;
@@ -125,7 +124,7 @@ public class ConversationViewModel extends AndroidViewModel
 	@Override
 	protected void onCleared() {
 		super.onCleared();
-		attachmentCreator.cancel();
+		attachmentCreator.deleteUnsentAttachments();
 	}
 
 	/**
@@ -201,23 +200,12 @@ public class ConversationViewModel extends AndroidViewModel
 	@UiThread
 	public LiveData<AttachmentResult> storeAttachments(Collection<Uri> uris,
 			boolean restart) {
-		MutableLiveData<AttachmentResult> delegate = new MutableLiveData<>();
-		// messagingGroupId is loaded with the contact
-		observeForeverOnce(messagingGroupId, groupId -> {
-			requireNonNull(groupId);
-			LiveData<AttachmentResult> result;
-			if (restart) result = attachmentCreator.getLiveAttachments();
-			else result = attachmentCreator.storeAttachments(groupId, uris);
-			result.observeForever(new Observer<AttachmentResult>() {
-				@Override
-				public void onChanged(@Nullable AttachmentResult value) {
-					requireNonNull(value);
-					if (value.isFinished()) result.removeObserver(this);
-					delegate.setValue(value);
-				}
-			});
-		});
-		return delegate;
+		if (restart) {
+			return attachmentCreator.getLiveAttachments();
+		} else {
+			// messagingGroupId is loaded with the contact
+			return attachmentCreator.storeAttachments(messagingGroupId, uris);
+		}
 	}
 
 	@Override
@@ -306,7 +294,7 @@ public class ConversationViewModel extends AndroidViewModel
 	}
 
 	private void storeMessage(PrivateMessage m) {
-		attachmentCreator.onAttachmentsSent();
+		attachmentCreator.onAttachmentsSent(m.getMessage().getId());
 		dbExecutor.execute(() -> {
 			try {
 				long start = now();

briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationVisitor.java

@@ -15,7 +15,6 @@ import org.briarproject.briar.api.forum.ForumInvitationRequest;
 import org.briarproject.briar.api.forum.ForumInvitationResponse;
 import org.briarproject.briar.api.introduction.IntroductionRequest;
 import org.briarproject.briar.api.introduction.IntroductionResponse;
-import org.briarproject.briar.api.messaging.AttachmentHeader;
 import org.briarproject.briar.api.messaging.PrivateMessageHeader;
 import org.briarproject.briar.api.privategroup.invitation.GroupInvitationRequest;
 import org.briarproject.briar.api.privategroup.invitation.GroupInvitationResponse;
@@ -56,8 +55,7 @@ class ConversationVisitor implements
 		if (h.getAttachmentHeaders().isEmpty()) {
 			attachments = emptyList();
 		} else {
-			attachments = attachmentCache
-					.getAttachmentItems(h.getId(), h.getAttachmentHeaders());
+			attachments = attachmentCache.getAttachmentItems(h);
 		}
 		if (h.isLocal()) {
 			item = new ConversationMessageItem(
@@ -295,7 +293,6 @@ class ConversationVisitor implements
 	}
 
 	interface AttachmentCache {
-		List<AttachmentItem> getAttachmentItems(MessageId m,
-				List<AttachmentHeader> headers);
+		List<AttachmentItem> getAttachmentItems(PrivateMessageHeader h);
 	}
 }

briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java

@@ -12,7 +12,6 @@ import org.briarproject.bramble.api.db.DatabaseExecutor;
 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.MessageId;
 import org.briarproject.briar.android.attachment.AttachmentItem;
 import org.briarproject.briar.android.viewmodel.LiveEvent;
 import org.briarproject.briar.android.viewmodel.MutableLiveEvent;
@@ -135,10 +134,10 @@ public class ImageViewModel extends AndroidViewModel {
 
 	private void saveImage(AttachmentItem attachment, OutputStreamProvider osp,
 			@Nullable Runnable afterCopy) {
-		MessageId messageId = attachment.getMessageId();
 		dbExecutor.execute(() -> {
 			try {
-				Attachment a = messagingManager.getAttachment(messageId);
+				Attachment a =
+						messagingManager.getAttachment(attachment.getHeader());
 				copyImageFromDb(a, osp, afterCopy);
 			} catch (DbException e) {
 				logException(LOG, WARNING, e);

briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarDataFetcher.java

@@ -9,8 +9,8 @@ import com.bumptech.glide.load.data.DataFetcher;
 import org.briarproject.bramble.api.db.DatabaseExecutor;
 import org.briarproject.bramble.api.db.DbException;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
-import org.briarproject.bramble.api.sync.MessageId;
 import org.briarproject.briar.android.attachment.AttachmentItem;
+import org.briarproject.briar.api.messaging.Attachment;
 import org.briarproject.briar.api.messaging.MessagingManager;
 
 import java.io.InputStream;
@@ -50,11 +50,12 @@ class BriarDataFetcher implements DataFetcher<InputStream> {
 	@Override
 	public void loadData(Priority priority,
 			DataCallback<? super InputStream> callback) {
-		MessageId id = attachment.getMessageId();
 		dbExecutor.execute(() -> {
 			if (cancel) return;
 			try {
-				inputStream = messagingManager.getAttachment(id).getStream();
+				Attachment a =
+						messagingManager.getAttachment(attachment.getHeader());
+				inputStream = a.getStream();
 				callback.onDataReady(inputStream);
 			} catch (DbException e) {
 				callback.onLoadFailed(e);

briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java

@@ -22,8 +22,6 @@ import org.briarproject.briar.android.attachment.AttachmentItemResult;
 import org.briarproject.briar.android.attachment.AttachmentManager;
 import org.briarproject.briar.android.attachment.AttachmentResult;
 import org.briarproject.briar.android.view.ImagePreview.ImagePreviewListener;
-import org.briarproject.briar.api.messaging.FileTooBigException;
-import org.jsoup.UnsupportedMimeTypeException;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -43,11 +41,9 @@ import static android.support.v4.content.ContextCompat.getColor;
 import static android.support.v4.view.AbsSavedState.EMPTY_STATE;
 import static android.view.View.GONE;
 import static android.widget.Toast.LENGTH_LONG;
-import static java.util.Objects.requireNonNull;
 import static org.briarproject.briar.android.util.UiUtils.resolveColorAttribute;
 import static org.briarproject.briar.api.messaging.MessagingConstants.IMAGE_MIME_TYPES;
 import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_ATTACHMENTS_PER_MESSAGE;
-import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_IMAGE_SIZE;
 import static uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt.STATE_DISMISSED;
 import static uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt.STATE_FINISHED;
 
@@ -190,16 +186,18 @@ public class TextAttachmentController extends TextSendController
 		result.observe(attachmentListener, new Observer<AttachmentResult>() {
 			@Override
 			public void onChanged(@Nullable AttachmentResult attachmentResult) {
-				requireNonNull(attachmentResult);
-				boolean finished = attachmentResult.isFinished();
-				boolean success = attachmentResult.isSuccess();
-				if (finished) {
+				if (attachmentResult == null) {
+					// The fresh LiveData was deliberately set to null.
+					// This means that we can stop observing it.
 					result.removeObserver(this);
-					if (!success) return;
+				} else {
+					boolean noError = onNewAttachmentItemResults(
+							attachmentResult.getItemResults());
+					if (noError && attachmentResult.isFinished()) {
+						onAllAttachmentsCreated();
+						result.removeObserver(this);
+					}
 				}
-				boolean noError = onNewAttachmentItemResults(
-						attachmentResult.getItemResults());
-				if (noError && success) onAllAttachmentsCreated();
 			}
 		});
 	}
@@ -209,7 +207,7 @@ public class TextAttachmentController extends TextSendController
 		if (!loadingUris) throw new AssertionError();
 		for (AttachmentItemResult result : itemResults) {
 			if (result.hasError()) {
-				onError(requireNonNull(result.getException()));
+				onError(result.getErrorMsg());
 				return false;
 			} else {
 				imagePreview.loadPreviewImage(result);
@@ -255,20 +253,12 @@ public class TextAttachmentController extends TextSendController
 	}
 
 	@UiThread
-	private void onError(Exception e) {
-		String errorMsg;
-		Context ctx = imagePreview.getContext();
-		if (e instanceof UnsupportedMimeTypeException) {
-			String mimeType = ((UnsupportedMimeTypeException) e).getMimeType();
-			errorMsg = ctx.getString(
-					R.string.image_attach_error_invalid_mime_type, mimeType);
-		} else if (e instanceof FileTooBigException) {
-			int mb = MAX_IMAGE_SIZE / 1024 / 1024;
-			errorMsg = ctx.getString(R.string.image_attach_error_too_big, mb);
-		} else {
-			errorMsg = ctx.getString(R.string.image_attach_error);
+	private void onError(@Nullable String errorMsg) {
+		if (errorMsg == null) {
+			errorMsg = imagePreview.getContext()
+					.getString(R.string.image_attach_error);
 		}
-		Toast.makeText(ctx, errorMsg, LENGTH_LONG).show();
+		Toast.makeText(textInput.getContext(), errorMsg, LENGTH_LONG).show();
 		onCancel();
 	}
 

briar-android/src/test/java/org/briarproject/briar/android/attachment/AttachmentRetrieverTest.java

@@ -9,7 +9,6 @@ import org.briarproject.briar.api.messaging.MessagingManager;
 import org.jmock.Expectations;
 import org.junit.Test;
 
-import java.io.BufferedInputStream;
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
 
@@ -25,32 +24,26 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase {
 			100, 50, 200, 75, 300
 	);
 	private final MessageId msgId = new MessageId(getRandomId());
-	private final Attachment attachment = new Attachment(
-			new BufferedInputStream(
-					new ByteArrayInputStream(getRandomBytes(42))));
-
 	private final MessagingManager messagingManager =
 			context.mock(MessagingManager.class);
 	private final ImageHelper imageHelper = context.mock(ImageHelper.class);
-	private final AttachmentRetriever controller =
-			new AttachmentRetriever(
-					messagingManager,
-					dimensions,
-					imageHelper
-			);
+	private final AttachmentRetriever retriever = new AttachmentRetriever(
+			messagingManager,
+			dimensions,
+			imageHelper
+	);
 
 	@Test
 	public void testNoSize() {
 		String mimeType = "image/jpeg";
-		AttachmentHeader h = getAttachmentHeader(mimeType);
+		Attachment attachment = getAttachment(mimeType);
 
 		context.checking(new Expectations() {{
 			oneOf(imageHelper).getExtensionFromMimeType(mimeType);
 			will(returnValue("jpg"));
 		}});
 
-		AttachmentItem item =
-				controller.getAttachmentItem(h, attachment, false);
+		AttachmentItem item = retriever.getAttachmentItem(attachment, false);
 		assertEquals(mimeType, item.getMimeType());
 		assertEquals("jpg", item.getExtension());
 		assertFalse(item.hasError());
@@ -59,22 +52,21 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase {
 	@Test
 	public void testNoSizeWrongMimeTypeProducesError() {
 		String mimeType = "application/octet-stream";
-		AttachmentHeader h = getAttachmentHeader(mimeType);
+		Attachment attachment = getAttachment(mimeType);
 
 		context.checking(new Expectations() {{
 			oneOf(imageHelper).getExtensionFromMimeType(mimeType);
 			will(returnValue(null));
 		}});
 
-		AttachmentItem item =
-				controller.getAttachmentItem(h, attachment, false);
+		AttachmentItem item = retriever.getAttachmentItem(attachment, false);
 		assertTrue(item.hasError());
 	}
 
 	@Test
 	public void testSmallJpegImage() {
 		String mimeType = "image/jpeg";
-		AttachmentHeader h = getAttachmentHeader(mimeType);
+		Attachment attachment = getAttachment(mimeType);
 
 		context.checking(new Expectations() {{
 			oneOf(imageHelper).decodeStream(with(any(InputStream.class)));
@@ -83,7 +75,7 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase {
 			will(returnValue("jpg"));
 		}});
 
-		AttachmentItem item = controller.getAttachmentItem(h, attachment, true);
+		AttachmentItem item = retriever.getAttachmentItem(attachment, true);
 		assertEquals(msgId, item.getMessageId());
 		assertEquals(160, item.getWidth());
 		assertEquals(240, item.getHeight());
@@ -97,7 +89,7 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase {
 	@Test
 	public void testBigJpegImage() {
 		String mimeType = "image/jpeg";
-		AttachmentHeader h = getAttachmentHeader(mimeType);
+		Attachment attachment = getAttachment(mimeType);
 
 		context.checking(new Expectations() {{
 			oneOf(imageHelper).decodeStream(with(any(InputStream.class)));
@@ -106,7 +98,7 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase {
 			will(returnValue("jpg"));
 		}});
 
-		AttachmentItem item = controller.getAttachmentItem(h, attachment, true);
+		AttachmentItem item = retriever.getAttachmentItem(attachment, true);
 		assertEquals(1728, item.getWidth());
 		assertEquals(2592, item.getHeight());
 		assertEquals(dimensions.maxWidth, item.getThumbnailWidth());
@@ -116,7 +108,7 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase {
 
 	@Test
 	public void testMalformedError() {
-		AttachmentHeader h = getAttachmentHeader("image/jpeg");
+		Attachment attachment = getAttachment("image/jpeg");
 
 		context.checking(new Expectations() {{
 			oneOf(imageHelper).decodeStream(with(any(InputStream.class)));
@@ -125,12 +117,13 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase {
 			will(returnValue(null));
 		}});
 
-		AttachmentItem item = controller.getAttachmentItem(h, attachment, true);
+		AttachmentItem item = retriever.getAttachmentItem(attachment, true);
 		assertTrue(item.hasError());
 	}
 
-	private AttachmentHeader getAttachmentHeader(String contentType) {
-		return new AttachmentHeader(msgId, contentType);
+	private Attachment getAttachment(String contentType) {
+		AttachmentHeader header = new AttachmentHeader(msgId, contentType);
+		InputStream in = new ByteArrayInputStream(getRandomBytes(42));
+		return new Attachment(header, in);
 	}
-
 }

briar-api/src/main/java/org/briarproject/briar/api/messaging/Attachment.java

@@ -1,15 +1,27 @@
 package org.briarproject.briar.api.messaging;
 
+import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+
 import java.io.InputStream;
 
+import javax.annotation.concurrent.Immutable;
+
+@Immutable
+@NotNullByDefault
 public class Attachment {
 
+	private final AttachmentHeader header;
 	private final InputStream stream;
 
-	public Attachment(InputStream stream) {
+	public Attachment(AttachmentHeader header, InputStream stream) {
+		this.header = header;
 		this.stream = stream;
 	}
 
+	public AttachmentHeader getHeader() {
+		return header;
+	}
+
 	public InputStream getStream() {
 		return stream;
 	}

briar-api/src/main/java/org/briarproject/briar/api/messaging/FileTooBigException.java

@@ -2,8 +2,5 @@ package org.briarproject.briar.api.messaging;
 
 import java.io.IOException;
 
-/**
- * An exception that is thrown when a file is too big to attach to a message.
- */
 public class FileTooBigException extends IOException {
 }

briar-api/src/main/java/org/briarproject/briar/api/messaging/InvalidAttachmentException.java

@@ -0,0 +1,14 @@
+package org.briarproject.briar.api.messaging;
+
+import org.briarproject.bramble.api.db.DbException;
+import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+
+/**
+ * An exception that is thrown when an {@link AttachmentHeader} is used to
+ * load an {@link Attachment}, and the header refers to a message that is not
+ * an attachment, or to an attachment that does not have the expected content
+ * type.
+ */
+@NotNullByDefault
+public class InvalidAttachmentException extends DbException {
+}

briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingManager.java

@@ -40,7 +40,7 @@ public interface MessagingManager extends ConversationClient {
 	/**
 	 * Stores a local attachment message.
 	 *
-	 * @throws FileTooBigException
+	 * @throws FileTooBigException If the attachment is too big
 	 */
 	AttachmentHeader addLocalAttachment(GroupId groupId, long timestamp,
 			String contentType, InputStream is) throws DbException, IOException;
@@ -68,9 +68,13 @@ public interface MessagingManager extends ConversationClient {
 	String getMessageText(MessageId m) throws DbException;
 
 	/**
-	 * Returns the attachment with the given ID.
+	 * Returns the attachment with the given message ID and content type.
+	 *
+	 * @throws InvalidAttachmentException If the header refers to a message
+	 * that is not an attachment, or to an attachment that does not have the
+	 * expected content type
 	 */
-	Attachment getAttachment(MessageId m) throws DbException;
+	Attachment getAttachment(AttachmentHeader h) throws DbException;
 
 	/**
 	 * Returns true if the contact with the given {@link ContactId} does support

briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java

@@ -23,6 +23,7 @@ import org.briarproject.bramble.api.sync.Message;
 import org.briarproject.bramble.api.sync.MessageId;
 import org.briarproject.bramble.api.sync.MessageStatus;
 import org.briarproject.bramble.api.sync.validation.IncomingMessageHook;
+import org.briarproject.bramble.api.system.Scheduler;
 import org.briarproject.bramble.api.versioning.ClientVersioningManager;
 import org.briarproject.bramble.api.versioning.ClientVersioningManager.ClientVersioningHook;
 import org.briarproject.briar.api.client.MessageTracker;
@@ -32,6 +33,7 @@ import org.briarproject.briar.api.conversation.ConversationMessageHeader;
 import org.briarproject.briar.api.messaging.Attachment;
 import org.briarproject.briar.api.messaging.AttachmentHeader;
 import org.briarproject.briar.api.messaging.FileTooBigException;
+import org.briarproject.briar.api.messaging.InvalidAttachmentException;
 import org.briarproject.briar.api.messaging.MessagingManager;
 import org.briarproject.briar.api.messaging.PrivateMessage;
 import org.briarproject.briar.api.messaging.PrivateMessageHeader;
@@ -46,13 +48,19 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.logging.Logger;
 
 import javax.annotation.concurrent.Immutable;
 import javax.inject.Inject;
 
 import static java.util.Collections.emptyList;
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static java.util.logging.Level.WARNING;
+import static java.util.logging.Logger.getLogger;
 import static org.briarproject.bramble.api.sync.SyncConstants.MAX_MESSAGE_BODY_LENGTH;
 import static org.briarproject.bramble.util.IoUtils.copyAndClose;
+import static org.briarproject.bramble.util.LogUtils.logException;
 import static org.briarproject.briar.client.MessageTrackerConstants.MSG_KEY_READ;
 import static org.briarproject.briar.messaging.MessageTypes.ATTACHMENT;
 import static org.briarproject.briar.messaging.MessageTypes.PRIVATE_MESSAGE;
@@ -71,6 +79,10 @@ class MessagingManagerImpl implements MessagingManager, IncomingMessageHook,
 		ConversationClient, OpenDatabaseHook, ContactHook,
 		ClientVersioningHook {
 
+	private static final Logger LOG =
+			getLogger(MessagingManagerImpl.class.getName());
+
+	private final ScheduledExecutorService scheduler;
 	private final DatabaseComponent db;
 	private final ClientHelper clientHelper;
 	private final MetadataParser metadataParser;
@@ -79,10 +91,12 @@ class MessagingManagerImpl implements MessagingManager, IncomingMessageHook,
 	private final ContactGroupFactory contactGroupFactory;
 
 	@Inject
-	MessagingManagerImpl(DatabaseComponent db, ClientHelper clientHelper,
+	MessagingManagerImpl(@Scheduler ScheduledExecutorService scheduler,
+			DatabaseComponent db, ClientHelper clientHelper,
 			ClientVersioningManager clientVersioningManager,
 			MetadataParser metadataParser, MessageTracker messageTracker,
 			ContactGroupFactory contactGroupFactory) {
+		this.scheduler = scheduler;
 		this.db = db;
 		this.clientHelper = clientHelper;
 		this.metadataParser = metadataParser;
@@ -238,9 +252,20 @@ class MessagingManagerImpl implements MessagingManager, IncomingMessageHook,
 				meta.put(MSG_KEY_ATTACHMENT_HEADERS, headers);
 			}
 			// Mark attachments as shared and permanent now we're ready to send
+			// FIXME: Revert
+			int i = 15;
 			for (AttachmentHeader a : m.getAttachmentHeaders()) {
-				db.setMessageShared(txn, a.getMessageId());
-				db.setMessagePermanent(txn, a.getMessageId());
+				scheduler.schedule(() -> {
+					try {
+						db.transaction(false, txn1 -> {
+							db.setMessageShared(txn1, a.getMessageId());
+							db.setMessagePermanent(txn1, a.getMessageId());
+						});
+					} catch (DbException e) {
+						logException(LOG, WARNING, e);
+					}
+				}, i, SECONDS);
+				i *= 2;
 			}
 			clientHelper.addLocalMessage(txn, m.getMessage(), meta, true,
 					false);
@@ -374,13 +399,20 @@ class MessagingManagerImpl implements MessagingManager, IncomingMessageHook,
 	}
 
 	@Override
-	public Attachment getAttachment(MessageId m) throws DbException {
+	public Attachment getAttachment(AttachmentHeader h) throws DbException {
 		// TODO: Support large messages
+		MessageId m = h.getMessageId();
 		byte[] body = clientHelper.getMessage(m).getBody();
 		try {
 			BdfDictionary meta = clientHelper.getMessageMetadataAsDictionary(m);
+			Long messageType = meta.getOptionalLong(MSG_KEY_MSG_TYPE);
+			if (messageType == null || messageType != ATTACHMENT)
+				throw new InvalidAttachmentException();
+			String contentType = meta.getString(MSG_KEY_CONTENT_TYPE);
+			if (!contentType.equals(h.getContentType()))
+				throw new InvalidAttachmentException();
 			int offset = meta.getLong(MSG_KEY_DESCRIPTOR_LENGTH).intValue();
-			return new Attachment(new ByteArrayInputStream(body, offset,
+			return new Attachment(h, new ByteArrayInputStream(body, offset,
 					body.length - offset));
 		} catch (FormatException e) {
 			throw new DbException(e);