Remove some appengine dependencies (#1874)

Some retriers are no longer needed because transactions are
automatically retried by the JPA transaction manager when there's a
transient exception.

<!-- Reviewable:start -->
- - -
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/google/nomulus/1874)
<!-- Reviewable:end -->
This commit is contained in:
Lai Jiang 2022-12-08 11:46:47 -05:00 committed by GitHub
parent 8c75ce08af
commit 71024c706d
20 changed files with 1054 additions and 1174 deletions

View file

@ -27,7 +27,6 @@ import static java.nio.charset.StandardCharsets.UTF_8;
import com.beust.jcommander.Parameter;
import com.beust.jcommander.Parameters;
import com.google.appengine.tools.remoteapi.RemoteApiException;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import com.google.common.base.Splitter;
@ -47,7 +46,6 @@ import google.registry.tools.params.TransitionListParameter.TokenStatusTransitio
import google.registry.util.CollectionUtils;
import google.registry.util.DomainNameUtils;
import google.registry.util.NonFinalForTesting;
import google.registry.util.Retrier;
import google.registry.util.StringGenerator;
import java.io.File;
import java.io.IOException;
@ -170,8 +168,6 @@ class GenerateAllocationTokensCommand implements Command {
@Named("base58StringGenerator")
StringGenerator stringGenerator;
@Inject Retrier retrier;
private static final int BATCH_SIZE = 20;
private static final Joiner SKIP_NULLS = Joiner.on(',').skipNulls();
@ -220,8 +216,7 @@ class GenerateAllocationTokensCommand implements Command {
return token.build();
})
.collect(toImmutableSet());
// Wrap in a retrier to deal with transient 404 errors (thrown as RemoteApiExceptions).
tokensSaved += retrier.callWithRetry(() -> saveTokens(tokens), RemoteApiException.class);
tokensSaved += saveTokens(tokens);
} while (tokensSaved < numTokens);
}
@ -287,7 +282,7 @@ class GenerateAllocationTokensCommand implements Command {
if (dryRun) {
savedTokens = tokens;
} else {
tm().transact(() -> tm().transact(() -> tm().putAll(tokens)));
tm().transact(() -> tm().putAll(tokens));
savedTokens = tm().transact(() -> tm().loadByEntities(tokens));
}
savedTokens.forEach(

View file

@ -18,8 +18,6 @@ import static google.registry.request.Action.Method.POST;
import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
import static javax.servlet.http.HttpServletResponse.SC_OK;
import com.google.appengine.api.datastore.DatastoreFailureException;
import com.google.appengine.api.datastore.DatastoreTimeoutException;
import com.google.common.flogger.FluentLogger;
import com.google.common.net.MediaType;
import google.registry.config.RegistryConfig.Config;
@ -86,19 +84,12 @@ public class WhoisAction implements Runnable {
try {
final WhoisCommand command = whoisReader.readCommand(input, false, now);
metricBuilder.setCommand(command);
WhoisResponseResults results =
retrier.callWithRetry(
() -> {
WhoisResponseResults results1;
try {
results1 = command.executeQuery(now).getResponse(PREFER_UNICODE, disclaimer);
} catch (WhoisException e) {
throw new UncheckedWhoisException(e);
}
return results1;
},
DatastoreTimeoutException.class,
DatastoreFailureException.class);
WhoisResponseResults results;
try {
results = command.executeQuery(now).getResponse(PREFER_UNICODE, disclaimer);
} catch (WhoisException e) {
throw new UncheckedWhoisException(e);
}
responseText = results.plainTextOutput();
setWhoisMetrics(metricBuilder, results.numResults(), SC_OK);
} catch (UncheckedWhoisException u) {

View file

@ -28,13 +28,8 @@ import static google.registry.util.DateTimeUtils.START_OF_TIME;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.joda.time.DateTimeZone.UTC;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import com.beust.jcommander.ParameterException;
import com.google.appengine.tools.remoteapi.RemoteApiException;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
@ -45,9 +40,6 @@ import google.registry.model.domain.token.AllocationToken.TokenStatus;
import google.registry.model.reporting.HistoryEntry.HistoryEntryId;
import google.registry.testing.DeterministicStringGenerator;
import google.registry.testing.DeterministicStringGenerator.Rule;
import google.registry.testing.FakeClock;
import google.registry.testing.FakeSleeper;
import google.registry.util.Retrier;
import google.registry.util.StringGenerator.Alphabets;
import java.io.File;
import java.util.Collection;
@ -55,7 +47,6 @@ import javax.annotation.Nullable;
import org.joda.time.DateTime;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentMatchers;
/** Unit tests for {@link GenerateAllocationTokensCommand}. */
class GenerateAllocationTokensCommandTest extends CommandTestCase<GenerateAllocationTokensCommand> {
@ -63,8 +54,6 @@ class GenerateAllocationTokensCommandTest extends CommandTestCase<GenerateAlloca
@BeforeEach
void beforeEach() {
command.stringGenerator = new DeterministicStringGenerator(Alphabets.BASE_58);
command.retrier =
new Retrier(new FakeSleeper(new FakeClock(DateTime.parse("2000-01-01TZ"))), 3);
}
@Test
@ -91,21 +80,6 @@ class GenerateAllocationTokensCommandTest extends CommandTestCase<GenerateAlloca
assertInStdout("123456789ABCDEFG");
}
@Test
void testSuccess_retry() throws Exception {
command = spy(command);
RemoteApiException fakeException = new RemoteApiException("foo", "foo", "foo", new Exception());
doThrow(fakeException)
.doThrow(fakeException)
.doCallRealMethod()
.when(command)
.saveTokens(ArgumentMatchers.any());
runCommand("--number", "1");
assertAllocationTokens(createToken("123456789ABCDEFG", null, null));
assertInStdout("123456789ABCDEFG");
verify(command, times(3)).saveTokens(ArgumentMatchers.any());
}
@Test
void testSuccess_tokenCollision() throws Exception {
AllocationToken existingToken =

View file

@ -38,8 +38,6 @@ import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import com.google.appengine.api.datastore.DatastoreFailureException;
import com.google.appengine.api.datastore.DatastoreTimeoutException;
import com.google.common.collect.ImmutableSet;
import com.google.common.net.InetAddresses;
import google.registry.model.contact.Contact;
@ -642,29 +640,4 @@ public class WhoisActionTest {
verify(action.whoisMetrics).recordWhoisMetric(eq(expected));
assertThat(response.getPayload()).isEqualTo("Internal Server Error");
}
@Test
void testRun_retryOnTransientFailure() throws Exception {
persistResource(loadRegistrar("TheRegistrar").asBuilder().setUrl("http://my.fake.url").build());
persistResource(FullFieldsTestEntityHelper.makeHost("ns1.cat.lol", "1.2.3.4"));
WhoisAction action = newWhoisAction("ns1.cat.lol");
WhoisResponse expectedResponse =
action
.whoisReader
.readCommand(action.input, false, clock.nowUtc())
.executeQuery(clock.nowUtc());
WhoisReader mockReader = mock(WhoisReader.class);
WhoisCommand mockCommand = mock(WhoisCommand.class);
when(mockReader.readCommand(any(Reader.class), eq(false), any(DateTime.class)))
.thenReturn(mockCommand);
when(mockCommand.executeQuery(any(DateTime.class)))
.thenThrow(new DatastoreFailureException("Expected transient exception #1"))
.thenThrow(new DatastoreTimeoutException("Expected transient exception #2"))
.thenReturn(expectedResponse);
action.whoisReader = mockReader;
action.run();
assertThat(response.getPayload()).isEqualTo(loadFile("whois_action_nameserver.txt"));
}
}