From d99278e7238acd472ded1a73d5b213c1dd27b70e Mon Sep 17 00:00:00 2001 From: gbrodman Date: Tue, 27 Apr 2021 20:37:09 -0400 Subject: [PATCH] Convert remaining read-only flow tests to dual-DB (#1107) Note that there are many flow tests that aren't @DualDatabaseTest-annotated yet but those will come later, as they will require more changes to the flows (other PRs are coming or in progress). This only includes the remaining EppResource flows that don't create a history entry. --- .../token/AllocationTokenFlowUtils.java | 3 +- .../flows/domain/DomainCheckFlowTest.java | 240 +++++++++--------- .../domain/DomainClaimsCheckFlowTest.java | 30 ++- .../token/AllocationTokenFlowUtilsTest.java | 30 ++- .../registry/flows/host/HostInfoFlowTest.java | 32 ++- 5 files changed, 174 insertions(+), 161 deletions(-) diff --git a/core/src/main/java/google/registry/flows/domain/token/AllocationTokenFlowUtils.java b/core/src/main/java/google/registry/flows/domain/token/AllocationTokenFlowUtils.java index cb7c75448..1282ce1bb 100644 --- a/core/src/main/java/google/registry/flows/domain/token/AllocationTokenFlowUtils.java +++ b/core/src/main/java/google/registry/flows/domain/token/AllocationTokenFlowUtils.java @@ -16,6 +16,7 @@ package google.registry.flows.domain.token; import static com.google.common.base.Preconditions.checkArgument; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; +import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; @@ -153,7 +154,7 @@ public class AllocationTokenFlowUtils { throw new InvalidAllocationTokenException(); } Optional maybeTokenEntity = - tm().loadByKeyIfPresent(VKey.create(AllocationToken.class, token)); + transactIfJpaTm(() -> tm().loadByKeyIfPresent(VKey.create(AllocationToken.class, token))); if (!maybeTokenEntity.isPresent()) { throw new InvalidAllocationTokenException(); } diff --git a/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java index a481aa21e..9f95b7dad 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java @@ -71,18 +71,20 @@ import google.registry.model.registry.Registry; import google.registry.model.registry.Registry.TldState; import google.registry.model.registry.label.ReservedList; import google.registry.model.reporting.HistoryEntry; +import google.registry.testing.DualDatabaseTest; import google.registry.testing.ReplayExtension; import google.registry.testing.SetClockExtension; +import google.registry.testing.TestOfyAndSql; import org.joda.money.CurrencyUnit; import org.joda.money.Money; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Order; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; /** Unit tests for {@link DomainCheckFlow}. */ +@DualDatabaseTest class DomainCheckFlowTest extends ResourceCheckFlowTestCase { @Order(value = Order.DEFAULT - 3) @@ -120,7 +122,7 @@ class DomainCheckFlowTest extends ResourceCheckFlowTestCase { @@ -66,25 +68,25 @@ public class DomainClaimsCheckFlowTest runFlowAssertResponse(loadFile(expectedXmlFilename)); } - @Test + @TestOfyAndSql void testSuccess_noClaims() throws Exception { doSuccessfulTest("domain_check_claims_response_none.xml"); } - @Test + @TestOfyAndSql void testSuccess_quietPeriod() throws Exception { createTld("tld", TldState.QUIET_PERIOD); doSuccessfulTest("domain_check_claims_response_none.xml"); } - @Test + @TestOfyAndSql void testSuccess_oneClaim() throws Exception { persistClaimsList( ImmutableMap.of("example2", "2013041500/2/6/9/rJ1NrDO92vDsAzf7EQzgjX4R0000000001")); doSuccessfulTest("domain_check_claims_response.xml"); } - @Test + @TestOfyAndSql void testSuccess_multipleTlds() throws Exception { setEppInput("domain_check_claims_multiple_tlds.xml"); createTld("tld1"); @@ -94,28 +96,28 @@ public class DomainClaimsCheckFlowTest doSuccessfulTest("domain_check_claims_response_multiple_tlds.xml"); } - @Test + @TestOfyAndSql void testSuccess_50IdsAllowed() throws Exception { // Make sure we don't have a regression that reduces the number of allowed checks. setEppInput("domain_check_claims_50.xml"); runFlow(); } - @Test + @TestOfyAndSql void testFailure_TooManyIds() { setEppInput("domain_check_claims_51.xml"); EppException thrown = assertThrows(TooManyResourceChecksException.class, this::runFlow); assertAboutEppExceptions().that(thrown).marshalsToXml(); } - @Test + @TestOfyAndSql void testFailure_tldDoesntExist() { setEppInput("domain_check_claims_bad_tld.xml"); EppException thrown = assertThrows(TldDoesNotExistException.class, this::runFlow); assertAboutEppExceptions().that(thrown).marshalsToXml(); } - @Test + @TestOfyAndSql void testFailure_notAuthorizedForTld() { persistResource( loadRegistrar("TheRegistrar").asBuilder().setAllowedTlds(ImmutableSet.of()).build()); @@ -123,7 +125,7 @@ public class DomainClaimsCheckFlowTest assertAboutEppExceptions().that(thrown).marshalsToXml(); } - @Test + @TestOfyAndSql void testSuccess_superuserNotAuthorizedForTld() throws Exception { persistClaimsList( ImmutableMap.of("example2", "2013041500/2/6/9/rJ1NrDO92vDsAzf7EQzgjX4R0000000001")); @@ -136,7 +138,7 @@ public class DomainClaimsCheckFlowTest CommitMode.LIVE, UserPrivileges.SUPERUSER, loadFile("domain_check_claims_response.xml")); } - @Test + @TestOfyAndSql void testFailure_predelgation() { createTld("tld", PREDELEGATION); setEppInput("domain_check_claims.xml"); @@ -144,7 +146,7 @@ public class DomainClaimsCheckFlowTest assertAboutEppExceptions().that(thrown).marshalsToXml(); } - @Test + @TestOfyAndSql void testFailure_allocationToken() { createTld("tld"); setEppInput("domain_check_claims_allocationtoken.xml"); @@ -153,7 +155,7 @@ public class DomainClaimsCheckFlowTest assertAboutEppExceptions().that(thrown).marshalsToXml(); } - @Test + @TestOfyAndSql void testFailure_multipleTlds_oneHasEndedClaims() { createTlds("tld1", "tld2"); persistResource( @@ -163,7 +165,7 @@ public class DomainClaimsCheckFlowTest assertAboutEppExceptions().that(thrown).marshalsToXml(); } - @Test + @TestOfyAndSql void testIcannActivityReportField_getsLogged() throws Exception { runFlow(); assertIcannReportingActivityFieldLogged("srs-dom-check"); diff --git a/core/src/test/java/google/registry/flows/domain/token/AllocationTokenFlowUtilsTest.java b/core/src/test/java/google/registry/flows/domain/token/AllocationTokenFlowUtilsTest.java index 3c8a2f3cd..bca6177b3 100644 --- a/core/src/test/java/google/registry/flows/domain/token/AllocationTokenFlowUtilsTest.java +++ b/core/src/test/java/google/registry/flows/domain/token/AllocationTokenFlowUtilsTest.java @@ -50,12 +50,14 @@ import google.registry.model.domain.token.AllocationToken.TokenStatus; import google.registry.model.registry.Registry; import google.registry.model.reporting.HistoryEntry; import google.registry.testing.AppEngineExtension; +import google.registry.testing.DualDatabaseTest; +import google.registry.testing.TestOfyAndSql; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; /** Unit tests for {@link AllocationTokenFlowUtils}. */ +@DualDatabaseTest class AllocationTokenFlowUtilsTest { private final AllocationTokenFlowUtils flowUtils = @@ -70,7 +72,7 @@ class AllocationTokenFlowUtilsTest { createTld("tld"); } - @Test + @TestOfyAndSql void test_validateToken_successfullyVerifiesValidToken() throws Exception { AllocationToken token = persistResource( @@ -85,12 +87,12 @@ class AllocationTokenFlowUtilsTest { .isEqualTo(token); } - @Test + @TestOfyAndSql void test_validateToken_failsOnNonexistentToken() { assertValidateThrowsEppException(InvalidAllocationTokenException.class); } - @Test + @TestOfyAndSql void test_validateToken_failsOnNullToken() { assertAboutEppExceptions() .that( @@ -106,7 +108,7 @@ class AllocationTokenFlowUtilsTest { .marshalsToXml(); } - @Test + @TestOfyAndSql void test_validateToken_callsCustomLogic() { AllocationTokenFlowUtils failingFlowUtils = new AllocationTokenFlowUtils(new FailingAllocationTokenCustomLogic()); @@ -125,7 +127,7 @@ class AllocationTokenFlowUtilsTest { assertThat(thrown).hasMessageThat().isEqualTo("failed for tests"); } - @Test + @TestOfyAndSql void test_validateToken_invalidForClientId() { persistResource( createOneMonthPromoTokenBuilder(DateTime.now(UTC).minusDays(1)) @@ -134,7 +136,7 @@ class AllocationTokenFlowUtilsTest { assertValidateThrowsEppException(AllocationTokenNotValidForRegistrarException.class); } - @Test + @TestOfyAndSql void test_validateToken_invalidForTld() { persistResource( createOneMonthPromoTokenBuilder(DateTime.now(UTC).minusDays(1)) @@ -143,19 +145,19 @@ class AllocationTokenFlowUtilsTest { assertValidateThrowsEppException(AllocationTokenNotValidForTldException.class); } - @Test + @TestOfyAndSql void test_validateToken_beforePromoStart() { persistResource(createOneMonthPromoTokenBuilder(DateTime.now(UTC).plusDays(1)).build()); assertValidateThrowsEppException(AllocationTokenNotInPromotionException.class); } - @Test + @TestOfyAndSql void test_validateToken_afterPromoEnd() { persistResource(createOneMonthPromoTokenBuilder(DateTime.now(UTC).minusMonths(2)).build()); assertValidateThrowsEppException(AllocationTokenNotInPromotionException.class); } - @Test + @TestOfyAndSql void test_validateToken_promoCancelled() { // the promo would be valid but it was cancelled 12 hours ago persistResource( @@ -170,7 +172,7 @@ class AllocationTokenFlowUtilsTest { assertValidateThrowsEppException(AllocationTokenNotInPromotionException.class); } - @Test + @TestOfyAndSql void test_checkDomainsWithToken_successfullyVerifiesValidToken() { persistResource( new AllocationToken.Builder().setToken("tokeN").setTokenType(SINGLE_USE).build()); @@ -189,7 +191,7 @@ class AllocationTokenFlowUtilsTest { .inOrder(); } - @Test + @TestOfyAndSql void test_checkDomainsWithToken_showsFailureMessageForRedeemedToken() { DomainBase domain = persistActiveDomain("example.tld"); Key historyEntryKey = Key.create(Key.create(domain), HistoryEntry.class, 1051L); @@ -217,7 +219,7 @@ class AllocationTokenFlowUtilsTest { .inOrder(); } - @Test + @TestOfyAndSql void test_checkDomainsWithToken_callsCustomLogic() { persistResource( new AllocationToken.Builder().setToken("tokeN").setTokenType(SINGLE_USE).build()); @@ -236,7 +238,7 @@ class AllocationTokenFlowUtilsTest { assertThat(thrown).hasMessageThat().isEqualTo("failed for tests"); } - @Test + @TestOfyAndSql void test_checkDomainsWithToken_resultsFromCustomLogicAreIntegrated() { persistResource( new AllocationToken.Builder().setToken("tokeN").setTokenType(SINGLE_USE).build()); diff --git a/core/src/test/java/google/registry/flows/host/HostInfoFlowTest.java b/core/src/test/java/google/registry/flows/host/HostInfoFlowTest.java index 739aec939..7a312b647 100644 --- a/core/src/test/java/google/registry/flows/host/HostInfoFlowTest.java +++ b/core/src/test/java/google/registry/flows/host/HostInfoFlowTest.java @@ -17,6 +17,7 @@ package google.registry.flows.host; import static com.google.common.truth.Truth.assertThat; import static google.registry.testing.DatabaseHelper.assertNoBillingEvents; import static google.registry.testing.DatabaseHelper.createTld; +import static google.registry.testing.DatabaseHelper.deleteResource; import static google.registry.testing.DatabaseHelper.newDomainBase; import static google.registry.testing.DatabaseHelper.persistNewRegistrar; import static google.registry.testing.DatabaseHelper.persistResource; @@ -35,15 +36,17 @@ import google.registry.flows.host.HostFlowUtils.HostNameNotPunyCodedException; import google.registry.model.domain.DomainBase; import google.registry.model.eppcommon.StatusValue; import google.registry.model.host.HostResource; +import google.registry.testing.DualDatabaseTest; import google.registry.testing.ReplayExtension; +import google.registry.testing.TestOfyAndSql; import javax.annotation.Nullable; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Order; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; /** Unit tests for {@link HostInfoFlow}. */ +@DualDatabaseTest class HostInfoFlowTest extends ResourceFlowTestCase { @Order(value = Order.DEFAULT - 2) @@ -80,7 +83,7 @@ class HostInfoFlowTest extends ResourceFlowTestCase .build()); } - @Test + @TestOfyAndSql void testSuccess() throws Exception { persistHostResource(); assertTransactionalFlow(false); @@ -93,7 +96,7 @@ class HostInfoFlowTest extends ResourceFlowTestCase assertNoBillingEvents(); } - @Test + @TestOfyAndSql void testSuccess_linked() throws Exception { persistHostResource(); persistResource( @@ -122,13 +125,16 @@ class HostInfoFlowTest extends ResourceFlowTestCase .setLastTransferTime(domainTransferTime) .setPersistedCurrentSponsorClientId("superclientid") .build()); + HostResource firstHost = persistHostResource(); persistResource( - persistHostResource() + firstHost .asBuilder() .setRepoId("CEEF-FOOBAR") .setSuperordinateDomain(domain.createVKey()) .setLastSuperordinateChange(lastSuperordinateChange) .build()); + // we shouldn't have two active hosts with the same hostname + deleteResource(firstHost); assertTransactionalFlow(false); runFlowAssertResponse( loadFile("host_info_response_superordinate_clientid.xml"), @@ -138,31 +144,31 @@ class HostInfoFlowTest extends ResourceFlowTestCase assertNoBillingEvents(); } - @Test + @TestOfyAndSql void testSuccess_withSuperordinateDomain_hostMovedAfterDomainTransfer() throws Exception { runTest_superordinateDomain( DateTime.parse("2000-01-08T09:00:00.0Z"), DateTime.parse("2000-03-01T01:00:00.0Z")); } - @Test + @TestOfyAndSql void testSuccess_withSuperordinateDomain_hostMovedBeforeDomainTransfer() throws Exception { runTest_superordinateDomain( DateTime.parse("2000-04-08T09:00:00.0Z"), DateTime.parse("2000-02-08T09:00:00.0Z")); } - @Test + @TestOfyAndSql void testSuccess_withSuperordinateDomain() throws Exception { runTest_superordinateDomain(DateTime.parse("2000-04-08T09:00:00.0Z"), null); } - @Test + @TestOfyAndSql void testFailure_neverExisted() throws Exception { ResourceDoesNotExistException thrown = assertThrows(ResourceDoesNotExistException.class, this::runFlow); assertThat(thrown).hasMessageThat().contains(String.format("(%s)", getUniqueIdFromCommand())); } - @Test + @TestOfyAndSql void testFailure_existedButWasDeleted() throws Exception { persistResource( persistHostResource().asBuilder().setDeletionTime(clock.nowUtc().minusDays(1)).build()); @@ -171,14 +177,14 @@ class HostInfoFlowTest extends ResourceFlowTestCase assertThat(thrown).hasMessageThat().contains(String.format("(%s)", getUniqueIdFromCommand())); } - @Test + @TestOfyAndSql void testFailure_nonLowerCaseHostname() { setEppInput("host_info.xml", ImmutableMap.of("HOSTNAME", "NS1.EXAMPLE.NET")); EppException thrown = assertThrows(HostNameNotLowerCaseException.class, this::runFlow); assertAboutEppExceptions().that(thrown).marshalsToXml(); } - @Test + @TestOfyAndSql void testFailure_nonPunyCodedHostname() { setEppInput("host_info.xml", ImmutableMap.of("HOSTNAME", "ns1.çauçalito.tld")); HostNameNotPunyCodedException thrown = @@ -186,14 +192,14 @@ class HostInfoFlowTest extends ResourceFlowTestCase assertThat(thrown).hasMessageThat().contains("expected ns1.xn--aualito-txac.tld"); } - @Test + @TestOfyAndSql void testFailure_nonCanonicalHostname() { setEppInput("host_info.xml", ImmutableMap.of("HOSTNAME", "ns1.example.tld.")); EppException thrown = assertThrows(HostNameNotNormalizedException.class, this::runFlow); assertAboutEppExceptions().that(thrown).marshalsToXml(); } - @Test + @TestOfyAndSql void testIcannActivityReportField_getsLogged() throws Exception { persistHostResource(); runFlow();