From 8bd9b35dfd6fe0da431a5f6d0535f82d8e73dce8 Mon Sep 17 00:00:00 2001 From: mcilwain Date: Fri, 22 Sep 2017 09:26:56 -0700 Subject: [PATCH] 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 --- .../rde/imports/RdeHostLinkAction.java | 138 +++++++++++++----- .../rde/imports/RdeHostLinkActionTest.java | 70 +++++++++ 2 files changed, 173 insertions(+), 35 deletions(-) diff --git a/java/google/registry/rde/imports/RdeHostLinkAction.java b/java/google/registry/rde/imports/RdeHostLinkAction.java index bd84da425..85c784b2d 100644 --- a/java/google/registry/rde/imports/RdeHostLinkAction.java +++ b/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()); - Optional superordinateDomain = - lookupSuperordinateDomain(hostName, DateTime.now(UTC)); - // if suporordinateDomain is null, this is an out of zone host and can't be linked - if (!superordinateDomain.isPresent()) { - getContext().incrementCounter("post-import hosts out of zone"); - logger.infofmt("Host %s is out of zone", xjcHost.getName()); - return; - } - if (superordinateDomain.get().getStatusValues().contains(StatusValue.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; - } - // at this point, the host is definitely in zone and should be linked - getContext().incrementCounter("post-import hosts in zone"); - final Key superordinateDomainKey = Key.create(superordinateDomain.get()); - ofy().transact(new VoidWork() { + final InternetDomainName hostName = InternetDomainName.from(xjcHost.getName()); + + HostLinkResult hostLinkResult = ofy().transact(new Work() { @Override - public void vrun() { + public HostLinkResult run() { + Optional superordinateDomain = + lookupSuperordinateDomain(hostName, ofy().getTransactionTime()); + // 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()) { + return HostLinkResult.HOST_OUT_OF_ZONE; + } + if (superordinateDomain.get().getStatusValues().contains(StatusValue.PENDING_DELETE)) { + return HostLinkResult.SUPERORDINATE_DOMAIN_IN_PENDING_DELETE; + } + Key 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())); - ofy().save() - .entity(host.asBuilder().setSuperordinateDomain(superordinateDomainKey).build()); + if (host == null) { + return HostLinkResult.HOST_NOT_FOUND; + } + // link domain to subordinate host + ofy().save().entities( + host.asBuilder().setSuperordinateDomain(superordinateDomainKey) + .setLastSuperordinateChange(ofy().getTransactionTime()) + .build(), + superordinateDomain.get().asBuilder() + .addSubordinateHost(host.getFullyQualifiedHostName()).build()); + return HostLinkResult.HOST_LINKED; } }); - 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) { + // 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()); + 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; + } + } catch (RuntimeException 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. + * + *

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 lookupSuperordinateDomain( + InternetDomainName hostName, DateTime now) { + Optional 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 { diff --git a/javatests/google/registry/rde/imports/RdeHostLinkActionTest.java b/javatests/google/registry/rde/imports/RdeHostLinkActionTest.java index 968b9125d..dc5a99bb5 100644 --- a/javatests/google/registry/rde/imports/RdeHostLinkActionTest.java +++ b/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 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 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.absent(), Optional.absent()); @@ -83,10 +100,63 @@ public class RdeHostLinkActionTest extends MapreduceTestCase DomainResource superordinateDomain = persistActiveDomain("example1.test"); Key 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 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 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 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 hosts = ofy().load().type(HostResource.class).list(); + assertThat(hosts).hasSize(1); + assertThat(hosts.get(0).getUpdateAutoTimestamp()).isEqualTo(newHost.getUpdateAutoTimestamp()); } private void runMapreduce() throws Exception {