From d670179e30ce3032a641e0957eb3906255a11645 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 27 Jan 2021 15:18:09 -0300 Subject: [PATCH] Access MessageTree only on UiThread and improve code in the process --- .../briar/android/forum/ForumViewModel.java | 49 ++++++++------- .../conversation/GroupViewModel.java | 53 ++++++++-------- .../android/threaded/ThreadListViewModel.java | 60 +++++-------------- 3 files changed, 70 insertions(+), 92 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/forum/ForumViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/forum/ForumViewModel.java index 5f5c51f1b..87aca3fb1 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/forum/ForumViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/forum/ForumViewModel.java @@ -94,8 +94,9 @@ class ForumViewModel extends ThreadListViewModel { ForumPostReceivedEvent f = (ForumPostReceivedEvent) e; if (f.getGroupId().equals(groupId)) { LOG.info("Forum post received, adding..."); - ForumPostItem item = buildItem(f.getHeader(), f.getText()); - addItem(item); + ForumPostItem item = + new ForumPostItem(f.getHeader(), f.getText()); + addItem(item, false); } } else if (e instanceof ForumInvitationResponseReceivedEvent) { ForumInvitationResponseReceivedEvent f = @@ -140,10 +141,23 @@ class ForumViewModel extends ThreadListViewModel { List headers = forumManager.getPostHeaders(txn, groupId); logDuration(LOG, "Loading headers", start); - return createItems(txn, headers, this::buildItem); + start = now(); + List items = new ArrayList<>(); + for (ForumPostHeader header : headers) { + items.add(loadItem(txn, header)); + } + logDuration(LOG, "Loading bodies and creating items", start); + return items; }, this::setItems); } + private ForumPostItem loadItem(Transaction txn, PostHeader header) + throws DbException { + if (!(header instanceof ForumPostHeader)) throw new AssertionError(); + String text = forumManager.getPostText(txn, header.getId()); + return new ForumPostItem((ForumPostHeader) header, text); + } + @Override public void createAndStoreMessage(String text, @Nullable MessageId parentId) { @@ -171,26 +185,15 @@ class ForumViewModel extends ThreadListViewModel { } private void storePost(ForumPost msg, String text) { - runOnDbThread(() -> { - try { - long start = now(); - ForumPostHeader header = forumManager.addLocalPost(msg); - addItemAsync(buildItem(header, text)); - logDuration(LOG, "Storing forum post", start); - } catch (DbException e) { - logException(LOG, WARNING, e); - } - }); - } - - private ForumPostItem buildItem(ForumPostHeader header, String text) { - return new ForumPostItem(header, text); - } - - @Override - protected String loadMessageText(Transaction txn, PostHeader header) - throws DbException { - return forumManager.getPostText(txn, header.getId()); + runOnDbThread(false, txn -> { + long start = now(); + ForumPostHeader header = forumManager.addLocalPost(txn, msg); + logDuration(LOG, "Storing forum post", start); + txn.attach(() -> { + ForumPostItem item = new ForumPostItem(header, text); + addItem(item, true); + }); + }, e -> logException(LOG, WARNING, e)); } @Override diff --git a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupViewModel.java index 9020efe1b..2b25ee75e 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupViewModel.java @@ -27,7 +27,6 @@ import org.briarproject.briar.android.viewmodel.MutableLiveEvent; import org.briarproject.briar.api.android.AndroidNotificationManager; import org.briarproject.briar.api.client.MessageTracker; import org.briarproject.briar.api.client.MessageTracker.GroupCount; -import org.briarproject.briar.api.client.PostHeader; import org.briarproject.briar.api.privategroup.GroupMember; import org.briarproject.briar.api.privategroup.GroupMessage; import org.briarproject.briar.api.privategroup.GroupMessageFactory; @@ -105,7 +104,7 @@ class GroupViewModel extends ThreadListViewModel { if (!g.isLocal() && g.getGroupId().equals(groupId)) { LOG.info("Group message received, adding..."); GroupMessageItem item = buildItem(g.getHeader(), g.getText()); - addItem(item); + addItem(item, false); // In case the join message comes from the creator, // we need to reload the sharing contacts // in case it was delayed and the sharing count is wrong (#850). @@ -171,10 +170,28 @@ class GroupViewModel extends ThreadListViewModel { List headers = privateGroupManager.getHeaders(txn, groupId); logDuration(LOG, "Loading headers", start); - return createItems(txn, headers, this::buildItem); + start = now(); + List items = new ArrayList<>(); + for (GroupMessageHeader header : headers) { + items.add(loadItem(txn, header)); + } + logDuration(LOG, "Loading bodies and creating items", start); + return items; }, this::setItems); } + private GroupMessageItem loadItem(Transaction txn, + GroupMessageHeader header) throws DbException { + String text; + if (header instanceof JoinMessageHeader) { + // will be looked up later + text = ""; + } else { + text = privateGroupManager.getMessageText(txn, header.getId()); + } + return buildItem(header, text); + } + private GroupMessageItem buildItem(GroupMessageHeader header, String text) { if (header instanceof JoinMessageHeader) { return new JoinMessageItem((JoinMessageHeader) header, text); @@ -182,16 +199,6 @@ class GroupViewModel extends ThreadListViewModel { return new GroupMessageItem(header, text); } - @Override - protected String loadMessageText( - Transaction txn, PostHeader header) throws DbException { - if (header instanceof JoinMessageHeader) { - // will be looked up later - return ""; - } - return privateGroupManager.getMessageText(txn, header.getId()); - } - @Override public void createAndStoreMessage(String text, @Nullable MessageId parentId) { @@ -222,17 +229,15 @@ class GroupViewModel extends ThreadListViewModel { } private void storePost(GroupMessage msg, String text) { - runOnDbThread(() -> { - try { - long start = now(); - GroupMessageHeader header = - privateGroupManager.addLocalMessage(msg); - addItemAsync(buildItem(header, text)); - logDuration(LOG, "Storing group message", start); - } catch (DbException e) { - logException(LOG, WARNING, e); - } - }); + runOnDbThread(false, txn -> { + long start = now(); + GroupMessageHeader header = + privateGroupManager.addLocalMessage(txn, msg); + logDuration(LOG, "Storing group message", start); + txn.attach(() -> + addItem(buildItem(header, text), true) + ); + }, e -> logException(LOG, WARNING, e)); } @Override diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListViewModel.java index c28847107..3e3adfeba 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListViewModel.java @@ -5,7 +5,6 @@ import android.app.Application; import org.briarproject.bramble.api.crypto.CryptoExecutor; import org.briarproject.bramble.api.db.DatabaseExecutor; import org.briarproject.bramble.api.db.DbException; -import org.briarproject.bramble.api.db.Transaction; import org.briarproject.bramble.api.db.TransactionManager; import org.briarproject.bramble.api.event.Event; import org.briarproject.bramble.api.event.EventBus; @@ -26,11 +25,8 @@ import org.briarproject.briar.android.viewmodel.LiveResult; import org.briarproject.briar.api.android.AndroidNotificationManager; import org.briarproject.briar.api.client.MessageTracker; import org.briarproject.briar.api.client.MessageTree; -import org.briarproject.briar.api.client.PostHeader; import org.briarproject.briar.client.MessageTreeImpl; -import java.util.ArrayList; -import java.util.Collection; import java.util.List; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicReference; @@ -46,9 +42,7 @@ import androidx.lifecycle.MutableLiveData; import static java.util.logging.Level.INFO; import static java.util.logging.Level.WARNING; 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; @MethodsNotNullByDefault @ParametersNotNullByDefault @@ -67,6 +61,7 @@ public abstract class ThreadListViewModel private final MessageTracker messageTracker; private final EventBus eventBus; + // UIThread private final MessageTree messageTree = new MessageTreeImpl<>(); private final MutableLiveData>> items = new MutableLiveData<>(); @@ -175,55 +170,34 @@ public abstract class ThreadListViewModel * Loads the ContactIds of all contacts the group is shared with * and adds them to {@link SharingController}. */ - public abstract void loadSharingContacts(); + protected abstract void loadSharingContacts(); @UiThread protected void setItems(LiveResult> items) { - this.items.setValue(items); - } - - @DatabaseExecutor - protected List createItems( - Transaction txn, Collection headers, ItemGetter itemGetter) - throws DbException { - long start = now(); - List items = new ArrayList<>(); - for (H header : headers) { - String text = loadMessageText(txn, header); - items.add(itemGetter.getItem(header, text)); + if (items.hasError()) { + this.items.setValue(items); + } else { + messageTree.clear(); + // not null, because hasError() is false + messageTree.add(items.getResultOrNull()); + LiveResult> result = + new LiveResult<>(messageTree.depthFirstOrder()); + this.items.setValue(result); } - logDuration(LOG, "Loading bodies and creating items", start); - - messageTree.clear(); - messageTree.add(items); - return messageTree.depthFirstOrder(); } /** * Add a remote item on the UI thread. - * The list will not scroll, but show an unread indicator. + * + * @param scrollToItem whether the list will scroll to the newly added item */ @UiThread - protected void addItem(I item) { + protected void addItem(I item, boolean scrollToItem) { messageTree.add(item); + if (scrollToItem) this.scrollToItem.set(item.getId()); items.setValue(new LiveResult<>(messageTree.depthFirstOrder())); } - /** - * Add a local item from the DB thread. - * The list will scroll to the new item. - */ - @DatabaseExecutor - protected void addItemAsync(I item) { - messageTree.add(item); - scrollToItem.set(item.getId()); - items.postValue(new LiveResult<>(messageTree.depthFirstOrder())); - } - - @DatabaseExecutor - protected abstract String loadMessageText(Transaction txn, - PostHeader header) throws DbException; - @UiThread void setReplyId(@Nullable MessageId id) { replyId = id; @@ -271,8 +245,4 @@ public abstract class ThreadListViewModel return scrollToItem.getAndSet(null); } - public interface ItemGetter { - I getItem(H header, String text); - } - }