mirror of
https://github.com/google/nomulus.git
synced 2025-05-28 16:30:12 +02:00
Make DomainInfoFlow (and application info) explicitly hit memcache
TESTED=For all tests, I added @Cache to DomainBase because otherwise the tests will fail. We aren't ready to do this in prod yet, which is why the tests have a TODO in them. The new tests fail if you change line 134 in Ofy to not use memcache and either use the unchanged original flow code, or use the new inlined code and change loadWithMemcache() to load(). They pass with the new inlined code that calls loadWithMemcache(), as long as the @Cache is added to DomainResource. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=154457655
This commit is contained in:
parent
e96b999a83
commit
f1129ea2b1
5 changed files with 73 additions and 17 deletions
|
@ -14,6 +14,7 @@
|
|||
|
||||
package google.registry.flows.domain;
|
||||
|
||||
import static com.google.common.collect.Sets.union;
|
||||
import static google.registry.flows.EppXmlTransformer.unmarshal;
|
||||
import static google.registry.flows.FlowUtils.validateClientIsLoggedIn;
|
||||
import static google.registry.flows.ResourceFlowUtils.verifyExistence;
|
||||
|
@ -21,13 +22,12 @@ import static google.registry.flows.ResourceFlowUtils.verifyOptionalAuthInfo;
|
|||
import static google.registry.flows.ResourceFlowUtils.verifyResourceOwnership;
|
||||
import static google.registry.flows.domain.DomainFlowUtils.addSecDnsExtensionIfPresent;
|
||||
import static google.registry.flows.domain.DomainFlowUtils.loadForeignKeyedDesignatedContacts;
|
||||
import static google.registry.flows.domain.DomainFlowUtils.prefetchReferencedResources;
|
||||
import static google.registry.flows.domain.DomainFlowUtils.verifyApplicationDomainMatchesTargetId;
|
||||
import static google.registry.model.EppResourceUtils.loadDomainApplication;
|
||||
import static google.registry.model.ofy.ObjectifyService.ofy;
|
||||
|
||||
import com.google.common.base.Optional;
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.googlecode.objectify.Key;
|
||||
import google.registry.flows.EppException;
|
||||
import google.registry.flows.EppException.ParameterValuePolicyErrorException;
|
||||
import google.registry.flows.EppException.RequiredParameterMissingException;
|
||||
|
@ -88,10 +88,14 @@ public final class DomainApplicationInfoFlow implements Flow {
|
|||
if (applicationId.isEmpty()) {
|
||||
throw new MissingApplicationIdException();
|
||||
}
|
||||
DomainApplication application = verifyExistence(
|
||||
DomainApplication application =
|
||||
ofy().loadWithMemcache().key(Key.create(DomainApplication.class, applicationId)).now();
|
||||
verifyExistence(
|
||||
DomainApplication.class,
|
||||
applicationId,
|
||||
loadDomainApplication(applicationId, clock.nowUtc()));
|
||||
application != null && clock.nowUtc().isBefore(application.getDeletionTime())
|
||||
? application
|
||||
: null);
|
||||
verifyApplicationDomainMatchesTargetId(application, targetId);
|
||||
verifyOptionalAuthInfo(authInfo, application);
|
||||
LaunchInfoExtension launchInfo = eppInput.getSingleExtension(LaunchInfoExtension.class);
|
||||
|
@ -101,13 +105,16 @@ public final class DomainApplicationInfoFlow implements Flow {
|
|||
// We don't support authInfo for applications, so if it's another registrar always fail.
|
||||
verifyResourceOwnership(clientId, application);
|
||||
boolean showDelegatedHosts = ((Info) resourceCommand).getHostsRequest().requestDelegated();
|
||||
prefetchReferencedResources(application);
|
||||
// Prefetch all referenced resources. Calling values() blocks until loading is done.
|
||||
ofy().loadWithMemcache()
|
||||
.values(union(application.getNameservers(), application.getReferencedContacts())).values();
|
||||
return responseBuilder
|
||||
.setResData(DomainInfoData.newBuilder()
|
||||
.setFullyQualifiedDomainName(application.getFullyQualifiedDomainName())
|
||||
.setRepoId(application.getRepoId())
|
||||
.setStatusValues(application.getStatusValues())
|
||||
.setRegistrant(ofy().load().key(application.getRegistrant()).now().getContactId())
|
||||
.setRegistrant(
|
||||
ofy().loadWithMemcache().key(application.getRegistrant()).now().getContactId())
|
||||
.setContacts(loadForeignKeyedDesignatedContacts(application.getContacts()))
|
||||
.setNameservers(showDelegatedHosts
|
||||
? application.loadNameserverFullyQualifiedHostNames()
|
||||
|
|
|
@ -917,12 +917,6 @@ public class DomainFlowUtils {
|
|||
.build();
|
||||
}
|
||||
|
||||
/** Bulk-load all referenced resources on a domain so they are in the session cache. */
|
||||
static void prefetchReferencedResources(DomainBase domain) {
|
||||
// Calling values() on the result blocks until loading is done.
|
||||
ofy().load().values(union(domain.getNameservers(), domain.getReferencedContacts())).values();
|
||||
}
|
||||
|
||||
static ImmutableSet<ForeignKeyedDesignatedContact> loadForeignKeyedDesignatedContacts(
|
||||
ImmutableSet<DesignatedContact> contacts) {
|
||||
ImmutableSet.Builder<ForeignKeyedDesignatedContact> builder = new ImmutableSet.Builder<>();
|
||||
|
|
|
@ -14,13 +14,14 @@
|
|||
|
||||
package google.registry.flows.domain;
|
||||
|
||||
import static com.google.common.collect.Sets.union;
|
||||
import static google.registry.flows.FlowUtils.validateClientIsLoggedIn;
|
||||
import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence;
|
||||
import static google.registry.flows.ResourceFlowUtils.verifyExistence;
|
||||
import static google.registry.flows.ResourceFlowUtils.verifyOptionalAuthInfo;
|
||||
import static google.registry.flows.domain.DomainFlowUtils.addSecDnsExtensionIfPresent;
|
||||
import static google.registry.flows.domain.DomainFlowUtils.handleFeeRequest;
|
||||
import static google.registry.flows.domain.DomainFlowUtils.loadForeignKeyedDesignatedContacts;
|
||||
import static google.registry.flows.domain.DomainFlowUtils.prefetchReferencedResources;
|
||||
import static google.registry.model.EppResourceUtils.loadByForeignKeyWithMemcache;
|
||||
import static google.registry.model.ofy.ObjectifyService.ofy;
|
||||
|
||||
import com.google.common.base.Optional;
|
||||
|
@ -94,17 +95,22 @@ public final class DomainInfoFlow implements Flow {
|
|||
extensionManager.validate();
|
||||
validateClientIsLoggedIn(clientId);
|
||||
DateTime now = clock.nowUtc();
|
||||
DomainResource domain = loadAndVerifyExistence(DomainResource.class, targetId, now);
|
||||
DomainResource domain = verifyExistence(
|
||||
DomainResource.class,
|
||||
targetId,
|
||||
loadByForeignKeyWithMemcache(DomainResource.class, targetId, now));
|
||||
verifyOptionalAuthInfo(authInfo, domain);
|
||||
customLogic.afterValidation(AfterValidationParameters.newBuilder().setDomain(domain).build());
|
||||
prefetchReferencedResources(domain);
|
||||
// Prefetch all referenced resources. Calling values() blocks until loading is done.
|
||||
ofy().loadWithMemcache()
|
||||
.values(union(domain.getNameservers(), domain.getReferencedContacts())).values();
|
||||
// Registrars can only see a few fields on unauthorized domains.
|
||||
// This is a policy decision that is left up to us by the rfcs.
|
||||
DomainInfoData.Builder infoBuilder = DomainInfoData.newBuilder()
|
||||
.setFullyQualifiedDomainName(domain.getFullyQualifiedDomainName())
|
||||
.setRepoId(domain.getRepoId())
|
||||
.setCurrentSponsorClientId(domain.getCurrentSponsorClientId())
|
||||
.setRegistrant(ofy().load().key(domain.getRegistrant()).now().getContactId());
|
||||
.setRegistrant(ofy().loadWithMemcache().key(domain.getRegistrant()).now().getContactId());
|
||||
// If authInfo is non-null, then the caller is authorized to see the full information since we
|
||||
// will have already verified the authInfo is valid.
|
||||
if (clientId.equals(domain.getCurrentSponsorClientId()) || authInfo.isPresent()) {
|
||||
|
|
|
@ -24,6 +24,7 @@ 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.MemcacheHelper.clearMemcache;
|
||||
import static google.registry.testing.MemcacheHelper.setMemcacheContents;
|
||||
import static google.registry.testing.TestDataHelper.loadFileWithSubstitutions;
|
||||
import static google.registry.util.DatastoreServiceUtils.KEY_TO_KIND_FUNCTION;
|
||||
|
||||
|
@ -52,6 +53,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.host.HostResource;
|
||||
import google.registry.model.index.DomainApplicationIndex;
|
||||
import google.registry.model.ofy.RequestCapturingAsyncDatastoreService;
|
||||
import google.registry.model.registry.Registry.TldState;
|
||||
import google.registry.model.smd.EncodedSignedMark;
|
||||
|
@ -328,6 +330,28 @@ public class DomainApplicationInfoFlowTest
|
|||
runFlow();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testLoadsComeFromMemcache() throws Exception {
|
||||
persistTestEntities(HostsState.HOSTS_EXIST, MarksState.NO_MARKS_EXIST);
|
||||
setMemcacheContents(
|
||||
Key.create(application),
|
||||
DomainApplicationIndex.createKey(application),
|
||||
Key.create(registrant),
|
||||
Key.create(contact),
|
||||
Key.create(host1),
|
||||
Key.create(host2));
|
||||
int numPreviousReads = RequestCapturingAsyncDatastoreService.getReads().size();
|
||||
doSuccessfulTest("domain_info_sunrise_response.xml", HostsState.HOSTS_EXIST);
|
||||
|
||||
// Everything should be in memcache so nothing should hit datastore.
|
||||
int numReadsInFlow = RequestCapturingAsyncDatastoreService.getReads().size() - numPreviousReads;
|
||||
// TODO(b/27424173): This is 1 because there is no @Cache annotation on DomainBase, and we
|
||||
// don't want to blindly add it because that's a production change that adds potentially
|
||||
// dangerous caching. When the recommendations from the audit in b/27424173 are done and we've
|
||||
// tested the new safer caching this should be set to 0.
|
||||
assertThat(numReadsInFlow).isEqualTo(1);
|
||||
}
|
||||
|
||||
/** Test that we load contacts and hosts as a batch rather than individually. */
|
||||
@Test
|
||||
public void testBatchLoadingOfReferences() throws Exception {
|
||||
|
|
|
@ -25,6 +25,7 @@ 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.MemcacheHelper.clearMemcache;
|
||||
import static google.registry.testing.MemcacheHelper.setMemcacheContents;
|
||||
import static google.registry.testing.TestDataHelper.loadFileWithSubstitutions;
|
||||
import static google.registry.util.DatastoreServiceUtils.KEY_TO_KIND_FUNCTION;
|
||||
|
||||
|
@ -55,6 +56,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.host.HostResource;
|
||||
import google.registry.model.index.ForeignKeyIndex;
|
||||
import google.registry.model.ofy.RequestCapturingAsyncDatastoreService;
|
||||
import google.registry.testing.AppEngineRule;
|
||||
import java.util.List;
|
||||
|
@ -633,6 +635,29 @@ public class DomainInfoFlowTest extends ResourceFlowTestCase<DomainInfoFlow, Dom
|
|||
runFlow();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testLoadsComeFromMemcache() throws Exception {
|
||||
persistTestEntities(false);
|
||||
setMemcacheContents(
|
||||
Key.create(domain),
|
||||
ForeignKeyIndex.createKey(domain),
|
||||
Key.create(registrant),
|
||||
Key.create(contact),
|
||||
Key.create(host1),
|
||||
Key.create(host2),
|
||||
Key.create(host3));
|
||||
int numPreviousReads = RequestCapturingAsyncDatastoreService.getReads().size();
|
||||
runFlowAssertResponse(loadFileWithSubstitutions(
|
||||
getClass(), "domain_info_response.xml", ImmutableMap.of("ROID", "2FF-TLD")));
|
||||
// Everything should be in memcache so nothing should hit datastore.
|
||||
int numReadsInFlow = RequestCapturingAsyncDatastoreService.getReads().size() - numPreviousReads;
|
||||
// TODO(b/27424173): This is 1 because there is no @Cache annotation on DomainBase, and we
|
||||
// don't want to blindly add it because that's a production change that adds potentially
|
||||
// dangerous caching. When the recommendations from the audit in b/27424173 are done and we've
|
||||
// tested the new safer caching this should be set to 0.
|
||||
assertThat(numReadsInFlow).isEqualTo(1);
|
||||
}
|
||||
|
||||
/** Test that we load contacts and hosts as a batch rather than individually. */
|
||||
@Test
|
||||
public void testBatchLoadingOfReferences() throws Exception {
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue