Create *InfoData objects instead of reusing *Resource objects

This is probably best from a code-cleanliness perspective anyways,
but the rationale is that tightly coupling the resources to the
info responses was a straightjacket that required all status
values and fields to be directly available on the resource. With
this change, I already was able to get rid of the preMarshal()
hackery, and I will be able to get rid of cloneWithLinkedStatus()
and most of the contents of cloneProjectedAtTime() for non-domains.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=144252924
This commit is contained in:
cgoldfeder 2017-01-11 14:49:10 -08:00 committed by Ben McIlwain
parent 96a71ded91
commit b0bcc1bb3d
22 changed files with 586 additions and 230 deletions

View file

@ -14,14 +14,24 @@
package google.registry.flows.domain;
import static com.google.appengine.tools.development.testing.LocalMemcacheServiceTestConfig.getLocalMemcacheService;
import static com.google.common.base.Predicates.equalTo;
import static com.google.common.io.BaseEncoding.base16;
import static com.google.common.truth.Truth.assertThat;
import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.testing.DatastoreHelper.assertNoBillingEvents;
import static google.registry.testing.DatastoreHelper.createTld;
import static google.registry.testing.DatastoreHelper.persistActiveContact;
import static google.registry.testing.DatastoreHelper.persistActiveHost;
import static google.registry.testing.DatastoreHelper.persistResource;
import static google.registry.testing.TestDataHelper.loadFileWithSubstitutions;
import static google.registry.util.DatastoreServiceUtils.KEY_TO_KIND_FUNCTION;
import com.google.appengine.api.memcache.MemcacheServicePb.MemcacheFlushRequest;
import com.google.appengine.tools.development.LocalRpcService;
import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@ -44,10 +54,12 @@ import google.registry.model.domain.secdns.DelegationSignerData;
import google.registry.model.eppcommon.AuthInfo.PasswordAuth;
import google.registry.model.eppcommon.StatusValue;
import google.registry.model.host.HostResource;
import google.registry.model.ofy.RequestCapturingAsyncDatastoreService;
import google.registry.model.registry.Registry.TldState;
import google.registry.model.smd.EncodedSignedMark;
import google.registry.testing.AppEngineRule;
import google.registry.testing.EppLoader;
import java.util.List;
import org.joda.time.DateTime;
import org.junit.Before;
import org.junit.Test;
@ -317,4 +329,32 @@ public class DomainApplicationInfoFlowTest
thrown.expect(ApplicationLaunchPhaseMismatchException.class);
runFlow();
}
/** Test that we load contacts and hosts as a batch rather than individually. */
@Test
public void testBatchLoadingOfReferences() throws Exception {
persistTestEntities(HostsState.HOSTS_EXIST, MarksState.NO_MARKS_EXIST);
// Clear out memcache and session cache so that we count actual datastore calls.
ofy().clearSessionCache();
getLocalMemcacheService().flushAll(
new LocalRpcService.Status(), MemcacheFlushRequest.newBuilder().build());
int numPreviousReads = RequestCapturingAsyncDatastoreService.getReads().size();
doSuccessfulTest("domain_info_sunrise_response.xml", HostsState.HOSTS_EXIST);
// Get all of the keys loaded in the flow, with each distinct load() call as a list of keys.
int numReadsWithContactsOrHosts = FluentIterable
.from(RequestCapturingAsyncDatastoreService.getReads())
.skip(numPreviousReads)
.filter(
new Predicate<List<com.google.appengine.api.datastore.Key>>() {
@Override
public boolean apply(List<com.google.appengine.api.datastore.Key> keys) {
return FluentIterable.from(keys)
.transform(KEY_TO_KIND_FUNCTION)
.anyMatch(Predicates.or(
equalTo(Key.getKind(ContactResource.class)),
equalTo(Key.getKind(HostResource.class))));
}})
.size();
assertThat(numReadsWithContactsOrHosts).isEqualTo(1);
}
}

View file

@ -14,7 +14,11 @@
package google.registry.flows.domain;
import static com.google.appengine.tools.development.testing.LocalMemcacheServiceTestConfig.getLocalMemcacheService;
import static com.google.common.base.Predicates.equalTo;
import static com.google.common.io.BaseEncoding.base16;
import static com.google.common.truth.Truth.assertThat;
import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.testing.DatastoreHelper.assertNoBillingEvents;
import static google.registry.testing.DatastoreHelper.createTld;
import static google.registry.testing.DatastoreHelper.newDomainResource;
@ -22,7 +26,13 @@ import static google.registry.testing.DatastoreHelper.persistActiveContact;
import static google.registry.testing.DatastoreHelper.persistActiveHost;
import static google.registry.testing.DatastoreHelper.persistResource;
import static google.registry.testing.TestDataHelper.loadFileWithSubstitutions;
import static google.registry.util.DatastoreServiceUtils.KEY_TO_KIND_FUNCTION;
import com.google.appengine.api.memcache.MemcacheServicePb.MemcacheFlushRequest;
import com.google.appengine.tools.development.LocalRpcService;
import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.googlecode.objectify.Key;
@ -46,7 +56,9 @@ import google.registry.model.domain.secdns.DelegationSignerData;
import google.registry.model.eppcommon.AuthInfo.PasswordAuth;
import google.registry.model.eppcommon.StatusValue;
import google.registry.model.host.HostResource;
import google.registry.model.ofy.RequestCapturingAsyncDatastoreService;
import google.registry.testing.AppEngineRule;
import java.util.List;
import org.joda.time.DateTime;
import org.junit.Before;
import org.junit.Test;
@ -612,4 +624,32 @@ public class DomainInfoFlowTest extends ResourceFlowTestCase<DomainInfoFlow, Dom
thrown.expect(RestoresAreAlwaysForOneYearException.class);
runFlow();
}
/** Test that we load contacts and hosts as a batch rather than individually. */
@Test
public void testBatchLoadingOfReferences() throws Exception {
persistTestEntities(false);
// Clear out memcache and session cache so that we count actual datastore calls.
ofy().clearSessionCache();
getLocalMemcacheService().flushAll(
new LocalRpcService.Status(), MemcacheFlushRequest.newBuilder().build());
int numPreviousReads = RequestCapturingAsyncDatastoreService.getReads().size();
doSuccessfulTest("domain_info_response.xml", false);
// Get all of the keys loaded in the flow, with each distinct load() call as a list of keys.
int numReadsWithContactsOrHosts = FluentIterable
.from(RequestCapturingAsyncDatastoreService.getReads())
.skip(numPreviousReads)
.filter(
new Predicate<List<com.google.appengine.api.datastore.Key>>() {
@Override
public boolean apply(List<com.google.appengine.api.datastore.Key> keys) {
return FluentIterable.from(keys)
.transform(KEY_TO_KIND_FUNCTION)
.anyMatch(Predicates.or(
equalTo(Key.getKind(ContactResource.class)),
equalTo(Key.getKind(HostResource.class))));
}})
.size();
assertThat(numReadsWithContactsOrHosts).isEqualTo(1);
}
}

View file

@ -14,9 +14,7 @@
package google.registry.model.domain;
import static com.google.appengine.tools.development.testing.LocalMemcacheServiceTestConfig.getLocalMemcacheService;
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.common.collect.Iterables.skip;
import static com.google.common.truth.Truth.assertThat;
import static google.registry.model.EppResourceUtils.loadByForeignKey;
import static google.registry.testing.DatastoreHelper.cloneAndSetAutoTimestamps;
@ -28,15 +26,12 @@ import static google.registry.testing.DomainResourceSubject.assertAboutDomains;
import static google.registry.util.DateTimeUtils.START_OF_TIME;
import static org.joda.money.CurrencyUnit.USD;
import com.google.appengine.api.memcache.MemcacheServicePb.MemcacheFlushRequest;
import com.google.appengine.tools.development.LocalRpcService;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Ordering;
import com.googlecode.objectify.Key;
import google.registry.flows.EppXmlTransformer;
import google.registry.model.EntityTestCase;
import google.registry.model.billing.BillingEvent;
import google.registry.model.billing.BillingEvent.Reason;
@ -48,11 +43,7 @@ import google.registry.model.domain.secdns.DelegationSignerData;
import google.registry.model.eppcommon.AuthInfo.PasswordAuth;
import google.registry.model.eppcommon.StatusValue;
import google.registry.model.eppcommon.Trid;
import google.registry.model.eppoutput.EppOutput;
import google.registry.model.eppoutput.EppResponse;
import google.registry.model.eppoutput.Result.Code;
import google.registry.model.host.HostResource;
import google.registry.model.ofy.RequestCapturingAsyncDatastoreService;
import google.registry.model.poll.PollMessage;
import google.registry.model.registry.Registry;
import google.registry.model.reporting.HistoryEntry;
@ -60,7 +51,6 @@ import google.registry.model.transfer.TransferData;
import google.registry.model.transfer.TransferData.TransferServerApproveEntity;
import google.registry.model.transfer.TransferStatus;
import google.registry.testing.ExceptionRule;
import google.registry.xml.ValidationMode;
import java.util.List;
import org.joda.money.Money;
import org.joda.time.DateTime;
@ -428,24 +418,6 @@ public class DomainResourceTest extends EntityTestCase {
renewedThreeTimes.autorenewBillingEvent));
}
@Test
public void testMarshalingLoadsResourcesEfficiently() throws Exception {
// All of the resources are in memcache because they were put there when initially persisted.
// Clear out memcache so that we count actual datastore calls.
getLocalMemcacheService().flushAll(
new LocalRpcService.Status(), MemcacheFlushRequest.newBuilder().build());
int numPreviousReads = RequestCapturingAsyncDatastoreService.getReads().size();
EppXmlTransformer.marshal(
EppOutput.create(new EppResponse.Builder()
.setResultFromCode(Code.SUCCESS)
.setResData(domain)
.setTrid(Trid.create(null, "abc"))
.build()),
ValidationMode.STRICT);
// Assert that there was only one call to datastore (that may have loaded many keys).
assertThat(skip(RequestCapturingAsyncDatastoreService.getReads(), numPreviousReads)).hasSize(1);
}
@Test
public void testToHydratedString_notCircular() {
domain.toHydratedString(); // If there are circular references, this will overflow the stack.

View file

@ -15,9 +15,11 @@
package google.registry.model.translators;
import static com.google.common.truth.Truth.assertThat;
import static google.registry.util.DateTimeUtils.START_OF_TIME;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import google.registry.flows.EppXmlTransformer;
import google.registry.model.eppcommon.StatusValue;
import google.registry.model.eppinput.EppInput;
@ -25,10 +27,11 @@ import google.registry.model.eppinput.EppInput.ResourceCommandWrapper;
import google.registry.model.eppoutput.EppOutput;
import google.registry.model.eppoutput.EppResponse;
import google.registry.model.host.HostCommand;
import google.registry.model.host.HostResource;
import google.registry.model.host.HostInfoData;
import google.registry.testing.AppEngineRule;
import google.registry.testing.EppLoader;
import google.registry.xml.ValidationMode;
import java.net.InetAddress;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
@ -50,8 +53,14 @@ public class StatusValueAdapterTest {
String marshalled = new String(
EppXmlTransformer.marshal(
EppOutput.create(new EppResponse.Builder()
.setResData(new HostResource.Builder()
.addStatusValue(StatusValue.CLIENT_UPDATE_PROHIBITED)
.setResData(HostInfoData.newBuilder()
.setCreationClientId("")
.setCreationTime(START_OF_TIME)
.setCurrentSponsorClientId("")
.setFullyQualifiedHostName("")
.setInetAddresses(ImmutableSet.<InetAddress>of())
.setRepoId("")
.setStatusValues(ImmutableSet.of(StatusValue.CLIENT_UPDATE_PROHIBITED))
.build())
.build()),
ValidationMode.LENIENT),

View file

@ -112,6 +112,7 @@ public class DatastoreHelper {
public static HostResource newHostResource(String hostName) {
return new HostResource.Builder()
.setFullyQualifiedHostName(hostName)
.setCreationClientId("TheRegistrar")
.setCurrentSponsorClientId("TheRegistrar")
.setCreationTimeForTest(START_OF_TIME)
.setRepoId(generateNewContactHostRoid())
@ -218,6 +219,7 @@ public class DatastoreHelper {
return new ContactResource.Builder()
.setRepoId(repoId)
.setContactId(contactId)
.setCreationClientId("TheRegistrar")
.setCurrentSponsorClientId("TheRegistrar")
.setAuthInfo(ContactAuthInfo.create(PasswordAuth.create("2fooBAR")))
.setCreationTimeForTest(START_OF_TIME)