Merge branch '196-mark-messages-read' into 'master'

Mark private messages read properly

Depends on !386.

This branch uses the same approach as forums to mark messages read, i.e. each message is marked read when it becomes visible, rather than marking all messages read in a batch when the activity finishes. This fixes two problems: messages not being marked read when isFinishing() is false, for example when leaving the activity via the home button, and a race condition between updating and loading the group count when leaving the activity, resulting in a stale unread message count in the contact list.

Closes #196.

See merge request !388

briar-android/src/org/briarproject/android/contact/ConversationActivity.java

@@ -28,7 +28,7 @@ import org.briarproject.R;
 import org.briarproject.android.ActivityComponent;
 import org.briarproject.android.BriarActivity;
 import org.briarproject.android.api.AndroidNotificationManager;
-import org.briarproject.android.contact.ConversationAdapter.RequestListener;
+import org.briarproject.android.contact.ConversationAdapter.ConversationListener;
 import org.briarproject.android.introduction.IntroductionActivity;
 import org.briarproject.android.util.AndroidUtils;
 import org.briarproject.android.view.BriarRecyclerView;
@@ -67,6 +67,8 @@ import org.briarproject.api.messaging.MessagingManager;
 import org.briarproject.api.messaging.PrivateMessage;
 import org.briarproject.api.messaging.PrivateMessageFactory;
 import org.briarproject.api.messaging.PrivateMessageHeader;
+import org.briarproject.api.nullsafety.MethodsNotNullByDefault;
+import org.briarproject.api.nullsafety.ParametersNotNullByDefault;
 import org.briarproject.api.plugins.ConnectionRegistry;
 import org.briarproject.api.privategroup.invitation.GroupInvitationManager;
 import org.briarproject.api.settings.Settings;
@@ -82,7 +84,6 @@ import org.jetbrains.annotations.NotNull;
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -106,8 +107,10 @@ import static java.util.logging.Level.WARNING;
 import static org.briarproject.android.fragment.SettingsFragment.SETTINGS_NAMESPACE;
 import static org.briarproject.api.messaging.MessagingConstants.MAX_PRIVATE_MESSAGE_BODY_LENGTH;
 
+@MethodsNotNullByDefault
+@ParametersNotNullByDefault
 public class ConversationActivity extends BriarActivity
-		implements EventListener, RequestListener, TextInputListener {
+		implements EventListener, ConversationListener, TextInputListener {
 
 	public static final String CONTACT_ID = "briar.CONTACT_ID";
 
@@ -239,7 +242,6 @@ public class ConversationActivity extends BriarActivity
 		eventBus.removeListener(this);
 		notificationManager.unblockContactNotification(contactId);
 		list.stopPeriodicUpdate();
-		if (isFinishing()) markMessagesRead();
 	}
 
 	@Override
@@ -492,41 +494,6 @@ public class ConversationActivity extends BriarActivity
 		});
 	}
 
-	private void markMessagesRead() {
-		Map<MessageId, GroupId> unread = new HashMap<>();
-		SparseArray<ConversationItem> list = adapter.getIncomingMessages();
-		for (int i = 0; i < list.size(); i++) {
-			ConversationItem item = list.valueAt(i);
-			if (!item.isRead())
-				unread.put(item.getId(), item.getGroupId());
-		}
-		if (unread.isEmpty()) return;
-		if (LOG.isLoggable(INFO))
-			LOG.info("Marking " + unread.size() + " messages read");
-		markMessagesRead(unread);
-	}
-
-	private void markMessagesRead(final Map<MessageId, GroupId> unread) {
-		runOnDbThread(new Runnable() {
-			@Override
-			public void run() {
-				try {
-					long now = System.currentTimeMillis();
-					for (Map.Entry<MessageId, GroupId> e : unread.entrySet()) {
-						messagingManager.setReadFlag(e.getValue(), e.getKey(),
-								true);
-					}
-					long duration = System.currentTimeMillis() - now;
-					if (LOG.isLoggable(INFO))
-						LOG.info("Marking read took " + duration + " ms");
-				} catch (DbException e) {
-					if (LOG.isLoggable(WARNING))
-						LOG.log(WARNING, e.toString(), e);
-				}
-			}
-		});
-	}
-
 	@Override
 	public void eventOccurred(Event e) {
 		if (e instanceof ContactRemovedEvent) {
@@ -840,6 +807,29 @@ public class ConversationActivity extends BriarActivity
 		});
 	}
 
+	@Override
+	public void onItemVisible(ConversationItem item) {
+		if (!item.isRead()) markMessageRead(item.getGroupId(), item.getId());
+	}
+
+	private void markMessageRead(final GroupId g, final MessageId m) {
+		runOnDbThread(new Runnable() {
+			@Override
+			public void run() {
+				try {
+					long now = System.currentTimeMillis();
+					messagingManager.setReadFlag(g, m, true);
+					long duration = System.currentTimeMillis() - now;
+					if (LOG.isLoggable(INFO))
+						LOG.info("Marking read took " + duration + " ms");
+				} catch (DbException e) {
+					if (LOG.isLoggable(WARNING))
+						LOG.log(WARNING, e.toString(), e);
+				}
+			}
+		});
+	}
+
 	@UiThread
 	@Override
 	public void respondToRequest(@NotNull final ConversationRequestItem item,

briar-android/src/org/briarproject/android/contact/ConversationAdapter.java

@@ -16,11 +16,11 @@ import org.briarproject.api.nullsafety.NotNullByDefault;
 class ConversationAdapter
 		extends BriarAdapter<ConversationItem, ConversationItemViewHolder> {
 
-	private RequestListener listener;
+	private ConversationListener listener;
 
-	ConversationAdapter(Context ctx, RequestListener requestListener) {
+	ConversationAdapter(Context ctx, ConversationListener conversationListener) {
 		super(ctx, ConversationItem.class);
-		listener = requestListener;
+		listener = conversationListener;
 	}
 
 	@LayoutRes
@@ -59,6 +59,7 @@ class ConversationAdapter
 		} else {
 			ui.bind(item);
 		}
+		listener.onItemVisible(item);
 	}
 
 	@Override
@@ -132,7 +133,10 @@ class ConversationAdapter
 
 	@UiThread
 	@NotNullByDefault
-	interface RequestListener {
+	interface ConversationListener {
+
+		void onItemVisible(ConversationItem item);
+
 		void respondToRequest(ConversationRequestItem item, boolean accept);
 	}
 

briar-android/src/org/briarproject/android/contact/ConversationRequestViewHolder.java

@@ -6,7 +6,7 @@ import android.view.View.OnClickListener;
 import android.widget.Button;
 
 import org.briarproject.R;
-import org.briarproject.android.contact.ConversationAdapter.RequestListener;
+import org.briarproject.android.contact.ConversationAdapter.ConversationListener;
 import org.briarproject.api.nullsafety.NotNullByDefault;
 
 import static android.view.View.GONE;
@@ -26,7 +26,7 @@ class ConversationRequestViewHolder extends ConversationNoticeInViewHolder {
 	}
 
 	void bind(ConversationItem conversationItem,
-			final RequestListener listener) {
+			final ConversationListener listener) {
 		super.bind(conversationItem);
 
 		final ConversationRequestItem item =

briar-core/src/org/briarproject/clients/BdfIncomingMessageHook.java

@@ -169,8 +169,9 @@ public abstract class BdfIncomingMessageHook implements IncomingMessageHook,
 				// update unread counter in group metadata
 				GroupCount c = getGroupCount(txn, g);
 				BdfDictionary d = new BdfDictionary();
-				d.put(GROUP_KEY_UNREAD_COUNT,
-						c.getUnreadCount() + (read ? -1 : 1));
+				int count = c.getUnreadCount() + (read ? -1 : 1);
+				if (count < 0) throw new DbException();
+				d.put(GROUP_KEY_UNREAD_COUNT, count);
 				clientHelper.mergeGroupMetadata(txn, g, d);
 			}
 			db.commitTransaction(txn);