[android] Refactor attachment loading to use LiveData

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

@@ -28,7 +28,7 @@ public class AttachmentRetrieverIntegrationTest {
 
 	private final ImageHelper imageHelper = new ImageHelperImpl();
 	private final AttachmentRetriever retriever =
-			new AttachmentRetrieverImpl(null, dimensions, imageHelper,
+			new AttachmentRetrieverImpl(null, null, dimensions, imageHelper,
 					new ImageSizeCalculator(imageHelper));
 
 	@Test

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

@@ -21,7 +21,13 @@ import static org.briarproject.briar.android.attachment.AttachmentItem.State.MIS
 @NotNullByDefault
 public class AttachmentItem implements Parcelable {
 
-	public enum State {LOADING, MISSING, AVAILABLE, ERROR}
+	public enum State {
+		LOADING, MISSING, AVAILABLE, ERROR;
+
+		public boolean isFinal() {
+			return this == AVAILABLE || this == ERROR;
+		}
+	}
 
 	private final AttachmentHeader header;
 	private final int width, height;

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

@@ -1,6 +1,5 @@
 package org.briarproject.briar.android.attachment;
 
-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;
@@ -8,11 +7,12 @@ import org.briarproject.bramble.api.sync.MessageId;
 import org.briarproject.briar.api.messaging.Attachment;
 import org.briarproject.briar.api.messaging.AttachmentHeader;
 import org.briarproject.briar.api.messaging.PrivateMessageHeader;
+import org.briarproject.briar.api.messaging.event.AttachmentReceivedEvent;
 
 import java.io.InputStream;
 import java.util.List;
 
-import androidx.annotation.Nullable;
+import androidx.lifecycle.LiveData;
 
 
 @NotNullByDefault
@@ -21,10 +21,19 @@ public interface AttachmentRetriever {
 	@DatabaseExecutor
 	Attachment getMessageAttachment(AttachmentHeader h) throws DbException;
 
-	List<AttachmentItem> getAttachmentItems(PrivateMessageHeader messageHeader);
+	/**
+	 * Returns a list of observable {@link LiveData}
+	 * that get updated as the state of their {@link AttachmentItem}s changes.
+	 */
+	List<LiveData<AttachmentItem>> getAttachmentItems(
+			PrivateMessageHeader messageHeader);
 
 	/**
 	 * Retrieves item size and adds the item to the cache, if available.
+	 * <p>
+	 * Use this to eagerly load the attachment size before it gets displayed.
+	 * This is needed for messages containing a single attachment.
+	 * Messages with more than one attachment use a standard size.
 	 */
 	@DatabaseExecutor
 	void cacheAttachmentItemWithSize(MessageId conversationMessageId,
@@ -37,15 +46,11 @@ public interface AttachmentRetriever {
 	AttachmentItem createAttachmentItem(Attachment a, boolean needsSize);
 
 	/**
-	 * Load an {@link AttachmentItem} from the database.
+	 * Loads an {@link AttachmentItem}
-	 *
+	 * that arrived via an {@link AttachmentReceivedEvent}
-	 * @return a pair of the {@link MessageId} of the conversation message
+	 * and notifies the associated {@link LiveData}.
-	 * and the {@link AttachmentItem}
-	 * or {@code null} when the private message did not yet arrive.
 	 */
-	@Nullable
 	@DatabaseExecutor
-	Pair<MessageId, AttachmentItem> loadAttachmentItem(MessageId attachmentId)
+	void loadAttachmentItem(MessageId attachmentId);
-			throws DbException;
 
 }

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

@@ -1,6 +1,5 @@
 package org.briarproject.briar.android.attachment;
 
-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.db.NoSuchMessageException;
@@ -18,15 +17,19 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Executor;
 import java.util.logging.Logger;
 
 import javax.inject.Inject;
 
-import androidx.annotation.Nullable;
+import androidx.lifecycle.LiveData;
+import androidx.lifecycle.MutableLiveData;
 
+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.IoUtils.tryToClose;
+import static org.briarproject.bramble.util.LogUtils.logException;
 import static org.briarproject.briar.android.attachment.AttachmentItem.State.AVAILABLE;
 import static org.briarproject.briar.android.attachment.AttachmentItem.State.ERROR;
 import static org.briarproject.briar.android.attachment.AttachmentItem.State.LOADING;
@@ -38,6 +41,8 @@ class AttachmentRetrieverImpl implements AttachmentRetriever {
 	private static final Logger LOG =
 			getLogger(AttachmentRetrieverImpl.class.getName());
 
+	@DatabaseExecutor
+	private final Executor dbExecutor;
 	private final MessagingManager messagingManager;
 	private final ImageHelper imageHelper;
 	private final ImageSizeCalculator imageSizeCalculator;
@@ -45,17 +50,17 @@ class AttachmentRetrieverImpl implements AttachmentRetriever {
 	private final int minWidth, maxWidth;
 	private final int minHeight, maxHeight;
 
-	// Info for AttachmentItems that are either still LOADING or MISSING
+	private final Map<MessageId, MutableLiveData<AttachmentItem>>
-	private final Map<MessageId, UnavailableItem> unavailableItems =
+			itemsWithSize = new ConcurrentHashMap<>();
-			new ConcurrentHashMap<>();
+	private final Map<MessageId, MutableLiveData<AttachmentItem>>
-	// We cache only items in their final state: AVAILABLE or ERROR
+			itemsWithoutSize = new ConcurrentHashMap<>();
-	private final Map<MessageId, AttachmentItem> itemCache =
-			new ConcurrentHashMap<>();
 
 	@Inject
-	AttachmentRetrieverImpl(MessagingManager messagingManager,
+	AttachmentRetrieverImpl(@DatabaseExecutor Executor dbExecutor,
+			MessagingManager messagingManager,
 			AttachmentDimensions dimensions, ImageHelper imageHelper,
 			ImageSizeCalculator imageSizeCalculator) {
+		this.dbExecutor = dbExecutor;
 		this.messagingManager = messagingManager;
 		this.imageHelper = imageHelper;
 		this.imageSizeCalculator = imageSizeCalculator;
@@ -74,20 +79,37 @@ class AttachmentRetrieverImpl implements AttachmentRetriever {
 	}
 
 	@Override
-	public List<AttachmentItem> getAttachmentItems(
+	public List<LiveData<AttachmentItem>> getAttachmentItems(
 			PrivateMessageHeader messageHeader) {
 		List<AttachmentHeader> headers = messageHeader.getAttachmentHeaders();
-		List<AttachmentItem> items = new ArrayList<>(headers.size());
+		List<LiveData<AttachmentItem>> items = new ArrayList<>(headers.size());
 		boolean needsSize = headers.size() == 1;
 		for (AttachmentHeader h : headers) {
-			AttachmentItem item = itemCache.get(h.getMessageId());
+			// try cache for existing item live data
-			if (item == null || (needsSize && !item.hasSize())) {
+			MutableLiveData<AttachmentItem> liveData;
-				item = new AttachmentItem(h, defaultSize, defaultSize, LOADING);
+			if (needsSize) liveData = itemsWithSize.get(h.getMessageId());
-				UnavailableItem unavailableItem = new UnavailableItem(
+			else {
-						messageHeader.getId(), h, needsSize);
+				// try items with size first, as they work as well
-				unavailableItems.put(h.getMessageId(), unavailableItem);
+				liveData = itemsWithSize.get(h.getMessageId());
+				if (liveData == null)
+					liveData = itemsWithoutSize.get(h.getMessageId());
 			}
-			items.add(item);
+
+			// create new live data with LOADING item if cache miss
+			if (liveData == null) {
+				AttachmentItem item = new AttachmentItem(h,
+						defaultSize, defaultSize, LOADING);
+				final MutableLiveData<AttachmentItem> finalLiveData =
+						new MutableLiveData<>(item);
+				// kick-off loading of attachment, will post to live data
+				dbExecutor.execute(
+						() -> loadAttachmentItem(h, needsSize, finalLiveData));
+				// add new LiveData to cache
+				liveData = finalLiveData;
+				if (needsSize) itemsWithSize.put(h.getMessageId(), liveData);
+				else itemsWithoutSize.put(h.getMessageId(), liveData);
+			}
+			items.add(liveData);
 		}
 		return items;
 	}
@@ -98,46 +120,63 @@ class AttachmentRetrieverImpl implements AttachmentRetriever {
 			AttachmentHeader h) throws DbException {
 		try {
 			Attachment a = messagingManager.getAttachment(h);
-			// this adds it to the cache automatically
+			AttachmentItem item = createAttachmentItem(a, true);
-			createAttachmentItem(a, true);
+			MutableLiveData<AttachmentItem> liveData =
+					new MutableLiveData<>(item);
+			itemsWithSize.put(h.getMessageId(), liveData);
 		} catch (NoSuchMessageException e) {
 			LOG.info("Attachment not received yet");
 		}
 	}
 
 	@Override
-	@Nullable
 	@DatabaseExecutor
-	public Pair<MessageId, AttachmentItem> loadAttachmentItem(
+	public void loadAttachmentItem(MessageId attachmentId) {
-			MessageId attachmentId) throws DbException {
+		// try to find LiveData for attachment in both caches
-		UnavailableItem unavailableItem = unavailableItems.get(attachmentId);
+		MutableLiveData<AttachmentItem> liveData;
-		if (unavailableItem == null) return null;
+		boolean needsSize = true;
+		liveData = itemsWithSize.get(attachmentId);
+		if (liveData == null) {
+			needsSize = false;
+			liveData = itemsWithoutSize.get(attachmentId);
+		}
 
-		MessageId conversationMessageId =
+		// If no LiveData for the attachment exists,
-				unavailableItem.getConversationMessageId();
+		// its message did not yet arrive and we can ignore it for now.
-		AttachmentHeader h = unavailableItem.getHeader();
+		if (liveData == null) return;
-		boolean needsSize = unavailableItem.needsSize();
 
+		// actually load the attachment item
+		AttachmentHeader h = requireNonNull(liveData.getValue()).getHeader();
+		loadAttachmentItem(h, needsSize, liveData);
+	}
+
+	/**
+	 * Loads an {@link AttachmentItem} from the database
+	 * and notifies the given {@link LiveData}.
+	 */
+	@DatabaseExecutor
+	private void loadAttachmentItem(AttachmentHeader h, boolean needsSize,
+			MutableLiveData<AttachmentItem> liveData) {
+		Attachment a;
 		AttachmentItem item;
 		try {
-			Attachment a = messagingManager.getAttachment(h);
+			a = messagingManager.getAttachment(h);
 			item = createAttachmentItem(a, needsSize);
-			unavailableItems.remove(attachmentId);
 		} catch (NoSuchMessageException e) {
 			LOG.info("Attachment not received yet");
-			// unavailable item is still tracked, no need to add it again
 			item = new AttachmentItem(h, defaultSize, defaultSize, MISSING);
+		} catch (DbException e) {
+			logException(LOG, WARNING, e);
+			item = new AttachmentItem(h, "", ERROR);
 		}
-		return new Pair<>(conversationMessageId, item);
+		liveData.postValue(item);
 	}
 
 	@Override
 	public AttachmentItem createAttachmentItem(Attachment a,
 			boolean needsSize) {
+		AttachmentItem item;
 		AttachmentHeader h = a.getHeader();
-		AttachmentItem item = itemCache.get(h.getMessageId());
-		if (item != null && (needsSize == item.hasSize())) return item;
-
 		if (needsSize) {
 			InputStream is = new BufferedInputStream(a.getStream());
 			Size size = imageSizeCalculator.getSize(is, h.getContentType());
@@ -153,7 +192,6 @@ class AttachmentRetrieverImpl implements AttachmentRetriever {
 			}
 			item = new AttachmentItem(h, extension, state);
 		}
-		itemCache.put(h.getMessageId(), item);
 		return item;
 	}
 

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

@@ -96,6 +96,7 @@ import androidx.appcompat.widget.Toolbar;
 import androidx.core.app.ActivityCompat;
 import androidx.core.app.ActivityOptionsCompat;
 import androidx.core.content.ContextCompat;
+import androidx.lifecycle.LiveData;
 import androidx.lifecycle.Observer;
 import androidx.lifecycle.ViewModelProvider;
 import androidx.lifecycle.ViewModelProviders;
@@ -130,7 +131,6 @@ import static org.briarproject.bramble.util.StringUtils.isNullOrEmpty;
 import static org.briarproject.bramble.util.StringUtils.join;
 import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_ATTACH_IMAGE;
 import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_INTRODUCTION;
-import static org.briarproject.briar.android.attachment.AttachmentItem.State.LOADING;
 import static org.briarproject.briar.android.conversation.ImageActivity.ATTACHMENTS;
 import static org.briarproject.briar.android.conversation.ImageActivity.ATTACHMENT_POSITION;
 import static org.briarproject.briar.android.conversation.ImageActivity.DATE;
@@ -558,7 +558,8 @@ public class ConversationActivity extends BriarActivity
 				LOG.info("Eagerly loading image size for latest message");
 				AttachmentHeader header = headers.get(0);
 				// get the item to retrieve its size
-				attachmentRetriever.cacheAttachmentItemWithSize(h.getId(), header);
+				attachmentRetriever
+						.cacheAttachmentItemWithSize(h.getId(), header);
 			}
 		} catch (DbException e) {
 			logException(LOG, WARNING, e);
@@ -639,33 +640,6 @@ public class ConversationActivity extends BriarActivity
 				&& adapter.isScrolledToBottom(layoutManager);
 	}
 
-	private void loadMessageAttachments(List<AttachmentItem> items) {
-		runOnDbThread(() -> {
-			for (AttachmentItem item : items) {
-				if (item.getState() == LOADING)
-					loadMessageAttachment(item.getMessageId(), false);
-			}
-		});
-	}
-
-	@DatabaseExecutor
-	private void loadMessageAttachment(MessageId attachmentId,
-			boolean allowedToFail) {
-		try {
-			Pair<MessageId, AttachmentItem> pair = attachmentRetriever
-					.loadAttachmentItem(attachmentId);
-			if (pair == null && allowedToFail) {
-				LOG.warning("Attachment arrived before message");
-				return;
-			} else if (pair == null) throw new AssertionError();
-			MessageId conversationMessageId = pair.getFirst();
-			AttachmentItem item = pair.getSecond();
-			updateMessageAttachment(conversationMessageId, item);
-		} catch (DbException e) {
-			logException(LOG, WARNING, e);
-		}
-	}
-
 	private void updateMessageAttachment(MessageId m, AttachmentItem item) {
 		runOnUiThreadUnlessDestroyed(() -> {
 			Pair<Integer, ConversationMessageItem> pair =
@@ -748,9 +722,8 @@ public class ConversationActivity extends BriarActivity
 
 	@UiThread
 	private void onAttachmentReceived(MessageId attachmentId) {
-		// This is allowed to fail, because the conversation message
+		runOnDbThread(
-		// might arrive *after* the attachment.
+				() -> attachmentRetriever.loadAttachmentItem(attachmentId));
-		runOnDbThread(() -> loadMessageAttachment(attachmentId, true));
 	}
 
 	@UiThread
@@ -1134,9 +1107,32 @@ public class ConversationActivity extends BriarActivity
 	 */
 	@Override
 	public List<AttachmentItem> getAttachmentItems(PrivateMessageHeader h) {
-		List<AttachmentItem> items = attachmentRetriever.getAttachmentItems(h);
+		List<LiveData<AttachmentItem>> liveDataList =
-		loadMessageAttachments(items);
+				attachmentRetriever.getAttachmentItems(h);
+		List<AttachmentItem> items = new ArrayList<>(liveDataList.size());
+		for (LiveData<AttachmentItem> liveData : liveDataList) {
+			liveData.observe(this, new AttachmentObserver(h.getId(), liveData));
+			items.add(requireNonNull(liveData.getValue()));
+		}
 		return items;
 	}
 
+	private class AttachmentObserver implements Observer<AttachmentItem> {
+		private final MessageId conversationMessageId;
+		private final LiveData<AttachmentItem> liveData;
+
+		private AttachmentObserver(MessageId conversationMessageId,
+				LiveData<AttachmentItem> liveData) {
+			this.conversationMessageId = conversationMessageId;
+			this.liveData = liveData;
+		}
+
+		@Override
+		public void onChanged(AttachmentItem attachmentItem) {
+			updateMessageAttachment(conversationMessageId, attachmentItem);
+			if (attachmentItem.getState().isFinal())
+				liveData.removeObserver(this);
+		}
+	}
+
 }

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

@@ -2,6 +2,7 @@ package org.briarproject.briar.android.attachment;
 
 import org.briarproject.bramble.api.sync.MessageId;
 import org.briarproject.bramble.test.BrambleMockTestCase;
+import org.briarproject.bramble.test.ImmediateExecutor;
 import org.briarproject.briar.api.messaging.Attachment;
 import org.briarproject.briar.api.messaging.AttachmentHeader;
 import org.briarproject.briar.api.messaging.MessagingManager;
@@ -11,6 +12,7 @@ import org.junit.Test;
 
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
+import java.util.concurrent.Executor;
 
 import static org.briarproject.bramble.test.TestUtils.getRandomBytes;
 import static org.briarproject.bramble.test.TestUtils.getRandomId;
@@ -33,8 +35,9 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase {
 		MessagingManager messagingManager =
 				context.mock(MessagingManager.class);
 		imageSizeCalculator = context.mock(ImageSizeCalculator.class);
-		retriever = new AttachmentRetrieverImpl(messagingManager, dimensions,
+		Executor dbExecutor = new ImmediateExecutor();
-				imageHelper, imageSizeCalculator);
+		retriever = new AttachmentRetrieverImpl(dbExecutor, messagingManager,
+				dimensions, imageHelper, imageSizeCalculator);
 	}
 
 	@Test