Ignore read-only when saving commit logs (#1584)

* Ignore read-only when saving commit logs

Ignore read-only when saving commit logs and commit log mutations so that we
can safely replicate in read-only mode.  This should be safe, as we only ever
to the situation of saving commit logs and mutations when something has
already actually been modified in a transaction, meaning that we should have hit
the "read only" sentinel already.

This also introduces the ability to set the Clock in the
TransactionManagerFactory so that we can test this functionality.

* Changes per review

* Fix issues affecting tests

- Restore clobbered async phase in testNoInMigrationState_doesNothing
- Restore system clock to TransactionManagerFactory to avoid affecting other
  tests.

core/src/main/java/google/registry/model/ofy/CommitLogMutation.java

@@ -69,7 +69,7 @@ public class CommitLogMutation extends ImmutableObject implements DatastoreOnlyE
    * converted to a raw Datastore Entity, serialized to bytes, and stored within the mutation.
    */
   public static CommitLogMutation create(Key<CommitLogManifest> parent, Object entity) {
-    return createFromRaw(parent, auditedOfy().save().toEntity(entity));
+    return createFromRaw(parent, auditedOfy().saveIgnoringReadOnlyWithBackup().toEntity(entity));
   }
 
   /**

core/src/main/java/google/registry/model/ofy/CommitLoggedWork.java

@@ -156,7 +156,7 @@ public class CommitLoggedWork<R> implements Runnable {
             .map(entity -> (ImmutableObject) CommitLogMutation.create(manifestKey, entity))
             .collect(toImmutableSet());
     auditedOfy()
-        .saveWithoutBackup()
+        .saveIgnoringReadOnlyWithoutBackup()
         .entities(
             new ImmutableSet.Builder<>()
                 .add(manifest)

core/src/main/java/google/registry/persistence/transaction/TransactionManagerFactory.java

@@ -17,7 +17,6 @@ package google.registry.persistence.transaction;
 import static com.google.common.base.Preconditions.checkState;
 import static google.registry.model.common.DatabaseMigrationStateSchedule.MigrationState.DATASTORE_PRIMARY_NO_ASYNC;
 import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
-import static org.joda.time.DateTimeZone.UTC;
 
 import com.google.appengine.api.utils.SystemProperty;
 import com.google.appengine.api.utils.SystemProperty.Environment.Value;
@@ -30,10 +29,11 @@ import google.registry.model.common.DatabaseMigrationStateSchedule.PrimaryDataba
 import google.registry.model.ofy.DatastoreTransactionManager;
 import google.registry.persistence.DaggerPersistenceComponent;
 import google.registry.tools.RegistryToolEnvironment;
+import google.registry.util.Clock;
 import google.registry.util.NonFinalForTesting;
+import google.registry.util.SystemClock;
 import java.util.Optional;
 import java.util.function.Supplier;
-import org.joda.time.DateTime;
 
 /** Factory class to create {@link TransactionManager} instance. */
 // TODO: Rename this to PersistenceFactory and move to persistence package.
@@ -44,6 +44,9 @@ public final class TransactionManagerFactory {
   /** Optional override to manually set the transaction manager per-test. */
   private static Optional<TransactionManager> tmForTest = Optional.empty();
 
+  /** The current clock (defined as a variable so we can override it in tests) */
+  private static Clock clock = new SystemClock();
+
   /** Supplier for jpaTm so that it is initialized only once, upon first usage. */
   @NonFinalForTesting
   private static Supplier<JpaTransactionManager> jpaTm =
@@ -105,7 +108,7 @@ public final class TransactionManagerFactory {
     if (onBeam) {
       return jpaTm();
     }
-    return DatabaseMigrationStateSchedule.getValueAtTime(DateTime.now(UTC))
+    return DatabaseMigrationStateSchedule.getValueAtTime(clock.nowUtc())
             .getPrimaryDatabase()
             .equals(PrimaryDatabase.DATASTORE)
         ? ofyTm()
@@ -195,7 +198,7 @@ public final class TransactionManagerFactory {
   }
 
   public static void assertNotReadOnlyMode() {
-    if (DatabaseMigrationStateSchedule.getValueAtTime(DateTime.now(UTC)).isReadOnly()) {
+    if (DatabaseMigrationStateSchedule.getValueAtTime(clock.nowUtc()).isReadOnly()) {
       throw new ReadOnlyModeException();
     }
   }
@@ -210,12 +213,18 @@ public final class TransactionManagerFactory {
    */
   @DeleteAfterMigration
   public static void assertAsyncActionsAreAllowed() {
-    if (DatabaseMigrationStateSchedule.getValueAtTime(DateTime.now(UTC))
+    if (DatabaseMigrationStateSchedule.getValueAtTime(clock.nowUtc())
         .equals(DATASTORE_PRIMARY_NO_ASYNC)) {
       throw new ReadOnlyModeException();
     }
   }
 
+  /** Allows us to set the clock used by the factory in unit tests. */
+  @VisibleForTesting
+  public static void setClockForTesting(Clock clock) {
+    TransactionManagerFactory.clock = clock;
+  }
+
   /** Registry is currently undergoing maintenance and is in read-only mode. */
   public static class ReadOnlyModeException extends IllegalStateException {
     public ReadOnlyModeException() {

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

@@ -44,6 +44,7 @@ import google.registry.model.server.Lock;
 import google.registry.persistence.VKey;
 import google.registry.persistence.transaction.JpaTransactionManagerImpl;
 import google.registry.persistence.transaction.TransactionEntity;
+import google.registry.persistence.transaction.TransactionManagerFactory;
 import google.registry.testing.AppEngineExtension;
 import google.registry.testing.DatabaseHelper;
 import google.registry.testing.FakeClock;
@@ -52,6 +53,7 @@ import google.registry.testing.InjectExtension;
 import google.registry.testing.ReplayExtension;
 import google.registry.testing.TestObject;
 import google.registry.util.RequestStatusChecker;
+import google.registry.util.SystemClock;
 import java.lang.reflect.Proxy;
 import java.util.List;
 import java.util.logging.Level;
@@ -105,6 +107,7 @@ public class ReplicateToDatastoreActionTest {
   void tearDown() {
     DatabaseHelper.removeDatabaseMigrationSchedule();
     fakeClock.disableAutoIncrement();
+    TransactionManagerFactory.setClockForTesting(new SystemClock());
   }
 
   @RetryingTest(4)
@@ -392,6 +395,50 @@ public class ReplicateToDatastoreActionTest {
                 + " DATASTORE_PRIMARY.");
   }
 
+  @Test
+  void replicationWorksInReadOnly() {
+
+    // Put us in SQL primary now, readonly in an hour, then in datastore primary after 25 hours.
+    // And we'll need the TransactionManagerFactory to use the fake clock.
+    DateTime now = fakeClock.nowUtc();
+    TransactionManagerFactory.setClockForTesting(fakeClock);
+    jpaTm()
+        .transact(
+            () ->
+                DatabaseMigrationStateSchedule.set(
+                    ImmutableSortedMap.<DateTime, MigrationState>naturalOrder()
+                        .put(START_OF_TIME, MigrationState.DATASTORE_ONLY)
+                        .put(START_OF_TIME.plusHours(1), MigrationState.DATASTORE_PRIMARY)
+                        .put(START_OF_TIME.plusHours(2), MigrationState.DATASTORE_PRIMARY_NO_ASYNC)
+                        .put(START_OF_TIME.plusHours(3), MigrationState.DATASTORE_PRIMARY_READ_ONLY)
+                        .put(START_OF_TIME.plusHours(4), MigrationState.SQL_PRIMARY_READ_ONLY)
+                        .put(START_OF_TIME.plusHours(5), MigrationState.SQL_PRIMARY)
+                        .put(now.plusHours(1), MigrationState.SQL_PRIMARY_READ_ONLY)
+                        .put(now.plusHours(25), MigrationState.DATASTORE_PRIMARY_READ_ONLY)
+                        .put(now.plusHours(26), MigrationState.DATASTORE_PRIMARY_READ_ONLY)
+                        .build()));
+
+    TestObject foo = TestObject.create("foo");
+    insertInDb(foo);
+    TestObject bar = TestObject.create("bar");
+    insertInDb(bar);
+    TestObject baz = TestObject.create("baz");
+    insertInDb(baz);
+    jpaTm().transact(() -> jpaTm().delete(baz.key()));
+
+    // get to read-only
+    fakeClock.advanceBy(Duration.standardDays(1));
+
+    // process the transaction in readonly.
+    action.run();
+
+    // Forward the next day (datastore primary).  Verify that datastore has all of the changes.
+    fakeClock.advanceBy(Duration.standardDays(1));
+    assertThat(ofyTm().transact(() -> ofyTm().loadByKey(foo.key()))).isEqualTo(foo);
+    assertThat(ofyTm().transact(() -> ofyTm().loadByKey(bar.key()))).isEqualTo(bar);
+    assertThat(ofyTm().transact(() -> ofyTm().loadByKeyIfPresent(baz.key()).isPresent())).isFalse();
+  }
+
   @Test
   void testFailure_cannotAcquireLock() {
     assumeTrue(ReplayExtension.replayTestsEnabled());