Fix NullPointerException in StaticPremiumPricingEngine (#1164)

* Fix NullPointerException in StaticPremiumPricingEngine

* Make getPremiumList return optional

* add isPresent checks
This commit is contained in:
sarahcaseybot 2021-05-18 10:55:27 -04:00 committed by GitHub
parent c1f0c29134
commit 21aeedae11
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 54 additions and 19 deletions

View file

@ -113,7 +113,7 @@ public class ExportPremiumTermsAction implements Runnable {
"Skipping premium terms export for TLD %s because Drive folder isn't specified", tld); "Skipping premium terms export for TLD %s because Drive folder isn't specified", tld);
return Optional.of("Skipping export because no Drive folder is associated with this TLD"); return Optional.of("Skipping export because no Drive folder is associated with this TLD");
} }
if (registry.getPremiumList() == null) { if (!registry.getPremiumList().isPresent()) {
logger.atInfo().log("No premium terms to export for TLD %s", tld); logger.atInfo().log("No premium terms to export for TLD %s", tld);
return Optional.of("No premium lists configured"); return Optional.of("No premium lists configured");
} }
@ -137,7 +137,8 @@ public class ExportPremiumTermsAction implements Runnable {
} }
private String getFormattedPremiumTerms(Registry registry) { private String getFormattedPremiumTerms(Registry registry) {
String premiumListName = registry.getPremiumList().getName(); checkState(registry.getPremiumList().isPresent(), "%s does not have a premium list", tld);
String premiumListName = registry.getPremiumList().get().getName();
checkState( checkState(
PremiumListDao.getLatestRevision(premiumListName).isPresent(), PremiumListDao.getLatestRevision(premiumListName).isPresent(),
"Could not load premium list for " + tld); "Could not load premium list for " + tld);

View file

@ -38,8 +38,11 @@ public final class StaticPremiumListPricingEngine implements PremiumPricingEngin
String tld = getTldFromDomainName(fullyQualifiedDomainName); String tld = getTldFromDomainName(fullyQualifiedDomainName);
String label = InternetDomainName.from(fullyQualifiedDomainName).parts().get(0); String label = InternetDomainName.from(fullyQualifiedDomainName).parts().get(0);
Registry registry = Registry.get(checkNotNull(tld, "tld")); Registry registry = Registry.get(checkNotNull(tld, "tld"));
Optional<Money> premiumPrice = Optional<Money> premiumPrice = Optional.empty();
PremiumListDao.getPremiumPrice(registry.getPremiumList().getName(), label); if (registry.getPremiumList().isPresent()) {
premiumPrice =
PremiumListDao.getPremiumPrice(registry.getPremiumList().get().getName(), label);
}
return DomainPrices.create( return DomainPrices.create(
premiumPrice.isPresent(), premiumPrice.isPresent(),
premiumPrice.orElse(registry.getStandardCreateCost()), premiumPrice.orElse(registry.getStandardCreateCost()),

View file

@ -607,8 +607,8 @@ public class Registry extends ImmutableObject implements Buildable, DatastoreAnd
} }
@Nullable @Nullable
public Key<PremiumList> getPremiumList() { public Optional<Key<PremiumList>> getPremiumList() {
return premiumList; return Optional.ofNullable(premiumList);
} }
public CurrencyUnit getCurrency() { public CurrencyUnit getCurrency() {

View file

@ -263,7 +263,7 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
@Override @Override
public boolean refersToKey(Registry registry, Key<? extends BaseDomainLabelList<?, ?>> key) { public boolean refersToKey(Registry registry, Key<? extends BaseDomainLabelList<?, ?>> key) {
return Objects.equals(registry.getPremiumList(), key); return Objects.equals(registry.getPremiumList().orElse(null), key);
} }
@Override @Override

View file

@ -70,7 +70,8 @@ public final class OteAccountBuilderTest {
private void assertTldExists(String tld, TldState tldState, Money eapFee) { private void assertTldExists(String tld, TldState tldState, Money eapFee) {
Registry registry = Registry.get(tld); Registry registry = Registry.get(tld);
assertThat(registry).isNotNull(); assertThat(registry).isNotNull();
assertThat(registry.getPremiumList().getName()).isEqualTo("default_sandbox_list"); assertThat(registry.getPremiumList()).isPresent();
assertThat(registry.getPremiumList().get().getName()).isEqualTo("default_sandbox_list");
assertThat(registry.getTldStateTransitions()).containsExactly(START_OF_TIME, tldState); assertThat(registry.getTldStateTransitions()).containsExactly(START_OF_TIME, tldState);
assertThat(registry.getDnsWriters()).containsExactly("VoidDnsWriter"); assertThat(registry.getDnsWriters()).containsExactly("VoidDnsWriter");
assertThat(registry.getAddGracePeriodLength()).isEqualTo(Duration.standardHours(1)); assertThat(registry.getAddGracePeriodLength()).isEqualTo(Duration.standardHours(1));

View file

@ -49,6 +49,7 @@ import google.registry.testing.DualDatabaseTest;
import google.registry.testing.TestOfyAndSql; import google.registry.testing.TestOfyAndSql;
import google.registry.testing.TestOfyOnly; import google.registry.testing.TestOfyOnly;
import java.math.BigDecimal; import java.math.BigDecimal;
import java.util.Optional;
import org.joda.money.Money; import org.joda.money.Money;
import org.joda.time.DateTime; import org.joda.time.DateTime;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
@ -249,9 +250,9 @@ public final class RegistryTest extends EntityTestCase {
void testSetPremiumList() { void testSetPremiumList() {
PremiumList pl2 = persistPremiumList("tld2", "lol,USD 50", "cat,USD 700"); PremiumList pl2 = persistPremiumList("tld2", "lol,USD 50", "cat,USD 700");
Registry registry = Registry.get("tld").asBuilder().setPremiumList(pl2).build(); Registry registry = Registry.get("tld").asBuilder().setPremiumList(pl2).build();
Key<PremiumList> plKey = registry.getPremiumList(); Optional<Key<PremiumList>> plKey = registry.getPremiumList();
assertThat(plKey).isNotNull(); assertThat(plKey).isPresent();
PremiumList stored = PremiumListDao.getLatestRevision(plKey.getName()).get(); PremiumList stored = PremiumListDao.getLatestRevision(plKey.get().getName()).get();
assertThat(stored.getName()).isEqualTo("tld2"); assertThat(stored.getName()).isEqualTo("tld2");
} }

View file

@ -15,12 +15,16 @@
package google.registry.tools; package google.registry.tools;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.createTld;
import static google.registry.testing.DatabaseHelper.persistPremiumList; import static google.registry.testing.DatabaseHelper.persistPremiumList;
import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.testing.DatabaseHelper.persistResource;
import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertThrows;
import com.beust.jcommander.ParameterException; import com.beust.jcommander.ParameterException;
import com.google.common.collect.ImmutableSet;
import google.registry.dns.writer.VoidDnsWriter;
import google.registry.model.pricing.StaticPremiumListPricingEngine;
import google.registry.model.registry.Registry; import google.registry.model.registry.Registry;
import google.registry.schema.tld.PremiumListDao; import google.registry.schema.tld.PremiumListDao;
import google.registry.testing.DeterministicStringGenerator; import google.registry.testing.DeterministicStringGenerator;
@ -82,6 +86,26 @@ class CreateDomainCommandTest extends EppToolCommandTestCase<CreateDomainCommand
@Test @Test
void testSuccess_multipleDomains() throws Exception { void testSuccess_multipleDomains() throws Exception {
createTld("abc"); createTld("abc");
runCommandForced(
"--client=NewRegistrar",
"--registrant=crr-admin",
"--admins=crr-admin",
"--techs=crr-tech",
"example.tld",
"example.abc");
eppVerifier.verifySent("domain_create_minimal.xml").verifySent("domain_create_minimal_abc.xml");
}
@Test
void testSuccess_premiumListNull() throws Exception {
Registry registry =
new Registry.Builder()
.setTldStr("abc")
.setPremiumPricingEngine(StaticPremiumListPricingEngine.NAME)
.setDnsWriters(ImmutableSet.of(VoidDnsWriter.NAME))
.setPremiumList(null)
.build();
tm().transact(() -> tm().put(registry));
runCommandForced( runCommandForced(
"--client=NewRegistrar", "--client=NewRegistrar",
"--registrant=crr-admin", "--registrant=crr-admin",

View file

@ -519,7 +519,9 @@ class CreateTldCommandTest extends CommandTestCase<CreateTldCommand> {
"--roid_suffix=Q9JYB4C", "--roid_suffix=Q9JYB4C",
"--dns_writers=FooDnsWriter", "--dns_writers=FooDnsWriter",
"xn--q9jyb4c"); "xn--q9jyb4c");
assertThat(Registry.get("xn--q9jyb4c").getPremiumList().getName()).isEqualTo("xn--q9jyb4c"); assertThat(Registry.get("xn--q9jyb4c").getPremiumList()).isPresent();
assertThat(Registry.get("xn--q9jyb4c").getPremiumList().get().getName())
.isEqualTo("xn--q9jyb4c");
} }
@Test @Test

View file

@ -73,7 +73,7 @@ class SetupOteCommandTest extends CommandTestCase<SetupOteCommand> {
assertThat(registry.getTldState(DateTime.now(UTC))).isEqualTo(tldState); assertThat(registry.getTldState(DateTime.now(UTC))).isEqualTo(tldState);
assertThat(registry.getDnsWriters()).containsExactly("VoidDnsWriter"); assertThat(registry.getDnsWriters()).containsExactly("VoidDnsWriter");
assertThat(registry.getPremiumList()).isNotNull(); assertThat(registry.getPremiumList()).isNotNull();
assertThat(registry.getPremiumList().getName()).isEqualTo("default_sandbox_list"); assertThat(registry.getPremiumList().get().getName()).isEqualTo("default_sandbox_list");
assertThat(registry.getAddGracePeriodLength()).isEqualTo(Duration.standardMinutes(60)); assertThat(registry.getAddGracePeriodLength()).isEqualTo(Duration.standardMinutes(60));
assertThat(registry.getRedemptionGracePeriodLength()).isEqualTo(Duration.standardMinutes(10)); assertThat(registry.getRedemptionGracePeriodLength()).isEqualTo(Duration.standardMinutes(10));
assertThat(registry.getPendingDeleteLength()).isEqualTo(Duration.standardMinutes(5)); assertThat(registry.getPendingDeleteLength()).isEqualTo(Duration.standardMinutes(5));

View file

@ -38,6 +38,7 @@ import com.google.common.collect.ImmutableSortedMap;
import com.googlecode.objectify.Key; import com.googlecode.objectify.Key;
import google.registry.model.registry.Registry; import google.registry.model.registry.Registry;
import google.registry.model.registry.label.PremiumList; import google.registry.model.registry.label.PremiumList;
import java.util.Optional;
import org.joda.money.Money; import org.joda.money.Money;
import org.joda.time.DateTime; import org.joda.time.DateTime;
import org.joda.time.Duration; import org.joda.time.Duration;
@ -838,21 +839,21 @@ class UpdateTldCommandTest extends CommandTestCase<UpdateTldCommand> {
@Test @Test
void testSuccess_removePremiumListWithNull() throws Exception { void testSuccess_removePremiumListWithNull() throws Exception {
runCommandForced("--premium_list=null", "xn--q9jyb4c"); runCommandForced("--premium_list=null", "xn--q9jyb4c");
assertThat(Registry.get("xn--q9jyb4c").getPremiumList()).isNull(); assertThat(Registry.get("xn--q9jyb4c").getPremiumList()).isEmpty();
} }
@Test @Test
void testSuccess_removePremiumListWithBlank() throws Exception { void testSuccess_removePremiumListWithBlank() throws Exception {
runCommandForced("--premium_list=", "xn--q9jyb4c"); runCommandForced("--premium_list=", "xn--q9jyb4c");
assertThat(Registry.get("xn--q9jyb4c").getPremiumList()).isNull(); assertThat(Registry.get("xn--q9jyb4c").getPremiumList()).isEmpty();
} }
@Test @Test
void testSuccess_premiumListNotRemovedWhenNotSpecified() throws Exception { void testSuccess_premiumListNotRemovedWhenNotSpecified() throws Exception {
runCommandForced("--add_reserved_lists=xn--q9jyb4c_r1,xn--q9jyb4c_r2", "xn--q9jyb4c"); runCommandForced("--add_reserved_lists=xn--q9jyb4c_r1,xn--q9jyb4c_r2", "xn--q9jyb4c");
Key<PremiumList> premiumListKey = Registry.get("xn--q9jyb4c").getPremiumList(); Optional<Key<PremiumList>> premiumListKey = Registry.get("xn--q9jyb4c").getPremiumList();
assertThat(premiumListKey).isNotNull(); assertThat(premiumListKey).isPresent();
assertThat(premiumListKey.getName()).isEqualTo("xn--q9jyb4c"); assertThat(premiumListKey.get().getName()).isEqualTo("xn--q9jyb4c");
} }
@Test @Test
@ -920,7 +921,9 @@ class UpdateTldCommandTest extends CommandTestCase<UpdateTldCommand> {
@Test @Test
void testSuccess_setPremiumList() throws Exception { void testSuccess_setPremiumList() throws Exception {
runCommandForced("--premium_list=xn--q9jyb4c", "xn--q9jyb4c"); runCommandForced("--premium_list=xn--q9jyb4c", "xn--q9jyb4c");
assertThat(Registry.get("xn--q9jyb4c").getPremiumList().getName()).isEqualTo("xn--q9jyb4c"); assertThat(Registry.get("xn--q9jyb4c").getPremiumList()).isPresent();
assertThat(Registry.get("xn--q9jyb4c").getPremiumList().get().getName())
.isEqualTo("xn--q9jyb4c");
} }
@Test @Test