diff --git a/briar-android/src/androidTestOfficial/java/org/briarproject/briar/android/attachment/AttachmentRetrieverIntegrationTest.java b/briar-android/src/androidTestOfficial/java/org/briarproject/briar/android/attachment/AttachmentRetrieverIntegrationTest.java index 55e847ce0..866aecec4 100644 --- a/briar-android/src/androidTestOfficial/java/org/briarproject/briar/android/attachment/AttachmentRetrieverIntegrationTest.java +++ b/briar-android/src/androidTestOfficial/java/org/briarproject/briar/android/attachment/AttachmentRetrieverIntegrationTest.java @@ -13,8 +13,9 @@ import java.util.Random; import androidx.test.ext.junit.runners.AndroidJUnit4; import static androidx.test.InstrumentationRegistry.getContext; +import static org.briarproject.briar.android.attachment.AttachmentItem.State.AVAILABLE; +import static org.briarproject.briar.android.attachment.AttachmentItem.State.ERROR; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @RunWith(AndroidJUnit4.class) @@ -27,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 @@ -35,7 +36,7 @@ public class AttachmentRetrieverIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getAssetInputStream("kitten_small.jpg"); Attachment a = new Attachment(h, is); - AttachmentItem item = retriever.getAttachmentItem(a, true); + AttachmentItem item = retriever.createAttachmentItem(a, true); assertEquals(msgId, item.getMessageId()); assertEquals(160, item.getWidth()); assertEquals(240, item.getHeight()); @@ -43,7 +44,7 @@ public class AttachmentRetrieverIntegrationTest { assertEquals(240, item.getThumbnailHeight()); assertEquals("image/jpeg", item.getMimeType()); assertJpgOrJpeg(item.getExtension()); - assertFalse(item.hasError()); + assertEquals(AVAILABLE, item.getState()); } @Test @@ -51,7 +52,7 @@ public class AttachmentRetrieverIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getAssetInputStream("kitten_original.jpg"); Attachment a = new Attachment(h, is); - AttachmentItem item = retriever.getAttachmentItem(a, true); + AttachmentItem item = retriever.createAttachmentItem(a, true); assertEquals(msgId, item.getMessageId()); assertEquals(1728, item.getWidth()); assertEquals(2592, item.getHeight()); @@ -59,7 +60,7 @@ public class AttachmentRetrieverIntegrationTest { assertEquals(dimensions.maxHeight, item.getThumbnailHeight()); assertEquals("image/jpeg", item.getMimeType()); assertJpgOrJpeg(item.getExtension()); - assertFalse(item.hasError()); + assertEquals(AVAILABLE, item.getState()); } @Test @@ -67,7 +68,7 @@ public class AttachmentRetrieverIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/png"); InputStream is = getAssetInputStream("kitten.png"); Attachment a = new Attachment(h, is); - AttachmentItem item = retriever.getAttachmentItem(a, true); + AttachmentItem item = retriever.createAttachmentItem(a, true); assertEquals(msgId, item.getMessageId()); assertEquals(737, item.getWidth()); assertEquals(510, item.getHeight()); @@ -75,7 +76,7 @@ public class AttachmentRetrieverIntegrationTest { assertEquals(138, item.getThumbnailHeight()); assertEquals("image/png", item.getMimeType()); assertEquals("png", item.getExtension()); - assertFalse(item.hasError()); + assertEquals(AVAILABLE, item.getState()); } @Test @@ -83,14 +84,14 @@ public class AttachmentRetrieverIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/gif"); InputStream is = getAssetInputStream("uber.gif"); Attachment a = new Attachment(h, is); - AttachmentItem item = retriever.getAttachmentItem(a, true); + AttachmentItem item = retriever.createAttachmentItem(a, true); assertEquals(1, item.getWidth()); assertEquals(1, item.getHeight()); assertEquals(dimensions.minHeight, item.getThumbnailWidth()); assertEquals(dimensions.minHeight, item.getThumbnailHeight()); assertEquals("image/gif", item.getMimeType()); assertEquals("gif", item.getExtension()); - assertFalse(item.hasError()); + assertEquals(AVAILABLE, item.getState()); } @Test @@ -98,14 +99,14 @@ public class AttachmentRetrieverIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getAssetInputStream("lottapixel.jpg"); Attachment a = new Attachment(h, is); - AttachmentItem item = retriever.getAttachmentItem(a, true); + AttachmentItem item = retriever.createAttachmentItem(a, true); assertEquals(64250, item.getWidth()); assertEquals(64250, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); assertEquals(dimensions.maxWidth, item.getThumbnailHeight()); assertEquals("image/jpeg", item.getMimeType()); assertJpgOrJpeg(item.getExtension()); - assertFalse(item.hasError()); + assertEquals(AVAILABLE, item.getState()); } @Test @@ -113,14 +114,14 @@ public class AttachmentRetrieverIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/png"); InputStream is = getAssetInputStream("image_io_crash.png"); Attachment a = new Attachment(h, is); - AttachmentItem item = retriever.getAttachmentItem(a, true); + AttachmentItem item = retriever.createAttachmentItem(a, true); assertEquals(1184, item.getWidth()); assertEquals(448, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); assertEquals(dimensions.minHeight, item.getThumbnailHeight()); assertEquals("image/png", item.getMimeType()); assertEquals("png", item.getExtension()); - assertFalse(item.hasError()); + assertEquals(AVAILABLE, item.getState()); } @Test @@ -128,14 +129,14 @@ public class AttachmentRetrieverIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/gif"); InputStream is = getAssetInputStream("gimp_crash.gif"); Attachment a = new Attachment(h, is); - AttachmentItem item = retriever.getAttachmentItem(a, true); + AttachmentItem item = retriever.createAttachmentItem(a, true); assertEquals(1, item.getWidth()); assertEquals(1, item.getHeight()); assertEquals(dimensions.minHeight, item.getThumbnailWidth()); assertEquals(dimensions.minHeight, item.getThumbnailHeight()); assertEquals("image/gif", item.getMimeType()); assertEquals("gif", item.getExtension()); - assertFalse(item.hasError()); + assertEquals(AVAILABLE, item.getState()); } @Test @@ -143,14 +144,14 @@ public class AttachmentRetrieverIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/gif"); InputStream is = getAssetInputStream("opti_png_afl.gif"); Attachment a = new Attachment(h, is); - AttachmentItem item = retriever.getAttachmentItem(a, true); + AttachmentItem item = retriever.createAttachmentItem(a, true); assertEquals(32, item.getWidth()); assertEquals(32, item.getHeight()); assertEquals(dimensions.minHeight, item.getThumbnailWidth()); assertEquals(dimensions.minHeight, item.getThumbnailHeight()); assertEquals("image/gif", item.getMimeType()); assertEquals("gif", item.getExtension()); - assertFalse(item.hasError()); + assertEquals(AVAILABLE, item.getState()); } @Test @@ -158,8 +159,8 @@ public class AttachmentRetrieverIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getAssetInputStream("libraw_error.jpg"); Attachment a = new Attachment(h, is); - AttachmentItem item = retriever.getAttachmentItem(a, true); - assertTrue(item.hasError()); + AttachmentItem item = retriever.createAttachmentItem(a, true); + assertEquals(ERROR, item.getState()); } @Test @@ -167,14 +168,14 @@ public class AttachmentRetrieverIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/gif"); InputStream is = getAssetInputStream("animated.gif"); Attachment a = new Attachment(h, is); - AttachmentItem item = retriever.getAttachmentItem(a, true); + AttachmentItem item = retriever.createAttachmentItem(a, true); assertEquals(65535, item.getWidth()); assertEquals(65535, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); assertEquals(dimensions.maxWidth, item.getThumbnailHeight()); assertEquals("image/gif", item.getMimeType()); assertEquals("gif", item.getExtension()); - assertFalse(item.hasError()); + assertEquals(AVAILABLE, item.getState()); } @Test @@ -182,14 +183,14 @@ public class AttachmentRetrieverIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/gif"); InputStream is = getAssetInputStream("animated2.gif"); Attachment a = new Attachment(h, is); - AttachmentItem item = retriever.getAttachmentItem(a, true); + AttachmentItem item = retriever.createAttachmentItem(a, true); assertEquals(10000, item.getWidth()); assertEquals(10000, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); assertEquals(dimensions.maxWidth, item.getThumbnailHeight()); assertEquals("image/gif", item.getMimeType()); assertEquals("gif", item.getExtension()); - assertFalse(item.hasError()); + assertEquals(AVAILABLE, item.getState()); } @Test @@ -197,14 +198,14 @@ public class AttachmentRetrieverIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/gif"); InputStream is = getAssetInputStream("error_large.gif"); Attachment a = new Attachment(h, is); - AttachmentItem item = retriever.getAttachmentItem(a, true); + AttachmentItem item = retriever.createAttachmentItem(a, true); assertEquals(16384, item.getWidth()); assertEquals(16384, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); assertEquals(dimensions.maxWidth, item.getThumbnailHeight()); assertEquals("image/gif", item.getMimeType()); assertEquals("gif", item.getExtension()); - assertFalse(item.hasError()); + assertEquals(AVAILABLE, item.getState()); } @Test @@ -212,14 +213,14 @@ public class AttachmentRetrieverIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getAssetInputStream("error_high.jpg"); Attachment a = new Attachment(h, is); - AttachmentItem item = retriever.getAttachmentItem(a, true); + AttachmentItem item = retriever.createAttachmentItem(a, true); assertEquals(1, item.getWidth()); assertEquals(10000, item.getHeight()); assertEquals(dimensions.minWidth, item.getThumbnailWidth()); assertEquals(dimensions.maxHeight, item.getThumbnailHeight()); assertEquals("image/jpeg", item.getMimeType()); assertJpgOrJpeg(item.getExtension()); - assertFalse(item.hasError()); + assertEquals(AVAILABLE, item.getState()); } @Test @@ -227,14 +228,14 @@ public class AttachmentRetrieverIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getAssetInputStream("error_wide.jpg"); Attachment a = new Attachment(h, is); - AttachmentItem item = retriever.getAttachmentItem(a, true); + AttachmentItem item = retriever.createAttachmentItem(a, true); assertEquals(1920, item.getWidth()); assertEquals(1, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); assertEquals(dimensions.minHeight, item.getThumbnailHeight()); assertEquals("image/jpeg", item.getMimeType()); assertJpgOrJpeg(item.getExtension()); - assertFalse(item.hasError()); + assertEquals(AVAILABLE, item.getState()); } private InputStream getAssetInputStream(String name) throws Exception { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreatorImpl.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreatorImpl.java index 882ef569f..60f498fdd 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreatorImpl.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreatorImpl.java @@ -34,6 +34,7 @@ import androidx.lifecycle.MutableLiveData; 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.attachment.AttachmentItem.State.ERROR; import static org.briarproject.briar.android.util.UiUtils.observeForeverOnce; import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_IMAGE_SIZE; @@ -109,8 +110,8 @@ class AttachmentCreatorImpl implements AttachmentCreator { // get and cache AttachmentItem for ImagePreview try { Attachment a = retriever.getMessageAttachment(h); - AttachmentItem item = retriever.getAttachmentItem(a, needsSize); - if (item.hasError()) throw new IOException(); + AttachmentItem item = retriever.createAttachmentItem(a, needsSize); + if (item.getState() == ERROR) throw new IOException(); AttachmentItemResult itemResult = new AttachmentItemResult(uri, item); itemResults.add(itemResult); @@ -167,13 +168,6 @@ class AttachmentCreatorImpl implements AttachmentCreator { @Override @UiThread public void onAttachmentsSent(MessageId id) { - List 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(); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItem.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItem.java index a72c31948..9ce231c7d 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItem.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItem.java @@ -7,24 +7,33 @@ import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.briar.api.messaging.AttachmentHeader; -import java.util.concurrent.atomic.AtomicLong; - import javax.annotation.concurrent.Immutable; import androidx.annotation.Nullable; +import static java.lang.System.arraycopy; import static java.util.Objects.requireNonNull; +import static org.briarproject.bramble.util.StringUtils.toHexString; +import static org.briarproject.briar.android.attachment.AttachmentItem.State.LOADING; +import static org.briarproject.briar.android.attachment.AttachmentItem.State.MISSING; @Immutable @NotNullByDefault public class AttachmentItem implements Parcelable { + public enum State { + LOADING, MISSING, AVAILABLE, ERROR; + + public boolean isFinal() { + return this == AVAILABLE || this == ERROR; + } + } + private final AttachmentHeader header; private final int width, height; private final String extension; private final int thumbnailWidth, thumbnailHeight; - private final boolean hasError; - private final long instanceId; + private final State state; public static final Creator CREATOR = new Creator() { @@ -39,19 +48,33 @@ public class AttachmentItem implements Parcelable { } }; - private static final AtomicLong NEXT_INSTANCE_ID = new AtomicLong(0); - AttachmentItem(AttachmentHeader header, int width, int height, String extension, int thumbnailWidth, int thumbnailHeight, - boolean hasError) { + State state) { this.header = header; this.width = width; this.height = height; this.extension = extension; this.thumbnailWidth = thumbnailWidth; this.thumbnailHeight = thumbnailHeight; - this.hasError = hasError; - instanceId = NEXT_INSTANCE_ID.getAndIncrement(); + this.state = state; + } + + /** + * Use only for {@link State MISSING} or {@link State LOADING} items. + */ + AttachmentItem(AttachmentHeader header, int width, int height, + State state) { + this(header, width, height, "", width, height, state); + if (state != MISSING && state != LOADING) + throw new IllegalArgumentException(); + } + + /** + * Use when the item does not need a size. + */ + AttachmentItem(AttachmentHeader header, String extension, State state) { + this(header, 0, 0, extension, 0, 0, state); } protected AttachmentItem(Parcel in) { @@ -64,8 +87,7 @@ public class AttachmentItem implements Parcelable { extension = requireNonNull(in.readString()); thumbnailWidth = in.readInt(); thumbnailHeight = in.readInt(); - hasError = in.readByte() != 0; - instanceId = in.readLong(); + state = State.valueOf(requireNonNull(in.readString())); header = new AttachmentHeader(messageId, mimeType); } @@ -101,12 +123,16 @@ public class AttachmentItem implements Parcelable { return thumbnailHeight; } - public boolean hasError() { - return hasError; + public State getState() { + return state; } - public String getTransitionName() { - return String.valueOf(instanceId); + public String getTransitionName(MessageId conversationItemId) { + int len = MessageId.LENGTH; + byte[] instanceId = new byte[len * 2]; + arraycopy(header.getMessageId().getBytes(), 0, instanceId, 0, len); + arraycopy(conversationItemId.getBytes(), 0, instanceId, len, len); + return toHexString(instanceId); } @Override @@ -123,14 +149,23 @@ public class AttachmentItem implements Parcelable { dest.writeString(extension); dest.writeInt(thumbnailWidth); dest.writeInt(thumbnailHeight); - dest.writeByte((byte) (hasError ? 1 : 0)); - dest.writeLong(instanceId); + dest.writeString(state.name()); } + /** + * This is used to identity if two items are the same, + * irrespective of their state or size. + */ @Override public boolean equals(@Nullable Object o) { return o instanceof AttachmentItem && - instanceId == ((AttachmentItem) o).instanceId; + header.getMessageId().equals( + ((AttachmentItem) o).header.getMessageId() + ); } + @Override + public int hashCode() { + return header.getMessageId().hashCode(); + } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetriever.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetriever.java index 33d709ab7..2bf6b69ea 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetriever.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetriever.java @@ -1,29 +1,63 @@ package org.briarproject.briar.android.attachment; +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.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 public interface AttachmentRetriever { - void cachePut(MessageId messageId, List attachments); - - @Nullable - List cacheGet(MessageId messageId); - + @DatabaseExecutor Attachment getMessageAttachment(AttachmentHeader h) throws DbException; + /** + * Returns a list of observable {@link LiveData} + * that get updated as the state of their {@link AttachmentItem}s changes. + */ + List> getAttachmentItems( + PrivateMessageHeader messageHeader); + + /** + * Retrieves item size and adds the item to the cache, if available. + *

+ * 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, + AttachmentHeader h) throws DbException; + /** * Creates an {@link AttachmentItem} from the {@link Attachment}'s * {@link InputStream} which will be closed when this method returns. */ - AttachmentItem getAttachmentItem(Attachment a, boolean needsSize); + AttachmentItem createAttachmentItem(Attachment a, boolean needsSize); + + /** + * Loads an {@link AttachmentItem} + * that arrived via an {@link AttachmentReceivedEvent} + * and notifies the associated {@link LiveData}. + * + * Note that you need to call {@link #getAttachmentItems(PrivateMessageHeader)} + * first to get the LiveData. + * + * It is possible that no LiveData is available, + * because the message of the AttachmentItem did not arrive, yet. + * In this case, the load wil be deferred until the message arrives. + */ + @DatabaseExecutor + void loadAttachmentItem(MessageId attachmentId); + } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetrieverImpl.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetrieverImpl.java index 53a376aea..1faa7dd1e 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetrieverImpl.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetrieverImpl.java @@ -1,25 +1,39 @@ package org.briarproject.briar.android.attachment; +import org.briarproject.bramble.api.db.DatabaseExecutor; import org.briarproject.bramble.api.db.DbException; +import org.briarproject.bramble.api.db.NoSuchMessageException; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.sync.MessageId; +import org.briarproject.briar.android.attachment.AttachmentItem.State; 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 java.io.BufferedInputStream; import java.io.InputStream; +import java.util.ArrayList; import java.util.List; -import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +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; +import static org.briarproject.briar.android.attachment.AttachmentItem.State.MISSING; @NotNullByDefault class AttachmentRetrieverImpl implements AttachmentRetriever { @@ -27,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; @@ -34,13 +50,17 @@ class AttachmentRetrieverImpl implements AttachmentRetriever { private final int minWidth, maxWidth; private final int minHeight, maxHeight; - private final Map> attachmentCache = - new ConcurrentHashMap<>(); + private final ConcurrentMap> + itemsWithSize = new ConcurrentHashMap<>(); + private final ConcurrentMap> + itemsWithoutSize = 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; @@ -52,40 +72,143 @@ class AttachmentRetrieverImpl implements AttachmentRetriever { } @Override - public void cachePut(MessageId messageId, - List attachments) { - attachmentCache.put(messageId, attachments); - } - - @Override - @Nullable - public List cacheGet(MessageId messageId) { - return attachmentCache.get(messageId); - } - - @Override + @DatabaseExecutor public Attachment getMessageAttachment(AttachmentHeader h) throws DbException { return messagingManager.getAttachment(h); } @Override - public AttachmentItem getAttachmentItem(Attachment a, boolean needsSize) { - AttachmentHeader h = a.getHeader(); - if (!needsSize) { - String extension = - imageHelper.getExtensionFromMimeType(h.getContentType()); - boolean hasError = false; - if (extension == null) { - extension = ""; - hasError = true; + public List> getAttachmentItems( + PrivateMessageHeader messageHeader) { + List headers = messageHeader.getAttachmentHeaders(); + List> items = new ArrayList<>(headers.size()); + boolean needsSize = headers.size() == 1; + for (AttachmentHeader h : headers) { + // try cache for existing item live data + MutableLiveData liveData; + if (needsSize) liveData = itemsWithSize.get(h.getMessageId()); + else { + // try items with size first, as they work as well + liveData = itemsWithSize.get(h.getMessageId()); + if (liveData == null) + liveData = itemsWithoutSize.get(h.getMessageId()); } - return new AttachmentItem(h, 0, 0, extension, 0, 0, hasError); + + // create new live data with LOADING item if cache miss + if (liveData == null) { + AttachmentItem item = new AttachmentItem(h, + defaultSize, defaultSize, LOADING); + liveData = new MutableLiveData<>(item); + // add new LiveData to cache, checking for concurrent updates + MutableLiveData oldLiveData; + if (needsSize) { + oldLiveData = itemsWithSize.putIfAbsent(h.getMessageId(), + liveData); + } else { + oldLiveData = itemsWithoutSize.putIfAbsent(h.getMessageId(), + liveData); + } + if (oldLiveData == null) { + // kick-off loading of attachment, will post to live data + MutableLiveData finalLiveData = liveData; + dbExecutor.execute(() -> + loadAttachmentItem(h, needsSize, finalLiveData)); + } else { + // Concurrent cache update - use the existing live data + liveData = oldLiveData; + } + } + items.add(liveData); + } + return items; + } + + @Override + @DatabaseExecutor + public void cacheAttachmentItemWithSize(MessageId conversationMessageId, + AttachmentHeader h) throws DbException { + // If a live data is already cached we don't need to do anything + if (itemsWithSize.containsKey(h.getMessageId())) return; + try { + Attachment a = messagingManager.getAttachment(h); + AttachmentItem item = createAttachmentItem(a, true); + MutableLiveData liveData = + new MutableLiveData<>(item); + // If a live data was concurrently cached, don't replace it + itemsWithSize.putIfAbsent(h.getMessageId(), liveData); + } catch (NoSuchMessageException e) { + LOG.info("Attachment not received yet"); + } + } + + @Override + @DatabaseExecutor + public void loadAttachmentItem(MessageId attachmentId) { + // try to find LiveData for attachment in both caches + MutableLiveData liveData; + boolean needsSize = true; + liveData = itemsWithSize.get(attachmentId); + if (liveData == null) { + needsSize = false; + liveData = itemsWithoutSize.get(attachmentId); } - InputStream is = new BufferedInputStream(a.getStream()); - Size size = imageSizeCalculator.getSize(is, h.getContentType()); + // If no LiveData for the attachment exists, + // its message did not yet arrive and we can ignore it for now. + if (liveData == null) return; + // 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 liveData) { + Attachment a; + AttachmentItem item; + try { + a = messagingManager.getAttachment(h); + item = createAttachmentItem(a, needsSize); + } catch (NoSuchMessageException e) { + LOG.info("Attachment not received yet"); + item = new AttachmentItem(h, defaultSize, defaultSize, MISSING); + } catch (DbException e) { + logException(LOG, WARNING, e); + item = new AttachmentItem(h, "", ERROR); + } + liveData.postValue(item); + } + + @Override + public AttachmentItem createAttachmentItem(Attachment a, + boolean needsSize) { + AttachmentItem item; + AttachmentHeader h = a.getHeader(); + if (needsSize) { + InputStream is = new BufferedInputStream(a.getStream()); + Size size = imageSizeCalculator.getSize(is, h.getContentType()); + tryToClose(is, LOG, WARNING); + item = createAttachmentItem(h, size); + } else { + String extension = + imageHelper.getExtensionFromMimeType(h.getContentType()); + State state = AVAILABLE; + if (extension == null) { + extension = ""; + state = ERROR; + } + item = new AttachmentItem(h, extension, state); + } + return item; + } + + private AttachmentItem createAttachmentItem(AttachmentHeader h, Size size) { // calculate thumbnail size Size thumbnailSize = new Size(defaultSize, defaultSize, size.mimeType); if (!size.error) { @@ -104,8 +227,9 @@ class AttachmentRetrieverImpl implements AttachmentRetriever { hasError = true; } if (extension == null) extension = ""; - return new AttachmentItem(h, size.width, size.height, extension, - thumbnailSize.width, thumbnailSize.height, hasError); + State state = hasError ? ERROR : AVAILABLE; + return new AttachmentItem(h, size.width, size.height, + extension, thumbnailSize.width, thumbnailSize.height, state); } private Size getThumbnailSize(int width, int height, String mimeType) { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java index 1e4a8c416..db32a1914 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java @@ -28,7 +28,6 @@ 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; @@ -65,17 +64,16 @@ import org.briarproject.briar.api.client.ProtocolStateException; import org.briarproject.briar.api.client.SessionId; import org.briarproject.briar.api.conversation.ConversationManager; import org.briarproject.briar.api.conversation.ConversationMessageHeader; +import org.briarproject.briar.api.conversation.ConversationMessageVisitor; import org.briarproject.briar.api.conversation.ConversationRequest; import org.briarproject.briar.api.conversation.ConversationResponse; import org.briarproject.briar.api.conversation.DeletionResult; import org.briarproject.briar.api.conversation.event.ConversationMessageReceivedEvent; import org.briarproject.briar.api.forum.ForumSharingManager; import org.briarproject.briar.api.introduction.IntroductionManager; -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; @@ -97,6 +95,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; @@ -118,8 +117,6 @@ import static androidx.core.app.ActivityOptionsCompat.makeSceneTransitionAnimati import static androidx.core.view.ViewCompat.setTransitionName; import static androidx.lifecycle.Lifecycle.State.STARTED; import static androidx.recyclerview.widget.SortedList.INVALID_POSITION; -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; @@ -136,6 +133,7 @@ import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_INTRO 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; +import static org.briarproject.briar.android.conversation.ImageActivity.ITEM_ID; import static org.briarproject.briar.android.conversation.ImageActivity.NAME; import static org.briarproject.briar.android.util.UiUtils.getAvatarTransitionName; import static org.briarproject.briar.android.util.UiUtils.getBulbTransitionName; @@ -185,8 +183,6 @@ public class ConversationActivity extends BriarActivity volatile GroupInvitationManager groupInvitationManager; private final Map textCache = new ConcurrentHashMap<>(); - private final Map missingAttachments = - new ConcurrentHashMap<>(); private final Observer contactNameObserver = name -> { requireNonNull(name); loadMessages(); @@ -540,6 +536,7 @@ public class ConversationActivity extends BriarActivity }); } + @DatabaseExecutor private void eagerlyLoadMessageSize(PrivateMessageHeader h) { try { MessageId id = h.getId(); @@ -556,21 +553,11 @@ public class ConversationActivity extends BriarActivity // images we use a grid so the size is fixed List headers = h.getAttachmentHeaders(); if (headers.size() == 1) { - List 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); - } - } + 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); } } catch (DbException e) { logException(LOG, WARNING, e); @@ -651,59 +638,18 @@ public class ConversationActivity extends BriarActivity && adapter.isScrolledToBottom(layoutManager); } - private void loadMessageAttachments(PrivateMessageHeader h) { - // TODO: Use placeholders for missing/invalid attachments - runOnDbThread(() -> { - try { - // TODO move getting the items off to IoExecutor, if size == 1 - List headers = h.getAttachmentHeaders(); - boolean needsSize = headers.size() == 1; - List 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); - } - }); - } - - private void displayMessageAttachments(MessageId m, - List items) { - runOnUiThreadUnlessDestroyed(() -> { - Pair pair = - adapter.getMessageItem(m); - if (pair != null) { - boolean scroll = shouldScrollWhenUpdatingMessage(); - pair.getSecond().setAttachments(items); - adapter.notifyItemChanged(pair.getFirst()); - if (scroll) scrollToBottom(); - } - }); + @UiThread + private void updateMessageAttachment(MessageId m, AttachmentItem item) { + Pair pair = adapter.getMessageItem(m); + if (pair != null && pair.getSecond().updateAttachments(item)) { + boolean scroll = shouldScrollWhenUpdatingMessage(); + adapter.notifyItemChanged(pair.getFirst()); + if (scroll) scrollToBottom(); + } } @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)) { @@ -763,15 +709,6 @@ 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 || @@ -780,7 +717,7 @@ public class ConversationActivity extends BriarActivity observeOnce(viewModel.getContactDisplayName(), this, name -> addConversationItem(h.accept(visitor))); } else { - // visitor also loads message text (if existing) + // visitor also loads message text and attachments (if existing) addConversationItem(h.accept(visitor)); } } @@ -1107,8 +1044,9 @@ public class ConversationActivity extends BriarActivity i.putExtra(ATTACHMENT_POSITION, attachments.indexOf(item)); i.putExtra(NAME, name); i.putExtra(DATE, messageItem.getTime()); + i.putExtra(ITEM_ID, messageItem.getId().getBytes()); // restoring list position should not trigger android bug #224270 - String transitionName = item.getTransitionName(); + String transitionName = item.getTransitionName(messageItem.getId()); ActivityOptionsCompat options = makeSceneTransitionAnimation(this, view, transitionName); ActivityCompat.startActivity(this, i, options.toBundle()); @@ -1147,15 +1085,41 @@ public class ConversationActivity extends BriarActivity return text; } + /** + * Called by {@link PrivateMessageHeader#accept(ConversationMessageVisitor)} + */ @Override public List getAttachmentItems(PrivateMessageHeader h) { - List attachments = - attachmentRetriever.cacheGet(h.getId()); - if (attachments == null) { - loadMessageAttachments(h); - return emptyList(); + List> liveDataList = + attachmentRetriever.getAttachmentItems(h); + List items = new ArrayList<>(liveDataList.size()); + for (LiveData liveData : liveDataList) { + // first remove all our observers to avoid having more than one + // in case we reload the conversation, e.g. after deleting messages + liveData.removeObservers(this); + // add a new observer + liveData.observe(this, new AttachmentObserver(h.getId(), liveData)); + items.add(requireNonNull(liveData.getValue())); + } + return items; + } + + private class AttachmentObserver implements Observer { + private final MessageId conversationMessageId; + private final LiveData liveData; + + private AttachmentObserver(MessageId conversationMessageId, + LiveData liveData) { + this.conversationMessageId = conversationMessageId; + this.liveData = liveData; + } + + @Override + public void onChanged(AttachmentItem attachmentItem) { + updateMessageAttachment(conversationMessageId, attachmentItem); + if (attachmentItem.getState().isFinal()) + liveData.removeObserver(this); } - return attachments; } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationMessageItem.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationMessageItem.java index b3e20b3f7..8fc50d299 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationMessageItem.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationMessageItem.java @@ -9,12 +9,13 @@ import java.util.List; import javax.annotation.concurrent.NotThreadSafe; import androidx.annotation.LayoutRes; +import androidx.annotation.UiThread; @NotThreadSafe @NotNullByDefault class ConversationMessageItem extends ConversationItem { - private List attachments; + private final List attachments; ConversationMessageItem(@LayoutRes int layoutRes, PrivateMessageHeader h, List attachments) { @@ -26,8 +27,14 @@ class ConversationMessageItem extends ConversationItem { return attachments; } - void setAttachments(List attachments) { - this.attachments = attachments; + @UiThread + boolean updateAttachments(AttachmentItem item) { + int pos = attachments.indexOf(item); + if (pos != -1 && attachments.get(pos).getState() != item.getState()) { + attachments.set(pos, item); + return true; + } + return false; } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java index 6670bf5ea..5becffce2 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java @@ -11,6 +11,9 @@ 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.TransactionManager; +import org.briarproject.bramble.api.event.Event; +import org.briarproject.bramble.api.event.EventBus; +import org.briarproject.bramble.api.event.EventListener; import org.briarproject.bramble.api.identity.AuthorId; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.settings.Settings; @@ -30,6 +33,7 @@ import org.briarproject.briar.api.messaging.MessagingManager; import org.briarproject.briar.api.messaging.PrivateMessage; import org.briarproject.briar.api.messaging.PrivateMessageFactory; import org.briarproject.briar.api.messaging.PrivateMessageHeader; +import org.briarproject.briar.api.messaging.event.AttachmentReceivedEvent; import java.util.Collection; import java.util.List; @@ -56,7 +60,7 @@ import static org.briarproject.briar.android.util.UiUtils.observeForeverOnce; @NotNullByDefault public class ConversationViewModel extends AndroidViewModel - implements AttachmentManager { + implements EventListener, AttachmentManager { private static Logger LOG = getLogger(ConversationViewModel.class.getName()); @@ -69,6 +73,7 @@ public class ConversationViewModel extends AndroidViewModel @DatabaseExecutor private final Executor dbExecutor; private final TransactionManager db; + private final EventBus eventBus; private final MessagingManager messagingManager; private final ContactManager contactManager; private final SettingsManager settingsManager; @@ -101,6 +106,7 @@ public class ConversationViewModel extends AndroidViewModel ConversationViewModel(Application application, @DatabaseExecutor Executor dbExecutor, TransactionManager db, + EventBus eventBus, MessagingManager messagingManager, ContactManager contactManager, SettingsManager settingsManager, @@ -110,6 +116,7 @@ public class ConversationViewModel extends AndroidViewModel super(application); this.dbExecutor = dbExecutor; this.db = db; + this.eventBus = eventBus; this.messagingManager = messagingManager; this.contactManager = contactManager; this.settingsManager = settingsManager; @@ -119,12 +126,27 @@ public class ConversationViewModel extends AndroidViewModel messagingGroupId = Transformations .map(contact, c -> messagingManager.getContactGroup(c).getId()); contactDeleted.setValue(false); + + eventBus.addListener(this); } @Override protected void onCleared() { super.onCleared(); attachmentCreator.deleteUnsentAttachments(); + eventBus.removeListener(this); + } + + @Override + public void eventOccurred(Event e) { + if (e instanceof AttachmentReceivedEvent) { + AttachmentReceivedEvent a = (AttachmentReceivedEvent) e; + if (a.getContactId().equals(contactId)) { + LOG.info("Attachment received"); + dbExecutor.execute(() -> attachmentRetriever + .loadAttachmentItem(a.getMessageId())); + } + } } /** diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageActivity.java index afac73601..5b816f26c 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageActivity.java @@ -16,6 +16,7 @@ import com.google.android.material.appbar.AppBarLayout; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; +import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.briar.R; import org.briarproject.briar.android.activity.ActivityComponent; import org.briarproject.briar.android.activity.BriarActivity; @@ -67,6 +68,7 @@ public class ImageActivity extends BriarActivity final static String ATTACHMENT_POSITION = "position"; final static String NAME = "name"; final static String DATE = "date"; + final static String ITEM_ID = "itemId"; @RequiresApi(api = 16) private final static int UI_FLAGS_DEFAULT = @@ -80,6 +82,7 @@ public class ImageActivity extends BriarActivity private AppBarLayout appBarLayout; private ViewPager viewPager; private List attachments; + private MessageId conversationMessageId; @Override public void injectActivity(ActivityComponent component) { @@ -98,9 +101,20 @@ public class ImageActivity extends BriarActivity setSceneTransitionAnimation(transition, null, transition); } + // Intent Extras + Intent i = getIntent(); + attachments = + requireNonNull(i.getParcelableArrayListExtra(ATTACHMENTS)); + int position = i.getIntExtra(ATTACHMENT_POSITION, -1); + if (position == -1) throw new IllegalStateException(); + String name = i.getStringExtra(NAME); + long time = i.getLongExtra(DATE, 0); + byte[] messageIdBytes = requireNonNull(i.getByteArrayExtra(ITEM_ID)); + // get View Model viewModel = ViewModelProviders.of(this, viewModelFactory) .get(ImageViewModel.class); + viewModel.expectAttachments(attachments); viewModel.getSaveState().observeEvent(this, this::onImageSaveStateChanged); @@ -124,16 +138,11 @@ public class ImageActivity extends BriarActivity TextView contactName = toolbar.findViewById(R.id.contactName); TextView dateView = toolbar.findViewById(R.id.dateView); - // Intent Extras - Intent i = getIntent(); - attachments = i.getParcelableArrayListExtra(ATTACHMENTS); - int position = i.getIntExtra(ATTACHMENT_POSITION, -1); - if (position == -1) throw new IllegalStateException(); - String name = i.getStringExtra(NAME); - long time = i.getLongExtra(DATE, 0); + // Set contact name and message time String date = formatDateAbsolute(this, time); contactName.setText(name); dateView.setText(date); + conversationMessageId = new MessageId(messageIdBytes); // Set up image ViewPager viewPager = findViewById(R.id.viewPager); @@ -320,8 +329,8 @@ public class ImageActivity extends BriarActivity @Override public Fragment getItem(int position) { - Fragment f = ImageFragment - .newInstance(attachments.get(position), isFirst); + Fragment f = ImageFragment.newInstance( + attachments.get(position), conversationMessageId, isFirst); isFirst = false; return f; } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageAdapter.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageAdapter.java index 9f456f71b..02f97730c 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageAdapter.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageAdapter.java @@ -49,7 +49,8 @@ class ImageAdapter extends Adapter { public ImageViewHolder onCreateViewHolder(ViewGroup viewGroup, int type) { View v = LayoutInflater.from(viewGroup.getContext()).inflate( R.layout.list_item_image, viewGroup, false); - return new ImageViewHolder(v, imageSize); + requireNonNull(conversationItem); + return new ImageViewHolder(v, imageSize, conversationItem.getId()); } @Override diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageFragment.java index 9f8ee39d4..09ec190eb 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageFragment.java @@ -15,6 +15,7 @@ import com.bumptech.glide.request.target.Target; import com.github.chrisbanes.photoview.PhotoView; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; +import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.briar.R; import org.briarproject.briar.android.activity.BaseActivity; import org.briarproject.briar.android.attachment.AttachmentItem; @@ -23,6 +24,7 @@ import org.briarproject.briar.android.conversation.glide.GlideApp; import javax.annotation.ParametersAreNonnullByDefault; import javax.inject.Inject; +import androidx.annotation.DrawableRes; import androidx.annotation.Nullable; import androidx.fragment.app.Fragment; import androidx.lifecycle.ViewModelProvider; @@ -32,27 +34,36 @@ import static android.os.Build.VERSION.SDK_INT; import static android.widget.ImageView.ScaleType.FIT_START; import static com.bumptech.glide.load.engine.DiskCacheStrategy.NONE; import static org.briarproject.bramble.api.nullsafety.NullSafety.requireNonNull; +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.conversation.ImageActivity.ATTACHMENT_POSITION; +import static org.briarproject.briar.android.conversation.ImageActivity.ITEM_ID; @MethodsNotNullByDefault @ParametersAreNonnullByDefault -public class ImageFragment extends Fragment { +public class ImageFragment extends Fragment + implements RequestListener { private final static String IS_FIRST = "isFirst"; + @DrawableRes + private static final int ERROR_RES = R.drawable.ic_image_broken; @Inject ViewModelProvider.Factory viewModelFactory; private AttachmentItem attachment; private boolean isFirst; + private MessageId conversationItemId; private ImageViewModel viewModel; private PhotoView photoView; - static ImageFragment newInstance(AttachmentItem a, boolean isFirst) { + static ImageFragment newInstance(AttachmentItem a, + MessageId conversationMessageId, boolean isFirst) { ImageFragment f = new ImageFragment(); Bundle args = new Bundle(); args.putParcelable(ATTACHMENT_POSITION, a); args.putBoolean(IS_FIRST, isFirst); + args.putByteArray(ITEM_ID, conversationMessageId.getBytes()); f.setArguments(args); return f; } @@ -70,6 +81,8 @@ public class ImageFragment extends Fragment { Bundle args = requireNonNull(getArguments()); attachment = requireNonNull(args.getParcelable(ATTACHMENT_POSITION)); isFirst = args.getBoolean(IS_FIRST); + conversationItemId = + new MessageId(requireNonNull(args.getByteArray(ITEM_ID))); } @Nullable @@ -87,50 +100,68 @@ public class ImageFragment extends Fragment { photoView.setScaleLevels(1, 2, 4); photoView.setOnClickListener(view -> viewModel.clickImage()); - // Request Listener - RequestListener listener = new RequestListener() { - - @Override - public boolean onLoadFailed(@Nullable GlideException e, - Object model, Target target, - boolean isFirstResource) { - if (getActivity() != null && isFirst) - getActivity().supportStartPostponedEnterTransition(); - return false; - } - - @Override - public boolean onResourceReady(Drawable resource, Object model, - Target target, DataSource dataSource, - boolean isFirstResource) { - if (SDK_INT >= 21 && !(resource instanceof Animatable)) { - // set transition name only when not animatable, - // because the animation won't start otherwise - photoView.setTransitionName( - attachment.getTransitionName()); - } - // Move image to the top if overlapping toolbar - if (viewModel.isOverlappingToolbar(photoView, resource)) { - photoView.setScaleType(FIT_START); - } - if (getActivity() != null && isFirst) { - getActivity().supportStartPostponedEnterTransition(); - } - return false; - } - }; - - // Load Image - GlideApp.with(this) - .load(attachment) - // TODO allow if size < maxTextureSize ? -// .override(SIZE_ORIGINAL) - .diskCacheStrategy(NONE) - .error(R.drawable.ic_image_broken) - .addListener(listener) - .into(photoView); + if (attachment.getState() == AVAILABLE) { + loadImage(); + // postponed transition will be started when Image was loaded + } else if (attachment.getState() == ERROR) { + photoView.setImageResource(ERROR_RES); + startPostponedTransition(); + } else { + photoView.setImageResource(R.drawable.ic_image_missing); + startPostponedTransition(); + // state is not final, so observe state changes + viewModel.getOnAttachmentReceived(attachment.getMessageId()) + .observeEvent(this, this::onAttachmentReceived); + } return v; } + private void loadImage() { + GlideApp.with(this) + .load(attachment) + // TODO allow if size < maxTextureSize ? +// .override(SIZE_ORIGINAL) + .diskCacheStrategy(NONE) + .error(ERROR_RES) + .addListener(this) + .into(photoView); + } + + private void onAttachmentReceived(Boolean received) { + if (received) loadImage(); + } + + @Override + public boolean onLoadFailed(@Nullable GlideException e, + Object model, Target target, + boolean isFirstResource) { + startPostponedTransition(); + return false; + } + + @Override + public boolean onResourceReady(Drawable resource, Object model, + Target target, DataSource dataSource, + boolean isFirstResource) { + if (SDK_INT >= 21 && !(resource instanceof Animatable)) { + // set transition name only when not animatable, + // because the animation won't start otherwise + photoView.setTransitionName( + attachment.getTransitionName(conversationItemId)); + } + // Move image to the top if overlapping toolbar + if (viewModel.isOverlappingToolbar(photoView, resource)) { + photoView.setScaleType(FIT_START); + } + startPostponedTransition(); + return false; + } + + private void startPostponedTransition() { + if (getActivity() != null && isFirst) { + getActivity().supportStartPostponedEnterTransition(); + } + } + } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewHolder.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewHolder.java index 76eaab5e3..659bcf29e 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewHolder.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewHolder.java @@ -7,6 +7,7 @@ import android.widget.ImageView; import com.bumptech.glide.load.Transformation; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.briar.R; import org.briarproject.briar.android.attachment.AttachmentItem; import org.briarproject.briar.android.conversation.glide.BriarImageTransformation; @@ -18,8 +19,12 @@ import androidx.recyclerview.widget.RecyclerView.ViewHolder; import androidx.recyclerview.widget.StaggeredGridLayoutManager.LayoutParams; import static android.os.Build.VERSION.SDK_INT; +import static android.widget.ImageView.ScaleType.CENTER_CROP; +import static android.widget.ImageView.ScaleType.FIT_CENTER; import static com.bumptech.glide.load.engine.DiskCacheStrategy.NONE; import static com.bumptech.glide.load.resource.drawable.DrawableTransitionOptions.withCrossFade; +import static org.briarproject.briar.android.attachment.AttachmentItem.State.AVAILABLE; +import static org.briarproject.briar.android.attachment.AttachmentItem.State.ERROR; @NotNullByDefault class ImageViewHolder extends ViewHolder { @@ -29,25 +34,33 @@ class ImageViewHolder extends ViewHolder { protected final ImageView imageView; private final int imageSize; + private final MessageId conversationItemId; - ImageViewHolder(View v, int imageSize) { + ImageViewHolder(View v, int imageSize, MessageId conversationItemId) { super(v); imageView = v.findViewById(R.id.imageView); this.imageSize = imageSize; + this.conversationItemId = conversationItemId; } void bind(AttachmentItem attachment, Radii r, boolean single, boolean needsStretch) { - if (attachment.hasError()) { - GlideApp.with(imageView) - .clear(imageView); - imageView.setImageResource(ERROR_RES); - } else { - setImageViewDimensions(attachment, single, needsStretch); - loadImage(attachment, r); - if (SDK_INT >= 21) { - imageView.setTransitionName(attachment.getTransitionName()); + setImageViewDimensions(attachment, single, needsStretch); + if (attachment.getState() != AVAILABLE) { + GlideApp.with(imageView).clear(imageView); + if (attachment.getState() == ERROR) { + imageView.setImageResource(ERROR_RES); + } else { + imageView.setImageResource(R.drawable.ic_image_missing); } + imageView.setScaleType(FIT_CENTER); + } else { + loadImage(attachment, r); + imageView.setScaleType(CENTER_CROP); + } + if (SDK_INT >= 21) { + imageView.setTransitionName( + attachment.getTransitionName(conversationItemId)); } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java index 0c251cf60..1a59b02bf 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java @@ -7,13 +7,18 @@ import android.view.View; import org.briarproject.bramble.api.db.DatabaseExecutor; import org.briarproject.bramble.api.db.DbException; +import org.briarproject.bramble.api.event.Event; +import org.briarproject.bramble.api.event.EventBus; +import org.briarproject.bramble.api.event.EventListener; 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; import org.briarproject.briar.api.messaging.Attachment; import org.briarproject.briar.api.messaging.MessagingManager; +import org.briarproject.briar.api.messaging.event.AttachmentReceivedEvent; import java.io.File; import java.io.FileOutputStream; @@ -22,6 +27,8 @@ import java.io.InputStream; import java.io.OutputStream; import java.text.SimpleDateFormat; import java.util.Date; +import java.util.HashMap; +import java.util.List; import java.util.Locale; import java.util.concurrent.Executor; import java.util.logging.Logger; @@ -35,22 +42,28 @@ import androidx.lifecycle.AndroidViewModel; import static android.media.MediaScannerConnection.scanFile; import static android.os.Environment.DIRECTORY_PICTURES; import static android.os.Environment.getExternalStoragePublicDirectory; +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.copyAndClose; import static org.briarproject.bramble.util.LogUtils.logException; @NotNullByDefault -public class ImageViewModel extends AndroidViewModel { +public class ImageViewModel extends AndroidViewModel implements EventListener { private static Logger LOG = getLogger(ImageViewModel.class.getName()); private final MessagingManager messagingManager; + private final EventBus eventBus; @DatabaseExecutor private final Executor dbExecutor; @IoExecutor private final Executor ioExecutor; + private boolean receivedAttachmentsInitialized = false; + private HashMap> receivedAttachments = + new HashMap<>(); + /** * true means there was an error saving the image, false if image was saved. */ @@ -62,13 +75,60 @@ public class ImageViewModel extends AndroidViewModel { @Inject ImageViewModel(Application application, - MessagingManager messagingManager, + MessagingManager messagingManager, EventBus eventBus, @DatabaseExecutor Executor dbExecutor, @IoExecutor Executor ioExecutor) { super(application); this.messagingManager = messagingManager; + this.eventBus = eventBus; this.dbExecutor = dbExecutor; this.ioExecutor = ioExecutor; + + eventBus.addListener(this); + } + + @Override + protected void onCleared() { + super.onCleared(); + eventBus.removeListener(this); + } + + @UiThread + @Override + public void eventOccurred(Event e) { + if (e instanceof AttachmentReceivedEvent) { + MessageId id = ((AttachmentReceivedEvent) e).getMessageId(); + MutableLiveEvent oldEvent; + if (receivedAttachmentsInitialized) { + oldEvent = receivedAttachments.get(id); + if (oldEvent != null) oldEvent.postEvent(true); + } else { + receivedAttachments.put(id, new MutableLiveEvent<>(true)); + } + } + } + + @UiThread + public void expectAttachments(List attachments) { + for (AttachmentItem item : attachments) { + // no need to track items that are in a final state already + if (item.getState().isFinal()) continue; + // add new live events, if not already added by eventOccurred() + MessageId id = item.getMessageId(); + if (!receivedAttachments.containsKey(id)) { + receivedAttachments.put(id, new MutableLiveEvent<>()); + } + } + receivedAttachmentsInitialized = true; + } + + /** + * Returns a LiveData for attachments in a non-final state. + * Note that you need to call {@link #expectAttachments(List)} first. + */ + @UiThread + LiveEvent getOnAttachmentReceived(MessageId messageId) { + return requireNonNull(receivedAttachments.get(messageId)); } void clickImage() { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/viewmodel/LiveEvent.java b/briar-android/src/main/java/org/briarproject/briar/android/viewmodel/LiveEvent.java index d7ff0cef5..d4e1b9cff 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/viewmodel/LiveEvent.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/viewmodel/LiveEvent.java @@ -12,6 +12,22 @@ import androidx.lifecycle.Observer; @NotNullByDefault public class LiveEvent extends LiveData> { + /** + * Creates a LiveEvent initialized with the given {@code value}. + * + * @param value initial value + */ + public LiveEvent(T value) { + super(new ConsumableEvent<>(value)); + } + + /** + * Creates a LiveEvent with no value assigned to it. + */ + public LiveEvent() { + super(); + } + public void observeEvent(LifecycleOwner owner, LiveEventHandler handler) { LiveEventObserver observer = new LiveEventObserver<>(handler); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/viewmodel/MutableLiveEvent.java b/briar-android/src/main/java/org/briarproject/briar/android/viewmodel/MutableLiveEvent.java index dac33ecfc..fda307d5d 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/viewmodel/MutableLiveEvent.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/viewmodel/MutableLiveEvent.java @@ -5,6 +5,22 @@ import org.briarproject.bramble.api.nullsafety.NotNullByDefault; @NotNullByDefault public class MutableLiveEvent extends LiveEvent { + /** + * Creates a MutableLiveEvent initialized with the given {@code value}. + * + * @param value initial value + */ + public MutableLiveEvent(T value) { + super(value); + } + + /** + * Creates a MutableLiveEvent with no value assigned to it. + */ + public MutableLiveEvent() { + super(); + } + public void postEvent(T value) { super.postValue(new ConsumableEvent<>(value)); } diff --git a/briar-android/src/main/res/drawable/ic_image_broken.xml b/briar-android/src/main/res/drawable/ic_image_broken.xml index 0baff6620..4fb057b05 100644 --- a/briar-android/src/main/res/drawable/ic_image_broken.xml +++ b/briar-android/src/main/res/drawable/ic_image_broken.xml @@ -1,6 +1,6 @@ + + diff --git a/briar-android/src/test/java/org/briarproject/briar/android/attachment/AttachmentRetrieverTest.java b/briar-android/src/test/java/org/briarproject/briar/android/attachment/AttachmentRetrieverTest.java index 4558c2ff8..5504da00b 100644 --- a/briar-android/src/test/java/org/briarproject/briar/android/attachment/AttachmentRetrieverTest.java +++ b/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,12 +12,13 @@ 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; +import static org.briarproject.briar.android.attachment.AttachmentItem.State.AVAILABLE; +import static org.briarproject.briar.android.attachment.AttachmentItem.State.ERROR; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; public class AttachmentRetrieverTest extends BrambleMockTestCase { @@ -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, - imageHelper, imageSizeCalculator); + Executor dbExecutor = new ImmediateExecutor(); + retriever = new AttachmentRetrieverImpl(dbExecutor, messagingManager, + dimensions, imageHelper, imageSizeCalculator); } @Test @@ -47,10 +50,10 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase { will(returnValue("jpg")); }}); - AttachmentItem item = retriever.getAttachmentItem(attachment, false); + AttachmentItem item = retriever.createAttachmentItem(attachment, false); assertEquals(mimeType, item.getMimeType()); assertEquals("jpg", item.getExtension()); - assertFalse(item.hasError()); + assertEquals(AVAILABLE, item.getState()); } @Test @@ -63,8 +66,8 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase { will(returnValue(null)); }}); - AttachmentItem item = retriever.getAttachmentItem(attachment, false); - assertTrue(item.hasError()); + AttachmentItem item = retriever.createAttachmentItem(attachment, false); + assertEquals(ERROR, item.getState()); } @Test @@ -80,7 +83,7 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase { will(returnValue("jpg")); }}); - AttachmentItem item = retriever.getAttachmentItem(attachment, true); + AttachmentItem item = retriever.createAttachmentItem(attachment, true); assertEquals(msgId, item.getMessageId()); assertEquals(160, item.getWidth()); assertEquals(240, item.getHeight()); @@ -88,7 +91,7 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase { assertEquals(240, item.getThumbnailHeight()); assertEquals(mimeType, item.getMimeType()); assertEquals("jpg", item.getExtension()); - assertFalse(item.hasError()); + assertEquals(AVAILABLE, item.getState()); } @Test @@ -104,12 +107,12 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase { will(returnValue("jpg")); }}); - AttachmentItem item = retriever.getAttachmentItem(attachment, true); + AttachmentItem item = retriever.createAttachmentItem(attachment, true); assertEquals(1728, item.getWidth()); assertEquals(2592, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); assertEquals(dimensions.maxHeight, item.getThumbnailHeight()); - assertFalse(item.hasError()); + assertEquals(AVAILABLE, item.getState()); } @Test @@ -125,8 +128,8 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase { will(returnValue(null)); }}); - AttachmentItem item = retriever.getAttachmentItem(attachment, true); - assertTrue(item.hasError()); + AttachmentItem item = retriever.createAttachmentItem(attachment, true); + assertEquals(ERROR, item.getState()); } private Attachment getAttachment(String contentType) { diff --git a/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingManager.java b/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingManager.java index 0dfecb8ab..ccb8f776f 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingManager.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingManager.java @@ -68,7 +68,7 @@ public interface MessagingManager extends ConversationClient { String getMessageText(MessageId m) throws DbException; /** - * Returns the attachment with the given message ID and content type. + * Returns the attachment with the given attachment header. * * @throws InvalidAttachmentException If the header refers to a message * that is not an attachment, or to an attachment that does not have the