From 5b25e38e4108f28761381008a4ea49fe773ec8fa Mon Sep 17 00:00:00 2001 From: Ehren Kret Date: Wed, 15 Sep 2021 16:12:07 -0500 Subject: [PATCH] Ensure badges are in ordered collections --- .../textsecuregcm/WhisperServerService.java | 3 +-- .../badges/ConfiguredProfileBadgeConverter.java | 9 ++++----- .../textsecuregcm/badges/ProfileBadgeConverter.java | 3 +-- .../textsecuregcm/controllers/ProfileController.java | 4 ++-- .../textsecuregcm/entities/Profile.java | 8 ++++---- .../whispersystems/textsecuregcm/storage/Account.java | 6 ++++-- .../badges/ConfiguredProfileBadgeConverterTest.java | 11 +++++------ .../tests/controllers/ProfileControllerTest.java | 3 ++- 8 files changed, 23 insertions(+), 24 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index fa1817b97..6870f213a 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -43,7 +43,6 @@ import java.util.Collections; import java.util.EnumSet; import java.util.List; import java.util.Optional; -import java.util.Set; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; import java.util.concurrent.ExecutorService; @@ -296,7 +295,7 @@ public class WhisperServerService extends Application Set.of(); // TODO: Provide an actual implementation. + ProfileBadgeConverter profileBadgeConverter = (acceptableLanguages, accountBadges) -> List.of(); // TODO: Provide an actual implementation. JdbiFactory jdbiFactory = new JdbiFactory(DefaultNameStrategy.CHECK_EMPTY); Jdbi accountJdbi = jdbiFactory.build(environment, config.getAccountsDatabaseConfiguration(), "accountdb"); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/badges/ConfiguredProfileBadgeConverter.java b/service/src/main/java/org/whispersystems/textsecuregcm/badges/ConfiguredProfileBadgeConverter.java index ac1842dbd..41cb2af71 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/badges/ConfiguredProfileBadgeConverter.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/badges/ConfiguredProfileBadgeConverter.java @@ -14,7 +14,6 @@ import java.util.Map; import java.util.Objects; import java.util.ResourceBundle; import java.util.ResourceBundle.Control; -import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; import org.whispersystems.textsecuregcm.configuration.BadgeConfiguration; @@ -50,11 +49,11 @@ public class ConfiguredProfileBadgeConverter implements ProfileBadgeConverter { } @Override - public Set convert( + public List convert( final List acceptableLanguages, - final Set accountBadges) { + final List accountBadges) { if (accountBadges.isEmpty()) { - return Set.of(); + return List.of(); } final Instant now = clock.instant(); @@ -94,6 +93,6 @@ public class ConfiguredProfileBadgeConverter implements ProfileBadgeConverter { .map(accountBadge -> new Badge(knownBadges.get(accountBadge.getName()).getImageUrl(), resourceBundle.getString(accountBadge.getName() + "_name"), resourceBundle.getString(accountBadge.getName() + "_description"))) - .collect(Collectors.toSet()); + .collect(Collectors.toList()); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/badges/ProfileBadgeConverter.java b/service/src/main/java/org/whispersystems/textsecuregcm/badges/ProfileBadgeConverter.java index e981964b2..a47710e99 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/badges/ProfileBadgeConverter.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/badges/ProfileBadgeConverter.java @@ -7,7 +7,6 @@ package org.whispersystems.textsecuregcm.badges; import java.util.List; import java.util.Locale; -import java.util.Set; import org.whispersystems.textsecuregcm.entities.Badge; import org.whispersystems.textsecuregcm.storage.AccountBadge; @@ -17,5 +16,5 @@ public interface ProfileBadgeConverter { * Converts the {@link AccountBadge}s for an account into the objects * that can be returned on a profile fetch. */ - Set convert(List acceptableLanguages, Set accountBadges); + List convert(List acceptableLanguages, List accountBadges); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ProfileController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ProfileController.java index 73ef53ac0..a0c0e5045 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ProfileController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ProfileController.java @@ -326,7 +326,7 @@ public class ProfileController { UserCapabilities.createForAccount(accountProfile.get()), username, accountProfile.get().getUuid(), - Set.of(), + List.of(), null); } @@ -399,7 +399,7 @@ public class ProfileController { UserCapabilities.createForAccount(accountProfile.get()), username.orElse(null), null, - Set.of(), + List.of(), null); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/Profile.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/Profile.java index 6e71a0070..640377516 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/Profile.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/Profile.java @@ -9,7 +9,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonSerialize; import com.google.common.annotations.VisibleForTesting; -import java.util.Set; +import java.util.List; import java.util.UUID; import org.signal.zkgroup.profiles.ProfileKeyCredentialResponse; @@ -49,7 +49,7 @@ public class Profile { private UUID uuid; @JsonProperty - private Set badges; + private List badges; @JsonProperty @JsonSerialize(using = ProfileKeyCredentialResponseAdapter.Serializing.class) @@ -61,7 +61,7 @@ public class Profile { public Profile( String name, String about, String aboutEmoji, String avatar, String paymentAddress, String identityKey, String unidentifiedAccess, boolean unrestrictedUnidentifiedAccess, UserCapabilities capabilities, String username, - UUID uuid, Set badges, ProfileKeyCredentialResponse credential) + UUID uuid, List badges, ProfileKeyCredentialResponse credential) { this.name = name; this.about = about; @@ -130,7 +130,7 @@ public class Profile { return uuid; } - public Set getBadges() { + public List getBadges() { return badges; } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java index fc04c2ad9..b7137b288 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java @@ -10,7 +10,9 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.annotations.VisibleForTesting; import java.time.Clock; import java.time.Instant; +import java.util.ArrayList; import java.util.HashSet; +import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -49,7 +51,7 @@ public class Account { private String avatar; @JsonProperty - private Set badges = new HashSet<>(); + private List badges = new ArrayList<>(); @JsonProperty private String registrationLock; @@ -313,7 +315,7 @@ public class Account { this.avatar = avatar; } - public Set getBadges() { + public List getBadges() { requireNotStale(); return badges; diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/badges/ConfiguredProfileBadgeConverterTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/badges/ConfiguredProfileBadgeConverterTest.java index 4efd59e0f..32ab220b4 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/badges/ConfiguredProfileBadgeConverterTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/badges/ConfiguredProfileBadgeConverterTest.java @@ -23,7 +23,6 @@ import java.util.ListResourceBundle; import java.util.Locale; import java.util.ResourceBundle; import java.util.ResourceBundle.Control; -import java.util.Set; import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -112,7 +111,7 @@ public class ConfiguredProfileBadgeConverterTest { BadgesConfiguration badgesConfiguration = createBadges(1); ConfiguredProfileBadgeConverter badgeConverter = new ConfiguredProfileBadgeConverter(clock, badgesConfiguration, resourceBundleFactory); - assertThat(badgeConverter.convert(List.of(Locale.getDefault()), Set.of())).isNotNull().isEmpty(); + assertThat(badgeConverter.convert(List.of(Locale.getDefault()), List.of())).isNotNull().isEmpty(); } @ParameterizedTest @@ -124,11 +123,11 @@ public class ConfiguredProfileBadgeConverterTest { setupResourceBundle(Locale.getDefault()); if (expectedBadge != null) { - assertThat(badgeConverter.convert(List.of(), Set.of(new AccountBadge(name, expiration, visible)))).isNotNull() + assertThat(badgeConverter.convert(List.of(), List.of(new AccountBadge(name, expiration, visible)))).isNotNull() .hasSize(1) .containsOnly(expectedBadge); } else { - assertThat(badgeConverter.convert(List.of(), Set.of(new AccountBadge(name, expiration, visible)))).isNotNull() + assertThat(badgeConverter.convert(List.of(), List.of(new AccountBadge(name, expiration, visible)))).isNotNull() .isEmpty(); } } @@ -162,7 +161,7 @@ public class ConfiguredProfileBadgeConverterTest { ArgumentCaptor controlArgumentCaptor = setupResourceBundle(enGb); badgeConverter.convert(List.of(enGb, en, esUs), - Set.of(new AccountBadge(nameFor(0), Instant.ofEpochSecond(43), true))); + List.of(new AccountBadge(nameFor(0), Instant.ofEpochSecond(43), true))); Control control = controlArgumentCaptor.getValue(); assertThatNullPointerException().isThrownBy(() -> control.getFormats(null)); @@ -187,7 +186,7 @@ public class ConfiguredProfileBadgeConverterTest { // this should always terminate at the system default locale since the development defined bundle should get // returned at that point anyhow badgeConverter.convert(List.of(enGb, Locale.getDefault(), en, esUs), - Set.of(new AccountBadge(nameFor(0), Instant.ofEpochSecond(43), true))); + List.of(new AccountBadge(nameFor(0), Instant.ofEpochSecond(43), true))); Control control2 = controlArgumentCaptor.getValue(); assertThat(control2.getFallbackLocale(ConfiguredProfileBadgeConverter.BASE_NAME, enGb)).isEqualTo( diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/ProfileControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/ProfileControllerTest.java index 02d13200a..3fd8ba668 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/ProfileControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/ProfileControllerTest.java @@ -22,6 +22,7 @@ import io.dropwizard.auth.PolymorphicAuthValueFactoryProvider; import io.dropwizard.testing.junit5.DropwizardExtensionsSupport; import io.dropwizard.testing.junit5.ResourceExtension; import java.util.Collections; +import java.util.List; import java.util.Optional; import java.util.Set; import javax.ws.rs.client.Entity; @@ -96,7 +97,7 @@ class ProfileControllerTest { profilesManager, usernamesManager, dynamicConfigurationManager, - (acceptableLanguages, accountBadges) -> Set.of(), // TODO: Test with some badges. + (acceptableLanguages, accountBadges) -> List.of(), // TODO: Test with some badges. s3client, postPolicyGenerator, policySigner,