From 0ccc7e3c06fd2a2e783a7674794a07ec80c80ae9 Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Wed, 13 Jan 2021 11:37:29 -0400 Subject: [PATCH] Distinguish between primary and secondary devices in participants list. --- .../webrtc/CallParticipantListUpdate.java | 66 +++++++------------ ...CallParticipantsListUpdatePopupWindow.java | 36 ++++------ .../CallParticipantViewState.java | 3 +- .../securesms/events/CallParticipant.java | 43 ++++++++++-- .../BeginCallActionProcessorDelegate.java | 6 +- .../service/webrtc/GroupActionProcessor.java | 19 +++++- .../webrtc/GroupPreJoinActionProcessor.java | 2 +- app/src/main/res/values/strings.xml | 6 +- .../webrtc/CallParticipantListUpdateTest.java | 52 ++++++++------- .../ParticipantCollectionTest.java | 3 +- 10 files changed, 132 insertions(+), 104 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/webrtc/CallParticipantListUpdate.java b/app/src/main/java/org/thoughtcrime/securesms/components/webrtc/CallParticipantListUpdate.java index 495768578..2fad73cd3 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/webrtc/CallParticipantListUpdate.java +++ b/app/src/main/java/org/thoughtcrime/securesms/components/webrtc/CallParticipantListUpdate.java @@ -1,6 +1,7 @@ package org.thoughtcrime.securesms.components.webrtc; import androidx.annotation.NonNull; +import androidx.annotation.VisibleForTesting; import com.annimon.stream.Collectors; import com.annimon.stream.Stream; @@ -21,19 +22,19 @@ import java.util.Set; */ public final class CallParticipantListUpdate { - private final Set added; - private final Set removed; + private final Set added; + private final Set removed; - CallParticipantListUpdate(@NonNull Set added, @NonNull Set removed) { + CallParticipantListUpdate(@NonNull Set added, @NonNull Set removed) { this.added = added; this.removed = removed; } - public @NonNull Set getAdded() { + public @NonNull Set getAdded() { return added; } - public @NonNull Set getRemoved() { + public @NonNull Set getRemoved() { return removed; } @@ -68,66 +69,47 @@ public final class CallParticipantListUpdate { public static @NonNull CallParticipantListUpdate computeDeltaUpdate(@NonNull List oldList, @NonNull List newList) { - Set primaries = getPrimaries(oldList, newList); - Set oldParticipants = Stream.of(oldList) + Set oldParticipants = Stream.of(oldList) .filter(p -> p.getCallParticipantId().getDemuxId() != CallParticipantId.DEFAULT_ID) - .map(p -> createHolder(p, primaries.contains(p.getCallParticipantId()))) + .map(CallParticipantListUpdate::createWrapper) .collect(Collectors.toSet()); - Set newParticipants = Stream.of(newList) + Set newParticipants = Stream.of(newList) .filter(p -> p.getCallParticipantId().getDemuxId() != CallParticipantId.DEFAULT_ID) - .map(p -> createHolder(p, primaries.contains(p.getCallParticipantId()))) + .map(CallParticipantListUpdate::createWrapper) .collect(Collectors.toSet()); - Set added = SetUtil.difference(newParticipants, oldParticipants); - Set removed = SetUtil.difference(oldParticipants, newParticipants); + Set added = SetUtil.difference(newParticipants, oldParticipants); + Set removed = SetUtil.difference(oldParticipants, newParticipants); return new CallParticipantListUpdate(added, removed); } - static Holder createHolder(@NonNull CallParticipant callParticipant, boolean isPrimary) { - return new Holder(callParticipant.getCallParticipantId(), callParticipant.getRecipient(), isPrimary); + @VisibleForTesting + static Wrapper createWrapper(@NonNull CallParticipant callParticipant) { + return new Wrapper(callParticipant); } - private static @NonNull Set getPrimaries(@NonNull List oldList, @NonNull List newList) { - return Stream.concat(Stream.of(oldList), Stream.of(newList)) - .map(CallParticipant::getCallParticipantId) - .distinctBy(CallParticipantId::getRecipientId) - .collect(Collectors.toSet()); - } + static final class Wrapper { + private final CallParticipant callParticipant; - static final class Holder { - private final CallParticipantId callParticipantId; - private final Recipient recipient; - private final boolean isPrimary; - - private Holder(@NonNull CallParticipantId callParticipantId, @NonNull Recipient recipient, boolean isPrimary) { - this.callParticipantId = callParticipantId; - this.recipient = recipient; - this.isPrimary = isPrimary; + private Wrapper(@NonNull CallParticipant callParticipant) { + this.callParticipant = callParticipant; } - public @NonNull Recipient getRecipient() { - return recipient; - } - - /** - * Denotes whether this was the first detected instance of this recipient when generating an update. See - * {@link CallParticipantListUpdate#computeDeltaUpdate(List, List)} - */ - public boolean isPrimary() { - return isPrimary; + public @NonNull CallParticipant getCallParticipant() { + return callParticipant; } @Override public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - Holder holder = (Holder) o; - return callParticipantId.equals(holder.callParticipantId); + Wrapper wrapper = (Wrapper) o; + return callParticipant.getCallParticipantId().equals(wrapper.callParticipant.getCallParticipantId()); } @Override public int hashCode() { - return Objects.hash(callParticipantId); + return Objects.hash(callParticipant.getCallParticipantId()); } } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/webrtc/CallParticipantsListUpdatePopupWindow.java b/app/src/main/java/org/thoughtcrime/securesms/components/webrtc/CallParticipantsListUpdatePopupWindow.java index e5bbc729e..6cde42600 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/webrtc/CallParticipantsListUpdatePopupWindow.java +++ b/app/src/main/java/org/thoughtcrime/securesms/components/webrtc/CallParticipantsListUpdatePopupWindow.java @@ -31,8 +31,8 @@ public class CallParticipantsListUpdatePopupWindow extends PopupWindow { private final AvatarImageView avatarImageView; private final TextView descriptionTextView; - private final Set pendingAdditions = new HashSet<>(); - private final Set pendingRemovals = new HashSet<>(); + private final Set pendingAdditions = new HashSet<>(); + private final Set pendingRemovals = new HashSet<>(); private boolean isEnabled = true; @@ -112,18 +112,18 @@ public class CallParticipantsListUpdatePopupWindow extends PopupWindow { avatarImageView.setVisibility(recipient == null ? View.GONE : View.VISIBLE); } - private void setDescription(@NonNull Set holders, boolean isAdded) { - if (holders.isEmpty()) { + private void setDescription(@NonNull Set wrappers, boolean isAdded) { + if (wrappers.isEmpty()) { descriptionTextView.setText(""); } else { - setDescriptionForRecipients(holders, isAdded); + setDescriptionForRecipients(wrappers, isAdded); } } - private void setDescriptionForRecipients(@NonNull Set recipients, boolean isAdded) { - Iterator iterator = recipients.iterator(); - Context context = getContentView().getContext(); - String description; + private void setDescriptionForRecipients(@NonNull Set recipients, boolean isAdded) { + Iterator iterator = recipients.iterator(); + Context context = getContentView().getContext(); + String description; switch (recipients.size()) { case 0: @@ -144,22 +144,14 @@ public class CallParticipantsListUpdatePopupWindow extends PopupWindow { descriptionTextView.setText(description); } - private @NonNull Recipient getNextRecipient(@NonNull Iterator holderIterator) { - return holderIterator.next().getRecipient(); + private @NonNull Recipient getNextRecipient(@NonNull Iterator wrapperIterator) { + return wrapperIterator.next().getCallParticipant().getRecipient(); } - private @NonNull String getNextDisplayName(@NonNull Iterator holderIterator) { - CallParticipantListUpdate.Holder holder = holderIterator.next(); - Recipient recipient = holder.getRecipient(); + private @NonNull String getNextDisplayName(@NonNull Iterator wrapperIterator) { + CallParticipantListUpdate.Wrapper wrapper = wrapperIterator.next(); - if (recipient.isSelf()) { - return getContentView().getContext().getString(R.string.CallParticipantsListUpdatePopupWindow__you_on_another_device); - } else if (holder.isPrimary()) { - return recipient.getDisplayName(getContentView().getContext()); - } else { - return getContentView().getContext().getString(R.string.CallParticipantsListUpdatePopupWindow__s_on_another_device, - recipient.getDisplayName(getContentView().getContext())); - } + return wrapper.getCallParticipant().getRecipientDisplayName(getContentView().getContext()); } private static @StringRes int getOneMemberDescriptionResourceId(boolean isAdded) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/webrtc/participantslist/CallParticipantViewState.java b/app/src/main/java/org/thoughtcrime/securesms/components/webrtc/participantslist/CallParticipantViewState.java index 6c5d471ae..b8c22b18d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/webrtc/participantslist/CallParticipantViewState.java +++ b/app/src/main/java/org/thoughtcrime/securesms/components/webrtc/participantslist/CallParticipantViewState.java @@ -25,8 +25,7 @@ public final class CallParticipantViewState extends RecipientMappingModel%1$s and %2$s left %1$s, %2$s and %3$s left %1$s, %2$s and %3$d others left - You (on another device) - %1$s (on another device) + + You + You (on another device) + %1$s (on another device) Deleting your account will: diff --git a/app/src/test/java/org/thoughtcrime/securesms/components/webrtc/CallParticipantListUpdateTest.java b/app/src/test/java/org/thoughtcrime/securesms/components/webrtc/CallParticipantListUpdateTest.java index 42d3ef5db..f87ebde06 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/components/webrtc/CallParticipantListUpdateTest.java +++ b/app/src/test/java/org/thoughtcrime/securesms/components/webrtc/CallParticipantListUpdateTest.java @@ -1,5 +1,7 @@ package org.thoughtcrime.securesms.components.webrtc; +import androidx.annotation.NonNull; + import com.annimon.stream.Stream; import org.hamcrest.Matchers; @@ -27,9 +29,9 @@ public class CallParticipantListUpdateTest { @Test public void givenEmptySets_thenExpectNoChanges() { // GIVEN - Set added = Collections.emptySet(); - Set removed = Collections.emptySet(); - CallParticipantListUpdate update = new CallParticipantListUpdate(added, removed); + Set added = Collections.emptySet(); + Set removed = Collections.emptySet(); + CallParticipantListUpdate update = new CallParticipantListUpdate(added, removed); // THEN assertTrue(update.hasNoChanges()); @@ -39,9 +41,9 @@ public class CallParticipantListUpdateTest { @Test public void givenOneEmptySet_thenExpectMultipleChanges() { // GIVEN - Set added = new HashSet<>(Arrays.asList(createHolders(1, 2, 3))); - Set removed = Collections.emptySet(); - CallParticipantListUpdate update = new CallParticipantListUpdate(added, removed); + Set added = new HashSet<>(Arrays.asList(createWrappers(1, 2, 3))); + Set removed = Collections.emptySet(); + CallParticipantListUpdate update = new CallParticipantListUpdate(added, removed); // THEN assertFalse(update.hasNoChanges()); @@ -51,9 +53,9 @@ public class CallParticipantListUpdateTest { @Test public void givenNoEmptySets_thenExpectMultipleChanges() { // GIVEN - Set added = new HashSet<>(Arrays.asList(createHolders(1, 2, 3))); - Set removed = new HashSet<>(Arrays.asList(createHolders(4, 5, 6))); - CallParticipantListUpdate update = new CallParticipantListUpdate(added, removed); + Set added = new HashSet<>(Arrays.asList(createWrappers(1, 2, 3))); + Set removed = new HashSet<>(Arrays.asList(createWrappers(4, 5, 6))); + CallParticipantListUpdate update = new CallParticipantListUpdate(added, removed); // THEN assertFalse(update.hasNoChanges()); @@ -63,9 +65,9 @@ public class CallParticipantListUpdateTest { @Test public void givenOneSetWithSingleItemAndAnEmptySet_thenExpectSingleChange() { // GIVEN - Set added = new HashSet<>(Arrays.asList(createHolders(1))); - Set removed = Collections.emptySet(); - CallParticipantListUpdate update = new CallParticipantListUpdate(added, removed); + Set added = new HashSet<>(Arrays.asList(createWrappers(1))); + Set removed = Collections.emptySet(); + CallParticipantListUpdate update = new CallParticipantListUpdate(added, removed); // THEN assertFalse(update.hasNoChanges()); @@ -83,7 +85,7 @@ public class CallParticipantListUpdateTest { // THEN assertFalse(update.hasNoChanges()); assertTrue(update.getRemoved().isEmpty()); - assertThat(update.getAdded(), Matchers.containsInAnyOrder(createHolders(1, 2, 3, 4, 5))); + assertThat(update.getAdded(), Matchers.containsInAnyOrder(createWrappers(1, 2, 3, 4, 5))); } @Test @@ -121,8 +123,8 @@ public class CallParticipantListUpdateTest { // THEN assertFalse(update.hasNoChanges()); - assertThat(update.getAdded(), Matchers.containsInAnyOrder(createHolders(6))); - assertThat(update.getRemoved(), Matchers.containsInAnyOrder(createHolders(1))); + assertThat(update.getAdded(), Matchers.containsInAnyOrder(createWrappers(6))); + assertThat(update.getRemoved(), Matchers.containsInAnyOrder(createWrappers(1))); } @Test @@ -134,17 +136,18 @@ public class CallParticipantListUpdateTest { CallParticipantListUpdate update = CallParticipantListUpdate.computeDeltaUpdate(Collections.emptyList(), list); // THEN - List isPrimaryList = Stream.of(update.getAdded()).map(CallParticipantListUpdate.Holder::isPrimary).toList(); + List isPrimaryList = Stream.of(update.getAdded()).map(wrapper -> wrapper.getCallParticipant().isPrimary()).toList(); assertThat(isPrimaryList, Matchers.containsInAnyOrder(true, false, false)); } - static CallParticipantListUpdate.Holder[] createHolders(long ... recipientIds) { - CallParticipantListUpdate.Holder[] ids = new CallParticipantListUpdate.Holder[recipientIds.length]; + static CallParticipantListUpdate.Wrapper[] createWrappers(long ... recipientIds) { + CallParticipantListUpdate.Wrapper[] ids = new CallParticipantListUpdate.Wrapper[recipientIds.length]; + Set primaries = new HashSet<>(); for (int i = 0; i < recipientIds.length; i++) { - CallParticipant participant = createParticipant(recipientIds[i], recipientIds[i]); + CallParticipant participant = createParticipant(recipientIds[i], recipientIds[i], primaries.contains(recipientIds[i]) ? CallParticipant.DeviceOrdinal.SECONDARY : CallParticipant.DeviceOrdinal.PRIMARY); - ids[i] = CallParticipantListUpdate.createHolder(participant, true); + ids[i] = CallParticipantListUpdate.createWrapper(participant); } return ids; @@ -162,17 +165,20 @@ public class CallParticipantListUpdateTest { private static List createParticipants(long[] recipientIds, long[] placeholderIds) { List participants = new ArrayList<>(recipientIds.length); + Set primaries = new HashSet<>(); + for (int i = 0; i < recipientIds.length; i++) { - participants.add(createParticipant(recipientIds[i], placeholderIds[i])); + participants.add(createParticipant(recipientIds[i], placeholderIds[i], primaries.contains(recipientIds[i]) ? CallParticipant.DeviceOrdinal.SECONDARY : CallParticipant.DeviceOrdinal.PRIMARY)); + primaries.add(recipientIds[i]); } return participants; } - private static CallParticipant createParticipant(long recipientId, long deMuxId) { + private static CallParticipant createParticipant(long recipientId, long deMuxId, @NonNull CallParticipant.DeviceOrdinal deviceOrdinal) { Recipient recipient = new Recipient(RecipientId.from(recipientId), mock(RecipientDetails.class), true); - return CallParticipant.createRemote(new CallParticipantId(deMuxId, recipient.getId()), recipient, null, new BroadcastVideoSink(null), false, false, -1, false, 0); + return CallParticipant.createRemote(new CallParticipantId(deMuxId, recipient.getId()), recipient, null, new BroadcastVideoSink(null), false, false, -1, false, 0, deviceOrdinal); } } \ No newline at end of file diff --git a/app/src/test/java/org/thoughtcrime/securesms/service/webrtc/collections/ParticipantCollectionTest.java b/app/src/test/java/org/thoughtcrime/securesms/service/webrtc/collections/ParticipantCollectionTest.java index 6ee9ae72d..46963fc98 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/service/webrtc/collections/ParticipantCollectionTest.java +++ b/app/src/test/java/org/thoughtcrime/securesms/service/webrtc/collections/ParticipantCollectionTest.java @@ -213,6 +213,7 @@ public class ParticipantCollectionTest { false, lastSpoke, false, - added); + added, + CallParticipant.DeviceOrdinal.PRIMARY); } } \ No newline at end of file