From 9d9bc4ca84dd02cfa18c7450a99049fa17ff8cad Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Sun, 14 Apr 2019 12:31:31 -0300 Subject: [PATCH] [android] Let AttachmentCreator return same LiveData after configuration changes --- .../android/attachment/AttachmentCreator.java | 57 +++++++++++-------- .../android/attachment/AttachmentManager.java | 2 +- .../conversation/ConversationViewModel.java | 8 +-- .../view/TextAttachmentController.java | 11 ++-- 4 files changed, 43 insertions(+), 35 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 { + 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(); } -- GitLab