From 948410a06445ee558cb95e403782221a8fe4451f Mon Sep 17 00:00:00 2001 From: Ernir Erlingsson Date: Fri, 5 May 2017 11:22:21 +0200 Subject: [PATCH] fixed unread buttons for threaded lists and akwizgran's comments --- .../android/controller/DbControllerImpl.java | 2 +- .../android/forum/ForumControllerImpl.java | 5 +- .../conversation/GroupControllerImpl.java | 6 +- .../android/threaded/ThreadItemAdapter.java | 16 +----- .../android/threaded/ThreadItemList.java | 2 +- .../android/threaded/ThreadItemListImpl.java | 2 +- .../android/threaded/ThreadListActivity.java | 21 ++++--- .../threaded/ThreadListController.java | 2 +- .../threaded/ThreadListControllerImpl.java | 45 ++++++++------- .../android/view/UnreadMessageButton.java | 11 +--- .../briar/client/MessageTrackerTest.java | 57 +++++++++++++++++++ .../briar/forum/ForumManagerTest.java | 18 ------ 12 files changed, 111 insertions(+), 76 deletions(-) create mode 100644 briar-core/src/test/java/org/briarproject/briar/client/MessageTrackerTest.java diff --git a/briar-android/src/main/java/org/briarproject/briar/android/controller/DbControllerImpl.java b/briar-android/src/main/java/org/briarproject/briar/android/controller/DbControllerImpl.java index eaeb7a06e..6ae01b8a5 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/controller/DbControllerImpl.java +++ b/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 diff --git a/briar-android/src/main/java/org/briarproject/briar/android/forum/ForumControllerImpl.java b/briar-android/src/main/java/org/briarproject/briar/android/forum/ForumControllerImpl.java index 4c52f6154..8d97e7810 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/forum/ForumControllerImpl.java +++ b/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; } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupControllerImpl.java b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupControllerImpl.java index 0c7530559..db6029e57 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupControllerImpl.java +++ b/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; } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemAdapter.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemAdapter.java index 0a36ac80e..99a4a3335 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemAdapter.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemAdapter.java @@ -66,17 +66,7 @@ public class ThreadItemAdapter revision++; } - // Useful when the adapter has not calculated the dimension yet - void postSetItemWithIdVisible(@Nullable final MessageId messageId) { - new Handler().post(new Runnable() { - @Override - public void run() { - setItemWithIdVisible(messageId); - } - }); - } - - void setItemWithIdVisible(@Nullable MessageId messageId) { + void setItemWithIdVisible(MessageId messageId) { if (messageId != null) { int pos = 0; for (I item : items) { @@ -169,7 +159,7 @@ public class ThreadItemAdapter /** * 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 /** * 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++) { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemList.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemList.java index 53d6ed633..d4f067c2f 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemList.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemList.java @@ -9,7 +9,7 @@ import javax.annotation.Nullable; public interface ThreadItemList extends List { @Nullable - MessageId getBottomVisibleItemId(); + MessageId getFirstVisibleItemId(); void setBottomVisibleItemId(@Nullable MessageId bottomVisibleItemId); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemListImpl.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemListImpl.java index 880d29739..f6fb80e68 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemListImpl.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemListImpl.java @@ -12,7 +12,7 @@ public class ThreadItemListImpl extends ArrayList private MessageId bottomVisibleItemId; @Override - public MessageId getBottomVisibleItemId() { + public MessageId getFirstVisibleItemId() { return bottomVisibleItemId; } 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 68cec97a2..b62f35c77 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 @@ -149,11 +149,12 @@ public abstract class ThreadListActivity items) { + adapter.setItems(items); + MessageId messageId = items.getFirstVisibleItemId(); + if (messageId != null) + adapter.setItemWithIdVisible(messageId); + updateUnreadCount(); + list.showData(); + } + protected void loadSharingContacts() { getController().loadSharingContacts( new UiResultExceptionHandler, DbException>( diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListController.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListController.java index 45aa69a0a..d39c8b453 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListController.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListController.java @@ -56,7 +56,7 @@ public interface ThreadListController buildItems(Collection headers) { + private ThreadItemList buildItems(Collection headers) + throws DbException { ThreadItemList items = new ThreadItemListImpl<>(); for (H h : headers) { items.add(buildItem(h, bodyCache.get(h.getId()))); } - try { - MessageId msgId = messageTracker.loadStoredMessageId(groupId); - if (LOG.isLoggable(INFO)) - LOG.info("Loaded last top visible message id " + msgId); - items.setBottomVisibleItemId(msgId); - } catch (DbException e) { - e.printStackTrace(); - } + MessageId msgId = messageTracker.loadStoredMessageId(groupId); + if (LOG.isLoggable(INFO)) + LOG.info("Loaded last top visible message id " + msgId); + items.setBottomVisibleItemId(msgId); return items; } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/UnreadMessageButton.java b/briar-android/src/main/java/org/briarproject/briar/android/view/UnreadMessageButton.java index e7f4fa92f..4a5360043 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/UnreadMessageButton.java +++ b/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 - .inflate(R.layout.unread_message_button, this, true); + inflater.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); -// fab.hide(); - unread.setVisibility(GONE); + setVisibility(INVISIBLE); } 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); -// if (!fab.isShown()) fab.show(); - unread.setVisibility(VISIBLE); + setVisibility(VISIBLE); unread.setText(String.valueOf(count)); } } diff --git a/briar-core/src/test/java/org/briarproject/briar/client/MessageTrackerTest.java b/briar-core/src/test/java/org/briarproject/briar/client/MessageTrackerTest.java new file mode 100644 index 000000000..6f841703f --- /dev/null +++ b/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(); + } +} diff --git a/briar-core/src/test/java/org/briarproject/briar/forum/ForumManagerTest.java b/briar-core/src/test/java/org/briarproject/briar/forum/ForumManagerTest.java index 09805ce44..30567fdf2 100644 --- a/briar-core/src/test/java/org/briarproject/briar/forum/ForumManagerTest.java +++ b/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)); - } - }