From f15072bc8d9e11a788750af5a061c36e38f14644 Mon Sep 17 00:00:00 2001
From: Greyson Parrelli <greyson@signal.org>
Date: Fri, 22 Apr 2022 08:32:07 -0400
Subject: [PATCH] Fix other group update description bugs and add tests.
---
.../model/GroupsV2UpdateMessageProducer.java | 55 +++++++++++----
.../GroupsV2UpdateMessageProducerTest.java | 70 ++++++++++++++++++-
2 files changed, 108 insertions(+), 17 deletions(-)
diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/model/GroupsV2UpdateMessageProducer.java b/app/src/main/java/org/thoughtcrime/securesms/database/model/GroupsV2UpdateMessageProducer.java
index d94174fcdb..91473d20ce 100644
--- a/app/src/main/java/org/thoughtcrime/securesms/database/model/GroupsV2UpdateMessageProducer.java
+++ b/app/src/main/java/org/thoughtcrime/securesms/database/model/GroupsV2UpdateMessageProducer.java
@@ -13,6 +13,7 @@ import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.PluralsRes;
import androidx.annotation.StringRes;
+import androidx.annotation.VisibleForTesting;
import com.google.protobuf.ByteString;
@@ -41,9 +42,11 @@ import org.whispersystems.signalservice.api.util.UuidUtil;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
+import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
@@ -865,33 +868,55 @@ final class GroupsV2UpdateMessageProducer {
return args.toArray();
}
- private static @NonNull Spannable makeRecipientsClickable(@NonNull Context context, @NonNull String template, @NonNull List<RecipientId> recipientIds, @Nullable Consumer<RecipientId> clickHandler) {
+ @VisibleForTesting
+ static @NonNull Spannable makeRecipientsClickable(@NonNull Context context, @NonNull String template, @NonNull List<RecipientId> recipientIds, @Nullable Consumer<RecipientId> clickHandler) {
SpannableStringBuilder builder = new SpannableStringBuilder();
int startIndex = 0;
- for (RecipientId recipientId : recipientIds) {
- String placeholder = makePlaceholder(recipientId);
- int placeHolderStart = template.indexOf(placeholder, startIndex);
- String beforeChunk = template.substring(startIndex, placeHolderStart);
+ Map<String, RecipientId> idByPlaceholder = new HashMap<>();
+ for (RecipientId id : recipientIds) {
+ idByPlaceholder.put(makePlaceholder(id), id);
+ }
+
+ while (startIndex < template.length()) {
+ Map.Entry<String, RecipientId> nearestEntry = null;
+ int nearestPosition = Integer.MAX_VALUE;
+
+ for (Map.Entry<String, RecipientId> entry : idByPlaceholder.entrySet()) {
+ String placeholder = entry.getKey();
+ int placeholderStart = template.indexOf(placeholder, startIndex);
- builder.append(beforeChunk);
- builder.append(SpanUtil.clickable(Recipient.resolved(recipientId).getDisplayName(context), 0, v -> {
- if (!recipientId.isUnknown() && clickHandler != null) {
- clickHandler.accept(recipientId);
+ if (placeholderStart >= 0 && placeholderStart < nearestPosition) {
+ nearestPosition = placeholderStart;
+ nearestEntry = entry;
}
- }));
+ }
- startIndex = placeHolderStart + placeholder.length();
- }
+ if (nearestEntry != null) {
+ String placeholder = nearestEntry.getKey();
+ RecipientId recipientId = nearestEntry.getValue();
- if (startIndex < template.length()) {
- builder.append(template.substring(startIndex));
+ String beforeChunk = template.substring(startIndex, nearestPosition);
+
+ builder.append(beforeChunk);
+ builder.append(SpanUtil.clickable(Recipient.resolved(recipientId).getDisplayName(context), 0, v -> {
+ if (!recipientId.isUnknown() && clickHandler != null) {
+ clickHandler.accept(recipientId);
+ }
+ }));
+
+ startIndex = nearestPosition + placeholder.length();
+ } else {
+ builder.append(template.substring(startIndex));
+ startIndex = template.length();
+ }
}
return builder;
}
- private static @NonNull String makePlaceholder(@NonNull RecipientId recipientId) {
+ @VisibleForTesting
+ static @NonNull String makePlaceholder(@NonNull RecipientId recipientId) {
return "{{SPAN_PLACEHOLDER_" + recipientId + "}}";
}
}
\ No newline at end of file
diff --git a/app/src/test/java/org/thoughtcrime/securesms/database/model/GroupsV2UpdateMessageProducerTest.java b/app/src/test/java/org/thoughtcrime/securesms/database/model/GroupsV2UpdateMessageProducerTest.java
index 84ce95db16..786c4ced5c 100644
--- a/app/src/test/java/org/thoughtcrime/securesms/database/model/GroupsV2UpdateMessageProducerTest.java
+++ b/app/src/test/java/org/thoughtcrime/securesms/database/model/GroupsV2UpdateMessageProducerTest.java
@@ -30,6 +30,7 @@ import org.whispersystems.signalservice.api.push.ServiceId;
import org.whispersystems.signalservice.api.util.UuidUtil;
import java.util.Arrays;
+import java.util.Collections;
import java.util.List;
import java.util.UUID;
import java.util.stream.Collectors;
@@ -1349,8 +1350,73 @@ public final class GroupsV2UpdateMessageProducerTest {
.build();
assertThat(describeNewGroup(group), is("Group updated."));
- }
-
+ }
+
+ @Test
+ public void makeRecipientsClickable_onePlaceholder() {
+ RecipientId id = RecipientId.from(1);
+
+ Spannable result = GroupsV2UpdateMessageProducer.makeRecipientsClickable(
+ ApplicationProvider.getApplicationContext(),
+ GroupsV2UpdateMessageProducer.makePlaceholder(id),
+ Collections.singletonList(id),
+ null
+ );
+
+ assertEquals("Alice", result.toString());
+ }
+
+ @Test
+ public void makeRecipientsClickable_twoPlaceholders_sameRecipient() {
+ RecipientId id = RecipientId.from(1);
+ String placeholder = GroupsV2UpdateMessageProducer.makePlaceholder(id);
+
+ Spannable result = GroupsV2UpdateMessageProducer.makeRecipientsClickable(
+ ApplicationProvider.getApplicationContext(),
+ placeholder + " " + placeholder,
+ Collections.singletonList(id),
+ null
+ );
+
+ assertEquals("Alice Alice", result.toString());
+ }
+
+ @Test
+ public void makeRecipientsClickable_twoPlaceholders_differentRecipient() {
+ RecipientId id1 = RecipientId.from(1);
+ RecipientId id2 = RecipientId.from(2);
+
+ String placeholder1 = GroupsV2UpdateMessageProducer.makePlaceholder(id1);
+ String placeholder2 = GroupsV2UpdateMessageProducer.makePlaceholder(id2);
+
+ Spannable result = GroupsV2UpdateMessageProducer.makeRecipientsClickable(
+ ApplicationProvider.getApplicationContext(),
+ placeholder1 + " " + placeholder2,
+ Arrays.asList(id1, id2),
+ null
+ );
+
+ assertEquals("Alice Bob", result.toString());
+ }
+
+ @Test
+ public void makeRecipientsClickable_complicated() {
+ RecipientId id1 = RecipientId.from(1);
+ RecipientId id2 = RecipientId.from(2);
+
+ String placeholder1 = GroupsV2UpdateMessageProducer.makePlaceholder(id1);
+ String placeholder2 = GroupsV2UpdateMessageProducer.makePlaceholder(id2);
+
+ Spannable result = GroupsV2UpdateMessageProducer.makeRecipientsClickable(
+ ApplicationProvider.getApplicationContext(),
+ placeholder1 + " said hello to " + placeholder2 + ", and " + placeholder2 + " said hello back to " + placeholder1 + ".",
+ Arrays.asList(id1, id2),
+ null
+ );
+
+ assertEquals("Alice said hello to Bob, and Bob said hello back to Alice.", result.toString());
+ }
+
private @NonNull List<String> describeChange(@NonNull DecryptedGroupChange change) {
return describeChange(null, change);
}
--
GitLab