Make return value of ClaimsListShard.getClaimKey() Optional

It was nullable all along, but wasn't tagged as such, and thus it was
possible to misuse the method from its call sites.

Also adds an assertion about no NORDN tasks being enqueued in a failing
domain create test for a required signed mark.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=187649865
This commit is contained in:
mcilwain 2018-03-02 11:55:47 -08:00 committed by jianglai
parent fa989e754b
commit ceed5bdd1c
8 changed files with 22 additions and 14 deletions

View file

@ -50,6 +50,7 @@ import google.registry.model.tmch.ClaimsListShard;
import google.registry.util.Clock;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import javax.inject.Inject;
import org.joda.time.DateTime;
@ -109,10 +110,10 @@ public final class DomainClaimsCheckFlow implements Flow {
verifyClaimsPeriodNotEnded(registry, now);
}
}
String claimKey = ClaimsListShard.get().getClaimKey(domainName.parts().get(0));
Optional<String> claimKey = ClaimsListShard.get().getClaimKey(domainName.parts().get(0));
launchChecksBuilder.add(
LaunchCheck.create(
LaunchCheckName.create(claimKey != null, targetId), claimKey));
LaunchCheckName.create(claimKey.isPresent(), targetId), claimKey.orElse(null)));
}
return responseBuilder
.setOnlyExtension(LaunchCheckResponseExtension.create(CLAIMS, launchChecksBuilder.build()))

View file

@ -934,7 +934,8 @@ public class DomainFlowUtils {
static void verifyClaimsNoticeIfAndOnlyIfNeeded(
InternetDomainName domainName, boolean hasSignedMarks, boolean hasClaimsNotice)
throws EppException {
boolean isInClaimsList = ClaimsListShard.get().getClaimKey(domainName.parts().get(0)) != null;
boolean isInClaimsList =
ClaimsListShard.get().getClaimKey(domainName.parts().get(0)).isPresent();
if (hasClaimsNotice && !isInClaimsList) {
throw new UnexpectedClaimsNoticeException(domainName.toString());
}

View file

@ -45,6 +45,7 @@ import google.registry.util.SystemSleeper;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.Callable;
import javax.annotation.Nullable;
import org.joda.time.DateTime;
@ -145,8 +146,9 @@ public class ClaimsListShard extends ImmutableObject {
return creationTime;
}
public String getClaimKey(String label) {
return labelsToKeys.get(label);
/** Returns the claim key for a given domain if there is one, empty otherwise. */
public Optional<String> getClaimKey(String label) {
return Optional.ofNullable(labelsToKeys.get(label));
}
public ImmutableMap<String, String> getLabelsToKeys() {

View file

@ -156,7 +156,7 @@ final class GenerateApplicationsReportCommand implements RemoteApiCommand {
&& (domainApplication.getLaunchNotice() == null
|| domainApplication.getLaunchNotice().getNoticeId() == null
|| isNullOrEmpty(domainApplication.getLaunchNotice().getNoticeId().getTcnId()))
&& ClaimsListShard.get().getClaimKey(label) != null) {
&& ClaimsListShard.get().getClaimKey(label).isPresent()) {
return Optional.of(makeLine(domainApplication, "Missing claims notice"));
}

View file

@ -889,6 +889,7 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase<DomainCreateFlow,
persistContactsAndHosts();
EppException thrown = assertThrows(MissingClaimsNoticeException.class, this::runFlow);
assertAboutEppExceptions().that(thrown).marshalsToXml();
assertNoTasksEnqueued(QUEUE_CLAIMS, QUEUE_SUNRISE);
}
@Test

View file

@ -15,6 +15,7 @@
package google.registry.model.tmch;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.testing.JUnitBackports.assertThrows;
import static google.registry.util.DateTimeUtils.START_OF_TIME;
@ -90,8 +91,8 @@ public class ClaimsListShardTest {
assertThat(ClaimsListShard.get().labelsToKeys).isEqualTo(unsharded.labelsToKeys);
List<ClaimsListShard> shards1 = ofy().load().type(ClaimsListShard.class).list();
assertThat(shards1).hasSize(4);
assertThat(ClaimsListShard.get().getClaimKey("1")).isEqualTo("1");
assertThat(ClaimsListShard.get().getClaimKey("a")).isNull();
assertThat(ClaimsListShard.get().getClaimKey("1")).hasValue("1");
assertThat(ClaimsListShard.get().getClaimKey("a")).isEmpty();
assertThat(ClaimsListShard.getCurrentRevision()).isEqualTo(shards1.get(0).parent);
// Create a smaller ClaimsList that will need only 2 shards to save.

View file

@ -15,6 +15,7 @@
package google.registry.tmch;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@ -36,7 +37,7 @@ public class TmchDnlActionTest extends TmchActionTestCase {
@Test
public void testDnl() throws Exception {
assertThat(ClaimsListShard.get().getClaimKey("xn----7sbejwbn3axu3d")).isNull();
assertThat(ClaimsListShard.get().getClaimKey("xn----7sbejwbn3axu3d")).isEmpty();
when(httpResponse.getContent())
.thenReturn(TmchTestData.loadBytes("dnl-latest.csv").read())
.thenReturn(TmchTestData.loadBytes("dnl-latest.sig").read());
@ -51,7 +52,7 @@ public class TmchDnlActionTest extends TmchActionTestCase {
ClaimsListShard claimsList = ClaimsListShard.get();
assertThat(claimsList.getCreationTime()).isEqualTo(DateTime.parse("2013-11-24T23:15:37.4Z"));
assertThat(claimsList.getClaimKey("xn----7sbejwbn3axu3d"))
.isEqualTo("2013112500/7/4/8/dIHW0DiuybvhdP8kIz");
assertThat(claimsList.getClaimKey("lolcat")).isNull();
.hasValue("2013112500/7/4/8/dIHW0DiuybvhdP8kIz");
assertThat(claimsList.getClaimKey("lolcat")).isEmpty();
}
}

View file

@ -15,6 +15,7 @@
package google.registry.tools;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
import static google.registry.testing.JUnitBackports.assertThrows;
import google.registry.model.tmch.ClaimsListShard;
@ -38,11 +39,11 @@ public class UploadClaimsListCommandTest extends CommandTestCase<UploadClaimsLis
ClaimsListShard claimsList = ClaimsListShard.get();
assertThat(claimsList.getCreationTime()).isEqualTo(DateTime.parse("2012-08-16T00:00:00.0Z"));
assertThat(claimsList.getClaimKey("example"))
.isEqualTo("2013041500/2/6/9/rJ1NrDO92vDsAzf7EQzgjX4R0000000001");
.hasValue("2013041500/2/6/9/rJ1NrDO92vDsAzf7EQzgjX4R0000000001");
assertThat(claimsList.getClaimKey("another-example"))
.isEqualTo("2013041500/6/A/5/alJAqG2vI2BmCv5PfUvuDkf40000000002");
.hasValue("2013041500/6/A/5/alJAqG2vI2BmCv5PfUvuDkf40000000002");
assertThat(claimsList.getClaimKey("anotherexample"))
.isEqualTo("2013041500/A/C/7/rHdC4wnrWRvPY6nneCVtQhFj0000000003");
.hasValue("2013041500/A/C/7/rHdC4wnrWRvPY6nneCVtQhFj0000000003");
}
@Test