Add retry logic for SheetSynchronizer

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=138211996

java/google/registry/export/sheet/SheetSynchronizer.java

@@ -23,9 +23,11 @@ import com.google.gdata.data.spreadsheet.ListFeed;
 import com.google.gdata.data.spreadsheet.SpreadsheetEntry;
 import com.google.gdata.data.spreadsheet.WorksheetEntry;
 import com.google.gdata.util.ServiceException;
+import google.registry.util.Retrier;
 import java.io.IOException;
 import java.net.URL;
 import java.util.List;
+import java.util.concurrent.Callable;
 import javax.inject.Inject;
 
 /** Generic data synchronization utility for Google Spreadsheets. */
@@ -36,6 +38,7 @@ class SheetSynchronizer {
 
   @Inject SpreadsheetService spreadsheetService;
   @Inject SheetSynchronizer() {}
+  @Inject Retrier retrier;
 
   /**
    * Replace the contents of a Google Spreadsheet with {@code data}.
@@ -73,11 +76,12 @@ class SheetSynchronizer {
     WorksheetEntry worksheet = spreadsheet.getWorksheets().get(0);
     worksheet.setRowCount(data.size() + 1); // account for header row
     worksheet = worksheet.update();
-    ListFeed listFeed = spreadsheetService.getFeed(worksheet.getListFeedUrl(), ListFeed.class);
+    final ListFeed listFeed =
+        spreadsheetService.getFeed(worksheet.getListFeedUrl(), ListFeed.class);
     List<ListEntry> entries = listFeed.getEntries();
     int commonSize = Math.min(entries.size(), data.size());
     for (int i = 0; i < commonSize; i++) {
-      ListEntry entry = entries.get(i);
+      final ListEntry entry = entries.get(i);
       CustomElementCollection elements = entry.getCustomElements();
       boolean mutated = false;
       for (ImmutableMap.Entry<String, String> cell : data.get(i).entrySet()) {
@@ -87,17 +91,31 @@ class SheetSynchronizer {
         }
       }
       if (mutated) {
-        entry.update();
+        // Wrap in a retrier to deal with transient HTTP failures, which are IOExceptions wrapped
+        // in RuntimeExceptions.
+        retrier.callWithRetry(new Callable<Void>() {
+          @Override
+          public Void call() throws Exception {
+            entry.update();
+            return null;
+          }}, RuntimeException.class);
       }
     }
     if (data.size() > entries.size()) {
       for (int i = entries.size(); i < data.size(); i++) {
-        ListEntry entry = listFeed.createEntry();
+        final ListEntry entry = listFeed.createEntry();
         CustomElementCollection elements = entry.getCustomElements();
         for (ImmutableMap.Entry<String, String> cell : data.get(i).entrySet()) {
           elements.setValueLocal(cell.getKey(), cell.getValue());
         }
-        listFeed.insert(entry);
+        // Wrap in a retrier to deal with transient HTTP failures, which are IOExceptions wrapped
+        // in RuntimeExceptions.
+        retrier.callWithRetry(new Callable<Void>() {
+          @Override
+          public Void call() throws Exception {
+            listFeed.insert(entry);
+            return null;
+          }}, RuntimeException.class);
       }
     }
   }

java/google/registry/flows/async/AsyncFlowEnqueuer.java

@@ -78,10 +78,10 @@ public final class AsyncFlowEnqueuer {
    */
   private void addTaskToQueueWithRetry(final Queue queue, final TaskOptions task) {
     retrier.callWithRetry(new Callable<Void>() {
-    @Override
+      @Override
-    public Void call() throws Exception {
+      public Void call() throws Exception {
-      queue.add(task);
+        queue.add(task);
-      return null;
+        return null;
-    }}, TransientFailureException.class);
+      }}, TransientFailureException.class);
   }
 }

javatests/google/registry/export/sheet/BUILD

@@ -23,6 +23,7 @@ java_library(
         "//java/google/registry/config",
         "//java/google/registry/export/sheet",
         "//java/google/registry/model",
+        "//java/google/registry/util",
         "//javatests/google/registry/testing",
     ],
 )

javatests/google/registry/export/sheet/SheetSynchronizerTest.java

@@ -18,6 +18,7 @@ import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
@@ -30,7 +31,11 @@ import com.google.gdata.data.spreadsheet.ListEntry;
 import com.google.gdata.data.spreadsheet.ListFeed;
 import com.google.gdata.data.spreadsheet.SpreadsheetEntry;
 import com.google.gdata.data.spreadsheet.WorksheetEntry;
+import google.registry.testing.FakeClock;
+import google.registry.testing.FakeSleeper;
+import google.registry.util.Retrier;
 import java.net.URL;
+import org.joda.time.DateTime;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -41,32 +46,34 @@ import org.junit.runners.JUnit4;
 @RunWith(JUnit4.class)
 public class SheetSynchronizerTest {
 
+  private static final int MAX_RETRIES = 3;
+
   private final SpreadsheetService spreadsheetService = mock(SpreadsheetService.class);
   private final SpreadsheetEntry spreadsheet = mock(SpreadsheetEntry.class);
   private final WorksheetEntry worksheet = mock(WorksheetEntry.class);
   private final ListFeed listFeed = mock(ListFeed.class);
   private final SheetSynchronizer sheetSynchronizer = new SheetSynchronizer();
+  private final FakeSleeper sleeper =
+      new FakeSleeper(new FakeClock(DateTime.parse("2000-01-01TZ")));
 
   @Before
   public void before() throws Exception {
     sheetSynchronizer.spreadsheetService = spreadsheetService;
+    sheetSynchronizer.retrier = new Retrier(sleeper, MAX_RETRIES);
     when(spreadsheetService.getEntry(any(URL.class), eq(SpreadsheetEntry.class)))
         .thenReturn(spreadsheet);
-    when(spreadsheet.getWorksheets())
+    when(spreadsheet.getWorksheets()).thenReturn(ImmutableList.of(worksheet));
-        .thenReturn(ImmutableList.of(worksheet));
+    when(worksheet.getListFeedUrl()).thenReturn(new URL("http://example.com/spreadsheet"));
-    when(worksheet.getListFeedUrl())
+    when(spreadsheetService.getFeed(any(URL.class), eq(ListFeed.class))).thenReturn(listFeed);
-        .thenReturn(new URL("http://example.com/spreadsheet"));
+    when(worksheet.update()).thenReturn(worksheet);
-    when(spreadsheetService.getFeed(any(URL.class), eq(ListFeed.class)))
-        .thenReturn(listFeed);
-    when(worksheet.update())
-        .thenReturn(worksheet);
   }
 
   @After
   public void after() throws Exception {
-    verify(spreadsheetService).getEntry(
+    verify(spreadsheetService)
-        new URL("https://spreadsheets.google.com/feeds/spreadsheets/foobar"),
+        .getEntry(
-        SpreadsheetEntry.class);
+            new URL("https://spreadsheets.google.com/feeds/spreadsheets/foobar"),
+            SpreadsheetEntry.class);
     verify(spreadsheet).getWorksheets();
     verify(worksheet).getListFeedUrl();
     verify(spreadsheetService).getFeed(new URL("http://example.com/spreadsheet"), ListFeed.class);
@@ -86,8 +93,7 @@ public class SheetSynchronizerTest {
   public void testSynchronize_bothContainSameRow_doNothing() throws Exception {
     ListEntry entry = makeListEntry(ImmutableMap.of("key", "value"));
     when(listFeed.getEntries()).thenReturn(ImmutableList.of(entry));
-    sheetSynchronizer.synchronize("foobar", ImmutableList.of(
+    sheetSynchronizer.synchronize("foobar", ImmutableList.of(ImmutableMap.of("key", "value")));
-        ImmutableMap.of("key", "value")));
     verify(worksheet).setRowCount(2);
     verify(worksheet).update();
     verify(entry, atLeastOnce()).getCustomElements();
@@ -98,8 +104,7 @@ public class SheetSynchronizerTest {
   public void testSynchronize_cellIsDifferent_updateRow() throws Exception {
     ListEntry entry = makeListEntry(ImmutableMap.of("key", "value"));
     when(listFeed.getEntries()).thenReturn(ImmutableList.of(entry));
-    sheetSynchronizer.synchronize("foobar", ImmutableList.of(
+    sheetSynchronizer.synchronize("foobar", ImmutableList.of(ImmutableMap.of("key", "new value")));
-        ImmutableMap.of("key", "new value")));
     verify(entry.getCustomElements()).setValueLocal("key", "new value");
     verify(entry).update();
     verify(worksheet).setRowCount(2);
@@ -108,13 +113,29 @@ public class SheetSynchronizerTest {
     verifyNoMoreInteractions(entry);
   }
 
+  @Test
+  public void testSynchronize_cellIsDifferent_updateRow_retriesOnException() throws Exception {
+    ListEntry entry = makeListEntry(ImmutableMap.of("key", "value"));
+    when(listFeed.getEntries()).thenReturn(ImmutableList.of(entry));
+    when(entry.update())
+        .thenThrow(new RuntimeException())
+        .thenThrow(new RuntimeException())
+        .thenReturn(entry);
+    sheetSynchronizer.synchronize("foobar", ImmutableList.of(ImmutableMap.of("key", "new value")));
+    verify(entry.getCustomElements()).setValueLocal("key", "new value");
+    verify(entry, times(3)).update();
+    verify(worksheet).setRowCount(2);
+    verify(worksheet).update();
+    verify(entry, atLeastOnce()).getCustomElements();
+    verifyNoMoreInteractions(entry);
+  }
+
   @Test
   public void testSynchronize_spreadsheetMissingRow_insertRow() throws Exception {
     ListEntry entry = makeListEntry(ImmutableMap.<String, String>of());
     when(listFeed.getEntries()).thenReturn(ImmutableList.<ListEntry>of());
     when(listFeed.createEntry()).thenReturn(entry);
-    sheetSynchronizer.synchronize("foobar", ImmutableList.of(
+    sheetSynchronizer.synchronize("foobar", ImmutableList.of(ImmutableMap.of("key", "value")));
-        ImmutableMap.of("key", "value")));
     verify(entry.getCustomElements()).setValueLocal("key", "value");
     verify(listFeed).insert(entry);
     verify(worksheet).setRowCount(2);
@@ -124,6 +145,26 @@ public class SheetSynchronizerTest {
     verifyNoMoreInteractions(entry);
   }
 
+  @Test
+  public void testSynchronize_spreadsheetMissingRow_insertRow_retriesOnException()
+      throws Exception {
+    ListEntry entry = makeListEntry(ImmutableMap.<String, String>of());
+    when(listFeed.getEntries()).thenReturn(ImmutableList.<ListEntry>of());
+    when(listFeed.createEntry()).thenReturn(entry);
+    when(listFeed.insert(entry))
+        .thenThrow(new RuntimeException())
+        .thenThrow(new RuntimeException())
+        .thenReturn(entry);
+    sheetSynchronizer.synchronize("foobar", ImmutableList.of(ImmutableMap.of("key", "value")));
+    verify(entry.getCustomElements()).setValueLocal("key", "value");
+    verify(listFeed, times(3)).insert(entry);
+    verify(worksheet).setRowCount(2);
+    verify(worksheet).update();
+    verify(listFeed).createEntry();
+    verify(entry, atLeastOnce()).getCustomElements();
+    verifyNoMoreInteractions(entry);
+  }
+
   @Test
   public void testSynchronize_spreadsheetRowNoLongerInData_deleteRow() throws Exception {
     ListEntry entry = makeListEntry(ImmutableMap.of("key", "value"));