mirror of
https://github.com/google/nomulus
synced 2026-05-13 11:21:46 +00:00
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.
12 KiB
12 KiB
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. - 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
toInstantandtoDateTimefromgoogle.registry.util.DateTimeUtils. NEVER write them asDateTimeUtils.toInstant(...)orgoogle.registry.util.DateTimeUtils.toInstant(...)at the call site; usetoInstant(...)andtoDateTime(...)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.
- 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)
- Idiomatic java.time Usage: Avoid redundant conversions between
InstantandDateTime. If a field or parameter is anInstant, use it directly. Do not convert toDateTimejust to call a deprecated method if anInstantalternative exists or can be easily created. - Millisecond Precision: Always truncate
Instant.now()to milliseconds (using.truncatedTo(ChronoUnit.MILLIS)) to maintain consistency with JodaDateTimeand the PostgreSQL schema (which enforces millisecond precision via JPA converters). - Clock Injection:
- Avoid direct calls to
Instant.now(),DateTime.now(),ZonedDateTime.now(), orSystem.currentTimeMillis(). - Inject
google.registry.util.Clock(production) orgoogle.registry.testing.FakeClock(tests). - Use
clock.nowDate()to get aZonedDateTimein UTC.
- Avoid direct calls to
- Beam Pipelines:
- Ensure
Clockis serializable (it is by default in this project) when used in BeamDoFns. - Pass the
Clockthrough the constructor or via Dagger provider methods in the pipeline module.
- Ensure
- Command-Line Tools:
- Use
@Inject Clock clock;inCommandimplementations. - The
clockfield should be package-private (no access modifier) to allow manual initialization in corresponding test classes. - In test classes (e.g.,
UpdateDomainCommandTest), manually setcommand.clock = fakeClock;in the@BeforeEachmethod. - Base test classes like
EppToolCommandTestCaseshould handle this assignment for their generic command types where applicable.
- Use
3. Dependency Injection (Dagger)
- Concrete Types: Dagger
injectmethods must use explicit concrete types. Genericinject(Command)methods will not work. - Test Components: Use
TestRegistryToolComponentfor command-line tool tests to bridge the gap betweenmainandnonprod/testsource 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.reTransactwill 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.
- Top-Level: Define database transactions (
5. Testing Best Practices
- FakeClock and Sleeper: Use
FakeClockandSleeperfor 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
Clockor other core utilities in a new or separate module (likeload-testing), ensureimplementation project(':common')is added to the module'sbuild.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.
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()(fromTransactionManagerFactory) 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
Buildablepattern with a nestedBuilder. - Temporal Logic: The project is migrating from Joda-Time to
java.time.- Core boundaries:
DateTimeUtils.START_OF_TIME_INSTANT(Unix Epoch) andEND_OF_TIME_INSTANT(Long.MAX_VALUE / 1000). - Year Arithmetic: Use
DateTimeUtils.plusYears()andDateTimeUtils.minusYears()to handle February 29th logic correctly.
- Core boundaries:
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 statusas 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 statusand 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:
@SuppressWarningsis 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
Instantoverload to a method that previously tookDateTimewill break allcreate(null)calls. You must cast them:create((Instant) null). - Type Erasure: Methods taking
Optional<DateTime>andOptional<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 testbefore attempting to run unit tests. Unit tests will not run if there are compilation errors in any part of thecoremodule. Before finalizing a PR or declaring a task done, you MUST run the entire build using./gradlew buildand verify that it succeeds completely without errors. Do not declare success if formatting checks (e.g.,spotlessCheckorjavaIncrementalFormatCheck) or tests fail. If formatting fails, run./gradlew spotlessApplyand then re-run./gradlew buildto verify everything passes.
🚫 Common Pitfalls to Avoid
- Do not go in circles with the build: If you see an
InlineMeSuggestererror, 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 checkoutthe last working state of the specific file and re-apply changes more surgically. replacetool context: When usingreplaceon large files (likeTld.javaorDomainBase.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
ghCLI 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-editfor follow-up fixes. - Clean Workspace: Always run
git statusand verify the repository state before declaring a task complete.