From 7b260184906e35aa21e60cc3aa991ba568a0fdee Mon Sep 17 00:00:00 2001 From: gbrodman Date: Fri, 18 Sep 2026 17:44:21 +0000 Subject: [PATCH] Update all generated IDs to use Longs instead of longs (#3236) 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 3b269d366..daf7a614d 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 @@ public class DnsRefreshRequest extends ImmutableObject { 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 5fe2e56b4..65744f566 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 6e36af254..e8c52a082 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 com.google.common.base.Preconditions.checkArgument; 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 javax.annotation.Nullable; 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 @@ abstract class CreateOrUpdateBulkPricingPackageCommand extends MutatingCommand { @Override protected final void init() throws Exception { + ImmutableList.Builder packagesBuilder = new ImmutableList.Builder<>(); for (String token : mainParameters) { tm().transact( () -> { @@ -110,9 +114,20 @@ abstract class CreateOrUpdateBulkPricingPackageCommand extends MutatingCommand { 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 8a7791aa7..41b0b44d1 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 @@ public class DnsRefreshRequestTest extends EntityTestCase { 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 df4726ca1..bbc6da4c9 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 @@ public class BulkPricingPackageTest extends EntityTestCase { .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 292dba040..a77218b94 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 @@ public class CreateBulkPricingPackageCommandTest 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(