Migrate bsa, dns, batch, and reporting packages to java.time by CydeWeys · Pull Request #3031 · google/nomulus

CydeWeys · 2026-04-30T21:01:45Z

This change is

    this.sigOutput = sigOutput;
-    signer = closer.register(new RydePgpSigningOutputStream(checkNotNull(rydeOutput), signingKey));
+    signer =
+        closer.register(new RydePgpSigningOutputStream(checkNotNull(receiverOutput), signingKey));


gbrodman · 2026-05-05T21:01:45Z

    // If cursor time is before now, upload the corresponding report
    cursors.entrySet().stream()
-        .filter(entry -> entry.getKey().getCursorTime().isBefore(clock.nowUtc()))
+        .filter(entry -> toInstant(entry.getKey().getCursorTime()).isBefore(clock.now()))


Fixed, but, your comment was generic enough that neither I nor Gemini immediately understood what your intended fix was.

yes, i did not see it as necessary to repeat the content of the automated comment to which i was replying

gbrodman · 2026-05-05T21:01:46Z

    Cursor cursor =
        loadByKey(Cursor.createScopedVKey(CursorType.ICANN_UPLOAD_ACTIVITY, Tld.get("tld")));
-    assertThat(cursor.getCursorTime()).isEqualTo(DateTime.parse("2006-08-02T10:00:00Z"));
+    assertThat(toInstant(cursor.getCursorTime())).isEqualTo(Instant.parse("2006-08-02T10:00:00Z"));


gbrodman · 2026-05-05T21:01:46Z

    Cursor cursor =
        loadByKey(Cursor.createScopedVKey(CursorType.ICANN_UPLOAD_ACTIVITY, Tld.get("tld")));
-    assertThat(cursor.getCursorTime()).isEqualTo(DateTime.parse("2006-07-01TZ"));
+    assertThat(toInstant(cursor.getCursorTime())).isEqualTo(Instant.parse("2006-07-01T00:00:00Z"));


gbrodman · 2026-05-05T21:01:47Z

    Cursor cursor =
        loadByKey(Cursor.createScopedVKey(CursorType.ICANN_UPLOAD_ACTIVITY, Tld.get("foo")));
-    assertThat(cursor.getCursorTime()).isEqualTo(DateTime.parse("2006-07-01TZ"));
+    assertThat(toInstant(cursor.getCursorTime())).isEqualTo(Instant.parse("2006-07-01T00:00:00Z"));


gbrodman

@gbrodman reviewed 184 files and all commit messages, and made 11 comments.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on CydeWeys).

core/src/main/java/google/registry/rde/RdeUtils.java line 41 at r3 (raw file):

  private static final int PEEK_SIZE = 2048;

  public static final DateTimeFormatter RDE_WATERMARK_FORMATTER =

this is used in more than just RDE, maybe consider moving it to a more generic class?

core/src/main/java/google/registry/rde/RydeEncoder.java line 144 at r3 (raw file):

    /** Returns the built {@link RydeEncoder}. */
    public RydeEncoder build() throws IOException {

why are these exceptions added?

core/src/main/java/google/registry/rde/RydeTar.java line 47 at r3 (raw file):

  static ImprovedOutputStream openTarWriter(
      @WillNotClose OutputStream os, long expectedSize, String filename, Instant modified)
      throws IOException {

why did it make these changes?

core/src/test/java/google/registry/batch/BulkDomainTransferActionTest.java line 82 at r3 (raw file):

        persistResource(
            persistDomainWithDependentResources(
                    "pendingdelete", "tld", now, minusDays(now, 1), plusDays(now, 30))

why are you switching the plusMonths to plusDays in this test?

gbrodman · 2026-05-05T21:01:45Z

    // If cursor time is before now, upload the corresponding report
    cursors.entrySet().stream()
-        .filter(entry -> entry.getKey().getCursorTime().isBefore(clock.nowUtc()))
+        .filter(entry -> toInstant(entry.getKey().getCursorTime()).isBefore(clock.now()))


gbrodman · 2026-05-05T21:01:46Z

    Cursor cursor =
        loadByKey(Cursor.createScopedVKey(CursorType.ICANN_UPLOAD_ACTIVITY, Tld.get("tld")));
-    assertThat(cursor.getCursorTime()).isEqualTo(DateTime.parse("2006-08-02T10:00:00Z"));
+    assertThat(toInstant(cursor.getCursorTime())).isEqualTo(Instant.parse("2006-08-02T10:00:00Z"));


gbrodman · 2026-05-05T21:01:46Z

    Cursor cursor =
        loadByKey(Cursor.createScopedVKey(CursorType.ICANN_UPLOAD_ACTIVITY, Tld.get("tld")));
-    assertThat(cursor.getCursorTime()).isEqualTo(DateTime.parse("2006-07-01TZ"));
+    assertThat(toInstant(cursor.getCursorTime())).isEqualTo(Instant.parse("2006-07-01T00:00:00Z"));


gbrodman · 2026-05-05T21:01:47Z

    Cursor cursor =
        loadByKey(Cursor.createScopedVKey(CursorType.ICANN_UPLOAD_ACTIVITY, Tld.get("foo")));
-    assertThat(cursor.getCursorTime()).isEqualTo(DateTime.parse("2006-07-01TZ"));
+    assertThat(toInstant(cursor.getCursorTime())).isEqualTo(Instant.parse("2006-07-01T00:00:00Z"));


CydeWeys

@CydeWeys made 5 comments and resolved 1 discussion.
Reviewable status: 172 of 184 files reviewed, 11 unresolved discussions (waiting on gbrodman).

core/src/main/java/google/registry/rde/RdeUtils.java line 41 at r3 (raw file):

Previously, gbrodman wrote…

this is used in more than just RDE, maybe consider moving it to a more generic class?

Done, not sure it's a big improvement though.

core/src/main/java/google/registry/rde/RydeEncoder.java line 144 at r3 (raw file):

Previously, gbrodman wrote…

why are these exceptions added?

Removed.

core/src/main/java/google/registry/rde/RydeTar.java line 47 at r3 (raw file):

Previously, gbrodman wrote…

why did it make these changes?

Removed.

CydeWeys · 2026-05-06T14:09:16Z

    // If cursor time is before now, upload the corresponding report
    cursors.entrySet().stream()
-        .filter(entry -> entry.getKey().getCursorTime().isBefore(clock.nowUtc()))
+        .filter(entry -> toInstant(entry.getKey().getCursorTime()).isBefore(clock.now()))


Fixed, but, your comment was generic enough that neither I nor Gemini immediately understood what your intended fix was.

gbrodman

@gbrodman reviewed 12 files and all commit messages, made 1 comment, and resolved 3 discussions.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on CydeWeys).

gbrodman · 2026-05-06T14:34:13Z

    // If cursor time is before now, upload the corresponding report
    cursors.entrySet().stream()
-        .filter(entry -> entry.getKey().getCursorTime().isBefore(clock.nowUtc()))
+        .filter(entry -> toInstant(entry.getKey().getCursorTime()).isBefore(clock.now()))


yes, i did not see it as necessary to repeat the content of the automated comment to which i was replying

CydeWeys

@CydeWeys made 1 comment and resolved 4 discussions.
Reviewable status: 168 of 195 files reviewed, 1 unresolved discussion (waiting on gbrodman).

core/src/test/java/google/registry/batch/BulkDomainTransferActionTest.java line 82 at r3 (raw file):

Previously, gbrodman wrote…

why are you switching the plusMonths to plusDays in this test?

Done.

gbrodman

@gbrodman reviewed 27 files and all commit messages, made 4 comments, and resolved 1 discussion.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on CydeWeys).

console-webapp/package-lock.json line 631 at r6 (raw file):

      }
    },
    "node_modules/@angular/build/node_modules/@types/node": {

the ai is not listening to you when you tell it to not modify this

core/src/main/java/google/registry/rde/RydeEncoder.java line 83 at r6 (raw file):

    } catch (Throwable e) {
      try {
        closer.close();

this shouldn't be in the constructor.......

core/src/test/java/google/registry/model/common/CursorTest.java line 42 at r6 (raw file):

  @BeforeEach
  void setUp() {
    createTld("tld");

if you're adding this, you can remove it in the test methods that have it

core/src/test/java/google/registry/tools/ListCursorsCommandTest.java line 42 at r6 (raw file):

  @BeforeEach
  void beforeEach() {
    createTld("tld");

i don't think this is necessary

CydeWeys

@CydeWeys made 2 comments.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on gbrodman).

console-webapp/package-lock.json line 631 at r6 (raw file):

Previously, gbrodman wrote…

the ai is not listening to you when you tell it to not modify this

I do wonder what the value of excluding this really is. If it's out of date (which it has been for awhile), it gets updated any time you run the full build, which I do manually a lot too. Not sure how worth it it is to continuously have to remember to exclude changes to this file (either for me, or the AI).

core/src/main/java/google/registry/rde/RydeEncoder.java line 83 at r6 (raw file):

Previously, gbrodman wrote…

this shouldn't be in the constructor.......

Why? It made these changes in response to an automated errorprone comment, about exceptions not being handled.

gbrodman

@gbrodman made 1 comment.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on CydeWeys).

core/src/main/java/google/registry/rde/RydeEncoder.java line 83 at r6 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

Why? It made these changes in response to an automated errorprone comment, about exceptions not being handled.

Think about it instead of letting the AI do it. Why would we close everything in the constructor of the object? We close it in, unsurprisingly, the close() method.

gbrodman

@gbrodman made 1 comment and resolved 1 discussion.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on CydeWeys).

console-webapp/package-lock.json line 631 at r6 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

I do wonder what the value of excluding this really is. If it's out of date (which it has been for awhile), it gets updated any time you run the full build, which I do manually a lot too. Not sure how worth it it is to continuously have to remember to exclude changes to this file (either for me, or the AI).

yeah idc

This commit migrates the BSA, DNS, batch, and reporting packages from Joda-Time to java.time. Key changes include: - Updated Sleeper, Clock, and BigqueryUtils to use java.time types natively. - Refactored models like RdeRevision and Tld to eliminate redundant Joda conversions, utilizing new DateTimeUtils static utilities for LocalDate. - Improved test safety by replacing dynamic Instant.now() calls with static parsed constants. - Migrated temporal arithmetic in test suites to use DateTimeUtils convenience methods (plusDays, minusDays). - Updated BigqueryUtils serialization to preserve millisecond precision and formatting for large years, ensuring consistency with previous Joda behavior. - Enhanced code readability by converting long concatenated strings to Java text blocks in LordnLogTest. - Resolved environmental test failures in SyncRegistrarsSheetTest by synchronizing the FakeClock with the JPA extension. - Updated project engineering standards (GEMINI.md) to prefer Truth's .hasValue() for Optional assertions. Verified with a clean full build and all relevant test suites passing.

CydeWeys

@CydeWeys made 1 comment.
Reviewable status: 167 of 220 files reviewed, 3 unresolved discussions (waiting on gbrodman).

core/src/main/java/google/registry/rde/RydeEncoder.java line 83 at r6 (raw file):

Previously, gbrodman wrote…

Think about it instead of letting the AI do it. Why would we close everything in the constructor of the object? We close it in, unsurprisingly, the close() method.

Reverted these changes. They're not even wrong though -- it is cleaner to close it immediately in a catch block rather than relying on this more obscure Closer method to do so. And it would only be closing them when an unrecoverable error is thrown, so doing so in the constructor is fine.

CydeWeys force-pushed the javatime-refactor-8th branch from 2fb2275 to 28812b7 Compare May 1, 2026 22:22

github-advanced-security AI found potential problems May 1, 2026

View reviewed changes

CydeWeys force-pushed the javatime-refactor-8th branch 2 times, most recently from 702da9d to a0b8231 Compare May 5, 2026 20:07

CydeWeys enabled auto-merge May 5, 2026 20:07

CydeWeys requested a review from gbrodman May 5, 2026 20:07

CydeWeys force-pushed the javatime-refactor-8th branch from a0b8231 to 67bdec7 Compare May 5, 2026 20:14

gbrodman requested changes May 5, 2026

View reviewed changes

CydeWeys force-pushed the javatime-refactor-8th branch 2 times, most recently from 7b518e2 to 1a9f62a Compare May 6, 2026 14:07

CydeWeys commented May 6, 2026

View reviewed changes

gbrodman reviewed May 6, 2026

View reviewed changes

CydeWeys force-pushed the javatime-refactor-8th branch from 1a9f62a to f18a93b Compare May 6, 2026 15:40

CydeWeys commented May 6, 2026

View reviewed changes

gbrodman requested changes May 6, 2026

View reviewed changes

CydeWeys commented May 6, 2026

View reviewed changes

gbrodman requested changes May 6, 2026

View reviewed changes

gbrodman reviewed May 6, 2026

View reviewed changes

CydeWeys force-pushed the javatime-refactor-8th branch from f18a93b to fac978f Compare May 6, 2026 19:05

CydeWeys commented May 6, 2026

View reviewed changes

Conversation

CydeWeys commented Apr 30, 2026 • edited by gbrodman Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gbrodman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

CydeWeys left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gbrodman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CydeWeys left a comment

Choose a reason for hiding this comment

Uh oh!

gbrodman left a comment

Choose a reason for hiding this comment

Uh oh!

CydeWeys left a comment

Choose a reason for hiding this comment

Uh oh!

gbrodman left a comment

Choose a reason for hiding this comment

Uh oh!

gbrodman left a comment

Choose a reason for hiding this comment

Uh oh!

CydeWeys left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CydeWeys commented Apr 30, 2026 •

edited by gbrodman

Loading