From 26efe6721112b2a0ec9c5964bc1681985b67e2d3 Mon Sep 17 00:00:00 2001 From: Lai Jiang Date: Thu, 4 May 2023 13:21:53 -0400 Subject: [PATCH] Remove DNS pull queue (#2000) This is the last dependency on GAE pull queue, therefore we can delete the pull queue config from queue.xml as well. --- .../batch/DeleteProberDataAction.java | 8 +- .../google/registry/dns/DnsConstants.java | 41 -- .../java/google/registry/dns/DnsModule.java | 19 +- .../java/google/registry/dns/DnsQueue.java | 170 ------ .../java/google/registry/dns/DnsUtils.java | 54 +- .../registry/dns/PublishDnsUpdatesAction.java | 25 +- .../registry/dns/ReadDnsQueueAction.java | 401 -------------- .../dns/ReadDnsRefreshRequestsAction.java | 13 +- .../google/registry/dns/RefreshDnsAction.java | 13 +- .../dns/RefreshDnsOnHostRenameAction.java | 8 +- .../default/WEB-INF/cloud-scheduler-tasks.xml | 10 - .../env/common/backend/WEB-INF/web.xml | 6 - .../env/common/default/WEB-INF/queue.xml | 5 - .../default/WEB-INF/cloud-scheduler-tasks.xml | 10 - .../default/WEB-INF/cloud-scheduler-tasks.xml | 10 - .../default/WEB-INF/cloud-scheduler-tasks.xml | 10 - .../default/WEB-INF/cloud-scheduler-tasks.xml | 10 - .../flows/domain/DomainCreateFlow.java | 5 +- .../flows/domain/DomainDeleteFlow.java | 7 +- .../domain/DomainRestoreRequestFlow.java | 9 +- .../flows/domain/DomainUpdateFlow.java | 19 +- .../registry/flows/host/HostCreateFlow.java | 5 +- .../registry/flows/host/HostDeleteFlow.java | 5 +- .../registry/flows/host/HostUpdateFlow.java | 17 +- .../model/common/DnsRefreshRequest.java | 2 +- .../backend/BackendRequestComponent.java | 3 - .../server/RefreshDnsForAllDomainsAction.java | 13 +- .../batch/DeleteExpiredDomainsActionTest.java | 3 - .../batch/DeleteProberDataActionTest.java | 12 +- .../google/registry/dns/DnsInjectionTest.java | 31 +- .../google/registry/dns/DnsQueueTest.java | 108 ---- .../google/registry/dns/DnsTestComponent.java | 4 +- .../google/registry/dns/DnsUtilsTest.java | 105 +--- .../dns/PublishDnsUpdatesActionTest.java | 48 +- .../registry/dns/ReadDnsQueueActionTest.java | 500 ------------------ .../dns/ReadDnsRefreshRequestsActionTest.java | 39 +- .../registry/dns/RefreshDnsActionTest.java | 36 +- .../dns/RefreshDnsOnHostRenameActionTest.java | 14 +- .../flows/EppLifecycleDomainTest.java | 3 - .../registry/flows/EppLifecycleHostTest.java | 3 - .../registry/flows/EppPointInTimeTest.java | 3 - .../registry/flows/EppTestComponent.java | 21 - .../google/registry/flows/FlowTestCase.java | 3 - .../registry/flows/ResourceFlowTestCase.java | 26 +- .../flows/domain/DomainCreateFlowTest.java | 28 +- .../flows/domain/DomainDeleteFlowTest.java | 7 +- .../domain/DomainRestoreRequestFlowTest.java | 5 +- .../flows/domain/DomainUpdateFlowTest.java | 58 +- .../flows/host/HostCreateFlowTest.java | 10 +- .../flows/host/HostDeleteFlowTest.java | 6 +- .../flows/host/HostUpdateFlowTest.java | 22 +- .../model/common/DnsRefreshRequestTest.java | 14 +- .../registry/server/RegistryTestServer.java | 3 - .../registry/testing/CloudTasksHelper.java | 14 +- .../registry/testing/DatabaseHelper.java | 50 +- .../registry/testing/DnsUtilsHelper.java | 73 --- .../registry/testing/TaskQueueExtension.java | 61 --- .../registry/testing/TaskQueueHelper.java | 374 ------------- .../RefreshDnsForAllDomainsActionTest.java | 52 +- .../module/backend/backend_routing.txt | 1 - 60 files changed, 324 insertions(+), 2311 deletions(-) delete mode 100644 core/src/main/java/google/registry/dns/DnsConstants.java delete mode 100644 core/src/main/java/google/registry/dns/DnsQueue.java delete mode 100644 core/src/main/java/google/registry/dns/ReadDnsQueueAction.java delete mode 100644 core/src/test/java/google/registry/dns/DnsQueueTest.java delete mode 100644 core/src/test/java/google/registry/dns/ReadDnsQueueActionTest.java delete mode 100644 core/src/test/java/google/registry/testing/DnsUtilsHelper.java delete mode 100644 core/src/test/java/google/registry/testing/TaskQueueExtension.java delete mode 100644 core/src/test/java/google/registry/testing/TaskQueueHelper.java diff --git a/core/src/main/java/google/registry/batch/DeleteProberDataAction.java b/core/src/main/java/google/registry/batch/DeleteProberDataAction.java index f9236a448..07804924f 100644 --- a/core/src/main/java/google/registry/batch/DeleteProberDataAction.java +++ b/core/src/main/java/google/registry/batch/DeleteProberDataAction.java @@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.batch.BatchModule.PARAM_DRY_RUN; import static google.registry.config.RegistryEnvironment.PRODUCTION; +import static google.registry.dns.DnsUtils.requestDomainDnsRefresh; import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_DELETE; import static google.registry.model.tld.Registries.getTldsOfType; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; @@ -32,7 +33,6 @@ import com.google.common.collect.Sets; import com.google.common.flogger.FluentLogger; import google.registry.config.RegistryConfig.Config; import google.registry.config.RegistryEnvironment; -import google.registry.dns.DnsUtils; import google.registry.model.CreateAutoTimestamp; import google.registry.model.EppResourceUtils; import google.registry.model.domain.Domain; @@ -98,8 +98,6 @@ public class DeleteProberDataAction implements Runnable { /** Number of domains to retrieve and delete per SQL transaction. */ private static final int BATCH_SIZE = 1000; - @Inject DnsUtils dnsUtils; - @Inject @Parameter(PARAM_DRY_RUN) boolean isDryRun; @@ -222,7 +220,7 @@ public class DeleteProberDataAction implements Runnable { } } - private void hardDeleteDomainsAndHosts( + private static void hardDeleteDomainsAndHosts( ImmutableList domainRepoIds, ImmutableList hostNames) { tm().query("DELETE FROM Host WHERE hostName IN :hostNames") .setParameter("hostNames", hostNames) @@ -264,6 +262,6 @@ public class DeleteProberDataAction implements Runnable { // messages, or auto-renews because those will all be hard-deleted the next time the job runs // anyway. tm().putAll(ImmutableList.of(deletedDomain, historyEntry)); - dnsUtils.requestDomainDnsRefresh(deletedDomain.getDomainName()); + requestDomainDnsRefresh(deletedDomain.getDomainName()); } } diff --git a/core/src/main/java/google/registry/dns/DnsConstants.java b/core/src/main/java/google/registry/dns/DnsConstants.java deleted file mode 100644 index a08e7e9bb..000000000 --- a/core/src/main/java/google/registry/dns/DnsConstants.java +++ /dev/null @@ -1,41 +0,0 @@ -// 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.dns; - -/** Static class for DNS-related constants. */ -public class DnsConstants { - private DnsConstants() {} - - /** The name of the DNS pull queue. */ - public static final String DNS_PULL_QUEUE_NAME = "dns-pull"; // See queue.xml. - - /** The name of the DNS publish push queue. */ - public static final String DNS_PUBLISH_PUSH_QUEUE_NAME = "dns-publish"; // See queue.xml. - - /** The parameter to use for storing the target type ("domain" or "host" or "zone"). */ - public static final String DNS_TARGET_TYPE_PARAM = "Target-Type"; - - /** The parameter to use for storing the target name (domain or host name) with the task. */ - public static final String DNS_TARGET_NAME_PARAM = "Target-Name"; - - /** The parameter to use for storing the creation time with the task. */ - public static final String DNS_TARGET_CREATE_TIME_PARAM = "Create-Time"; - - /** The possible values of the {@code DNS_TARGET_TYPE_PARAM} parameter. */ - public enum TargetType { - DOMAIN, - HOST - } -} diff --git a/core/src/main/java/google/registry/dns/DnsModule.java b/core/src/main/java/google/registry/dns/DnsModule.java index d6e09247d..fc8e93156 100644 --- a/core/src/main/java/google/registry/dns/DnsModule.java +++ b/core/src/main/java/google/registry/dns/DnsModule.java @@ -14,8 +14,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.RefreshDnsOnHostRenameAction.PARAM_HOST_KEY; import static google.registry.request.RequestParameters.extractEnumParameter; import static google.registry.request.RequestParameters.extractIntParameter; @@ -24,20 +22,17 @@ import static google.registry.request.RequestParameters.extractOptionalParameter import static google.registry.request.RequestParameters.extractRequiredParameter; import static google.registry.request.RequestParameters.extractSetOfParameters; -import com.google.appengine.api.taskqueue.Queue; -import com.google.appengine.api.taskqueue.QueueFactory; import com.google.common.hash.HashFunction; import com.google.common.hash.Hashing; import dagger.Binds; import dagger.Module; import dagger.Provides; -import google.registry.dns.DnsConstants.TargetType; +import google.registry.dns.DnsUtils.TargetType; import google.registry.dns.writer.DnsWriterZone; import google.registry.request.Parameter; import google.registry.request.RequestParameters; import java.util.Optional; import java.util.Set; -import javax.inject.Named; import javax.servlet.http.HttpServletRequest; import org.joda.time.DateTime; @@ -71,18 +66,6 @@ public abstract class DnsModule { return Hashing.murmur3_32_fixed(); } - @Provides - @Named(DNS_PULL_QUEUE_NAME) - static Queue provideDnsPullQueue() { - return QueueFactory.getQueue(DNS_PULL_QUEUE_NAME); - } - - @Provides - @Named(DNS_PUBLISH_PUSH_QUEUE_NAME) - static Queue provideDnsUpdatePushQueue() { - return QueueFactory.getQueue(DNS_PUBLISH_PUSH_QUEUE_NAME); - } - @Provides @Parameter(PARAM_PUBLISH_TASK_ENQUEUED) static DateTime provideCreateTime(HttpServletRequest req) { diff --git a/core/src/main/java/google/registry/dns/DnsQueue.java b/core/src/main/java/google/registry/dns/DnsQueue.java deleted file mode 100644 index a54b4f53f..000000000 --- a/core/src/main/java/google/registry/dns/DnsQueue.java +++ /dev/null @@ -1,170 +0,0 @@ -// 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.dns; - -import static com.google.appengine.api.taskqueue.QueueFactory.getQueue; -import static com.google.common.base.Preconditions.checkArgument; -import static google.registry.dns.DnsConstants.DNS_PULL_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.model.tld.Registries.assertTldExists; -import static google.registry.request.RequestParameters.PARAM_TLD; -import static google.registry.util.DomainNameUtils.getTldFromDomainName; -import static java.util.concurrent.TimeUnit.MILLISECONDS; - -import com.google.appengine.api.taskqueue.Queue; -import com.google.appengine.api.taskqueue.QueueConstants; -import com.google.appengine.api.taskqueue.TaskHandle; -import com.google.appengine.api.taskqueue.TaskOptions; -import com.google.appengine.api.taskqueue.TaskOptions.Method; -import com.google.appengine.api.taskqueue.TransientFailureException; -import com.google.apphosting.api.DeadlineExceededException; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ImmutableList; -import com.google.common.flogger.FluentLogger; -import com.google.common.net.InternetDomainName; -import com.google.common.util.concurrent.RateLimiter; -import google.registry.config.RegistryConfig; -import google.registry.dns.DnsConstants.TargetType; -import google.registry.model.tld.Registries; -import google.registry.util.Clock; -import google.registry.util.NonFinalForTesting; -import java.util.List; -import java.util.Optional; -import java.util.logging.Level; -import javax.inject.Inject; -import javax.inject.Named; -import org.joda.time.Duration; - -/** - * Methods for manipulating the queue used for DNS write tasks. - * - *

This includes a {@link RateLimiter} to limit the {@link Queue#leaseTasks} call rate to 9 QPS, - * to stay under the 10 QPS limit for this function. - * - *

Note that overlapping calls to {@link ReadDnsQueueAction} (the only place where {@link - * DnsQueue#leaseTasks} is used) will have different rate limiters, so they could exceed the allowed - * rate. This should be rare though - because {@link DnsQueue#leaseTasks} is only used in {@link - * ReadDnsQueueAction}, which is run as a cron job with running time shorter than the cron repeat - * time - meaning there should never be two instances running at once. - * - * @see RegistryConfig.ConfigModule#provideReadDnsRefreshRequestsRuntime() - */ -public class DnsQueue { - - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - - private final Queue queue; - - final Clock clock; - - // Queue.leaseTasks is limited to 10 requests per second as per - // https://cloud.google.com/appengine/docs/standard/java/javadoc/com/google/appengine/api/taskqueue/Queue.html - // "If you generate more than 10 LeaseTasks requests per second, only the first 10 requests will - // return results. The others will return no results." - private static final RateLimiter rateLimiter = RateLimiter.create(9); - - @Inject - DnsQueue(@Named(DNS_PULL_QUEUE_NAME) Queue queue, Clock clock) { - this.queue = queue; - this.clock = clock; - } - - Clock getClock() { - return clock; - } - - @VisibleForTesting - public static DnsQueue createForTesting(Clock clock) { - return new DnsQueue(getQueue(DNS_PULL_QUEUE_NAME), clock); - } - - @NonFinalForTesting - @VisibleForTesting - long leaseTasksBatchSize = QueueConstants.maxLeaseCount(); - - /** Enqueues the given task type with the given target name to the DNS queue. */ - private TaskHandle addToQueue( - TargetType targetType, String targetName, String tld, Duration countdown) { - logger.atInfo().log( - "Adding task type=%s, target=%s, tld=%s to pull queue %s (%d tasks currently on queue).", - targetType, targetName, tld, DNS_PULL_QUEUE_NAME, queue.fetchStatistics().getNumTasks()); - return queue.add( - TaskOptions.Builder.withDefaults() - .method(Method.PULL) - .countdownMillis(countdown.getMillis()) - .param(DNS_TARGET_TYPE_PARAM, targetType.toString()) - .param(DNS_TARGET_NAME_PARAM, targetName) - .param(DNS_TARGET_CREATE_TIME_PARAM, clock.nowUtc().toString()) - .param(PARAM_TLD, tld)); - } - - /** Adds a task to the queue to refresh the DNS information for the specified subordinate host. */ - TaskHandle addHostRefreshTask(String hostName) { - Optional tld = Registries.findTldForName(InternetDomainName.from(hostName)); - checkArgument( - tld.isPresent(), String.format("%s is not a subordinate host to a known tld", hostName)); - return addToQueue(TargetType.HOST, hostName, tld.get().toString(), Duration.ZERO); - } - - /** Enqueues a task to refresh DNS for the specified domain now. */ - TaskHandle addDomainRefreshTask(String domainName) { - return addDomainRefreshTask(domainName, Duration.ZERO); - } - - /** Enqueues a task to refresh DNS for the specified domain at some point in the future. */ - TaskHandle addDomainRefreshTask(String domainName, Duration countdown) { - return addToQueue( - TargetType.DOMAIN, - domainName, - assertTldExists(getTldFromDomainName(domainName)), - countdown); - } - - /** - * Returns the maximum number of tasks that can be leased with {@link #leaseTasks}. - * - *

If this many tasks are returned, then there might be more tasks still waiting in the queue. - * - *

If less than this number of tasks are returned, then there are no more items in the queue. - */ - public long getLeaseTasksBatchSize() { - return leaseTasksBatchSize; - } - - /** Returns handles for a batch of tasks, leased for the specified duration. */ - public List leaseTasks(Duration leaseDuration) { - try { - rateLimiter.acquire(); - int numTasks = queue.fetchStatistics().getNumTasks(); - logger.at((numTasks >= leaseTasksBatchSize) ? Level.WARNING : Level.INFO).log( - "There are %d tasks in the DNS queue '%s'.", numTasks, DNS_PULL_QUEUE_NAME); - return queue.leaseTasks(leaseDuration.getMillis(), MILLISECONDS, leaseTasksBatchSize); - } catch (TransientFailureException | DeadlineExceededException e) { - logger.atSevere().withCause(e).log("Failed leasing tasks too fast."); - return ImmutableList.of(); - } - } - - /** Delete a list of tasks, removing them from the queue permanently. */ - public void deleteTasks(List tasks) { - try { - queue.deleteTask(tasks); - } catch (TransientFailureException | DeadlineExceededException e) { - logger.atSevere().withCause(e).log("Failed deleting tasks too fast."); - } - } -} diff --git a/core/src/main/java/google/registry/dns/DnsUtils.java b/core/src/main/java/google/registry/dns/DnsUtils.java index bb3b5f62e..9652798e1 100644 --- a/core/src/main/java/google/registry/dns/DnsUtils.java +++ b/core/src/main/java/google/registry/dns/DnsUtils.java @@ -19,57 +19,42 @@ import static google.registry.persistence.transaction.TransactionManagerFactory. import com.google.common.collect.ImmutableList; import com.google.common.net.InternetDomainName; -import google.registry.dns.DnsConstants.TargetType; -import google.registry.model.common.DatabaseMigrationStateSchedule; -import google.registry.model.common.DatabaseMigrationStateSchedule.MigrationState; import google.registry.model.common.DnsRefreshRequest; import google.registry.model.tld.Registries; import google.registry.model.tld.Tld; import java.util.Collection; import java.util.Optional; -import javax.inject.Inject; import org.joda.time.DateTime; import org.joda.time.Duration; /** Utility class to handle DNS refresh requests. */ -// TODO: Make this a static util function once we are done with the DNS pull queue migration. -public class DnsUtils { +public final class DnsUtils { - private final DnsQueue dnsQueue; + /** The name of the DNS publish push queue. */ + public static final String DNS_PUBLISH_PUSH_QUEUE_NAME = "dns-publish"; // See queue.xml. - @Inject - DnsUtils(DnsQueue dnsQueue) { - this.dnsQueue = dnsQueue; - } + private DnsUtils() {} - private void requestDnsRefresh(String name, TargetType type, Duration delay) { + private static void requestDnsRefresh(String name, TargetType type, Duration delay) { // Throws an IllegalArgumentException if the name is not under a managed TLD -- we only update // DNS for names that are under our management. String tld = Registries.findTldForNameOrThrow(InternetDomainName.from(name)).toString(); - if (usePullQueue()) { - if (TargetType.HOST.equals(type)) { - dnsQueue.addHostRefreshTask(name); - } else { - dnsQueue.addDomainRefreshTask(name, delay); - } - } else { - tm().transact( - () -> - tm().insert( - new DnsRefreshRequest( - type, name, tld, tm().getTransactionTime().plus(delay)))); - } + tm().transact( + () -> + tm().insert( + new DnsRefreshRequest( + type, name, tld, tm().getTransactionTime().plus(delay)))); } - public void requestDomainDnsRefresh(String domainName, Duration delay) { + public static void requestDomainDnsRefresh(String domainName, Duration delay) { requestDnsRefresh(domainName, TargetType.DOMAIN, delay); } - public void requestDomainDnsRefresh(String domainName) { + public static void requestDomainDnsRefresh(String domainName) { requestDomainDnsRefresh(domainName, Duration.ZERO); } - public void requestHostDnsRefresh(String hostName) { + public static void requestHostDnsRefresh(String hostName) { requestDnsRefresh(hostName, TargetType.HOST, Duration.ZERO); } @@ -85,7 +70,7 @@ public class DnsUtils { *

  • The last time they were processed is before the cooldown period. * */ - public ImmutableList readAndUpdateRequestsWithLatestProcessTime( + public static ImmutableList readAndUpdateRequestsWithLatestProcessTime( String tld, Duration cooldown, int batchSize) { return tm().transact( () -> { @@ -105,7 +90,7 @@ public class DnsUtils { // queued up for publishing, not when it is actually published by the DNS // writer. This timestamp acts as a cooldown so the same request will not be // retried too frequently. See DnsRefreshRequest.getLastProcessTime for a - // detailed explaination. + // detailed explanation. .map(e -> e.updateProcessTime(transactionTime)) .collect(toImmutableList()); tm().updateAll(requests); @@ -119,7 +104,7 @@ public class DnsUtils { *

    Note that if a request entity has already been deleted, the method still succeeds without * error because all we care about is that it no longer exists after the method runs. */ - public void deleteRequests(Collection requests) { + public static void deleteRequests(Collection requests) { tm().transact( () -> tm().delete( @@ -140,8 +125,9 @@ public class DnsUtils { return dnsAPlusAaaaTtl.getStandardSeconds(); } - private boolean usePullQueue() { - return !DatabaseMigrationStateSchedule.getValueAtTime(dnsQueue.getClock().nowUtc()) - .equals(MigrationState.DNS_SQL); + /** The possible values of the {@code DNS_TARGET_TYPE_PARAM} parameter. */ + public enum TargetType { + DOMAIN, + HOST } } diff --git a/core/src/main/java/google/registry/dns/PublishDnsUpdatesAction.java b/core/src/main/java/google/registry/dns/PublishDnsUpdatesAction.java index b74eb7a57..90d0675ca 100644 --- a/core/src/main/java/google/registry/dns/PublishDnsUpdatesAction.java +++ b/core/src/main/java/google/registry/dns/PublishDnsUpdatesAction.java @@ -15,7 +15,6 @@ package google.registry.dns; import static com.google.common.collect.ImmutableList.toImmutableList; -import static google.registry.dns.DnsConstants.DNS_PUBLISH_PUSH_QUEUE_NAME; 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; @@ -23,6 +22,9 @@ 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_TIME; +import static google.registry.dns.DnsUtils.DNS_PUBLISH_PUSH_QUEUE_NAME; +import static google.registry.dns.DnsUtils.requestDomainDnsRefresh; +import static google.registry.dns.DnsUtils.requestHostDnsRefresh; import static google.registry.model.EppResourceUtils.loadByForeignKey; import static google.registry.request.Action.Method.POST; import static google.registry.request.RequestParameters.PARAM_TLD; @@ -85,7 +87,6 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private final DnsUtils dnsUtils; private final DnsWriterProxy dnsWriterProxy; private final DnsMetrics dnsMetrics; private final Duration timeout; @@ -94,10 +95,10 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable { /** * The DNS writer to use for this batch. * - *

    This comes from the fanout in {@link ReadDnsQueueAction} which dispatches each batch to be - * published by each DNS writer on the TLD. So this field contains the value of one of the DNS - * writers configured in {@link Tld#getDnsWriters()}, as of the time the batch was written out - * (and not necessarily currently). + *

    This comes from the fanout in {@link ReadDnsRefreshRequestsAction} which dispatches each + * batch to be published by each DNS writer on the TLD. So this field contains the value of one of + * the DNS writers configured in {@link Tld#getDnsWriters()}, as of the time the batch was written + * out (and not necessarily currently). */ private final String dnsWriter; @@ -139,7 +140,6 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable { @Config("gSuiteOutgoingEmailAddress") InternetAddress gSuiteOutgoingEmailAddress, @Header(APP_ENGINE_RETRY_HEADER) Optional appEngineRetryCount, @Header(CLOUD_TASKS_RETRY_HEADER) Optional cloudTasksRetryCount, - DnsUtils dnsUtils, DnsWriterProxy dnsWriterProxy, DnsMetrics dnsMetrics, LockHandler lockHandler, @@ -147,12 +147,11 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable { CloudTasksUtils cloudTasksUtils, SendEmailService sendEmailService, Response response) { - this.dnsUtils = dnsUtils; this.dnsWriterProxy = dnsWriterProxy; this.dnsMetrics = dnsMetrics; this.timeout = timeout; this.sendEmailService = sendEmailService; - this.retryCount = + retryCount = cloudTasksRetryCount.orElse( appEngineRetryCount.orElseThrow( () -> new IllegalStateException("Missing a valid retry count header"))); @@ -276,7 +275,7 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable { return null; } - private InternetAddress emailToInternetAddress(String email) { + private static InternetAddress emailToInternetAddress(String email) { try { return new InternetAddress(email, true); } catch (Exception e) { @@ -306,7 +305,7 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable { registrar.get().getContacts().stream() .filter(c -> c.getTypes().contains(RegistrarPoc.Type.ADMIN)) .map(RegistrarPoc::getEmailAddress) - .map(this::emailToInternetAddress) + .map(PublishDnsUpdatesAction::emailToInternetAddress) .collect(toImmutableList()); sendEmailService.sendEmail( @@ -356,10 +355,10 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable { private void requeueBatch() { logger.atInfo().log("Requeueing batch for retry."); for (String domain : nullToEmpty(domains)) { - dnsUtils.requestDomainDnsRefresh(domain); + requestDomainDnsRefresh(domain); } for (String host : nullToEmpty(hosts)) { - dnsUtils.requestHostDnsRefresh(host); + requestHostDnsRefresh(host); } } diff --git a/core/src/main/java/google/registry/dns/ReadDnsQueueAction.java b/core/src/main/java/google/registry/dns/ReadDnsQueueAction.java deleted file mode 100644 index 466fc8492..000000000 --- a/core/src/main/java/google/registry/dns/ReadDnsQueueAction.java +++ /dev/null @@ -1,401 +0,0 @@ -// 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.dns; - -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_TIME; -import static google.registry.request.RequestParameters.PARAM_TLD; -import static google.registry.util.DomainNameUtils.getSecondLevelDomain; -import static java.nio.charset.StandardCharsets.UTF_8; - -import com.google.appengine.api.taskqueue.TaskHandle; -import com.google.auto.value.AutoValue; -import com.google.cloud.tasks.v2.Task; -import com.google.common.collect.ComparisonChain; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableMultimap; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSetMultimap; -import com.google.common.collect.Iterables; -import com.google.common.collect.Ordering; -import com.google.common.flogger.FluentLogger; -import com.google.common.hash.HashFunction; -import com.google.common.hash.Hashing; -import google.registry.batch.CloudTasksUtils; -import google.registry.config.RegistryConfig.Config; -import google.registry.dns.DnsConstants.TargetType; -import google.registry.model.tld.Registries; -import google.registry.model.tld.Tld; -import google.registry.request.Action; -import google.registry.request.Action.Service; -import google.registry.request.Parameter; -import google.registry.request.auth.Auth; -import google.registry.util.Clock; -import java.io.UnsupportedEncodingException; -import java.util.Collection; -import java.util.Comparator; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.stream.Collectors; -import javax.inject.Inject; -import org.joda.time.DateTime; -import org.joda.time.Duration; - -/** - * Action for fanning out DNS refresh tasks by TLD, using data taken from the DNS pull queue. - * - *

    Parameters Reference

    - * - *
      - *
    • {@code jitterSeconds} Randomly delay each task by up to this many seconds. - *
    - */ -@Action( - service = Action.Service.BACKEND, - path = "/_dr/cron/readDnsQueue", - automaticallyPrintOk = true, - auth = Auth.AUTH_INTERNAL_OR_ADMIN) -public final class ReadDnsQueueAction implements Runnable { - - private static final String PARAM_JITTER_SECONDS = "jitterSeconds"; - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - - /** - * Buffer time since the end of this action until retriable tasks are available again. - * - *

    We read batches of tasks from the queue in a loop. Any task that will need to be retried has - * to be kept out of the queue for the duration of this action - otherwise we will lease it again - * in a subsequent loop. - * - *

    The 'requestedMaximumDuration' value is the maximum delay between the first and last calls - * to lease tasks, hence we want the lease duration to be (slightly) longer than that. - * LEASE_PADDING is the value we add to {@link #requestedMaximumDuration} to make sure the lease - * duration is indeed longer. - */ - private static final Duration LEASE_PADDING = Duration.standardMinutes(1); - - private final int tldUpdateBatchSize; - private final Duration requestedMaximumDuration; - private final Optional jitterSeconds; - private final Clock clock; - private final DnsQueue dnsQueue; - private final HashFunction hashFunction; - private final CloudTasksUtils cloudTasksUtils; - - @Inject - ReadDnsQueueAction( - @Config("dnsTldUpdateBatchSize") int tldUpdateBatchSize, - @Config("readDnsRefreshRequestsActionRuntime") Duration requestedMaximumDuration, - @Parameter(PARAM_JITTER_SECONDS) Optional jitterSeconds, - Clock clock, - DnsQueue dnsQueue, - HashFunction hashFunction, - CloudTasksUtils cloudTasksUtils) { - this.tldUpdateBatchSize = tldUpdateBatchSize; - this.requestedMaximumDuration = requestedMaximumDuration; - this.jitterSeconds = jitterSeconds; - this.clock = clock; - this.dnsQueue = dnsQueue; - this.hashFunction = hashFunction; - this.cloudTasksUtils = cloudTasksUtils; - } - - /** Container for items we pull out of the DNS pull queue and process for fanout. */ - @AutoValue - abstract static class RefreshItem implements Comparable { - static RefreshItem create(TargetType type, String name, DateTime creationTime) { - return new AutoValue_ReadDnsQueueAction_RefreshItem(type, name, creationTime); - } - - abstract TargetType type(); - - abstract String name(); - - abstract DateTime creationTime(); - - @Override - public int compareTo(RefreshItem other) { - return ComparisonChain.start() - .compare(this.type(), other.type()) - .compare(this.name(), other.name()) - .compare(this.creationTime(), other.creationTime()) - .result(); - } - } - - /** Leases all tasks from the pull queue and creates per-tld update actions for them. */ - @Override - public void run() { - DateTime requestedEndTime = clock.nowUtc().plus(requestedMaximumDuration); - ImmutableSet tlds = Registries.getTlds(); - while (requestedEndTime.isAfterNow()) { - List tasks = dnsQueue.leaseTasks(requestedMaximumDuration.plus(LEASE_PADDING)); - logger.atInfo().log("Leased %d DNS update tasks.", tasks.size()); - if (!tasks.isEmpty()) { - dispatchTasks(ImmutableSet.copyOf(tasks), tlds); - } - if (tasks.size() < dnsQueue.getLeaseTasksBatchSize()) { - return; - } - } - } - - /** A set of tasks grouped based on the action to take on them. */ - @AutoValue - abstract static class ClassifiedTasks { - - /** - * List of tasks we want to keep in the queue (want to retry in the future). - * - *

    Normally, any task we lease from the queue will be deleted - either because we are going - * to process it now (these tasks are part of refreshItemsByTld), or because these tasks are - * "corrupt" in some way (don't parse, don't have the required parameters etc.). - * - *

    Some tasks however are valid, but can't be processed at the moment. These tasks will be - * kept in the queue for future processing. - * - *

    This includes tasks belonging to paused TLDs (which we want to process once the TLD is - * unpaused) and tasks belonging to (currently) unknown TLDs. - */ - abstract ImmutableSet tasksToKeep(); - - /** The paused TLDs for which we found at least one refresh request. */ - abstract ImmutableSet pausedTlds(); - - /** - * The unknown TLDs for which we found at least one refresh request. - * - *

    Unknown TLDs might have valid requests because the list of TLDs is heavily cached. Hence, - * when we add a new TLD - it is possible that some instances will not have that TLD in their - * list yet. We don't want to discard these tasks, so we wait a bit and retry them again. - * - *

    This is less likely for production TLDs but is quite likely for test TLDs where we might - * create a TLD and then use it within seconds. - */ - abstract ImmutableSet unknownTlds(); - - /** - * All the refresh items we need to actually fulfill, grouped by TLD. - * - *

    By default, the multimap is ordered - this can be changed with the builder. - * - *

    The items for each TLD will be grouped together, and domains and hosts will be grouped - * within a TLD. - * - *

    The grouping and ordering of domains and hosts is not technically necessary, but a - * predictable ordering makes it possible to write detailed tests. - */ - abstract ImmutableSetMultimap refreshItemsByTld(); - - static Builder builder() { - Builder builder = new AutoValue_ReadDnsQueueAction_ClassifiedTasks.Builder(); - builder - .refreshItemsByTldBuilder() - .orderKeysBy(Ordering.natural()) - .orderValuesBy(Ordering.natural()); - return builder; - } - - @AutoValue.Builder - abstract static class Builder { - abstract ImmutableSet.Builder tasksToKeepBuilder(); - abstract ImmutableSet.Builder pausedTldsBuilder(); - abstract ImmutableSet.Builder unknownTldsBuilder(); - abstract ImmutableSetMultimap.Builder refreshItemsByTldBuilder(); - - abstract ClassifiedTasks build(); - } - } - - /** - * Creates per-tld update actions for tasks leased from the pull queue. - * - *

    Will return "irrelevant" tasks to the queue for future processing. "Irrelevant" tasks are - * tasks for paused TLDs or tasks for TLDs not part of {@link Registries#getTlds()}. - */ - private void dispatchTasks(ImmutableSet tasks, ImmutableSet tlds) { - ClassifiedTasks classifiedTasks = classifyTasks(tasks, tlds); - if (!classifiedTasks.pausedTlds().isEmpty()) { - logger.atInfo().log( - "The dns-pull queue is paused for TLDs: %s.", classifiedTasks.pausedTlds()); - } - if (!classifiedTasks.unknownTlds().isEmpty()) { - logger.atWarning().log( - "The dns-pull queue has unknown TLDs: %s.", classifiedTasks.unknownTlds()); - } - bucketRefreshItems(classifiedTasks.refreshItemsByTld()); - if (!classifiedTasks.tasksToKeep().isEmpty()) { - logger.atWarning().log( - "Keeping %d DNS update tasks in the queue.", classifiedTasks.tasksToKeep().size()); - } - // Delete the tasks we don't want to see again from the queue. - // - // tasksToDelete includes both the tasks that we already fulfilled in this call (were part of - // refreshItemsByTld) and "corrupt" tasks we weren't able to parse correctly (this shouldn't - // happen, and we logged a "severe" error) - // - // We let the lease on the rest of the tasks (the tasks we want to keep) expire on its own. The - // reason we don't release these tasks back to the queue immediately is that we are going to - // immediately read another batch of tasks from the queue - and we don't want to get the same - // tasks again. - ImmutableSet tasksToDelete = - difference(tasks, classifiedTasks.tasksToKeep()).immutableCopy(); - logger.atInfo().log("Removing %d DNS update tasks from the queue.", tasksToDelete.size()); - dnsQueue.deleteTasks(tasksToDelete.asList()); - logger.atInfo().log("Done processing DNS tasks."); - } - - /** - * Classifies the given tasks based on what action we need to take on them. - * - *

    Note that some tasks might appear in multiple categories (if multiple actions are to be - * taken on them) or in no category (if no action is to be taken on them) - */ - private static ClassifiedTasks classifyTasks( - ImmutableSet tasks, ImmutableSet tlds) { - - ClassifiedTasks.Builder classifiedTasksBuilder = ClassifiedTasks.builder(); - - // Read all tasks on the DNS pull queue and load them into the refresh item multimap. - for (TaskHandle task : tasks) { - try { - Map params = ImmutableMap.copyOf(task.extractParams()); - DateTime creationTime = DateTime.parse(params.get(DNS_TARGET_CREATE_TIME_PARAM)); - String tld = params.get(PARAM_TLD); - if (tld == null) { - logger.atSevere().log( - "Discarding invalid DNS refresh request %s; no TLD specified.", task); - } else if (!tlds.contains(tld)) { - classifiedTasksBuilder.tasksToKeepBuilder().add(task); - classifiedTasksBuilder.unknownTldsBuilder().add(tld); - } else if (Tld.get(tld).getDnsPaused()) { - classifiedTasksBuilder.tasksToKeepBuilder().add(task); - classifiedTasksBuilder.pausedTldsBuilder().add(tld); - } else { - String typeString = params.get(DNS_TARGET_TYPE_PARAM); - String name = params.get(DNS_TARGET_NAME_PARAM); - TargetType type = TargetType.valueOf(typeString); - switch (type) { - case DOMAIN: - case HOST: - classifiedTasksBuilder - .refreshItemsByTldBuilder() - .put(tld, RefreshItem.create(type, name, creationTime)); - break; - default: - logger.atSevere().log( - "Discarding DNS refresh request %s of type %s.", task, typeString); - break; - } - } - } catch (RuntimeException | UnsupportedEncodingException e) { - logger.atSevere().withCause(e).log("Discarding invalid DNS refresh request %s.", task); - } - } - return classifiedTasksBuilder.build(); - } - - /** - * Subdivides the tld to {@link RefreshItem} multimap into buckets by lock index, if applicable. - * - *

    If the tld has numDnsPublishLocks <= 1, we enqueue all updates on the default lock 1 of 1. - */ - private void bucketRefreshItems(ImmutableSetMultimap refreshItemsByTld) { - // Loop through the multimap by TLD and generate refresh tasks for the hosts and domains for - // each configured DNS writer. - for (Map.Entry> tldRefreshItemsEntry - : refreshItemsByTld.asMap().entrySet()) { - String tld = tldRefreshItemsEntry.getKey(); - int numPublishLocks = Tld.get(tld).getNumDnsPublishLocks(); - // 1 lock or less implies no TLD-wide locks, simply enqueue everything under lock 1 of 1 - if (numPublishLocks <= 1) { - enqueueUpdates(tld, 1, 1, tldRefreshItemsEntry.getValue()); - } else { - tldRefreshItemsEntry.getValue().stream() - .collect( - toImmutableSetMultimap( - refreshItem -> getLockIndex(tld, numPublishLocks, refreshItem), - refreshItem -> refreshItem)) - .asMap() - .forEach((key, value) -> enqueueUpdates(tld, key, numPublishLocks, value)); - } - } - } - - /** - * Returns the lock index for a given refreshItem. - * - *

    We hash the second level domain for all records, to group in-bailiwick hosts (the only ones - * we refresh DNS for) with their superordinate domains. We use consistent hashing to determine - * the lock index because it gives us [0,N) bucketing properties out of the box, then add 1 to - * make indexes within [1,N]. - */ - private int getLockIndex(String tld, int numPublishLocks, RefreshItem refreshItem) { - String domain = getSecondLevelDomain(refreshItem.name(), tld); - return Hashing.consistentHash(hashFunction.hashString(domain, UTF_8), numPublishLocks) + 1; - } - - /** - * Creates DNS refresh tasks for all writers for the tld within a lock index and batches large - * updates into smaller chunks. - */ - private void enqueueUpdates( - String tld, int lockIndex, int numPublishLocks, Collection items) { - for (List chunk : Iterables.partition(items, tldUpdateBatchSize)) { - DateTime earliestCreateTime = - chunk.stream().map(RefreshItem::creationTime).min(Comparator.naturalOrder()).get(); - for (String dnsWriter : Tld.get(tld).getDnsWriters()) { - Task task = - cloudTasksUtils.createPostTaskWithJitter( - PublishDnsUpdatesAction.PATH, - Service.BACKEND, - ImmutableMultimap.builder() - .put(PARAM_TLD, tld) - .put(PARAM_DNS_WRITER, dnsWriter) - .put(PARAM_LOCK_INDEX, Integer.toString(lockIndex)) - .put(PARAM_NUM_PUBLISH_LOCKS, Integer.toString(numPublishLocks)) - .put(PARAM_PUBLISH_TASK_ENQUEUED, clock.nowUtc().toString()) - .put(PARAM_REFRESH_REQUEST_TIME, earliestCreateTime.toString()) - .put( - PARAM_DOMAINS, - chunk.stream() - .filter(item -> item.type() == TargetType.DOMAIN) - .map(RefreshItem::name) - .collect(Collectors.joining(","))) - .put( - PARAM_HOSTS, - chunk.stream() - .filter(item -> item.type() == TargetType.HOST) - .map(RefreshItem::name) - .collect(Collectors.joining(","))) - .build(), - jitterSeconds); - cloudTasksUtils.enqueue(DNS_PUBLISH_PUSH_QUEUE_NAME, task); - } - } - } -} diff --git a/core/src/main/java/google/registry/dns/ReadDnsRefreshRequestsAction.java b/core/src/main/java/google/registry/dns/ReadDnsRefreshRequestsAction.java index 243657f08..0e4cf5338 100644 --- a/core/src/main/java/google/registry/dns/ReadDnsRefreshRequestsAction.java +++ b/core/src/main/java/google/registry/dns/ReadDnsRefreshRequestsAction.java @@ -15,7 +15,6 @@ package google.registry.dns; import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap; -import static google.registry.dns.DnsConstants.DNS_PUBLISH_PUSH_QUEUE_NAME; import static google.registry.dns.DnsModule.PARAM_DNS_JITTER_SECONDS; import static google.registry.dns.DnsModule.PARAM_DNS_WRITER; import static google.registry.dns.DnsModule.PARAM_DOMAINS; @@ -24,6 +23,9 @@ 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_TIME; +import static google.registry.dns.DnsUtils.DNS_PUBLISH_PUSH_QUEUE_NAME; +import static google.registry.dns.DnsUtils.deleteRequests; +import static google.registry.dns.DnsUtils.readAndUpdateRequestsWithLatestProcessTime; import static google.registry.request.Action.Method.POST; import static google.registry.request.RequestParameters.PARAM_TLD; import static google.registry.util.DateTimeUtils.END_OF_TIME; @@ -39,7 +41,7 @@ import com.google.common.hash.HashFunction; import com.google.common.hash.Hashing; import google.registry.batch.CloudTasksUtils; import google.registry.config.RegistryConfig.Config; -import google.registry.dns.DnsConstants.TargetType; +import google.registry.dns.DnsUtils.TargetType; import google.registry.model.common.DnsRefreshRequest; import google.registry.model.tld.Tld; import google.registry.request.Action; @@ -72,7 +74,6 @@ public final class ReadDnsRefreshRequestsAction implements Runnable { private final Optional jitterSeconds; private final String tld; private final Clock clock; - private final DnsUtils dnsUtils; private final HashFunction hashFunction; private final CloudTasksUtils cloudTasksUtils; @@ -83,7 +84,6 @@ public final class ReadDnsRefreshRequestsAction implements Runnable { @Parameter(PARAM_DNS_JITTER_SECONDS) Optional jitterSeconds, @Parameter(PARAM_TLD) String tld, Clock clock, - DnsUtils dnsUtils, HashFunction hashFunction, CloudTasksUtils cloudTasksUtils) { this.tldUpdateBatchSize = tldUpdateBatchSize; @@ -91,7 +91,6 @@ public final class ReadDnsRefreshRequestsAction implements Runnable { this.jitterSeconds = jitterSeconds; this.tld = tld; this.clock = clock; - this.dnsUtils = dnsUtils; this.hashFunction = hashFunction; this.cloudTasksUtils = cloudTasksUtils; } @@ -112,7 +111,7 @@ public final class ReadDnsRefreshRequestsAction implements Runnable { int processBatchSize = tldUpdateBatchSize * Tld.get(tld).getNumDnsPublishLocks(); while (requestedEndTime.isAfter(clock.nowUtc())) { ImmutableList requests = - dnsUtils.readAndUpdateRequestsWithLatestProcessTime( + readAndUpdateRequestsWithLatestProcessTime( tld, requestedMaximumDuration, processBatchSize); logger.atInfo().log("Read %d DNS update requests for TLD %s.", requests.size(), tld); if (!requests.isEmpty()) { @@ -138,7 +137,7 @@ public final class ReadDnsRefreshRequestsAction implements Runnable { (lockIndex, bucketedRequests) -> { try { enqueueUpdates(lockIndex, numPublishLocks, bucketedRequests); - dnsUtils.deleteRequests(bucketedRequests); + deleteRequests(bucketedRequests); logger.atInfo().log( "Processed %d DNS update requests for TLD %s.", bucketedRequests.size(), tld); } catch (Exception e) { diff --git a/core/src/main/java/google/registry/dns/RefreshDnsAction.java b/core/src/main/java/google/registry/dns/RefreshDnsAction.java index 3cb81c6ce..4418b8491 100644 --- a/core/src/main/java/google/registry/dns/RefreshDnsAction.java +++ b/core/src/main/java/google/registry/dns/RefreshDnsAction.java @@ -14,9 +14,11 @@ package google.registry.dns; +import static google.registry.dns.DnsUtils.requestDomainDnsRefresh; +import static google.registry.dns.DnsUtils.requestHostDnsRefresh; import static google.registry.model.EppResourceUtils.loadByForeignKey; -import google.registry.dns.DnsConstants.TargetType; +import google.registry.dns.DnsUtils.TargetType; import google.registry.model.EppResource; import google.registry.model.EppResource.ForeignKeyedEppResource; import google.registry.model.annotations.ExternalMessagingName; @@ -39,7 +41,6 @@ import javax.inject.Inject; public final class RefreshDnsAction implements Runnable { private final Clock clock; - private final DnsUtils dnsUtils; private final String domainOrHostName; private final TargetType type; @@ -47,12 +48,10 @@ public final class RefreshDnsAction implements Runnable { RefreshDnsAction( @Parameter("domainOrHostName") String domainOrHostName, @Parameter("type") TargetType type, - Clock clock, - DnsUtils dnsUtils) { + Clock clock) { this.domainOrHostName = domainOrHostName; this.type = type; this.clock = clock; - this.dnsUtils = dnsUtils; } @Override @@ -63,11 +62,11 @@ public final class RefreshDnsAction implements Runnable { switch (type) { case DOMAIN: loadAndVerifyExistence(Domain.class, domainOrHostName); - dnsUtils.requestDomainDnsRefresh(domainOrHostName); + requestDomainDnsRefresh(domainOrHostName); break; case HOST: verifyHostIsSubordinate(loadAndVerifyExistence(Host.class, domainOrHostName)); - dnsUtils.requestHostDnsRefresh(domainOrHostName); + requestHostDnsRefresh(domainOrHostName); break; default: throw new BadRequestException("Unsupported type: " + type); diff --git a/core/src/main/java/google/registry/dns/RefreshDnsOnHostRenameAction.java b/core/src/main/java/google/registry/dns/RefreshDnsOnHostRenameAction.java index 573349ae6..e70d1f913 100644 --- a/core/src/main/java/google/registry/dns/RefreshDnsOnHostRenameAction.java +++ b/core/src/main/java/google/registry/dns/RefreshDnsOnHostRenameAction.java @@ -14,6 +14,7 @@ package google.registry.dns; +import static google.registry.dns.DnsUtils.requestDomainDnsRefresh; import static google.registry.dns.RefreshDnsOnHostRenameAction.PATH; import static google.registry.model.EppResourceUtils.getLinkedDomainKeys; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; @@ -45,14 +46,11 @@ public class RefreshDnsOnHostRenameAction implements Runnable { private final VKey hostKey; private final Response response; - private final DnsUtils dnsUtils; @Inject - RefreshDnsOnHostRenameAction( - @Parameter(PARAM_HOST_KEY) String hostKey, Response response, DnsUtils dnsUtils) { + RefreshDnsOnHostRenameAction(@Parameter(PARAM_HOST_KEY) String hostKey, Response response) { this.hostKey = VKey.createEppVKeyFromString(hostKey); this.response = response; - this.dnsUtils = dnsUtils; } @Override @@ -76,7 +74,7 @@ public class RefreshDnsOnHostRenameAction implements Runnable { .stream() .map(domainKey -> tm().loadByKey(domainKey)) .filter(Domain::shouldPublishToDns) - .forEach(domain -> dnsUtils.requestDomainDnsRefresh(domain.getDomainName())); + .forEach(domain -> requestDomainDnsRefresh(domain.getDomainName())); } if (!hostValid) { diff --git a/core/src/main/java/google/registry/env/alpha/default/WEB-INF/cloud-scheduler-tasks.xml b/core/src/main/java/google/registry/env/alpha/default/WEB-INF/cloud-scheduler-tasks.xml index 49be09491..50b543b37 100644 --- a/core/src/main/java/google/registry/env/alpha/default/WEB-INF/cloud-scheduler-tasks.xml +++ b/core/src/main/java/google/registry/env/alpha/default/WEB-INF/cloud-scheduler-tasks.xml @@ -129,16 +129,6 @@ 0 5 * * * - - - readDnsQueue - - Lease all tasks from the dns-pull queue, group by TLD, and invoke PublishDnsUpdates for each - group. - - */1 * * * * - - diff --git a/core/src/main/java/google/registry/env/common/backend/WEB-INF/web.xml b/core/src/main/java/google/registry/env/common/backend/WEB-INF/web.xml index 0437d04e6..2432624a1 100644 --- a/core/src/main/java/google/registry/env/common/backend/WEB-INF/web.xml +++ b/core/src/main/java/google/registry/env/common/backend/WEB-INF/web.xml @@ -151,12 +151,6 @@ /_dr/task/nordnVerify - - - backend-servlet - /_dr/cron/readDnsQueue - - backend-servlet diff --git a/core/src/main/java/google/registry/env/common/default/WEB-INF/queue.xml b/core/src/main/java/google/registry/env/common/default/WEB-INF/queue.xml index ff780cff4..5382b3fd0 100644 --- a/core/src/main/java/google/registry/env/common/default/WEB-INF/queue.xml +++ b/core/src/main/java/google/registry/env/common/default/WEB-INF/queue.xml @@ -1,11 +1,6 @@ - - dns-pull - pull - - dns-refresh diff --git a/core/src/main/java/google/registry/env/crash/default/WEB-INF/cloud-scheduler-tasks.xml b/core/src/main/java/google/registry/env/crash/default/WEB-INF/cloud-scheduler-tasks.xml index 63f7d8ed3..d282db1c3 100644 --- a/core/src/main/java/google/registry/env/crash/default/WEB-INF/cloud-scheduler-tasks.xml +++ b/core/src/main/java/google/registry/env/crash/default/WEB-INF/cloud-scheduler-tasks.xml @@ -129,16 +129,6 @@ 0 5 * * * - - - readDnsQueue - - Lease all tasks from the dns-pull queue, group by TLD, and invoke PublishDnsUpdates for each - group. - - */1 * * * * - - diff --git a/core/src/main/java/google/registry/env/production/default/WEB-INF/cloud-scheduler-tasks.xml b/core/src/main/java/google/registry/env/production/default/WEB-INF/cloud-scheduler-tasks.xml index a5a583ad1..622f9bc17 100644 --- a/core/src/main/java/google/registry/env/production/default/WEB-INF/cloud-scheduler-tasks.xml +++ b/core/src/main/java/google/registry/env/production/default/WEB-INF/cloud-scheduler-tasks.xml @@ -201,16 +201,6 @@ 0 5 * * * - - - readDnsQueue - - Lease all tasks from the dns-pull queue, group by TLD, and invoke PublishDnsUpdates for each - group. - - */1 * * * * - - diff --git a/core/src/main/java/google/registry/env/qa/default/WEB-INF/cloud-scheduler-tasks.xml b/core/src/main/java/google/registry/env/qa/default/WEB-INF/cloud-scheduler-tasks.xml index b0d967687..0ce2ad22e 100644 --- a/core/src/main/java/google/registry/env/qa/default/WEB-INF/cloud-scheduler-tasks.xml +++ b/core/src/main/java/google/registry/env/qa/default/WEB-INF/cloud-scheduler-tasks.xml @@ -11,16 +11,6 @@ 7 0 * * * - - - readDnsQueue - - Lease all tasks from the dns-pull queue, group by TLD, and invoke PublishDnsUpdates for each - group. - - */1 * * * * - - diff --git a/core/src/main/java/google/registry/env/sandbox/default/WEB-INF/cloud-scheduler-tasks.xml b/core/src/main/java/google/registry/env/sandbox/default/WEB-INF/cloud-scheduler-tasks.xml index 8a22fe8d6..20eeba06b 100644 --- a/core/src/main/java/google/registry/env/sandbox/default/WEB-INF/cloud-scheduler-tasks.xml +++ b/core/src/main/java/google/registry/env/sandbox/default/WEB-INF/cloud-scheduler-tasks.xml @@ -143,16 +143,6 @@ 0 5 * * * - - - readDnsQueue - - Lease all tasks from the dns-pull queue, group by TLD, and invoke PublishDnsUpdates for each - group. - - */1 * * * * - - diff --git a/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java b/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java index 3a50b2fc9..7c89b604f 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java @@ -16,6 +16,7 @@ package google.registry.flows.domain; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static google.registry.dns.DnsUtils.requestDomainDnsRefresh; import static google.registry.flows.FlowUtils.persistEntityChanges; import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.verifyResourceDoesNotExist; @@ -59,7 +60,6 @@ import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.net.InternetDomainName; -import google.registry.dns.DnsUtils; import google.registry.flows.EppException; import google.registry.flows.EppException.CommandUseErrorException; import google.registry.flows.EppException.ParameterValuePolicyErrorException; @@ -227,7 +227,6 @@ public final class DomainCreateFlow implements TransactionalFlow { @Inject DomainCreateFlowCustomLogic flowCustomLogic; @Inject DomainFlowTmchUtils tmchUtils; @Inject DomainPricingLogic pricingLogic; - @Inject DnsUtils dnsUtils; @Inject DomainCreateFlow() {} @@ -426,7 +425,7 @@ public final class DomainCreateFlow implements TransactionalFlow { allocationToken.get(), domainHistory.getHistoryEntryId())); } if (domain.shouldPublishToDns()) { - dnsUtils.requestDomainDnsRefresh(domain.getDomainName()); + requestDomainDnsRefresh(domain.getDomainName()); } EntityChanges entityChanges = flowCustomLogic.beforeSave( diff --git a/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java b/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java index fce3c28dc..93919c01f 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java @@ -16,6 +16,7 @@ package google.registry.flows.domain; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Strings.isNullOrEmpty; +import static google.registry.dns.DnsUtils.requestDomainDnsRefresh; import static google.registry.flows.FlowUtils.createHistoryEntryId; import static google.registry.flows.FlowUtils.persistEntityChanges; import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn; @@ -44,7 +45,6 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Sets; import google.registry.batch.AsyncTaskEnqueuer; -import google.registry.dns.DnsUtils; import google.registry.flows.EppException; import google.registry.flows.EppException.AssociationProhibitsOperationException; import google.registry.flows.ExtensionManager; @@ -130,7 +130,6 @@ public final class DomainDeleteFlow implements TransactionalFlow { @Inject @TargetId String targetId; @Inject @Superuser boolean isSuperuser; @Inject DomainHistory.Builder historyBuilder; - @Inject DnsUtils dnsUtils; @Inject Trid trid; @Inject AsyncTaskEnqueuer asyncTaskEnqueuer; @Inject EppResponse.Builder responseBuilder; @@ -262,7 +261,7 @@ public final class DomainDeleteFlow implements TransactionalFlow { // If there's a pending transfer, the gaining client's autorenew billing // event and poll message will already have been deleted in // ResourceDeleteFlow since it's listed in serverApproveEntities. - dnsUtils.requestDomainDnsRefresh(existingDomain.getDomainName()); + requestDomainDnsRefresh(existingDomain.getDomainName()); entitiesToSave.add(newDomain, domainHistory); EntityChanges entityChanges = @@ -431,7 +430,7 @@ public final class DomainDeleteFlow implements TransactionalFlow { /** Domain to be deleted has subordinate hosts. */ static class DomainToDeleteHasHostsException extends AssociationProhibitsOperationException { - public DomainToDeleteHasHostsException() { + DomainToDeleteHasHostsException() { super("Domain to be deleted has subordinate hosts"); } } diff --git a/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java b/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java index 598d2e8ed..af4b95aad 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java @@ -14,6 +14,7 @@ package google.registry.flows.domain; +import static google.registry.dns.DnsUtils.requestDomainDnsRefresh; import static google.registry.flows.FlowUtils.createHistoryEntryId; import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; @@ -34,7 +35,6 @@ import static google.registry.util.DateTimeUtils.END_OF_TIME; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.net.InternetDomainName; -import google.registry.dns.DnsUtils; import google.registry.flows.EppException; import google.registry.flows.EppException.CommandUseErrorException; import google.registry.flows.EppException.StatusProhibitsOperationException; @@ -122,7 +122,6 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow { @Inject @TargetId String targetId; @Inject @Superuser boolean isSuperuser; @Inject DomainHistory.Builder historyBuilder; - @Inject DnsUtils dnsUtils; @Inject EppResponse.Builder responseBuilder; @Inject DomainPricingLogic pricingLogic; @Inject DomainRestoreRequestFlow() {} @@ -185,7 +184,7 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow { entitiesToSave.add(newDomain, domainHistory, autorenewEvent, autorenewPollMessage); tm().putAll(entitiesToSave.build()); tm().delete(existingDomain.getDeletePollMessage()); - dnsUtils.requestDomainDnsRefresh(existingDomain.getDomainName()); + requestDomainDnsRefresh(existingDomain.getDomainName()); return responseBuilder .setExtensions(createResponseExtensions(feesAndCredits, feeUpdate, isExpired)) .build(); @@ -305,14 +304,14 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow { /** Restore command cannot have other changes specified. */ static class RestoreCommandIncludesChangesException extends CommandUseErrorException { - public RestoreCommandIncludesChangesException() { + RestoreCommandIncludesChangesException() { super("Restore command cannot have other changes specified"); } } /** Domain is not eligible for restore. */ static class DomainNotEligibleForRestoreException extends StatusProhibitsOperationException { - public DomainNotEligibleForRestoreException() { + DomainNotEligibleForRestoreException() { super("Domain is not eligible for restore"); } } diff --git a/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java b/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java index 70df61193..846c0dd71 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java @@ -19,6 +19,7 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.ImmutableSortedSet.toImmutableSortedSet; import static com.google.common.collect.Sets.symmetricDifference; import static com.google.common.collect.Sets.union; +import static google.registry.dns.DnsUtils.requestDomainDnsRefresh; import static google.registry.flows.FlowUtils.persistEntityChanges; import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.checkSameValuesNotAddedAndRemoved; @@ -49,7 +50,6 @@ import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Ordering; import com.google.common.collect.Sets; import com.google.common.net.InternetDomainName; -import google.registry.dns.DnsUtils; import google.registry.flows.EppException; import google.registry.flows.ExtensionManager; import google.registry.flows.FlowModule.RegistrarId; @@ -156,7 +156,6 @@ public final class DomainUpdateFlow implements TransactionalFlow { @Inject @Superuser boolean isSuperuser; @Inject Trid trid; @Inject DomainHistory.Builder historyBuilder; - @Inject DnsUtils dnsUtils; @Inject EppResponse.Builder responseBuilder; @Inject DomainUpdateFlowCustomLogic flowCustomLogic; @Inject DomainPricingLogic pricingLogic; @@ -183,7 +182,7 @@ public final class DomainUpdateFlow implements TransactionalFlow { historyBuilder.setType(DOMAIN_UPDATE).setDomain(newDomain).build(); validateNewState(newDomain); if (requiresDnsUpdate(existingDomain, newDomain)) { - dnsUtils.requestDomainDnsRefresh(targetId); + requestDomainDnsRefresh(targetId); } ImmutableSet.Builder entitiesToSave = new ImmutableSet.Builder<>(); entitiesToSave.add(newDomain, domainHistory); @@ -207,7 +206,7 @@ public final class DomainUpdateFlow implements TransactionalFlow { } /** Determines if any of the changes to new domain should trigger DNS update. */ - private boolean requiresDnsUpdate(Domain existingDomain, Domain newDomain) { + private static boolean requiresDnsUpdate(Domain existingDomain, Domain newDomain) { return existingDomain.shouldPublishToDns() != newDomain.shouldPublishToDns() || !Objects.equals(newDomain.getDsData(), existingDomain.getDsData()) || !Objects.equals(newDomain.getNsHosts(), existingDomain.getNsHosts()); @@ -256,7 +255,7 @@ public final class DomainUpdateFlow implements TransactionalFlow { // We have to verify no duplicate contacts _before_ constructing the domain because it is // illegal to construct a domain with duplicate contacts. Sets.SetView newContacts = - Sets.union(Sets.difference(domain.getContacts(), remove.getContacts()), add.getContacts()); + union(Sets.difference(domain.getContacts(), remove.getContacts()), add.getContacts()); validateNoDuplicateContacts(newContacts); Domain.Builder domainBuilder = @@ -301,7 +300,7 @@ public final class DomainUpdateFlow implements TransactionalFlow { return domainBuilder.build(); } - private void validateRegistrantIsntBeingRemoved(Change change) throws EppException { + private static void validateRegistrantIsntBeingRemoved(Change change) throws EppException { if (change.getRegistrantContactId() != null && change.getRegistrantContactId().isEmpty()) { throw new MissingRegistrantException(); } @@ -314,7 +313,7 @@ public final class DomainUpdateFlow implements TransactionalFlow { * compliant with the additions or amendments, otherwise existing data can become invalid and * cause Domain update failure. */ - private void validateNewState(Domain newDomain) throws EppException { + private static void validateNewState(Domain newDomain) throws EppException { validateRequiredContactsPresent(newDomain.getRegistrant(), newDomain.getContacts()); validateDsData(newDomain.getDsData()); validateNameserversCountForTld( @@ -368,17 +367,17 @@ public final class DomainUpdateFlow implements TransactionalFlow { .collect(toImmutableSortedSet(Ordering.natural())); String msg; - if (addedServerStatuses.size() > 0 && removedServerStatuses.size() > 0) { + if (!addedServerStatuses.isEmpty() && !removedServerStatuses.isEmpty()) { msg = String.format( "The registry administrator has added the status(es) %s and removed the status(es)" + " %s.", addedServerStatuses, removedServerStatuses); - } else if (addedServerStatuses.size() > 0) { + } else if (!addedServerStatuses.isEmpty()) { msg = String.format( "The registry administrator has added the status(es) %s.", addedServerStatuses); - } else if (removedServerStatuses.size() > 0) { + } else if (!removedServerStatuses.isEmpty()) { msg = String.format( "The registry administrator has removed the status(es) %s.", removedServerStatuses); diff --git a/core/src/main/java/google/registry/flows/host/HostCreateFlow.java b/core/src/main/java/google/registry/flows/host/HostCreateFlow.java index 63bbcfbf3..34cad3a18 100644 --- a/core/src/main/java/google/registry/flows/host/HostCreateFlow.java +++ b/core/src/main/java/google/registry/flows/host/HostCreateFlow.java @@ -14,6 +14,7 @@ package google.registry.flows.host; +import static google.registry.dns.DnsUtils.requestHostDnsRefresh; import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.verifyResourceDoesNotExist; import static google.registry.flows.host.HostFlowUtils.lookupSuperordinateDomain; @@ -28,7 +29,6 @@ import static google.registry.util.CollectionUtils.isNullOrEmpty; import com.google.common.collect.ImmutableSet; import google.registry.config.RegistryConfig.Config; -import google.registry.dns.DnsUtils; import google.registry.flows.EppException; import google.registry.flows.EppException.ParameterValueRangeErrorException; import google.registry.flows.EppException.RequiredParameterMissingException; @@ -85,7 +85,6 @@ public final class HostCreateFlow implements TransactionalFlow { @Inject @RegistrarId String registrarId; @Inject @TargetId String targetId; @Inject HostHistory.Builder historyBuilder; - @Inject DnsUtils dnsUtils; @Inject EppResponse.Builder responseBuilder; @Inject @@ -138,7 +137,7 @@ public final class HostCreateFlow implements TransactionalFlow { .build()); // Only update DNS if this is a subordinate host. External hosts have no glue to write, so // they are only written as NS records from the referencing domain. - dnsUtils.requestHostDnsRefresh(targetId); + requestHostDnsRefresh(targetId); } tm().insertAll(entitiesToSave); return responseBuilder.setResData(HostCreateData.create(targetId, now)).build(); diff --git a/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java b/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java index 07b1a647a..d8e7db25e 100644 --- a/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java +++ b/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java @@ -14,6 +14,7 @@ package google.registry.flows.host; +import static google.registry.dns.DnsUtils.requestHostDnsRefresh; import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.checkLinkedDomains; import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; @@ -24,7 +25,6 @@ import static google.registry.model.eppoutput.Result.Code.SUCCESS; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.common.collect.ImmutableSet; -import google.registry.dns.DnsUtils; import google.registry.flows.EppException; import google.registry.flows.ExtensionManager; import google.registry.flows.FlowModule.RegistrarId; @@ -71,7 +71,6 @@ public final class HostDeleteFlow implements TransactionalFlow { StatusValue.PENDING_DELETE, StatusValue.SERVER_DELETE_PROHIBITED); - @Inject DnsUtils dnsUtils; @Inject ExtensionManager extensionManager; @Inject @RegistrarId String registrarId; @Inject @TargetId String targetId; @@ -104,7 +103,7 @@ public final class HostDeleteFlow implements TransactionalFlow { } Host newHost = existingHost.asBuilder().setStatusValues(null).setDeletionTime(now).build(); if (existingHost.isSubordinate()) { - dnsUtils.requestHostDnsRefresh(existingHost.getHostName()); + requestHostDnsRefresh(existingHost.getHostName()); tm().update( tm().loadByKey(existingHost.getSuperordinateDomain()) .asBuilder() diff --git a/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java b/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java index b459d5318..fdaeb78e0 100644 --- a/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java +++ b/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java @@ -16,6 +16,7 @@ package google.registry.flows.host; import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.collect.Sets.union; +import static google.registry.dns.DnsUtils.requestHostDnsRefresh; import static google.registry.dns.RefreshDnsOnHostRenameAction.PARAM_HOST_KEY; import static google.registry.dns.RefreshDnsOnHostRenameAction.QUEUE_HOST_RENAME; import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn; @@ -37,7 +38,6 @@ import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; import google.registry.batch.AsyncTaskEnqueuer; import google.registry.batch.CloudTasksUtils; -import google.registry.dns.DnsUtils; import google.registry.dns.RefreshDnsOnHostRenameAction; import google.registry.flows.EppException; import google.registry.flows.EppException.ObjectAlreadyExistsException; @@ -124,7 +124,6 @@ public final class HostUpdateFlow implements TransactionalFlow { @Inject @Superuser boolean isSuperuser; @Inject HostHistory.Builder historyBuilder; @Inject AsyncTaskEnqueuer asyncTaskEnqueuer; - @Inject DnsUtils dnsUtils; @Inject EppResponse.Builder responseBuilder; @Inject CloudTasksUtils cloudTasksUtils; @@ -241,7 +240,7 @@ public final class HostUpdateFlow implements TransactionalFlow { verifyNoDisallowedStatuses(existingHost, DISALLOWED_STATUSES); } - private void verifyHasIpsIffIsExternal(Update command, Host existingHost, Host newHost) + private static void verifyHasIpsIffIsExternal(Update command, Host existingHost, Host newHost) throws EppException { boolean wasSubordinate = existingHost.isSubordinate(); boolean willBeSubordinate = newHost.isSubordinate(); @@ -266,14 +265,14 @@ public final class HostUpdateFlow implements TransactionalFlow { // Only update DNS for subordinate hosts. External hosts have no glue to write, so they // are only written as NS records from the referencing domain. if (existingHost.isSubordinate()) { - dnsUtils.requestHostDnsRefresh(existingHost.getHostName()); + requestHostDnsRefresh(existingHost.getHostName()); } // In case of a rename, there are many updates we need to queue up. if (((Update) resourceCommand).getInnerChange().getHostName() != null) { // If the renamed host is also subordinate, then we must enqueue an update to write the new // glue. if (newHost.isSubordinate()) { - dnsUtils.requestHostDnsRefresh(newHost.getHostName()); + requestHostDnsRefresh(newHost.getHostName()); } // We must also enqueue updates for all domains that use this host as their nameserver so // that their NS records can be updated to point at the new name. @@ -317,14 +316,14 @@ public final class HostUpdateFlow implements TransactionalFlow { /** Host with specified name already exists. */ static class HostAlreadyExistsException extends ObjectAlreadyExistsException { - public HostAlreadyExistsException(String hostName) { + HostAlreadyExistsException(String hostName) { super(String.format("Object with given ID (%s) already exists", hostName)); } } /** Cannot add IP addresses to an external host. */ static class CannotAddIpToExternalHostException extends ParameterValueRangeErrorException { - public CannotAddIpToExternalHostException() { + CannotAddIpToExternalHostException() { super("Cannot add IP addresses to external hosts"); } } @@ -332,14 +331,14 @@ public final class HostUpdateFlow implements TransactionalFlow { /** Cannot remove all IP addresses from a subordinate host. */ static class CannotRemoveSubordinateHostLastIpException extends StatusProhibitsOperationException { - public CannotRemoveSubordinateHostLastIpException() { + CannotRemoveSubordinateHostLastIpException() { super("Cannot remove all IP addresses from a subordinate host"); } } /** Cannot rename an external host. */ static class CannotRenameExternalHostException extends StatusProhibitsOperationException { - public CannotRenameExternalHostException() { + CannotRenameExternalHostException() { super("Cannot rename an external host"); } } diff --git a/core/src/main/java/google/registry/model/common/DnsRefreshRequest.java b/core/src/main/java/google/registry/model/common/DnsRefreshRequest.java index c5717e757..b65ccae99 100644 --- a/core/src/main/java/google/registry/model/common/DnsRefreshRequest.java +++ b/core/src/main/java/google/registry/model/common/DnsRefreshRequest.java @@ -18,7 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static google.registry.util.DateTimeUtils.START_OF_TIME; -import google.registry.dns.DnsConstants.TargetType; +import google.registry.dns.DnsUtils.TargetType; import google.registry.dns.PublishDnsUpdatesAction; import google.registry.model.ImmutableObject; import google.registry.persistence.VKey; diff --git a/core/src/main/java/google/registry/module/backend/BackendRequestComponent.java b/core/src/main/java/google/registry/module/backend/BackendRequestComponent.java index b3175728e..5aba5714c 100644 --- a/core/src/main/java/google/registry/module/backend/BackendRequestComponent.java +++ b/core/src/main/java/google/registry/module/backend/BackendRequestComponent.java @@ -32,7 +32,6 @@ import google.registry.cron.CronModule; import google.registry.cron.TldFanoutAction; import google.registry.dns.DnsModule; import google.registry.dns.PublishDnsUpdatesAction; -import google.registry.dns.ReadDnsQueueAction; import google.registry.dns.ReadDnsRefreshRequestsAction; import google.registry.dns.RefreshDnsAction; import google.registry.dns.RefreshDnsOnHostRenameAction; @@ -143,8 +142,6 @@ interface BackendRequestComponent { PublishSpec11ReportAction publishSpec11ReportAction(); - ReadDnsQueueAction readDnsQueueAction(); - ReadDnsRefreshRequestsAction readDnsRefreshRequestsAction(); RdeReportAction rdeReportAction(); 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 b7fa2af7a..295945eb0 100644 --- a/core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java +++ b/core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java @@ -15,13 +15,13 @@ package google.registry.tools.server; import static com.google.common.base.Preconditions.checkArgument; +import static google.registry.dns.DnsUtils.requestDomainDnsRefresh; import static google.registry.model.tld.Registries.assertTldsExist; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.request.RequestParameters.PARAM_TLDS; import com.google.common.collect.ImmutableSet; import com.google.common.flogger.FluentLogger; -import google.registry.dns.DnsUtils; import google.registry.request.Action; import google.registry.request.Parameter; import google.registry.request.Response; @@ -43,10 +43,10 @@ import org.joda.time.Duration; * run internally, or by pretending to be internal by setting the X-AppEngine-QueueName header, * which only admin users can do. * - *

    You must pass in a number of smearMinutes as a URL parameter so that the DNS - * queue doesn't get overloaded. A rough rule of thumb for Cloud DNS is 1 minute per every 1,000 - * domains. This smears the updates out over the next N minutes. For small TLDs consisting of fewer - * than 1,000 domains, passing in 1 is fine (which will execute all the updates immediately). + *

    You must pass in a number of {@code smearMinutes} as a URL parameter so that the DNS queue + * doesn't get overloaded. A rough rule of thumb for Cloud DNS is 1 minute per every 1,000 domains. + * This smears the updates out over the next N minutes. For small TLDs consisting of fewer than + * 1,000 domains, passing in 1 is fine (which will execute all the updates immediately). */ @Action( service = Action.Service.TOOLS, @@ -66,7 +66,6 @@ public class RefreshDnsForAllDomainsAction implements Runnable { @Parameter("smearMinutes") int smearMinutes; - @Inject DnsUtils dnsUtils; @Inject Clock clock; @Inject Random random; @@ -91,7 +90,7 @@ public class RefreshDnsForAllDomainsAction implements Runnable { domainName -> { try { // Smear the task execution time over the next N minutes. - dnsUtils.requestDomainDnsRefresh( + requestDomainDnsRefresh( domainName, Duration.standardMinutes(random.nextInt(smearMinutes))); } catch (Throwable t) { logger.atSevere().withCause(t).log( diff --git a/core/src/test/java/google/registry/batch/DeleteExpiredDomainsActionTest.java b/core/src/test/java/google/registry/batch/DeleteExpiredDomainsActionTest.java index abb738807..6fe869496 100644 --- a/core/src/test/java/google/registry/batch/DeleteExpiredDomainsActionTest.java +++ b/core/src/test/java/google/registry/batch/DeleteExpiredDomainsActionTest.java @@ -43,7 +43,6 @@ import google.registry.testing.DatabaseHelper; import google.registry.testing.FakeClock; import google.registry.testing.FakeLockHandler; import google.registry.testing.FakeResponse; -import google.registry.testing.TaskQueueExtension; import java.util.Optional; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; @@ -59,8 +58,6 @@ class DeleteExpiredDomainsActionTest { final JpaIntegrationTestExtension jpa = new JpaTestExtensions.Builder().withClock(clock).buildIntegrationTestExtension(); - @RegisterExtension final TaskQueueExtension taskQueue = new TaskQueueExtension(); - private final FakeResponse response = new FakeResponse(); private DeleteExpiredDomainsAction action; diff --git a/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java b/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java index d757fe952..7e0f0bb41 100644 --- a/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java +++ b/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static com.google.common.truth.Truth8.assertThat; import static google.registry.model.EppResourceUtils.loadByForeignKey; +import static google.registry.testing.DatabaseHelper.assertDomainDnsRequests; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.loadByEntitiesIfPresent; import static google.registry.testing.DatabaseHelper.loadByEntity; @@ -30,11 +31,9 @@ import static google.registry.testing.DatabaseHelper.persistSimpleResource; import static google.registry.util.DateTimeUtils.END_OF_TIME; import static org.joda.time.DateTimeZone.UTC; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.Mockito.mock; import com.google.common.collect.ImmutableSet; import google.registry.config.RegistryEnvironment; -import google.registry.dns.DnsUtils; import google.registry.model.ImmutableObject; import google.registry.model.billing.BillingBase.Reason; import google.registry.model.billing.BillingEvent; @@ -47,7 +46,6 @@ import google.registry.model.tld.Tld.TldType; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; import google.registry.testing.DatabaseHelper; -import google.registry.testing.DnsUtilsHelper; import google.registry.testing.SystemPropertyExtension; import java.util.Optional; import java.util.Set; @@ -72,9 +70,6 @@ class DeleteProberDataActionTest { private DeleteProberDataAction action; - private final DnsUtils dnsUtils = mock(DnsUtils.class); - private final DnsUtilsHelper dnsUtilsHelper = new DnsUtilsHelper(dnsUtils); - @BeforeEach void beforeEach() { // Entities in these two should not be touched. @@ -99,7 +94,6 @@ class DeleteProberDataActionTest { private void resetAction() { action = new DeleteProberDataAction(); - action.dnsUtils = dnsUtils; action.isDryRun = false; action.tlds = ImmutableSet.of(); action.registryAdminRegistrarId = "TheRegistrar"; @@ -201,7 +195,7 @@ class DeleteProberDataActionTest { DateTime timeAfterDeletion = DateTime.now(UTC); assertThat(loadByForeignKey(Domain.class, "blah.ib-any.test", timeAfterDeletion)).isEmpty(); assertThat(loadByEntity(domain).getDeletionTime()).isLessThan(timeAfterDeletion); - dnsUtilsHelper.assertDomainDnsRequests("blah.ib-any.test"); + assertDomainDnsRequests("blah.ib-any.test"); } @Test @@ -218,7 +212,7 @@ class DeleteProberDataActionTest { action.run(); assertThat(loadByForeignKey(Domain.class, "blah.ib-any.test", timeAfterDeletion)).isEmpty(); assertThat(loadByEntity(domain).getDeletionTime()).isLessThan(timeAfterDeletion); - dnsUtilsHelper.assertDomainDnsRequests("blah.ib-any.test"); + assertDomainDnsRequests("blah.ib-any.test"); } @Test diff --git a/core/src/test/java/google/registry/dns/DnsInjectionTest.java b/core/src/test/java/google/registry/dns/DnsInjectionTest.java index 777d1e317..37dd60245 100644 --- a/core/src/test/java/google/registry/dns/DnsInjectionTest.java +++ b/core/src/test/java/google/registry/dns/DnsInjectionTest.java @@ -15,11 +15,12 @@ package google.registry.dns; import static com.google.common.truth.Truth.assertThat; +import static google.registry.testing.DatabaseHelper.assertHostDnsRequests; +import static google.registry.testing.DatabaseHelper.assertNoDnsRequests; +import static google.registry.testing.DatabaseHelper.assertNoDnsRequestsExcept; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.persistActiveDomain; import static google.registry.testing.DatabaseHelper.persistActiveSubordinateHost; -import static google.registry.testing.TaskQueueHelper.assertDnsTasksEnqueued; -import static google.registry.testing.TaskQueueHelper.assertNoDnsTasksEnqueued; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -30,7 +31,6 @@ import google.registry.request.HttpException.NotFoundException; import google.registry.request.RequestModule; import google.registry.testing.CloudTasksHelper.CloudTasksHelperModule; import google.registry.testing.FakeClock; -import google.registry.testing.TaskQueueExtension; import java.io.PrintWriter; import java.io.StringWriter; import javax.servlet.http.HttpServletRequest; @@ -43,18 +43,15 @@ import org.junit.jupiter.api.extension.RegisterExtension; /** Unit tests for Dagger injection of the DNS package. */ public final class DnsInjectionTest { - @RegisterExtension - final JpaIntegrationTestExtension jpa = - new JpaTestExtensions.Builder().buildIntegrationTestExtension(); - - @RegisterExtension final TaskQueueExtension taskQueue = new TaskQueueExtension(); - private final HttpServletRequest req = mock(HttpServletRequest.class); private final HttpServletResponse rsp = mock(HttpServletResponse.class); private final StringWriter httpOutput = new StringWriter(); private final FakeClock clock = new FakeClock(DateTime.parse("2014-01-01TZ")); private DnsTestComponent component; - private DnsQueue dnsQueue; + + @RegisterExtension + final JpaIntegrationTestExtension jpa = + new JpaTestExtensions.Builder().withClock(clock).buildIntegrationTestExtension(); @BeforeEach void beforeEach() throws Exception { @@ -64,18 +61,18 @@ public final class DnsInjectionTest { .requestModule(new RequestModule(req, rsp)) .cloudTasksHelperModule(new CloudTasksHelperModule(clock)) .build(); - dnsQueue = component.dnsQueue(); createTld("lol"); } @Test - void testReadDnsQueueAction_injectsAndWorks() { + void testReadDnsRefreshRequestsAction_injectsAndWorks() { persistActiveSubordinateHost("ns1.example.lol", persistActiveDomain("example.lol")); clock.advanceOneMilli(); - dnsQueue.addDomainRefreshTask("example.lol"); + DnsUtils.requestDomainDnsRefresh("example.lol"); when(req.getParameter("tld")).thenReturn("lol"); - component.readDnsQueueAction().run(); - assertNoDnsTasksEnqueued(); + clock.advanceOneMilli(); + component.readDnsRefreshRequestsAction().run(); + assertNoDnsRequests(); } @Test @@ -84,7 +81,7 @@ public final class DnsInjectionTest { when(req.getParameter("type")).thenReturn("domain"); when(req.getParameter("name")).thenReturn("example.lol"); component.refreshDns().run(); - assertDnsTasksEnqueued("example.lol"); + assertNoDnsRequestsExcept("example.lol"); } @Test @@ -102,7 +99,7 @@ public final class DnsInjectionTest { when(req.getParameter("type")).thenReturn("host"); when(req.getParameter("name")).thenReturn("ns1.example.lol"); component.refreshDns().run(); - assertDnsTasksEnqueued("ns1.example.lol"); + assertHostDnsRequests("ns1.example.lol"); } @Test diff --git a/core/src/test/java/google/registry/dns/DnsQueueTest.java b/core/src/test/java/google/registry/dns/DnsQueueTest.java deleted file mode 100644 index c2112a6cf..000000000 --- a/core/src/test/java/google/registry/dns/DnsQueueTest.java +++ /dev/null @@ -1,108 +0,0 @@ -// 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.dns; - -import static com.google.common.truth.Truth.assertThat; -import static google.registry.testing.DatabaseHelper.createTld; -import static google.registry.testing.TaskQueueHelper.assertNoTasksEnqueued; -import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued; -import static org.junit.jupiter.api.Assertions.assertThrows; - -import google.registry.persistence.transaction.JpaTestExtensions; -import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; -import google.registry.testing.FakeClock; -import google.registry.testing.TaskQueueExtension; -import google.registry.testing.TaskQueueHelper.TaskMatcher; -import org.joda.time.DateTime; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; - -/** Unit tests for {@link DnsQueue}. */ -public class DnsQueueTest { - - @RegisterExtension - final JpaIntegrationTestExtension jpa = - new JpaTestExtensions.Builder().buildIntegrationTestExtension(); - - @RegisterExtension final TaskQueueExtension taskQueue = new TaskQueueExtension(); - - private DnsQueue dnsQueue; - private final FakeClock clock = new FakeClock(DateTime.parse("2010-01-01T10:00:00Z")); - - @BeforeEach - void beforeEach() { - dnsQueue = DnsQueue.createForTesting(clock); - dnsQueue.leaseTasksBatchSize = 10; - } - - @Test - void test_addHostRefreshTask_success() { - createTld("tld"); - dnsQueue.addHostRefreshTask("octopus.tld"); - assertTasksEnqueued( - "dns-pull", - new TaskMatcher() - .param("Target-Type", "HOST") - .param("Target-Name", "octopus.tld") - .param("Create-Time", "2010-01-01T10:00:00.000Z") - .param("tld", "tld")); - } - - @Test - void test_addHostRefreshTask_failsOnUnknownTld() { - IllegalArgumentException thrown = - assertThrows( - IllegalArgumentException.class, - () -> { - try { - dnsQueue.addHostRefreshTask("octopus.notatld"); - } finally { - assertNoTasksEnqueued("dns-pull"); - } - }); - assertThat(thrown) - .hasMessageThat() - .contains("octopus.notatld is not a subordinate host to a known tld"); - } - - @Test - void test_addDomainRefreshTask_success() { - createTld("tld"); - dnsQueue.addDomainRefreshTask("octopus.tld"); - assertTasksEnqueued( - "dns-pull", - new TaskMatcher() - .param("Target-Type", "DOMAIN") - .param("Target-Name", "octopus.tld") - .param("Create-Time", "2010-01-01T10:00:00.000Z") - .param("tld", "tld")); - } - - @Test - void test_addDomainRefreshTask_failsOnUnknownTld() { - IllegalArgumentException thrown = - assertThrows( - IllegalArgumentException.class, - () -> { - try { - dnsQueue.addDomainRefreshTask("fake.notatld"); - } finally { - assertNoTasksEnqueued("dns-pull"); - } - }); - assertThat(thrown).hasMessageThat().contains("TLD notatld does not exist"); - } -} diff --git a/core/src/test/java/google/registry/dns/DnsTestComponent.java b/core/src/test/java/google/registry/dns/DnsTestComponent.java index 81c8020c3..b0e1baada 100644 --- a/core/src/test/java/google/registry/dns/DnsTestComponent.java +++ b/core/src/test/java/google/registry/dns/DnsTestComponent.java @@ -35,7 +35,7 @@ import javax.inject.Singleton; VoidDnsWriterModule.class, }) interface DnsTestComponent { - DnsQueue dnsQueue(); RefreshDnsAction refreshDns(); - ReadDnsQueueAction readDnsQueueAction(); + + ReadDnsRefreshRequestsAction readDnsRefreshRequestsAction(); } diff --git a/core/src/test/java/google/registry/dns/DnsUtilsTest.java b/core/src/test/java/google/registry/dns/DnsUtilsTest.java index 778ebc0e9..e1a8c16a3 100644 --- a/core/src/test/java/google/registry/dns/DnsUtilsTest.java +++ b/core/src/test/java/google/registry/dns/DnsUtilsTest.java @@ -20,20 +20,10 @@ import static google.registry.persistence.transaction.TransactionManagerFactory. import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.loadAllOf; import static google.registry.util.DateTimeUtils.START_OF_TIME; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Iterables; -import com.google.common.collect.Ordering; -import google.registry.dns.DnsConstants.TargetType; -import google.registry.model.common.DatabaseMigrationStateSchedule; -import google.registry.model.common.DatabaseMigrationStateSchedule.MigrationState; +import google.registry.dns.DnsUtils.TargetType; import google.registry.model.common.DnsRefreshRequest; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; @@ -42,7 +32,6 @@ import java.util.Comparator; import org.joda.time.DateTime; import org.joda.time.Duration; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -54,52 +43,22 @@ public class DnsUtilsTest { private static final String domainName = "test.tld"; private static final String hostName = "ns1.test.tld"; - private final DnsQueue dnsQueue = mock(DnsQueue.class); - private final DnsUtils dnsUtils = new DnsUtils(dnsQueue); private final FakeClock clock = new FakeClock(DateTime.parse("2020-02-02T01:23:45Z")); @RegisterExtension JpaIntegrationTestExtension jpa = new JpaTestExtensions.Builder().withClock(clock).buildIntegrationTestExtension(); - @BeforeAll - static void beforeAll() { - DatabaseMigrationStateSchedule.useUncachedForTest(); - } - @BeforeEach void beforeEach() { createTld(tld); - when(dnsQueue.getClock()).thenReturn(clock); - } - - @Test - void testSuccess_hostRefresh_pullQueue() { - dnsUtils.requestHostDnsRefresh(hostName); - verify(dnsQueue).addHostRefreshTask(hostName); - assertThat(loadAllOf(DnsRefreshRequest.class)).isEmpty(); - } - - @Test - void testSuccess_domainRefresh_pullQueue() { - dnsUtils.requestDomainDnsRefresh(domainName); - verify(dnsQueue).addDomainRefreshTask(domainName, Duration.ZERO); - assertThat(loadAllOf(DnsRefreshRequest.class)).isEmpty(); - } - - @Test - void testSuccess_domainRefreshWithDelay_pullQueue() { - dnsUtils.requestDomainDnsRefresh(domainName, Duration.standardMinutes(3)); - verify(dnsQueue).addDomainRefreshTask(domainName, Duration.standardMinutes(3)); - assertThat(loadAllOf(DnsRefreshRequest.class)).isEmpty(); } @Test void testFailure_hostRefresh_unmanagedHost() { String unmanagedHostName = "ns1.another.example"; Assertions.assertThrows( - IllegalArgumentException.class, () -> dnsUtils.requestHostDnsRefresh(unmanagedHostName)); - verify(dnsQueue, never()).addHostRefreshTask(anyString()); + IllegalArgumentException.class, () -> DnsUtils.requestHostDnsRefresh(unmanagedHostName)); assertThat(loadAllOf(DnsRefreshRequest.class)).isEmpty(); } @@ -108,34 +67,27 @@ public class DnsUtilsTest { String unmanagedDomainName = "another.example"; Assertions.assertThrows( IllegalArgumentException.class, - () -> dnsUtils.requestDomainDnsRefresh(unmanagedDomainName)); - verify(dnsQueue, never()).addDomainRefreshTask(anyString(), any(Duration.class)); + () -> DnsUtils.requestDomainDnsRefresh(unmanagedDomainName)); assertThat(loadAllOf(DnsRefreshRequest.class)).isEmpty(); } @Test void testSuccess_hostRefresh() { - useDnsSql(); - dnsUtils.requestHostDnsRefresh(hostName); - verify(dnsQueue, never()).addHostRefreshTask(anyString()); + DnsUtils.requestHostDnsRefresh(hostName); DnsRefreshRequest request = Iterables.getOnlyElement(loadAllOf(DnsRefreshRequest.class)); assertRequest(request, TargetType.HOST, hostName, tld, clock.nowUtc()); } @Test void testSuccess_domainRefresh() { - useDnsSql(); - dnsUtils.requestDomainDnsRefresh(domainName); - verify(dnsQueue, never()).addDomainRefreshTask(anyString(), any(Duration.class)); + DnsUtils.requestDomainDnsRefresh(domainName); DnsRefreshRequest request = Iterables.getOnlyElement(loadAllOf(DnsRefreshRequest.class)); assertRequest(request, TargetType.DOMAIN, domainName, tld, clock.nowUtc()); } @Test void testSuccess_domainRefreshWithDelay() { - useDnsSql(); - dnsUtils.requestDomainDnsRefresh(domainName, Duration.standardMinutes(3)); - verify(dnsQueue, never()).addDomainRefreshTask(anyString(), any(Duration.class)); + DnsUtils.requestDomainDnsRefresh(domainName, Duration.standardMinutes(3)); DnsRefreshRequest request = Iterables.getOnlyElement(loadAllOf(DnsRefreshRequest.class)); assertRequest(request, TargetType.DOMAIN, domainName, tld, clock.nowUtc().plusMinutes(3)); } @@ -182,7 +134,7 @@ public class DnsUtilsTest { // Requests within cooldown period not included. requests = - dnsUtils.readAndUpdateRequestsWithLatestProcessTime("tld", Duration.standardMinutes(1), 4); + DnsUtils.readAndUpdateRequestsWithLatestProcessTime("tld", Duration.standardMinutes(1), 4); assertThat(requests.size()).isEqualTo(1); assertRequest( requests.get(0), @@ -195,7 +147,7 @@ public class DnsUtilsTest { @Test void testSuccess_deleteRequests() { - dnsUtils.deleteRequests(processRequests()); + DnsUtils.deleteRequests(processRequests()); ImmutableList remainingRequests = loadAllOf(DnsRefreshRequest.class).stream() .sorted(Comparator.comparing(DnsRefreshRequest::getRequestTime)) @@ -222,31 +174,30 @@ public class DnsUtilsTest { tm().transact(() -> tm().delete(remainingRequests.get(2))); assertThat(loadAllOf(DnsRefreshRequest.class).size()).isEqualTo(2); // Should not throw even though one of the request is already deleted. - dnsUtils.deleteRequests(remainingRequests); + DnsUtils.deleteRequests(remainingRequests); assertThat(loadAllOf(DnsRefreshRequest.class).size()).isEqualTo(0); } private ImmutableList processRequests() { - useDnsSql(); createTld("example"); // Domain Included. - dnsUtils.requestDomainDnsRefresh("test1.tld", Duration.standardMinutes(1)); + DnsUtils.requestDomainDnsRefresh("test1.tld", Duration.standardMinutes(1)); // This one should be returned before test1.tld, even though it's added later, because of // the delay specified in test1.tld. - dnsUtils.requestDomainDnsRefresh("test2.tld"); + DnsUtils.requestDomainDnsRefresh("test2.tld"); // Not included because the TLD is not under management. - dnsUtils.requestDomainDnsRefresh("something.example", Duration.standardMinutes(2)); + DnsUtils.requestDomainDnsRefresh("something.example", Duration.standardMinutes(2)); clock.advanceBy(Duration.standardMinutes(3)); // Host included. - dnsUtils.requestHostDnsRefresh("ns1.test2.tld"); + DnsUtils.requestHostDnsRefresh("ns1.test2.tld"); // Not included because the request time is in the future - dnsUtils.requestDomainDnsRefresh("test4.tld", Duration.standardMinutes(2)); + DnsUtils.requestDomainDnsRefresh("test4.tld", Duration.standardMinutes(2)); // Included after the previous one. Same request time, order by insertion order (i.e. ID); - dnsUtils.requestDomainDnsRefresh("test5.tld"); + DnsUtils.requestDomainDnsRefresh("test5.tld"); // Not included because batch size is exceeded; - dnsUtils.requestDomainDnsRefresh("test6.tld"); + DnsUtils.requestDomainDnsRefresh("test6.tld"); clock.advanceBy(Duration.standardMinutes(1)); - return dnsUtils.readAndUpdateRequestsWithLatestProcessTime( + return DnsUtils.readAndUpdateRequestsWithLatestProcessTime( "tld", Duration.standardMinutes(1), 4); } @@ -268,26 +219,4 @@ public class DnsUtilsTest { assertThat(request.getRequestTime()).isEqualTo(requestTime); assertThat(request.getLastProcessTime()).isEqualTo(processTime); } - - private void useDnsSql() { - DateTime currentTime = clock.nowUtc(); - clock.setTo(START_OF_TIME); - tm().transact( - () -> - DatabaseMigrationStateSchedule.set( - new ImmutableSortedMap.Builder(Ordering.natural()) - .put(START_OF_TIME, MigrationState.DATASTORE_ONLY) - .put(START_OF_TIME.plusMillis(1), MigrationState.DATASTORE_PRIMARY) - .put(START_OF_TIME.plusMillis(2), MigrationState.DATASTORE_PRIMARY_NO_ASYNC) - .put( - START_OF_TIME.plusMillis(3), MigrationState.DATASTORE_PRIMARY_READ_ONLY) - .put(START_OF_TIME.plusMillis(4), MigrationState.SQL_PRIMARY_READ_ONLY) - .put(START_OF_TIME.plusMillis(5), MigrationState.SQL_PRIMARY) - .put(START_OF_TIME.plusMillis(6), MigrationState.SQL_ONLY) - .put(START_OF_TIME.plusMillis(7), MigrationState.SEQUENCE_BASED_ALLOCATE_ID) - .put(START_OF_TIME.plusMillis(8), MigrationState.NORDN_SQL) - .put(START_OF_TIME.plusMillis(9), MigrationState.DNS_SQL) - .build())); - clock.setTo(currentTime); - } } diff --git a/core/src/test/java/google/registry/dns/PublishDnsUpdatesActionTest.java b/core/src/test/java/google/registry/dns/PublishDnsUpdatesActionTest.java index 39b8aef18..571fd0a09 100644 --- a/core/src/test/java/google/registry/dns/PublishDnsUpdatesActionTest.java +++ b/core/src/test/java/google/registry/dns/PublishDnsUpdatesActionTest.java @@ -15,7 +15,6 @@ package google.registry.dns; import static com.google.common.truth.Truth.assertThat; -import static google.registry.dns.DnsConstants.DNS_PUBLISH_PUSH_QUEUE_NAME; 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; @@ -23,8 +22,13 @@ 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_TIME; +import static google.registry.dns.DnsUtils.DNS_PUBLISH_PUSH_QUEUE_NAME; import static google.registry.dns.PublishDnsUpdatesAction.RETRIES_BEFORE_PERMANENT_FAILURE; import static google.registry.request.RequestParameters.PARAM_TLD; +import static google.registry.testing.DatabaseHelper.assertDomainDnsRequests; +import static google.registry.testing.DatabaseHelper.assertHostDnsRequests; +import static google.registry.testing.DatabaseHelper.assertNoDnsRequests; +import static google.registry.testing.DatabaseHelper.assertNoDnsRequestsExcept; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.persistActiveDomain; import static google.registry.testing.DatabaseHelper.persistActiveSubordinateHost; @@ -54,7 +58,6 @@ import google.registry.request.HttpException.ServiceUnavailableException; import google.registry.request.lock.LockHandler; import google.registry.testing.CloudTasksHelper; import google.registry.testing.CloudTasksHelper.TaskMatcher; -import google.registry.testing.DnsUtilsHelper; import google.registry.testing.FakeClock; import google.registry.testing.FakeLockHandler; import google.registry.testing.FakeResponse; @@ -83,8 +86,6 @@ public class PublishDnsUpdatesActionTest { private final FakeLockHandler lockHandler = new FakeLockHandler(true); private final DnsWriter dnsWriter = mock(DnsWriter.class); private final DnsMetrics dnsMetrics = mock(DnsMetrics.class); - private final DnsUtils dnsUtils = mock(DnsUtils.class); - private final DnsUtilsHelper dnsUtilsHelper = new DnsUtilsHelper(dnsUtils); private final CloudTasksHelper cloudTasksHelper = new CloudTasksHelper(); private PublishDnsUpdatesAction action; private InternetAddress outgoingRegistry; @@ -95,6 +96,7 @@ public class PublishDnsUpdatesActionTest { @BeforeEach void beforeEach() throws Exception { createTld("xn--q9jyb4c"); + createTld("com"); outgoingRegistry = new InternetAddress("outgoing@registry.example"); registrySupportEmail = Lazies.of(new InternetAddress("registry@test.com")); registryCcEmail = Lazies.of(new InternetAddress("registry-cc@test.com")); @@ -161,7 +163,6 @@ public class PublishDnsUpdatesActionTest { outgoingRegistry, Optional.ofNullable(retryCount), Optional.empty(), - dnsUtils, new DnsWriterProxy(ImmutableMap.of("correctWriter", dnsWriter)), dnsMetrics, lockHandler, @@ -195,7 +196,7 @@ public class PublishDnsUpdatesActionTest { Duration.standardHours(2), Duration.standardHours(1)); verifyNoMoreInteractions(dnsMetrics); - dnsUtilsHelper.assertNoMoreDnsRequests(); + assertNoDnsRequests(); assertThat(response.getStatus()).isEqualTo(SC_OK); } @@ -222,7 +223,7 @@ public class PublishDnsUpdatesActionTest { Duration.standardHours(2), Duration.standardHours(1)); verifyNoMoreInteractions(dnsMetrics); - dnsUtilsHelper.assertNoMoreDnsRequests(); + assertNoDnsRequests(); assertThat(response.getStatus()).isEqualTo(SC_OK); } @@ -275,7 +276,7 @@ public class PublishDnsUpdatesActionTest { Duration.standardHours(2), Duration.standardHours(1)); verifyNoMoreInteractions(dnsMetrics); - dnsUtilsHelper.assertNoMoreDnsRequests(); + assertNoDnsRequests(); } @Test @@ -495,7 +496,7 @@ public class PublishDnsUpdatesActionTest { Duration.standardHours(2), Duration.standardHours(1)); verifyNoMoreInteractions(dnsMetrics); - dnsUtilsHelper.assertNoMoreDnsRequests(); + assertNoDnsRequests(); } @Test @@ -525,7 +526,7 @@ public class PublishDnsUpdatesActionTest { Duration.standardHours(2), Duration.standardHours(1)); verifyNoMoreInteractions(dnsMetrics); - dnsUtilsHelper.assertNoMoreDnsRequests(); + assertNoDnsRequests(); } @Test @@ -553,7 +554,7 @@ public class PublishDnsUpdatesActionTest { Duration.standardHours(2), Duration.standardHours(1)); verifyNoMoreInteractions(dnsMetrics); - dnsUtilsHelper.assertNoMoreDnsRequests(); + assertNoDnsRequests(); } @Test @@ -579,9 +580,9 @@ public class PublishDnsUpdatesActionTest { Duration.standardHours(2), Duration.standardHours(1)); verifyNoMoreInteractions(dnsMetrics); - dnsUtilsHelper.assertDomainDnsRequests("example.com"); - dnsUtilsHelper.assertHostDnsRequests("ns1.example.com"); - dnsUtilsHelper.assertNoMoreDnsRequests(); + assertDomainDnsRequests("example.com"); + assertHostDnsRequests("ns1.example.com"); + assertNoDnsRequestsExcept("example.com", "ns1.example.com"); } @Test @@ -607,9 +608,9 @@ public class PublishDnsUpdatesActionTest { Duration.standardHours(2), Duration.standardHours(1)); verifyNoMoreInteractions(dnsMetrics); - dnsUtilsHelper.assertDomainDnsRequests("example.com"); - dnsUtilsHelper.assertHostDnsRequests("ns1.example.com"); - dnsUtilsHelper.assertNoMoreDnsRequests(); + assertDomainDnsRequests("example.com"); + assertHostDnsRequests("ns1.example.com"); + assertNoDnsRequestsExcept("example.com", "ns1.example.com"); } @Test @@ -631,11 +632,12 @@ public class PublishDnsUpdatesActionTest { Duration.standardHours(2), Duration.standardHours(1)); verifyNoMoreInteractions(dnsMetrics); - dnsUtilsHelper.assertDomainDnsRequests("example.com"); - dnsUtilsHelper.assertDomainDnsRequests("example2.com"); - dnsUtilsHelper.assertHostDnsRequests("ns1.example.com"); - dnsUtilsHelper.assertHostDnsRequests("ns2.example.com"); - dnsUtilsHelper.assertHostDnsRequests("ns1.example2.com"); - dnsUtilsHelper.assertNoMoreDnsRequests(); + assertDomainDnsRequests("example.com"); + assertDomainDnsRequests("example2.com"); + assertHostDnsRequests("ns1.example.com"); + assertHostDnsRequests("ns2.example.com"); + assertHostDnsRequests("ns1.example2.com"); + assertNoDnsRequestsExcept( + "example.com", "example2.com", "ns1.example.com", "ns2.example.com", "ns1.example2.com"); } } diff --git a/core/src/test/java/google/registry/dns/ReadDnsQueueActionTest.java b/core/src/test/java/google/registry/dns/ReadDnsQueueActionTest.java deleted file mode 100644 index 22d448665..000000000 --- a/core/src/test/java/google/registry/dns/ReadDnsQueueActionTest.java +++ /dev/null @@ -1,500 +0,0 @@ -// 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.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.truth.Truth.assertThat; -import static com.google.common.truth.Truth8.assertThat; -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.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.request.RequestParameters.PARAM_TLD; -import static google.registry.testing.DatabaseHelper.createTlds; -import static google.registry.testing.DatabaseHelper.persistResource; - -import com.google.appengine.api.taskqueue.QueueFactory; -import com.google.appengine.api.taskqueue.TaskOptions; -import com.google.cloud.tasks.v2.HttpMethod; -import com.google.cloud.tasks.v2.Task; -import com.google.common.base.Splitter; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMultimap; -import com.google.common.collect.ImmutableSet; -import com.google.common.hash.Hashing; -import com.google.common.net.InternetDomainName; -import google.registry.dns.DnsConstants.TargetType; -import google.registry.model.tld.Tld; -import google.registry.model.tld.Tld.TldType; -import google.registry.persistence.transaction.JpaTestExtensions; -import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; -import google.registry.testing.CloudTasksHelper; -import google.registry.testing.CloudTasksHelper.TaskMatcher; -import google.registry.testing.FakeClock; -import google.registry.testing.TaskQueueExtension; -import google.registry.testing.TaskQueueHelper; -import google.registry.testing.UriParameters; -import java.nio.charset.StandardCharsets; -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; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.extension.RegisterExtension; -import org.junitpioneer.jupiter.RetryingTest; - -/** Unit tests for {@link ReadDnsQueueAction}. */ -public class ReadDnsQueueActionTest { - - private static final int TEST_TLD_UPDATE_BATCH_SIZE = 100; - private DnsQueue dnsQueue; - // Because of a bug in the queue test environment - b/73372999 - we must set the fake date of the - // test in the future. Set to year 3000 so it'll remain in the future for a very long time. - private final FakeClock clock = new FakeClock(DateTime.parse("3000-01-01TZ")); - private final CloudTasksHelper cloudTasksHelper = new CloudTasksHelper(clock); - - @RegisterExtension - final JpaIntegrationTestExtension jpa = - new JpaTestExtensions.Builder().withClock(clock).buildIntegrationTestExtension(); - - @RegisterExtension final TaskQueueExtension taskQueue = new TaskQueueExtension(); - - @BeforeEach - void beforeEach() { - // Because of b/73372999 - the FakeClock can't be in the past, or the TaskQueues stop working. - // To make sure it's never in the past, we set the date far-far into the future - clock.setTo(DateTime.parse("3000-01-01TZ")); - createTlds("com", "net", "example", "multilock.uk"); - persistResource(Tld.get("com").asBuilder().setDnsWriters(ImmutableSet.of("comWriter")).build()); - persistResource(Tld.get("net").asBuilder().setDnsWriters(ImmutableSet.of("netWriter")).build()); - persistResource( - Tld.get("example") - .asBuilder() - .setTldType(TldType.TEST) - .setDnsWriters(ImmutableSet.of("exampleWriter")) - .build()); - persistResource( - Tld.get("multilock.uk") - .asBuilder() - .setNumDnsPublishLocks(1000) - .setDnsWriters(ImmutableSet.of("multilockWriter")) - .build()); - dnsQueue = DnsQueue.createForTesting(clock); - } - - private void run() { - ReadDnsQueueAction action = - new ReadDnsQueueAction( - TEST_TLD_UPDATE_BATCH_SIZE, - Duration.standardSeconds(10), - Optional.empty(), - clock, - dnsQueue, - Hashing.murmur3_32(), - cloudTasksHelper.getTestCloudTasksUtils()); - // Advance the time a little, to ensure that leaseTasks() returns all tasks. - clock.advanceBy(Duration.standardHours(1)); - action.run(); - } - - private static TaskOptions createRefreshTask(String name, TargetType type) { - TaskOptions options = - TaskOptions.Builder.withMethod(TaskOptions.Method.PULL) - .param(DNS_TARGET_TYPE_PARAM, type.toString()) - .param(DNS_TARGET_NAME_PARAM, name) - .param(DNS_TARGET_CREATE_TIME_PARAM, "3000-01-01TZ"); - String tld = InternetDomainName.from(name).parts().reverse().get(0); - return options.param("tld", tld); - } - - private static TaskQueueHelper.TaskMatcher createDomainRefreshTaskMatcher(String name) { - return new TaskQueueHelper.TaskMatcher() - .param(DNS_TARGET_NAME_PARAM, name) - .param(DNS_TARGET_TYPE_PARAM, TargetType.DOMAIN.toString()); - } - - private void assertTldsEnqueuedInPushQueue(ImmutableMultimap tldsToDnsWriters) { - // By default, the publishDnsUpdates tasks will be enqueued one hour after the update items were - // created in the pull queue. This is because of the clock.advanceBy in run() - cloudTasksHelper.assertTasksEnqueued( - DNS_PUBLISH_PUSH_QUEUE_NAME, - transform( - tldsToDnsWriters.entries().asList(), - (Entry tldToDnsWriter) -> - new TaskMatcher() - .url(PublishDnsUpdatesAction.PATH) - .param("tld", tldToDnsWriter.getKey()) - .param("dnsWriter", tldToDnsWriter.getValue()) - .param("requestTime", "3000-01-01T00:00:00.000Z") - .param("enqueued", "3000-01-01T01:00:00.000Z") - // Single-lock TLDs should use lock 1 of 1 by default - .param("lockIndex", "1") - .param("numPublishLocks", "1") - .header("content-type", "application/x-www-form-urlencoded"))); - } - - @RetryingTest(4) - void testSuccess_methodPostIsDefault() { - dnsQueue.addDomainRefreshTask("domain.com"); - dnsQueue.addDomainRefreshTask("domain.net"); - dnsQueue.addDomainRefreshTask("domain.example"); - - run(); - - TaskQueueHelper.assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); - cloudTasksHelper.assertTasksEnqueued( - DNS_PUBLISH_PUSH_QUEUE_NAME, - new TaskMatcher().method(HttpMethod.POST), - new TaskMatcher().method(HttpMethod.POST), - new TaskMatcher().method(HttpMethod.POST)); - } - - @RetryingTest(4) - void testSuccess_allSingleLockTlds() { - dnsQueue.addDomainRefreshTask("domain.com"); - dnsQueue.addDomainRefreshTask("domain.net"); - dnsQueue.addDomainRefreshTask("domain.example"); - - run(); - - TaskQueueHelper.assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); - assertTldsEnqueuedInPushQueue( - ImmutableMultimap.of("com", "comWriter", "net", "netWriter", "example", "exampleWriter")); - } - - @RetryingTest(4) - void testSuccess_moreUpdatesThanQueueBatchSize() { - // The task queue has a batch size of 1000 (that's the maximum number of items you can lease at - // once). - ImmutableList domains = - IntStream.range(0, 1500) - .mapToObj(i -> String.format("domain_%04d.com", i)) - .collect(toImmutableList()); - domains.forEach(dnsQueue::addDomainRefreshTask); - - run(); - - TaskQueueHelper.assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); - ImmutableList queuedTasks = - ImmutableList.copyOf(cloudTasksHelper.getTestTasksFor(DNS_PUBLISH_PUSH_QUEUE_NAME)); - // ReadDnsQueueAction batches items per TLD in batches of size 100. - // So for 1500 items in the DNS queue, we expect 15 items in the push queue - assertThat(queuedTasks).hasSize(15); - // Check all the expected domains are indeed enqueued - assertThat( - queuedTasks.stream() - .flatMap( - task -> - UriParameters.parse( - task.getAppEngineHttpRequest() - .getBody() - .toString(StandardCharsets.UTF_8)) - .get("domains") - .stream()) - .flatMap(values -> Splitter.on(',').splitToStream(values))) - .containsExactlyElementsIn(domains); - } - - @RetryingTest(4) - void testSuccess_twoDnsWriters() { - persistResource( - Tld.get("com") - .asBuilder() - .setDnsWriters(ImmutableSet.of("comWriter", "otherWriter")) - .build()); - dnsQueue.addDomainRefreshTask("domain.com"); - - run(); - - TaskQueueHelper.assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); - assertTldsEnqueuedInPushQueue(ImmutableMultimap.of("com", "comWriter", "com", "otherWriter")); - } - - @RetryingTest(4) - void testSuccess_differentUpdateTimes_usesMinimum() { - clock.setTo(DateTime.parse("3000-02-03TZ")); - dnsQueue.addDomainRefreshTask("domain1.com"); - clock.setTo(DateTime.parse("3000-02-04TZ")); - dnsQueue.addDomainRefreshTask("domain2.com"); - clock.setTo(DateTime.parse("3000-02-05TZ")); - dnsQueue.addDomainRefreshTask("domain3.com"); - - run(); - - TaskQueueHelper.assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); - cloudTasksHelper.assertTasksEnqueued( - DNS_PUBLISH_PUSH_QUEUE_NAME, - new TaskMatcher() - .param("enqueued", "3000-02-05T01:00:00.000Z") - .param("requestTime", "3000-02-03T00:00:00.000Z") - .param("tld", "com") - .param("dnsWriter", "comWriter") - .param("domains", "domain1.com,domain2.com,domain3.com") - .param("hosts", "") - .param("lockIndex", "1") - .param("numPublishLocks", "1")); - } - - @RetryingTest(4) - void testSuccess_oneTldPaused_returnedToQueue() { - persistResource(Tld.get("net").asBuilder().setDnsPaused(true).build()); - dnsQueue.addDomainRefreshTask("domain.com"); - dnsQueue.addDomainRefreshTask("domain.net"); - dnsQueue.addDomainRefreshTask("domain.example"); - - run(); - - TaskQueueHelper.assertTasksEnqueued( - DNS_PULL_QUEUE_NAME, createDomainRefreshTaskMatcher("domain.net")); - assertTldsEnqueuedInPushQueue( - ImmutableMultimap.of("com", "comWriter", "example", "exampleWriter")); - } - - @RetryingTest(4) - void testSuccess_oneTldUnknown_returnedToQueue() { - dnsQueue.addDomainRefreshTask("domain.com"); - dnsQueue.addDomainRefreshTask("domain.example"); - QueueFactory.getQueue(DNS_PULL_QUEUE_NAME) - .add( - TaskOptions.Builder.withDefaults() - .method(TaskOptions.Method.PULL) - .param(DNS_TARGET_TYPE_PARAM, TargetType.DOMAIN.toString()) - .param(DNS_TARGET_NAME_PARAM, "domain.unknown") - .param(DNS_TARGET_CREATE_TIME_PARAM, "3000-01-01TZ") - .param(PARAM_TLD, "unknown")); - - run(); - - TaskQueueHelper.assertTasksEnqueued( - DNS_PULL_QUEUE_NAME, createDomainRefreshTaskMatcher("domain.unknown")); - assertTldsEnqueuedInPushQueue( - ImmutableMultimap.of("com", "comWriter", "example", "exampleWriter")); - } - - @RetryingTest(4) - void testSuccess_corruptTaskTldMismatch_published() { - // TODO(mcilwain): what's the correct action to take in this case? - dnsQueue.addDomainRefreshTask("domain.com"); - dnsQueue.addDomainRefreshTask("domain.example"); - QueueFactory.getQueue(DNS_PULL_QUEUE_NAME) - .add( - TaskOptions.Builder.withDefaults() - .method(TaskOptions.Method.PULL) - .param(DNS_TARGET_TYPE_PARAM, TargetType.DOMAIN.toString()) - .param(DNS_TARGET_NAME_PARAM, "domain.wrongtld") - .param(DNS_TARGET_CREATE_TIME_PARAM, "3000-01-01TZ") - .param(PARAM_TLD, "net")); - - run(); - - TaskQueueHelper.assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); - assertTldsEnqueuedInPushQueue( - ImmutableMultimap.of("com", "comWriter", "example", "exampleWriter", "net", "netWriter")); - } - - @RetryingTest(4) - void testSuccess_corruptTaskNoTld_discarded() { - dnsQueue.addDomainRefreshTask("domain.com"); - dnsQueue.addDomainRefreshTask("domain.example"); - QueueFactory.getQueue(DNS_PULL_QUEUE_NAME) - .add( - TaskOptions.Builder.withDefaults() - .method(TaskOptions.Method.PULL) - .param(DNS_TARGET_TYPE_PARAM, TargetType.DOMAIN.toString()) - .param(DNS_TARGET_NAME_PARAM, "domain.net")); - - run(); - - // The corrupt task isn't in the pull queue, but also isn't in the push queue - TaskQueueHelper.assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); - assertTldsEnqueuedInPushQueue( - ImmutableMultimap.of("com", "comWriter", "example", "exampleWriter")); - } - - @RetryingTest(4) - void testSuccess_corruptTaskNoName_discarded() { - dnsQueue.addDomainRefreshTask("domain.com"); - dnsQueue.addDomainRefreshTask("domain.example"); - QueueFactory.getQueue(DNS_PULL_QUEUE_NAME) - .add( - TaskOptions.Builder.withDefaults() - .method(TaskOptions.Method.PULL) - .param(DNS_TARGET_TYPE_PARAM, TargetType.DOMAIN.toString()) - .param(PARAM_TLD, "net")); - - run(); - - // The corrupt task isn't in the pull queue, but also isn't in the push queue - TaskQueueHelper.assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); - assertTldsEnqueuedInPushQueue( - ImmutableMultimap.of("com", "comWriter", "example", "exampleWriter")); - } - - @RetryingTest(4) - void testSuccess_corruptTaskNoType_discarded() { - dnsQueue.addDomainRefreshTask("domain.com"); - dnsQueue.addDomainRefreshTask("domain.example"); - QueueFactory.getQueue(DNS_PULL_QUEUE_NAME) - .add( - TaskOptions.Builder.withDefaults() - .method(TaskOptions.Method.PULL) - .param(DNS_TARGET_NAME_PARAM, "domain.net") - .param(PARAM_TLD, "net")); - - run(); - - // The corrupt task isn't in the pull queue, but also isn't in the push queue - TaskQueueHelper.assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); - assertTldsEnqueuedInPushQueue( - ImmutableMultimap.of("com", "comWriter", "example", "exampleWriter")); - } - - @RetryingTest(4) - void testSuccess_corruptTaskWrongType_discarded() { - dnsQueue.addDomainRefreshTask("domain.com"); - dnsQueue.addDomainRefreshTask("domain.example"); - QueueFactory.getQueue(DNS_PULL_QUEUE_NAME) - .add( - TaskOptions.Builder.withDefaults() - .method(TaskOptions.Method.PULL) - .param(DNS_TARGET_TYPE_PARAM, "Wrong type") - .param(DNS_TARGET_NAME_PARAM, "domain.net") - .param(PARAM_TLD, "net")); - - run(); - - // The corrupt task isn't in the pull queue, but also isn't in the push queue - TaskQueueHelper.assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); - assertTldsEnqueuedInPushQueue( - ImmutableMultimap.of("com", "comWriter", "example", "exampleWriter")); - } - - private static String makeCommaSeparatedRange(int from, int to, String format) { - return IntStream.range(from, to) - .mapToObj(i -> String.format(format, i)) - .collect(Collectors.joining(",")); - } - - @RetryingTest(4) - void testSuccess_manyDomainsAndHosts() { - for (int i = 0; i < 150; i++) { - // 0: domain; 1: host 1; 2: host 2 - for (int thingType = 0; thingType < 3; thingType++) { - for (String tld : ImmutableList.of("com", "net")) { - String domainName = String.format("domain%04d.%s", i, tld); - switch (thingType) { - case 1: - getQueue(DNS_PULL_QUEUE_NAME) - .add(createRefreshTask("ns1." + domainName, TargetType.HOST)); - break; - case 2: - getQueue(DNS_PULL_QUEUE_NAME) - .add(createRefreshTask("ns2." + domainName, TargetType.HOST)); - break; - default: - dnsQueue.addDomainRefreshTask(domainName); - break; - } - } - } - } - - run(); - - TaskQueueHelper.assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); - cloudTasksHelper.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"))); - } - - @RetryingTest(4) - void testSuccess_lockGroupsHostBySuperordinateDomain() { - dnsQueue.addDomainRefreshTask("hello.multilock.uk"); - dnsQueue.addHostRefreshTask("ns1.abc.hello.multilock.uk"); - dnsQueue.addHostRefreshTask("ns2.hello.multilock.uk"); - dnsQueue.addDomainRefreshTask("another.multilock.uk"); - dnsQueue.addHostRefreshTask("ns3.def.another.multilock.uk"); - dnsQueue.addHostRefreshTask("ns4.another.multilock.uk"); - - run(); - - TaskQueueHelper.assertNoTasksEnqueued(DNS_PULL_QUEUE_NAME); - // Expect two different groups; in-balliwick hosts are locked with their superordinate domains. - cloudTasksHelper.assertTasksEnqueued( - DNS_PUBLISH_PUSH_QUEUE_NAME, - new TaskMatcher() - .url(PublishDnsUpdatesAction.PATH) - .param("tld", "multilock.uk") - .param("dnsWriter", "multilockWriter") - .param("requestTime", "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,ns2.hello.multilock.uk") - .header("content-type", "application/x-www-form-urlencoded"), - new TaskMatcher() - .url(PublishDnsUpdatesAction.PATH) - .param("tld", "multilock.uk") - .param("dnsWriter", "multilockWriter") - .param("requestTime", "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,ns4.another.multilock.uk") - .header("content-type", "application/x-www-form-urlencoded")); - } -} diff --git a/core/src/test/java/google/registry/dns/ReadDnsRefreshRequestsActionTest.java b/core/src/test/java/google/registry/dns/ReadDnsRefreshRequestsActionTest.java index 2bbb7af2e..af698e5d0 100644 --- a/core/src/test/java/google/registry/dns/ReadDnsRefreshRequestsActionTest.java +++ b/core/src/test/java/google/registry/dns/ReadDnsRefreshRequestsActionTest.java @@ -15,10 +15,6 @@ package google.registry.dns; import static com.google.common.truth.Truth.assertThat; -import static google.registry.model.common.DatabaseMigrationStateSchedule.MigrationState; -import static google.registry.model.common.DatabaseMigrationStateSchedule.set; -import static google.registry.model.common.DatabaseMigrationStateSchedule.useUncachedForTest; -import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.loadAllOf; import static google.registry.testing.DatabaseHelper.persistResource; @@ -38,9 +34,7 @@ import static org.mockito.Mockito.verify; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSortedMap; -import com.google.common.collect.Ordering; -import google.registry.dns.DnsConstants.TargetType; +import google.registry.dns.DnsUtils.TargetType; import google.registry.model.common.DnsRefreshRequest; import google.registry.model.common.DnsRefreshRequestTest; import google.registry.model.tld.Tld; @@ -54,7 +48,6 @@ import java.util.Optional; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; import org.joda.time.Duration; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -65,7 +58,6 @@ public class ReadDnsRefreshRequestsActionTest { private final FakeClock clock = new FakeClock(DateTime.parse("2020-02-02T01:23:45Z")); private final CloudTasksHelper cloudTasksHelper = new CloudTasksHelper(clock); - private final DnsUtils dnsUtils = new DnsUtils(null); private final Optional jitterSeconds = Optional.of(5); @RegisterExtension @@ -80,20 +72,13 @@ public class ReadDnsRefreshRequestsActionTest { jitterSeconds, "tld", clock, - dnsUtils, null, cloudTasksHelper.getTestCloudTasksUtils())); private ImmutableList requests; - @BeforeAll - static void beforeAll() { - useUncachedForTest(); - } - @BeforeEach void beforeEach() { - useDnsSql(); persistResource( createTld("tld") .asBuilder() @@ -271,26 +256,4 @@ public class ReadDnsRefreshRequestsActionTest { .isAtMost(Duration.standardSeconds(jitterSeconds.get())); }); } - - private void useDnsSql() { - DateTime currentTime = clock.nowUtc(); - clock.setTo(START_OF_TIME); - tm().transact( - () -> - set( - new ImmutableSortedMap.Builder(Ordering.natural()) - .put(START_OF_TIME, MigrationState.DATASTORE_ONLY) - .put(START_OF_TIME.plusMillis(1), MigrationState.DATASTORE_PRIMARY) - .put(START_OF_TIME.plusMillis(2), MigrationState.DATASTORE_PRIMARY_NO_ASYNC) - .put( - START_OF_TIME.plusMillis(3), MigrationState.DATASTORE_PRIMARY_READ_ONLY) - .put(START_OF_TIME.plusMillis(4), MigrationState.SQL_PRIMARY_READ_ONLY) - .put(START_OF_TIME.plusMillis(5), MigrationState.SQL_PRIMARY) - .put(START_OF_TIME.plusMillis(6), MigrationState.SQL_ONLY) - .put(START_OF_TIME.plusMillis(7), MigrationState.SEQUENCE_BASED_ALLOCATE_ID) - .put(START_OF_TIME.plusMillis(8), MigrationState.NORDN_SQL) - .put(START_OF_TIME.plusMillis(9), MigrationState.DNS_SQL) - .build())); - clock.setTo(currentTime); - } } diff --git a/core/src/test/java/google/registry/dns/RefreshDnsActionTest.java b/core/src/test/java/google/registry/dns/RefreshDnsActionTest.java index 7c4144c2a..8b47b791b 100644 --- a/core/src/test/java/google/registry/dns/RefreshDnsActionTest.java +++ b/core/src/test/java/google/registry/dns/RefreshDnsActionTest.java @@ -15,20 +15,22 @@ package google.registry.dns; import static com.google.common.truth.Truth.assertThat; +import static google.registry.testing.DatabaseHelper.assertDomainDnsRequests; +import static google.registry.testing.DatabaseHelper.assertHostDnsRequests; +import static google.registry.testing.DatabaseHelper.assertNoDnsRequests; +import static google.registry.testing.DatabaseHelper.assertNoDnsRequestsExcept; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.persistActiveDomain; import static google.registry.testing.DatabaseHelper.persistActiveHost; import static google.registry.testing.DatabaseHelper.persistActiveSubordinateHost; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.Mockito.mock; -import google.registry.dns.DnsConstants.TargetType; +import google.registry.dns.DnsUtils.TargetType; import google.registry.model.domain.Domain; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; import google.registry.request.HttpException.BadRequestException; import google.registry.request.HttpException.NotFoundException; -import google.registry.testing.DnsUtilsHelper; import google.registry.testing.FakeClock; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -41,12 +43,10 @@ public class RefreshDnsActionTest { final JpaIntegrationTestExtension jpa = new JpaTestExtensions.Builder().buildIntegrationTestExtension(); - private final DnsUtils dnsUtils = mock(DnsUtils.class); - private final DnsUtilsHelper dnsUtilsHelper = new DnsUtilsHelper(dnsUtils); private final FakeClock clock = new FakeClock(); private void run(TargetType type, String name) { - new RefreshDnsAction(name, type, clock, dnsUtils).run(); + new RefreshDnsAction(name, type, clock).run(); } @BeforeEach @@ -58,9 +58,9 @@ public class RefreshDnsActionTest { void testSuccess_host() { Domain domain = persistActiveDomain("example.xn--q9jyb4c"); persistActiveSubordinateHost("ns1.example.xn--q9jyb4c", domain); - run(TargetType.HOST, "ns1.example.xn--q9jyb4c"); - dnsUtilsHelper.assertHostDnsRequests("ns1.example.xn--q9jyb4c"); - dnsUtilsHelper.assertNoMoreDnsRequests(); + run(DnsUtils.TargetType.HOST, "ns1.example.xn--q9jyb4c"); + assertHostDnsRequests("ns1.example.xn--q9jyb4c"); + assertNoDnsRequestsExcept("ns1.example.xn--q9jyb4c"); } @Test @@ -72,9 +72,9 @@ public class RefreshDnsActionTest { BadRequestException.class, () -> { try { - run(TargetType.HOST, "ns1.example.xn--q9jyb4c"); + run(DnsUtils.TargetType.HOST, "ns1.example.xn--q9jyb4c"); } finally { - dnsUtilsHelper.assertNoMoreDnsRequests(); + assertNoDnsRequests(); } }); assertThat(thrown) @@ -85,23 +85,25 @@ public class RefreshDnsActionTest { @Test void testSuccess_domain() { persistActiveDomain("example.xn--q9jyb4c"); - run(TargetType.DOMAIN, "example.xn--q9jyb4c"); - dnsUtilsHelper.assertDomainDnsRequests("example.xn--q9jyb4c"); - dnsUtilsHelper.assertNoMoreDnsRequests(); + run(DnsUtils.TargetType.DOMAIN, "example.xn--q9jyb4c"); + assertDomainDnsRequests("example.xn--q9jyb4c"); + assertNoDnsRequestsExcept("example.xn--q9jyb4c"); } @Test void testFailure_unqualifiedName() { - assertThrows(BadRequestException.class, () -> run(TargetType.DOMAIN, "example")); + assertThrows(BadRequestException.class, () -> run(DnsUtils.TargetType.DOMAIN, "example")); } @Test void testFailure_hostDoesNotExist() { - assertThrows(NotFoundException.class, () -> run(TargetType.HOST, "ns1.example.xn--q9jyb4c")); + assertThrows( + NotFoundException.class, () -> run(DnsUtils.TargetType.HOST, "ns1.example.xn--q9jyb4c")); } @Test void testFailure_domainDoesNotExist() { - assertThrows(NotFoundException.class, () -> run(TargetType.DOMAIN, "example.xn--q9jyb4c")); + assertThrows( + NotFoundException.class, () -> run(DnsUtils.TargetType.DOMAIN, "example.xn--q9jyb4c")); } } diff --git a/core/src/test/java/google/registry/dns/RefreshDnsOnHostRenameActionTest.java b/core/src/test/java/google/registry/dns/RefreshDnsOnHostRenameActionTest.java index df691a573..f875fab03 100644 --- a/core/src/test/java/google/registry/dns/RefreshDnsOnHostRenameActionTest.java +++ b/core/src/test/java/google/registry/dns/RefreshDnsOnHostRenameActionTest.java @@ -15,6 +15,8 @@ package google.registry.dns; import static com.google.common.truth.Truth.assertThat; +import static google.registry.testing.DatabaseHelper.assertDomainDnsRequests; +import static google.registry.testing.DatabaseHelper.assertNoDnsRequests; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.newDomain; import static google.registry.testing.DatabaseHelper.persistActiveHost; @@ -23,14 +25,12 @@ import static google.registry.testing.DatabaseHelper.persistDomainAsDeleted; import static google.registry.testing.DatabaseHelper.persistResource; import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT; import static javax.servlet.http.HttpServletResponse.SC_OK; -import static org.mockito.Mockito.mock; import com.google.common.collect.ImmutableSet; import google.registry.model.eppcommon.StatusValue; import google.registry.model.host.Host; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; -import google.registry.testing.DnsUtilsHelper; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import org.joda.time.DateTime; @@ -42,8 +42,6 @@ import org.junit.jupiter.api.extension.RegisterExtension; public class RefreshDnsOnHostRenameActionTest { private final FakeClock clock = new FakeClock(DateTime.parse("2015-01-15T11:22:33Z")); - private final DnsUtils dnsUtils = mock(DnsUtils.class); - private final DnsUtilsHelper dnsUtilsHelper = new DnsUtilsHelper(dnsUtils); private final FakeResponse response = new FakeResponse(); @RegisterExtension @@ -53,7 +51,7 @@ public class RefreshDnsOnHostRenameActionTest { private RefreshDnsOnHostRenameAction action; private void createAction(String hostKey) { - action = new RefreshDnsOnHostRenameAction(hostKey, response, dnsUtils); + action = new RefreshDnsOnHostRenameAction(hostKey, response); } @BeforeEach @@ -75,7 +73,7 @@ public class RefreshDnsOnHostRenameActionTest { persistDomainAsDeleted(newDomain("deleted.tld", host), clock.nowUtc().minusDays(1)); createAction(host.createVKey().stringify()); action.run(); - dnsUtilsHelper.assertDomainDnsRequests("example.tld", "otherexample.tld"); + assertDomainDnsRequests("example.tld", "otherexample.tld"); assertThat(response.getStatus()).isEqualTo(SC_OK); } @@ -83,7 +81,7 @@ public class RefreshDnsOnHostRenameActionTest { void testFailure_nonexistentHost() { createAction("kind:Host@sql:rO0ABXQABGJsYWg"); action.run(); - dnsUtilsHelper.assertNoMoreDnsRequests(); + assertNoDnsRequests(); assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); assertThat(response.getPayload()) .isEqualTo("Host to refresh does not exist: VKey(sql:blah)"); @@ -95,7 +93,7 @@ public class RefreshDnsOnHostRenameActionTest { persistResource(newDomain("example.tld", host)); createAction(host.createVKey().stringify()); action.run(); - dnsUtilsHelper.assertNoMoreDnsRequests(); + assertNoDnsRequests(); assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); assertThat(response.getPayload()) .isEqualTo("Host to refresh is already deleted: ns1.example.tld"); diff --git a/core/src/test/java/google/registry/flows/EppLifecycleDomainTest.java b/core/src/test/java/google/registry/flows/EppLifecycleDomainTest.java index 523526e60..1202c2466 100644 --- a/core/src/test/java/google/registry/flows/EppLifecycleDomainTest.java +++ b/core/src/test/java/google/registry/flows/EppLifecycleDomainTest.java @@ -47,7 +47,6 @@ import google.registry.model.tld.Tld; import google.registry.model.tld.Tld.TldState; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; -import google.registry.testing.TaskQueueExtension; import google.registry.tmch.TmchData; import google.registry.tmch.TmchTestData; import org.joda.money.Money; @@ -72,8 +71,6 @@ class EppLifecycleDomainTest extends EppTestCase { final JpaIntegrationTestExtension jpa = new JpaTestExtensions.Builder().withClock(clock).buildIntegrationTestExtension(); - @RegisterExtension final TaskQueueExtension taskQueue = new TaskQueueExtension(); - @BeforeEach void beforeEach() { createTlds("example", "tld"); diff --git a/core/src/test/java/google/registry/flows/EppLifecycleHostTest.java b/core/src/test/java/google/registry/flows/EppLifecycleHostTest.java index 8780ba8a6..b48fa2272 100644 --- a/core/src/test/java/google/registry/flows/EppLifecycleHostTest.java +++ b/core/src/test/java/google/registry/flows/EppLifecycleHostTest.java @@ -27,7 +27,6 @@ import google.registry.model.domain.Domain; import google.registry.model.host.Host; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; -import google.registry.testing.TaskQueueExtension; import org.joda.time.DateTime; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -39,8 +38,6 @@ class EppLifecycleHostTest extends EppTestCase { final JpaIntegrationTestExtension jpa = new JpaTestExtensions.Builder().withClock(clock).buildIntegrationTestExtension(); - @RegisterExtension final TaskQueueExtension taskQueue = new TaskQueueExtension(); - @Test void testLifecycle() throws Exception { assertThatLoginSucceeds("NewRegistrar", "foo-BAR2"); diff --git a/core/src/test/java/google/registry/flows/EppPointInTimeTest.java b/core/src/test/java/google/registry/flows/EppPointInTimeTest.java index 64f50308a..2bb1108e1 100644 --- a/core/src/test/java/google/registry/flows/EppPointInTimeTest.java +++ b/core/src/test/java/google/registry/flows/EppPointInTimeTest.java @@ -36,7 +36,6 @@ import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationT import google.registry.testing.EppLoader; import google.registry.testing.FakeClock; import google.registry.testing.FakeHttpSession; -import google.registry.testing.TaskQueueExtension; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -51,8 +50,6 @@ class EppPointInTimeTest { final JpaIntegrationTestExtension jpa = new JpaTestExtensions.Builder().withClock(clock).buildIntegrationTestExtension(); - @RegisterExtension final TaskQueueExtension taskQueue = new TaskQueueExtension(); - private EppLoader eppLoader; @BeforeEach diff --git a/core/src/test/java/google/registry/flows/EppTestComponent.java b/core/src/test/java/google/registry/flows/EppTestComponent.java index d3c5c8359..f6ddf3788 100644 --- a/core/src/test/java/google/registry/flows/EppTestComponent.java +++ b/core/src/test/java/google/registry/flows/EppTestComponent.java @@ -23,8 +23,6 @@ import google.registry.batch.AsyncTaskEnqueuerTest; import google.registry.batch.CloudTasksUtils; import google.registry.config.RegistryConfig.ConfigModule; import google.registry.config.RegistryConfig.ConfigModule.TmchCaMode; -import google.registry.dns.DnsQueue; -import google.registry.dns.DnsUtils; import google.registry.flows.custom.CustomLogicFactory; import google.registry.flows.custom.TestCustomLogicFactory; import google.registry.flows.domain.DomainFlowTmchUtils; @@ -32,7 +30,6 @@ import google.registry.monitoring.whitebox.EppMetric; import google.registry.request.RequestScope; import google.registry.request.lock.LockHandler; import google.registry.testing.CloudTasksHelper; -import google.registry.testing.DnsUtilsHelper; import google.registry.testing.FakeClock; import google.registry.testing.FakeLockHandler; import google.registry.testing.FakeSleeper; @@ -54,23 +51,17 @@ public interface EppTestComponent { class FakesAndMocksModule { private AsyncTaskEnqueuer asyncTaskEnqueuer; - private DnsQueue dnsQueue; private DomainFlowTmchUtils domainFlowTmchUtils; private EppMetric.Builder metricBuilder; private FakeClock clock; private FakeLockHandler lockHandler; private Sleeper sleeper; private CloudTasksHelper cloudTasksHelper; - private DnsUtilsHelper dnsUtilsHelper; public CloudTasksHelper getCloudTasksHelper() { return cloudTasksHelper; } - public DnsUtilsHelper getDnsUtilsHelper() { - return dnsUtilsHelper; - } - public EppMetric.Builder getMetricBuilder() { return metricBuilder; } @@ -85,11 +76,9 @@ public interface EppTestComponent { new DomainFlowTmchUtils( new TmchXmlSignature(new TmchCertificateAuthority(TmchCaMode.PILOT, clock))); instance.sleeper = new FakeSleeper(instance.clock); - instance.dnsQueue = DnsQueue.createForTesting(clock); instance.metricBuilder = EppMetric.builderForRequest(clock); instance.lockHandler = new FakeLockHandler(true); instance.cloudTasksHelper = cloudTasksHelper; - instance.dnsUtilsHelper = new DnsUtilsHelper(); return instance; } @@ -103,11 +92,6 @@ public interface EppTestComponent { return cloudTasksHelper.getTestCloudTasksUtils(); } - @Provides - DnsUtils provideDnsUtils() { - return dnsUtilsHelper.getDnsUtils(); - } - @Provides Clock provideClock() { return clock; @@ -123,11 +107,6 @@ public interface EppTestComponent { return new TestCustomLogicFactory(); } - @Provides - DnsQueue provideDnsQueue() { - return dnsQueue; - } - @Provides DomainFlowTmchUtils provideDomainFlowTmchUtils() { return domainFlowTmchUtils; diff --git a/core/src/test/java/google/registry/flows/FlowTestCase.java b/core/src/test/java/google/registry/flows/FlowTestCase.java index 3854bf962..e123cbe89 100644 --- a/core/src/test/java/google/registry/flows/FlowTestCase.java +++ b/core/src/test/java/google/registry/flows/FlowTestCase.java @@ -45,7 +45,6 @@ import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; import google.registry.testing.CloudTasksHelper; import google.registry.testing.DatabaseHelper; -import google.registry.testing.DnsUtilsHelper; import google.registry.testing.EppLoader; import google.registry.testing.FakeClock; import google.registry.testing.FakeHttpSession; @@ -85,7 +84,6 @@ public abstract class FlowTestCase { protected TransportCredentials credentials = new PasswordOnlyTransportCredentials(); protected EppRequestSource eppRequestSource = EppRequestSource.UNIT_TEST; protected CloudTasksHelper cloudTasksHelper; - protected DnsUtilsHelper dnsUtilsHelper; private EppMetric.Builder eppMetricBuilder; @@ -217,7 +215,6 @@ public abstract class FlowTestCase { FakesAndMocksModule fakesAndMocksModule = FakesAndMocksModule.create(clock); cloudTasksHelper = fakesAndMocksModule.getCloudTasksHelper(); - dnsUtilsHelper = fakesAndMocksModule.getDnsUtilsHelper(); // Run the flow. return DaggerEppTestComponent.builder() .fakesAndMocksModule(fakesAndMocksModule) diff --git a/core/src/test/java/google/registry/flows/ResourceFlowTestCase.java b/core/src/test/java/google/registry/flows/ResourceFlowTestCase.java index 1690a0638..e338b70b2 100644 --- a/core/src/test/java/google/registry/flows/ResourceFlowTestCase.java +++ b/core/src/test/java/google/registry/flows/ResourceFlowTestCase.java @@ -14,12 +14,10 @@ package google.registry.flows; -import static google.registry.batch.AsyncTaskEnqueuer.PARAM_RESOURCE_KEY; import static google.registry.model.EppResourceUtils.loadByForeignKey; import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.LogsSubject.assertAboutLogs; -import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -30,7 +28,6 @@ import google.registry.model.contact.ContactBase; import google.registry.model.contact.ContactHistory; import google.registry.model.domain.DomainBase; import google.registry.model.domain.DomainHistory; -import google.registry.model.eppcommon.Trid; import google.registry.model.eppinput.EppInput.ResourceCommandWrapper; import google.registry.model.eppinput.ResourceCommand; import google.registry.model.host.HostBase; @@ -39,14 +36,13 @@ import google.registry.model.reporting.HistoryEntry; import google.registry.model.tmch.ClaimsList; import google.registry.model.tmch.ClaimsListDao; import google.registry.testing.DatabaseHelper; -import google.registry.testing.TaskQueueHelper.TaskMatcher; import google.registry.testing.TestCacheExtension; import google.registry.util.JdkLoggerConfig; import google.registry.util.TypeUtils.TypeInstantiator; +import java.time.Duration; import java.util.logging.Level; import javax.annotation.Nullable; import org.joda.time.DateTime; -import org.joda.time.Duration; import org.json.simple.JSONValue; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.extension.RegisterExtension; @@ -64,11 +60,11 @@ public abstract class ResourceFlowTestCase void assertAsyncDeletionTaskEnqueued( - T resource, String requestingClientId, Trid trid, boolean isSuperuser) { - TaskMatcher expected = - new TaskMatcher() - .etaDelta(Duration.standardSeconds(75), Duration.standardSeconds(105)) // expected: 90 - .param(PARAM_RESOURCE_KEY, resource.createVKey().stringify()) - .param("requestingClientId", requestingClientId) - .param("serverTransactionId", trid.getServerTransactionId()) - .param("isSuperuser", Boolean.toString(isSuperuser)) - .param("requestedTime", clock.nowUtc().toString()); - trid.getClientTransactionId() - .ifPresent(clTrid -> expected.param("clientTransactionId", clTrid)); - assertTasksEnqueued("async-delete-pull", expected); - } - protected void assertClientIdFieldLogged(String registrarId) { assertAboutLogs() .that(logHandler) diff --git a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java index b0c9a6ef7..402a9622d 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java @@ -39,6 +39,8 @@ import static google.registry.model.tld.Tld.TldState.START_DATE_SUNRISE; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.pricing.PricingEngineProxy.isDomainPremium; import static google.registry.testing.DatabaseHelper.assertBillingEvents; +import static google.registry.testing.DatabaseHelper.assertDomainDnsRequests; +import static google.registry.testing.DatabaseHelper.assertNoDnsRequests; import static google.registry.testing.DatabaseHelper.assertPollMessagesForResource; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.createTlds; @@ -176,7 +178,6 @@ import google.registry.model.tld.Tld.TldType; import google.registry.monitoring.whitebox.EppMetric; import google.registry.persistence.VKey; import google.registry.testing.DatabaseHelper; -import google.registry.testing.TaskQueueExtension; import google.registry.tmch.LordnTaskUtils.LordnPhase; import google.registry.tmch.SmdrlCsvParser; import google.registry.tmch.TmchData; @@ -190,15 +191,12 @@ import org.joda.time.DateTime; import org.joda.time.Duration; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; import org.junitpioneer.jupiter.cartesian.CartesianTest; import org.junitpioneer.jupiter.cartesian.CartesianTest.Values; /** Unit tests for {@link DomainCreateFlow}. */ class DomainCreateFlowTest extends ResourceFlowTestCase { - @RegisterExtension TaskQueueExtension taskQueue = new TaskQueueExtension(); - private static final String CLAIMS_KEY = "2013041500/2/6/9/rJ1NrDO92vDsAzf7EQzgjX4R0000000001"; // This is a time when SMD itself is not valid, but its signing certificates are. It will // trigger a different exception than when any signing cert is not valid yet. @@ -378,7 +376,7 @@ class DomainCreateFlowTest extends ResourceFlowTestCase ds.cloneWithDomainRepoId(resource.getRepoId())) .collect(toImmutableSet())); if (dnsTaskEnqueued) { - dnsUtilsHelper.assertDomainDnsRequests("example.tld"); + assertDomainDnsRequests("example.tld"); } else { - dnsUtilsHelper.assertNoMoreDnsRequests(); + assertNoDnsRequests(); } } @@ -855,8 +860,7 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase { void testSuccess_externalNeverExisted() throws Exception { doSuccessfulTest(); assertAboutHosts().that(reloadResourceByForeignKey()).hasSuperordinateDomain(null); - dnsUtilsHelper.assertNoMoreDnsRequests(); + assertNoDnsRequests(); } @Test @@ -127,7 +129,7 @@ class HostCreateFlowTest extends ResourceFlowTestCase { loadByForeignKey(Domain.class, "example.tld", clock.nowUtc()).get(); assertAboutHosts().that(host).hasSuperordinateDomain(superordinateDomain.createVKey()); assertThat(superordinateDomain.getSubordinateHosts()).containsExactly("ns1.example.tld"); - dnsUtilsHelper.assertHostDnsRequests("ns1.example.tld"); + assertHostDnsRequests("ns1.example.tld"); } @Test @@ -144,7 +146,7 @@ class HostCreateFlowTest extends ResourceFlowTestCase { persistDeletedHost(getUniqueIdFromCommand(), clock.nowUtc().minusDays(1)); doSuccessfulTest(); assertAboutHosts().that(reloadResourceByForeignKey()).hasSuperordinateDomain(null); - dnsUtilsHelper.assertNoMoreDnsRequests(); + assertNoDnsRequests(); } @Test @@ -156,7 +158,7 @@ class HostCreateFlowTest extends ResourceFlowTestCase { loadByForeignKey(Domain.class, "example.tld", clock.nowUtc()).get(); assertAboutHosts().that(host).hasSuperordinateDomain(superordinateDomain.createVKey()); assertThat(superordinateDomain.getSubordinateHosts()).containsExactly("ns1.example.tld"); - dnsUtilsHelper.assertHostDnsRequests("ns1.example.tld"); + assertHostDnsRequests("ns1.example.tld"); } @Test diff --git a/core/src/test/java/google/registry/flows/host/HostDeleteFlowTest.java b/core/src/test/java/google/registry/flows/host/HostDeleteFlowTest.java index 945f4cc19..958717d6d 100644 --- a/core/src/test/java/google/registry/flows/host/HostDeleteFlowTest.java +++ b/core/src/test/java/google/registry/flows/host/HostDeleteFlowTest.java @@ -15,7 +15,9 @@ package google.registry.flows.host; import static com.google.common.truth.Truth.assertThat; +import static google.registry.testing.DatabaseHelper.assertHostDnsRequests; import static google.registry.testing.DatabaseHelper.assertNoBillingEvents; +import static google.registry.testing.DatabaseHelper.assertNoDnsRequests; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.loadByKey; import static google.registry.testing.DatabaseHelper.newHost; @@ -314,10 +316,10 @@ class HostDeleteFlowTest extends ResourceFlowTestCase { .hasType(Type.HOST_DELETE); assertNoBillingEvents(); if (isSubordinate) { - dnsUtilsHelper.assertHostDnsRequests(deletedHost.getHostName()); + assertHostDnsRequests(deletedHost.getHostName()); assertThat(loadByKey(deletedHost.getSuperordinateDomain()).getSubordinateHosts()).isEmpty(); } else { - dnsUtilsHelper.assertNoMoreDnsRequests(); + assertNoDnsRequests(); } assertLastHistoryContainsResource(deletedHost); } diff --git a/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java b/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java index 2cd95f8e3..0d8100962 100644 --- a/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java +++ b/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java @@ -19,7 +19,9 @@ import static com.google.common.truth.Truth.assertThat; import static google.registry.dns.RefreshDnsOnHostRenameAction.PARAM_HOST_KEY; import static google.registry.dns.RefreshDnsOnHostRenameAction.QUEUE_HOST_RENAME; import static google.registry.model.EppResourceUtils.loadByForeignKey; +import static google.registry.testing.DatabaseHelper.assertHostDnsRequests; import static google.registry.testing.DatabaseHelper.assertNoBillingEvents; +import static google.registry.testing.DatabaseHelper.assertNoDnsRequests; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.getOnlyHistoryEntryOfType; import static google.registry.testing.DatabaseHelper.loadByEntity; @@ -183,7 +185,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { persistActiveSubordinateHost(oldHostName(), persistActiveDomain("example.tld")); Host renamedHost = doSuccessfulTest(); assertThat(renamedHost.isSubordinate()).isTrue(); - dnsUtilsHelper.assertHostDnsRequests("ns1.example.tld", "ns2.example.tld"); + assertHostDnsRequests("ns1.example.tld", "ns2.example.tld"); VKey oldVKeyAfterRename = ForeignKeyUtils.load(Host.class, oldHostName(), clock.nowUtc()); assertThat(oldVKeyAfterRename).isNull(); } @@ -232,7 +234,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { .and() .hasOnlyOneHistoryEntryWhich() .hasType(HistoryEntry.Type.HOST_UPDATE); - dnsUtilsHelper.assertHostDnsRequests("ns1.example.tld"); + assertHostDnsRequests("ns1.example.tld"); } @Test @@ -258,7 +260,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { .and() .hasOnlyOneHistoryEntryWhich() .hasType(HistoryEntry.Type.HOST_UPDATE); - dnsUtilsHelper.assertHostDnsRequests("ns1.example.tld"); + assertHostDnsRequests("ns1.example.tld"); } @Test @@ -292,7 +294,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { .hasLastTransferTime(oneDayAgo); Domain reloadedDomain = loadByEntity(domain).cloneProjectedAtTime(now); assertThat(reloadedDomain.getSubordinateHosts()).containsExactly("ns2.example.tld"); - dnsUtilsHelper.assertHostDnsRequests("ns1.example.tld", "ns2.example.tld"); + assertHostDnsRequests("ns1.example.tld", "ns2.example.tld"); } @Test @@ -327,7 +329,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { assertThat(loadByEntity(foo).cloneProjectedAtTime(now).getSubordinateHosts()).isEmpty(); assertThat(loadByEntity(example).cloneProjectedAtTime(now).getSubordinateHosts()) .containsExactly("ns2.example.tld"); - dnsUtilsHelper.assertHostDnsRequests("ns2.foo.tld", "ns2.example.tld"); + assertHostDnsRequests("ns2.foo.tld", "ns2.example.tld"); } @Test @@ -364,7 +366,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { assertThat(reloadedFooDomain.getSubordinateHosts()).isEmpty(); Domain reloadedTldDomain = loadByEntity(tldDomain).cloneProjectedAtTime(now); assertThat(reloadedTldDomain.getSubordinateHosts()).containsExactly("ns2.example.tld"); - dnsUtilsHelper.assertHostDnsRequests("ns1.example.foo", "ns2.example.tld"); + assertHostDnsRequests("ns1.example.foo", "ns2.example.tld"); } @Test @@ -407,7 +409,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { assertThat(renamedHost.getLastTransferTime()).isEqualTo(oneDayAgo); Domain reloadedDomain = loadByEntity(domain).cloneProjectedAtTime(clock.nowUtc()); assertThat(reloadedDomain.getSubordinateHosts()).isEmpty(); - dnsUtilsHelper.assertHostDnsRequests("ns1.example.foo"); + assertHostDnsRequests("ns1.example.foo"); } @Test @@ -419,7 +421,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { persistActiveHost(oldHostName()); assertThat(domain.getSubordinateHosts()).isEmpty(); assertThrows(CannotRenameExternalHostException.class, this::runFlow); - dnsUtilsHelper.assertNoMoreDnsRequests(); + assertNoDnsRequests(); } @Test @@ -443,7 +445,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { .hasLastTransferTime(null); assertThat(loadByEntity(domain).cloneProjectedAtTime(now).getSubordinateHosts()) .containsExactly("ns2.example.tld"); - dnsUtilsHelper.assertHostDnsRequests("ns2.example.tld"); + assertHostDnsRequests("ns2.example.tld"); } @Test @@ -468,7 +470,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { .hasPersistedCurrentSponsorRegistrarId("TheRegistrar") .and() .hasLastTransferTime(null); - dnsUtilsHelper.assertNoMoreDnsRequests(); + assertNoDnsRequests(); } @Test diff --git a/core/src/test/java/google/registry/model/common/DnsRefreshRequestTest.java b/core/src/test/java/google/registry/model/common/DnsRefreshRequestTest.java index 94fee6f03..5898a7ff8 100644 --- a/core/src/test/java/google/registry/model/common/DnsRefreshRequestTest.java +++ b/core/src/test/java/google/registry/model/common/DnsRefreshRequestTest.java @@ -22,7 +22,7 @@ import static google.registry.util.DateTimeUtils.START_OF_TIME; import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.collect.ImmutableList; -import google.registry.dns.DnsConstants.TargetType; +import google.registry.dns.DnsUtils; import google.registry.model.EntityTestCase; import org.junit.jupiter.api.Test; @@ -34,7 +34,8 @@ public class DnsRefreshRequestTest extends EntityTestCase { } private final DnsRefreshRequest request = - new DnsRefreshRequest(TargetType.DOMAIN, "test.example", "example", fakeClock.nowUtc()); + new DnsRefreshRequest( + DnsUtils.TargetType.DOMAIN, "test.example", "example", fakeClock.nowUtc()); @Test void testPersistence() { @@ -56,15 +57,18 @@ public class DnsRefreshRequestTest extends EntityTestCase { // name assertThrows( NullPointerException.class, - () -> new DnsRefreshRequest(TargetType.DOMAIN, null, "example", fakeClock.nowUtc())); + () -> + new DnsRefreshRequest(DnsUtils.TargetType.DOMAIN, null, "example", fakeClock.nowUtc())); // tld assertThrows( NullPointerException.class, - () -> new DnsRefreshRequest(TargetType.DOMAIN, "test.example", null, fakeClock.nowUtc())); + () -> + new DnsRefreshRequest( + DnsUtils.TargetType.DOMAIN, "test.example", null, fakeClock.nowUtc())); // request time assertThrows( NullPointerException.class, - () -> new DnsRefreshRequest(TargetType.DOMAIN, "test.example", "example", null)); + () -> new DnsRefreshRequest(DnsUtils.TargetType.DOMAIN, "test.example", "example", null)); } @Test diff --git a/core/src/test/java/google/registry/server/RegistryTestServer.java b/core/src/test/java/google/registry/server/RegistryTestServer.java index 0b32d4204..dff48f576 100644 --- a/core/src/test/java/google/registry/server/RegistryTestServer.java +++ b/core/src/test/java/google/registry/server/RegistryTestServer.java @@ -71,9 +71,6 @@ public final class RegistryTestServer { route("/_dr/task/nordnUpload", BackendServlet.class), route("/_dr/task/nordnVerify", BackendServlet.class), - // Process DNS pull queue - route("/_dr/cron/readDnsQueue", BackendServlet.class), - // Registrar Console route("/registrar", FrontendServlet.class), route("/registrar-create", FrontendServlet.class), diff --git a/core/src/test/java/google/registry/testing/CloudTasksHelper.java b/core/src/test/java/google/registry/testing/CloudTasksHelper.java index 84a34eff7..45f0fbb55 100644 --- a/core/src/test/java/google/registry/testing/CloudTasksHelper.java +++ b/core/src/test/java/google/registry/testing/CloudTasksHelper.java @@ -48,6 +48,7 @@ import google.registry.model.ImmutableObject; import google.registry.util.Retrier; import java.io.Serializable; import java.net.URI; +import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collection; @@ -69,11 +70,6 @@ import org.joda.time.DateTime; /** * Static utility functions for testing task queues. * - *

    This class is mostly derived from {@link TaskQueueHelper}. It does not implement as many - * helper methods because we have not yet encountered all the use cases with Cloud Tasks. As more - * and more Task Queue API usage is migrated to Cloud Tasks we may replicate more methods from the - * latter. - * *

    Note the use of {@link AtomicInteger} {@code nextInstanceId} here. When a {@link * FakeCloudTasksClient} instance, and by extension the {@link CloudTasksHelper} instance that * contains it is serialized/deserialized, as happens in a Beam pipeline, we to want to push tasks @@ -101,7 +97,7 @@ public class CloudTasksHelper implements Serializable { private final CloudTasksUtils cloudTasksUtils; public CloudTasksHelper(FakeClock clock) { - this.cloudTasksUtils = + cloudTasksUtils = new CloudTasksUtils( new Retrier(new FakeSleeper(clock), 1), clock, @@ -246,7 +242,7 @@ public class CloudTasksHelper implements Serializable { new URI( String.format( "https://nomulus.foo%s", task.getAppEngineHttpRequest().getRelativeUri())); - } catch (java.net.URISyntaxException e) { + } catch (URISyntaxException e) { throw new IllegalArgumentException(e); } taskName = task.getName(); @@ -291,7 +287,7 @@ public class CloudTasksHelper implements Serializable { builder.put("headers", headers); builder.put("params", params); builder.put("scheduleTime", scheduleTime); - return Maps.filterValues(builder, not(in(asList(null, "", Collections.EMPTY_MAP)))); + return Maps.filterValues(builder, not(in(asList(null, "", Collections.emptyMap())))); } } @@ -388,7 +384,7 @@ public class CloudTasksHelper implements Serializable { .join( Maps.transformValues( expected.toMap(), - input -> "\t" + String.valueOf(input).replaceAll("\n", "\n\t"))); + input -> '\t' + String.valueOf(input).replaceAll("\n", "\n\t"))); } } } diff --git a/core/src/test/java/google/registry/testing/DatabaseHelper.java b/core/src/test/java/google/registry/testing/DatabaseHelper.java index 4fa789bfd..db630e738 100644 --- a/core/src/test/java/google/registry/testing/DatabaseHelper.java +++ b/core/src/test/java/google/registry/testing/DatabaseHelper.java @@ -33,6 +33,7 @@ import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableO import static google.registry.model.ImmutableObjectSubject.immutableObjectCorrespondence; import static google.registry.model.ResourceTransferUtils.createTransferResponse; import static google.registry.model.tld.Tld.TldState.GENERAL_AVAILABILITY; +import static google.registry.persistence.transaction.QueryComposer.Comparator.EQ; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.pricing.PricingEngineProxy.getDomainRenewCost; import static google.registry.util.CollectionUtils.difference; @@ -56,6 +57,8 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Iterables; import com.google.common.net.InetAddresses; +import google.registry.dns.DnsUtils; +import google.registry.dns.DnsUtils.TargetType; import google.registry.dns.writer.VoidDnsWriter; import google.registry.model.Buildable; import google.registry.model.EppResource; @@ -70,6 +73,7 @@ import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingRecurrence; import google.registry.model.common.DatabaseMigrationStateSchedule; import google.registry.model.common.DatabaseMigrationStateSchedule.MigrationState; +import google.registry.model.common.DnsRefreshRequest; import google.registry.model.contact.Contact; import google.registry.model.contact.ContactAuthInfo; import google.registry.model.contact.ContactHistory; @@ -124,7 +128,7 @@ import org.joda.time.Duration; /** Static utils for setting up test resources. */ public final class DatabaseHelper { - // If the clock is defined, it will always be advanced by one millsecond after a transaction. + // If the clock is defined, it will always be advanced by one millisecond after a transaction. private static FakeClock clock; private static final Supplier DEFAULT_PREMIUM_LIST_CONTENTS = @@ -1352,5 +1356,49 @@ public final class DatabaseHelper { DatabaseMigrationStateSchedule.CACHE.invalidateAll(); } + private static ImmutableList getDnsRefreshRequests(TargetType type, String... names) { + return tm().transact( + () -> + tm().createQueryComposer(DnsRefreshRequest.class).where("type", EQ, type).stream() + .map(DnsRefreshRequest::getName) + .filter(e -> ImmutableList.copyOf(names).contains(e)) + .collect(toImmutableList())); + } + + public static void assertDomainDnsRequests(String... domainNames) { + assertThat(getDnsRefreshRequests(DnsUtils.TargetType.DOMAIN, domainNames)) + .containsExactlyElementsIn(domainNames); + } + + public static void assertHostDnsRequests(String... hostNames) { + assertThat(getDnsRefreshRequests(DnsUtils.TargetType.HOST, hostNames)) + .containsExactlyElementsIn(hostNames); + } + + public static void assertNoDnsRequestsExcept(String... names) { + assertThat( + loadAllOf(DnsRefreshRequest.class).stream() + .filter(e -> !ImmutableList.copyOf(names).contains(e.getName())) + .count()) + .isEqualTo(0); + } + + public static void assertNoDnsRequests() { + assertNoDnsRequestsExcept(); + } + + public static void assertDomainDnsRequestWithRequestTime( + String domainName, DateTime requestTime) { + assertThat( + tm().transact( + () -> + tm().createQueryComposer(DnsRefreshRequest.class) + .where("type", EQ, DnsUtils.TargetType.DOMAIN) + .where("name", EQ, domainName) + .where("requestTime", EQ, requestTime) + .count())) + .isEqualTo(1); + } + private DatabaseHelper() {} } diff --git a/core/src/test/java/google/registry/testing/DnsUtilsHelper.java b/core/src/test/java/google/registry/testing/DnsUtilsHelper.java deleted file mode 100644 index e4bbce268..000000000 --- a/core/src/test/java/google/registry/testing/DnsUtilsHelper.java +++ /dev/null @@ -1,73 +0,0 @@ -// Copyright 2023 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.testing; - -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; - -import google.registry.dns.DnsUtils; -import google.registry.model.annotations.DeleteAfterMigration; -import org.joda.time.Duration; - -/** - * Test helper for {@link DnsUtils}. - * - *

    This is a temporary test class that is only used during DNS pull queue migration. Once we are - * no longer using the pull queue method, we can just assert on the inserted SQL entry instead in - * {@link DatabaseHelper}. - */ -@DeleteAfterMigration -public class DnsUtilsHelper { - - private final DnsUtils dnsUtils; - - public DnsUtilsHelper() { - dnsUtils = mock(DnsUtils.class); - } - - public DnsUtilsHelper(DnsUtils dnsUtils) { - this.dnsUtils = dnsUtils; - } - - public DnsUtils getDnsUtils() { - return dnsUtils; - } - - public void assertDomainDnsRequests(String... domainNames) { - for (String domainName : domainNames) { - verify(dnsUtils).requestDomainDnsRefresh(domainName); - } - } - - public void assertDomainDnsRequestWithDelay(String domainName, Duration delay) { - verify(dnsUtils).requestDomainDnsRefresh(domainName, delay); - } - - public void assertNoDomainDnsRequestWithDelay(String domainName, Duration delay) { - verify(dnsUtils, never()).requestDomainDnsRefresh(domainName, delay); - } - - public void assertHostDnsRequests(String... hostNames) { - for (String hostName : hostNames) { - verify(dnsUtils).requestHostDnsRefresh(hostName); - } - } - - public void assertNoMoreDnsRequests() { - verifyNoMoreInteractions(dnsUtils); - } -} diff --git a/core/src/test/java/google/registry/testing/TaskQueueExtension.java b/core/src/test/java/google/registry/testing/TaskQueueExtension.java deleted file mode 100644 index 5da9d67e3..000000000 --- a/core/src/test/java/google/registry/testing/TaskQueueExtension.java +++ /dev/null @@ -1,61 +0,0 @@ -// Copyright 2023 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.testing; - -import static com.google.common.io.Files.asCharSink; -import static google.registry.util.ResourceUtils.readResourceUtf8; -import static java.nio.charset.StandardCharsets.UTF_8; - -import com.google.appengine.tools.development.testing.LocalServiceTestHelper; -import com.google.appengine.tools.development.testing.LocalTaskQueueTestConfig; -import com.google.apphosting.api.ApiProxy; -import google.registry.model.annotations.DeleteAfterMigration; -import java.nio.file.Files; -import java.nio.file.Path; -import org.junit.jupiter.api.extension.AfterEachCallback; -import org.junit.jupiter.api.extension.BeforeEachCallback; -import org.junit.jupiter.api.extension.ExtensionContext; - -/** JUnit extension that sets up App Engine task queue environment. */ -@DeleteAfterMigration -public final class TaskQueueExtension implements BeforeEachCallback, AfterEachCallback { - - /** - * The GAE testing library requires queue.xml to be a file, not a resource in a jar, so we read it - * in here and write it to a temporary file later. - */ - private static final String QUEUE_XML = - readResourceUtf8("google/registry/env/common/default/WEB-INF/queue.xml"); - - private LocalServiceTestHelper helper; - private Path queueFile; - - @Override - public void beforeEach(ExtensionContext context) throws Exception { - queueFile = Files.createTempFile("queue", ".xml"); - asCharSink(queueFile.toFile(), UTF_8).write(QUEUE_XML); - helper = - new LocalServiceTestHelper( - new LocalTaskQueueTestConfig().setQueueXmlPath(queueFile.toAbsolutePath().toString())); - helper.setUp(); - } - - @Override - public void afterEach(ExtensionContext context) throws Exception { - helper.tearDown(); - Files.delete(queueFile); - ApiProxy.setEnvironmentForCurrentThread(null); - } -} diff --git a/core/src/test/java/google/registry/testing/TaskQueueHelper.java b/core/src/test/java/google/registry/testing/TaskQueueHelper.java deleted file mode 100644 index 318c48e35..000000000 --- a/core/src/test/java/google/registry/testing/TaskQueueHelper.java +++ /dev/null @@ -1,374 +0,0 @@ -// 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.testing; - -import static com.google.appengine.tools.development.testing.LocalTaskQueueTestConfig.getLocalTaskQueue; -import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.base.Preconditions.checkState; -import static com.google.common.base.Predicates.in; -import static com.google.common.base.Predicates.not; -import static com.google.common.collect.ImmutableList.toImmutableList; -import static com.google.common.collect.Iterables.getFirst; -import static com.google.common.collect.Multisets.containsOccurrences; -import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.Truth.assertWithMessage; -import static com.google.common.truth.Truth8.assertThat; -import static google.registry.util.DiffUtils.prettyPrintEntityDeepDiff; -import static java.nio.charset.StandardCharsets.UTF_8; -import static java.util.Arrays.asList; -import static java.util.stream.Collectors.joining; - -import com.google.appengine.api.taskqueue.dev.QueueStateInfo; -import com.google.appengine.api.taskqueue.dev.QueueStateInfo.HeaderWrapper; -import com.google.appengine.api.taskqueue.dev.QueueStateInfo.TaskStateInfo; -import com.google.common.base.Ascii; -import com.google.common.base.Joiner; -import com.google.common.collect.ArrayListMultimap; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMultimap; -import com.google.common.collect.ImmutableMultiset; -import com.google.common.collect.Maps; -import com.google.common.collect.Multimap; -import com.google.common.net.HttpHeaders; -import com.google.common.net.MediaType; -import google.registry.dns.DnsConstants; -import google.registry.model.ImmutableObject; -import java.net.URI; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.NoSuchElementException; -import java.util.Objects; -import java.util.function.Function; -import java.util.function.Predicate; -import javax.annotation.Nonnull; -import org.joda.time.Duration; - -/** Static utility functions for testing task queues. */ -public class TaskQueueHelper { - - /** - * Matcher to match against the tasks in the task queue. Fields that aren't set are not compared. - */ - public static class TaskMatcher implements Predicate { - - private final MatchableTaskInfo expected; - - public TaskMatcher() { - expected = new MatchableTaskInfo(); - } - - /** - * Constructor to create a TaskMatcher that should exactly match an existing TaskStateInfo. - * - * This is useful for checking that a pre-existing task as returned by TaskStateInfo is still - * in the queue; we can't just directly compare the lists of TaskStateInfos because they have - * no equals() override and there's no guarantee that reference equality is sufficient. - */ - private TaskMatcher(TaskStateInfo taskStateInfo) { - expected = new MatchableTaskInfo(taskStateInfo); - } - - public TaskMatcher taskName(String taskName) { - expected.taskName = taskName; - return this; - } - - public TaskMatcher url(String url) { - expected.url = url; - return this; - } - - /** - * Sets the HTTP method to match against. WARNING: due to b/38459667, pull queue tasks will - * report "POST" as their method. - */ - public TaskMatcher method(String method) { - expected.method = method; - return this; - } - - public TaskMatcher payload(String payload) { - checkState(expected.params.isEmpty(), "Cannot add a payload to a TaskMatcher with params"); - expected.payload = payload; - return this; - } - - public TaskMatcher tag(String tag) { - expected.tag = tag; - return this; - } - - public TaskMatcher header(String name, String value) { - // Lowercase for case-insensitive comparison. - expected.headers.put(Ascii.toLowerCase(name), value); - return this; - } - - public TaskMatcher param(String key, String value) { - checkState(expected.payload == null, "Cannot add params to a TaskMatcher with a payload"); - checkNotNull(value, "Test error: A task can never have a null value, so don't assert it"); - expected.params.put(key, value); - return this; - } - - public TaskMatcher etaDelta(Duration lowerBound, Duration upperBound) { - checkState(!lowerBound.isShorterThan(Duration.ZERO), "lowerBound must be non-negative"); - checkState( - upperBound.isLongerThan(lowerBound), "upperBound must be greater than lowerBound"); - expected.etaDeltaLowerBound = lowerBound.getStandardSeconds(); - expected.etaDeltaUpperBound = upperBound.getStandardSeconds(); - return this; - } - - /** - * Returns {@code true} if there are not more occurrences in {@code sub} of each of its entries - * than there are in {@code super}. - */ - private static boolean containsEntries( - Multimap superMultimap, Multimap subMultimap) { - return containsOccurrences( - ImmutableMultiset.copyOf(superMultimap.entries()), - ImmutableMultiset.copyOf(subMultimap.entries())); - } - - /** - * Returns true if the fields set on the current object match the given task info. This is not - * quite the same contract as {@link #equals}, since it will ignore null fields. - * - *

    Match fails if any headers or params expected on the TaskMatcher are not found on the - * TaskStateInfo. Note that the inverse is not true (i.e. there may be extra headers on the - * TaskStateInfo). - */ - @Override - public boolean test(@Nonnull TaskStateInfo info) { - MatchableTaskInfo actual = new MatchableTaskInfo(info); - return (expected.taskName == null || Objects.equals(expected.taskName, actual.taskName)) - && (expected.url == null || Objects.equals(expected.url, actual.url)) - && (expected.method == null || Objects.equals(expected.method, actual.method)) - && (expected.payload == null || Objects.equals(expected.payload, actual.payload)) - && (expected.tag == null || Objects.equals(expected.tag, actual.tag)) - && (expected.etaDeltaLowerBound == null - || expected.etaDeltaLowerBound <= actual.etaDelta) - && (expected.etaDeltaUpperBound == null - || expected.etaDeltaUpperBound >= actual.etaDelta) - && containsEntries(actual.params, expected.params) - && containsEntries(actual.headers, expected.headers); - } - - @Override - public String toString() { - return Joiner.on('\n') - .withKeyValueSeparator(":\n") - .join( - Maps.transformValues( - expected.toMap(), - input -> "\t" + String.valueOf(input).replaceAll("\n", "\n\t"))); - } - } - - /** Returns the info object for the provided queue name. */ - public static QueueStateInfo getQueueInfo(String queueName) { - return getLocalTaskQueue().getQueueStateInfo().get(queueName); - } - - /** - * Ensures that the tasks in the named queue are exactly those with the expected property - * values after being transformed with the provided property getter function. - */ - public static void assertTasksEnqueuedWithProperty( - String queueName, - Function propertyGetter, - String... expectedTaskProperties) { - // Ordering is irrelevant but duplicates should be considered independently. - assertThat(getQueueInfo(queueName).getTaskInfo().stream().map(propertyGetter)) - .containsExactly((Object[]) expectedTaskProperties); - } - - /** Ensures that the tasks in the named queue are exactly those with the expected names. */ - public static void assertTasksEnqueued(String queueName, String... expectedTaskNames) { - Function nameGetter = TaskStateInfo::getTaskName; - assertTasksEnqueuedWithProperty(queueName, nameGetter, expectedTaskNames); - } - - public static void assertTasksEnqueued(String queueName, Iterable taskStateInfos) { - ImmutableList.Builder taskMatchers = new ImmutableList.Builder<>(); - for (TaskStateInfo taskStateInfo : taskStateInfos) { - taskMatchers.add(new TaskMatcher(taskStateInfo)); - } - assertTasksEnqueued(queueName, taskMatchers.build()); - } - - /** - * Ensures that the only tasks in the named queue are exactly those that match the expected - * matchers. - */ - public static void assertTasksEnqueued(String queueName, TaskMatcher... taskMatchers) { - assertTasksEnqueued(queueName, Arrays.asList(taskMatchers)); - } - - /** - * Ensures that the only tasks in the named queue are exactly those that match the expected - * matchers. - */ - public static void assertTasksEnqueued(String queueName, Collection taskMatchers) { - QueueStateInfo qsi = getQueueInfo(queueName); - assertThat(qsi.getTaskInfo()).hasSize(taskMatchers.size()); - List taskInfos = new ArrayList<>(qsi.getTaskInfo()); - for (final TaskMatcher taskMatcher : taskMatchers) { - try { - taskInfos.remove(taskInfos.stream().filter(taskMatcher).findFirst().get()); - } catch (NoSuchElementException e) { - final Map taskMatcherMap = taskMatcher.expected.toMap(); - assertWithMessage( - "Task not found in queue %s:\n\n%s\n\nPotential candidate match diffs:\n\n%s", - queueName, - taskMatcher, - taskInfos.stream() - .map( - input -> - prettyPrintEntityDeepDiff( - taskMatcherMap, - Maps.filterKeys( - new MatchableTaskInfo(input).toMap(), - in(taskMatcherMap.keySet())))) - .collect(joining("\n"))) - .fail(); - } - } - } - - public static ImmutableList> getQueuedParams(String queueName) { - return getQueueInfo(queueName) - .getTaskInfo() - .stream() - .map(MatchableTaskInfo::new) - .map(taskInfo -> ImmutableMultimap.copyOf(taskInfo.params)) - .collect(toImmutableList()); - } - - /** Empties the task queue. */ - public static void clearTaskQueue(String queueName) { - getLocalTaskQueue().flushQueue(queueName); - } - - /** Asserts at least one task exists in {@code queue}. */ - public static void assertAtLeastOneTaskIsEnqueued(String queue) { - assertThat(getQueueInfo(queue).getCountTasks()).isGreaterThan(0); - } - - /** Ensures that the named queue contains no tasks. */ - public static void assertNoTasksEnqueued(String ... queueNames) { - for (String queueName : queueNames) { - assertThat(getQueueInfo(queueName).getCountTasks()).isEqualTo(0); - } - } - - /** Returns the value for the param on a task info, or empty if it is missing. */ - private static String getParamFromTaskInfo(TaskStateInfo taskInfo, String paramName) { - return getFirst(UriParameters.parse(taskInfo.getBody()).get(paramName), ""); - } - - /** Ensures that the DNS queue tasks are exactly those for the expected target names. */ - public static void assertDnsTasksEnqueued(String... expectedTaskTargetNames) { - assertTasksEnqueuedWithProperty( - DnsConstants.DNS_PULL_QUEUE_NAME, - taskStateInfo -> getParamFromTaskInfo(taskStateInfo, DnsConstants.DNS_TARGET_NAME_PARAM), - expectedTaskTargetNames); - } - - /** Ensures that the DNS queue does not contain any tasks. */ - public static void assertNoDnsTasksEnqueued() { - assertNoTasksEnqueued(DnsConstants.DNS_PULL_QUEUE_NAME); - } - - /** An adapter to clean up a {@link TaskStateInfo} for ease of matching. */ - private static class MatchableTaskInfo extends ImmutableObject { - - String taskName; - String method; - String url; - String payload; - String tag; - Double etaDelta; - Long etaDeltaLowerBound; - Long etaDeltaUpperBound; - Multimap headers = ArrayListMultimap.create(); - Multimap params = ArrayListMultimap.create(); - - MatchableTaskInfo() {} - - MatchableTaskInfo(TaskStateInfo info) { - URI uri; - try { - uri = new URI(info.getUrl()); - } catch (java.net.URISyntaxException e) { - throw new IllegalArgumentException(e); - } - this.taskName = info.getTaskName(); - this.method = info.getMethod(); - this.url = uri.getPath(); - this.payload = info.getBody(); - this.etaDelta = info.getEtaDelta(); - if (info.getTagAsBytes() != null) { - this.tag = new String(info.getTagAsBytes(), UTF_8); - } - ImmutableMultimap.Builder headerBuilder = new ImmutableMultimap.Builder<>(); - for (HeaderWrapper header : info.getHeaders()) { - // Lowercase header name for comparison since HTTP - // header names are case-insensitive. - headerBuilder.put(Ascii.toLowerCase(header.getKey()), header.getValue()); - } - this.headers = headerBuilder.build(); - ImmutableMultimap.Builder inputParams = new ImmutableMultimap.Builder<>(); - String query = uri.getQuery(); - if (query != null) { - inputParams.putAll(UriParameters.parse(query)); - } - boolean hasFormDataContentType = - headers.containsEntry( - Ascii.toLowerCase(HttpHeaders.CONTENT_TYPE), MediaType.FORM_DATA.toString()); - // Try decoding the body as a parameter map if it either has the "x-www-form-urlencoded" - // content type, or if it's a POST or PULL task (in which case parameters should be encoded - // into the body automatically upon being enqueued). Note that pull queue tasks also report - // "POST" as their method (which is misleading - see b/38459667) so we just check for "POST". - if (hasFormDataContentType || "POST".equals(this.method)) { - // Note that UriParameters.parse() does not throw an IAE on a bad query string (e.g. one - // where parameters are not properly URL-encoded); it always does a best-effort parse. - inputParams.putAll(UriParameters.parse(info.getBody())); - } - this.params = inputParams.build(); - } - - public Map toMap() { - Map builder = new HashMap<>(); - builder.put("taskName", taskName); - builder.put("url", url); - builder.put("method", method); - builder.put("headers", headers.asMap()); - builder.put("params", params.asMap()); - builder.put("payload", payload); - builder.put("tag", tag); - builder.put("etaDelta", etaDelta); - builder.put("etaDeltaLowerBound", etaDeltaLowerBound); - builder.put("etaDeltaUpperBound", etaDeltaUpperBound); - return Maps.filterValues(builder, not(in(asList(null, "", Collections.EMPTY_MAP)))); - } - } -} 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 75bb48b6f..0892169fb 100644 --- a/core/src/test/java/google/registry/tools/server/RefreshDnsForAllDomainsActionTest.java +++ b/core/src/test/java/google/registry/tools/server/RefreshDnsForAllDomainsActionTest.java @@ -15,27 +15,20 @@ package google.registry.tools.server; import static com.google.common.truth.Truth.assertThat; +import static google.registry.testing.DatabaseHelper.assertDomainDnsRequestWithRequestTime; +import static google.registry.testing.DatabaseHelper.assertNoDnsRequestsExcept; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.persistActiveDomain; import static google.registry.testing.DatabaseHelper.persistDeletedDomain; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verifyNoMoreInteractions; import com.google.common.collect.ImmutableSet; -import google.registry.dns.DnsUtils; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; -import google.registry.testing.DnsUtilsHelper; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import java.util.Random; -import org.apache.http.HttpStatus; import org.joda.time.DateTime; -import org.joda.time.Duration; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -44,14 +37,12 @@ import org.junit.jupiter.api.extension.RegisterExtension; public class RefreshDnsForAllDomainsActionTest { private final FakeClock clock = new FakeClock(DateTime.parse("2020-02-02T02:02:02Z")); - private final DnsUtils dnsUtils = mock(DnsUtils.class); - private final DnsUtilsHelper dnsUtilsHelper = new DnsUtilsHelper(dnsUtils); private RefreshDnsForAllDomainsAction action; private final FakeResponse response = new FakeResponse(); @RegisterExtension final JpaIntegrationTestExtension jpa = - new JpaTestExtensions.Builder().buildIntegrationTestExtension(); + new JpaTestExtensions.Builder().withClock(clock).buildIntegrationTestExtension(); @BeforeEach void beforeEach() { @@ -60,37 +51,18 @@ public class RefreshDnsForAllDomainsActionTest { action.random = new Random(); action.random.setSeed(123L); action.clock = clock; - action.dnsUtils = dnsUtils; action.response = response; - createTld("bar"); } - @Test - void test_runAction_errorRequestDnsRefresh() throws Exception { - persistActiveDomain("foo.bar"); - persistActiveDomain("baz.bar"); - persistActiveDomain("low.bar"); - action.tlds = ImmutableSet.of("bar"); - doThrow(new RuntimeException("Error enqueuing task.")) - .when(dnsUtils) - .requestDomainDnsRefresh(eq("baz.bar"), any(Duration.class)); - action.run(); - dnsUtilsHelper.assertDomainDnsRequestWithDelay("low.bar", Duration.ZERO); - dnsUtilsHelper.assertDomainDnsRequestWithDelay("baz.bar", Duration.ZERO); - dnsUtilsHelper.assertDomainDnsRequestWithDelay("foo.bar", Duration.ZERO); - verifyNoMoreInteractions(dnsUtils); - assertThat(response.getStatus()).isEqualTo(HttpStatus.SC_INTERNAL_SERVER_ERROR); - } - @Test void test_runAction_successfullyEnqueuesDnsRefreshes() throws Exception { persistActiveDomain("foo.bar"); persistActiveDomain("low.bar"); action.tlds = ImmutableSet.of("bar"); action.run(); - dnsUtilsHelper.assertDomainDnsRequestWithDelay("foo.bar", Duration.ZERO); - dnsUtilsHelper.assertDomainDnsRequestWithDelay("low.bar", Duration.ZERO); + assertDomainDnsRequestWithRequestTime("foo.bar", clock.nowUtc()); + assertDomainDnsRequestWithRequestTime("low.bar", clock.nowUtc()); } @Test @@ -100,8 +72,8 @@ public class RefreshDnsForAllDomainsActionTest { action.tlds = ImmutableSet.of("bar"); action.smearMinutes = 1000; action.run(); - dnsUtilsHelper.assertDomainDnsRequestWithDelay("foo.bar", Duration.standardMinutes(450)); - dnsUtilsHelper.assertDomainDnsRequestWithDelay("low.bar", Duration.standardMinutes(782)); + assertDomainDnsRequestWithRequestTime("foo.bar", clock.nowUtc().plusMinutes(450)); + assertDomainDnsRequestWithRequestTime("low.bar", clock.nowUtc().plusMinutes(782)); } @Test @@ -110,8 +82,8 @@ public class RefreshDnsForAllDomainsActionTest { persistDeletedDomain("deleted.bar", clock.nowUtc().minusYears(1)); action.tlds = ImmutableSet.of("bar"); action.run(); - dnsUtilsHelper.assertDomainDnsRequestWithDelay("foo.bar", Duration.ZERO); - dnsUtilsHelper.assertNoDomainDnsRequestWithDelay("deleted.bar", Duration.ZERO); + assertDomainDnsRequestWithRequestTime("foo.bar", clock.nowUtc()); + assertNoDnsRequestsExcept("foo.bar"); } @Test @@ -122,9 +94,9 @@ public class RefreshDnsForAllDomainsActionTest { persistActiveDomain("ignore.baz"); action.tlds = ImmutableSet.of("bar"); action.run(); - dnsUtilsHelper.assertDomainDnsRequestWithDelay("foo.bar", Duration.ZERO); - dnsUtilsHelper.assertDomainDnsRequestWithDelay("low.bar", Duration.ZERO); - dnsUtilsHelper.assertNoDomainDnsRequestWithDelay("ignore.baz", Duration.ZERO); + assertDomainDnsRequestWithRequestTime("foo.bar", clock.nowUtc()); + assertDomainDnsRequestWithRequestTime("low.bar", clock.nowUtc()); + assertNoDnsRequestsExcept("foo.bar", "low.bar"); } @Test diff --git a/core/src/test/resources/google/registry/module/backend/backend_routing.txt b/core/src/test/resources/google/registry/module/backend/backend_routing.txt index 62b3e241b..49fc24c1f 100644 --- a/core/src/test/resources/google/registry/module/backend/backend_routing.txt +++ b/core/src/test/resources/google/registry/module/backend/backend_routing.txt @@ -1,6 +1,5 @@ PATH CLASS METHODS OK AUTH_METHODS MIN USER_POLICY /_dr/cron/fanout TldFanoutAction GET y INTERNAL,API APP ADMIN -/_dr/cron/readDnsQueue ReadDnsQueueAction GET y INTERNAL,API APP ADMIN /_dr/dnsRefresh RefreshDnsAction GET y INTERNAL,API APP ADMIN /_dr/task/brdaCopy BrdaCopyAction POST y INTERNAL,API APP ADMIN /_dr/task/copyDetailReports CopyDetailReportsAction POST n INTERNAL,API APP ADMIN