From ceed5bdd1c80a60c40d155c37763f03030e6622f Mon Sep 17 00:00:00 2001 From: mcilwain Date: Fri, 2 Mar 2018 11:55:47 -0800 Subject: [PATCH] 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 --- .../registry/flows/domain/DomainClaimsCheckFlow.java | 5 +++-- java/google/registry/flows/domain/DomainFlowUtils.java | 3 ++- java/google/registry/model/tmch/ClaimsListShard.java | 6 ++++-- .../registry/tools/GenerateApplicationsReportCommand.java | 2 +- .../google/registry/flows/domain/DomainCreateFlowTest.java | 1 + .../google/registry/model/tmch/ClaimsListShardTest.java | 5 +++-- javatests/google/registry/tmch/TmchDnlActionTest.java | 7 ++++--- .../google/registry/tools/UploadClaimsListCommandTest.java | 7 ++++--- 8 files changed, 22 insertions(+), 14 deletions(-) diff --git a/java/google/registry/flows/domain/DomainClaimsCheckFlow.java b/java/google/registry/flows/domain/DomainClaimsCheckFlow.java index 959290033..3c5848f13 100644 --- a/java/google/registry/flows/domain/DomainClaimsCheckFlow.java +++ b/java/google/registry/flows/domain/DomainClaimsCheckFlow.java @@ -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 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())) diff --git a/java/google/registry/flows/domain/DomainFlowUtils.java b/java/google/registry/flows/domain/DomainFlowUtils.java index ace82776f..7dfd5ff4e 100644 --- a/java/google/registry/flows/domain/DomainFlowUtils.java +++ b/java/google/registry/flows/domain/DomainFlowUtils.java @@ -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()); } diff --git a/java/google/registry/model/tmch/ClaimsListShard.java b/java/google/registry/model/tmch/ClaimsListShard.java index 39f4ea2b5..eeefc76c7 100644 --- a/java/google/registry/model/tmch/ClaimsListShard.java +++ b/java/google/registry/model/tmch/ClaimsListShard.java @@ -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 getClaimKey(String label) { + return Optional.ofNullable(labelsToKeys.get(label)); } public ImmutableMap getLabelsToKeys() { diff --git a/java/google/registry/tools/GenerateApplicationsReportCommand.java b/java/google/registry/tools/GenerateApplicationsReportCommand.java index 02cd5b736..b4a882862 100644 --- a/java/google/registry/tools/GenerateApplicationsReportCommand.java +++ b/java/google/registry/tools/GenerateApplicationsReportCommand.java @@ -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")); } diff --git a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java index 1a0f4f882..702776503 100644 --- a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java @@ -889,6 +889,7 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase 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. diff --git a/javatests/google/registry/tmch/TmchDnlActionTest.java b/javatests/google/registry/tmch/TmchDnlActionTest.java index 0ea4927dc..fdd448ef0 100644 --- a/javatests/google/registry/tmch/TmchDnlActionTest.java +++ b/javatests/google/registry/tmch/TmchDnlActionTest.java @@ -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(); } } diff --git a/javatests/google/registry/tools/UploadClaimsListCommandTest.java b/javatests/google/registry/tools/UploadClaimsListCommandTest.java index 8e48371c6..8b6e80e03 100644 --- a/javatests/google/registry/tools/UploadClaimsListCommandTest.java +++ b/javatests/google/registry/tools/UploadClaimsListCommandTest.java @@ -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