Ensure badges are in ordered collections

This commit is contained in:
Ehren Kret 2021-09-15 16:12:07 -05:00
parent 2fb400280b
commit 5b25e38e41
8 changed files with 23 additions and 24 deletions

View File

@ -43,7 +43,6 @@ import java.util.Collections;
import java.util.EnumSet; import java.util.EnumSet;
import java.util.List; import java.util.List;
import java.util.Optional; import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.BlockingQueue; import java.util.concurrent.BlockingQueue;
import java.util.concurrent.ExecutorService; import java.util.concurrent.ExecutorService;
@ -296,7 +295,7 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
environment.getObjectMapper().setVisibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.NONE); environment.getObjectMapper().setVisibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.NONE);
environment.getObjectMapper().setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY); environment.getObjectMapper().setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY);
ProfileBadgeConverter profileBadgeConverter = (acceptableLanguages, accountBadges) -> 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); JdbiFactory jdbiFactory = new JdbiFactory(DefaultNameStrategy.CHECK_EMPTY);
Jdbi accountJdbi = jdbiFactory.build(environment, config.getAccountsDatabaseConfiguration(), "accountdb"); Jdbi accountJdbi = jdbiFactory.build(environment, config.getAccountsDatabaseConfiguration(), "accountdb");

View File

@ -14,7 +14,6 @@ import java.util.Map;
import java.util.Objects; import java.util.Objects;
import java.util.ResourceBundle; import java.util.ResourceBundle;
import java.util.ResourceBundle.Control; import java.util.ResourceBundle.Control;
import java.util.Set;
import java.util.function.Function; import java.util.function.Function;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import org.whispersystems.textsecuregcm.configuration.BadgeConfiguration; import org.whispersystems.textsecuregcm.configuration.BadgeConfiguration;
@ -50,11 +49,11 @@ public class ConfiguredProfileBadgeConverter implements ProfileBadgeConverter {
} }
@Override @Override
public Set<Badge> convert( public List<Badge> convert(
final List<Locale> acceptableLanguages, final List<Locale> acceptableLanguages,
final Set<AccountBadge> accountBadges) { final List<AccountBadge> accountBadges) {
if (accountBadges.isEmpty()) { if (accountBadges.isEmpty()) {
return Set.of(); return List.of();
} }
final Instant now = clock.instant(); final Instant now = clock.instant();
@ -94,6 +93,6 @@ public class ConfiguredProfileBadgeConverter implements ProfileBadgeConverter {
.map(accountBadge -> new Badge(knownBadges.get(accountBadge.getName()).getImageUrl(), .map(accountBadge -> new Badge(knownBadges.get(accountBadge.getName()).getImageUrl(),
resourceBundle.getString(accountBadge.getName() + "_name"), resourceBundle.getString(accountBadge.getName() + "_name"),
resourceBundle.getString(accountBadge.getName() + "_description"))) resourceBundle.getString(accountBadge.getName() + "_description")))
.collect(Collectors.toSet()); .collect(Collectors.toList());
} }
} }

View File

@ -7,7 +7,6 @@ package org.whispersystems.textsecuregcm.badges;
import java.util.List; import java.util.List;
import java.util.Locale; import java.util.Locale;
import java.util.Set;
import org.whispersystems.textsecuregcm.entities.Badge; import org.whispersystems.textsecuregcm.entities.Badge;
import org.whispersystems.textsecuregcm.storage.AccountBadge; import org.whispersystems.textsecuregcm.storage.AccountBadge;
@ -17,5 +16,5 @@ public interface ProfileBadgeConverter {
* Converts the {@link AccountBadge}s for an account into the objects * Converts the {@link AccountBadge}s for an account into the objects
* that can be returned on a profile fetch. * that can be returned on a profile fetch.
*/ */
Set<Badge> convert(List<Locale> acceptableLanguages, Set<AccountBadge> accountBadges); List<Badge> convert(List<Locale> acceptableLanguages, List<AccountBadge> accountBadges);
} }

View File

@ -326,7 +326,7 @@ public class ProfileController {
UserCapabilities.createForAccount(accountProfile.get()), UserCapabilities.createForAccount(accountProfile.get()),
username, username,
accountProfile.get().getUuid(), accountProfile.get().getUuid(),
Set.of(), List.of(),
null); null);
} }
@ -399,7 +399,7 @@ public class ProfileController {
UserCapabilities.createForAccount(accountProfile.get()), UserCapabilities.createForAccount(accountProfile.get()),
username.orElse(null), username.orElse(null),
null, null,
Set.of(), List.of(),
null); null);
} }

View File

@ -9,7 +9,7 @@ import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize; import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import java.util.Set; import java.util.List;
import java.util.UUID; import java.util.UUID;
import org.signal.zkgroup.profiles.ProfileKeyCredentialResponse; import org.signal.zkgroup.profiles.ProfileKeyCredentialResponse;
@ -49,7 +49,7 @@ public class Profile {
private UUID uuid; private UUID uuid;
@JsonProperty @JsonProperty
private Set<Badge> badges; private List<Badge> badges;
@JsonProperty @JsonProperty
@JsonSerialize(using = ProfileKeyCredentialResponseAdapter.Serializing.class) @JsonSerialize(using = ProfileKeyCredentialResponseAdapter.Serializing.class)
@ -61,7 +61,7 @@ public class Profile {
public Profile( public Profile(
String name, String about, String aboutEmoji, String avatar, String paymentAddress, String identityKey, String name, String about, String aboutEmoji, String avatar, String paymentAddress, String identityKey,
String unidentifiedAccess, boolean unrestrictedUnidentifiedAccess, UserCapabilities capabilities, String username, String unidentifiedAccess, boolean unrestrictedUnidentifiedAccess, UserCapabilities capabilities, String username,
UUID uuid, Set<Badge> badges, ProfileKeyCredentialResponse credential) UUID uuid, List<Badge> badges, ProfileKeyCredentialResponse credential)
{ {
this.name = name; this.name = name;
this.about = about; this.about = about;
@ -130,7 +130,7 @@ public class Profile {
return uuid; return uuid;
} }
public Set<Badge> getBadges() { public List<Badge> getBadges() {
return badges; return badges;
} }
} }

View File

@ -10,7 +10,9 @@ import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import java.time.Clock; import java.time.Clock;
import java.time.Instant; import java.time.Instant;
import java.util.ArrayList;
import java.util.HashSet; import java.util.HashSet;
import java.util.List;
import java.util.Objects; import java.util.Objects;
import java.util.Optional; import java.util.Optional;
import java.util.Set; import java.util.Set;
@ -49,7 +51,7 @@ public class Account {
private String avatar; private String avatar;
@JsonProperty @JsonProperty
private Set<AccountBadge> badges = new HashSet<>(); private List<AccountBadge> badges = new ArrayList<>();
@JsonProperty @JsonProperty
private String registrationLock; private String registrationLock;
@ -313,7 +315,7 @@ public class Account {
this.avatar = avatar; this.avatar = avatar;
} }
public Set<AccountBadge> getBadges() { public List<AccountBadge> getBadges() {
requireNotStale(); requireNotStale();
return badges; return badges;

View File

@ -23,7 +23,6 @@ import java.util.ListResourceBundle;
import java.util.Locale; import java.util.Locale;
import java.util.ResourceBundle; import java.util.ResourceBundle;
import java.util.ResourceBundle.Control; import java.util.ResourceBundle.Control;
import java.util.Set;
import java.util.stream.Stream; import java.util.stream.Stream;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
@ -112,7 +111,7 @@ public class ConfiguredProfileBadgeConverterTest {
BadgesConfiguration badgesConfiguration = createBadges(1); BadgesConfiguration badgesConfiguration = createBadges(1);
ConfiguredProfileBadgeConverter badgeConverter = new ConfiguredProfileBadgeConverter(clock, badgesConfiguration, ConfiguredProfileBadgeConverter badgeConverter = new ConfiguredProfileBadgeConverter(clock, badgesConfiguration,
resourceBundleFactory); resourceBundleFactory);
assertThat(badgeConverter.convert(List.of(Locale.getDefault()), Set.of())).isNotNull().isEmpty(); assertThat(badgeConverter.convert(List.of(Locale.getDefault()), List.of())).isNotNull().isEmpty();
} }
@ParameterizedTest @ParameterizedTest
@ -124,11 +123,11 @@ public class ConfiguredProfileBadgeConverterTest {
setupResourceBundle(Locale.getDefault()); setupResourceBundle(Locale.getDefault());
if (expectedBadge != null) { 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) .hasSize(1)
.containsOnly(expectedBadge); .containsOnly(expectedBadge);
} else { } 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(); .isEmpty();
} }
} }
@ -162,7 +161,7 @@ public class ConfiguredProfileBadgeConverterTest {
ArgumentCaptor<Control> controlArgumentCaptor = setupResourceBundle(enGb); ArgumentCaptor<Control> controlArgumentCaptor = setupResourceBundle(enGb);
badgeConverter.convert(List.of(enGb, en, esUs), 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(); Control control = controlArgumentCaptor.getValue();
assertThatNullPointerException().isThrownBy(() -> control.getFormats(null)); 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 // this should always terminate at the system default locale since the development defined bundle should get
// returned at that point anyhow // returned at that point anyhow
badgeConverter.convert(List.of(enGb, Locale.getDefault(), en, esUs), 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(); Control control2 = controlArgumentCaptor.getValue();
assertThat(control2.getFallbackLocale(ConfiguredProfileBadgeConverter.BASE_NAME, enGb)).isEqualTo( assertThat(control2.getFallbackLocale(ConfiguredProfileBadgeConverter.BASE_NAME, enGb)).isEqualTo(

View File

@ -22,6 +22,7 @@ import io.dropwizard.auth.PolymorphicAuthValueFactoryProvider;
import io.dropwizard.testing.junit5.DropwizardExtensionsSupport; import io.dropwizard.testing.junit5.DropwizardExtensionsSupport;
import io.dropwizard.testing.junit5.ResourceExtension; import io.dropwizard.testing.junit5.ResourceExtension;
import java.util.Collections; import java.util.Collections;
import java.util.List;
import java.util.Optional; import java.util.Optional;
import java.util.Set; import java.util.Set;
import javax.ws.rs.client.Entity; import javax.ws.rs.client.Entity;
@ -96,7 +97,7 @@ class ProfileControllerTest {
profilesManager, profilesManager,
usernamesManager, usernamesManager,
dynamicConfigurationManager, dynamicConfigurationManager,
(acceptableLanguages, accountBadges) -> Set.of(), // TODO: Test with some badges. (acceptableLanguages, accountBadges) -> List.of(), // TODO: Test with some badges.
s3client, s3client,
postPolicyGenerator, postPolicyGenerator,
policySigner, policySigner,