From b254269d2f02d2e068abd96c79b12ed8c6a48f7b Mon Sep 17 00:00:00 2001 From: jianglai Date: Thu, 18 Oct 2018 13:45:30 -0700 Subject: [PATCH] Refactor LordnTask to LordnTaskUtils Made it clear that it is a util class and moved some of the functions only used in NordnUploadAction (to NordnUploadAction). Also used Retrier to handle retries when leasing tasks. These changes allow us to no longer use InjectRule in related unit tests. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=217761117 --- .../flows/domain/DomainAllocateFlow.java | 4 +- .../flows/domain/DomainCreateFlow.java | 4 +- .../{LordnTask.java => LordnTaskUtils.java} | 63 +------------- .../registry/tmch/NordnUploadAction.java | 87 +++++++++++++++---- .../registry/tools/GenerateLordnCommand.java | 28 +++--- .../flows/domain/DomainCreateFlowTest.java | 4 +- .../registry/testing/DatastoreHelper.java | 7 +- ...nTaskTest.java => LordnTaskUtilsTest.java} | 73 +--------------- .../registry/tmch/NordnUploadActionTest.java | 74 ++++++++++++++-- .../google/registry/tmch/TmchTestSuite.java | 2 +- 10 files changed, 170 insertions(+), 176 deletions(-) rename java/google/registry/tmch/{LordnTask.java => LordnTaskUtils.java} (65%) rename javatests/google/registry/tmch/{LordnTaskTest.java => LordnTaskUtilsTest.java} (63%) diff --git a/java/google/registry/flows/domain/DomainAllocateFlow.java b/java/google/registry/flows/domain/DomainAllocateFlow.java index 76fb423c0..3812805d6 100644 --- a/java/google/registry/flows/domain/DomainAllocateFlow.java +++ b/java/google/registry/flows/domain/DomainAllocateFlow.java @@ -88,7 +88,7 @@ import google.registry.model.reporting.DomainTransactionRecord; import google.registry.model.reporting.DomainTransactionRecord.TransactionReportField; import google.registry.model.reporting.HistoryEntry; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; -import google.registry.tmch.LordnTask; +import google.registry.tmch.LordnTaskUtils; import java.util.Optional; import java.util.Set; import javax.inject.Inject; @@ -385,7 +385,7 @@ public class DomainAllocateFlow implements TransactionalFlow { dnsQueue.get().addDomainRefreshTask(newDomain.getFullyQualifiedDomainName()); } if (allocateCreate.getSmdId() != null || allocateCreate.getNotice() != null) { - LordnTask.enqueueDomainResourceTask(newDomain); + LordnTaskUtils.enqueueDomainResourceTask(newDomain); } } diff --git a/java/google/registry/flows/domain/DomainCreateFlow.java b/java/google/registry/flows/domain/DomainCreateFlow.java index 3feb56153..09dce93b1 100644 --- a/java/google/registry/flows/domain/DomainCreateFlow.java +++ b/java/google/registry/flows/domain/DomainCreateFlow.java @@ -114,7 +114,7 @@ import google.registry.model.reporting.DomainTransactionRecord; import google.registry.model.reporting.DomainTransactionRecord.TransactionReportField; import google.registry.model.reporting.HistoryEntry; import google.registry.model.reporting.IcannReportingTypes.ActivityReportField; -import google.registry.tmch.LordnTask; +import google.registry.tmch.LordnTaskUtils; import java.util.Optional; import javax.inject.Inject; import org.joda.time.DateTime; @@ -608,7 +608,7 @@ public class DomainCreateFlow implements TransactionalFlow { dnsQueue.addDomainRefreshTask(newDomain.getFullyQualifiedDomainName()); } if (hasClaimsNotice || hasSignedMarks) { - LordnTask.enqueueDomainResourceTask(newDomain); + LordnTaskUtils.enqueueDomainResourceTask(newDomain); } } diff --git a/java/google/registry/tmch/LordnTask.java b/java/google/registry/tmch/LordnTaskUtils.java similarity index 65% rename from java/google/registry/tmch/LordnTask.java rename to java/google/registry/tmch/LordnTaskUtils.java index 27308ac36..f1b0b3453 100644 --- a/java/google/registry/tmch/LordnTask.java +++ b/java/google/registry/tmch/LordnTaskUtils.java @@ -15,36 +15,22 @@ package google.registry.tmch; import static com.google.appengine.api.taskqueue.QueueFactory.getQueue; -import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import static google.registry.model.ofy.ObjectifyService.ofy; -import com.google.appengine.api.taskqueue.LeaseOptions; -import com.google.appengine.api.taskqueue.Queue; -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.base.Joiner; -import com.google.common.base.Strings; -import com.google.common.collect.ImmutableList; -import com.google.common.util.concurrent.Uninterruptibles; import google.registry.model.domain.DomainResource; import google.registry.model.registrar.Registrar; -import google.registry.util.NonFinalForTesting; -import google.registry.util.TaskQueueUtils; -import java.util.List; import java.util.Optional; -import java.util.concurrent.TimeUnit; import org.joda.time.DateTime; -import org.joda.time.Duration; /** * Helper methods for creating tasks containing CSV line data in the lordn-sunrise and lordn-claims * queues based on DomainResource changes. */ -public class LordnTask { +public final class LordnTaskUtils { public static final String QUEUE_SUNRISE = "lordn-sunrise"; public static final String QUEUE_CLAIMS = "lordn-claims"; @@ -52,51 +38,6 @@ public class LordnTask { + "registration-datetime,ack-datetime,application-datetime"; public static final String COLUMNS_SUNRISE = "roid,domain-name,SMD-id,registrar-id," + "registration-datetime,application-datetime"; - private static final Duration LEASE_PERIOD = Duration.standardHours(1); - - @NonFinalForTesting - private static Long backOffMillis = 2000L; - - /** - * Converts a list of queue tasks, each containing a row of CSV data, into a single newline- - * delimited String. - */ - public static String convertTasksToCsv(List tasks, DateTime now, String columns) { - String header = String.format("1,%s,%d\n%s\n", now, tasks.size(), columns); - StringBuilder csv = new StringBuilder(header); - for (TaskHandle task : checkNotNull(tasks)) { - String payload = new String(task.getPayload()); - if (!Strings.isNullOrEmpty(payload)) { - csv.append(payload).append("\n"); - } - } - return csv.toString(); - } - - /** Leases and returns all tasks from the queue with the specified tag tld, in batches. */ - public static List loadAllTasks(Queue queue, String tld) { - ImmutableList.Builder allTasks = new ImmutableList.Builder<>(); - int numErrors = 0; - long backOff = backOffMillis; - while (true) { - try { - List tasks = queue.leaseTasks(LeaseOptions.Builder - .withTag(tld) - .leasePeriod(LEASE_PERIOD.getMillis(), TimeUnit.MILLISECONDS) - .countLimit(TaskQueueUtils.getBatchSize())); - allTasks.addAll(tasks); - if (tasks.isEmpty()) { - return allTasks.build(); - } - } catch (TransientFailureException | DeadlineExceededException e) { - if (++numErrors >= 3) { - throw new RuntimeException("Error leasing tasks", e); - } - Uninterruptibles.sleepUninterruptibly(backOff, TimeUnit.MILLISECONDS); - backOff *= 2; - } - } - } /** * Enqueues a task in the LORDN queue representing a line of CSV for LORDN export. @@ -156,4 +97,6 @@ public class LordnTask { // have null iana ids. return String.valueOf(registrar.get().getIanaIdentifier()); } + + private LordnTaskUtils() {} } diff --git a/java/google/registry/tmch/NordnUploadAction.java b/java/google/registry/tmch/NordnUploadAction.java index f49b2514c..2fc69c0c2 100644 --- a/java/google/registry/tmch/NordnUploadAction.java +++ b/java/google/registry/tmch/NordnUploadAction.java @@ -19,21 +19,27 @@ import static com.google.appengine.api.taskqueue.TaskOptions.Builder.withUrl; import static com.google.appengine.api.urlfetch.FetchOptions.Builder.validateCertificate; import static com.google.appengine.api.urlfetch.HTTPMethod.POST; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.net.HttpHeaders.LOCATION; import static com.google.common.net.MediaType.CSV_UTF_8; -import static google.registry.tmch.LordnTask.COLUMNS_CLAIMS; -import static google.registry.tmch.LordnTask.COLUMNS_SUNRISE; -import static google.registry.tmch.LordnTask.convertTasksToCsv; +import static google.registry.tmch.LordnTaskUtils.COLUMNS_CLAIMS; +import static google.registry.tmch.LordnTaskUtils.COLUMNS_SUNRISE; import static google.registry.util.UrlFetchUtils.getHeaderFirst; import static google.registry.util.UrlFetchUtils.setPayloadMultipart; +import static java.nio.charset.StandardCharsets.UTF_8; import static javax.servlet.http.HttpServletResponse.SC_ACCEPTED; +import com.google.appengine.api.taskqueue.LeaseOptions; import com.google.appengine.api.taskqueue.Queue; import com.google.appengine.api.taskqueue.TaskHandle; import com.google.appengine.api.taskqueue.TaskOptions; +import com.google.appengine.api.taskqueue.TransientFailureException; import com.google.appengine.api.urlfetch.HTTPRequest; import com.google.appengine.api.urlfetch.HTTPResponse; import com.google.appengine.api.urlfetch.URLFetchService; +import com.google.apphosting.api.DeadlineExceededException; +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; import com.google.common.flogger.FluentLogger; import google.registry.config.RegistryConfig.Config; import google.registry.request.Action; @@ -41,6 +47,7 @@ import google.registry.request.Parameter; import google.registry.request.RequestParameters; import google.registry.request.auth.Auth; import google.registry.util.Clock; +import google.registry.util.Retrier; import google.registry.util.TaskQueueUtils; import google.registry.util.UrlFetchException; import java.io.IOException; @@ -48,29 +55,30 @@ import java.net.URL; import java.util.List; import java.util.Optional; import java.util.Random; +import java.util.concurrent.TimeUnit; import javax.inject.Inject; import org.joda.time.DateTime; import org.joda.time.Duration; /** * Action that reads the NORDN pull queues, uploads claims and sunrise marks data to TMCH, and - * enqueues subsequent upload verification tasks. A unique actionLogId is generated and passed - * along to the verify action so that connected verify tasks can be identified by looking at logs. + * enqueues subsequent upload verification tasks. A unique actionLogId is generated and passed along + * to the verify action so that connected verify tasks can be identified by looking at logs. * * @see NordnVerifyAction */ @Action( - path = NordnUploadAction.PATH, - method = Action.Method.POST, - automaticallyPrintOk = true, - auth = Auth.AUTH_INTERNAL_ONLY -) + path = NordnUploadAction.PATH, + method = Action.Method.POST, + automaticallyPrintOk = true, + auth = Auth.AUTH_INTERNAL_ONLY) public final class NordnUploadAction implements Runnable { static final String PATH = "/_dr/task/nordnUpload"; static final String LORDN_PHASE_PARAM = "lordn-phase"; private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private static final Duration LEASE_PERIOD = Duration.standardHours(1); /** * A unique (enough) id that is outputted in log lines to make it clear which log lines are @@ -80,6 +88,7 @@ public final class NordnUploadAction implements Runnable { private final String actionLogId = String.valueOf(1000000000 + new Random().nextInt(1000000000)); @Inject Clock clock; + @Inject Retrier retrier; @Inject Random random; @Inject LordnRequestInitializer lordnRequestInitializer; @Inject URLFetchService fetchService; @@ -108,15 +117,54 @@ public final class NordnUploadAction implements Runnable { } } + /** + * Converts a list of queue tasks, each containing a row of CSV data, into a single newline- + * delimited String. + */ + static String convertTasksToCsv(List tasks, DateTime now, String columns) { + String header = String.format("1,%s,%d\n%s\n", now, tasks.size(), columns); + StringBuilder csv = new StringBuilder(header); + for (TaskHandle task : checkNotNull(tasks)) { + String payload = new String(task.getPayload(), UTF_8); + if (!Strings.isNullOrEmpty(payload)) { + csv.append(payload).append("\n"); + } + } + return csv.toString(); + } + + /** Leases and returns all tasks from the queue with the specified tag tld, in batches. */ + List loadAllTasks(Queue queue, String tld) { + ImmutableList.Builder allTasks = new ImmutableList.Builder<>(); + while (true) { + List tasks = + retrier.callWithRetry( + () -> + queue.leaseTasks( + LeaseOptions.Builder.withTag(tld) + .leasePeriod(LEASE_PERIOD.getMillis(), TimeUnit.MILLISECONDS) + .countLimit(TaskQueueUtils.getBatchSize())), + TransientFailureException.class, + DeadlineExceededException.class); + if (tasks.isEmpty()) { + return allTasks.build(); + } + allTasks.addAll(tasks); + } + } + private void processLordnTasks() throws IOException { checkArgument(phase.equals(PARAM_LORDN_PHASE_SUNRISE) || phase.equals(PARAM_LORDN_PHASE_CLAIMS), "Invalid phase specified to Nordn servlet: %s.", phase); DateTime now = clock.nowUtc(); - Queue queue = getQueue( - phase.equals(PARAM_LORDN_PHASE_SUNRISE) ? LordnTask.QUEUE_SUNRISE : LordnTask.QUEUE_CLAIMS); + Queue queue = + getQueue( + phase.equals(PARAM_LORDN_PHASE_SUNRISE) + ? LordnTaskUtils.QUEUE_SUNRISE + : LordnTaskUtils.QUEUE_CLAIMS); String columns = phase.equals(PARAM_LORDN_PHASE_SUNRISE) ? COLUMNS_SUNRISE : COLUMNS_CLAIMS; - List tasks = LordnTask.loadAllTasks(queue, tld); + List tasks = loadAllTasks(queue, tld); if (!tasks.isEmpty()) { String csvData = convertTasksToCsv(tasks, now, columns); uploadCsvToLordn(String.format("/LORDN/%s/%s", tld, phase), csvData); @@ -146,16 +194,19 @@ public final class NordnUploadAction implements Runnable { actionLogId, rsp.getResponseCode(), rsp.getContent()); if (rsp.getResponseCode() != SC_ACCEPTED) { throw new UrlFetchException( - String.format("LORDN upload task %s error: Failed to upload LORDN claims to MarksDB", - actionLogId), - req, rsp); + String.format( + "LORDN upload task %s error: Failed to upload LORDN claims to MarksDB", actionLogId), + req, + rsp); } Optional location = getHeaderFirst(rsp, LOCATION); if (!location.isPresent()) { throw new UrlFetchException( - String.format("LORDN upload task %s error: MarksDB failed to provide a Location header", + String.format( + "LORDN upload task %s error: MarksDB failed to provide a Location header", actionLogId), - req, rsp); + req, + rsp); } getQueue(NordnVerifyAction.QUEUE).add(makeVerifyTask(new URL(location.get()))); } diff --git a/java/google/registry/tools/GenerateLordnCommand.java b/java/google/registry/tools/GenerateLordnCommand.java index 153530aee..81815f117 100644 --- a/java/google/registry/tools/GenerateLordnCommand.java +++ b/java/google/registry/tools/GenerateLordnCommand.java @@ -22,7 +22,7 @@ import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; import com.google.common.collect.ImmutableList; import google.registry.model.domain.DomainResource; -import google.registry.tmch.LordnTask; +import google.registry.tmch.LordnTaskUtils; import google.registry.tools.params.PathParameter; import java.io.IOException; import java.nio.file.Files; @@ -61,26 +61,28 @@ final class GenerateLordnCommand implements CommandWithRemoteApi { for (DomainResource domain : ofy().load().type(DomainResource.class).filter("tld", tld)) { String status = " "; if (domain.getLaunchNotice() == null && domain.getSmdId() != null) { - sunriseCsv.add(LordnTask.getCsvLineForSunriseDomain(domain, domain.getCreationTime())); + sunriseCsv.add(LordnTaskUtils.getCsvLineForSunriseDomain(domain, domain.getCreationTime())); status = "S"; } else if (domain.getLaunchNotice() != null || domain.getSmdId() != null) { - claimsCsv.add(LordnTask.getCsvLineForClaimsDomain(domain, domain.getCreationTime())); + claimsCsv.add(LordnTaskUtils.getCsvLineForClaimsDomain(domain, domain.getCreationTime())); status = "C"; } System.out.printf("%s[%s] ", domain.getFullyQualifiedDomainName(), status); } ImmutableList claimsRows = claimsCsv.build(); - ImmutableList claimsAll = new ImmutableList.Builder() - .add(String.format("1,%s,%d", now, claimsRows.size())) - .add(LordnTask.COLUMNS_CLAIMS) - .addAll(claimsRows) - .build(); + ImmutableList claimsAll = + new ImmutableList.Builder() + .add(String.format("1,%s,%d", now, claimsRows.size())) + .add(LordnTaskUtils.COLUMNS_CLAIMS) + .addAll(claimsRows) + .build(); ImmutableList sunriseRows = sunriseCsv.build(); - ImmutableList sunriseAll = new ImmutableList.Builder() - .add(String.format("1,%s,%d", now.plusMillis(1), sunriseRows.size())) - .add(LordnTask.COLUMNS_SUNRISE) - .addAll(sunriseRows) - .build(); + ImmutableList sunriseAll = + new ImmutableList.Builder() + .add(String.format("1,%s,%d", now.plusMillis(1), sunriseRows.size())) + .add(LordnTaskUtils.COLUMNS_SUNRISE) + .addAll(sunriseRows) + .build(); Files.write(claimsOutputPath, claimsAll, UTF_8); Files.write(sunriseOutputPath, sunriseAll, UTF_8); } diff --git a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java index e7fff05bd..33ab45c30 100644 --- a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java @@ -52,8 +52,8 @@ import static google.registry.testing.TaskQueueHelper.assertDnsTasksEnqueued; import static google.registry.testing.TaskQueueHelper.assertNoDnsTasksEnqueued; import static google.registry.testing.TaskQueueHelper.assertNoTasksEnqueued; import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued; -import static google.registry.tmch.LordnTask.QUEUE_CLAIMS; -import static google.registry.tmch.LordnTask.QUEUE_SUNRISE; +import static google.registry.tmch.LordnTaskUtils.QUEUE_CLAIMS; +import static google.registry.tmch.LordnTaskUtils.QUEUE_SUNRISE; import static google.registry.util.DateTimeUtils.END_OF_TIME; import static google.registry.util.DateTimeUtils.START_OF_TIME; import static org.joda.money.CurrencyUnit.EUR; diff --git a/javatests/google/registry/testing/DatastoreHelper.java b/javatests/google/registry/testing/DatastoreHelper.java index 70e20427c..897d0431c 100644 --- a/javatests/google/registry/testing/DatastoreHelper.java +++ b/javatests/google/registry/testing/DatastoreHelper.java @@ -97,7 +97,7 @@ import google.registry.model.smd.EncodedSignedMark; import google.registry.model.transfer.TransferData; import google.registry.model.transfer.TransferData.Builder; import google.registry.model.transfer.TransferStatus; -import google.registry.tmch.LordnTask; +import google.registry.tmch.LordnTaskUtils; import java.util.Arrays; import java.util.List; import java.util.Set; @@ -363,9 +363,10 @@ public class DatastoreHelper { /** Persists a domain and enqueues a LORDN task of the appropriate type for it. */ public static DomainResource persistDomainAndEnqueueLordn(final DomainResource domain) { final DomainResource persistedDomain = persistResource(domain); - // Calls {@link LordnTask#enqueueDomainResourceTask} wrapped in an ofy transaction so that the + // Calls {@link LordnTaskUtils#enqueueDomainResourceTask} wrapped in an ofy transaction so that + // the // transaction time is set correctly. - ofy().transactNew(() -> LordnTask.enqueueDomainResourceTask(persistedDomain)); + ofy().transactNew(() -> LordnTaskUtils.enqueueDomainResourceTask(persistedDomain)); return persistedDomain; } diff --git a/javatests/google/registry/tmch/LordnTaskTest.java b/javatests/google/registry/tmch/LordnTaskUtilsTest.java similarity index 63% rename from javatests/google/registry/tmch/LordnTaskTest.java rename to javatests/google/registry/tmch/LordnTaskUtilsTest.java index d2229d95d..9ce2bf00a 100644 --- a/javatests/google/registry/tmch/LordnTaskTest.java +++ b/javatests/google/registry/tmch/LordnTaskUtilsTest.java @@ -22,18 +22,7 @@ import static google.registry.testing.DatastoreHelper.persistActiveContact; import static google.registry.testing.DatastoreHelper.persistDomainAndEnqueueLordn; import static google.registry.testing.JUnitBackports.assertThrows; import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued; -import static org.mockito.Matchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -import com.google.appengine.api.taskqueue.LeaseOptions; -import com.google.appengine.api.taskqueue.Queue; -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.collect.ImmutableList; import com.googlecode.objectify.Key; import google.registry.model.domain.DomainResource; import google.registry.model.domain.launch.LaunchNotice; @@ -44,7 +33,6 @@ import google.registry.testing.FakeClock; import google.registry.testing.InjectRule; import google.registry.testing.TaskQueueHelper.TaskMatcher; import google.registry.util.Clock; -import java.util.List; import org.joda.time.DateTime; import org.junit.Before; import org.junit.Rule; @@ -52,9 +40,9 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -/** Unit tests for {@link LordnTask}. */ +/** Unit tests for {@link LordnTaskUtils}. */ @RunWith(JUnit4.class) -public class LordnTaskTest { +public class LordnTaskUtilsTest { private static final Clock clock = new FakeClock(DateTime.parse("2010-05-01T10:11:12Z")); @@ -70,32 +58,9 @@ public class LordnTaskTest { public void before() { createTld("example"); inject.setStaticField(Ofy.class, "clock", clock); - inject.setStaticField(LordnTask.class, "backOffMillis", 1L); } - @Test - public void test_convertTasksToCsv() { - List tasks = ImmutableList.of( - makeTaskHandle("task1", "example", "csvLine1", "lordn-sunrise"), - makeTaskHandle("task2", "example", "csvLine2", "lordn-sunrise"), - makeTaskHandle("task3", "example", "ending", "lordn-sunrise")); - assertThat(LordnTask.convertTasksToCsv(tasks, clock.nowUtc(), "col1,col2")) - .isEqualTo("1,2010-05-01T10:11:12.000Z,3\ncol1,col2\ncsvLine1\ncsvLine2\nending\n"); - } - @Test - public void test_convertTasksToCsv_doesntFailOnEmptyTasks() { - assertThat( - LordnTask.convertTasksToCsv(ImmutableList.of(), clock.nowUtc(), "col1,col2")) - .isEqualTo("1,2010-05-01T10:11:12.000Z,0\ncol1,col2\n"); - } - - @Test - public void test_convertTasksToCsv_throwsNpeOnNullTasks() { - assertThrows( - NullPointerException.class, - () -> LordnTask.convertTasksToCsv(null, clock.nowUtc(), "header")); - } private DomainResource.Builder newDomainBuilder(DateTime applicationTime) { return new DomainResource.Builder() @@ -173,38 +138,6 @@ public class LordnTaskTest { public void test_enqueueDomainResourceTask_throwsNpeOnNullDomain() { assertThrows( NullPointerException.class, - () -> ofy().transactNew(() -> LordnTask.enqueueDomainResourceTask(null))); - } - - @SuppressWarnings("unchecked") - @Test - public void test_loadAllTasks_retryLogic_thirdTrysTheCharm() { - Queue queue = mock(Queue.class); - TaskHandle task = new TaskHandle(TaskOptions.Builder.withTaskName("blah"), "blah"); - when(queue.leaseTasks(any(LeaseOptions.class))) - .thenThrow(TransientFailureException.class) - .thenThrow(DeadlineExceededException.class) - .thenReturn(ImmutableList.of(task), ImmutableList.of()); - assertThat(LordnTask.loadAllTasks(queue, "tld")).containsExactly(task); - } - - @SuppressWarnings("unchecked") - @Test - public void test_loadAllTasks_retryLogic_allFailures() { - Queue queue = mock(Queue.class); - when(queue.leaseTasks(any(LeaseOptions.class))).thenThrow(TransientFailureException.class); - RuntimeException thrown = - assertThrows(RuntimeException.class, () -> LordnTask.loadAllTasks(queue, "tld")); - assertThat(thrown).hasMessageThat().contains("Error leasing tasks"); - } - - private static TaskHandle makeTaskHandle( - String taskName, - String tag, - String payload, - String queue) { - return new TaskHandle( - TaskOptions.Builder.withPayload(payload).method(Method.PULL).tag(tag).taskName(taskName), - queue); + () -> ofy().transactNew(() -> LordnTaskUtils.enqueueDomainResourceTask(null))); } } diff --git a/javatests/google/registry/tmch/NordnUploadActionTest.java b/javatests/google/registry/tmch/NordnUploadActionTest.java index 67fd66a34..2ec070283 100644 --- a/javatests/google/registry/tmch/NordnUploadActionTest.java +++ b/javatests/google/registry/tmch/NordnUploadActionTest.java @@ -32,13 +32,21 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static javax.servlet.http.HttpServletResponse.SC_ACCEPTED; import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import com.google.appengine.api.taskqueue.LeaseOptions; +import com.google.appengine.api.taskqueue.Queue; +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.appengine.api.urlfetch.HTTPHeader; import com.google.appengine.api.urlfetch.HTTPRequest; import com.google.appengine.api.urlfetch.HTTPResponse; import com.google.appengine.api.urlfetch.URLFetchService; +import com.google.apphosting.api.DeadlineExceededException; import com.google.common.base.VerifyException; import com.google.common.collect.ImmutableList; import google.registry.model.domain.DomainResource; @@ -55,6 +63,7 @@ import google.registry.util.Retrier; import google.registry.util.TaskQueueUtils; import google.registry.util.UrlFetchException; import java.net.URL; +import java.util.List; import java.util.Optional; import java.util.Random; import org.joda.time.DateTime; @@ -71,15 +80,15 @@ import org.mockito.Mock; @RunWith(JUnit4.class) public class NordnUploadActionTest { - private static final String CLAIMS_CSV = "1,2000-01-01T00:00:00.000Z,1\n" + private static final String CLAIMS_CSV = "1,2010-05-01T10:11:12.000Z,1\n" + "roid,domain-name,notice-id,registrar-id,registration-datetime,ack-datetime," + "application-datetime\n" - + "2-TLD,claims-landrush1.tld,landrush1tcn,99999,2000-01-01T00:00:00.000Z," + + "2-TLD,claims-landrush1.tld,landrush1tcn,99999,2010-05-01T10:11:12.000Z," + "1969-12-31T23:00:00.000Z,1969-12-31T00:00:00.000Z\n"; - private static final String SUNRISE_CSV = "1,2000-01-01T00:00:00.000Z,1\n" + private static final String SUNRISE_CSV = "1,2010-05-01T10:11:12.000Z,1\n" + "roid,domain-name,SMD-id,registrar-id,registration-datetime,application-datetime\n" - + "2-TLD,sunrise1.tld,my-smdid,99999,2000-01-01T00:00:00.000Z,1969-12-31T00:00:00.000Z\n"; + + "2-TLD,sunrise1.tld,my-smdid,99999,2010-05-01T10:11:12.000Z,1969-12-31T00:00:00.000Z\n"; private static final String LOCATION_URL = "http://trololol"; @@ -94,7 +103,7 @@ public class NordnUploadActionTest { @Mock private HTTPResponse httpResponse; @Captor private ArgumentCaptor httpRequestCaptor; - private final FakeClock clock = new FakeClock(DateTime.parse("2000-01-01TZ")); + private final FakeClock clock = new FakeClock(DateTime.parse("2010-05-01T10:11:12Z")); private final LordnRequestInitializer lordnRequestInitializer = new LordnRequestInitializer(); private final NordnUploadAction action = new NordnUploadAction(); @@ -117,6 +126,54 @@ public class NordnUploadActionTest { action.tld = "tld"; action.tmchMarksdbUrl = "http://127.0.0.1"; action.random = new Random(); + action.retrier = new Retrier(new FakeSleeper(clock), 3); + } + + @Test + public void test_convertTasksToCsv() { + List tasks = + ImmutableList.of( + makeTaskHandle("task1", "example", "csvLine1", "lordn-sunrise"), + makeTaskHandle("task2", "example", "csvLine2", "lordn-sunrise"), + makeTaskHandle("task3", "example", "ending", "lordn-sunrise")); + assertThat(NordnUploadAction.convertTasksToCsv(tasks, clock.nowUtc(), "col1,col2")) + .isEqualTo("1,2010-05-01T10:11:12.000Z,3\ncol1,col2\ncsvLine1\ncsvLine2\nending\n"); + } + + @Test + public void test_convertTasksToCsv_doesntFailOnEmptyTasks() { + assertThat(NordnUploadAction.convertTasksToCsv(ImmutableList.of(), clock.nowUtc(), "col1,col2")) + .isEqualTo("1,2010-05-01T10:11:12.000Z,0\ncol1,col2\n"); + } + + @Test + public void test_convertTasksToCsv_throwsNpeOnNullTasks() { + assertThrows( + NullPointerException.class, + () -> NordnUploadAction.convertTasksToCsv(null, clock.nowUtc(), "header")); + } + + @SuppressWarnings("unchecked") + @Test + public void test_loadAllTasks_retryLogic_thirdTrysTheCharm() { + Queue queue = mock(Queue.class); + TaskHandle task = new TaskHandle(TaskOptions.Builder.withTaskName("blah"), "blah"); + when(queue.leaseTasks(any(LeaseOptions.class))) + .thenThrow(TransientFailureException.class) + .thenThrow(DeadlineExceededException.class) + .thenReturn(ImmutableList.of(task), ImmutableList.of()); + assertThat(action.loadAllTasks(queue, "tld")).containsExactly(task); + } + + @SuppressWarnings("unchecked") + @Test + public void test_loadAllTasks_retryLogic_allFailures() { + Queue queue = mock(Queue.class); + when(queue.leaseTasks(any(LeaseOptions.class))) + .thenThrow(new TransientFailureException("some transient error")); + RuntimeException thrown = + assertThrows(TransientFailureException.class, () -> action.loadAllTasks(queue, "tld")); + assertThat(thrown).hasMessageThat().isEqualTo("some transient error"); } @Test @@ -230,4 +287,11 @@ public class NordnUploadActionTest { .setApplicationTime(domain.getCreationTime().minusDays(1)) .build()); } + + private static TaskHandle makeTaskHandle( + String taskName, String tag, String payload, String queue) { + return new TaskHandle( + TaskOptions.Builder.withPayload(payload).method(Method.PULL).tag(tag).taskName(taskName), + queue); + } } diff --git a/javatests/google/registry/tmch/TmchTestSuite.java b/javatests/google/registry/tmch/TmchTestSuite.java index 9917ca32d..8d9facd3b 100644 --- a/javatests/google/registry/tmch/TmchTestSuite.java +++ b/javatests/google/registry/tmch/TmchTestSuite.java @@ -21,7 +21,7 @@ import org.junit.runners.Suite.SuiteClasses; /** Convenience class to run all TMCH tests inside IDE with one keystroke. */ @RunWith(Suite.class) @SuiteClasses({ - LordnTaskTest.class, + LordnTaskUtilsTest.class, NordnUploadAction.class, NordnVerifyAction.class, SmdrlCsvParserTest.class,