From ff70315d5cf3f9abcb50e9cc440c90bcfda0fc9d Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 4 Jan 2021 16:19:29 -0300 Subject: [PATCH] Address small things found in code review of group list view model migration. --- .../privategroup/list/GroupListFragment.java | 2 -- .../privategroup/list/GroupListViewModel.java | 33 ++++++++----------- .../briar/android/viewmodel/DbViewModel.java | 25 +++++++------- .../android/viewmodel/LiveDataTestUtil.java | 8 ++--- 4 files changed, 31 insertions(+), 37 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListFragment.java index fabed8c79..dcd023e5f 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListFragment.java @@ -152,8 +152,6 @@ public class GroupListFragment extends BaseFragment implements */ @Override public void onClick(View v) { - // The snackbar dismisses itself when this is called - // and does not come back until the fragment gets recreated. Intent i = new Intent(getContext(), GroupInvitationActivity.class); startActivity(i); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListViewModel.java index bc0d50682..25857fca5 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListViewModel.java @@ -5,7 +5,6 @@ import android.app.Application; import org.briarproject.bramble.api.contact.ContactManager; import org.briarproject.bramble.api.db.DatabaseExecutor; import org.briarproject.bramble.api.db.DbException; -import org.briarproject.bramble.api.db.NoSuchGroupException; import org.briarproject.bramble.api.db.Transaction; import org.briarproject.bramble.api.db.TransactionManager; import org.briarproject.bramble.api.event.Event; @@ -152,22 +151,18 @@ class GroupListViewModel extends DbViewModel implements EventListener { List items = new ArrayList<>(groups.size()); Map authorInfos = new HashMap<>(); for (PrivateGroup g : groups) { - try { - GroupId id = g.getId(); - AuthorId authorId = g.getCreator().getId(); - AuthorInfo authorInfo; - if (authorInfos.containsKey(authorId)) { - authorInfo = requireNonNull(authorInfos.get(authorId)); - } else { - authorInfo = contactManager.getAuthorInfo(txn, authorId); - authorInfos.put(authorId, authorInfo); - } - GroupCount count = groupManager.getGroupCount(txn, id); - boolean dissolved = groupManager.isDissolved(txn, id); - items.add(new GroupItem(g, authorInfo, count, dissolved)); - } catch (NoSuchGroupException e) { - // Continue + GroupId id = g.getId(); + AuthorId authorId = g.getCreator().getId(); + AuthorInfo authorInfo; + if (authorInfos.containsKey(authorId)) { + authorInfo = requireNonNull(authorInfos.get(authorId)); + } else { + authorInfo = contactManager.getAuthorInfo(txn, authorId); + authorInfos.put(authorId, authorInfo); } + GroupCount count = groupManager.getGroupCount(txn, id); + boolean dissolved = groupManager.isDissolved(txn, id); + items.add(new GroupItem(g, authorInfo, count, dissolved)); } Collections.sort(items); logDuration(LOG, "Loading groups", start); @@ -177,7 +172,7 @@ class GroupListViewModel extends DbViewModel implements EventListener { @UiThread private void onGroupMessageAdded(GroupMessageHeader header) { GroupId g = header.getGroupId(); - List list = updateListItem(groupItems, + List list = updateListItems(groupItems, itemToTest -> itemToTest.getId().equals(g), itemToUpdate -> new GroupItem(itemToUpdate, header)); if (list == null) return; @@ -188,7 +183,7 @@ class GroupListViewModel extends DbViewModel implements EventListener { @UiThread private void onGroupDissolved(GroupId groupId) { - List list = updateListItem(groupItems, + List list = updateListItems(groupItems, itemToTest -> itemToTest.getId().equals(groupId), itemToUpdate -> new GroupItem(itemToUpdate, true)); if (list == null) return; @@ -198,7 +193,7 @@ class GroupListViewModel extends DbViewModel implements EventListener { @UiThread private void onGroupRemoved(GroupId groupId) { List list = - removeListItem(groupItems, i -> i.getId().equals(groupId)); + removeListItems(groupItems, i -> i.getId().equals(groupId)); if (list == null) return; groupItems.setValue(new LiveResult<>(list)); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/viewmodel/DbViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/viewmodel/DbViewModel.java index d14260aae..b6966c95a 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/viewmodel/DbViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/viewmodel/DbViewModel.java @@ -62,7 +62,7 @@ public abstract class DbViewModel extends AndroidViewModel { *

* If you need a list of items to be displayed in a * {@link RecyclerView.Adapter}, - * use {@link #loadList(DbCallable, UiCallable)} instead. + * use {@link #loadList(DbCallable, UiConsumer)} instead. */ protected void runOnDbThread(Runnable task) { dbExecutor.execute(() -> { @@ -92,13 +92,13 @@ public abstract class DbViewModel extends AndroidViewModel { */ protected > void loadList( DbCallable task, - UiCallable> uiUpdate) { + UiConsumer> uiConsumer) { dbExecutor.execute(() -> { try { lifecycleManager.waitForDatabase(); db.transaction(true, txn -> { T t = task.call(txn); - txn.attach(() -> uiUpdate.call(new LiveResult<>(t))); + txn.attach(() -> uiConsumer.accept(new LiveResult<>(t))); }); } catch (InterruptedException e) { LOG.warning("Interrupted while waiting for database"); @@ -106,15 +106,15 @@ public abstract class DbViewModel extends AndroidViewModel { } catch (DbException e) { logException(LOG, WARNING, e); androidExecutor.runOnUiThread( - () -> uiUpdate.call(new LiveResult<>(e))); + () -> uiConsumer.accept(new LiveResult<>(e))); } }); } @NotNullByDefault - public interface UiCallable { + public interface UiConsumer { @UiThread - void call(T t); + void accept(T t); } /** @@ -130,10 +130,10 @@ public abstract class DbViewModel extends AndroidViewModel { * */ @Nullable - protected List updateListItem( + protected List updateListItems( LiveData>> liveData, Function test, Function replacer) { - List items = getList(liveData); + List items = getListCopy(liveData); if (items == null) return null; ListIterator iterator = items.listIterator(); @@ -161,9 +161,9 @@ public abstract class DbViewModel extends AndroidViewModel { * */ @Nullable - protected List removeListItem( + protected List removeListItems( LiveData>> liveData, Function test) { - List items = getList(liveData); + List items = getListCopy(liveData); if (items == null) return null; ListIterator iterator = items.listIterator(); @@ -179,11 +179,12 @@ public abstract class DbViewModel extends AndroidViewModel { } /** - * Retrieves the list of items from the given LiveData + * Retrieves a copy of the list of items from the given LiveData * or null if it is not available. + * The list copy can be safely mutated. */ @Nullable - private List getList(LiveData>> liveData) { + private List getListCopy(LiveData>> liveData) { LiveResult> value = liveData.getValue(); if (value == null) return null; List list = value.getResultOrNull(); diff --git a/briar-android/src/test/java/org/briarproject/briar/android/viewmodel/LiveDataTestUtil.java b/briar-android/src/test/java/org/briarproject/briar/android/viewmodel/LiveDataTestUtil.java index 0c26fba5d..f1983ab73 100644 --- a/briar-android/src/test/java/org/briarproject/briar/android/viewmodel/LiveDataTestUtil.java +++ b/briar-android/src/test/java/org/briarproject/briar/android/viewmodel/LiveDataTestUtil.java @@ -7,6 +7,7 @@ package org.briarproject.briar.android.viewmodel; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import androidx.annotation.Nullable; import androidx.lifecycle.LiveData; @@ -15,12 +16,12 @@ import androidx.lifecycle.Observer; public class LiveDataTestUtil { public static T getOrAwaitValue(final LiveData liveData) throws InterruptedException { - final Object[] data = new Object[1]; + final AtomicReference data = new AtomicReference<>(); final CountDownLatch latch = new CountDownLatch(1); Observer observer = new Observer() { @Override public void onChanged(@Nullable T o) { - data[0] = o; + data.set(o); latch.countDown(); liveData.removeObserver(this); } @@ -30,7 +31,6 @@ public class LiveDataTestUtil { if (!latch.await(2, TimeUnit.SECONDS)) { throw new RuntimeException("LiveData value was never set."); } - //noinspection unchecked - return (T) data[0]; + return data.get(); } }