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 0565f3161..285c311f9 100644
--- a/core/src/main/java/google/registry/model/domain/DomainBase.java
+++ b/core/src/main/java/google/registry/model/domain/DomainBase.java
@@ -14,7 +14,6 @@
package google.registry.model.domain;
-
import com.googlecode.objectify.Key;
import google.registry.model.EppResource;
import google.registry.model.EppResource.ForeignKeyedEppResource;
@@ -82,12 +81,26 @@ public class DomainBase extends DomainContent
return super.nsHosts;
}
+ /**
+ * Returns the set of {@link GracePeriod} associated with the domain.
+ *
+ * This is the getter method specific for Hibernate to access the field so it is set to
+ * private. The caller can use the public {@link #getGracePeriods()} to get the grace periods.
+ *
+ *
Note that we need to set `insertable = false, updatable = false` for @JoinColumn, otherwise
+ * Hibernate would try to set the foreign key to null(through an UPDATE TABLE sql) instead of
+ * deleting the whole entry from the table when the {@link GracePeriod} is removed from the set.
+ */
@Access(AccessType.PROPERTY)
@OneToMany(
cascade = {CascadeType.ALL},
fetch = FetchType.EAGER,
orphanRemoval = true)
- @JoinColumn(name = "domainRepoId", referencedColumnName = "repoId")
+ @JoinColumn(
+ name = "domainRepoId",
+ referencedColumnName = "repoId",
+ insertable = false,
+ updatable = false)
@SuppressWarnings("UnusedMethod")
private Set getInternalGracePeriods() {
return gracePeriods;
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 3c8143d22..32ecd0071 100644
--- a/core/src/main/java/google/registry/model/domain/DomainContent.java
+++ b/core/src/main/java/google/registry/model/domain/DomainContent.java
@@ -280,12 +280,10 @@ public class DomainContent extends EppResource
// 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());
- }
+ gracePeriods =
+ nullToEmptyImmutableCopy(gracePeriods).stream()
+ .map(gracePeriod -> gracePeriod.cloneWithDomainRepoId(getRepoId()))
+ .collect(toImmutableSet());
}
@PostLoad
@@ -698,7 +696,13 @@ public class DomainContent extends EppResource
}
checkArgumentNotNull(instance.getRegistrant(), "Missing registrant");
instance.tld = getTldFromDomainName(instance.fullyQualifiedDomainName);
- return super.build();
+
+ T newDomain = super.build();
+ // Hibernate throws exception if gracePeriods is null because we enabled all cascadable
+ // operations and orphan removal.
+ newDomain.gracePeriods =
+ newDomain.gracePeriods == null ? ImmutableSet.of() : newDomain.gracePeriods;
+ return newDomain;
}
public B setDomainName(String domainName) {
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 a6ecd6fab..1957534a0 100644
--- a/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java
+++ b/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java
@@ -14,6 +14,7 @@
package google.registry.model.domain;
+import static com.google.common.truth.Truth.assertThat;
import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects;
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import static google.registry.testing.SqlHelper.assertThrowForeignKeyViolation;
@@ -21,6 +22,7 @@ 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;
+import static org.junit.jupiter.api.Assertions.fail;
import com.google.common.collect.ImmutableSet;
import google.registry.model.contact.ContactResource;
@@ -37,7 +39,7 @@ import google.registry.persistence.transaction.JpaTestRules;
import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageExtension;
import google.registry.testing.DatastoreEntityExtension;
import google.registry.testing.FakeClock;
-import javax.persistence.EntityManager;
+import java.util.Arrays;
import org.joda.time.DateTime;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Order;
@@ -119,77 +121,217 @@ public class DomainBaseSqlTest {
@Test
void testDomainBasePersistence() {
- jpaTm()
- .transact(
- () -> {
- // Persist the contacts. Note that these need to be persisted before the domain
- // otherwise we get a foreign key constraint error. If we ever decide to defer the
- // relevant foreign key checks to commit time, then the order would not matter.
- jpaTm().saveNew(contact);
- jpaTm().saveNew(contact2);
-
- // Persist the domain.
- jpaTm().saveNew(domain);
-
- // Persist the host. This does _not_ need to be persisted before the domain,
- // because only the row in the join table (DomainHost) is subject to foreign key
- // constraints, and Hibernate knows to insert it after domain and host.
- jpaTm().saveNew(host);
- });
+ persistDomain();
jpaTm()
.transact(
() -> {
- // Load the domain in its entirety.
- EntityManager em = jpaTm().getEntityManager();
- DomainBase result = em.find(DomainBase.class, "4-COM");
-
- // Fix DS data, since we can't persist it yet.
- result =
- result
- .asBuilder()
- .setDsData(
- ImmutableSet.of(
- DelegationSignerData.create(1, 2, 3, new byte[] {0, 1, 2})))
- .build();
-
- // Fix the original creation timestamp (this gets initialized on first write)
- DomainBase org = domain.asBuilder().setCreationTime(result.getCreationTime()).build();
-
- // Note that the equality comparison forces a lazy load of all fields.
- assertAboutImmutableObjects()
- .that(result)
- .isEqualExceptFields(org, "updateTimestamp");
+ DomainBase result = jpaTm().load(domain.createVKey());
+ assertEqualDomainExcept(result);
});
}
@Test
void testHostForeignKeyConstraints() {
assertThrowForeignKeyViolation(
- () -> {
- jpaTm()
- .transact(
- () -> {
- // Persist the domain without the associated host object.
- jpaTm().saveNew(contact);
- jpaTm().saveNew(contact2);
- jpaTm().saveNew(domain);
- });
- });
+ () ->
+ jpaTm()
+ .transact(
+ () -> {
+ // Persist the domain without the associated host object.
+ jpaTm().saveNew(contact);
+ jpaTm().saveNew(contact2);
+ jpaTm().saveNew(domain);
+ }));
}
@Test
void testContactForeignKeyConstraints() {
assertThrowForeignKeyViolation(
- () -> {
- jpaTm()
- .transact(
- () -> {
- // Persist the domain without the associated contact objects.
- jpaTm().saveNew(domain);
- jpaTm().saveNew(host);
- });
- });
+ () ->
+ jpaTm()
+ .transact(
+ () -> {
+ // Persist the domain without the associated contact objects.
+ jpaTm().saveNew(domain);
+ jpaTm().saveNew(host);
+ }));
+ }
+
+ @Test
+ void testResaveDomain_succeeds() {
+ persistDomain();
+ jpaTm()
+ .transact(
+ () -> {
+ DomainBase persisted = jpaTm().load(domain.createVKey());
+ jpaTm().saveNewOrUpdate(persisted.asBuilder().build());
+ });
+ jpaTm()
+ .transact(
+ () -> {
+ // Load the domain in its entirety.
+ DomainBase result = jpaTm().load(domain.createVKey());
+ assertEqualDomainExcept(result);
+ });
+ }
+
+ @Test
+ void testModifyGracePeriod_setEmptyCollectionSuccessfully() {
+ persistDomain();
+ jpaTm()
+ .transact(
+ () -> {
+ DomainBase persisted = jpaTm().load(domain.createVKey());
+ DomainBase modified =
+ persisted.asBuilder().setGracePeriods(ImmutableSet.of()).build();
+ jpaTm().saveNewOrUpdate(modified);
+ });
+
+ jpaTm()
+ .transact(
+ () -> {
+ DomainBase persisted = jpaTm().load(domain.createVKey());
+ assertThat(persisted.getGracePeriods()).isEmpty();
+ });
+ }
+
+ @Test
+ void testModifyGracePeriod_setNullCollectionSuccessfully() {
+ persistDomain();
+ jpaTm()
+ .transact(
+ () -> {
+ DomainBase persisted = jpaTm().load(domain.createVKey());
+ DomainBase modified = persisted.asBuilder().setGracePeriods(null).build();
+ jpaTm().saveNewOrUpdate(modified);
+ });
+
+ jpaTm()
+ .transact(
+ () -> {
+ DomainBase persisted = jpaTm().load(domain.createVKey());
+ assertThat(persisted.getGracePeriods()).isEmpty();
+ });
+ }
+
+ @Test
+ void testModifyGracePeriod_addThenRemoveSuccessfully() {
+ persistDomain();
+ jpaTm()
+ .transact(
+ () -> {
+ DomainBase persisted = jpaTm().load(domain.createVKey());
+ DomainBase modified =
+ persisted
+ .asBuilder()
+ .addGracePeriod(
+ GracePeriod.create(
+ GracePeriodStatus.RENEW, "4-COM", END_OF_TIME, "registrar1", null))
+ .build();
+ jpaTm().saveNewOrUpdate(modified);
+ });
+
+ jpaTm()
+ .transact(
+ () -> {
+ DomainBase persisted = jpaTm().load(domain.createVKey());
+ assertThat(persisted.getGracePeriods().size()).isEqualTo(2);
+ persisted
+ .getGracePeriods()
+ .forEach(
+ gracePeriod -> {
+ assertThat(gracePeriod.id).isNotNull();
+ if (gracePeriod.getType() == GracePeriodStatus.ADD) {
+ assertAboutImmutableObjects()
+ .that(gracePeriod)
+ .isEqualExceptFields(
+ GracePeriod.create(
+ GracePeriodStatus.ADD,
+ "4-COM",
+ END_OF_TIME,
+ "registrar1",
+ null),
+ "id");
+ } else if (gracePeriod.getType() == GracePeriodStatus.RENEW) {
+ assertAboutImmutableObjects()
+ .that(gracePeriod)
+ .isEqualExceptFields(
+ GracePeriod.create(
+ GracePeriodStatus.RENEW,
+ "4-COM",
+ END_OF_TIME,
+ "registrar1",
+ null),
+ "id");
+ } else {
+ fail("Unexpected GracePeriod: " + gracePeriod);
+ }
+ });
+ assertEqualDomainExcept(persisted, "gracePeriods");
+ });
+
+ jpaTm()
+ .transact(
+ () -> {
+ DomainBase persisted = jpaTm().load(domain.createVKey());
+ DomainBase.Builder builder = persisted.asBuilder();
+ for (GracePeriod gracePeriod : persisted.getGracePeriods()) {
+ if (gracePeriod.getType() == GracePeriodStatus.RENEW) {
+ builder.removeGracePeriod(gracePeriod);
+ }
+ }
+ jpaTm().saveNewOrUpdate(builder.build());
+ });
+
+ jpaTm()
+ .transact(
+ () -> {
+ DomainBase persisted = jpaTm().load(domain.createVKey());
+ assertEqualDomainExcept(persisted);
+ });
+ }
+
+ @Test
+ void testModifyGracePeriod_removeThenAddSuccessfully() {
+ persistDomain();
+ jpaTm()
+ .transact(
+ () -> {
+ DomainBase persisted = jpaTm().load(domain.createVKey());
+ DomainBase modified =
+ persisted.asBuilder().setGracePeriods(ImmutableSet.of()).build();
+ jpaTm().saveNewOrUpdate(modified);
+ });
+
+ jpaTm()
+ .transact(
+ () -> {
+ DomainBase persisted = jpaTm().load(domain.createVKey());
+ assertThat(persisted.getGracePeriods()).isEmpty();
+ DomainBase modified =
+ persisted
+ .asBuilder()
+ .addGracePeriod(
+ GracePeriod.create(
+ GracePeriodStatus.ADD, "4-COM", END_OF_TIME, "registrar1", null))
+ .build();
+ jpaTm().saveNewOrUpdate(modified);
+ });
+
+ jpaTm()
+ .transact(
+ () -> {
+ DomainBase persisted = jpaTm().load(domain.createVKey());
+ assertThat(persisted.getGracePeriods().size()).isEqualTo(1);
+ assertAboutImmutableObjects()
+ .that(persisted.getGracePeriods().iterator().next())
+ .isEqualExceptFields(
+ GracePeriod.create(
+ GracePeriodStatus.ADD, "4-COM", END_OF_TIME, "registrar1", null),
+ "id");
+ assertEqualDomainExcept(persisted, "gracePeriods");
+ });
}
@Test
@@ -232,4 +374,42 @@ public class DomainBaseSqlTest {
.setPersistedCurrentSponsorClientId("registrar1")
.build();
}
+
+ private void persistDomain() {
+ jpaTm()
+ .transact(
+ () -> {
+ // Persist the contacts. Note that these need to be persisted before the domain
+ // otherwise we get a foreign key constraint error. If we ever decide to defer the
+ // relevant foreign key checks to commit time, then the order would not matter.
+ jpaTm().saveNew(contact);
+ jpaTm().saveNew(contact2);
+
+ // Persist the domain.
+ jpaTm().saveNew(domain);
+
+ // Persist the host. This does _not_ need to be persisted before the domain,
+ // because only the row in the join table (DomainHost) is subject to foreign key
+ // constraints, and Hibernate knows to insert it after domain and host.
+ jpaTm().saveNew(host);
+ });
+ }
+
+ private void assertEqualDomainExcept(DomainBase thatDomain, String... excepts) {
+ // Fix DS data, since we can't persist it yet.
+ thatDomain =
+ thatDomain
+ .asBuilder()
+ .setDsData(ImmutableSet.of(DelegationSignerData.create(1, 2, 3, new byte[] {0, 1, 2})))
+ .build();
+
+ // Fix the original creation timestamp (this gets initialized on first write)
+ DomainBase org = domain.asBuilder().setCreationTime(thatDomain.getCreationTime()).build();
+
+ String[] moreExcepts = Arrays.copyOf(excepts, excepts.length + 1);
+ moreExcepts[moreExcepts.length - 1] = "updateTimestamp";
+
+ // Note that the equality comparison forces a lazy load of all fields.
+ assertAboutImmutableObjects().that(thatDomain).isEqualExceptFields(org, moreExcepts);
+ }
}