Add validation annotations to DynamicExperimentEnrollmentConfiguration

signalapp · May 24, 2024 · f6d1e56 · f6d1e56
1 parent 9ec4f0b
commit f6d1e56
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 6 deletions.
diff --git a/...systems/textsecuregcm/configuration/dynamic/DynamicExperimentEnrollmentConfiguration.java b/...systems/textsecuregcm/configuration/dynamic/DynamicExperimentEnrollmentConfiguration.java
@@ -6,21 +6,21 @@
 package org.whispersystems.textsecuregcm.configuration.dynamic;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
-import com.google.common.annotations.VisibleForTesting;
-
-import javax.validation.Valid;
-import javax.validation.constraints.Max;
-import javax.validation.constraints.Min;
 import java.util.Collections;
 import java.util.Set;
 import java.util.UUID;
+import javax.validation.Valid;
+import javax.validation.constraints.Max;
+import javax.validation.constraints.Min;
+import javax.validation.constraints.NotNull;
 
 public class DynamicExperimentEnrollmentConfiguration {
 
   public static class UuidSelector {
 
     @JsonProperty
     @Valid
+    @NotNull
     private Set<UUID> uuids = Collections.emptySet();
 
     /**
@@ -45,7 +45,9 @@ public int getUuidEnrollmentPercentage() {
 
   }
 
-  private UuidSelector uuidSelector = new UuidSelector();
+  @Valid
+  @NotNull
+  private final UuidSelector uuidSelector = new UuidSelector();
 
   /**
    * If the UUID is not enrolled via {@link UuidSelector#uuids}, what is the percentage chance it should be enrolled.

diff --git a/...java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfigurationTest.java b/...java/org/whispersystems/textsecuregcm/configuration/dynamic/DynamicConfigurationTest.java
@@ -44,6 +44,22 @@ void testParseExperimentConfig() throws JsonProcessingException {
       assertFalse(emptyConfig.getExperimentEnrollmentConfiguration("test").isPresent());
     }
 
+    {
+      final String invalid = REQUIRED_CONFIG.concat("""
+          experiments:
+            percentageOnly:
+              enrollmentPercentage: 12
+            uuidsAndPercentage:
+              uuidSelector:
+                # the below results in uuids = null
+                uuids:
+          """);
+      final Optional<DynamicConfiguration> maybeConfig =
+          DynamicConfigurationManager.parseConfiguration(invalid, DynamicConfiguration.class);
+
+      assertFalse(maybeConfig.isPresent());
+    }
+
     {
       final String experimentConfigYaml = REQUIRED_CONFIG.concat("""
           experiments: