diff --git a/core/src/main/java/google/registry/flows/contact/ContactDeleteFlow.java b/core/src/main/java/google/registry/flows/contact/ContactDeleteFlow.java index d7a0cb371..f79146f08 100644 --- a/core/src/main/java/google/registry/flows/contact/ContactDeleteFlow.java +++ b/core/src/main/java/google/registry/flows/contact/ContactDeleteFlow.java @@ -25,6 +25,7 @@ import static google.registry.model.ResourceTransferUtils.handlePendingTransferO import static google.registry.model.eppoutput.Result.Code.SUCCESS; import static google.registry.model.eppoutput.Result.Code.SUCCESS_WITH_ACTION_PENDING; import static google.registry.model.transfer.TransferStatus.SERVER_CANCELLED; +import static google.registry.persistence.transaction.TransactionManagerFactory.assertAsyncActionsAreAllowed; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.common.collect.ImmutableSet; @@ -94,6 +95,7 @@ public final class ContactDeleteFlow implements TransactionalFlow { extensionManager.register(MetadataExtension.class); validateRegistrarIsLoggedIn(registrarId); extensionManager.validate(); + assertAsyncActionsAreAllowed(); DateTime now = tm().getTransactionTime(); checkLinkedDomains(targetId, now, ContactResource.class, DomainBase::getReferencedContacts); ContactResource existingContact = loadAndVerifyExistence(ContactResource.class, targetId, now); diff --git a/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java b/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java index 67c185f1b..d00125cea 100644 --- a/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java +++ b/core/src/main/java/google/registry/flows/host/HostDeleteFlow.java @@ -22,6 +22,7 @@ import static google.registry.flows.ResourceFlowUtils.verifyResourceOwnership; import static google.registry.flows.host.HostFlowUtils.validateHostName; import static google.registry.model.eppoutput.Result.Code.SUCCESS; import static google.registry.model.eppoutput.Result.Code.SUCCESS_WITH_ACTION_PENDING; +import static google.registry.persistence.transaction.TransactionManagerFactory.assertAsyncActionsAreAllowed; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.google.common.collect.ImmutableSet; @@ -96,6 +97,7 @@ public final class HostDeleteFlow implements TransactionalFlow { extensionManager.register(MetadataExtension.class); validateRegistrarIsLoggedIn(registrarId); extensionManager.validate(); + assertAsyncActionsAreAllowed(); DateTime now = tm().getTransactionTime(); validateHostName(targetId); checkLinkedDomains(targetId, now, HostResource.class, DomainBase::getNameservers); diff --git a/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java b/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java index 6d7566015..a396b564d 100644 --- a/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java +++ b/core/src/main/java/google/registry/flows/host/HostUpdateFlow.java @@ -28,6 +28,7 @@ import static google.registry.flows.host.HostFlowUtils.verifySuperordinateDomain import static google.registry.flows.host.HostFlowUtils.verifySuperordinateDomainOwnership; import static google.registry.model.index.ForeignKeyIndex.loadAndGetKey; import static google.registry.model.reporting.HistoryEntry.Type.HOST_UPDATE; +import static google.registry.persistence.transaction.TransactionManagerFactory.assertAsyncActionsAreAllowed; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.util.CollectionUtils.isNullOrEmpty; @@ -136,6 +137,9 @@ public final class HostUpdateFlow implements TransactionalFlow { validateHostName(targetId); HostResource existingHost = loadAndVerifyExistence(HostResource.class, targetId, now); boolean isHostRename = suppliedNewHostName != null; + if (isHostRename) { + assertAsyncActionsAreAllowed(); + } String oldHostName = targetId; String newHostName = firstNonNull(suppliedNewHostName, oldHostName); DomainBase oldSuperordinateDomain = diff --git a/core/src/main/java/google/registry/model/common/DatabaseMigrationStateSchedule.java b/core/src/main/java/google/registry/model/common/DatabaseMigrationStateSchedule.java index 8bd4f7572..a740a2dc5 100644 --- a/core/src/main/java/google/registry/model/common/DatabaseMigrationStateSchedule.java +++ b/core/src/main/java/google/registry/model/common/DatabaseMigrationStateSchedule.java @@ -30,6 +30,7 @@ import google.registry.model.annotations.DeleteAfterMigration; import google.registry.model.common.TimedTransitionProperty.TimedTransition; import google.registry.model.replay.SqlOnlyEntity; import java.time.Duration; +import java.util.Arrays; import javax.persistence.Entity; import javax.persistence.PersistenceException; import org.joda.time.DateTime; @@ -62,11 +63,28 @@ public class DatabaseMigrationStateSchedule extends CrossTldSingleton implements * not the phase is read-only. */ public enum MigrationState { + /** Datastore is the only DB being used. */ DATASTORE_ONLY(PrimaryDatabase.DATASTORE, false, ReplayDirection.NO_REPLAY), + + /** Datastore is the primary DB, with changes replicated to Cloud SQL. */ DATASTORE_PRIMARY(PrimaryDatabase.DATASTORE, false, ReplayDirection.DATASTORE_TO_SQL), + + /** Datastore is the primary DB, with replication, and async actions are disallowed. */ + DATASTORE_PRIMARY_NO_ASYNC(PrimaryDatabase.DATASTORE, false, ReplayDirection.DATASTORE_TO_SQL), + + /** Datastore is the primary DB, with replication, and all mutating actions are disallowed. */ DATASTORE_PRIMARY_READ_ONLY(PrimaryDatabase.DATASTORE, true, ReplayDirection.DATASTORE_TO_SQL), + + /** + * Cloud SQL is the primary DB, with replication back to Datastore, and all mutating actions are + * disallowed. + */ SQL_PRIMARY_READ_ONLY(PrimaryDatabase.CLOUD_SQL, true, ReplayDirection.SQL_TO_DATASTORE), + + /** Cloud SQL is the primary DB, with changes replicated to Datastore. */ SQL_PRIMARY(PrimaryDatabase.CLOUD_SQL, false, ReplayDirection.SQL_TO_DATASTORE), + + /** Cloud SQL is the only DB being used. */ SQL_ONLY(PrimaryDatabase.CLOUD_SQL, false, ReplayDirection.NO_REPLAY); private final PrimaryDatabase primaryDatabase; @@ -146,11 +164,17 @@ public class DatabaseMigrationStateSchedule extends CrossTldSingleton implements .putAll( MigrationState.DATASTORE_PRIMARY, MigrationState.DATASTORE_ONLY, + MigrationState.DATASTORE_PRIMARY_NO_ASYNC) + .putAll( + MigrationState.DATASTORE_PRIMARY_NO_ASYNC, + MigrationState.DATASTORE_ONLY, + MigrationState.DATASTORE_PRIMARY, MigrationState.DATASTORE_PRIMARY_READ_ONLY) .putAll( MigrationState.DATASTORE_PRIMARY_READ_ONLY, MigrationState.DATASTORE_ONLY, MigrationState.DATASTORE_PRIMARY, + MigrationState.DATASTORE_PRIMARY_NO_ASYNC, MigrationState.SQL_PRIMARY_READ_ONLY, MigrationState.SQL_PRIMARY) .putAll( @@ -165,10 +189,9 @@ public class DatabaseMigrationStateSchedule extends CrossTldSingleton implements MigrationState.SQL_ONLY, MigrationState.SQL_PRIMARY_READ_ONLY, MigrationState.SQL_PRIMARY); + // In addition, we can always transition from a state to itself (useful when updating the map). - for (MigrationState migrationState : MigrationState.values()) { - builder.put(migrationState, migrationState); - } + Arrays.stream(MigrationState.values()).forEach(state -> builder.put(state, state)); return builder.build(); } @@ -246,7 +269,7 @@ public class DatabaseMigrationStateSchedule extends CrossTldSingleton implements * A provided map of transitions may be valid by itself (i.e. it shifts states properly, doesn't * skip states, and doesn't backtrack incorrectly) while still being invalid. In addition to the * transitions in the map being valid, the single transition from the current map at the current - * time to the new map at the current time time must also be valid. + * time to the new map at the current time must also be valid. */ private static void validateTransitionAtCurrentTime( TimedTransitionProperty newTransitions) { diff --git a/core/src/main/java/google/registry/persistence/transaction/TransactionManagerFactory.java b/core/src/main/java/google/registry/persistence/transaction/TransactionManagerFactory.java index 891748d96..41572a105 100644 --- a/core/src/main/java/google/registry/persistence/transaction/TransactionManagerFactory.java +++ b/core/src/main/java/google/registry/persistence/transaction/TransactionManagerFactory.java @@ -15,6 +15,7 @@ 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; @@ -23,6 +24,7 @@ import com.google.appengine.api.utils.SystemProperty.Environment.Value; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Suppliers; import google.registry.config.RegistryEnvironment; +import google.registry.model.annotations.DeleteAfterMigration; import google.registry.model.common.DatabaseMigrationStateSchedule; import google.registry.model.common.DatabaseMigrationStateSchedule.PrimaryDatabase; import google.registry.model.ofy.DatastoreTransactionManager; @@ -198,6 +200,22 @@ public final class TransactionManagerFactory { } } + /** + * Asserts that async actions (contact/host deletes and host renames) are allowed. + * + *

These are allowed at all times except during the {@link + * DatabaseMigrationStateSchedule.MigrationState#DATASTORE_PRIMARY_NO_ASYNC} stage. Note that + * {@link ReadOnlyModeException} may well be thrown during other read-only stages inside the + * transaction manager; this method specifically checks only async actions. + */ + @DeleteAfterMigration + public static void assertAsyncActionsAreAllowed() { + if (DatabaseMigrationStateSchedule.getValueAtTime(DateTime.now(UTC)) + .equals(DATASTORE_PRIMARY_NO_ASYNC)) { + throw new ReadOnlyModeException(); + } + } + /** Registry is currently undergoing maintenance and is in read-only mode. */ public static class ReadOnlyModeException extends IllegalStateException { public ReadOnlyModeException() { diff --git a/core/src/test/java/google/registry/flows/contact/ContactDeleteFlowTest.java b/core/src/test/java/google/registry/flows/contact/ContactDeleteFlowTest.java index f908ffade..9745ca68a 100644 --- a/core/src/test/java/google/registry/flows/contact/ContactDeleteFlowTest.java +++ b/core/src/test/java/google/registry/flows/contact/ContactDeleteFlowTest.java @@ -254,6 +254,15 @@ class ContactDeleteFlowTest extends ResourceFlowTestCasenaturalOrder() .put(START_OF_TIME, DATASTORE_ONLY) .put(startTime.plusHours(1), DATASTORE_PRIMARY) - .put(startTime.plusHours(2), DATASTORE_PRIMARY_READ_ONLY) + .put(startTime.plusHours(2), DATASTORE_PRIMARY_NO_ASYNC) + .put(startTime.plusHours(3), DATASTORE_PRIMARY_READ_ONLY) .build(); assertThat( assertThrows( @@ -163,7 +168,8 @@ public class DatabaseMigrationStateScheduleTest extends EntityTestCase { fakeClock.setTo(START_OF_TIME.plusDays(1)); AllocationToken token = new AllocationToken.Builder().setToken("token").setTokenType(TokenType.SINGLE_USE).build(); - runValidTransition(DATASTORE_PRIMARY, DATASTORE_PRIMARY_READ_ONLY); + runValidTransition(DATASTORE_PRIMARY, DATASTORE_PRIMARY_NO_ASYNC); + runValidTransition(DATASTORE_PRIMARY_NO_ASYNC, DATASTORE_PRIMARY_READ_ONLY); assertThrows(ReadOnlyModeException.class, () -> persistResource(token)); runValidTransition(DATASTORE_PRIMARY_READ_ONLY, SQL_PRIMARY_READ_ONLY); assertThrows(ReadOnlyModeException.class, () -> persistResource(token)); diff --git a/core/src/test/java/google/registry/model/replay/ReplicateToDatastoreActionTest.java b/core/src/test/java/google/registry/model/replay/ReplicateToDatastoreActionTest.java index 23aa77e2f..509ce63da 100644 --- a/core/src/test/java/google/registry/model/replay/ReplicateToDatastoreActionTest.java +++ b/core/src/test/java/google/registry/model/replay/ReplicateToDatastoreActionTest.java @@ -366,8 +366,9 @@ public class ReplicateToDatastoreActionTest { ImmutableSortedMap.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_READ_ONLY) - .put(START_OF_TIME.plusHours(3), MigrationState.SQL_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) .put(now.plusHours(1), MigrationState.SQL_PRIMARY_READ_ONLY) .put(now.plusHours(2), MigrationState.DATASTORE_PRIMARY_READ_ONLY) .put(now.plusHours(3), MigrationState.DATASTORE_PRIMARY) diff --git a/core/src/test/java/google/registry/persistence/converter/DatabaseMigrationScheduleTransitionConverterTest.java b/core/src/test/java/google/registry/persistence/converter/DatabaseMigrationScheduleTransitionConverterTest.java index 4531aca7b..85f268ad0 100644 --- a/core/src/test/java/google/registry/persistence/converter/DatabaseMigrationScheduleTransitionConverterTest.java +++ b/core/src/test/java/google/registry/persistence/converter/DatabaseMigrationScheduleTransitionConverterTest.java @@ -47,7 +47,9 @@ public class DatabaseMigrationScheduleTransitionConverterTest { MigrationState.DATASTORE_ONLY, DateTime.parse("2001-01-01T00:00:00.0Z"), MigrationState.DATASTORE_PRIMARY, - DateTime.parse("2002-01-01T00:00:00.0Z"), + DateTime.parse("2002-01-01T01:00:00.0Z"), + MigrationState.DATASTORE_PRIMARY_NO_ASYNC, + DateTime.parse("2002-01-01T02:00:00.0Z"), MigrationState.DATASTORE_PRIMARY_READ_ONLY, DateTime.parse("2002-01-02T00:00:00.0Z"), MigrationState.SQL_PRIMARY, diff --git a/core/src/test/java/google/registry/testing/DatabaseHelper.java b/core/src/test/java/google/registry/testing/DatabaseHelper.java index 753c3bd99..2e6c9a7b9 100644 --- a/core/src/test/java/google/registry/testing/DatabaseHelper.java +++ b/core/src/test/java/google/registry/testing/DatabaseHelper.java @@ -1429,6 +1429,33 @@ public class DatabaseHelper { return entity; } + /** + * Sets a DATASTORE_PRIMARY_NO_ASYNC state on the {@link DatabaseMigrationStateSchedule}. + * + *

In order to allow for tests to manipulate the clock how they need, we start the transitions + * one millisecond after the clock's current time (in case the clock's current value is + * START_OF_TIME). We then advance the clock one second so that we're in the + * DATASTORE_PRIMARY_READ_ONLY phase. + * + *

We must use the current time, otherwise the setting of the migration state will fail due to + * an invalid transition. + */ + public static void setMigrationScheduleToDatastorePrimaryNoAsync(FakeClock fakeClock) { + DateTime now = fakeClock.nowUtc(); + jpaTm() + .transact( + () -> + DatabaseMigrationStateSchedule.set( + ImmutableSortedMap.of( + START_OF_TIME, + MigrationState.DATASTORE_ONLY, + now.plusMillis(1), + MigrationState.DATASTORE_PRIMARY, + now.plusMillis(2), + MigrationState.DATASTORE_PRIMARY_NO_ASYNC))); + fakeClock.advanceBy(Duration.standardSeconds(1)); + } + /** * Sets a DATASTORE_PRIMARY_READ_ONLY state on the {@link DatabaseMigrationStateSchedule}. * @@ -1452,6 +1479,8 @@ public class DatabaseHelper { now.plusMillis(1), MigrationState.DATASTORE_PRIMARY, now.plusMillis(2), + MigrationState.DATASTORE_PRIMARY_NO_ASYNC, + now.plusMillis(3), MigrationState.DATASTORE_PRIMARY_READ_ONLY))); fakeClock.advanceBy(Duration.standardSeconds(1)); } @@ -1478,8 +1507,10 @@ public class DatabaseHelper { now.plusMillis(1), MigrationState.DATASTORE_PRIMARY, now.plusMillis(2), - MigrationState.DATASTORE_PRIMARY_READ_ONLY, + MigrationState.DATASTORE_PRIMARY_NO_ASYNC, now.plusMillis(3), + MigrationState.DATASTORE_PRIMARY_READ_ONLY, + now.plusMillis(4), MigrationState.SQL_PRIMARY))); fakeClock.advanceBy(Duration.standardSeconds(1)); } diff --git a/core/src/test/java/google/registry/tools/GetDatabaseMigrationStateCommandTest.java b/core/src/test/java/google/registry/tools/GetDatabaseMigrationStateCommandTest.java index 88ae00f4e..7b3a910bc 100644 --- a/core/src/test/java/google/registry/tools/GetDatabaseMigrationStateCommandTest.java +++ b/core/src/test/java/google/registry/tools/GetDatabaseMigrationStateCommandTest.java @@ -54,10 +54,12 @@ public class GetDatabaseMigrationStateCommandTest now.plusHours(1), MigrationState.DATASTORE_PRIMARY, now.plusHours(2), - MigrationState.DATASTORE_PRIMARY_READ_ONLY, + MigrationState.DATASTORE_PRIMARY_NO_ASYNC, now.plusHours(3), - MigrationState.SQL_PRIMARY, + MigrationState.DATASTORE_PRIMARY_READ_ONLY, now.plusHours(4), + MigrationState.SQL_PRIMARY, + now.plusHours(5), MigrationState.SQL_ONLY); jpaTm().transact(() -> DatabaseMigrationStateSchedule.set(transitions)); runCommand(); diff --git a/core/src/test/java/google/registry/tools/SetDatabaseMigrationStateCommandTest.java b/core/src/test/java/google/registry/tools/SetDatabaseMigrationStateCommandTest.java index 42eb2fca6..e77f8faee 100644 --- a/core/src/test/java/google/registry/tools/SetDatabaseMigrationStateCommandTest.java +++ b/core/src/test/java/google/registry/tools/SetDatabaseMigrationStateCommandTest.java @@ -64,14 +64,21 @@ public class SetDatabaseMigrationStateCommandTest void testSuccess_fullSchedule() throws Exception { DateTime now = fakeClock.nowUtc(); DateTime datastorePrimary = now.plusHours(1); - DateTime datastorePrimaryReadOnly = now.plusHours(2); - DateTime sqlPrimary = now.plusHours(3); - DateTime sqlOnly = now.plusHours(4); + DateTime datastorePrimaryNoAsync = now.plusHours(2); + DateTime datastorePrimaryReadOnly = now.plusHours(3); + DateTime sqlPrimary = now.plusHours(4); + DateTime sqlOnly = now.plusHours(5); runCommandForced( String.format( "--migration_schedule=%s=DATASTORE_ONLY,%s=DATASTORE_PRIMARY," - + "%s=DATASTORE_PRIMARY_READ_ONLY,%s=SQL_PRIMARY,%s=SQL_ONLY", - START_OF_TIME, datastorePrimary, datastorePrimaryReadOnly, sqlPrimary, sqlOnly)); + + "%s=DATASTORE_PRIMARY_NO_ASYNC,%s=DATASTORE_PRIMARY_READ_ONLY," + + "%s=SQL_PRIMARY,%s=SQL_ONLY", + START_OF_TIME, + datastorePrimary, + datastorePrimaryNoAsync, + datastorePrimaryReadOnly, + sqlPrimary, + sqlOnly)); assertThat(DatabaseMigrationStateSchedule.get().toValueMap()) .containsExactlyEntriesIn( ImmutableSortedMap.of( @@ -79,6 +86,8 @@ public class SetDatabaseMigrationStateCommandTest MigrationState.DATASTORE_ONLY, datastorePrimary, MigrationState.DATASTORE_PRIMARY, + datastorePrimaryNoAsync, + MigrationState.DATASTORE_PRIMARY_NO_ASYNC, datastorePrimaryReadOnly, MigrationState.DATASTORE_PRIMARY_READ_ONLY, sqlPrimary, @@ -110,8 +119,9 @@ public class SetDatabaseMigrationStateCommandTest runCommandForced( String.format( "--migration_schedule=%s=DATASTORE_ONLY,%s=DATASTORE_PRIMARY," - + "%s=DATASTORE_PRIMARY_READ_ONLY,%s=DATASTORE_PRIMARY", - START_OF_TIME, now.plusHours(1), now.plusHours(2), now.plusHours(3))); + + "%s=DATASTORE_PRIMARY_NO_ASYNC,%s=DATASTORE_PRIMARY_READ_ONLY," + + "%s=DATASTORE_PRIMARY", + START_OF_TIME, now.plusHours(1), now.plusHours(2), now.plusHours(3), now.plusHours(4))); assertThat(DatabaseMigrationStateSchedule.get().toValueMap()) .containsExactlyEntriesIn( ImmutableSortedMap.of( @@ -120,8 +130,10 @@ public class SetDatabaseMigrationStateCommandTest now.plusHours(1), MigrationState.DATASTORE_PRIMARY, now.plusHours(2), - MigrationState.DATASTORE_PRIMARY_READ_ONLY, + MigrationState.DATASTORE_PRIMARY_NO_ASYNC, now.plusHours(3), + MigrationState.DATASTORE_PRIMARY_READ_ONLY, + now.plusHours(4), MigrationState.DATASTORE_PRIMARY)); } @@ -152,9 +164,12 @@ public class SetDatabaseMigrationStateCommandTest () -> runCommandForced( String.format( - "--migration_schedule=%s=DATASTORE_ONLY," - + "%s=DATASTORE_PRIMARY,%s=DATASTORE_PRIMARY_READ_ONLY", - START_OF_TIME, now.minusHours(2), now.minusHours(1))))) + "--migration_schedule=%s=DATASTORE_ONLY,%s=DATASTORE_PRIMARY," + + "%s=DATASTORE_PRIMARY_NO_ASYNC,%s=DATASTORE_PRIMARY_READ_ONLY", + START_OF_TIME, + now.minusHours(3), + now.minusHours(2), + now.minusHours(1))))) .hasMessageThat() .isEqualTo( "Cannot transition from current state-as-of-now DATASTORE_ONLY " diff --git a/core/src/test/resources/google/registry/flows/domain/domain_create_response_noasync.xml b/core/src/test/resources/google/registry/flows/domain/domain_create_response_noasync.xml new file mode 100644 index 000000000..1317779fa --- /dev/null +++ b/core/src/test/resources/google/registry/flows/domain/domain_create_response_noasync.xml @@ -0,0 +1,19 @@ + + + + Command completed successfully + + + + %DOMAIN% + 1999-04-03T22:00:01.0Z + 2001-04-03T22:00:01.0Z + + + + ABC-12345 + server-trid + + +