Load private group text lazily too, what the heck.

This commit is contained in:
akwizgran
2025-04-29 12:50:52 +01:00
parent 1542be20db
commit 16104b84b2
10 changed files with 41 additions and 79 deletions

View File

@@ -9,7 +9,7 @@ import javax.annotation.concurrent.NotThreadSafe;
public class ForumPostItem extends ThreadItem { public class ForumPostItem extends ThreadItem {
ForumPostItem(ForumPostHeader h) { 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()); h.getAuthorInfo(), h.isRead());
} }

View File

@@ -1,14 +1,10 @@
package org.briarproject.briar.android.privategroup.conversation; 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.GroupId;
import org.briarproject.bramble.api.sync.MessageId;
import org.briarproject.briar.R; import org.briarproject.briar.R;
import org.briarproject.briar.android.threaded.ThreadItem; import org.briarproject.briar.android.threaded.ThreadItem;
import org.briarproject.briar.api.identity.AuthorInfo;
import org.briarproject.briar.api.privategroup.GroupMessageHeader; import org.briarproject.briar.api.privategroup.GroupMessageHeader;
import javax.annotation.Nullable;
import javax.annotation.concurrent.NotThreadSafe; import javax.annotation.concurrent.NotThreadSafe;
import androidx.annotation.LayoutRes; import androidx.annotation.LayoutRes;
@@ -20,16 +16,10 @@ public class GroupMessageItem extends ThreadItem {
private final GroupId groupId; private final GroupId groupId;
private GroupMessageItem(MessageId messageId, GroupId groupId, GroupMessageItem(GroupMessageHeader h) {
@Nullable MessageId parentId, String text, long timestamp, super(h.getId(), h.getParentId(), h.getTimestamp(), h.getAuthor(),
Author author, AuthorInfo authorInfo, boolean isRead) { h.getAuthorInfo(), h.isRead());
super(messageId, parentId, text, timestamp, author, authorInfo, isRead); this.groupId = h.getGroupId();
this.groupId = groupId;
}
GroupMessageItem(GroupMessageHeader h, String text) {
this(h.getId(), h.getGroupId(), h.getParentId(), text, h.getTimestamp(),
h.getAuthor(), h.getAuthorInfo(), h.isRead());
} }
public GroupId getGroupId() { public GroupId getGroupId() {

View File

@@ -99,7 +99,7 @@ class GroupViewModel extends ThreadListViewModel<GroupMessageItem> {
// only act on non-local messages in this group // only act on non-local messages in this group
if (!g.isLocal() && g.getGroupId().equals(groupId)) { if (!g.isLocal() && g.getGroupId().equals(groupId)) {
LOG.info("Group message received, adding..."); LOG.info("Group message received, adding...");
GroupMessageItem item = buildItem(g.getHeader(), g.getText()); GroupMessageItem item = buildItem(g.getHeader());
addItem(item, false); addItem(item, false);
// In case the join message comes from the creator, // In case the join message comes from the creator,
// we need to reload the sharing contacts // we need to reload the sharing contacts
@@ -167,33 +167,19 @@ class GroupViewModel extends ThreadListViewModel<GroupMessageItem> {
List<GroupMessageHeader> headers = List<GroupMessageHeader> headers =
privateGroupManager.getHeaders(txn, groupId); privateGroupManager.getHeaders(txn, groupId);
logDuration(LOG, "Loading headers", start); logDuration(LOG, "Loading headers", start);
start = now();
List<GroupMessageItem> items = new ArrayList<>(); List<GroupMessageItem> items = new ArrayList<>();
for (GroupMessageHeader header : headers) { for (GroupMessageHeader header : headers) {
items.add(loadItem(txn, header)); items.add(buildItem(header));
} }
logDuration(LOG, "Loading bodies and creating items", start);
return items; return items;
}, this::setItems); }, this::setItems);
} }
private GroupMessageItem loadItem(Transaction txn, private GroupMessageItem buildItem(GroupMessageHeader header) {
GroupMessageHeader header) throws DbException {
String text;
if (header instanceof JoinMessageHeader) { if (header instanceof JoinMessageHeader) {
// will be looked up later return new JoinMessageItem((JoinMessageHeader) header);
text = "";
} else {
text = privateGroupManager.getMessageText(txn, header.getId());
} }
return buildItem(header, text); return new GroupMessageItem(header);
}
private GroupMessageItem buildItem(GroupMessageHeader header, String text) {
if (header instanceof JoinMessageHeader) {
return new JoinMessageItem((JoinMessageHeader) header, text);
}
return new GroupMessageItem(header, text);
} }
@Override @Override
@@ -221,19 +207,17 @@ class GroupViewModel extends ThreadListViewModel<GroupMessageItem> {
LOG.info("Creating group message..."); LOG.info("Creating group message...");
GroupMessage msg = groupMessageFactory.createGroupMessage(groupId, GroupMessage msg = groupMessageFactory.createGroupMessage(groupId,
timestamp, parentId, author, text, previousMsgId); 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 -> { runOnDbThread(false, txn -> {
long start = now(); long start = now();
GroupMessageHeader header = GroupMessageHeader header =
privateGroupManager.addLocalMessage(txn, msg); privateGroupManager.addLocalMessage(txn, msg);
logDuration(LOG, "Storing group message", start); logDuration(LOG, "Storing group message", start);
txn.attach(() -> txn.attach(() -> addItem(buildItem(header), true));
addItem(buildItem(header, text), true)
);
}, this::handleException); }, this::handleException);
} }

View File

@@ -14,8 +14,8 @@ class JoinMessageItem extends GroupMessageItem {
private final boolean isInitial; private final boolean isInitial;
JoinMessageItem(JoinMessageHeader h, String text) { JoinMessageItem(JoinMessageHeader h) {
super(h, text); super(h);
isInitial = h.isInitial(); isInitial = h.isInitial();
} }

View File

@@ -26,10 +26,8 @@ class JoinMessageItemViewHolder
} }
@Override @Override
public void bind(GroupMessageItem item, LifecycleOwner lifecycleOwner, protected void setText(GroupMessageItem item, LifecycleOwner lifecycleOwner,
ThreadItemListener<GroupMessageItem> listener) { ThreadItemListener<GroupMessageItem> listener) {
super.bind(item, lifecycleOwner, listener);
if (isCreator) bindForCreator((JoinMessageItem) item); if (isCreator) bindForCreator((JoinMessageItem) item);
else bind((JoinMessageItem) item); else bind((JoinMessageItem) item);
} }

View File

@@ -53,19 +53,7 @@ public abstract class BaseThreadItemViewHolder<I extends ThreadItem>
public void bind(I item, LifecycleOwner lifecycleOwner, public void bind(I item, LifecycleOwner lifecycleOwner,
ThreadItemListener<I> listener) { ThreadItemListener<I> listener) {
boundMessageId = item.getId(); boundMessageId = item.getId();
String text = item.getText(); setText(item, lifecycleOwner, listener);
if (text == null) {
textView.setText(null);
LiveData<String> 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));
}
Linkify.addLinks(textView, Linkify.WEB_URLS); Linkify.addLinks(textView, Linkify.WEB_URLS);
makeLinksClickable(textView, listener::onLinkClick); makeLinksClickable(textView, listener::onLinkClick);
@@ -82,6 +70,18 @@ public abstract class BaseThreadItemViewHolder<I extends ThreadItem>
} }
} }
protected void setText(I item, LifecycleOwner lifecycleOwner,
ThreadItemListener<I> listener) {
textView.setText(null);
LiveData<String> 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() { private void animateFadeOut() {
setIsRecyclable(false); setIsRecyclable(false);
ValueAnimator anim = new ValueAnimator(); ValueAnimator anim = new ValueAnimator();
@@ -94,6 +94,7 @@ public abstract class BaseThreadItemViewHolder<I extends ThreadItem>
@Override @Override
public void onAnimationStart(Animator animation) { public void onAnimationStart(Animator animation) {
} }
@Override @Override
public void onAnimationEnd(Animator animation) { public void onAnimationEnd(Animator animation) {
layout.setBackgroundResource( layout.setBackgroundResource(
@@ -101,9 +102,11 @@ public abstract class BaseThreadItemViewHolder<I extends ThreadItem>
layout.setActivated(false); layout.setActivated(false);
setIsRecyclable(true); setIsRecyclable(true);
} }
@Override @Override
public void onAnimationCancel(Animator animation) { public void onAnimationCancel(Animator animation) {
} }
@Override @Override
public void onAnimationRepeat(Animator animation) { public void onAnimationRepeat(Animator animation) {
} }

View File

@@ -19,8 +19,6 @@ public abstract class ThreadItem implements MessageNode {
private final MessageId messageId; private final MessageId messageId;
@Nullable @Nullable
private final MessageId parentId; private final MessageId parentId;
@Nullable
private final String text;
private final long timestamp; private final long timestamp;
private final Author author; private final Author author;
private final AuthorInfo authorInfo; private final AuthorInfo authorInfo;
@@ -28,11 +26,10 @@ public abstract class ThreadItem implements MessageNode {
private boolean isRead, highlighted; private boolean isRead, highlighted;
public ThreadItem(MessageId messageId, @Nullable MessageId parentId, public ThreadItem(MessageId messageId, @Nullable MessageId parentId,
@Nullable String text, long timestamp, Author author, long timestamp, Author author, AuthorInfo authorInfo,
AuthorInfo authorInfo, boolean isRead) { boolean isRead) {
this.messageId = messageId; this.messageId = messageId;
this.parentId = parentId; this.parentId = parentId;
this.text = text;
this.timestamp = timestamp; this.timestamp = timestamp;
this.author = author; this.author = author;
this.authorInfo = authorInfo; this.authorInfo = authorInfo;
@@ -40,11 +37,6 @@ public abstract class ThreadItem implements MessageNode {
this.highlighted = false; this.highlighted = false;
} }
@Nullable
public String getText() {
return text;
}
public int getLevel() { public int getLevel() {
return level; return level;
} }

View File

@@ -85,6 +85,9 @@ public abstract class ThreadListActivity<I extends ThreadItem, A extends ThreadI
scrollListener = new ThreadScrollListener<>(adapter, viewModel, scrollListener = new ThreadScrollListener<>(adapter, viewModel,
upButton, downButton); upButton, downButton);
list.getRecyclerView().addOnScrollListener(scrollListener); 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 -> { upButton.setOnClickListener(v -> {
int position = adapter.getVisibleUnreadPosTop(layoutManager); int position = adapter.getVisibleUnreadPosTop(layoutManager);

View File

@@ -17,14 +17,12 @@ public class GroupMessageAddedEvent extends Event {
private final GroupId groupId; private final GroupId groupId;
private final GroupMessageHeader header; private final GroupMessageHeader header;
private final String text;
private final boolean local; private final boolean local;
public GroupMessageAddedEvent(GroupId groupId, GroupMessageHeader header, public GroupMessageAddedEvent(GroupId groupId, GroupMessageHeader header,
String text, boolean local) { boolean local) {
this.groupId = groupId; this.groupId = groupId;
this.header = header; this.header = header;
this.text = text;
this.local = local; this.local = local;
} }
@@ -36,10 +34,6 @@ public class GroupMessageAddedEvent extends Event {
return header; return header;
} }
public String getText() {
return text;
}
public boolean isLocal() { public boolean isLocal() {
return local; return local;
} }

View File

@@ -170,6 +170,7 @@ class PrivateGroupManagerImpl extends BdfIncomingMessageHook
txn -> getPreviousMsgId(txn, g)); txn -> getPreviousMsgId(txn, g));
} }
@Override
public MessageId getPreviousMsgId(Transaction txn, GroupId g) public MessageId getPreviousMsgId(Transaction txn, GroupId g)
throws DbException { throws DbException {
try { try {
@@ -605,9 +606,7 @@ class PrivateGroupManagerImpl extends BdfIncomingMessageHook
throws DbException, FormatException { throws DbException, FormatException {
GroupMessageHeader header = getGroupMessageHeader(txn, m.getGroupId(), GroupMessageHeader header = getGroupMessageHeader(txn, m.getGroupId(),
m.getId(), meta, Collections.emptyMap()); m.getId(), meta, Collections.emptyMap());
String text = getMessageText(clientHelper.toList(m)); txn.attach(new GroupMessageAddedEvent(m.getGroupId(), header, local));
txn.attach(new GroupMessageAddedEvent(m.getGroupId(), header, text,
local));
} }
private void attachJoinMessageAddedEvent(Transaction txn, Message m, private void attachJoinMessageAddedEvent(Transaction txn, Message m,
@@ -615,8 +614,7 @@ class PrivateGroupManagerImpl extends BdfIncomingMessageHook
throws DbException, FormatException { throws DbException, FormatException {
JoinMessageHeader header = getJoinMessageHeader(txn, m.getGroupId(), JoinMessageHeader header = getJoinMessageHeader(txn, m.getGroupId(),
m.getId(), meta, Collections.emptyMap()); m.getId(), meta, Collections.emptyMap());
txn.attach(new GroupMessageAddedEvent(m.getGroupId(), header, "", txn.attach(new GroupMessageAddedEvent(m.getGroupId(), header, local));
local));
} }
private void addMember(Transaction txn, GroupId g, Author a, Visibility v) private void addMember(Transaction txn, GroupId g, Author a, Visibility v)