From 393e54ce91e7098236980731db5f920b897049f3 Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Tue, 4 Aug 2020 14:13:59 -0300 Subject: [PATCH] Update how we mark messages as read. --- .../conversation/ConversationActivity.java | 16 ------ .../conversation/ConversationAdapter.java | 8 ++- .../conversation/ConversationFragment.java | 25 ++++++++- .../conversation/ConversationViewModel.java | 8 +++ .../conversation/MarkReadHelper.java | 55 +++++++++++++++++++ .../securesms/database/MessagingDatabase.java | 7 ++- .../securesms/database/MmsDatabase.java | 15 +++-- .../securesms/database/SmsDatabase.java | 13 ++++- .../securesms/database/ThreadDatabase.java | 29 +++++++--- 9 files changed, 142 insertions(+), 34 deletions(-) create mode 100644 app/src/main/java/org/thoughtcrime/securesms/conversation/MarkReadHelper.java diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationActivity.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationActivity.java index 66fd465f7..890bcad01 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationActivity.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationActivity.java @@ -521,7 +521,6 @@ public class ConversationActivity extends PassphraseRequiredActivity } ApplicationDependencies.getMessageNotifier().setVisibleThread(threadId); - markThreadAsRead(); } @Override @@ -2285,21 +2284,6 @@ public class ConversationActivity extends PassphraseRequiredActivity : MediaConstraints.getMmsMediaConstraints(sendButton.getSelectedTransport().getSimSubscriptionId().or(-1)); } - private void markThreadAsRead() { - new AsyncTask() { - @Override - protected Void doInBackground(Long... params) { - Context context = ConversationActivity.this; - List messageIds = DatabaseFactory.getThreadDatabase(context).setRead(params[0], false); - - ApplicationDependencies.getMessageNotifier().updateNotification(context); - MarkReadReceiver.process(context, messageIds); - - return null; - } - }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR, threadId); - } - private void markLastSeen() { new AsyncTask() { @Override diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationAdapter.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationAdapter.java index cdc19e02c..ea677a371 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationAdapter.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationAdapter.java @@ -253,7 +253,9 @@ public class ConversationAdapter protected @Nullable ConversationMessage getItem(int position) { position = hasHeader() ? position - 1 : position; - if (position < fastRecords.size()) { + if (position == -1) { + return null; + } else if (position < fastRecords.size()) { return fastRecords.get(position); } else { int correctedPosition = position - fastRecords.size(); @@ -307,6 +309,10 @@ public class ConversationAdapter viewHolder.setText(viewHolder.itemView.getContext().getResources().getQuantityString(R.plurals.ConversationAdapter_n_unread_messages, (position + 1), (position + 1))); } + boolean hasNoConversationMessages() { + return super.getItemCount() + fastRecords.size() == 0; + } + /** * The presence of a header may throw off the position you'd like to jump to. This will return * an adjusted message position based on adapter state. diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationFragment.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationFragment.java index 10c6b61b7..77bba8940 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationFragment.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationFragment.java @@ -129,9 +129,9 @@ import java.io.InputStream; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; -import java.util.LinkedList; import java.util.List; import java.util.Locale; +import java.util.Objects; import java.util.Set; @SuppressLint("StaticFieldLeak") @@ -166,6 +166,7 @@ public class ConversationFragment extends LoggingFragment { private MessageRequestViewModel messageRequestViewModel; private ConversationViewModel conversationViewModel; private SnapToTopDataObserver snapToTopDataObserver; + private MarkReadHelper markReadHelper; public static void prepare(@NonNull Context context) { FrameLayout parent = new FrameLayout(context); @@ -420,6 +421,7 @@ public class ConversationFragment extends LoggingFragment { this.recipient = Recipient.live(getActivity().getIntent().getParcelableExtra(ConversationActivity.RECIPIENT_EXTRA)); this.threadId = this.getActivity().getIntent().getLongExtra(ConversationActivity.THREAD_ID_EXTRA, -1); this.unknownSenderView = new UnknownSenderView(getActivity(), recipient.get(), threadId, () -> clearHeaderIfNotTyping(getListAdapter())); + this.markReadHelper = new MarkReadHelper(threadId, requireContext()); conversationViewModel.onConversationDataAvailable(threadId, startingPosition); @@ -658,6 +660,7 @@ public class ConversationFragment extends LoggingFragment { if (threadDeleted) { threadId = -1; + conversationViewModel.clearThreadId(); listener.setThreadId(threadId); } } @@ -697,6 +700,7 @@ public class ConversationFragment extends LoggingFragment { if (threadDeleted) { threadId = -1; + conversationViewModel.clearThreadId(); listener.setThreadId(threadId); } } @@ -1070,6 +1074,8 @@ public class ConversationFragment extends LoggingFragment { wasAtBottom = currentlyAtBottom; wasAtZoomScrollHeight = currentlyAtZoomScrollHeight; lastPositionId = positionId; + + postMarkAsReadRequest(); } @Override @@ -1081,6 +1087,23 @@ public class ConversationFragment extends LoggingFragment { } } + private void postMarkAsReadRequest() { + if (getListAdapter().hasNoConversationMessages()) { + return; + } + + int position = getListLayoutManager().findFirstVisibleItemPosition(); + if (position >= (isTypingIndicatorShowing() ? 1 : 0)) { + ConversationMessage item = getListAdapter().getItem(position); + if (item != null) { + long timestamp = item.getMessageRecord() + .getDateReceived(); + + markReadHelper.onViewsRevealed(timestamp); + } + } + } + private boolean isAtZoomScrollHeight() { return getListLayoutManager().findFirstCompletelyVisibleItemPosition() > 4; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationViewModel.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationViewModel.java index dd73eb3bf..05b5c6a66 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationViewModel.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationViewModel.java @@ -2,7 +2,9 @@ package org.thoughtcrime.securesms.conversation; import android.app.Application; +import androidx.annotation.MainThread; import androidx.annotation.NonNull; +import androidx.annotation.WorkerThread; import androidx.lifecycle.LiveData; import androidx.lifecycle.MutableLiveData; import androidx.lifecycle.Transformations; @@ -94,6 +96,7 @@ class ConversationViewModel extends ViewModel { mediaRepository.getMediaInBucket(context, Media.ALL_MEDIA_BUCKET_ID, recentMedia::postValue); } + @MainThread void onConversationDataAvailable(long threadId, int startingPosition) { Log.d(TAG, "[onConversationDataAvailable] threadId: " + threadId + ", startingPosition: " + startingPosition); this.jumpToPosition = startingPosition; @@ -101,6 +104,11 @@ class ConversationViewModel extends ViewModel { this.threadId.setValue(threadId); } + void clearThreadId() { + this.jumpToPosition = -1; + this.threadId.postValue(-1L); + } + @NonNull LiveData> getRecentMedia() { return recentMedia; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/MarkReadHelper.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/MarkReadHelper.java new file mode 100644 index 000000000..e007fcf05 --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/MarkReadHelper.java @@ -0,0 +1,55 @@ +package org.thoughtcrime.securesms.conversation; + +import android.content.Context; + +import androidx.annotation.NonNull; + +import org.thoughtcrime.securesms.database.DatabaseFactory; +import org.thoughtcrime.securesms.database.MessagingDatabase; +import org.thoughtcrime.securesms.database.ThreadDatabase; +import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; +import org.thoughtcrime.securesms.logging.Log; +import org.thoughtcrime.securesms.notifications.MarkReadReceiver; +import org.thoughtcrime.securesms.util.Debouncer; +import org.thoughtcrime.securesms.util.concurrent.SerialMonoLifoExecutor; +import org.thoughtcrime.securesms.util.concurrent.SignalExecutors; + +import java.util.List; +import java.util.concurrent.Executor; + +class MarkReadHelper { + private static final String TAG = Log.tag(MarkReadHelper.class); + + private static final long DEBOUNCE_TIMEOUT = 100; + private static final Executor EXECUTOR = new SerialMonoLifoExecutor(SignalExecutors.BOUNDED); + + private final long threadId; + private final Context context; + private final Debouncer debouncer = new Debouncer(DEBOUNCE_TIMEOUT); + private long latestTimestamp; + + MarkReadHelper(long threadId, @NonNull Context context) { + this.threadId = threadId; + this.context = context.getApplicationContext(); + } + + public void onViewsRevealed(long timestamp) { + if (timestamp <= latestTimestamp) { + return; + } + + latestTimestamp = timestamp; + + debouncer.publish(() -> { + EXECUTOR.execute(() -> { + ThreadDatabase threadDatabase = DatabaseFactory.getThreadDatabase(context); + List infos = threadDatabase.setReadSince(threadId, false, timestamp); + + Log.d(TAG, "Marking " + infos.size() + " messages as read."); + + ApplicationDependencies.getMessageNotifier().updateNotification(context); + MarkReadReceiver.process(context, infos); + }); + }); + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/MessagingDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/MessagingDatabase.java index e5b9f785e..52ce19ea1 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MessagingDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MessagingDatabase.java @@ -46,6 +46,7 @@ public abstract class MessagingDatabase extends Database implements MmsSmsColumn protected abstract String getTableName(); protected abstract String getTypeField(); protected abstract String getDateSentColumnName(); + protected abstract String getDateReceivedColumnName(); public abstract void markExpireStarted(long messageId); public abstract void markExpireStarted(long messageId, long startTime); @@ -144,12 +145,16 @@ public abstract class MessagingDatabase extends Database implements MmsSmsColumn return String.format(Locale.ENGLISH, "(%s OR %s) AND %s", isSent, isReceived, isSecure); } - public void setReactionsSeen(long threadId) { + public void setReactionsSeen(long threadId, long sinceTimestamp) { SQLiteDatabase db = databaseHelper.getWritableDatabase(); ContentValues values = new ContentValues(); String whereClause = THREAD_ID + " = ? AND " + REACTIONS_UNREAD + " = ?"; String[] whereArgs = new String[]{String.valueOf(threadId), "1"}; + if (sinceTimestamp > -1) { + whereClause += " AND " + getDateReceivedColumnName() + " <= " + sinceTimestamp; + } + values.put(REACTIONS_UNREAD, 0); values.put(REACTIONS_LAST_SEEN, System.currentTimeMillis()); diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/MmsDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/MmsDatabase.java index 677ff1f2f..e83d571e4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MmsDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MmsDatabase.java @@ -248,6 +248,11 @@ public class MmsDatabase extends MessagingDatabase { return DATE_SENT; } + @Override + protected String getDateReceivedColumnName() { + return DATE_RECEIVED; + } + @Override protected String getTypeField() { return MESSAGE_BOX; @@ -632,12 +637,14 @@ public class MmsDatabase extends MessagingDatabase { database.update(TABLE_NAME, contentValues, ID_WHERE, new String[] {String.valueOf(id)}); } - - public List setMessagesRead(long threadId) { - return setMessagesRead(THREAD_ID + " = ? AND " + READ + " = 0", new String[] {String.valueOf(threadId)}); + public List setMessagesReadSince(long threadId, long sinceTimestamp) { + if (sinceTimestamp == -1) { + return setMessagesRead(THREAD_ID + " = ? AND " + READ + " = 0", new String[] {String.valueOf(threadId)}); + } else { + return setMessagesRead(THREAD_ID + " = ? AND " + READ + " = 0 AND " + DATE_RECEIVED + " <= ?", new String[]{String.valueOf(threadId), String.valueOf(sinceTimestamp)}); + } } - public List setEntireThreadRead(long threadId) { return setMessagesRead(THREAD_ID + " = ?", new String[] {String.valueOf(threadId)}); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/SmsDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/SmsDatabase.java index c34c36549..7f65c26c2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/SmsDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/SmsDatabase.java @@ -150,6 +150,11 @@ public class SmsDatabase extends MessagingDatabase { return DATE_SENT; } + @Override + protected String getDateReceivedColumnName() { + return DATE_RECEIVED; + } + @Override protected String getTypeField() { return TYPE; @@ -509,8 +514,12 @@ public class SmsDatabase extends MessagingDatabase { return setMessagesRead(THREAD_ID + " = ?", new String[] {String.valueOf(threadId)}); } - public List setMessagesRead(long threadId) { - return setMessagesRead(THREAD_ID + " = ? AND " + READ + " = 0", new String[] {String.valueOf(threadId)}); + public List setMessagesReadSince(long threadId, long sinceTimestamp) { + if (sinceTimestamp == -1) { + return setMessagesRead(THREAD_ID + " = ? AND " + READ + " = 0", new String[] {String.valueOf(threadId)}); + } else { + return setMessagesRead(THREAD_ID + " = ? AND " + READ + " = 0 AND " + DATE_RECEIVED + " <= ?", new String[] {String.valueOf(threadId),String.valueOf(sinceTimestamp)}); + } } public List setAllMessagesRead() { diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/ThreadDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/ThreadDatabase.java index a364adad3..31cd85d9e 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/ThreadDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/ThreadDatabase.java @@ -340,10 +340,18 @@ public class ThreadDatabase extends Database { } public List setRead(long threadId, boolean lastSeen) { - return setRead(Collections.singletonList(threadId), lastSeen); + return setReadInternal(Collections.singletonList(threadId), lastSeen, -1); + } + + public List setReadSince(long threadId, boolean lastSeen, long sinceTimestamp) { + return setReadInternal(Collections.singletonList(threadId), lastSeen, sinceTimestamp); } public List setRead(Collection threadIds, boolean lastSeen) { + return setReadInternal(threadIds, lastSeen, -1); + } + + private List setReadInternal(Collection threadIds, boolean lastSeen, long sinceTimestamp) { SQLiteDatabase db = databaseHelper.getWritableDatabase(); List smsRecords = new LinkedList<>(); @@ -354,20 +362,23 @@ public class ThreadDatabase extends Database { try { ContentValues contentValues = new ContentValues(2); contentValues.put(READ, ReadStatus.READ.serialize()); - contentValues.put(UNREAD_COUNT, 0); if (lastSeen) { - contentValues.put(LAST_SEEN, System.currentTimeMillis()); + contentValues.put(LAST_SEEN, sinceTimestamp == -1 ? System.currentTimeMillis() : sinceTimestamp); } for (long threadId : threadIds) { + smsRecords.addAll(DatabaseFactory.getSmsDatabase(context).setMessagesReadSince(threadId, sinceTimestamp)); + mmsRecords.addAll(DatabaseFactory.getMmsDatabase(context).setMessagesReadSince(threadId, sinceTimestamp)); + + DatabaseFactory.getSmsDatabase(context).setReactionsSeen(threadId, sinceTimestamp); + DatabaseFactory.getMmsDatabase(context).setReactionsSeen(threadId, sinceTimestamp); + + int unreadCount = DatabaseFactory.getMmsSmsDatabase(context).getUnreadCount(threadId); + + contentValues.put(UNREAD_COUNT, unreadCount); + db.update(TABLE_NAME, contentValues, ID_WHERE, new String[]{threadId + ""}); - - smsRecords.addAll(DatabaseFactory.getSmsDatabase(context).setMessagesRead(threadId)); - mmsRecords.addAll(DatabaseFactory.getMmsDatabase(context).setMessagesRead(threadId)); - - DatabaseFactory.getSmsDatabase(context).setReactionsSeen(threadId); - DatabaseFactory.getMmsDatabase(context).setReactionsSeen(threadId); } db.setTransactionSuccessful();