diff --git a/core/src/main/java/google/registry/reporting/icann/IcannReportingUploadAction.java b/core/src/main/java/google/registry/reporting/icann/IcannReportingUploadAction.java index 8898306d1..f55814896 100644 --- a/core/src/main/java/google/registry/reporting/icann/IcannReportingUploadAction.java +++ b/core/src/main/java/google/registry/reporting/icann/IcannReportingUploadAction.java @@ -63,7 +63,7 @@ import org.joda.time.Duration; *

Parameters: * *

subdir: the subdirectory of gs://[project-id]-reporting/ to retrieve reports from. For - * example: "manual/dir" means reports will be stored under gs://[project-id]-reporting/manual/dir. + * example, "manual/dir" means reports will be stored under gs://[project-id]-reporting/manual/dir. * Defaults to "icann/monthly/[last month in yyyy-MM format]". */ @Action( @@ -98,6 +98,17 @@ public final class IcannReportingUploadAction implements Runnable { @Inject IcannReportingUploadAction() {} + /** + * Get the scheduled time for the next month of the given {@link DateTime}. + * + *

The scheduled time is always the second day of next month at 10AM UTC. This is because the + * staging action is scheduled to run at 9AM UTC on that day, and there is no reason to run the + * upload job before that. See {@code icannReportingStaging} in {@code cloud-scheduler.xml}. + */ + private static DateTime getScheduledTimeForCurrentMonth(DateTime time) { + return time.withDayOfMonth(2).withHourOfDay(10).withMinuteOfHour(0).plusMonths(1); + } + @Override public void run() { if (!lockHandler.executeWithLocks( @@ -170,12 +181,10 @@ public final class IcannReportingUploadAction implements Runnable { if (success) { Cursor newCursor = Cursor.createScoped( - cursorType, - cursorTime.withTimeAtStartOfDay().withDayOfMonth(1).plusMonths(1), - Tld.get(tldStr)); - // In order to keep the transactions short-lived, we load all of the cursors in a single + cursorType, getScheduledTimeForCurrentMonth(cursorTime), Tld.get(tldStr)); + // In order to keep the transactions short-lived, we load all the cursors in a single // transaction then later use per-cursor transactions when checking + saving the cursors. We - // run behind a lock so the cursors shouldn't be changed, but double check to be sure. + // run behind a lock, so the cursors shouldn't be changed, but double check to be sure. success = tm().transact( () -> { @@ -235,7 +244,7 @@ public final class IcannReportingUploadAction implements Runnable { /** * Return a map with the Cursor and scope for each key in the keyMap. If the key from the keyMap - * does not have an existing cursor, create a new cursor with a default cursorTime of the first of + * does not have an existing cursor, create a new cursor with a default cursorTime at the first of * next month. */ private ImmutableMap defaultNullCursorsToNextMonthAndAddToMap( @@ -245,15 +254,13 @@ public final class IcannReportingUploadAction implements Runnable { ImmutableMap.Builder cursors = new ImmutableMap.Builder<>(); keyMap.forEach( (key, registry) -> { - // Cursor time is defaulted to the first of next month since a new tld will not yet have a - // report staged for upload. + // Cursor time is defaulted to 10AM on the second day of next month since a new tld will + // not yet have a report staged for upload. Cursor cursor = cursorMap.getOrDefault( key, Cursor.createScoped( - type, - clock.nowUtc().withDayOfMonth(1).withTimeAtStartOfDay().plusMonths(1), - registry)); + type, getScheduledTimeForCurrentMonth(clock.nowUtc()), registry)); if (!cursorMap.containsValue(cursor)) { tm().put(cursor); } @@ -278,7 +285,7 @@ public final class IcannReportingUploadAction implements Runnable { } private void emailUploadResults(ImmutableMap reportSummary) { - if (reportSummary.size() == 0) { + if (reportSummary.isEmpty()) { logger.atInfo().log("No uploads were attempted today; skipping notification email."); return; } diff --git a/core/src/test/java/google/registry/reporting/icann/IcannReportingUploadActionTest.java b/core/src/test/java/google/registry/reporting/icann/IcannReportingUploadActionTest.java index 88344c87e..c4ee60215 100644 --- a/core/src/test/java/google/registry/reporting/icann/IcannReportingUploadActionTest.java +++ b/core/src/test/java/google/registry/reporting/icann/IcannReportingUploadActionTest.java @@ -130,11 +130,12 @@ class IcannReportingUploadActionTest { .sendEmail( EmailMessage.create( "ICANN Monthly report upload summary: 3/4 succeeded", - "Report Filename - Upload status:\n" - + "foo-activity-200606.csv - SUCCESS\n" - + "foo-transactions-200606.csv - SUCCESS\n" - + "tld-activity-200606.csv - FAILURE\n" - + "tld-transactions-200606.csv - SUCCESS", + """ + Report Filename - Upload status: + foo-activity-200606.csv - SUCCESS + foo-transactions-200606.csv - SUCCESS + tld-activity-200606.csv - FAILURE + tld-transactions-200606.csv - SUCCESS""", new InternetAddress("recipient@example.com"))); } @@ -164,9 +165,10 @@ class IcannReportingUploadActionTest { .sendEmail( EmailMessage.create( "ICANN Monthly report upload summary: 2/2 succeeded", - "Report Filename - Upload status:\n" - + "tld-activity-200512.csv - SUCCESS\n" - + "tld-transactions-200512.csv - SUCCESS", + """ + Report Filename - Upload status: + tld-activity-200512.csv - SUCCESS + tld-transactions-200512.csv - SUCCESS""", new InternetAddress("recipient@example.com"))); } @@ -179,7 +181,7 @@ class IcannReportingUploadActionTest { action.run(); Cursor cursor = loadByKey(Cursor.createScopedVKey(CursorType.ICANN_UPLOAD_ACTIVITY, Tld.get("tld"))); - assertThat(cursor.getCursorTime()).isEqualTo(DateTime.parse("2006-08-01TZ")); + assertThat(cursor.getCursorTime()).isEqualTo(DateTime.parse("2006-08-02T10:00:00Z")); } @Test @@ -207,11 +209,12 @@ class IcannReportingUploadActionTest { .sendEmail( EmailMessage.create( "ICANN Monthly report upload summary: 3/4 succeeded", - "Report Filename - Upload status:\n" - + "foo-activity-200606.csv - SUCCESS\n" - + "foo-transactions-200606.csv - SUCCESS\n" - + "tld-activity-200606.csv - FAILURE\n" - + "tld-transactions-200606.csv - SUCCESS", + """ + Report Filename - Upload status: + foo-activity-200606.csv - SUCCESS + foo-transactions-200606.csv - SUCCESS + tld-activity-200606.csv - FAILURE + tld-transactions-200606.csv - SUCCESS""", new InternetAddress("recipient@example.com"))); } @@ -266,11 +269,12 @@ class IcannReportingUploadActionTest { .sendEmail( EmailMessage.create( "ICANN Monthly report upload summary: 3/4 succeeded", - "Report Filename - Upload status:\n" - + "foo-activity-200606.csv - SUCCESS\n" - + "foo-transactions-200606.csv - SUCCESS\n" - + "tld-activity-200606.csv - FAILURE\n" - + "tld-transactions-200606.csv - SUCCESS", + """ + Report Filename - Upload status: + foo-activity-200606.csv - SUCCESS + foo-transactions-200606.csv - SUCCESS + tld-activity-200606.csv - FAILURE + tld-transactions-200606.csv - SUCCESS""", new InternetAddress("recipient@example.com"))); } @@ -313,14 +317,14 @@ class IcannReportingUploadActionTest { IcannReportingUploadAction action = createAction(); action.lockHandler = new FakeLockHandler(false); ServiceUnavailableException thrown = - assertThrows(ServiceUnavailableException.class, () -> action.run()); + assertThrows(ServiceUnavailableException.class, action::run); assertThat(thrown) .hasMessageThat() .contains("Lock for IcannReportingUploadAction already in use"); } @Test - void testSuccess_nullCursorsInitiatedToFirstOfNextMonth() throws Exception { + void testSuccess_nullCursorsInitiatedToSecondOfNextMonth() throws Exception { createTlds("new"); IcannReportingUploadAction action = createAction(); @@ -335,18 +339,20 @@ class IcannReportingUploadActionTest { .sendEmail( EmailMessage.create( "ICANN Monthly report upload summary: 3/4 succeeded", - "Report Filename - Upload status:\n" - + "foo-activity-200606.csv - SUCCESS\n" - + "foo-transactions-200606.csv - SUCCESS\n" - + "tld-activity-200606.csv - FAILURE\n" - + "tld-transactions-200606.csv - SUCCESS", + """ + Report Filename - Upload status: + foo-activity-200606.csv - SUCCESS + foo-transactions-200606.csv - SUCCESS + tld-activity-200606.csv - FAILURE + tld-transactions-200606.csv - SUCCESS""", new InternetAddress("recipient@example.com"))); Cursor newActivityCursor = loadByKey(Cursor.createScopedVKey(CursorType.ICANN_UPLOAD_ACTIVITY, Tld.get("new"))); - assertThat(newActivityCursor.getCursorTime()).isEqualTo(DateTime.parse("2006-08-01TZ")); + assertThat(newActivityCursor.getCursorTime()).isEqualTo(DateTime.parse("2006-08-02T10:00:00Z")); Cursor newTransactionCursor = loadByKey(Cursor.createScopedVKey(CursorType.ICANN_UPLOAD_TX, Tld.get("new"))); - assertThat(newTransactionCursor.getCursorTime()).isEqualTo(DateTime.parse("2006-08-01TZ")); + assertThat(newTransactionCursor.getCursorTime()) + .isEqualTo(DateTime.parse("2006-08-02T10:00:00Z")); } }