From 41411b0e2ee55ac68738e817a22a2a03e591479c Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Thu, 20 Jun 2019 18:24:23 -0300 Subject: [PATCH] Refactor attachment loading to support incremental display once loaded --- .../AttachmentRetrieverIntegrationTest.java | 59 ++++----- .../attachment/AttachmentCreatorImpl.java | 12 +- .../android/attachment/AttachmentItem.java | 61 ++++++--- .../attachment/AttachmentRetriever.java | 27 ++-- .../attachment/AttachmentRetrieverImpl.java | 118 ++++++++++++++---- .../android/attachment/UnavailableItem.java | 34 +++++ .../conversation/ConversationActivity.java | 102 ++++++--------- .../conversation/ConversationMessageItem.java | 13 +- .../android/conversation/ImageActivity.java | 8 +- .../android/conversation/ImageAdapter.java | 5 +- .../android/conversation/ImageFragment.java | 11 +- .../android/conversation/ImageViewHolder.java | 27 ++-- .../src/main/res/drawable/ic_image_broken.xml | 4 +- .../main/res/drawable/ic_image_missing.xml | 10 ++ .../attachment/AttachmentRetrieverTest.java | 24 ++-- .../briar/api/messaging/MessagingManager.java | 2 +- 16 files changed, 333 insertions(+), 184 deletions(-) create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/attachment/UnavailableItem.java create mode 100644 briar-android/src/main/res/drawable/ic_image_missing.xml 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..9c4ffc8d5 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) @@ -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..a19827eab 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,27 @@ 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} + 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 +42,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 MISSING} or {@link 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 +81,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 +117,20 @@ 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); + } + + boolean hasSize() { + return width != 0 && height != 0; } @Override @@ -123,14 +147,15 @@ 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()); } @Override public boolean equals(@Nullable Object o) { return o instanceof AttachmentItem && - instanceId == ((AttachmentItem) o).instanceId; + header.getMessageId().equals( + ((AttachmentItem) o).header.getMessageId() + ); } } 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..f12b01ecb 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,40 @@ 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; 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 java.io.InputStream; import java.util.List; -import androidx.annotation.Nullable; - @NotNullByDefault public interface AttachmentRetriever { - void cachePut(MessageId messageId, List attachments); - - @Nullable - List cacheGet(MessageId messageId); - + @DatabaseExecutor Attachment getMessageAttachment(AttachmentHeader h) throws DbException; + List getAttachmentItems(PrivateMessageHeader messageHeader); + + /** + * Retrieves item size and adds the item to the cache, if available. + */ + @DatabaseExecutor + void cacheAttachmentItem(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); + + @DatabaseExecutor + Pair loadAttachmentItem(MessageId attachmentId) + throws DbException; + } 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..30ee170bb 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,14 +1,20 @@ 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; 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; @@ -16,10 +22,12 @@ import java.util.logging.Logger; import javax.inject.Inject; -import androidx.annotation.Nullable; - import static java.util.logging.Level.WARNING; import static java.util.logging.Logger.getLogger; +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 { @@ -34,7 +42,11 @@ class AttachmentRetrieverImpl implements AttachmentRetriever { private final int minWidth, maxWidth; private final int minHeight, maxHeight; - private final Map> attachmentCache = + // Info for AttachmentItems that are either still LOADING or MISSING + private final Map unavailableItems = + new ConcurrentHashMap<>(); + // We cache only items in their final state: AVAILABLE or ERROR + private final Map itemCache = new ConcurrentHashMap<>(); @Inject @@ -52,40 +64,95 @@ 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) { + public List getAttachmentItems( + PrivateMessageHeader messageHeader) { + List headers = messageHeader.getAttachmentHeaders(); + List items = new ArrayList<>(headers.size()); + boolean needsSize = headers.size() == 1; + for (AttachmentHeader h : headers) { + AttachmentItem item = itemCache.get(h.getMessageId()); + if (item == null || (needsSize && !item.hasSize())) { + item = new AttachmentItem(h, defaultSize, defaultSize, LOADING); + UnavailableItem unavailableItem = new UnavailableItem( + messageHeader.getId(), h, needsSize); + unavailableItems.put(h.getMessageId(), unavailableItem); + } + items.add(item); + } + return items; + } + + @Override + @DatabaseExecutor + public void cacheAttachmentItem(MessageId conversationMessageId, + AttachmentHeader h) throws DbException { + try { + Attachment a = messagingManager.getAttachment(h); + // this adds it to the cache automatically + createAttachmentItem(a, true); + } catch (NoSuchMessageException e) { + LOG.info("Attachment not received yet"); + } + } + + @Override + @DatabaseExecutor + public Pair loadAttachmentItem( + MessageId attachmentId) throws DbException { + UnavailableItem unavailableItem = unavailableItems.get(attachmentId); + if (unavailableItem == null) throw new AssertionError(); + + MessageId conversationMessageId = + unavailableItem.getConversationMessageId(); + AttachmentHeader h = unavailableItem.getHeader(); + boolean needsSize = unavailableItem.needsSize(); + + AttachmentItem item; + try { + Attachment 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); + } + return new Pair<>(conversationMessageId, item); + } + + @Override + public AttachmentItem createAttachmentItem(Attachment a, + boolean needsSize) { AttachmentHeader h = a.getHeader(); - if (!needsSize) { + 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()); + item = createAttachmentItem(h, size); + } else { String extension = imageHelper.getExtensionFromMimeType(h.getContentType()); - boolean hasError = false; + State state = AVAILABLE; if (extension == null) { extension = ""; - hasError = true; + state = ERROR; } - return new AttachmentItem(h, 0, 0, extension, 0, 0, hasError); + item = new AttachmentItem(h, extension, state); } + itemCache.put(h.getMessageId(), item); + return item; + } - InputStream is = new BufferedInputStream(a.getStream()); - Size size = imageSizeCalculator.getSize(is, h.getContentType()); - + private AttachmentItem createAttachmentItem(AttachmentHeader h, Size size) { // calculate thumbnail size Size thumbnailSize = new Size(defaultSize, defaultSize, size.mimeType); if (!size.error) { @@ -104,8 +171,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/attachment/UnavailableItem.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/UnavailableItem.java new file mode 100644 index 000000000..9aa648020 --- /dev/null +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/UnavailableItem.java @@ -0,0 +1,34 @@ +package org.briarproject.briar.android.attachment; + +import org.briarproject.bramble.api.sync.MessageId; +import org.briarproject.briar.api.messaging.AttachmentHeader; + +import javax.annotation.concurrent.Immutable; + +@Immutable +class UnavailableItem { + + private final MessageId conversationMessageId; + private final AttachmentHeader header; + private final boolean needsSize; + + UnavailableItem(MessageId conversationMessageId, + AttachmentHeader header, boolean needsSize) { + this.conversationMessageId = conversationMessageId; + this.header = header; + this.needsSize = needsSize; + } + + MessageId getConversationMessageId() { + return conversationMessageId; + } + + AttachmentHeader getHeader() { + return header; + } + + boolean needsSize() { + return needsSize; + } + +} 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 6aa6986c7..1aba980d7 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 @@ -27,7 +27,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,13 +64,13 @@ 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; @@ -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; @@ -133,9 +130,11 @@ 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; +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 +184,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(); @@ -264,6 +261,7 @@ public class ConversationActivity extends BriarActivity adapter = new ConversationAdapter(this, this); list = findViewById(R.id.conversationView); layoutManager = new LinearLayoutManager(this); + layoutManager.setStackFromEnd(true); list.setLayoutManager(layoutManager); list.setAdapter(adapter); list.setEmptyText(getString(R.string.no_private_messages)); @@ -540,6 +538,7 @@ public class ConversationActivity extends BriarActivity }); } + @DatabaseExecutor private void eagerlyLoadMessageSize(PrivateMessageHeader h) { try { MessageId id = h.getId(); @@ -556,21 +555,10 @@ 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.cacheAttachmentItem(h.getId(), header); } } catch (DbException e) { logException(LOG, WARNING, e); @@ -651,44 +639,34 @@ public class ConversationActivity extends BriarActivity && adapter.isScrolledToBottom(layoutManager); } - private void loadMessageAttachments(PrivateMessageHeader h) { - // TODO: Use placeholders for missing/invalid attachments + private void loadMessageAttachments(List items) { 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); + for (AttachmentItem item : items) { + if (item.getState() == LOADING) + loadMessageAttachment(item.getMessageId()); } }); } - private void displayMessageAttachments(MessageId m, - List items) { + @DatabaseExecutor + private void loadMessageAttachment(MessageId attachmentId) { + try { + Pair pair = + attachmentRetriever.loadAttachmentItem(attachmentId); + 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 pair = adapter.getMessageItem(m); - if (pair != null) { + if (pair != null && pair.getSecond().updateAttachments(item)) { boolean scroll = shouldScrollWhenUpdatingMessage(); - pair.getSecond().setAttachments(items); adapter.notifyItemChanged(pair.getFirst()); if (scroll) scrollToBottom(); } @@ -765,11 +743,7 @@ public class ConversationActivity extends BriarActivity @UiThread private void onAttachmentReceived(MessageId attachmentId) { - PrivateMessageHeader h = missingAttachments.remove(attachmentId); - if (h != null) { - LOG.info("Missing attachment received"); - loadMessageAttachments(h); - } + runOnDbThread(() -> loadMessageAttachment(attachmentId)); } @UiThread @@ -780,7 +754,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 +1081,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 +1122,14 @@ 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(); - } - return attachments; + List items = attachmentRetriever.getAttachmentItems(h); + loadMessageAttachments(items); + return items; } } 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/ImageActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageActivity.java index 749ef495c..874e4b95f 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 @@ -17,6 +17,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; @@ -69,6 +70,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 = @@ -82,6 +84,7 @@ public class ImageActivity extends BriarActivity private AppBarLayout appBarLayout; private ViewPager viewPager; private List attachments; + private MessageId conversationMessageId; @Override public void injectActivity(ActivityComponent component) { @@ -136,6 +139,7 @@ public class ImageActivity extends BriarActivity String date = formatDateAbsolute(this, time); contactName.setText(name); dateView.setText(date); + conversationMessageId = new MessageId(i.getByteArrayExtra(ITEM_ID)); // Set up image ViewPager viewPager = findViewById(R.id.viewPager); @@ -325,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..f0401b4cc 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 @@ -58,7 +59,7 @@ class ImageAdapter extends Adapter { // get item requireNonNull(conversationItem); AttachmentItem item = items.get(position); - // set onClick listener + // set onClick listener, if not missing or error imageViewHolder.itemView.setOnClickListener(v -> listener.onAttachmentClicked(v, conversationItem, item) ); 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..c2f368df6 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; @@ -33,6 +34,7 @@ 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.conversation.ImageActivity.ATTACHMENT_POSITION; +import static org.briarproject.briar.android.conversation.ImageActivity.ITEM_ID; @MethodsNotNullByDefault @ParametersAreNonnullByDefault @@ -45,14 +47,17 @@ public class ImageFragment extends Fragment { 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 +75,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 @@ -107,7 +114,7 @@ public class ImageFragment extends Fragment { // set transition name only when not animatable, // because the animation won't start otherwise photoView.setTransitionName( - attachment.getTransitionName()); + attachment.getTransitionName(conversationItemId)); } // Move image to the top if overlapping toolbar if (viewModel.isOverlappingToolbar(photoView, resource)) { 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..344d7dde0 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); + 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 { - setImageViewDimensions(attachment, single, needsStretch); loadImage(attachment, r); if (SDK_INT >= 21) { - imageView.setTransitionName(attachment.getTransitionName()); + imageView.setTransitionName( + attachment.getTransitionName(conversationItemId)); } + imageView.setScaleType(CENTER_CROP); } } 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 318d05b36..096a4b505 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..8fae2c224 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 @@ -14,9 +14,9 @@ import java.io.InputStream; 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 { @@ -47,10 +47,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 +63,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 +80,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 +88,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 +104,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 +125,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