Move score floor to dynamic configuration, add distribution summary

This commit is contained in:
Chris Eager
2022-03-02 15:18:33 -08:00
committed by GitHub
parent 9fc5002619
commit eee6307789
11 changed files with 167 additions and 117 deletions

View File

@@ -26,10 +26,15 @@ import org.whispersystems.textsecuregcm.util.ua.ClientPlatform;
class DynamicConfigurationTest {
private static final String REQUIRED_CONFIG = """
captcha:
scoreFloor: 1.0
""";
@Test
void testParseExperimentConfig() throws JsonProcessingException {
{
final String emptyConfigYaml = "test: true";
final String emptyConfigYaml = REQUIRED_CONFIG.concat("test: true");
final DynamicConfiguration emptyConfig =
DynamicConfigurationManager.parseConfiguration(emptyConfigYaml, DynamicConfiguration.class).orElseThrow();
@@ -37,7 +42,7 @@ class DynamicConfigurationTest {
}
{
final String experimentConfigYaml = """
final String experimentConfigYaml = REQUIRED_CONFIG.concat("""
experiments:
percentageOnly:
enrollmentPercentage: 12
@@ -49,7 +54,7 @@ class DynamicConfigurationTest {
uuidsOnly:
enrolledUuids:
- 71618739-114c-4b1f-bb0d-6478a44eb600
""";
""");
final DynamicConfiguration config =
DynamicConfigurationManager.parseConfiguration(experimentConfigYaml, DynamicConfiguration.class).orElseThrow();
@@ -78,7 +83,7 @@ class DynamicConfigurationTest {
@Test
void testParsePreRegistrationExperiments() throws JsonProcessingException {
{
final String emptyConfigYaml = "test: true";
final String emptyConfigYaml = REQUIRED_CONFIG.concat("test: true");
final DynamicConfiguration emptyConfig =
DynamicConfigurationManager.parseConfiguration(emptyConfigYaml, DynamicConfiguration.class).orElseThrow();
@@ -86,7 +91,7 @@ class DynamicConfigurationTest {
}
{
final String experimentConfigYaml = """
final String experimentConfigYaml = REQUIRED_CONFIG.concat("""
preRegistrationExperiments:
percentageOnly:
enrollmentPercentage: 17
@@ -107,7 +112,7 @@ class DynamicConfigurationTest {
- +120255551212
excludedCountryCodes:
- 47
""";
""");
final DynamicConfiguration config =
DynamicConfigurationManager.parseConfiguration(experimentConfigYaml, DynamicConfiguration.class).orElseThrow();
@@ -160,7 +165,7 @@ class DynamicConfigurationTest {
@Test
void testParseRemoteDeprecationConfig() throws JsonProcessingException {
{
final String emptyConfigYaml = "test: true";
final String emptyConfigYaml = REQUIRED_CONFIG.concat("test: true");
final DynamicConfiguration emptyConfig =
DynamicConfigurationManager.parseConfiguration(emptyConfigYaml, DynamicConfiguration.class).orElseThrow();
@@ -168,7 +173,7 @@ class DynamicConfigurationTest {
}
{
final String remoteDeprecationConfig = """
final String remoteDeprecationConfig = REQUIRED_CONFIG.concat("""
remoteDeprecation:
minimumVersions:
IOS: 1.2.3
@@ -178,7 +183,7 @@ class DynamicConfigurationTest {
blockedVersions:
DESKTOP:
- 1.4.0-beta.2
""";
""");
final DynamicConfiguration config =
DynamicConfigurationManager.parseConfiguration(remoteDeprecationConfig, DynamicConfiguration.class).orElseThrow();
@@ -199,7 +204,7 @@ class DynamicConfigurationTest {
@Test
void testParseFeatureFlags() throws JsonProcessingException {
{
final String emptyConfigYaml = "test: true";
final String emptyConfigYaml = REQUIRED_CONFIG.concat("test: true");
final DynamicConfiguration emptyConfig =
DynamicConfigurationManager.parseConfiguration(emptyConfigYaml, DynamicConfiguration.class).orElseThrow();
@@ -207,10 +212,10 @@ class DynamicConfigurationTest {
}
{
final String featureFlagYaml = """
final String featureFlagYaml = REQUIRED_CONFIG.concat("""
featureFlags:
- testFlag
""";
""");
final DynamicConfiguration emptyConfig =
DynamicConfigurationManager.parseConfiguration(featureFlagYaml, DynamicConfiguration.class).orElseThrow();
@@ -222,7 +227,7 @@ class DynamicConfigurationTest {
@Test
void testParseTwilioConfiguration() throws JsonProcessingException {
{
final String emptyConfigYaml = "test: true";
final String emptyConfigYaml = REQUIRED_CONFIG.concat("test: true");
final DynamicConfiguration emptyConfig =
DynamicConfigurationManager.parseConfiguration(emptyConfigYaml, DynamicConfiguration.class).orElseThrow();
@@ -230,12 +235,12 @@ class DynamicConfigurationTest {
}
{
final String twilioConfigYaml = """
final String twilioConfigYaml = REQUIRED_CONFIG.concat("""
twilio:
numbers:
- 2135551212
- 2135551313
""";
""");
final DynamicTwilioConfiguration config =
DynamicConfigurationManager.parseConfiguration(twilioConfigYaml, DynamicConfiguration.class).orElseThrow()
@@ -248,7 +253,7 @@ class DynamicConfigurationTest {
@Test
void testParsePaymentsConfiguration() throws JsonProcessingException {
{
final String emptyConfigYaml = "test: true";
final String emptyConfigYaml = REQUIRED_CONFIG.concat("test: true");
final DynamicConfiguration emptyConfig =
DynamicConfigurationManager.parseConfiguration(emptyConfigYaml, DynamicConfiguration.class).orElseThrow();
@@ -256,11 +261,11 @@ class DynamicConfigurationTest {
}
{
final String paymentsConfigYaml = """
final String paymentsConfigYaml = REQUIRED_CONFIG.concat("""
payments:
disallowedPrefixes:
- +44
""";
""");
final DynamicPaymentsConfiguration config =
DynamicConfigurationManager.parseConfiguration(paymentsConfigYaml, DynamicConfiguration.class).orElseThrow()
@@ -271,34 +276,47 @@ class DynamicConfigurationTest {
}
@Test
void testParseSignupCaptchaConfiguration() throws JsonProcessingException {
void testParseCaptchaConfiguration() throws JsonProcessingException {
{
final String emptyConfigYaml = "test: true";
final DynamicConfiguration emptyConfig =
DynamicConfigurationManager.parseConfiguration(emptyConfigYaml, DynamicConfiguration.class).orElseThrow();
assertTrue(emptyConfig.getSignupCaptchaConfiguration().getCountryCodes().isEmpty());
assertTrue(DynamicConfigurationManager.parseConfiguration(emptyConfigYaml, DynamicConfiguration.class).isEmpty(),
"empty config should not validate");
}
{
final String signupCaptchaConfig = """
signupCaptcha:
countryCodes:
final String captchaConfig = """
captcha:
signupCountryCodes:
- 1
scoreFloor: null
""";
final DynamicSignupCaptchaConfiguration config =
DynamicConfigurationManager.parseConfiguration(signupCaptchaConfig, DynamicConfiguration.class).orElseThrow()
.getSignupCaptchaConfiguration();
assertTrue(DynamicConfigurationManager.parseConfiguration(captchaConfig, DynamicConfiguration.class).isEmpty(),
"score floor must not be null");
}
assertEquals(Set.of("1"), config.getCountryCodes());
{
final String captchaConfig = """
captcha:
signupCountryCodes:
- 1
scoreFloor: 0.9
""";
final DynamicCaptchaConfiguration config =
DynamicConfigurationManager.parseConfiguration(captchaConfig, DynamicConfiguration.class).orElseThrow()
.getCaptchaConfiguration();
assertEquals(Set.of("1"), config.getSignupCountryCodes());
assertEquals(0.9f, config.getScoreFloor().floatValue());
}
}
@Test
void testParseLimits() throws JsonProcessingException {
{
final String emptyConfigYaml = "test: true";
final String emptyConfigYaml = REQUIRED_CONFIG.concat("test: true");
final DynamicConfiguration emptyConfig =
DynamicConfigurationManager.parseConfiguration(emptyConfigYaml, DynamicConfiguration.class).orElseThrow();
@@ -307,12 +325,12 @@ class DynamicConfigurationTest {
}
{
final String limitsConfig = """
final String limitsConfig = REQUIRED_CONFIG.concat("""
limits:
rateLimitReset:
bucketSize: 17
leakRatePerMinute: 44
""";
""");
final RateLimitConfiguration resetRateLimitConfiguration =
DynamicConfigurationManager.parseConfiguration(limitsConfig, DynamicConfiguration.class).orElseThrow()
@@ -326,7 +344,7 @@ class DynamicConfigurationTest {
@Test
void testParseRateLimitReset() throws JsonProcessingException {
{
final String emptyConfigYaml = "test: true";
final String emptyConfigYaml = REQUIRED_CONFIG.concat("test: true");
final DynamicConfiguration emptyConfig =
DynamicConfigurationManager.parseConfiguration(emptyConfigYaml, DynamicConfiguration.class).orElseThrow();
@@ -334,13 +352,13 @@ class DynamicConfigurationTest {
}
{
final String rateLimitChallengeConfig = """
final String rateLimitChallengeConfig = REQUIRED_CONFIG.concat("""
rateLimitChallenge:
clientSupportedVersions:
IOS: 5.1.0
ANDROID: 5.2.0
DESKTOP: 5.0.0
""";
""");
DynamicRateLimitChallengeConfiguration rateLimitChallengeConfiguration =
DynamicConfigurationManager.parseConfiguration(rateLimitChallengeConfig, DynamicConfiguration.class).orElseThrow()
@@ -357,7 +375,7 @@ class DynamicConfigurationTest {
@Test
void testParseDirectoryReconciler() throws JsonProcessingException {
{
final String emptyConfigYaml = "test: true";
final String emptyConfigYaml = REQUIRED_CONFIG.concat("test: true");
final DynamicConfiguration emptyConfig =
DynamicConfigurationManager.parseConfiguration(emptyConfigYaml, DynamicConfiguration.class).orElseThrow();
@@ -365,10 +383,10 @@ class DynamicConfigurationTest {
}
{
final String directoryReconcilerConfig = """
final String directoryReconcilerConfig = REQUIRED_CONFIG.concat("""
directoryReconciler:
enabled: false
""";
""");
DynamicDirectoryReconcilerConfiguration directoryReconcilerConfiguration =
DynamicConfigurationManager.parseConfiguration(directoryReconcilerConfig, DynamicConfiguration.class).orElseThrow()

View File

@@ -1,12 +1,13 @@
package org.whispersystems.textsecuregcm.storage;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.time.Duration;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration;
import software.amazon.awssdk.core.SdkBytes;
import software.amazon.awssdk.services.appconfigdata.AppConfigDataClient;
@@ -14,10 +15,15 @@ import software.amazon.awssdk.services.appconfigdata.model.GetLatestConfiguratio
import software.amazon.awssdk.services.appconfigdata.model.GetLatestConfigurationResponse;
import software.amazon.awssdk.services.appconfigdata.model.StartConfigurationSessionRequest;
import software.amazon.awssdk.services.appconfigdata.model.StartConfigurationSessionResponse;
import java.util.concurrent.TimeUnit;
class DynamicConfigurationManagerTest {
private static final SdkBytes VALID_CONFIG = SdkBytes.fromUtf8String("""
test: true
captcha:
scoreFloor: 1.0
""");
private DynamicConfigurationManager<DynamicConfiguration> dynamicConfigurationManager;
private AppConfigDataClient appConfig;
private StartConfigurationSessionRequest startConfigurationSession;
@@ -35,7 +41,7 @@ class DynamicConfigurationManagerTest {
}
@Test
void testGetInitalConfig() {
void testGetInitialConfig() {
when(appConfig.startConfigurationSession(startConfigurationSession))
.thenReturn(StartConfigurationSessionResponse.builder()
.initialConfigurationToken("initial")
@@ -45,7 +51,7 @@ class DynamicConfigurationManagerTest {
when(appConfig.getLatestConfiguration(GetLatestConfigurationRequest.builder()
.configurationToken("initial").build()))
.thenReturn(GetLatestConfigurationResponse.builder()
.configuration(SdkBytes.fromUtf8String("test: true"))
.configuration(VALID_CONFIG)
.nextPollConfigurationToken("next").build());
// subsequent config calls will return empty (no update)
@@ -55,8 +61,10 @@ class DynamicConfigurationManagerTest {
.configuration(SdkBytes.fromUtf8String(""))
.nextPollConfigurationToken("next").build());
dynamicConfigurationManager.start();
assertThat(dynamicConfigurationManager.getConfiguration()).isNotNull();
assertTimeoutPreemptively(Duration.ofSeconds(5), () -> {
dynamicConfigurationManager.start();
assertThat(dynamicConfigurationManager.getConfiguration()).isNotNull();
});
}
@Test
@@ -77,7 +85,7 @@ class DynamicConfigurationManagerTest {
when(appConfig.getLatestConfiguration(GetLatestConfigurationRequest.builder().
configurationToken("goodconfig").build()))
.thenReturn(GetLatestConfigurationResponse.builder()
.configuration(SdkBytes.fromUtf8String("test: true"))
.configuration(VALID_CONFIG)
.nextPollConfigurationToken("next").build());
// all subsequent config calls will return an empty config (no update)
@@ -86,13 +94,15 @@ class DynamicConfigurationManagerTest {
.thenReturn(GetLatestConfigurationResponse.builder()
.configuration(SdkBytes.fromUtf8String(""))
.nextPollConfigurationToken("next").build());
dynamicConfigurationManager.start();
assertThat(dynamicConfigurationManager.getConfiguration()).isNotNull();
assertTimeoutPreemptively(Duration.ofSeconds(5), () -> {
dynamicConfigurationManager.start();
assertThat(dynamicConfigurationManager.getConfiguration()).isNotNull();
});
}
@Test
@Timeout(value=5, unit= TimeUnit.SECONDS)
void testGetConfigMultiple() throws InterruptedException {
void testGetConfigMultiple() {
when(appConfig.startConfigurationSession(startConfigurationSession))
.thenReturn(StartConfigurationSessionResponse.builder()
.initialConfigurationToken("0")
@@ -102,7 +112,7 @@ class DynamicConfigurationManagerTest {
when(appConfig.getLatestConfiguration(GetLatestConfigurationRequest.builder().
configurationToken("0").build()))
.thenReturn(GetLatestConfigurationResponse.builder()
.configuration(SdkBytes.fromUtf8String("test: true"))
.configuration(VALID_CONFIG)
.nextPollConfigurationToken("1").build());
// config update with a real config
@@ -112,6 +122,8 @@ class DynamicConfigurationManagerTest {
.configuration(SdkBytes.fromUtf8String("""
featureFlags:
- testFlag
captcha:
scoreFloor: 1.0
"""))
.nextPollConfigurationToken("2").build());
@@ -122,11 +134,16 @@ class DynamicConfigurationManagerTest {
.configuration(SdkBytes.fromUtf8String(""))
.nextPollConfigurationToken("2").build());
// we should eventually get the updated config (or the test will timeout)
dynamicConfigurationManager.start();
while (dynamicConfigurationManager.getConfiguration().getActiveFeatureFlags().isEmpty()) {
Thread.sleep(100);
}
assertThat(dynamicConfigurationManager.getConfiguration().getActiveFeatureFlags()).containsExactly("testFlag");
// the internal waiting done by dynamic configuration manager catches the InterruptedException used
// by JUnits @Timeout, so we use assertTimeoutPreemptively
assertTimeoutPreemptively(Duration.ofSeconds(5), () -> {
// we should eventually get the updated config (or the test will timeout)
dynamicConfigurationManager.start();
while (dynamicConfigurationManager.getConfiguration().getActiveFeatureFlags().isEmpty()) {
Thread.sleep(100);
}
assertThat(dynamicConfigurationManager.getConfiguration().getActiveFeatureFlags()).containsExactly("testFlag");
});
}
}

View File

@@ -52,7 +52,6 @@ import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.ArgumentCaptor;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.whispersystems.textsecuregcm.auth.AuthenticatedAccount;
import org.whispersystems.textsecuregcm.auth.AuthenticationCredentials;
@@ -61,8 +60,8 @@ import org.whispersystems.textsecuregcm.auth.ExternalServiceCredentialGenerator;
import org.whispersystems.textsecuregcm.auth.StoredRegistrationLock;
import org.whispersystems.textsecuregcm.auth.StoredVerificationCode;
import org.whispersystems.textsecuregcm.auth.TurnTokenGenerator;
import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicCaptchaConfiguration;
import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration;
import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicSignupCaptchaConfiguration;
import org.whispersystems.textsecuregcm.controllers.AccountController;
import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException;
import org.whispersystems.textsecuregcm.entities.AccountAttributes;
@@ -263,9 +262,9 @@ class AccountControllerTest {
when(dynamicConfigurationManager.getConfiguration())
.thenReturn(dynamicConfiguration);
DynamicSignupCaptchaConfiguration signupCaptchaConfig = new DynamicSignupCaptchaConfiguration();
DynamicCaptchaConfiguration signupCaptchaConfig = new DynamicCaptchaConfiguration();
when(dynamicConfiguration.getSignupCaptchaConfiguration()).thenReturn(signupCaptchaConfig);
when(dynamicConfiguration.getCaptchaConfiguration()).thenReturn(signupCaptchaConfig);
}
when(abusiveHostRules.getAbusiveHostRulesFor(eq(ABUSIVE_HOST))).thenReturn(Collections.singletonList(new AbusiveHostRule(ABUSIVE_HOST, true, Collections.emptyList())));
when(abusiveHostRules.getAbusiveHostRulesFor(eq(RESTRICTED_HOST))).thenReturn(Collections.singletonList(new AbusiveHostRule(RESTRICTED_HOST, false, Collections.singletonList("+123"))));
@@ -1741,9 +1740,9 @@ class AccountControllerTest {
when(dynamicConfigurationManager.getConfiguration())
.thenReturn(dynamicConfiguration);
DynamicSignupCaptchaConfiguration signupCaptchaConfig = new DynamicSignupCaptchaConfiguration();
signupCaptchaConfig.setCountryCodes(countryCodes);
when(dynamicConfiguration.getSignupCaptchaConfiguration())
DynamicCaptchaConfiguration signupCaptchaConfig = new DynamicCaptchaConfiguration();
signupCaptchaConfig.setSignupCountryCodes(countryCodes);
when(dynamicConfiguration.getCaptchaConfiguration())
.thenReturn(signupCaptchaConfig);
Response response =