From 16104b84b2afc9eb4708f02e30919e577b2b8e46 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 29 Apr 2025 12:50:52 +0100 Subject: [PATCH] Load private group text lazily too, what the heck. --- .../briar/android/forum/ForumPostItem.java | 2 +- .../conversation/GroupMessageItem.java | 18 +++-------- .../conversation/GroupViewModel.java | 32 +++++-------------- .../conversation/JoinMessageItem.java | 4 +-- .../JoinMessageItemViewHolder.java | 4 +-- .../threaded/BaseThreadItemViewHolder.java | 29 +++++++++-------- .../briar/android/threaded/ThreadItem.java | 12 ++----- .../android/threaded/ThreadListActivity.java | 3 ++ .../event/GroupMessageAddedEvent.java | 8 +---- .../privategroup/PrivateGroupManagerImpl.java | 8 ++--- 10 files changed, 41 insertions(+), 79 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/forum/ForumPostItem.java b/briar-android/src/main/java/org/briarproject/briar/android/forum/ForumPostItem.java index 393cb5031..51e171856 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/forum/ForumPostItem.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/forum/ForumPostItem.java @@ -9,7 +9,7 @@ import javax.annotation.concurrent.NotThreadSafe; public class ForumPostItem extends ThreadItem { ForumPostItem(ForumPostHeader h) { - super(h.getId(), h.getParentId(), null, h.getTimestamp(), h.getAuthor(), + super(h.getId(), h.getParentId(), h.getTimestamp(), h.getAuthor(), h.getAuthorInfo(), h.isRead()); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupMessageItem.java b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupMessageItem.java index 5c3aa2ca9..1dd092aea 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupMessageItem.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupMessageItem.java @@ -1,14 +1,10 @@ package org.briarproject.briar.android.privategroup.conversation; -import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.sync.GroupId; -import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.briar.R; import org.briarproject.briar.android.threaded.ThreadItem; -import org.briarproject.briar.api.identity.AuthorInfo; import org.briarproject.briar.api.privategroup.GroupMessageHeader; -import javax.annotation.Nullable; import javax.annotation.concurrent.NotThreadSafe; import androidx.annotation.LayoutRes; @@ -20,16 +16,10 @@ public class GroupMessageItem extends ThreadItem { private final GroupId groupId; - private GroupMessageItem(MessageId messageId, GroupId groupId, - @Nullable MessageId parentId, String text, long timestamp, - Author author, AuthorInfo authorInfo, boolean isRead) { - super(messageId, parentId, text, timestamp, author, authorInfo, isRead); - this.groupId = groupId; - } - - GroupMessageItem(GroupMessageHeader h, String text) { - this(h.getId(), h.getGroupId(), h.getParentId(), text, h.getTimestamp(), - h.getAuthor(), h.getAuthorInfo(), h.isRead()); + GroupMessageItem(GroupMessageHeader h) { + super(h.getId(), h.getParentId(), h.getTimestamp(), h.getAuthor(), + h.getAuthorInfo(), h.isRead()); + this.groupId = h.getGroupId(); } public GroupId getGroupId() { 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 2fe22f515..5902dc7ce 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 @@ -99,7 +99,7 @@ class GroupViewModel extends ThreadListViewModel { // only act on non-local messages in this group if (!g.isLocal() && g.getGroupId().equals(groupId)) { LOG.info("Group message received, adding..."); - GroupMessageItem item = buildItem(g.getHeader(), g.getText()); + GroupMessageItem item = buildItem(g.getHeader()); addItem(item, false); // In case the join message comes from the creator, // we need to reload the sharing contacts @@ -167,33 +167,19 @@ class GroupViewModel extends ThreadListViewModel { List headers = privateGroupManager.getHeaders(txn, groupId); logDuration(LOG, "Loading headers", start); - start = now(); List items = new ArrayList<>(); for (GroupMessageHeader header : headers) { - items.add(loadItem(txn, header)); + items.add(buildItem(header)); } - logDuration(LOG, "Loading bodies and creating items", start); return items; }, this::setItems); } - private GroupMessageItem loadItem(Transaction txn, - GroupMessageHeader header) throws DbException { - String text; + private GroupMessageItem buildItem(GroupMessageHeader header) { if (header instanceof JoinMessageHeader) { - // will be looked up later - text = ""; - } else { - text = privateGroupManager.getMessageText(txn, header.getId()); + return new JoinMessageItem((JoinMessageHeader) header); } - return buildItem(header, text); - } - - private GroupMessageItem buildItem(GroupMessageHeader header, String text) { - if (header instanceof JoinMessageHeader) { - return new JoinMessageItem((JoinMessageHeader) header, text); - } - return new GroupMessageItem(header, text); + return new GroupMessageItem(header); } @Override @@ -221,19 +207,17 @@ class GroupViewModel extends ThreadListViewModel { LOG.info("Creating group message..."); GroupMessage msg = groupMessageFactory.createGroupMessage(groupId, timestamp, parentId, author, text, previousMsgId); - storePost(msg, text); + storePost(msg); }); } - private void storePost(GroupMessage msg, String text) { + private void storePost(GroupMessage msg) { 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) - ); + txn.attach(() -> addItem(buildItem(header), true)); }, this::handleException); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/JoinMessageItem.java b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/JoinMessageItem.java index 234fcccfb..4be17ff3d 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/JoinMessageItem.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/JoinMessageItem.java @@ -14,8 +14,8 @@ class JoinMessageItem extends GroupMessageItem { private final boolean isInitial; - JoinMessageItem(JoinMessageHeader h, String text) { - super(h, text); + JoinMessageItem(JoinMessageHeader h) { + super(h); isInitial = h.isInitial(); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/JoinMessageItemViewHolder.java b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/JoinMessageItemViewHolder.java index a27ff7324..b4e5e475b 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/JoinMessageItemViewHolder.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/JoinMessageItemViewHolder.java @@ -26,10 +26,8 @@ class JoinMessageItemViewHolder } @Override - public void bind(GroupMessageItem item, LifecycleOwner lifecycleOwner, + protected void setText(GroupMessageItem item, LifecycleOwner lifecycleOwner, ThreadItemListener listener) { - super.bind(item, lifecycleOwner, listener); - if (isCreator) bindForCreator((JoinMessageItem) item); else bind((JoinMessageItem) item); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/BaseThreadItemViewHolder.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/BaseThreadItemViewHolder.java index 589208ea5..e49952bc9 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/BaseThreadItemViewHolder.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/BaseThreadItemViewHolder.java @@ -53,19 +53,7 @@ public abstract class BaseThreadItemViewHolder public void bind(I item, LifecycleOwner lifecycleOwner, ThreadItemListener listener) { boundMessageId = item.getId(); - String text = item.getText(); - if (text == null) { - textView.setText(null); - LiveData textLiveData = listener.loadItemText(item.getId()); - textLiveData.observe(lifecycleOwner, t -> { - // Check that the ViewHolder hasn't been re-bound while loading - if (item.getId().equals(boundMessageId)) { - textView.setText(t); - } - }); - } else { - textView.setText(trim(text)); - } + setText(item, lifecycleOwner, listener); Linkify.addLinks(textView, Linkify.WEB_URLS); makeLinksClickable(textView, listener::onLinkClick); @@ -82,6 +70,18 @@ public abstract class BaseThreadItemViewHolder } } + protected void setText(I item, LifecycleOwner lifecycleOwner, + ThreadItemListener listener) { + textView.setText(null); + LiveData textLiveData = listener.loadItemText(item.getId()); + textLiveData.observe(lifecycleOwner, t -> { + // Check that the ViewHolder hasn't been re-bound while loading + if (item.getId().equals(boundMessageId)) { + textView.setText(trim(t)); + } + }); + } + private void animateFadeOut() { setIsRecyclable(false); ValueAnimator anim = new ValueAnimator(); @@ -94,6 +94,7 @@ public abstract class BaseThreadItemViewHolder @Override public void onAnimationStart(Animator animation) { } + @Override public void onAnimationEnd(Animator animation) { layout.setBackgroundResource( @@ -101,9 +102,11 @@ public abstract class BaseThreadItemViewHolder layout.setActivated(false); setIsRecyclable(true); } + @Override public void onAnimationCancel(Animator animation) { } + @Override public void onAnimationRepeat(Animator animation) { } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItem.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItem.java index 7e3c1a724..6f457e571 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItem.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItem.java @@ -19,8 +19,6 @@ public abstract class ThreadItem implements MessageNode { private final MessageId messageId; @Nullable private final MessageId parentId; - @Nullable - private final String text; private final long timestamp; private final Author author; private final AuthorInfo authorInfo; @@ -28,11 +26,10 @@ public abstract class ThreadItem implements MessageNode { private boolean isRead, highlighted; public ThreadItem(MessageId messageId, @Nullable MessageId parentId, - @Nullable String text, long timestamp, Author author, - AuthorInfo authorInfo, boolean isRead) { + long timestamp, Author author, AuthorInfo authorInfo, + boolean isRead) { this.messageId = messageId; this.parentId = parentId; - this.text = text; this.timestamp = timestamp; this.author = author; this.authorInfo = authorInfo; @@ -40,11 +37,6 @@ public abstract class ThreadItem implements MessageNode { this.highlighted = false; } - @Nullable - public String getText() { - return text; - } - public int getLevel() { return level; } 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 a5e054efb..3b737e384 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 @@ -85,6 +85,9 @@ public abstract class ThreadListActivity(adapter, viewModel, upButton, downButton); list.getRecyclerView().addOnScrollListener(scrollListener); + // This is a tradeoff between memory consumption for cached views + // and the cost of loading message text from the database + list.getRecyclerView().setItemViewCacheSize(20); upButton.setOnClickListener(v -> { int position = adapter.getVisibleUnreadPosTop(layoutManager); diff --git a/briar-api/src/main/java/org/briarproject/briar/api/privategroup/event/GroupMessageAddedEvent.java b/briar-api/src/main/java/org/briarproject/briar/api/privategroup/event/GroupMessageAddedEvent.java index 55effdd98..f76231cd4 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/privategroup/event/GroupMessageAddedEvent.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/privategroup/event/GroupMessageAddedEvent.java @@ -17,14 +17,12 @@ public class GroupMessageAddedEvent extends Event { private final GroupId groupId; private final GroupMessageHeader header; - private final String text; private final boolean local; public GroupMessageAddedEvent(GroupId groupId, GroupMessageHeader header, - String text, boolean local) { + boolean local) { this.groupId = groupId; this.header = header; - this.text = text; this.local = local; } @@ -36,10 +34,6 @@ public class GroupMessageAddedEvent extends Event { return header; } - public String getText() { - return text; - } - public boolean isLocal() { return local; } diff --git a/briar-core/src/main/java/org/briarproject/briar/privategroup/PrivateGroupManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/privategroup/PrivateGroupManagerImpl.java index 1e06214ca..07f258f2e 100644 --- a/briar-core/src/main/java/org/briarproject/briar/privategroup/PrivateGroupManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/privategroup/PrivateGroupManagerImpl.java @@ -170,6 +170,7 @@ class PrivateGroupManagerImpl extends BdfIncomingMessageHook txn -> getPreviousMsgId(txn, g)); } + @Override public MessageId getPreviousMsgId(Transaction txn, GroupId g) throws DbException { try { @@ -605,9 +606,7 @@ class PrivateGroupManagerImpl extends BdfIncomingMessageHook throws DbException, FormatException { GroupMessageHeader header = getGroupMessageHeader(txn, m.getGroupId(), m.getId(), meta, Collections.emptyMap()); - String text = getMessageText(clientHelper.toList(m)); - txn.attach(new GroupMessageAddedEvent(m.getGroupId(), header, text, - local)); + txn.attach(new GroupMessageAddedEvent(m.getGroupId(), header, local)); } private void attachJoinMessageAddedEvent(Transaction txn, Message m, @@ -615,8 +614,7 @@ class PrivateGroupManagerImpl extends BdfIncomingMessageHook throws DbException, FormatException { JoinMessageHeader header = getJoinMessageHeader(txn, m.getGroupId(), m.getId(), meta, Collections.emptyMap()); - txn.attach(new GroupMessageAddedEvent(m.getGroupId(), header, "", - local)); + txn.attach(new GroupMessageAddedEvent(m.getGroupId(), header, local)); } private void addMember(Transaction txn, GroupId g, Author a, Visibility v)