From 91049d2c53ef57c53f7dbb63b22e804c71af24b9 Mon Sep 17 00:00:00 2001 From: cgoldfeder Date: Mon, 6 Feb 2017 11:38:37 -0800 Subject: [PATCH] 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 --- .../batch/DeleteContactsAndHostsAction.java | 2 +- .../google/registry/dns/RefreshDnsAction.java | 2 +- .../registry/flows/host/HostUpdateFlow.java | 62 +++++++++---------- .../registry/model/host/HostResource.java | 4 ++ .../flows/host/HostUpdateFlowTest.java | 6 +- 5 files changed, 40 insertions(+), 36 deletions(-) diff --git a/java/google/registry/batch/DeleteContactsAndHostsAction.java b/java/google/registry/batch/DeleteContactsAndHostsAction.java index 4bab775ac..7fb6ce362 100644 --- a/java/google/registry/batch/DeleteContactsAndHostsAction.java +++ b/java/google/registry/batch/DeleteContactsAndHostsAction.java @@ -367,7 +367,7 @@ public class DeleteContactsAndHostsAction implements Runnable { historyEntryForDelete); } else if (existingResource instanceof HostResource) { HostResource host = (HostResource) existingResource; - if (host.getSuperordinateDomain() != null) { + if (host.isSubordinate()) { dnsQueue.addHostRefreshTask(host.getFullyQualifiedHostName()); ofy().save().entity( ofy().load().key(host.getSuperordinateDomain()).now().asBuilder() diff --git a/java/google/registry/dns/RefreshDnsAction.java b/java/google/registry/dns/RefreshDnsAction.java index 3ba1bc2f2..4c880d515 100644 --- a/java/google/registry/dns/RefreshDnsAction.java +++ b/java/google/registry/dns/RefreshDnsAction.java @@ -70,7 +70,7 @@ public final class RefreshDnsAction implements Runnable { } private static void verifyHostIsSubordinate(HostResource host) { - if (host.getSuperordinateDomain() == null) { + if (!host.isSubordinate()) { throw new BadRequestException( String.format("%s isn't a subordinate hostname", host.getFullyQualifiedHostName())); } diff --git a/java/google/registry/flows/host/HostUpdateFlow.java b/java/google/registry/flows/host/HostUpdateFlow.java index df838c956..c7ccb31a3 100644 --- a/java/google/registry/flows/host/HostUpdateFlow.java +++ b/java/google/registry/flows/host/HostUpdateFlow.java @@ -202,10 +202,10 @@ public final class HostUpdateFlow implements TransactionalFlow { private void verifyHasIpsIffIsExternal( Update command, HostResource existingResource, HostResource newResource) throws EppException { - boolean wasExternal = existingResource.getSuperordinateDomain() == null; - boolean wasSubordinate = !wasExternal; - boolean willBeExternal = newResource.getSuperordinateDomain() == null; - boolean willBeSubordinate = !willBeExternal; + boolean wasSubordinate = existingResource.isSubordinate(); + boolean wasExternal = !wasSubordinate; + boolean willBeSubordinate = newResource.isSubordinate(); + boolean willBeExternal = !willBeSubordinate; boolean newResourceHasIps = !isNullOrEmpty(newResource.getInetAddresses()); boolean commandAddsIps = !isNullOrEmpty(command.getInnerAdd().getInetAddresses()); // 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) { // 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. - if (existingResource.getSuperordinateDomain() != null) { + if (existingResource.isSubordinate()) { dnsQueue.addHostRefreshTask(existingResource.getFullyQualifiedHostName()); } // In case of a rename, there are many updates we need to queue up. if (((Update) resourceCommand).getInnerChange().getFullyQualifiedHostName() != null) { // If the renamed host is also subordinate, then we must enqueue an update to write the new // glue. - if (newResource.getSuperordinateDomain() != null) { + if (newResource.isSubordinate()) { dnsQueue.addHostRefreshTask(newResource.getFullyQualifiedHostName()); } // We must also enqueue updates for all domains that use this host as their nameserver so @@ -245,31 +245,31 @@ public final class HostUpdateFlow implements TransactionalFlow { } private void updateSuperordinateDomains(HostResource existingResource, HostResource newResource) { - Key oldSuperordinateDomain = existingResource.getSuperordinateDomain(); - Key newSuperordinateDomain = newResource.getSuperordinateDomain(); - if (oldSuperordinateDomain != null || newSuperordinateDomain != null) { - if (Objects.equals(oldSuperordinateDomain, newSuperordinateDomain)) { - ofy().save().entity( - ofy().load().key(oldSuperordinateDomain).now().asBuilder() - .removeSubordinateHost(existingResource.getFullyQualifiedHostName()) - .addSubordinateHost(newResource.getFullyQualifiedHostName()) - .build()); - } else { - if (oldSuperordinateDomain != null) { - ofy().save().entity( - ofy().load().key(oldSuperordinateDomain).now() - .asBuilder() - .removeSubordinateHost(existingResource.getFullyQualifiedHostName()) - .build()); - } - if (newSuperordinateDomain != null) { - ofy().save().entity( - ofy().load().key(newSuperordinateDomain).now() - .asBuilder() - .addSubordinateHost(newResource.getFullyQualifiedHostName()) - .build()); - } - } + if (existingResource.isSubordinate() + && newResource.isSubordinate() + && Objects.equals( + existingResource.getSuperordinateDomain(), + newResource.getSuperordinateDomain())) { + ofy().save().entity( + ofy().load().key(existingResource.getSuperordinateDomain()).now().asBuilder() + .removeSubordinateHost(existingResource.getFullyQualifiedHostName()) + .addSubordinateHost(newResource.getFullyQualifiedHostName()) + .build()); + return; + } + if (existingResource.isSubordinate()) { + ofy().save().entity( + ofy().load().key(existingResource.getSuperordinateDomain()).now() + .asBuilder() + .removeSubordinateHost(existingResource.getFullyQualifiedHostName()) + .build()); + } + if (newResource.isSubordinate()) { + ofy().save().entity( + ofy().load().key(newResource.getSuperordinateDomain()).now() + .asBuilder() + .addSubordinateHost(newResource.getFullyQualifiedHostName()) + .build()); } } diff --git a/java/google/registry/model/host/HostResource.java b/java/google/registry/model/host/HostResource.java index f2e981447..ef020fba4 100644 --- a/java/google/registry/model/host/HostResource.java +++ b/java/google/registry/model/host/HostResource.java @@ -96,6 +96,10 @@ public class HostResource extends EppResource implements ForeignKeyedEppResource return superordinateDomain; } + public boolean isSubordinate() { + return superordinateDomain != null; + } + public ImmutableSet getInetAddresses() { return nullToEmptyImmutableCopy(inetAddresses); } diff --git a/javatests/google/registry/flows/host/HostUpdateFlowTest.java b/javatests/google/registry/flows/host/HostUpdateFlowTest.java index 0264328ca..924b87a87 100644 --- a/javatests/google/registry/flows/host/HostUpdateFlowTest.java +++ b/javatests/google/registry/flows/host/HostUpdateFlowTest.java @@ -139,7 +139,7 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase oldFkiBeforeRename = @@ -162,7 +162,7 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase