From c5fa6343d5d96823bd29d829fa83af413e636f23 Mon Sep 17 00:00:00 2001 From: Shicong Huang Date: Thu, 6 Aug 2020 10:26:19 -0400 Subject: [PATCH] Add SQL schema for GracePeriod (#709) * Add SQL schema for GracePeriod * Remove the join table * Add a domainRepoId in GracePeriod * Move the clone logic to GracePeriod * Rebase on HEAD --- .../flows/domain/DomainCreateFlow.java | 3 +- .../flows/domain/DomainDeleteFlow.java | 14 ++- .../flows/domain/DomainRenewFlow.java | 3 +- .../domain/DomainTransferApproveFlow.java | 5 +- .../registry/model/domain/DomainBase.java | 16 +++ .../registry/model/domain/DomainContent.java | 23 +++- .../registry/model/domain/GracePeriod.java | 110 +++++------------ .../model/domain/GracePeriodBase.java | 114 ++++++++++++++++++ .../batch/ResaveEntityActionTest.java | 4 +- .../beam/initsql/DomainBaseUtilTest.java | 1 + .../beam/initsql/InitSqlPipelineTest.java | 6 +- .../google/registry/flows/FlowTestCase.java | 1 + .../flows/domain/DomainCreateFlowTest.java | 3 +- .../flows/domain/DomainDeleteFlowTest.java | 35 ++++-- .../flows/domain/DomainInfoFlowTest.java | 59 +++++++-- .../flows/domain/DomainRenewFlowTest.java | 1 + .../domain/DomainRestoreRequestFlowTest.java | 6 +- .../domain/DomainTransferApproveFlowTest.java | 2 + .../domain/DomainTransferRequestFlowTest.java | 4 + .../model/billing/BillingEventTest.java | 15 ++- .../model/domain/DomainBaseSqlTest.java | 4 + .../registry/model/domain/DomainBaseTest.java | 57 +++++++-- .../model/domain/GracePeriodTest.java | 12 +- .../rde/DomainBaseToXjcConverterTest.java | 2 + .../java/google/registry/rde/RdeFixtures.java | 2 + .../registry/testing/DatastoreHelper.java | 5 +- .../whois/DomainWhoisResponseTest.java | 8 +- .../flyway/V45__add_grace_period_table.sql | 41 +++++++ .../sql/schema/db-schema.sql.generated | 17 ++- .../resources/sql/schema/nomulus.golden.sql | 80 ++++++++++++ 30 files changed, 518 insertions(+), 135 deletions(-) create mode 100644 core/src/main/java/google/registry/model/domain/GracePeriodBase.java create mode 100644 db/src/main/resources/sql/flyway/V45__add_grace_period_table.sql diff --git a/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java b/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java index 87e4811b0..9c8e6ec0e 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java @@ -360,7 +360,8 @@ public class DomainCreateFlow implements TransactionalFlow { command.getNameservers().stream().collect(toImmutableSet())) .setStatusValues(statuses.build()) .setContacts(command.getContacts()) - .addGracePeriod(GracePeriod.forBillingEvent(GracePeriodStatus.ADD, createBillingEvent)) + .addGracePeriod( + GracePeriod.forBillingEvent(GracePeriodStatus.ADD, repoId, createBillingEvent)) .build(); entitiesToSave.add( newDomain, diff --git a/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java b/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java index 059c3f590..d58e3d17e 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java @@ -186,14 +186,18 @@ public final class DomainDeleteFlow implements TransactionalFlow { DateTime redemptionTime = now.plus(redemptionGracePeriodLength); asyncTaskEnqueuer.enqueueAsyncResave( existingDomain, now, ImmutableSortedSet.of(redemptionTime, deletionTime)); - builder.setDeletionTime(deletionTime) + builder + .setDeletionTime(deletionTime) .setStatusValues(ImmutableSet.of(StatusValue.PENDING_DELETE)) // Clear out all old grace periods and add REDEMPTION, which does not include a key to a // billing event because there isn't one for a domain delete. - .setGracePeriods(ImmutableSet.of(GracePeriod.createWithoutBillingEvent( - GracePeriodStatus.REDEMPTION, - redemptionTime, - clientId))); + .setGracePeriods( + ImmutableSet.of( + GracePeriod.createWithoutBillingEvent( + GracePeriodStatus.REDEMPTION, + existingDomain.getRepoId(), + redemptionTime, + clientId))); // Note: The expiration time is unchanged, so if it's before the new deletion time, there will // be a "phantom autorenew" where the expiration time advances. No poll message will be // produced (since we are ending the autorenew recurrences at "now" below) and the billing diff --git a/core/src/main/java/google/registry/flows/domain/DomainRenewFlow.java b/core/src/main/java/google/registry/flows/domain/DomainRenewFlow.java index 1267f9b9f..6bdb78f34 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainRenewFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainRenewFlow.java @@ -183,7 +183,8 @@ public final class DomainRenewFlow implements TransactionalFlow { .setAutorenewBillingEvent(newAutorenewEvent.createVKey()) .setAutorenewPollMessage(newAutorenewPollMessage.createVKey()) .addGracePeriod( - GracePeriod.forBillingEvent(GracePeriodStatus.RENEW, explicitRenewEvent)) + GracePeriod.forBillingEvent( + GracePeriodStatus.RENEW, existingDomain.getRepoId(), explicitRenewEvent)) .build(); EntityChanges entityChanges = flowCustomLogic.beforeSave( diff --git a/core/src/main/java/google/registry/flows/domain/DomainTransferApproveFlow.java b/core/src/main/java/google/registry/flows/domain/DomainTransferApproveFlow.java index 3865af460..603300a95 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainTransferApproveFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainTransferApproveFlow.java @@ -192,7 +192,10 @@ public final class DomainTransferApproveFlow implements TransactionalFlow { .setGracePeriods( billingEvent.isPresent() ? ImmutableSet.of( - GracePeriod.forBillingEvent(GracePeriodStatus.TRANSFER, billingEvent.get())) + GracePeriod.forBillingEvent( + GracePeriodStatus.TRANSFER, + existingDomain.getRepoId(), + billingEvent.get())) : ImmutableSet.of()) .setLastEppUpdateTime(now) .setLastEppUpdateClientId(clientId) diff --git a/core/src/main/java/google/registry/model/domain/DomainBase.java b/core/src/main/java/google/registry/model/domain/DomainBase.java index a24cb061d..3a5580d73 100644 --- a/core/src/main/java/google/registry/model/domain/DomainBase.java +++ b/core/src/main/java/google/registry/model/domain/DomainBase.java @@ -14,6 +14,7 @@ package google.registry.model.domain; + import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Entity; import google.registry.model.EppResource; @@ -27,9 +28,13 @@ import google.registry.schema.replay.DatastoreAndSqlEntity; import java.util.Set; import javax.persistence.Access; import javax.persistence.AccessType; +import javax.persistence.CascadeType; import javax.persistence.Column; import javax.persistence.ElementCollection; +import javax.persistence.FetchType; +import javax.persistence.JoinColumn; import javax.persistence.JoinTable; +import javax.persistence.OneToMany; import org.joda.time.DateTime; /** @@ -74,6 +79,17 @@ public class DomainBase extends DomainContent return super.nsHosts; } + @Access(AccessType.PROPERTY) + @OneToMany( + cascade = {CascadeType.ALL}, + fetch = FetchType.EAGER, + orphanRemoval = true) + @JoinColumn(name = "domainRepoId", referencedColumnName = "repoId") + @SuppressWarnings("UnusedMethod") + private Set getInternalGracePeriods() { + return gracePeriods; + } + @Override public VKey createVKey() { return VKey.create(DomainBase.class, getRepoId(), Key.create(this)); diff --git a/core/src/main/java/google/registry/model/domain/DomainContent.java b/core/src/main/java/google/registry/model/domain/DomainContent.java index b24ca950f..be7fc5e33 100644 --- a/core/src/main/java/google/registry/model/domain/DomainContent.java +++ b/core/src/main/java/google/registry/model/domain/DomainContent.java @@ -73,7 +73,6 @@ import javax.persistence.AccessType; import javax.persistence.AttributeOverride; import javax.persistence.AttributeOverrides; import javax.persistence.Column; -import javax.persistence.ElementCollection; import javax.persistence.Embeddable; import javax.persistence.Embedded; import javax.persistence.MappedSuperclass; @@ -234,7 +233,7 @@ public class DomainContent extends EppResource VKey autorenewPollMessage; /** The unexpired grace periods for this domain (some of which may not be active yet). */ - @Transient @ElementCollection Set gracePeriods; + @Transient Set gracePeriods; /** * The id of the signed mark that was used to create this domain in sunrise. @@ -260,6 +259,18 @@ public class DomainContent extends EppResource allContacts = allContacts.stream().map(contact -> contact.reconstitute()).collect(toImmutableSet()); setContactFields(allContacts, true); + + // We have to return the cloned object here because the original object's + // hashcode is not correct due to the change to its domainRepoId. The cloned + // object will have a null hashcode so that it can get a recalculated hashcode + // when its hashCode() is invoked. + // TODO(b/162739503): Remove this after fully migrating to Cloud SQL. + if (gracePeriods != null) { + gracePeriods = + gracePeriods.stream() + .map(gracePeriod -> gracePeriod.cloneWithDomainRepoId(getRepoId())) + .collect(toImmutableSet()); + } } @PostLoad @@ -354,6 +365,12 @@ public class DomainContent extends EppResource this.nsHosts = nsHosts; } + // Hibernate needs this in order to populate gracePeriods but no one else should ever use it + @SuppressWarnings("UnusedMethod") + private void setInternalGracePeriods(Set gracePeriods) { + this.gracePeriods = gracePeriods; + } + public final String getCurrentSponsorClientId() { return getPersistedCurrentSponsorClientId(); } @@ -448,6 +465,7 @@ public class DomainContent extends EppResource ImmutableSet.of( GracePeriod.create( GracePeriodStatus.TRANSFER, + domain.getRepoId(), transferExpirationTime.plus( Registry.get(domain.getTld()).getTransferGracePeriodLength()), transferData.getGainingClientId(), @@ -483,6 +501,7 @@ public class DomainContent extends EppResource .addGracePeriod( GracePeriod.createForRecurring( GracePeriodStatus.AUTO_RENEW, + domain.getRepoId(), lastAutorenewTime.plus( Registry.get(domain.getTld()).getAutoRenewGracePeriodLength()), domain.getCurrentSponsorClientId(), diff --git a/core/src/main/java/google/registry/model/domain/GracePeriod.java b/core/src/main/java/google/registry/model/domain/GracePeriod.java index 5b5aa12a5..b002373e2 100644 --- a/core/src/main/java/google/registry/model/domain/GracePeriod.java +++ b/core/src/main/java/google/registry/model/domain/GracePeriod.java @@ -18,16 +18,14 @@ import static com.google.common.base.Preconditions.checkArgument; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import com.googlecode.objectify.annotation.Embed; -import com.googlecode.objectify.annotation.Ignore; -import google.registry.model.ImmutableObject; import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.Recurring; import google.registry.model.domain.rgp.GracePeriodStatus; import google.registry.persistence.VKey; import javax.annotation.Nullable; -import javax.persistence.Column; -import javax.persistence.GeneratedValue; -import javax.persistence.GenerationType; +import javax.persistence.Entity; +import javax.persistence.Index; +import javax.persistence.Table; import org.joda.time.DateTime; /** @@ -37,75 +35,13 @@ import org.joda.time.DateTime; * the resource is loaded from Datastore. */ @Embed -@javax.persistence.Entity -public class GracePeriod extends ImmutableObject { - - @javax.persistence.Id - @GeneratedValue(strategy = GenerationType.IDENTITY) - @Ignore - /** Unique id required for hibernate representation. */ - long id; - - /** The type of grace period. */ - GracePeriodStatus type; - - /** When the grace period ends. */ - DateTime expirationTime; - - /** The registrar to bill. */ - @Column(name = "registrarId") - String clientId; - - /** - * The one-time billing event corresponding to the action that triggered this grace period, or - * null if not applicable. Not set for autorenew grace periods (which instead use the field {@code - * billingEventRecurring}) or for redemption grace periods (since deletes have no cost). - */ - // NB: Would @IgnoreSave(IfNull.class), but not allowed for @Embed collections. - VKey billingEventOneTime = null; - - /** - * The recurring billing event corresponding to the action that triggered this grace period, if - * applicable - i.e. if the action was an autorenew - or null in all other cases. - */ - // NB: Would @IgnoreSave(IfNull.class), but not allowed for @Embed collections. - VKey billingEventRecurring = null; - - public GracePeriodStatus getType() { - return type; - } - - public DateTime getExpirationTime() { - return expirationTime; - } - - public String getClientId() { - return clientId; - } - - /** Returns true if this GracePeriod has an associated BillingEvent; i.e. if it's refundable. */ - public boolean hasBillingEvent() { - return billingEventOneTime != null || billingEventRecurring != null; - } - - /** - * Returns the one time billing event. The value will only be non-null if the type of this grace - * period is not AUTO_RENEW. - */ - public VKey getOneTimeBillingEvent() { - return billingEventOneTime; - } - - /** - * Returns the recurring billing event. The value will only be non-null if the type of this grace - * period is AUTO_RENEW. - */ - public VKey getRecurringBillingEvent() { - return billingEventRecurring; - } +@Entity +@Table(indexes = @Index(columnList = "domainRepoId")) +public class GracePeriod extends GracePeriodBase { private static GracePeriod createInternal( GracePeriodStatus type, + String domainRepoId, DateTime expirationTime, String clientId, @Nullable VKey billingEventOneTime, @@ -117,6 +53,7 @@ public class GracePeriod extends ImmutableObject { "Recurring billing events must be present on (and only on) autorenew grace periods"); GracePeriod instance = new GracePeriod(); instance.type = checkArgumentNotNull(type); + instance.domainRepoId = checkArgumentNotNull(domainRepoId); instance.expirationTime = checkArgumentNotNull(expirationTime); instance.clientId = checkArgumentNotNull(clientId); instance.billingEventOneTime = billingEventOneTime; @@ -133,32 +70,51 @@ public class GracePeriod extends ImmutableObject { */ public static GracePeriod create( GracePeriodStatus type, + String domainRepoId, DateTime expirationTime, String clientId, @Nullable VKey billingEventOneTime) { - return createInternal(type, expirationTime, clientId, billingEventOneTime, null); + return createInternal(type, domainRepoId, expirationTime, clientId, billingEventOneTime, null); } /** Creates a GracePeriod for a Recurring billing event. */ public static GracePeriod createForRecurring( GracePeriodStatus type, + String domainRepoId, DateTime expirationTime, String clientId, VKey billingEventRecurring) { checkArgumentNotNull(billingEventRecurring, "billingEventRecurring cannot be null"); - return createInternal(type, expirationTime, clientId, null, billingEventRecurring); + return createInternal( + type, domainRepoId, expirationTime, clientId, null, billingEventRecurring); } /** Creates a GracePeriod with no billing event. */ public static GracePeriod createWithoutBillingEvent( - GracePeriodStatus type, DateTime expirationTime, String clientId) { - return createInternal(type, expirationTime, clientId, null, null); + GracePeriodStatus type, String domainRepoId, DateTime expirationTime, String clientId) { + return createInternal(type, domainRepoId, expirationTime, clientId, null, null); } /** Constructs a GracePeriod of the given type from the provided one-time BillingEvent. */ public static GracePeriod forBillingEvent( - GracePeriodStatus type, BillingEvent.OneTime billingEvent) { + GracePeriodStatus type, String domainRepoId, BillingEvent.OneTime billingEvent) { return create( - type, billingEvent.getBillingTime(), billingEvent.getClientId(), billingEvent.createVKey()); + type, + domainRepoId, + billingEvent.getBillingTime(), + billingEvent.getClientId(), + billingEvent.createVKey()); } + + /** + * Returns a clone of this {@link GracePeriod} with {@link #domainRepoId} set to the given value. + * + *

TODO(b/162739503): Remove this function after fully migrating to Cloud SQL. + */ + public GracePeriod cloneWithDomainRepoId(String domainRepoId) { + GracePeriod clone = clone(this); + clone.domainRepoId = checkArgumentNotNull(domainRepoId); + return clone; + } + } diff --git a/core/src/main/java/google/registry/model/domain/GracePeriodBase.java b/core/src/main/java/google/registry/model/domain/GracePeriodBase.java new file mode 100644 index 000000000..cca2db53b --- /dev/null +++ b/core/src/main/java/google/registry/model/domain/GracePeriodBase.java @@ -0,0 +1,114 @@ +// Copyright 2020 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.model.domain; + +import com.googlecode.objectify.annotation.Embed; +import com.googlecode.objectify.annotation.Ignore; +import google.registry.model.ImmutableObject; +import google.registry.model.billing.BillingEvent; +import google.registry.model.billing.BillingEvent.OneTime; +import google.registry.model.domain.rgp.GracePeriodStatus; +import google.registry.persistence.VKey; +import javax.persistence.Column; +import javax.persistence.EnumType; +import javax.persistence.Enumerated; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.MappedSuperclass; +import org.joda.time.DateTime; + +/** Base class containing common fields and methods for {@link GracePeriod}. */ +@Embed +@MappedSuperclass +public class GracePeriodBase extends ImmutableObject { + + /** Unique id required for hibernate representation. */ + @javax.persistence.Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + @Ignore + Long id; + + /** Repository id for the domain which this grace period belongs to. */ + @Ignore + @Column(nullable = false) + String domainRepoId; + + /** The type of grace period. */ + @Column(nullable = false) + @Enumerated(EnumType.STRING) + GracePeriodStatus type; + + /** When the grace period ends. */ + @Column(nullable = false) + DateTime expirationTime; + + /** The registrar to bill. */ + @Column(name = "registrarId", nullable = false) + String clientId; + + /** + * The one-time billing event corresponding to the action that triggered this grace period, or + * null if not applicable. Not set for autorenew grace periods (which instead use the field {@code + * billingEventRecurring}) or for redemption grace periods (since deletes have no cost). + */ + // NB: Would @IgnoreSave(IfNull.class), but not allowed for @Embed collections. + @Column(name = "billing_event_id") + VKey billingEventOneTime = null; + + /** + * The recurring billing event corresponding to the action that triggered this grace period, if + * applicable - i.e. if the action was an autorenew - or null in all other cases. + */ + // NB: Would @IgnoreSave(IfNull.class), but not allowed for @Embed collections. + @Column(name = "billing_recurrence_id") + VKey billingEventRecurring = null; + + public GracePeriodStatus getType() { + return type; + } + + public String getDomainRepoId() { + return domainRepoId; + } + + public DateTime getExpirationTime() { + return expirationTime; + } + + public String getClientId() { + return clientId; + } + + /** Returns true if this GracePeriod has an associated BillingEvent; i.e. if it's refundable. */ + public boolean hasBillingEvent() { + return billingEventOneTime != null || billingEventRecurring != null; + } + + /** + * Returns the one time billing event. The value will only be non-null if the type of this grace + * period is not AUTO_RENEW. + */ + public VKey getOneTimeBillingEvent() { + return billingEventOneTime; + } + + /** + * Returns the recurring billing event. The value will only be non-null if the type of this grace + * period is AUTO_RENEW. + */ + public VKey getRecurringBillingEvent() { + return billingEventRecurring; + } +} diff --git a/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java b/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java index dc78b4c39..2f9d07c57 100644 --- a/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java +++ b/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java @@ -117,9 +117,10 @@ public class ResaveEntityActionTest { @Test void test_domainPendingDeletion_isResavedAndReenqueued() { + DomainBase newDomain = newDomainBase("domain.tld"); DomainBase domain = persistResource( - newDomainBase("domain.tld") + newDomain .asBuilder() .setDeletionTime(clock.nowUtc().plusDays(35)) .setStatusValues(ImmutableSet.of(StatusValue.PENDING_DELETE)) @@ -127,6 +128,7 @@ public class ResaveEntityActionTest { ImmutableSet.of( GracePeriod.createWithoutBillingEvent( GracePeriodStatus.REDEMPTION, + newDomain.getRepoId(), clock.nowUtc().plusDays(30), "TheRegistrar"))) .build()); diff --git a/core/src/test/java/google/registry/beam/initsql/DomainBaseUtilTest.java b/core/src/test/java/google/registry/beam/initsql/DomainBaseUtilTest.java index 5b6f027a7..911fd7395 100644 --- a/core/src/test/java/google/registry/beam/initsql/DomainBaseUtilTest.java +++ b/core/src/test/java/google/registry/beam/initsql/DomainBaseUtilTest.java @@ -161,6 +161,7 @@ public class DomainBaseUtilTest { .addGracePeriod( GracePeriod.create( GracePeriodStatus.ADD, + "4-COM", fakeClock.nowUtc().plusDays(1), "registrar", null)) diff --git a/core/src/test/java/google/registry/beam/initsql/InitSqlPipelineTest.java b/core/src/test/java/google/registry/beam/initsql/InitSqlPipelineTest.java index b813a0a13..56aadd261 100644 --- a/core/src/test/java/google/registry/beam/initsql/InitSqlPipelineTest.java +++ b/core/src/test/java/google/registry/beam/initsql/InitSqlPipelineTest.java @@ -214,7 +214,11 @@ class InitSqlPipelineTest { .setSmdId("smdid") .addGracePeriod( GracePeriod.create( - GracePeriodStatus.ADD, fakeClock.nowUtc().plusDays(1), "registrar", null)) + GracePeriodStatus.ADD, + "4-COM", + fakeClock.nowUtc().plusDays(1), + "registrar", + null)) .build()); exportDir = store.export(exportRootDir.getAbsolutePath(), ALL_KINDS, ImmutableSet.of()); commitLogDir = Files.createDirectory(tmpDir.resolve("commits")).toFile(); diff --git a/core/src/test/java/google/registry/flows/FlowTestCase.java b/core/src/test/java/google/registry/flows/FlowTestCase.java index a522aaa9d..8215905d1 100644 --- a/core/src/test/java/google/registry/flows/FlowTestCase.java +++ b/core/src/test/java/google/registry/flows/FlowTestCase.java @@ -185,6 +185,7 @@ public abstract class FlowTestCase { builder.put( GracePeriod.create( entry.getKey().getType(), + entry.getKey().getDomainRepoId(), entry.getKey().getExpirationTime(), entry.getKey().getClientId(), null), diff --git a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java index a604de315..ba222b882 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java @@ -325,7 +325,8 @@ class DomainCreateFlowTest extends ResourceFlowTestCase BillingEvent.Cancellation.forGracePeriod( GracePeriod.createWithoutBillingEvent( - GracePeriodStatus.REDEMPTION, now.plusDays(1), "a registrar"), + GracePeriodStatus.REDEMPTION, + domain.getRepoId(), + now.plusDays(1), + "a registrar"), historyEntry, "foo.tld")); assertThat(thrown).hasMessageThat().contains("grace period without billing event"); diff --git a/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java b/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java index bf9ce1eb2..809a90273 100644 --- a/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java +++ b/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java @@ -18,6 +18,7 @@ import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableO import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.testing.SqlHelper.assertThrowForeignKeyViolation; import static google.registry.testing.SqlHelper.saveRegistrar; +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; @@ -25,6 +26,7 @@ import com.google.common.collect.ImmutableSet; import google.registry.model.contact.ContactResource; import google.registry.model.domain.DesignatedContact.Type; import google.registry.model.domain.launch.LaunchNotice; +import google.registry.model.domain.rgp.GracePeriodStatus; import google.registry.model.domain.secdns.DelegationSignerData; import google.registry.model.eppcommon.AuthInfo.PasswordAuth; import google.registry.model.eppcommon.StatusValue; @@ -100,6 +102,8 @@ public class DomainBaseSqlTest { .setLaunchNotice( LaunchNotice.create("tcnid", "validatorId", START_OF_TIME, START_OF_TIME)) .setSmdId("smdid") + .addGracePeriod( + GracePeriod.create(GracePeriodStatus.ADD, "4-COM", END_OF_TIME, "registrar1", null)) .build(); host = diff --git a/core/src/test/java/google/registry/model/domain/DomainBaseTest.java b/core/src/test/java/google/registry/model/domain/DomainBaseTest.java index 7b7f9c68f..864e54d13 100644 --- a/core/src/test/java/google/registry/model/domain/DomainBaseTest.java +++ b/core/src/test/java/google/registry/model/domain/DomainBaseTest.java @@ -154,6 +154,7 @@ public class DomainBaseTest extends EntityTestCase { .addGracePeriod( GracePeriod.create( GracePeriodStatus.ADD, + "4-COM", fakeClock.nowUtc().plusDays(1), "registrar", null)) @@ -371,7 +372,11 @@ public class DomainBaseTest extends EntityTestCase { // Okay for billing event to be null since the point of this grace period is just // to check that the transfer will clear all existing grace periods. GracePeriod.create( - GracePeriodStatus.ADD, fakeClock.nowUtc().plusDays(100), "foo", null)) + GracePeriodStatus.ADD, + domain.getRepoId(), + fakeClock.nowUtc().plusDays(100), + "foo", + null)) .build(); DomainBase afterTransfer = domain.cloneProjectedAtTime(fakeClock.nowUtc().plusDays(1)); DateTime newExpirationTime = oldExpirationTime.plusYears(1); @@ -382,6 +387,7 @@ public class DomainBaseTest extends EntityTestCase { .containsExactly( GracePeriod.create( GracePeriodStatus.TRANSFER, + domain.getRepoId(), fakeClock .nowUtc() .plusDays(1) @@ -499,9 +505,24 @@ public class DomainBaseTest extends EntityTestCase { void testStackedGracePeriods() { ImmutableList gracePeriods = ImmutableList.of( - GracePeriod.create(GracePeriodStatus.ADD, fakeClock.nowUtc().plusDays(3), "foo", null), - GracePeriod.create(GracePeriodStatus.ADD, fakeClock.nowUtc().plusDays(2), "bar", null), - GracePeriod.create(GracePeriodStatus.ADD, fakeClock.nowUtc().plusDays(1), "baz", null)); + GracePeriod.create( + GracePeriodStatus.ADD, + domain.getRepoId(), + fakeClock.nowUtc().plusDays(3), + "foo", + null), + GracePeriod.create( + GracePeriodStatus.ADD, + domain.getRepoId(), + fakeClock.nowUtc().plusDays(2), + "bar", + null), + GracePeriod.create( + GracePeriodStatus.ADD, + domain.getRepoId(), + fakeClock.nowUtc().plusDays(1), + "baz", + null)); domain = domain.asBuilder().setGracePeriods(ImmutableSet.copyOf(gracePeriods)).build(); for (int i = 1; i < 3; ++i) { assertThat(domain.cloneProjectedAtTime(fakeClock.nowUtc().plusDays(i)).getGracePeriods()) @@ -513,14 +534,32 @@ public class DomainBaseTest extends EntityTestCase { void testGracePeriodsByType() { ImmutableSet addGracePeriods = ImmutableSet.of( - GracePeriod.create(GracePeriodStatus.ADD, fakeClock.nowUtc().plusDays(3), "foo", null), - GracePeriod.create(GracePeriodStatus.ADD, fakeClock.nowUtc().plusDays(1), "baz", null)); + GracePeriod.create( + GracePeriodStatus.ADD, + domain.getRepoId(), + fakeClock.nowUtc().plusDays(3), + "foo", + null), + GracePeriod.create( + GracePeriodStatus.ADD, + domain.getRepoId(), + fakeClock.nowUtc().plusDays(1), + "baz", + null)); ImmutableSet renewGracePeriods = ImmutableSet.of( GracePeriod.create( - GracePeriodStatus.RENEW, fakeClock.nowUtc().plusDays(3), "foo", null), + GracePeriodStatus.RENEW, + domain.getRepoId(), + fakeClock.nowUtc().plusDays(3), + "foo", + null), GracePeriod.create( - GracePeriodStatus.RENEW, fakeClock.nowUtc().plusDays(1), "baz", null)); + GracePeriodStatus.RENEW, + domain.getRepoId(), + fakeClock.nowUtc().plusDays(1), + "baz", + null)); domain = domain .asBuilder() @@ -589,6 +628,7 @@ public class DomainBaseTest extends EntityTestCase { .containsExactly( GracePeriod.createForRecurring( GracePeriodStatus.AUTO_RENEW, + domain.getRepoId(), oldExpirationTime .plusYears(2) .plus(Registry.get("com").getAutoRenewGracePeriodLength()), @@ -757,6 +797,7 @@ public class DomainBaseTest extends EntityTestCase { ImmutableSet.of( GracePeriod.createForRecurring( GracePeriodStatus.AUTO_RENEW, + domain.getRepoId(), now.plusDays(1), "NewRegistrar", recurringBillKey))) diff --git a/core/src/test/java/google/registry/model/domain/GracePeriodTest.java b/core/src/test/java/google/registry/model/domain/GracePeriodTest.java index 1fd9f6d0a..a4eb11971 100644 --- a/core/src/test/java/google/registry/model/domain/GracePeriodTest.java +++ b/core/src/test/java/google/registry/model/domain/GracePeriodTest.java @@ -63,8 +63,9 @@ public class GracePeriodTest { @Test void testSuccess_forBillingEvent() { - GracePeriod gracePeriod = GracePeriod.forBillingEvent(GracePeriodStatus.ADD, onetime); + GracePeriod gracePeriod = GracePeriod.forBillingEvent(GracePeriodStatus.ADD, "1-TEST", onetime); assertThat(gracePeriod.getType()).isEqualTo(GracePeriodStatus.ADD); + assertThat(gracePeriod.getDomainRepoId()).isEqualTo("1-TEST"); assertThat(gracePeriod.getOneTimeBillingEvent()).isEqualTo(onetime.createVKey()); assertThat(gracePeriod.getRecurringBillingEvent()).isNull(); assertThat(gracePeriod.getClientId()).isEqualTo("TheRegistrar"); @@ -74,9 +75,11 @@ public class GracePeriodTest { @Test void testSuccess_createWithoutBillingEvent() { - GracePeriod gracePeriod = GracePeriod.createWithoutBillingEvent( - GracePeriodStatus.REDEMPTION, now, "TheRegistrar"); + GracePeriod gracePeriod = + GracePeriod.createWithoutBillingEvent( + GracePeriodStatus.REDEMPTION, "1-TEST", now, "TheRegistrar"); assertThat(gracePeriod.getType()).isEqualTo(GracePeriodStatus.REDEMPTION); + assertThat(gracePeriod.getDomainRepoId()).isEqualTo("1-TEST"); assertThat(gracePeriod.getOneTimeBillingEvent()).isNull(); assertThat(gracePeriod.getRecurringBillingEvent()).isNull(); assertThat(gracePeriod.getClientId()).isEqualTo("TheRegistrar"); @@ -89,7 +92,7 @@ public class GracePeriodTest { IllegalArgumentException thrown = assertThrows( IllegalArgumentException.class, - () -> GracePeriod.forBillingEvent(GracePeriodStatus.AUTO_RENEW, onetime)); + () -> GracePeriod.forBillingEvent(GracePeriodStatus.AUTO_RENEW, "1-TEST", onetime)); assertThat(thrown).hasMessageThat().contains("autorenew"); } @@ -101,6 +104,7 @@ public class GracePeriodTest { () -> GracePeriod.createForRecurring( GracePeriodStatus.RENEW, + "1-TEST", now.plusDays(1), "TheRegistrar", VKey.create(Recurring.class, 12345))); diff --git a/core/src/test/java/google/registry/rde/DomainBaseToXjcConverterTest.java b/core/src/test/java/google/registry/rde/DomainBaseToXjcConverterTest.java index 25c78486f..d3ba39ecc 100644 --- a/core/src/test/java/google/registry/rde/DomainBaseToXjcConverterTest.java +++ b/core/src/test/java/google/registry/rde/DomainBaseToXjcConverterTest.java @@ -278,6 +278,7 @@ public class DomainBaseToXjcConverterTest { ImmutableSet.of( GracePeriod.forBillingEvent( GracePeriodStatus.RENEW, + domain.getRepoId(), persistResource( new BillingEvent.OneTime.Builder() .setReason(Reason.RENEW) @@ -291,6 +292,7 @@ public class DomainBaseToXjcConverterTest { .build())), GracePeriod.create( GracePeriodStatus.TRANSFER, + domain.getRepoId(), DateTime.parse("1920-01-01T00:00:00Z"), "foo", null))) diff --git a/core/src/test/java/google/registry/rde/RdeFixtures.java b/core/src/test/java/google/registry/rde/RdeFixtures.java index 2dfe3fa92..4630522c5 100644 --- a/core/src/test/java/google/registry/rde/RdeFixtures.java +++ b/core/src/test/java/google/registry/rde/RdeFixtures.java @@ -122,6 +122,7 @@ final class RdeFixtures { ImmutableSet.of( GracePeriod.forBillingEvent( GracePeriodStatus.RENEW, + domain.getRepoId(), persistResource( new BillingEvent.OneTime.Builder() .setReason(Reason.RENEW) @@ -135,6 +136,7 @@ final class RdeFixtures { .build())), GracePeriod.create( GracePeriodStatus.TRANSFER, + domain.getRepoId(), DateTime.parse("1992-01-01T00:00:00Z"), "foo", null))) diff --git a/core/src/test/java/google/registry/testing/DatastoreHelper.java b/core/src/test/java/google/registry/testing/DatastoreHelper.java index a355d697c..29dd9d47b 100644 --- a/core/src/test/java/google/registry/testing/DatastoreHelper.java +++ b/core/src/test/java/google/registry/testing/DatastoreHelper.java @@ -504,9 +504,10 @@ public class DatastoreHelper { DateTime creationTime, DateTime expirationTime) { String domainName = String.format("%s.%s", label, tld); + String repoId = generateNewDomainRoid(tld); DomainBase domain = new DomainBase.Builder() - .setRepoId(generateNewDomainRoid(tld)) + .setRepoId(repoId) .setDomainName(domainName) .setPersistedCurrentSponsorClientId("TheRegistrar") .setCreationClientId("TheRegistrar") @@ -519,7 +520,7 @@ public class DatastoreHelper { DesignatedContact.create(Type.TECH, contact.createVKey()))) .setAuthInfo(DomainAuthInfo.create(PasswordAuth.create("fooBAR"))) .addGracePeriod( - GracePeriod.create(GracePeriodStatus.ADD, now.plusDays(10), "foo", null)) + GracePeriod.create(GracePeriodStatus.ADD, repoId, now.plusDays(10), "foo", null)) .build(); HistoryEntry historyEntryDomainCreate = persistResource( diff --git a/core/src/test/java/google/registry/whois/DomainWhoisResponseTest.java b/core/src/test/java/google/registry/whois/DomainWhoisResponseTest.java index 77ad49c98..ae18a63b8 100644 --- a/core/src/test/java/google/registry/whois/DomainWhoisResponseTest.java +++ b/core/src/test/java/google/registry/whois/DomainWhoisResponseTest.java @@ -228,11 +228,12 @@ class DomainWhoisResponseTest { VKey adminResourceKey = adminContact.createVKey(); VKey techResourceKey = techContact.createVKey(); + String repoId = "3-TLD"; domainBase = persistResource( new DomainBase.Builder() .setDomainName("example.tld") - .setRepoId("3-TLD") + .setRepoId(repoId) .setLastEppUpdateTime(DateTime.parse("2009-05-29T20:13:00Z")) .setCreationTimeForTest(DateTime.parse("2000-10-08T00:45:00Z")) .setRegistrationExpirationTime(DateTime.parse("2010-10-08T00:44:59Z")) @@ -252,8 +253,9 @@ class DomainWhoisResponseTest { .setDsData(ImmutableSet.of(DelegationSignerData.create(1, 2, 3, "deadface"))) .setGracePeriods( ImmutableSet.of( - GracePeriod.create(GracePeriodStatus.ADD, END_OF_TIME, "", null), - GracePeriod.create(GracePeriodStatus.TRANSFER, END_OF_TIME, "", null))) + GracePeriod.create(GracePeriodStatus.ADD, repoId, END_OF_TIME, "", null), + GracePeriod.create( + GracePeriodStatus.TRANSFER, repoId, END_OF_TIME, "", null))) .build()); } diff --git a/db/src/main/resources/sql/flyway/V45__add_grace_period_table.sql b/db/src/main/resources/sql/flyway/V45__add_grace_period_table.sql new file mode 100644 index 000000000..4ce0e5cb7 --- /dev/null +++ b/db/src/main/resources/sql/flyway/V45__add_grace_period_table.sql @@ -0,0 +1,41 @@ +-- Copyright 2020 The Nomulus Authors. All Rights Reserved. +-- +-- Licensed under the Apache License, Version 2.0 (the "License"); +-- you may not use this file except in compliance with the License. +-- You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. + +create table "GracePeriod" ( + id bigserial not null, + billing_event_id int8, + billing_recurrence_id int8, + registrar_id text not null, + domain_repo_id text not null, + expiration_time timestamptz not null, + type text not null, + primary key (id) +); + +alter table if exists "GracePeriod" + add constraint FK2mys4hojm6ev2g9tmy5aq6m7g + foreign key (domain_repo_id) + references "Domain"; + +alter table if exists "GracePeriod" + add constraint fk_grace_period_billing_event_id + foreign key (billing_event_id) + references "BillingEvent"; + +alter table if exists "GracePeriod" + add constraint fk_grace_period_billing_recurrence_id + foreign key (billing_recurrence_id) + references "BillingRecurrence"; + +create index IDXj1mtx98ndgbtb1bkekahms18w on "GracePeriod" (domain_repo_id); diff --git a/db/src/main/resources/sql/schema/db-schema.sql.generated b/db/src/main/resources/sql/schema/db-schema.sql.generated index e8fcce36e..3a849b02e 100644 --- a/db/src/main/resources/sql/schema/db-schema.sql.generated +++ b/db/src/main/resources/sql/schema/db-schema.sql.generated @@ -339,11 +339,12 @@ create sequence history_id_sequence start 1 increment 1; create table "GracePeriod" ( id bigserial not null, - billing_event_one_time int8, - billing_event_recurring int8, - registrar_id text, - expiration_time timestamptz, - type int4, + billing_event_id int8, + billing_recurrence_id int8, + registrar_id text not null, + domain_repo_id text not null, + expiration_time timestamptz not null, + type text not null, primary key (id) ); @@ -596,6 +597,7 @@ create index IDXrh4xmrot9bd63o382ow9ltfig on "DomainHistory" (creation_time); create index IDXaro1omfuaxjwmotk3vo00trwm on "DomainHistory" (history_registrar_id); create index IDXsu1nam10cjes9keobapn5jvxj on "DomainHistory" (history_type); create index IDX6w3qbtgce93cal2orjg1tw7b7 on "DomainHistory" (history_modification_time); +create index IDXj1mtx98ndgbtb1bkekahms18w on "GracePeriod" (domain_repo_id); create index IDXfg2nnjlujxo6cb9fha971bq2n on "HostHistory" (creation_time); create index IDX1iy7njgb7wjmj9piml4l2g0qi on "HostHistory" (history_registrar_id); create index IDXkkwbwcwvrdkkqothkiye4jiff on "HostHistory" (host_name); @@ -632,6 +634,11 @@ create index spec11threatmatch_check_date_idx on "Spec11ThreatMatch" (check_date foreign key (domain_repo_id) references "Domain"; + alter table if exists "GracePeriod" + add constraint FK2mys4hojm6ev2g9tmy5aq6m7g + foreign key (domain_repo_id) + references "Domain"; + alter table if exists "PremiumEntry" add constraint FKo0gw90lpo1tuee56l0nb6y6g5 foreign key (revision_id) diff --git a/db/src/main/resources/sql/schema/nomulus.golden.sql b/db/src/main/resources/sql/schema/nomulus.golden.sql index 52e9988c0..bb46d4ac2 100644 --- a/db/src/main/resources/sql/schema/nomulus.golden.sql +++ b/db/src/main/resources/sql/schema/nomulus.golden.sql @@ -488,6 +488,40 @@ CREATE TABLE public."DomainHost" ( ); +-- +-- Name: GracePeriod; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public."GracePeriod" ( + id bigint NOT NULL, + billing_event_id bigint, + billing_recurrence_id bigint, + registrar_id text NOT NULL, + domain_repo_id text NOT NULL, + expiration_time timestamp with time zone NOT NULL, + type text NOT NULL +); + + +-- +-- Name: GracePeriod_id_seq; Type: SEQUENCE; Schema: public; Owner: - +-- + +CREATE SEQUENCE public."GracePeriod_id_seq" + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + +-- +-- Name: GracePeriod_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - +-- + +ALTER SEQUENCE public."GracePeriod_id_seq" OWNED BY public."GracePeriod".id; + + -- -- Name: HostHistory; Type: TABLE; Schema: public; Owner: - -- @@ -900,6 +934,13 @@ ALTER TABLE ONLY public."BillingRecurrence" ALTER COLUMN billing_recurrence_id S ALTER TABLE ONLY public."ClaimsList" ALTER COLUMN revision_id SET DEFAULT nextval('public."ClaimsList_revision_id_seq"'::regclass); +-- +-- Name: GracePeriod id; Type: DEFAULT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public."GracePeriod" ALTER COLUMN id SET DEFAULT nextval('public."GracePeriod_id_seq"'::regclass); + + -- -- Name: PollMessage poll_message_id; Type: DEFAULT; Schema: public; Owner: - -- @@ -1022,6 +1063,14 @@ ALTER TABLE ONLY public."Domain" ADD CONSTRAINT "Domain_pkey" PRIMARY KEY (repo_id); +-- +-- Name: GracePeriod GracePeriod_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public."GracePeriod" + ADD CONSTRAINT "GracePeriod_pkey" PRIMARY KEY (id); + + -- -- Name: HostHistory HostHistory_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -1310,6 +1359,13 @@ CREATE INDEX idxhmv411mdqo5ibn4vy7ykxpmlv ON public."BillingEvent" USING btree ( CREATE INDEX idxhp33wybmb6tbpr1bq7ttwk8je ON public."ContactHistory" USING btree (history_registrar_id); +-- +-- Name: idxj1mtx98ndgbtb1bkekahms18w; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX idxj1mtx98ndgbtb1bkekahms18w ON public."GracePeriod" USING btree (domain_repo_id); + + -- -- Name: idxj77pfwhui9f0i7wjq6lmibovj; Type: INDEX; Schema: public; Owner: - -- @@ -1488,6 +1544,14 @@ ALTER TABLE ONLY public."RegistryLock" ADD CONSTRAINT fk2lhcwpxlnqijr96irylrh1707 FOREIGN KEY (relock_revision_id) REFERENCES public."RegistryLock"(revision_id); +-- +-- Name: GracePeriod fk2mys4hojm6ev2g9tmy5aq6m7g; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public."GracePeriod" + ADD CONSTRAINT fk2mys4hojm6ev2g9tmy5aq6m7g FOREIGN KEY (domain_repo_id) REFERENCES public."Domain"(repo_id); + + -- -- Name: Domain fk2u3srsfbei272093m3b3xwj23; Type: FK CONSTRAINT; Schema: public; Owner: - -- @@ -1728,6 +1792,22 @@ ALTER TABLE ONLY public."DomainHost" ADD CONSTRAINT fk_domainhost_host_valid FOREIGN KEY (host_repo_id) REFERENCES public."HostResource"(repo_id); +-- +-- Name: GracePeriod fk_grace_period_billing_event_id; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public."GracePeriod" + ADD CONSTRAINT fk_grace_period_billing_event_id FOREIGN KEY (billing_event_id) REFERENCES public."BillingEvent"(billing_event_id); + + +-- +-- Name: GracePeriod fk_grace_period_billing_recurrence_id; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public."GracePeriod" + ADD CONSTRAINT fk_grace_period_billing_recurrence_id FOREIGN KEY (billing_recurrence_id) REFERENCES public."BillingRecurrence"(billing_recurrence_id); + + -- -- Name: HostResource fk_host_resource_superordinate_domain; Type: FK CONSTRAINT; Schema: public; Owner: - --