Skip to content

Commit 60e84e7

Browse files
authored
Begin migration of joda DateTime to java.time.Instant (#2992)
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
1 parent aedfdd4 commit 60e84e7

66 files changed

Lines changed: 530 additions & 213 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

common/src/main/java/google/registry/util/Clock.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package google.registry.util;
1616

1717
import java.io.Serializable;
18+
import java.time.Instant;
1819
import javax.annotation.concurrent.ThreadSafe;
1920
import org.joda.time.DateTime;
2021

@@ -30,5 +31,9 @@
3031
public interface Clock extends Serializable {
3132

3233
/** Returns current time in UTC timezone. */
34+
@Deprecated
3335
DateTime nowUtc();
36+
37+
/** Returns current Instant (which is always in UTC). */
38+
Instant now();
3439
}

common/src/main/java/google/registry/util/DateTimeUtils.java

Lines changed: 87 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@
2020
import com.google.common.collect.Lists;
2121
import com.google.common.collect.Ordering;
2222
import java.sql.Date;
23+
import java.time.Instant;
24+
import java.time.ZoneOffset;
25+
import javax.annotation.Nullable;
2326
import org.joda.time.DateTime;
2427
import org.joda.time.DateTimeZone;
2528
import org.joda.time.LocalDate;
@@ -28,7 +31,10 @@
2831
public abstract class DateTimeUtils {
2932

3033
/** The start of the epoch, in a convenient constant. */
31-
public static final DateTime START_OF_TIME = new DateTime(0, DateTimeZone.UTC);
34+
@Deprecated public static final DateTime START_OF_TIME = new DateTime(0, DateTimeZone.UTC);
35+
36+
/** The start of the UNIX epoch (which is defined in UTC), in a convenient constant. */
37+
public static final Instant START_INSTANT = Instant.ofEpochMilli(0);
3238

3339
/**
3440
* A date in the far future that we can treat as infinity.
@@ -37,19 +43,40 @@ public abstract class DateTimeUtils {
3743
* but Java uses milliseconds, so this is the largest representable date that will survive a
3844
* round-trip through the database.
3945
*/
46+
@Deprecated
4047
public static final DateTime END_OF_TIME = new DateTime(Long.MAX_VALUE / 1000, DateTimeZone.UTC);
4148

49+
/**
50+
* An instant in the far future that we can treat as infinity.
51+
*
52+
* <p>This value is (2^63-1)/1000 rounded down. Postgres can store dates as 64 bit microseconds,
53+
* but Java uses milliseconds, so this is the largest representable date that will survive a
54+
* round-trip through the database.
55+
*/
56+
public static final Instant END_INSTANT = Instant.ofEpochMilli(Long.MAX_VALUE / 1000);
57+
4258
/** Returns the earliest of a number of given {@link DateTime} instances. */
4359
public static DateTime earliestOf(DateTime first, DateTime... rest) {
60+
return earliestDateTimeOf(Lists.asList(first, rest));
61+
}
62+
63+
/** Returns the earliest of a number of given {@link Instant} instances. */
64+
public static Instant earliestOf(Instant first, Instant... rest) {
4465
return earliestOf(Lists.asList(first, rest));
4566
}
4667

4768
/** Returns the earliest element in a {@link DateTime} iterable. */
48-
public static DateTime earliestOf(Iterable<DateTime> dates) {
69+
public static DateTime earliestDateTimeOf(Iterable<DateTime> dates) {
4970
checkArgument(!Iterables.isEmpty(dates));
5071
return Ordering.<DateTime>natural().min(dates);
5172
}
5273

74+
/** Returns the earliest element in a {@link Instant} iterable. */
75+
public static Instant earliestOf(Iterable<Instant> instants) {
76+
checkArgument(!Iterables.isEmpty(instants));
77+
return Ordering.<Instant>natural().min(instants);
78+
}
79+
5380
/** Returns the latest of a number of given {@link DateTime} instances. */
5481
public static DateTime latestOf(DateTime first, DateTime... rest) {
5582
return latestOf(Lists.asList(first, rest));
@@ -66,34 +93,92 @@ public static boolean isBeforeOrAt(DateTime timeToCheck, DateTime timeToCompareT
6693
return !timeToCheck.isAfter(timeToCompareTo);
6794
}
6895

96+
/** Returns whether the first {@link Instant} is equal to or earlier than the second. */
97+
public static boolean isBeforeOrAt(Instant timeToCheck, Instant timeToCompareTo) {
98+
return !timeToCheck.isAfter(timeToCompareTo);
99+
}
100+
69101
/** Returns whether the first {@link DateTime} is equal to or later than the second. */
70102
public static boolean isAtOrAfter(DateTime timeToCheck, DateTime timeToCompareTo) {
71103
return !timeToCheck.isBefore(timeToCompareTo);
72104
}
73105

106+
/** Returns whether the first {@link Instant} is equal to or later than the second. */
107+
public static boolean isAtOrAfter(Instant timeToCheck, Instant timeToCompareTo) {
108+
return !timeToCheck.isBefore(timeToCompareTo);
109+
}
110+
74111
/**
75112
* Adds years to a date, in the {@code Duration} sense of semantic years. Use this instead of
76113
* {@link DateTime#plusYears} to ensure that we never end up on February 29.
77114
*/
115+
@Deprecated
78116
public static DateTime leapSafeAddYears(DateTime now, int years) {
79117
checkArgument(years >= 0);
80118
return years == 0 ? now : now.plusYears(1).plusYears(years - 1);
81119
}
82120

121+
/**
122+
* Adds years to a date, in the {@code Duration} sense of semantic years. Use this instead of
123+
* {@link java.time.ZonedDateTime#plusYears} to ensure that we never end up on February 29.
124+
*/
125+
public static Instant leapSafeAddYears(Instant now, long years) {
126+
checkArgument(years >= 0);
127+
return (years == 0)
128+
? now
129+
: now.atZone(ZoneOffset.UTC).plusYears(1).plusYears(years - 1).toInstant();
130+
}
131+
83132
/**
84133
* Subtracts years from a date, in the {@code Duration} sense of semantic years. Use this instead
85134
* of {@link DateTime#minusYears} to ensure that we never end up on February 29.
86135
*/
136+
@Deprecated
87137
public static DateTime leapSafeSubtractYears(DateTime now, int years) {
88138
checkArgument(years >= 0);
89139
return years == 0 ? now : now.minusYears(1).minusYears(years - 1);
90140
}
91141

142+
/**
143+
* Subtracts years from a date, in the {@code Duration} sense of semantic years. Use this instead
144+
* of {@link java.time.ZonedDateTime#minusYears} to ensure that we never end up on February 29.
145+
*/
146+
public static Instant leapSafeSubtractYears(Instant now, int years) {
147+
checkArgument(years >= 0);
148+
return (years == 0)
149+
? now
150+
: now.atZone(ZoneOffset.UTC).minusYears(1).minusYears(years - 1).toInstant();
151+
}
152+
92153
public static Date toSqlDate(LocalDate localDate) {
93154
return new Date(localDate.toDateTimeAtStartOfDay().getMillis());
94155
}
95156

96157
public static LocalDate toLocalDate(Date date) {
97158
return new LocalDate(date.getTime(), DateTimeZone.UTC);
98159
}
160+
161+
/** Convert a joda {@link DateTime} to a java.time {@link Instant}, null-safe. */
162+
@Nullable
163+
public static Instant toInstant(@Nullable DateTime dateTime) {
164+
return (dateTime == null) ? null : Instant.ofEpochMilli(dateTime.getMillis());
165+
}
166+
167+
/** Convert a java.time {@link Instant} to a joda {@link DateTime}, null-safe. */
168+
@Nullable
169+
public static DateTime toDateTime(@Nullable Instant instant) {
170+
return (instant == null) ? null : new DateTime(instant.toEpochMilli(), DateTimeZone.UTC);
171+
}
172+
173+
public static Instant plusYears(Instant instant, int years) {
174+
return instant.atZone(ZoneOffset.UTC).plusYears(years).toInstant();
175+
}
176+
177+
public static Instant plusDays(Instant instant, int days) {
178+
return instant.atZone(ZoneOffset.UTC).plusDays(days).toInstant();
179+
}
180+
181+
public static Instant minusDays(Instant instant, int days) {
182+
return instant.atZone(ZoneOffset.UTC).minusDays(days).toInstant();
183+
}
99184
}

common/src/main/java/google/registry/util/SystemClock.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import static org.joda.time.DateTimeZone.UTC;
1818

1919
import jakarta.inject.Inject;
20+
import java.time.Instant;
2021
import javax.annotation.concurrent.ThreadSafe;
2122
import org.joda.time.DateTime;
2223

@@ -34,4 +35,9 @@ public SystemClock() {}
3435
public DateTime nowUtc() {
3536
return DateTime.now(UTC);
3637
}
38+
39+
@Override
40+
public Instant now() {
41+
return Instant.now();
42+
}
3743
}

common/src/testing/java/google/registry/testing/FakeClock.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static org.joda.time.Duration.millis;
2020

2121
import google.registry.util.Clock;
22+
import java.time.Instant;
2223
import java.util.concurrent.atomic.AtomicLong;
2324
import javax.annotation.concurrent.ThreadSafe;
2425
import org.joda.time.DateTime;
@@ -54,6 +55,11 @@ public DateTime nowUtc() {
5455
return new DateTime(currentTimeMillis.addAndGet(autoIncrementStepMs), UTC);
5556
}
5657

58+
@Override
59+
public Instant now() {
60+
return Instant.ofEpochMilli(currentTimeMillis.addAndGet(autoIncrementStepMs));
61+
}
62+
5763
/**
5864
* Sets the increment applied to the clock whenever it is queried. The increment is zero by
5965
* default: the clock is left unchanged when queried.

core/src/main/java/google/registry/batch/BulkDomainTransferAction.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8;
1818
import static google.registry.flows.FlowUtils.marshalWithLenientRetry;
1919
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
20+
import static google.registry.util.DateTimeUtils.END_INSTANT;
2021
import static jakarta.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
2122
import static jakarta.servlet.http.HttpServletResponse.SC_NO_CONTENT;
2223
import static jakarta.servlet.http.HttpServletResponse.SC_OK;
@@ -39,7 +40,6 @@
3940
import google.registry.request.Response;
4041
import google.registry.request.auth.Auth;
4142
import google.registry.request.lock.LockHandler;
42-
import google.registry.util.DateTimeUtils;
4343
import jakarta.inject.Inject;
4444
import jakarta.inject.Named;
4545
import java.util.Optional;
@@ -212,7 +212,7 @@ private void runTransferFlowInTransaction(String domainName) {
212212

213213
private boolean shouldSkipDomain(String domainName) {
214214
Optional<Domain> maybeDomain =
215-
ForeignKeyUtils.loadResource(Domain.class, domainName, tm().getTransactionTime());
215+
ForeignKeyUtils.loadResource(Domain.class, domainName, tm().getTxTime());
216216
if (maybeDomain.isEmpty()) {
217217
logger.atWarning().log("Domain '%s' was already deleted", domainName);
218218
missingDomains++;
@@ -232,7 +232,7 @@ private boolean shouldSkipDomain(String domainName) {
232232
return true;
233233
}
234234
if (domain.getStatusValues().contains(StatusValue.PENDING_DELETE)
235-
|| !domain.getDeletionTime().equals(DateTimeUtils.END_OF_TIME)) {
235+
|| !domain.getDeletionTime().equals(END_INSTANT)) {
236236
logger.atWarning().log("Domain '%s' is in PENDING_DELETE", domainName);
237237
pendingDelete++;
238238
return true;

core/src/main/java/google/registry/batch/DeleteExpiredDomainsAction.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8;
1919
import static google.registry.flows.FlowUtils.marshalWithLenientRetry;
2020
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
21+
import static google.registry.util.DateTimeUtils.END_INSTANT;
2122
import static google.registry.util.DateTimeUtils.END_OF_TIME;
2223
import static google.registry.util.ResourceUtils.readResourceUtf8;
2324
import static jakarta.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
@@ -177,7 +178,7 @@ private boolean runDomainDeleteFlow(Domain domain) {
177178
"Failed to delete domain %s because of its autorenew end time: %s.",
178179
transDomain.getDomainName(), transDomain.getAutorenewEndTime());
179180
return Optional.empty();
180-
} else if (domain.getDeletionTime().isBefore(END_OF_TIME)) {
181+
} else if (domain.getDeletionTime().isBefore(END_INSTANT)) {
181182
logger.atSevere().log(
182183
"Failed to delete domain %s because it was already deleted on %s.",
183184
transDomain.getDomainName(), transDomain.getDeletionTime());

core/src/main/java/google/registry/batch/RelockDomainAction.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ private void verifyDomainAndLockState(RegistryLock oldLock, Domain domain) {
188188
"Domain %s has a pending delete.",
189189
domainName);
190190
checkArgument(
191-
!DateTimeUtils.isAtOrAfter(tm().getTransactionTime(), domain.getDeletionTime()),
191+
!DateTimeUtils.isAtOrAfter(tm().getTxTime(), domain.getDeletionTime()),
192192
"Domain %s has been deleted.",
193193
domainName);
194194
checkArgument(

core/src/main/java/google/registry/beam/billing/ExpandBillingRecurrencesPipeline.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import static google.registry.util.DateTimeUtils.START_OF_TIME;
2525
import static google.registry.util.DateTimeUtils.earliestOf;
2626
import static google.registry.util.DateTimeUtils.latestOf;
27+
import static google.registry.util.DateTimeUtils.toInstant;
2728
import static org.apache.beam.sdk.values.TypeDescriptors.voids;
2829

2930
import com.google.common.collect.ImmutableMap;
@@ -372,7 +373,7 @@ private void expandOneRecurrence(
372373
// during ARGP).
373374
//
374375
// See: DomainFlowUtils#createCancellingRecords
375-
domain.getDeletionTime().isBefore(billingTime)
376+
domain.getDeletionTime().isBefore(toInstant(billingTime))
376377
? ImmutableSet.of()
377378
: ImmutableSet.of(
378379
DomainTransactionRecord.create(

core/src/main/java/google/registry/flows/ResourceFlowUtils.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import static com.google.common.collect.Sets.intersection;
1818
import static google.registry.model.EppResourceUtils.isLinked;
19+
import static google.registry.util.DateTimeUtils.toInstant;
1920

2021
import com.google.common.collect.ImmutableSet;
2122
import com.google.common.collect.Sets;
@@ -44,6 +45,7 @@
4445
import google.registry.model.host.Host;
4546
import google.registry.model.transfer.TransferStatus;
4647
import google.registry.persistence.VKey;
48+
import java.time.Instant;
4749
import java.util.List;
4850
import java.util.Objects;
4951
import java.util.Optional;
@@ -89,6 +91,11 @@ public static void verifyTransferInitiator(String registrarId, Domain domain)
8991

9092
public static <R extends EppResource & ForeignKeyedEppResource> R loadAndVerifyExistence(
9193
Class<R> clazz, String targetId, DateTime now) throws ResourceDoesNotExistException {
94+
return loadAndVerifyExistence(clazz, targetId, toInstant(now));
95+
}
96+
97+
public static <R extends EppResource & ForeignKeyedEppResource> R loadAndVerifyExistence(
98+
Class<R> clazz, String targetId, Instant now) throws ResourceDoesNotExistException {
9299
return verifyExistence(clazz, targetId, ForeignKeyUtils.loadResource(clazz, targetId, now));
93100
}
94101

@@ -197,8 +204,8 @@ public static void verifyAllStatusesAreClientSettable(Set<StatusValue> statusVal
197204
*
198205
* @param domain is the domain already projected at approvalTime
199206
*/
200-
public static DateTime computeExDateForApprovalTime(
201-
DomainBase domain, DateTime approvalTime, Period period) {
207+
public static Instant computeExDateForApprovalTime(
208+
DomainBase domain, Instant approvalTime, Period period) {
202209
boolean inAutoRenew = domain.getGracePeriodStatuses().contains(GracePeriodStatus.AUTO_RENEW);
203210
// inAutoRenew is set to false if the period is zero because a zero-period transfer should not
204211
// subsume an autorenew.

core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ public EppResponse run() throws EppException {
236236
}
237237

238238
// Cancel any grace periods that were still active, and set the expiration time accordingly.
239-
DateTime newExpirationTime = existingDomain.getRegistrationExpirationTime();
239+
DateTime newExpirationTime = existingDomain.getRegistrationExpirationDateTime();
240240
for (GracePeriod gracePeriod : existingDomain.getGracePeriods()) {
241241
// No cancellation is written if the grace period was not for a billable event.
242242
if (gracePeriod.hasBillingEvent()) {
@@ -289,7 +289,7 @@ public EppResponse run() throws EppException {
289289
flowCustomLogic.beforeResponse(
290290
BeforeResponseParameters.newBuilder()
291291
.setResultCode(
292-
newDomain.getDeletionTime().isAfter(now)
292+
newDomain.getDeletionDateTime().isAfter(now)
293293
? SUCCESS_WITH_ACTION_PENDING
294294
: SUCCESS)
295295
.setResponseExtensions(

0 commit comments

Comments
 (0)