Tidy up RefreshDnsAction

This reworks the logic in RefreshDnsAction by factoring out a few
helper methods so the core logic is simpler and more straightforward.

Also added a couple tests to DnsInjectionTest that seemed worth having
for symmetry.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=134110706
This commit is contained in:
nickfelt 2016-09-23 13:28:25 -07:00 committed by Ben McIlwain
parent 1b4e73a50f
commit 0bc6e7b728
2 changed files with 39 additions and 23 deletions

View file

@ -18,6 +18,8 @@ import static google.registry.model.EppResourceUtils.loadByForeignKey;
import google.registry.dns.DnsConstants.TargetType; import google.registry.dns.DnsConstants.TargetType;
import google.registry.model.EppResource; import google.registry.model.EppResource;
import google.registry.model.EppResource.ForeignKeyedEppResource;
import google.registry.model.annotations.ExternalMessagingName;
import google.registry.model.domain.DomainResource; import google.registry.model.domain.DomainResource;
import google.registry.model.host.HostResource; import google.registry.model.host.HostResource;
import google.registry.request.Action; import google.registry.request.Action;
@ -42,38 +44,35 @@ public final class RefreshDnsAction implements Runnable {
if (!domainOrHostName.contains(".")) { if (!domainOrHostName.contains(".")) {
throw new BadRequestException("URL parameter 'name' must be fully qualified"); throw new BadRequestException("URL parameter 'name' must be fully qualified");
} }
boolean domainLookup;
Class<? extends EppResource> clazz;
switch (type) { switch (type) {
case DOMAIN: case DOMAIN:
domainLookup = true; loadAndVerifyExistence(DomainResource.class, domainOrHostName);
clazz = DomainResource.class; dnsQueue.addDomainRefreshTask(domainOrHostName);
break; break;
case HOST: case HOST:
domainLookup = false; verifyHostIsSubordinate(loadAndVerifyExistence(HostResource.class, domainOrHostName));
clazz = HostResource.class; dnsQueue.addHostRefreshTask(domainOrHostName);
break; break;
default: default:
throw new BadRequestException("Unsupported type: " + type); throw new BadRequestException("Unsupported type: " + type);
} }
}
EppResource eppResource = loadByForeignKey(clazz, domainOrHostName, clock.nowUtc()); private <T extends EppResource & ForeignKeyedEppResource>
if (eppResource == null) { T loadAndVerifyExistence(Class<T> clazz, String foreignKey) {
T resource = loadByForeignKey(clazz, foreignKey, clock.nowUtc());
if (resource == null) {
String typeName = clazz.getAnnotation(ExternalMessagingName.class).value();
throw new NotFoundException( throw new NotFoundException(
String.format("%s %s not found", type, domainOrHostName)); String.format("%s %s not found", typeName, domainOrHostName));
} }
return resource;
}
if (domainLookup) { private static void verifyHostIsSubordinate(HostResource host) {
dnsQueue.addDomainRefreshTask(domainOrHostName); if (host.getSuperordinateDomain() == null) {
} else { throw new BadRequestException(
if (((HostResource) eppResource).getSuperordinateDomain() == null) { String.format("%s isn't a subordinate hostname", host.getFullyQualifiedHostName()));
throw new BadRequestException(
String.format("%s isn't a subordinate hostname", domainOrHostName));
} else {
// Don't enqueue host refresh tasks for external hosts.
dnsQueue.addHostRefreshTask(domainOrHostName);
}
} }
} }
} }

View file

@ -85,7 +85,7 @@ public final class DnsInjectionTest {
} }
@Test @Test
public void testWhoisHttpServer_injectsAndWorks() throws Exception { public void testRefreshDns_domain_injectsAndWorks() throws Exception {
persistActiveDomain("example.lol"); persistActiveDomain("example.lol");
when(req.getParameter("type")).thenReturn("domain"); when(req.getParameter("type")).thenReturn("domain");
when(req.getParameter("name")).thenReturn("example.lol"); when(req.getParameter("name")).thenReturn("example.lol");
@ -94,10 +94,27 @@ public final class DnsInjectionTest {
} }
@Test @Test
public void testWhoisHttpServer_missingDomain_throwsNotFound() throws Exception { public void testRefreshDns_missingDomain_throwsNotFound() throws Exception {
when(req.getParameter("type")).thenReturn("domain"); when(req.getParameter("type")).thenReturn("domain");
when(req.getParameter("name")).thenReturn("example.lol"); when(req.getParameter("name")).thenReturn("example.lol");
thrown.expect(NotFoundException.class, "DOMAIN example.lol not found"); thrown.expect(NotFoundException.class, "domain example.lol not found");
component.refreshDns().run();
}
@Test
public void testRefreshDns_host_injectsAndWorks() throws Exception {
persistActiveSubordinateHost("ns1.example.lol", persistActiveDomain("example.lol"));
when(req.getParameter("type")).thenReturn("host");
when(req.getParameter("name")).thenReturn("ns1.example.lol");
component.refreshDns().run();
assertDnsTasksEnqueued("ns1.example.lol");
}
@Test
public void testRefreshDns_missingHost_throwsNotFound() throws Exception {
when(req.getParameter("type")).thenReturn("host");
when(req.getParameter("name")).thenReturn("ns1.example.lol");
thrown.expect(NotFoundException.class, "host ns1.example.lol not found");
component.refreshDns().run(); component.refreshDns().run();
} }
} }