Remove transition code for set of parameter refactoring

None of the logged warnings happened in the last week.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=204307010

java/google/registry/batch/BatchModule.java

@@ -96,6 +96,6 @@ public class BatchModule {
   @Provides
   @Parameter(PARAM_RESAVE_TIMES)
   static ImmutableSet<DateTime> provideResaveTimes(HttpServletRequest req) {
-    return extractSetOfDatetimeParameters(req, PARAM_RESAVE_TIMES, null);
+    return extractSetOfDatetimeParameters(req, PARAM_RESAVE_TIMES);
   }
 }

java/google/registry/module/backend/BackendModule.java

@@ -45,8 +45,7 @@ public class BackendModule {
   @Provides
   @Parameter(RequestParameters.PARAM_TLDS)
   static ImmutableSet<String> provideTlds(HttpServletRequest req) {
-    ImmutableSet<String> tlds =
-        extractSetOfParameters(req, RequestParameters.PARAM_TLDS, RequestParameters.PARAM_TLD);
+    ImmutableSet<String> tlds = extractSetOfParameters(req, RequestParameters.PARAM_TLDS);
     assertTldsExist(tlds);
     return tlds;
   }

java/google/registry/rde/RdeModule.java

@@ -59,7 +59,7 @@ public abstract class RdeModule {
   @Provides
   @Parameter(PARAM_WATERMARKS)
   static ImmutableSet<DateTime> provideWatermarks(HttpServletRequest req) {
-    return extractSetOfDatetimeParameters(req, PARAM_WATERMARKS, PARAM_WATERMARK);
+    return extractSetOfDatetimeParameters(req, PARAM_WATERMARKS);
   }
 
   @Provides

java/google/registry/request/RequestParameters.java

@@ -23,7 +23,6 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import com.google.common.base.Ascii;
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.flogger.FluentLogger;
 import google.registry.request.HttpException.BadRequestException;
 import java.util.Optional;
 import javax.annotation.Nullable;
@@ -33,8 +32,6 @@ import org.joda.time.DateTime;
 /** Utilities for extracting parameters from HTTP requests. */
 public final class RequestParameters {
 
-  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
   /** The standardized request parameter name used by any action taking a tld parameter. */
   public static final String PARAM_TLD = "tld";
   /** The standardized request parameter name used by any action taking multiple tld parameters. */
@@ -101,64 +98,33 @@ public final class RequestParameters {
   /**
    * Returns all GET or POST parameters associated with {@code name} (or {@code nameLegacy}).
    *
-   * <p>While transitioning from "param=key1&param=key2" to "params=key1,key2" style of set inputing
-   * - we will be accepting all options (with a warning for "wrong" uses).
+   * <p>The parameter value is assumed to be a comma-delimited set of values - so tlds=com,net would
+   * result in ImmutableSet.of("com", "net").
    *
-   * <p>TODO(b/78226288): remove transition code (including "legacyName") once there are no more
-   * warnings.
+   * <p>Empty strings are not supported, and are automatically removed from the result.
+   *
+   * <p>Both missing parameter and parameter with empty value result in an empty set.
    *
    * @param req the request that has the parameter
-   * @param name the name of the parameter, should be in plural form (e.g. tlds=com,net)
-   * @param legacyName the legacy, singular form, name. Used only while transitioning from the
-   *     tld=com&tld=net form, in case we forgot to fix some reference. Null if there was no
-   *     singular form to transition from.
+   * @param name the name of the parameter, should be in plural form (e.g. tlds=, not tld=)
    */
-  public static ImmutableSet<String> extractSetOfParameters(
-      HttpServletRequest req, String name, @Nullable String legacyName) {
+  public static ImmutableSet<String> extractSetOfParameters(HttpServletRequest req, String name) {
     String[] parameters = req.getParameterValues(name);
-    if (legacyName != null) {
-      String[] legacyParameters = req.getParameterValues(legacyName);
-      if (legacyParameters != null) {
-        if (parameters == null) {
-          logger.atWarning().log(
-              "Bad 'set of parameters' input! Used legacy name %s instead of new name %s",
-              legacyName, name);
-          parameters = legacyParameters;
-        } else {
-          logger.atSevere().log(
-              "Bad 'set of parameters' input! Used both legacy name %s and new name %s! "
-                  + "Ignoring lagacy name.",
-              legacyName, name);
-        }
-      }
-    }
-    if (parameters == null) {
+    if (parameters == null || parameters.length == 0) {
       return ImmutableSet.of();
     }
     if (parameters.length > 1) {
-      logger.atWarning().log(
-          "Bad 'set of parameters' input! "
-              + "Received multiple values instead of single comma-delimited value for parameter %s",
-          name);
-      return ImmutableSet.copyOf(parameters);
+      throw new BadRequestException(
+          String.format(
+              "Bad 'set of parameters' input! Received multiple values instead of single "
+                  + "comma-delimited value for parameter %s",
+              name));
     }
-    if (parameters[0].isEmpty()) {
-      return ImmutableSet.of();
-    }
-    return ImmutableSet.copyOf(Splitter.on(',').split(parameters[0]));
-  }
-
-  /**
-   * Returns all GET or POST parameters associated with {@code name}.
-   *
-   * <p>While transitioning from "param=key1&param=key2" to "params=key1,key2" style of set inputing
-   * - we will be accepting all options (with a warning for "wrong" uses).
-   *
-   * <p>TODO(b/78226288): remove transition code (including "legacyName") once there are no more
-   * warnings.
-   */
-  public static ImmutableSet<String> extractSetOfParameters(HttpServletRequest req, String name) {
-    return extractSetOfParameters(req, name, null);
+    return Splitter.on(',')
+        .splitToList(parameters[0])
+        .stream()
+        .filter(s -> !s.isEmpty())
+        .collect(toImmutableSet());
   }
 
   /**
@@ -260,9 +226,9 @@ public final class RequestParameters {
    * @throws BadRequestException if one of the parameter values is not a valid {@link DateTime}.
    */
   public static ImmutableSet<DateTime> extractSetOfDatetimeParameters(
-      HttpServletRequest req, String name, @Nullable String legacyName) {
+      HttpServletRequest req, String name) {
     try {
-      return extractSetOfParameters(req, name, legacyName)
+      return extractSetOfParameters(req, name)
           .stream()
           .filter(not(String::isEmpty))
           .map(DateTime::parse)

java/google/registry/tools/server/ToolsServerModule.java

@@ -88,7 +88,7 @@ public class ToolsServerModule {
   @Provides
   @Parameter(RequestParameters.PARAM_TLDS)
   static ImmutableSet<String> provideTlds(HttpServletRequest req) {
-    return extractSetOfParameters(req, RequestParameters.PARAM_TLDS, RequestParameters.PARAM_TLD);
+    return extractSetOfParameters(req, RequestParameters.PARAM_TLDS);
   }
 
   @Provides

javatests/google/registry/request/RequestParametersTest.java

@@ -23,6 +23,7 @@ import static google.registry.request.RequestParameters.extractOptionalEnumParam
 import static google.registry.request.RequestParameters.extractOptionalParameter;
 import static google.registry.request.RequestParameters.extractRequiredDatetimeParameter;
 import static google.registry.request.RequestParameters.extractRequiredParameter;
+import static google.registry.request.RequestParameters.extractSetOfParameters;
 import static google.registry.testing.JUnitBackports.assertThrows;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -78,6 +79,43 @@ public class RequestParametersTest {
     assertThat(extractOptionalParameter(req, "spin")).isEmpty();
   }
 
+  @Test
+  public void testExtractSetOfParameters_notPresent_returnsEmpty() {
+    assertThat(extractSetOfParameters(req, "spin")).isEmpty();
+  }
+
+  @Test
+  public void testExtractSetOfParameters_empty_returnsEmpty() {
+    when(req.getParameterValues("spin")).thenReturn(new String[]{""});
+    assertThat(extractSetOfParameters(req, "spin")).isEmpty();
+  }
+
+  @Test
+  public void testExtractSetOfParameters_oneValue_returnsValue() {
+    when(req.getParameterValues("spin")).thenReturn(new String[]{"bog"});
+    assertThat(extractSetOfParameters(req, "spin")).containsExactly("bog");
+  }
+
+  @Test
+  public void testExtractSetOfParameters_multipleValues_returnsAll() {
+    when(req.getParameterValues("spin")).thenReturn(new String[]{"bog,gob"});
+    assertThat(extractSetOfParameters(req, "spin")).containsExactly("bog", "gob");
+  }
+
+  @Test
+  public void testExtractSetOfParameters_multipleValuesWithEmpty_removesEmpty() {
+    when(req.getParameterValues("spin")).thenReturn(new String[]{",bog,,gob,"});
+    assertThat(extractSetOfParameters(req, "spin")).containsExactly("bog", "gob");
+  }
+
+  @Test
+  public void testExtractSetOfParameters_multipleParameters_error() {
+    when(req.getParameterValues("spin")).thenReturn(new String[]{"bog", "gob"});
+    BadRequestException thrown =
+        assertThrows(BadRequestException.class, () -> extractSetOfParameters(req, "spin"));
+    assertThat(thrown).hasMessageThat().contains("spin");
+  }
+
   @Test
   public void testExtractBooleanParameter_notPresent_returnsFalse() {
     assertThat(extractBooleanParameter(req, "love")).isFalse();