Link domain to subordinate hosts on RDE import

Based on the original pull request below with some modifications for code drift
over time (including adding handling/testing for the case where superordinate
domains are in pending delete, and thus can't have hosts linked to them).

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=169698157

java/google/registry/rde/imports/RdeHostLinkAction.java

@@ -14,20 +14,25 @@
 
 package google.registry.rde.imports;
 
-import static google.registry.flows.host.HostFlowUtils.lookupSuperordinateDomain;
+import static com.google.common.base.Preconditions.checkState;
 import static google.registry.mapreduce.MapreduceRunner.PARAM_MAP_SHARDS;
+import static google.registry.model.EppResourceUtils.loadByForeignKey;
 import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.model.registry.Registries.findTldForName;
 import static google.registry.util.PipelineUtils.createJobPath;
-import static org.joda.time.DateTimeZone.UTC;
 
 import com.google.appengine.tools.mapreduce.Mapper;
+import com.google.common.base.Joiner;
 import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
 import com.google.common.net.InternetDomainName;
 import com.googlecode.objectify.Key;
-import com.googlecode.objectify.VoidWork;
+import com.googlecode.objectify.Work;
 import google.registry.config.RegistryConfig.Config;
+import google.registry.flows.host.HostFlowUtils;
 import google.registry.mapreduce.MapreduceRunner;
+import google.registry.model.EppResource;
 import google.registry.model.domain.DomainResource;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.host.HostResource;
@@ -104,47 +109,110 @@ public class RdeHostLinkAction implements Runnable {
       final XjcRdeHost xjcHost = fragment.getInstance().getValue();
       logger.infofmt("Attempting to link superordinate domain for host %s", xjcHost.getName());
       try {
-        InternetDomainName hostName = InternetDomainName.from(xjcHost.getName());
+        final InternetDomainName hostName = InternetDomainName.from(xjcHost.getName());
+
+        HostLinkResult hostLinkResult = ofy().transact(new Work<HostLinkResult>() {
+          @Override
+          public HostLinkResult run() {
             Optional<DomainResource> superordinateDomain =
-            lookupSuperordinateDomain(hostName, DateTime.now(UTC));
+                lookupSuperordinateDomain(hostName, ofy().getTransactionTime());
-        // if suporordinateDomain is null, this is an out of zone host and can't be linked
+            // if suporordinateDomain is absent, this is an out of zone host and can't be linked.
+            // absent is only returned for out of zone hosts, and an exception is thrown for in
+            // zone hosts with no superordinate domain.
             if (!superordinateDomain.isPresent()) {
-          getContext().incrementCounter("post-import hosts out of zone");
+              return HostLinkResult.HOST_OUT_OF_ZONE;
-          logger.infofmt("Host %s is out of zone", xjcHost.getName());
-          return;
             }
             if (superordinateDomain.get().getStatusValues().contains(StatusValue.PENDING_DELETE)) {
+              return HostLinkResult.SUPERORDINATE_DOMAIN_IN_PENDING_DELETE;
+            }
+            Key<DomainResource> superordinateDomainKey = Key.create(superordinateDomain.get());
+            // link host to superordinate domain and set time of last superordinate change to
+            // the time of the import
+            HostResource host =
+                ofy().load().now(Key.create(HostResource.class, xjcHost.getRoid()));
+            if (host == null) {
+              return HostLinkResult.HOST_NOT_FOUND;
+            }
+            // link domain to subordinate host
+            ofy().save().<EppResource>entities(
+                host.asBuilder().setSuperordinateDomain(superordinateDomainKey)
+                    .setLastSuperordinateChange(ofy().getTransactionTime())
+                    .build(),
+                superordinateDomain.get().asBuilder()
+                    .addSubordinateHost(host.getFullyQualifiedHostName()).build());
+            return HostLinkResult.HOST_LINKED;
+          }
+        });
+        // increment counter and log appropriately based on result of transaction
+        switch (hostLinkResult) {
+          case HOST_LINKED:
+            getContext().incrementCounter("post-import hosts linked");
+            logger.infofmt(
+                "Successfully linked host %s to superordinate domain", xjcHost.getName());
+            // Record number of hosts successfully linked
+            break;
+          case HOST_NOT_FOUND:
+            getContext().incrementCounter("hosts not found");
+            logger.severefmt(
+                "Host with name %s and repoid %s not found",
+                xjcHost.getName(),
+                xjcHost.getRoid());
+            break;
+          case SUPERORDINATE_DOMAIN_IN_PENDING_DELETE:
             getContext()
                 .incrementCounter(
                     "post-import hosts with superordinate domains in pending delete");
             logger.infofmt(
                 "Host %s has a superordinate domain in pending delete", xjcHost.getName());
-          return;
+            break;
+          case HOST_OUT_OF_ZONE:
+            getContext().incrementCounter("post-import hosts out of zone");
+            logger.infofmt("Host %s is out of zone", xjcHost.getName());
+            break;
         }
-        // at this point, the host is definitely in zone and should be linked
+      } catch (RuntimeException e) {
-        getContext().incrementCounter("post-import hosts in zone");
-        final Key<DomainResource> superordinateDomainKey = Key.create(superordinateDomain.get());
-        ofy().transact(new VoidWork() {
-          @Override
-          public void vrun() {
-            HostResource host =
-                ofy().load().now(Key.create(HostResource.class, xjcHost.getRoid()));
-            ofy().save()
-                .entity(host.asBuilder().setSuperordinateDomain(superordinateDomainKey).build());
-          }
-        });
-        logger.infofmt(
-            "Successfully linked host %s to superordinate domain %s",
-            xjcHost.getName(),
-            superordinateDomain.get().getFullyQualifiedDomainName());
-        // Record number of hosts successfully linked
-        getContext().incrementCounter("post-import hosts linked");
-      } catch (Exception e) {
         // Record the number of hosts with unexpected errors
         getContext().incrementCounter("post-import host errors");
-        throw new HostLinkException(xjcHost.getName(), xjcHost.toString(), e);
+        logger.severefmt(e, "Error linking host %s; xml=%s", xjcHost.getName(), xjcHost);
       }
     }
+
+    /**
+     * Return the {@link DomainResource} this host is subordinate to, or absent for out of zone
+     * hosts.
+     *
+     * <p>We use this instead of {@link HostFlowUtils#lookupSuperordinateDomain} because we don't
+     * want to use the EPP exception classes for the case when the superordinate domain doesn't
+     * exist or isn't active.
+     *
+     * @throws IllegalStateException for hosts without superordinate domains
+     */
+    private static Optional<DomainResource> lookupSuperordinateDomain(
+        InternetDomainName hostName, DateTime now) {
+      Optional<InternetDomainName> tld = findTldForName(hostName);
+      // out of zone hosts cannot be linked
+      if (!tld.isPresent()) {
+        return Optional.absent();
+      }
+      // This is a subordinate host
+      String domainName =
+          Joiner.on('.')
+              .join(
+                  Iterables.skip(
+                      hostName.parts(), hostName.parts().size() - (tld.get().parts().size() + 1)));
+      DomainResource superordinateDomain = loadByForeignKey(DomainResource.class, domainName, now);
+      // Hosts can't be linked if domains import hasn't been run
+      checkState(
+          superordinateDomain != null, "Superordinate domain does not exist: %s", domainName);
+      return Optional.of(superordinateDomain);
+    }
+  }
+
+  private static enum HostLinkResult {
+    HOST_NOT_FOUND,
+    HOST_OUT_OF_ZONE,
+    SUPERORDINATE_DOMAIN_IN_PENDING_DELETE,
+    HOST_LINKED;
   }
 
   private static class HostLinkException extends RuntimeException {

javatests/google/registry/rde/imports/RdeHostLinkActionTest.java

@@ -15,17 +15,21 @@
 package google.registry.rde.imports;
 
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.model.eppcommon.StatusValue.PENDING_DELETE;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.testing.DatastoreHelper.createTld;
+import static google.registry.testing.DatastoreHelper.newDomainResource;
 import static google.registry.testing.DatastoreHelper.newHostResource;
 import static google.registry.testing.DatastoreHelper.persistActiveDomain;
 import static google.registry.testing.DatastoreHelper.persistResource;
+import static google.registry.util.DateTimeUtils.END_OF_TIME;
 
 import com.google.appengine.tools.cloudstorage.GcsFilename;
 import com.google.appengine.tools.cloudstorage.GcsService;
 import com.google.appengine.tools.cloudstorage.GcsServiceFactory;
 import com.google.appengine.tools.cloudstorage.RetryParams;
 import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.io.ByteSource;
 import com.google.common.io.ByteStreams;
 import com.googlecode.objectify.Key;
@@ -34,14 +38,20 @@ import google.registry.gcs.GcsUtils;
 import google.registry.mapreduce.MapreduceRunner;
 import google.registry.model.domain.DomainResource;
 import google.registry.model.host.HostResource;
+import google.registry.model.index.ForeignKeyIndex.ForeignKeyDomainIndex;
+import google.registry.model.ofy.Ofy;
 import google.registry.request.Response;
+import google.registry.testing.FakeClock;
 import google.registry.testing.FakeResponse;
+import google.registry.testing.InjectRule;
 import google.registry.testing.mapreduce.MapreduceTestCase;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.List;
+import org.joda.time.DateTime;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -57,14 +67,21 @@ public class RdeHostLinkActionTest extends MapreduceTestCase<RdeHostLinkAction>
   private static final GcsService GCS_SERVICE =
       GcsServiceFactory.createGcsService(RetryParams.getDefaultInstance());
 
+  @Rule
+  public final InjectRule inject = new InjectRule();
+
   private MapreduceRunner mrRunner;
 
   private Response response;
 
   private final Optional<Integer> mapShards = Optional.absent();
 
+  private final FakeClock clock = new FakeClock();
+
   @Before
   public void before() throws Exception {
+    clock.setTo(DateTime.parse("2016-05-07T14:55:38Z"));
+    inject.setStaticField(Ofy.class, "clock", clock);
     createTld("test");
     response = new FakeResponse();
     mrRunner = new MapreduceRunner(Optional.<Integer>absent(), Optional.<Integer>absent());
@@ -83,10 +100,63 @@ public class RdeHostLinkActionTest extends MapreduceTestCase<RdeHostLinkAction>
     DomainResource superordinateDomain = persistActiveDomain("example1.test");
     Key<DomainResource> superOrdinateDomainKey = Key.create(superordinateDomain);
     pushToGcs(DEPOSIT_1_HOST);
+    // set transaction time to slightly after resource save
+    clock.advanceOneMilli();
     runMapreduce();
+    // verify that host is linked to domain
     List<HostResource> hosts = ofy().load().type(HostResource.class).list();
     assertThat(hosts).hasSize(1);
     assertThat(hosts.get(0).getSuperordinateDomain()).isEqualTo(superOrdinateDomainKey);
+    assertThat(hosts.get(0).getLastSuperordinateChange())
+        .isEqualTo(DateTime.parse("2016-05-07T14:55:38.001Z"));
+    // verify that domain is linked to host
+    List<DomainResource> domains = ofy().load().type(DomainResource.class).list();
+    assertThat(domains).hasSize(1);
+    assertThat(domains.get(0).getSubordinateHosts()).containsExactly("ns1.example1.test");
+  }
+
+  @Test
+  public void test_mapreduceIgnoresHostWithWrongRepoid() throws Exception {
+    // in-zone host with different repoid is ignored, since it would already be linked
+    // from a separate epp create or import.
+    // Create host and domain first
+    HostResource newHost = persistResource(
+        newHostResource("ns1.example1.test")
+            .asBuilder()
+            .setRepoId("wrong-repoid")
+            .build());
+    DomainResource superordinateDomain = persistActiveDomain("example1.test");
+    ForeignKeyDomainIndex.create(superordinateDomain, END_OF_TIME);
+    pushToGcs(DEPOSIT_1_HOST);
+    // set transaction time to slightly after resource save
+    clock.advanceOneMilli();
+    runMapreduce();
+    // verify that host has not been updated
+    List<HostResource> hosts = ofy().load().type(HostResource.class).list();
+    assertThat(hosts).hasSize(1);
+    assertThat(hosts.get(0).getUpdateAutoTimestamp()).isEqualTo(newHost.getUpdateAutoTimestamp());
+  }
+
+  @Test
+  public void test_mapreduceIgnoresHostWithSuperordinateDomainPendingDeletion() throws Exception {
+    HostResource newHost = persistResource(
+        newHostResource("ns1.example1.test")
+            .asBuilder()
+            .setRepoId("Hns1_example1_test-TEST")
+            .build());
+    persistResource(
+        newDomainResource("example1.test")
+            .asBuilder()
+            .setStatusValues(ImmutableSet.of(PENDING_DELETE))
+            .build());
+    pushToGcs(DEPOSIT_1_HOST);
+    // set transaction time to slightly after resource save
+    clock.advanceOneMilli();
+    runMapreduce();
+    // verify that host has not been updated
+    List<HostResource> hosts = ofy().load().type(HostResource.class).list();
+    assertThat(hosts).hasSize(1);
+    assertThat(hosts.get(0).getUpdateAutoTimestamp()).isEqualTo(newHost.getUpdateAutoTimestamp());
   }
 
   private void runMapreduce() throws Exception {