Improve handling of missing and invalid attachments.

This commit is contained in:
akwizgran
2019-06-18 15:24:49 +01:00
parent ed20b2d8d6
commit 593a0c4632
8 changed files with 109 additions and 112 deletions

View File

@@ -98,7 +98,7 @@ public class AttachmentCreator {
// get and cache AttachmentItem for ImagePreview
try {
Attachment a = retriever.getMessageAttachment(h);
AttachmentItem item = retriever.getAttachmentItem(h, a, needsSize);
AttachmentItem item = retriever.getAttachmentItem(a, needsSize);
if (item.hasError()) throw new IOException();
AttachmentItemResult itemResult =
new AttachmentItemResult(uri, item);

View File

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

View File

@@ -37,6 +37,7 @@ import org.briarproject.bramble.api.contact.event.ContactRemovedEvent;
import org.briarproject.bramble.api.db.DatabaseExecutor;
import org.briarproject.bramble.api.db.DbException;
import org.briarproject.bramble.api.db.NoSuchContactException;
import org.briarproject.bramble.api.db.NoSuchMessageException;
import org.briarproject.bramble.api.event.Event;
import org.briarproject.bramble.api.event.EventBus;
import org.briarproject.bramble.api.event.EventListener;
@@ -107,6 +108,7 @@ import static android.support.v7.util.SortedList.INVALID_POSITION;
import static android.view.Gravity.RIGHT;
import static android.widget.Toast.LENGTH_SHORT;
import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static java.util.Collections.sort;
import static java.util.Objects.requireNonNull;
import static java.util.logging.Level.INFO;
@@ -434,29 +436,36 @@ public class ConversationActivity extends BriarActivity
});
}
private void eagerlyLoadMessageSize(PrivateMessageHeader h)
throws DbException {
MessageId id = h.getId();
// If the message has text, load it
if (h.hasText()) {
String text = textCache.get(id);
if (text == null) {
LOG.info("Eagerly loading text for latest message");
text = messagingManager.getMessageText(id);
textCache.put(id, requireNonNull(text));
private void eagerlyLoadMessageSize(PrivateMessageHeader h) {
try {
MessageId id = h.getId();
// If the message has text, load it
if (h.hasText()) {
String text = textCache.get(id);
if (text == null) {
LOG.info("Eagerly loading text for latest message");
text = messagingManager.getMessageText(id);
textCache.put(id, requireNonNull(text));
}
}
}
// If the message has a single image, load its size - for multiple
// images we use a grid so the size is fixed
if (h.getAttachmentHeaders().size() == 1) {
List<AttachmentItem> items = attachmentRetriever.cacheGet(id);
if (items == null) {
LOG.info("Eagerly loading image size for latest message");
items = attachmentRetriever.getAttachmentItems(
attachmentRetriever.getMessageAttachments(
h.getAttachmentHeaders()));
attachmentRetriever.cachePut(id, items);
// If the message has a single image, load its size - for multiple
// images we use a grid so the size is fixed
List<AttachmentHeader> headers = h.getAttachmentHeaders();
if (headers.size() == 1) {
List<AttachmentItem> items = attachmentRetriever.cacheGet(id);
if (items == null) {
LOG.info("Eagerly loading image size for latest message");
Attachment a = attachmentRetriever
.getMessageAttachment(headers.get(0));
AttachmentItem item =
attachmentRetriever.getAttachmentItem(a, true);
attachmentRetriever.cachePut(id, singletonList(item));
}
}
} catch (NoSuchMessageException e) {
LOG.info("Attachment not received yet");
} catch (DbException e) {
logException(LOG, WARNING, e);
}
}
@@ -536,14 +545,23 @@ public class ConversationActivity extends BriarActivity
private void loadMessageAttachments(MessageId messageId,
List<AttachmentHeader> headers) {
// TODO: Use placeholders for missing/invalid attachments
runOnDbThread(() -> {
try {
List<Pair<AttachmentHeader, Attachment>> attachments =
attachmentRetriever.getMessageAttachments(headers);
// TODO move getting the items off to IoExecutor, if size == 1
List<AttachmentItem> items =
attachmentRetriever.getAttachmentItems(attachments);
boolean needsSize = headers.size() == 1;
List<AttachmentItem> items = new ArrayList<>(headers.size());
for (AttachmentHeader h : headers) {
Attachment a = attachmentRetriever.getMessageAttachment(h);
AttachmentItem item =
attachmentRetriever.getAttachmentItem(a, needsSize);
items.add(item);
}
// TODO: Don't cache items unless all are present and valid
attachmentRetriever.cachePut(messageId, items);
displayMessageAttachments(messageId, items);
} catch (NoSuchMessageException e) {
LOG.info("Attachment not received yet");
} catch (DbException e) {
logException(LOG, WARNING, e);
}
@@ -553,7 +571,6 @@ public class ConversationActivity extends BriarActivity
private void displayMessageAttachments(MessageId m,
List<AttachmentItem> items) {
runOnUiThreadUnlessDestroyed(() -> {
attachmentRetriever.cachePut(m, items);
Pair<Integer, ConversationMessageItem> pair =
adapter.getMessageItem(m);
if (pair != null) {
@@ -567,6 +584,7 @@ public class ConversationActivity extends BriarActivity
@Override
public void eventOccurred(Event e) {
// TODO: Load missing attachments when they arrive
if (e instanceof ContactRemovedEvent) {
ContactRemovedEvent c = (ContactRemovedEvent) e;
if (c.getContactId().equals(contactId)) {