Skip to content

Begin migration of joda DateTime to java.time.Instant#2992

Open
CydeWeys wants to merge 1 commit intogoogle:masterfrom
CydeWeys:datetime-to-instants
Open

Begin migration of joda DateTime to java.time.Instant#2992
CydeWeys wants to merge 1 commit intogoogle:masterfrom
CydeWeys:datetime-to-instants

Conversation

@CydeWeys
Copy link
Copy Markdown
Member

@CydeWeys CydeWeys commented Mar 27, 2026

java.time has been around since Java 8 and was based on joda DateTime, so this is an overdue migration. We're migrating specifically to Instant in most places rather than ZonedDateTime because we were always using DateTimes in UTC to reference a specific instant, which is exactly what Instants are for. ZonedDateTime set to UTC may still be useful in some places that are heavy on date math (especially in tests).

There is a lot more work to be done after this, but I wanted to put together a manual PR showing my overall approach for how to do the migration that I can then hopefully follow along with AI to continue making these changes throughout the codebase. The basic approach is to migrate a small number of methods at a time, marking the old methods as @deprecated when possible (not always possible because of @InlineMe restrictions). This PR doesn't yet migrate any DateTime fields in the model classes, so that's the one remaining type of refactor to figure out after this. We won't be changing how any of the data is actually stored in the database.

BUG= http://b/496985355


This change is Reviewable

@CydeWeys CydeWeys requested a review from ptkach March 27, 2026 20:13
@CydeWeys CydeWeys force-pushed the datetime-to-instants branch from 2745693 to 7248b29 Compare March 27, 2026 20:16
Copy link
Copy Markdown

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@CydeWeys CydeWeys force-pushed the datetime-to-instants branch 2 times, most recently from 0ae2854 to 218f732 Compare March 30, 2026 18:22
@CydeWeys CydeWeys requested a review from gbrodman March 30, 2026 19:30
java.time has been around since Java 8 and was based on joda DateTime, so this
is an overdue migration.  We're migrating specifically to Instant in most places
rather than ZonedDateTime because we were always using DateTimes in UTC to
reference a specific instant, which is exactly what Instants are
for. ZonedDateTime set to UTC may still be useful in some places that are heavy
on date math (especially in tests).

There is a lot more work to be done after this, but I wanted to put together a
manual PR showing my overall approach for how to do the migration that I can
then hopefully follow along with AI to continue making these changes throughout
the codebase. The basic approach is to migrate a small number of methods at a
time, marking the old methods as @deprecated when possible (not always possible
because of @InlineMe restrictions). This PR doesn't yet migrate any DateTime
fields in the model classes, so that's the one remaining type of refactor to
figure out after this. We won't be changing how any of the data is actually
stored in the database.

BUG= http://b/496985355
@CydeWeys CydeWeys force-pushed the datetime-to-instants branch from 218f732 to 20a2301 Compare March 30, 2026 21:19
Copy link
Copy Markdown
Collaborator

@gbrodman gbrodman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gbrodman reviewed 66 files and all commit messages, and made 5 comments.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on CydeWeys and ptkach).


common/src/main/java/google/registry/util/DateTimeUtils.java line 102 at r1 (raw file):

  /** Returns whether the first {@link DateTime} is equal to or later than the second. */
  public static boolean isAtOrAfter(DateTime timeToCheck, DateTime timeToCompareTo) {

i would assume that these DateTime methods should be deprecated as well


common/src/main/java/google/registry/util/DateTimeUtils.java line 123 at r1 (raw file):

  /**
   * Adds years to a date, in the {@code Duration} sense of semantic years. Use this instead of
   * {@link DateTime#plusYears} to ensure that we never end up on February 29.

I don't think this comment is up to date. You can't directly add years to an instant for other reasons though.


common/src/main/java/google/registry/util/DateTimeUtils.java line 36 at r2 (raw file):

  @Deprecated public static final DateTime START_OF_TIME = new DateTime(0, DateTimeZone.UTC);

  /** The start of the UNIX epoch in UTC, in a convenient constant. */

Technically, this isn't "in UTC" -- we treat it as being in UTC but a Java instant is not explicitly zoned


common/src/main/java/google/registry/util/DateTimeUtils.java line 133 at r2 (raw file):

  /**
   * Subtracts years from a date, in the {@code Duration} sense of semantic years. Use this instead

ditto re: this comment not up to date (mentioning DateTime)


util/src/test/java/google/registry/util/DateTimeUtilsTest.java line 42 at r2 (raw file):

/** Unit tests for {@link DateTimeUtils}. */
class DateTimeUtilsTest {

I'm assuming that the plan is to just change all the existing arithmetic tests to use Instants instead of DateTimes in the future when we remove DateTimes. Assuming that's the case, we should probably make a note of it in a TODO comment somewhere in this class

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants