fixed unread buttons for threaded lists and akwizgran's comments

briar-android/src/main/java/org/briarproject/briar/android/controller/DbControllerImpl.java

@@ -17,7 +17,7 @@ public class DbControllerImpl implements DbController {
 	private static final Logger LOG =
 			Logger.getLogger(DbControllerImpl.class.getName());
 
-	private final Executor dbExecutor;
+	protected final Executor dbExecutor;
 	private final LifecycleManager lifecycleManager;
 
 	@Inject

briar-android/src/main/java/org/briarproject/briar/android/forum/ForumControllerImpl.java

@@ -17,6 +17,7 @@ import org.briarproject.briar.android.controller.handler.ResultExceptionHandler;
 import org.briarproject.briar.android.forum.ForumController.ForumListener;
 import org.briarproject.briar.android.threaded.ThreadListControllerImpl;
 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.forum.Forum;
 import org.briarproject.briar.api.forum.ForumInvitationResponse;
@@ -55,10 +56,10 @@ class ForumControllerImpl extends
 			LifecycleManager lifecycleManager, IdentityManager identityManager,
 			@CryptoExecutor Executor cryptoExecutor,
 			ForumManager forumManager, ForumSharingManager forumSharingManager,
-			EventBus eventBus, Clock clock,
+			EventBus eventBus, Clock clock, MessageTracker messageTracker,
 			AndroidNotificationManager notificationManager) {
 		super(dbExecutor, lifecycleManager, identityManager, cryptoExecutor,
-				eventBus, clock, notificationManager);
+				eventBus, clock, notificationManager, messageTracker);
 		this.forumManager = forumManager;
 		this.forumSharingManager = forumSharingManager;
 	}

briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupControllerImpl.java

@@ -17,6 +17,7 @@ import org.briarproject.briar.android.controller.handler.ResultExceptionHandler;
 import org.briarproject.briar.android.privategroup.conversation.GroupController.GroupListener;
 import org.briarproject.briar.android.threaded.ThreadListControllerImpl;
 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.privategroup.GroupMember;
 import org.briarproject.briar.api.privategroup.GroupMessage;
@@ -60,9 +61,10 @@ class GroupControllerImpl extends
 			@CryptoExecutor Executor cryptoExecutor,
 			PrivateGroupManager privateGroupManager,
 			GroupMessageFactory groupMessageFactory, EventBus eventBus,
-			Clock clock, AndroidNotificationManager notificationManager) {
+			MessageTracker messageTracker, Clock clock,
+			AndroidNotificationManager notificationManager) {
 		super(dbExecutor, lifecycleManager, identityManager, cryptoExecutor,
-				eventBus, clock, notificationManager);
+				eventBus, clock, notificationManager, messageTracker);
 		this.privateGroupManager = privateGroupManager;
 		this.groupMessageFactory = groupMessageFactory;
 	}

briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemAdapter.java

@@ -66,17 +66,7 @@ public class ThreadItemAdapter<I extends ThreadItem>
 		revision++;
 	}
 
-	// Useful when the adapter has not calculated the dimension yet
+	void setItemWithIdVisible(MessageId messageId) {
-	void postSetItemWithIdVisible(@Nullable final MessageId messageId) {
-		new Handler().post(new Runnable() {
-			@Override
-			public void run() {
-				setItemWithIdVisible(messageId);
-			}
-		});
-	}
-
-	void setItemWithIdVisible(@Nullable MessageId messageId) {
 		if (messageId != null) {
 			int pos = 0;
 			for (I item : items) {
@@ -169,7 +159,7 @@ public class ThreadItemAdapter<I extends ThreadItem>
 	/**
 	 * Returns the position of the first unread item below the current viewport
 	 */
-	public int getVisibleUnreadPosBottom() {
+	int getVisibleUnreadPosBottom() {
 		final int positionBottom = layoutManager.findLastVisibleItemPosition();
 		if (positionBottom == NO_POSITION) return NO_POSITION;
 		for (int i = positionBottom + 1; i < items.size(); i++) {
@@ -181,7 +171,7 @@ public class ThreadItemAdapter<I extends ThreadItem>
 	/**
 	 * Returns the position of the first unread item above the current viewport
 	 */
-	public int getVisibleUnreadPosTop() {
+	int getVisibleUnreadPosTop() {
 		final int positionTop = layoutManager.findFirstVisibleItemPosition();
 		int position = NO_POSITION;
 		for (int i = 0; i < items.size(); i++) {

briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemList.java

@@ -9,7 +9,7 @@ import javax.annotation.Nullable;
 public interface ThreadItemList<I extends ThreadItem> extends List<I> {
 
 	@Nullable
-	MessageId getBottomVisibleItemId();
+	MessageId getFirstVisibleItemId();
 
 	void setBottomVisibleItemId(@Nullable MessageId bottomVisibleItemId);
 }

briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemListImpl.java

@@ -12,7 +12,7 @@ public class ThreadItemListImpl<I extends ThreadItem> extends ArrayList<I>
 	private MessageId bottomVisibleItemId;
 
 	@Override
-	public MessageId getBottomVisibleItemId() {
+	public MessageId getFirstVisibleItemId() {
 		return bottomVisibleItemId;
 	}
 

briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListActivity.java

@@ -149,11 +149,12 @@ public abstract class ThreadListActivity<G extends NamedGroup, A extends ThreadI
 
 	@Override
 	@Nullable
-	public MessageId getLastVisibleMessageId() {
+	public MessageId getFirstVisibleMessageId() {
 		if (layoutManager != null && adapter != null) {
 			int position =
-					layoutManager.findLastCompletelyVisibleItemPosition();
+					layoutManager.findFirstVisibleItemPosition();
-			return adapter.getItemAt(position).getId();
+			I i = adapter.getItemAt(position);
+			return i == null ? null : adapter.getItemAt(position).getId();
 		}
 		return null;
 	}
@@ -190,10 +191,7 @@ public abstract class ThreadListActivity<G extends NamedGroup, A extends ThreadI
 							if (items.isEmpty()) {
 								list.showData();
 							} else {
-								adapter.setItems(items);
+								initList(items);
-								adapter.postSetItemWithIdVisible(
-										items.getBottomVisibleItemId());
-								list.showData();
 								updateTextInput(replyId);
 							}
 						} else {
@@ -209,6 +207,15 @@ public abstract class ThreadListActivity<G extends NamedGroup, A extends ThreadI
 				});
 	}
 
+	private void initList(final ThreadItemList<I> items) {
+		adapter.setItems(items);
+		MessageId messageId = items.getFirstVisibleItemId();
+		if (messageId != null)
+			adapter.setItemWithIdVisible(messageId);
+		updateUnreadCount();
+		list.showData();
+	}
+
 	protected void loadSharingContacts() {
 		getController().loadSharingContacts(
 				new UiResultExceptionHandler<Collection<ContactId>, DbException>(

briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListController.java

@@ -56,7 +56,7 @@ public interface ThreadListController<G extends NamedGroup, I extends ThreadItem
 	interface ThreadListDataSource extends DestroyableContext {
 
 		@UiThread @Nullable
-		MessageId getLastVisibleMessageId();
+		MessageId getFirstVisibleMessageId();
 	}
 
 }

briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListControllerImpl.java

@@ -34,8 +34,6 @@ import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.Executor;
 import java.util.logging.Logger;
 
-import javax.inject.Inject;
-
 import static java.util.logging.Level.INFO;
 import static java.util.logging.Level.WARNING;
 
@@ -56,20 +54,21 @@ public abstract class ThreadListControllerImpl<G extends NamedGroup, I extends T
 	protected final AndroidNotificationManager notificationManager;
 	protected final Executor cryptoExecutor;
 	protected final Clock clock;
+	private final MessageTracker messageTracker;
 	protected volatile L listener;
-	@Inject
-	MessageTracker messageTracker;
 
 	protected ThreadListControllerImpl(@DatabaseExecutor Executor dbExecutor,
 			LifecycleManager lifecycleManager, IdentityManager identityManager,
 			@CryptoExecutor Executor cryptoExecutor, EventBus eventBus,
-			Clock clock, AndroidNotificationManager notificationManager) {
+			Clock clock, AndroidNotificationManager notificationManager,
+			MessageTracker messageTracker) {
 		super(dbExecutor, lifecycleManager);
 		this.identityManager = identityManager;
 		this.cryptoExecutor = cryptoExecutor;
 		this.notificationManager = notificationManager;
 		this.clock = clock;
 		this.eventBus = eventBus;
+		this.messageTracker = messageTracker;
 	}
 
 	@Override
@@ -100,14 +99,19 @@ public abstract class ThreadListControllerImpl<G extends NamedGroup, I extends T
 
 	@Override
 	public void onActivityDestroy() {
-		try {
+		dbExecutor.execute(new Runnable() {
-			messageTracker
+			@Override
-					.storeMessageId(groupId,
+			public void run() {
-							listener.getLastVisibleMessageId());
+				try {
-		} catch (DbException e) {
+					messageTracker
-			if (LOG.isLoggable(WARNING))
+							.storeMessageId(groupId,
-				LOG.log(WARNING, e.toString(), e);
+									listener.getFirstVisibleMessageId());
-		}
+				} catch (DbException e) {
+					if (LOG.isLoggable(WARNING))
+						LOG.log(WARNING, e.toString(), e);
+				}
+			}
+		});
 	}
 
 	@CallSuper
@@ -304,19 +308,16 @@ public abstract class ThreadListControllerImpl<G extends NamedGroup, I extends T
 	@DatabaseExecutor
 	protected abstract void deleteNamedGroup(G groupItem) throws DbException;
 
-	private ThreadItemList<I> buildItems(Collection<H> headers) {
+	private ThreadItemList<I> buildItems(Collection<H> headers)
+			throws DbException {
 		ThreadItemList<I> items = new ThreadItemListImpl<>();
 		for (H h : headers) {
 			items.add(buildItem(h, bodyCache.get(h.getId())));
 		}
-		try {
+		MessageId msgId = messageTracker.loadStoredMessageId(groupId);
-			MessageId msgId = messageTracker.loadStoredMessageId(groupId);
+		if (LOG.isLoggable(INFO))
-			if (LOG.isLoggable(INFO))
+			LOG.info("Loaded last top visible message id " + msgId);
-				LOG.info("Loaded last top visible message id " + msgId);
+		items.setBottomVisibleItemId(msgId);
-			items.setBottomVisibleItemId(msgId);
-		} catch (DbException e) {
-			e.printStackTrace();
-		}
 		return items;
 	}
 

briar-android/src/main/java/org/briarproject/briar/android/view/UnreadMessageButton.java

@@ -36,8 +36,7 @@ public class UnreadMessageButton extends FrameLayout {
 
 		LayoutInflater inflater = (LayoutInflater) context
 				.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
-		inflater
+		inflater.inflate(R.layout.unread_message_button, this, true);
-				.inflate(R.layout.unread_message_button, this, true);
 
 		fab = (FloatingActionButton) findViewById(R.id.fab);
 		unread = (TextView) findViewById(R.id.unreadCountView);
@@ -64,15 +63,11 @@ public class UnreadMessageButton extends FrameLayout {
 
 	public void setUnreadCount(int count) {
 		if (count == 0) {
-			fab.setVisibility(GONE);
+			setVisibility(INVISIBLE);
-//			fab.hide();
-			unread.setVisibility(GONE);
 		} else {
 			// FIXME: Use animations when upgrading to support library 24.2.0
 			//        https://code.google.com/p/android/issues/detail?id=216469
-			fab.setVisibility(VISIBLE);
+			setVisibility(VISIBLE);
-//			if (!fab.isShown()) fab.show();
-			unread.setVisibility(VISIBLE);
 			unread.setText(String.valueOf(count));
 		}
 	}

briar-core/src/test/java/org/briarproject/briar/client/MessageTrackerTest.java

@@ -0,0 +1,57 @@
+package org.briarproject.briar.client;
+
+import org.briarproject.bramble.api.client.ClientHelper;
+import org.briarproject.bramble.api.data.BdfDictionary;
+import org.briarproject.bramble.api.data.BdfEntry;
+import org.briarproject.bramble.api.db.DatabaseComponent;
+import org.briarproject.bramble.api.sync.GroupId;
+import org.briarproject.bramble.api.sync.MessageId;
+import org.briarproject.bramble.test.TestUtils;
+import org.briarproject.briar.api.client.MessageTracker;
+import org.briarproject.briar.test.BriarTestCase;
+import org.jmock.Expectations;
+import org.jmock.Mockery;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Test;
+
+import static org.briarproject.briar.client.MessageTrackerConstants.GROUP_KEY_STORED_MESSAGE_ID;
+
+public class MessageTrackerTest extends BriarTestCase {
+
+	protected final Mockery context = new Mockery();
+	protected final GroupId groupId = new GroupId(TestUtils.getRandomId());
+	protected final ClientHelper clientHelper =
+			context.mock(ClientHelper.class);
+	private final DatabaseComponent db = context.mock(DatabaseComponent.class);
+	private final MessageId messageId = new MessageId(TestUtils.getRandomId());
+	private final MessageTracker messageTracker =
+			new MessageTrackerImpl(db, clientHelper);
+	private final BdfDictionary dictionary = BdfDictionary.of(
+			new BdfEntry(GROUP_KEY_STORED_MESSAGE_ID, messageId)
+	);
+
+	@Test
+	public void testMessageStore() throws Exception {
+		context.checking(new Expectations() {{
+			oneOf(clientHelper).mergeGroupMetadata(groupId, dictionary);
+		}});
+		messageTracker.storeMessageId(groupId, messageId);
+	}
+
+	@Test
+	public void testMessageLoad() throws Exception {
+		context.checking(new Expectations() {{
+			oneOf(clientHelper).getGroupMetadataAsDictionary(groupId);
+			will(returnValue(dictionary));
+		}});
+		MessageId loadedId = messageTracker.loadStoredMessageId(groupId);
+		Assert.assertNotNull(loadedId);
+		Assert.assertTrue(messageId.equals(loadedId));
+	}
+
+	@After
+	public void checkExpectations() {
+		context.assertIsSatisfied();
+	}
+}

briar-core/src/test/java/org/briarproject/briar/forum/ForumManagerTest.java

@@ -1,10 +1,7 @@
 package org.briarproject.briar.forum;
 
-import org.briarproject.bramble.api.db.DbException;
 import org.briarproject.bramble.api.sync.GroupId;
-import org.briarproject.bramble.api.sync.MessageId;
 import org.briarproject.bramble.test.TestDatabaseModule;
-import org.briarproject.bramble.test.TestUtils;
 import org.briarproject.briar.api.forum.Forum;
 import org.briarproject.briar.api.forum.ForumManager;
 import org.briarproject.briar.api.forum.ForumPost;
@@ -13,7 +10,6 @@ import org.briarproject.briar.api.forum.ForumSharingManager;
 import org.briarproject.briar.test.BriarIntegrationTest;
 import org.briarproject.briar.test.BriarIntegrationTestComponent;
 import org.briarproject.briar.test.DaggerBriarIntegrationTestComponent;
-import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -227,18 +223,4 @@ public class ForumManagerTest
 		assertEquals(1, forumManager1.getPostHeaders(g1).size());
 	}
 
-	@Test
-	public void testMessageStoreAndLoad() {
-		MessageId msgId = new MessageId(TestUtils.getRandomId());
-		MessageId loadedId = null;
-		try {
-			messageTracker0.storeMessageId(groupId0, msgId);
-			loadedId = messageTracker0.loadStoredMessageId(groupId0);
-		} catch (DbException e) {
-			e.printStackTrace();
-		}
-		Assert.assertNotNull(loadedId);
-		Assert.assertTrue(msgId.equals(loadedId));
-	}
-
 }