Remove orphaned DomainTransactionRecords (#1444)

This is what's causing https://b.corp.google.com/issues/208274109, where
there are DTR rows with null foreign key values.

We should probably wait to make the columns officially non-null until we
get this in and verify that we can do so.
This commit is contained in:
gbrodman 2021-12-02 16:41:54 -05:00 committed by GitHub
parent f5d9ee4e4d
commit 6006e253a4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 22 additions and 43 deletions

View file

@ -196,7 +196,8 @@ public class DomainHistory extends HistoryEntry implements SqlEntity {
@Access(AccessType.PROPERTY)
@OneToMany(
cascade = {CascadeType.ALL},
fetch = FetchType.EAGER)
fetch = FetchType.EAGER,
orphanRemoval = true)
@JoinColumn(name = "historyRevisionId", referencedColumnName = "historyRevisionId")
@JoinColumn(name = "domainRepoId", referencedColumnName = "domainRepoId")
@SuppressWarnings("unused")

View file

@ -95,22 +95,21 @@ public class ReplayQueue {
// Sort the changes into an order that will work for insertion into the database.
jpaTm()
.transact(
() -> {
changes.entrySet().stream()
.sorted(ReplayQueue::compareByPriority)
.forEach(
entry -> {
if (entry.getValue().equals(TransactionInfo.Delete.SENTINEL)) {
VKey<?> vkey = VKey.from(entry.getKey());
ReplaySpecializer.beforeSqlDelete(vkey);
jpaTm().delete(vkey);
} else {
((DatastoreEntity) entry.getValue())
.toSqlEntity()
.ifPresent(jpaTm()::put);
}
});
});
() ->
changes.entrySet().stream()
.sorted(ReplayQueue::compareByPriority)
.forEach(
entry -> {
if (entry.getValue().equals(TransactionInfo.Delete.SENTINEL)) {
VKey<?> vkey = VKey.from(entry.getKey());
ReplaySpecializer.beforeSqlDelete(vkey);
jpaTm().delete(vkey);
} else {
((DatastoreEntity) entry.getValue())
.toSqlEntity()
.ifPresent(jpaTm()::put);
}
}));
}
}
}

View file

@ -1087,13 +1087,6 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, Domain
null));
setUpGracePeriodDurations();
clock.advanceOneMilli();
earlierHistoryEntry =
persistResource(
earlierHistoryEntry
.asBuilder()
.setType(DOMAIN_CREATE)
.setModificationTime(TIME_BEFORE_FLOW.minusDays(2))
.build());
runFlow();
HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_DELETE);
// Transaction record should just be the grace period delete

View file

@ -16,7 +16,6 @@ package google.registry.model.history;
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.tld.Registry.TldState.GENERAL_AVAILABILITY;
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
@ -326,17 +325,10 @@ public class DomainHistoryTest extends EntityTestCase {
}
static void assertDomainHistoriesEqual(DomainHistory one, DomainHistory two) {
assertAboutImmutableObjects().that(one).isEqualExceptFields(two, "domainContent");
assertAboutImmutableObjects()
.that(one)
.isEqualExceptFields(
two, "domainContent", "domainRepoId", "nsHosts", "domainTransactionRecords");
assertThat(one.getDomainContent().map(DomainContent::getDomainName))
.isEqualTo(two.getDomainContent().map(DomainContent::getDomainName));
// NB: the record's ID gets reset by Hibernate, causing the hash code to differ so we have to
// compare it separately
assertThat(one.getDomainTransactionRecords())
.comparingElementsUsing(immutableObjectCorrespondence())
.containsExactlyElementsIn(two.getDomainTransactionRecords());
.that(one.getDomainContent().get())
.isEqualExceptFields(two.getDomainContent().get(), "updateTimestamp");
}
private DomainHistory createDomainHistory(DomainContent domain) {

View file

@ -25,7 +25,6 @@ import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.Assert.assertThrows;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import google.registry.model.EntityTestCase;
import google.registry.model.contact.ContactHistory;
import google.registry.model.domain.DomainBase;
@ -79,15 +78,10 @@ class HistoryEntryTest extends EntityTestCase {
void testPersistence() {
transactIfJpaTm(
() -> {
HistoryEntry fromDatabase = tm().loadByEntity(domainHistory);
DomainHistory fromDatabase = tm().loadByEntity(domainHistory);
assertAboutImmutableObjects()
.that(fromDatabase)
.isEqualExceptFields(
domainHistory, "nsHosts", "domainTransactionRecords", "domainContent");
assertAboutImmutableObjects()
.that(Iterables.getOnlyElement(fromDatabase.getDomainTransactionRecords()))
.isEqualExceptFields(
Iterables.getOnlyElement(domainHistory.getDomainTransactionRecords()), "id");
.isEqualExceptFields(domainHistory, "domainContent");
});
}