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 4bd89e3d9d1133cd7df0ddf00abe0b69da11e2ea..1a1a5b7d260200d59d9906a4d6665170b132c906 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<Uri> uris; + private final Collection<Uri> 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<Uri> uris, boolean needsSize) { + Collection<Uri> 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 ea8341de6f9e800fb07daa3d78c45b29b4f95d4a..d694868b7adb98beaf351c3063b917f8f94ad815 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<Uri, AttachmentItem> unsentItems = - new ConcurrentHashMap<>(); - private final Map<Uri, MutableLiveData<AttachmentItemResult>> - liveDataResult = new ConcurrentHashMap<>(); + private final CopyOnWriteArrayList<Uri> uris = new CopyOnWriteArrayList<>(); + private final CopyOnWriteArrayList<AttachmentItemResult> itemResults = + new CopyOnWriteArrayList<>(); @Nullable - private MutableLiveData<Boolean> liveDataFinished = null; + private volatile MutableLiveData<AttachmentResult> result = null; @Nullable private AttachmentCreationTask task; @@ -65,56 +63,49 @@ public class AttachmentCreator { } @UiThread - public AttachmentResult storeAttachments(GroupId groupId, - Collection<Uri> uris, boolean restart) { - List<LiveData<AttachmentItemResult>> itemResults = new ArrayList<>(); + public LiveData<AttachmentResult> storeAttachments( + LiveData<GroupId> groupId, Collection<Uri> newUris, boolean restart) { + MutableLiveData<AttachmentResult> 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<AttachmentItemResult> 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<Uri> urisToStore = new ArrayList<>(); - for (Uri uri : uris) { - urisToStore.add(uri); - MutableLiveData<AttachmentItemResult> 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<AttachmentResult> 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<AttachmentItemResult> 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<AttachmentResult> 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<AttachmentItemResult> 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<AttachmentResult> result = this.result; + if (result == null) return; + result.postValue(new AttachmentResult(itemResults, true)); } @UiThread public List<AttachmentHeader> getAttachmentHeadersForSending() { - List<AttachmentHeader> headers = - new ArrayList<>(unsentItems.values().size()); - for (AttachmentItem item : unsentItems.values()) { - headers.add(item.getHeader()); + List<AttachmentHeader> 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<AttachmentItem> 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<AttachmentItemResult> liveData : liveDataResult - .values()) { - if (liveData.getValue() == null) { - liveData.setValue(null); - } + MutableLiveData<AttachmentResult> 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<AttachmentItem> itemsToDelete = - new ArrayList<>(unsentItems.values()); + // Make a copy for the IoExecutor as we clear the itemResults soon + List<AttachmentHeader> 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 1c5cc8dd382b061d153e605da0139060efb38142..1254a851d8b3b4c0f2fed7d95003258ce4dae0f9 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 f478b2c53d49bfe8dd9fdff9070ef607df691a85..f69f6085ae98782088a6e7172bd71ef6f1f463ca 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> uri, boolean restart); + LiveData<AttachmentResult> storeAttachments(Collection<Uri> uri, + boolean restart); List<AttachmentHeader> 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 4e347b6fbe29641eb03de52e6b47f60ce915ec01..776d2ab59b57425882dd278401d3c71ac7b0f3fb 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<LiveData<AttachmentItemResult>> itemResults; - private final LiveData<Boolean> finished; + private final Collection<AttachmentItemResult> itemResults; + private final boolean finished; - public AttachmentResult( - Collection<LiveData<AttachmentItemResult>> itemResults, - LiveData<Boolean> finished) { + public AttachmentResult(Collection<AttachmentItemResult> itemResults, + boolean finished) { this.itemResults = itemResults; this.finished = finished; } - public Collection<LiveData<AttachmentItemResult>> getItemResults() { + public Collection<AttachmentItemResult> getItemResults() { return itemResults; } - public LiveData<Boolean> 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 40b21f0f73543b0ad53c8c42b2bb84dc2823b7f7..158e26e8437a3cf6e22d9ac681cba09679ba6c08 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> contact = new MutableLiveData<>(); private final LiveData<AuthorId> contactAuthorId = Transformations.map(contact, c -> c.getAuthor().getId()); private final LiveData<String> contactName = Transformations.map(contact, UiUtils::getContactDisplayName); + private final LiveData<GroupId> messagingGroupId; private final MutableLiveData<Boolean> imageSupport = new MutableLiveData<>(); private final MutableLiveEvent<Boolean> 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<AttachmentHeader> 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<Uri> uris, + public LiveData<AttachmentResult> storeAttachments(Collection<Uri> 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 2a0585371e2d7b4ae85e7a88b0e5b77e557c148d..7bdb7d93f8abf4757dd309bb17e06a6822ad2aaa 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 dcbfd9ec3e12e14a54b108157c1b66f0c3b2be54..5e7bf32532a2ae20e2399014af53693d9f1291e1 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<ImagePreviewViewHolder> { 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 5f549a86346e06cbb5ba6f46f6d9421d8c27e4c4..34a0978464d68f3450bb8cfc11357ba38ad71b39 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<ImagePreviewItem> items = ImagePreviewItem.fromUris(imageUris); imagePreview.showPreview(items); // store attachments and show preview when successful - AttachmentResult result = + LiveData<AttachmentResult> result = attachmentManager.storeAttachments(imageUris, restart); - for (LiveData<AttachmentItemResult> liveData : result - .getItemResults()) { - onLiveDataReturned(liveData); - } - result.getFinished().observe(imageListener, new Observer<Boolean>() { + result.observe(imageListener, new Observer<AttachmentResult>() { @Override - public void onChanged(@Nullable Boolean finished) { - if (finished != null && finished) onAllAttachmentsCreated(); - result.getFinished().removeObserver(this); - } - }); - } - - private void onLiveDataReturned(LiveData<AttachmentItemResult> liveData) { - liveData.observe(imageListener, new Observer<AttachmentItemResult>() { - @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<AttachmentItemResult> 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() {