diff --git a/java/google/registry/flows/domain/DomainApplicationInfoFlow.java b/java/google/registry/flows/domain/DomainApplicationInfoFlow.java index acc361b1c..89013141b 100644 --- a/java/google/registry/flows/domain/DomainApplicationInfoFlow.java +++ b/java/google/registry/flows/domain/DomainApplicationInfoFlow.java @@ -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() diff --git a/java/google/registry/flows/domain/DomainFlowUtils.java b/java/google/registry/flows/domain/DomainFlowUtils.java index aa5103c4f..2527cc40c 100644 --- a/java/google/registry/flows/domain/DomainFlowUtils.java +++ b/java/google/registry/flows/domain/DomainFlowUtils.java @@ -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 loadForeignKeyedDesignatedContacts( ImmutableSet contacts) { ImmutableSet.Builder builder = new ImmutableSet.Builder<>(); diff --git a/java/google/registry/flows/domain/DomainInfoFlow.java b/java/google/registry/flows/domain/DomainInfoFlow.java index b2d4c717d..a8953195e 100644 --- a/java/google/registry/flows/domain/DomainInfoFlow.java +++ b/java/google/registry/flows/domain/DomainInfoFlow.java @@ -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()) { diff --git a/javatests/google/registry/flows/domain/DomainApplicationInfoFlowTest.java b/javatests/google/registry/flows/domain/DomainApplicationInfoFlowTest.java index ca41a6ba7..f4b6ea8f3 100644 --- a/javatests/google/registry/flows/domain/DomainApplicationInfoFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainApplicationInfoFlowTest.java @@ -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 { diff --git a/javatests/google/registry/flows/domain/DomainInfoFlowTest.java b/javatests/google/registry/flows/domain/DomainInfoFlowTest.java index d111ad42c..32b0f8c22 100644 --- a/javatests/google/registry/flows/domain/DomainInfoFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainInfoFlowTest.java @@ -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