diff --git a/core/src/main/java/google/registry/tmch/NordnUploadAction.java b/core/src/main/java/google/registry/tmch/NordnUploadAction.java index df80682b2..b015b3ba9 100644 --- a/core/src/main/java/google/registry/tmch/NordnUploadAction.java +++ b/core/src/main/java/google/registry/tmch/NordnUploadAction.java @@ -39,8 +39,11 @@ 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.Joiner; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSortedSet; +import com.google.common.collect.Ordering; import com.google.common.flogger.FluentLogger; import google.registry.config.RegistryConfig.Config; import google.registry.request.Action; @@ -125,15 +128,19 @@ public final class NordnUploadAction implements Runnable { * 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); + // Use a Set for deduping purposes so we can be idempotent in case tasks happened to be + // enqueued multiple times for a given domain create. + ImmutableSortedSet.Builder builder = + new ImmutableSortedSet.Builder<>(Ordering.natural()); for (TaskHandle task : checkNotNull(tasks)) { String payload = new String(task.getPayload(), UTF_8); if (!Strings.isNullOrEmpty(payload)) { - csv.append(payload).append("\n"); + builder.add(payload + '\n'); } } - return csv.toString(); + ImmutableSortedSet csvLines = builder.build(); + String header = String.format("1,%s,%d\n%s\n", now, csvLines.size(), columns); + return header + Joiner.on("").join(csvLines); } /** Leases and returns all tasks from the queue with the specified tag tld, in batches. */ @@ -168,6 +175,11 @@ public final class NordnUploadAction implements Runnable { : LordnTaskUtils.QUEUE_CLAIMS); String columns = phase.equals(PARAM_LORDN_PHASE_SUNRISE) ? COLUMNS_SUNRISE : COLUMNS_CLAIMS; List tasks = loadAllTasks(queue, tld); + // Note: This upload/task deletion isn't done atomically (it's not clear how one would do so + // anyway). As a result, it is possible that the upload might succeed yet the deletion of + // enqueued tasks might fail. If so, this would result in the same lines being uploaded to NORDN + // across mulitple uploads. This is probably OK; all that we really cannot have is a missing + // line. if (!tasks.isEmpty()) { String csvData = convertTasksToCsv(tasks, now, columns); uploadCsvToLordn(String.format("/LORDN/%s/%s", tld, phase), csvData); diff --git a/core/src/test/java/google/registry/tmch/NordnUploadActionTest.java b/core/src/test/java/google/registry/tmch/NordnUploadActionTest.java index dc74eb286..86e7be307 100644 --- a/core/src/test/java/google/registry/tmch/NordnUploadActionTest.java +++ b/core/src/test/java/google/registry/tmch/NordnUploadActionTest.java @@ -138,13 +138,26 @@ class NordnUploadActionTest { void test_convertTasksToCsv() { List tasks = ImmutableList.of( - makeTaskHandle("task1", "example", "csvLine1", "lordn-sunrise"), makeTaskHandle("task2", "example", "csvLine2", "lordn-sunrise"), + makeTaskHandle("task1", "example", "csvLine1", "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"); } + @MockitoSettings(strictness = Strictness.LENIENT) + @Test + void test_convertTasksToCsv_dedupesDuplicates() { + List tasks = + ImmutableList.of( + makeTaskHandle("task2", "example", "csvLine2", "lordn-sunrise"), + makeTaskHandle("task1", "example", "csvLine1", "lordn-sunrise"), + makeTaskHandle("task3", "example", "ending", "lordn-sunrise"), + makeTaskHandle("task1", "example", "csvLine1", "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"); + } + @MockitoSettings(strictness = Strictness.LENIENT) @Test void test_convertTasksToCsv_doesntFailOnEmptyTasks() {