From 961f9e7844b0e97f64dfef6ad4d0b858a6744c6b Mon Sep 17 00:00:00 2001 From: Lai Jiang Date: Wed, 9 Nov 2022 17:21:20 -0500 Subject: [PATCH] Re-add RefreshDnsOnHostRenameAction (#1845) This class was accidentally deleted in #1661. This PR recreates it by mostly re-adding its SQL-based code flow: https://cs.opensource.google/nomulus/nomulus/+/master:core/src/test/java/google/registry/batch/RefreshDnsOnHostRenameActionTest.java;drc=9912e35ea297e969a428efdb1f8f01c86d794305;bpv=0;bpt=0 It does away with a pull queue due to incompatibility with Cloud Tasks. Given what we have seen (about 700 tasks enqueued since May 2022), it does not add much value in batching this operation anyway. Also deleted AsyncTaskMetrics, which is not used any more. I don't think we need to re-add metrics for this class either. --- .../registry/batch/AsyncTaskEnqueuer.java | 3 - .../registry/batch/AsyncTaskMetrics.java | 165 ------------------ .../java/google/registry/dns/DnsModule.java | 9 +- .../dns/RefreshDnsOnHostRenameAction.java | 90 ++++++++++ .../env/common/backend/WEB-INF/web.xml | 6 - .../env/common/default/WEB-INF/queue.xml | 6 + .../flows/contact/ContactDeleteFlow.java | 2 - .../registry/flows/host/HostDeleteFlow.java | 2 - .../registry/flows/host/HostUpdateFlow.java | 24 ++- .../backend/BackendRequestComponent.java | 3 + .../registry/batch/AsyncTaskMetricsTest.java | 47 ----- .../dns/RefreshDnsOnHostRenameActionTest.java | 110 ++++++++++++ .../registry/flows/EppTestComponent.java | 6 + .../domain/DomainTransferRequestFlowTest.java | 22 +-- .../flows/host/HostUpdateFlowTest.java | 18 +- .../module/backend/backend_routing.txt | 1 + 16 files changed, 261 insertions(+), 253 deletions(-) delete mode 100644 core/src/main/java/google/registry/batch/AsyncTaskMetrics.java create mode 100644 core/src/main/java/google/registry/dns/RefreshDnsOnHostRenameAction.java delete mode 100644 core/src/test/java/google/registry/batch/AsyncTaskMetricsTest.java create mode 100644 core/src/test/java/google/registry/dns/RefreshDnsOnHostRenameActionTest.java diff --git a/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java b/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java index f0c567c28..9b2f151be 100644 --- a/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java +++ b/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java @@ -35,15 +35,12 @@ public final class AsyncTaskEnqueuer { /** The HTTP parameter names used by async flows. */ public static final String PARAM_RESOURCE_KEY = "resourceKey"; - public static final String PARAM_HOST_KEY = "hostKey"; public static final String PARAM_REQUESTED_TIME = "requestedTime"; public static final String PARAM_RESAVE_TIMES = "resaveTimes"; /** The task queue names used by async flows. */ public static final String QUEUE_ASYNC_ACTIONS = "async-actions"; - public static final String QUEUE_ASYNC_HOST_RENAME = "async-host-rename-pull"; - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final Duration MAX_ASYNC_ETA = Duration.standardDays(30); diff --git a/core/src/main/java/google/registry/batch/AsyncTaskMetrics.java b/core/src/main/java/google/registry/batch/AsyncTaskMetrics.java deleted file mode 100644 index 019110a75..000000000 --- a/core/src/main/java/google/registry/batch/AsyncTaskMetrics.java +++ /dev/null @@ -1,165 +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.batch; - -import static com.google.appengine.api.taskqueue.QueueConstants.maxLeaseCount; -import static com.google.monitoring.metrics.EventMetric.DEFAULT_FITTER; -import static google.registry.batch.AsyncTaskMetrics.OperationType.CONTACT_AND_HOST_DELETE; -import static google.registry.batch.AsyncTaskMetrics.OperationType.DNS_REFRESH; - -import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ImmutableSet; -import com.google.common.flogger.FluentLogger; -import com.google.monitoring.metrics.DistributionFitter; -import com.google.monitoring.metrics.EventMetric; -import com.google.monitoring.metrics.FibonacciFitter; -import com.google.monitoring.metrics.IncrementableMetric; -import com.google.monitoring.metrics.LabelDescriptor; -import com.google.monitoring.metrics.MetricRegistryImpl; -import google.registry.util.Clock; -import javax.inject.Inject; -import org.joda.time.DateTime; -import org.joda.time.Duration; - -/** - * Instrumentation for async flows (contact/host deletion and DNS refreshes). - * - * @see AsyncTaskEnqueuer - */ -public class AsyncTaskMetrics { - - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - - private final Clock clock; - - @Inject - public AsyncTaskMetrics(Clock clock) { - this.clock = clock; - } - - /** - * A Fibonacci fitter used for bucketing the batch count. - * - *

We use a Fibonacci filter because it provides better resolution at the low end than an - * exponential fitter, which is important because most batch sizes are likely to be very low, - * despite going up to 1,000 on the high end. Also, the precision is better, as batch size is - * inherently an integer, whereas an exponential fitter with an exponent base less than 2 would - * have unintuitive boundaries. - */ - private static final DistributionFitter FITTER_BATCH_SIZE = - FibonacciFitter.create(maxLeaseCount()); - - private static final ImmutableSet LABEL_DESCRIPTORS = - ImmutableSet.of( - LabelDescriptor.create("operation_type", "The type of async flow operation."), - LabelDescriptor.create("result", "The result of the async flow operation.")); - - @VisibleForTesting - static final IncrementableMetric asyncFlowOperationCounts = - MetricRegistryImpl.getDefault() - .newIncrementableMetric( - "/async_flows/operations", - "Count of Async Flow Operations", - "count", - LABEL_DESCRIPTORS); - - @VisibleForTesting - static final EventMetric asyncFlowOperationProcessingTime = - MetricRegistryImpl.getDefault() - .newEventMetric( - "/async_flows/processing_time", - "Async Flow Processing Time", - "milliseconds", - LABEL_DESCRIPTORS, - DEFAULT_FITTER); - - @VisibleForTesting - static final EventMetric asyncFlowBatchSize = - MetricRegistryImpl.getDefault() - .newEventMetric( - "/async_flows/batch_size", - "Async Operation Batch Size", - "batch size", - ImmutableSet.of( - LabelDescriptor.create("operation_type", "The type of async flow operation.")), - FITTER_BATCH_SIZE); - - /** The type of asynchronous operation. */ - public enum OperationType { - CONTACT_DELETE("contactDelete"), - HOST_DELETE("hostDelete"), - CONTACT_AND_HOST_DELETE("contactAndHostDelete"), - DNS_REFRESH("dnsRefresh"); - - private final String metricLabelValue; - - OperationType(String metricLabelValue) { - this.metricLabelValue = metricLabelValue; - } - - String getMetricLabelValue() { - return metricLabelValue; - } - } - - /** The result of an asynchronous operation. */ - public enum OperationResult { - /** The operation processed correctly and the result was success. */ - SUCCESS("success"), - - /** The operation processed correctly and the result was failure. */ - FAILURE("failure"), - - /** The operation did not process correctly due to some unexpected error. */ - ERROR("error"), - - /** The operation was skipped because the request is now stale. */ - STALE("stale"); - - private final String metricLabelValue; - - OperationResult(String metricLabelValue) { - this.metricLabelValue = metricLabelValue; - } - - String getMetricLabelValue() { - return metricLabelValue; - } - } - - public void recordAsyncFlowResult( - OperationType operationType, OperationResult operationResult, DateTime whenEnqueued) { - asyncFlowOperationCounts.increment( - operationType.getMetricLabelValue(), operationResult.getMetricLabelValue()); - long processingMillis = new Duration(whenEnqueued, clock.nowUtc()).getMillis(); - asyncFlowOperationProcessingTime.record( - processingMillis, - operationType.getMetricLabelValue(), - operationResult.getMetricLabelValue()); - logger.atInfo().log( - "Asynchronous %s operation took %d ms to process, yielding result: %s.", - operationType.getMetricLabelValue(), - processingMillis, - operationResult.getMetricLabelValue()); - } - - public void recordContactHostDeletionBatchSize(long batchSize) { - asyncFlowBatchSize.record(batchSize, CONTACT_AND_HOST_DELETE.getMetricLabelValue()); - } - - public void recordDnsRefreshBatchSize(long batchSize) { - asyncFlowBatchSize.record(batchSize, DNS_REFRESH.getMetricLabelValue()); - } -} diff --git a/core/src/main/java/google/registry/dns/DnsModule.java b/core/src/main/java/google/registry/dns/DnsModule.java index 4a06206c2..4b182eb2a 100644 --- a/core/src/main/java/google/registry/dns/DnsModule.java +++ b/core/src/main/java/google/registry/dns/DnsModule.java @@ -16,6 +16,7 @@ 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; import static google.registry.request.RequestParameters.extractRequiredParameter; @@ -61,7 +62,7 @@ public abstract class DnsModule { */ @Provides static HashFunction provideHashFunction() { - return Hashing.murmur3_32(); + return Hashing.murmur3_32_fixed(); } @Provides @@ -118,6 +119,12 @@ public abstract class DnsModule { return extractSetOfParameters(req, PARAM_HOSTS); } + @Provides + @Parameter(PARAM_HOST_KEY) + static String provideResourceKey(HttpServletRequest req) { + return extractRequiredParameter(req, PARAM_HOST_KEY); + } + @Provides @Parameter("domainOrHostName") static String provideName(HttpServletRequest req) { diff --git a/core/src/main/java/google/registry/dns/RefreshDnsOnHostRenameAction.java b/core/src/main/java/google/registry/dns/RefreshDnsOnHostRenameAction.java new file mode 100644 index 000000000..7e27281a0 --- /dev/null +++ b/core/src/main/java/google/registry/dns/RefreshDnsOnHostRenameAction.java @@ -0,0 +1,90 @@ +// Copyright 2022 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 google.registry.dns.RefreshDnsOnHostRenameAction.PATH; +import static google.registry.model.EppResourceUtils.getLinkedDomainKeys; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; +import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT; + +import com.google.common.net.MediaType; +import google.registry.model.EppResourceUtils; +import google.registry.model.domain.Domain; +import google.registry.model.host.Host; +import google.registry.persistence.VKey; +import google.registry.request.Action; +import google.registry.request.Action.Service; +import google.registry.request.Parameter; +import google.registry.request.Response; +import google.registry.request.auth.Auth; +import javax.inject.Inject; +import org.joda.time.DateTime; + +@Action( + service = Service.BACKEND, + path = PATH, + method = Action.Method.POST, + auth = Auth.AUTH_INTERNAL_OR_ADMIN) +public class RefreshDnsOnHostRenameAction implements Runnable { + + public static final String QUEUE_HOST_RENAME = "async-host-rename"; + public static final String PARAM_HOST_KEY = "hostKey"; + public static final String PATH = "/_dr/task/refreshDnsOnHostRename"; + + private final VKey hostKey; + private final Response response; + private final DnsQueue dnsQueue; + + @Inject + RefreshDnsOnHostRenameAction( + @Parameter(PARAM_HOST_KEY) String hostKey, Response response, DnsQueue dnsQueue) { + this.hostKey = VKey.createEppVKeyFromString(hostKey); + this.response = response; + this.dnsQueue = dnsQueue; + } + + @Override + public void run() { + tm().transact( + () -> { + DateTime now = tm().getTransactionTime(); + Host host = tm().loadByKeyIfPresent(hostKey).orElse(null); + boolean hostValid = true; + String failureMessage = null; + if (host == null) { + hostValid = false; + failureMessage = String.format("Host to refresh does not exist: %s", hostKey); + } else if (EppResourceUtils.isDeleted(host, now)) { + hostValid = false; + failureMessage = + String.format("Host to refresh is already deleted: %s", host.getHostName()); + } else { + getLinkedDomainKeys( + host.createVKey(), host.getUpdateTimestamp().getTimestamp(), null) + .stream() + .map(domainKey -> tm().loadByKey(domainKey)) + .filter(Domain::shouldPublishToDns) + .forEach(domain -> dnsQueue.addDomainRefreshTask(domain.getDomainName())); + } + + if (!hostValid) { + // Set the response status code to be 204 so to not retry. + response.setContentType(MediaType.PLAIN_TEXT_UTF_8); + response.setStatus(SC_NO_CONTENT); + response.setPayload(failureMessage); + } + }); + } +} 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 8ffdfa4a4..573a57d06 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 @@ -244,12 +244,6 @@ /_dr/task/resaveEntity - - - backend-servlet - /_dr/task/dnsRefreshForHostRename - - 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 6fc9c9ab1..6700c2420 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 @@ -46,6 +46,12 @@ + + + async-host-rename + 1/s + + beam-reporting diff --git a/core/src/main/java/google/registry/flows/contact/ContactDeleteFlow.java b/core/src/main/java/google/registry/flows/contact/ContactDeleteFlow.java index 938fba9fb..4e55cb6ce 100644 --- a/core/src/main/java/google/registry/flows/contact/ContactDeleteFlow.java +++ b/core/src/main/java/google/registry/flows/contact/ContactDeleteFlow.java @@ -27,7 +27,6 @@ import static google.registry.model.transfer.TransferStatus.SERVER_CANCELLED; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.common.collect.ImmutableSet; -import google.registry.batch.AsyncTaskEnqueuer; import google.registry.flows.EppException; import google.registry.flows.ExtensionManager; import google.registry.flows.FlowModule.RegistrarId; @@ -79,7 +78,6 @@ public final class ContactDeleteFlow implements TransactionalFlow { @Inject @Superuser boolean isSuperuser; @Inject Optional authInfo; @Inject ContactHistory.Builder historyBuilder; - @Inject AsyncTaskEnqueuer asyncTaskEnqueuer; @Inject EppResponse.Builder responseBuilder; @Inject 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 1b766f0ce..0b5160eef 100644 --- a/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java +++ b/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java @@ -24,7 +24,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.batch.AsyncTaskEnqueuer; import google.registry.dns.DnsQueue; import google.registry.flows.EppException; import google.registry.flows.ExtensionManager; @@ -79,7 +78,6 @@ public final class HostDeleteFlow implements TransactionalFlow { @Inject Trid trid; @Inject @Superuser boolean isSuperuser; @Inject HostHistory.Builder historyBuilder; - @Inject AsyncTaskEnqueuer asyncTaskEnqueuer; @Inject EppResponse.Builder responseBuilder; @Inject 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 665ad7a9d..abf303faa 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,8 @@ 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.RefreshDnsOnHostRenameAction.PARAM_HOST_KEY; +import static google.registry.dns.RefreshDnsOnHostRenameAction.QUEUE_HOST_RENAME; import static google.registry.flows.FlowUtils.validateRegistrarIsLoggedIn; import static google.registry.flows.ResourceFlowUtils.checkSameValuesNotAddedAndRemoved; import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; @@ -30,9 +32,12 @@ import static google.registry.model.reporting.HistoryEntry.Type.HOST_UPDATE; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.util.CollectionUtils.isNullOrEmpty; +import com.google.cloud.tasks.v2.Task; +import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; import google.registry.batch.AsyncTaskEnqueuer; import google.registry.dns.DnsQueue; +import google.registry.dns.RefreshDnsOnHostRenameAction; import google.registry.flows.EppException; import google.registry.flows.EppException.ObjectAlreadyExistsException; import google.registry.flows.EppException.ParameterValueRangeErrorException; @@ -59,6 +64,8 @@ import google.registry.model.host.HostCommand.Update.Change; import google.registry.model.host.HostHistory; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; import google.registry.persistence.VKey; +import google.registry.request.Action.Service; +import google.registry.util.CloudTasksUtils; import java.util.Objects; import java.util.Optional; import javax.inject.Inject; @@ -107,9 +114,8 @@ public final class HostUpdateFlow implements TransactionalFlow { * requires special checking, since you must be able to clear the status off the object with an * update. */ - private static final ImmutableSet DISALLOWED_STATUSES = ImmutableSet.of( - StatusValue.PENDING_DELETE, - StatusValue.SERVER_UPDATE_PROHIBITED); + private static final ImmutableSet DISALLOWED_STATUSES = + ImmutableSet.of(StatusValue.PENDING_DELETE, StatusValue.SERVER_UPDATE_PROHIBITED); @Inject ResourceCommand resourceCommand; @Inject ExtensionManager extensionManager; @@ -120,7 +126,10 @@ public final class HostUpdateFlow implements TransactionalFlow { @Inject AsyncTaskEnqueuer asyncTaskEnqueuer; @Inject DnsQueue dnsQueue; @Inject EppResponse.Builder responseBuilder; - @Inject HostUpdateFlow() {} + @Inject CloudTasksUtils cloudTasksUtils; + + @Inject + HostUpdateFlow() {} @Override public EppResponse run() throws EppException { @@ -268,7 +277,12 @@ public final class HostUpdateFlow implements TransactionalFlow { } // 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. - // TODO(jianglai): implement a SQL based solution. + Task task = + cloudTasksUtils.createPostTask( + RefreshDnsOnHostRenameAction.PATH, + Service.BACKEND.toString(), + ImmutableMultimap.of(PARAM_HOST_KEY, existingHost.createVKey().stringify())); + cloudTasksUtils.enqueue(QUEUE_HOST_RENAME, task); } } 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 dca1f8170..be7ca5859 100644 --- a/core/src/main/java/google/registry/module/backend/BackendRequestComponent.java +++ b/core/src/main/java/google/registry/module/backend/BackendRequestComponent.java @@ -34,6 +34,7 @@ import google.registry.dns.DnsModule; import google.registry.dns.PublishDnsUpdatesAction; import google.registry.dns.ReadDnsQueueAction; import google.registry.dns.RefreshDnsAction; +import google.registry.dns.RefreshDnsOnHostRenameAction; import google.registry.dns.writer.VoidDnsWriterModule; import google.registry.dns.writer.clouddns.CloudDnsWriterModule; import google.registry.dns.writer.dnsupdate.DnsUpdateConfigModule; @@ -153,6 +154,8 @@ interface BackendRequestComponent { RefreshDnsAction refreshDnsAction(); + RefreshDnsOnHostRenameAction refreshDnsOnHostRenameAction(); + RelockDomainAction relockDomainAction(); ResaveAllEppResourcesPipelineAction resaveAllEppResourcesPipelineAction(); diff --git a/core/src/test/java/google/registry/batch/AsyncTaskMetricsTest.java b/core/src/test/java/google/registry/batch/AsyncTaskMetricsTest.java deleted file mode 100644 index d4af63ffe..000000000 --- a/core/src/test/java/google/registry/batch/AsyncTaskMetricsTest.java +++ /dev/null @@ -1,47 +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.batch; - -import static com.google.monitoring.metrics.contrib.DistributionMetricSubject.assertThat; -import static com.google.monitoring.metrics.contrib.LongMetricSubject.assertThat; -import static google.registry.batch.AsyncTaskMetrics.OperationResult.SUCCESS; -import static google.registry.batch.AsyncTaskMetrics.OperationType.CONTACT_AND_HOST_DELETE; - -import com.google.common.collect.ImmutableSet; -import google.registry.testing.FakeClock; -import org.junit.jupiter.api.Test; - -/** Unit tests for {@link AsyncTaskMetrics}. */ -class AsyncTaskMetricsTest { - - private final FakeClock clock = new FakeClock(); - private final AsyncTaskMetrics asyncTaskMetrics = new AsyncTaskMetrics(clock); - - @Test - void testRecordAsyncFlowResult_calculatesDurationMillisCorrectly() { - asyncTaskMetrics.recordAsyncFlowResult( - CONTACT_AND_HOST_DELETE, - SUCCESS, - clock.nowUtc().minusMinutes(10).minusSeconds(5).minusMillis(566)); - assertThat(AsyncTaskMetrics.asyncFlowOperationCounts) - .hasValueForLabels(1, "contactAndHostDelete", "success") - .and() - .hasNoOtherValues(); - assertThat(AsyncTaskMetrics.asyncFlowOperationProcessingTime) - .hasDataSetForLabels(ImmutableSet.of(605566.0), "contactAndHostDelete", "success") - .and() - .hasNoOtherValues(); - } -} diff --git a/core/src/test/java/google/registry/dns/RefreshDnsOnHostRenameActionTest.java b/core/src/test/java/google/registry/dns/RefreshDnsOnHostRenameActionTest.java new file mode 100644 index 000000000..8233d66e3 --- /dev/null +++ b/core/src/test/java/google/registry/dns/RefreshDnsOnHostRenameActionTest.java @@ -0,0 +1,110 @@ +// Copyright 2022 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.DatabaseHelper.newDomain; +import static google.registry.testing.DatabaseHelper.persistActiveHost; +import static google.registry.testing.DatabaseHelper.persistDeletedHost; +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 static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +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.FakeClock; +import google.registry.testing.FakeResponse; +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 RefreshDnsOnHostRenameAction}. */ +public class RefreshDnsOnHostRenameActionTest { + + private final FakeClock clock = new FakeClock(DateTime.parse("2015-01-15T11:22:33Z")); + private final DnsQueue dnsQueue = mock(DnsQueue.class); + private final FakeResponse response = new FakeResponse(); + + @RegisterExtension + final JpaIntegrationTestExtension jpa = + new JpaTestExtensions.Builder().withClock(clock).buildIntegrationTestExtension(); + + private RefreshDnsOnHostRenameAction action; + + private void createAction(String hostKey) { + action = new RefreshDnsOnHostRenameAction(hostKey, response, dnsQueue); + } + + private void assertDnsTasksEnqueued(String... domains) { + for (String domain : domains) { + verify(dnsQueue).addDomainRefreshTask(domain); + } + verifyNoMoreInteractions(dnsQueue); + } + + @BeforeEach + void beforeEach() { + createTld("tld"); + } + + @Test + void testSuccess() { + Host host = persistActiveHost("ns1.example.tld"); + persistResource(newDomain("example.tld", host)); + persistResource(newDomain("otherexample.tld", host)); + persistResource(newDomain("untouched.tld", persistActiveHost("ns2.example.tld"))); + persistResource( + newDomain("suspended.tld", host) + .asBuilder() + .setStatusValues(ImmutableSet.of(StatusValue.CLIENT_HOLD)) + .build()); + persistDomainAsDeleted(newDomain("deleted.tld", host), clock.nowUtc().minusDays(1)); + createAction(host.createVKey().stringify()); + action.run(); + assertDnsTasksEnqueued("example.tld", "otherexample.tld"); + assertThat(response.getStatus()).isEqualTo(SC_OK); + } + + @Test + void testFailure_nonexistentHost() { + createAction("kind:Host@sql:rO0ABXQABGJsYWg"); + action.run(); + assertDnsTasksEnqueued(); + assertThat(response.getStatus()).isEqualTo(SC_NO_CONTENT); + assertThat(response.getPayload()) + .isEqualTo("Host to refresh does not exist: VKey(sql:blah)"); + } + + @Test + void testFailure_deletedHost() { + Host host = persistDeletedHost("ns1.example.tld", clock.nowUtc().minusDays(1)); + persistResource(newDomain("example.tld", host)); + createAction(host.createVKey().stringify()); + action.run(); + assertDnsTasksEnqueued(); + 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/EppTestComponent.java b/core/src/test/java/google/registry/flows/EppTestComponent.java index beeda8fba..b60039c7a 100644 --- a/core/src/test/java/google/registry/flows/EppTestComponent.java +++ b/core/src/test/java/google/registry/flows/EppTestComponent.java @@ -36,6 +36,7 @@ import google.registry.testing.FakeSleeper; import google.registry.tmch.TmchCertificateAuthority; import google.registry.tmch.TmchXmlSignature; import google.registry.util.Clock; +import google.registry.util.CloudTasksUtils; import google.registry.util.Sleeper; import javax.inject.Singleton; @@ -89,6 +90,11 @@ public interface EppTestComponent { return asyncTaskEnqueuer; } + @Provides + CloudTasksUtils provideCloudTasksUtils() { + return cloudTasksHelper.getTestCloudTasksUtils(); + } + @Provides Clock provideClock() { return clock; diff --git a/core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java index a60349f5d..a6ff7362f 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java @@ -372,18 +372,14 @@ class DomainTransferRequestFlowTest assertThat(transferApprovedPollMessage.getEventTime()).isEqualTo(implicitTransferTime); assertThat(autorenewPollMessage.getEventTime()).isEqualTo(expectedExpirationTime); assertThat( - transferApprovedPollMessage - .getResponseData() - .stream() + transferApprovedPollMessage.getResponseData().stream() .filter(TransferResponse.class::isInstance) .map(TransferResponse.class::cast) .collect(onlyElement()) .getTransferStatus()) .isEqualTo(TransferStatus.SERVER_APPROVED); PendingActionNotificationResponse panData = - transferApprovedPollMessage - .getResponseData() - .stream() + transferApprovedPollMessage.getResponseData().stream() .filter(PendingActionNotificationResponse.class::isInstance) .map(PendingActionNotificationResponse.class::cast) .collect(onlyElement()); @@ -394,30 +390,24 @@ class DomainTransferRequestFlowTest // transfer pending message, and a transfer approved message (both OneTime messages). assertThat(getPollMessages("TheRegistrar", implicitTransferTime)).hasSize(2); PollMessage losingTransferPendingPollMessage = - getPollMessages("TheRegistrar", clock.nowUtc()) - .stream() + getPollMessages("TheRegistrar", clock.nowUtc()).stream() .filter(pollMessage -> TransferStatus.PENDING.getMessage().equals(pollMessage.getMsg())) .collect(onlyElement()); PollMessage losingTransferApprovedPollMessage = - getPollMessages("TheRegistrar", implicitTransferTime) - .stream() + getPollMessages("TheRegistrar", implicitTransferTime).stream() .filter(Predicates.not(Predicates.equalTo(losingTransferPendingPollMessage))) .collect(onlyElement()); assertThat(losingTransferPendingPollMessage.getEventTime()).isEqualTo(clock.nowUtc()); assertThat(losingTransferApprovedPollMessage.getEventTime()).isEqualTo(implicitTransferTime); assertThat( - losingTransferPendingPollMessage - .getResponseData() - .stream() + losingTransferPendingPollMessage.getResponseData().stream() .filter(TransferResponse.class::isInstance) .map(TransferResponse.class::cast) .collect(onlyElement()) .getTransferStatus()) .isEqualTo(TransferStatus.PENDING); assertThat( - losingTransferApprovedPollMessage - .getResponseData() - .stream() + losingTransferApprovedPollMessage.getResponseData().stream() .filter(TransferResponse.class::isInstance) .map(TransferResponse.class::cast) .collect(onlyElement()) 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 35daa495d..4432a16dc 100644 --- a/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java +++ b/core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java @@ -16,6 +16,8 @@ package google.registry.flows.host; import static com.google.common.base.Strings.nullToEmpty; 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.assertNoBillingEvents; import static google.registry.testing.DatabaseHelper.createTld; @@ -38,10 +40,12 @@ import static google.registry.testing.TaskQueueHelper.assertNoDnsTasksEnqueued; import static google.registry.util.DateTimeUtils.END_OF_TIME; import static org.junit.jupiter.api.Assertions.assertThrows; +import com.google.cloud.tasks.v2.HttpMethod; import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.net.InetAddresses; +import google.registry.dns.RefreshDnsOnHostRenameAction; import google.registry.flows.EppException; import google.registry.flows.EppRequestSource; import google.registry.flows.FlowUtils.NotLoggedInException; @@ -75,6 +79,7 @@ import google.registry.model.tld.Registry; import google.registry.model.transfer.DomainTransferData; import google.registry.model.transfer.TransferStatus; import google.registry.persistence.VKey; +import google.registry.testing.CloudTasksHelper.TaskMatcher; import google.registry.testing.DatabaseHelper; import javax.annotation.Nullable; import org.joda.time.DateTime; @@ -199,12 +204,13 @@ class HostUpdateFlowTest extends ResourceFlowTestCase { Host renamedHost = doSuccessfulTest(); assertThat(renamedHost.isSubordinate()).isTrue(); // Task enqueued to change the NS record of the referencing domain. - // TODO(jianglai): add assertion on host rename refresh based on SQL impementation. - // assertTasksEnqueued( - // QUEUE_ASYNC_HOST_RENAME, - // new TaskMatcher() - // .param(PARAM_HOST_KEY, renamedHost.createVKey().stringify()) - // .param("requestedTime", clock.nowUtc().toString())); + cloudTasksHelper.assertTasksEnqueued( + QUEUE_HOST_RENAME, + new TaskMatcher() + .url(RefreshDnsOnHostRenameAction.PATH) + .method(HttpMethod.POST) + .service("backend") + .param(PARAM_HOST_KEY, renamedHost.createVKey().stringify())); } @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 e2b2e43c9..c1046c396 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 @@ -24,6 +24,7 @@ PATH CLASS /_dr/task/rdeReport RdeReportAction POST n INTERNAL,API APP ADMIN /_dr/task/rdeStaging RdeStagingAction GET,POST n INTERNAL,API APP ADMIN /_dr/task/rdeUpload RdeUploadAction POST n INTERNAL,API APP ADMIN +/_dr/task/refreshDnsOnHostRename RefreshDnsOnHostRenameAction POST n INTERNAL,API APP ADMIN /_dr/task/relockDomain RelockDomainAction POST y INTERNAL,API APP ADMIN /_dr/task/resaveAllEppResourcesPipeline ResaveAllEppResourcesPipelineAction GET n INTERNAL,API APP ADMIN /_dr/task/resaveEntity ResaveEntityAction POST n INTERNAL,API APP ADMIN