diff --git a/java/google/registry/cron/TldFanoutAction.java b/java/google/registry/cron/TldFanoutAction.java index f1fd453df..2a601493e 100644 --- a/java/google/registry/cron/TldFanoutAction.java +++ b/java/google/registry/cron/TldFanoutAction.java @@ -16,13 +16,13 @@ package google.registry.cron; import static com.google.appengine.api.taskqueue.QueueFactory.getQueue; import static com.google.appengine.api.taskqueue.TaskOptions.Builder.withUrl; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Predicates.in; import static com.google.common.base.Predicates.not; import static com.google.common.base.Strings.nullToEmpty; 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.collect.Sets.difference; import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8; import static google.registry.model.registry.Registries.getTldsOfType; import static google.registry.model.registry.Registry.TldType.REAL; @@ -42,10 +42,11 @@ import google.registry.request.ParameterMap; import google.registry.request.RequestParameters; import google.registry.request.Response; import google.registry.request.auth.Auth; +import google.registry.util.FormattingLogger; import google.registry.util.TaskEnqueuer; import java.util.Optional; import java.util.Random; -import java.util.Set; +import java.util.stream.Stream; import javax.inject.Inject; /** @@ -58,10 +59,10 @@ import javax.inject.Inject; *
  • {@code queue} (Required) Name of the App Engine push queue to which this task should be sent. *
  • {@code forEachRealTld} Launch the task in each real TLD namespace. *
  • {@code forEachTestTld} Launch the task in each test TLD namespace. - *
  • {@code runInEmpty} Launch the task in the empty namespace. + *
  • {@code runInEmpty} Launch the task once, without the TLD argument. *
  • {@code exclude} TLDs to exclude. *
  • {@code jitterSeconds} Randomly delay each task by up to this many seconds. - *
  • Any other parameters specified will be passed through as POST parameters to the called task. + *
  • Any other parameters specified will be passed through as POST parameters to the called task. * * *

    Patharg Reference

    @@ -98,9 +99,10 @@ public final class TldFanoutAction implements Runnable { EXCLUDE_PARAM, JITTER_SECONDS_PARAM); - private static final String TLD_PATHARG = ":tld"; private static final Random random = new Random(); + private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); + @Inject TaskEnqueuer taskEnqueuer; @Inject Response response; @Inject @Parameter(ENDPOINT_PARAM) String endpoint; @@ -115,28 +117,40 @@ public final class TldFanoutAction implements Runnable { @Override public void run() { - Set tlds = - difference( - Streams.concat( - Streams.stream(runInEmpty ? ImmutableSet.of("") : ImmutableSet.of()), - Streams.stream( - forEachRealTld ? getTldsOfType(REAL) : ImmutableSet.of()), - Streams.stream( - forEachTestTld ? getTldsOfType(TEST) : ImmutableSet.of())) - .collect(toImmutableSet()), - excludes); + checkArgument( + !(runInEmpty && (forEachTestTld || forEachRealTld)), + "runInEmpty and forEach*Tld are mutually exclusive"); + checkArgument( + runInEmpty || forEachTestTld || forEachRealTld, + "At least one of runInEmpty, forEachTestTld, forEachRealTld must be given"); + checkArgument( + !(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(tld -> !excludes.contains(tld)) + .collect(toImmutableSet()); Multimap flowThruParams = filterKeys(params, not(in(CONTROL_PARAMS))); Queue taskQueue = getQueue(queue); StringBuilder outputPayload = new StringBuilder( String.format("OK: Launched the following %d tasks in queue %s\n", tlds.size(), queue)); + logger.infofmt("Launching %d tasks in queue %s", tlds.size(), queue); + if (tlds.isEmpty()) { + logger.warning("No TLDs to fan-out!"); + } for (String tld : tlds) { TaskOptions taskOptions = createTaskOptions(tld, flowThruParams); TaskHandle taskHandle = taskEnqueuer.enqueue(taskQueue, taskOptions); outputPayload.append( String.format( - "- Task: %s, tld: %s, endpoint: %s\n", + "- Task: '%s', tld: '%s', endpoint: '%s'\n", taskHandle.getName(), tld, taskOptions.getUrl())); + logger.infofmt("Task: '%s', tld: '%s', endpoint: '%s'", + taskHandle.getName(), tld, taskOptions.getUrl()); } response.setContentType(PLAIN_TEXT_UTF_8); response.setPayload(outputPayload.toString()); @@ -144,12 +158,14 @@ public final class TldFanoutAction implements Runnable { private TaskOptions createTaskOptions(String tld, Multimap params) { TaskOptions options = - withUrl(endpoint.replace(TLD_PATHARG, String.valueOf(tld))) + withUrl(endpoint) .countdownMillis( - jitterSeconds.isPresent() - ? random.nextInt((int) SECONDS.toMillis(jitterSeconds.get())) - : 0); - options.param(RequestParameters.PARAM_TLD, tld); + jitterSeconds + .map(seconds -> random.nextInt((int) SECONDS.toMillis(seconds))) + .orElse(0)); + if (!tld.isEmpty()) { + options.param(RequestParameters.PARAM_TLD, tld); + } for (String param : params.keySet()) { // TaskOptions.param() does not accept null values. options.param(param, nullToEmpty((getFirst(params.get(param), null)))); 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 30686205a..bb0589566 100644 --- a/java/google/registry/env/alpha/default/WEB-INF/cron.xml +++ b/java/google/registry/env/alpha/default/WEB-INF/cron.xml @@ -5,9 +5,7 @@ /cron/fanout params: queue= endpoint= // URL Path of servlet, which may contain placeholders: - // :tld - Replaced with the TLD, e.g. foo, soy - // :registrar - Replaced with registrar clientId - runInEmpty // Run in the empty namespace + runInEmpty // Run once, with no tld parameter forEachRealTld // Run for tlds with getTldType() == TldType.REAL forEachTestTld // Run for tlds with getTldType() == TldType.TEST exclude=TLD1[&exclude=TLD2] // exclude something otherwise included 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 9352939a2..8189f7089 100644 --- a/java/google/registry/env/production/default/WEB-INF/cron.xml +++ b/java/google/registry/env/production/default/WEB-INF/cron.xml @@ -5,9 +5,7 @@ /cron/fanout params: queue= endpoint= // URL Path of servlet, which may contain placeholders: - // :tld - Replaced with the TLD, e.g. foo, soy - // :registrar - Replaced with registrar clientId - runInEmpty // Run in the empty namespace + runInEmpty // Run once, with no tld parameter forEachRealTld // Run for tlds with getTldType() == TldType.REAL forEachTestTld // Run for tlds with getTldType() == TldType.TEST exclude=TLD1[&exclude=TLD2] // exclude something otherwise included 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 62ef3a97d..b410c933d 100644 --- a/java/google/registry/env/sandbox/default/WEB-INF/cron.xml +++ b/java/google/registry/env/sandbox/default/WEB-INF/cron.xml @@ -5,9 +5,7 @@ /cron/fanout params: queue= endpoint= // URL Path of servlet, which may contain placeholders: - // :tld - Replaced with the TLD, e.g. foo, soy - // :registrar - Replaced with registrar clientId - runInEmpty // Run in the empty namespace + runInEmpty // Run once, with no tld parameter forEachRealTld // Run for tlds with getTldType() == TldType.REAL forEachTestTld // Run for tlds with getTldType() == TldType.TEST exclude=TLD1[&exclude=TLD2] // exclude something otherwise included diff --git a/javatests/google/registry/cron/TldFanoutActionTest.java b/javatests/google/registry/cron/TldFanoutActionTest.java index 31ead3030..265456cc4 100644 --- a/javatests/google/registry/cron/TldFanoutActionTest.java +++ b/javatests/google/registry/cron/TldFanoutActionTest.java @@ -14,14 +14,14 @@ package google.registry.cron; +import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.Iterables.getLast; -import static com.google.common.collect.Lists.transform; import static com.google.common.truth.Truth.assertThat; import static google.registry.testing.DatastoreHelper.createTlds; import static google.registry.testing.DatastoreHelper.persistResource; +import static google.registry.testing.JUnitBackports.assertThrows; import static google.registry.testing.TaskQueueHelper.assertNoTasksEnqueued; import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued; -import static java.util.Arrays.asList; import com.google.appengine.api.taskqueue.dev.QueueStateInfo.TaskStateInfo; import com.google.appengine.tools.development.testing.LocalTaskQueueTestConfig; @@ -38,6 +38,7 @@ import google.registry.util.Retrier; import google.registry.util.TaskEnqueuer; import java.util.List; import java.util.Optional; +import java.util.stream.Stream; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -101,24 +102,21 @@ public class TldFanoutActionTest { private static void assertTasks(String... tasks) throws Exception { assertTasksEnqueued( QUEUE, - transform( - asList(tasks), - (String namespace) -> + Stream.of(tasks).map( + namespace -> new TaskMatcher() .url(ENDPOINT) .header("content-type", "application/x-www-form-urlencoded") - .param("tld", namespace))); + .param("tld", namespace)) + .collect(toImmutableList())); } - @Test - public void testSuccess_pathargTld() throws Exception { - run(getParamsMap( - "forEachRealTld", "", - "endpoint", "/the/servlet/:tld")); - assertTasksEnqueued(QUEUE, - new TaskMatcher().url("/the/servlet/com"), - new TaskMatcher().url("/the/servlet/net"), - new TaskMatcher().url("/the/servlet/org")); + private static void assertTaskWithoutTld() throws Exception { + assertTasksEnqueued( + QUEUE, + new TaskMatcher() + .url(ENDPOINT) + .header("content-type", "application/x-www-form-urlencoded")); } @Test @@ -128,15 +126,14 @@ public class TldFanoutActionTest { } @Test - public void testSuccess_noTlds() throws Exception { - run(getParamsMap()); - assertNoTasksEnqueued(QUEUE); + public void testFailure_noTlds() throws Exception { + assertThrows(IllegalArgumentException.class, () -> run(getParamsMap())); } @Test public void testSuccess_runInEmpty() throws Exception { run(getParamsMap("runInEmpty", "")); - assertTasks(""); + assertTaskWithoutTld(); } @Test @@ -151,12 +148,6 @@ public class TldFanoutActionTest { assertTasks("example"); } - @Test - public void testSuccess_runInEmptyAndRunInRealTld() throws Exception { - run(getParamsMap("runInEmpty", "", "forEachRealTld", "")); - assertTasks("", "com", "net", "org"); - } - @Test public void testSuccess_forEachTestTldAndForEachRealTld() throws Exception { run(getParamsMap( @@ -165,16 +156,10 @@ public class TldFanoutActionTest { assertTasks("com", "net", "org", "example"); } - @Test - public void testSuccess_runInEmptyAndForEachTestTld() throws Exception { - run(getParamsMap("runInEmpty", "", "forEachTestTld", "")); - assertTasks("", "example"); - } - @Test public void testSuccess_runEverywhere() throws Exception { - run(getParamsMap("runInEmpty", "", "forEachTestTld", "", "forEachRealTld", "")); - assertTasks("", "com", "net", "org", "example"); + run(getParamsMap("forEachTestTld", "", "forEachRealTld", "")); + assertTasks("com", "net", "org", "example"); } @Test @@ -196,11 +181,43 @@ public class TldFanoutActionTest { @Test public void testSuccess_excludeNonexistentTlds() throws Exception { run(getParamsMap( - "runInEmpty", "", "forEachTestTld", "", "forEachRealTld", "", "exclude", "foo")); - assertTasks("", "com", "net", "org", "example"); + assertTasks("com", "net", "org", "example"); + } + + @Test + public void testFailure_runInEmptyAndTest() throws Exception { + assertThrows( + IllegalArgumentException.class, + () -> + run( + getParamsMap( + "runInEmpty", "", + "forEachTestTld", ""))); + } + + @Test + public void testFailure_runInEmptyAndReal() throws Exception { + assertThrows( + IllegalArgumentException.class, + () -> + run( + getParamsMap( + "runInEmpty", "", + "forEachRealTld", ""))); + } + + @Test + public void testFailure_runInEmptyAndExclude() throws Exception { + assertThrows( + IllegalArgumentException.class, + () -> + run( + getParamsMap( + "runInEmpty", "", + "exclude", "foo"))); } @Test @@ -212,7 +229,7 @@ public class TldFanoutActionTest { @Test public void testSuccess_returnHttpResponse() throws Exception { - run(getParamsMap("forEachRealTld", "", "endpoint", "/the/servlet/:tld")); + run(getParamsMap("forEachRealTld", "", "endpoint", "/the/servlet")); List taskList = LocalTaskQueueTestConfig.getLocalTaskQueue().getQueueStateInfo().get(QUEUE).getTaskInfo(); @@ -220,12 +237,27 @@ public class TldFanoutActionTest { assertThat(taskList).hasSize(3); String expectedResponse = String.format( "OK: Launched the following 3 tasks in queue the-queue\n" - + "- Task: %s, tld: com, endpoint: /the/servlet/com\n" - + "- Task: %s, tld: net, endpoint: /the/servlet/net\n" - + "- Task: %s, tld: org, endpoint: /the/servlet/org\n", + + "- Task: '%s', tld: 'com', endpoint: '/the/servlet'\n" + + "- Task: '%s', tld: 'net', endpoint: '/the/servlet'\n" + + "- Task: '%s', tld: 'org', endpoint: '/the/servlet'\n", taskList.get(0).getTaskName(), taskList.get(1).getTaskName(), taskList.get(2).getTaskName()); assertThat(response.getPayload()).isEqualTo(expectedResponse); } + + @Test + public void testSuccess_returnHttpResponse_runInEmpty() throws Exception { + run(getParamsMap("runInEmpty", "", "endpoint", "/the/servlet")); + + List taskList = + LocalTaskQueueTestConfig.getLocalTaskQueue().getQueueStateInfo().get(QUEUE).getTaskInfo(); + + assertThat(taskList).hasSize(1); + String expectedResponse = String.format( + "OK: Launched the following 1 tasks in queue the-queue\n" + + "- Task: '%s', tld: '', endpoint: '/the/servlet'\n", + taskList.get(0).getTaskName()); + assertThat(response.getPayload()).isEqualTo(expectedResponse); + } }