Address small things found in code review

of group list view model migration.
This commit is contained in:
Torsten Grote
2021-01-04 16:19:29 -03:00
parent f197243273
commit ff70315d5c
4 changed files with 31 additions and 37 deletions

View File

@@ -152,8 +152,6 @@ public class GroupListFragment extends BaseFragment implements
*/ */
@Override @Override
public void onClick(View v) { 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); Intent i = new Intent(getContext(), GroupInvitationActivity.class);
startActivity(i); startActivity(i);
} }

View File

@@ -5,7 +5,6 @@ import android.app.Application;
import org.briarproject.bramble.api.contact.ContactManager; import org.briarproject.bramble.api.contact.ContactManager;
import org.briarproject.bramble.api.db.DatabaseExecutor; import org.briarproject.bramble.api.db.DatabaseExecutor;
import org.briarproject.bramble.api.db.DbException; 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.Transaction;
import org.briarproject.bramble.api.db.TransactionManager; import org.briarproject.bramble.api.db.TransactionManager;
import org.briarproject.bramble.api.event.Event; import org.briarproject.bramble.api.event.Event;
@@ -152,22 +151,18 @@ class GroupListViewModel extends DbViewModel implements EventListener {
List<GroupItem> items = new ArrayList<>(groups.size()); List<GroupItem> items = new ArrayList<>(groups.size());
Map<AuthorId, AuthorInfo> authorInfos = new HashMap<>(); Map<AuthorId, AuthorInfo> authorInfos = new HashMap<>();
for (PrivateGroup g : groups) { for (PrivateGroup g : groups) {
try { GroupId id = g.getId();
GroupId id = g.getId(); AuthorId authorId = g.getCreator().getId();
AuthorId authorId = g.getCreator().getId(); AuthorInfo authorInfo;
AuthorInfo authorInfo; if (authorInfos.containsKey(authorId)) {
if (authorInfos.containsKey(authorId)) { authorInfo = requireNonNull(authorInfos.get(authorId));
authorInfo = requireNonNull(authorInfos.get(authorId)); } else {
} else { authorInfo = contactManager.getAuthorInfo(txn, authorId);
authorInfo = contactManager.getAuthorInfo(txn, authorId); authorInfos.put(authorId, authorInfo);
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
} }
GroupCount count = groupManager.getGroupCount(txn, id);
boolean dissolved = groupManager.isDissolved(txn, id);
items.add(new GroupItem(g, authorInfo, count, dissolved));
} }
Collections.sort(items); Collections.sort(items);
logDuration(LOG, "Loading groups", start); logDuration(LOG, "Loading groups", start);
@@ -177,7 +172,7 @@ class GroupListViewModel extends DbViewModel implements EventListener {
@UiThread @UiThread
private void onGroupMessageAdded(GroupMessageHeader header) { private void onGroupMessageAdded(GroupMessageHeader header) {
GroupId g = header.getGroupId(); GroupId g = header.getGroupId();
List<GroupItem> list = updateListItem(groupItems, List<GroupItem> list = updateListItems(groupItems,
itemToTest -> itemToTest.getId().equals(g), itemToTest -> itemToTest.getId().equals(g),
itemToUpdate -> new GroupItem(itemToUpdate, header)); itemToUpdate -> new GroupItem(itemToUpdate, header));
if (list == null) return; if (list == null) return;
@@ -188,7 +183,7 @@ class GroupListViewModel extends DbViewModel implements EventListener {
@UiThread @UiThread
private void onGroupDissolved(GroupId groupId) { private void onGroupDissolved(GroupId groupId) {
List<GroupItem> list = updateListItem(groupItems, List<GroupItem> list = updateListItems(groupItems,
itemToTest -> itemToTest.getId().equals(groupId), itemToTest -> itemToTest.getId().equals(groupId),
itemToUpdate -> new GroupItem(itemToUpdate, true)); itemToUpdate -> new GroupItem(itemToUpdate, true));
if (list == null) return; if (list == null) return;
@@ -198,7 +193,7 @@ class GroupListViewModel extends DbViewModel implements EventListener {
@UiThread @UiThread
private void onGroupRemoved(GroupId groupId) { private void onGroupRemoved(GroupId groupId) {
List<GroupItem> list = List<GroupItem> list =
removeListItem(groupItems, i -> i.getId().equals(groupId)); removeListItems(groupItems, i -> i.getId().equals(groupId));
if (list == null) return; if (list == null) return;
groupItems.setValue(new LiveResult<>(list)); groupItems.setValue(new LiveResult<>(list));
} }

View File

@@ -62,7 +62,7 @@ public abstract class DbViewModel extends AndroidViewModel {
* <p> * <p>
* If you need a list of items to be displayed in a * If you need a list of items to be displayed in a
* {@link RecyclerView.Adapter}, * {@link RecyclerView.Adapter},
* use {@link #loadList(DbCallable, UiCallable)} instead. * use {@link #loadList(DbCallable, UiConsumer)} instead.
*/ */
protected void runOnDbThread(Runnable task) { protected void runOnDbThread(Runnable task) {
dbExecutor.execute(() -> { dbExecutor.execute(() -> {
@@ -92,13 +92,13 @@ public abstract class DbViewModel extends AndroidViewModel {
*/ */
protected <T extends List<?>> void loadList( protected <T extends List<?>> void loadList(
DbCallable<T, DbException> task, DbCallable<T, DbException> task,
UiCallable<LiveResult<T>> uiUpdate) { UiConsumer<LiveResult<T>> uiConsumer) {
dbExecutor.execute(() -> { dbExecutor.execute(() -> {
try { try {
lifecycleManager.waitForDatabase(); lifecycleManager.waitForDatabase();
db.transaction(true, txn -> { db.transaction(true, txn -> {
T t = task.call(txn); T t = task.call(txn);
txn.attach(() -> uiUpdate.call(new LiveResult<>(t))); txn.attach(() -> uiConsumer.accept(new LiveResult<>(t)));
}); });
} catch (InterruptedException e) { } catch (InterruptedException e) {
LOG.warning("Interrupted while waiting for database"); LOG.warning("Interrupted while waiting for database");
@@ -106,15 +106,15 @@ public abstract class DbViewModel extends AndroidViewModel {
} catch (DbException e) { } catch (DbException e) {
logException(LOG, WARNING, e); logException(LOG, WARNING, e);
androidExecutor.runOnUiThread( androidExecutor.runOnUiThread(
() -> uiUpdate.call(new LiveResult<>(e))); () -> uiConsumer.accept(new LiveResult<>(e)));
} }
}); });
} }
@NotNullByDefault @NotNullByDefault
public interface UiCallable<T> { public interface UiConsumer<T> {
@UiThread @UiThread
void call(T t); void accept(T t);
} }
/** /**
@@ -130,10 +130,10 @@ public abstract class DbViewModel extends AndroidViewModel {
* </ul> * </ul>
*/ */
@Nullable @Nullable
protected <T> List<T> updateListItem( protected <T> List<T> updateListItems(
LiveData<LiveResult<List<T>>> liveData, Function<T, Boolean> test, LiveData<LiveResult<List<T>>> liveData, Function<T, Boolean> test,
Function<T, T> replacer) { Function<T, T> replacer) {
List<T> items = getList(liveData); List<T> items = getListCopy(liveData);
if (items == null) return null; if (items == null) return null;
ListIterator<T> iterator = items.listIterator(); ListIterator<T> iterator = items.listIterator();
@@ -161,9 +161,9 @@ public abstract class DbViewModel extends AndroidViewModel {
* </ul> * </ul>
*/ */
@Nullable @Nullable
protected <T> List<T> removeListItem( protected <T> List<T> removeListItems(
LiveData<LiveResult<List<T>>> liveData, Function<T, Boolean> test) { LiveData<LiveResult<List<T>>> liveData, Function<T, Boolean> test) {
List<T> items = getList(liveData); List<T> items = getListCopy(liveData);
if (items == null) return null; if (items == null) return null;
ListIterator<T> iterator = items.listIterator(); ListIterator<T> 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. * or null if it is not available.
* The list copy can be safely mutated.
*/ */
@Nullable @Nullable
private <T> List<T> getList(LiveData<LiveResult<List<T>>> liveData) { private <T> List<T> getListCopy(LiveData<LiveResult<List<T>>> liveData) {
LiveResult<List<T>> value = liveData.getValue(); LiveResult<List<T>> value = liveData.getValue();
if (value == null) return null; if (value == null) return null;
List<T> list = value.getResultOrNull(); List<T> list = value.getResultOrNull();

View File

@@ -7,6 +7,7 @@ package org.briarproject.briar.android.viewmodel;
import java.util.concurrent.CountDownLatch; import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import androidx.lifecycle.LiveData; import androidx.lifecycle.LiveData;
@@ -15,12 +16,12 @@ import androidx.lifecycle.Observer;
public class LiveDataTestUtil { public class LiveDataTestUtil {
public static <T> T getOrAwaitValue(final LiveData<T> liveData) public static <T> T getOrAwaitValue(final LiveData<T> liveData)
throws InterruptedException { throws InterruptedException {
final Object[] data = new Object[1]; final AtomicReference<T> data = new AtomicReference<>();
final CountDownLatch latch = new CountDownLatch(1); final CountDownLatch latch = new CountDownLatch(1);
Observer<T> observer = new Observer<T>() { Observer<T> observer = new Observer<T>() {
@Override @Override
public void onChanged(@Nullable T o) { public void onChanged(@Nullable T o) {
data[0] = o; data.set(o);
latch.countDown(); latch.countDown();
liveData.removeObserver(this); liveData.removeObserver(this);
} }
@@ -30,7 +31,6 @@ public class LiveDataTestUtil {
if (!latch.await(2, TimeUnit.SECONDS)) { if (!latch.await(2, TimeUnit.SECONDS)) {
throw new RuntimeException("LiveData value was never set."); throw new RuntimeException("LiveData value was never set.");
} }
//noinspection unchecked return data.get();
return (T) data[0];
} }
} }