From 49f14b5e1b65d043475d57916dfce6a50431b626 Mon Sep 17 00:00:00 2001 From: Ben McIlwain Date: Fri, 3 Apr 2026 16:38:26 -0400 Subject: [PATCH] Set clock precision to milliseconds for Datetime->Instant migration (#2999) Our existing precision is milliseconds so we want to stick with that for Instants. If we want to increase the precision globally after that we can do so all in one go post-migration, but for now, it would be a bad thing to have mixed precision going on just depending on whether a class happens to be migrated yet or not. This PR also migrates all existing DateTime.nowUtc() calls to use the Clock interface, so that when they are migrated they will get the benefit of this precision-setting as well. BUG= http://b/496985355 --- GEMINI.md | 52 +++++++ .../main/java/google/registry/util/Clock.java | 7 + .../google/registry/util/DateTimeUtils.java | 6 + .../google/registry/util/SystemClock.java | 6 +- .../batch/DeleteProberDataAction.java | 12 +- ...ingCertificateNotificationEmailAction.java | 9 +- .../beam/spec11/SafeBrowsingTransforms.java | 16 +- .../registry/beam/spec11/Spec11Pipeline.java | 6 +- .../registry/bigquery/BigqueryConnection.java | 24 ++- .../model/tld/label/PremiumListDao.java | 43 +++--- .../model/tld/label/PremiumListUtils.java | 6 +- .../model/tld/label/ReservedList.java | 9 +- .../tools/CreateAnchorTenantCommand.java | 4 +- .../registry/tools/CreateDomainCommand.java | 4 +- .../CreateOrUpdatePremiumListCommand.java | 8 +- .../tools/CreateOrUpdateRegistrarCommand.java | 6 +- .../CreateOrUpdateReservedListCommand.java | 4 + .../tools/CreateReservedListCommand.java | 3 +- .../google/registry/tools/EppToolCommand.java | 4 + .../tools/GenerateDnsReportCommand.java | 3 +- .../tools/GenerateZoneFilesCommand.java | 15 +- .../tools/GetHistoryEntriesCommand.java | 7 +- .../registry/tools/RenewDomainCommand.java | 5 - .../tools/UniformRapidSuspensionCommand.java | 4 - .../registry/tools/UpdateDomainCommand.java | 4 - .../tools/UpdatePremiumListCommand.java | 3 +- .../console/ConsoleDumDownloadAction.java | 4 +- .../batch/DeleteProberDataActionTest.java | 11 +- ...ertificateNotificationEmailActionTest.java | 3 +- .../spec11/SafeBrowsingTransformsTest.java | 5 +- .../beam/spec11/Spec11PipelineTest.java | 3 +- .../export/ExportPremiumTermsActionTest.java | 3 +- .../domain/DomainTransferApproveFlowTest.java | 29 ++-- .../domain/DomainTransferRequestFlowTest.java | 28 ++-- .../model/tld/label/PremiumListDaoTest.java | 140 ++++++++++-------- .../model/tld/label/PremiumListTest.java | 4 +- .../model/tld/label/PremiumListUtilsTest.java | 11 +- .../icann/IcannReportingStagerTest.java | 5 +- .../registry/testing/DatabaseHelper.java | 2 +- .../tools/AckPollMessagesCommandTest.java | 10 +- .../tools/ConfigureTldCommandTest.java | 2 +- .../tools/CreateAnchorTenantCommandTest.java | 1 + .../tools/CreateDomainCommandTest.java | 1 + ...ateOrUpdatePremiumListCommandTestCase.java | 1 + ...teOrUpdateReservedListCommandTestCase.java | 1 + .../tools/CreatePremiumListCommandTest.java | 6 - .../tools/CreateRegistrarCommandTest.java | 1 + .../tools/CreateReservedListCommandTest.java | 3 +- .../tools/EppToolCommandTestCase.java | 1 + .../tools/GenerateDnsReportCommandTest.java | 14 +- .../registry/tools/GetDomainCommandTest.java | 2 +- .../tools/GetHistoryEntriesCommandTest.java | 65 ++++---- .../registry/tools/GetHostCommandTest.java | 2 +- .../registry/tools/SetupOteCommandTest.java | 4 +- .../tools/UpdatePremiumListCommandTest.java | 13 +- .../tools/UpdateRegistrarCommandTest.java | 1 + .../tools/UpdateReservedListCommandTest.java | 2 - .../ValidateLoginCredentialsCommandTest.java | 1 + .../module/CertificateSupplierModule.java | 5 +- .../handler/SslClientInitializerTest.java | 15 +- .../handler/SslServerInitializerTest.java | 30 ++-- .../module/CertificateSupplierModuleTest.java | 9 +- .../monitoring/blackbox/token/EppToken.java | 27 ++-- .../blackbox/token/EppTokenTest.java | 6 +- .../registry/proxy/EppProtocolModuleTest.java | 13 +- .../proxy/handler/EppServiceHandlerTest.java | 5 +- .../google/registry/util/PosixTarHeader.java | 5 +- .../util/SelfSignedCaCertificate.java | 14 +- .../registry/util/PosixTarHeaderTest.java | 8 +- 69 files changed, 466 insertions(+), 320 deletions(-) create mode 100644 GEMINI.md diff --git a/GEMINI.md b/GEMINI.md new file mode 100644 index 000000000..6a4488ab8 --- /dev/null +++ b/GEMINI.md @@ -0,0 +1,52 @@ +# Engineering Standards for Gemini CLI + +This document outlines foundational mandates, architectural patterns, and project-specific conventions to ensure high-quality, idiomatic, and consistent code from the first iteration. + +## Core Mandates + +### 1. Rigorous Import Management +- **Addition:** When adding new symbols, ensure the corresponding import is added. +- **Removal:** When removing the last usage of a class or symbol from a file (e.g., removing a `@Inject Clock clock;` field), **immediately remove the associated import**. Do not wait for a build failure to identify unused imports. +- **Checkstyle:** Proactively fix common checkstyle errors (line length > 100, formatting, unused imports) during the initial code write. Do not wait for CI/build failures to address these, as iterative fixes are inefficient. +- **Verification:** Before finalizing any change, scan the imports section for redundancy. + +### 2. Time and Precision Handling (java.time Migration) +- **Millisecond Precision:** Always truncate `Instant.now()` to milliseconds (using `.truncatedTo(ChronoUnit.MILLIS)`) to maintain consistency with Joda `DateTime` and the PostgreSQL schema (which enforces millisecond precision via JPA converters). +- **Clock Injection:** + - Avoid direct calls to `Instant.now()`, `DateTime.now()`, `ZonedDateTime.now()`, or `System.currentTimeMillis()`. + - Inject `google.registry.util.Clock` (production) or `google.registry.testing.FakeClock` (tests). + - Use `clock.nowDate()` to get a `ZonedDateTime` in UTC. +- **Beam Pipelines:** + - Ensure `Clock` is serializable (it is by default in this project) when used in Beam `DoFn`s. + - Pass the `Clock` through the constructor or via Dagger provider methods in the pipeline module. +- **Command-Line Tools:** + - Use `@Inject Clock clock;` in `Command` implementations. + - The `clock` field should be **package-private** (no access modifier) to allow manual initialization in corresponding test classes. + - In test classes (e.g., `UpdateDomainCommandTest`), manually set `command.clock = fakeClock;` in the `@BeforeEach` method. + - Base test classes like `EppToolCommandTestCase` should handle this assignment for their generic command types where applicable. + +### 3. Dependency Injection (Dagger) +- **Concrete Types:** Dagger `inject` methods must use explicit concrete types. Generic `inject(Command)` methods will not work. +- **Test Components:** Use `TestRegistryToolComponent` for command-line tool tests to bridge the gap between `main` and `nonprod/test` source sets. + +### 4. Database Consistency +- **JPA Converters:** Be aware that JPA converters (like `DateTimeConverter`) may perform truncation or transformation. Ensure application-level logic matches these transformations to avoid "dirty" state or unexpected diffs. +- **Transaction Management:** + - **Top-Level:** Define database transactions (`tm().transact(...)`) at the highest possible level in the call chain (e.g., in an Action, a Command, or a Flow). This ensures all operations are atomic and handled by the retry logic. + - **DAO Methods:** Avoid declaring transactions inside low-level DAO methods. Use `tm().assertInTransaction()` to ensure that these methods are only called within a valid transactional context. + - **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. + +### 5. Testing Best Practices +- **FakeClock and Sleeper:** Use `FakeClock` and `Sleeper` for any logic involving timeouts, delays, or expiration. +- **Empirical Reproduction:** Before fixing a bug, always create a test case that reproduces the failure. +- **Base Classes:** Leverage `CommandTestCase`, `EppToolCommandTestCase`, etc., to reduce boilerplate and ensure consistent setup (e.g., clock initialization). + +### 6. Project Dependencies +- **Common Module:** When using `Clock` or other core utilities in a new or separate module (like `load-testing`), ensure `implementation project(':common')` is added to the module's `build.gradle`. + +## Performance and Efficiency +- **Turn Minimization:** Aim for "perfect" code in the first iteration. Iterative fixes for checkstyle or compilation errors consume significant context and time. +- **Context Management:** Use sub-agents for batch refactoring or high-volume output tasks to keep the main session history lean and efficient. diff --git a/common/src/main/java/google/registry/util/Clock.java b/common/src/main/java/google/registry/util/Clock.java index 9943aaa7c..ba6e8c4b3 100644 --- a/common/src/main/java/google/registry/util/Clock.java +++ b/common/src/main/java/google/registry/util/Clock.java @@ -16,6 +16,8 @@ package google.registry.util; import java.io.Serializable; import java.time.Instant; +import java.time.ZoneOffset; +import java.time.ZonedDateTime; import javax.annotation.concurrent.ThreadSafe; import org.joda.time.DateTime; @@ -36,4 +38,9 @@ public interface Clock extends Serializable { /** Returns current Instant (which is always in UTC). */ Instant now(); + + /** Returns the current time as a {@link ZonedDateTime} in UTC. */ + default ZonedDateTime nowDate() { + return ZonedDateTime.ofInstant(now(), ZoneOffset.UTC); + } } diff --git a/common/src/main/java/google/registry/util/DateTimeUtils.java b/common/src/main/java/google/registry/util/DateTimeUtils.java index 1c7650eda..0272d137f 100644 --- a/common/src/main/java/google/registry/util/DateTimeUtils.java +++ b/common/src/main/java/google/registry/util/DateTimeUtils.java @@ -170,6 +170,12 @@ public abstract class DateTimeUtils { return (instant == null) ? null : new DateTime(instant.toEpochMilli(), DateTimeZone.UTC); } + /** Convert a java.time {@link java.time.Instant} to a joda {@link org.joda.time.Instant}. */ + @Nullable + public static org.joda.time.Instant toJodaInstant(@Nullable java.time.Instant instant) { + return (instant == null) ? null : org.joda.time.Instant.ofEpochMilli(instant.toEpochMilli()); + } + public static Instant plusYears(Instant instant, int years) { return instant.atZone(ZoneOffset.UTC).plusYears(years).toInstant(); } diff --git a/common/src/main/java/google/registry/util/SystemClock.java b/common/src/main/java/google/registry/util/SystemClock.java index 52ac9893f..7e4e7551c 100644 --- a/common/src/main/java/google/registry/util/SystemClock.java +++ b/common/src/main/java/google/registry/util/SystemClock.java @@ -38,6 +38,10 @@ public class SystemClock implements Clock { @Override public Instant now() { - return Instant.now(); + // Truncate to milliseconds to match the precision of Joda DateTime and our database schema + // (which uses millisecond precision via DateTimeConverter). This prevents subtle comparison + // bugs where a high-precision Instant would be considered "after" a truncated database + // timestamp. + return Instant.now().truncatedTo(java.time.temporal.ChronoUnit.MILLIS); } } diff --git a/core/src/main/java/google/registry/batch/DeleteProberDataAction.java b/core/src/main/java/google/registry/batch/DeleteProberDataAction.java index c34718b48..f442f308a 100644 --- a/core/src/main/java/google/registry/batch/DeleteProberDataAction.java +++ b/core/src/main/java/google/registry/batch/DeleteProberDataAction.java @@ -28,7 +28,6 @@ import static google.registry.request.RequestParameters.PARAM_BATCH_SIZE; import static google.registry.request.RequestParameters.PARAM_DRY_RUN; import static google.registry.request.RequestParameters.PARAM_TLDS; import static google.registry.util.RegistryEnvironment.PRODUCTION; -import static org.joda.time.DateTimeZone.UTC; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; @@ -44,6 +43,7 @@ import google.registry.model.tld.Tld.TldType; import google.registry.request.Action; import google.registry.request.Parameter; import google.registry.request.auth.Auth; +import google.registry.util.Clock; import google.registry.util.RegistryEnvironment; import jakarta.inject.Inject; import jakarta.persistence.TypedQuery; @@ -111,16 +111,20 @@ public class DeleteProberDataAction implements Runnable { String registryAdminRegistrarId; + private final Clock clock; + @Inject DeleteProberDataAction( @Parameter(PARAM_DRY_RUN) boolean isDryRun, @Parameter(PARAM_TLDS) ImmutableSet tlds, @Parameter(PARAM_BATCH_SIZE) Optional batchSize, - @Config("registryAdminClientId") String registryAdminRegistrarId) { + @Config("registryAdminClientId") String registryAdminRegistrarId, + Clock clock) { this.isDryRun = isDryRun; this.tlds = tlds; this.batchSize = batchSize.orElse(DEFAULT_BATCH_SIZE); this.registryAdminRegistrarId = registryAdminRegistrarId; + this.clock = clock; } @Override @@ -145,7 +149,7 @@ public class DeleteProberDataAction implements Runnable { AtomicInteger softDeletedDomains = new AtomicInteger(); AtomicInteger hardDeletedDomains = new AtomicInteger(); AtomicReference> domainsBatch = new AtomicReference<>(); - DateTime startTime = DateTime.now(UTC); + DateTime startTime = clock.nowUtc(); do { tm().transact( TRANSACTION_REPEATABLE_READ, @@ -164,7 +168,7 @@ public class DeleteProberDataAction implements Runnable { hardDeletedDomains.get(), batchSize); // Automatically kill the job if it is running for over 20 hours - } while (DateTime.now(UTC).isBefore(startTime.plusHours(20)) + } while (clock.nowUtc().isBefore(startTime.plusHours(20)) && domainsBatch.get().size() == batchSize); logger.atInfo().log( "%s %d domains.", diff --git a/core/src/main/java/google/registry/batch/SendExpiringCertificateNotificationEmailAction.java b/core/src/main/java/google/registry/batch/SendExpiringCertificateNotificationEmailAction.java index 4ee917951..02cd0dc6f 100644 --- a/core/src/main/java/google/registry/batch/SendExpiringCertificateNotificationEmailAction.java +++ b/core/src/main/java/google/registry/batch/SendExpiringCertificateNotificationEmailAction.java @@ -19,7 +19,6 @@ import static google.registry.persistence.transaction.TransactionManagerFactory. import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import static org.apache.http.HttpStatus.SC_INTERNAL_SERVER_ERROR; import static org.apache.http.HttpStatus.SC_OK; -import static org.joda.time.DateTimeZone.UTC; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; @@ -37,6 +36,7 @@ import google.registry.model.registrar.RegistrarPoc.Type; import google.registry.request.Action; import google.registry.request.Response; import google.registry.request.auth.Auth; +import google.registry.util.Clock; import google.registry.util.EmailMessage; import jakarta.inject.Inject; import jakarta.mail.internet.AddressException; @@ -73,6 +73,7 @@ public class SendExpiringCertificateNotificationEmailAction implements Runnable private final GmailClient gmailClient; private final String expirationWarningEmailSubjectText; private final Response response; + private final Clock clock; @Inject public SendExpiringCertificateNotificationEmailAction( @@ -80,12 +81,14 @@ public class SendExpiringCertificateNotificationEmailAction implements Runnable @Config("expirationWarningEmailSubjectText") String expirationWarningEmailSubjectText, GmailClient gmailClient, CertificateChecker certificateChecker, - Response response) { + Response response, + Clock clock) { this.certificateChecker = certificateChecker; this.expirationWarningEmailSubjectText = expirationWarningEmailSubjectText; this.gmailClient = gmailClient; this.expirationWarningEmailBodyText = expirationWarningEmailBodyText; this.response = response; + this.clock = clock; } @Override @@ -186,7 +189,7 @@ public class SendExpiringCertificateNotificationEmailAction implements Runnable */ updateLastNotificationSentDate( registrar, - DateTime.now(UTC).minusMinutes((int) UPDATE_TIME_OFFSET.getStandardMinutes()), + clock.nowUtc().minusMinutes((int) UPDATE_TIME_OFFSET.getStandardMinutes()), certificateType); return true; } catch (Exception e) { diff --git a/core/src/main/java/google/registry/beam/spec11/SafeBrowsingTransforms.java b/core/src/main/java/google/registry/beam/spec11/SafeBrowsingTransforms.java index f6d5ce460..b984af286 100644 --- a/core/src/main/java/google/registry/beam/spec11/SafeBrowsingTransforms.java +++ b/core/src/main/java/google/registry/beam/spec11/SafeBrowsingTransforms.java @@ -21,6 +21,8 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableSet; import com.google.common.flogger.FluentLogger; import com.google.common.io.CharStreams; +import google.registry.util.Clock; +import google.registry.util.DateTimeUtils; import google.registry.util.Retrier; import java.io.IOException; import java.io.InputStreamReader; @@ -40,7 +42,6 @@ import org.apache.http.entity.ContentType; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClients; import org.apache.http.protocol.HTTP; -import org.joda.time.Instant; import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; @@ -74,6 +75,8 @@ public class SafeBrowsingTransforms { /** Provides the SafeBrowsing API key at runtime. */ private final String apiKey; + private final Clock clock; + /** * Maps a domain name's {@code domainName} to its corresponding {@link DomainNameInfo} to * facilitate batching SafeBrowsing API requests. @@ -101,9 +104,10 @@ public class SafeBrowsingTransforms { * HttpClients#createDefault()}. */ @SuppressWarnings("unchecked") - EvaluateSafeBrowsingFn(String apiKey, Retrier retrier) { + EvaluateSafeBrowsingFn(String apiKey, Retrier retrier, Clock clock) { this.apiKey = apiKey; this.retrier = retrier; + this.clock = clock; closeableHttpClientSupplier = (Supplier & Serializable) HttpClients::createDefault; } @@ -115,9 +119,10 @@ public class SafeBrowsingTransforms { */ @VisibleForTesting EvaluateSafeBrowsingFn( - String apiKey, Retrier retrier, Supplier clientSupplier) { + String apiKey, Retrier retrier, Clock clock, Supplier clientSupplier) { this.apiKey = apiKey; this.retrier = retrier; + this.clock = clock; closeableHttpClientSupplier = clientSupplier; } @@ -126,7 +131,10 @@ public class SafeBrowsingTransforms { public void finishBundle(FinishBundleContext context) { if (!domainNameInfoBuffer.isEmpty()) { ImmutableSet> results = evaluateAndFlush(); - results.forEach((kv) -> context.output(kv, Instant.now(), GlobalWindow.INSTANCE)); + results.forEach( + (kv) -> + context.output( + kv, DateTimeUtils.toJodaInstant(clock.now()), GlobalWindow.INSTANCE)); } } diff --git a/core/src/main/java/google/registry/beam/spec11/Spec11Pipeline.java b/core/src/main/java/google/registry/beam/spec11/Spec11Pipeline.java index 8f0bc467d..8c304c2c3 100644 --- a/core/src/main/java/google/registry/beam/spec11/Spec11Pipeline.java +++ b/core/src/main/java/google/registry/beam/spec11/Spec11Pipeline.java @@ -30,6 +30,7 @@ import google.registry.model.reporting.Spec11ThreatMatch; import google.registry.model.reporting.Spec11ThreatMatch.ThreatType; import google.registry.persistence.PersistenceModule.TransactionIsolationLevel; import google.registry.persistence.VKey; +import google.registry.util.Clock; import google.registry.util.Retrier; import google.registry.util.UtilsModule; import jakarta.inject.Singleton; @@ -263,8 +264,9 @@ public class Spec11Pipeline implements Serializable { } @Provides - EvaluateSafeBrowsingFn provideSafeBrowsingFn(Spec11PipelineOptions options, Retrier retrier) { - return new EvaluateSafeBrowsingFn(options.getSafeBrowsingApiKey(), retrier); + EvaluateSafeBrowsingFn provideSafeBrowsingFn( + Spec11PipelineOptions options, Retrier retrier, Clock clock) { + return new EvaluateSafeBrowsingFn(options.getSafeBrowsingApiKey(), retrier, clock); } @Provides diff --git a/core/src/main/java/google/registry/bigquery/BigqueryConnection.java b/core/src/main/java/google/registry/bigquery/BigqueryConnection.java index 93bf6fae9..d001645f8 100644 --- a/core/src/main/java/google/registry/bigquery/BigqueryConnection.java +++ b/core/src/main/java/google/registry/bigquery/BigqueryConnection.java @@ -24,7 +24,6 @@ import static com.google.common.util.concurrent.Futures.transformAsync; import static com.google.common.util.concurrent.MoreExecutors.directExecutor; import static google.registry.bigquery.BigqueryUtils.toJobReferenceString; import static google.registry.config.RegistryConfig.getProjectId; -import static org.joda.time.DateTimeZone.UTC; import com.google.api.client.googleapis.json.GoogleJsonResponseException; import com.google.api.client.http.AbstractInputStreamContent; @@ -58,6 +57,7 @@ import com.google.common.util.concurrent.MoreExecutors; import google.registry.bigquery.BigqueryUtils.DestinationFormat; import google.registry.bigquery.BigqueryUtils.TableType; import google.registry.bigquery.BigqueryUtils.WriteDisposition; +import google.registry.util.Clock; import google.registry.util.NonFinalForTesting; import google.registry.util.Sleeper; import google.registry.util.SqlTemplate; @@ -69,7 +69,6 @@ import java.util.List; import java.util.Random; import java.util.concurrent.ExecutorService; import javax.annotation.Nullable; -import org.joda.time.DateTime; import org.joda.time.Duration; /** Class encapsulating parameters and state for accessing the Bigquery API. */ @@ -94,6 +93,9 @@ public class BigqueryConnection implements AutoCloseable { /** Bigquery client instance wrapped by this class. */ private final Bigquery bigquery; + /** Clock instance for this connection. */ + private final Clock clock; + /** Executor service for bigquery jobs. */ private ListeningExecutorService service; @@ -109,8 +111,9 @@ public class BigqueryConnection implements AutoCloseable { /** Duration to wait between polls for job status. */ private Duration pollInterval = Duration.millis(1000); - BigqueryConnection(Bigquery bigquery) { + BigqueryConnection(Bigquery bigquery, Clock clock) { this.bigquery = bigquery; + this.clock = clock; } /** Builder for a {@link BigqueryConnection}, since the latter is immutable once created. */ @@ -118,8 +121,8 @@ public class BigqueryConnection implements AutoCloseable { private BigqueryConnection instance; @Inject - Builder(Bigquery bigquery) { - instance = new BigqueryConnection(bigquery); + Builder(Bigquery bigquery, Clock clock) { + instance = new BigqueryConnection(bigquery, clock); } /** @@ -195,6 +198,11 @@ public class BigqueryConnection implements AutoCloseable { private final TableReference tableRef = new TableReference(); private TableType type = TableType.TABLE; private WriteDisposition writeDisposition = WriteDisposition.WRITE_EMPTY; + private final Clock clock; + + public Builder(Clock clock) { + this.clock = clock; + } public Builder datasetId(String datasetId) { tableRef.setDatasetId(datasetId); @@ -217,7 +225,7 @@ public class BigqueryConnection implements AutoCloseable { } public Builder timeToLive(Duration duration) { - this.table.setExpirationTime(DateTime.now(UTC).plus(duration).getMillis()); + this.table.setExpirationTime(clock.nowUtc().plus(duration).getMillis()); return this; } @@ -302,7 +310,7 @@ public class BigqueryConnection implements AutoCloseable { /** Returns a partially built DestinationTable with the default dataset and overwrite behavior. */ public DestinationTable.Builder buildDestinationTable(String tableName) { - return new DestinationTable.Builder() + return new DestinationTable.Builder(clock) .datasetId(datasetId) .type(TableType.TABLE) .name(tableName) @@ -314,7 +322,7 @@ public class BigqueryConnection implements AutoCloseable { * temporary table dataset, with the default TTL and overwrite behavior. */ public DestinationTable.Builder buildTemporaryTable() { - return new DestinationTable.Builder() + return new DestinationTable.Builder(clock) .datasetId(TEMP_DATASET_NAME) .type(TableType.TABLE) .name(getRandomTableName()) diff --git a/core/src/main/java/google/registry/model/tld/label/PremiumListDao.java b/core/src/main/java/google/registry/model/tld/label/PremiumListDao.java index 531eb8336..7a5fc608f 100644 --- a/core/src/main/java/google/registry/model/tld/label/PremiumListDao.java +++ b/core/src/main/java/google/registry/model/tld/label/PremiumListDao.java @@ -129,34 +129,31 @@ public final class PremiumListDao { public static PremiumList save(String name, CurrencyUnit currencyUnit, List inputData) { checkArgument(!inputData.isEmpty(), "New premium list data cannot be empty"); - return save(PremiumListUtils.parseToPremiumList(name, currencyUnit, inputData)); + tm().assertInTransaction(); + return save( + PremiumListUtils.parseToPremiumList( + name, currencyUnit, inputData, tm().getTransactionTime())); } /** Saves the given premium list (and its premium list entries) to Cloud SQL. */ public static PremiumList save(PremiumList premiumListToPersist) { - PremiumList persisted = - tm().transact( - () -> { - // Make a new copy in each attempt to insert. See javadoc of the insert method for - // more information. - PremiumList premiumList = premiumListToPersist.asBuilder().build(); - tm().insert(premiumList); - tm().getEntityManager().flush(); // This populates the revisionId. - long revisionId = premiumList.getRevisionId(); + tm().assertInTransaction(); + // Make a new copy in each attempt to insert. See javadoc of the insert method for + // more information. + PremiumList premiumList = premiumListToPersist.asBuilder().build(); + tm().insert(premiumList); + tm().getEntityManager().flush(); // This populates the revisionId. + long revisionId = premiumList.getRevisionId(); - if (!isNullOrEmpty(premiumList.getLabelsToPrices())) { - ImmutableSet.Builder entries = new ImmutableSet.Builder<>(); - premiumList - .getLabelsToPrices() - .forEach( - (key, value) -> - entries.add(PremiumEntry.create(revisionId, value, key))); - tm().insertAll(entries.build()); - } - return premiumList; - }); - premiumListCache.invalidate(persisted.getName()); - return persisted; + if (!isNullOrEmpty(premiumList.getLabelsToPrices())) { + ImmutableSet.Builder entries = new ImmutableSet.Builder<>(); + premiumList + .getLabelsToPrices() + .forEach((key, value) -> entries.add(PremiumEntry.create(revisionId, value, key))); + tm().insertAll(entries.build()); + } + premiumListCache.invalidate(premiumList.getName()); + return premiumList; } public static void delete(PremiumList premiumList) { diff --git a/core/src/main/java/google/registry/model/tld/label/PremiumListUtils.java b/core/src/main/java/google/registry/model/tld/label/PremiumListUtils.java index d25f9d61d..463be206d 100644 --- a/core/src/main/java/google/registry/model/tld/label/PremiumListUtils.java +++ b/core/src/main/java/google/registry/model/tld/label/PremiumListUtils.java @@ -14,8 +14,6 @@ package google.registry.model.tld.label; -import static org.joda.time.DateTimeZone.UTC; - import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import google.registry.model.tld.label.PremiumList.PremiumEntry; @@ -29,12 +27,12 @@ import org.joda.time.DateTime; public class PremiumListUtils { public static PremiumList parseToPremiumList( - String name, CurrencyUnit currencyUnit, List inputData) { + String name, CurrencyUnit currencyUnit, List inputData, DateTime creationTime) { PremiumList partialPremiumList = new PremiumList.Builder() .setName(name) .setCurrency(currencyUnit) - .setCreationTimestamp(DateTime.now(UTC)) + .setCreationTimestamp(creationTime) .build(); ImmutableMap prices = partialPremiumList.parse(inputData); Map priceAmounts = Maps.transformValues(prices, PremiumEntry::getValue); diff --git a/core/src/main/java/google/registry/model/tld/label/ReservedList.java b/core/src/main/java/google/registry/model/tld/label/ReservedList.java index 402025430..0905a3995 100644 --- a/core/src/main/java/google/registry/model/tld/label/ReservedList.java +++ b/core/src/main/java/google/registry/model/tld/label/ReservedList.java @@ -24,10 +24,10 @@ import static google.registry.model.tld.label.ReservationType.FULLY_BLOCKED; import static google.registry.persistence.transaction.QueryComposer.Comparator.EQ; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.util.CollectionUtils.nullToEmpty; -import static org.joda.time.DateTimeZone.UTC; import com.github.benmanes.caffeine.cache.LoadingCache; import com.google.common.base.Splitter; +import com.google.common.base.Stopwatch; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.UncheckedExecutionException; @@ -47,7 +47,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; import javax.annotation.Nullable; -import org.joda.time.DateTime; /** * A list of reserved domain labels that are blocked from being registered for various reasons. @@ -234,7 +233,7 @@ public final class ReservedList */ private static ImmutableSet getReservedListEntries( String label, String tldStr) { - DateTime startTime = DateTime.now(UTC); + Stopwatch stopwatch = Stopwatch.createStarted(); Tld tld = Tld.get(checkNotNull(tldStr, "tld must not be null")); ImmutableSet.Builder entriesBuilder = new ImmutableSet.Builder<>(); ImmutableSet.Builder metricMatchesBuilder = @@ -251,9 +250,7 @@ public final class ReservedList } ImmutableSet entries = entriesBuilder.build(); DomainLabelMetrics.recordReservedListCheckOutcome( - tldStr, - metricMatchesBuilder.build(), - DateTime.now(UTC).getMillis() - startTime.getMillis()); + tldStr, metricMatchesBuilder.build(), stopwatch.elapsed().toMillis()); return entries; } diff --git a/core/src/main/java/google/registry/tools/CreateAnchorTenantCommand.java b/core/src/main/java/google/registry/tools/CreateAnchorTenantCommand.java index 4e392cb16..4e38b534f 100644 --- a/core/src/main/java/google/registry/tools/CreateAnchorTenantCommand.java +++ b/core/src/main/java/google/registry/tools/CreateAnchorTenantCommand.java @@ -19,7 +19,6 @@ import static com.google.common.base.Strings.isNullOrEmpty; import static google.registry.model.tld.Tlds.findTldForNameOrThrow; import static google.registry.pricing.PricingEngineProxy.getDomainCreateCost; import static google.registry.util.StringGenerator.DEFAULT_PASSWORD_LENGTH; -import static org.joda.time.DateTimeZone.UTC; import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; @@ -30,7 +29,6 @@ import google.registry.util.StringGenerator; import jakarta.inject.Inject; import jakarta.inject.Named; import org.joda.money.Money; -import org.joda.time.DateTime; /** A command to create a new anchor tenant domain. */ @Parameters(separators = " =", commandDescription = "Provision a domain for an anchor tenant.") @@ -86,7 +84,7 @@ final class CreateAnchorTenantCommand extends MutatingEppToolCommand { Money cost = null; if (fee) { - cost = getDomainCreateCost(domainName, DateTime.now(UTC), DEFAULT_ANCHOR_TENANT_PERIOD_YEARS); + cost = getDomainCreateCost(domainName, clock.nowUtc(), DEFAULT_ANCHOR_TENANT_PERIOD_YEARS); } setSoyTemplate(CreateAnchorTenantSoyInfo.getInstance(), diff --git a/core/src/main/java/google/registry/tools/CreateDomainCommand.java b/core/src/main/java/google/registry/tools/CreateDomainCommand.java index 101e50177..ae0fd42ab 100644 --- a/core/src/main/java/google/registry/tools/CreateDomainCommand.java +++ b/core/src/main/java/google/registry/tools/CreateDomainCommand.java @@ -17,7 +17,6 @@ package google.registry.tools; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Strings.isNullOrEmpty; import static google.registry.pricing.PricingEngineProxy.getPricesForDomainName; -import static org.joda.time.DateTimeZone.UTC; import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; @@ -28,7 +27,6 @@ import google.registry.util.StringGenerator; import jakarta.inject.Inject; import jakarta.inject.Named; import org.joda.money.Money; -import org.joda.time.DateTime; /** A command to create a new domain via EPP. */ @Parameters(separators = " =", commandDescription = "Create a new domain via EPP.") @@ -64,7 +62,7 @@ final class CreateDomainCommand extends CreateOrUpdateDomainCommand { for (String domain : domains) { String currency = null; String cost = null; - DomainPrices prices = getPricesForDomainName(domain, DateTime.now(UTC)); + DomainPrices prices = getPricesForDomainName(domain, clock.nowUtc()); // Check if the domain is premium and set the fee on the create command if so. if (prices.isPremium()) { diff --git a/core/src/main/java/google/registry/tools/CreateOrUpdatePremiumListCommand.java b/core/src/main/java/google/registry/tools/CreateOrUpdatePremiumListCommand.java index 13ef81552..170b92577 100644 --- a/core/src/main/java/google/registry/tools/CreateOrUpdatePremiumListCommand.java +++ b/core/src/main/java/google/registry/tools/CreateOrUpdatePremiumListCommand.java @@ -14,10 +14,14 @@ package google.registry.tools; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; + import com.beust.jcommander.Parameter; import com.google.common.flogger.FluentLogger; import google.registry.model.tld.label.PremiumListDao; import google.registry.tools.params.PathParameter; +import google.registry.util.Clock; +import jakarta.inject.Inject; import java.nio.file.Path; import java.util.List; import javax.annotation.Nullable; @@ -29,6 +33,8 @@ import org.joda.money.CurrencyUnit; */ abstract class CreateOrUpdatePremiumListCommand extends ConfirmingCommand { + @Inject Clock clock; + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); protected List inputData; protected CurrencyUnit currency; @@ -54,7 +60,7 @@ abstract class CreateOrUpdatePremiumListCommand extends ConfirmingCommand { String.format("Saved premium list %s with %d entries.", name, inputData.size()); try { logger.atInfo().log("Saving premium list for TLD %s.", name); - PremiumListDao.save(name, currency, inputData); + tm().transact(() -> PremiumListDao.save(name, currency, inputData)); logger.atInfo().log(message); } catch (Throwable e) { message = "Unexpected error saving premium list from nomulus tool command."; diff --git a/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java b/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java index 469cd2c02..f8a4f619d 100644 --- a/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java +++ b/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java @@ -21,7 +21,6 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.util.RegistrarUtils.normalizeRegistrarName; import static java.nio.charset.StandardCharsets.US_ASCII; -import static org.joda.time.DateTimeZone.UTC; import com.beust.jcommander.Parameter; import com.google.common.base.Ascii; @@ -37,6 +36,7 @@ import google.registry.tools.params.OptionalStringParameter; import google.registry.tools.params.PathParameter.InputFile; import google.registry.tools.params.StringListParameter; import google.registry.util.CidrAddressBlock; +import google.registry.util.Clock; import jakarta.inject.Inject; import java.nio.file.Files; import java.nio.file.Path; @@ -56,6 +56,8 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand { @Inject CertificateChecker certificateChecker; + @Inject Clock clock; + @Parameter(description = "Client identifier of the registrar account", required = true) List mainParameters; @@ -272,7 +274,7 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand { @Override protected final void init() throws Exception { initRegistrarCommand(); - DateTime now = DateTime.now(UTC); + DateTime now = clock.nowUtc(); for (String clientId : mainParameters) { Registrar oldRegistrar = getOldRegistrar(clientId); Registrar.Builder builder = diff --git a/core/src/main/java/google/registry/tools/CreateOrUpdateReservedListCommand.java b/core/src/main/java/google/registry/tools/CreateOrUpdateReservedListCommand.java index 6ede7fd6a..a8fe390f9 100644 --- a/core/src/main/java/google/registry/tools/CreateOrUpdateReservedListCommand.java +++ b/core/src/main/java/google/registry/tools/CreateOrUpdateReservedListCommand.java @@ -19,6 +19,8 @@ import com.google.common.flogger.FluentLogger; import google.registry.model.tld.label.ReservedList; import google.registry.model.tld.label.ReservedListDao; import google.registry.tools.params.PathParameter; +import google.registry.util.Clock; +import jakarta.inject.Inject; import java.nio.file.Path; import java.util.stream.Collectors; import javax.annotation.Nullable; @@ -29,6 +31,8 @@ import javax.annotation.Nullable; */ public abstract class CreateOrUpdateReservedListCommand extends ConfirmingCommand { + @Inject Clock clock; + static final FluentLogger logger = FluentLogger.forEnclosingClass(); @Nullable diff --git a/core/src/main/java/google/registry/tools/CreateReservedListCommand.java b/core/src/main/java/google/registry/tools/CreateReservedListCommand.java index 7cbe44c35..968a01409 100644 --- a/core/src/main/java/google/registry/tools/CreateReservedListCommand.java +++ b/core/src/main/java/google/registry/tools/CreateReservedListCommand.java @@ -18,7 +18,6 @@ import static com.google.common.base.Preconditions.checkArgument; import static google.registry.model.tld.Tlds.assertTldExists; import static google.registry.util.ListNamingUtils.convertFilePathToName; import static java.nio.charset.StandardCharsets.UTF_8; -import static org.joda.time.DateTimeZone.UTC; import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; @@ -51,7 +50,7 @@ final class CreateReservedListCommand extends CreateOrUpdateReservedListCommand if (!override) { validateListName(name); } - DateTime now = DateTime.now(UTC); + DateTime now = clock.nowUtc(); List allLines = Files.readAllLines(input, UTF_8); reservedList = new ReservedList.Builder() diff --git a/core/src/main/java/google/registry/tools/EppToolCommand.java b/core/src/main/java/google/registry/tools/EppToolCommand.java index fcc22130f..09d6665d1 100644 --- a/core/src/main/java/google/registry/tools/EppToolCommand.java +++ b/core/src/main/java/google/registry/tools/EppToolCommand.java @@ -38,6 +38,8 @@ import com.google.template.soy.data.SoyRecord; import com.google.template.soy.parseinfo.SoyFileInfo; import com.google.template.soy.parseinfo.SoyTemplateInfo; import google.registry.model.registrar.Registrar; +import google.registry.util.Clock; +import jakarta.inject.Inject; import java.io.IOException; import java.net.URLEncoder; import java.util.ArrayList; @@ -49,6 +51,8 @@ import java.util.Objects; /** A command to execute an epp command. */ abstract class EppToolCommand extends ConfirmingCommand implements CommandWithConnection { + @Inject Clock clock; + @Parameter( names = {"-u", "--superuser"}, description = "Run in superuser mode") diff --git a/core/src/main/java/google/registry/tools/GenerateDnsReportCommand.java b/core/src/main/java/google/registry/tools/GenerateDnsReportCommand.java index 88e0a3679..80508f14d 100644 --- a/core/src/main/java/google/registry/tools/GenerateDnsReportCommand.java +++ b/core/src/main/java/google/registry/tools/GenerateDnsReportCommand.java @@ -56,8 +56,7 @@ final class GenerateDnsReportCommand implements Command { validateWith = PathParameter.OutputFile.class) private Path output = Paths.get("/dev/stdout"); - @Inject - Clock clock; + @Inject Clock clock; @Override public void run() throws Exception { diff --git a/core/src/main/java/google/registry/tools/GenerateZoneFilesCommand.java b/core/src/main/java/google/registry/tools/GenerateZoneFilesCommand.java index b08ca56f4..c82fd402d 100644 --- a/core/src/main/java/google/registry/tools/GenerateZoneFilesCommand.java +++ b/core/src/main/java/google/registry/tools/GenerateZoneFilesCommand.java @@ -15,7 +15,6 @@ package google.registry.tools; import static google.registry.model.tld.Tlds.assertTldsExist; -import static org.joda.time.DateTimeZone.UTC; import static org.joda.time.Duration.standardMinutes; import com.beust.jcommander.Parameter; @@ -23,6 +22,8 @@ import com.beust.jcommander.Parameters; import com.google.common.collect.ImmutableMap; import google.registry.tools.params.DateParameter; import google.registry.tools.server.GenerateZoneFilesAction; +import google.registry.util.Clock; +import jakarta.inject.Inject; import java.io.IOException; import java.util.List; import java.util.Map; @@ -40,10 +41,13 @@ final class GenerateZoneFilesCommand implements CommandWithConnection { // Default to latest midnight that's at least 2 minutes ago. @Parameter( names = "--export_date", - description = "The date to generate the file for (defaults to today, or yesterday if run " - + "before 00:02).", + description = + "The date to generate the file for (defaults to today, or yesterday if run " + + "before 00:02).", validateWith = DateParameter.class) - private DateTime exportDate = DateTime.now(UTC).minus(standardMinutes(2)).withTimeAtStartOfDay(); + private DateTime exportDate; + + @Inject Clock clock; private ServiceConnection connection; @@ -54,6 +58,9 @@ final class GenerateZoneFilesCommand implements CommandWithConnection { @Override public void run() throws IOException { + if (exportDate == null) { + exportDate = clock.nowUtc().minus(standardMinutes(2)).withTimeAtStartOfDay(); + } assertTldsExist(mainParameters); ImmutableMap params = ImmutableMap.of( "tlds", mainParameters, diff --git a/core/src/main/java/google/registry/tools/GetHistoryEntriesCommand.java b/core/src/main/java/google/registry/tools/GetHistoryEntriesCommand.java index 394cbc702..9c2938d05 100644 --- a/core/src/main/java/google/registry/tools/GetHistoryEntriesCommand.java +++ b/core/src/main/java/google/registry/tools/GetHistoryEntriesCommand.java @@ -17,7 +17,6 @@ package google.registry.tools; import static com.google.common.base.Preconditions.checkArgument; import static google.registry.util.DateTimeUtils.END_OF_TIME; import static google.registry.util.DateTimeUtils.START_OF_TIME; -import static org.joda.time.DateTimeZone.UTC; import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; @@ -26,7 +25,9 @@ import google.registry.model.reporting.HistoryEntry; import google.registry.model.reporting.HistoryEntryDao; import google.registry.persistence.VKey; import google.registry.tools.CommandUtilities.ResourceType; +import google.registry.util.Clock; import google.registry.xml.XmlTransformer; +import jakarta.inject.Inject; import org.joda.time.DateTime; /** Command to show history entries. */ @@ -35,6 +36,8 @@ import org.joda.time.DateTime; commandDescription = "Show history entries that occurred in a given time range") final class GetHistoryEntriesCommand implements Command { + @Inject Clock clock; + @Parameter( names = {"-a", "--after"}, description = "Only show history entries that occurred at or after this time") @@ -58,7 +61,7 @@ final class GetHistoryEntriesCommand implements Command { checkArgument( type != null && uniqueId != null, "If either of 'type' or 'id' are set then both must be"); - VKey parentKey = type.getKey(uniqueId, DateTime.now(UTC)); + VKey parentKey = type.getKey(uniqueId, clock.nowUtc()); historyEntries = HistoryEntryDao.loadHistoryObjectsForResource(parentKey, after, before); } else { historyEntries = HistoryEntryDao.loadAllHistoryObjects(after, before); diff --git a/core/src/main/java/google/registry/tools/RenewDomainCommand.java b/core/src/main/java/google/registry/tools/RenewDomainCommand.java index 838db13ce..eab7a6621 100644 --- a/core/src/main/java/google/registry/tools/RenewDomainCommand.java +++ b/core/src/main/java/google/registry/tools/RenewDomainCommand.java @@ -26,8 +26,6 @@ import com.google.template.soy.data.SoyMapData; import google.registry.flows.ResourceFlowUtils; import google.registry.model.domain.Domain; import google.registry.tools.soy.DomainRenewSoyInfo; -import google.registry.util.Clock; -import jakarta.inject.Inject; import java.util.List; import org.joda.time.DateTime; import org.joda.time.format.DateTimeFormat; @@ -63,9 +61,6 @@ final class RenewDomainCommand extends MutatingEppToolCommand { arity = 1) Boolean requestedByRegistrar; - @Inject - Clock clock; - private static final DateTimeFormatter DATE_FORMATTER = DateTimeFormat.forPattern("YYYY-MM-dd"); @Override diff --git a/core/src/main/java/google/registry/tools/UniformRapidSuspensionCommand.java b/core/src/main/java/google/registry/tools/UniformRapidSuspensionCommand.java index 77290a852..099aef520 100644 --- a/core/src/main/java/google/registry/tools/UniformRapidSuspensionCommand.java +++ b/core/src/main/java/google/registry/tools/UniformRapidSuspensionCommand.java @@ -37,8 +37,6 @@ import google.registry.model.host.Host; import google.registry.tools.params.NameserversParameter; import google.registry.tools.soy.DomainRenewSoyInfo; import google.registry.tools.soy.UniformRapidSuspensionSoyInfo; -import google.registry.util.Clock; -import jakarta.inject.Inject; import jakarta.xml.bind.annotation.adapters.HexBinaryAdapter; import java.util.ArrayList; import java.util.HashSet; @@ -122,8 +120,6 @@ final class UniformRapidSuspensionCommand extends MutatingEppToolCommand { /** Set of status values to remove. */ ImmutableSet removeStatuses; - @Inject Clock clock; - @Override protected void initMutatingEppToolCommand() throws ResourceFlowUtils.ResourceDoesNotExistException { diff --git a/core/src/main/java/google/registry/tools/UpdateDomainCommand.java b/core/src/main/java/google/registry/tools/UpdateDomainCommand.java index e9474c8f7..90f0f8ca9 100644 --- a/core/src/main/java/google/registry/tools/UpdateDomainCommand.java +++ b/core/src/main/java/google/registry/tools/UpdateDomainCommand.java @@ -33,8 +33,6 @@ import google.registry.model.domain.GracePeriodBase; import google.registry.model.eppcommon.StatusValue; import google.registry.tools.params.NameserversParameter; import google.registry.tools.soy.DomainUpdateSoyInfo; -import google.registry.util.Clock; -import jakarta.inject.Inject; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -49,8 +47,6 @@ final class UpdateDomainCommand extends CreateOrUpdateDomainCommand { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - @Inject Clock clock; - @Parameter(names = "--statuses", description = "Comma-separated list of statuses to set.") private List statuses = new ArrayList<>(); diff --git a/core/src/main/java/google/registry/tools/UpdatePremiumListCommand.java b/core/src/main/java/google/registry/tools/UpdatePremiumListCommand.java index 4e8d7ed31..bee609b54 100644 --- a/core/src/main/java/google/registry/tools/UpdatePremiumListCommand.java +++ b/core/src/main/java/google/registry/tools/UpdatePremiumListCommand.java @@ -62,7 +62,8 @@ class UpdatePremiumListCommand extends CreateOrUpdatePremiumListCommand { inputData = Files.readAllLines(inputFile, UTF_8); checkArgument(!inputData.isEmpty(), "New premium list data cannot be empty"); currency = existingList.getCurrency(); - PremiumList updatedPremiumList = PremiumListUtils.parseToPremiumList(name, currency, inputData); + PremiumList updatedPremiumList = + PremiumListUtils.parseToPremiumList(name, currency, inputData, clock.nowUtc()); if (!existingList .getLabelsToPrices() .entrySet() diff --git a/core/src/main/java/google/registry/ui/server/console/ConsoleDumDownloadAction.java b/core/src/main/java/google/registry/ui/server/console/ConsoleDumDownloadAction.java index e056d116a..c609a7d02 100644 --- a/core/src/main/java/google/registry/ui/server/console/ConsoleDumDownloadAction.java +++ b/core/src/main/java/google/registry/ui/server/console/ConsoleDumDownloadAction.java @@ -16,7 +16,6 @@ package google.registry.ui.server.console; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.request.Action.Method.GET; -import static org.joda.time.DateTimeZone.UTC; import com.google.common.collect.ImmutableList; import com.google.common.flogger.FluentLogger; @@ -35,7 +34,6 @@ import jakarta.servlet.http.HttpServletResponse; import java.io.IOException; import org.apache.commons.csv.CSVFormat; import org.apache.commons.csv.CSVPrinter; -import org.joda.time.DateTime; @Action( service = Service.CONSOLE, @@ -86,7 +84,7 @@ public class ConsoleDumDownloadAction extends ConsoleApiAction { .setHeader("Cache-Control", "max-age=86400"); // 86400 seconds = 1 day consoleApiParams .response() - .setDateHeader("Expires", DateTime.now(UTC).withTimeAtStartOfDay().plusDays(1)); + .setDateHeader("Expires", clock.nowUtc().withTimeAtStartOfDay().plusDays(1)); try (var writer = consoleApiParams.response().getWriter()) { CSVPrinter csvPrinter = new CSVPrinter(writer, CSVFormat.DEFAULT); diff --git a/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java b/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java index 96c2d0c98..2621c99eb 100644 --- a/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java +++ b/core/src/test/java/google/registry/batch/DeleteProberDataActionTest.java @@ -45,6 +45,7 @@ import google.registry.model.tld.Tld.TldType; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; import google.registry.testing.DatabaseHelper; +import google.registry.testing.FakeClock; import google.registry.testing.SystemPropertyExtension; import google.registry.util.RegistryEnvironment; import java.time.Instant; @@ -62,9 +63,11 @@ class DeleteProberDataActionTest { private static final DateTime DELETION_TIME = DateTime.parse("2010-01-01T00:00:00.000Z"); + private final FakeClock clock = new FakeClock(DateTime.now(UTC)); + @RegisterExtension final JpaIntegrationTestExtension jpa = - new JpaTestExtensions.Builder().buildIntegrationTestExtension(); + new JpaTestExtensions.Builder().withClock(clock).buildIntegrationTestExtension(); @RegisterExtension final SystemPropertyExtension systemPropertyExtension = new SystemPropertyExtension(); @@ -94,7 +97,9 @@ class DeleteProberDataActionTest { } private void resetAction() { - action = new DeleteProberDataAction(false, ImmutableSet.of(), Optional.empty(), "TheRegistrar"); + action = + new DeleteProberDataAction( + false, ImmutableSet.of(), Optional.empty(), "TheRegistrar", clock); } @AfterEach @@ -124,7 +129,7 @@ class DeleteProberDataActionTest { Set oaEntities = persistLotsOfDomains("oa-canary.test"); // Create action with batch size of 3 DeleteProberDataAction batchedAction = - new DeleteProberDataAction(false, ImmutableSet.of(), Optional.of(3), "TheRegistrar"); + new DeleteProberDataAction(false, ImmutableSet.of(), Optional.of(3), "TheRegistrar", clock); batchedAction.run(); assertAllAbsent(ibEntities); assertAllAbsent(oaEntities); diff --git a/core/src/test/java/google/registry/batch/SendExpiringCertificateNotificationEmailActionTest.java b/core/src/test/java/google/registry/batch/SendExpiringCertificateNotificationEmailActionTest.java index d944a3e09..42ac0ff4a 100644 --- a/core/src/test/java/google/registry/batch/SendExpiringCertificateNotificationEmailActionTest.java +++ b/core/src/test/java/google/registry/batch/SendExpiringCertificateNotificationEmailActionTest.java @@ -97,7 +97,8 @@ class SendExpiringCertificateNotificationEmailActionTest { EXPIRATION_WARNING_EMAIL_SUBJECT_TEXT, sendEmailService, certificateChecker, - response); + response, + clock); sampleRegistrar = persistResource(createRegistrar("clientId", "sampleRegistrar", null, null).build()); diff --git a/core/src/test/java/google/registry/beam/spec11/SafeBrowsingTransformsTest.java b/core/src/test/java/google/registry/beam/spec11/SafeBrowsingTransformsTest.java index ac27c0d8d..54bbf6eb4 100644 --- a/core/src/test/java/google/registry/beam/spec11/SafeBrowsingTransformsTest.java +++ b/core/src/test/java/google/registry/beam/spec11/SafeBrowsingTransformsTest.java @@ -85,10 +85,13 @@ class SafeBrowsingTransformsTest { private final CloseableHttpClient mockHttpClient = mock(CloseableHttpClient.class, withSettings().serializable()); + private final FakeClock clock = new FakeClock(); + private final EvaluateSafeBrowsingFn safeBrowsingFn = new EvaluateSafeBrowsingFn( "API_KEY", - new Retrier(new FakeSleeper(new FakeClock()), 1), + new Retrier(new FakeSleeper(clock), 1), + clock, Suppliers.ofInstance(mockHttpClient)); @RegisterExtension diff --git a/core/src/test/java/google/registry/beam/spec11/Spec11PipelineTest.java b/core/src/test/java/google/registry/beam/spec11/Spec11PipelineTest.java index f4b6ba5b9..a25185ef0 100644 --- a/core/src/test/java/google/registry/beam/spec11/Spec11PipelineTest.java +++ b/core/src/test/java/google/registry/beam/spec11/Spec11PipelineTest.java @@ -187,7 +187,8 @@ class Spec11PipelineTest { EvaluateSafeBrowsingFn safeBrowsingFn = new EvaluateSafeBrowsingFn( SAFE_BROWSING_API_KEY, - new Retrier(new FakeSleeper(new FakeClock()), 1), + new Retrier(new FakeSleeper(fakeClock), 1), + fakeClock, Suppliers.ofInstance(mockHttpClient)); when(mockHttpClient.execute(any(HttpPost.class))).thenAnswer(new HttpResponder()); Spec11Pipeline spec11Pipeline = new Spec11Pipeline(options, safeBrowsingFn); diff --git a/core/src/test/java/google/registry/export/ExportPremiumTermsActionTest.java b/core/src/test/java/google/registry/export/ExportPremiumTermsActionTest.java index d064cf6d7..1b6c24705 100644 --- a/core/src/test/java/google/registry/export/ExportPremiumTermsActionTest.java +++ b/core/src/test/java/google/registry/export/ExportPremiumTermsActionTest.java @@ -17,6 +17,7 @@ package google.registry.export; import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8; import static com.google.common.truth.Truth.assertThat; import static google.registry.export.ExportPremiumTermsAction.EXPORT_MIME_TYPE; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.deleteTld; import static google.registry.testing.DatabaseHelper.persistResource; @@ -75,7 +76,7 @@ public class ExportPremiumTermsActionTest { @BeforeEach void beforeEach() throws Exception { createTld("tld"); - PremiumList pl = PremiumListDao.save("pl-name", USD, PREMIUM_NAMES); + PremiumList pl = tm().transact(() -> PremiumListDao.save("pl-name", USD, PREMIUM_NAMES)); persistResource( Tld.get("tld").asBuilder().setPremiumList(pl).setDriveFolderId("folder_id").build()); when(driveConnection.createOrUpdateFile( diff --git a/core/src/test/java/google/registry/flows/domain/DomainTransferApproveFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainTransferApproveFlowTest.java index 30a9339e3..9e2adb869 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainTransferApproveFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainTransferApproveFlowTest.java @@ -24,6 +24,7 @@ import static google.registry.model.reporting.DomainTransactionRecord.Transactio import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_CREATE; import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_TRANSFER_APPROVE; import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_TRANSFER_REQUEST; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.assertBillingEventsForResource; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.deleteTestDomain; @@ -512,12 +513,14 @@ class DomainTransferApproveFlowTest @Test void testSuccess_nonpremiumPriceRenewalBehavior_carriesOver() throws Exception { PremiumList pl = - PremiumListDao.save( - new PremiumList.Builder() - .setCurrency(USD) - .setName("tld") - .setLabelsToPrices(ImmutableMap.of("example", new BigDecimal("67.89"))) - .build()); + tm().transact( + () -> + PremiumListDao.save( + new PremiumList.Builder() + .setCurrency(USD) + .setName("tld") + .setLabelsToPrices(ImmutableMap.of("example", new BigDecimal("67.89"))) + .build())); persistResource(Tld.get("tld").asBuilder().setPremiumList(pl).build()); domain = loadByEntity(domain); persistResource( @@ -558,12 +561,14 @@ class DomainTransferApproveFlowTest @Test void testSuccess_specifiedPriceRenewalBehavior_carriesOver() throws Exception { PremiumList pl = - PremiumListDao.save( - new PremiumList.Builder() - .setCurrency(USD) - .setName("tld") - .setLabelsToPrices(ImmutableMap.of("example", new BigDecimal("67.89"))) - .build()); + tm().transact( + () -> + PremiumListDao.save( + new PremiumList.Builder() + .setCurrency(USD) + .setName("tld") + .setLabelsToPrices(ImmutableMap.of("example", new BigDecimal("67.89"))) + .build())); persistResource(Tld.get("tld").asBuilder().setPremiumList(pl).build()); domain = loadByEntity(domain); persistResource( diff --git a/core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java index 7afa97cd6..57eebb83a 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java @@ -1160,12 +1160,14 @@ class DomainTransferRequestFlowTest throws Exception { setupDomain("example", "tld"); PremiumList pl = - PremiumListDao.save( - new PremiumList.Builder() - .setCurrency(USD) - .setName("tld") - .setLabelsToPrices(ImmutableMap.of("example", new BigDecimal("67.89"))) - .build()); + tm().transact( + () -> + PremiumListDao.save( + new PremiumList.Builder() + .setCurrency(USD) + .setName("tld") + .setLabelsToPrices(ImmutableMap.of("example", new BigDecimal("67.89"))) + .build())); persistResource(Tld.get("tld").asBuilder().setPremiumList(pl).build()); domain = loadByEntity(domain); persistResource( @@ -1214,12 +1216,14 @@ class DomainTransferRequestFlowTest throws Exception { setupDomain("example", "tld"); PremiumList pl = - PremiumListDao.save( - new PremiumList.Builder() - .setCurrency(USD) - .setName("tld") - .setLabelsToPrices(ImmutableMap.of("example", new BigDecimal("67.89"))) - .build()); + tm().transact( + () -> + PremiumListDao.save( + new PremiumList.Builder() + .setCurrency(USD) + .setName("tld") + .setLabelsToPrices(ImmutableMap.of("example", new BigDecimal("67.89"))) + .build())); persistResource(Tld.get("tld").asBuilder().setPremiumList(pl).build()); domain = loadByEntity(domain); persistResource( diff --git a/core/src/test/java/google/registry/model/tld/label/PremiumListDaoTest.java b/core/src/test/java/google/registry/model/tld/label/PremiumListDaoTest.java index 0591f5e98..d7b4225b3 100644 --- a/core/src/test/java/google/registry/model/tld/label/PremiumListDaoTest.java +++ b/core/src/test/java/google/registry/model/tld/label/PremiumListDaoTest.java @@ -84,7 +84,7 @@ public class PremiumListDaoTest { @Test void saveNew_worksSuccessfully() { - PremiumListDao.save(testList); + tm().transact(() -> PremiumListDao.save(testList)); tm().transact( () -> { Optional persistedListOpt = PremiumListDao.getLatestRevision("testname"); @@ -118,24 +118,26 @@ public class PremiumListDaoTest { @Test void update_worksSuccessfully() { - PremiumListDao.save(testList); + tm().transact(() -> PremiumListDao.save(testList)); Optional persistedList = PremiumListDao.getLatestRevision("testname"); assertThat(persistedList).isPresent(); long firstRevisionId = persistedList.get().getRevisionId(); - PremiumListDao.save( - new PremiumList.Builder() - .setName("testname") - .setCurrency(USD) - .setLabelsToPrices( - ImmutableMap.of( - "save", - BigDecimal.valueOf(55343.12), - "new", - BigDecimal.valueOf(0.01), - "silver", - BigDecimal.valueOf(30.03))) - .setCreationTimestamp(fakeClock.nowUtc()) - .build()); + tm().transact( + () -> + PremiumListDao.save( + new PremiumList.Builder() + .setName("testname") + .setCurrency(USD) + .setLabelsToPrices( + ImmutableMap.of( + "save", + BigDecimal.valueOf(55343.12), + "new", + BigDecimal.valueOf(0.01), + "silver", + BigDecimal.valueOf(30.03))) + .setCreationTimestamp(fakeClock.nowUtc()) + .build())); tm().transact( () -> { Optional savedListOpt = PremiumListDao.getLatestRevision("testname"); @@ -158,7 +160,7 @@ public class PremiumListDaoTest { @Test void checkExists_worksSuccessfully() { assertThat(PremiumListDao.getLatestRevision("testname")).isEmpty(); - PremiumListDao.save(testList); + tm().transact(() -> PremiumListDao.save(testList)); assertThat(PremiumListDao.getLatestRevision("testname")).isPresent(); } @@ -169,20 +171,24 @@ public class PremiumListDaoTest { @Test void getLatestRevision_worksSuccessfully() { - PremiumListDao.save( - new PremiumList.Builder() - .setName("list1") - .setCurrency(USD) - .setLabelsToPrices(ImmutableMap.of("wrong", BigDecimal.valueOf(1000.50))) - .setCreationTimestamp(fakeClock.nowUtc()) - .build()); - PremiumListDao.save( - new PremiumList.Builder() - .setName("list1") - .setCurrency(USD) - .setLabelsToPrices(TEST_PRICES) - .setCreationTimestamp(fakeClock.nowUtc()) - .build()); + tm().transact( + () -> + PremiumListDao.save( + new PremiumList.Builder() + .setName("list1") + .setCurrency(USD) + .setLabelsToPrices(ImmutableMap.of("wrong", BigDecimal.valueOf(1000.50))) + .setCreationTimestamp(fakeClock.nowUtc()) + .build())); + tm().transact( + () -> + PremiumListDao.save( + new PremiumList.Builder() + .setName("list1") + .setCurrency(USD) + .setLabelsToPrices(TEST_PRICES) + .setCreationTimestamp(fakeClock.nowUtc()) + .build())); tm().transact( () -> { Optional persistedList = PremiumListDao.getLatestRevision("list1"); @@ -196,13 +202,15 @@ public class PremiumListDaoTest { @Test void getLabelsToPrices_worksForJpy() { - PremiumListDao.save( - new PremiumList.Builder() - .setName("list1") - .setCurrency(JPY) - .setLabelsToPrices(TEST_PRICES) - .setCreationTimestamp(fakeClock.nowUtc()) - .build()); + tm().transact( + () -> + PremiumListDao.save( + new PremiumList.Builder() + .setName("list1") + .setCurrency(JPY) + .setLabelsToPrices(TEST_PRICES) + .setCreationTimestamp(fakeClock.nowUtc()) + .build())); tm().transact( () -> { PremiumList premiumList = PremiumListDao.getLatestRevision("list1").get(); @@ -220,13 +228,15 @@ public class PremiumListDaoTest { @Test void getPremiumPrice_worksSuccessfully() { PremiumList premiumList = - PremiumListDao.save( - new PremiumList.Builder() - .setName("premlist") - .setCurrency(USD) - .setLabelsToPrices(TEST_PRICES) - .setCreationTimestamp(fakeClock.nowUtc()) - .build()); + tm().transact( + () -> + PremiumListDao.save( + new PremiumList.Builder() + .setName("premlist") + .setCurrency(USD) + .setLabelsToPrices(TEST_PRICES) + .setCreationTimestamp(fakeClock.nowUtc()) + .build())); persistResource(newTld("foobar", "FOOBAR").asBuilder().setPremiumList(premiumList).build()); assertThat(PremiumListDao.getPremiumPrice("premlist", "silver")).hasValue(Money.of(USD, 10.23)); assertThat(PremiumListDao.getPremiumPrice("premlist", "gold")).hasValue(Money.of(USD, 1305.47)); @@ -236,20 +246,22 @@ public class PremiumListDaoTest { @Test void testGetPremiumPrice_worksForJPY() { PremiumList premiumList = - PremiumListDao.save( - new PremiumList.Builder() - .setName("premlist") - .setCurrency(JPY) - .setLabelsToPrices( - ImmutableMap.of( - "silver", - BigDecimal.valueOf(10.00), - "gold", - BigDecimal.valueOf(1000.0), - "palladium", - BigDecimal.valueOf(15000))) - .setCreationTimestamp(fakeClock.nowUtc()) - .build()); + tm().transact( + () -> + PremiumListDao.save( + new PremiumList.Builder() + .setName("premlist") + .setCurrency(JPY) + .setLabelsToPrices( + ImmutableMap.of( + "silver", + BigDecimal.valueOf(10.00), + "gold", + BigDecimal.valueOf(1000.0), + "palladium", + BigDecimal.valueOf(15000))) + .setCreationTimestamp(fakeClock.nowUtc()) + .build())); persistResource(newTld("foobar", "FOOBAR").asBuilder().setPremiumList(premiumList).build()); assertThat(PremiumListDao.getPremiumPrice("premlist", "silver")).hasValue(moneyOf(JPY, 10)); assertThat(PremiumListDao.getPremiumPrice("premlist", "gold")).hasValue(moneyOf(JPY, 1000)); @@ -262,14 +274,18 @@ public class PremiumListDaoTest { IllegalArgumentException thrown = assertThrows( IllegalArgumentException.class, - () -> PremiumListDao.save("test-list", CurrencyUnit.GBP, ImmutableList.of())); + () -> + tm().transact( + () -> + PremiumListDao.save( + "test-list", CurrencyUnit.GBP, ImmutableList.of()))); assertThat(thrown).hasMessageThat().isEqualTo("New premium list data cannot be empty"); } @Test void test_savePremiumList_clearsCache() { assertThat(PremiumListDao.premiumListCache.getIfPresent("testname")).isNull(); - PremiumListDao.save(testList); + tm().transact(() -> PremiumListDao.save(testList)); PremiumList pl = PremiumListDao.getLatestRevision("testname").get(); assertThat(PremiumListDao.premiumListCache.getIfPresent("testname")).hasValue(pl); tm().transact(() -> PremiumListDao.save("testname", USD, ImmutableList.of("test,USD 1"))); @@ -288,7 +304,7 @@ public class PremiumListDaoTest { .setLabelsToPrices(prices) .setCreationTimestamp(fakeClock.nowUtc()) .build(); - PremiumListDao.save(list); + tm().transact(() -> PremiumListDao.save(list)); long duration = stopwatch.stop().elapsed(TimeUnit.MILLISECONDS); if (duration >= 6000) { // Don't fail directly since we can't rely on what sort of machines the test is running on diff --git a/core/src/test/java/google/registry/model/tld/label/PremiumListTest.java b/core/src/test/java/google/registry/model/tld/label/PremiumListTest.java index 538a85eab..cb627add4 100644 --- a/core/src/test/java/google/registry/model/tld/label/PremiumListTest.java +++ b/core/src/test/java/google/registry/model/tld/label/PremiumListTest.java @@ -16,6 +16,7 @@ package google.registry.model.tld.label; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.persistPremiumList; import static google.registry.testing.DatabaseHelper.persistReservedList; @@ -101,7 +102,8 @@ public class PremiumListTest { @Test void testParse_canIncludeOrNotIncludeCurrencyUnit() { - PremiumListDao.save("tld", USD, ImmutableList.of("rofl,USD 90", "paper, 80")); + tm().transact( + () -> PremiumListDao.save("tld", USD, ImmutableList.of("rofl,USD 90", "paper, 80"))); assertThat(PremiumListDao.getPremiumPrice("tld", "rofl")).hasValue(Money.of(USD, 90)); assertThat(PremiumListDao.getPremiumPrice("tld", "paper")).hasValue(Money.of(USD, 80)); } diff --git a/core/src/test/java/google/registry/model/tld/label/PremiumListUtilsTest.java b/core/src/test/java/google/registry/model/tld/label/PremiumListUtilsTest.java index 7abd8f5cb..c1d7b525d 100644 --- a/core/src/test/java/google/registry/model/tld/label/PremiumListUtilsTest.java +++ b/core/src/test/java/google/registry/model/tld/label/PremiumListUtilsTest.java @@ -21,16 +21,22 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.collect.ImmutableList; import java.math.BigDecimal; +import org.joda.time.DateTime; import org.junit.jupiter.api.Test; /** Unit tests for {@link PremiumListUtils}. */ class PremiumListUtilsTest { + private static final DateTime SAMPLE_TIME = DateTime.parse("2026-01-26T21:06:12.284Z"); + @Test void parseInputToPremiumList_works() { PremiumList premiumList = parseToPremiumList( - "testlist", USD, ImmutableList.of("foo,USD 99.50", "bar,USD 30", "baz,USD 10")); + "testlist", + USD, + ImmutableList.of("foo,USD 99.50", "bar,USD 30", "baz,USD 10"), + SAMPLE_TIME); assertThat(premiumList.getName()).isEqualTo("testlist"); assertThat(premiumList.getLabelsToPrices()) .containsExactly("foo", twoDigits(99.50), "bar", twoDigits(30), "baz", twoDigits(10)); @@ -45,7 +51,8 @@ class PremiumListUtilsTest { parseToPremiumList( "testlist", USD, - ImmutableList.of("foo,USD 99.50", "bar,USD 30", "baz,JPY 990"))); + ImmutableList.of("foo,USD 99.50", "bar,USD 30", "baz,JPY 990"), + SAMPLE_TIME)); assertThat(thrown).hasMessageThat().isEqualTo("The currency unit must be USD"); } diff --git a/core/src/test/java/google/registry/reporting/icann/IcannReportingStagerTest.java b/core/src/test/java/google/registry/reporting/icann/IcannReportingStagerTest.java index 77092e5e1..c7c875c97 100644 --- a/core/src/test/java/google/registry/reporting/icann/IcannReportingStagerTest.java +++ b/core/src/test/java/google/registry/reporting/icann/IcannReportingStagerTest.java @@ -31,15 +31,18 @@ import google.registry.bigquery.BigqueryConnection.DestinationTable; import google.registry.bigquery.BigqueryUtils.TableType; import google.registry.gcs.GcsUtils; import google.registry.reporting.icann.IcannReportingModule.ReportType; +import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; +import org.joda.time.DateTime; import org.joda.time.YearMonth; import org.junit.jupiter.api.Test; /** Unit tests for {@link google.registry.reporting.icann.IcannReportingStager}. */ class IcannReportingStagerTest { + private final FakeClock clock = new FakeClock(DateTime.parse("2026-01-26T21:06:12.284Z")); private BigqueryConnection bigquery = mock(BigqueryConnection.class); FakeResponse response = new FakeResponse(); private YearMonth yearMonth = new YearMonth(2017, 6); @@ -63,7 +66,7 @@ class IcannReportingStagerTest { when(bigquery.startQuery(any(String.class), any(DestinationTable.class))) .thenReturn(fakeFuture()); DestinationTable.Builder tableBuilder = - new DestinationTable.Builder() + new DestinationTable.Builder(clock) .datasetId("testdataset") .type(TableType.TABLE) .name("tablename") diff --git a/core/src/test/java/google/registry/testing/DatabaseHelper.java b/core/src/test/java/google/registry/testing/DatabaseHelper.java index a31841dd1..c78fb1558 100644 --- a/core/src/test/java/google/registry/testing/DatabaseHelper.java +++ b/core/src/test/java/google/registry/testing/DatabaseHelper.java @@ -346,7 +346,7 @@ public final class DatabaseHelper { // increasing sequence, if we don't pad out the ID here, we would have to renumber hundreds of // unit tests. tm().reTransact(tm()::allocateId); - PremiumListDao.save(premiumList); + tm().transact(() -> PremiumListDao.save(premiumList)); maybeAdvanceClock(); return premiumList; } diff --git a/core/src/test/java/google/registry/tools/AckPollMessagesCommandTest.java b/core/src/test/java/google/registry/tools/AckPollMessagesCommandTest.java index 01d6e4ada..2077a8220 100644 --- a/core/src/test/java/google/registry/tools/AckPollMessagesCommandTest.java +++ b/core/src/test/java/google/registry/tools/AckPollMessagesCommandTest.java @@ -30,7 +30,6 @@ import google.registry.model.poll.PollMessage.OneTime; import google.registry.model.reporting.HistoryEntry; import google.registry.persistence.VKey; import google.registry.testing.DatabaseHelper; -import google.registry.testing.FakeClock; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -38,13 +37,12 @@ import org.junit.jupiter.api.Test; /** Unit tests for {@link AckPollMessagesCommand}. */ public class AckPollMessagesCommandTest extends CommandTestCase { - private final FakeClock clock = new FakeClock(DateTime.parse("2015-02-04T08:16:32.064Z")); - private DomainHistory domainHistory; @BeforeEach final void beforeEach() { - command.clock = clock; + command.clock = fakeClock; + fakeClock.setTo(DateTime.parse("2015-02-04T08:16:32.064Z")); createTld("tld"); Domain domain = DatabaseHelper.newDomain("example.tld").asBuilder().setRepoId("FSDGS-TLD").build(); @@ -52,12 +50,12 @@ public class AckPollMessagesCommandTest extends CommandTestCase // since the old entity is always null and file cannot be empty, the prompt will NOT be "No entity // changes to apply." void commandPrompt_successStageNewEntity() throws Exception { - CreatePremiumListCommand command = new CreatePremiumListCommand(); command.inputFile = Paths.get(premiumTermsPath); command.currencyUnit = "USD"; command.prompt(); @@ -63,7 +62,6 @@ class CreatePremiumListCommandTest @Test void commandPrompt_successStageNewEntityWithOverride() throws Exception { - CreatePremiumListCommand command = new CreatePremiumListCommand(); String alterTld = "override"; command.inputFile = Paths.get(premiumTermsPath); command.override = true; @@ -75,7 +73,6 @@ class CreatePremiumListCommandTest @Test void commandPrompt_failureNoInputFile() { - CreatePremiumListCommand command = new CreatePremiumListCommand(); assertThrows(NullPointerException.class, command::prompt); } @@ -83,7 +80,6 @@ class CreatePremiumListCommandTest void commandPrompt_failurePremiumListAlreadyExists() { String randomStr = "random"; DatabaseHelper.createTld(randomStr); - CreatePremiumListCommand command = new CreatePremiumListCommand(); command.name = randomStr; command.currencyUnit = "USD"; IllegalArgumentException thrown = assertThrows(IllegalArgumentException.class, command::prompt); @@ -92,7 +88,6 @@ class CreatePremiumListCommandTest @Test void commandPrompt_failureMismatchedTldFileName_noOverride() throws Exception { - CreatePremiumListCommand command = new CreatePremiumListCommand(); String fileName = "random"; Path tmpPath = tmpDir.resolve(String.format("%s.txt", fileName)); Files.write(new byte[0], tmpPath.toFile()); @@ -111,7 +106,6 @@ class CreatePremiumListCommandTest @Test void commandPrompt_failureMismatchedTldName_noOverride() { - CreatePremiumListCommand command = new CreatePremiumListCommand(); String fileName = "random"; command.name = fileName; command.currencyUnit = "USD"; diff --git a/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java b/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java index 8b083181d..e6be401b4 100644 --- a/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java +++ b/core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java @@ -58,6 +58,7 @@ class CreateRegistrarCommandTest extends CommandTestCase @BeforeEach void beforeEach() { + command.clock = fakeClock; command.setConnection(connection); command.certificateChecker = new CertificateChecker( diff --git a/core/src/test/java/google/registry/tools/CreateReservedListCommandTest.java b/core/src/test/java/google/registry/tools/CreateReservedListCommandTest.java index 4575b0731..e9ffb09b4 100644 --- a/core/src/test/java/google/registry/tools/CreateReservedListCommandTest.java +++ b/core/src/test/java/google/registry/tools/CreateReservedListCommandTest.java @@ -36,6 +36,7 @@ class CreateReservedListCommandTest @BeforeEach void beforeEach() { + command.clock = fakeClock; createTlds("xn--q9jyb4c", "soy"); } @@ -162,7 +163,6 @@ class CreateReservedListCommandTest @Test void testStageEntityChange_succeeds() throws Exception { - CreateReservedListCommand command = new CreateReservedListCommand(); // file content is populated in @BeforeEach of CreateOrUpdateReservedListCommandTestCase.java command.input = Paths.get(reservedTermsPath); command.init(); @@ -176,7 +176,6 @@ class CreateReservedListCommandTest void testStageEntityChange_succeedsWithEmptyFile() throws Exception { Path tmpPath = tmpDir.resolve("xn--q9jyb4c_common-tmp.txt"); Files.write(new byte[0], tmpPath.toFile()); - CreateReservedListCommand command = new CreateReservedListCommand(); command.input = tmpPath; command.init(); assertThat(command.prompt()).contains("reservedListMap=[]"); diff --git a/core/src/test/java/google/registry/tools/EppToolCommandTestCase.java b/core/src/test/java/google/registry/tools/EppToolCommandTestCase.java index 8f1666f6f..d0102d99d 100644 --- a/core/src/test/java/google/registry/tools/EppToolCommandTestCase.java +++ b/core/src/test/java/google/registry/tools/EppToolCommandTestCase.java @@ -33,6 +33,7 @@ public abstract class EppToolCommandTestCase extends C @BeforeEach public void beforeEachEppToolCommandTestCase() { + command.clock = fakeClock; // Create two TLDs for commands that allow multiple TLDs at once. createTlds("tld", "tld2"); eppVerifier = EppToolVerifier.create(command).expectRegistrarId("NewRegistrar"); diff --git a/core/src/test/java/google/registry/tools/GenerateDnsReportCommandTest.java b/core/src/test/java/google/registry/tools/GenerateDnsReportCommandTest.java index cc0b6bf5b..50d6e844a 100644 --- a/core/src/test/java/google/registry/tools/GenerateDnsReportCommandTest.java +++ b/core/src/test/java/google/registry/tools/GenerateDnsReportCommandTest.java @@ -22,7 +22,6 @@ import static google.registry.testing.DatabaseHelper.persistActiveDomain; import static google.registry.testing.DatabaseHelper.persistActiveHost; import static google.registry.testing.DatabaseHelper.persistResource; import static java.nio.charset.StandardCharsets.UTF_8; -import static org.joda.time.DateTimeZone.UTC; import static org.junit.jupiter.api.Assertions.assertThrows; import com.beust.jcommander.ParameterException; @@ -36,13 +35,11 @@ import google.registry.model.domain.secdns.DomainDsData; import google.registry.model.eppcommon.StatusValue; import google.registry.model.host.Host; import google.registry.testing.DatabaseHelper; -import google.registry.testing.FakeClock; import java.io.IOException; import java.io.Reader; import java.nio.file.Files; import java.nio.file.Path; import java.util.List; -import org.joda.time.DateTime; import org.json.simple.JSONValue; import org.json.simple.parser.ParseException; import org.junit.jupiter.api.BeforeEach; @@ -51,8 +48,6 @@ import org.junit.jupiter.api.Test; /** Unit tests for {@link GenerateDnsReportCommand}. */ class GenerateDnsReportCommandTest extends CommandTestCase { - private final DateTime now = DateTime.now(UTC); - private final FakeClock clock = new FakeClock(); private Path output; private Object getOutputAsJson() throws IOException, ParseException { @@ -117,8 +112,7 @@ class GenerateDnsReportCommandTest extends CommandTestCase) getOutputAsJson()) .containsExactly(DOMAIN2_OUTPUT, NAMESERVER1_OUTPUT, NAMESERVER2_OUTPUT); @@ -178,7 +172,7 @@ class GenerateDnsReportCommandTest extends CommandTestCase output = (Iterable) getOutputAsJson(); assertThat(output).containsAnyOf(DOMAIN1_OUTPUT, DOMAIN1_OUTPUT_ALT); @@ -207,7 +201,7 @@ class GenerateDnsReportCommandTest extends CommandTestCase) getOutputAsJson()) diff --git a/core/src/test/java/google/registry/tools/GetDomainCommandTest.java b/core/src/test/java/google/registry/tools/GetDomainCommandTest.java index 61a1f30bc..291541787 100644 --- a/core/src/test/java/google/registry/tools/GetDomainCommandTest.java +++ b/core/src/test/java/google/registry/tools/GetDomainCommandTest.java @@ -30,8 +30,8 @@ class GetDomainCommandTest extends CommandTestCase { @BeforeEach void beforeEach() { - createTld("tld"); command.clock = fakeClock; + createTld("tld"); } @Test diff --git a/core/src/test/java/google/registry/tools/GetHistoryEntriesCommandTest.java b/core/src/test/java/google/registry/tools/GetHistoryEntriesCommandTest.java index 3fd5efe85..1c0a0d18a 100644 --- a/core/src/test/java/google/registry/tools/GetHistoryEntriesCommandTest.java +++ b/core/src/test/java/google/registry/tools/GetHistoryEntriesCommandTest.java @@ -22,7 +22,6 @@ import static google.registry.testing.FullFieldsTestEntityHelper.makeHistoryEntr import google.registry.model.domain.Domain; import google.registry.model.domain.Period; import google.registry.model.reporting.HistoryEntry; -import google.registry.testing.FakeClock; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -30,12 +29,12 @@ import org.junit.jupiter.api.Test; /** Unit tests for {@link GetClaimsListCommand}. */ class GetHistoryEntriesCommandTest extends CommandTestCase { - private final FakeClock clock = new FakeClock(DateTime.parse("2000-01-01T00:00:00Z")); - private Domain domain; @BeforeEach void beforeEach() { + fakeClock.setTo(DateTime.parse("2000-01-01T00:00:00Z")); + command.clock = fakeClock; createTld("tld"); domain = persistActiveDomain("example.tld"); } @@ -48,18 +47,18 @@ class GetHistoryEntriesCommandTest extends CommandTestCase - + Client: TheRegistrar + Time: 2000-01-01T00:00:00.000Z + Client TRID: ABC-123 + Server TRID: server-trid + + - """); + """); } @Test @@ -70,8 +69,8 @@ class GetHistoryEntriesCommandTest extends CommandTestCase - + Client: TheRegistrar + Time: 2000-01-01T00:00:00.000Z + Client TRID: ABC-123 + Server TRID: server-trid + + - """); + """); } @Test @@ -122,20 +121,20 @@ class GetHistoryEntriesCommandTest extends CommandTestCase - + Client: TheRegistrar + Time: 2000-01-01T00:00:00.000Z + Client TRID: null + Server TRID: null + + - """); + """); } } diff --git a/core/src/test/java/google/registry/tools/GetHostCommandTest.java b/core/src/test/java/google/registry/tools/GetHostCommandTest.java index 46aca5928..87a0a1baa 100644 --- a/core/src/test/java/google/registry/tools/GetHostCommandTest.java +++ b/core/src/test/java/google/registry/tools/GetHostCommandTest.java @@ -30,8 +30,8 @@ class GetHostCommandTest extends CommandTestCase { @BeforeEach void beforeEach() { - createTld("tld"); command.clock = fakeClock; + createTld("tld"); } @Test diff --git a/core/src/test/java/google/registry/tools/SetupOteCommandTest.java b/core/src/test/java/google/registry/tools/SetupOteCommandTest.java index 8383d0c9a..d52a85e20 100644 --- a/core/src/test/java/google/registry/tools/SetupOteCommandTest.java +++ b/core/src/test/java/google/registry/tools/SetupOteCommandTest.java @@ -47,7 +47,6 @@ import google.registry.model.tld.Tld.TldState; import google.registry.testing.CloudTasksHelper; import google.registry.testing.CloudTasksHelper.TaskMatcher; import google.registry.testing.DeterministicStringGenerator; -import google.registry.testing.FakeClock; import google.registry.util.CidrAddressBlock; import java.security.cert.CertificateParsingException; import java.util.Optional; @@ -70,8 +69,9 @@ class SetupOteCommandTest extends CommandTestCase { @BeforeEach void beforeEach() { + fakeClock.setTo(DateTime.parse("2018-07-07TZ")); + command.clock = fakeClock; command.passwordGenerator = passwordGenerator; - command.clock = new FakeClock(DateTime.parse("2018-07-07TZ")); command.maybeGroupEmailAddress = Optional.of("group@example.com"); command.cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils(); command.iamClient = iamClient; diff --git a/core/src/test/java/google/registry/tools/UpdatePremiumListCommandTest.java b/core/src/test/java/google/registry/tools/UpdatePremiumListCommandTest.java index b76ab2cda..c494ae2dd 100644 --- a/core/src/test/java/google/registry/tools/UpdatePremiumListCommandTest.java +++ b/core/src/test/java/google/registry/tools/UpdatePremiumListCommandTest.java @@ -41,6 +41,7 @@ class UpdatePremiumListCommandTest @BeforeEach void beforeEach() { + command.clock = fakeClock; registry = createTld(TLD_TEST, USD, initialPremiumListData); } @@ -60,7 +61,6 @@ class UpdatePremiumListCommandTest File tmpFile = tmpDir.resolve(String.format("%s.txt", TLD_TEST)).toFile(); String newPremiumListData = "omg,USD 1234"; Files.asCharSink(tmpFile, UTF_8).write(newPremiumListData); - UpdatePremiumListCommand command = new UpdatePremiumListCommand(); command.inputFile = Paths.get(tmpFile.getPath()); command.name = TLD_TEST; assertThat(command.prompt()).contains("Update premium list for prime?"); @@ -69,7 +69,6 @@ class UpdatePremiumListCommandTest @Test void commandPrompt_successStageNoChange() throws Exception { File tmpFile = tmpDir.resolve(String.format("%s.txt", TLD_TEST)).toFile(); - UpdatePremiumListCommand command = new UpdatePremiumListCommand(); command.inputFile = Paths.get(tmpFile.getPath()); command.name = TLD_TEST; assertThat(command.prompt()) @@ -82,7 +81,6 @@ class UpdatePremiumListCommandTest String newPremiumListData = "eth,USD 9999"; Files.asCharSink(tmpFile, UTF_8).write(newPremiumListData); - UpdatePremiumListCommand command = new UpdatePremiumListCommand(); // data come from @beforeEach of CreateOrUpdatePremiumListCommandTestCase.java command.inputFile = Paths.get(tmpFile.getPath()); runCommandForced("--name=" + TLD_TEST, "--input=" + command.inputFile); @@ -96,7 +94,6 @@ class UpdatePremiumListCommandTest void commandRun_successNoChange() throws Exception { File tmpFile = tmpDir.resolve(String.format("%s.txt", TLD_TEST)).toFile(); - UpdatePremiumListCommand command = new UpdatePremiumListCommand(); command.inputFile = Paths.get(tmpFile.getPath()); runCommandForced("--name=" + TLD_TEST, "--input=" + command.inputFile); @@ -113,7 +110,6 @@ class UpdatePremiumListCommandTest String newPremiumListData = "eth,USD 9999"; Files.asCharSink(newPremiumFile, UTF_8).write(newPremiumListData); - UpdatePremiumListCommand command = new UpdatePremiumListCommand(); // data come from @beforeEach of CreateOrUpdatePremiumListCommandTestCase.java command.inputFile = Paths.get(newPremiumFile.getPath()); runCommandForced("--name=" + TLD_TEST, "--input=" + command.inputFile); @@ -129,7 +125,6 @@ class UpdatePremiumListCommandTest String premiumTerms = "foo,USD 9000\ndoge,USD 100\nelon,USD 2021"; Files.asCharSink(tmpFile, UTF_8).write(premiumTerms); - UpdatePremiumListCommand command = new UpdatePremiumListCommand(); command.inputFile = Paths.get(tmpFile.getPath()); runCommandForced("--name=" + TLD_TEST, "--input=" + command.inputFile); @@ -146,7 +141,6 @@ class UpdatePremiumListCommandTest Path tmpPath = tmpDir.resolve(String.format("%s.txt", TLD_TEST)); Files.write(new byte[0], tmpPath.toFile()); - UpdatePremiumListCommand command = new UpdatePremiumListCommand(); command.inputFile = tmpPath; command.name = TLD_TEST; IllegalArgumentException thrown = assertThrows(IllegalArgumentException.class, command::prompt); @@ -156,7 +150,6 @@ class UpdatePremiumListCommandTest @Test void commandPrompt_failureNoPreviousVersion() { registry = createTld("random", null, null); - UpdatePremiumListCommand command = new UpdatePremiumListCommand(); command.name = "random"; IllegalArgumentException thrown = assertThrows(IllegalArgumentException.class, command::prompt); assertThat(thrown) @@ -166,13 +159,11 @@ class UpdatePremiumListCommandTest @Test void commandPrompt_failureNoInputFile() { - UpdatePremiumListCommand command = new UpdatePremiumListCommand(); assertThrows(NullPointerException.class, command::prompt); } @Test void commandPrompt_failureTldFromNameDoesNotExist() { - UpdatePremiumListCommand command = new UpdatePremiumListCommand(); command.name = "random2"; IllegalArgumentException thrown = assertThrows(IllegalArgumentException.class, command::prompt); assertThat(thrown) @@ -182,7 +173,6 @@ class UpdatePremiumListCommandTest @Test void commandPrompt_failureTldFromInputFileDoesNotExist() { - UpdatePremiumListCommand command = new UpdatePremiumListCommand(); // using tld extracted from file name but this tld is not part of the registry command.inputFile = Paths.get(tmpDir.resolve("random3.txt").toFile().getPath()); IllegalArgumentException thrown = assertThrows(IllegalArgumentException.class, command::prompt); @@ -197,7 +187,6 @@ class UpdatePremiumListCommandTest String newPremiumListData = "eth,USD 9999"; Files.asCharSink(tmpFile, UTF_8).write(newPremiumListData); - UpdatePremiumListCommand command = new UpdatePremiumListCommand(); command.inputFile = Paths.get(tmpFile.getPath()); runCommandForced("--name=" + TLD_TEST, "--input=" + command.inputFile, "--dry_run"); diff --git a/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java b/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java index d1233d73e..7b2291dd2 100644 --- a/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java +++ b/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java @@ -53,6 +53,7 @@ class UpdateRegistrarCommandTest extends CommandTestCase @BeforeEach void beforeEach() { + command.clock = fakeClock; command.certificateChecker = new CertificateChecker( ImmutableSortedMap.of(START_OF_TIME, 825, DateTime.parse("2020-09-01T00:00:00Z"), 398), diff --git a/core/src/test/java/google/registry/tools/UpdateReservedListCommandTest.java b/core/src/test/java/google/registry/tools/UpdateReservedListCommandTest.java index 52d3f5f12..8d7f4943e 100644 --- a/core/src/test/java/google/registry/tools/UpdateReservedListCommandTest.java +++ b/core/src/test/java/google/registry/tools/UpdateReservedListCommandTest.java @@ -98,7 +98,6 @@ class UpdateReservedListCommandTest Files.asCharSink(reservedTermsFile, UTF_8).write(reservedTermsCsv); reservedTermsPath = reservedTermsFile.getPath(); // create a command instance and assign its input - UpdateReservedListCommand command = new UpdateReservedListCommand(); command.input = Paths.get(reservedTermsPath); // run again with terms from example_reserved_terms.csv command.init(); @@ -110,7 +109,6 @@ class UpdateReservedListCommandTest void testSuccess_withChanges() throws Exception { // changes come from example_reserved_terms.csv, which are populated in @BeforeEach of // CreateOrUpdateReservedListCommandTestCases.java - UpdateReservedListCommand command = new UpdateReservedListCommand(); command.input = Paths.get(reservedTermsPath); command.init(); diff --git a/core/src/test/java/google/registry/tools/ValidateLoginCredentialsCommandTest.java b/core/src/test/java/google/registry/tools/ValidateLoginCredentialsCommandTest.java index a21e9dcf6..5ce83554a 100644 --- a/core/src/test/java/google/registry/tools/ValidateLoginCredentialsCommandTest.java +++ b/core/src/test/java/google/registry/tools/ValidateLoginCredentialsCommandTest.java @@ -50,6 +50,7 @@ class ValidateLoginCredentialsCommandTest extends CommandTestCase sslServerInitializer = new SslServerInitializer<>( true, @@ -169,13 +170,13 @@ class SslServerInitializerTest { @ParameterizedTest @MethodSource("provideTestCombinations") void testSuccess_trustAnyClientCert(SslProvider sslProvider) throws Exception { - SelfSignedCaCertificate serverSsc = SelfSignedCaCertificate.create(SSL_HOST); + SelfSignedCaCertificate serverSsc = SelfSignedCaCertificate.create(SSL_HOST, new FakeClock()); LocalAddress localAddress = new LocalAddress("TRUST_ANY_CLIENT_CERT_" + sslProvider); nettyExtension.setUpServer( localAddress, getServerHandler(true, false, sslProvider, serverSsc.key(), serverSsc.cert())); - SelfSignedCaCertificate clientSsc = SelfSignedCaCertificate.create(); + SelfSignedCaCertificate clientSsc = SelfSignedCaCertificate.create(new FakeClock()); nettyExtension.setUpClient( localAddress, getClientHandler(sslProvider, serverSsc.cert(), clientSsc.key(), clientSsc.cert())); @@ -193,7 +194,7 @@ class SslServerInitializerTest { @ParameterizedTest @MethodSource("provideTestCombinations") void testFailure_cipherNotAccepted(SslProvider sslProvider) throws Exception { - SelfSignedCaCertificate serverSsc = SelfSignedCaCertificate.create(SSL_HOST); + SelfSignedCaCertificate serverSsc = SelfSignedCaCertificate.create(SSL_HOST, new FakeClock()); LocalAddress localAddress = new LocalAddress("CIPHER_NOT_ACCEPTED_" + sslProvider); nettyExtension.setUpServer( @@ -220,7 +221,7 @@ class SslServerInitializerTest { @ParameterizedTest @MethodSource("provideTestCombinations") void testSuccess_someCiphersNotAccepted(SslProvider sslProvider) throws Exception { - SelfSignedCaCertificate serverSsc = SelfSignedCaCertificate.create(SSL_HOST); + SelfSignedCaCertificate serverSsc = SelfSignedCaCertificate.create(SSL_HOST, new FakeClock()); LocalAddress localAddress = new LocalAddress("SOME_CIPHERS_NOT_ACCEPTED_" + sslProvider); nettyExtension.setUpServer( @@ -258,7 +259,7 @@ class SslServerInitializerTest { @ParameterizedTest @MethodSource("provideTestCombinations") void testFailure_protocolNotAccepted(SslProvider sslProvider) throws Exception { - SelfSignedCaCertificate serverSsc = SelfSignedCaCertificate.create(SSL_HOST); + SelfSignedCaCertificate serverSsc = SelfSignedCaCertificate.create(SSL_HOST, new FakeClock()); LocalAddress localAddress = new LocalAddress("PROTOCOL_NOT_ACCEPTED_" + sslProvider); nettyExtension.setUpServer( @@ -288,7 +289,7 @@ class SslServerInitializerTest { @ParameterizedTest @MethodSource("provideTestCombinations") void testFailure_clientCertExpired(SslProvider sslProvider) throws Exception { - SelfSignedCaCertificate serverSsc = SelfSignedCaCertificate.create(SSL_HOST); + SelfSignedCaCertificate serverSsc = SelfSignedCaCertificate.create(SSL_HOST, new FakeClock()); LocalAddress localAddress = new LocalAddress("CLIENT_CERT_EXPIRED_" + sslProvider); nettyExtension.setUpServer( @@ -309,7 +310,7 @@ class SslServerInitializerTest { @ParameterizedTest @MethodSource("provideTestCombinations") void testFailure_clientCertNotYetValid(SslProvider sslProvider) throws Exception { - SelfSignedCaCertificate serverSsc = SelfSignedCaCertificate.create(SSL_HOST); + SelfSignedCaCertificate serverSsc = SelfSignedCaCertificate.create(SSL_HOST, new FakeClock()); LocalAddress localAddress = new LocalAddress("CLIENT_CERT_EXPIRED_" + sslProvider); nettyExtension.setUpServer( @@ -330,7 +331,7 @@ class SslServerInitializerTest { @ParameterizedTest @MethodSource("provideTestCombinations") void testSuccess_doesNotRequireClientCert(SslProvider sslProvider) throws Exception { - SelfSignedCaCertificate serverSsc = SelfSignedCaCertificate.create(SSL_HOST); + SelfSignedCaCertificate serverSsc = SelfSignedCaCertificate.create(SSL_HOST, new FakeClock()); LocalAddress localAddress = new LocalAddress("DOES_NOT_REQUIRE_CLIENT_CERT_" + sslProvider); nettyExtension.setUpServer( @@ -353,7 +354,7 @@ class SslServerInitializerTest { @MethodSource("provideTestCombinations") void testSuccess_CertSignedByOtherCa(SslProvider sslProvider) throws Exception { // The self-signed cert of the CA. - SelfSignedCaCertificate caSsc = SelfSignedCaCertificate.create(); + SelfSignedCaCertificate caSsc = SelfSignedCaCertificate.create(new FakeClock()); KeyPair keyPair = getKeyPair(); X509Certificate serverCert = signKeyPair(caSsc, keyPair, SSL_HOST); LocalAddress localAddress = new LocalAddress("CERT_SIGNED_BY_OTHER_CA_" + sslProvider); @@ -368,7 +369,7 @@ class SslServerInitializerTest { // Serving both the server cert, and the CA cert serverCert, caSsc.cert())); - SelfSignedCaCertificate clientSsc = SelfSignedCaCertificate.create(); + SelfSignedCaCertificate clientSsc = SelfSignedCaCertificate.create(new FakeClock()); nettyExtension.setUpClient( localAddress, getClientHandler( @@ -392,7 +393,7 @@ class SslServerInitializerTest { @ParameterizedTest @MethodSource("provideTestCombinations") void testFailure_requireClientCertificate(SslProvider sslProvider) throws Exception { - SelfSignedCaCertificate serverSsc = SelfSignedCaCertificate.create(SSL_HOST); + SelfSignedCaCertificate serverSsc = SelfSignedCaCertificate.create(SSL_HOST, new FakeClock()); LocalAddress localAddress = new LocalAddress("REQUIRE_CLIENT_CERT_" + sslProvider); nettyExtension.setUpServer( @@ -417,13 +418,14 @@ class SslServerInitializerTest { @ParameterizedTest @MethodSource("provideTestCombinations") void testFailure_wrongHostnameInCertificate(SslProvider sslProvider) throws Exception { - SelfSignedCaCertificate serverSsc = SelfSignedCaCertificate.create("wrong.com"); + SelfSignedCaCertificate serverSsc = + SelfSignedCaCertificate.create("wrong.com", new FakeClock()); LocalAddress localAddress = new LocalAddress("WRONG_HOSTNAME_" + sslProvider); nettyExtension.setUpServer( localAddress, getServerHandler(true, false, sslProvider, serverSsc.key(), serverSsc.cert())); - SelfSignedCaCertificate clientSsc = SelfSignedCaCertificate.create(); + SelfSignedCaCertificate clientSsc = SelfSignedCaCertificate.create(new FakeClock()); nettyExtension.setUpClient( localAddress, getClientHandler(sslProvider, serverSsc.cert(), clientSsc.key(), clientSsc.cert())); diff --git a/networking/src/test/java/google/registry/networking/module/CertificateSupplierModuleTest.java b/networking/src/test/java/google/registry/networking/module/CertificateSupplierModuleTest.java index 70ffbfe3e..13acae86a 100644 --- a/networking/src/test/java/google/registry/networking/module/CertificateSupplierModuleTest.java +++ b/networking/src/test/java/google/registry/networking/module/CertificateSupplierModuleTest.java @@ -26,6 +26,8 @@ import dagger.Component; import dagger.Module; import dagger.Provides; import google.registry.networking.module.CertificateSupplierModule.Mode; +import google.registry.testing.FakeClock; +import google.registry.util.Clock; import google.registry.util.SelfSignedCaCertificate; import jakarta.inject.Named; import jakarta.inject.Singleton; @@ -59,7 +61,7 @@ class CertificateSupplierModuleTest { @BeforeEach void beforeEach() throws Exception { - ssc = SelfSignedCaCertificate.create(); + ssc = SelfSignedCaCertificate.create(new FakeClock()); KeyPair keyPair = getKeyPair(); key = keyPair.getPrivate(); cert = signKeyPair(ssc, keyPair, "example.tld"); @@ -147,6 +149,11 @@ class CertificateSupplierModuleTest { // Make the supplier always return the save value for test to save time. return Duration.ofDays(1); } + + @Provides + Clock provideClock() { + return new FakeClock(); + } } /** diff --git a/prober/src/main/java/google/registry/monitoring/blackbox/token/EppToken.java b/prober/src/main/java/google/registry/monitoring/blackbox/token/EppToken.java index 45f57d501..b0e68fc09 100644 --- a/prober/src/main/java/google/registry/monitoring/blackbox/token/EppToken.java +++ b/prober/src/main/java/google/registry/monitoring/blackbox/token/EppToken.java @@ -18,6 +18,7 @@ import com.google.common.annotations.VisibleForTesting; import google.registry.monitoring.blackbox.exception.UndeterminedStateException; import google.registry.monitoring.blackbox.message.EppRequestMessage; import google.registry.monitoring.blackbox.message.OutboundMessageType; +import google.registry.util.Clock; import io.netty.channel.Channel; import jakarta.inject.Inject; import jakarta.inject.Named; @@ -33,6 +34,7 @@ public abstract class EppToken extends Token { private static AtomicInteger clientIdSuffix = new AtomicInteger(); protected final String tld; + protected final Clock clock; private String host; private String currentDomainName; @@ -40,15 +42,16 @@ public abstract class EppToken extends Token { * Always the constructor used to provide any {@link EppToken}, with {@code tld} and {@code host} * specified by Dagger. */ - protected EppToken(String tld, String host) { + protected EppToken(String tld, String host, Clock clock) { this.tld = tld; this.host = host; + this.clock = clock; currentDomainName = newDomainName(getNewTRID()); } /** Constructor used when passing on same {@link Channel} to next {@link Token}. */ - protected EppToken(String tld, String host, Channel channel) { - this(tld, host); + protected EppToken(String tld, String host, Clock clock, Channel channel) { + this(tld, host, clock); setChannel(channel); } @@ -79,7 +82,7 @@ public abstract class EppToken extends Token { private String getNewTRID() { return String.format( "prober-%s-%d-%d", - "localhost", System.currentTimeMillis(), clientIdSuffix.incrementAndGet()); + "localhost", clock.nowUtc().getMillis(), clientIdSuffix.incrementAndGet()); } /** Return a fully qualified domain label to use, derived from the client transaction ID. */ @@ -103,13 +106,13 @@ public abstract class EppToken extends Token { public static class Transient extends EppToken { @Inject - public Transient(@Named("eppTld") String tld, @Named("eppHost") String host) { - super(tld, host); + public Transient(@Named("eppTld") String tld, @Named("eppHost") String host, Clock clock) { + super(tld, host, clock); } @Override public Token next() { - return new Transient(tld, host()); + return new Transient(tld, host(), clock); } } @@ -121,18 +124,18 @@ public abstract class EppToken extends Token { public static class Persistent extends EppToken { @Inject - public Persistent(@Named("eppTld") String tld, @Named("eppHost") String host) { - super(tld, host); + public Persistent(@Named("eppTld") String tld, @Named("eppHost") String host, Clock clock) { + super(tld, host, clock); } /** Constructor used on call to {@code next} to preserve channel. */ - private Persistent(String tld, String host, Channel channel) { - super(tld, host, channel); + private Persistent(String tld, String host, Clock clock, Channel channel) { + super(tld, host, clock, channel); } @Override public Token next() { - return new Persistent(tld, host(), channel()); + return new Persistent(tld, host(), clock, channel()); } } } diff --git a/prober/src/test/java/google/registry/monitoring/blackbox/token/EppTokenTest.java b/prober/src/test/java/google/registry/monitoring/blackbox/token/EppTokenTest.java index e41273149..91e4f27f8 100644 --- a/prober/src/test/java/google/registry/monitoring/blackbox/token/EppTokenTest.java +++ b/prober/src/test/java/google/registry/monitoring/blackbox/token/EppTokenTest.java @@ -19,6 +19,7 @@ import static com.google.common.truth.Truth.assertThat; import google.registry.monitoring.blackbox.exception.UndeterminedStateException; import google.registry.monitoring.blackbox.message.EppRequestMessage; import google.registry.monitoring.blackbox.util.EppUtils; +import google.registry.testing.FakeClock; import io.netty.channel.Channel; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -28,9 +29,10 @@ class EppTokenTest { private static String TEST_HOST = "host"; private static String TEST_TLD = "tld"; + private final FakeClock fakeClock = new FakeClock(); - private EppToken persistentEppToken = new EppToken.Persistent(TEST_TLD, TEST_HOST); - private EppToken transientEppToken = new EppToken.Transient(TEST_TLD, TEST_HOST); + private EppToken persistentEppToken = new EppToken.Persistent(TEST_TLD, TEST_HOST, fakeClock); + private EppToken transientEppToken = new EppToken.Transient(TEST_TLD, TEST_HOST, fakeClock); @Test void testMessageModificationSuccess_PersistentToken() throws UndeterminedStateException { diff --git a/proxy/src/test/java/google/registry/proxy/EppProtocolModuleTest.java b/proxy/src/test/java/google/registry/proxy/EppProtocolModuleTest.java index f6fb6f534..f8fe7272f 100644 --- a/proxy/src/test/java/google/registry/proxy/EppProtocolModuleTest.java +++ b/proxy/src/test/java/google/registry/proxy/EppProtocolModuleTest.java @@ -24,6 +24,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.base.Throwables; import google.registry.proxy.handler.HttpsRelayServiceHandler.NonOkHttpResponseException; +import google.registry.testing.FakeClock; import google.registry.util.SelfSignedCaCertificate; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; @@ -49,11 +50,11 @@ class EppProtocolModuleTest extends ProtocolModuleTest { private static final byte[] HELLO_BYTES = """ - - - - - """ + + + + + """ .getBytes(UTF_8); private X509Certificate certificate; @@ -122,7 +123,7 @@ class EppProtocolModuleTest extends ProtocolModuleTest { @Override void beforeEach() throws Exception { testComponent = makeTestComponent(); - certificate = SelfSignedCaCertificate.create().cert(); + certificate = SelfSignedCaCertificate.create(new FakeClock()).cert(); initializeChannel( ch -> { ch.attr(REMOTE_ADDRESS_KEY).set(CLIENT_ADDRESS); diff --git a/proxy/src/test/java/google/registry/proxy/handler/EppServiceHandlerTest.java b/proxy/src/test/java/google/registry/proxy/handler/EppServiceHandlerTest.java index cc4c4578d..8ccab79f1 100644 --- a/proxy/src/test/java/google/registry/proxy/handler/EppServiceHandlerTest.java +++ b/proxy/src/test/java/google/registry/proxy/handler/EppServiceHandlerTest.java @@ -32,6 +32,7 @@ import com.google.common.io.BaseEncoding; import google.registry.proxy.TestUtils; import google.registry.proxy.handler.HttpsRelayServiceHandler.NonOkHttpResponseException; import google.registry.proxy.metric.FrontendMetrics; +import google.registry.testing.FakeClock; import google.registry.util.ProxyHttpHeaders; import google.registry.util.SelfSignedCaCertificate; import io.netty.buffer.ByteBuf; @@ -114,7 +115,7 @@ class EppServiceHandlerTest { @BeforeEach void beforeEach() throws Exception { - clientCertificate = SelfSignedCaCertificate.create().cert(); + clientCertificate = SelfSignedCaCertificate.create(new FakeClock()).cert(); channel = setUpNewChannel(eppServiceHandler); } @@ -171,7 +172,7 @@ class EppServiceHandlerTest { new EppServiceHandler( RELAY_HOST, RELAY_PATH, false, () -> ID_TOKEN, HELLO.getBytes(UTF_8), metrics); EmbeddedChannel channel2 = setUpNewChannel(eppServiceHandler2); - X509Certificate clientCertificate2 = SelfSignedCaCertificate.create().cert(); + X509Certificate clientCertificate2 = SelfSignedCaCertificate.create(new FakeClock()).cert(); setHandshakeSuccess(channel2, clientCertificate2); String certHash2 = getCertificateHash(clientCertificate2); diff --git a/util/src/main/java/google/registry/util/PosixTarHeader.java b/util/src/main/java/google/registry/util/PosixTarHeader.java index 4d9b40550..5c68086c2 100644 --- a/util/src/main/java/google/registry/util/PosixTarHeader.java +++ b/util/src/main/java/google/registry/util/PosixTarHeader.java @@ -324,12 +324,12 @@ public final class PosixTarHeader { private final byte[] header = new byte[HEADER_LENGTH]; private boolean hasName = false; private boolean hasSize = false; + private boolean hasMtime = false; public Builder() { setMode(DEFAULT_MODE); setUid(DEFAULT_UID); setGid(DEFAULT_GID); - setMtime(DateTime.now(UTC)); setType(DEFAULT_TYPE); setMagic(); setVersion(); @@ -418,6 +418,7 @@ public final class PosixTarHeader { public Builder setMtime(DateTime mtime) { checkNotNull(mtime, "mtime"); setField("mtime", 136, 12, String.format("%011o", mtime.getMillis() / MILLIS_PER_SECOND)); + hasMtime = true; return this; } @@ -483,8 +484,10 @@ public final class PosixTarHeader { public PosixTarHeader build() { checkState(hasName, "name not set"); checkState(hasSize, "size not set"); + checkState(hasMtime, "mtime not set"); hasName = false; hasSize = false; + hasMtime = false; setChksum(); // Calculate the checksum last. return new PosixTarHeader(header.clone()); } diff --git a/util/src/main/java/google/registry/util/SelfSignedCaCertificate.java b/util/src/main/java/google/registry/util/SelfSignedCaCertificate.java index 189c77cf5..4f58929ce 100644 --- a/util/src/main/java/google/registry/util/SelfSignedCaCertificate.java +++ b/util/src/main/java/google/registry/util/SelfSignedCaCertificate.java @@ -15,7 +15,6 @@ package google.registry.util; import static com.google.common.base.Preconditions.checkArgument; -import static org.joda.time.DateTimeZone.UTC; import com.google.common.collect.ImmutableMap; import java.math.BigInteger; @@ -42,8 +41,6 @@ import org.joda.time.DateTime; public class SelfSignedCaCertificate { private static final String DEFAULT_ISSUER_FQDN = "registry-test"; - private static final DateTime DEFAULT_NOT_BEFORE = DateTime.now(UTC).minusHours(1); - private static final DateTime DEFAULT_NOT_AFTER = DateTime.now(UTC).plusDays(1); private static final Random RANDOM = new Random(); private static final BouncyCastleProvider PROVIDER = new BouncyCastleProvider(); @@ -68,13 +65,16 @@ public class SelfSignedCaCertificate { return cert; } - public static SelfSignedCaCertificate create() throws Exception { + public static SelfSignedCaCertificate create(Clock clock) throws Exception { return create( - keyGen.generateKeyPair(), DEFAULT_ISSUER_FQDN, DEFAULT_NOT_BEFORE, DEFAULT_NOT_AFTER); + keyGen.generateKeyPair(), + DEFAULT_ISSUER_FQDN, + clock.nowUtc().minusHours(1), + clock.nowUtc().plusDays(1)); } - public static SelfSignedCaCertificate create(String fqdn) throws Exception { - return create(fqdn, DEFAULT_NOT_BEFORE, DEFAULT_NOT_AFTER); + public static SelfSignedCaCertificate create(String fqdn, Clock clock) throws Exception { + return create(fqdn, clock.nowUtc().minusHours(1), clock.nowUtc().plusDays(1)); } public static SelfSignedCaCertificate create(String fqdn, DateTime from, DateTime to) diff --git a/util/src/test/java/google/registry/util/PosixTarHeaderTest.java b/util/src/test/java/google/registry/util/PosixTarHeaderTest.java index da8a1a70c..1b0d41bc9 100644 --- a/util/src/test/java/google/registry/util/PosixTarHeaderTest.java +++ b/util/src/test/java/google/registry/util/PosixTarHeaderTest.java @@ -27,6 +27,7 @@ import java.io.InputStream; import java.util.Arrays; import java.util.zip.GZIPInputStream; import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; import org.joda.time.format.ISODateTimeFormat; import org.junit.jupiter.api.Test; @@ -199,7 +200,11 @@ class PosixTarHeaderTest { @Test void testBadChecksum() { PosixTarHeader header = - new PosixTarHeader.Builder().setName("(◕‿◕).txt").setSize(31337).build(); + new PosixTarHeader.Builder() + .setName("(◕‿◕).txt") + .setSize(31337) + .setMtime(DateTime.now(DateTimeZone.UTC)) + .build(); byte[] bytes = header.getBytes(); bytes[150] = '0'; bytes[151] = '0'; @@ -234,6 +239,7 @@ class PosixTarHeaderTest { new PosixTarHeader.Builder() .setName("(•︵•).txt") // Awwwww! It looks so sad... .setSize(123) + .setMtime(DateTime.now(DateTimeZone.UTC)) .build()) .testEquals(); }