mirror of
https://github.com/google/nomulus.git
synced 2025-05-31 01:34:05 +02:00
Fail gracefully when copying detailed reports (#181)
* Fail gracefully when copying detailed reports When the detailed reports are copied from GCS to registrars' Drive folders, do not fail the entire copy operation when a single registrar fails. Instead, send an alert email about the failure, and continue to copy the rest of the reports. Also, instead of creating duplicates, overwrite the existing files on Drive. BUG=127690361
This commit is contained in:
Lai Jiang
GitHub
parent
7352f9b4a6
commit
e17cb52bf7
3 changed files with 69 additions and 22 deletions
|
|
@ -22,6 +22,7 @@ import static javax.servlet.http.HttpServletResponse.SC_OK;
|
|||
import com.google.appengine.tools.cloudstorage.GcsFilename;
|
||||
import com.google.common.base.Splitter;
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.common.collect.ImmutableMap;
|
||||
import com.google.common.collect.Iterables;
|
||||
import com.google.common.flogger.FluentLogger;
|
||||
import com.google.common.io.ByteStreams;
|
||||
|
|
@ -38,6 +39,7 @@ import google.registry.util.Retrier;
|
|||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.util.Optional;
|
||||
import java.util.stream.Collectors;
|
||||
import javax.inject.Inject;
|
||||
|
||||
/** Copy all registrar detail reports in a given bucket's subdirectory from GCS to Drive. */
|
||||
|
|
@ -95,6 +97,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<String, Throwable>();
|
||||
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.
|
||||
|
|
@ -117,7 +121,7 @@ public final class CopyDetailReportsAction implements Runnable {
|
|||
try (InputStream input =
|
||||
gcsUtils.openInputStream(
|
||||
new GcsFilename(billingBucket, invoiceDirectoryPrefix + detailReportName))) {
|
||||
driveConnection.createFile(
|
||||
driveConnection.createOrUpdateFile(
|
||||
detailReportName,
|
||||
MediaType.CSV_UTF_8,
|
||||
driveFolderId,
|
||||
|
|
@ -129,15 +133,31 @@ public final class CopyDetailReportsAction implements Runnable {
|
|||
},
|
||||
IOException.class);
|
||||
} catch (Throwable e) {
|
||||
emailUtils.sendAlertEmail(
|
||||
String alertMessage =
|
||||
String.format(
|
||||
"Warning: CopyDetailReportsAction failed.\nEncountered: %s on file: %s",
|
||||
getRootCause(e).getMessage(), detailReportName));
|
||||
throw e;
|
||||
"Warning: CopyDetailReportsAction failed for registrar %s.\n"
|
||||
+ "Encountered: %s on file: %s",
|
||||
registrarId, getRootCause(e).getMessage(), detailReportName);
|
||||
copyErrorsBuilder.put(registrarId, e);
|
||||
logger.atSevere().withCause(e).log(alertMessage);
|
||||
}
|
||||
}
|
||||
response.setStatus(SC_OK);
|
||||
response.setContentType(MediaType.PLAIN_TEXT_UTF_8);
|
||||
response.setPayload("Copied detail reports.");
|
||||
StringBuilder payload = new StringBuilder().append("Copied detail reports.\n");
|
||||
ImmutableMap<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()));
|
||||
}
|
||||
response.setPayload(payload.toString());
|
||||
emailUtils.sendAlertEmail(payload.toString());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -54,6 +54,10 @@ public class DriveConnection {
|
|||
/**
|
||||
* Creates a file with the given parent.
|
||||
*
|
||||
* <p>If a file with the same path already exists, a duplicate is created. If overwriting the
|
||||
* existing file is the desired behavior, use {@link #createOrUpdateFile(String, MediaType,
|
||||
* String, byte[])} instead.
|
||||
*
|
||||
* @returns the file id.
|
||||
*/
|
||||
public String createFile(String title, MediaType mimeType, String parentFolderId, byte[] bytes)
|
||||
|
|
|
|||
|
|
@ -18,10 +18,10 @@ import static com.google.common.truth.Truth.assertThat;
|
|||
import static google.registry.testing.DatastoreHelper.loadRegistrar;
|
||||
import static google.registry.testing.DatastoreHelper.persistResource;
|
||||
import static google.registry.testing.GcsTestingUtils.writeGcsFile;
|
||||
import static google.registry.testing.JUnitBackports.assertThrows;
|
||||
import static java.nio.charset.StandardCharsets.UTF_8;
|
||||
import static javax.servlet.http.HttpServletResponse.SC_OK;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.ArgumentMatchers.eq;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.times;
|
||||
import static org.mockito.Mockito.verify;
|
||||
|
|
@ -68,6 +68,7 @@ public class CopyDetailReportsActionTest {
|
|||
@Before
|
||||
public void setUp() {
|
||||
persistResource(loadRegistrar("TheRegistrar").asBuilder().setDriveFolderId("0B-12345").build());
|
||||
persistResource(loadRegistrar("NewRegistrar").asBuilder().setDriveFolderId("0B-54321").build());
|
||||
response = new FakeResponse();
|
||||
driveConnection = mock(DriveConnection.class);
|
||||
emailUtils = mock(BillingEmailUtils.class);
|
||||
|
|
@ -96,21 +97,21 @@ public class CopyDetailReportsActionTest {
|
|||
|
||||
action.run();
|
||||
verify(driveConnection)
|
||||
.createFile(
|
||||
.createOrUpdateFile(
|
||||
"invoice_details_2017-10_TheRegistrar_test.csv",
|
||||
MediaType.CSV_UTF_8,
|
||||
"0B-12345",
|
||||
"hello,world\n1,2".getBytes(UTF_8));
|
||||
|
||||
verify(driveConnection)
|
||||
.createFile(
|
||||
.createOrUpdateFile(
|
||||
"invoice_details_2017-10_TheRegistrar_hello.csv",
|
||||
MediaType.CSV_UTF_8,
|
||||
"0B-12345",
|
||||
"hola,mundo\n3,4".getBytes(UTF_8));
|
||||
assertThat(response.getStatus()).isEqualTo(SC_OK);
|
||||
assertThat(response.getContentType()).isEqualTo(MediaType.PLAIN_TEXT_UTF_8);
|
||||
assertThat(response.getPayload()).isEqualTo("Copied detail reports.");
|
||||
assertThat(response.getPayload()).isEqualTo("Copied detail reports.\n");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -126,7 +127,7 @@ public class CopyDetailReportsActionTest {
|
|||
"hello,world\n1,2".getBytes(UTF_8));
|
||||
action.run();
|
||||
verify(driveConnection)
|
||||
.createFile(
|
||||
.createOrUpdateFile(
|
||||
"invoice_details_2017-10_TheRegistrar_hello.csv",
|
||||
MediaType.CSV_UTF_8,
|
||||
"0B-12345",
|
||||
|
|
@ -135,7 +136,7 @@ public class CopyDetailReportsActionTest {
|
|||
verifyNoMoreInteractions(driveConnection);
|
||||
assertThat(response.getStatus()).isEqualTo(SC_OK);
|
||||
assertThat(response.getContentType()).isEqualTo(MediaType.PLAIN_TEXT_UTF_8);
|
||||
assertThat(response.getPayload()).isEqualTo("Copied detail reports.");
|
||||
assertThat(response.getPayload()).isEqualTo("Copied detail reports.\n");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -144,41 +145,63 @@ public class CopyDetailReportsActionTest {
|
|||
gcsService,
|
||||
new GcsFilename("test-bucket", "results/invoice_details_2017-10_TheRegistrar_hello.csv"),
|
||||
"hola,mundo\n3,4".getBytes(UTF_8));
|
||||
when(driveConnection.createFile(any(), any(), any(), any()))
|
||||
when(driveConnection.createOrUpdateFile(any(), any(), any(), any()))
|
||||
.thenThrow(new IOException("expected"))
|
||||
.thenReturn("success");
|
||||
|
||||
action.run();
|
||||
verify(driveConnection, times(2))
|
||||
.createFile(
|
||||
.createOrUpdateFile(
|
||||
"invoice_details_2017-10_TheRegistrar_hello.csv",
|
||||
MediaType.CSV_UTF_8,
|
||||
"0B-12345",
|
||||
"hola,mundo\n3,4".getBytes(UTF_8));
|
||||
assertThat(response.getStatus()).isEqualTo(SC_OK);
|
||||
assertThat(response.getContentType()).isEqualTo(MediaType.PLAIN_TEXT_UTF_8);
|
||||
assertThat(response.getPayload()).isEqualTo("Copied detail reports.");
|
||||
assertThat(response.getPayload()).isEqualTo("Copied detail reports.\n");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testFail_tooManyFailures_sendsAlertEmail() throws IOException {
|
||||
public void testFail_tooManyFailures_sendsAlertEmail_continues() throws IOException {
|
||||
writeGcsFile(
|
||||
gcsService,
|
||||
new GcsFilename("test-bucket", "results/invoice_details_2017-10_TheRegistrar_hello.csv"),
|
||||
"hola,mundo\n3,4".getBytes(UTF_8));
|
||||
when(driveConnection.createFile(any(), any(), any(), any()))
|
||||
writeGcsFile(
|
||||
gcsService,
|
||||
new GcsFilename("test-bucket", "results/invoice_details_2017-10_NewRegistrar_test.csv"),
|
||||
"hello,world\n1,2".getBytes(UTF_8));
|
||||
when(driveConnection.createOrUpdateFile(
|
||||
eq("invoice_details_2017-10_TheRegistrar_hello.csv"), any(), any(), any()))
|
||||
.thenThrow(new IOException("expected"));
|
||||
|
||||
RuntimeException thrown = assertThrows(RuntimeException.class, action::run);
|
||||
assertThat(thrown).hasMessageThat().isEqualTo("java.io.IOException: expected");
|
||||
action.run();
|
||||
verify(driveConnection, times(3))
|
||||
.createFile(
|
||||
.createOrUpdateFile(
|
||||
"invoice_details_2017-10_TheRegistrar_hello.csv",
|
||||
MediaType.CSV_UTF_8,
|
||||
"0B-12345",
|
||||
"hola,mundo\n3,4".getBytes(UTF_8));
|
||||
verify(emailUtils).sendAlertEmail("Warning: CopyDetailReportsAction failed.\nEncountered: "
|
||||
+ "expected on file: invoice_details_2017-10_TheRegistrar_hello.csv");
|
||||
verify(driveConnection)
|
||||
.createOrUpdateFile(
|
||||
"invoice_details_2017-10_NewRegistrar_test.csv",
|
||||
MediaType.CSV_UTF_8,
|
||||
"0B-54321",
|
||||
"hello,world\n1,2".getBytes(UTF_8));
|
||||
verify(emailUtils)
|
||||
.sendAlertEmail(
|
||||
"Copied detail reports.\n"
|
||||
+ "The following errors were encountered:\n"
|
||||
+ "Registrar: TheRegistrar\n"
|
||||
+ "Error: java.io.IOException: expected\n");
|
||||
assertThat(response.getStatus()).isEqualTo(SC_OK);
|
||||
assertThat(response.getContentType()).isEqualTo(MediaType.PLAIN_TEXT_UTF_8);
|
||||
assertThat(response.getPayload())
|
||||
.isEqualTo(
|
||||
"Copied detail reports.\n"
|
||||
+ "The following errors were encountered:\n"
|
||||
+ "Registrar: TheRegistrar\n"
|
||||
+ "Error: java.io.IOException: expected\n");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue