From 658ac080c02b4c12ea19e4fe37355d1c32ddaf51 Mon Sep 17 00:00:00 2001 From: Gus Brodman Date: Fri, 18 Sep 2026 11:50:18 -0400 Subject: [PATCH] Update all generated IDs to use Longs instead of longs This makes tracking whether or not an entity has been persisted in the database much simpler, since we can use null as a pure sentinel value to say "this object has not been persisted yet". This makes things like "updateAll" much easier and simpler to implement efficiently later because that will rely on knowing what entities are inserts vs merges --- .../model/common/DnsRefreshRequest.java | 6 ++-- .../domain/token/BulkPricingPackage.java | 4 +-- ...eateOrUpdateBulkPricingPackageCommand.java | 21 ++++++++++++-- .../model/common/DnsRefreshRequestTest.java | 2 ++ .../domain/token/BulkPricingPackageTest.java | 6 +++- .../CreateBulkPricingPackageCommandTest.java | 28 +++++++++++++++++++ 6 files changed, 57 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/google/registry/model/common/DnsRefreshRequest.java b/core/src/main/java/google/registry/model/common/DnsRefreshRequest.java index 3b269d36632..daf7a614d4a 100644 --- a/core/src/main/java/google/registry/model/common/DnsRefreshRequest.java +++ b/core/src/main/java/google/registry/model/common/DnsRefreshRequest.java @@ -41,7 +41,7 @@ public class DnsRefreshRequest extends ImmutableObject { @GeneratedValue(strategy = GenerationType.IDENTITY) @Id @SuppressWarnings("unused") - protected long id; + protected Long id; @Column(nullable = false) @Enumerated(EnumType.STRING) @@ -109,9 +109,7 @@ private DnsRefreshRequest( checkNotNull(tld, "TLD cannot be null"); checkNotNull(requestTime, "Request time cannot be null"); checkNotNull(lastProcessTime, "Last process time cannot be null"); - if (id != null) { - this.id = id; - } + this.id = id; this.type = type; this.name = name; this.tld = tld; diff --git a/core/src/main/java/google/registry/model/domain/token/BulkPricingPackage.java b/core/src/main/java/google/registry/model/domain/token/BulkPricingPackage.java index 5fe2e56b418..65744f56657 100644 --- a/core/src/main/java/google/registry/model/domain/token/BulkPricingPackage.java +++ b/core/src/main/java/google/registry/model/domain/token/BulkPricingPackage.java @@ -49,7 +49,7 @@ public class BulkPricingPackage extends ImmutableObject implements Buildable { @Id @GeneratedValue(strategy = GenerationType.IDENTITY) @Column(name = "package_promotion_id") - long bulkPricingId; + Long bulkPricingId; /** The allocation token string for the bulk pricing package. */ @Column(nullable = false) @@ -84,7 +84,7 @@ public class BulkPricingPackage extends ImmutableObject implements Buildable { */ @Nullable Instant lastNotificationSent; - public long getId() { + public Long getId() { return bulkPricingId; } diff --git a/core/src/main/java/google/registry/tools/CreateOrUpdateBulkPricingPackageCommand.java b/core/src/main/java/google/registry/tools/CreateOrUpdateBulkPricingPackageCommand.java index 6e36af254d7..e8c52a0820e 100644 --- a/core/src/main/java/google/registry/tools/CreateOrUpdateBulkPricingPackageCommand.java +++ b/core/src/main/java/google/registry/tools/CreateOrUpdateBulkPricingPackageCommand.java @@ -18,6 +18,7 @@ import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.beust.jcommander.Parameter; +import com.google.common.collect.ImmutableList; import google.registry.model.domain.token.AllocationToken; import google.registry.model.domain.token.AllocationToken.TokenType; import google.registry.model.domain.token.BulkPricingPackage; @@ -30,7 +31,7 @@ import org.joda.money.Money; /** Shared base class for commands to create or update a {@link BulkPricingPackage} object. */ -abstract class CreateOrUpdateBulkPricingPackageCommand extends MutatingCommand { +abstract class CreateOrUpdateBulkPricingPackageCommand extends ConfirmingCommand { @Parameter(description = "Allocation token String of the bulk token", required = true) List mainParameters; @@ -61,6 +62,8 @@ abstract class CreateOrUpdateBulkPricingPackageCommand extends MutatingCommand { "The next date that the bulk pricing package should be billed for its annual fee") Instant nextBillingDate; + private ImmutableList packagesToSave; + /** Returns the existing BulkPricingPackage or null if it does not exist. */ @Nullable abstract BulkPricingPackage getOldBulkPricingPackage(String token); @@ -87,6 +90,7 @@ boolean clearLastNotificationSent() { @Override protected final void init() throws Exception { + ImmutableList.Builder packagesBuilder = new ImmutableList.Builder<>(); for (String token : mainParameters) { tm().transact( () -> { @@ -110,9 +114,20 @@ protected final void init() throws Exception { if (clearLastNotificationSent()) { builder.setLastNotificationSent((Instant) null); } - BulkPricingPackage newBulkPricingPackage = builder.build(); - stageEntityChange(oldBulkPricingPackage, newBulkPricingPackage); + packagesBuilder.add(builder.build()); }); } + packagesToSave = packagesBuilder.build(); + } + + @Override + protected String prompt() { + return String.format("Save %d bulk pricing package(s)?", packagesToSave.size()); + } + + @Override + protected String execute() { + tm().transact(() -> tm().putAll(packagesToSave)); + return String.format("Saved %d bulk pricing package(s).", packagesToSave.size()); } } diff --git a/core/src/test/java/google/registry/model/common/DnsRefreshRequestTest.java b/core/src/test/java/google/registry/model/common/DnsRefreshRequestTest.java index 8a7791aa790..41b0b44d1b4 100644 --- a/core/src/test/java/google/registry/model/common/DnsRefreshRequestTest.java +++ b/core/src/test/java/google/registry/model/common/DnsRefreshRequestTest.java @@ -38,6 +38,7 @@ public class DnsRefreshRequestTest extends EntityTestCase { @Test void testPersistence() { + assertThat(request.id).isNull(); assertThat(request.getLastProcessTime()).isEqualTo(START_INSTANT); fakeClock.advanceOneMilli(); tm().transact(() -> tm().insert(request)); @@ -45,6 +46,7 @@ void testPersistence() { ImmutableList requests = loadAllOf(DnsRefreshRequest.class); assertThat(requests.size()).isEqualTo(1); assertThat(requests.get(0)).isEqualTo(request); + assertThat(requests.get(0).id).isNotNull(); } @Test diff --git a/core/src/test/java/google/registry/model/domain/token/BulkPricingPackageTest.java b/core/src/test/java/google/registry/model/domain/token/BulkPricingPackageTest.java index df4726ca103..bbc6da4c9eb 100644 --- a/core/src/test/java/google/registry/model/domain/token/BulkPricingPackageTest.java +++ b/core/src/test/java/google/registry/model/domain/token/BulkPricingPackageTest.java @@ -69,9 +69,13 @@ void testPersistence() { .setNextBillingDate(Instant.parse("2011-11-12T05:00:00Z")) .build(); + assertThat(bulkPricingPackage.getId()).isNull(); tm().transact(() -> tm().put(bulkPricingPackage)); + BulkPricingPackage persisted = + tm().transact(() -> BulkPricingPackage.loadByTokenString("abc123")).get(); + assertThat(persisted.getId()).isNotNull(); assertAboutImmutableObjects() - .that(tm().transact(() -> BulkPricingPackage.loadByTokenString("abc123")).get()) + .that(persisted) .isEqualExceptFields(bulkPricingPackage, "bulkPricingId"); } diff --git a/core/src/test/java/google/registry/tools/CreateBulkPricingPackageCommandTest.java b/core/src/test/java/google/registry/tools/CreateBulkPricingPackageCommandTest.java index 292dba040b0..a77218b947e 100644 --- a/core/src/test/java/google/registry/tools/CreateBulkPricingPackageCommandTest.java +++ b/core/src/test/java/google/registry/tools/CreateBulkPricingPackageCommandTest.java @@ -70,6 +70,34 @@ void testSuccess() throws Exception { assertThat(bulkPricingPackage.getLastNotificationSent()).isEmpty(); } + @Test + void testSuccess_multipleTokens() throws Exception { + for (String token : ImmutableSet.of("abc123", "def456")) { + persistResource( + new AllocationToken.Builder() + .setToken(token) + .setTokenType(TokenType.BULK_PRICING) + .setCreationTimeForTest(Instant.parse("2010-11-12T05:00:00Z")) + .setAllowedTlds(ImmutableSet.of("foo")) + .setAllowedRegistrarIds(ImmutableSet.of("TheRegistrar")) + .setRenewalPriceBehavior(RenewalPriceBehavior.SPECIFIED) + .setRenewalPrice(Money.of(USD, 0)) + .setAllowedEppActions(ImmutableSet.of(CommandName.CREATE)) + .setDiscountFraction(1.0) + .build()); + } + runCommandForced( + "--max_domains=100", + "--max_creates=500", + "--price=USD 1000.00", + "--next_billing_date=2012-03-17T00:00:00Z", + "abc123", + "def456"); + + assertThat(tm().transact(() -> BulkPricingPackage.loadByTokenString("abc123"))).isPresent(); + assertThat(tm().transact(() -> BulkPricingPackage.loadByTokenString("def456"))).isPresent(); + } + @Test void testFailure_tokenIsNotBulkType() throws Exception { persistResource(