Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
8 changes: 5 additions & 3 deletions GEMINI.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ This document outlines foundational mandates, architectural patterns, and projec
- **Utility/Cache Methods:** Use `tm().reTransact(...)` for utility methods or Caffeine cache loaders that might be invoked from both transactional and non-transactional paths.
- `reTransact` will join an existing transaction if one is present (acting as a no-op) or start a new one if not.
- This is particularly useful for in-memory caches where the loader must be able to fetch data regardless of whether the caller is currently in a transaction.
- **Transactional Time:** Ensure code that relies on `tm().getTransactionTime()` is executed within a transaction context.
- **Test Helpers & Timestamps:** If a static test helper method (like in `DatabaseHelper`) needs the database transaction time but might be called from outside a transaction, using `tm().reTransact(tm()::getTxTime)` is acceptable. However, NEVER wrap it redundantly like `tm().transact(() -> tm().reTransact(tm()::getTxTime))`. If you are just setting an arbitrary timestamp in a test where the exact DB transaction time isn't strictly required, prefer `Instant.now()` or `clock.now()` to avoid creating unnecessary database transactions.
- **Production Code:** In production code, if a flow fails because it is calling `getTxTime()` outside of a transaction, you must wrap the *caller* in a transaction instead of adding an unnecessary `reTransact()` around `getTxTime()`.
- **Transactional Time:** Ensure code that relies on `tm().getTransactionTime()` (or `tm().getTxTime()`) is executed within a transaction context.

### 5. Testing Best Practices
- **FakeClock and Sleeper:** Use `FakeClock` and `Sleeper` for any logic involving timeouts, delays, or expiration.
Expand Down Expand Up @@ -94,7 +96,7 @@ This document captures high-level architectural patterns, lessons learned from l
- **Dependency Injection:** Dagger 2 is used extensively. If you see "cannot find symbol" errors for classes starting with `Dagger...`, the project is in a state where annotation processing failed. Fix compilation in core models first to restore generated code.
- **Value Types:** AutoValue and "ImmutableObject" patterns are dominant. Most models follow a `Buildable` pattern with a nested `Builder`.
- **Temporal Logic:** The project is migrating from Joda-Time to `java.time`.
- Core boundaries: `DateTimeUtils.START_OF_TIME_INSTANT` (Unix Epoch) and `END_OF_TIME_INSTANT` (Long.MAX_VALUE / 1000).
- Core boundaries: `DateTimeUtils.START_INSTANT` (Unix Epoch) and `DateTimeUtils.END_INSTANT` (Long.MAX_VALUE / 1000).
- Year Arithmetic: Use `DateTimeUtils.plusYears()` and `DateTimeUtils.minusYears()` to handle February 29th logic correctly.

## Source Control
Expand Down Expand Up @@ -150,7 +152,7 @@ This project treats Error Prone warnings as errors.
- **Static Imports:** Methods like `toDateTime`, `toInstant`, `plusYears`, `plusMonths`, and `minusDays` from `DateTimeUtils` MUST be statically imported. Do NOT use them fully qualified (e.g., `DateTimeUtils.plusMonths(...)`).

- **Redundant Parses:** Never write `toDateTime(Instant.parse(...))` or `toInstant(DateTime.parse(...))`. If you need a `DateTime`, use `DateTime.parse(...)` directly. If you need an `Instant`, use `Instant.parse(...)` directly.
- **cloneProjectedAtTime vs cloneProjectedAtInstant:** When converting tests and logic that use `clock.now()` to project resource state into the future or past, do not wrap the Java `Instant` in `toDateTime()` just to call `cloneProjectedAtTime()`. Instead, switch the method call to use the native `cloneProjectedAtInstant()` method which is available on all `EppResource` models.
- **cloneProjectedAtTime vs cloneProjectedAtTime:** When converting tests and logic that use `clock.now()` to project resource state into the future or past, do not wrap the Java `Instant` in `toDateTime()` just to call `cloneProjectedAtTime()`. Instead, switch the method call to use the native `cloneProjectedAtTime()` method which is available on all `EppResource` models.
- **Do not go in circles with the build:** If you see an `InlineMeSuggester` error, apply the suppression to **ALL** similar methods in that file and related files in one turn. Do not fix them one by one. Furthermore, do not run a global `./gradlew build` when a scoped `./gradlew :core:build` or `./gradlew :core:test` is faster and more appropriate. Run global builds only when doing final verification.
- **Exception Conversion in Tests:** When migrating time types (e.g., from Joda `DateTime` to Java `Instant`), be extremely careful with tests that verify parsing failures (e.g., `assertThrows(IllegalArgumentException.class, ...)`). Joda's `DateTime.parse()` throws an `IllegalArgumentException` on failure, but `Instant.parse()` throws a `java.time.format.DateTimeParseException`. You must update the expected exception type in these tests to ensure they actually test the correct behavior, and verify the tests are not failing prematurely on the first line if it contains invalid data meant to be ignored.
- Dagger/AutoValue corruption: If you modify a builder or a component incorrectly, Dagger will fail to generate code, leading to hundreds of "cannot find symbol" errors. If this happens, `git checkout` the last working state of the specific file and re-apply changes more surgically.
Expand Down
5 changes: 0 additions & 5 deletions common/src/main/java/google/registry/util/Clock.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import javax.annotation.concurrent.ThreadSafe;
import org.joda.time.DateTime;

/**
* A clock that tells the current time in milliseconds or nanoseconds.
Expand All @@ -32,10 +31,6 @@
@ThreadSafe
public interface Clock extends Serializable {

/** Returns current time in UTC timezone. */
@Deprecated
DateTime nowUtc();

/** Returns current Instant (which is always in UTC). */
Instant now();

Expand Down
117 changes: 15 additions & 102 deletions common/src/main/java/google/registry/util/DateTimeUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Ordering;
import java.sql.Date;
import java.time.Instant;
import java.time.LocalDate;
import java.time.ZoneOffset;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeFormatterBuilder;
Expand All @@ -31,27 +31,13 @@
import java.time.temporal.ChronoUnit;
import javax.annotation.Nullable;
import org.joda.time.DateTime;
import org.joda.time.LocalDate;
import org.joda.time.ReadableDuration;

/** Utilities methods and constants related to Joda {@link DateTime} objects. */
public abstract class DateTimeUtils {

/** The start of the epoch, in a convenient constant. */
@Deprecated public static final DateTime START_OF_TIME = new DateTime(0, UTC);

/** The start of the UNIX epoch (which is defined in UTC), in a convenient constant. */
public static final Instant START_INSTANT = Instant.ofEpochMilli(0);

/**
* A date in the far future that we can treat as infinity.
*
* <p>This value is (2^63-1)/1000 rounded down. Postgres can store dates as 64 bit microseconds,
* but Java uses milliseconds, so this is the largest representable date that will survive a
* round-trip through the database.
*/
@Deprecated public static final DateTime END_OF_TIME = new DateTime(Long.MAX_VALUE / 1000, UTC);

/**
* An instant in the far future that we can treat as infinity.
*
Expand All @@ -71,7 +57,7 @@ public abstract class DateTimeUtils {
*/
private static final DateTimeFormatter ISO_8601_FORMATTER =
new DateTimeFormatterBuilder()
.appendValue(ChronoField.YEAR, 4, 10, SignStyle.NORMAL)
.appendValue(ChronoField.YEAR, 4, 10, SignStyle.NOT_NEGATIVE)
.appendPattern("-MM-dd'T'HH:mm:ss.SSS'Z'")
.toFormatter()
.withZone(ZoneOffset.UTC);
Expand Down Expand Up @@ -102,55 +88,28 @@ public static Instant parseInstant(String timestamp) {
}
}

/** Returns the earliest of a number of given {@link DateTime} instances. */
public static DateTime earliestOf(DateTime first, DateTime... rest) {
return earliestDateTimeOf(Lists.asList(first, rest));
}

/** Returns the earliest of a number of given {@link Instant} instances. */
public static Instant earliestOf(Instant first, Instant... rest) {
return earliestOf(Lists.asList(first, rest));
}

/** Returns the earliest element in a {@link DateTime} iterable. */
public static DateTime earliestDateTimeOf(Iterable<DateTime> dates) {
checkArgument(!Iterables.isEmpty(dates));
return Ordering.<DateTime>natural().min(dates);
}

/** Returns the earliest element in a {@link Instant} iterable. */
public static Instant earliestOf(Iterable<Instant> instants) {
checkArgument(!Iterables.isEmpty(instants));
return Ordering.<Instant>natural().min(instants);
}

/** Returns the latest of a number of given {@link DateTime} instances. */
public static DateTime latestOf(DateTime first, DateTime... rest) {
return latestDateTimeOf(Lists.asList(first, rest));
}

/** Returns the latest of a number of given {@link Instant} instances. */
public static Instant latestOf(Instant first, Instant... rest) {
return latestOf(Lists.asList(first, rest));
}

/** Returns the latest element in a {@link DateTime} iterable. */
public static DateTime latestDateTimeOf(Iterable<DateTime> dates) {
checkArgument(!Iterables.isEmpty(dates));
return Ordering.<DateTime>natural().max(dates);
}

/** Returns the latest element in a {@link Instant} iterable. */
public static Instant latestOf(Iterable<Instant> instants) {
checkArgument(!Iterables.isEmpty(instants));
return Ordering.<Instant>natural().max(instants);
}

/** Returns whether the first {@link DateTime} is equal to or earlier than the second. */
public static boolean isBeforeOrAt(DateTime timeToCheck, DateTime timeToCompareTo) {
return !timeToCheck.isAfter(timeToCompareTo);
}

/** Converts a Joda-Time Duration to a java.time.Duration. */
@Nullable
public static java.time.Duration toJavaDuration(@Nullable ReadableDuration duration) {
Expand All @@ -168,25 +127,11 @@ public static boolean isBeforeOrAt(Instant timeToCheck, Instant timeToCompareTo)
return !timeToCheck.isAfter(timeToCompareTo);
}

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

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

/**
* 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.
*/
public static DateTime plusYears(DateTime now, int years) {
checkArgument(years >= 0);
return years == 0 ? now : now.plusYears(1).plusYears(years - 1);
}

/**
* Adds years to a date, in the {@code Duration} sense of semantic years. Use this instead of
* {@link java.time.ZonedDateTime#plusYears} to ensure that we never end up on February 29.
Expand All @@ -210,15 +155,6 @@ public static Instant minusMonths(Instant now, int months) {
return now.atZone(ZoneOffset.UTC).minusMonths(months).toInstant();
}

/**
* Subtracts years from a date, in the {@code Duration} sense of semantic years. Use this instead
* of {@link DateTime#minusYears} to ensure that we never end up on February 29.
*/
public static DateTime minusYears(DateTime now, int years) {
checkArgument(years >= 0);
return years == 0 ? now : now.minusYears(1).minusYears(years - 1);
}

/**
* Subtracts years from a date, in the {@code Duration} sense of semantic years. Use this instead
* of {@link java.time.ZonedDateTime#minusYears} to ensure that we never end up on February 29.
Expand All @@ -230,40 +166,9 @@ public static Instant minusYears(Instant now, long years) {
: now.atZone(ZoneOffset.UTC).minusYears(1).minusYears(years - 1).toInstant();
}

/**
* @deprecated Use {@link #plusYears(DateTime, int)}
*/
@Deprecated
@SuppressWarnings("InlineMeSuggester")
public static DateTime leapSafeAddYears(DateTime now, int years) {
return plusYears(now, years);
}

/**
* @deprecated Use {@link #minusYears(DateTime, int)}
*/
@Deprecated
@SuppressWarnings("InlineMeSuggester")
public static DateTime leapSafeSubtractYears(DateTime now, int years) {
return minusYears(now, years);
}

public static Date toSqlDate(LocalDate localDate) {
return new Date(localDate.toDateTimeAtStartOfDay().getMillis());
}

public static LocalDate toLocalDate(Date date) {
return new LocalDate(date.getTime(), UTC);
}

/** Converts a java.time.LocalDate to a Joda-Time LocalDate. */
public static LocalDate toJodaLocalDate(java.time.LocalDate localDate) {
return new LocalDate(localDate.getYear(), localDate.getMonthValue(), localDate.getDayOfMonth());
}

/** Converts an Instant to a Joda-Time LocalDate in UTC. */
public static LocalDate toJodaLocalDate(Instant instant) {
return new LocalDate(instant.toEpochMilli(), UTC);
/** Converts an Instant to a java.time.LocalDate in UTC. */
public static LocalDate toLocalDate(Instant instant) {
return instant.atZone(ZoneOffset.UTC).toLocalDate();
}

/** Convert a joda {@link DateTime} to a java.time {@link Instant}, null-safe. */
Expand Down Expand Up @@ -300,11 +205,19 @@ public static Instant minusMinutes(Instant instant, long minutes) {
return instant.minus(minutes, ChronoUnit.MINUTES);
}

public static Instant plusDays(Instant instant, int days) {
public static Instant plusWeeks(Instant instant, int weeks) {
return instant.atZone(ZoneOffset.UTC).plusWeeks(weeks).toInstant();
}

public static Instant minusWeeks(Instant instant, int weeks) {
return instant.atZone(ZoneOffset.UTC).minusWeeks(weeks).toInstant();
}

public static Instant plusDays(Instant instant, long days) {
return instant.atZone(ZoneOffset.UTC).plusDays(days).toInstant();
}

public static Instant minusDays(Instant instant, int days) {
public static Instant minusDays(Instant instant, long days) {
return instant.atZone(ZoneOffset.UTC).minusDays(days).toInstant();
}
}
42 changes: 2 additions & 40 deletions common/src/main/java/google/registry/util/Sleeper.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import java.time.Duration;
import javax.annotation.concurrent.ThreadSafe;
import org.joda.time.ReadableDuration;

/**
* An object which accepts requests to put the current thread to sleep.
Expand All @@ -31,26 +30,7 @@ public interface Sleeper {
*
* @throws InterruptedException if this thread was interrupted
*/
void sleep(ReadableDuration duration) throws InterruptedException;

/**
* Puts the current thread to sleep.
*
* @throws InterruptedException if this thread was interrupted
*/
default void sleep(Duration duration) throws InterruptedException {
sleep(DateTimeUtils.toJodaDuration(duration));
}

/**
* Puts the current thread to sleep, ignoring interrupts.
*
* <p>If {@link InterruptedException} was caught, then {@code Thread.currentThread().interrupt()}
* will be called at the end of the {@code duration}.
*
* @see com.google.common.util.concurrent.Uninterruptibles#sleepUninterruptibly
*/
void sleepUninterruptibly(ReadableDuration duration);
void sleep(Duration duration) throws InterruptedException;

/**
* Puts the current thread to sleep, ignoring interrupts.
Expand All @@ -60,25 +40,7 @@ default void sleep(Duration duration) throws InterruptedException {
*
* @see com.google.common.util.concurrent.Uninterruptibles#sleepUninterruptibly
*/
default void sleepUninterruptibly(Duration duration) {
sleepUninterruptibly(DateTimeUtils.toJodaDuration(duration));
}

/**
* Puts the current thread to interruptible sleep.
*
* <p>This is a convenience method for {@link #sleep} that properly converts an {@link
* InterruptedException} to a {@link RuntimeException}.
*/
default void sleepInterruptibly(ReadableDuration duration) {
try {
sleep(duration);
} catch (InterruptedException e) {
// Restore current thread's interrupted state.
Thread.currentThread().interrupt();
throw new RuntimeException("Interrupted.", e);
}
}
void sleepUninterruptibly(Duration duration);

/**
* Puts the current thread to interruptible sleep.
Expand Down
8 changes: 0 additions & 8 deletions common/src/main/java/google/registry/util/SystemClock.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,10 @@
package google.registry.util;

import static java.time.temporal.ChronoUnit.MILLIS;
import static org.joda.time.DateTimeZone.UTC;

import jakarta.inject.Inject;
import java.time.Instant;
import javax.annotation.concurrent.ThreadSafe;
import org.joda.time.DateTime;

/** Clock implementation that proxies to the real system clock. */
@ThreadSafe
Expand All @@ -31,12 +29,6 @@ public class SystemClock implements Clock {
@Inject
public SystemClock() {}

/** Returns the current time. */
@Override
public DateTime nowUtc() {
return DateTime.now(UTC);
}

@Override
public Instant now() {
// Truncate to milliseconds to match the precision of Joda DateTime and our database schema
Expand Down
15 changes: 7 additions & 8 deletions common/src/main/java/google/registry/util/SystemSleeper.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,12 @@
package google.registry.util;

import static com.google.common.base.Preconditions.checkArgument;
import static google.registry.util.DateTimeUtils.toJavaDuration;

import com.google.common.util.concurrent.Uninterruptibles;
import jakarta.inject.Inject;
import java.io.Serializable;
import java.time.Duration;
import javax.annotation.concurrent.ThreadSafe;
import org.joda.time.ReadableDuration;

/** Implementation of {@link Sleeper} for production use. */
@ThreadSafe
Expand All @@ -33,14 +32,14 @@ public final class SystemSleeper implements Sleeper, Serializable {
public SystemSleeper() {}

@Override
public void sleep(ReadableDuration duration) throws InterruptedException {
checkArgument(duration.getMillis() >= 0);
Thread.sleep(duration.getMillis());
public void sleep(Duration duration) throws InterruptedException {
checkArgument(!duration.isNegative(), "Duration must be non-negative");
Thread.sleep(duration.toMillis());
}

@Override
public void sleepUninterruptibly(ReadableDuration duration) {
checkArgument(duration.getMillis() >= 0);
Uninterruptibles.sleepUninterruptibly(toJavaDuration(duration));
public void sleepUninterruptibly(Duration duration) {
checkArgument(!duration.isNegative(), "Duration must be non-negative");
Uninterruptibles.sleepUninterruptibly(duration);
}
}
Loading
Loading