Fix some small transactional issues in SQL mode (#1662)

* Fix some small transactional issues in SQL mode

These weren't caught until I switched the default database type in tests
to be SQL (separate PR). Fortunately these don't seem to be catastrophic
This commit is contained in:
gbrodman 2022-06-09 15:01:34 -04:00 committed by GitHub
parent 623356b1e8
commit e30b3f9e0b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 38 additions and 30 deletions

View file

@ -49,11 +49,14 @@ final class GetAllocationTokenCommand implements CommandWithRemoteApi {
tokens.stream()
.map(t -> VKey.create(AllocationToken.class, t))
.collect(toImmutableList());
tm().loadByKeysIfPresent(tokenKeys)
.forEach((k, v) -> builder.put(k.getSqlKey().toString(), v));
tm().transact(
() ->
tm().loadByKeysIfPresent(tokenKeys)
.forEach((k, v) -> builder.put(k.getSqlKey().toString(), v)));
}
ImmutableMap<String, AllocationToken> loadedTokens = builder.build();
ImmutableMap<VKey<DomainBase>, DomainBase> domains = loadRedeemedDomains(loadedTokens.values());
ImmutableMap<VKey<DomainBase>, DomainBase> domains =
tm().transact(() -> loadRedeemedDomains(loadedTokens.values()));
for (String token : mainParameters) {
if (loadedTokens.containsKey(token)) {

View file

@ -204,7 +204,8 @@ final class UniformRapidSuspensionCommand extends MutatingEppToolCommand {
private ImmutableSortedSet<String> getExistingNameservers(DomainBase domain) {
ImmutableSortedSet.Builder<String> nameservers = ImmutableSortedSet.naturalOrder();
for (HostResource host : tm().loadByKeys(domain.getNameservers()).values()) {
for (HostResource host :
tm().transact(() -> tm().loadByKeys(domain.getNameservers()).values())) {
nameservers.add(host.getForeignKey());
}
return nameservers.build();

View file

@ -29,13 +29,15 @@ import com.googlecode.objectify.Key;
import google.registry.model.domain.DomainBase;
import google.registry.model.domain.token.AllocationToken;
import google.registry.model.reporting.HistoryEntry;
import google.registry.testing.DualDatabaseTest;
import google.registry.testing.TestSqlOnly;
import org.joda.time.DateTime;
import org.junit.jupiter.api.Test;
/** Unit tests for {@link GetAllocationTokenCommand}. */
@DualDatabaseTest
class GetAllocationTokenCommandTest extends CommandTestCase<GetAllocationTokenCommand> {
@Test
@TestSqlOnly
void testSuccess_oneToken() throws Exception {
createTlds("bar");
AllocationToken token =
@ -49,7 +51,7 @@ class GetAllocationTokenCommandTest extends CommandTestCase<GetAllocationTokenCo
assertInStdout(token.toString(), "Token foo was not redeemed.");
}
@Test
@TestSqlOnly
void testSuccess_multipleTokens() throws Exception {
createTlds("baz");
ImmutableList<AllocationToken> tokens =
@ -73,7 +75,7 @@ class GetAllocationTokenCommandTest extends CommandTestCase<GetAllocationTokenCo
"Token fii was not redeemed.");
}
@Test
@TestSqlOnly
void testSuccess_redeemedToken() throws Exception {
createTld("tld");
DomainBase domain =
@ -93,7 +95,7 @@ class GetAllocationTokenCommandTest extends CommandTestCase<GetAllocationTokenCo
"Token foo was redeemed to create domain fqqdn.tld at 2016-04-07T22:19:17.044Z.");
}
@Test
@TestSqlOnly
void testSuccess_oneTokenDoesNotExist() throws Exception {
createTlds("bar");
AllocationToken token =
@ -108,7 +110,7 @@ class GetAllocationTokenCommandTest extends CommandTestCase<GetAllocationTokenCo
token.toString(), "Token foo was not redeemed.", "ERROR: Token bar does not exist.");
}
@Test
@TestSqlOnly
void testFailure_noAllocationTokensSpecified() {
assertThrows(ParameterException.class, this::runCommand);
}

View file

@ -30,12 +30,14 @@ import google.registry.model.domain.secdns.DelegationSignerData;
import google.registry.model.eppcommon.StatusValue;
import google.registry.model.host.HostResource;
import google.registry.persistence.VKey;
import google.registry.testing.DualDatabaseTest;
import google.registry.testing.TestSqlOnly;
import javax.xml.bind.annotation.adapters.HexBinaryAdapter;
import org.joda.time.DateTime;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
/** Unit tests for {@link UniformRapidSuspensionCommand}. */
@DualDatabaseTest
class UniformRapidSuspensionCommandTest
extends EppToolCommandTestCase<UniformRapidSuspensionCommand> {
@ -72,7 +74,7 @@ class UniformRapidSuspensionCommandTest
domainBase.asBuilder().setNameservers(hostRefs.build()).setDsData(dsData).build());
}
@Test
@TestSqlOnly
void testCommand_addsLocksReplacesHostsAndDsDataPrintsUndo() throws Exception {
persistDomainWithHosts(defaultDomainBase, defaultDsData, ns1, ns2);
runCommandForced(
@ -93,7 +95,7 @@ class UniformRapidSuspensionCommandTest
assertNotInStdout("--restore_client_hold");
}
@Test
@TestSqlOnly
void testCommand_respectsExistingHost() throws Exception {
persistDomainWithHosts(defaultDomainBase, defaultDsData, urs2, ns1);
runCommandForced(
@ -111,7 +113,7 @@ class UniformRapidSuspensionCommandTest
assertNotInStdout("--locks_to_preserve");
}
@Test
@TestSqlOnly
void testCommand_generatesUndoForUndelegatedDomain() throws Exception {
persistActiveDomain("evil.tld");
runCommandForced(
@ -124,7 +126,7 @@ class UniformRapidSuspensionCommandTest
assertNotInStdout("--locks_to_preserve");
}
@Test
@TestSqlOnly
void testCommand_generatesUndoWithLocksToPreserve() throws Exception {
persistResource(
newDomainBase("evil.tld").asBuilder()
@ -137,7 +139,7 @@ class UniformRapidSuspensionCommandTest
assertInStdout("--locks_to_preserve serverDeleteProhibited");
}
@Test
@TestSqlOnly
void testCommand_removeClientHold() throws Exception {
persistResource(
newDomainBase("evil.tld")
@ -162,7 +164,7 @@ class UniformRapidSuspensionCommandTest
assertInStdout("--restore_client_hold");
}
@Test
@TestSqlOnly
void testUndo_removesLocksReplacesHostsAndDsData() throws Exception {
persistDomainWithHosts(defaultDomainBase, defaultDsData, urs1, urs2);
runCommandForced(
@ -178,7 +180,7 @@ class UniformRapidSuspensionCommandTest
assertNotInStdout("--undo"); // Undo shouldn't print a new undo command.
}
@Test
@TestSqlOnly
void testUndo_respectsLocksToPreserveFlag() throws Exception {
persistDomainWithHosts(defaultDomainBase, defaultDsData, urs1, urs2);
runCommandForced(
@ -195,7 +197,7 @@ class UniformRapidSuspensionCommandTest
assertNotInStdout("--undo"); // Undo shouldn't print a new undo command.
}
@Test
@TestSqlOnly
void testUndo_restoresClientHolds() throws Exception {
persistDomainWithHosts(defaultDomainBase, defaultDsData, urs1, urs2);
runCommandForced(
@ -212,7 +214,7 @@ class UniformRapidSuspensionCommandTest
assertNotInStdout("--undo"); // Undo shouldn't print a new undo command.
}
@Test
@TestSqlOnly
void testAutorenews_setToFalseByDefault() throws Exception {
persistResource(
newDomainBase("evil.tld")
@ -224,7 +226,7 @@ class UniformRapidSuspensionCommandTest
assertInStdout("<superuser:autorenews>false</superuser:autorenews>");
}
@Test
@TestSqlOnly
void testAutorenews_setToTrueWhenUndo() throws Exception {
persistResource(
newDomainBase("evil.tld")
@ -241,7 +243,7 @@ class UniformRapidSuspensionCommandTest
assertInStdout("<superuser:autorenews>true</superuser:autorenews>");
}
@Test
@TestSqlOnly
void testRenewOneYearWithoutUndo_verifyReasonWithoutUndo() throws Exception {
persistDomainWithHosts(
newDomainBase("evil.tld")
@ -278,7 +280,7 @@ class UniformRapidSuspensionCommandTest
.verifySentAny();
}
@Test
@TestSqlOnly
void testRenewOneYearWithUndo_verifyReasonWithUndo() throws Exception {
persistDomainWithHosts(
newDomainBase("evil.tld")
@ -316,7 +318,7 @@ class UniformRapidSuspensionCommandTest
.verifySentAny();
}
@Test
@TestSqlOnly
void testRenewOneYear_verifyBothRenewAndUpdateFlowsAreTriggered() throws Exception {
persistDomainWithHosts(
newDomainBase("evil.tld")
@ -361,7 +363,7 @@ class UniformRapidSuspensionCommandTest
eppVerifier.verifyNoMoreSent();
}
@Test
@TestSqlOnly
void testFailure_locksToPreserveWithoutUndo() {
persistActiveDomain("evil.tld");
IllegalArgumentException thrown =
@ -375,7 +377,7 @@ class UniformRapidSuspensionCommandTest
assertThat(thrown).hasMessageThat().contains("--undo");
}
@Test
@TestSqlOnly
void testFailure_domainNameRequired() {
persistActiveDomain("evil.tld");
ParameterException thrown =
@ -387,7 +389,7 @@ class UniformRapidSuspensionCommandTest
assertThat(thrown).hasMessageThat().contains("--domain_name");
}
@Test
@TestSqlOnly
void testFailure_renewOneYearRequired() {
persistActiveDomain("evil.tld");
ParameterException thrown =
@ -395,7 +397,7 @@ class UniformRapidSuspensionCommandTest
assertThat(thrown).hasMessageThat().contains("--renew_one_year");
}
@Test
@TestSqlOnly
void testFailure_extraFieldInDsData() {
persistActiveDomain("evil.tld");
IllegalArgumentException thrown =
@ -409,7 +411,7 @@ class UniformRapidSuspensionCommandTest
.contains("dsRecord 1 1 1 abc 1 should have 4 parts, but has 5");
}
@Test
@TestSqlOnly
void testFailure_missingFieldInDsData() {
persistActiveDomain("evil.tld");
IllegalArgumentException thrown =
@ -421,7 +423,7 @@ class UniformRapidSuspensionCommandTest
assertThat(thrown).hasMessageThat().contains("dsRecord 1 1 1 should have 4 parts, but has 3");
}
@Test
@TestSqlOnly
void testFailure_malformedDsData() {
persistActiveDomain("evil.tld");
IllegalArgumentException thrown =