diff --git a/java/google/registry/tools/ListDomainsCommand.java b/java/google/registry/tools/ListDomainsCommand.java index d6fb9ac30..f9a634a48 100644 --- a/java/google/registry/tools/ListDomainsCommand.java +++ b/java/google/registry/tools/ListDomainsCommand.java @@ -14,20 +14,24 @@ package google.registry.tools; +import static com.google.common.base.Preconditions.checkArgument; + import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; +import com.google.common.base.Joiner; import com.google.common.collect.ImmutableMap; import google.registry.tools.server.ListDomainsAction; +import java.util.List; -/** Command to list all second-level domains associated with a TLD. */ -@Parameters(separators = " =", commandDescription = "List domains associated with a TLD.") +/** Command to list all second-level domains on specified TLD(s). */ +@Parameters(separators = " =", commandDescription = "List domains on TLD(s).") final class ListDomainsCommand extends ListObjectsCommand { @Parameter( - names = {"-t", "--tld"}, - description = "Top level domain whose second-level domains shall be listed.", + names = {"-t", "--tld", "--tlds"}, + description = "Comma-delimited list of top-level domain(s) to list second-level domains of.", required = true) - private String tld; + private List tlds; @Override String getCommandPath() { @@ -38,6 +42,8 @@ final class ListDomainsCommand extends ListObjectsCommand { * (in addition to the usual ones). */ @Override ImmutableMap getParameterMap() { - return ImmutableMap.of("tld", tld); + String tldsParam = Joiner.on(',').join(tlds); + checkArgument(tldsParam.length() < 1024, "Total length of TLDs is too long for URL parameter"); + return ImmutableMap.of("tlds", tldsParam); } } diff --git a/java/google/registry/tools/server/ListDomainsAction.java b/java/google/registry/tools/server/ListDomainsAction.java index 0837b6cf0..d54a58e7c 100644 --- a/java/google/registry/tools/server/ListDomainsAction.java +++ b/java/google/registry/tools/server/ListDomainsAction.java @@ -14,28 +14,38 @@ package google.registry.tools.server; +import static com.google.common.base.Preconditions.checkArgument; import static google.registry.model.EppResourceUtils.queryNotDeleted; import static google.registry.model.registry.Registries.assertTldExists; import static google.registry.request.Action.Method.GET; import static google.registry.request.Action.Method.POST; -import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableSortedSet; +import com.google.common.collect.Lists; import google.registry.model.domain.DomainResource; import google.registry.request.Action; import google.registry.request.Parameter; import google.registry.util.Clock; import java.util.Comparator; +import java.util.List; import javax.inject.Inject; /** An action that lists domains, for use by the {@code nomulus list_domains} command. */ @Action(path = ListDomainsAction.PATH, method = {GET, POST}) public final class ListDomainsAction extends ListObjectsAction { + /** An App Engine limitation on how many subqueries can be used in a single query. */ + private static final int MAX_NUM_SUBQUERIES = 30; + private static final Comparator COMPARATOR = + new Comparator() { + @Override + public int compare(DomainResource a, DomainResource b) { + return a.getFullyQualifiedDomainName().compareTo(b.getFullyQualifiedDomainName()); + }}; public static final String PATH = "/_dr/admin/list/domains"; - public static final String TLD_PARAM = "tld"; - @Inject @Parameter("tld") String tld; + @Inject @Parameter("tlds") ImmutableSet tlds; @Inject Clock clock; @Inject ListDomainsAction() {} @@ -46,12 +56,15 @@ public final class ListDomainsAction extends ListObjectsAction { @Override public ImmutableSet loadObjects() { - return FluentIterable - .from(queryNotDeleted(DomainResource.class, clock.nowUtc(), "tld", assertTldExists(tld))) - .toSortedSet(new Comparator() { - @Override - public int compare(DomainResource a, DomainResource b) { - return a.getFullyQualifiedDomainName().compareTo(b.getFullyQualifiedDomainName()); - }}); + checkArgument(!tlds.isEmpty(), "Must specify TLDs to query"); + for (String tld : tlds) { + assertTldExists(tld); + } + ImmutableSortedSet.Builder builder = + new ImmutableSortedSet.Builder(COMPARATOR); + for (List batch : Lists.partition(tlds.asList(), MAX_NUM_SUBQUERIES)) { + builder.addAll(queryNotDeleted(DomainResource.class, clock.nowUtc(), "tld in", batch)); + } + return builder.build(); } } diff --git a/java/google/registry/tools/server/ToolsServerModule.java b/java/google/registry/tools/server/ToolsServerModule.java index 59f567f90..d08303a01 100644 --- a/java/google/registry/tools/server/ToolsServerModule.java +++ b/java/google/registry/tools/server/ToolsServerModule.java @@ -20,6 +20,8 @@ import static google.registry.request.RequestParameters.extractOptionalParameter import static google.registry.request.RequestParameters.extractRequiredParameter; import com.google.common.base.Optional; +import com.google.common.base.Splitter; +import com.google.common.collect.ImmutableSet; import dagger.Module; import dagger.Provides; import google.registry.request.Parameter; @@ -81,6 +83,13 @@ public class ToolsServerModule { return extractRequiredParameter(req, "tld"); } + @Provides + @Parameter("tlds") + static ImmutableSet provideTlds(HttpServletRequest req) { + String tldsString = extractRequiredParameter(req, "tlds"); + return ImmutableSet.copyOf(Splitter.on(',').split(tldsString)); + } + @Provides @Parameter("rawKeys") static String provideRawKeys(HttpServletRequest req) { diff --git a/javatests/google/registry/tools/ListDomainsCommandTest.java b/javatests/google/registry/tools/ListDomainsCommandTest.java index 9ab58d953..37ce7e498 100644 --- a/javatests/google/registry/tools/ListDomainsCommandTest.java +++ b/javatests/google/registry/tools/ListDomainsCommandTest.java @@ -14,7 +14,11 @@ package google.registry.tools; +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; import google.registry.tools.server.ListDomainsAction; +import java.util.List; +import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.runners.MockitoJUnitRunner; @@ -32,7 +36,15 @@ public class ListDomainsCommandTest extends ListObjectsCommandTestCase getTlds() { + return ImmutableList.of("foo"); + } + + @Test + public void test_tldsParamTooLong() throws Exception { + String tldsParam = "--tld=foo,bar" + Strings.repeat(",baz", 300); + thrown.expect( + IllegalArgumentException.class, "Total length of TLDs is too long for URL parameter"); + runCommand(tldsParam); } } diff --git a/javatests/google/registry/tools/ListHostsCommandTest.java b/javatests/google/registry/tools/ListHostsCommandTest.java index 6109b9f62..2263893d1 100644 --- a/javatests/google/registry/tools/ListHostsCommandTest.java +++ b/javatests/google/registry/tools/ListHostsCommandTest.java @@ -27,9 +27,4 @@ public class ListHostsCommandTest extends ListObjectsCommandTestCase abstract String getTaskPath(); /** - * The TLD to be used (for those subclasses that use TLDs; may be null). + * The TLD to be used (for those subclasses that use TLDs; defaults to empty). */ - abstract String getTld(); + protected List getTlds() { + return ImmutableList.of(); + } /** - * The TLD argument to be passed on the command line; null if not needed. + * The TLDs argument to be passed on the command line; null if not needed. */ - String tldArgumentString; + @Nullable String tldsParameter; @Before public void init() throws Exception { - String tld = getTld(); - if (tld == null) { - tldArgumentString = null; - } else { - createTld(tld); - tldArgumentString = "--tld=" + tld; - } + tldsParameter = getTlds().isEmpty() ? null : ("--tld=" + Joiner.on(',').join(getTlds())); command.setConnection(connection); when( connection.send( @@ -89,9 +88,8 @@ public abstract class ListObjectsCommandTestCase if (fullFieldNames.isPresent()) { params.put(FULL_FIELD_NAMES_PARAM, fullFieldNames.get()); } - String tld = getTld(); - if (tld != null) { - params.put("tld", tld); + if (!getTlds().isEmpty()) { + params.put("tlds", Joiner.on(',').join(getTlds())); } verify(connection).send( eq(getTaskPath()), @@ -102,74 +100,74 @@ public abstract class ListObjectsCommandTestCase @Test public void testRun_noFields() throws Exception { - if (tldArgumentString == null) { + if (tldsParameter == null) { runCommand(); } else { - runCommand(tldArgumentString); + runCommand(tldsParameter); } verifySent(null, Optional.absent(), Optional.absent()); } @Test public void testRun_oneField() throws Exception { - if (tldArgumentString == null) { + if (tldsParameter == null) { runCommand("--fields=fieldName"); } else { - runCommand("--fields=fieldName", tldArgumentString); + runCommand("--fields=fieldName", tldsParameter); } verifySent("fieldName", Optional.absent(), Optional.absent()); } @Test public void testRun_wildcardField() throws Exception { - if (tldArgumentString == null) { + if (tldsParameter == null) { runCommand("--fields=*"); } else { - runCommand("--fields=*", tldArgumentString); + runCommand("--fields=*", tldsParameter); } verifySent("*", Optional.absent(), Optional.absent()); } @Test public void testRun_header() throws Exception { - if (tldArgumentString == null) { + if (tldsParameter == null) { runCommand("--fields=fieldName", "--header=true"); } else { - runCommand("--fields=fieldName", "--header=true", tldArgumentString); + runCommand("--fields=fieldName", "--header=true", tldsParameter); } verifySent("fieldName", Optional.of(Boolean.TRUE), Optional.absent()); } @Test public void testRun_noHeader() throws Exception { - if (tldArgumentString == null) { + if (tldsParameter == null) { runCommand("--fields=fieldName", "--header=false"); } else { - runCommand("--fields=fieldName", "--header=false", tldArgumentString); + runCommand("--fields=fieldName", "--header=false", tldsParameter); } verifySent("fieldName", Optional.of(Boolean.FALSE), Optional.absent()); } @Test public void testRun_fullFieldNames() throws Exception { - if (tldArgumentString == null) { + if (tldsParameter == null) { runCommand("--fields=fieldName", "--full_field_names"); } else { - runCommand("--fields=fieldName", "--full_field_names", tldArgumentString); + runCommand("--fields=fieldName", "--full_field_names", tldsParameter); } verifySent("fieldName", Optional.absent(), Optional.of(Boolean.TRUE)); } @Test public void testRun_allParameters() throws Exception { - if (tldArgumentString == null) { + if (tldsParameter == null) { runCommand("--fields=fieldName,otherFieldName,*", "--header=true", "--full_field_names"); } else { runCommand( "--fields=fieldName,otherFieldName,*", "--header=true", "--full_field_names", - tldArgumentString); + tldsParameter); } verifySent( "fieldName,otherFieldName,*", Optional.of(Boolean.TRUE), Optional.of(Boolean.TRUE)); diff --git a/javatests/google/registry/tools/ListPremiumListsCommandTest.java b/javatests/google/registry/tools/ListPremiumListsCommandTest.java index 02b20b575..288bbc1a7 100644 --- a/javatests/google/registry/tools/ListPremiumListsCommandTest.java +++ b/javatests/google/registry/tools/ListPremiumListsCommandTest.java @@ -28,9 +28,4 @@ public class ListPremiumListsCommandTest final String getTaskPath() { return ListPremiumListsAction.PATH; } - - @Override - final String getTld() { - return null; - } } diff --git a/javatests/google/registry/tools/ListRegistrarsCommandTest.java b/javatests/google/registry/tools/ListRegistrarsCommandTest.java index c549459de..491f193af 100644 --- a/javatests/google/registry/tools/ListRegistrarsCommandTest.java +++ b/javatests/google/registry/tools/ListRegistrarsCommandTest.java @@ -28,9 +28,4 @@ public class ListRegistrarsCommandTest final String getTaskPath() { return ListRegistrarsAction.PATH; } - - @Override - final String getTld() { - return null; - } } diff --git a/javatests/google/registry/tools/ListReservedListsCommandTest.java b/javatests/google/registry/tools/ListReservedListsCommandTest.java index d92b31e05..585910766 100644 --- a/javatests/google/registry/tools/ListReservedListsCommandTest.java +++ b/javatests/google/registry/tools/ListReservedListsCommandTest.java @@ -28,9 +28,4 @@ public class ListReservedListsCommandTest final String getTaskPath() { return ListReservedListsAction.PATH; } - - @Override - final String getTld() { - return null; - } } diff --git a/javatests/google/registry/tools/ListTldsCommandTest.java b/javatests/google/registry/tools/ListTldsCommandTest.java index e8b77508b..1045c4acb 100644 --- a/javatests/google/registry/tools/ListTldsCommandTest.java +++ b/javatests/google/registry/tools/ListTldsCommandTest.java @@ -21,16 +21,10 @@ import google.registry.tools.server.ListTldsAction; * * @see ListObjectsCommandTestCase */ -public class ListTldsCommandTest - extends ListObjectsCommandTestCase { +public class ListTldsCommandTest extends ListObjectsCommandTestCase { @Override final String getTaskPath() { return ListTldsAction.PATH; } - - @Override - final String getTld() { - return null; - } } diff --git a/javatests/google/registry/tools/server/ListDomainsActionTest.java b/javatests/google/registry/tools/server/ListDomainsActionTest.java index e1c27ecc2..51ba81922 100644 --- a/javatests/google/registry/tools/server/ListDomainsActionTest.java +++ b/javatests/google/registry/tools/server/ListDomainsActionTest.java @@ -19,6 +19,7 @@ import static google.registry.testing.DatastoreHelper.createTlds; import static google.registry.testing.DatastoreHelper.persistActiveDomain; import com.google.common.base.Optional; +import com.google.common.collect.ImmutableSet; import google.registry.testing.FakeClock; import org.joda.time.DateTime; import org.junit.Before; @@ -42,19 +43,19 @@ public class ListDomainsActionTest extends ListActionTestCase { } @Test - public void testRun_invalidRequest_missingTld() throws Exception { - action.tld = null; + public void testRun_invalidRequest_missingTlds() throws Exception { + action.tlds = ImmutableSet.of(); testRunError( action, null, null, null, - "^Null or empty TLD specified$"); + "^Must specify TLDs to query$"); } @Test public void testRun_invalidRequest_invalidTld() throws Exception { - action.tld = "%%%badtld%%%"; + action.tlds = ImmutableSet.of("%%%badtld%%%"); testRunError( action, null, @@ -65,7 +66,7 @@ public class ListDomainsActionTest extends ListActionTestCase { @Test public void testRun_noParameters() throws Exception { - action.tld = "foo"; + action.tlds = ImmutableSet.of("foo"); testRunSuccess( action, null, @@ -75,7 +76,7 @@ public class ListDomainsActionTest extends ListActionTestCase { @Test public void testRun_twoLinesWithIdOnly() throws Exception { - action.tld = "foo"; + action.tlds = ImmutableSet.of("foo"); createTlds("bar", "sim"); persistActiveDomain("dontlist.bar"); persistActiveDomain("example1.foo"); @@ -91,9 +92,28 @@ public class ListDomainsActionTest extends ListActionTestCase { "^example2.foo$"); } + @Test + public void testRun_multipleTlds() throws Exception { + action.tlds = ImmutableSet.of("bar", "foo"); + createTlds("bar", "sim"); + persistActiveDomain("dolist.bar"); + persistActiveDomain("example1.foo"); + persistActiveDomain("example2.foo"); + persistActiveDomain("notlistedaswell.sim"); + testRunSuccess( + action, + null, + null, + null, + "^dolist.bar", + "^example1.foo$", + "^example2.foo$"); + } + + @Test public void testRun_twoLinesWithIdOnlyNoHeader() throws Exception { - action.tld = "foo"; + action.tlds = ImmutableSet.of("foo"); persistActiveDomain("example1.foo"); persistActiveDomain("example2.foo"); testRunSuccess( @@ -107,7 +127,7 @@ public class ListDomainsActionTest extends ListActionTestCase { @Test public void testRun_twoLinesWithIdOnlyExplicitHeader() throws Exception { - action.tld = "foo"; + action.tlds = ImmutableSet.of("foo"); persistActiveDomain("example1.foo"); persistActiveDomain("example2.foo"); testRunSuccess( @@ -123,7 +143,7 @@ public class ListDomainsActionTest extends ListActionTestCase { @Test public void testRun_twoLinesWithRepoId() throws Exception { - action.tld = "foo"; + action.tlds = ImmutableSet.of("foo"); persistActiveDomain("example1.foo"); persistActiveDomain("example3.foo"); testRunSuccess( @@ -139,7 +159,7 @@ public class ListDomainsActionTest extends ListActionTestCase { @Test public void testRun_twoLinesWithRepoIdNoHeader() throws Exception { - action.tld = "foo"; + action.tlds = ImmutableSet.of("foo"); persistActiveDomain("example1.foo"); persistActiveDomain("example3.foo"); testRunSuccess( @@ -153,7 +173,7 @@ public class ListDomainsActionTest extends ListActionTestCase { @Test public void testRun_twoLinesWithRepoIdExplicitHeader() throws Exception { - action.tld = "foo"; + action.tlds = ImmutableSet.of("foo"); persistActiveDomain("example1.foo"); persistActiveDomain("example3.foo"); testRunSuccess( @@ -169,7 +189,7 @@ public class ListDomainsActionTest extends ListActionTestCase { @Test public void testRun_twoLinesWithWildcard() throws Exception { - action.tld = "foo"; + action.tlds = ImmutableSet.of("foo"); persistActiveDomain("example1.foo"); persistActiveDomain("example3.foo"); testRunSuccess( @@ -185,7 +205,7 @@ public class ListDomainsActionTest extends ListActionTestCase { @Test public void testRun_twoLinesWithWildcardAndAnotherField() throws Exception { - action.tld = "foo"; + action.tlds = ImmutableSet.of("foo"); persistActiveDomain("example1.foo"); persistActiveDomain("example3.foo"); testRunSuccess( @@ -201,7 +221,7 @@ public class ListDomainsActionTest extends ListActionTestCase { @Test public void testRun_withBadField_returnsError() throws Exception { - action.tld = "foo"; + action.tlds = ImmutableSet.of("foo"); persistActiveDomain("example2.foo"); persistActiveDomain("example1.foo"); testRunError(