From 50c5f856a285ce2464b456454162897bb759668b Mon Sep 17 00:00:00 2001 From: cgoldfeder Date: Tue, 31 Jan 2017 13:28:41 -0800 Subject: [PATCH] Document StatusValue better and add per-resource restrictions This generalizes the "LINKED can't be anywhere" idea into more targeted restrictions. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=146158204 --- java/google/registry/model/EppResource.java | 11 ++- .../registry/model/eppcommon/StatusValue.java | 77 +++++++++++++++++-- .../rde/DomainResourceToXjcConverterTest.java | 4 +- .../rde/HostResourceToXjcConverterTest.java | 8 +- .../google/registry/rde/RdeFixtures.java | 4 +- ...pReduce_withDomain_producesExpectedXml.xml | 4 +- .../GenerateEscrowDepositCommandTest.java | 4 +- .../xn--q9jyb4c_2010-10-17_full_S1_R0.xml | 2 +- 8 files changed, 88 insertions(+), 26 deletions(-) diff --git a/java/google/registry/model/EppResource.java b/java/google/registry/model/EppResource.java index 16d7c2223..b2a308c61 100644 --- a/java/google/registry/model/EppResource.java +++ b/java/google/registry/model/EppResource.java @@ -243,9 +243,14 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable { /** Set this resource's status values. */ public B setStatusValues(ImmutableSet statusValues) { - checkArgument( - !nullToEmpty(statusValues).contains(StatusValue.LINKED), - "LINKED is a virtual status value that should never be set on an EppResource"); + Class resourceClass = getInstance().getClass(); + for (StatusValue statusValue : nullToEmpty(statusValues)) { + checkArgument( + !statusValue.isForbiddenOn(resourceClass), + "The %s status cannot be set on %s", + statusValue, + resourceClass.getSimpleName()); + } getInstance().status = statusValues; return thisCastToDerived(); } diff --git a/java/google/registry/model/eppcommon/StatusValue.java b/java/google/registry/model/eppcommon/StatusValue.java index 3a0626596..297515040 100644 --- a/java/google/registry/model/eppcommon/StatusValue.java +++ b/java/google/registry/model/eppcommon/StatusValue.java @@ -18,6 +18,12 @@ import static com.google.common.base.CaseFormat.LOWER_CAMEL; import static com.google.common.base.CaseFormat.UPPER_UNDERSCORE; import static com.google.common.base.Strings.nullToEmpty; +import com.google.common.collect.ImmutableSet; +import google.registry.model.EppResource; +import google.registry.model.contact.ContactResource; +import google.registry.model.domain.DomainApplication; +import google.registry.model.domain.DomainResource; +import google.registry.model.host.HostResource; import google.registry.model.translators.EnumToAttributeAdapter.EppEnum; import google.registry.model.translators.StatusValueAdapter; import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter; @@ -43,22 +49,69 @@ public enum StatusValue implements EppEnum { CLIENT_RENEW_PROHIBITED, CLIENT_TRANSFER_PROHIBITED, CLIENT_UPDATE_PROHIBITED, - INACTIVE, /** - * A status that means a resource has an incoming reference from an active domain. + * A status for a domain with no nameservers that has all the other requirements for {@link #OK}. + * + *

Only domains can have this status, and it supersedes OK. + */ + INACTIVE(ContactResource.class, HostResource.class, DomainApplication.class), + + /** + * A status for a resource has an incoming reference from an active domain. * *

LINKED is a "virtual" status value that should never be persisted to Datastore on any * resource. It must be computed on the fly when we need it, as the set of domains using a * resource can change at any time. */ - LINKED, + LINKED(ContactResource.class, DomainApplication.class, DomainResource.class, HostResource.class), + /** + * A status for a resource that has no other statuses. + * + *

Domains that have no other statuses but also have no nameservers get {@link #INACTIVE} + * instead. The spec also allows a resource to have {@link #LINKED} along with OK, but we + * implement LINKED as a virtual status that gets appended to outputs (such as info commands) on + * the fly, so we can ignore LINKED when dealing with persisted resources. + */ OK, - PENDING_CREATE, - PENDING_DELETE, - PENDING_TRANSFER, - PENDING_UPDATE, + + /** + * A status for a resource undergoing asynchronous creation. + * + *

We only use this for unallocated applications. + */ + PENDING_CREATE(ContactResource.class, DomainResource.class, HostResource.class), + + /** + * A status for a resource undergoing asynchronous deletion or for a recently deleted domain. + * + *

Contacts and hosts are deleted asynchronously because we need to check their incoming + * references with strong consistency, requiring a mapreduce. + * + *

Domains that are deleted after the add grace period ends go into a redemption grace period, + * and when that ends they go into pending delete for 5 days. + * + *

Applications are deleted synchronously and can never have this status. + */ + PENDING_DELETE(DomainApplication.class), + + /** + * A status for a resource with an unresolved transfer request. + * + *

Applications can't be transferred. Hosts transfer indirectly via superordinate domain. + */ + // TODO(b/34844887): Remove PENDING_TRANSFER from all host resources and forbid it here. + PENDING_TRANSFER(DomainApplication.class), + + /** + * A status for a resource undergoing an asynchronous update. + * + *

This status is here for completeness, but it is not used by our system. + */ + PENDING_UPDATE( + ContactResource.class, DomainApplication.class, DomainResource.class, HostResource.class), + SERVER_DELETE_PROHIBITED, SERVER_HOLD, SERVER_RENEW_PROHIBITED, @@ -66,6 +119,12 @@ public enum StatusValue implements EppEnum { SERVER_UPDATE_PROHIBITED; private final String xmlName = UPPER_UNDERSCORE.to(LOWER_CAMEL, name()); + private final ImmutableSet> forbiddenOn; + + @SafeVarargs + private StatusValue(Class... forbiddenOn) { + this.forbiddenOn = ImmutableSet.copyOf(forbiddenOn); + } @Override public String getXmlName() { @@ -81,6 +140,10 @@ public enum StatusValue implements EppEnum { return xmlName.startsWith("server") && xmlName.endsWith("Prohibited"); } + public boolean isForbiddenOn(Class resource) { + return forbiddenOn.contains(resource); + } + public static StatusValue fromXmlName(String xmlName) { return StatusValue.valueOf(LOWER_CAMEL.to(UPPER_UNDERSCORE, nullToEmpty(xmlName))); } diff --git a/javatests/google/registry/rde/DomainResourceToXjcConverterTest.java b/javatests/google/registry/rde/DomainResourceToXjcConverterTest.java index 61fac4833..6b4f3d59a 100644 --- a/javatests/google/registry/rde/DomainResourceToXjcConverterTest.java +++ b/javatests/google/registry/rde/DomainResourceToXjcConverterTest.java @@ -408,9 +408,7 @@ public class DomainResourceToXjcConverterTest { .setLastEppUpdateClientId("CeilingCat") .setLastEppUpdateTime(DateTime.parse("1920-01-01T00:00:00Z")) .setRepoId(repoId) - .setStatusValues(ImmutableSet.of( - StatusValue.OK, - StatusValue.PENDING_UPDATE)) + .setStatusValues(ImmutableSet.of(StatusValue.OK)) .build()); } } diff --git a/javatests/google/registry/rde/HostResourceToXjcConverterTest.java b/javatests/google/registry/rde/HostResourceToXjcConverterTest.java index 755100b57..85da38faa 100644 --- a/javatests/google/registry/rde/HostResourceToXjcConverterTest.java +++ b/javatests/google/registry/rde/HostResourceToXjcConverterTest.java @@ -71,7 +71,7 @@ public class HostResourceToXjcConverterTest { .setLastEppUpdateClientId("CeilingCat") .setLastEppUpdateTime(DateTime.parse("1920-01-01T00:00:00Z")) .setRepoId("2-roid") - .setStatusValues(ImmutableSet.of(StatusValue.PENDING_UPDATE)) + .setStatusValues(ImmutableSet.of(StatusValue.OK)) .build()); assertThat(bean.getAddrs()).hasSize(1); @@ -94,8 +94,8 @@ public class HostResourceToXjcConverterTest { assertThat(bean.getRoid()).isEqualTo("2-roid"); assertThat(bean.getStatuses()).hasSize(1); - assertThat(bean.getStatuses().get(0).getS()).isEqualTo(XjcHostStatusValueType.PENDING_UPDATE); - assertThat(bean.getStatuses().get(0).getS().toString()).isEqualTo("PENDING_UPDATE"); + assertThat(bean.getStatuses().get(0).getS()).isEqualTo(XjcHostStatusValueType.OK); + assertThat(bean.getStatuses().get(0).getS().toString()).isEqualTo("OK"); assertThat(bean.getStatuses().get(0).getValue()).isNull(); assertThat(bean.getStatuses().get(0).getLang()).isEqualTo("en"); @@ -120,7 +120,7 @@ public class HostResourceToXjcConverterTest { .setLastEppUpdateClientId("CeilingCat") .setLastEppUpdateTime(DateTime.parse("1920-01-01T00:00:00Z")) .setRepoId("2-LOL") - .setStatusValues(ImmutableSet.of(StatusValue.PENDING_UPDATE)) + .setStatusValues(ImmutableSet.of(StatusValue.OK)) .build()); assertThat(bean.getAddrs()).hasSize(1); assertThat(bean.getAddrs().get(0).getIp().value()).isEqualTo("v6"); diff --git a/javatests/google/registry/rde/RdeFixtures.java b/javatests/google/registry/rde/RdeFixtures.java index 147d509a0..46d6f0259 100644 --- a/javatests/google/registry/rde/RdeFixtures.java +++ b/javatests/google/registry/rde/RdeFixtures.java @@ -233,9 +233,7 @@ final class RdeFixtures { .setLastTransferTime(DateTime.parse("1910-01-01T00:00:00Z")) .setLastEppUpdateClientId("CeilingCat") .setLastEppUpdateTime(clock.nowUtc()) - .setStatusValues(ImmutableSet.of( - StatusValue.OK, - StatusValue.PENDING_UPDATE)) + .setStatusValues(ImmutableSet.of(StatusValue.OK)) .build()); } diff --git a/javatests/google/registry/rde/testdata/testMapReduce_withDomain_producesExpectedXml.xml b/javatests/google/registry/rde/testdata/testMapReduce_withDomain_producesExpectedXml.xml index 89328d080..de89bde83 100644 --- a/javatests/google/registry/rde/testdata/testMapReduce_withDomain_producesExpectedXml.xml +++ b/javatests/google/registry/rde/testdata/testMapReduce_withDomain_producesExpectedXml.xml @@ -104,7 +104,7 @@ bird.or.devil.xn--q9jyb4c 8-ROID - + 1.2.3.4 BusinessCat LawyerCat @@ -117,7 +117,7 @@ ns2.cat.xn--q9jyb4c 9-ROID - + bad:f00d:cafe::15:beef BusinessCat LawyerCat diff --git a/javatests/google/registry/tools/GenerateEscrowDepositCommandTest.java b/javatests/google/registry/tools/GenerateEscrowDepositCommandTest.java index b91e15664..a83236aa2 100644 --- a/javatests/google/registry/tools/GenerateEscrowDepositCommandTest.java +++ b/javatests/google/registry/tools/GenerateEscrowDepositCommandTest.java @@ -204,9 +204,7 @@ public class GenerateEscrowDepositCommandTest .setLastTransferTime(DateTime.parse("1910-01-01T00:00:00Z")) .setLastEppUpdateClientId("CeilingCat") .setLastEppUpdateTime(clock.nowUtc()) - .setStatusValues(ImmutableSet.of( - StatusValue.OK, - StatusValue.PENDING_UPDATE)) + .setStatusValues(ImmutableSet.of(StatusValue.OK)) .build(); } diff --git a/javatests/google/registry/tools/testdata/xn--q9jyb4c_2010-10-17_full_S1_R0.xml b/javatests/google/registry/tools/testdata/xn--q9jyb4c_2010-10-17_full_S1_R0.xml index 77872275c..85254bede 100644 --- a/javatests/google/registry/tools/testdata/xn--q9jyb4c_2010-10-17_full_S1_R0.xml +++ b/javatests/google/registry/tools/testdata/xn--q9jyb4c_2010-10-17_full_S1_R0.xml @@ -40,7 +40,7 @@ ns1.cat.lol 5-ROID - + feed::a:bee BusinessCat LawyerCat