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

Remove UNRESERVED as a reservation type

Browse source 
This is a follow-up to Lai's refactoring of the get reservation types
code to return a set rather than a single type. Since we're always
returning a set now, the more natural way to represent a label that is
not reserved is to return an empty set rather than a set containing
UNRESERVED.

Also fixes some minor style issues I ran across regarding static
importing and test method naming that I ran across (no logic
implications).

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=151132116
This commit is contained in:
mcilwain 2017-03-24 09:09:06 -07:00 • committed by Ben McIlwain
parent 4260fb573f
commit b03bd3b525
12 changed files with 83 additions and 90 deletions
Download patch file Download diff file Expand all files Collapse all files

3
docs/operational-procedures/reserved-list-management.md
View file

@ -9,9 +9,6 @@ Reserved lists are handled in a similar way to [premium
lists](./premium-list-management.md), except that instead of each label having
a price, it has a reservation type. The valid values for reservation types are:
* **`UNRESERVED`** - The default value for any label that isn't reserved.
Labels that aren't explictly under any other status implictly have this
value.
* **`NAMESERVER_RESTRICTED`** - Only nameservers included here can be set on a
domain with this label. If the a label in this type exists on multiple
reserved lists that are applied to the same TLD. The set of allowed

6
java/google/registry/export/ExportUtils.java
View file

@ -14,8 +14,6 @@
package google.registry.export;
import static google.registry.model.registry.label.ReservationType.UNRESERVED;
import com.google.common.base.Joiner;
import com.googlecode.objectify.Key;
import google.registry.config.RegistryConfig.Config;
@ -45,9 +43,7 @@ public final class ExportUtils {
ReservedList reservedList = ReservedList.load(key).get();
if (reservedList.getShouldPublish()) {
for (ReservedListEntry entry : reservedList.getReservedListEntries().values()) {
if (entry.getValue() != UNRESERVED) {
reservedTerms.add(entry.getLabel());
}
reservedTerms.add(entry.getLabel());
}
}
}

20
java/google/registry/flows/domain/DomainCheckFlow.java
View file

@ -24,10 +24,10 @@ import static google.registry.flows.domain.DomainFlowUtils.validateDomainNameWit
import static google.registry.flows.domain.DomainFlowUtils.verifyNotInPredelegation;
import static google.registry.model.EppResourceUtils.checkResourcesExist;
import static google.registry.model.index.DomainApplicationIndex.loadActiveApplicationsByDomainName;
import static google.registry.model.registry.label.ReservationType.UNRESERVED;
import static google.registry.model.registry.label.ReservationType.getTypeOfHighestSeverity;
import static google.registry.pricing.PricingEngineProxy.isDomainPremium;
import com.google.common.base.Optional;
import com.google.common.base.Predicate;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
@ -150,8 +150,8 @@ public final class DomainCheckFlow implements Flow {
Set<String> existingIds = checkResourcesExist(DomainResource.class, targetIds, now);
ImmutableList.Builder<DomainCheck> checks = new ImmutableList.Builder<>();
for (String targetId : targetIds) {
String message = getMessageForCheck(domainNames.get(targetId), existingIds, now);
checks.add(DomainCheck.create(message == null, targetId, message));
Optional<String> message = getMessageForCheck(domainNames.get(targetId), existingIds, now);
checks.add(DomainCheck.create(!message.isPresent(), targetId, message.orNull()));
}
BeforeResponseReturnData responseData =
customLogic.beforeResponse(
@ -166,10 +166,10 @@ public final class DomainCheckFlow implements Flow {
.build();
}
private String getMessageForCheck(
private Optional<String> getMessageForCheck(
InternetDomainName domainName, Set<String> existingIds, DateTime now) {
if (existingIds.contains(domainName.toString())) {
return "In use";
return Optional.of("In use");
}
Registry registry = Registry.get(domainName.parent().toString());
if (PENDING_ALLOCATION_TLD_STATES.contains(registry.getTldState(now))
@ -179,17 +179,19 @@ public final class DomainCheckFlow implements Flow {
public boolean apply(DomainApplication input) {
return !input.getApplicationStatus().isFinalStatus();
}})) {
return "Pending allocation";
return Optional.of("Pending allocation");
}
ImmutableSet<ReservationType> reservationTypes = getReservationTypes(domainName);
if (reservationTypes.equals(ImmutableSet.of(UNRESERVED))
if (reservationTypes.isEmpty()
&& isDomainPremium(domainName.toString(), now)
&& registry.getPremiumPriceAckRequired()
&& eppInput.getSingleExtension(FeeCheckCommandExtension.class) == null) {
return "Premium names require EPP ext.";
return Optional.of("Premium names require EPP ext.");
}
return getTypeOfHighestSeverity(reservationTypes).getMessageForCheck();
return reservationTypes.isEmpty()
? Optional.<String>absent()
: Optional.of(getTypeOfHighestSeverity(reservationTypes).getMessageForCheck());
}
/** Handle the fee check extension. */

6
java/google/registry/flows/domain/DomainFlowUtils.java
View file

@ -26,7 +26,9 @@ import static google.registry.model.EppResourceUtils.loadByForeignKey;
import static google.registry.model.domain.DomainResource.MAX_REGISTRATION_YEARS;
import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.model.registry.Registries.findTldForName;
import static google.registry.model.registry.label.ReservationType.FULLY_BLOCKED;
import static google.registry.model.registry.label.ReservationType.NAMESERVER_RESTRICTED;
import static google.registry.model.registry.label.ReservationType.RESERVED_FOR_ANCHOR_TENANT;
import static google.registry.model.registry.label.ReservedList.getAllowedNameservers;
import static google.registry.pricing.PricingEngineProxy.isDomainPremium;
import static google.registry.tldconfig.idn.IdnLabelValidator.findValidIdnTableForTld;
@ -391,8 +393,8 @@ public class DomainFlowUtils {
private static boolean isReserved(InternetDomainName domainName, boolean isSunrise) {
ImmutableSet<ReservationType> types = getReservationTypes(domainName);
return types.contains(ReservationType.FULLY_BLOCKED)
|| types.contains(ReservationType.RESERVED_FOR_ANCHOR_TENANT)
return types.contains(FULLY_BLOCKED)
|| types.contains(RESERVED_FOR_ANCHOR_TENANT)
|| !(isSunrise || intersection(TYPES_ALLOWED_FOR_CREATE_ONLY_IN_SUNRISE, types).isEmpty());
}

3
java/google/registry/model/eppoutput/CheckData.java
View file

@ -17,6 +17,7 @@ package google.registry.model.eppoutput;
import com.google.common.collect.ImmutableList;
import google.registry.model.ImmutableObject;
import google.registry.model.eppoutput.EppResponse.ResponseData;
import javax.annotation.Nullable;
import javax.xml.bind.annotation.XmlAttribute;
import javax.xml.bind.annotation.XmlElement;
import javax.xml.bind.annotation.XmlElements;
@ -124,7 +125,7 @@ public abstract class CheckData extends ImmutableObject implements ResponseData
/** A version with domain namespacing. */
@XmlType(namespace = "urn:ietf:params:xml:ns:domain-1.0")
public static class DomainCheck extends Check {
public static DomainCheck create(boolean avail, String name, String reason) {
public static DomainCheck create(boolean avail, String name, @Nullable String reason) {
return init(new DomainCheck(), CheckName.create(avail, name), reason);
}

3
java/google/registry/model/registry/label/DomainLabelMetrics.java
View file

@ -166,8 +166,7 @@ class DomainLabelMetrics {
String mostSevereReservedList =
matches.isEmpty() ? "(none)" : mostSevereMatch.reservedListName();
String mostSevereReservationType =
(matches.isEmpty() ? ReservationType.UNRESERVED : mostSevereMatch.reservationType())
.toString();
(matches.isEmpty() ? "(none)" : mostSevereMatch.reservationType()).toString();
reservedListChecks.increment(
tld, matchCount, mostSevereReservedList, mostSevereReservationType);
reservedListProcessingTime.record(

35
java/google/registry/model/registry/label/ReservationType.java
View file

@ -14,8 +14,11 @@
package google.registry.model.registry.label;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
import com.google.common.collect.Ordering;
import java.util.Set;
import javax.annotation.Nullable;
@ -27,13 +30,12 @@ public enum ReservationType {
// label has multiple reservation types, its message is the that of the one with the highest
// severity.
UNRESERVED(null, 0),
NAMESERVER_RESTRICTED("Nameserver restricted", 1),
ALLOWED_IN_SUNRISE("Reserved for non-sunrise", 2),
MISTAKEN_PREMIUM("Reserved", 3),
RESERVED_FOR_ANCHOR_TENANT("Reserved", 4),
NAME_COLLISION("Cannot be delegated", 5),
FULLY_BLOCKED("Reserved", 6);
NAMESERVER_RESTRICTED("Nameserver restricted", 0),
ALLOWED_IN_SUNRISE("Reserved for non-sunrise", 1),
MISTAKEN_PREMIUM("Reserved", 2),
RESERVED_FOR_ANCHOR_TENANT("Reserved", 3),
NAME_COLLISION("Cannot be delegated", 4),
FULLY_BLOCKED("Reserved", 5);
@Nullable
private final String messageForCheck;
@ -48,20 +50,23 @@ public enum ReservationType {
return messageForCheck;
}
private static final Ordering<ReservationType> ORDERING = new Ordering<ReservationType>() {
@Override
public int compare(ReservationType left, ReservationType right) {
return Integer.compare(left.ordinal(), right.ordinal());
}
};
/**
* Returns the {@code ReservationType} with the highest severity, used when a label has multiple
* reservation types and a reservation message is needed.
*
* @param types the set of reservation types that a label is associated with.
* @return the reservation type with the highest severity.
*/
public static ReservationType getTypeOfHighestSeverity(Set<ReservationType> types) {
ReservationType mostSevereType = UNRESERVED;
for (ReservationType type : types) {
if (type.compareTo(mostSevereType) > 0) {
mostSevereType = type;
}
}
return mostSevereType;
checkArgumentNotNull(types, "types must not be null");
checkArgument(!types.isEmpty(), "types must not be empty");
return ORDERING.max(types);
}
}

31
java/google/registry/model/registry/label/ReservedList.java
View file

@ -25,7 +25,6 @@ import static google.registry.model.ofy.Ofy.RECOMMENDED_MEMCACHE_EXPIRATION;
import static google.registry.model.registry.label.ReservationType.FULLY_BLOCKED;
import static google.registry.model.registry.label.ReservationType.NAMESERVER_RESTRICTED;
import static google.registry.model.registry.label.ReservationType.RESERVED_FOR_ANCHOR_TENANT;
import static google.registry.model.registry.label.ReservationType.UNRESERVED;
import static google.registry.util.CollectionUtils.nullToEmpty;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static org.joda.time.DateTimeZone.UTC;
@ -37,9 +36,9 @@ import com.google.common.base.Splitter;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.net.InternetDomainName;
import com.google.common.util.concurrent.UncheckedExecutionException;
import com.googlecode.objectify.Key;
@ -211,27 +210,25 @@ public final class ReservedList
}
/**
* Queries the set of all reserved lists associated with the specified tld and returns the
* reservation types of the label. If the label is in none of the lists, it returns a set that
* contains UNRESERVED.
* Queries the set of all reserved lists associated with the specified TLD and returns the
* reservation types of the label.
*
* <p>If the label is in none of the lists, it returns an empty set.
*/
public static ImmutableSet<ReservationType> getReservationTypes(String label, String tld) {
checkNotNull(label, "label");
if (label.length() == 0) {
return ImmutableSet.of(FULLY_BLOCKED);
}
ImmutableSet<ReservedListEntry> entries = getReservedListEntries(label, tld);
return entries.isEmpty()
? ImmutableSet.of(UNRESERVED)
: ImmutableSet.copyOf(
Iterables.transform(
entries,
new Function<ReservedListEntry, ReservationType>() {
@Override
public ReservationType apply(ReservedListEntry reservedListEntry) {
return reservedListEntry.reservationType;
}
}));
return FluentIterable.from(getReservedListEntries(label, tld))
.transform(
new Function<ReservedListEntry, ReservationType>() {
@Override
public ReservationType apply(ReservedListEntry reservedListEntry) {
return reservedListEntry.reservationType;
}
})
.toSet();
}
/**

3
javatests/google/registry/export/ExportReservedTermsActionTest.java
View file

@ -74,8 +74,7 @@ public class ExportReservedTermsActionTest {
ReservedList rl = persistReservedList(
"tld-reserved",
"lol,FULLY_BLOCKED",
"cat,FULLY_BLOCKED",
"jimmy,UNRESERVED");
"cat,FULLY_BLOCKED");
createTld("tld");
persistResource(Registry.get("tld").asBuilder()
.setReservedLists(rl)

7
javatests/google/registry/export/ExportUtilsTest.java
View file

@ -41,18 +41,15 @@ public class ExportUtilsTest {
ReservedList rl1 = persistReservedList(
"tld-reserved1",
"lol,FULLY_BLOCKED",
"cat,FULLY_BLOCKED",
"jimmy,UNRESERVED");
"cat,FULLY_BLOCKED");
ReservedList rl2 = persistReservedList(
"tld-reserved2",
"lol,NAME_COLLISION",
"cat,UNRESERVED",
"snow,FULLY_BLOCKED");
ReservedList rl3 = persistReservedList(
"tld-reserved3",
false,
"tine,FULLY_BLOCKED",
"oval,UNRESERVED");
"tine,FULLY_BLOCKED");
createTld("tld");
persistResource(Registry.get("tld").asBuilder().setReservedLists(rl1, rl2, rl3).build());
// Should not contain jimmy, tine, or oval.

55
javatests/google/registry/model/registry/label/ReservedListTest.java
View file

@ -25,7 +25,6 @@ import static google.registry.model.registry.label.ReservationType.MISTAKEN_PREM
import static google.registry.model.registry.label.ReservationType.NAMESERVER_RESTRICTED;
import static google.registry.model.registry.label.ReservationType.NAME_COLLISION;
import static google.registry.model.registry.label.ReservationType.RESERVED_FOR_ANCHOR_TENANT;
import static google.registry.model.registry.label.ReservationType.UNRESERVED;
import static google.registry.model.registry.label.ReservedList.getAllowedNameservers;
import static google.registry.model.registry.label.ReservedList.getReservationTypes;
import static google.registry.model.registry.label.ReservedList.matchesAnchorTenantReservation;
@ -82,44 +81,45 @@ public class ReservedListTest {
private static void verifyUnreservedCheckCount(int unreservedCount) {
assertThat(reservedListChecks)
.hasValueForLabels(unreservedCount, "tld", "0", "(none)", UNRESERVED.toString())
.hasValueForLabels(unreservedCount, "tld", "0", "(none)", "(none)")
.and()
.hasNoOtherValues();
assertThat(reservedListProcessingTime)
.hasAnyValueForLabels("tld", "0", "(none)", UNRESERVED.toString())
.hasAnyValueForLabels("tld", "0", "(none)", "(none)")
.and()
.hasNoOtherValues();
assertThat(reservedListHits).hasNoOtherValues();
}
@Test
public void testGetReservation_allLabelsAreUnreserved_withNoReservedLists() throws Exception {
public void testGetReservationTypes_allLabelsAreUnreserved_withNoReservedLists()
throws Exception {
assertThat(Registry.get("tld").getReservedLists()).isEmpty();
assertThat(getReservationTypes("doodle", "tld")).containsExactly(UNRESERVED);
assertThat(getReservationTypes("access", "tld")).containsExactly(UNRESERVED);
assertThat(getReservationTypes("rich", "tld")).containsExactly(UNRESERVED);
assertThat(getReservationTypes("doodle", "tld")).isEmpty();
assertThat(getReservationTypes("access", "tld")).isEmpty();
assertThat(getReservationTypes("rich", "tld")).isEmpty();
verifyUnreservedCheckCount(3);
}
@Test
public void testZeroReservedLists_doesNotCauseError() throws Exception {
assertThat(getReservationTypes("doodle", "tld")).containsExactly(UNRESERVED);
assertThat(getReservationTypes("doodle", "tld")).isEmpty();
verifyUnreservedCheckCount(1);
}
@Test
public void testGetReservation_twoLetterCodesAreAvailable() {
public void testGetReservationTypes_twoLetterCodesAreAvailable() {
for (String sld : ImmutableList.of("aa", "az", "zz", "91", "1n", "j5")) {
assertThat(getReservationTypes(sld, "tld")).containsExactly(UNRESERVED);
assertThat(getReservationTypes(sld, "tld")).isEmpty();
}
verifyUnreservedCheckCount(6);
}
@Test
public void testGetReservation_singleCharacterDomainsAreAllowed() {
public void testGetReservationTypes_singleCharacterDomainsAreAllowed() {
// This isn't quite exhaustive but it's close.
for (char c = 'a'; c <= 'z'; c++) {
assertThat(getReservationTypes("" + c, "tld")).containsExactly(UNRESERVED);
assertThat(getReservationTypes("" + c, "tld")).isEmpty();
}
verifyUnreservedCheckCount(26);
}
@ -148,13 +148,13 @@ public class ReservedListTest {
assertThat(matchesAnchorTenantReservation(InternetDomainName.from("random.tld"), "abcdefg"))
.isFalse();
assertThat(reservedListChecks)
.hasValueForLabels(1, "tld", "0", "(none)", UNRESERVED.toString())
.hasValueForLabels(1, "tld", "0", "(none)", "(none)")
.and()
.hasValueForLabels(6, "tld", "1", "reserved1", RESERVED_FOR_ANCHOR_TENANT.toString())
.and()
.hasNoOtherValues();
assertThat(reservedListProcessingTime)
.hasAnyValueForLabels("tld", "0", "(none)", UNRESERVED.toString())
.hasAnyValueForLabels("tld", "0", "(none)", "(none)")
.and()
.hasAnyValueForLabels("tld", "1", "reserved1", RESERVED_FOR_ANCHOR_TENANT.toString())
.and()
@ -229,7 +229,7 @@ public class ReservedListTest {
.and()
.hasValueForLabels(1, "tld", "1", "reserved2", NAMESERVER_RESTRICTED.toString())
.and()
.hasValueForLabels(1, "tld", "0", "(none)", UNRESERVED.toString())
.hasValueForLabels(1, "tld", "0", "(none)", "(none)")
.and()
.hasNoOtherValues();
assertThat(reservedListProcessingTime)
@ -243,7 +243,7 @@ public class ReservedListTest {
.and()
.hasAnyValueForLabels("tld", "1", "reserved2", NAMESERVER_RESTRICTED.toString())
.and()
.hasAnyValueForLabels("tld", "0", "(none)", UNRESERVED.toString())
.hasAnyValueForLabels("tld", "0", "(none)", "(none)")
.and()
.hasNoOtherValues();
assertThat(reservedListHits)
@ -261,8 +261,7 @@ public class ReservedListTest {
}
@Test
public void testMatchesAnchorTenantReservation_duplicatingAuthCodes()
throws Exception {
public void testMatchesAnchorTenantReservation_duplicatingAuthCodes() throws Exception {
ReservedList rl1 = persistReservedList("reserved1", "lol,RESERVED_FOR_ANCHOR_TENANT,foo");
ReservedList rl2 = persistReservedList("reserved2", "lol,RESERVED_FOR_ANCHOR_TENANT,foo");
createTld("tld");
@ -276,7 +275,7 @@ public class ReservedListTest {
}
@Test
public void testGetReservation_concatsMultipleListsCorrectly() throws Exception {
public void testGetReservationTypes_concatsMultipleListsCorrectly() throws Exception {
ReservedList rl1 = persistReservedList(
"reserved1",
"lol,FULLY_BLOCKED # yup",
@ -292,9 +291,9 @@ public class ReservedListTest {
assertThat(getReservationTypes("cat", "tld")).containsExactly(FULLY_BLOCKED);
assertThat(getReservationTypes("roflcopter", "tld")).containsExactly(FULLY_BLOCKED);
assertThat(getReservationTypes("snowcrash", "tld")).containsExactly(FULLY_BLOCKED);
assertThat(getReservationTypes("doge", "tld")).containsExactly(UNRESERVED);
assertThat(getReservationTypes("doge", "tld")).isEmpty();
assertThat(reservedListChecks)
.hasValueForLabels(1, "tld", "0", "(none)", UNRESERVED.toString())
.hasValueForLabels(1, "tld", "0", "(none)", "(none)")
.and()
.hasValueForLabels(2, "tld", "1", "reserved1", FULLY_BLOCKED.toString())
.and()
@ -302,7 +301,7 @@ public class ReservedListTest {
.and()
.hasNoOtherValues();
assertThat(reservedListProcessingTime)
.hasAnyValueForLabels("tld", "0", "(none)", UNRESERVED.toString())
.hasAnyValueForLabels("tld", "0", "(none)", "(none)")
.and()
.hasAnyValueForLabels("tld", "1", "reserved1", FULLY_BLOCKED.toString())
.and()
@ -318,7 +317,7 @@ public class ReservedListTest {
}
@Test
public void testGetReservation_returnsAllReservationTypesFromMultipleListsForTheSameLabel()
public void testGetReservationTypes_returnsAllReservationTypesFromMultipleListsForTheSameLabel()
throws Exception {
ReservedList rl1 =
persistReservedList("reserved1", "lol,NAME_COLLISION # yup", "cat,FULLY_BLOCKED");
@ -335,7 +334,7 @@ public class ReservedListTest {
@Test
public void testGetReservation_worksAfterReservedListRemovedUsingSet() throws Exception {
public void testGetReservationTypes_worksAfterReservedListRemovedUsingSet() throws Exception {
ReservedList rl1 = persistReservedList(
"reserved1", "lol,FULLY_BLOCKED", "cat,FULLY_BLOCKED");
ReservedList rl2 = persistReservedList(
@ -348,17 +347,17 @@ public class ReservedListTest {
"roflcopter.tld should be unreserved"
+ " after unsetting the registry's second reserved list")
.that(getReservationTypes("roflcopter", "tld"))
.containsExactly(UNRESERVED);
.isEmpty();
assertThat(reservedListChecks)
.hasValueForLabels(1, "tld", "1", "reserved2", FULLY_BLOCKED.toString())
.and()
.hasValueForLabels(1, "tld", "0", "(none)", UNRESERVED.toString())
.hasValueForLabels(1, "tld", "0", "(none)", "(none)")
.and()
.hasNoOtherValues();
assertThat(reservedListProcessingTime)
.hasAnyValueForLabels("tld", "1", "reserved2", FULLY_BLOCKED.toString())
.and()
.hasAnyValueForLabels("tld", "0", "(none)", UNRESERVED.toString())
.hasAnyValueForLabels("tld", "0", "(none)", "(none)")
.and()
.hasNoOtherValues();
assertThat(reservedListHits)
@ -368,7 +367,7 @@ public class ReservedListTest {
}
@Test
public void testGetReservation_combinesMultipleLists() throws Exception {
public void testGetReservationTypes_combinesMultipleLists() throws Exception {
ReservedList rl1 = persistReservedList(
"reserved1", "lol,NAME_COLLISION", "roflcopter,ALLOWED_IN_SUNRISE");
ReservedList rl2 = persistReservedList("reserved2", "lol,FULLY_BLOCKED");

1
javatests/google/registry/model/schema.txt
View file

@ -757,7 +757,6 @@ enum google.registry.model.registry.label.ReservationType {
NAMESERVER_RESTRICTED;
NAME_COLLISION;
RESERVED_FOR_ANCHOR_TENANT;
UNRESERVED;
}
class google.registry.model.registry.label.ReservedList {
@Id java.lang.String name;