From 590e69cb9fa04714378c295094247412572de6c4 Mon Sep 17 00:00:00 2001 From: Juan Celhay Date: Tue, 17 Sep 2024 12:02:25 -0400 Subject: [PATCH] Add activeOrDeletedSince parameter to RefreshDnsForAllDomainsAction (#2556) --- .../server/RefreshDnsForAllDomainsAction.java | 19 ++++-- .../tools/server/ToolsServerModule.java | 8 +++ .../RefreshDnsForAllDomainsActionTest.java | 36 ++++++++++- .../tools/server/ToolsServerModuleTest.java | 59 +++++++++++++++++++ 4 files changed, 115 insertions(+), 7 deletions(-) create mode 100644 core/src/test/java/google/registry/tools/server/ToolsServerModuleTest.java diff --git a/core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java b/core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java index 7c91d36c7f0..43b46138a16 100644 --- a/core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java +++ b/core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java @@ -40,6 +40,7 @@ import javax.inject.Inject; import org.apache.arrow.util.VisibleForTesting; import org.apache.http.HttpStatus; +import org.joda.time.DateTime; import org.joda.time.Duration; /** @@ -51,6 +52,10 @@ * *

You may pass in a {@code batchSize} for the batched read of domains from the database. This is * recommended to be somewhere between 200 and 500. The default value is 250. + * + *

If {@code activeOrDeletedSince} is passed in the request, this action will enqueue DNS publish + * tasks on all domains with a deletion time equal or greater than the value provided, including + * domains that have since been deleted. */ @Action( service = GaeService.TOOLS, @@ -81,17 +86,21 @@ public class RefreshDnsForAllDomainsAction implements Runnable { private final Random random; + private final DateTime activeOrDeletedSince; + @Inject RefreshDnsForAllDomainsAction( Response response, @Parameter(PARAM_TLDS) ImmutableSet tlds, @Parameter(PARAM_BATCH_SIZE) Optional batchSize, @Parameter("refreshQps") Optional refreshQps, + @Parameter("activeOrDeletedSince") Optional activeOrDeletedSince, Random random) { this.response = response; this.tlds = tlds; this.batchSize = batchSize.orElse(DEFAULT_BATCH_SIZE); this.refreshQps = refreshQps.orElse(DEFAULT_REFRESH_QPS); + this.activeOrDeletedSince = activeOrDeletedSince.orElse(END_OF_TIME); this.random = random; } @@ -119,11 +128,11 @@ public void run() { private Duration calculateSmear() { Long activeDomains = tm().query( - "SELECT COUNT(*) FROM Domain WHERE tld IN (:tlds) AND deletionTime =" - + " :endOfTime", + "SELECT COUNT(*) FROM Domain WHERE tld IN (:tlds) AND deletionTime >=" + + " :activeOrDeletedSince", Long.class) .setParameter("tlds", tlds) - .setParameter("endOfTime", END_OF_TIME) + .setParameter("activeOrDeletedSince", activeOrDeletedSince) .getSingleResult(); return Duration.standardSeconds(Math.max(activeDomains / refreshQps, 1)); } @@ -132,12 +141,12 @@ private ImmutableList getBatch(Optional lastInPreviousBatch) { String sql = String.format( "SELECT domainName FROM Domain WHERE tld IN (:tlds) AND" - + " deletionTime = :endOfTime %s ORDER BY domainName ASC", + + " deletionTime >= :activeOrDeletedSince %s ORDER BY domainName ASC", lastInPreviousBatch.isPresent() ? "AND domainName > :lastInPreviousBatch" : ""); TypedQuery query = tm().query(sql, String.class) .setParameter("tlds", tlds) - .setParameter("endOfTime", END_OF_TIME); + .setParameter("activeOrDeletedSince", activeOrDeletedSince); lastInPreviousBatch.ifPresent(l -> query.setParameter("lastInPreviousBatch", l)); return query.setMaxResults(batchSize).getResultStream().collect(toImmutableList()); } diff --git a/core/src/main/java/google/registry/tools/server/ToolsServerModule.java b/core/src/main/java/google/registry/tools/server/ToolsServerModule.java index c7979b84875..36665215641 100644 --- a/core/src/main/java/google/registry/tools/server/ToolsServerModule.java +++ b/core/src/main/java/google/registry/tools/server/ToolsServerModule.java @@ -16,6 +16,7 @@ import static com.google.common.base.Strings.emptyToNull; import static google.registry.request.RequestParameters.extractIntParameter; +import static google.registry.request.RequestParameters.extractOptionalDatetimeParameter; import static google.registry.request.RequestParameters.extractOptionalIntParameter; import static google.registry.request.RequestParameters.extractOptionalParameter; import static google.registry.request.RequestParameters.extractRequiredParameter; @@ -26,6 +27,7 @@ import google.registry.tools.server.UpdateUserGroupAction.Mode; import jakarta.servlet.http.HttpServletRequest; import java.util.Optional; +import org.joda.time.DateTime; /** Dagger module for the tools package. */ @Module @@ -75,6 +77,12 @@ static Optional provideRefreshQps(HttpServletRequest req) { return extractOptionalIntParameter(req, "refreshQps"); } + @Provides + @Parameter("activeOrDeletedSince") + static Optional provideDeletionTime(HttpServletRequest req) { + return extractOptionalDatetimeParameter(req, "activeOrDeletedSince"); + } + @Provides static Mode provideGroupUpdateMode(HttpServletRequest req) { return Mode.valueOf(extractRequiredParameter(req, "groupUpdateMode")); diff --git a/core/src/test/java/google/registry/tools/server/RefreshDnsForAllDomainsActionTest.java b/core/src/test/java/google/registry/tools/server/RefreshDnsForAllDomainsActionTest.java index e1c64389395..07a5800303e 100644 --- a/core/src/test/java/google/registry/tools/server/RefreshDnsForAllDomainsActionTest.java +++ b/core/src/test/java/google/registry/tools/server/RefreshDnsForAllDomainsActionTest.java @@ -56,7 +56,12 @@ void beforeEach() { createTld("bar"); action = new RefreshDnsForAllDomainsAction( - response, ImmutableSet.of("bar"), Optional.of(10), Optional.empty(), new Random()); + response, + ImmutableSet.of("bar"), + Optional.of(10), + Optional.empty(), + Optional.empty(), + new Random()); } @Test @@ -75,7 +80,12 @@ void test_runAction_smearsOutDnsRefreshes() throws Exception { // Set batch size to 1 since each batch will be enqueud at the same time action = new RefreshDnsForAllDomainsAction( - response, ImmutableSet.of("bar"), Optional.of(1), Optional.of(7), new Random()); + response, + ImmutableSet.of("bar"), + Optional.of(1), + Optional.of(7), + Optional.empty(), + new Random()); tm().transact(() -> action.refreshBatch(Optional.empty(), Duration.standardMinutes(1000))); tm().transact(() -> action.refreshBatch(Optional.empty(), Duration.standardMinutes(1000))); ImmutableList refreshRequests = @@ -98,6 +108,28 @@ void test_runAction_doesntRefreshDeletedDomain() throws Exception { assertNoDnsRequestsExcept("foo.bar"); } + @Test + void test_runAction_refreshesDeletedDomain_whenActiveOrDeletedSinceIsProvided() throws Exception { + action = + new RefreshDnsForAllDomainsAction( + response, + ImmutableSet.of("bar"), + Optional.of(1), + Optional.of(7), + Optional.of(clock.nowUtc().minusYears(3)), + new Random()); + persistActiveDomain("foo.bar"); + persistDeletedDomain("deleted1.bar", clock.nowUtc().minusYears(1)); + persistDeletedDomain("deleted3.bar", clock.nowUtc().minusYears(3)); + persistDeletedDomain("deleted5.bar", clock.nowUtc().minusYears(5)); + action.run(); + assertDomainDnsRequestWithRequestTime("foo.bar", clock.nowUtc()); + assertDomainDnsRequestWithRequestTime("deleted1.bar", clock.nowUtc()); + assertDomainDnsRequestWithRequestTime("deleted3.bar", clock.nowUtc()); + + assertNoDnsRequestsExcept("foo.bar", "deleted1.bar", "deleted3.bar"); + } + @Test void test_runAction_ignoresDomainsOnOtherTlds() throws Exception { createTld("baz"); diff --git a/core/src/test/java/google/registry/tools/server/ToolsServerModuleTest.java b/core/src/test/java/google/registry/tools/server/ToolsServerModuleTest.java new file mode 100644 index 00000000000..a87485ee30c --- /dev/null +++ b/core/src/test/java/google/registry/tools/server/ToolsServerModuleTest.java @@ -0,0 +1,59 @@ +// Copyright 2017 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.tools.server; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import google.registry.request.HttpException.BadRequestException; +import jakarta.servlet.http.HttpServletRequest; +import java.util.Optional; +import org.joda.time.DateTime; +import org.junit.jupiter.api.Test; + +/** Unit tests for {@link ToolsServerModule}. */ +public class ToolsServerModuleTest { + + private final HttpServletRequest request = mock(HttpServletRequest.class); + + @Test + void test_provideDeletionTime() throws Exception { + when(request.getParameter("activeOrDeletedSince")).thenReturn("1991-07-01T00:00:00Z"); + + DateTime expected = DateTime.parse("1991-07-01T00:00:00Z"); + Optional dateTimeParam = ToolsServerModule.provideDeletionTime(request); + + assertThat(dateTimeParam).isEqualTo(Optional.of(expected)); + } + + @Test + void test_doesNotprovideDeletionTimeOnEmptyParam() throws Exception { + when(request.getParameter("activeOrDeletedSince")).thenReturn(""); + + assertThat(ToolsServerModule.provideDeletionTime(request)).isEqualTo(Optional.empty()); + } + + @Test + void test_provideDeletionTime_incorrectDateFormat_throwsBadRequestException() throws Exception { + when(request.getParameter("activeOrDeletedSince")).thenReturn("error404?"); + + BadRequestException thrown = + assertThrows( + BadRequestException.class, () -> ToolsServerModule.provideDeletionTime(request)); + assertThat(thrown).hasMessageThat().contains("Bad ISO 8601 timestamp: activeOrDeletedSince"); + } +}