From 55f4600a69d360e93376025dfcba187eddb92e49 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 15 Feb 2019 09:13:36 -0200 Subject: [PATCH 01/17] [android] Create attachments before showing previews --- .../briar/android/blog/ReblogFragment.java | 5 +- .../android/blog/WriteBlogPostActivity.java | 5 +- .../conversation/AttachmentController.java | 55 ++++++++ .../conversation/ConversationActivity.java | 10 +- .../conversation/ConversationViewModel.java | 129 +++++++++--------- .../IntroductionMessageFragment.java | 5 +- .../android/sharing/BaseMessageFragment.java | 5 +- .../android/threaded/ThreadListActivity.java | 5 +- .../briar/android/view/ImagePreview.java | 16 +-- .../android/view/ImagePreviewAdapter.java | 18 +-- .../briar/android/view/ImagePreviewItem.java | 53 +++++++ .../android/view/ImagePreviewViewHolder.java | 8 +- .../view/TextAttachmentController.java | 47 +++++-- .../android/view/TextSendController.java | 4 +- briar-android/src/main/res/values/strings.xml | 2 +- .../briar/api/messaging/AttachmentHeader.java | 11 ++ .../api/messaging/MessagingConstants.java | 15 ++ .../briar/api/messaging/MessagingManager.java | 5 + .../briar/messaging/MessagingManagerImpl.java | 5 + 19 files changed, 291 insertions(+), 112 deletions(-) create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewItem.java diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/ReblogFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/ReblogFragment.java index 17e596cb6..3fcdddaa0 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/ReblogFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/ReblogFragment.java @@ -1,6 +1,5 @@ package org.briarproject.briar.android.blog; -import android.net.Uri; import android.os.Bundle; import android.view.LayoutInflater; import android.view.View; @@ -21,6 +20,7 @@ import org.briarproject.briar.android.fragment.BaseFragment; import org.briarproject.briar.android.view.TextInputView; import org.briarproject.briar.android.view.TextSendController; import org.briarproject.briar.android.view.TextSendController.SendListener; +import org.briarproject.briar.api.messaging.AttachmentHeader; import java.util.List; @@ -121,7 +121,8 @@ public class ReblogFragment extends BaseFragment implements SendListener { } @Override - public void onSendClick(@Nullable String text, List imageUris) { + public void onSendClick(@Nullable String text, + List headers) { ui.input.hideSoftKeyboard(); feedController.repeatPost(item, text, new UiExceptionHandler(this) { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/WriteBlogPostActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/WriteBlogPostActivity.java index 5b68499cd..1ea824b79 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/WriteBlogPostActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/WriteBlogPostActivity.java @@ -1,7 +1,6 @@ package org.briarproject.briar.android.blog; import android.content.Intent; -import android.net.Uri; import android.os.Bundle; import android.support.annotation.Nullable; import android.view.KeyEvent; @@ -27,6 +26,7 @@ import org.briarproject.briar.api.android.AndroidNotificationManager; import org.briarproject.briar.api.blog.BlogManager; import org.briarproject.briar.api.blog.BlogPost; import org.briarproject.briar.api.blog.BlogPostFactory; +import org.briarproject.briar.api.messaging.AttachmentHeader; import java.security.GeneralSecurityException; import java.util.List; @@ -120,7 +120,8 @@ public class WriteBlogPostActivity extends BriarActivity } @Override - public void onSendClick(@Nullable String text, List imageUris) { + public void onSendClick(@Nullable String text, + List headers) { if (isNullOrEmpty(text)) throw new AssertionError(); // hide publish button, show progress bar 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 11833f2ed..a9c997dc2 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 @@ -1,8 +1,11 @@ package org.briarproject.briar.android.conversation; +import android.content.ContentResolver; import android.graphics.BitmapFactory; import android.graphics.BitmapFactory.Options; +import android.net.Uri; import android.support.annotation.Nullable; +import android.support.annotation.VisibleForTesting; import android.support.media.ExifInterface; import android.webkit.MimeTypeMap; @@ -12,6 +15,7 @@ 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.GroupId; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.briar.android.conversation.ImageHelper.DecodeResult; import org.briarproject.briar.api.messaging.Attachment; @@ -25,6 +29,7 @@ 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; @@ -54,6 +59,7 @@ class AttachmentController { private final int minWidth, maxWidth; private final int minHeight, maxHeight; + private final List unsent = new CopyOnWriteArrayList<>(); private final Map> attachmentCache = new ConcurrentHashMap<>(); @@ -114,6 +120,54 @@ class AttachmentController { return attachments; } + @DatabaseExecutor + AttachmentHeader createAttachmentHeader(ContentResolver contentResolver, + GroupId groupId, Uri uri) + throws IOException, DbException { + InputStream is = contentResolver.openInputStream(uri); + if (is == null) throw new IOException(); + String contentType = contentResolver.getType(uri); + if (contentType == null) throw new IOException("null content type"); + long timestamp = System.currentTimeMillis(); + AttachmentHeader h = messagingManager + .addLocalAttachment(groupId, timestamp, contentType, is); + tryToClose(is, LOG, WARNING); + unsent.add(h); + return h; + } + + @DatabaseExecutor + void deleteUnsentAttachments() { + for (AttachmentHeader h : unsent) { + try { + messagingManager.removeAttachment(h); + } catch (DbException e) { + logException(LOG, WARNING, e); + } + } + } + + List getUnsentAttachments() { + return new ArrayList<>(unsent); + } + + void markAttachmentsSent() { + unsent.clear(); + } + + @DatabaseExecutor + AttachmentItem getAttachmentItem(ContentResolver contentResolver, Uri uri, + AttachmentHeader h, boolean needsSize) throws IOException { + InputStream is = null; + try { + is = contentResolver.openInputStream(uri); + if (is == null) throw new IOException(); + return getAttachmentItem(h, new Attachment(is), needsSize); + } finally { + if (is != null) tryToClose(is, LOG, WARNING); + } + } + /** * Creates {@link AttachmentItem}s from the passed headers and Attachments. *

@@ -135,6 +189,7 @@ class AttachmentController { * Creates an {@link AttachmentItem} from the {@link Attachment}'s * {@link InputStream} which will be closed when this method returns. */ + @VisibleForTesting AttachmentItem getAttachmentItem(AttachmentHeader h, Attachment a, boolean needsSize) { MessageId messageId = h.getMessageId(); 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 c844f030c..fac65e937 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 @@ -6,7 +6,6 @@ import android.arch.lifecycle.ViewModelProvider; import android.arch.lifecycle.ViewModelProviders; import android.content.DialogInterface; import android.content.Intent; -import android.net.Uri; import android.os.Bundle; import android.os.Parcelable; import android.support.annotation.Nullable; @@ -264,7 +263,7 @@ public class ConversationActivity extends BriarActivity if (FEATURE_FLAG_IMAGE_ATTACHMENTS) { ImagePreview imagePreview = findViewById(R.id.imagePreview); sendController = new TextAttachmentController(textInputView, - imagePreview, this, this); + imagePreview, this, this, viewModel); observeOnce(viewModel.hasImageSupport(), this, hasSupport -> { if (hasSupport != null && hasSupport) { // remove cast when removing FEATURE_FLAG_IMAGE_ATTACHMENTS @@ -658,12 +657,13 @@ public class ConversationActivity extends BriarActivity } @Override - public void onSendClick(@Nullable String text, List imageUris) { - if (isNullOrEmpty(text) && imageUris.isEmpty()) + public void onSendClick(@Nullable String text, + List attachmentHeaders) { + if (isNullOrEmpty(text) && attachmentHeaders.isEmpty()) throw new AssertionError(); long timestamp = System.currentTimeMillis(); timestamp = Math.max(timestamp, getMinTimestampForNewMessage()); - viewModel.sendMessage(text, imageUris, timestamp); + viewModel.sendMessage(text, attachmentHeaders, timestamp); textInputView.clearText(); } 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 ba1457e7c..9182540ea 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,7 +11,6 @@ import android.support.annotation.Nullable; import android.support.annotation.UiThread; import org.briarproject.bramble.api.FormatException; -import org.briarproject.bramble.api.Pair; import org.briarproject.bramble.api.contact.Contact; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.contact.ContactManager; @@ -27,10 +26,11 @@ import org.briarproject.bramble.api.settings.SettingsManager; import org.briarproject.bramble.api.sync.GroupId; import org.briarproject.bramble.api.sync.Message; import org.briarproject.bramble.api.sync.MessageId; +import org.briarproject.bramble.api.system.AndroidExecutor; import org.briarproject.briar.android.util.UiUtils; +import org.briarproject.briar.android.view.TextAttachmentController.AttachmentManager; 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.AttachmentHeader; import org.briarproject.briar.api.messaging.MessagingManager; import org.briarproject.briar.api.messaging.PrivateMessage; @@ -38,10 +38,11 @@ import org.briarproject.briar.api.messaging.PrivateMessageFactory; import org.briarproject.briar.api.messaging.PrivateMessageHeader; import java.io.IOException; -import java.io.InputStream; 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; @@ -50,7 +51,6 @@ import javax.inject.Inject; 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.logDuration; import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.LogUtils.now; @@ -59,7 +59,8 @@ import static org.briarproject.briar.android.settings.SettingsFragment.SETTINGS_ import static org.briarproject.briar.android.util.UiUtils.observeForeverOnce; @NotNullByDefault -public class ConversationViewModel extends AndroidViewModel { +public class ConversationViewModel extends AndroidViewModel implements + AttachmentManager { private static Logger LOG = getLogger(ConversationViewModel.class.getName()); @@ -73,6 +74,7 @@ public class ConversationViewModel extends AndroidViewModel { @CryptoExecutor private final Executor cryptoExecutor; private final TransactionManager db; + private final AndroidExecutor androidExecutor; private final MessagingManager messagingManager; private final ContactManager contactManager; private final SettingsManager settingsManager; @@ -100,17 +102,20 @@ public class ConversationViewModel extends AndroidViewModel { new MutableLiveData<>(); private final MutableLiveData addedHeader = new MutableLiveData<>(); + // TODO move to AttachmentController + private final Map unsentItems = new HashMap<>(); @Inject ConversationViewModel(Application application, @DatabaseExecutor Executor dbExecutor, @CryptoExecutor Executor cryptoExecutor, TransactionManager db, - MessagingManager messagingManager, ContactManager contactManager, - SettingsManager settingsManager, + AndroidExecutor androidExecutor, MessagingManager messagingManager, + ContactManager contactManager, SettingsManager settingsManager, PrivateMessageFactory privateMessageFactory) { super(application); this.dbExecutor = dbExecutor; this.cryptoExecutor = cryptoExecutor; + this.androidExecutor = androidExecutor; this.db = db; this.messagingManager = messagingManager; this.contactManager = contactManager; @@ -121,6 +126,12 @@ public class ConversationViewModel extends AndroidViewModel { contactDeleted.setValue(false); } + @Override + protected void onCleared() { + super.onCleared(); + attachmentController.deleteUnsentAttachments(); + } + /** * Setting the {@link ContactId} automatically triggers loading of other * data. @@ -176,15 +187,56 @@ public class ConversationViewModel extends AndroidViewModel { }); } - void sendMessage(@Nullable String text, List uris, long timestamp) { + void sendMessage(@Nullable String text, + List attachmentHeaders, long timestamp) { if (messagingGroupId.getValue() == null) loadGroupId(); observeForeverOnce(messagingGroupId, groupId -> { if (groupId == null) return; - // calls through to creating and storing the message - storeAttachments(groupId, text, uris, timestamp); + createMessage(groupId, text, attachmentHeaders, timestamp); }); } + @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(() + -> { + if (groupId == null) throw new IllegalStateException(); + long start = now(); + try { + ContentResolver contentResolver = + getApplication().getContentResolver(); + AttachmentHeader h = attachmentController + .createAttachmentHeader(contentResolver, groupId, uri); + unsentItems.put(uri, attachmentController + .getAttachmentItem(contentResolver, uri, h, needsSize)); + androidExecutor.runOnUiThread(onSuccess); + } catch (DbException | IOException e) { + logException(LOG, WARNING, e); + androidExecutor.runOnUiThread(onError); + } + logDuration(LOG, "Storing attachment", start); + })); + } + + @Override + public List getAttachments() { + return attachmentController.getUnsentAttachments(); + } + + @Override + public void removeAttachments() { + unsentItems.clear(); + dbExecutor.execute(attachmentController::deleteUnsentAttachments); + } + private void loadGroupId() { if (contactId == null) throw new IllegalStateException(); dbExecutor.execute(() -> { @@ -252,58 +304,8 @@ public class ConversationViewModel extends AndroidViewModel { }); } - private void storeAttachments(GroupId groupId, @Nullable String text, - List uris, long timestamp) { - dbExecutor.execute(() -> { - long start = now(); - List attachments = new ArrayList<>(); - List items = new ArrayList<>(); - boolean needsSize = uris.size() == 1; - for (Uri uri : uris) { - Pair pair = - createAttachmentHeader(groupId, uri, timestamp, - needsSize); - if (pair == null) continue; - attachments.add(pair.getFirst()); - items.add(pair.getSecond()); - } - logDuration(LOG, "Storing attachments", start); - createMessage(groupId, text, attachments, items, timestamp); - }); - } - - @Nullable - @DatabaseExecutor - private Pair createAttachmentHeader( - GroupId groupId, Uri uri, long timestamp, boolean needsSize) { - InputStream is = null; - try { - ContentResolver contentResolver = - getApplication().getContentResolver(); - is = contentResolver.openInputStream(uri); - if (is == null) throw new IOException(); - String contentType = contentResolver.getType(uri); - if (contentType == null) throw new IOException("null content type"); - AttachmentHeader h = messagingManager - .addLocalAttachment(groupId, timestamp, contentType, is); - is.close(); - // re-open stream to get AttachmentItem - is = contentResolver.openInputStream(uri); - if (is == null) throw new IOException(); - AttachmentItem item = attachmentController - .getAttachmentItem(h, new Attachment(is), needsSize); - return new Pair<>(h, item); - } catch (DbException | IOException e) { - logException(LOG, WARNING, e); - return null; - } finally { - if (is != null) tryToClose(is, LOG, WARNING); - } - } - private void createMessage(GroupId groupId, @Nullable String text, - List attachments, List aItems, - long timestamp) { + List attachments, long timestamp) { cryptoExecutor.execute(() -> { try { // TODO remove when text can be null in the backend @@ -311,7 +313,6 @@ public class ConversationViewModel extends AndroidViewModel { PrivateMessage pm = privateMessageFactory .createPrivateMessage(groupId, timestamp, msgText, attachments); - attachmentController.put(pm.getMessage().getId(), aItems); storeMessage(pm, msgText, attachments); } catch (FormatException e) { throw new RuntimeException(e); @@ -331,6 +332,10 @@ public class ConversationViewModel extends AndroidViewModel { 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(); // 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/introduction/IntroductionMessageFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/introduction/IntroductionMessageFragment.java index dee769cf3..70478b884 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/introduction/IntroductionMessageFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/introduction/IntroductionMessageFragment.java @@ -1,7 +1,6 @@ package org.briarproject.briar.android.introduction; import android.content.Context; -import android.net.Uri; import android.os.Bundle; import android.support.annotation.Nullable; import android.support.v7.app.ActionBar; @@ -26,6 +25,7 @@ import org.briarproject.briar.android.view.TextInputView; import org.briarproject.briar.android.view.TextSendController; import org.briarproject.briar.android.view.TextSendController.SendListener; import org.briarproject.briar.api.introduction.IntroductionManager; +import org.briarproject.briar.api.messaging.AttachmentHeader; import java.util.List; import java.util.logging.Logger; @@ -193,7 +193,8 @@ public class IntroductionMessageFragment extends BaseFragment } @Override - public void onSendClick(@Nullable String text, List imageUris) { + public void onSendClick(@Nullable String text, + List headers) { // disable button to prevent accidental double invitations ui.message.setReady(false); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/sharing/BaseMessageFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/sharing/BaseMessageFragment.java index 57c299fa4..eaff51eb4 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/sharing/BaseMessageFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/sharing/BaseMessageFragment.java @@ -1,7 +1,6 @@ package org.briarproject.briar.android.sharing; import android.content.Context; -import android.net.Uri; import android.os.Bundle; import android.support.annotation.Nullable; import android.support.annotation.StringRes; @@ -19,6 +18,7 @@ import org.briarproject.briar.android.fragment.BaseFragment; import org.briarproject.briar.android.view.LargeTextInputView; import org.briarproject.briar.android.view.TextSendController; import org.briarproject.briar.android.view.TextSendController.SendListener; +import org.briarproject.briar.api.messaging.AttachmentHeader; import java.util.List; @@ -83,7 +83,8 @@ public abstract class BaseMessageFragment extends BaseFragment } @Override - public void onSendClick(@Nullable String text, List imageUris) { + public void onSendClick(@Nullable String text, + List headers) { // disable button to prevent accidental double actions sendController.setReady(false); message.hideSoftKeyboard(); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListActivity.java index f74626b67..80456c453 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListActivity.java @@ -1,7 +1,6 @@ package org.briarproject.briar.android.threaded; import android.content.Intent; -import android.net.Uri; import android.os.Bundle; import android.os.Parcelable; import android.support.annotation.CallSuper; @@ -34,6 +33,7 @@ import org.briarproject.briar.android.view.TextSendController; import org.briarproject.briar.android.view.TextSendController.SendListener; import org.briarproject.briar.android.view.UnreadMessageButton; import org.briarproject.briar.api.client.NamedGroup; +import org.briarproject.briar.api.messaging.AttachmentHeader; import java.util.Collection; import java.util.List; @@ -341,7 +341,8 @@ public abstract class ThreadListActivity imageUris) { + public void onSendClick(@Nullable String text, + List headers) { if (isNullOrEmpty(text)) throw new AssertionError(); I replyItem = adapter.getHighlightedItem(); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java index f4158a495..78f74a189 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java @@ -1,7 +1,6 @@ package org.briarproject.briar.android.view; import android.content.Context; -import android.net.Uri; import android.support.annotation.Nullable; import android.support.constraint.ConstraintLayout; import android.support.v7.widget.RecyclerView; @@ -60,21 +59,22 @@ public class ImagePreview extends ConstraintLayout { this.listener = listener; } - void showPreview(Collection imageUris) { + void showPreview(Collection items) { if (listener == null) throw new IllegalStateException(); - if (imageUris.size() == 1) { + if (items.size() == 1) { LayoutParams params = (LayoutParams) imageList.getLayoutParams(); params.width = MATCH_PARENT; imageList.setLayoutParams(params); } setVisibility(VISIBLE); - imageList.setAdapter(new ImagePreviewAdapter(imageUris, listener)); + ImagePreviewAdapter adapter = new ImagePreviewAdapter(items, listener); + imageList.setAdapter(adapter); } - void removeUri(Uri uri) { + void loadPreviewImage(ImagePreviewItem item) { ImagePreviewAdapter adapter = - (ImagePreviewAdapter) imageList.getAdapter(); - requireNonNull(adapter).removeUri(uri); + ((ImagePreviewAdapter) imageList.getAdapter()); + requireNonNull(adapter).loadItemPreview(item); } interface ImagePreviewListener { @@ -86,7 +86,7 @@ public class ImagePreview extends ConstraintLayout { * * Warning: Glide may call this multiple times. */ - void onUriError(Uri uri); + void onError(); void onCancel(); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java index a6508e4eb..70735fdd6 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java @@ -1,6 +1,5 @@ package org.briarproject.briar.android.view; -import android.net.Uri; import android.support.annotation.LayoutRes; import android.support.v7.widget.RecyclerView.Adapter; import android.view.LayoutInflater; @@ -15,17 +14,19 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; +import static android.support.v7.widget.RecyclerView.NO_POSITION; import static java.util.Objects.requireNonNull; @NotNullByDefault class ImagePreviewAdapter extends Adapter { - private final List items; + private final List items; private final ImagePreviewListener listener; @LayoutRes private final int layout; - ImagePreviewAdapter(Collection items, ImagePreviewListener listener) { + ImagePreviewAdapter(Collection items, + ImagePreviewListener listener) { this.items = new ArrayList<>(items); this.listener = listener; this.layout = items.size() == 1 ? @@ -52,11 +53,12 @@ class ImagePreviewAdapter extends Adapter { return items.size(); } - void removeUri(Uri uri) { - int pos = items.indexOf(uri); - if (pos == -1) return; - items.remove(uri); - notifyItemRemoved(pos); + void loadItemPreview(ImagePreviewItem item) { + int pos = items.indexOf(item); + if (pos == NO_POSITION) throw new AssertionError(); + ImagePreviewItem newItem = items.get(pos); + newItem.setWaitForLoading(false); + notifyItemChanged(pos, newItem); } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewItem.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewItem.java new file mode 100644 index 000000000..71576015f --- /dev/null +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewItem.java @@ -0,0 +1,53 @@ +package org.briarproject.briar.android.view; + +import android.net.Uri; +import android.support.annotation.Nullable; + +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +@NotNullByDefault +class ImagePreviewItem { + + private final Uri uri; + private boolean waitForLoading = true; + + private ImagePreviewItem(Uri uri) { + this.uri = uri; + } + + Uri getUri() { + return uri; + } + + void setWaitForLoading(boolean waitForLoading) { + this.waitForLoading = waitForLoading; + } + + boolean waitForLoading() { + return waitForLoading; + } + + static List fromUris(Collection uris) { + List items = new ArrayList<>(uris.size()); + for (Uri uri : uris) { + items.add(new ImagePreviewItem(uri)); + } + return items; + } + + @Override + public boolean equals(@Nullable Object o) { + return o instanceof ImagePreviewItem && + uri.equals(((ImagePreviewItem) o).uri); + } + + @Override + public int hashCode() { + return uri.hashCode(); + } + +} diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java index c16ce14ca..a18ae3c17 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java @@ -1,7 +1,6 @@ package org.briarproject.briar.android.view; import android.graphics.drawable.Drawable; -import android.net.Uri; import android.support.annotation.DrawableRes; import android.support.annotation.Nullable; import android.support.v7.widget.RecyclerView.ViewHolder; @@ -42,9 +41,10 @@ class ImagePreviewViewHolder extends ViewHolder { this.progressBar = v.findViewById(R.id.progressBar); } - void bind(Uri uri) { + void bind(ImagePreviewItem item) { + if (item.waitForLoading()) return; GlideApp.with(imageView) - .load(uri) + .load(item.getUri()) .diskCacheStrategy(NONE) .error(ERROR_RES) .downsample(FIT_CENTER) @@ -55,7 +55,7 @@ class ImagePreviewViewHolder extends ViewHolder { Object model, Target target, boolean isFirstResource) { progressBar.setVisibility(INVISIBLE); - listener.onUriError(uri); + listener.onError(); return false; } 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 3a73ba695..bbfeca9b4 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 @@ -16,6 +16,7 @@ import android.widget.Toast; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.briar.R; import org.briarproject.briar.android.view.ImagePreview.ImagePreviewListener; +import org.briarproject.briar.api.messaging.AttachmentHeader; import java.util.ArrayList; import java.util.List; @@ -46,6 +47,7 @@ public class TextAttachmentController extends TextSendController private final ImagePreview imagePreview; private final AttachImageListener imageListener; private final CompositeSendButton sendButton; + private final AttachmentManager attachmentManager; private CharSequence textHint; private List imageUris = emptyList(); @@ -53,10 +55,12 @@ public class TextAttachmentController extends TextSendController private boolean loadingPreviews = false; public TextAttachmentController(TextInputView v, ImagePreview imagePreview, - SendListener listener, AttachImageListener imageListener) { + SendListener listener, AttachImageListener imageListener, + AttachmentManager attachmentManager) { super(v, listener, false); this.imageListener = imageListener; this.imagePreview = imagePreview; + this.attachmentManager = attachmentManager; this.imagePreview.setImagePreviewListener(this); sendButton = (CompositeSendButton) compositeSendButton; @@ -84,7 +88,8 @@ public class TextAttachmentController extends TextSendController @Override public void onSendEvent() { if (canSend()) { - listener.onSendClick(textInput.getText(), imageUris); + listener.onSendClick(textInput.getText(), + attachmentManager.getAttachments()); reset(); } } @@ -139,7 +144,15 @@ public class TextAttachmentController extends TextSendController loadingPreviews = true; updateViewState(); textInput.setHint(R.string.image_caption_hint); - imagePreview.showPreview(imageUris); + List items = ImagePreviewItem.fromUris(imageUris); + imagePreview.showPreview(items); + // store attachments and show preview when successful + boolean needsSize = items.size() == 1; + for (ImagePreviewItem item : items) { + attachmentManager.storeAttachment(item.getUri(), needsSize, + () -> imagePreview.loadPreviewImage(item), + this::onError); + } } private void reset() { @@ -180,22 +193,16 @@ public class TextAttachmentController extends TextSendController } @Override - public void onUriError(Uri uri) { - boolean removed = imageUris.remove(uri); - if (!removed) { - // we have removed this Uri already, do not remove it again - return; - } - imagePreview.removeUri(uri); - if (imageUris.isEmpty()) onCancel(); + public void onError() { Toast.makeText(textInput.getContext(), R.string.image_attach_error, LENGTH_LONG).show(); - checkAllPreviewsLoaded(); + onCancel(); } @Override public void onCancel() { textInput.clearText(); + attachmentManager.removeAttachments(); reset(); } @@ -265,4 +272,20 @@ public class TextAttachmentController extends TextSendController void onAttachImage(Intent intent); } + public interface AttachmentManager { + /** + * Stores a new attachment in the database. + * + * @param uri The Uri of the attachment to store. + * @param onSuccess will be run on the UiThread when the attachment was stored successfully. + * @param onError will be run on the UiThread when the attachment could not be stored. + */ + void storeAttachment(Uri uri, boolean needsSize, Runnable onSuccess, + Runnable onError); + + List getAttachments(); + + void removeAttachments(); + } + } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/TextSendController.java b/briar-android/src/main/java/org/briarproject/briar/android/view/TextSendController.java index f0a864afe..725d23baa 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/TextSendController.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/TextSendController.java @@ -1,6 +1,5 @@ package org.briarproject.briar.android.view; -import android.net.Uri; import android.os.Parcelable; import android.support.annotation.Nullable; import android.support.annotation.UiThread; @@ -10,6 +9,7 @@ import android.view.View; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.briar.R; import org.briarproject.briar.android.view.EmojiTextInputView.TextInputListener; +import org.briarproject.briar.api.messaging.AttachmentHeader; import java.util.List; @@ -85,7 +85,7 @@ public class TextSendController implements TextInputListener { } public interface SendListener { - void onSendClick(@Nullable String text, List imageUris); + void onSendClick(@Nullable String text, List headers); } } diff --git a/briar-android/src/main/res/values/strings.xml b/briar-android/src/main/res/values/strings.xml index 144581e5d..4565d2a55 100644 --- a/briar-android/src/main/res/values/strings.xml +++ b/briar-android/src/main/res/values/strings.xml @@ -129,7 +129,7 @@ Type message Add a caption (optional) Attach image - Could not attach image + Could not attach image(s) Change contact name Contact name Change diff --git a/briar-api/src/main/java/org/briarproject/briar/api/messaging/AttachmentHeader.java b/briar-api/src/main/java/org/briarproject/briar/api/messaging/AttachmentHeader.java index 685e2ea3a..970401211 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/messaging/AttachmentHeader.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/messaging/AttachmentHeader.java @@ -25,4 +25,15 @@ public class AttachmentHeader { return contentType; } + @Override + public boolean equals(Object o) { + return o instanceof AttachmentHeader && + messageId.equals(((AttachmentHeader) o).messageId); + } + + @Override + public int hashCode() { + return messageId.hashCode(); + } + } diff --git a/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingConstants.java b/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingConstants.java index 1efdd748d..c6114dc66 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingConstants.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingConstants.java @@ -8,4 +8,19 @@ public interface MessagingConstants { * The maximum length of a private message's text in UTF-8 bytes. */ int MAX_PRIVATE_MESSAGE_TEXT_LENGTH = MAX_MESSAGE_BODY_LENGTH - 1024; + + /** + * The supported mime types for image attachments. + */ + String[] IMAGE_MIME_TYPES = { + "image/jpeg", + "image/png", + "image/gif", + }; + + /** + * The maximum allowed size of image attachments. + */ + int MAX_IMAGE_SIZE = 6 * 1024 * 1024; + } 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 a4f91af5c..d3ba4acf1 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 @@ -41,6 +41,11 @@ public interface MessagingManager extends ConversationClient { AttachmentHeader addLocalAttachment(GroupId groupId, long timestamp, String contentType, InputStream is) throws DbException, IOException; + /** + * Removes an unsent attachment. + */ + void removeAttachment(AttachmentHeader header) throws DbException; + /** * Returns the ID of the contact with the given private conversation. */ 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 44073b377..d46d9d8e0 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 @@ -166,6 +166,11 @@ class MessagingManagerImpl extends ConversationClientImpl return new AttachmentHeader(new MessageId(b), "image/png"); } + @Override + public void removeAttachment(AttachmentHeader header) throws DbException { + // TODO add real implementation + } + private ContactId getContactId(Transaction txn, GroupId g) throws DbException { try { From 6167ba5c46b68ff16bf9396b391f0c3a024cb15f Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 18 Feb 2019 11:49:13 -0300 Subject: [PATCH 02/17] [android] move unsent attachment cache logic into AttachmentController --- .../conversation/AttachmentController.java | 70 +++++++++++++------ .../android/conversation/AttachmentItem.java | 27 ++++--- .../conversation/ConversationViewModel.java | 27 ++----- .../view/TextAttachmentController.java | 4 +- .../briar/messaging/MessagingManagerImpl.java | 2 +- 5 files changed, 72 insertions(+), 58 deletions(-) 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 From f76f9be4edb35473671e85f54cc1db7f647c8a5e Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 18 Feb 2019 15:05:04 -0300 Subject: [PATCH 03/17] Reject attachments that exceed the allowed size Closes #1468 --- .../conversation/AttachmentResult.java | 43 +++++++++++++++++++ .../conversation/ConversationViewModel.java | 20 +++++++-- .../briar/android/view/ImagePreviewItem.java | 2 +- .../view/TextAttachmentController.java | 38 +++++++++++----- briar-android/src/main/res/values/strings.xml | 1 + .../api/messaging/FileTooBigException.java | 6 +++ .../api/messaging/MessagingConstants.java | 3 +- .../briar/api/messaging/MessagingManager.java | 2 + .../briar/messaging/MessagingManagerImpl.java | 40 +++++++++++------ 9 files changed, 125 insertions(+), 30 deletions(-) create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentResult.java create mode 100644 briar-api/src/main/java/org/briarproject/briar/api/messaging/FileTooBigException.java diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentResult.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentResult.java new file mode 100644 index 000000000..926d32fab --- /dev/null +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentResult.java @@ -0,0 +1,43 @@ +package org.briarproject.briar.android.conversation; + +import android.net.Uri; + +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import javax.annotation.Nullable; +import javax.annotation.concurrent.Immutable; + +@Immutable +@NotNullByDefault +public class AttachmentResult { + + @Nullable + private final Uri uri; + @Nullable + private final String errorMsg; + + public AttachmentResult(Uri uri) { + this.uri = uri; + this.errorMsg = null; + } + + public AttachmentResult(@Nullable String errorMsg) { + this.uri = null; + this.errorMsg = errorMsg; + } + + @Nullable + public Uri getUri() { + return uri; + } + + public boolean isError() { + return errorMsg != null; + } + + @Nullable + public String getErrorMsg() { + return errorMsg; + } + +} 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 b428ee901..304933943 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 @@ -27,11 +27,13 @@ import org.briarproject.bramble.api.sync.GroupId; import org.briarproject.bramble.api.sync.Message; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.bramble.api.system.AndroidExecutor; +import org.briarproject.briar.R; import org.briarproject.briar.android.util.UiUtils; import org.briarproject.briar.android.view.TextAttachmentController.AttachmentManager; import org.briarproject.briar.android.viewmodel.LiveEvent; import org.briarproject.briar.android.viewmodel.MutableLiveEvent; import org.briarproject.briar.api.messaging.AttachmentHeader; +import org.briarproject.briar.api.messaging.FileTooBigException; import org.briarproject.briar.api.messaging.MessagingManager; import org.briarproject.briar.api.messaging.PrivateMessage; import org.briarproject.briar.api.messaging.PrivateMessageFactory; @@ -54,6 +56,7 @@ import static org.briarproject.bramble.util.LogUtils.now; import static org.briarproject.briar.android.conversation.AttachmentDimensions.getAttachmentDimensions; import static org.briarproject.briar.android.settings.SettingsFragment.SETTINGS_NAMESPACE; import static org.briarproject.briar.android.util.UiUtils.observeForeverOnce; +import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_IMAGE_SIZE; @NotNullByDefault public class ConversationViewModel extends AndroidViewModel implements @@ -192,9 +195,11 @@ public class ConversationViewModel extends AndroidViewModel implements } @Override - public void storeAttachment(Uri uri, boolean needsSize, Runnable onSuccess, - Runnable onError) { + public LiveData storeAttachment(Uri uri, + boolean needsSize) { if (messagingGroupId.getValue() == null) loadGroupId(); + // use LiveData to not keep references to view scope + MutableLiveData result = new MutableLiveData<>(); observeForeverOnce(messagingGroupId, groupId -> dbExecutor.execute(() -> { if (groupId == null) throw new IllegalStateException(); @@ -204,13 +209,20 @@ public class ConversationViewModel extends AndroidViewModel implements getApplication().getContentResolver(); attachmentController.createAttachmentHeader(contentResolver, groupId, uri, needsSize); - androidExecutor.runOnUiThread(onSuccess); + result.postValue(new AttachmentResult(uri)); + } catch(FileTooBigException e) { + logException(LOG, WARNING, e); + int mb = MAX_IMAGE_SIZE / 1024 / 1024; + String errorMsg = getApplication() + .getString(R.string.image_attach_error_too_big, mb); + result.postValue(new AttachmentResult(errorMsg)); } catch (DbException | IOException e) { logException(LOG, WARNING, e); - androidExecutor.runOnUiThread(onError); + result.postValue(new AttachmentResult((String) null)); } logDuration(LOG, "Storing attachment", start); })); + return result; } @Override diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewItem.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewItem.java index 71576015f..bf1ba71fb 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewItem.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewItem.java @@ -15,7 +15,7 @@ class ImagePreviewItem { private final Uri uri; private boolean waitForLoading = true; - private ImagePreviewItem(Uri uri) { + ImagePreviewItem(Uri uri) { this.uri = uri; } 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 0cc1cb32e..db8723caf 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 @@ -1,6 +1,8 @@ package org.briarproject.briar.android.view; import android.app.Activity; +import android.arch.lifecycle.LifecycleOwner; +import android.arch.lifecycle.LiveData; import android.content.ClipData; import android.content.Context; import android.content.Intent; @@ -15,6 +17,7 @@ import android.widget.Toast; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.briar.R; +import org.briarproject.briar.android.conversation.AttachmentResult; import org.briarproject.briar.android.view.ImagePreview.ImagePreviewListener; import org.briarproject.briar.api.messaging.AttachmentHeader; @@ -149,9 +152,17 @@ public class TextAttachmentController extends TextSendController // store attachments and show preview when successful boolean needsSize = items.size() == 1; for (ImagePreviewItem item : items) { - attachmentManager.storeAttachment(item.getUri(), needsSize, - () -> imagePreview.loadPreviewImage(item), - this::onError); + attachmentManager.storeAttachment(item.getUri(), needsSize) + .observe(imageListener, this::onAttachmentResultReceived); + } + } + + private void onAttachmentResultReceived(AttachmentResult result) { + if (result.isError() || result.getUri() == null) { + onError(result.getErrorMsg()); + } else { + ImagePreviewItem item = new ImagePreviewItem(result.getUri()); + imagePreview.loadPreviewImage(item); } } @@ -194,8 +205,16 @@ public class TextAttachmentController extends TextSendController @Override public void onError() { - Toast.makeText(textInput.getContext(), R.string.image_attach_error, - LENGTH_LONG).show(); + onError(null); + } + + @UiThread + private void onError(@Nullable String errorMsg) { + if (errorMsg == null) { + errorMsg = imagePreview.getContext() + .getString(R.string.image_attach_error); + } + Toast.makeText(textInput.getContext(), errorMsg, LENGTH_LONG).show(); onCancel(); } @@ -268,7 +287,7 @@ public class TextAttachmentController extends TextSendController }; } - public interface AttachImageListener { + public interface AttachImageListener extends LifecycleOwner { void onAttachImage(Intent intent); } @@ -277,11 +296,10 @@ public class TextAttachmentController extends TextSendController * Stores a new attachment in the database. * * @param uri The Uri of the attachment to store. - * @param onSuccess will be run on the UiThread when the attachment was stored successfully. - * @param onError will be run on the UiThread when the attachment could not be stored. + * @param needsSize true if this is the only image in the message + * and therefore needs to know its size. */ - void storeAttachment(Uri uri, boolean needsSize, Runnable onSuccess, - Runnable onError); + LiveData storeAttachment(Uri uri, boolean needsSize); List getAttachmentHeaders(); diff --git a/briar-android/src/main/res/values/strings.xml b/briar-android/src/main/res/values/strings.xml index 4565d2a55..7c397a351 100644 --- a/briar-android/src/main/res/values/strings.xml +++ b/briar-android/src/main/res/values/strings.xml @@ -130,6 +130,7 @@ Add a caption (optional) Attach image Could not attach image(s) + Image too big. Limit is %d MB. Change contact name Contact name Change diff --git a/briar-api/src/main/java/org/briarproject/briar/api/messaging/FileTooBigException.java b/briar-api/src/main/java/org/briarproject/briar/api/messaging/FileTooBigException.java new file mode 100644 index 000000000..f7d9c7c04 --- /dev/null +++ b/briar-api/src/main/java/org/briarproject/briar/api/messaging/FileTooBigException.java @@ -0,0 +1,6 @@ +package org.briarproject.briar.api.messaging; + +import java.io.IOException; + +public class FileTooBigException extends IOException { +} diff --git a/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingConstants.java b/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingConstants.java index c6114dc66..c1be55bdf 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingConstants.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingConstants.java @@ -20,7 +20,8 @@ public interface MessagingConstants { /** * The maximum allowed size of image attachments. + * TODO: Different limit for GIFs? */ - int MAX_IMAGE_SIZE = 6 * 1024 * 1024; + int MAX_IMAGE_SIZE = MAX_MESSAGE_BODY_LENGTH; // 6 * 1024 * 1024; } 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 d3ba4acf1..d069e337b 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 @@ -37,6 +37,8 @@ public interface MessagingManager extends ConversationClient { /** * Stores a local attachment message. + * + * @throws FileTooBigException */ AttachmentHeader addLocalAttachment(GroupId groupId, long timestamp, String contentType, InputStream is) throws DbException, IOException; 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 e73dce953..8917be036 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 @@ -18,14 +18,17 @@ import org.briarproject.bramble.api.sync.Group; import org.briarproject.bramble.api.sync.Group.Visibility; import org.briarproject.bramble.api.sync.GroupId; import org.briarproject.bramble.api.sync.Message; +import org.briarproject.bramble.api.sync.MessageFactory; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.bramble.api.sync.MessageStatus; import org.briarproject.bramble.api.versioning.ClientVersioningManager; import org.briarproject.bramble.api.versioning.ClientVersioningManager.ClientVersioningHook; +import org.briarproject.bramble.util.IoUtils; import org.briarproject.briar.api.client.MessageTracker; import org.briarproject.briar.api.conversation.ConversationMessageHeader; import org.briarproject.briar.api.messaging.Attachment; import org.briarproject.briar.api.messaging.AttachmentHeader; +import org.briarproject.briar.api.messaging.FileTooBigException; import org.briarproject.briar.api.messaging.MessagingManager; import org.briarproject.briar.api.messaging.PrivateMessage; import org.briarproject.briar.api.messaging.PrivateMessageHeader; @@ -33,18 +36,18 @@ import org.briarproject.briar.api.messaging.event.PrivateMessageReceivedEvent; import org.briarproject.briar.client.ConversationClientImpl; import java.io.ByteArrayInputStream; +import java.io.EOFException; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; import java.util.Collection; import java.util.Map; -import java.util.Random; import javax.annotation.concurrent.Immutable; import javax.inject.Inject; import static java.util.Collections.emptyList; -import static org.briarproject.bramble.util.StringUtils.fromHexString; +import static org.briarproject.bramble.api.sync.SyncConstants.MAX_MESSAGE_BODY_LENGTH; import static org.briarproject.briar.client.MessageTrackerConstants.MSG_KEY_READ; @Immutable @@ -55,15 +58,18 @@ class MessagingManagerImpl extends ConversationClientImpl private final ClientVersioningManager clientVersioningManager; private final ContactGroupFactory contactGroupFactory; + private final MessageFactory messageFactory; @Inject MessagingManagerImpl(DatabaseComponent db, ClientHelper clientHelper, ClientVersioningManager clientVersioningManager, MetadataParser metadataParser, MessageTracker messageTracker, - ContactGroupFactory contactGroupFactory) { + ContactGroupFactory contactGroupFactory, + MessageFactory messageFactory) { super(db, clientHelper, metadataParser, messageTracker); this.clientVersioningManager = clientVersioningManager; this.contactGroupFactory = contactGroupFactory; + this.messageFactory = messageFactory; } @Override @@ -158,17 +164,25 @@ class MessagingManagerImpl extends ConversationClientImpl @Override public AttachmentHeader addLocalAttachment(GroupId groupId, long timestamp, - String contentType, InputStream is) throws IOException { + String contentType, InputStream is) + throws DbException, IOException { // TODO add real implementation - if (is.available() == 0) throw new IOException(); - byte[] b = new byte[MessageId.LENGTH]; - new Random().nextBytes(b); - return new AttachmentHeader(new MessageId(b), contentType); + byte[] body = new byte[MAX_MESSAGE_BODY_LENGTH]; + try { + IoUtils.read(is, body); + } catch (EOFException ignored) { + } + if (is.available() > 0) throw new FileTooBigException(); + is.close(); + Message m = messageFactory.createMessage(groupId, timestamp, body); + clientHelper.addLocalMessage(m, new BdfDictionary(), false); + return new AttachmentHeader(m.getId(), contentType); } @Override public void removeAttachment(AttachmentHeader header) throws DbException { - // TODO add real implementation + db.transaction(false, + txn -> db.removeMessage(txn, header.getMessageId())); } private ContactId getContactId(Transaction txn, GroupId g) @@ -247,12 +261,10 @@ class MessagingManagerImpl extends ConversationClientImpl } @Override - public Attachment getAttachment(MessageId m) { + public Attachment getAttachment(MessageId mId) throws DbException { // TODO add real implementation - byte[] bytes = fromHexString("89504E470D0A1A0A0000000D49484452" + - "000000010000000108060000001F15C4" + - "890000000A49444154789C6300010000" + - "0500010D0A2DB40000000049454E44AE426082"); + Message m = clientHelper.getMessage(mId); + byte[] bytes = m.getBody(); return new Attachment(new ByteArrayInputStream(bytes)); } From fc6275b037c45f6a07078f2c28fe9d7f20a16c4f Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 18 Feb 2019 15:24:34 -0300 Subject: [PATCH 04/17] [android] reject invalid mime types for image attachments --- .../conversation/AttachmentController.java | 9 +++++++++ .../conversation/ConversationViewModel.java | 20 ++++++++++++------- briar-android/src/main/res/values/strings.xml | 1 + .../AttachmentControllerTest.java | 17 ---------------- 4 files changed, 23 insertions(+), 24 deletions(-) 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 9b4b256d7..bd52c84db 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 @@ -44,6 +44,7 @@ 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; +import static org.briarproject.briar.api.messaging.MessagingConstants.IMAGE_MIME_TYPES; @NotNullByDefault class AttachmentController { @@ -146,6 +147,14 @@ class AttachmentController { unsentItems.put(uri, item); } + boolean isValidMimeType(@Nullable String mimeType) { + if (mimeType == null) return false; + for (String supportedType : IMAGE_MIME_TYPES) { + if (supportedType.equals(mimeType)) return true; + } + return false; + } + @DatabaseExecutor void deleteUnsentAttachments() { for (AttachmentItem item : unsentItems.values()) { 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 304933943..2ab1992ef 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 @@ -74,7 +74,6 @@ public class ConversationViewModel extends AndroidViewModel implements @CryptoExecutor private final Executor cryptoExecutor; private final TransactionManager db; - private final AndroidExecutor androidExecutor; private final MessagingManager messagingManager; private final ContactManager contactManager; private final SettingsManager settingsManager; @@ -107,13 +106,12 @@ public class ConversationViewModel extends AndroidViewModel implements ConversationViewModel(Application application, @DatabaseExecutor Executor dbExecutor, @CryptoExecutor Executor cryptoExecutor, TransactionManager db, - AndroidExecutor androidExecutor, MessagingManager messagingManager, - ContactManager contactManager, SettingsManager settingsManager, + MessagingManager messagingManager, ContactManager contactManager, + SettingsManager settingsManager, PrivateMessageFactory privateMessageFactory) { super(application); this.dbExecutor = dbExecutor; this.cryptoExecutor = cryptoExecutor; - this.androidExecutor = androidExecutor; this.db = db; this.messagingManager = messagingManager; this.contactManager = contactManager; @@ -197,16 +195,24 @@ public class ConversationViewModel extends AndroidViewModel implements @Override public LiveData storeAttachment(Uri uri, boolean needsSize) { - if (messagingGroupId.getValue() == null) loadGroupId(); // use LiveData to not keep references to view scope MutableLiveData result = new MutableLiveData<>(); + // check first if mime type is supported + ContentResolver contentResolver = + getApplication().getContentResolver(); + String mimeType = contentResolver.getType(uri); + if (!attachmentController.isValidMimeType(mimeType)) { + String errorMsg = getApplication().getString( + R.string.image_attach_error_invalid_mime_type, mimeType); + result.setValue(new AttachmentResult(errorMsg)); + return result; + } + if (messagingGroupId.getValue() == null) loadGroupId(); observeForeverOnce(messagingGroupId, groupId -> dbExecutor.execute(() -> { if (groupId == null) throw new IllegalStateException(); long start = now(); try { - ContentResolver contentResolver = - getApplication().getContentResolver(); attachmentController.createAttachmentHeader(contentResolver, groupId, uri, needsSize); result.postValue(new AttachmentResult(uri)); diff --git a/briar-android/src/main/res/values/strings.xml b/briar-android/src/main/res/values/strings.xml index 7c397a351..a80cae6e3 100644 --- a/briar-android/src/main/res/values/strings.xml +++ b/briar-android/src/main/res/values/strings.xml @@ -131,6 +131,7 @@ Attach image Could not attach image(s) Image too big. Limit is %d MB. + Image format unsupported: %s Change contact name Contact name Change diff --git a/briar-android/src/test/java/org/briarproject/briar/android/conversation/AttachmentControllerTest.java b/briar-android/src/test/java/org/briarproject/briar/android/conversation/AttachmentControllerTest.java index d66b69697..f5abfc706 100644 --- a/briar-android/src/test/java/org/briarproject/briar/android/conversation/AttachmentControllerTest.java +++ b/briar-android/src/test/java/org/briarproject/briar/android/conversation/AttachmentControllerTest.java @@ -94,23 +94,6 @@ public class AttachmentControllerTest extends BrambleMockTestCase { assertFalse(item.hasError()); } - @Test - public void testImageHealsWrongMimeType() { - AttachmentHeader h = getAttachmentHeader("image/png"); - - context.checking(new Expectations() {{ - oneOf(imageHelper).decodeStream(with(any(InputStream.class))); - will(returnValue(new DecodeResult(160, 240, "image/jpeg"))); - oneOf(imageHelper).getExtensionFromMimeType("image/jpeg"); - will(returnValue("jpg")); - }}); - - AttachmentItem item = controller.getAttachmentItem(h, attachment, true); - assertEquals("image/jpeg", item.getMimeType()); - assertEquals("jpg", item.getExtension()); - assertFalse(item.hasError()); - } - @Test public void testBigJpegImage() { String mimeType = "image/jpeg"; From 279692670971cb51988315a9463871d8da691993 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 18 Feb 2019 16:34:22 -0300 Subject: [PATCH 05/17] [android] Load image preview from database instead of content Uri --- .../conversation/AttachmentController.java | 6 ++-- .../conversation/AttachmentResult.java | 11 ++++++- .../conversation/ConversationViewModel.java | 9 +++--- .../briar/android/view/ImagePreview.java | 5 ++-- .../android/view/ImagePreviewAdapter.java | 13 ++++---- .../briar/android/view/ImagePreviewItem.java | 30 +++++++++++-------- .../android/view/ImagePreviewViewHolder.java | 2 +- .../view/TextAttachmentController.java | 3 +- 8 files changed, 49 insertions(+), 30 deletions(-) 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 bd52c84db..f7479beb7 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 @@ -38,6 +38,7 @@ import static android.support.media.ExifInterface.ORIENTATION_TRANSVERSE; import static android.support.media.ExifInterface.TAG_IMAGE_LENGTH; import static android.support.media.ExifInterface.TAG_IMAGE_WIDTH; import static android.support.media.ExifInterface.TAG_ORIENTATION; +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; @@ -122,13 +123,13 @@ class AttachmentController { } @DatabaseExecutor - void createAttachmentHeader(ContentResolver contentResolver, + AttachmentItem 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; + return requireNonNull(unsentItems.get(uri)); } long start = now(); InputStream is = contentResolver.openInputStream(uri); @@ -145,6 +146,7 @@ class AttachmentController { getAttachmentItem(contentResolver, uri, h, needsSize); if (item.hasError()) throw new IOException(); unsentItems.put(uri, item); + return item; } boolean isValidMimeType(@Nullable String mimeType) { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentResult.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentResult.java index 926d32fab..2a84252a2 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentResult.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentResult.java @@ -14,15 +14,19 @@ public class AttachmentResult { @Nullable private final Uri uri; @Nullable + private final AttachmentItem item; + @Nullable private final String errorMsg; - public AttachmentResult(Uri uri) { + public AttachmentResult(Uri uri, AttachmentItem item) { this.uri = uri; + this.item = item; this.errorMsg = null; } public AttachmentResult(@Nullable String errorMsg) { this.uri = null; + this.item = null; this.errorMsg = errorMsg; } @@ -31,6 +35,11 @@ public class AttachmentResult { return uri; } + @Nullable + public AttachmentItem getItem() { + return item; + } + public boolean isError() { return errorMsg != null; } 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 2ab1992ef..663e94e98 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 @@ -213,9 +213,10 @@ public class ConversationViewModel extends AndroidViewModel implements if (groupId == null) throw new IllegalStateException(); long start = now(); try { - attachmentController.createAttachmentHeader(contentResolver, - groupId, uri, needsSize); - result.postValue(new AttachmentResult(uri)); + AttachmentItem item = attachmentController + .createAttachmentHeader(contentResolver, groupId, uri, + needsSize); + result.postValue(new AttachmentResult(uri, item)); } catch(FileTooBigException e) { logException(LOG, WARNING, e); int mb = MAX_IMAGE_SIZE / 1024 / 1024; @@ -224,7 +225,7 @@ public class ConversationViewModel extends AndroidViewModel implements result.postValue(new AttachmentResult(errorMsg)); } catch (DbException | IOException e) { logException(LOG, WARNING, e); - result.postValue(new AttachmentResult((String) null)); + result.postValue(new AttachmentResult(null)); } logDuration(LOG, "Storing attachment", start); })); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java index 78f74a189..9e71c30d3 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java @@ -9,6 +9,7 @@ import android.view.LayoutInflater; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.briar.R; +import org.briarproject.briar.android.conversation.AttachmentResult; import java.util.Collection; @@ -71,10 +72,10 @@ public class ImagePreview extends ConstraintLayout { imageList.setAdapter(adapter); } - void loadPreviewImage(ImagePreviewItem item) { + void loadPreviewImage(AttachmentResult result) { ImagePreviewAdapter adapter = ((ImagePreviewAdapter) imageList.getAdapter()); - requireNonNull(adapter).loadItemPreview(item); + requireNonNull(adapter).loadItemPreview(result); } interface ImagePreviewListener { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java index 70735fdd6..a5587ccaf 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java @@ -8,6 +8,7 @@ import android.view.ViewGroup; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.briar.R; +import org.briarproject.briar.android.conversation.AttachmentResult; import org.briarproject.briar.android.view.ImagePreview.ImagePreviewListener; import java.util.ArrayList; @@ -53,12 +54,14 @@ class ImagePreviewAdapter extends Adapter { return items.size(); } - void loadItemPreview(ImagePreviewItem item) { - int pos = items.indexOf(item); + void loadItemPreview(AttachmentResult result) { + ImagePreviewItem newItem = + new ImagePreviewItem(requireNonNull(result.getUri())); + int pos = items.indexOf(newItem); if (pos == NO_POSITION) throw new AssertionError(); - ImagePreviewItem newItem = items.get(pos); - newItem.setWaitForLoading(false); - notifyItemChanged(pos, newItem); + ImagePreviewItem item = items.get(pos); + item.setItem(requireNonNull(result.getItem())); + notifyItemChanged(pos, item); } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewItem.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewItem.java index bf1ba71fb..e9c1d0585 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewItem.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewItem.java @@ -4,6 +4,7 @@ import android.net.Uri; import android.support.annotation.Nullable; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.briar.android.conversation.AttachmentItem; import java.util.ArrayList; import java.util.Collection; @@ -13,22 +14,12 @@ import java.util.List; class ImagePreviewItem { private final Uri uri; - private boolean waitForLoading = true; + @Nullable + private AttachmentItem item; ImagePreviewItem(Uri uri) { this.uri = uri; - } - - Uri getUri() { - return uri; - } - - void setWaitForLoading(boolean waitForLoading) { - this.waitForLoading = waitForLoading; - } - - boolean waitForLoading() { - return waitForLoading; + this.item = null; } static List fromUris(Collection uris) { @@ -39,6 +30,19 @@ class ImagePreviewItem { return items; } + Uri getUri() { + return uri; + } + + public void setItem(AttachmentItem item) { + this.item = item; + } + + @Nullable + public AttachmentItem getItem() { + return item; + } + @Override public boolean equals(@Nullable Object o) { return o instanceof ImagePreviewItem && diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java index a18ae3c17..b7b9c8509 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java @@ -42,7 +42,7 @@ class ImagePreviewViewHolder extends ViewHolder { } void bind(ImagePreviewItem item) { - if (item.waitForLoading()) return; + if (item.getItem() == null) return; GlideApp.with(imageView) .load(item.getUri()) .diskCacheStrategy(NONE) 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 db8723caf..0b6e30ef5 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 @@ -161,8 +161,7 @@ public class TextAttachmentController extends TextSendController if (result.isError() || result.getUri() == null) { onError(result.getErrorMsg()); } else { - ImagePreviewItem item = new ImagePreviewItem(result.getUri()); - imagePreview.loadPreviewImage(item); + imagePreview.loadPreviewImage(result); } } From baedb14e2bbfd64e6e7d25666e5a4b12bde8d593 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 22 Feb 2019 12:09:50 -0300 Subject: [PATCH 06/17] [android] allow attaching only of images with supported mime type --- .../briar/android/view/TextAttachmentController.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) 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 0b6e30ef5..8c05f0b97 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 @@ -31,6 +31,7 @@ import static android.content.Intent.ACTION_GET_CONTENT; import static android.content.Intent.ACTION_OPEN_DOCUMENT; import static android.content.Intent.CATEGORY_OPENABLE; import static android.content.Intent.EXTRA_ALLOW_MULTIPLE; +import static android.content.Intent.EXTRA_MIME_TYPES; import static android.os.Build.VERSION.SDK_INT; import static android.support.v4.content.ContextCompat.getColor; import static android.support.v4.view.AbsSavedState.EMPTY_STATE; @@ -39,6 +40,7 @@ import static android.widget.Toast.LENGTH_LONG; import static java.util.Collections.emptyList; import static java.util.Objects.requireNonNull; import static org.briarproject.briar.android.util.UiUtils.resolveColorAttribute; +import static org.briarproject.briar.api.messaging.MessagingConstants.IMAGE_MIME_TYPES; import static uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt.STATE_DISMISSED; import static uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt.STATE_FINISHED; @@ -118,12 +120,18 @@ public class TextAttachmentController extends TextSendController builder.show(); return; } + Intent intent = getAttachFileIntent(); + requireNonNull(imageListener).onAttachImage(intent); + } + + private Intent getAttachFileIntent() { Intent intent = new Intent(SDK_INT >= 19 ? ACTION_OPEN_DOCUMENT : ACTION_GET_CONTENT); - intent.addCategory(CATEGORY_OPENABLE); intent.setType("image/*"); + intent.addCategory(CATEGORY_OPENABLE); + if (SDK_INT >= 19) intent.putExtra(EXTRA_MIME_TYPES, IMAGE_MIME_TYPES); if (SDK_INT >= 18) intent.putExtra(EXTRA_ALLOW_MULTIPLE, true); - requireNonNull(imageListener).onAttachImage(intent); + return intent; } public void onImageReceived(@Nullable Intent resultData) { From 70d29af2bacf83dc9e56b14df5b658c9cfd795af Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Thu, 28 Mar 2019 15:05:48 -0300 Subject: [PATCH 07/17] [android] Allow sending message with attachments before previews are loaded --- .../briar/android/view/ImagePreview.java | 12 +---- .../android/view/ImagePreviewAdapter.java | 8 +--- .../android/view/ImagePreviewViewHolder.java | 8 +--- .../view/TextAttachmentController.java | 44 ++++++++----------- 4 files changed, 23 insertions(+), 49 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java index 9e71c30d3..5c66d87b2 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java @@ -68,7 +68,7 @@ public class ImagePreview extends ConstraintLayout { imageList.setLayoutParams(params); } setVisibility(VISIBLE); - ImagePreviewAdapter adapter = new ImagePreviewAdapter(items, listener); + ImagePreviewAdapter adapter = new ImagePreviewAdapter(items); imageList.setAdapter(adapter); } @@ -79,16 +79,6 @@ public class ImagePreview extends ConstraintLayout { } interface ImagePreviewListener { - - void onPreviewLoaded(); - - /** - * Called when Glide can't load a preview image. - * - * Warning: Glide may call this multiple times. - */ - void onError(); - void onCancel(); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java index a5587ccaf..0982e4523 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java @@ -9,7 +9,6 @@ import android.view.ViewGroup; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.briar.R; import org.briarproject.briar.android.conversation.AttachmentResult; -import org.briarproject.briar.android.view.ImagePreview.ImagePreviewListener; import java.util.ArrayList; import java.util.Collection; @@ -22,14 +21,11 @@ import static java.util.Objects.requireNonNull; class ImagePreviewAdapter extends Adapter { private final List items; - private final ImagePreviewListener listener; @LayoutRes private final int layout; - ImagePreviewAdapter(Collection items, - ImagePreviewListener listener) { + ImagePreviewAdapter(Collection items) { this.items = new ArrayList<>(items); - this.listener = listener; this.layout = items.size() == 1 ? R.layout.list_item_image_preview_single : R.layout.list_item_image_preview; @@ -40,7 +36,7 @@ class ImagePreviewAdapter extends Adapter { int type) { View v = LayoutInflater.from(viewGroup.getContext()) .inflate(layout, viewGroup, false); - return new ImagePreviewViewHolder(v, requireNonNull(listener)); + return new ImagePreviewViewHolder(v); } @Override diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java index b7b9c8509..f3c400238 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java @@ -16,7 +16,6 @@ import com.bumptech.glide.request.target.Target; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.briar.R; import org.briarproject.briar.android.conversation.glide.GlideApp; -import org.briarproject.briar.android.view.ImagePreview.ImagePreviewListener; import static android.view.View.INVISIBLE; import static com.bumptech.glide.load.engine.DiskCacheStrategy.NONE; @@ -29,14 +28,11 @@ class ImagePreviewViewHolder extends ViewHolder { @DrawableRes private static final int ERROR_RES = R.drawable.ic_image_broken; - private final ImagePreviewListener listener; - private final ImageView imageView; private final ProgressBar progressBar; - ImagePreviewViewHolder(View v, ImagePreviewListener listener) { + ImagePreviewViewHolder(View v) { super(v); - this.listener = listener; this.imageView = v.findViewById(R.id.imageView); this.progressBar = v.findViewById(R.id.progressBar); } @@ -55,7 +51,6 @@ class ImagePreviewViewHolder extends ViewHolder { Object model, Target target, boolean isFirstResource) { progressBar.setVisibility(INVISIBLE); - listener.onError(); return false; } @@ -64,7 +59,6 @@ class ImagePreviewViewHolder extends ViewHolder { Object model, Target target, DataSource dataSource, boolean isFirstResource) { progressBar.setVisibility(INVISIBLE); - listener.onPreviewLoaded(); return false; } }) 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 8c05f0b97..056cda1f9 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 @@ -27,6 +27,7 @@ import java.util.List; import uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt; import uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt.PromptStateChangeListener; +import static android.arch.lifecycle.Lifecycle.State.DESTROYED; import static android.content.Intent.ACTION_GET_CONTENT; import static android.content.Intent.ACTION_OPEN_DOCUMENT; import static android.content.Intent.CATEGORY_OPENABLE; @@ -56,8 +57,8 @@ public class TextAttachmentController extends TextSendController private CharSequence textHint; private List imageUris = emptyList(); - private int previewsLoaded = 0; - private boolean loadingPreviews = false; + private int urisLoaded = 0; + private boolean loadingUris = false; public TextAttachmentController(TextInputView v, ImagePreview imagePreview, SendListener listener, AttachImageListener imageListener, @@ -76,10 +77,10 @@ public class TextAttachmentController extends TextSendController @Override protected void updateViewState() { - textInput.setEnabled(ready && !loadingPreviews); - boolean sendEnabled = ready && !loadingPreviews && + textInput.setEnabled(ready && !loadingUris); + boolean sendEnabled = ready && !loadingUris && (!textIsEmpty || canSendEmptyText()); - if (loadingPreviews) { + if (loadingUris) { sendButton.showProgress(true); } else if (imageUris.isEmpty()) { sendButton.showProgress(false); @@ -121,7 +122,9 @@ public class TextAttachmentController extends TextSendController return; } Intent intent = getAttachFileIntent(); - requireNonNull(imageListener).onAttachImage(intent); + if (imageListener.getLifecycle().getCurrentState() != DESTROYED) { + requireNonNull(imageListener).onAttachImage(intent); + } } private Intent getAttachFileIntent() { @@ -152,7 +155,7 @@ public class TextAttachmentController extends TextSendController private void onNewUris() { if (imageUris.isEmpty()) return; - loadingPreviews = true; + loadingUris = true; updateViewState(); textInput.setHint(R.string.image_caption_hint); List items = ImagePreviewItem.fromUris(imageUris); @@ -170,6 +173,8 @@ public class TextAttachmentController extends TextSendController onError(result.getErrorMsg()); } else { imagePreview.loadPreviewImage(result); + urisLoaded++; + checkAllUrisLoaded(); } } @@ -180,9 +185,9 @@ public class TextAttachmentController extends TextSendController imagePreview.setVisibility(GONE); // reset image URIs imageUris = emptyList(); - // no preview has been loaded - previewsLoaded = 0; - loadingPreviews = false; + // no URIs has been loaded + urisLoaded = 0; + loadingUris = false; // show the image button again, so images can get attached updateViewState(); } @@ -204,17 +209,6 @@ public class TextAttachmentController extends TextSendController return state.getSuperState(); } - @Override - public void onPreviewLoaded() { - previewsLoaded++; - checkAllPreviewsLoaded(); - } - - @Override - public void onError() { - onError(null); - } - @UiThread private void onError(@Nullable String errorMsg) { if (errorMsg == null) { @@ -232,10 +226,10 @@ public class TextAttachmentController extends TextSendController reset(); } - private void checkAllPreviewsLoaded() { - if (previewsLoaded == imageUris.size()) { - loadingPreviews = false; - // all previews were loaded + private void checkAllUrisLoaded() { + if (urisLoaded == imageUris.size()) { + loadingUris = false; + // all images were turned into attachments updateViewState(); } } From bb5a6c02415d70b356c4dd8a8a785798cd834445 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 29 Mar 2019 15:31:10 -0300 Subject: [PATCH 08/17] [android] Add assertions to TextAttachmentController --- .../briar/android/view/TextAttachmentController.java | 5 +++++ 1 file changed, 5 insertions(+) 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 056cda1f9..9774463b2 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 @@ -94,6 +94,7 @@ public class TextAttachmentController extends TextSendController @Override public void onSendEvent() { if (canSend()) { + if (loadingUris) throw new AssertionError(); listener.onSendClick(textInput.getText(), attachmentManager.getAttachmentHeaders()); reset(); @@ -139,6 +140,7 @@ public class TextAttachmentController extends TextSendController public void onImageReceived(@Nullable Intent resultData) { if (resultData == null) return; + if (loadingUris) throw new AssertionError(); if (resultData.getData() != null) { imageUris = new ArrayList<>(1); imageUris.add(resultData.getData()); @@ -155,6 +157,7 @@ public class TextAttachmentController extends TextSendController private void onNewUris() { if (imageUris.isEmpty()) return; + if (loadingUris || urisLoaded != 0) throw new AssertionError(); loadingUris = true; updateViewState(); textInput.setHint(R.string.image_caption_hint); @@ -169,6 +172,7 @@ public class TextAttachmentController extends TextSendController } private void onAttachmentResultReceived(AttachmentResult result) { + if (!loadingUris) return; // if this is false, the user cancelled if (result.isError() || result.getUri() == null) { onError(result.getErrorMsg()); } else { @@ -227,6 +231,7 @@ public class TextAttachmentController extends TextSendController } private void checkAllUrisLoaded() { + if (!loadingUris) throw new AssertionError(); if (urisLoaded == imageUris.size()) { loadingUris = false; // all images were turned into attachments From 11eefaedcf0b3d25afb309defb3a6ab2da577dda Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 5 Apr 2019 15:48:31 -0300 Subject: [PATCH 09/17] Refactor attachment creation --- .../AttachmentRetrieverIntegrationTest.java} | 36 +-- .../attachment/AttachmentCreationTask.java | 114 ++++++++++ .../android/attachment/AttachmentCreator.java | 206 ++++++++++++++++++ .../AttachmentDimensions.java | 13 +- .../AttachmentItem.java | 20 +- .../AttachmentItemResult.java} | 8 +- .../android/attachment/AttachmentManager.java | 20 ++ .../android/attachment/AttachmentResult.java | 33 +++ .../AttachmentRetriever.java} | 108 ++------- .../ImageHelper.java | 2 +- .../conversation/ConversationActivity.java | 22 +- .../conversation/ConversationListener.java | 1 + .../conversation/ConversationMessageItem.java | 1 + .../ConversationMessageViewHolder.java | 1 + .../conversation/ConversationViewModel.java | 117 ++++------ .../conversation/ConversationVisitor.java | 1 + .../android/conversation/ImageActivity.java | 1 + .../android/conversation/ImageAdapter.java | 1 + .../android/conversation/ImageFragment.java | 1 + .../android/conversation/ImageViewHolder.java | 1 + .../android/conversation/ImageViewModel.java | 1 + .../conversation/glide/BriarDataFetcher.java | 2 +- .../glide/BriarDataFetcherFactory.java | 2 +- .../conversation/glide/BriarGlideModule.java | 2 +- .../conversation/glide/BriarModelLoader.java | 2 +- .../glide/BriarModelLoaderFactory.java | 2 +- .../briar/android/view/ImagePreview.java | 4 +- .../android/view/ImagePreviewAdapter.java | 4 +- .../briar/android/view/ImagePreviewItem.java | 6 +- .../android/view/ImagePreviewViewHolder.java | 4 +- .../view/TextAttachmentController.java | 93 ++++---- .../AttachmentRetrieverTest.java} | 10 +- .../MarkEnforcingInputStreamTest.java | 2 +- .../briar/messaging/MessagingManagerImpl.java | 4 + 34 files changed, 557 insertions(+), 288 deletions(-) rename briar-android/src/androidTestOfficial/java/org/briarproject/briar/android/{conversation/AttachmentControllerIntegrationTest.java => attachment/AttachmentRetrieverIntegrationTest.java} (89%) create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java rename briar-android/src/main/java/org/briarproject/briar/android/{conversation => attachment}/AttachmentDimensions.java (75%) rename briar-android/src/main/java/org/briarproject/briar/android/{conversation => attachment}/AttachmentItem.java (87%) rename briar-android/src/main/java/org/briarproject/briar/android/{conversation/AttachmentResult.java => attachment/AttachmentItemResult.java} (77%) create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentResult.java rename briar-android/src/main/java/org/briarproject/briar/android/{conversation/AttachmentController.java => attachment/AttachmentRetriever.java} (71%) rename briar-android/src/main/java/org/briarproject/briar/android/{conversation => attachment}/ImageHelper.java (90%) rename briar-android/src/test/java/org/briarproject/briar/android/{conversation/AttachmentControllerTest.java => attachment/AttachmentRetrieverTest.java} (93%) rename briar-android/src/test/java/org/briarproject/briar/android/{conversation => attachment}/MarkEnforcingInputStreamTest.java (97%) diff --git a/briar-android/src/androidTestOfficial/java/org/briarproject/briar/android/conversation/AttachmentControllerIntegrationTest.java b/briar-android/src/androidTestOfficial/java/org/briarproject/briar/android/attachment/AttachmentRetrieverIntegrationTest.java similarity index 89% rename from briar-android/src/androidTestOfficial/java/org/briarproject/briar/android/conversation/AttachmentControllerIntegrationTest.java rename to briar-android/src/androidTestOfficial/java/org/briarproject/briar/android/attachment/AttachmentRetrieverIntegrationTest.java index d639230b8..eea2427f3 100644 --- a/briar-android/src/androidTestOfficial/java/org/briarproject/briar/android/conversation/AttachmentControllerIntegrationTest.java +++ b/briar-android/src/androidTestOfficial/java/org/briarproject/briar/android/attachment/AttachmentRetrieverIntegrationTest.java @@ -1,4 +1,4 @@ -package org.briarproject.briar.android.conversation; +package org.briarproject.briar.android.attachment; import android.content.res.AssetManager; import android.support.test.InstrumentationRegistry; @@ -21,7 +21,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @RunWith(AndroidJUnit4.class) -public class AttachmentControllerIntegrationTest { +public class AttachmentRetrieverIntegrationTest { private static final String smallKitten = "https://upload.wikimedia.org/wikipedia/commons/thumb/0/06/Kitten_in_Rizal_Park%2C_Manila.jpg/160px-Kitten_in_Rizal_Park%2C_Manila.jpg"; @@ -47,15 +47,15 @@ public class AttachmentControllerIntegrationTest { ); private final MessageId msgId = new MessageId(getRandomId()); - private final AttachmentController controller = - new AttachmentController(null, dimensions); + private final AttachmentRetriever retriever = + new AttachmentRetriever(null, dimensions); @Test public void testSmallJpegImage() throws Exception { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getUrlInputStream(smallKitten); Attachment a = new Attachment(is); - AttachmentItem item = controller.getAttachmentItem(h, a, true); + AttachmentItem item = retriever.getAttachmentItem(h, a, true); assertEquals(msgId, item.getMessageId()); assertEquals(160, item.getWidth()); assertEquals(240, item.getHeight()); @@ -71,7 +71,7 @@ public class AttachmentControllerIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getUrlInputStream(originalKitten); Attachment a = new Attachment(is); - AttachmentItem item = controller.getAttachmentItem(h, a, true); + AttachmentItem item = retriever.getAttachmentItem(h, a, true); assertEquals(msgId, item.getMessageId()); assertEquals(1728, item.getWidth()); assertEquals(2592, item.getHeight()); @@ -87,7 +87,7 @@ public class AttachmentControllerIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/png"); InputStream is = getUrlInputStream(pngKitten); Attachment a = new Attachment(is); - AttachmentItem item = controller.getAttachmentItem(h, a, true); + AttachmentItem item = retriever.getAttachmentItem(h, a, true); assertEquals(msgId, item.getMessageId()); assertEquals(737, item.getWidth()); assertEquals(510, item.getHeight()); @@ -103,7 +103,7 @@ public class AttachmentControllerIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getUrlInputStream(uberGif); Attachment a = new Attachment(is); - AttachmentItem item = controller.getAttachmentItem(h, a, true); + AttachmentItem item = retriever.getAttachmentItem(h, a, true); assertEquals(1, item.getWidth()); assertEquals(1, item.getHeight()); assertEquals(dimensions.minHeight, item.getThumbnailWidth()); @@ -118,7 +118,7 @@ public class AttachmentControllerIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getUrlInputStream(lottaPixel); Attachment a = new Attachment(is); - AttachmentItem item = controller.getAttachmentItem(h, a, true); + AttachmentItem item = retriever.getAttachmentItem(h, a, true); assertEquals(64250, item.getWidth()); assertEquals(64250, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); @@ -133,7 +133,7 @@ public class AttachmentControllerIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getUrlInputStream(imageIoCrash); Attachment a = new Attachment(is); - AttachmentItem item = controller.getAttachmentItem(h, a, true); + AttachmentItem item = retriever.getAttachmentItem(h, a, true); assertEquals(1184, item.getWidth()); assertEquals(448, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); @@ -148,7 +148,7 @@ public class AttachmentControllerIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getUrlInputStream(gimpCrash); Attachment a = new Attachment(is); - AttachmentItem item = controller.getAttachmentItem(h, a, true); + AttachmentItem item = retriever.getAttachmentItem(h, a, true); assertEquals(1, item.getWidth()); assertEquals(1, item.getHeight()); assertEquals(dimensions.minHeight, item.getThumbnailWidth()); @@ -163,7 +163,7 @@ public class AttachmentControllerIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getUrlInputStream(optiPngAfl); Attachment a = new Attachment(is); - AttachmentItem item = controller.getAttachmentItem(h, a, true); + AttachmentItem item = retriever.getAttachmentItem(h, a, true); assertEquals(32, item.getWidth()); assertEquals(32, item.getHeight()); assertEquals(dimensions.minHeight, item.getThumbnailWidth()); @@ -178,7 +178,7 @@ public class AttachmentControllerIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getUrlInputStream(librawError); Attachment a = new Attachment(is); - AttachmentItem item = controller.getAttachmentItem(h, a, true); + AttachmentItem item = retriever.getAttachmentItem(h, a, true); assertTrue(item.hasError()); } @@ -187,7 +187,7 @@ public class AttachmentControllerIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/gif"); InputStream is = getAssetInputStream("animated.gif"); Attachment a = new Attachment(is); - AttachmentItem item = controller.getAttachmentItem(h, a, true); + AttachmentItem item = retriever.getAttachmentItem(h, a, true); assertEquals(65535, item.getWidth()); assertEquals(65535, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); @@ -202,7 +202,7 @@ public class AttachmentControllerIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/gif"); InputStream is = getAssetInputStream("animated2.gif"); Attachment a = new Attachment(is); - AttachmentItem item = controller.getAttachmentItem(h, a, true); + AttachmentItem item = retriever.getAttachmentItem(h, a, true); assertEquals(10000, item.getWidth()); assertEquals(10000, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); @@ -217,7 +217,7 @@ public class AttachmentControllerIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/gif"); InputStream is = getAssetInputStream("error_large.gif"); Attachment a = new Attachment(is); - AttachmentItem item = controller.getAttachmentItem(h, a, true); + AttachmentItem item = retriever.getAttachmentItem(h, a, true); assertEquals(16384, item.getWidth()); assertEquals(16384, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); @@ -232,7 +232,7 @@ public class AttachmentControllerIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getAssetInputStream("error_high.jpg"); Attachment a = new Attachment(is); - AttachmentItem item = controller.getAttachmentItem(h, a, true); + AttachmentItem item = retriever.getAttachmentItem(h, a, true); assertEquals(1, item.getWidth()); assertEquals(10000, item.getHeight()); assertEquals(dimensions.minWidth, item.getThumbnailWidth()); @@ -247,7 +247,7 @@ public class AttachmentControllerIntegrationTest { AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg"); InputStream is = getAssetInputStream("error_wide.jpg"); Attachment a = new Attachment(is); - AttachmentItem item = controller.getAttachmentItem(h, a, true); + AttachmentItem item = retriever.getAttachmentItem(h, a, true); assertEquals(1920, item.getWidth()); assertEquals(1, item.getHeight()); assertEquals(dimensions.maxWidth, item.getThumbnailWidth()); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java new file mode 100644 index 000000000..3e4e64767 --- /dev/null +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java @@ -0,0 +1,114 @@ +package org.briarproject.briar.android.attachment; + +import android.content.ContentResolver; +import android.net.Uri; +import android.support.annotation.Nullable; + +import org.briarproject.bramble.api.db.DbException; +import org.briarproject.bramble.api.lifecycle.IoExecutor; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.sync.GroupId; +import org.briarproject.briar.api.messaging.AttachmentHeader; +import org.briarproject.briar.api.messaging.MessagingManager; +import org.jsoup.UnsupportedMimeTypeException; + +import java.io.IOException; +import java.io.InputStream; +import java.util.List; +import java.util.logging.Logger; + +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; +import static org.briarproject.briar.api.messaging.MessagingConstants.IMAGE_MIME_TYPES; + +@NotNullByDefault +class AttachmentCreationTask { + + private static Logger LOG = + getLogger(AttachmentCreationTask.class.getName()); + + private final MessagingManager messagingManager; + private final ContentResolver contentResolver; + private final GroupId groupId; + private final List uris; + private final boolean needsSize; + @Nullable + private AttachmentCreator attachmentCreator; + + private volatile boolean canceled = false; + + AttachmentCreationTask(MessagingManager messagingManager, + ContentResolver contentResolver, + AttachmentCreator attachmentCreator, GroupId groupId, + List uris, boolean needsSize) { + this.messagingManager = messagingManager; + this.contentResolver = contentResolver; + this.groupId = groupId; + this.uris = uris; + this.needsSize = needsSize; + this.attachmentCreator = attachmentCreator; + } + + public void cancel() { + canceled = true; + attachmentCreator = null; + } + + @IoExecutor + public void storeAttachments() { + for (Uri uri: uris) processUri(uri); + if (!canceled && attachmentCreator != null) + attachmentCreator.onAttachmentCreationFinished(); + attachmentCreator = null; + } + + @IoExecutor + private void processUri(Uri uri) { + if (canceled) return; + try { + AttachmentHeader h = storeAttachment(uri); + if (attachmentCreator != null) { + attachmentCreator + .onAttachmentHeaderReceived(uri, h, needsSize); + } + } catch (DbException | IOException e) { + logException(LOG, WARNING, e); + if (attachmentCreator != null) { + attachmentCreator.onAttachmentError(uri, e); + canceled = true; + } + } + } + + @IoExecutor + private AttachmentHeader storeAttachment(Uri uri) + throws IOException, DbException { + long start = now(); + String contentType = contentResolver.getType(uri); + if (contentType == null) throw new IOException("null content type"); + if (!isValidMimeType(contentType)) + throw new UnsupportedMimeTypeException("", contentType, + uri.toString()); + InputStream is = contentResolver.openInputStream(uri); + if (is == null) throw new IOException(); + long timestamp = System.currentTimeMillis(); + AttachmentHeader h = messagingManager + .addLocalAttachment(groupId, timestamp, contentType, is); + tryToClose(is, LOG, WARNING); + logDuration(LOG, "Storing attachment", start); + return h; + } + + private boolean isValidMimeType(@Nullable String mimeType) { + if (mimeType == null) return false; + for (String supportedType : IMAGE_MIME_TYPES) { + if (supportedType.equals(mimeType)) return true; + } + return false; + } + +} diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java new file mode 100644 index 000000000..32f12c2fc --- /dev/null +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java @@ -0,0 +1,206 @@ +package org.briarproject.briar.android.attachment; + + +import android.app.Application; +import android.arch.lifecycle.LiveData; +import android.arch.lifecycle.MutableLiveData; +import android.net.Uri; +import android.support.annotation.Nullable; +import android.support.annotation.UiThread; + +import org.briarproject.bramble.api.db.DbException; +import org.briarproject.bramble.api.lifecycle.IoExecutor; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.sync.GroupId; +import org.briarproject.bramble.api.sync.MessageId; +import org.briarproject.briar.R; +import org.briarproject.briar.api.messaging.Attachment; +import org.briarproject.briar.api.messaging.AttachmentHeader; +import org.briarproject.briar.api.messaging.FileTooBigException; +import org.briarproject.briar.api.messaging.MessagingManager; +import org.jsoup.UnsupportedMimeTypeException; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executor; +import java.util.logging.Logger; + +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.LogUtils.logException; +import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_IMAGE_SIZE; + +@NotNullByDefault +public class AttachmentCreator { + + private static Logger LOG = getLogger(AttachmentCreator.class.getName()); + + private final Application app; + @IoExecutor + private final Executor ioExecutor; + private final MessagingManager messagingManager; + private final AttachmentRetriever controller; + + private final Map unsentItems = + new ConcurrentHashMap<>(); + private final Map> + liveDataResult = new ConcurrentHashMap<>(); + + @Nullable + private MutableLiveData liveDataFinished = null; + @Nullable + private AttachmentCreationTask task; + + public AttachmentCreator(Application app, @IoExecutor Executor ioExecutor, + MessagingManager messagingManager, + AttachmentRetriever controller) { + this.app = app; + this.ioExecutor = ioExecutor; + this.messagingManager = messagingManager; + this.controller = controller; + } + + @UiThread + public AttachmentResult storeAttachments(GroupId groupId, + Collection uris) { + if (task != null && !isStoring()) throw new AssertionError(); + List> itemResults = new ArrayList<>(); + List urisToStore = new ArrayList<>(); + for (Uri uri : uris) { + MutableLiveData liveData = + new MutableLiveData<>(); + itemResults.add(liveData); + liveDataResult.put(uri, liveData); + if (unsentItems.containsKey(uri)) { + // This can happen due to configuration changes. + // So don't create a new attachment, if we have one already. + AttachmentItem item = requireNonNull(unsentItems.get(uri)); + AttachmentItemResult result = + new AttachmentItemResult(uri, item); + liveData.setValue(result); + } else { + urisToStore.add(uri); + } + } + boolean needsSize = uris.size() == 1; + task = new AttachmentCreationTask(messagingManager, + app.getContentResolver(), this, groupId, urisToStore, + needsSize); + ioExecutor.execute(() -> task.storeAttachments()); + liveDataFinished = new MutableLiveData<>(); + return new AttachmentResult(itemResults, liveDataFinished); + } + + @IoExecutor + void onAttachmentHeaderReceived(Uri uri, AttachmentHeader h, + boolean needsSize) { + // get and cache AttachmentItem for ImagePreview + try { + Attachment a = controller.getMessageAttachment(h); + AttachmentItem item = controller.getAttachmentItem(h, a, needsSize); + if (item.hasError()) throw new IOException(); + unsentItems.put(uri, item); + MutableLiveData result = + liveDataResult.get(uri); + if (result != null) { // might have been cleared on UiThread + result.postValue(new AttachmentItemResult(uri, item)); + } + } catch (IOException | DbException e) { + logException(LOG, WARNING, e); + onAttachmentError(uri, e); + } + } + + @IoExecutor + void onAttachmentError(Uri uri, Throwable t) { + String errorMsg; + if (t instanceof UnsupportedMimeTypeException) { + String mimeType = ((UnsupportedMimeTypeException) t).getMimeType(); + errorMsg = app.getString( + R.string.image_attach_error_invalid_mime_type, mimeType); + } else if (t instanceof FileTooBigException) { + int mb = MAX_IMAGE_SIZE / 1024 / 1024; + errorMsg = app.getString(R.string.image_attach_error_too_big, mb); + } else { + errorMsg = null; // generic error + } + MutableLiveData result = liveDataResult.get(uri); + if (result != null) + result.postValue(new AttachmentItemResult(errorMsg)); + // expect to receive a cancel from the UI + } + + @IoExecutor + void onAttachmentCreationFinished() { + if (liveDataFinished != null) liveDataFinished.postValue(true); + } + + @UiThread + public List getAttachmentHeadersForSending() { + List headers = + new ArrayList<>(unsentItems.values().size()); + for (AttachmentItem item : unsentItems.values()) { + headers.add(item.getHeader()); + } + return headers; + } + + /** + * Marks the attachments as sent and adds the items to the cache for display + * + * @param id The MessageId of the sent message. + */ + public void onAttachmentsSent(MessageId id) { + controller.cachePut(id, new ArrayList<>(unsentItems.values())); + resetState(); + } + + @UiThread + public void cancel() { + if (task == null) throw new AssertionError(); + task.cancel(); + // let observers know that they can remove themselves + for (MutableLiveData liveData : liveDataResult + .values()) { + if (liveData.getValue() == null) { + liveData.setValue(null); + } + } + if (liveDataFinished != null) liveDataFinished.setValue(false); + deleteUnsentAttachments(); + resetState(); + } + + @UiThread + private void resetState() { + task = null; + liveDataResult.clear(); + liveDataFinished = null; + unsentItems.clear(); + } + + @UiThread + public void deleteUnsentAttachments() { + List itemsToDelete = + new ArrayList<>(unsentItems.values()); + ioExecutor.execute(() -> { + for (AttachmentItem item : itemsToDelete) { + try { + messagingManager.removeAttachment(item.getHeader()); + } catch (DbException e) { + logException(LOG, WARNING, e); + } + } + }); + } + + private boolean isStoring() { + return liveDataFinished != null; + } + +} diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentDimensions.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentDimensions.java similarity index 75% rename from briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentDimensions.java rename to briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentDimensions.java index 081a6480b..9c60101ee 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentDimensions.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentDimensions.java @@ -1,11 +1,16 @@ -package org.briarproject.briar.android.conversation; +package org.briarproject.briar.android.attachment; import android.content.res.Resources; import android.support.annotation.VisibleForTesting; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.briar.R; -class AttachmentDimensions { +import javax.annotation.concurrent.Immutable; + +@Immutable +@NotNullByDefault +public class AttachmentDimensions { final int defaultSize; final int minWidth, maxWidth; @@ -21,7 +26,7 @@ class AttachmentDimensions { this.maxHeight = maxHeight; } - static AttachmentDimensions getAttachmentDimensions(Resources res) { + public static AttachmentDimensions getAttachmentDimensions(Resources res) { int defaultSize = res.getDimensionPixelSize(R.dimen.message_bubble_image_default); int minWidth = res.getDimensionPixelSize( @@ -33,7 +38,7 @@ class AttachmentDimensions { int maxHeight = res.getDimensionPixelSize( R.dimen.message_bubble_image_max_height); return new AttachmentDimensions(defaultSize, minWidth, maxWidth, - minHeight, minHeight); + minHeight, maxHeight); } } 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/attachment/AttachmentItem.java similarity index 87% rename from briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentItem.java rename to briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItem.java index 598336daa..8caafac7c 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentItem.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItem.java @@ -1,4 +1,4 @@ -package org.briarproject.briar.android.conversation; +package org.briarproject.briar.android.attachment; import android.os.Parcel; import android.os.Parcelable; @@ -12,6 +12,8 @@ import java.util.concurrent.atomic.AtomicLong; import javax.annotation.concurrent.Immutable; +import static java.util.Objects.requireNonNull; + @Immutable @NotNullByDefault public class AttachmentItem implements Parcelable { @@ -57,8 +59,8 @@ public class AttachmentItem implements Parcelable { MessageId messageId = new MessageId(messageIdByte); width = in.readInt(); height = in.readInt(); - String mimeType = in.readString(); - extension = in.readString(); + String mimeType = requireNonNull(in.readString()); + extension = requireNonNull(in.readString()); thumbnailWidth = in.readInt(); thumbnailHeight = in.readInt(); hasError = in.readByte() != 0; @@ -82,27 +84,27 @@ public class AttachmentItem implements Parcelable { return height; } - String getMimeType() { + public String getMimeType() { return header.getContentType(); } - String getExtension() { + public String getExtension() { return extension; } - int getThumbnailWidth() { + public int getThumbnailWidth() { return thumbnailWidth; } - int getThumbnailHeight() { + public int getThumbnailHeight() { return thumbnailHeight; } - boolean hasError() { + public boolean hasError() { return hasError; } - String getTransitionName() { + public String getTransitionName() { return String.valueOf(instanceId); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentResult.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItemResult.java similarity index 77% rename from briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentResult.java rename to briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItemResult.java index 2a84252a2..1c5cc8dd3 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentResult.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItemResult.java @@ -1,4 +1,4 @@ -package org.briarproject.briar.android.conversation; +package org.briarproject.briar.android.attachment; import android.net.Uri; @@ -9,7 +9,7 @@ import javax.annotation.concurrent.Immutable; @Immutable @NotNullByDefault -public class AttachmentResult { +public class AttachmentItemResult { @Nullable private final Uri uri; @@ -18,13 +18,13 @@ public class AttachmentResult { @Nullable private final String errorMsg; - public AttachmentResult(Uri uri, AttachmentItem item) { + public AttachmentItemResult(Uri uri, AttachmentItem item) { this.uri = uri; this.item = item; this.errorMsg = null; } - public AttachmentResult(@Nullable String errorMsg) { + public AttachmentItemResult(@Nullable String errorMsg) { this.uri = null; this.item = null; this.errorMsg = errorMsg; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java new file mode 100644 index 000000000..8b5709d5c --- /dev/null +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java @@ -0,0 +1,20 @@ +package org.briarproject.briar.android.attachment; + +import android.net.Uri; +import android.support.annotation.UiThread; + +import org.briarproject.briar.api.messaging.AttachmentHeader; + +import java.util.Collection; +import java.util.List; + +@UiThread +public interface AttachmentManager{ + + AttachmentResult storeAttachments(Collection uri); + + List getAttachmentHeadersForSending(); + + void cancel(); + +} diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentResult.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentResult.java new file mode 100644 index 000000000..4e347b6fb --- /dev/null +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentResult.java @@ -0,0 +1,33 @@ +package org.briarproject.briar.android.attachment; + +import android.arch.lifecycle.LiveData; + +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import java.util.Collection; + +import javax.annotation.concurrent.Immutable; + +@Immutable +@NotNullByDefault +public class AttachmentResult { + + private final Collection> itemResults; + private final LiveData finished; + + public AttachmentResult( + Collection> itemResults, + LiveData finished) { + this.itemResults = itemResults; + this.finished = finished; + } + + public Collection> getItemResults() { + return itemResults; + } + + public LiveData getFinished() { + return finished; + } + +} 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/attachment/AttachmentRetriever.java similarity index 71% rename from briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentController.java rename to briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetriever.java index f7479beb7..d3fb1188b 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentController.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentRetriever.java @@ -1,9 +1,7 @@ -package org.briarproject.briar.android.conversation; +package org.briarproject.briar.android.attachment; -import android.content.ContentResolver; import android.graphics.BitmapFactory; import android.graphics.BitmapFactory.Options; -import android.net.Uri; import android.support.annotation.Nullable; import android.support.annotation.VisibleForTesting; import android.support.media.ExifInterface; @@ -15,9 +13,8 @@ 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.GroupId; import org.briarproject.bramble.api.sync.MessageId; -import org.briarproject.briar.android.conversation.ImageHelper.DecodeResult; +import org.briarproject.briar.android.attachment.ImageHelper.DecodeResult; import org.briarproject.briar.api.messaging.Attachment; import org.briarproject.briar.api.messaging.AttachmentHeader; import org.briarproject.briar.api.messaging.MessagingManager; @@ -38,20 +35,18 @@ import static android.support.media.ExifInterface.ORIENTATION_TRANSVERSE; import static android.support.media.ExifInterface.TAG_IMAGE_LENGTH; import static android.support.media.ExifInterface.TAG_IMAGE_WIDTH; import static android.support.media.ExifInterface.TAG_ORIENTATION; -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.logDuration; import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.LogUtils.now; -import static org.briarproject.briar.api.messaging.MessagingConstants.IMAGE_MIME_TYPES; @NotNullByDefault -class AttachmentController { +public class AttachmentRetriever { private static final Logger LOG = - getLogger(AttachmentController.class.getName()); + getLogger(AttachmentRetriever.class.getName()); private static final int READ_LIMIT = 1024 * 8192; private final MessagingManager messagingManager; @@ -60,12 +55,11 @@ class AttachmentController { private final int minWidth, maxWidth; private final int minHeight, maxHeight; - private final Map unsentItems = - new ConcurrentHashMap<>(); private final Map> attachmentCache = new ConcurrentHashMap<>(); - AttachmentController(MessagingManager messagingManager, + @VisibleForTesting + AttachmentRetriever(MessagingManager messagingManager, AttachmentDimensions dimensions, ImageHelper imageHelper) { this.messagingManager = messagingManager; this.imageHelper = imageHelper; @@ -76,7 +70,7 @@ class AttachmentController { maxHeight = dimensions.maxHeight; } - AttachmentController(MessagingManager messagingManager, + public AttachmentRetriever(MessagingManager messagingManager, AttachmentDimensions dimensions) { this(messagingManager, dimensions, new ImageHelper() { @Override @@ -99,17 +93,17 @@ class AttachmentController { }); } - void put(MessageId messageId, List attachments) { + public void cachePut(MessageId messageId, List attachments) { attachmentCache.put(messageId, attachments); } @Nullable - List get(MessageId messageId) { + public List cacheGet(MessageId messageId) { return attachmentCache.get(messageId); } @DatabaseExecutor - List> getMessageAttachments( + public List> getMessageAttachments( List headers) throws DbException { long start = now(); List> attachments = @@ -118,86 +112,13 @@ class AttachmentController { Attachment a = messagingManager.getAttachment(h.getMessageId()); attachments.add(new Pair<>(h, a)); } - logDuration(LOG, "Loading attachment", start); + logDuration(LOG, "Loading attachments", start); return attachments; } @DatabaseExecutor - AttachmentItem 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 requireNonNull(unsentItems.get(uri)); - } - long start = now(); - InputStream is = contentResolver.openInputStream(uri); - if (is == null) throw new IOException(); - String contentType = contentResolver.getType(uri); - if (contentType == null) throw new IOException("null content type"); - long timestamp = System.currentTimeMillis(); - AttachmentHeader h = messagingManager - .addLocalAttachment(groupId, timestamp, contentType, is); - tryToClose(is, LOG, WARNING); - 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); - return item; - } - - boolean isValidMimeType(@Nullable String mimeType) { - if (mimeType == null) return false; - for (String supportedType : IMAGE_MIME_TYPES) { - if (supportedType.equals(mimeType)) return true; - } - return false; - } - - @DatabaseExecutor - void deleteUnsentAttachments() { - for (AttachmentItem item : unsentItems.values()) { - try { - messagingManager.removeAttachment(item.getHeader()); - } catch (DbException e) { - logException(LOG, WARNING, e); - } - } - unsentItems.clear(); - } - - List getUnsentAttachmentHeaders() { - List headers = - new ArrayList<>(unsentItems.values().size()); - for (AttachmentItem item : unsentItems.values()) { - headers.add(item.getHeader()); - } - return headers; - } - - /** - * 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 - private AttachmentItem getAttachmentItem(ContentResolver contentResolver, - Uri uri, AttachmentHeader h, boolean needsSize) throws IOException { - InputStream is = null; - try { - is = contentResolver.openInputStream(uri); - if (is == null) throw new IOException(); - return getAttachmentItem(h, new Attachment(is), needsSize); - } finally { - if (is != null) tryToClose(is, LOG, WARNING); - } + Attachment getMessageAttachment(AttachmentHeader h) throws DbException { + return messagingManager.getAttachment(h.getMessageId()); } /** @@ -205,7 +126,7 @@ class AttachmentController { *

* Note: This closes the {@link Attachment}'s {@link InputStream}. */ - List getAttachmentItems( + public List getAttachmentItems( List> attachments) { boolean needsSize = attachments.size() == 1; List items = new ArrayList<>(attachments.size()); @@ -221,7 +142,6 @@ class AttachmentController { * Creates an {@link AttachmentItem} from the {@link Attachment}'s * {@link InputStream} which will be closed when this method returns. */ - @VisibleForTesting AttachmentItem getAttachmentItem(AttachmentHeader h, Attachment a, boolean needsSize) { if (!needsSize) { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageHelper.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/ImageHelper.java similarity index 90% rename from briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageHelper.java rename to briar-android/src/main/java/org/briarproject/briar/android/attachment/ImageHelper.java index 203a45222..1264d6511 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageHelper.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/ImageHelper.java @@ -1,4 +1,4 @@ -package org.briarproject.briar.android.conversation; +package org.briarproject.briar.android.attachment; import android.support.annotation.Nullable; 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 fac65e937..925655815 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 @@ -52,6 +52,8 @@ import org.briarproject.bramble.api.sync.event.MessagesSentEvent; import org.briarproject.briar.R; import org.briarproject.briar.android.activity.ActivityComponent; import org.briarproject.briar.android.activity.BriarActivity; +import org.briarproject.briar.android.attachment.AttachmentItem; +import org.briarproject.briar.android.attachment.AttachmentRetriever; import org.briarproject.briar.android.blog.BlogActivity; import org.briarproject.briar.android.conversation.ConversationVisitor.AttachmentCache; import org.briarproject.briar.android.conversation.ConversationVisitor.TextCache; @@ -183,7 +185,7 @@ public class ConversationActivity extends BriarActivity loadMessages(); }; - private AttachmentController attachmentController; + private AttachmentRetriever attachmentRetriever; private ConversationViewModel viewModel; private ConversationVisitor visitor; private ConversationAdapter adapter; @@ -218,7 +220,7 @@ public class ConversationActivity extends BriarActivity viewModel = ViewModelProviders.of(this, viewModelFactory) .get(ConversationViewModel.class); - attachmentController = viewModel.getAttachmentController(); + attachmentRetriever = viewModel.getAttachmentRetriever(); setContentView(R.layout.activity_conversation); @@ -456,13 +458,13 @@ public class ConversationActivity extends BriarActivity // 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 items = attachmentController.get(id); + List items = attachmentRetriever.cacheGet(id); if (items == null) { LOG.info("Eagerly loading image size for latest message"); - items = attachmentController.getAttachmentItems( - attachmentController.getMessageAttachments( + items = attachmentRetriever.getAttachmentItems( + attachmentRetriever.getMessageAttachments( h.getAttachmentHeaders())); - attachmentController.put(id, items); + attachmentRetriever.cachePut(id, items); } } } @@ -544,10 +546,10 @@ public class ConversationActivity extends BriarActivity runOnDbThread(() -> { try { List> attachments = - attachmentController.getMessageAttachments(headers); + attachmentRetriever.getMessageAttachments(headers); // TODO move getting the items off to IoExecutor, if size == 1 List items = - attachmentController.getAttachmentItems(attachments); + attachmentRetriever.getAttachmentItems(attachments); displayMessageAttachments(messageId, items); } catch (DbException e) { logException(LOG, WARNING, e); @@ -558,7 +560,7 @@ public class ConversationActivity extends BriarActivity private void displayMessageAttachments(MessageId m, List items) { runOnUiThreadUnlessDestroyed(() -> { - attachmentController.put(m, items); + attachmentRetriever.cachePut(m, items); Pair pair = adapter.getMessageItem(m); if (pair != null) { @@ -903,7 +905,7 @@ public class ConversationActivity extends BriarActivity @Override public List getAttachmentItems(MessageId m, List headers) { - List attachments = attachmentController.get(m); + List attachments = attachmentRetriever.cacheGet(m); if (attachments == null) { loadMessageAttachments(m, headers); return emptyList(); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationListener.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationListener.java index d7c445fae..d961ff1e2 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationListener.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationListener.java @@ -4,6 +4,7 @@ import android.support.annotation.UiThread; import android.view.View; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.briar.android.attachment.AttachmentItem; @UiThread @NotNullByDefault 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 1732e3ab8..6b853932a 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 @@ -3,6 +3,7 @@ package org.briarproject.briar.android.conversation; import android.support.annotation.LayoutRes; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.briar.android.attachment.AttachmentItem; import org.briarproject.briar.api.messaging.PrivateMessageHeader; import java.util.List; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationMessageViewHolder.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationMessageViewHolder.java index 94c80639a..6524129b4 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationMessageViewHolder.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationMessageViewHolder.java @@ -9,6 +9,7 @@ import android.view.ViewGroup; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.briar.R; +import org.briarproject.briar.android.attachment.AttachmentItem; import static android.support.constraint.ConstraintSet.WRAP_CONTENT; import static android.support.v4.content.ContextCompat.getColor; 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 663e94e98..aa8718c0f 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 @@ -5,7 +5,6 @@ import android.arch.lifecycle.AndroidViewModel; import android.arch.lifecycle.LiveData; import android.arch.lifecycle.MutableLiveData; import android.arch.lifecycle.Transformations; -import android.content.ContentResolver; import android.net.Uri; import android.support.annotation.Nullable; import android.support.annotation.UiThread; @@ -20,26 +19,26 @@ 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.identity.AuthorId; +import org.briarproject.bramble.api.lifecycle.IoExecutor; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.settings.Settings; import org.briarproject.bramble.api.settings.SettingsManager; import org.briarproject.bramble.api.sync.GroupId; import org.briarproject.bramble.api.sync.Message; import org.briarproject.bramble.api.sync.MessageId; -import org.briarproject.bramble.api.system.AndroidExecutor; -import org.briarproject.briar.R; +import org.briarproject.briar.android.attachment.AttachmentCreator; +import org.briarproject.briar.android.attachment.AttachmentManager; +import org.briarproject.briar.android.attachment.AttachmentResult; +import org.briarproject.briar.android.attachment.AttachmentRetriever; import org.briarproject.briar.android.util.UiUtils; -import org.briarproject.briar.android.view.TextAttachmentController.AttachmentManager; import org.briarproject.briar.android.viewmodel.LiveEvent; import org.briarproject.briar.android.viewmodel.MutableLiveEvent; import org.briarproject.briar.api.messaging.AttachmentHeader; -import org.briarproject.briar.api.messaging.FileTooBigException; 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 java.io.IOException; import java.util.Collection; import java.util.List; import java.util.concurrent.Executor; @@ -53,14 +52,12 @@ import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.util.LogUtils.logDuration; import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.LogUtils.now; -import static org.briarproject.briar.android.conversation.AttachmentDimensions.getAttachmentDimensions; +import static org.briarproject.briar.android.attachment.AttachmentDimensions.getAttachmentDimensions; import static org.briarproject.briar.android.settings.SettingsFragment.SETTINGS_NAMESPACE; -import static org.briarproject.briar.android.util.UiUtils.observeForeverOnce; -import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_IMAGE_SIZE; @NotNullByDefault -public class ConversationViewModel extends AndroidViewModel implements - AttachmentManager { +public class ConversationViewModel extends AndroidViewModel + implements AttachmentManager { private static Logger LOG = getLogger(ConversationViewModel.class.getName()); @@ -78,10 +75,13 @@ public class ConversationViewModel extends AndroidViewModel implements private final ContactManager contactManager; private final SettingsManager settingsManager; private final PrivateMessageFactory privateMessageFactory; - private final AttachmentController attachmentController; + private final AttachmentRetriever attachmentRetriever; + private final AttachmentCreator attachmentCreator; @Nullable private ContactId contactId = null; + @Nullable + private volatile GroupId messagingGroupId = null; private final MutableLiveData contact = new MutableLiveData<>(); private final LiveData contactAuthorId = Transformations.map(contact, c -> c.getAuthor().getId()); @@ -97,15 +97,14 @@ public class ConversationViewModel extends AndroidViewModel implements new MutableLiveData<>(); private final MutableLiveData contactDeleted = new MutableLiveData<>(); - private final MutableLiveData messagingGroupId = - new MutableLiveData<>(); private final MutableLiveData addedHeader = new MutableLiveData<>(); @Inject ConversationViewModel(Application application, @DatabaseExecutor Executor dbExecutor, - @CryptoExecutor Executor cryptoExecutor, TransactionManager db, + @CryptoExecutor Executor cryptoExecutor, + @IoExecutor Executor ioExecutor, TransactionManager db, MessagingManager messagingManager, ContactManager contactManager, SettingsManager settingsManager, PrivateMessageFactory privateMessageFactory) { @@ -117,15 +116,17 @@ public class ConversationViewModel extends AndroidViewModel implements this.contactManager = contactManager; this.settingsManager = settingsManager; this.privateMessageFactory = privateMessageFactory; - this.attachmentController = new AttachmentController(messagingManager, + this.attachmentRetriever = new AttachmentRetriever(messagingManager, getAttachmentDimensions(application.getResources())); + this.attachmentCreator = new AttachmentCreator(getApplication(), + ioExecutor, messagingManager, attachmentRetriever); contactDeleted.setValue(false); } @Override protected void onCleared() { super.onCleared(); - attachmentController.deleteUnsentAttachments(); + attachmentCreator.deleteUnsentAttachments(); } /** @@ -149,6 +150,10 @@ public class ConversationViewModel extends AndroidViewModel implements contact.postValue(c); logDuration(LOG, "Loading contact", start); start = now(); + messagingGroupId = + messagingManager.getConversationId(contactId); + logDuration(LOG, "Load conversation GroupId", start); + start = now(); checkFeaturesAndOnboarding(contactId); logDuration(LOG, "Checking for image support", start); } catch (NoSuchContactException e) { @@ -185,73 +190,29 @@ public class ConversationViewModel extends AndroidViewModel implements void sendMessage(@Nullable String text, List attachmentHeaders, long timestamp) { - if (messagingGroupId.getValue() == null) loadGroupId(); - observeForeverOnce(messagingGroupId, groupId -> { - if (groupId == null) return; - createMessage(groupId, text, attachmentHeaders, timestamp); - }); + GroupId groupId = messagingGroupId; + if (groupId == null) throw new IllegalStateException(); + createMessage(groupId, text, attachmentHeaders, timestamp); } @Override - public LiveData storeAttachment(Uri uri, - boolean needsSize) { - // use LiveData to not keep references to view scope - MutableLiveData result = new MutableLiveData<>(); - // check first if mime type is supported - ContentResolver contentResolver = - getApplication().getContentResolver(); - String mimeType = contentResolver.getType(uri); - if (!attachmentController.isValidMimeType(mimeType)) { - String errorMsg = getApplication().getString( - R.string.image_attach_error_invalid_mime_type, mimeType); - result.setValue(new AttachmentResult(errorMsg)); - return result; - } - if (messagingGroupId.getValue() == null) loadGroupId(); - observeForeverOnce(messagingGroupId, groupId -> dbExecutor.execute(() - -> { - if (groupId == null) throw new IllegalStateException(); - long start = now(); - try { - AttachmentItem item = attachmentController - .createAttachmentHeader(contentResolver, groupId, uri, - needsSize); - result.postValue(new AttachmentResult(uri, item)); - } catch(FileTooBigException e) { - logException(LOG, WARNING, e); - int mb = MAX_IMAGE_SIZE / 1024 / 1024; - String errorMsg = getApplication() - .getString(R.string.image_attach_error_too_big, mb); - result.postValue(new AttachmentResult(errorMsg)); - } catch (DbException | IOException e) { - logException(LOG, WARNING, e); - result.postValue(new AttachmentResult(null)); - } - logDuration(LOG, "Storing attachment", start); - })); - return result; + @UiThread + public AttachmentResult storeAttachments(Collection uris) { + GroupId groupId = messagingGroupId; + if (groupId == null) throw new IllegalStateException(); + return attachmentCreator.storeAttachments(groupId, uris); } @Override - public List getAttachmentHeaders() { - return attachmentController.getUnsentAttachmentHeaders(); + @UiThread + public List getAttachmentHeadersForSending() { + return attachmentCreator.getAttachmentHeadersForSending(); } @Override - public void removeAttachments() { - dbExecutor.execute(attachmentController::deleteUnsentAttachments); - } - - private void loadGroupId() { - if (contactId == null) throw new IllegalStateException(); - dbExecutor.execute(() -> { - try { - messagingGroupId.postValue( - messagingManager.getConversationId(contactId)); - } catch (DbException e) { - logException(LOG, WARNING, e); - } - }); + @UiThread + public void cancel() { + attachmentCreator.cancel(); } @DatabaseExecutor @@ -337,7 +298,7 @@ public class ConversationViewModel extends AndroidViewModel implements message.getId(), message.getGroupId(), message.getTimestamp(), true, true, false, false, text != null, attachments); - attachmentController.onAttachmentsSent(m.getMessage().getId()); + attachmentCreator.onAttachmentsSent(m.getMessage().getId()); // TODO add text to cache when available here addedHeader.postValue(h); } catch (DbException e) { @@ -351,8 +312,8 @@ public class ConversationViewModel extends AndroidViewModel implements addedHeader.setValue(null); } - AttachmentController getAttachmentController() { - return attachmentController; + AttachmentRetriever getAttachmentRetriever() { + return attachmentRetriever; } LiveData getContact() { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationVisitor.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationVisitor.java index 87ec94f07..48f39e666 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationVisitor.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationVisitor.java @@ -7,6 +7,7 @@ import android.support.annotation.UiThread; 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.api.blog.BlogInvitationRequest; import org.briarproject.briar.api.blog.BlogInvitationResponse; import org.briarproject.briar.api.conversation.ConversationMessageVisitor; 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 aebb3069e..1e1f3bf36 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 @@ -31,6 +31,7 @@ import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; import org.briarproject.briar.R; import org.briarproject.briar.android.activity.ActivityComponent; import org.briarproject.briar.android.activity.BriarActivity; +import org.briarproject.briar.android.attachment.AttachmentItem; import org.briarproject.briar.android.util.BriarSnackbarBuilder; import org.briarproject.briar.android.view.PullDownLayout; 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 8c9e15d30..f97a5f233 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 @@ -12,6 +12,7 @@ import android.view.WindowManager; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.briar.R; +import org.briarproject.briar.android.attachment.AttachmentItem; import org.briarproject.briar.android.conversation.glide.Radii; import java.util.ArrayList; 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 1d1b0d1fd..684e216a8 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 @@ -21,6 +21,7 @@ import com.github.chrisbanes.photoview.PhotoView; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.briar.R; import org.briarproject.briar.android.activity.BaseActivity; +import org.briarproject.briar.android.attachment.AttachmentItem; import org.briarproject.briar.android.conversation.glide.GlideApp; import javax.annotation.ParametersAreNonnullByDefault; 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 f76e3c2ec..4019bb4f4 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 @@ -11,6 +11,7 @@ import com.bumptech.glide.load.Transformation; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.briar.R; +import org.briarproject.briar.android.attachment.AttachmentItem; import org.briarproject.briar.android.conversation.glide.BriarImageTransformation; import org.briarproject.briar.android.conversation.glide.GlideApp; import org.briarproject.briar.android.conversation.glide.Radii; 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 c8c9451c9..33f29d3e3 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 @@ -13,6 +13,7 @@ import org.briarproject.bramble.api.db.DbException; 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; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarDataFetcher.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarDataFetcher.java index ac239f4a4..1394f8035 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarDataFetcher.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarDataFetcher.java @@ -10,7 +10,7 @@ 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.android.conversation.AttachmentItem; +import org.briarproject.briar.android.attachment.AttachmentItem; import org.briarproject.briar.api.messaging.MessagingManager; import java.io.InputStream; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarDataFetcherFactory.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarDataFetcherFactory.java index 87f55a170..69a920ae9 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarDataFetcherFactory.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarDataFetcherFactory.java @@ -2,7 +2,7 @@ package org.briarproject.briar.android.conversation.glide; import org.briarproject.bramble.api.db.DatabaseExecutor; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; -import org.briarproject.briar.android.conversation.AttachmentItem; +import org.briarproject.briar.android.attachment.AttachmentItem; import org.briarproject.briar.api.messaging.MessagingManager; import java.util.concurrent.Executor; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarGlideModule.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarGlideModule.java index a1a66ab33..a477f2f44 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarGlideModule.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarGlideModule.java @@ -10,7 +10,7 @@ import com.bumptech.glide.module.AppGlideModule; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.briar.android.BriarApplication; -import org.briarproject.briar.android.conversation.AttachmentItem; +import org.briarproject.briar.android.attachment.AttachmentItem; import java.io.InputStream; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarModelLoader.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarModelLoader.java index b4e132392..dd5008f67 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarModelLoader.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarModelLoader.java @@ -8,7 +8,7 @@ import com.bumptech.glide.signature.ObjectKey; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; import org.briarproject.briar.android.BriarApplication; -import org.briarproject.briar.android.conversation.AttachmentItem; +import org.briarproject.briar.android.attachment.AttachmentItem; import java.io.InputStream; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarModelLoaderFactory.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarModelLoaderFactory.java index 7d077e50e..07ed62227 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarModelLoaderFactory.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/glide/BriarModelLoaderFactory.java @@ -6,7 +6,7 @@ import com.bumptech.glide.load.model.MultiModelLoaderFactory; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.briar.android.BriarApplication; -import org.briarproject.briar.android.conversation.AttachmentItem; +import org.briarproject.briar.android.attachment.AttachmentItem; import java.io.InputStream; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java index 5c66d87b2..2a0585371 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java @@ -9,7 +9,7 @@ import android.view.LayoutInflater; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.briar.R; -import org.briarproject.briar.android.conversation.AttachmentResult; +import org.briarproject.briar.android.attachment.AttachmentItemResult; import java.util.Collection; @@ -72,7 +72,7 @@ public class ImagePreview extends ConstraintLayout { imageList.setAdapter(adapter); } - void loadPreviewImage(AttachmentResult result) { + void loadPreviewImage(AttachmentItemResult result) { ImagePreviewAdapter adapter = ((ImagePreviewAdapter) imageList.getAdapter()); requireNonNull(adapter).loadItemPreview(result); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java index 0982e4523..dcbfd9ec3 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java @@ -8,7 +8,7 @@ import android.view.ViewGroup; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.briar.R; -import org.briarproject.briar.android.conversation.AttachmentResult; +import org.briarproject.briar.android.attachment.AttachmentItemResult; import java.util.ArrayList; import java.util.Collection; @@ -50,7 +50,7 @@ class ImagePreviewAdapter extends Adapter { return items.size(); } - void loadItemPreview(AttachmentResult result) { + void loadItemPreview(AttachmentItemResult result) { ImagePreviewItem newItem = new ImagePreviewItem(requireNonNull(result.getUri())); int pos = items.indexOf(newItem); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewItem.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewItem.java index e9c1d0585..5a4f24648 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewItem.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewItem.java @@ -4,7 +4,7 @@ import android.net.Uri; import android.support.annotation.Nullable; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; -import org.briarproject.briar.android.conversation.AttachmentItem; +import org.briarproject.briar.android.attachment.AttachmentItem; import java.util.ArrayList; import java.util.Collection; @@ -30,10 +30,6 @@ class ImagePreviewItem { return items; } - Uri getUri() { - return uri; - } - public void setItem(AttachmentItem item) { this.item = item; } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java index f3c400238..4fefb2ae5 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java @@ -38,9 +38,9 @@ class ImagePreviewViewHolder extends ViewHolder { } void bind(ImagePreviewItem item) { - if (item.getItem() == null) return; + if (item.getItem() == null) return; // shows progress bar GlideApp.with(imageView) - .load(item.getUri()) + .load(item.getItem()) .diskCacheStrategy(NONE) .error(ERROR_RES) .downsample(FIT_CENTER) 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 9774463b2..a6434c865 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 @@ -3,6 +3,7 @@ package org.briarproject.briar.android.view; import android.app.Activity; import android.arch.lifecycle.LifecycleOwner; import android.arch.lifecycle.LiveData; +import android.arch.lifecycle.Observer; import android.content.ClipData; import android.content.Context; import android.content.Intent; @@ -17,9 +18,10 @@ import android.widget.Toast; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.briar.R; -import org.briarproject.briar.android.conversation.AttachmentResult; +import org.briarproject.briar.android.attachment.AttachmentItemResult; +import org.briarproject.briar.android.attachment.AttachmentManager; +import org.briarproject.briar.android.attachment.AttachmentResult; import org.briarproject.briar.android.view.ImagePreview.ImagePreviewListener; -import org.briarproject.briar.api.messaging.AttachmentHeader; import java.util.ArrayList; import java.util.List; @@ -38,7 +40,6 @@ import static android.support.v4.content.ContextCompat.getColor; import static android.support.v4.view.AbsSavedState.EMPTY_STATE; import static android.view.View.GONE; import static android.widget.Toast.LENGTH_LONG; -import static java.util.Collections.emptyList; import static java.util.Objects.requireNonNull; import static org.briarproject.briar.android.util.UiUtils.resolveColorAttribute; import static org.briarproject.briar.api.messaging.MessagingConstants.IMAGE_MIME_TYPES; @@ -55,9 +56,8 @@ public class TextAttachmentController extends TextSendController private final CompositeSendButton sendButton; private final AttachmentManager attachmentManager; - private CharSequence textHint; - private List imageUris = emptyList(); - private int urisLoaded = 0; + private final List imageUris = new ArrayList<>(); + private final CharSequence textHint; private boolean loadingUris = false; public TextAttachmentController(TextInputView v, ImagePreview imagePreview, @@ -96,7 +96,7 @@ public class TextAttachmentController extends TextSendController if (canSend()) { if (loadingUris) throw new AssertionError(); listener.onSendClick(textInput.getText(), - attachmentManager.getAttachmentHeaders()); + attachmentManager.getAttachmentHeadersForSending()); reset(); } } @@ -140,14 +140,12 @@ public class TextAttachmentController extends TextSendController public void onImageReceived(@Nullable Intent resultData) { if (resultData == null) return; - if (loadingUris) throw new AssertionError(); + if (loadingUris || !imageUris.isEmpty()) throw new AssertionError(); if (resultData.getData() != null) { - imageUris = new ArrayList<>(1); imageUris.add(resultData.getData()); onNewUris(); } else if (SDK_INT >= 18 && resultData.getClipData() != null) { ClipData clipData = resultData.getClipData(); - imageUris = new ArrayList<>(clipData.getItemCount()); for (int i = 0; i < clipData.getItemCount(); i++) { imageUris.add(clipData.getItemAt(i).getUri()); } @@ -157,40 +155,62 @@ public class TextAttachmentController extends TextSendController private void onNewUris() { if (imageUris.isEmpty()) return; - if (loadingUris || urisLoaded != 0) throw new AssertionError(); + if (loadingUris) throw new AssertionError(); loadingUris = true; updateViewState(); textInput.setHint(R.string.image_caption_hint); List items = ImagePreviewItem.fromUris(imageUris); imagePreview.showPreview(items); // store attachments and show preview when successful - boolean needsSize = items.size() == 1; - for (ImagePreviewItem item : items) { - attachmentManager.storeAttachment(item.getUri(), needsSize) - .observe(imageListener, this::onAttachmentResultReceived); + AttachmentResult result = attachmentManager.storeAttachments(imageUris); + for (LiveData liveData : result + .getItemResults()) { + onLiveDataReturned(liveData); } + result.getFinished().observe(imageListener, new Observer() { + @Override + public void onChanged(@Nullable Boolean finished) { + if (finished != null && finished) onAllAttachmentsCreated(); + result.getFinished().removeObserver(this); + } + }); } - private void onAttachmentResultReceived(AttachmentResult result) { - if (!loadingUris) return; // if this is false, the user cancelled + private void onLiveDataReturned(LiveData liveData) { + liveData.observe(imageListener, new Observer() { + @Override + public void onChanged(@Nullable AttachmentItemResult result) { + if (result != null) { + onAttachmentResultReceived(result); + } + liveData.removeObserver(this); + } + }); + } + + private void onAttachmentResultReceived(AttachmentItemResult result) { + if (!loadingUris) throw new AssertionError(); if (result.isError() || result.getUri() == null) { onError(result.getErrorMsg()); } else { imagePreview.loadPreviewImage(result); - urisLoaded++; - checkAllUrisLoaded(); } } + private void onAllAttachmentsCreated() { + if (!loadingUris) throw new AssertionError(); + loadingUris = false; + updateViewState(); + } + private void reset() { // restore hint textInput.setHint(textHint); // hide image layout imagePreview.setVisibility(GONE); // reset image URIs - imageUris = emptyList(); - // no URIs has been loaded - urisLoaded = 0; + imageUris.clear(); + // definitely not loading anymore loadingUris = false; // show the image button again, so images can get attached updateViewState(); @@ -208,7 +228,8 @@ public class TextAttachmentController extends TextSendController @Nullable public Parcelable onRestoreInstanceState(Parcelable inState) { SavedState state = (SavedState) inState; - imageUris = requireNonNull(state.imageUris); + if (!imageUris.isEmpty()) throw new AssertionError(); + if (state.imageUris != null) imageUris.addAll(state.imageUris); onNewUris(); return state.getSuperState(); } @@ -226,19 +247,10 @@ public class TextAttachmentController extends TextSendController @Override public void onCancel() { textInput.clearText(); - attachmentManager.removeAttachments(); + attachmentManager.cancel(); reset(); } - private void checkAllUrisLoaded() { - if (!loadingUris) throw new AssertionError(); - if (urisLoaded == imageUris.size()) { - loadingUris = false; - // all images were turned into attachments - updateViewState(); - } - } - public void showImageOnboarding(Activity activity, Runnable onOnboardingSeen) { PromptStateChangeListener listener = (prompt, state) -> { @@ -297,19 +309,4 @@ public class TextAttachmentController extends TextSendController void onAttachImage(Intent intent); } - public interface AttachmentManager { - /** - * Stores a new attachment in the database. - * - * @param uri The Uri of the attachment to store. - * @param needsSize true if this is the only image in the message - * and therefore needs to know its size. - */ - LiveData storeAttachment(Uri uri, boolean needsSize); - - List getAttachmentHeaders(); - - void removeAttachments(); - } - } diff --git a/briar-android/src/test/java/org/briarproject/briar/android/conversation/AttachmentControllerTest.java b/briar-android/src/test/java/org/briarproject/briar/android/attachment/AttachmentRetrieverTest.java similarity index 93% rename from briar-android/src/test/java/org/briarproject/briar/android/conversation/AttachmentControllerTest.java rename to briar-android/src/test/java/org/briarproject/briar/android/attachment/AttachmentRetrieverTest.java index f5abfc706..f0f38bdb1 100644 --- a/briar-android/src/test/java/org/briarproject/briar/android/conversation/AttachmentControllerTest.java +++ b/briar-android/src/test/java/org/briarproject/briar/android/attachment/AttachmentRetrieverTest.java @@ -1,8 +1,8 @@ -package org.briarproject.briar.android.conversation; +package org.briarproject.briar.android.attachment; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.bramble.test.BrambleMockTestCase; -import org.briarproject.briar.android.conversation.ImageHelper.DecodeResult; +import org.briarproject.briar.android.attachment.ImageHelper.DecodeResult; import org.briarproject.briar.api.messaging.Attachment; import org.briarproject.briar.api.messaging.AttachmentHeader; import org.briarproject.briar.api.messaging.MessagingManager; @@ -19,7 +19,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -public class AttachmentControllerTest extends BrambleMockTestCase { +public class AttachmentRetrieverTest extends BrambleMockTestCase { private final AttachmentDimensions dimensions = new AttachmentDimensions( 100, 50, 200, 75, 300 @@ -32,8 +32,8 @@ public class AttachmentControllerTest extends BrambleMockTestCase { private final MessagingManager messagingManager = context.mock(MessagingManager.class); private final ImageHelper imageHelper = context.mock(ImageHelper.class); - private final AttachmentController controller = - new AttachmentController( + private final AttachmentRetriever controller = + new AttachmentRetriever( messagingManager, dimensions, imageHelper diff --git a/briar-android/src/test/java/org/briarproject/briar/android/conversation/MarkEnforcingInputStreamTest.java b/briar-android/src/test/java/org/briarproject/briar/android/attachment/MarkEnforcingInputStreamTest.java similarity index 97% rename from briar-android/src/test/java/org/briarproject/briar/android/conversation/MarkEnforcingInputStreamTest.java rename to briar-android/src/test/java/org/briarproject/briar/android/attachment/MarkEnforcingInputStreamTest.java index 45a30a499..14defdf4d 100644 --- a/briar-android/src/test/java/org/briarproject/briar/android/conversation/MarkEnforcingInputStreamTest.java +++ b/briar-android/src/test/java/org/briarproject/briar/android/attachment/MarkEnforcingInputStreamTest.java @@ -1,4 +1,4 @@ -package org.briarproject.briar.android.conversation; +package org.briarproject.briar.android.attachment; import com.bumptech.glide.util.MarkEnforcingInputStream; 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 8917be036..366eb768c 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 @@ -174,6 +174,10 @@ class MessagingManagerImpl extends ConversationClientImpl } if (is.available() > 0) throw new FileTooBigException(); is.close(); + try { + Thread.sleep(1000); + } catch (InterruptedException ignored) { + } Message m = messageFactory.createMessage(groupId, timestamp, body); clientHelper.addLocalMessage(m, new BdfDictionary(), false); return new AttachmentHeader(m.getId(), contentType); From 7358091699ff7e5d9c3c8547a65e58d4bbd8884e Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 12 Apr 2019 09:25:45 -0300 Subject: [PATCH 10/17] [android] Address first round of review comments for attachments --- .../attachment/AttachmentCreationTask.java | 19 +++++++++++-------- .../android/attachment/AttachmentCreator.java | 13 ++++++------- .../android/attachment/AttachmentManager.java | 2 +- .../view/TextAttachmentController.java | 10 ++++++++++ 4 files changed, 28 insertions(+), 16 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java index 3e4e64767..4bd89e3d9 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java @@ -37,7 +37,7 @@ class AttachmentCreationTask { private final List uris; private final boolean needsSize; @Nullable - private AttachmentCreator attachmentCreator; + private volatile AttachmentCreator attachmentCreator; private volatile boolean canceled = false; @@ -61,9 +61,10 @@ class AttachmentCreationTask { @IoExecutor public void storeAttachments() { for (Uri uri: uris) processUri(uri); + AttachmentCreator attachmentCreator = this.attachmentCreator; if (!canceled && attachmentCreator != null) attachmentCreator.onAttachmentCreationFinished(); - attachmentCreator = null; + this.attachmentCreator = null; } @IoExecutor @@ -71,16 +72,17 @@ class AttachmentCreationTask { if (canceled) return; try { AttachmentHeader h = storeAttachment(uri); + AttachmentCreator attachmentCreator = this.attachmentCreator; if (attachmentCreator != null) { - attachmentCreator - .onAttachmentHeaderReceived(uri, h, needsSize); + attachmentCreator.onAttachmentHeaderReceived(uri, h, needsSize); } } catch (DbException | IOException e) { logException(LOG, WARNING, e); + AttachmentCreator attachmentCreator = this.attachmentCreator; if (attachmentCreator != null) { attachmentCreator.onAttachmentError(uri, e); - canceled = true; } + canceled = true; } } @@ -90,9 +92,10 @@ class AttachmentCreationTask { long start = now(); String contentType = contentResolver.getType(uri); if (contentType == null) throw new IOException("null content type"); - if (!isValidMimeType(contentType)) - throw new UnsupportedMimeTypeException("", contentType, - uri.toString()); + if (!isValidMimeType(contentType)) { + String uriString = uri.toString(); + throw new UnsupportedMimeTypeException("", contentType, uriString); + } InputStream is = contentResolver.openInputStream(uri); if (is == null) throw new IOException(); long timestamp = System.currentTimeMillis(); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java index 32f12c2fc..e03a0d7b5 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java @@ -44,7 +44,7 @@ public class AttachmentCreator { @IoExecutor private final Executor ioExecutor; private final MessagingManager messagingManager; - private final AttachmentRetriever controller; + private final AttachmentRetriever retriever; private final Map unsentItems = new ConcurrentHashMap<>(); @@ -57,12 +57,11 @@ public class AttachmentCreator { private AttachmentCreationTask task; public AttachmentCreator(Application app, @IoExecutor Executor ioExecutor, - MessagingManager messagingManager, - AttachmentRetriever controller) { + MessagingManager messagingManager, AttachmentRetriever retriever) { this.app = app; this.ioExecutor = ioExecutor; this.messagingManager = messagingManager; - this.controller = controller; + this.retriever = retriever; } @UiThread @@ -101,8 +100,8 @@ public class AttachmentCreator { boolean needsSize) { // get and cache AttachmentItem for ImagePreview try { - Attachment a = controller.getMessageAttachment(h); - AttachmentItem item = controller.getAttachmentItem(h, a, needsSize); + Attachment a = retriever.getMessageAttachment(h); + AttachmentItem item = retriever.getAttachmentItem(h, a, needsSize); if (item.hasError()) throw new IOException(); unsentItems.put(uri, item); MutableLiveData result = @@ -156,7 +155,7 @@ public class AttachmentCreator { * @param id The MessageId of the sent message. */ public void onAttachmentsSent(MessageId id) { - controller.cachePut(id, new ArrayList<>(unsentItems.values())); + retriever.cachePut(id, new ArrayList<>(unsentItems.values())); resetState(); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java index 8b5709d5c..194af99ec 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java @@ -9,7 +9,7 @@ import java.util.Collection; import java.util.List; @UiThread -public interface AttachmentManager{ +public interface AttachmentManager { AttachmentResult storeAttachments(Collection uri); 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 a6434c865..b7bd4f23f 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 @@ -138,6 +138,16 @@ public class TextAttachmentController extends TextSendController return intent; } + /** + * This is called with the result Intent + * returned by the Activity started with {@link #getAttachFileIntent()}. + *

+ * This method must be called at most once per call to + * {@link AttachImageListener#onAttachImage(Intent)}. + * Normally, this is true if called from + * {@link Activity#onActivityResult(int, int, Intent)} since this is called + * at most once per call to {@link Activity#startActivityForResult(Intent, int)}. + */ public void onImageReceived(@Nullable Intent resultData) { if (resultData == null) return; if (loadingUris || !imageUris.isEmpty()) throw new AssertionError(); From 9d9bc4ca84dd02cfa18c7450a99049fa17ff8cad Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Sun, 14 Apr 2019 12:31:31 -0300 Subject: [PATCH 11/17] [android] Let AttachmentCreator return same LiveData after configuration changes --- .../android/attachment/AttachmentCreator.java | 59 +++++++++++-------- .../android/attachment/AttachmentManager.java | 2 +- .../conversation/ConversationViewModel.java | 8 +-- .../view/TextAttachmentController.java | 11 ++-- 4 files changed, 44 insertions(+), 36 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java index e03a0d7b5..ea8341de6 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java @@ -29,7 +29,6 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; import java.util.logging.Logger; -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.LogUtils.logException; @@ -46,6 +45,7 @@ public class AttachmentCreator { private final MessagingManager messagingManager; private final AttachmentRetriever retriever; + // store unsent items separately, as LiveData might not return latest value private final Map unsentItems = new ConcurrentHashMap<>(); private final Map> @@ -66,32 +66,38 @@ public class AttachmentCreator { @UiThread public AttachmentResult storeAttachments(GroupId groupId, - Collection uris) { - if (task != null && !isStoring()) throw new AssertionError(); + Collection uris, boolean restart) { List> itemResults = new ArrayList<>(); - List urisToStore = new ArrayList<>(); - for (Uri uri : uris) { - MutableLiveData liveData = - new MutableLiveData<>(); - itemResults.add(liveData); - liveDataResult.put(uri, liveData); - if (unsentItems.containsKey(uri)) { - // This can happen due to configuration changes. - // So don't create a new attachment, if we have one already. - AttachmentItem item = requireNonNull(unsentItems.get(uri)); - AttachmentItemResult result = - new AttachmentItemResult(uri, item); - liveData.setValue(result); - } else { - urisToStore.add(uri); + if (restart) { + // This can happen due to configuration changes. + // So don't create new attachments, if we have (or creating) them. + // Instead, re-subscribe to the existing LiveData. + if (task == null || isNotStoring()) throw new AssertionError(); + for (Uri uri : uris) { + // We don't want to expose mutable(!) LiveData + LiveData liveData = + liveDataResult.get(uri); + if (liveData == null) throw new IllegalStateException(); + itemResults.add(liveData); } + if (liveDataFinished == null) throw new IllegalStateException(); + } else { + if (task != null && isNotStoring()) throw new AssertionError(); + List urisToStore = new ArrayList<>(); + for (Uri uri : uris) { + urisToStore.add(uri); + MutableLiveData liveData = + new MutableLiveData<>(); + liveDataResult.put(uri, liveData); + itemResults.add(liveData); + } + boolean needsSize = uris.size() == 1; + task = new AttachmentCreationTask(messagingManager, + app.getContentResolver(), this, groupId, urisToStore, + needsSize); + ioExecutor.execute(() -> task.storeAttachments()); + liveDataFinished = new MutableLiveData<>(); } - boolean needsSize = uris.size() == 1; - task = new AttachmentCreationTask(messagingManager, - app.getContentResolver(), this, groupId, urisToStore, - needsSize); - ioExecutor.execute(() -> task.storeAttachments()); - liveDataFinished = new MutableLiveData<>(); return new AttachmentResult(itemResults, liveDataFinished); } @@ -185,6 +191,7 @@ public class AttachmentCreator { @UiThread public void deleteUnsentAttachments() { + // Make a copy for the IoExecutor as we clear the unsentItems soon List itemsToDelete = new ArrayList<>(unsentItems.values()); ioExecutor.execute(() -> { @@ -198,8 +205,8 @@ public class AttachmentCreator { }); } - private boolean isStoring() { - return liveDataFinished != null; + private boolean isNotStoring() { + return liveDataFinished == null; } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java index 194af99ec..f478b2c53 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java @@ -11,7 +11,7 @@ import java.util.List; @UiThread public interface AttachmentManager { - AttachmentResult storeAttachments(Collection uri); + AttachmentResult storeAttachments(Collection uri, boolean restart); List getAttachmentHeadersForSending(); 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 aa8718c0f..40b21f0f7 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 @@ -150,8 +150,7 @@ public class ConversationViewModel extends AndroidViewModel contact.postValue(c); logDuration(LOG, "Loading contact", start); start = now(); - messagingGroupId = - messagingManager.getConversationId(contactId); + messagingGroupId = messagingManager.getContactGroup(c).getId(); logDuration(LOG, "Load conversation GroupId", start); start = now(); checkFeaturesAndOnboarding(contactId); @@ -197,10 +196,11 @@ public class ConversationViewModel extends AndroidViewModel @Override @UiThread - public AttachmentResult storeAttachments(Collection uris) { + public AttachmentResult storeAttachments(Collection uris, + boolean restart) { GroupId groupId = messagingGroupId; if (groupId == null) throw new IllegalStateException(); - return attachmentCreator.storeAttachments(groupId, uris); + return attachmentCreator.storeAttachments(groupId, uris, restart); } @Override 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 b7bd4f23f..5f549a863 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 @@ -153,17 +153,17 @@ public class TextAttachmentController extends TextSendController if (loadingUris || !imageUris.isEmpty()) throw new AssertionError(); if (resultData.getData() != null) { imageUris.add(resultData.getData()); - onNewUris(); + onNewUris(false); } else if (SDK_INT >= 18 && resultData.getClipData() != null) { ClipData clipData = resultData.getClipData(); for (int i = 0; i < clipData.getItemCount(); i++) { imageUris.add(clipData.getItemAt(i).getUri()); } - onNewUris(); + onNewUris(false); } } - private void onNewUris() { + private void onNewUris(boolean restart) { if (imageUris.isEmpty()) return; if (loadingUris) throw new AssertionError(); loadingUris = true; @@ -172,7 +172,8 @@ public class TextAttachmentController extends TextSendController List items = ImagePreviewItem.fromUris(imageUris); imagePreview.showPreview(items); // store attachments and show preview when successful - AttachmentResult result = attachmentManager.storeAttachments(imageUris); + AttachmentResult result = + attachmentManager.storeAttachments(imageUris, restart); for (LiveData liveData : result .getItemResults()) { onLiveDataReturned(liveData); @@ -240,7 +241,7 @@ public class TextAttachmentController extends TextSendController SavedState state = (SavedState) inState; if (!imageUris.isEmpty()) throw new AssertionError(); if (state.imageUris != null) imageUris.addAll(state.imageUris); - onNewUris(); + onNewUris(true); return state.getSuperState(); } From cd3174a64328d8a67c516bb31441098c765cd358 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 1 May 2019 11:58:45 -0300 Subject: [PATCH 12/17] [android] Fix view recycling issue of image previews --- .../android/view/ImagePreviewViewHolder.java | 55 +++++++++++-------- 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java index 4fefb2ae5..856eb30bb 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java @@ -18,6 +18,7 @@ import org.briarproject.briar.R; import org.briarproject.briar.android.conversation.glide.GlideApp; import static android.view.View.INVISIBLE; +import static android.view.View.VISIBLE; import static com.bumptech.glide.load.engine.DiskCacheStrategy.NONE; import static com.bumptech.glide.load.resource.bitmap.DownsampleStrategy.FIT_CENTER; import static com.bumptech.glide.load.resource.drawable.DrawableTransitionOptions.withCrossFade; @@ -38,31 +39,37 @@ class ImagePreviewViewHolder extends ViewHolder { } void bind(ImagePreviewItem item) { - if (item.getItem() == null) return; // shows progress bar - GlideApp.with(imageView) - .load(item.getItem()) - .diskCacheStrategy(NONE) - .error(ERROR_RES) - .downsample(FIT_CENTER) - .transition(withCrossFade()) - .addListener(new RequestListener() { - @Override - public boolean onLoadFailed(@Nullable GlideException e, - Object model, Target target, - boolean isFirstResource) { - progressBar.setVisibility(INVISIBLE); - return false; - } + if (item.getItem() == null) { + progressBar.setVisibility(VISIBLE); + GlideApp.with(imageView) + .clear(imageView); + } else { + GlideApp.with(imageView) + .load(item.getItem()) + .diskCacheStrategy(NONE) + .error(ERROR_RES) + .downsample(FIT_CENTER) + .transition(withCrossFade()) + .addListener(new RequestListener() { + @Override + public boolean onLoadFailed(@Nullable GlideException e, + Object model, Target target, + boolean isFirstResource) { + progressBar.setVisibility(INVISIBLE); + return false; + } - @Override - public boolean onResourceReady(Drawable resource, - Object model, Target target, - DataSource dataSource, boolean isFirstResource) { - progressBar.setVisibility(INVISIBLE); - return false; - } - }) - .into(imageView); + @Override + public boolean onResourceReady(Drawable resource, + Object model, Target target, + DataSource dataSource, + boolean isFirstResource) { + progressBar.setVisibility(INVISIBLE); + return false; + } + }) + .into(imageView); + } } } From 67b7517f2b50e3981ad635fb8184bf4797aa37da Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 1 May 2019 16:06:09 -0300 Subject: [PATCH 13/17] [android] refactor AttachmentCreator to return a single LiveData --- .../attachment/AttachmentCreationTask.java | 6 +- .../android/attachment/AttachmentCreator.java | 148 +++++++++--------- .../attachment/AttachmentItemResult.java | 12 +- .../android/attachment/AttachmentManager.java | 4 +- .../android/attachment/AttachmentResult.java | 15 +- .../conversation/ConversationViewModel.java | 25 +-- .../briar/android/view/ImagePreview.java | 6 +- .../android/view/ImagePreviewAdapter.java | 13 +- .../view/TextAttachmentController.java | 50 +++--- 9 files changed, 145 insertions(+), 134 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java index 4bd89e3d9..1a1a5b7d2 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java @@ -14,7 +14,7 @@ import org.jsoup.UnsupportedMimeTypeException; import java.io.IOException; import java.io.InputStream; -import java.util.List; +import java.util.Collection; import java.util.logging.Logger; import static java.util.logging.Level.WARNING; @@ -34,7 +34,7 @@ class AttachmentCreationTask { private final MessagingManager messagingManager; private final ContentResolver contentResolver; private final GroupId groupId; - private final List uris; + private final Collection uris; private final boolean needsSize; @Nullable private volatile AttachmentCreator attachmentCreator; @@ -44,7 +44,7 @@ class AttachmentCreationTask { AttachmentCreationTask(MessagingManager messagingManager, ContentResolver contentResolver, AttachmentCreator attachmentCreator, GroupId groupId, - List uris, boolean needsSize) { + Collection uris, boolean needsSize) { this.messagingManager = messagingManager; this.contentResolver = contentResolver; this.groupId = groupId; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java index ea8341de6..d694868b7 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java @@ -24,14 +24,14 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Executor; import java.util.logging.Logger; 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.util.UiUtils.observeForeverOnce; import static org.briarproject.briar.api.messaging.MessagingConstants.MAX_IMAGE_SIZE; @NotNullByDefault @@ -45,14 +45,12 @@ public class AttachmentCreator { private final MessagingManager messagingManager; private final AttachmentRetriever retriever; - // store unsent items separately, as LiveData might not return latest value - private final Map unsentItems = - new ConcurrentHashMap<>(); - private final Map> - liveDataResult = new ConcurrentHashMap<>(); + private final CopyOnWriteArrayList uris = new CopyOnWriteArrayList<>(); + private final CopyOnWriteArrayList itemResults = + new CopyOnWriteArrayList<>(); @Nullable - private MutableLiveData liveDataFinished = null; + private volatile MutableLiveData result = null; @Nullable private AttachmentCreationTask task; @@ -65,56 +63,49 @@ public class AttachmentCreator { } @UiThread - public AttachmentResult storeAttachments(GroupId groupId, - Collection uris, boolean restart) { - List> itemResults = new ArrayList<>(); + public LiveData storeAttachments( + LiveData groupId, Collection newUris, boolean restart) { + MutableLiveData result; if (restart) { // This can happen due to configuration changes. - // So don't create new attachments, if we have (or creating) them. - // Instead, re-subscribe to the existing LiveData. - if (task == null || isNotStoring()) throw new AssertionError(); - for (Uri uri : uris) { - // We don't want to expose mutable(!) LiveData - LiveData liveData = - liveDataResult.get(uri); - if (liveData == null) throw new IllegalStateException(); - itemResults.add(liveData); - } - if (liveDataFinished == null) throw new IllegalStateException(); + // So don't create new attachments. They are already being created + // and returned by the existing LiveData. + result = this.result; + if (task == null || uris.isEmpty() || result == null) + throw new IllegalStateException(); + // A task is already running. It will update the result LiveData. + // So nothing more to do here. } else { - if (task != null && isNotStoring()) throw new AssertionError(); - List urisToStore = new ArrayList<>(); - for (Uri uri : uris) { - urisToStore.add(uri); - MutableLiveData liveData = - new MutableLiveData<>(); - liveDataResult.put(uri, liveData); - itemResults.add(liveData); - } - boolean needsSize = uris.size() == 1; - task = new AttachmentCreationTask(messagingManager, - app.getContentResolver(), this, groupId, urisToStore, - needsSize); - ioExecutor.execute(() -> task.storeAttachments()); - liveDataFinished = new MutableLiveData<>(); + if (this.result != null || !uris.isEmpty()) + throw new IllegalStateException(); + result = new MutableLiveData<>(); + this.result = result; + uris.addAll(newUris); + observeForeverOnce(groupId, id -> { + if (id == null) throw new IllegalStateException(); + boolean needsSize = uris.size() == 1; + task = new AttachmentCreationTask(messagingManager, + app.getContentResolver(), this, id, uris, needsSize); + ioExecutor.execute(() -> task.storeAttachments()); + }); } - return new AttachmentResult(itemResults, liveDataFinished); + return result; } @IoExecutor void onAttachmentHeaderReceived(Uri uri, AttachmentHeader h, boolean needsSize) { + MutableLiveData result = this.result; + if (result == null) return; // get and cache AttachmentItem for ImagePreview try { Attachment a = retriever.getMessageAttachment(h); AttachmentItem item = retriever.getAttachmentItem(h, a, needsSize); if (item.hasError()) throw new IOException(); - unsentItems.put(uri, item); - MutableLiveData result = - liveDataResult.get(uri); - if (result != null) { // might have been cleared on UiThread - result.postValue(new AttachmentItemResult(uri, item)); - } + AttachmentItemResult itemResult = + new AttachmentItemResult(uri, item); + itemResults.add(itemResult); + result.postValue(new AttachmentResult(itemResults, false)); } catch (IOException | DbException e) { logException(LOG, WARNING, e); onAttachmentError(uri, e); @@ -123,6 +114,9 @@ public class AttachmentCreator { @IoExecutor void onAttachmentError(Uri uri, Throwable t) { + MutableLiveData result = this.result; + if (result == null) return; + // get error message String errorMsg; if (t instanceof UnsupportedMimeTypeException) { String mimeType = ((UnsupportedMimeTypeException) t).getMimeType(); @@ -134,23 +128,29 @@ public class AttachmentCreator { } else { errorMsg = null; // generic error } - MutableLiveData result = liveDataResult.get(uri); - if (result != null) - result.postValue(new AttachmentItemResult(errorMsg)); + AttachmentItemResult itemResult = + new AttachmentItemResult(uri, errorMsg); + itemResults.add(itemResult); + result.postValue(new AttachmentResult(itemResults, false)); // expect to receive a cancel from the UI } @IoExecutor void onAttachmentCreationFinished() { - if (liveDataFinished != null) liveDataFinished.postValue(true); + if (uris.size() != itemResults.size()) + throw new IllegalStateException(); + MutableLiveData result = this.result; + if (result == null) return; + result.postValue(new AttachmentResult(itemResults, true)); } @UiThread public List getAttachmentHeadersForSending() { - List headers = - new ArrayList<>(unsentItems.values().size()); - for (AttachmentItem item : unsentItems.values()) { - headers.add(item.getHeader()); + List headers = 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(); + headers.add(itemResult.getItem().getHeader()); } return headers; } @@ -161,22 +161,28 @@ public class AttachmentCreator { * @param id The MessageId of the sent message. */ public void onAttachmentsSent(MessageId id) { - retriever.cachePut(id, new ArrayList<>(unsentItems.values())); + 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(); } + /** + * Needs to be called when created attachments will not be sent anymore. + */ @UiThread public void cancel() { if (task == null) throw new AssertionError(); task.cancel(); // let observers know that they can remove themselves - for (MutableLiveData liveData : liveDataResult - .values()) { - if (liveData.getValue() == null) { - liveData.setValue(null); - } + MutableLiveData result = this.result; + if (result != null) { + result.setValue(null); } - if (liveDataFinished != null) liveDataFinished.setValue(false); deleteUnsentAttachments(); resetState(); } @@ -184,20 +190,24 @@ public class AttachmentCreator { @UiThread private void resetState() { task = null; - liveDataResult.clear(); - liveDataFinished = null; - unsentItems.clear(); + uris.clear(); + itemResults.clear(); + result = null; } @UiThread public void deleteUnsentAttachments() { - // Make a copy for the IoExecutor as we clear the unsentItems soon - List itemsToDelete = - new ArrayList<>(unsentItems.values()); + // Make a copy for the IoExecutor as we clear the itemResults soon + List headers = new ArrayList<>(itemResults.size()); + for (AttachmentItemResult itemResult : itemResults) { + // check if we are trying to send attachment items with errors + if (itemResult.getItem() != null) + headers.add(itemResult.getItem().getHeader()); + } ioExecutor.execute(() -> { - for (AttachmentItem item : itemsToDelete) { + for (AttachmentHeader header : headers) { try { - messagingManager.removeAttachment(item.getHeader()); + messagingManager.removeAttachment(header); } catch (DbException e) { logException(LOG, WARNING, e); } @@ -205,8 +215,4 @@ public class AttachmentCreator { }); } - private boolean isNotStoring() { - return liveDataFinished == null; - } - } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItemResult.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItemResult.java index 1c5cc8dd3..1254a851d 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItemResult.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItemResult.java @@ -11,26 +11,24 @@ import javax.annotation.concurrent.Immutable; @NotNullByDefault public class AttachmentItemResult { - @Nullable private final Uri uri; @Nullable private final AttachmentItem item; @Nullable private final String errorMsg; - public AttachmentItemResult(Uri uri, AttachmentItem item) { + AttachmentItemResult(Uri uri, AttachmentItem item) { this.uri = uri; this.item = item; this.errorMsg = null; } - public AttachmentItemResult(@Nullable String errorMsg) { - this.uri = null; + AttachmentItemResult(Uri uri, @Nullable String errorMsg) { + this.uri = uri; this.item = null; this.errorMsg = errorMsg; } - @Nullable public Uri getUri() { return uri; } @@ -40,8 +38,8 @@ public class AttachmentItemResult { return item; } - public boolean isError() { - return errorMsg != null; + public boolean hasError() { + return item == null; } @Nullable diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java index f478b2c53..f69f6085a 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentManager.java @@ -1,5 +1,6 @@ package org.briarproject.briar.android.attachment; +import android.arch.lifecycle.LiveData; import android.net.Uri; import android.support.annotation.UiThread; @@ -11,7 +12,8 @@ import java.util.List; @UiThread public interface AttachmentManager { - AttachmentResult storeAttachments(Collection uri, boolean restart); + LiveData storeAttachments(Collection uri, + boolean restart); List getAttachmentHeadersForSending(); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentResult.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentResult.java index 4e347b6fb..776d2ab59 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentResult.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentResult.java @@ -1,7 +1,5 @@ package org.briarproject.briar.android.attachment; -import android.arch.lifecycle.LiveData; - import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import java.util.Collection; @@ -12,21 +10,20 @@ import javax.annotation.concurrent.Immutable; @NotNullByDefault public class AttachmentResult { - private final Collection> itemResults; - private final LiveData finished; + private final Collection itemResults; + private final boolean finished; - public AttachmentResult( - Collection> itemResults, - LiveData finished) { + public AttachmentResult(Collection itemResults, + boolean finished) { this.itemResults = itemResults; this.finished = finished; } - public Collection> getItemResults() { + public Collection getItemResults() { return itemResults; } - public LiveData getFinished() { + public boolean isFinished() { return finished; } 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 40b21f0f7..158e26e84 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 @@ -54,6 +54,7 @@ import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.LogUtils.now; import static org.briarproject.briar.android.attachment.AttachmentDimensions.getAttachmentDimensions; import static org.briarproject.briar.android.settings.SettingsFragment.SETTINGS_NAMESPACE; +import static org.briarproject.briar.android.util.UiUtils.observeForeverOnce; @NotNullByDefault public class ConversationViewModel extends AndroidViewModel @@ -80,13 +81,12 @@ public class ConversationViewModel extends AndroidViewModel @Nullable private ContactId contactId = null; - @Nullable - private volatile GroupId messagingGroupId = null; private final MutableLiveData contact = new MutableLiveData<>(); private final LiveData contactAuthorId = Transformations.map(contact, c -> c.getAuthor().getId()); private final LiveData contactName = Transformations.map(contact, UiUtils::getContactDisplayName); + private final LiveData messagingGroupId; private final MutableLiveData imageSupport = new MutableLiveData<>(); private final MutableLiveEvent showImageOnboarding = @@ -120,6 +120,8 @@ public class ConversationViewModel extends AndroidViewModel getAttachmentDimensions(application.getResources())); this.attachmentCreator = new AttachmentCreator(getApplication(), ioExecutor, messagingManager, attachmentRetriever); + messagingGroupId = Transformations + .map(contact, c -> messagingManager.getContactGroup(c).getId()); contactDeleted.setValue(false); } @@ -150,9 +152,6 @@ public class ConversationViewModel extends AndroidViewModel contact.postValue(c); logDuration(LOG, "Loading contact", start); start = now(); - messagingGroupId = messagingManager.getContactGroup(c).getId(); - logDuration(LOG, "Load conversation GroupId", start); - start = now(); checkFeaturesAndOnboarding(contactId); logDuration(LOG, "Checking for image support", start); } catch (NoSuchContactException e) { @@ -189,18 +188,20 @@ public class ConversationViewModel extends AndroidViewModel void sendMessage(@Nullable String text, List attachmentHeaders, long timestamp) { - GroupId groupId = messagingGroupId; - if (groupId == null) throw new IllegalStateException(); - createMessage(groupId, text, attachmentHeaders, timestamp); + // messagingGroupId is loaded with the contact + observeForeverOnce(messagingGroupId, groupId -> { + if (groupId == null) throw new IllegalStateException(); + createMessage(groupId, text, attachmentHeaders, timestamp); + }); } @Override @UiThread - public AttachmentResult storeAttachments(Collection uris, + public LiveData storeAttachments(Collection uris, boolean restart) { - GroupId groupId = messagingGroupId; - if (groupId == null) throw new IllegalStateException(); - return attachmentCreator.storeAttachments(groupId, uris, restart); + // messagingGroupId is loaded with the contact + return attachmentCreator + .storeAttachments(messagingGroupId, uris, restart); } @Override diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java index 2a0585371..7bdb7d93f 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java @@ -15,6 +15,7 @@ import java.util.Collection; import static android.content.Context.LAYOUT_INFLATER_SERVICE; import static android.support.v4.content.ContextCompat.getColor; +import static android.support.v7.widget.RecyclerView.NO_POSITION; import static android.view.ViewGroup.LayoutParams.MATCH_PARENT; import static java.util.Objects.requireNonNull; @@ -75,7 +76,10 @@ public class ImagePreview extends ConstraintLayout { void loadPreviewImage(AttachmentItemResult result) { ImagePreviewAdapter adapter = ((ImagePreviewAdapter) imageList.getAdapter()); - requireNonNull(adapter).loadItemPreview(result); + int pos = requireNonNull(adapter).loadItemPreview(result); + if (pos != NO_POSITION) { + imageList.smoothScrollToPosition(pos); + } } interface ImagePreviewListener { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java index dcbfd9ec3..5e7bf3253 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java @@ -50,14 +50,17 @@ class ImagePreviewAdapter extends Adapter { return items.size(); } - void loadItemPreview(AttachmentItemResult result) { - ImagePreviewItem newItem = - new ImagePreviewItem(requireNonNull(result.getUri())); + int loadItemPreview(AttachmentItemResult result) { + ImagePreviewItem newItem = new ImagePreviewItem(result.getUri()); int pos = items.indexOf(newItem); if (pos == NO_POSITION) throw new AssertionError(); ImagePreviewItem item = items.get(pos); - item.setItem(requireNonNull(result.getItem())); - notifyItemChanged(pos, item); + if (item.getItem() == null) { + item.setItem(requireNonNull(result.getItem())); + notifyItemChanged(pos, item); + return pos; + } + return NO_POSITION; } } 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 5f549a863..34a097846 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 @@ -24,6 +24,7 @@ import org.briarproject.briar.android.attachment.AttachmentResult; import org.briarproject.briar.android.view.ImagePreview.ImagePreviewListener; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt; @@ -172,40 +173,39 @@ public class TextAttachmentController extends TextSendController List items = ImagePreviewItem.fromUris(imageUris); imagePreview.showPreview(items); // store attachments and show preview when successful - AttachmentResult result = + LiveData result = attachmentManager.storeAttachments(imageUris, restart); - for (LiveData liveData : result - .getItemResults()) { - onLiveDataReturned(liveData); - } - result.getFinished().observe(imageListener, new Observer() { + result.observe(imageListener, new Observer() { @Override - public void onChanged(@Nullable Boolean finished) { - if (finished != null && finished) onAllAttachmentsCreated(); - result.getFinished().removeObserver(this); - } - }); - } - - private void onLiveDataReturned(LiveData liveData) { - liveData.observe(imageListener, new Observer() { - @Override - public void onChanged(@Nullable AttachmentItemResult result) { - if (result != null) { - onAttachmentResultReceived(result); + public void onChanged(@Nullable AttachmentResult attachmentResult) { + if (attachmentResult == null) { + // The fresh LiveData was deliberately set to null. + // This means that we can stop observing it. + result.removeObserver(this); + } else { + boolean noError = onNewAttachmentItemResults( + attachmentResult.getItemResults()); + if (noError && attachmentResult.isFinished()) { + onAllAttachmentsCreated(); + result.removeObserver(this); + } } - liveData.removeObserver(this); } }); } - private void onAttachmentResultReceived(AttachmentItemResult result) { + private boolean onNewAttachmentItemResults( + Collection itemResults) { if (!loadingUris) throw new AssertionError(); - if (result.isError() || result.getUri() == null) { - onError(result.getErrorMsg()); - } else { - imagePreview.loadPreviewImage(result); + for (AttachmentItemResult result : itemResults) { + if (result.hasError()) { + onError(result.getErrorMsg()); + return false; + } else { + imagePreview.loadPreviewImage(result); + } } + return true; } private void onAllAttachmentsCreated() { From 4ee4905e067ead386be799dbd5e64756b0b925aa Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 15 May 2019 16:45:53 -0300 Subject: [PATCH 14/17] [android] migrate added conversation header to new LiveEvent --- .../android/conversation/ConversationActivity.java | 3 +-- .../android/conversation/ConversationViewModel.java | 13 ++++--------- 2 files changed, 5 insertions(+), 11 deletions(-) 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 925655815..dfbddcd7a 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 @@ -243,7 +243,7 @@ public class ConversationActivity extends BriarActivity requireNonNull(deleted); if (deleted) finish(); }); - viewModel.getAddedPrivateMessage().observe(this, + viewModel.getAddedPrivateMessage().observeEvent(this, this::onAddedPrivateMessage); setTransitionName(toolbarAvatar, getAvatarTransitionName(contactId)); @@ -678,7 +678,6 @@ public class ConversationActivity extends BriarActivity private void onAddedPrivateMessage(@Nullable PrivateMessageHeader h) { if (h == null) return; addConversationItem(h.accept(visitor)); - viewModel.onAddedPrivateMessageSeen(); } private void askToRemoveContact() { 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 158e26e84..85a208228 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 @@ -97,8 +97,8 @@ public class ConversationViewModel extends AndroidViewModel new MutableLiveData<>(); private final MutableLiveData contactDeleted = new MutableLiveData<>(); - private final MutableLiveData addedHeader = - new MutableLiveData<>(); + private final MutableLiveEvent addedHeader = + new MutableLiveEvent<>(); @Inject ConversationViewModel(Application application, @@ -301,18 +301,13 @@ public class ConversationViewModel extends AndroidViewModel text != null, attachments); attachmentCreator.onAttachmentsSent(m.getMessage().getId()); // TODO add text to cache when available here - addedHeader.postValue(h); + addedHeader.postEvent(h); } catch (DbException e) { logException(LOG, WARNING, e); } }); } - @UiThread - void onAddedPrivateMessageSeen() { - addedHeader.setValue(null); - } - AttachmentRetriever getAttachmentRetriever() { return attachmentRetriever; } @@ -349,7 +344,7 @@ public class ConversationViewModel extends AndroidViewModel return contactDeleted; } - LiveData getAddedPrivateMessage() { + LiveEvent getAddedPrivateMessage() { return addedHeader; } From c07a0a2fd7ec1f3cb11c76a2090eb1a18127e8ed Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 14 Jun 2019 09:22:03 -0300 Subject: [PATCH 15/17] [android] address review comments for rejecting unsupported images --- .../attachment/AttachmentCreationTask.java | 3 +- .../android/attachment/AttachmentCreator.java | 54 ++++++++++--------- .../conversation/ConversationViewModel.java | 9 ++-- 3 files changed, 36 insertions(+), 30 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java index 1a1a5b7d2..d9e77d1d8 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreationTask.java @@ -106,8 +106,7 @@ class AttachmentCreationTask { return h; } - private boolean isValidMimeType(@Nullable String mimeType) { - if (mimeType == null) return false; + private boolean isValidMimeType(String mimeType) { for (String supportedType : IMAGE_MIME_TYPES) { if (supportedType.equals(mimeType)) return true; } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java index d694868b7..a8af9d89a 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java @@ -64,31 +64,35 @@ public class AttachmentCreator { @UiThread public LiveData storeAttachments( - LiveData groupId, Collection newUris, boolean restart) { - MutableLiveData result; - if (restart) { - // This can happen due to configuration changes. - // So don't create new attachments. They are already being created - // and returned by the existing LiveData. - result = this.result; - if (task == null || uris.isEmpty() || result == null) - throw new IllegalStateException(); - // A task is already running. It will update the result LiveData. - // So nothing more to do here. - } else { - if (this.result != null || !uris.isEmpty()) - throw new IllegalStateException(); - result = new MutableLiveData<>(); - this.result = result; - uris.addAll(newUris); - observeForeverOnce(groupId, id -> { - if (id == null) throw new IllegalStateException(); - boolean needsSize = uris.size() == 1; - task = new AttachmentCreationTask(messagingManager, - app.getContentResolver(), this, id, uris, needsSize); - ioExecutor.execute(() -> task.storeAttachments()); - }); - } + LiveData groupId, Collection newUris) { + if (this.result != null || !uris.isEmpty()) + throw new IllegalStateException(); + MutableLiveData result = new MutableLiveData<>(); + this.result = result; + uris.addAll(newUris); + observeForeverOnce(groupId, id -> { + if (id == null) throw new IllegalStateException(); + boolean needsSize = uris.size() == 1; + task = new AttachmentCreationTask(messagingManager, + app.getContentResolver(), this, id, uris, needsSize); + ioExecutor.execute(() -> task.storeAttachments()); + }); + return result; + } + + /** + * This should be only called after configuration changes. + * In this case we should not create new attachments. + * They are already being created and returned by the existing LiveData. + */ + @UiThread + public LiveData getLiveAttachments() { + // + MutableLiveData result = this.result; + if (task == null || uris.isEmpty() || result == null) + throw new IllegalStateException(); + // A task is already running. It will update the result LiveData. + // So nothing more to do here. return result; } 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 85a208228..a746f8ac6 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 @@ -199,9 +199,12 @@ public class ConversationViewModel extends AndroidViewModel @UiThread public LiveData storeAttachments(Collection uris, boolean restart) { - // messagingGroupId is loaded with the contact - return attachmentCreator - .storeAttachments(messagingGroupId, uris, restart); + if (restart) { + return attachmentCreator.getLiveAttachments(); + } else { + // messagingGroupId is loaded with the contact + return attachmentCreator.storeAttachments(messagingGroupId, uris); + } } @Override From 1f91842c5274a44d6b7e1f46a5471e9df693ce56 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 14 Jun 2019 10:03:43 -0300 Subject: [PATCH 16/17] [android] re-use the same LiveData for AttachmentResults --- .../android/attachment/AttachmentCreator.java | 26 +++++-------------- .../conversation/ConversationViewModel.java | 7 ++++- 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java index a8af9d89a..41dd9ea3b 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java @@ -49,8 +49,8 @@ public class AttachmentCreator { private final CopyOnWriteArrayList itemResults = new CopyOnWriteArrayList<>(); - @Nullable - private volatile MutableLiveData result = null; + private final MutableLiveData result = + new MutableLiveData<>(); @Nullable private AttachmentCreationTask task; @@ -65,10 +65,8 @@ public class AttachmentCreator { @UiThread public LiveData storeAttachments( LiveData groupId, Collection newUris) { - if (this.result != null || !uris.isEmpty()) + if (task != null || !uris.isEmpty()) throw new IllegalStateException(); - MutableLiveData result = new MutableLiveData<>(); - this.result = result; uris.addAll(newUris); observeForeverOnce(groupId, id -> { if (id == null) throw new IllegalStateException(); @@ -87,9 +85,7 @@ public class AttachmentCreator { */ @UiThread public LiveData getLiveAttachments() { - // - MutableLiveData result = this.result; - if (task == null || uris.isEmpty() || result == null) + if (task == null || uris.isEmpty()) throw new IllegalStateException(); // A task is already running. It will update the result LiveData. // So nothing more to do here. @@ -99,8 +95,6 @@ public class AttachmentCreator { @IoExecutor void onAttachmentHeaderReceived(Uri uri, AttachmentHeader h, boolean needsSize) { - MutableLiveData result = this.result; - if (result == null) return; // get and cache AttachmentItem for ImagePreview try { Attachment a = retriever.getMessageAttachment(h); @@ -118,8 +112,6 @@ public class AttachmentCreator { @IoExecutor void onAttachmentError(Uri uri, Throwable t) { - MutableLiveData result = this.result; - if (result == null) return; // get error message String errorMsg; if (t instanceof UnsupportedMimeTypeException) { @@ -143,8 +135,6 @@ public class AttachmentCreator { void onAttachmentCreationFinished() { if (uris.size() != itemResults.size()) throw new IllegalStateException(); - MutableLiveData result = this.result; - if (result == null) return; result.postValue(new AttachmentResult(itemResults, true)); } @@ -164,6 +154,7 @@ public class AttachmentCreator { * * @param id The MessageId of the sent message. */ + @UiThread public void onAttachmentsSent(MessageId id) { List items = new ArrayList<>(itemResults.size()); for (AttachmentItemResult itemResult : itemResults) { @@ -182,11 +173,6 @@ public class AttachmentCreator { public void cancel() { if (task == null) throw new AssertionError(); task.cancel(); - // let observers know that they can remove themselves - MutableLiveData result = this.result; - if (result != null) { - result.setValue(null); - } deleteUnsentAttachments(); resetState(); } @@ -196,7 +182,7 @@ public class AttachmentCreator { task = null; uris.clear(); itemResults.clear(); - result = null; + result.setValue(null); } @UiThread 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 a746f8ac6..0c5fd7649 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 @@ -6,6 +6,7 @@ import android.arch.lifecycle.LiveData; import android.arch.lifecycle.MutableLiveData; import android.arch.lifecycle.Transformations; import android.net.Uri; +import android.os.Handler; import android.support.annotation.Nullable; import android.support.annotation.UiThread; @@ -46,6 +47,7 @@ import java.util.logging.Logger; import javax.inject.Inject; +import static android.os.Looper.getMainLooper; import static java.util.Objects.requireNonNull; import static java.util.logging.Level.WARNING; import static java.util.logging.Logger.getLogger; @@ -302,7 +304,10 @@ public class ConversationViewModel extends AndroidViewModel message.getId(), message.getGroupId(), message.getTimestamp(), true, true, false, false, text != null, attachments); - attachmentCreator.onAttachmentsSent(m.getMessage().getId()); + MessageId id = m.getMessage().getId(); + // use the UiThread to call onAttachmentsSent + new Handler(getMainLooper()).post(() -> + attachmentCreator.onAttachmentsSent(id)); // TODO add text to cache when available here addedHeader.postEvent(h); } catch (DbException e) { From ad2d3e70d633587d8e286e25f81e56cd3fcf6f9b Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 17 Jun 2019 13:22:38 -0300 Subject: [PATCH 17/17] [android] address thread-safety issues of attachment creation --- .../android/attachment/AttachmentCreator.java | 20 ++++++++++++++----- .../conversation/ConversationViewModel.java | 7 +------ 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java index 41dd9ea3b..c22d46024 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java @@ -103,7 +103,7 @@ public class AttachmentCreator { AttachmentItemResult itemResult = new AttachmentItemResult(uri, item); itemResults.add(itemResult); - result.postValue(new AttachmentResult(itemResults, false)); + result.postValue(getResult(false)); } catch (IOException | DbException e) { logException(LOG, WARNING, e); onAttachmentError(uri, e); @@ -127,15 +127,13 @@ public class AttachmentCreator { AttachmentItemResult itemResult = new AttachmentItemResult(uri, errorMsg); itemResults.add(itemResult); - result.postValue(new AttachmentResult(itemResults, false)); + result.postValue(getResult(false)); // expect to receive a cancel from the UI } @IoExecutor void onAttachmentCreationFinished() { - if (uris.size() != itemResults.size()) - throw new IllegalStateException(); - result.postValue(new AttachmentResult(itemResults, true)); + result.postValue(getResult(true)); } @UiThread @@ -205,4 +203,16 @@ public class AttachmentCreator { }); } + private AttachmentResult getResult(boolean finished) { + // Make a copy of the list, + // because our copy will continue to change in the background. + // (As it's a CopyOnWriteArrayList, + // the code that receives the result can safely do simple things + // like iterating over the list, + // but anything that involves calling more than one list method + // is still unsafe.) + Collection items = new ArrayList<>(itemResults); + return new AttachmentResult(items, finished); + } + } 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 0c5fd7649..a1f4428b9 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 @@ -6,7 +6,6 @@ import android.arch.lifecycle.LiveData; import android.arch.lifecycle.MutableLiveData; import android.arch.lifecycle.Transformations; import android.net.Uri; -import android.os.Handler; import android.support.annotation.Nullable; import android.support.annotation.UiThread; @@ -47,7 +46,6 @@ import java.util.logging.Logger; import javax.inject.Inject; -import static android.os.Looper.getMainLooper; import static java.util.Objects.requireNonNull; import static java.util.logging.Level.WARNING; import static java.util.logging.Logger.getLogger; @@ -294,6 +292,7 @@ public class ConversationViewModel extends AndroidViewModel private void storeMessage(PrivateMessage m, @Nullable String text, List attachments) { + attachmentCreator.onAttachmentsSent(m.getMessage().getId()); dbExecutor.execute(() -> { try { long start = now(); @@ -304,10 +303,6 @@ public class ConversationViewModel extends AndroidViewModel message.getId(), message.getGroupId(), message.getTimestamp(), true, true, false, false, text != null, attachments); - MessageId id = m.getMessage().getId(); - // use the UiThread to call onAttachmentsSent - new Handler(getMainLooper()).post(() -> - attachmentCreator.onAttachmentsSent(id)); // TODO add text to cache when available here addedHeader.postEvent(h); } catch (DbException e) {