mirror of
https://github.com/google/nomulus
synced 2026-01-06 13:36:48 +00:00
Fix error handling in CopyDetailReportsAction (#2793)
* Fix error handling in CopyDetailReportsAction The action tries to record errors per registrar in an ImmutableMap, without realizing that there may be duplicate keys due to retries. Switched to the `buildKeepingLast` method to build the map. * Addressing comments and rebase
This commit is contained in:
@@ -18,11 +18,12 @@ import static com.google.common.base.Throwables.getRootCause;
|
||||
import static google.registry.request.Action.Method.POST;
|
||||
import static jakarta.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
|
||||
import static jakarta.servlet.http.HttpServletResponse.SC_OK;
|
||||
import static java.util.stream.Collectors.joining;
|
||||
|
||||
import com.google.cloud.storage.BlobId;
|
||||
import com.google.common.base.Splitter;
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.common.collect.ImmutableMap;
|
||||
import com.google.common.collect.ImmutableMultimap;
|
||||
import com.google.common.collect.Iterables;
|
||||
import com.google.common.flogger.FluentLogger;
|
||||
import com.google.common.io.ByteStreams;
|
||||
@@ -41,7 +42,6 @@ import jakarta.inject.Inject;
|
||||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.util.Optional;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
/** Copy all registrar detail reports in a given bucket's subdirectory from GCS to Drive. */
|
||||
@Action(
|
||||
@@ -98,7 +98,8 @@ public final class CopyDetailReportsAction implements Runnable {
|
||||
response.setPayload(String.format("Failure, encountered %s", e.getMessage()));
|
||||
return;
|
||||
}
|
||||
ImmutableMap.Builder<String, Throwable> copyErrorsBuilder = new ImmutableMap.Builder<>();
|
||||
ImmutableMultimap.Builder<String, Throwable> copyErrorsBuilder =
|
||||
new ImmutableMultimap.Builder<>();
|
||||
for (String detailReportName : detailReportObjectNames) {
|
||||
// The standard report format is "invoice_details_yyyy-MM_registrarId_tld.csv
|
||||
// TODO(larryruili): Determine a safer way of enforcing this.
|
||||
@@ -145,17 +146,18 @@ public final class CopyDetailReportsAction implements Runnable {
|
||||
response.setStatus(SC_OK);
|
||||
response.setContentType(MediaType.PLAIN_TEXT_UTF_8);
|
||||
StringBuilder payload = new StringBuilder().append("Copied detail reports.\n");
|
||||
ImmutableMap<String, Throwable> copyErrors = copyErrorsBuilder.build();
|
||||
ImmutableMultimap<String, Throwable> copyErrors = copyErrorsBuilder.build();
|
||||
if (!copyErrors.isEmpty()) {
|
||||
payload.append("The following errors were encountered:\n");
|
||||
payload.append(
|
||||
copyErrors.entrySet().stream()
|
||||
.map(
|
||||
entrySet ->
|
||||
String.format(
|
||||
"Registrar: %s\nError: %s\n",
|
||||
entrySet.getKey(), entrySet.getValue().getMessage()))
|
||||
.collect(Collectors.joining()));
|
||||
for (var registrarId : copyErrors.keySet()) {
|
||||
payload.append(
|
||||
String.format(
|
||||
"Registrar: %s\nError: %s\n",
|
||||
registrarId,
|
||||
copyErrors.get(registrarId).stream()
|
||||
.map(Throwable::getMessage)
|
||||
.collect(joining("\n\t"))));
|
||||
}
|
||||
}
|
||||
response.setPayload(payload.toString());
|
||||
emailUtils.sendAlertEmail(payload.toString());
|
||||
|
||||
@@ -178,21 +178,56 @@ class CopyDetailReportsActionTest {
|
||||
verify(emailUtils)
|
||||
.sendAlertEmail(
|
||||
"""
|
||||
Copied detail reports.
|
||||
The following errors were encountered:
|
||||
Registrar: TheRegistrar
|
||||
Error: java.io.IOException: expected
|
||||
""");
|
||||
Copied detail reports.
|
||||
The following errors were encountered:
|
||||
Registrar: TheRegistrar
|
||||
Error: java.io.IOException: expected
|
||||
""");
|
||||
assertThat(response.getStatus()).isEqualTo(SC_OK);
|
||||
assertThat(response.getContentType()).isEqualTo(MediaType.PLAIN_TEXT_UTF_8);
|
||||
assertThat(response.getPayload())
|
||||
.isEqualTo(
|
||||
"""
|
||||
Copied detail reports.
|
||||
The following errors were encountered:
|
||||
Registrar: TheRegistrar
|
||||
Error: java.io.IOException: expected
|
||||
""");
|
||||
Copied detail reports.
|
||||
The following errors were encountered:
|
||||
Registrar: TheRegistrar
|
||||
Error: java.io.IOException: expected
|
||||
""");
|
||||
}
|
||||
|
||||
@Test
|
||||
void testFail_tooManyFailures_one_registrar_sendsAlertEmail_continues() throws IOException {
|
||||
gcsUtils.createFromBytes(
|
||||
BlobId.of("test-bucket", "results/invoice_details_2017-10_TheRegistrar_hello.csv"),
|
||||
"hola,mundo\n3,4".getBytes(UTF_8));
|
||||
|
||||
gcsUtils.createFromBytes(
|
||||
BlobId.of("test-bucket", "results/invoice_details_2017-10_TheRegistrar_test.csv"),
|
||||
"hello,world\n1,2".getBytes(UTF_8));
|
||||
when(driveConnection.createOrUpdateFile(any(), any(), any(), any()))
|
||||
.thenThrow(new IOException("expected"));
|
||||
|
||||
action.run();
|
||||
verify(emailUtils)
|
||||
.sendAlertEmail(
|
||||
"""
|
||||
Copied detail reports.
|
||||
The following errors were encountered:
|
||||
Registrar: TheRegistrar
|
||||
Error: java.io.IOException: expected
|
||||
\tjava.io.IOException: expected
|
||||
""");
|
||||
assertThat(response.getStatus()).isEqualTo(SC_OK);
|
||||
assertThat(response.getContentType()).isEqualTo(MediaType.PLAIN_TEXT_UTF_8);
|
||||
assertThat(response.getPayload())
|
||||
.isEqualTo(
|
||||
"""
|
||||
Copied detail reports.
|
||||
The following errors were encountered:
|
||||
Registrar: TheRegistrar
|
||||
Error: java.io.IOException: expected
|
||||
\tjava.io.IOException: expected
|
||||
""");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
Reference in New Issue
Block a user