mirror of
https://github.com/google/nomulus
synced 2026-09-27 18:34:15 +00:00
Migrate core temporal models and related entities to java.time.Instant (#3001)
This comprehensive refactor continues the migration from Joda-Time to java.time (Instant), focusing on core timestamp models, transition properties, and their integration across the codebase. Key changes: - Migrated CreateAutoTimestamp and UpdateAutoTimestamp to use Instant internally, providing Joda-Time bridge methods for backward compatibility. - Updated TimedTransitionProperty to handle Instant-based transition maps and updated corresponding Hibernate UserTypes (TimedTransitionBaseUserType). - Migrated GracePeriod, BillingBase, BillingEvent, PollMessage, and PendingActionNotificationResponse fields (e.g., expirationTime, eventTime) to Instant. - Migrated additional core entities (DomainBase, Registrar, HostBase, LaunchNotice, BsaLabel, DomainTransactionRecord) to use Instant for registrationExpirationTime, lastTransferTime, creationTime, etc. - Updated Tld and FeatureFlag models to use Instant for claimsPeriodEnd, bsaEnrollStartTime, and status transitions. - Enhanced CLI tools and parameters (TransitionListParameter, InstantParameter, RequestParameters) to support Instant-based input and output. - Updated EntityYamlUtils with custom Instant serializers/deserializers to maintain format consistency (e.g., .SSSZ precision) required for YAML-based tests. - Implemented UtcInstantAdapter to ensure JAXB XML serialization maintains millisecond accuracy, matching legacy Joda-Time behavior. - Resolved Hibernate 6 type mismatches in JPQL and Native queries by ensuring consistent use of Instant for comparisons. - Updated GEMINI.md with project-specific engineering standards, including the 'one commit per PR' mandate, full-build validation requirement, and commit message style rules. - Cleaned up unnecessary @JsonIgnore and @JsonProperty annotations that were previously added to methods with parameters or redundant fields. - Refactored DateTimeUtils to use strongly-typed overloads and standardized naming (earliestOf, latestOf) while avoiding type erasure clashes. - Cleaned up fully qualified calls to toDateTime and toInstant by adding static imports across core model and flow files. - Refactored test suites to use clock.now() (Instant) instead of nowUtc() (DateTime) and removed custom Truth subjects in favor of standard assertions.
This commit is contained in:
@@ -7,10 +7,15 @@ This document outlines foundational mandates, architectural patterns, and projec
|
||||
### 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.
|
||||
- **No Redundant Qualifications:** NEVER use fully qualified class names (e.g., `java.time.temporal.ChronoUnit.DAYS`) in code when an import can be used instead. Always prefer adding an import and using the simple name.
|
||||
- **Static Imports for DateTimeUtils:** Always statically import `toInstant` and `toDateTime` from `google.registry.util.DateTimeUtils`. NEVER write them as `DateTimeUtils.toInstant(...)` or `google.registry.util.DateTimeUtils.toInstant(...)` at the call site; use `toInstant(...)` and `toDateTime(...)` directly.
|
||||
- **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.
|
||||
- **Verification**: Before finalizing any change, scan the imports section for redundancy.
|
||||
- **License Headers**: When creating new files, ensure the license header uses the current year (e.g., 2026). Existing files should retain their original year.
|
||||
|
||||
### 2. Time and Precision Handling (java.time Migration)
|
||||
## 2. Time and Precision Handling (java.time Migration)
|
||||
|
||||
- **Idiomatic java.time Usage:** Avoid redundant conversions between `Instant` and `DateTime`. If a field or parameter is an `Instant`, use it directly. Do not convert to `DateTime` just to call a deprecated method if an `Instant` alternative exists or can be easily created.
|
||||
- **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()`.
|
||||
@@ -50,3 +55,71 @@ This document outlines foundational mandates, architectural patterns, and projec
|
||||
## 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.
|
||||
|
||||
---
|
||||
|
||||
# Gemini Engineering Guide: Nomulus Codebase
|
||||
|
||||
This document captures high-level architectural patterns, lessons learned from large-scale refactorings (like the Joda-Time to `java.time` migration), and specific instructions to avoid common pitfalls in this environment.
|
||||
|
||||
## 🏛 Architecture Overview
|
||||
|
||||
- **Transaction Management:** The codebase uses a custom wrapper around JPA. Always use `tm()` (from `TransactionManagerFactory`) to interact with the database.
|
||||
- **Dependency Injection:** Dagger 2 is used extensively. If you see "cannot find symbol" errors for classes starting with `Dagger...`, the project is in a state where annotation processing failed. Fix compilation in core models first to restore generated code.
|
||||
- **Value Types:** AutoValue and "ImmutableObject" patterns are dominant. Most models follow a `Buildable` pattern with a nested `Builder`.
|
||||
- **Temporal Logic:** The project is migrating from Joda-Time to `java.time`.
|
||||
- Core boundaries: `DateTimeUtils.START_OF_TIME_INSTANT` (Unix Epoch) and `END_OF_TIME_INSTANT` (Long.MAX_VALUE / 1000).
|
||||
- Year Arithmetic: Use `DateTimeUtils.plusYears()` and `DateTimeUtils.minusYears()` to handle February 29th logic correctly.
|
||||
|
||||
## Source Control
|
||||
- **One Commit Per PR:** All changes for a single PR must be squashed into a single commit before merging.
|
||||
- **Default to Amend:** Once an initial commit is created for a PR, all subsequent functional changes should be amended into that same commit by default (`git commit --amend --no-edit`). This ensures the PR remains a single, clean unit of work throughout the development lifecycle.
|
||||
- **Commit Message Style:** Follow standard Git commit best practices. The subject line (first line) should be concise, capitalized, and **must not end with punctuation** (e.g., a period).
|
||||
- **Final Validation:** Always run `git status` as the final step before declaring a task complete to ensure all changes are committed and the working directory is clean.
|
||||
- **Commit Verification:** After any commit or amendment, explicitly verify the success of the operation (e.g., using `git status` and reviewing the diff). Never report a Git operation as "done" without having first successfully executed the command and confirmed the repository state.
|
||||
- **Diff Review:** Before finalizing a task, review the full diff (e.g., `git diff HEAD^`) to ensure all changes are functional and relevant. Identify and revert any formatting-only changes in files that do not contain functional updates to keep the commit focused.
|
||||
|
||||
## Refactoring & Migration Guardrails
|
||||
|
||||
|
||||
### 1. Compiler Warnings are Errors (`-Werror`)
|
||||
This project treats Error Prone warnings as errors.
|
||||
- **`@InlineMeSuggester`**: When creating deprecated Joda-Time bridge methods (e.g., `getTimestamp() -> return toDateTime(getTimestampInstant())`), you **MUST** immediately add `@SuppressWarnings("InlineMeSuggester")`. If you don't, the build will fail.
|
||||
- **Repeatable Annotations**: `@SuppressWarnings` is **NOT** repeatable in this environment. If a method or class already has a suppression (e.g., `@SuppressWarnings("unchecked")`), you must merge them:
|
||||
- ❌ `@SuppressWarnings("unchecked") @SuppressWarnings("InlineMeSuggester")`
|
||||
- ✅ `@SuppressWarnings({"unchecked", "InlineMeSuggester"})`
|
||||
|
||||
### 2. Resolving Ambiguity
|
||||
- **Null Overloads**: Adding an `Instant` overload to a method that previously took `DateTime` will break all `create(null)` calls. You must cast them: `create((Instant) null)`.
|
||||
- **Type Erasure**: Methods taking `Optional<DateTime>` and `Optional<Instant>` will clash due to erasure. Use distinct names, e.g., `setAutorenewEndTimeInstant(Optional<Instant> time)`.
|
||||
|
||||
### 3. Build Strategy
|
||||
- **Surgical Changes**: In large-scale migrations, focus on "leaf" nodes first (Utilities -> Models -> Flows -> Actions).
|
||||
- **PR Size**: Minimize PR size by retaining Joda-Time bridge methods for high-level "Action" and "Flow" classes unless a full migration is requested. Reverting changes to DNS and Reporting logic while updating the underlying models is a valid strategy to keep PRs reviewable.
|
||||
- **Validation**: Always run `./gradlew build -x test` before attempting to run unit tests. Unit tests will not run if there are compilation errors in any part of the `core` module. Before finalizing a PR or declaring a task done, you MUST run the entire build using `./gradlew build` and verify that it succeeds completely without errors. Do not declare success if formatting checks (e.g., `spotlessCheck` or `javaIncrementalFormatCheck`) or tests fail. If formatting fails, run `./gradlew spotlessApply` and then re-run `./gradlew build` to verify everything passes.
|
||||
|
||||
## 🚫 Common Pitfalls to Avoid
|
||||
|
||||
- **Do not go in circles with the build:** If you see an `InlineMeSuggester` error, apply the suppression to **ALL** similar methods in that file and related files in one turn. Do not fix them one by one.
|
||||
- Dagger/AutoValue corruption: If you modify a builder or a component incorrectly, Dagger will fail to generate code, leading to hundreds of "cannot find symbol" errors. If this happens, `git checkout` the last working state of the specific file and re-apply changes more surgically.
|
||||
- **`replace` tool context**: When using `replace` on large files (like `Tld.java` or `DomainBase.java`), provide significant surrounding context. These files have many similar method signatures (getters/setters) that can lead to incorrect replacements.
|
||||
|
||||
---
|
||||
|
||||
# GitHub and Pull Request Protocol
|
||||
|
||||
This protocol defines the standard for interacting with GitHub repositories and processing Pull Request (PR) feedback.
|
||||
|
||||
## 1. Interaction via `gh` CLI
|
||||
- **Primary Tool:** ALWAYS use the `gh` CLI for all GitHub-related operations (listing PRs, viewing PR content, checking status, adding comments).
|
||||
- **Credential Safety:** Never expose tokens or credentials in shell commands.
|
||||
|
||||
## 2. Processing PR Feedback
|
||||
- **Systematic Review:** When asked to address PR comments, first fetch all comments using `gh pr view <number> --json reviews,comments`.
|
||||
- **Minimal Scope Expansion:** Address comments surgically. If a fix requires changes beyond a few lines or expands the PR's original scope significantly, DO NOT implement it without explicit user approval. Instead, report the issue to the user.
|
||||
- **Verification:** After addressing feedback, run the full build (`./gradlew build`) and relevant tests to ensure no regressions were introduced.
|
||||
|
||||
## 3. PR Lifecycle Management
|
||||
- **One Commit Per PR:** Ensure all changes are squashed into a single, clean commit. Use `git commit --amend --no-edit` for follow-up fixes.
|
||||
- **Clean Workspace:** Always run `git status` and verify the repository state before declaring a task complete.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user