mirrors/nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus synced 2026-01-10 16:00:52 +00:00
Code Issues Releases Wiki Activity

Remove use of shouldPublishField from ReservedList (#2324)

Browse Source 
* Remove use of shouldPublishField from ReservedList

* Remove from tests

* Update test comment

* Fix indentation

* fix test comment

* Fix test

* fix test

* Make shouldPublish column nullable
This commit is contained in:
sarahcaseybot
2024-02-27 15:39:58 -05:00
committed by GitHub
parent 15368ee1c6
commit a4bd85068b
21 changed files with 54 additions and 146 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
core/src/main/java/google/registry/export/ExportUtils.java
View File

@@ -46,10 +46,8 @@ public final class ExportUtils {
() ->
new IllegalStateException(
String.format("Reserved list %s does not exist", reservedListName)));
if (reservedList.getShouldPublish()) {
for (ReservedListEntry entry : reservedList.getReservedListEntries().values()) {
reservedTerms.add(entry.getDomainLabel());
}
for (ReservedListEntry entry : reservedList.getReservedListEntries().values()) {
reservedTerms.add(entry.getDomainLabel());
}
}
Joiner.on("\n").appendTo(termsBuilder, reservedTerms);

16
core/src/main/java/google/registry/model/tld/label/ReservedList.java
View File

@@ -71,7 +71,8 @@ public final class ReservedList
*/
@Insignificant @Transient Map<String, ReservedListEntry> reservedListMap;
@Column(nullable = false)
// TODO(b/321053918): Remove this field once the column is nullable
@Column(nullable = true)
boolean shouldPublish = true;
@PreRemove
@@ -180,14 +181,6 @@ public final class ReservedList
return !getReferencingTlds().isEmpty();
}
/**
* Returns whether this reserved list is included in the concatenated list of reserved terms
* published to Google Drive for viewing by registrars.
*/
public boolean getShouldPublish() {
return shouldPublish;
}
/**
* Returns a {@link Map} of domain labels to {@link ReservedListEntry}.
*
@@ -330,11 +323,6 @@ public final class ReservedList
return this;
}
public Builder setShouldPublish(boolean shouldPublish) {
getInstance().shouldPublish = shouldPublish;
return this;
}
/**
* Updates the reservedListMap from input lines.
*

8
core/src/main/java/google/registry/tools/CreateOrUpdateReservedListCommand.java
View File

@@ -44,14 +44,6 @@ public abstract class CreateOrUpdateReservedListCommand extends ConfirmingComman
required = true)
Path input;
@Nullable
@Parameter(
names = "--should_publish",
description =
"Whether the list is published to the concatenated list on Drive (defaults to true).",
arity = 1)
Boolean shouldPublish;
ReservedList reservedList;
@Override

2
core/src/main/java/google/registry/tools/CreateReservedListCommand.java
View File

@@ -53,12 +53,10 @@ final class CreateReservedListCommand extends CreateOrUpdateReservedListCommand
}
DateTime now = DateTime.now(UTC);
List<String> allLines = Files.readAllLines(input, UTF_8);
boolean shouldPublish = this.shouldPublish == null || this.shouldPublish;
reservedList =
new ReservedList.Builder()
.setName(name)
.setReservedListMapFromLines(allLines)
.setShouldPublish(shouldPublish)
.setCreationTimestamp(now)
.build();

27
core/src/main/java/google/registry/tools/UpdateReservedListCommand.java
View File

@@ -60,38 +60,21 @@ final class UpdateReservedListCommand extends CreateOrUpdateReservedListCommand
new IllegalArgumentException(
String.format(
"Could not update reserved list %s because it doesn't exist.", name)));
boolean shouldPublish =
this.shouldPublish == null ? existingReservedList.getShouldPublish() : this.shouldPublish;
List<String> allLines = Files.readAllLines(input, UTF_8);
ReservedList.Builder updated =
existingReservedList
.asBuilder()
.setReservedListMapFromLines(allLines)
.setShouldPublish(shouldPublish);
existingReservedList.asBuilder().setReservedListMapFromLines(allLines);
reservedList = updated.build();
boolean shouldPublishChanged =
existingReservedList.getShouldPublish() != reservedList.getShouldPublish();
boolean reservedListEntriesChanged =
!existingReservedList
.getReservedListEntries()
.equals(reservedList.getReservedListEntries());
if (!shouldPublishChanged && !reservedListEntriesChanged) {
if (!reservedListEntriesChanged) {
newChange = false;
return "No entity changes to apply.";
}
String result = String.format("Update reserved list for %s?\n", name);
if (shouldPublishChanged) {
result +=
String.format(
"shouldPublish: %s -> %s\n",
existingReservedList.getShouldPublish(), reservedList.getShouldPublish());
}
if (reservedListEntriesChanged) {
result +=
prettyPrintEntityDeepDiff(
existingReservedList.getReservedListEntries(), reservedList.getReservedListEntries());
}
return result;
return String.format("Update reserved list for %s?\n", name)
+ prettyPrintEntityDeepDiff(
existingReservedList.getReservedListEntries(), reservedList.getReservedListEntries());
}
@Override

3
core/src/test/java/google/registry/bsa/ReservedDomainsTestingUtils.java
View File

@@ -43,7 +43,6 @@ public final class ReservedDomainsTestingUtils {
new ReservedList.Builder()
.setName(listName)
.setCreationTimestamp(START_OF_TIME)
.setShouldPublish(true)
.setReservedListMap(entries)
.build());
}
@@ -76,7 +75,6 @@ public final class ReservedDomainsTestingUtils {
new ReservedList.Builder()
.setName(listName)
.setCreationTimestamp(START_OF_TIME)
.setShouldPublish(true)
.setReservedListMap(
new ImmutableMap.Builder<String, ReservedListEntry>()
.putAll(existingEntries)
@@ -97,7 +95,6 @@ public final class ReservedDomainsTestingUtils {
new ReservedList.Builder()
.setName(listName)
.setCreationTimestamp(START_OF_TIME)
.setShouldPublish(true)
.setReservedListMap(newEntries)
.build());
}

1
core/src/test/java/google/registry/bsa/UploadBsaUnavailableDomainsActionTest.java
View File

@@ -73,7 +73,6 @@ public class UploadBsaUnavailableDomainsActionTest {
ReservedList reservedList =
persistReservedList(
"tld-reserved_list",
true,
"tine,FULLY_BLOCKED",
"flagrant,NAME_COLLISION",
"jimmy,RESERVED_FOR_SPECIFIC_USE");

7
core/src/test/java/google/registry/export/ExportUtilsTest.java
View File

@@ -43,13 +43,8 @@ class ExportUtilsTest {
"tld-reserved2",
"lol,NAME_COLLISION",
"snow,FULLY_BLOCKED");
ReservedList rl3 = persistReservedList(
"tld-reserved3",
false,
"tine,FULLY_BLOCKED");
createTld("tld");
persistResource(Tld.get("tld").asBuilder().setReservedLists(rl1, rl2, rl3).build());
// Should not contain jimmy, tine, or oval.
persistResource(Tld.get("tld").asBuilder().setReservedLists(rl1, rl2).build());
assertThat(new ExportUtils("# This is a disclaimer.").exportReservedTerms(Tld.get("tld")))
.isEqualTo("# This is a disclaimer.\ncat\nlol\nsnow\n");
}

6
core/src/test/java/google/registry/model/tld/TldTest.java
View File

@@ -300,7 +300,6 @@ public final class TldTest extends EntityTestCase {
.setName("tld-reserved15")
.setReservedListMapFromLines(
ImmutableList.of("potato,FULLY_BLOCKED", "phone,FULLY_BLOCKED"))
.setShouldPublish(true)
.setCreationTimestamp(fakeClock.nowUtc())
.build());
ReservedList rl16 =
@@ -309,7 +308,6 @@ public final class TldTest extends EntityTestCase {
.setName("tld-reserved16")
.setReservedListMapFromLines(
ImmutableList.of("port,FULLY_BLOCKED", "manteau,FULLY_BLOCKED"))
.setShouldPublish(true)
.setCreationTimestamp(fakeClock.nowUtc())
.build());
Tld registry1 =
@@ -347,7 +345,6 @@ public final class TldTest extends EntityTestCase {
.setName("tld-reserved5")
.setReservedListMapFromLines(
ImmutableList.of("potato,FULLY_BLOCKED", "phone,FULLY_BLOCKED"))
.setShouldPublish(true)
.setCreationTimestamp(fakeClock.nowUtc())
.build());
ReservedList rl6 =
@@ -356,7 +353,6 @@ public final class TldTest extends EntityTestCase {
.setName("tld-reserved6")
.setReservedListMapFromLines(
ImmutableList.of("port,FULLY_BLOCKED", "manteau,FULLY_BLOCKED"))
.setShouldPublish(true)
.setCreationTimestamp(fakeClock.nowUtc())
.build());
Tld r = Tld.get("tld").asBuilder().setReservedLists(ImmutableSet.of(rl5, rl6)).build();
@@ -372,7 +368,6 @@ public final class TldTest extends EntityTestCase {
.setName("tld-reserved15")
.setReservedListMapFromLines(
ImmutableList.of("potato,FULLY_BLOCKED", "phone,FULLY_BLOCKED"))
.setShouldPublish(true)
.setCreationTimestamp(fakeClock.nowUtc())
.build());
persistReservedList(
@@ -380,7 +375,6 @@ public final class TldTest extends EntityTestCase {
.setName("tld-reserved16")
.setReservedListMapFromLines(
ImmutableList.of("port,FULLY_BLOCKED", "manteau,FULLY_BLOCKED"))
.setShouldPublish(true)
.setCreationTimestamp(fakeClock.nowUtc())
.build());
Tld r =

4
core/src/test/java/google/registry/model/tld/label/ReservedListDaoTest.java
View File

@@ -52,7 +52,6 @@ public class ReservedListDaoTest {
new ReservedList.Builder()
.setName("testlist")
.setCreationTimestamp(fakeClock.nowUtc())
.setShouldPublish(false)
.setReservedListMap(testReservations)
.build();
}
@@ -102,7 +101,6 @@ public class ReservedListDaoTest {
assertThat(persistedList.getRevisionId()).isNotNull();
assertThat(persistedList.getCreationTimestamp()).isEqualTo(fakeClock.nowUtc());
assertThat(persistedList.getName()).isEqualTo("testlist");
assertThat(persistedList.getShouldPublish()).isFalse();
assertThat(persistedList.getReservedListEntries()).containsExactlyEntriesIn(testReservations);
}
@@ -112,7 +110,6 @@ public class ReservedListDaoTest {
new ReservedList.Builder()
.setName("testlist")
.setCreationTimestamp(fakeClock.nowUtc())
.setShouldPublish(false)
.setReservedListMap(
ImmutableMap.of(
"old",
@@ -124,7 +121,6 @@ public class ReservedListDaoTest {
assertThat(persistedList.getRevisionId()).isNotNull();
assertThat(persistedList.getCreationTimestamp()).isEqualTo(fakeClock.nowUtc());
assertThat(persistedList.getName()).isEqualTo("testlist");
assertThat(persistedList.getShouldPublish()).isFalse();
assertThat(persistedList.getReservedListEntries()).containsExactlyEntriesIn(testReservations);
}
}

8
core/src/test/java/google/registry/testing/DatabaseHelper.java
View File

@@ -345,23 +345,17 @@ public final class DatabaseHelper {
domain.asBuilder().setAutorenewBillingEvent(billingRecurrence.createVKey()).build());
}
public static ReservedList persistReservedList(String listName, String... lines) {
return persistReservedList(listName, true, lines);
}
public static ReservedList persistReservedList(ReservedList reservedList) {
ReservedListDao.save(reservedList);
maybeAdvanceClock();
return reservedList;
}
public static ReservedList persistReservedList(
String listName, boolean shouldPublish, String... lines) {
public static ReservedList persistReservedList(String listName, String... lines) {
ReservedList reservedList =
new ReservedList.Builder()
.setName(listName)
.setReservedListMapFromLines(ImmutableList.copyOf(lines))
.setShouldPublish(shouldPublish)
.setCreationTimestamp(DateTime.now(DateTimeZone.UTC))
.build();
return persistReservedList(reservedList);

2
core/src/test/java/google/registry/tools/CreateOrUpdateReservedListCommandTestCase.java
View File

@@ -97,12 +97,10 @@ abstract class CreateOrUpdateReservedListCommandTestCase<
ReservedList createCloudSqlReservedList(
String name,
DateTime creationTime,
boolean shouldPublish,
ImmutableMap<String, ReservedListEntry> labelsToEntries) {
return new ReservedList.Builder()
.setName(name)
.setCreationTimestamp(creationTime)
.setShouldPublish(shouldPublish)
.setReservedListMap(labelsToEntries)
.build();
}

21
core/src/test/java/google/registry/tools/CreateReservedListCommandTest.java
View File

@@ -56,27 +56,6 @@ class CreateReservedListCommandTest
assertThat(ReservedList.get("xn--q9jyb4c_common-reserved")).isPresent();
}
@Test
void testSuccess_shouldPublishDefaultsToTrue() throws Exception {
runCommandForced("--input=" + reservedTermsPath);
assertThat(ReservedList.get("xn--q9jyb4c_common-reserved")).isPresent();
assertThat(ReservedList.get("xn--q9jyb4c_common-reserved").get().getShouldPublish()).isTrue();
}
@Test
void testSuccess_shouldPublishSetToTrue_works() throws Exception {
runCommandForced("--input=" + reservedTermsPath, "--should_publish=true");
assertThat(ReservedList.get("xn--q9jyb4c_common-reserved")).isPresent();
assertThat(ReservedList.get("xn--q9jyb4c_common-reserved").get().getShouldPublish()).isTrue();
}
@Test
void testSuccess_shouldPublishSetToFalse_works() throws Exception {
runCommandForced("--input=" + reservedTermsPath, "--should_publish=false");
assertThat(ReservedList.get("xn--q9jyb4c_common-reserved")).isPresent();
assertThat(ReservedList.get("xn--q9jyb4c_common-reserved").get().getShouldPublish()).isFalse();
}
@Test
void testFailure_reservedListWithThatNameAlreadyExists() {
ReservedList rl = persistReservedList("xn--q9jyb4c_foo", "jones,FULLY_BLOCKED");

26
core/src/test/java/google/registry/tools/UpdateReservedListCommandTest.java
View File

@@ -37,16 +37,15 @@ class UpdateReservedListCommandTest
@BeforeEach
void beforeEach() {
populateInitialReservedListInDatabase(true);
populateInitialReservedListInDatabase();
}
private void populateInitialReservedListInDatabase(boolean shouldPublish) {
private void populateInitialReservedListInDatabase() {
persistReservedList(
new ReservedList.Builder()
.setName("xn--q9jyb4c_common-reserved")
.setReservedListMapFromLines(ImmutableList.of("helicopter,FULLY_BLOCKED"))
.setCreationTimestamp(START_OF_TIME)
.setShouldPublish(shouldPublish)
.build());
}
@@ -60,25 +59,6 @@ class UpdateReservedListCommandTest
runSuccessfulUpdateTest("--input=" + reservedTermsPath);
}
@Test
void testSuccess_shouldPublish_setToFalseCorrectly() throws Exception {
runSuccessfulUpdateTest("--input=" + reservedTermsPath, "--should_publish=false");
assertThat(ReservedList.get("xn--q9jyb4c_common-reserved")).isPresent();
ReservedList reservedList = ReservedList.get("xn--q9jyb4c_common-reserved").get();
assertThat(reservedList.getShouldPublish()).isFalse();
assertInStdout("Update reserved list for xn--q9jyb4c_common-reserved?");
assertInStdout("shouldPublish: true -> false");
}
@Test
void testSuccess_shouldPublish_doesntOverrideFalseIfNotSpecified() throws Exception {
populateInitialReservedListInDatabase(false);
runCommandForced("--input=" + reservedTermsPath);
assertThat(ReservedList.get("xn--q9jyb4c_common-reserved")).isPresent();
ReservedList reservedList = ReservedList.get("xn--q9jyb4c_common-reserved").get();
assertThat(reservedList.getShouldPublish()).isFalse();
}
private void runSuccessfulUpdateTest(String... args) throws Exception {
runCommandForced(args);
assertThat(ReservedList.get("xn--q9jyb4c_common-reserved")).isPresent();
@@ -133,11 +113,9 @@ class UpdateReservedListCommandTest
// CreateOrUpdateReservedListCommandTestCases.java
UpdateReservedListCommand command = new UpdateReservedListCommand();
command.input = Paths.get(reservedTermsPath);
command.shouldPublish = false;
command.init();
assertThat(command.prompt()).contains("Update reserved list for xn--q9jyb4c_common-reserved?");
assertThat(command.prompt()).contains("shouldPublish: true -> false");
assertThat(command.prompt()).contains("helicopter: helicopter,FULLY_BLOCKED -> null");
assertThat(command.prompt()).contains("baddies: null -> baddies,FULLY_BLOCKED");
assertThat(command.prompt()).contains("ford: null -> ford,FULLY_BLOCKED # random comment");

16
core/src/test/java/google/registry/tools/server/ListReservedListsActionTest.java
View File

@@ -31,8 +31,8 @@ class ListReservedListsActionTest extends ListActionTestCase {
@BeforeEach
void beforeEach() {
ReservedList rl1 = persistReservedList("xn--q9jyb4c-published", true, "blah,FULLY_BLOCKED");
ReservedList rl2 = persistReservedList("xn--q9jyb4c-private", false, "dugong,FULLY_BLOCKED");
ReservedList rl1 = persistReservedList("xn--q9jyb4c-published", "blah,FULLY_BLOCKED");
ReservedList rl2 = persistReservedList("xn--q9jyb4c-private", "dugong,FULLY_BLOCKED");
createTld("xn--q9jyb4c");
persistResource(Tld.get("xn--q9jyb4c").asBuilder().setReservedLists(rl1, rl2).build());
action = new ListReservedListsAction();
@@ -53,13 +53,13 @@ class ListReservedListsActionTest extends ListActionTestCase {
void testRun_withParameters() {
testRunSuccess(
action,
Optional.of("shouldPublish"),
Optional.of("revisionId"),
Optional.empty(),
Optional.empty(),
"^name\\s+shouldPublish\\s*$",
"^name\\s+revisionId\\s*$",
"^-+\\s+-+\\s*$",
"^xn--q9jyb4c-private\\s+false\\s*$",
"^xn--q9jyb4c-published\\s+true\\s*$");
"^xn--q9jyb4c-private\\s+2\\s*$",
"^xn--q9jyb4c-published\\s+1\\s*$");
}
@Test
@@ -71,8 +71,8 @@ class ListReservedListsActionTest extends ListActionTestCase {
Optional.empty(),
"^name\\s+.*shouldPublish.*",
"^-+\\s+-+",
"^xn--q9jyb4c-private\\s+.*false",
"^xn--q9jyb4c-published\\s+.*true");
"^xn--q9jyb4c-private\\s+.*",
"^xn--q9jyb4c-published\\s+.*");
}
@Test