Don't throw errors when existing premium list is empty (#1479)

* Don't throw errors when existing premium list is empty

This state is possible to get into when things go wrong and it shouldn't prevent
saving new revisions of the list. Note that it will continue to throw errors if
you attempt to save a new revision that is blank (which is usually a mistake).

See http://b/211774375
This commit is contained in:
Ben McIlwain 2021-12-30 17:22:43 -05:00 committed by GitHub
parent eefb4c71aa
commit 2b38ad8a25
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 38 additions and 7 deletions

View file

@ -14,6 +14,7 @@
package google.registry.model.tld.label;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static google.registry.config.RegistryConfig.getDomainLabelListCacheDuration;
import static google.registry.config.RegistryConfig.getSingletonCachePersistDuration;
@ -154,6 +155,7 @@ public class PremiumListDao {
}
public static PremiumList save(String name, CurrencyUnit currencyUnit, List<String> inputData) {
checkArgument(!inputData.isEmpty(), "New premium list data cannot be empty");
return save(PremiumListUtils.parseToPremiumList(name, currencyUnit, inputData));
}

View file

@ -14,7 +14,6 @@
package google.registry.model.tld.label;
import static com.google.common.base.Preconditions.checkArgument;
import static org.joda.time.DateTimeZone.UTC;
import com.google.common.collect.ImmutableMap;
@ -38,7 +37,6 @@ public class PremiumListUtils {
.setCreationTimestamp(DateTime.now(UTC))
.build();
ImmutableMap<String, PremiumEntry> prices = partialPremiumList.parse(inputData);
checkArgument(inputData.size() > 0, "Input cannot be empty");
Map<String, BigDecimal> priceAmounts = Maps.transformValues(prices, PremiumEntry::getValue);
return partialPremiumList.asBuilder().setLabelsToPrices(priceAmounts).build();
}

View file

@ -16,6 +16,7 @@ package google.registry.tools;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static google.registry.model.tld.label.PremiumListUtils.parseToPremiumList;
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import static google.registry.util.ListNamingUtils.convertFilePathToName;
import static java.nio.charset.StandardCharsets.UTF_8;
@ -27,7 +28,6 @@ import com.google.common.collect.Streams;
import google.registry.model.tld.label.PremiumList;
import google.registry.model.tld.label.PremiumList.PremiumEntry;
import google.registry.model.tld.label.PremiumListDao;
import google.registry.model.tld.label.PremiumListUtils;
import java.nio.file.Files;
import java.util.List;
import java.util.Optional;
@ -45,11 +45,11 @@ class UpdatePremiumListCommand extends CreateOrUpdatePremiumListCommand {
String.format("Could not update premium list %s because it doesn't exist.", name));
List<String> existingEntry = getExistingPremiumEntry(list.get()).asList();
inputData = Files.readAllLines(inputFile, UTF_8);
checkArgument(!inputData.isEmpty(), "New premium list data cannot be empty");
currency = list.get().getCurrency();
// reconstructing existing premium list to bypass Hibernate lazy initialization exception
PremiumList existingPremiumList =
PremiumListUtils.parseToPremiumList(name, currency, existingEntry);
PremiumList updatedPremiumList = PremiumListUtils.parseToPremiumList(name, currency, inputData);
PremiumList existingPremiumList = parseToPremiumList(name, currency, existingEntry);
PremiumList updatedPremiumList = parseToPremiumList(name, currency, inputData);
return String.format(
"Update premium list for %s?\n Old List: %s\n New List: %s",

View file

@ -22,6 +22,7 @@ import static google.registry.testing.DatabaseHelper.persistResource;
import static org.joda.money.CurrencyUnit.JPY;
import static org.joda.money.CurrencyUnit.USD;
import static org.joda.time.Duration.standardDays;
import static org.junit.jupiter.api.Assertions.assertThrows;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@ -239,6 +240,15 @@ public class PremiumListDaoTest {
.hasValue(moneyOf(JPY, 15000));
}
@Test
void testSave_throwsOnEmptyInputData() {
IllegalArgumentException thrown =
assertThrows(
IllegalArgumentException.class,
() -> PremiumListDao.save("test-list", CurrencyUnit.GBP, ImmutableList.of()));
assertThat(thrown).hasMessageThat().isEqualTo("New premium list data cannot be empty");
}
@Test
void test_savePremiumList_clearsCache() {
assertThat(PremiumListDao.premiumListCache.getIfPresent("testname")).isNull();

View file

@ -84,6 +84,27 @@ class UpdatePremiumListCommandTest<C extends UpdatePremiumListCommand>
assertThat(entries.contains("eth,USD 9999.00")).isTrue();
}
@Test
void commandRun_successUpdateList_whenExistingListIsEmpty() throws Exception {
File existingPremiumFile = tmpDir.resolve(TLD_TEST + ".txt").toFile();
Files.asCharSink(existingPremiumFile, UTF_8).write("");
File newPremiumFile = tmpDir.resolve(String.format("%s.txt", TLD_TEST)).toFile();
String newPremiumListData = "eth,USD 9999";
Files.asCharSink(newPremiumFile, UTF_8).write(newPremiumListData);
UpdatePremiumListCommand command = new UpdatePremiumListCommand();
// data come from @beforeEach of CreateOrUpdatePremiumListCommandTestCase.java
command.inputFile = Paths.get(newPremiumFile.getPath());
runCommandForced("--name=" + TLD_TEST, "--input=" + command.inputFile);
ImmutableSet<String> entries =
command.getExistingPremiumEntry(PremiumListDao.getLatestRevision(TLD_TEST).get());
assertThat(entries.size()).isEqualTo(1);
// verify that list is updated; cannot use only string since price is formatted;
assertThat(entries.contains("eth,USD 9999.00")).isTrue();
}
@Test
void commandRun_successUpdateMultiLineList() throws Exception {
File tmpFile = tmpDir.resolve(TLD_TEST + ".txt").toFile();
@ -112,7 +133,7 @@ class UpdatePremiumListCommandTest<C extends UpdatePremiumListCommand>
command.inputFile = tmpPath;
command.name = TLD_TEST;
IllegalArgumentException thrown = assertThrows(IllegalArgumentException.class, command::prompt);
assertThat(thrown).hasMessageThat().contains("Input cannot be empty");
assertThat(thrown).hasMessageThat().isEqualTo("New premium list data cannot be empty");
}
@Test