Fix IcannReportingUploadAction to upload reports from the previous month (#425)

* Fix IcannReportingUploadAction to upload reports from the previous month

getFileName now sets the file name of the report to upload to use the month before cursor time.

IcannReportingUploadAction no longer uploads the MANIFEST.txt file since it is not required based on (https://tools.ietf.org/html/draft-lozano-icann-registry-interfaces-07#page-9) and the previous implementation of this action did not upload it.

Deletes the ICANN_UPLOAD_MANIFEST cursor since it is no loner needed.

* Add the ICANN_UPLOAD_MANIFEST cursor back

core/src/main/java/google/registry/model/common/Cursor.java

@@ -87,7 +87,9 @@ public class Cursor extends ImmutableObject {
     /** Cursor for tracking monthly uploads of ICANN activity reports. */
     ICANN_UPLOAD_ACTIVITY(Registry.class),
 
-    /** Cursor for tracking monthly upload of MANIFEST.txt to ICANN. */
+    // TODO(sarahbot) Delete this cursor once all data in the database that refers to it is removed.
+    /** Cursor for tracking monthly uploads of MANIFEST.txt to ICANN. No longer used. */
+    @Deprecated
     ICANN_UPLOAD_MANIFEST(EntityGroupRoot.class);
 
     /** See the definition of scope on {@link #getScopeClass}. */

core/src/main/java/google/registry/reporting/icann/IcannReportingUploadAction.java

@@ -20,7 +20,6 @@ import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8;
 import static google.registry.model.common.Cursor.getCursorTimeOrStartOfTime;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.model.transaction.TransactionManagerFactory.tm;
-import static google.registry.reporting.icann.IcannReportingModule.MANIFEST_FILE_NAME;
 import static google.registry.reporting.icann.IcannReportingModule.PARAM_SUBDIR;
 import static google.registry.request.Action.Method.POST;
 import static javax.servlet.http.HttpServletResponse.SC_OK;
@@ -65,8 +64,9 @@ import org.joda.time.Duration;
  * Action that uploads the monthly activity/transactions reports from GCS to ICANN via an HTTP PUT.
  *
  * <p>This should be run after {@link IcannReportingStagingAction}, which writes out the month's
- * reports and a MANIFEST.txt file. This action reads the filenames from the MANIFEST.txt, and
+ * reports and a MANIFEST.txt file. This action checks each ICANN_UPLOAD_TX and
- * attempts to upload every file in the manifest to ICANN's endpoint.
+ * ICANN_UPLOAD_ACTIVITY cursor and uploads the corresponding report if the cursor time is before
+ * now.
  *
  * <p>Parameters:
  *
@@ -181,32 +181,22 @@ public final class IcannReportingUploadAction implements Runnable {
 
     // Set cursor to first day of next month if the upload succeeded
     if (success) {
-      Cursor newCursor;
+      Cursor newCursor =
-      if (cursorType.equals(CursorType.ICANN_UPLOAD_MANIFEST)) {
+          Cursor.create(
-        newCursor =
+              cursorType,
-            Cursor.createGlobal(
+              cursorTime.withTimeAtStartOfDay().withDayOfMonth(1).plusMonths(1),
-                cursorType, cursorTime.withTimeAtStartOfDay().withDayOfMonth(1).plusMonths(1));
+              Registry.get(tldStr));
-      } else {
-        newCursor =
-            Cursor.create(
-                cursorType,
-                cursorTime.withTimeAtStartOfDay().withDayOfMonth(1).plusMonths(1),
-                Registry.get(tldStr));
-      }
       tm().transact(() -> ofy().save().entity(newCursor));
     }
   }
 
   private String getFileName(CursorType cursorType, DateTime cursorTime, String tld) {
-    if (cursorType.equals(CursorType.ICANN_UPLOAD_MANIFEST)) {
-      return MANIFEST_FILE_NAME;
-    }
     return String.format(
         "%s%s%d%02d.csv",
         tld,
         (cursorType.equals(CursorType.ICANN_UPLOAD_ACTIVITY) ? "-activity-" : "-transactions-"),
         cursorTime.year().get(),
-        cursorTime.monthOfYear().get());
+        cursorTime.withDayOfMonth(1).minusMonths(1).monthOfYear().get());
   }
 
   /** Returns a map of each cursor to the CursorType and tld. */
@@ -222,7 +212,6 @@ public final class IcannReportingUploadAction implements Runnable {
     ImmutableSet.Builder<Key<Cursor>> keys = new ImmutableSet.Builder<>();
     keys.addAll(activityKeyMap.keySet());
     keys.addAll(transactionKeyMap.keySet());
-    keys.add(Cursor.createGlobalKey(CursorType.ICANN_UPLOAD_MANIFEST));
 
     Map<Key<Cursor>, Cursor> cursorMap = ofy().load().keys(keys.build());
     ImmutableMap.Builder<Cursor, CursorInfo> cursors = new ImmutableMap.Builder<>();
@@ -230,11 +219,6 @@ public final class IcannReportingUploadAction implements Runnable {
         activityKeyMap, CursorType.ICANN_UPLOAD_ACTIVITY, cursorMap, cursors);
     defaultNullCursorsToNextMonthAndAddToMap(
         transactionKeyMap, CursorType.ICANN_UPLOAD_TX, cursorMap, cursors);
-    Cursor manifestCursor =
-        cursorMap.getOrDefault(
-            Cursor.createGlobalKey(CursorType.ICANN_UPLOAD_MANIFEST),
-            Cursor.createGlobal(CursorType.ICANN_UPLOAD_MANIFEST, clock.nowUtc().minusDays(1)));
-    cursors.put(manifestCursor, CursorInfo.create(CursorType.ICANN_UPLOAD_MANIFEST, null));
     return cursors.build();
   }
 
@@ -259,7 +243,14 @@ public final class IcannReportingUploadAction implements Runnable {
           // report staged for upload.
           Cursor cursor =
               cursorMap.getOrDefault(
-                  key, Cursor.create(type, clock.nowUtc().minusDays(1), registry));
+                  key,
+                  Cursor.create(
+                      type,
+                      clock.nowUtc().withDayOfMonth(1).withTimeAtStartOfDay().plusMonths(1),
+                      registry));
+          if (!cursorMap.containsValue(cursor)) {
+            tm().transact(() -> ofy().save().entity(cursor));
+          }
           cursors.put(cursor, CursorInfo.create(type, registry.getTldStr()));
         });
   }

core/src/test/java/google/registry/reporting/icann/IcannReportingUploadActionTest.java

@@ -64,9 +64,6 @@ public class IcannReportingUploadActionTest {
 
   private static final byte[] PAYLOAD_SUCCESS = "test,csv\n13,37".getBytes(UTF_8);
   private static final byte[] PAYLOAD_FAIL = "ahah,csv\n12,34".getBytes(UTF_8);
-  private static final byte[] MANIFEST_PAYLOAD =
-      "tld-transactions-200606.csv\ntld-activity-200606.csv\nfoo-transactions-200606.csv\nfoo-activity-200606.csv\n"
-          .getBytes(UTF_8);
   private final IcannHttpReporter mockReporter = mock(IcannHttpReporter.class);
   private final SendEmailService emailService = mock(SendEmailService.class);
   private final FakeResponse response = new FakeResponse();
@@ -111,30 +108,23 @@ public class IcannReportingUploadActionTest {
         gcsService,
         new GcsFilename("basin/icann/monthly/2006-06", "foo-activity-200606.csv"),
         PAYLOAD_SUCCESS);
-    writeGcsFile(
-        gcsService,
-        new GcsFilename("basin/icann/monthly/2006-06", "MANIFEST.txt"),
-        MANIFEST_PAYLOAD);
     when(mockReporter.send(PAYLOAD_SUCCESS, "tld-transactions-200606.csv")).thenReturn(true);
     when(mockReporter.send(PAYLOAD_SUCCESS, "foo-transactions-200606.csv")).thenReturn(true);
     when(mockReporter.send(PAYLOAD_FAIL, "tld-activity-200606.csv")).thenReturn(false);
     when(mockReporter.send(PAYLOAD_SUCCESS, "foo-activity-200606.csv")).thenReturn(true);
-    when(mockReporter.send(MANIFEST_PAYLOAD, "MANIFEST.txt")).thenReturn(true);
+    clock.setTo(DateTime.parse("2006-07-05T00:30:00Z"));
-    clock.setTo(DateTime.parse("2006-06-06T00:30:00Z"));
     persistResource(
         Cursor.create(
-            CursorType.ICANN_UPLOAD_ACTIVITY, DateTime.parse("2006-06-06TZ"), Registry.get("tld")));
+            CursorType.ICANN_UPLOAD_ACTIVITY, DateTime.parse("2006-07-01TZ"), Registry.get("tld")));
     persistResource(
         Cursor.create(
-            CursorType.ICANN_UPLOAD_TX, DateTime.parse("2006-06-06TZ"), Registry.get("tld")));
+            CursorType.ICANN_UPLOAD_TX, DateTime.parse("2006-07-01TZ"), Registry.get("tld")));
-    persistResource(
-        Cursor.createGlobal(CursorType.ICANN_UPLOAD_MANIFEST, DateTime.parse("2006-07-06TZ")));
     persistResource(
         Cursor.create(
-            CursorType.ICANN_UPLOAD_ACTIVITY, DateTime.parse("2006-06-06TZ"), Registry.get("foo")));
+            CursorType.ICANN_UPLOAD_ACTIVITY, DateTime.parse("2006-07-01TZ"), Registry.get("foo")));
     persistResource(
         Cursor.create(
-            CursorType.ICANN_UPLOAD_TX, DateTime.parse("2006-06-06TZ"), Registry.get("foo")));
+            CursorType.ICANN_UPLOAD_TX, DateTime.parse("2006-07-01TZ"), Registry.get("foo")));
     loggerToIntercept.addHandler(logHandler);
   }
 
@@ -176,7 +166,7 @@ public class IcannReportingUploadActionTest {
             .load()
             .key(Cursor.createKey(CursorType.ICANN_UPLOAD_ACTIVITY, Registry.get("tld")))
             .now();
-    assertThat(cursor.getCursorTime()).isEqualTo(DateTime.parse("2006-07-01TZ"));
+    assertThat(cursor.getCursorTime()).isEqualTo(DateTime.parse("2006-08-01TZ"));
   }
 
   @Test
@@ -195,24 +185,6 @@ public class IcannReportingUploadActionTest {
                 new InternetAddress("sender@example.com")));
   }
 
-  @Test
-  public void testSuccess_uploadManifest() throws Exception {
-    persistResource(
-        Cursor.createGlobal(CursorType.ICANN_UPLOAD_MANIFEST, DateTime.parse("2006-06-06TZ")));
-    IcannReportingUploadAction action = createAction();
-    action.run();
-    ofy().clearSessionCache();
-    Cursor cursor =
-        ofy().load().key(Cursor.createGlobalKey(CursorType.ICANN_UPLOAD_MANIFEST)).now();
-    assertThat(cursor.getCursorTime()).isEqualTo(DateTime.parse("2006-07-01TZ"));
-    verify(mockReporter).send(PAYLOAD_FAIL, "tld-activity-200606.csv");
-    verify(mockReporter).send(PAYLOAD_SUCCESS, "foo-activity-200606.csv");
-    verify(mockReporter).send(PAYLOAD_SUCCESS, "foo-transactions-200606.csv");
-    verify(mockReporter).send(PAYLOAD_SUCCESS, "tld-transactions-200606.csv");
-    verify(mockReporter).send(MANIFEST_PAYLOAD, "MANIFEST.txt");
-    verifyNoMoreInteractions(mockReporter);
-  }
-
   @Test
   public void testSuccess_withRetry() throws Exception {
     IcannReportingUploadAction action = createAction();
@@ -262,7 +234,7 @@ public class IcannReportingUploadActionTest {
             .load()
             .key(Cursor.createKey(CursorType.ICANN_UPLOAD_ACTIVITY, Registry.get("tld")))
             .now();
-    assertThat(cursor.getCursorTime()).isEqualTo(DateTime.parse("2006-06-06TZ"));
+    assertThat(cursor.getCursorTime()).isEqualTo(DateTime.parse("2006-07-01TZ"));
   }
 
   @Test
@@ -276,7 +248,7 @@ public class IcannReportingUploadActionTest {
             .load()
             .key(Cursor.createKey(CursorType.ICANN_UPLOAD_ACTIVITY, Registry.get("foo")))
             .now();
-    assertThat(cursor.getCursorTime()).isEqualTo(DateTime.parse("2006-06-06TZ"));
+    assertThat(cursor.getCursorTime()).isEqualTo(DateTime.parse("2006-07-01TZ"));
     verifyNoMoreInteractions(mockReporter);
   }
 
@@ -323,8 +295,8 @@ public class IcannReportingUploadActionTest {
   public void testWarning_fileNotStagedYet() throws Exception {
     persistResource(
         Cursor.create(
-            CursorType.ICANN_UPLOAD_ACTIVITY, DateTime.parse("2006-07-01TZ"), Registry.get("foo")));
+            CursorType.ICANN_UPLOAD_ACTIVITY, DateTime.parse("2006-08-01TZ"), Registry.get("foo")));
-    clock.setTo(DateTime.parse("2006-07-01T00:30:00Z"));
+    clock.setTo(DateTime.parse("2006-08-01T00:30:00Z"));
     IcannReportingUploadAction action = createAction();
     action.subdir = "icann/monthly/2006-07";
     action.run();
@@ -349,39 +321,25 @@ public class IcannReportingUploadActionTest {
   }
 
   @Test
-  public void testSuccess_nullCursors() throws Exception {
+  public void testSuccess_nullCursorsInitiatedToFirstOfNextMonth() throws Exception {
     createTlds("new");
-    writeGcsFile(
-        gcsService,
-        new GcsFilename("basin/icann/monthly/2006-06", "new-transactions-200606.csv"),
-        PAYLOAD_SUCCESS);
-    writeGcsFile(
-        gcsService,
-        new GcsFilename("basin/icann/monthly/2006-06", "new-activity-200606.csv"),
-        PAYLOAD_SUCCESS);
-    when(mockReporter.send(PAYLOAD_SUCCESS, "new-transactions-200606.csv")).thenReturn(true);
-    when(mockReporter.send(PAYLOAD_SUCCESS, "new-activity-200606.csv")).thenReturn(true);
 
     IcannReportingUploadAction action = createAction();
     action.run();
     verify(mockReporter).send(PAYLOAD_SUCCESS, "foo-activity-200606.csv");
     verify(mockReporter).send(PAYLOAD_FAIL, "tld-activity-200606.csv");
-    verify(mockReporter).send(PAYLOAD_SUCCESS, "new-activity-200606.csv");
     verify(mockReporter).send(PAYLOAD_SUCCESS, "foo-transactions-200606.csv");
     verify(mockReporter).send(PAYLOAD_SUCCESS, "tld-transactions-200606.csv");
-    verify(mockReporter).send(PAYLOAD_SUCCESS, "new-transactions-200606.csv");
     verifyNoMoreInteractions(mockReporter);
 
     verify(emailService)
         .sendEmail(
             EmailMessage.create(
-                "ICANN Monthly report upload summary: 5/6 succeeded",
+                "ICANN Monthly report upload summary: 3/4 succeeded",
                 "Report Filename - Upload status:\n"
                     + "foo-activity-200606.csv - SUCCESS\n"
-                    + "new-activity-200606.csv - SUCCESS\n"
                     + "tld-activity-200606.csv - FAILURE\n"
                     + "foo-transactions-200606.csv - SUCCESS\n"
-                    + "new-transactions-200606.csv - SUCCESS\n"
                     + "tld-transactions-200606.csv - SUCCESS",
                 new InternetAddress("recipient@example.com"),
                 new InternetAddress("sender@example.com")));
@@ -391,10 +349,10 @@ public class IcannReportingUploadActionTest {
             .load()
             .key(Cursor.createKey(CursorType.ICANN_UPLOAD_ACTIVITY, Registry.get("new")))
             .now();
-    assertThat(newActivityCursor.getCursorTime()).isEqualTo(DateTime.parse("2006-07-01TZ"));
+    assertThat(newActivityCursor.getCursorTime()).isEqualTo(DateTime.parse("2006-08-01TZ"));
     Cursor newTransactionCursor =
         ofy().load().key(Cursor.createKey(CursorType.ICANN_UPLOAD_TX, Registry.get("new"))).now();
-    assertThat(newTransactionCursor.getCursorTime()).isEqualTo(DateTime.parse("2006-07-01TZ"));
+    assertThat(newTransactionCursor.getCursorTime()).isEqualTo(DateTime.parse("2006-08-01TZ"));
   }
 }