diff --git a/java/google/registry/batch/DeleteProberDataAction.java b/java/google/registry/batch/DeleteProberDataAction.java index a7f9fad09..91bf9c320 100644 --- a/java/google/registry/batch/DeleteProberDataAction.java +++ b/java/google/registry/batch/DeleteProberDataAction.java @@ -23,7 +23,7 @@ import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.registry.Registries.getTldsOfType; import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_DELETE; import static google.registry.request.Action.Method.POST; -import static google.registry.request.RequestParameters.PARAM_TLD; +import static google.registry.request.RequestParameters.PARAM_TLDS; import static org.joda.time.DateTimeZone.UTC; import com.google.appengine.tools.mapreduce.Mapper; @@ -76,7 +76,7 @@ public class DeleteProberDataAction implements Runnable { @Inject @Parameter(PARAM_DRY_RUN) boolean isDryRun; /** List of TLDs to work on. If empty - will work on all TLDs that end with .test. */ - @Inject @Parameter(PARAM_TLD) ImmutableSet tlds; + @Inject @Parameter(PARAM_TLDS) ImmutableSet tlds; @Inject @Config("registryAdminClientId") String registryAdminClientId; @Inject MapreduceRunner mrRunner; @Inject Response response; diff --git a/java/google/registry/cron/CronModule.java b/java/google/registry/cron/CronModule.java index d2dbd09e1..3927b04ba 100644 --- a/java/google/registry/cron/CronModule.java +++ b/java/google/registry/cron/CronModule.java @@ -30,45 +30,53 @@ import javax.servlet.http.HttpServletRequest; @Module public final class CronModule { + public static final String ENDPOINT_PARAM = "endpoint"; + public static final String QUEUE_PARAM = "queue"; + public static final String FOR_EACH_REAL_TLD_PARAM = "forEachRealTld"; + public static final String FOR_EACH_TEST_TLD_PARAM = "forEachTestTld"; + public static final String RUN_IN_EMPTY_PARAM = "runInEmpty"; + public static final String EXCLUDE_PARAM = "exclude"; + public static final String JITTER_SECONDS_PARAM = "jitterSeconds"; + @Provides - @Parameter("endpoint") + @Parameter(ENDPOINT_PARAM) static String provideEndpoint(HttpServletRequest req) { - return extractRequiredParameter(req, "endpoint"); + return extractRequiredParameter(req, ENDPOINT_PARAM); } @Provides - @Parameter("exclude") + @Parameter(EXCLUDE_PARAM) static ImmutableSet provideExcludes(HttpServletRequest req) { - return extractSetOfParameters(req, "exclude"); + return extractSetOfParameters(req, EXCLUDE_PARAM); } @Provides - @Parameter("queue") + @Parameter(QUEUE_PARAM) static String provideQueue(HttpServletRequest req) { - return extractRequiredParameter(req, "queue"); + return extractRequiredParameter(req, QUEUE_PARAM); } @Provides - @Parameter("runInEmpty") + @Parameter(RUN_IN_EMPTY_PARAM) static boolean provideRunInEmpty(HttpServletRequest req) { - return extractBooleanParameter(req, "runInEmpty"); + return extractBooleanParameter(req, RUN_IN_EMPTY_PARAM); } @Provides - @Parameter("forEachRealTld") + @Parameter(FOR_EACH_REAL_TLD_PARAM) static boolean provideForEachRealTld(HttpServletRequest req) { - return extractBooleanParameter(req, "forEachRealTld"); + return extractBooleanParameter(req, FOR_EACH_REAL_TLD_PARAM); } @Provides - @Parameter("forEachTestTld") + @Parameter(FOR_EACH_TEST_TLD_PARAM) static boolean provideForEachTestTld(HttpServletRequest req) { - return extractBooleanParameter(req, "forEachTestTld"); + return extractBooleanParameter(req, FOR_EACH_TEST_TLD_PARAM); } @Provides - @Parameter("jitterSeconds") + @Parameter(JITTER_SECONDS_PARAM) static Optional provideJitterSeconds(HttpServletRequest req) { - return extractOptionalIntParameter(req, "jitterSeconds"); + return extractOptionalIntParameter(req, JITTER_SECONDS_PARAM); } } diff --git a/java/google/registry/cron/TldFanoutAction.java b/java/google/registry/cron/TldFanoutAction.java index 7f50aaaf2..3677a8585 100644 --- a/java/google/registry/cron/TldFanoutAction.java +++ b/java/google/registry/cron/TldFanoutAction.java @@ -24,6 +24,13 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.Iterables.getFirst; import static com.google.common.collect.Multimaps.filterKeys; import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8; +import static google.registry.cron.CronModule.ENDPOINT_PARAM; +import static google.registry.cron.CronModule.EXCLUDE_PARAM; +import static google.registry.cron.CronModule.FOR_EACH_REAL_TLD_PARAM; +import static google.registry.cron.CronModule.FOR_EACH_TEST_TLD_PARAM; +import static google.registry.cron.CronModule.JITTER_SECONDS_PARAM; +import static google.registry.cron.CronModule.QUEUE_PARAM; +import static google.registry.cron.CronModule.RUN_IN_EMPTY_PARAM; import static google.registry.model.registry.Registries.getTldsOfType; import static google.registry.model.registry.Registry.TldType.REAL; import static google.registry.model.registry.Registry.TldType.TEST; @@ -80,14 +87,6 @@ import javax.inject.Inject; ) public final class TldFanoutAction implements Runnable { - private static final String ENDPOINT_PARAM = "endpoint"; - private static final String QUEUE_PARAM = "queue"; - private static final String FOR_EACH_REAL_TLD_PARAM = "forEachRealTld"; - private static final String FOR_EACH_TEST_TLD_PARAM = "forEachTestTld"; - private static final String RUN_IN_EMPTY_PARAM = "runInEmpty"; - private static final String EXCLUDE_PARAM = "exclude"; - private static final String JITTER_SECONDS_PARAM = "jitterSeconds"; - /** A set of control params to TldFanoutAction that aren't passed down to the executing action. */ private static final ImmutableSet CONTROL_PARAMS = ImmutableSet.of( @@ -127,12 +126,13 @@ public final class TldFanoutAction implements Runnable { !(runInEmpty && !excludes.isEmpty()), "Can't specify 'exclude' with 'runInEmpty'"); ImmutableSet tlds = - Streams.concat( - runInEmpty ? Stream.of("") : Stream.of(), - forEachRealTld ? getTldsOfType(REAL).stream() : Stream.of(), - forEachTestTld ? getTldsOfType(TEST).stream() : Stream.of()) - .filter(not(in(excludes))) - .collect(toImmutableSet()); + runInEmpty + ? ImmutableSet.of("") + : Streams.concat( + forEachRealTld ? getTldsOfType(REAL).stream() : Stream.of(), + forEachTestTld ? getTldsOfType(TEST).stream() : Stream.of()) + .filter(not(in(excludes))) + .collect(toImmutableSet()); Multimap flowThruParams = filterKeys(params, not(in(CONTROL_PARAMS))); Queue taskQueue = getQueue(queue); StringBuilder outputPayload = diff --git a/java/google/registry/dns/DnsModule.java b/java/google/registry/dns/DnsModule.java index 6489dd3c1..4a06206c2 100644 --- a/java/google/registry/dns/DnsModule.java +++ b/java/google/registry/dns/DnsModule.java @@ -16,13 +16,6 @@ package google.registry.dns; import static google.registry.dns.DnsConstants.DNS_PUBLISH_PUSH_QUEUE_NAME; import static google.registry.dns.DnsConstants.DNS_PULL_QUEUE_NAME; -import static google.registry.dns.PublishDnsUpdatesAction.PARAM_DNS_WRITER; -import static google.registry.dns.PublishDnsUpdatesAction.PARAM_DOMAINS; -import static google.registry.dns.PublishDnsUpdatesAction.PARAM_HOSTS; -import static google.registry.dns.PublishDnsUpdatesAction.PARAM_LOCK_INDEX; -import static google.registry.dns.PublishDnsUpdatesAction.PARAM_NUM_PUBLISH_LOCKS; -import static google.registry.dns.PublishDnsUpdatesAction.PARAM_PUBLISH_TASK_ENQUEUED; -import static google.registry.dns.PublishDnsUpdatesAction.PARAM_REFRESH_REQUEST_CREATED; import static google.registry.request.RequestParameters.extractEnumParameter; import static google.registry.request.RequestParameters.extractIntParameter; import static google.registry.request.RequestParameters.extractRequiredParameter; @@ -48,6 +41,14 @@ import org.joda.time.DateTime; @Module public abstract class DnsModule { + public static final String PARAM_DNS_WRITER = "dnsWriter"; + public static final String PARAM_LOCK_INDEX = "lockIndex"; + public static final String PARAM_NUM_PUBLISH_LOCKS = "numPublishLocks"; + public static final String PARAM_DOMAINS = "domains"; + public static final String PARAM_HOSTS = "hosts"; + public static final String PARAM_PUBLISH_TASK_ENQUEUED = "enqueued"; + public static final String PARAM_REFRESH_REQUEST_CREATED = "itemsCreated"; + @Binds @DnsWriterZone abstract String provideZoneName(@Parameter(RequestParameters.PARAM_TLD) String tld); diff --git a/java/google/registry/dns/PublishDnsUpdatesAction.java b/java/google/registry/dns/PublishDnsUpdatesAction.java index 08c89fbec..0141b74aa 100644 --- a/java/google/registry/dns/PublishDnsUpdatesAction.java +++ b/java/google/registry/dns/PublishDnsUpdatesAction.java @@ -14,6 +14,13 @@ package google.registry.dns; +import static google.registry.dns.DnsModule.PARAM_DNS_WRITER; +import static google.registry.dns.DnsModule.PARAM_DOMAINS; +import static google.registry.dns.DnsModule.PARAM_HOSTS; +import static google.registry.dns.DnsModule.PARAM_LOCK_INDEX; +import static google.registry.dns.DnsModule.PARAM_NUM_PUBLISH_LOCKS; +import static google.registry.dns.DnsModule.PARAM_PUBLISH_TASK_ENQUEUED; +import static google.registry.dns.DnsModule.PARAM_REFRESH_REQUEST_CREATED; import static google.registry.request.Action.Method.POST; import static google.registry.request.RequestParameters.PARAM_TLD; import static google.registry.util.CollectionUtils.nullToEmpty; @@ -49,13 +56,6 @@ import org.joda.time.Duration; public final class PublishDnsUpdatesAction implements Runnable, Callable { public static final String PATH = "/_dr/task/publishDnsUpdates"; - public static final String PARAM_DNS_WRITER = "dnsWriter"; - public static final String PARAM_LOCK_INDEX = "lockIndex"; - public static final String PARAM_NUM_PUBLISH_LOCKS = "numPublishLocks"; - public static final String PARAM_DOMAINS = "domains"; - public static final String PARAM_HOSTS = "hosts"; - public static final String PARAM_PUBLISH_TASK_ENQUEUED = "enqueued"; - public static final String PARAM_REFRESH_REQUEST_CREATED = "itemsCreated"; public static final String LOCK_NAME = "DNS updates"; private static final FluentLogger logger = FluentLogger.forEnclosingClass(); diff --git a/java/google/registry/dns/ReadDnsQueueAction.java b/java/google/registry/dns/ReadDnsQueueAction.java index 21a8e6e94..d6bfec63c 100644 --- a/java/google/registry/dns/ReadDnsQueueAction.java +++ b/java/google/registry/dns/ReadDnsQueueAction.java @@ -14,13 +14,20 @@ package google.registry.dns; -import static com.google.appengine.api.taskqueue.TaskOptions.Builder.withUrl; import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap; import static com.google.common.collect.Sets.difference; import static google.registry.dns.DnsConstants.DNS_PUBLISH_PUSH_QUEUE_NAME; import static google.registry.dns.DnsConstants.DNS_TARGET_CREATE_TIME_PARAM; import static google.registry.dns.DnsConstants.DNS_TARGET_NAME_PARAM; import static google.registry.dns.DnsConstants.DNS_TARGET_TYPE_PARAM; +import static google.registry.dns.DnsModule.PARAM_DNS_WRITER; +import static google.registry.dns.DnsModule.PARAM_DOMAINS; +import static google.registry.dns.DnsModule.PARAM_HOSTS; +import static google.registry.dns.DnsModule.PARAM_LOCK_INDEX; +import static google.registry.dns.DnsModule.PARAM_NUM_PUBLISH_LOCKS; +import static google.registry.dns.DnsModule.PARAM_PUBLISH_TASK_ENQUEUED; +import static google.registry.dns.DnsModule.PARAM_REFRESH_REQUEST_CREATED; +import static google.registry.request.RequestParameters.PARAM_TLD; import static google.registry.util.DomainNameUtils.getSecondLevelDomain; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.concurrent.TimeUnit.SECONDS; @@ -44,7 +51,6 @@ import google.registry.model.registry.Registries; import google.registry.model.registry.Registry; import google.registry.request.Action; import google.registry.request.Parameter; -import google.registry.request.RequestParameters; import google.registry.request.auth.Auth; import google.registry.util.Clock; import google.registry.util.TaskQueueUtils; @@ -55,6 +61,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Random; +import java.util.stream.Collectors; import javax.inject.Inject; import javax.inject.Named; import org.joda.time.DateTime; @@ -265,7 +272,7 @@ public final class ReadDnsQueueAction implements Runnable { try { Map params = ImmutableMap.copyOf(task.extractParams()); DateTime creationTime = DateTime.parse(params.get(DNS_TARGET_CREATE_TIME_PARAM)); - String tld = params.get(RequestParameters.PARAM_TLD); + String tld = params.get(PARAM_TLD); if (tld == null) { logger.atSevere().log( "Discarding invalid DNS refresh request %s; no TLD specified.", task); @@ -353,31 +360,33 @@ public final class ReadDnsQueueAction implements Runnable { DateTime earliestCreateTime = chunk.stream().map(RefreshItem::creationTime).min(Comparator.naturalOrder()).get(); for (String dnsWriter : Registry.get(tld).getDnsWriters()) { - TaskOptions options = - withUrl(PublishDnsUpdatesAction.PATH) + taskQueueUtils.enqueue( + dnsPublishPushQueue, + TaskOptions.Builder.withUrl(PublishDnsUpdatesAction.PATH) .countdownMillis( jitterSeconds .map(seconds -> random.nextInt((int) SECONDS.toMillis(seconds))) .orElse(0)) - .param(RequestParameters.PARAM_TLD, tld) - .param(PublishDnsUpdatesAction.PARAM_DNS_WRITER, dnsWriter) - .param(PublishDnsUpdatesAction.PARAM_LOCK_INDEX, Integer.toString(lockIndex)) + .param(PARAM_TLD, tld) + .param(PARAM_DNS_WRITER, dnsWriter) + .param(PARAM_LOCK_INDEX, Integer.toString(lockIndex)) + .param(PARAM_NUM_PUBLISH_LOCKS, Integer.toString(numPublishLocks)) + .param(PARAM_PUBLISH_TASK_ENQUEUED, clock.nowUtc().toString()) + .param(PARAM_REFRESH_REQUEST_CREATED, earliestCreateTime.toString()) .param( - PublishDnsUpdatesAction.PARAM_NUM_PUBLISH_LOCKS, - Integer.toString(numPublishLocks)) + PARAM_DOMAINS, + chunk + .stream() + .filter(item -> item.type() == TargetType.DOMAIN) + .map(RefreshItem::name) + .collect(Collectors.joining(","))) .param( - PublishDnsUpdatesAction.PARAM_PUBLISH_TASK_ENQUEUED, clock.nowUtc().toString()) - .param( - PublishDnsUpdatesAction.PARAM_REFRESH_REQUEST_CREATED, - earliestCreateTime.toString()); - for (RefreshItem refreshItem : chunk) { - options.param( - (refreshItem.type() == TargetType.HOST) - ? PublishDnsUpdatesAction.PARAM_HOSTS - : PublishDnsUpdatesAction.PARAM_DOMAINS, - refreshItem.name()); - } - taskQueueUtils.enqueue(dnsPublishPushQueue, options); + PARAM_HOSTS, + chunk + .stream() + .filter(item -> item.type() == TargetType.HOST) + .map(RefreshItem::name) + .collect(Collectors.joining(",")))); } } } diff --git a/java/google/registry/env/alpha/default/WEB-INF/cron.xml b/java/google/registry/env/alpha/default/WEB-INF/cron.xml index f833af46f..62d31c406 100644 --- a/java/google/registry/env/alpha/default/WEB-INF/cron.xml +++ b/java/google/registry/env/alpha/default/WEB-INF/cron.xml @@ -4,11 +4,11 @@ diff --git a/java/google/registry/env/crash/default/WEB-INF/cron.xml b/java/google/registry/env/crash/default/WEB-INF/cron.xml index 730f04024..cab6f5fef 100644 --- a/java/google/registry/env/crash/default/WEB-INF/cron.xml +++ b/java/google/registry/env/crash/default/WEB-INF/cron.xml @@ -4,11 +4,11 @@ diff --git a/java/google/registry/env/production/default/WEB-INF/cron.xml b/java/google/registry/env/production/default/WEB-INF/cron.xml index 14cda880d..c1db58a70 100644 --- a/java/google/registry/env/production/default/WEB-INF/cron.xml +++ b/java/google/registry/env/production/default/WEB-INF/cron.xml @@ -4,11 +4,11 @@ diff --git a/java/google/registry/env/sandbox/default/WEB-INF/cron.xml b/java/google/registry/env/sandbox/default/WEB-INF/cron.xml index c36dfc566..ac5412605 100644 --- a/java/google/registry/env/sandbox/default/WEB-INF/cron.xml +++ b/java/google/registry/env/sandbox/default/WEB-INF/cron.xml @@ -4,11 +4,11 @@ diff --git a/java/google/registry/module/backend/BackendModule.java b/java/google/registry/module/backend/BackendModule.java index c617c7d86..a84a7fa1c 100644 --- a/java/google/registry/module/backend/BackendModule.java +++ b/java/google/registry/module/backend/BackendModule.java @@ -43,9 +43,10 @@ public class BackendModule { } @Provides - @Parameter(RequestParameters.PARAM_TLD) + @Parameter(RequestParameters.PARAM_TLDS) static ImmutableSet provideTlds(HttpServletRequest req) { - ImmutableSet tlds = extractSetOfParameters(req, RequestParameters.PARAM_TLD); + ImmutableSet tlds = + extractSetOfParameters(req, RequestParameters.PARAM_TLDS, RequestParameters.PARAM_TLD); assertTldsExist(tlds); return tlds; } diff --git a/java/google/registry/rde/RdeModule.java b/java/google/registry/rde/RdeModule.java index ac22b7f12..9bfaba82e 100644 --- a/java/google/registry/rde/RdeModule.java +++ b/java/google/registry/rde/RdeModule.java @@ -41,6 +41,7 @@ import org.joda.time.DateTime; public final class RdeModule { public static final String PARAM_WATERMARK = "watermark"; + public static final String PARAM_WATERMARKS = "watermarks"; public static final String PARAM_MANUAL = "manual"; public static final String PARAM_DIRECTORY = "directory"; public static final String PARAM_MODE = "mode"; @@ -54,9 +55,9 @@ public final class RdeModule { } @Provides - @Parameter(PARAM_WATERMARK) + @Parameter(PARAM_WATERMARKS) static ImmutableSet provideWatermarks(HttpServletRequest req) { - return extractSetOfDatetimeParameters(req, PARAM_WATERMARK); + return extractSetOfDatetimeParameters(req, PARAM_WATERMARKS, PARAM_WATERMARK); } @Provides diff --git a/java/google/registry/rde/RdeStagingAction.java b/java/google/registry/rde/RdeStagingAction.java index 046ab5a2a..fed0bc3a2 100644 --- a/java/google/registry/rde/RdeStagingAction.java +++ b/java/google/registry/rde/RdeStagingAction.java @@ -205,8 +205,8 @@ public final class RdeStagingAction implements Runnable { @Inject @Parameter(RdeModule.PARAM_MANUAL) boolean manual; @Inject @Parameter(RdeModule.PARAM_DIRECTORY) Optional directory; @Inject @Parameter(RdeModule.PARAM_MODE) ImmutableSet modeStrings; - @Inject @Parameter(RequestParameters.PARAM_TLD) ImmutableSet tlds; - @Inject @Parameter(RdeModule.PARAM_WATERMARK) ImmutableSet watermarks; + @Inject @Parameter(RequestParameters.PARAM_TLDS) ImmutableSet tlds; + @Inject @Parameter(RdeModule.PARAM_WATERMARKS) ImmutableSet watermarks; @Inject @Parameter(RdeModule.PARAM_REVISION) Optional revision; @Inject @Parameter(RdeModule.PARAM_LENIENT) boolean lenient; diff --git a/java/google/registry/request/RequestParameters.java b/java/google/registry/request/RequestParameters.java index e24b4317a..23b5b5412 100644 --- a/java/google/registry/request/RequestParameters.java +++ b/java/google/registry/request/RequestParameters.java @@ -14,12 +14,16 @@ package google.registry.request; +import static com.google.common.base.Predicates.not; import static com.google.common.base.Strings.emptyToNull; import static com.google.common.base.Strings.isNullOrEmpty; import static com.google.common.base.Strings.nullToEmpty; +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; @@ -29,8 +33,12 @@ import org.joda.time.DateTime; /** Utilities for extracting parameters from HTTP requests. */ public final class RequestParameters { - /** The standardized request parameter name used by any action that takes a tld parameter. */ + 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. */ + public static final String PARAM_TLDS = "tlds"; /** * Returns first GET or POST parameter associated with {@code name}. @@ -90,10 +98,67 @@ public final class RequestParameters { } } - /** Returns all GET or POST parameters associated with {@code name}. */ - public static ImmutableSet extractSetOfParameters(HttpServletRequest req, String name) { + /** + * Returns all GET or POST parameters associated with {@code name} (or {@code nameLegacy}). + * + *

While transitioning from "param=key1¶m=key2" to "params=key1,key2" style of set inputing + * - we will be accepting all options (with a warning for "wrong" uses). + * + *

TODO(b/78226288): remove transition code (including "legacyName") once there are no more + * warnings. + * + * @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. + */ + public static ImmutableSet extractSetOfParameters( + HttpServletRequest req, String name, @Nullable String legacyName) { String[] parameters = req.getParameterValues(name); - return parameters == null ? ImmutableSet.of() : ImmutableSet.copyOf(parameters); + 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) { + 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); + } + 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}. + * + *

While transitioning from "param=key1¶m=key2" to "params=key1,key2" style of set inputing + * - we will be accepting all options (with a warning for "wrong" uses). + * + *

TODO(b/78226288): remove transition code (including "legacyName") once there are no more + * warnings. + */ + public static ImmutableSet extractSetOfParameters(HttpServletRequest req, String name) { + return extractSetOfParameters(req, name, null); } /** @@ -195,22 +260,16 @@ public final class RequestParameters { * @throws BadRequestException if one of the parameter values is not a valid {@link DateTime}. */ public static ImmutableSet extractSetOfDatetimeParameters( - HttpServletRequest req, String name) { - String[] stringParams = req.getParameterValues(name); - if (stringParams == null) { - return ImmutableSet.of(); + HttpServletRequest req, String name, @Nullable String legacyName) { + try { + return extractSetOfParameters(req, name, legacyName) + .stream() + .filter(not(String::isEmpty)) + .map(DateTime::parse) + .collect(toImmutableSet()); + } catch (IllegalArgumentException e) { + throw new BadRequestException("Bad ISO 8601 timestamp: " + name); } - ImmutableSet.Builder datesBuilder = new ImmutableSet.Builder<>(); - for (String stringParam : stringParams) { - try { - if (!isNullOrEmpty(stringParam)) { - datesBuilder.add(DateTime.parse(stringParam)); - } - } catch (IllegalArgumentException e) { - throw new BadRequestException("Bad ISO 8601 timestamp: " + name); - } - } - return datesBuilder.build(); } private static boolean equalsFalse(@Nullable String value) { diff --git a/java/google/registry/tools/GenerateEscrowDepositCommand.java b/java/google/registry/tools/GenerateEscrowDepositCommand.java index b65a5e0fa..244a68f35 100644 --- a/java/google/registry/tools/GenerateEscrowDepositCommand.java +++ b/java/google/registry/tools/GenerateEscrowDepositCommand.java @@ -16,6 +16,12 @@ package google.registry.tools; import static com.google.appengine.api.taskqueue.TaskOptions.Builder.withUrl; import static google.registry.model.registry.Registries.assertTldsExist; +import static google.registry.rde.RdeModule.PARAM_DIRECTORY; +import static google.registry.rde.RdeModule.PARAM_MANUAL; +import static google.registry.rde.RdeModule.PARAM_MODE; +import static google.registry.rde.RdeModule.PARAM_REVISION; +import static google.registry.rde.RdeModule.PARAM_WATERMARKS; +import static google.registry.request.RequestParameters.PARAM_TLDS; import com.beust.jcommander.Parameter; import com.beust.jcommander.ParameterException; @@ -24,12 +30,11 @@ import com.google.appengine.api.modules.ModulesService; import com.google.appengine.api.taskqueue.Queue; import com.google.appengine.api.taskqueue.TaskOptions; import google.registry.model.rde.RdeMode; -import google.registry.rde.RdeModule; import google.registry.rde.RdeStagingAction; -import google.registry.request.RequestParameters; import google.registry.tools.Command.RemoteApiCommand; import google.registry.tools.params.DateTimeParameter; import java.util.List; +import java.util.stream.Collectors; import javax.inject.Inject; import javax.inject.Named; import org.joda.time.DateTime; @@ -101,17 +106,15 @@ final class GenerateEscrowDepositCommand implements RemoteApiCommand { TaskOptions opts = withUrl(RdeStagingAction.PATH) .header("Host", hostname) - .param(RdeModule.PARAM_MANUAL, String.valueOf(true)) - .param(RdeModule.PARAM_MODE, mode.toString()) - .param(RdeModule.PARAM_DIRECTORY, outdir); - for (String tld : tlds) { - opts = opts.param(RequestParameters.PARAM_TLD, tld); - } - for (DateTime watermark : watermarks) { - opts = opts.param(RdeModule.PARAM_WATERMARK, watermark.toString()); - } + .param(PARAM_MANUAL, String.valueOf(true)) + .param(PARAM_MODE, mode.toString()) + .param(PARAM_DIRECTORY, outdir) + .param(PARAM_TLDS, tlds.stream().collect(Collectors.joining(","))) + .param( + PARAM_WATERMARKS, + watermarks.stream().map(DateTime::toString).collect(Collectors.joining(","))); if (revision != null) { - opts = opts.param(RdeModule.PARAM_REVISION, String.valueOf(revision)); + opts = opts.param(PARAM_REVISION, String.valueOf(revision)); } queue.add(opts); } diff --git a/java/google/registry/tools/server/ListDomainsAction.java b/java/google/registry/tools/server/ListDomainsAction.java index bd14fb1b0..bf64ef60e 100644 --- a/java/google/registry/tools/server/ListDomainsAction.java +++ b/java/google/registry/tools/server/ListDomainsAction.java @@ -20,6 +20,7 @@ import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.registry.Registries.assertTldsExist; import static google.registry.request.Action.Method.GET; import static google.registry.request.Action.Method.POST; +import static google.registry.request.RequestParameters.PARAM_TLDS; import static java.util.Comparator.comparing; import com.google.common.collect.ImmutableSet; @@ -45,7 +46,7 @@ public final class ListDomainsAction extends ListObjectsAction { private static final int MAX_NUM_SUBQUERIES = 30; public static final String PATH = "/_dr/admin/list/domains"; - @Inject @Parameter("tlds") ImmutableSet tlds; + @Inject @Parameter(PARAM_TLDS) ImmutableSet tlds; @Inject @Parameter("limit") int limit; @Inject Clock clock; @Inject ListDomainsAction() {} diff --git a/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java b/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java index cfd7a8e73..d9479dfe0 100644 --- a/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java +++ b/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java @@ -17,6 +17,7 @@ package google.registry.tools.server; import static google.registry.mapreduce.inputs.EppResourceInputs.createEntityInput; import static google.registry.model.EppResourceUtils.isActive; import static google.registry.model.registry.Registries.assertTldsExist; +import static google.registry.request.RequestParameters.PARAM_TLDS; import static google.registry.util.PipelineUtils.createJobPath; import com.google.appengine.tools.mapreduce.Mapper; @@ -57,7 +58,7 @@ public class RefreshDnsForAllDomainsAction implements Runnable { @Inject MapreduceRunner mrRunner; @Inject Response response; - @Inject @Parameter("tlds") ImmutableSet tlds; + @Inject @Parameter(PARAM_TLDS) ImmutableSet tlds; @Inject RefreshDnsForAllDomainsAction() {} @Override diff --git a/java/google/registry/tools/server/ToolsServerModule.java b/java/google/registry/tools/server/ToolsServerModule.java index 9809d8af5..25762e533 100644 --- a/java/google/registry/tools/server/ToolsServerModule.java +++ b/java/google/registry/tools/server/ToolsServerModule.java @@ -19,12 +19,13 @@ import static google.registry.request.RequestParameters.extractBooleanParameter; import static google.registry.request.RequestParameters.extractIntParameter; import static google.registry.request.RequestParameters.extractOptionalParameter; import static google.registry.request.RequestParameters.extractRequiredParameter; +import static google.registry.request.RequestParameters.extractSetOfParameters; -import com.google.common.base.Splitter; import com.google.common.collect.ImmutableSet; import dagger.Module; import dagger.Provides; import google.registry.request.Parameter; +import google.registry.request.RequestParameters; import java.util.Optional; import javax.servlet.http.HttpServletRequest; @@ -79,16 +80,15 @@ public class ToolsServerModule { } @Provides - @Parameter("tld") + @Parameter(RequestParameters.PARAM_TLD) static String provideTld(HttpServletRequest req) { - return extractRequiredParameter(req, "tld"); + return extractRequiredParameter(req, RequestParameters.PARAM_TLD); } @Provides - @Parameter("tlds") + @Parameter(RequestParameters.PARAM_TLDS) static ImmutableSet provideTlds(HttpServletRequest req) { - String tldsString = extractRequiredParameter(req, "tlds"); - return ImmutableSet.copyOf(Splitter.on(',').split(tldsString)); + return extractSetOfParameters(req, RequestParameters.PARAM_TLDS, RequestParameters.PARAM_TLD); } @Provides diff --git a/javatests/google/registry/dns/ReadDnsQueueActionTest.java b/javatests/google/registry/dns/ReadDnsQueueActionTest.java index 44a6f4b0d..ed6237879 100644 --- a/javatests/google/registry/dns/ReadDnsQueueActionTest.java +++ b/javatests/google/registry/dns/ReadDnsQueueActionTest.java @@ -17,6 +17,7 @@ package google.registry.dns; import static com.google.appengine.api.taskqueue.QueueFactory.getQueue; import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.Lists.transform; +import static com.google.common.collect.MoreCollectors.onlyElement; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; import static google.registry.dns.DnsConstants.DNS_PUBLISH_PUSH_QUEUE_NAME; @@ -35,6 +36,7 @@ import com.google.appengine.api.taskqueue.QueueFactory; import com.google.appengine.api.taskqueue.TaskOptions; import com.google.appengine.api.taskqueue.TaskOptions.Method; import com.google.common.base.Joiner; +import com.google.common.base.Splitter; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; @@ -48,10 +50,9 @@ import google.registry.testing.FakeClock; import google.registry.testing.TaskQueueHelper.TaskMatcher; import google.registry.util.Retrier; import google.registry.util.TaskQueueUtils; -import java.util.ArrayList; -import java.util.List; import java.util.Map.Entry; import java.util.Optional; +import java.util.stream.Collectors; import java.util.stream.IntStream; import org.joda.time.DateTime; import org.joda.time.Duration; @@ -218,7 +219,8 @@ public class ReadDnsQueueActionTest { assertThat( queuedParams .stream() - .flatMap(params -> params.get("domains").stream())) + .map(params -> params.get("domains").stream().collect(onlyElement())) + .flatMap(values -> Splitter.on(',').splitToList(values).stream())) .containsExactlyElementsIn(domains); } @@ -249,19 +251,17 @@ public class ReadDnsQueueActionTest { run(); assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); - assertThat(getQueuedParams(DNS_PUBLISH_PUSH_QUEUE_NAME)) + assertThat(getQueuedParams(DNS_PUBLISH_PUSH_QUEUE_NAME)).hasSize(1); + assertThat(getQueuedParams(DNS_PUBLISH_PUSH_QUEUE_NAME).get(0)) .containsExactly( - new ImmutableMultimap.Builder() - .put("enqueued", "3000-02-05T01:00:00.000Z") - .put("itemsCreated", "3000-02-03T00:00:00.000Z") - .put("tld", "com") - .put("dnsWriter", "comWriter") - .put("domains", "domain1.com") - .put("domains", "domain2.com") - .put("domains", "domain3.com") - .put("lockIndex", "1") - .put("numPublishLocks", "1") - .build()); + "enqueued", "3000-02-05T01:00:00.000Z", + "itemsCreated", "3000-02-03T00:00:00.000Z", + "tld", "com", + "dnsWriter", "comWriter", + "domains", "domain1.com,domain2.com,domain3.com", + "hosts", "", + "lockIndex", "1", + "numPublishLocks", "1"); } @Test @@ -411,44 +411,32 @@ public class ReadDnsQueueActionTest { new TaskMatcher().url(PublishDnsUpdatesAction.PATH).param("hosts", "ns1.domain.com")); } + private static String makeCommaSeparatedRange(int from, int to, String format) { + return IntStream.range(from, to) + .mapToObj(i -> String.format(format, i)) + .collect(Collectors.joining(",")); + } + @Test public void testSuccess_manyDomainsAndHosts() throws Exception { - List expectedTasks = new ArrayList<>(); - for (String tld : ImmutableList.of("com", "net")) { - int refreshItemsInTask = 0; - TaskMatcher task = null; + for (int i = 0; i < 150; i++) { // 0: domain; 1: host 1; 2: host 2 for (int thingType = 0; thingType < 3; thingType++) { - for (int i = 0; i < 150; i++) { + for (String tld : ImmutableList.of("com", "net")) { String domainName = String.format("domain%04d.%s", i, tld); - // If we don't have an existing task into which to dump new refreshes, create one. - if (task == null) { - task = new TaskMatcher().url(PublishDnsUpdatesAction.PATH); - expectedTasks.add(task); - refreshItemsInTask = 0; - } switch (thingType) { case 1: getQueue(DNS_PULL_QUEUE_NAME) .add(createRefreshTask("ns1." + domainName, TargetType.HOST)); - task.param("hosts", "ns1." + domainName); break; case 2: getQueue(DNS_PULL_QUEUE_NAME) .add(createRefreshTask("ns2." + domainName, TargetType.HOST)); - task.param("hosts", "ns2." + domainName); break; default: dnsQueue.addDomainRefreshTask(domainName); - task.param("domains", domainName); break; } - // If this task is now full up, wash our hands of it, so that we'll start a new one the - // next time through the loop. - refreshItemsInTask++; - if (refreshItemsInTask >= TEST_TLD_UPDATE_BATCH_SIZE) { - task = null; - } } } } @@ -456,7 +444,48 @@ public class ReadDnsQueueActionTest { run(); assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); - assertTasksEnqueued(DNS_PUBLISH_PUSH_QUEUE_NAME, expectedTasks); + assertTasksEnqueued( + DNS_PUBLISH_PUSH_QUEUE_NAME, + new TaskMatcher() + .url(PublishDnsUpdatesAction.PATH) + .param("domains", makeCommaSeparatedRange(0, 100, "domain%04d.com")) + .param("hosts", ""), + new TaskMatcher() + .url(PublishDnsUpdatesAction.PATH) + .param("domains", makeCommaSeparatedRange(100, 150, "domain%04d.com")) + .param("hosts", makeCommaSeparatedRange(0, 50, "ns1.domain%04d.com")), + new TaskMatcher() + .url(PublishDnsUpdatesAction.PATH) + .param("domains", "") + .param("hosts", makeCommaSeparatedRange(50, 150, "ns1.domain%04d.com")), + new TaskMatcher() + .url(PublishDnsUpdatesAction.PATH) + .param("domains", "") + .param("hosts", makeCommaSeparatedRange(0, 100, "ns2.domain%04d.com")), + new TaskMatcher() + .url(PublishDnsUpdatesAction.PATH) + .param("domains", "") + .param("hosts", makeCommaSeparatedRange(100, 150, "ns2.domain%04d.com")), + new TaskMatcher() + .url(PublishDnsUpdatesAction.PATH) + .param("domains", makeCommaSeparatedRange(0, 100, "domain%04d.net")) + .param("hosts", ""), + new TaskMatcher() + .url(PublishDnsUpdatesAction.PATH) + .param("domains", makeCommaSeparatedRange(100, 150, "domain%04d.net")) + .param("hosts", makeCommaSeparatedRange(0, 50, "ns1.domain%04d.net")), + new TaskMatcher() + .url(PublishDnsUpdatesAction.PATH) + .param("domains", "") + .param("hosts", makeCommaSeparatedRange(50, 150, "ns1.domain%04d.net")), + new TaskMatcher() + .url(PublishDnsUpdatesAction.PATH) + .param("domains", "") + .param("hosts", makeCommaSeparatedRange(0, 100, "ns2.domain%04d.net")), + new TaskMatcher() + .url(PublishDnsUpdatesAction.PATH) + .param("domains", "") + .param("hosts", makeCommaSeparatedRange(100, 150, "ns2.domain%04d.net"))); } @Test @@ -481,8 +510,7 @@ public class ReadDnsQueueActionTest { .param("itemsCreated", "3000-01-01T00:00:00.000Z") .param("enqueued", "3000-01-01T01:00:00.000Z") .param("domains", "hello.multilock.uk") - .param("hosts", "ns1.abc.hello.multilock.uk") - .param("hosts", "ns2.hello.multilock.uk") + .param("hosts", "ns1.abc.hello.multilock.uk,ns2.hello.multilock.uk") .header("content-type", "application/x-www-form-urlencoded"), new TaskMatcher() .url(PublishDnsUpdatesAction.PATH) @@ -491,8 +519,7 @@ public class ReadDnsQueueActionTest { .param("itemsCreated", "3000-01-01T00:00:00.000Z") .param("enqueued", "3000-01-01T01:00:00.000Z") .param("domains", "another.multilock.uk") - .param("hosts", "ns3.def.another.multilock.uk") - .param("hosts", "ns4.another.multilock.uk") + .param("hosts", "ns3.def.another.multilock.uk,ns4.another.multilock.uk") .header("content-type", "application/x-www-form-urlencoded")); } } diff --git a/javatests/google/registry/tools/GenerateEscrowDepositCommandTest.java b/javatests/google/registry/tools/GenerateEscrowDepositCommandTest.java index 13b102aec..123fca0e7 100644 --- a/javatests/google/registry/tools/GenerateEscrowDepositCommandTest.java +++ b/javatests/google/registry/tools/GenerateEscrowDepositCommandTest.java @@ -193,8 +193,8 @@ public class GenerateEscrowDepositCommandTest .url("/_dr/task/rdeStaging") .header("Host", "1.backend.test.localhost") .param("mode", "THIN") - .param("watermark", "2017-01-01T00:00:00.000Z") - .param("tld", "tld") + .param("watermarks", "2017-01-01T00:00:00.000Z") + .param("tlds", "tld") .param("directory", "test") .param("manual", "true") .param("revision", "42")); @@ -209,8 +209,8 @@ public class GenerateEscrowDepositCommandTest .url("/_dr/task/rdeStaging") .header("Host", "1.backend.test.localhost") .param("mode", "THIN") - .param("watermark", "2017-01-01T00:00:00.000Z") - .param("tld", "tld") + .param("watermarks", "2017-01-01T00:00:00.000Z") + .param("tlds", "tld") .param("directory", "test") .param("manual", "true")); } @@ -224,8 +224,8 @@ public class GenerateEscrowDepositCommandTest .url("/_dr/task/rdeStaging") .header("Host", "1.backend.test.localhost") .param("mode", "FULL") - .param("watermark", "2017-01-01T00:00:00.000Z") - .param("tld", "tld") + .param("watermarks", "2017-01-01T00:00:00.000Z") + .param("tlds", "tld") .param("directory", "test") .param("manual", "true") .param("revision", "42")); @@ -245,10 +245,8 @@ public class GenerateEscrowDepositCommandTest .url("/_dr/task/rdeStaging") .header("Host", "1.backend.test.localhost") .param("mode", "THIN") - .param("watermark", "2017-01-01T00:00:00.000Z") - .param("watermark", "2017-01-02T00:00:00.000Z") - .param("tld", "tld") - .param("tld", "anothertld") + .param("watermarks", "2017-01-01T00:00:00.000Z,2017-01-02T00:00:00.000Z") + .param("tlds", "tld,anothertld") .param("directory", "test") .param("manual", "true") .param("revision", "42"));