zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-05-13 07:57:13 +02:00
Code Issues Projects Releases Packages Wiki Activity

Improve handling of lastSubordinateChange and beef up tests

Browse source 
See [] for the comments that led to this.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=147796087
This commit is contained in:
cgoldfeder 2017-02-16 18:54:16 -08:00 • committed by Ben McIlwain
parent be30ecdf66
commit c23bbe35bb
5 changed files with 159 additions and 124 deletions
Download patch file Download diff file Expand all files Collapse all files

20
java/google/registry/flows/host/HostUpdateFlow.java
View file

@ -128,9 +128,10 @@ public final class HostUpdateFlow implements TransactionalFlow {
boolean isHostRename = suppliedNewHostName != null;
String oldHostName = targetId;
String newHostName = firstNonNull(suppliedNewHostName, oldHostName);
Optional<DomainResource> superordinateDomain =
// Note that lookupSuperordinateDomain calls cloneProjectedAtTime on the domain for us.
Optional<DomainResource> newSuperordinateDomain =
Optional.fromNullable(lookupSuperordinateDomain(validateHostName(newHostName), now));
verifyUpdateAllowed(command, existingHost, superordinateDomain.orNull());
verifyUpdateAllowed(command, existingHost, newSuperordinateDomain.orNull());
if (isHostRename && loadAndGetKey(HostResource.class, newHostName, now) != null) {
throw new HostAlreadyExistsException(newHostName);
}
@ -138,6 +139,13 @@ public final class HostUpdateFlow implements TransactionalFlow {
AddRemove remove = command.getInnerRemove();
checkSameValuesNotAddedAndRemoved(add.getStatusValues(), remove.getStatusValues());
checkSameValuesNotAddedAndRemoved(add.getInetAddresses(), remove.getInetAddresses());
Key<DomainResource> newSuperordinateDomainKey =
newSuperordinateDomain.isPresent() ? Key.create(newSuperordinateDomain.get()) : null;
// If the superordinateDomain field is changing, set the lastSuperordinateChange to now.
DateTime lastSuperordinateChange =
Objects.equals(newSuperordinateDomainKey, existingHost.getSuperordinateDomain())
? existingHost.getLastSuperordinateChange()
: now;
HostResource newHost = existingHost.asBuilder()
.setFullyQualifiedHostName(newHostName)
.addStatusValues(add.getStatusValues())
@ -146,12 +154,8 @@ public final class HostUpdateFlow implements TransactionalFlow {
.removeInetAddresses(remove.getInetAddresses())
.setLastEppUpdateTime(now)
.setLastEppUpdateClientId(clientId)
// The superordinateDomain can be missing if the new name is external.
// Note that the value of superordinateDomain is projected to the current time inside of
// the lookupSuperordinateDomain(...) call above, so that it will never be stale.
.setSuperordinateDomain(
superordinateDomain.isPresent() ? Key.create(superordinateDomain.get()) : null)
.setLastSuperordinateChange(superordinateDomain.isPresent() ? now : null)
.setSuperordinateDomain(newSuperordinateDomainKey)
.setLastSuperordinateChange(lastSuperordinateChange)
.build()
// Rely on the host's cloneProjectedAtTime() method to handle setting of transfer data.
.cloneProjectedAtTime(now);

7
java/google/registry/model/host/HostResource.java
View file

@ -85,8 +85,11 @@ public class HostResource extends EppResource implements ForeignKeyedEppResource
DateTime lastTransferTime;
/**
* The most recent time that the superordinate domain was changed, or null if this host is
* external.
* The most recent time that the {@link #superordinateDomain} field was changed.
*
* <p>This should be updated whenever the superordinate domain changes, including when it is set
* to null. This field will be null for new hosts that have never experienced a change of
* superordinate domain.
*/
DateTime lastSuperordinateChange;

27
javatests/google/registry/flows/host/HostCreateFlowTest.java
View file

@ -15,7 +15,7 @@
package google.registry.flows.host;
import static com.google.common.truth.Truth.assertThat;
import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.model.EppResourceUtils.loadByForeignKey;
import static google.registry.testing.DatastoreHelper.assertNoBillingEvents;
import static google.registry.testing.DatastoreHelper.createTld;
import static google.registry.testing.DatastoreHelper.persistActiveDomain;
@ -27,6 +27,7 @@ import static google.registry.testing.TaskQueueHelper.assertNoDnsTasksEnqueued;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import com.googlecode.objectify.Key;
import google.registry.flows.EppXmlTransformer.IpAddressVersionMismatchException;
import google.registry.flows.ResourceFlowTestCase;
import google.registry.flows.exceptions.ResourceAlreadyExistsException;
@ -39,6 +40,7 @@ import google.registry.flows.host.HostFlowUtils.HostNameTooLongException;
import google.registry.flows.host.HostFlowUtils.HostNameTooShallowException;
import google.registry.flows.host.HostFlowUtils.InvalidHostNameException;
import google.registry.flows.host.HostFlowUtils.SuperordinateDomainDoesNotExistException;
import google.registry.model.domain.DomainResource;
import google.registry.model.host.HostResource;
import google.registry.model.reporting.HistoryEntry;
import org.joda.time.DateTime;
@ -80,6 +82,7 @@ public class HostCreateFlowTest extends ResourceFlowTestCase<HostCreateFlow, Hos
runFlowAssertResponse(readFile("host_create_response.xml"));
// Check that the host was created and persisted with a history entry.
assertAboutHosts().that(reloadResourceByForeignKey())
.hasLastSuperordinateChange(null).and()
.hasOnlyOneHistoryEntryWhich()
.hasType(HistoryEntry.Type.HOST_CREATE);
assertNoBillingEvents();
@ -101,17 +104,18 @@ public class HostCreateFlowTest extends ResourceFlowTestCase<HostCreateFlow, Hos
@Test
public void testSuccess_externalNeverExisted() throws Exception {
doSuccessfulTest();
assertAboutHosts().that(reloadResourceByForeignKey()).hasSuperordinateDomain(null);
assertNoDnsTasksEnqueued();
}
@Test
public void testSuccess_internalNeverExisted() throws Exception {
doSuccessfulInternalTest("tld");
assertThat(ofy().load().key(reloadResourceByForeignKey().getSuperordinateDomain())
.now().getFullyQualifiedDomainName())
.isEqualTo("example.tld");
assertThat(ofy().load().key(reloadResourceByForeignKey().getSuperordinateDomain())
.now().getSubordinateHosts()).containsExactly("ns1.example.tld");
HostResource host = reloadResourceByForeignKey();
DomainResource superordinateDomain =
loadByForeignKey(DomainResource.class, "example.tld", clock.nowUtc());
assertAboutHosts().that(host).hasSuperordinateDomain(Key.create(superordinateDomain));
assertThat(superordinateDomain.getSubordinateHosts()).containsExactly("ns1.example.tld");
assertDnsTasksEnqueued("ns1.example.tld");
}
@ -119,6 +123,7 @@ public class HostCreateFlowTest extends ResourceFlowTestCase<HostCreateFlow, Hos
public void testSuccess_externalExistedButWasDeleted() throws Exception {
persistDeletedHost(getUniqueIdFromCommand(), clock.nowUtc().minusDays(1));
doSuccessfulTest();
assertAboutHosts().that(reloadResourceByForeignKey()).hasSuperordinateDomain(null);
assertNoDnsTasksEnqueued();
}
@ -126,11 +131,11 @@ public class HostCreateFlowTest extends ResourceFlowTestCase<HostCreateFlow, Hos
public void testSuccess_internalExistedButWasDeleted() throws Exception {
persistDeletedHost(getUniqueIdFromCommand(), clock.nowUtc().minusDays(1));
doSuccessfulInternalTest("tld");
assertThat(ofy().load().key(reloadResourceByForeignKey().getSuperordinateDomain())
.now().getFullyQualifiedDomainName())
.isEqualTo("example.tld");
assertThat(ofy().load().key(reloadResourceByForeignKey().getSuperordinateDomain())
.now().getSubordinateHosts()).containsExactly("ns1.example.tld");
HostResource host = reloadResourceByForeignKey();
DomainResource superordinateDomain =
loadByForeignKey(DomainResource.class, "example.tld", clock.nowUtc());
assertAboutHosts().that(host).hasSuperordinateDomain(Key.create(superordinateDomain));
assertThat(superordinateDomain.getSubordinateHosts()).containsExactly("ns1.example.tld");
assertDnsTasksEnqueued("ns1.example.tld");
}

203
javatests/google/registry/flows/host/HostUpdateFlowTest.java
View file

@ -18,6 +18,7 @@ import static com.google.common.base.Strings.nullToEmpty;
import static com.google.common.truth.Truth.assertThat;
import static google.registry.flows.async.AsyncFlowEnqueuer.QUEUE_ASYNC_HOST_RENAME;
import static google.registry.model.EppResourceUtils.loadByForeignKey;
import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.testing.DatastoreHelper.assertNoBillingEvents;
import static google.registry.testing.DatastoreHelper.createTld;
import static google.registry.testing.DatastoreHelper.getOnlyHistoryEntryOfType;
@ -174,80 +175,107 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Hos
setEppInput("host_update_name_unchanged.xml");
createTld("tld");
DomainResource domain = persistActiveDomain("example.tld");
persistActiveSubordinateHost(oldHostName(), domain);
HostResource oldHost = persistActiveSubordinateHost(oldHostName(), domain);
clock.advanceOneMilli();
runFlowAssertResponse(readFile("host_update_response.xml"));
// The example xml doesn't do a host rename, so reloading the host should work.
assertAboutHosts().that(reloadResourceByForeignKey())
.hasLastSuperordinateChange(oldHost.getLastSuperordinateChange()).and()
.hasSuperordinateDomain(Key.create(domain)).and()
.hasOnlyOneHistoryEntryWhich()
.hasType(HistoryEntry.Type.HOST_UPDATE);
assertDnsTasksEnqueued("ns1.example.tld");
}
@Test
public void testSuccess_renameWithSameSuperordinateDomain() throws Exception {
public void testSuccess_internalToInternalOnSameDomain() throws Exception {
setEppHostUpdateInput(
"ns1.example.tld",
"ns2.example.tld",
"<host:addr ip=\"v4\">192.0.2.22</host:addr>",
"<host:addr ip=\"v6\">1080:0:0:0:8:800:200C:417A</host:addr>");
createTld("tld");
DomainResource domain = persistActiveDomain("example.tld");
persistActiveSubordinateHost(oldHostName(), domain);
persistResource(
domain.asBuilder()
DomainResource domain = persistResource(newDomainResource("example.tld")
.asBuilder()
.setSubordinateHosts(ImmutableSet.of(oldHostName()))
.build());
assertThat(
loadByForeignKey(
DomainResource.class, "example.tld", clock.nowUtc()).getSubordinateHosts())
.containsExactly("ns1.example.tld");
HostResource oldHost = persistActiveSubordinateHost(oldHostName(), domain);
assertThat(domain.getSubordinateHosts()).containsExactly("ns1.example.tld");
HostResource renamedHost = doSuccessfulTest();
assertThat(renamedHost.getSuperordinateDomain()).isEqualTo(Key.create(domain));
assertThat(
loadByForeignKey(
DomainResource.class, "example.tld", clock.nowUtc()).getSubordinateHosts())
.containsExactly("ns2.example.tld");
assertAboutHosts().that(renamedHost)
.hasSuperordinateDomain(Key.create(domain)).and()
.hasLastSuperordinateChange(oldHost.getLastSuperordinateChange());
DomainResource reloadedDomain =
ofy().load().entity(domain).now().cloneProjectedAtTime(clock.nowUtc());
assertThat(reloadedDomain.getSubordinateHosts()).containsExactly("ns2.example.tld");
assertDnsTasksEnqueued("ns1.example.tld", "ns2.example.tld");
}
@Test
public void testSuccess_internalToSameInternal() throws Exception {
public void testSuccess_internalToInternalOnSameTld() throws Exception {
setEppHostUpdateInput(
"ns2.foo.tld",
"ns2.example.tld",
"<host:addr ip=\"v4\">192.0.2.22</host:addr>",
"<host:addr ip=\"v6\">1080:0:0:0:8:800:200C:417A</host:addr>");
createTld("tld");
DomainResource foo = persistActiveDomain("foo.tld");
DomainResource example = persistActiveDomain("example.tld");
persistActiveSubordinateHost(oldHostName(), foo);
persistResource(
foo.asBuilder()
DomainResource foo = persistResource(newDomainResource("foo.tld").asBuilder()
.setSubordinateHosts(ImmutableSet.of(oldHostName()))
.build());
assertThat(
loadByForeignKey(
DomainResource.class, "foo.tld", clock.nowUtc()).getSubordinateHosts())
.containsExactly("ns2.foo.tld");
assertThat(
loadByForeignKey(
DomainResource.class, "example.tld", clock.nowUtc()).getSubordinateHosts())
.isEmpty();
persistActiveSubordinateHost(oldHostName(), foo);
assertThat(foo.getSubordinateHosts()).containsExactly("ns2.foo.tld");
assertThat(example.getSubordinateHosts()).isEmpty();
HostResource renamedHost = doSuccessfulTest();
assertThat(renamedHost.getSuperordinateDomain()).isEqualTo(Key.create(example));
assertThat(
loadByForeignKey(
DomainResource.class, "foo.tld", clock.nowUtc()).getSubordinateHosts())
DateTime now = clock.nowUtc();
assertAboutHosts().that(renamedHost)
.hasSuperordinateDomain(Key.create(example)).and()
.hasLastSuperordinateChange(now);
assertThat(ofy().load().entity(foo).now().cloneProjectedAtTime(now).getSubordinateHosts())
.isEmpty();
assertThat(
loadByForeignKey(
DomainResource.class, "example.tld", clock.nowUtc()).getSubordinateHosts())
assertThat(ofy().load().entity(example).now().cloneProjectedAtTime(now).getSubordinateHosts())
.containsExactly("ns2.example.tld");
assertDnsTasksEnqueued("ns2.foo.tld", "ns2.example.tld");
}
@Test
public void testSuccess_internalToInternalOnDifferentTld() throws Exception {
setEppHostUpdateInput(
"ns1.example.foo",
"ns2.example.tld",
"<host:addr ip=\"v4\">192.0.2.22</host:addr>",
"<host:addr ip=\"v6\">1080:0:0:0:8:800:200C:417A</host:addr>");
createTld("foo");
createTld("tld");
DomainResource fooDomain = persistResource(newDomainResource("example.foo").asBuilder()
.setSubordinateHosts(ImmutableSet.of(oldHostName()))
.build());
DomainResource tldDomain = persistActiveDomain("example.tld");
HostResource oldHost = persistActiveSubordinateHost(oldHostName(), fooDomain);
assertThat(oldHost.getCurrentSponsorClientId()).isEqualTo("TheRegistrar");
assertThat(fooDomain.getSubordinateHosts()).containsExactly("ns1.example.foo");
assertThat(tldDomain.getSubordinateHosts()).isEmpty();
HostResource renamedHost = doSuccessfulTest();
DateTime now = clock.nowUtc();
assertAboutHosts().that(renamedHost)
.hasSuperordinateDomain(Key.create(tldDomain)).and()
.hasLastSuperordinateChange(now);
DomainResource reloadedFooDomain =
ofy().load().entity(fooDomain).now().cloneProjectedAtTime(now);
assertThat(reloadedFooDomain.getSubordinateHosts()).isEmpty();
DomainResource reloadedTldDomain =
ofy().load().entity(tldDomain).now().cloneProjectedAtTime(now);
assertThat(reloadedTldDomain.getSubordinateHosts()).containsExactly("ns2.example.tld");
assertDnsTasksEnqueued("ns1.example.foo", "ns2.example.tld");
// Ensure that the client id is read off the domain because this is a subordinate host now.
persistResource(
tldDomain.asBuilder().setCurrentSponsorClientId("it_should_be_this").build());
assertThat(
renamedHost.cloneProjectedAtTime(clock.nowUtc().plusMinutes(1)).getCurrentSponsorClientId())
.isEqualTo("it_should_be_this");
}
@Test
public void testSuccess_internalToExternal() throws Exception {
setEppHostUpdateInput(
@ -256,62 +284,28 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Hos
null,
"<host:addr ip=\"v6\">1080:0:0:0:8:800:200C:417A</host:addr>");
createTld("foo");
DomainResource domain = persistActiveDomain("example.foo");
persistActiveSubordinateHost(oldHostName(), domain);
persistResource(
domain.asBuilder()
DomainResource domain = persistResource(newDomainResource("example.foo").asBuilder()
.setSubordinateHosts(ImmutableSet.of(oldHostName()))
.build());
assertThat(
loadByForeignKey(
DomainResource.class, "example.foo", clock.nowUtc()).getSubordinateHosts())
.containsExactly("ns1.example.foo");
assertThat(domain.getCurrentSponsorClientId()).isEqualTo("TheRegistrar");
HostResource oldHost = persistResource(
persistActiveSubordinateHost(oldHostName(), domain).asBuilder()
.setCurrentSponsorClientId("ClientThatShouldBeSupersededByDomainClient")
.build());
assertThat(oldHost.isSubordinate()).isTrue();
assertThat(domain.getSubordinateHosts()).containsExactly("ns1.example.foo");
HostResource renamedHost = doSuccessfulTest();
assertThat(renamedHost.isSubordinate()).isFalse();
// 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).
assertThat(renamedHost.getCurrentSponsorClientId()).isEqualTo("TheRegistrar");
assertThat(
loadByForeignKey(
DomainResource.class, "example.foo", clock.nowUtc()).getSubordinateHosts())
.isEmpty();
assertAboutHosts().that(renamedHost)
// Ensure that the client id is copied off of the superordinate domain.
.hasCurrentSponsorClientId("TheRegistrar").and()
.hasSuperordinateDomain(null).and()
.hasLastSuperordinateChange(clock.nowUtc());
DomainResource reloadedDomain =
ofy().load().entity(domain).now().cloneProjectedAtTime(clock.nowUtc());
assertThat(reloadedDomain.getSubordinateHosts()).isEmpty();
assertDnsTasksEnqueued("ns1.example.foo");
}
@Test
public void testSuccess_internalToDifferentInternal() throws Exception {
setEppHostUpdateInput(
"ns1.example.foo",
"ns2.example.tld",
"<host:addr ip=\"v4\">192.0.2.22</host:addr>",
"<host:addr ip=\"v6\">1080:0:0:0:8:800:200C:417A</host:addr>");
createTld("foo");
persistActiveDomain("example.foo");
createTld("tld");
DomainResource tldDomain = persistActiveDomain("example.tld");
persistActiveHost(oldHostName());
assertThat(loadByForeignKey(
HostResource.class, oldHostName(), clock.nowUtc()).getCurrentSponsorClientId())
.isEqualTo("TheRegistrar");
assertThat(
loadByForeignKey(
DomainResource.class, "example.tld", clock.nowUtc()).getSubordinateHosts())
.isEmpty();
HostResource renamedHost = doSuccessfulTest();
assertThat(renamedHost.getSuperordinateDomain()).isEqualTo(Key.create(tldDomain));
assertThat(loadByForeignKey(
DomainResource.class, "example.tld", clock.nowUtc()).getSubordinateHosts())
.containsExactly("ns2.example.tld");
assertDnsTasksEnqueued("ns2.example.tld");
// Ensure that the client id is read off the domain because this is a subordinate host now.
persistResource(
tldDomain.asBuilder().setCurrentSponsorClientId("it_should_be_this").build());
assertThat(
renamedHost.cloneProjectedAtTime(clock.nowUtc().plusMinutes(1)).getCurrentSponsorClientId())
.isEqualTo("it_should_be_this");
}
@Test
public void testSuccess_externalToInternal() throws Exception {
setEppHostUpdateInput(
@ -321,28 +315,41 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Hos
null);
createTld("tld");
DomainResource domain = persistActiveDomain("example.tld");
persistActiveHost(oldHostName());
assertThat(loadByForeignKey(
HostResource.class, oldHostName(), clock.nowUtc()).getCurrentSponsorClientId())
.isEqualTo("TheRegistrar");
assertThat(loadByForeignKey(
DomainResource.class, "example.tld", clock.nowUtc()).getSubordinateHosts())
.isEmpty();
HostResource oldHost = persistActiveHost(oldHostName());
assertThat(oldHost.getCurrentSponsorClientId()).isEqualTo("TheRegistrar");
assertThat(domain.getSubordinateHosts()).isEmpty();
HostResource renamedHost = doSuccessfulTest();
assertThat(renamedHost.getSuperordinateDomain()).isEqualTo(Key.create(domain));
assertThat(loadByForeignKey(
DomainResource.class, "example.tld", clock.nowUtc()).getSubordinateHosts())
DateTime now = clock.nowUtc();
assertAboutHosts().that(renamedHost)
.hasSuperordinateDomain(Key.create(domain)).and()
.hasLastSuperordinateChange(now);
assertThat(ofy().load().entity(domain).now().cloneProjectedAtTime(now).getSubordinateHosts())
.containsExactly("ns2.example.tld");
assertDnsTasksEnqueued("ns2.example.tld");
// Ensure that the client id is read off the domain because this is a subordinate host now.
persistResource(
domain.asBuilder().setCurrentSponsorClientId("it_should_be_this").build());
assertThat(
renamedHost.cloneProjectedAtTime(clock.nowUtc().plusMinutes(1)).getCurrentSponsorClientId())
renamedHost.cloneProjectedAtTime(now.plusMinutes(1)).getCurrentSponsorClientId())
.isEqualTo("it_should_be_this");
}
@Test
public void testSuccess_externalToExternal() throws Exception {
setEppHostUpdateInput(
"ns1.example.foo",
"ns2.example.tld",
null,
null);
HostResource oldHost = persistActiveHost(oldHostName());
assertThat(oldHost.getCurrentSponsorClientId()).isEqualTo("TheRegistrar");
HostResource renamedHost = doSuccessfulTest();
assertAboutHosts().that(renamedHost)
.hasSuperordinateDomain(null).and()
.hasLastSuperordinateChange(null);
assertNoDnsTasksEnqueued();
}
@Test
public void testSuccess_superuserClientUpdateProhibited() throws Exception {
setEppInput("host_update_add_status.xml");
@ -550,7 +557,7 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Hos
// update, not what it is later changed to.
assertThat(renamedHost.getLastTransferTime()).isEqualTo(lastTransferTime);
// External hosts should always have null lastSuperordinateChange.
assertThat(renamedHost.getLastSuperordinateChange()).isNull();
assertThat(renamedHost.getLastSuperordinateChange()).isEqualTo(clock.nowUtc());
}
@Test

16
javatests/google/registry/testing/HostResourceSubject.java
View file

@ -19,6 +19,8 @@ import static com.google.common.truth.Truth.assertAbout;
import com.google.common.truth.AbstractVerb.DelegatedVerb;
import com.google.common.truth.FailureStrategy;
import com.googlecode.objectify.Key;
import google.registry.model.domain.DomainResource;
import google.registry.model.host.HostResource;
import google.registry.testing.TruthChainer.And;
import org.joda.time.DateTime;
@ -52,4 +54,18 @@ public final class HostResourceSubject
actual().getLastTransferTime(),
"lastTransferTime");
}
public And<HostResourceSubject> hasLastSuperordinateChange(DateTime lastSuperordinateChange) {
return hasValue(
lastSuperordinateChange,
actual().getLastSuperordinateChange(),
"has lastSuperordinateChange");
}
public And<HostResourceSubject> hasSuperordinateDomain(Key<DomainResource> superordinateDomain) {
return hasValue(
superordinateDomain,
actual().getSuperordinateDomain(),
"has superordinateDomain");
}
}