Test that marshaling a domain only does one datastore fetch.

This is true even though the domain has three fields (a contact,
a host, and the registrant) whose foreign keys need to be loaded.

This CL also adds the generic ability to do these sort of tests
elsewhere in the code, by instrumenting the datastore instance
used by Objectify to store static counts of method calls.

TESTED=patched in a rollback of [] and confirmed that the
test failed because there were three reads.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=123885768

java/google/registry/model/ofy/ObjectifyService.java

@@ -20,6 +20,8 @@ import static com.google.common.base.Predicates.not;
 import static com.googlecode.objectify.ObjectifyService.factory;
 import static google.registry.util.TypeUtils.hasAnnotation;
 
+import com.google.appengine.api.datastore.AsyncDatastoreService;
+import com.google.appengine.api.datastore.DatastoreServiceConfig;
 import com.google.appengine.api.datastore.DatastoreServiceFactory;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Iterables;
@@ -99,6 +101,16 @@ public class ObjectifyService {
       @Override
       public Objectify begin() {
         return new SessionKeyExposingObjectify(this);
+      }
+
+      @Override
+      protected AsyncDatastoreService createRawAsyncDatastoreService(DatastoreServiceConfig cfg) {
+        // In the unit test environment, wrap the datastore service in a proxy that can be used to
+        // examine the number of requests sent to datastore.
+        AsyncDatastoreService service = super.createRawAsyncDatastoreService(cfg);
+        return RegistryEnvironment.get().equals(RegistryEnvironment.UNITTEST)
+            ? new RequestCountingAsyncDatastoreService(service)
+            : service;
       }});
 
     // Translators must be registered before any entities can be registered.

java/google/registry/model/ofy/RequestCountingAsyncDatastoreService.java

@@ -0,0 +1,189 @@
+// Copyright 2016 The Domain Registry Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package google.registry.model.ofy;
+
+import com.google.appengine.api.datastore.AsyncDatastoreService;
+import com.google.appengine.api.datastore.DatastoreAttributes;
+import com.google.appengine.api.datastore.Entity;
+import com.google.appengine.api.datastore.Index;
+import com.google.appengine.api.datastore.Index.IndexState;
+import com.google.appengine.api.datastore.Key;
+import com.google.appengine.api.datastore.KeyRange;
+import com.google.appengine.api.datastore.PreparedQuery;
+import com.google.appengine.api.datastore.Query;
+import com.google.appengine.api.datastore.Transaction;
+import com.google.appengine.api.datastore.TransactionOptions;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicInteger;
+
+/** A proxy for {@link AsyncDatastoreService} that exposes call counts. */
+public class RequestCountingAsyncDatastoreService implements AsyncDatastoreService {
+
+  private final AsyncDatastoreService delegate;
+
+  // We use static counters because we care about overall calls to datastore, not calls via a
+  // specific instance of the service.
+
+  private static AtomicInteger reads = new AtomicInteger();
+  private static AtomicInteger puts = new AtomicInteger();
+  private static AtomicInteger deletes = new AtomicInteger();
+
+  RequestCountingAsyncDatastoreService(AsyncDatastoreService delegate) {
+    this.delegate = delegate;
+  }
+
+  public static int getReadsCount() {
+    return reads.get();
+  }
+
+  public static int getPutsCount() {
+    return puts.get();
+  }
+
+  public static int getDeletesCount() {
+    return deletes.get();
+  }
+
+  @Override
+  public Collection<Transaction> getActiveTransactions() {
+    return delegate.getActiveTransactions();
+  }
+
+  @Override
+  public Transaction getCurrentTransaction() {
+    return delegate.getCurrentTransaction();
+  }
+
+  @Override
+  public Transaction getCurrentTransaction(Transaction transaction) {
+    return delegate.getCurrentTransaction(transaction);
+  }
+
+  @Override
+  public PreparedQuery prepare(Query query) {
+    return delegate.prepare(query);
+  }
+
+  @Override
+  public PreparedQuery prepare(Transaction transaction, Query query) {
+    return delegate.prepare(transaction, query);
+  }
+
+  @Override
+  public Future<KeyRange> allocateIds(String kind, long num) {
+    return delegate.allocateIds(kind, num);
+  }
+
+  @Override
+  public Future<KeyRange> allocateIds(Key parent, String kind, long num) {
+    return delegate.allocateIds(parent, kind, num);
+  }
+
+  @Override
+  public Future<Transaction> beginTransaction() {
+    return delegate.beginTransaction();
+  }
+
+  @Override
+  public Future<Transaction> beginTransaction(TransactionOptions transaction) {
+    return delegate.beginTransaction(transaction);
+  }
+
+  @Override
+  public Future<Void> delete(Key... keys) {
+    deletes.incrementAndGet();
+    return delegate.delete(keys);
+  }
+
+  @Override
+  public Future<Void> delete(Iterable<Key> keys) {
+    deletes.incrementAndGet();
+    return delegate.delete(keys);
+  }
+
+  @Override
+  public Future<Void> delete(Transaction transaction, Key... keys) {
+    deletes.incrementAndGet();
+    return delegate.delete(transaction, keys);
+  }
+
+  @Override
+  public Future<Void> delete(Transaction transaction, Iterable<Key> keys) {
+    deletes.incrementAndGet();
+    return delegate.delete(transaction, keys);
+  }
+
+  @Override
+  public Future<Entity> get(Key key) {
+    reads.incrementAndGet();
+    return delegate.get(key);
+  }
+
+  @Override
+  public Future<Map<Key, Entity>> get(Iterable<Key> keys) {
+    reads.incrementAndGet();
+    return delegate.get(keys);
+  }
+
+  @Override
+  public Future<Entity> get(Transaction transaction, Key key) {
+    reads.incrementAndGet();
+    return delegate.get(transaction, key);
+  }
+
+  @Override
+  public Future<Map<Key, Entity>> get(Transaction transaction, Iterable<Key> keys) {
+    reads.incrementAndGet();
+    return delegate.get(transaction, keys);
+  }
+
+  @Override
+  public Future<DatastoreAttributes> getDatastoreAttributes() {
+    return delegate.getDatastoreAttributes();
+  }
+
+  @Override
+  public Future<Map<Index, IndexState>> getIndexes() {
+    return delegate.getIndexes();
+  }
+
+  @Override
+  public Future<Key> put(Entity entity) {
+    puts.incrementAndGet();
+    return delegate.put(entity);
+  }
+
+  @Override
+  public Future<List<Key>> put(Iterable<Entity> entities) {
+    puts.incrementAndGet();
+    return delegate.put(entities);
+  }
+
+  @Override
+  public Future<Key> put(Transaction transaction, Entity entity) {
+    puts.incrementAndGet();
+    return delegate.put(transaction, entity);
+  }
+
+  @Override
+  public Future<List<Key>> put(Transaction transaction, Iterable<Entity> entities) {
+    puts.incrementAndGet();
+    return delegate.put(transaction, entities);
+  }
+}

javatests/google/registry/model/domain/DomainResourceTest.java

@@ -14,6 +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.truth.Truth.assertThat;
 import static google.registry.model.EppResourceUtils.loadByUniqueId;
@@ -26,6 +27,8 @@ 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;
@@ -35,6 +38,7 @@ import com.google.common.collect.Ordering;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.Ref;
 
+import google.registry.flows.EppXmlTransformer;
 import google.registry.model.EntityTestCase;
 import google.registry.model.billing.BillingEvent;
 import google.registry.model.billing.BillingEvent.Reason;
@@ -45,7 +49,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.eppcommon.Trid;
+import google.registry.model.eppoutput.EppOutput;
+import google.registry.model.eppoutput.Response;
+import google.registry.model.eppoutput.Result;
+import google.registry.model.eppoutput.Result.Code;
 import google.registry.model.host.HostResource;
+import google.registry.model.ofy.RequestCountingAsyncDatastoreService;
 import google.registry.model.poll.PollMessage;
 import google.registry.model.registry.Registry;
 import google.registry.model.reporting.HistoryEntry;
@@ -53,6 +62,7 @@ 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 org.joda.money.Money;
 import org.joda.time.DateTime;
@@ -95,7 +105,7 @@ public class DomainResourceTest extends EntityTestCase {
         .setRepoId("4-COM")
             .setCreationClientId("a registrar")
             .setLastEppUpdateTime(clock.nowUtc())
-            .setLastEppUpdateClientId("another registrar")
+            .setLastEppUpdateClientId("AnotherRegistrar")
             .setLastTransferTime(clock.nowUtc())
             .setStatusValues(ImmutableSet.of(
                 StatusValue.CLIENT_DELETE_PROHIBITED,
@@ -110,7 +120,7 @@ public class DomainResourceTest extends EntityTestCase {
                 Ref.create(contactResource2))))
             .setNameservers(ImmutableSet.of(Ref.create(hostResource)))
             .setSubordinateHosts(ImmutableSet.of("ns1.example.com"))
-            .setCurrentSponsorClientId("a third registrar")
+            .setCurrentSponsorClientId("ThirdRegistrar")
             .setRegistrationExpirationTime(clock.nowUtc().plusYears(1))
             .setAuthInfo(DomainAuthInfo.create(PasswordAuth.create("password")))
             .setDsData(ImmutableSet.of(DelegationSignerData.create(1, 2, 3, new byte[] {0, 1, 2})))
@@ -415,4 +425,21 @@ public class DomainResourceTest extends EntityTestCase {
             renewedThreeTimes.getCurrentSponsorClientId(),
             Ref.create(renewedThreeTimes.autorenewBillingEvent.key())));
   }
+
+  @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 previousReads = RequestCountingAsyncDatastoreService.getReadsCount();
+    EppXmlTransformer.marshal(
+        EppOutput.create(new Response.Builder()
+            .setResult(Result.create(Code.Success))
+            .setResData(ImmutableList.of(domain))
+            .setTrid(Trid.create(null, "abc"))
+            .build()),
+        ValidationMode.STRICT);
+    assertThat(RequestCountingAsyncDatastoreService.getReadsCount() - previousReads).isEqualTo(1);
+  }
 }