diff --git a/java/google/registry/env/common/default/WEB-INF/datastore-indexes.xml b/java/google/registry/env/common/default/WEB-INF/datastore-indexes.xml index abb5d595a..26f3416dc 100644 --- a/java/google/registry/env/common/default/WEB-INF/datastore-indexes.xml +++ b/java/google/registry/env/common/default/WEB-INF/datastore-indexes.xml @@ -1,99 +1,105 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/java/google/registry/model/EppResource.java b/java/google/registry/model/EppResource.java index b993b876d..e83ce96e9 100644 --- a/java/google/registry/model/EppResource.java +++ b/java/google/registry/model/EppResource.java @@ -83,6 +83,7 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable { // Map the method to XML, not the field, because if we map the field (with an adaptor class) it // will never be omitted from the xml even if the timestamp inside creationTime is null and we // return null from the adaptor. (Instead it gets written as an empty tag.) + @Index CreateAutoTimestamp creationTime = CreateAutoTimestamp.create(null); /** diff --git a/java/google/registry/tools/ListDomainsCommand.java b/java/google/registry/tools/ListDomainsCommand.java index dcfab9998..b5be4c709 100644 --- a/java/google/registry/tools/ListDomainsCommand.java +++ b/java/google/registry/tools/ListDomainsCommand.java @@ -33,17 +33,22 @@ final class ListDomainsCommand extends ListObjectsCommand { required = true) private List tlds; + @Parameter( + names = {"-n", "--limit"}, + description = "Max number of domains to list, most recent first; defaults to no limit." + ) + private int maxDomains = Integer.MAX_VALUE; + @Override String getCommandPath() { return ListDomainsAction.PATH; } - /** Returns a map of parameters to be sent to the server - * (in addition to the usual ones). */ + /** Returns a map of parameters to be sent to the server (in addition to the usual ones). */ @Override ImmutableMap getParameterMap() { 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); + return ImmutableMap.of("tlds", tldsParam, "limit", maxDomains); } } diff --git a/java/google/registry/tools/ListObjectsCommand.java b/java/google/registry/tools/ListObjectsCommand.java index 1199ee7c1..bd96cb75d 100644 --- a/java/google/registry/tools/ListObjectsCommand.java +++ b/java/google/registry/tools/ListObjectsCommand.java @@ -65,11 +65,9 @@ abstract class ListObjectsCommand implements RemoteApiCommand, ServerSideCommand /** Returns the path to the servlet task. */ abstract String getCommandPath(); - /** Returns a map of parameters to be sent to the server - * (in addition to the usual ones). */ - @Nullable + /** Returns a map of parameters to be sent to the server (in addition to the usual ones). */ ImmutableMap getParameterMap() { - return null; + return ImmutableMap.of(); } @Override @@ -84,10 +82,7 @@ abstract class ListObjectsCommand implements RemoteApiCommand, ServerSideCommand if (fullFieldNames) { params.put(FULL_FIELD_NAMES_PARAM, Boolean.TRUE); } - ImmutableMap extraParams = getParameterMap(); - if (extraParams != null) { - params.putAll(extraParams); - } + params.putAll(getParameterMap()); // Call the server and get the response data. String response = connection.send( getCommandPath(), diff --git a/java/google/registry/tools/server/ListDomainsAction.java b/java/google/registry/tools/server/ListDomainsAction.java index cac5f1222..bd14fb1b0 100644 --- a/java/google/registry/tools/server/ListDomainsAction.java +++ b/java/google/registry/tools/server/ListDomainsAction.java @@ -15,22 +15,23 @@ package google.registry.tools.server; import static com.google.common.base.Preconditions.checkArgument; -import static google.registry.model.EppResourceUtils.queryNotDeleted; +import static com.google.common.collect.ImmutableSet.toImmutableSet; +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 java.util.Comparator.comparing; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSortedSet; -import com.google.common.collect.Lists; +import google.registry.model.EppResource; +import google.registry.model.EppResourceUtils; import google.registry.model.domain.DomainResource; import google.registry.request.Action; import google.registry.request.Parameter; import google.registry.request.auth.Auth; import google.registry.util.Clock; -import java.util.List; import javax.inject.Inject; +import org.joda.time.DateTime; /** An action that lists domains, for use by the {@code nomulus list_domains} command. */ @Action( @@ -45,6 +46,7 @@ public final class ListDomainsAction extends ListObjectsAction { public static final String PATH = "/_dr/admin/list/domains"; @Inject @Parameter("tlds") ImmutableSet tlds; + @Inject @Parameter("limit") int limit; @Inject Clock clock; @Inject ListDomainsAction() {} @@ -56,12 +58,27 @@ public final class ListDomainsAction extends ListObjectsAction { @Override public ImmutableSet loadObjects() { checkArgument(!tlds.isEmpty(), "Must specify TLDs to query"); + checkArgument( + tlds.size() <= MAX_NUM_SUBQUERIES, + "Cannot query more than %s TLDs simultaneously", + MAX_NUM_SUBQUERIES); assertTldsExist(tlds); - ImmutableSortedSet.Builder builder = - new ImmutableSortedSet.Builder<>(comparing(DomainResource::getFullyQualifiedDomainName)); - for (List batch : Lists.partition(tlds.asList(), MAX_NUM_SUBQUERIES)) { - builder.addAll(queryNotDeleted(DomainResource.class, clock.nowUtc(), "tld in", batch)); - } - return builder.build(); + DateTime now = clock.nowUtc(); + return ofy() + .load() + .type(DomainResource.class) + .filter("tld in", tlds) + // Get the N most recently created domains (requires ordering in descending order). + .order("-creationTime") + .limit(limit) + .list() + .stream() + .map(EppResourceUtils.transformAtTime(now)) + // Deleted entities must be filtered out post-query because queries don't allow ordering + // with two filters. + .filter(d -> d.getDeletionTime().isAfter(now)) + // Sort back to ascending order for nicer display. + .sorted(comparing(EppResource::getCreationTime)) + .collect(toImmutableSet()); } } diff --git a/java/google/registry/tools/server/ListObjectsAction.java b/java/google/registry/tools/server/ListObjectsAction.java index 2ef6f68f6..01252a691 100644 --- a/java/google/registry/tools/server/ListObjectsAction.java +++ b/java/google/registry/tools/server/ListObjectsAction.java @@ -107,6 +107,7 @@ public abstract class ListObjectsAction implements Ru // Get the object data first, so we can figure out the list of all available fields using the // data if necessary. ImmutableSet objects = loadObjects(); + logger.infofmt("Loaded %d objects.", objects.size()); // Get the list of fields we should return. ImmutableSet fieldsToUse = getFieldsToUse(objects); // Convert the data into a table. diff --git a/java/google/registry/tools/server/ToolsServerModule.java b/java/google/registry/tools/server/ToolsServerModule.java index 95f87e55b..9809d8af5 100644 --- a/java/google/registry/tools/server/ToolsServerModule.java +++ b/java/google/registry/tools/server/ToolsServerModule.java @@ -16,6 +16,7 @@ package google.registry.tools.server; import static com.google.common.base.Strings.emptyToNull; 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; @@ -90,6 +91,12 @@ public class ToolsServerModule { return ImmutableSet.copyOf(Splitter.on(',').split(tldsString)); } + @Provides + @Parameter("limit") + static int provideLimit(HttpServletRequest req) { + return extractIntParameter(req, "limit"); + } + @Provides @Parameter("rawKeys") static String provideRawKeys(HttpServletRequest req) { diff --git a/javatests/google/registry/testing/DatastoreHelper.java b/javatests/google/registry/testing/DatastoreHelper.java index 8f79ef987..c836097ed 100644 --- a/javatests/google/registry/testing/DatastoreHelper.java +++ b/javatests/google/registry/testing/DatastoreHelper.java @@ -311,6 +311,11 @@ public class DatastoreHelper { return persistResource(newDomainResource(domainName)); } + public static DomainResource persistActiveDomain(String domainName, DateTime creationTime) { + return persistResource( + newDomainResource(domainName).asBuilder().setCreationTimeForTest(creationTime).build()); + } + public static DomainApplication persistActiveDomainApplication(String domainName) { return persistResource(newDomainApplication(domainName)); } diff --git a/javatests/google/registry/tools/ListDomainsCommandTest.java b/javatests/google/registry/tools/ListDomainsCommandTest.java index 53c0bd617..af2da2d51 100644 --- a/javatests/google/registry/tools/ListDomainsCommandTest.java +++ b/javatests/google/registry/tools/ListDomainsCommandTest.java @@ -16,11 +16,13 @@ package google.registry.tools; import static com.google.common.truth.Truth.assertThat; import static google.registry.testing.JUnitBackports.assertThrows; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.verify; import com.google.common.base.Strings; -import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.net.MediaType; import google.registry.tools.server.ListDomainsAction; -import java.util.List; import org.junit.Test; /** @@ -36,17 +38,28 @@ public class ListDomainsCommandTest extends ListObjectsCommandTestCase getTlds() { - return ImmutableList.of("foo"); + protected ImmutableMap getOtherParameters() { + return ImmutableMap.of("tlds", "foo", "limit", Integer.MAX_VALUE); } @Test - public void test_tldsParamTooLong() throws Exception { - String tldsParam = "--tld=foo,bar" + Strings.repeat(",baz", 300); + public void test_tldsParamTooLong() { + String tldsParam = "--tlds=foo,bar" + Strings.repeat(",baz", 300); IllegalArgumentException thrown = assertThrows(IllegalArgumentException.class, () -> runCommand(tldsParam)); assertThat(thrown) .hasMessageThat() .contains("Total length of TLDs is too long for URL parameter"); } + + @Test + public void test_bothParamsSpecified() throws Exception { + runCommand("--tlds=foo,bar", "--limit=100"); + verify(connection) + .send( + eq(getTaskPath()), + eq(ImmutableMap.of("tlds", "foo,bar", "limit", 100)), + eq(MediaType.PLAIN_TEXT_UTF_8), + eq(new byte[0])); + } } diff --git a/javatests/google/registry/tools/ListObjectsCommandTestCase.java b/javatests/google/registry/tools/ListObjectsCommandTestCase.java index 6b662d23d..3f31fe9f4 100644 --- a/javatests/google/registry/tools/ListObjectsCommandTestCase.java +++ b/javatests/google/registry/tools/ListObjectsCommandTestCase.java @@ -14,6 +14,7 @@ package google.registry.tools; +import static com.google.common.collect.ImmutableList.toImmutableList; import static google.registry.request.JsonResponse.JSON_SAFETY_PREFIX; import static google.registry.tools.server.ListObjectsAction.FIELDS_PARAM; import static google.registry.tools.server.ListObjectsAction.FULL_FIELD_NAMES_PARAM; @@ -24,14 +25,11 @@ import static org.mockito.Matchers.eq; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.net.MediaType; import google.registry.tools.ServerSideCommand.Connection; -import java.util.List; import java.util.Optional; -import javax.annotation.Nullable; import org.junit.Before; import org.junit.Test; import org.mockito.Mock; @@ -40,32 +38,31 @@ import org.mockito.Mock; public abstract class ListObjectsCommandTestCase extends CommandTestCase { - @Mock - Connection connection; + @Mock Connection connection; - /** - * Where to find the servlet task; set by the subclass. - */ + /** Where to find the servlet task; set by the subclass. */ abstract String getTaskPath(); - /** - * The TLD to be used (for those subclasses that use TLDs; defaults to empty). - */ - protected List getTlds() { - return ImmutableList.of(); + /** The other parameters to be used (for those subclasses that use them; defaults to empty). */ + protected ImmutableMap getOtherParameters() { + return ImmutableMap.of(); } - /** - * The TLDs argument to be passed on the command line; null if not needed. - */ - @Nullable String tldsParameter; + ImmutableList otherParams = ImmutableList.of(); @Before public void init() throws Exception { - tldsParameter = getTlds().isEmpty() ? null : ("--tld=" + Joiner.on(',').join(getTlds())); + ImmutableMap otherParameters = getOtherParameters(); + if (!otherParameters.isEmpty()) { + otherParams = + otherParameters + .entrySet() + .stream() + .map(entry -> String.format("--%s=%s", entry.getKey(), entry.getValue())) + .collect(toImmutableList()); + } command.setConnection(connection); - when( - connection.send( + when(connection.send( eq(getTaskPath()), anyMapOf(String.class, Object.class), eq(MediaType.PLAIN_TEXT_UTF_8), @@ -74,9 +71,8 @@ public abstract class ListObjectsCommandTestCase } private void verifySent( - String fields, - Optional printHeaderRow, - Optional fullFieldNames) throws Exception { + String fields, Optional printHeaderRow, Optional fullFieldNames) + throws Exception { ImmutableMap.Builder params = new ImmutableMap.Builder<>(); if (fields != null) { @@ -84,88 +80,68 @@ public abstract class ListObjectsCommandTestCase } printHeaderRow.ifPresent(aBoolean -> params.put(PRINT_HEADER_ROW_PARAM, aBoolean)); fullFieldNames.ifPresent(aBoolean -> params.put(FULL_FIELD_NAMES_PARAM, aBoolean)); - if (!getTlds().isEmpty()) { - params.put("tlds", Joiner.on(',').join(getTlds())); - } - verify(connection).send( - eq(getTaskPath()), - eq(params.build()), - eq(MediaType.PLAIN_TEXT_UTF_8), - eq(new byte[0])); + params.putAll(getOtherParameters()); + verify(connection) + .send( + eq(getTaskPath()), eq(params.build()), eq(MediaType.PLAIN_TEXT_UTF_8), eq(new byte[0])); } @Test public void testRun_noFields() throws Exception { - if (tldsParameter == null) { - runCommand(); - } else { - runCommand(tldsParameter); - } + runCommand(otherParams); verifySent(null, Optional.empty(), Optional.empty()); } @Test public void testRun_oneField() throws Exception { - if (tldsParameter == null) { - runCommand("--fields=fieldName"); - } else { - runCommand("--fields=fieldName", tldsParameter); - } + runCommand( + new ImmutableList.Builder().addAll(otherParams).add("--fields=fieldName").build()); verifySent("fieldName", Optional.empty(), Optional.empty()); } @Test public void testRun_wildcardField() throws Exception { - if (tldsParameter == null) { - runCommand("--fields=*"); - } else { - runCommand("--fields=*", tldsParameter); - } + runCommand(new ImmutableList.Builder().addAll(otherParams).add("--fields=*").build()); verifySent("*", Optional.empty(), Optional.empty()); } @Test public void testRun_header() throws Exception { - if (tldsParameter == null) { - runCommand("--fields=fieldName", "--header=true"); - } else { - runCommand("--fields=fieldName", "--header=true", tldsParameter); - } + runCommand( + new ImmutableList.Builder() + .addAll(otherParams) + .add("--fields=fieldName", "--header=true") + .build()); verifySent("fieldName", Optional.of(Boolean.TRUE), Optional.empty()); } @Test public void testRun_noHeader() throws Exception { - if (tldsParameter == null) { - runCommand("--fields=fieldName", "--header=false"); - } else { - runCommand("--fields=fieldName", "--header=false", tldsParameter); - } + runCommand( + new ImmutableList.Builder() + .addAll(otherParams) + .add("--fields=fieldName", "--header=false") + .build()); verifySent("fieldName", Optional.of(Boolean.FALSE), Optional.empty()); } @Test public void testRun_fullFieldNames() throws Exception { - if (tldsParameter == null) { - runCommand("--fields=fieldName", "--full_field_names"); - } else { - runCommand("--fields=fieldName", "--full_field_names", tldsParameter); - } + runCommand( + new ImmutableList.Builder() + .addAll(otherParams) + .add("--fields=fieldName", "--full_field_names") + .build()); verifySent("fieldName", Optional.empty(), Optional.of(Boolean.TRUE)); } @Test public void testRun_allParameters() throws Exception { - if (tldsParameter == null) { - runCommand("--fields=fieldName,otherFieldName,*", "--header=true", "--full_field_names"); - } else { - runCommand( - "--fields=fieldName,otherFieldName,*", - "--header=true", - "--full_field_names", - tldsParameter); - } - verifySent( - "fieldName,otherFieldName,*", Optional.of(Boolean.TRUE), Optional.of(Boolean.TRUE)); + runCommand( + new ImmutableList.Builder() + .addAll(otherParams) + .add("--fields=fieldName,otherFieldName,*", "--header=true", "--full_field_names") + .build()); + verifySent("fieldName,otherFieldName,*", Optional.of(Boolean.TRUE), Optional.of(Boolean.TRUE)); } } diff --git a/javatests/google/registry/tools/server/ListDomainsActionTest.java b/javatests/google/registry/tools/server/ListDomainsActionTest.java index 5dfd28f95..358598265 100644 --- a/javatests/google/registry/tools/server/ListDomainsActionTest.java +++ b/javatests/google/registry/tools/server/ListDomainsActionTest.java @@ -39,7 +39,8 @@ public class ListDomainsActionTest extends ListActionTestCase { public void init() throws Exception { createTld("foo"); action = new ListDomainsAction(); - action.clock = new FakeClock(DateTime.parse("2000-01-01TZ")); + action.clock = new FakeClock(DateTime.parse("2018-01-01TZ")); + action.limit = Integer.MAX_VALUE; } @Test @@ -110,7 +111,6 @@ public class ListDomainsActionTest extends ListActionTestCase { "^example2.foo$"); } - @Test public void testRun_twoLinesWithIdOnlyNoHeader() throws Exception { action.tlds = ImmutableSet.of("foo"); @@ -231,4 +231,23 @@ public class ListDomainsActionTest extends ListActionTestCase { null, "^Field 'badfield' not found - recognized fields are:"); } + + @Test + public void testRun_limitFiltersOutOldestDomains() { + createTlds("bar", "baz"); + action.tlds = ImmutableSet.of("foo", "bar"); + action.limit = 2; + persistActiveDomain("example4.foo", DateTime.parse("2017-04-01TZ")); + persistActiveDomain("example1.foo", DateTime.parse("2017-01-01TZ")); + persistActiveDomain("example2.bar", DateTime.parse("2017-02-01TZ")); + persistActiveDomain("example3.bar", DateTime.parse("2017-03-01TZ")); + persistActiveDomain("example5.baz", DateTime.parse("2018-01-01TZ")); + testRunSuccess( + action, + null, + null, + null, + "^example3.bar$", + "^example4.foo$"); + } }