Clean up ReplicateToDatastoreAction and tests (#1299)

* Clean up ReplicateToDatastoreAction and tests

1. applyTransaction should throw an error if it fails; this allows us to
have more information in the caller (and it shouldn't usually happen)
2. Set a response code + payload now, since this is an action that is
called by cron
3. Add a method to the test log subject that allows us to check if a
severe log with a particular Throwable cause was logged (since the cause
isn't contained in the log message itself directly)

core/src/test/java/google/registry/model/replay/ReplicateToDatastoreActionTest.java

@@ -19,6 +19,7 @@ import static google.registry.persistence.transaction.TransactionManagerFactory.
 import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm;
 import static google.registry.testing.LogsSubject.assertAboutLogs;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
+import static org.junit.Assert.assertThrows;
 
 import com.google.common.base.Suppliers;
 import com.google.common.collect.ImmutableSortedMap;
@@ -149,14 +150,14 @@ public class ReplicateToDatastoreActionTest {
     assertThat(txns2).hasSize(2);
 
     // Apply the first batch.
-    assertThat(task.applyTransaction(txns1.get(0))).isFalse();
+    task.applyTransaction(txns1.get(0));
 
     // Remove the foo record so we can ensure that this transaction doesn't get doublle-played.
     ofyTm().transact(() -> ofyTm().delete(foo.key()));
 
     // Apply the second batch.
     for (TransactionEntity txn : txns2) {
-      assertThat(task.applyTransaction(txn)).isFalse();
+      task.applyTransaction(txn);
     }
 
     // Verify that the first transaction didn't get replayed but the second one did.
@@ -179,12 +180,25 @@ public class ReplicateToDatastoreActionTest {
 
     List<TransactionEntity> txns = task.getTransactionBatch();
     assertThat(txns).hasSize(1);
-    assertThat(task.applyTransaction(txns.get(0))).isTrue();
+    assertThat(assertThrows(IllegalStateException.class, () -> task.applyTransaction(txns.get(0))))
+        .hasMessageThat()
+        .isEqualTo("Missing transaction: last txn id = -1, next available txn = 1");
+  }
+
+  @Test
+  void testMissingTransactions_fullTask() {
+    // Write a transaction (should have a transaction id of 1).
+    TestEntity foo = new TestEntity("foo");
+    jpaTm().transact(() -> jpaTm().insert(foo));
+
+    // Force the last transaction id back to -1 so that we look for transaction 0.
+    ofyTm().transact(() -> ofyTm().insert(new LastSqlTransaction(-1)));
+    task.run();
     assertAboutLogs()
         .that(logHandler)
-        .hasLogAtLevelWithMessage(
-            Level.SEVERE,
-            "Missing transaction: last transaction id = -1, next available transaction = 1");
+        .hasSevereLogWithCause(
+            new IllegalStateException(
+                "Missing transaction: last txn id = -1, next available txn = 1"));
   }
 
   @Test

core/src/test/java/google/registry/testing/LogsSubject.java

@@ -14,6 +14,7 @@
 
 package google.registry.testing;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.truth.Truth.assertAbout;
 import static com.google.common.truth.Truth.assertWithMessage;
 
@@ -26,6 +27,7 @@ import com.google.common.truth.StringSubject;
 import com.google.common.truth.Subject;
 import google.registry.testing.TruthChainer.Which;
 import java.util.List;
+import java.util.Objects;
 import java.util.logging.Handler;
 import java.util.logging.Level;
 import java.util.logging.LogRecord;
@@ -41,7 +43,13 @@ public class LogsSubject extends Subject {
   }
 
   private static final Correspondence<String, String> CONTAINS_CORRESPONDENCE =
-      Correspondence.from((actual, expected) -> actual.contains(expected), "contains");
+      Correspondence.from(String::contains, "contains");
+
+  private static final Correspondence<Throwable, Throwable> THROWABLE_CORRESPONDENCE =
+      Correspondence.from(
+          (t1, t2) ->
+              t1.getClass().equals(t2.getClass()) && t1.getMessage().equals(t2.getMessage()),
+          "throwableEquivalent");
 
   private List<String> getMessagesAtLevel(Level level) {
     ImmutableList.Builder<String> builder = new ImmutableList.Builder<>();
@@ -57,6 +65,19 @@ public class LogsSubject extends Subject {
     check("atLevel(%s)", level).that(getMessagesAtLevel(level)).isEmpty();
   }
 
+  public void hasSevereLogWithCause(Throwable throwable) {
+    ImmutableList<Throwable> actualThrowables =
+        actual.getStoredLogRecords().stream()
+            .filter(record -> record.getLevel().equals(Level.SEVERE))
+            .map(LogRecord::getThrown)
+            .filter(Objects::nonNull)
+            .collect(toImmutableList());
+    check("atSevere")
+        .that(actualThrowables)
+        .comparingElementsUsing(THROWABLE_CORRESPONDENCE)
+        .contains(throwable);
+  }
+
   public Which<StringSubject> hasLogAtLevelWithMessage(Level level, String message) {
     List<String> messagesAtLevel = getMessagesAtLevel(level);
     check("atLevel(%s)", level)

core/src/test/java/google/registry/testing/ReplayExtension.java

@@ -199,10 +199,7 @@ public class ReplayExtension implements BeforeEachCallback, AfterEachCallback {
     do {
       transactionBatch = sqlToDsReplicator.getTransactionBatch();
       for (TransactionEntity txn : transactionBatch) {
-        if (sqlToDsReplicator.applyTransaction(txn)) {
-          throw new RuntimeException(
-              "Error when replaying to Datastore in tests; see logs for more details");
-        }
+        sqlToDsReplicator.applyTransaction(txn);
         if (compare) {
           ofyTm().transact(() -> compareSqlTransaction(txn));
         }