diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentController.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentController.java index a9c997dc2..9b4b256d7 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentController.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentController.java @@ -29,7 +29,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.CopyOnWriteArrayList; import java.util.logging.Logger; import static android.support.media.ExifInterface.ORIENTATION_ROTATE_270; @@ -59,7 +58,8 @@ class AttachmentController { private final int minWidth, maxWidth; private final int minHeight, maxHeight; - private final List unsent = new CopyOnWriteArrayList<>(); + private final Map unsentItems = + new ConcurrentHashMap<>(); private final Map> attachmentCache = new ConcurrentHashMap<>(); @@ -121,9 +121,15 @@ class AttachmentController { } @DatabaseExecutor - AttachmentHeader createAttachmentHeader(ContentResolver contentResolver, - GroupId groupId, Uri uri) + void createAttachmentHeader(ContentResolver contentResolver, + GroupId groupId, Uri uri, boolean needsSize) throws IOException, DbException { + if (unsentItems.containsKey(uri)) { + // This can happen due to configuration (screen orientation) change. + // So don't create a new attachment, if we have one already. + return; + } + long start = now(); InputStream is = contentResolver.openInputStream(uri); if (is == null) throw new IOException(); String contentType = contentResolver.getType(uri); @@ -132,32 +138,47 @@ class AttachmentController { AttachmentHeader h = messagingManager .addLocalAttachment(groupId, timestamp, contentType, is); tryToClose(is, LOG, WARNING); - unsent.add(h); - return h; + logDuration(LOG, "Storing attachment", start); + // get and store AttachmentItem for ImagePreview + AttachmentItem item = + getAttachmentItem(contentResolver, uri, h, needsSize); + if (item.hasError()) throw new IOException(); + unsentItems.put(uri, item); } @DatabaseExecutor void deleteUnsentAttachments() { - for (AttachmentHeader h : unsent) { + for (AttachmentItem item : unsentItems.values()) { try { - messagingManager.removeAttachment(h); + messagingManager.removeAttachment(item.getHeader()); } catch (DbException e) { logException(LOG, WARNING, e); } } + unsentItems.clear(); } - List getUnsentAttachments() { - return new ArrayList<>(unsent); + List getUnsentAttachmentHeaders() { + List headers = + new ArrayList<>(unsentItems.values().size()); + for (AttachmentItem item : unsentItems.values()) { + headers.add(item.getHeader()); + } + return headers; } - void markAttachmentsSent() { - unsent.clear(); + /** + * Marks the attachments as sent and adds the items to the cache for display + * @param id The MessageId of the sent message. + */ + void onAttachmentsSent(MessageId id) { + attachmentCache.put(id, new ArrayList<>(unsentItems.values())); + unsentItems.clear(); } @DatabaseExecutor - AttachmentItem getAttachmentItem(ContentResolver contentResolver, Uri uri, - AttachmentHeader h, boolean needsSize) throws IOException { + private AttachmentItem getAttachmentItem(ContentResolver contentResolver, + Uri uri, AttachmentHeader h, boolean needsSize) throws IOException { InputStream is = null; try { is = contentResolver.openInputStream(uri); @@ -192,17 +213,15 @@ class AttachmentController { @VisibleForTesting AttachmentItem getAttachmentItem(AttachmentHeader h, Attachment a, boolean needsSize) { - MessageId messageId = h.getMessageId(); if (!needsSize) { - String mimeType = h.getContentType(); - String extension = imageHelper.getExtensionFromMimeType(mimeType); + String extension = + imageHelper.getExtensionFromMimeType(h.getContentType()); boolean hasError = false; if (extension == null) { extension = ""; hasError = true; } - return new AttachmentItem(messageId, 0, 0, mimeType, extension, 0, - 0, hasError); + return new AttachmentItem(h, 0, 0, extension, 0, 0, hasError); } Size size = new Size(); @@ -240,10 +259,17 @@ class AttachmentController { // get file extension String extension = imageHelper.getExtensionFromMimeType(size.mimeType); boolean hasError = extension == null || size.error; + if (!h.getContentType().equals(size.mimeType)) { + if (LOG.isLoggable(WARNING)) { + LOG.warning("Header has different mime type (" + + h.getContentType() + ") than image (" + size.mimeType + + ")."); + } + hasError = true; + } if (extension == null) extension = ""; - return new AttachmentItem(messageId, size.width, size.height, - size.mimeType, extension, thumbnailSize.width, - thumbnailSize.height, hasError); + return new AttachmentItem(h, size.width, size.height, extension, + thumbnailSize.width, thumbnailSize.height, hasError); } /** diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentItem.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentItem.java index 8c109676b..598336daa 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentItem.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentItem.java @@ -6,6 +6,7 @@ import android.support.annotation.Nullable; 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; @@ -15,9 +16,9 @@ import javax.annotation.concurrent.Immutable; @NotNullByDefault public class AttachmentItem implements Parcelable { - private final MessageId messageId; + private final AttachmentHeader header; private final int width, height; - private final String mimeType, extension; + private final String extension; private final int thumbnailWidth, thumbnailHeight; private final boolean hasError; private final long instanceId; @@ -37,13 +38,12 @@ public class AttachmentItem implements Parcelable { private static final AtomicLong NEXT_INSTANCE_ID = new AtomicLong(0); - AttachmentItem(MessageId messageId, int width, int height, String mimeType, + AttachmentItem(AttachmentHeader header, int width, int height, String extension, int thumbnailWidth, int thumbnailHeight, boolean hasError) { - this.messageId = messageId; + this.header = header; this.width = width; this.height = height; - this.mimeType = mimeType; this.extension = extension; this.thumbnailWidth = thumbnailWidth; this.thumbnailHeight = thumbnailHeight; @@ -54,19 +54,24 @@ public class AttachmentItem implements Parcelable { protected AttachmentItem(Parcel in) { byte[] messageIdByte = new byte[MessageId.LENGTH]; in.readByteArray(messageIdByte); - messageId = new MessageId(messageIdByte); + MessageId messageId = new MessageId(messageIdByte); width = in.readInt(); height = in.readInt(); - mimeType = in.readString(); + String mimeType = in.readString(); extension = in.readString(); thumbnailWidth = in.readInt(); thumbnailHeight = in.readInt(); hasError = in.readByte() != 0; instanceId = in.readLong(); + header = new AttachmentHeader(messageId, mimeType); + } + + AttachmentHeader getHeader() { + return header; } public MessageId getMessageId() { - return messageId; + return header.getMessageId(); } int getWidth() { @@ -78,7 +83,7 @@ public class AttachmentItem implements Parcelable { } String getMimeType() { - return mimeType; + return header.getContentType(); } String getExtension() { @@ -108,10 +113,10 @@ public class AttachmentItem implements Parcelable { @Override public void writeToParcel(Parcel dest, int flags) { - dest.writeByteArray(messageId.getBytes()); + dest.writeByteArray(header.getMessageId().getBytes()); dest.writeInt(width); dest.writeInt(height); - dest.writeString(mimeType); + dest.writeString(header.getContentType()); dest.writeString(extension); dest.writeInt(thumbnailWidth); dest.writeInt(thumbnailHeight); 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 9182540ea..b428ee901 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 @@ -38,11 +38,8 @@ import org.briarproject.briar.api.messaging.PrivateMessageFactory; import org.briarproject.briar.api.messaging.PrivateMessageHeader; import java.io.IOException; -import java.util.ArrayList; import java.util.Collection; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.concurrent.Executor; import java.util.logging.Logger; @@ -102,8 +99,6 @@ public class ConversationViewModel extends AndroidViewModel implements new MutableLiveData<>(); private final MutableLiveData addedHeader = new MutableLiveData<>(); - // TODO move to AttachmentController - private final Map unsentItems = new HashMap<>(); @Inject ConversationViewModel(Application application, @@ -199,12 +194,6 @@ public class ConversationViewModel extends AndroidViewModel implements @Override public void storeAttachment(Uri uri, boolean needsSize, Runnable onSuccess, Runnable onError) { - if (unsentItems.containsKey(uri)) { - // This can happen due to configuration (screen orientation) change. - // So don't create a new attachment, if we have one already. - androidExecutor.runOnUiThread(onSuccess); - return; - } if (messagingGroupId.getValue() == null) loadGroupId(); observeForeverOnce(messagingGroupId, groupId -> dbExecutor.execute(() -> { @@ -213,10 +202,8 @@ public class ConversationViewModel extends AndroidViewModel implements try { ContentResolver contentResolver = getApplication().getContentResolver(); - AttachmentHeader h = attachmentController - .createAttachmentHeader(contentResolver, groupId, uri); - unsentItems.put(uri, attachmentController - .getAttachmentItem(contentResolver, uri, h, needsSize)); + attachmentController.createAttachmentHeader(contentResolver, + groupId, uri, needsSize); androidExecutor.runOnUiThread(onSuccess); } catch (DbException | IOException e) { logException(LOG, WARNING, e); @@ -227,13 +214,12 @@ public class ConversationViewModel extends AndroidViewModel implements } @Override - public List getAttachments() { - return attachmentController.getUnsentAttachments(); + public List getAttachmentHeaders() { + return attachmentController.getUnsentAttachmentHeaders(); } @Override public void removeAttachments() { - unsentItems.clear(); dbExecutor.execute(attachmentController::deleteUnsentAttachments); } @@ -332,10 +318,7 @@ public class ConversationViewModel extends AndroidViewModel implements message.getId(), message.getGroupId(), message.getTimestamp(), true, true, false, false, text != null, attachments); - attachmentController.put(m.getMessage().getId(), - new ArrayList<>(unsentItems.values())); - unsentItems.clear(); - attachmentController.markAttachmentsSent(); + attachmentController.onAttachmentsSent(m.getMessage().getId()); // TODO add text to cache when available here addedHeader.postValue(h); } catch (DbException e) { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java b/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java index bbfeca9b4..0cc1cb32e 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java @@ -89,7 +89,7 @@ public class TextAttachmentController extends TextSendController public void onSendEvent() { if (canSend()) { listener.onSendClick(textInput.getText(), - attachmentManager.getAttachments()); + attachmentManager.getAttachmentHeaders()); reset(); } } @@ -283,7 +283,7 @@ public class TextAttachmentController extends TextSendController void storeAttachment(Uri uri, boolean needsSize, Runnable onSuccess, Runnable onError); - List getAttachments(); + List getAttachmentHeaders(); void removeAttachments(); } diff --git a/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java index d46d9d8e0..e73dce953 100644 --- a/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java @@ -163,7 +163,7 @@ class MessagingManagerImpl extends ConversationClientImpl if (is.available() == 0) throw new IOException(); byte[] b = new byte[MessageId.LENGTH]; new Random().nextBytes(b); - return new AttachmentHeader(new MessageId(b), "image/png"); + return new AttachmentHeader(new MessageId(b), contentType); } @Override