From a313b744a059590fa2b25e136b72f465538a9e6d Mon Sep 17 00:00:00 2001 From: Michael Muller Date: Tue, 24 Nov 2020 11:42:00 -0500 Subject: [PATCH] Fix DomainHistory merge issues (#884) * Reproduce DomainHistory double write failure * Add fix for cascade sets and clean up hacks * Fix DatastoreHelper to work with name change. * Remove Ignored entities from ofy schema --- .../registry/model/domain/DomainHistory.java | 6 +- .../model/history/DomainHistoryTest.java | 72 ++++++++++++++++++- .../google/registry/model/schema.txt | 20 ------ 3 files changed, 75 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/google/registry/model/domain/DomainHistory.java b/core/src/main/java/google/registry/model/domain/DomainHistory.java index bf9c09d69..6eac53689 100644 --- a/core/src/main/java/google/registry/model/domain/DomainHistory.java +++ b/core/src/main/java/google/registry/model/domain/DomainHistory.java @@ -101,6 +101,7 @@ public class DomainHistory extends HistoryEntry implements SqlEntity { @Column(name = "host_repo_id") Set> nsHosts; + @Ignore @OneToMany( cascade = {CascadeType.ALL}, fetch = FetchType.EAGER, @@ -117,8 +118,9 @@ public class DomainHistory extends HistoryEntry implements SqlEntity { insertable = false, updatable = false) }) - Set dsDataHistories; + Set dsDataHistories = ImmutableSet.of(); + @Ignore @OneToMany( cascade = {CascadeType.ALL}, fetch = FetchType.EAGER, @@ -135,7 +137,7 @@ public class DomainHistory extends HistoryEntry implements SqlEntity { insertable = false, updatable = false) }) - Set gracePeriodHistories; + Set gracePeriodHistories = ImmutableSet.of(); @Override @Nullable diff --git a/core/src/test/java/google/registry/model/history/DomainHistoryTest.java b/core/src/test/java/google/registry/model/history/DomainHistoryTest.java index 020542020..f786d4bfa 100644 --- a/core/src/test/java/google/registry/model/history/DomainHistoryTest.java +++ b/core/src/test/java/google/registry/model/history/DomainHistoryTest.java @@ -18,6 +18,7 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.truth.Truth.assertThat; import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects; import static google.registry.model.ImmutableObjectSubject.immutableObjectCorrespondence; +import static google.registry.model.registry.Registry.TldState.GENERAL_AVAILABILITY; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.createTld; @@ -25,9 +26,11 @@ import static google.registry.testing.DatabaseHelper.newContactResourceWithRoid; import static google.registry.testing.DatabaseHelper.newDomainBase; import static google.registry.testing.DatabaseHelper.newHostResourceWithRoid; import static google.registry.util.DateTimeUtils.END_OF_TIME; +import static google.registry.util.DateTimeUtils.START_OF_TIME; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableSortedMap; import com.googlecode.objectify.Key; import google.registry.model.EntityTestCase; import google.registry.model.contact.ContactResource; @@ -40,10 +43,14 @@ import google.registry.model.domain.rgp.GracePeriodStatus; import google.registry.model.domain.secdns.DelegationSignerData; import google.registry.model.eppcommon.Trid; import google.registry.model.host.HostResource; +import google.registry.model.registrar.Registrar; +import google.registry.model.registry.Registries; +import google.registry.model.registry.Registry; import google.registry.model.reporting.DomainTransactionRecord; import google.registry.model.reporting.DomainTransactionRecord.TransactionReportField; import google.registry.model.reporting.HistoryEntry; import google.registry.persistence.VKey; +import google.registry.testing.DatabaseHelper; import google.registry.testing.DualDatabaseTest; import google.registry.testing.TestOfyOnly; import google.registry.testing.TestSqlOnly; @@ -115,7 +122,8 @@ public class DomainHistoryTest extends EntityTestCase { DomainHistory domainHistory = createDomainHistory(domain); tm().transact(() -> tm().insert(domainHistory)); - // retrieving a HistoryEntry or a DomainHistory with the same key should return the same object + // retrieving a HistoryEntry or a DomainHistory with the same key should return the same + // object // note: due to the @EntitySubclass annotation. all Keys for DomainHistory objects will have // type HistoryEntry VKey domainHistoryVKey = domainHistory.createVKey(); @@ -127,6 +135,68 @@ public class DomainHistoryTest extends EntityTestCase { assertThat(domainHistoryFromDb).isEqualTo(historyEntryFromDb); } + @TestOfyOnly + void testDoubleWriteOfOfyResource() { + // We have to add the registry to ofy, since we're currently loading the cache from ofy. We + // also have to add it to SQL to satisfy the foreign key constraints of the registrar. + Registry registry = + DatabaseHelper.newRegistry( + "tld", "TLD", ImmutableSortedMap.of(START_OF_TIME, GENERAL_AVAILABILITY)); + tm().transact(() -> tm().insert(registry)); + Registries.resetCache(); + jpaTm() + .transact( + () -> { + jpaTm().insert(registry); + Registrar registrar = + appEngine + .makeRegistrar2() + .asBuilder() + .setAllowedTlds(ImmutableSet.of("tld")) + .build(); + jpaTm().insert(registrar); + }); + + HostResource host = newHostResourceWithRoid("ns1.example.com", "host1"); + ContactResource contact = newContactResourceWithRoid("contactId", "contact1"); + + // Set up the host and domain objects in both databases. + tm().transact( + () -> { + tm().insert(host); + tm().insert(contact); + }); + jpaTm() + .transact( + () -> { + jpaTm().insert(host); + jpaTm().insert(contact); + }); + fakeClock.advanceOneMilli(); + DomainBase domain = + newDomainBase("example.tld", "domainRepoId", contact) + .asBuilder() + .setNameservers(host.createVKey()) + .build(); + tm().transact(() -> tm().insert(domain)); + jpaTm().transact(() -> jpaTm().insert(domain)); + fakeClock.advanceOneMilli(); + + DomainHistory domainHistory = createDomainHistory(domain); + tm().transact(() -> tm().insert(domainHistory)); + + // Load the DomainHistory object from the datastore. + VKey domainHistoryVKey = domainHistory.createVKey(); + DomainHistory domainHistoryFromDb = tm().transact(() -> tm().load(domainHistoryVKey)); + + // attempt to write to SQL. + jpaTm().transact(() -> jpaTm().insert(domainHistoryFromDb)); + + // Reload and rewrite. + DomainHistory domainHistoryFromDb2 = tm().transact(() -> tm().load(domainHistoryVKey)); + jpaTm().transact(() -> jpaTm().put(domainHistoryFromDb2)); + } + static DomainBase createDomainWithContactsAndHosts() { createTld("tld"); HostResource host = newHostResourceWithRoid("ns1.example.com", "host1"); diff --git a/core/src/test/resources/google/registry/model/schema.txt b/core/src/test/resources/google/registry/model/schema.txt index cb16afffd..4543727ff 100644 --- a/core/src/test/resources/google/registry/model/schema.txt +++ b/core/src/test/resources/google/registry/model/schema.txt @@ -273,8 +273,6 @@ class google.registry.model.domain.DomainHistory { java.lang.String clientId; java.lang.String otherClientId; java.lang.String reason; - java.util.Set gracePeriodHistories; - java.util.Set dsDataHistories; java.util.Set domainTransactionRecords; org.joda.time.DateTime modificationTime; } @@ -286,16 +284,6 @@ class google.registry.model.domain.GracePeriod { java.lang.String clientId; org.joda.time.DateTime expirationTime; } -class google.registry.model.domain.GracePeriod$GracePeriodHistory { - google.registry.model.domain.rgp.GracePeriodStatus type; - google.registry.persistence.VKey billingEventOneTime; - google.registry.persistence.VKey billingEventRecurring; - java.lang.Long domainHistoryRevisionId; - java.lang.Long gracePeriodHistoryRevisionId; - java.lang.Long gracePeriodId; - java.lang.String clientId; - org.joda.time.DateTime expirationTime; -} class google.registry.model.domain.Period { google.registry.model.domain.Period$Unit unit; java.lang.Integer value; @@ -328,14 +316,6 @@ class google.registry.model.domain.secdns.DelegationSignerData { int digestType; int keyTag; } -class google.registry.model.domain.secdns.DomainDsDataHistory { - byte[] digest; - int algorithm; - int digestType; - int keyTag; - java.lang.Long domainHistoryRevisionId; - java.lang.Long dsDataHistoryRevisionId; -} class google.registry.model.domain.token.AllocationToken { @Id java.lang.String token; boolean discountPremiums;