mirrors/nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus synced 2026-08-15 11:46:08 +00:00
Code Issues Releases Wiki Activity

Use raw EntityManager to load during beforeSqlSave (#1253)

Browse Source 
If we use the transaction manager methods, JpaTransactionManagerImpl
will attempt to detach the EppResource in question that we're loading --
this fails because that entity has been saved in the same transaction
already. We don't need detaching during these methods (it's just for
resource population) so we can use the raw loads to get around it.
This commit is contained in:
gbrodman
2021-07-26 19:14:49 -04:00
committed by GitHub
parent c4c5ac85da
commit afa5a353f1
12 changed files with 158 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/src/test/java/google/registry/model/history/ContactHistoryTest.java
+37 -8
View File
@@ -18,6 +18,7 @@ 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.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.testing.DatabaseHelper.newContactResource;
import static google.registry.testing.DatabaseHelper.newContactResourceWithRoid;
import static google.registry.testing.SqlHelper.saveRegistrar;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -47,7 +48,7 @@ public class ContactHistoryTest extends EntityTestCase {
jpaTm().transact(() -> jpaTm().insert(contact));
VKey<ContactResource> contactVKey = contact.createVKey();
ContactResource contactFromDb = jpaTm().transact(() -> jpaTm().loadByKey(contactVKey));
ContactHistory contactHistory = createContactHistory(contactFromDb, contact.getRepoId());
ContactHistory contactHistory = createContactHistory(contactFromDb);
jpaTm().transact(() -> jpaTm().insert(contactHistory));
jpaTm()
.transact(
@@ -67,10 +68,7 @@ public class ContactHistoryTest extends EntityTestCase {
VKey<ContactResource> contactVKey = contact.createVKey();
ContactResource contactFromDb = jpaTm().transact(() -> jpaTm().loadByKey(contactVKey));
ContactHistory contactHistory =
createContactHistory(contactFromDb, contact.getRepoId())
.asBuilder()
.setContact(null)
.build();
createContactHistory(contactFromDb).asBuilder().setContact(null).build();
jpaTm().transact(() -> jpaTm().insert(contactHistory));
jpaTm()
@@ -91,7 +89,7 @@ public class ContactHistoryTest extends EntityTestCase {
VKey<ContactResource> contactVKey = contact.createVKey();
ContactResource contactFromDb = tm().transact(() -> tm().loadByKey(contactVKey));
fakeClock.advanceOneMilli();
ContactHistory contactHistory = createContactHistory(contactFromDb, contact.getRepoId());
ContactHistory contactHistory = createContactHistory(contactFromDb);
tm().transact(() -> tm().insert(contactHistory));
// retrieving a HistoryEntry or a ContactHistory with the same key should return the same object
@@ -106,7 +104,38 @@ public class ContactHistoryTest extends EntityTestCase {
assertThat(hostHistoryFromDb).isEqualTo(historyEntryFromDb);
}
private ContactHistory createContactHistory(ContactBase contact, String contactRepoId) {
@Test
void testBeforeSqlSave_afterContactPersisted() {
saveRegistrar("TheRegistrar");
ContactResource contactResource = newContactResource("contactId");
ContactHistory contactHistory =
new ContactHistory.Builder()
.setType(HistoryEntry.Type.HOST_CREATE)
.setXmlBytes("<xml></xml>".getBytes(UTF_8))
.setModificationTime(fakeClock.nowUtc())
.setClientId("TheRegistrar")
.setTrid(Trid.create("ABC-123", "server-trid"))
.setBySuperuser(false)
.setReason("reason")
.setRequestedByRegistrar(true)
.setContactRepoId(contactResource.getRepoId())
.build();
jpaTm()
.transact(
() -> {
jpaTm().put(contactResource);
contactHistory.beforeSqlSaveOnReplay();
jpaTm().put(contactHistory);
});
jpaTm()
.transact(
() ->
assertAboutImmutableObjects()
.that(jpaTm().loadByEntity(contactResource))
.hasFieldsEqualTo(jpaTm().loadByEntity(contactHistory).getContactBase().get()));
}
private ContactHistory createContactHistory(ContactBase contact) {
return new ContactHistory.Builder()
.setType(HistoryEntry.Type.HOST_CREATE)
.setXmlBytes("<xml></xml>".getBytes(UTF_8))
@@ -117,7 +146,7 @@ public class ContactHistoryTest extends EntityTestCase {
.setReason("reason")
.setRequestedByRegistrar(true)
.setContact(contact)
.setContactRepoId(contactRepoId)
.setContactRepoId(contact.getRepoId())
.build();
}
core/src/test/java/google/registry/model/history/DomainHistoryTest.java
+38
View File
@@ -190,6 +190,44 @@ public class DomainHistoryTest extends EntityTestCase {
jpaTm().transact(() -> jpaTm().put(domainHistoryFromDb2));
}
@TestSqlOnly
void testBeforeSqlSave_afterDomainPersisted() {
DomainBase domain = createDomainWithContactsAndHosts();
DomainHistory historyWithoutResource =
new DomainHistory.Builder()
.setType(HistoryEntry.Type.DOMAIN_CREATE)
.setXmlBytes("<xml></xml>".getBytes(UTF_8))
.setModificationTime(fakeClock.nowUtc())
.setClientId("TheRegistrar")
.setTrid(Trid.create("ABC-123", "server-trid"))
.setBySuperuser(false)
.setReason("reason")
.setRequestedByRegistrar(true)
.setDomainRepoId(domain.getRepoId())
.setOtherClientId("otherClient")
.setPeriod(Period.create(1, Period.Unit.YEARS))
.build();
jpaTm()
.transact(
() -> {
jpaTm()
.put(
domain
.asBuilder()
.setPersistedCurrentSponsorClientId("NewRegistrar")
.build());
historyWithoutResource.beforeSqlSaveOnReplay();
jpaTm().put(historyWithoutResource);
});
jpaTm()
.transact(
() ->
assertAboutImmutableObjects()
.that(jpaTm().loadByEntity(domain))
.hasFieldsEqualTo(
jpaTm().loadByEntity(historyWithoutResource).getDomainContent().get()));
}
static DomainBase createDomainWithContactsAndHosts() {
createTld("tld");
HostResource host = newHostResourceWithRoid("ns1.example.com", "host1");
core/src/test/java/google/registry/model/history/HostHistoryTest.java
+37 -6
View File
@@ -18,6 +18,7 @@ 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.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.testing.DatabaseHelper.newHostResource;
import static google.registry.testing.DatabaseHelper.newHostResourceWithRoid;
import static google.registry.testing.SqlHelper.saveRegistrar;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -48,7 +49,7 @@ public class HostHistoryTest extends EntityTestCase {
VKey<HostResource> hostVKey =
VKey.create(HostResource.class, "host1", Key.create(HostResource.class, "host1"));
HostResource hostFromDb = jpaTm().transact(() -> jpaTm().loadByKey(hostVKey));
HostHistory hostHistory = createHostHistory(hostFromDb, host.getRepoId());
HostHistory hostHistory = createHostHistory(hostFromDb);
jpaTm().transact(() -> jpaTm().insert(hostHistory));
jpaTm()
.transact(
@@ -66,8 +67,7 @@ public class HostHistoryTest extends EntityTestCase {
jpaTm().transact(() -> jpaTm().insert(host));
HostResource hostFromDb = jpaTm().transact(() -> jpaTm().loadByKey(host.createVKey()));
HostHistory hostHistory =
createHostHistory(hostFromDb, host.getRepoId()).asBuilder().setHost(null).build();
HostHistory hostHistory = createHostHistory(hostFromDb).asBuilder().setHost(null).build();
jpaTm().transact(() -> jpaTm().insert(hostHistory));
jpaTm()
@@ -88,7 +88,7 @@ public class HostHistoryTest extends EntityTestCase {
VKey<HostResource> hostVKey =
VKey.create(HostResource.class, "host1", Key.create(HostResource.class, "host1"));
HostResource hostFromDb = tm().transact(() -> tm().loadByKey(hostVKey));
HostHistory hostHistory = createHostHistory(hostFromDb, host.getRepoId());
HostHistory hostHistory = createHostHistory(hostFromDb);
fakeClock.advanceOneMilli();
tm().transact(() -> tm().insert(hostHistory));
@@ -104,6 +104,37 @@ public class HostHistoryTest extends EntityTestCase {
assertThat(hostHistoryFromDb).isEqualTo(historyEntryFromDb);
}
@Test
void testBeforeSqlSave_afterHostPersisted() {
saveRegistrar("TheRegistrar");
HostResource hostResource = newHostResource("ns1.example.tld");
HostHistory hostHistory =
new HostHistory.Builder()
.setType(HistoryEntry.Type.HOST_CREATE)
.setXmlBytes("<xml></xml>".getBytes(UTF_8))
.setModificationTime(fakeClock.nowUtc())
.setClientId("TheRegistrar")
.setTrid(Trid.create("ABC-123", "server-trid"))
.setBySuperuser(false)
.setReason("reason")
.setRequestedByRegistrar(true)
.setHostRepoId(hostResource.getRepoId())
.build();
jpaTm()
.transact(
() -> {
jpaTm().put(hostResource);
hostHistory.beforeSqlSaveOnReplay();
jpaTm().put(hostHistory);
});
jpaTm()
.transact(
() ->
assertAboutImmutableObjects()
.that(jpaTm().loadByEntity(hostResource))
.hasFieldsEqualTo(jpaTm().loadByEntity(hostHistory).getHostBase().get()));
}
private void assertHostHistoriesEqual(HostHistory one, HostHistory two) {
assertAboutImmutableObjects().that(one).isEqualExceptFields(two, "hostBase");
assertAboutImmutableObjects()
@@ -111,7 +142,7 @@ public class HostHistoryTest extends EntityTestCase {
.isEqualExceptFields(two.getHostBase().orElse(null), "repoId");
}
private HostHistory createHostHistory(HostBase hostBase, String hostRepoId) {
private HostHistory createHostHistory(HostBase hostBase) {
return new HostHistory.Builder()
.setType(HistoryEntry.Type.HOST_CREATE)
.setXmlBytes("<xml></xml>".getBytes(UTF_8))
@@ -122,7 +153,7 @@ public class HostHistoryTest extends EntityTestCase {
.setReason("reason")
.setRequestedByRegistrar(true)
.setHost(hostBase)
.setHostRepoId(hostRepoId)
.setHostRepoId(hostBase.getRepoId())
.build();
}
}
core/src/test/java/google/registry/model/history/LegacyHistoryObjectTest.java
+4 -12
View File
@@ -74,7 +74,7 @@ public class LegacyHistoryObjectTest extends EntityTestCase {
assertAboutImmutableObjects()
.that(legacyHistoryEntry)
.isEqualExceptFields(fromObjectify, "contactBase", "contactRepoId");
assertThat(fromObjectify instanceof ContactHistory).isTrue();
assertThat(fromObjectify).isInstanceOf(ContactHistory.class);
ContactHistory legacyContactHistory = (ContactHistory) fromObjectify;
// Next, save that from-Datastore object in SQL and verify we can load it back in
@@ -124,7 +124,7 @@ public class LegacyHistoryObjectTest extends EntityTestCase {
"nsHosts",
"dsDataHistories",
"gracePeriodHistories");
assertThat(fromObjectify instanceof DomainHistory).isTrue();
assertThat(fromObjectify).isInstanceOf(DomainHistory.class);
DomainHistory legacyDomainHistory = (DomainHistory) fromObjectify;
// Next, save that from-Datastore object in SQL and verify we can load it back in
@@ -137,15 +137,7 @@ public class LegacyHistoryObjectTest extends EntityTestCase {
// Don't compare nsHosts directly because one is null and the other is empty
assertAboutImmutableObjects()
.that(legacyDomainHistory)
.isEqualExceptFields(
// NB: period, transaction records, and other client ID are added in #794
legacyHistoryFromSql,
"period",
"domainTransactionRecords",
"otherClientId",
"nsHosts",
"dsDataHistories",
"gracePeriodHistories");
.isEqualExceptFields(legacyHistoryFromSql, "nsHosts");
assertThat(nullToEmpty(legacyDomainHistory.getNsHosts()))
.isEqualTo(nullToEmpty(legacyHistoryFromSql.getNsHosts()));
});
@@ -174,7 +166,7 @@ public class LegacyHistoryObjectTest extends EntityTestCase {
assertAboutImmutableObjects()
.that(legacyHistoryEntry)
.isEqualExceptFields(fromObjectify, "hostBase", "hostRepoId");
assertThat(fromObjectify instanceof HostHistory).isTrue();
assertThat(fromObjectify).isInstanceOf(HostHistory.class);
HostHistory legacyHostHistory = (HostHistory) fromObjectify;
// Next, save that from-Datastore object in SQL and verify we can load it back in
core/src/test/java/google/registry/testing/TestObject.java
+2 -1
View File
@@ -75,7 +75,8 @@ public class TestObject extends ImmutableObject implements DatastoreAndSqlEntity
beforeSqlDeleteCallCount++;
}
public static void beforeSqlSave(TestObject testObject) {
@Override
public void beforeSqlSaveOnReplay() {
beforeSqlSaveCallCount++;
}