Replace 'host.getSubordinateHost() != null' with 'host.isSubordinate()'

This is a cleanup in preparation for the next change that does a lot
of work with subordinate hosts, to make it easier to reason about in
complex code.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=146689904
This commit is contained in:
cgoldfeder 2017-02-06 11:38:37 -08:00 committed by Ben McIlwain
parent 07c2dfb976
commit 91049d2c53
5 changed files with 40 additions and 36 deletions

View file

@ -367,7 +367,7 @@ public class DeleteContactsAndHostsAction implements Runnable {
historyEntryForDelete); historyEntryForDelete);
} else if (existingResource instanceof HostResource) { } else if (existingResource instanceof HostResource) {
HostResource host = (HostResource) existingResource; HostResource host = (HostResource) existingResource;
if (host.getSuperordinateDomain() != null) { if (host.isSubordinate()) {
dnsQueue.addHostRefreshTask(host.getFullyQualifiedHostName()); dnsQueue.addHostRefreshTask(host.getFullyQualifiedHostName());
ofy().save().entity( ofy().save().entity(
ofy().load().key(host.getSuperordinateDomain()).now().asBuilder() ofy().load().key(host.getSuperordinateDomain()).now().asBuilder()

View file

@ -70,7 +70,7 @@ public final class RefreshDnsAction implements Runnable {
} }
private static void verifyHostIsSubordinate(HostResource host) { private static void verifyHostIsSubordinate(HostResource host) {
if (host.getSuperordinateDomain() == null) { if (!host.isSubordinate()) {
throw new BadRequestException( throw new BadRequestException(
String.format("%s isn't a subordinate hostname", host.getFullyQualifiedHostName())); String.format("%s isn't a subordinate hostname", host.getFullyQualifiedHostName()));
} }

View file

@ -202,10 +202,10 @@ public final class HostUpdateFlow implements TransactionalFlow {
private void verifyHasIpsIffIsExternal( private void verifyHasIpsIffIsExternal(
Update command, HostResource existingResource, HostResource newResource) throws EppException { Update command, HostResource existingResource, HostResource newResource) throws EppException {
boolean wasExternal = existingResource.getSuperordinateDomain() == null; boolean wasSubordinate = existingResource.isSubordinate();
boolean wasSubordinate = !wasExternal; boolean wasExternal = !wasSubordinate;
boolean willBeExternal = newResource.getSuperordinateDomain() == null; boolean willBeSubordinate = newResource.isSubordinate();
boolean willBeSubordinate = !willBeExternal; boolean willBeExternal = !willBeSubordinate;
boolean newResourceHasIps = !isNullOrEmpty(newResource.getInetAddresses()); boolean newResourceHasIps = !isNullOrEmpty(newResource.getInetAddresses());
boolean commandAddsIps = !isNullOrEmpty(command.getInnerAdd().getInetAddresses()); boolean commandAddsIps = !isNullOrEmpty(command.getInnerAdd().getInetAddresses());
// These checks are order-dependent. For example a subordinate-to-external rename that adds new // These checks are order-dependent. For example a subordinate-to-external rename that adds new
@ -228,14 +228,14 @@ public final class HostUpdateFlow implements TransactionalFlow {
private void enqueueTasks(HostResource existingResource, HostResource newResource) { private void enqueueTasks(HostResource existingResource, HostResource newResource) {
// Only update DNS for subordinate hosts. External hosts have no glue to write, so they // Only update DNS for subordinate hosts. External hosts have no glue to write, so they
// are only written as NS records from the referencing domain. // are only written as NS records from the referencing domain.
if (existingResource.getSuperordinateDomain() != null) { if (existingResource.isSubordinate()) {
dnsQueue.addHostRefreshTask(existingResource.getFullyQualifiedHostName()); dnsQueue.addHostRefreshTask(existingResource.getFullyQualifiedHostName());
} }
// In case of a rename, there are many updates we need to queue up. // In case of a rename, there are many updates we need to queue up.
if (((Update) resourceCommand).getInnerChange().getFullyQualifiedHostName() != null) { if (((Update) resourceCommand).getInnerChange().getFullyQualifiedHostName() != null) {
// If the renamed host is also subordinate, then we must enqueue an update to write the new // If the renamed host is also subordinate, then we must enqueue an update to write the new
// glue. // glue.
if (newResource.getSuperordinateDomain() != null) { if (newResource.isSubordinate()) {
dnsQueue.addHostRefreshTask(newResource.getFullyQualifiedHostName()); dnsQueue.addHostRefreshTask(newResource.getFullyQualifiedHostName());
} }
// We must also enqueue updates for all domains that use this host as their nameserver so // We must also enqueue updates for all domains that use this host as their nameserver so
@ -245,33 +245,33 @@ public final class HostUpdateFlow implements TransactionalFlow {
} }
private void updateSuperordinateDomains(HostResource existingResource, HostResource newResource) { private void updateSuperordinateDomains(HostResource existingResource, HostResource newResource) {
Key<DomainResource> oldSuperordinateDomain = existingResource.getSuperordinateDomain(); if (existingResource.isSubordinate()
Key<DomainResource> newSuperordinateDomain = newResource.getSuperordinateDomain(); && newResource.isSubordinate()
if (oldSuperordinateDomain != null || newSuperordinateDomain != null) { && Objects.equals(
if (Objects.equals(oldSuperordinateDomain, newSuperordinateDomain)) { existingResource.getSuperordinateDomain(),
newResource.getSuperordinateDomain())) {
ofy().save().entity( ofy().save().entity(
ofy().load().key(oldSuperordinateDomain).now().asBuilder() ofy().load().key(existingResource.getSuperordinateDomain()).now().asBuilder()
.removeSubordinateHost(existingResource.getFullyQualifiedHostName()) .removeSubordinateHost(existingResource.getFullyQualifiedHostName())
.addSubordinateHost(newResource.getFullyQualifiedHostName()) .addSubordinateHost(newResource.getFullyQualifiedHostName())
.build()); .build());
} else { return;
if (oldSuperordinateDomain != null) { }
if (existingResource.isSubordinate()) {
ofy().save().entity( ofy().save().entity(
ofy().load().key(oldSuperordinateDomain).now() ofy().load().key(existingResource.getSuperordinateDomain()).now()
.asBuilder() .asBuilder()
.removeSubordinateHost(existingResource.getFullyQualifiedHostName()) .removeSubordinateHost(existingResource.getFullyQualifiedHostName())
.build()); .build());
} }
if (newSuperordinateDomain != null) { if (newResource.isSubordinate()) {
ofy().save().entity( ofy().save().entity(
ofy().load().key(newSuperordinateDomain).now() ofy().load().key(newResource.getSuperordinateDomain()).now()
.asBuilder() .asBuilder()
.addSubordinateHost(newResource.getFullyQualifiedHostName()) .addSubordinateHost(newResource.getFullyQualifiedHostName())
.build()); .build());
} }
} }
}
}
/** Host with specified name already exists. */ /** Host with specified name already exists. */
static class HostAlreadyExistsException extends ObjectAlreadyExistsException { static class HostAlreadyExistsException extends ObjectAlreadyExistsException {

View file

@ -96,6 +96,10 @@ public class HostResource extends EppResource implements ForeignKeyedEppResource
return superordinateDomain; return superordinateDomain;
} }
public boolean isSubordinate() {
return superordinateDomain != null;
}
public ImmutableSet<InetAddress> getInetAddresses() { public ImmutableSet<InetAddress> getInetAddresses() {
return nullToEmptyImmutableCopy(inetAddresses); return nullToEmptyImmutableCopy(inetAddresses);
} }

View file

@ -139,7 +139,7 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Hos
public void testSuccess_rename_noOtherHostEverUsedTheOldName() throws Exception { public void testSuccess_rename_noOtherHostEverUsedTheOldName() throws Exception {
persistActiveHost(oldHostName()); persistActiveHost(oldHostName());
HostResource renamedHost = doSuccessfulTest(); HostResource renamedHost = doSuccessfulTest();
assertThat(renamedHost.getSuperordinateDomain()).isNull(); assertThat(renamedHost.isSubordinate()).isFalse();
assertNoDnsTasksEnqueued(); // No tasks enqueued since it's a rename of an external host. assertNoDnsTasksEnqueued(); // No tasks enqueued since it's a rename of an external host.
// The old ForeignKeyIndex is invalidated at the time we did the rename. // The old ForeignKeyIndex is invalidated at the time we did the rename.
ForeignKeyIndex<HostResource> oldFkiBeforeRename = ForeignKeyIndex<HostResource> oldFkiBeforeRename =
@ -162,7 +162,7 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Hos
.setNameservers(ImmutableSet.of(Key.create(host))) .setNameservers(ImmutableSet.of(Key.create(host)))
.build()); .build());
HostResource renamedHost = doSuccessfulTest(); HostResource renamedHost = doSuccessfulTest();
assertThat(renamedHost.getSuperordinateDomain()).isNull(); assertThat(renamedHost.isSubordinate()).isFalse();
// Task enqueued to change the NS record of the referencing domain via mapreduce. // Task enqueued to change the NS record of the referencing domain via mapreduce.
assertTasksEnqueued( assertTasksEnqueued(
QUEUE_ASYNC_HOST_RENAME, QUEUE_ASYNC_HOST_RENAME,
@ -267,7 +267,7 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Hos
DomainResource.class, "example.foo", clock.nowUtc()).getSubordinateHosts()) DomainResource.class, "example.foo", clock.nowUtc()).getSubordinateHosts())
.containsExactly("ns1.example.foo"); .containsExactly("ns1.example.foo");
HostResource renamedHost = doSuccessfulTest(); HostResource renamedHost = doSuccessfulTest();
assertThat(renamedHost.getSuperordinateDomain()).isNull(); assertThat(renamedHost.isSubordinate()).isFalse();
// Ensure that the client id is set to the new registrar correctly (and that this necessarily // Ensure that the client id is set to the new registrar correctly (and that this necessarily
// comes from the field on the host itself, because the superordinate domain is null). // comes from the field on the host itself, because the superordinate domain is null).
assertThat(renamedHost.getCurrentSponsorClientId()).isEqualTo("TheRegistrar"); assertThat(renamedHost.getCurrentSponsorClientId()).isEqualTo("TheRegistrar");