diff --git a/core/src/main/java/google/registry/model/DatabaseMigrationUtils.java b/core/src/main/java/google/registry/model/DatabaseMigrationUtils.java new file mode 100644 index 000000000..6b13b0397 --- /dev/null +++ b/core/src/main/java/google/registry/model/DatabaseMigrationUtils.java @@ -0,0 +1,38 @@ +// Copyright 2020 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.model; + +import com.google.common.flogger.FluentLogger; +import google.registry.config.RegistryEnvironment; + +/** Utility methods related to migrating dual-read/dual-write entities. */ +public class DatabaseMigrationUtils { + + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + + /** Throws exceptions only in unit tests, otherwise only logs exceptions. */ + public static void suppressExceptionUnlessInTest(Runnable work, String message) { + try { + work.run(); + } catch (Exception e) { + if (RegistryEnvironment.get().equals(RegistryEnvironment.UNITTEST)) { + throw e; + } + logger.atWarning().withCause(e).log(message); + } + } + + private DatabaseMigrationUtils() {} +} diff --git a/core/src/main/java/google/registry/model/smd/SignedMarkRevocationList.java b/core/src/main/java/google/registry/model/smd/SignedMarkRevocationList.java index e37ea1c3a..5a2528958 100644 --- a/core/src/main/java/google/registry/model/smd/SignedMarkRevocationList.java +++ b/core/src/main/java/google/registry/model/smd/SignedMarkRevocationList.java @@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.Iterables.isEmpty; import static google.registry.model.CacheUtils.memoizeWithShortExpiration; +import static google.registry.model.DatabaseMigrationUtils.suppressExceptionUnlessInTest; import static google.registry.model.common.EntityGroupRoot.getCrossTldKey; import static google.registry.model.ofy.ObjectifyService.allocateId; import static google.registry.model.ofy.ObjectifyService.ofy; @@ -32,7 +33,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.common.collect.MapDifference; import com.google.common.collect.Maps; -import com.google.common.flogger.FluentLogger; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.EmbedMap; import com.googlecode.objectify.annotation.Entity; @@ -82,8 +82,6 @@ import org.joda.time.DateTime; @NotBackedUp(reason = Reason.EXTERNALLY_SOURCED) public class SignedMarkRevocationList extends ImmutableObject implements NonReplicatedEntity { - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - @VisibleForTesting static final int SHARD_SIZE = 10000; /** Common ancestor for queries. */ @@ -121,12 +119,11 @@ public class SignedMarkRevocationList extends ImmutableObject implements NonRepl memoizeWithShortExpiration( () -> { SignedMarkRevocationList datastoreList = loadFromDatastore(); - // Also load the list from Cloud SQL, compare the two lists, and log if different. - try { - loadAndCompareCloudSqlList(datastoreList); - } catch (Throwable t) { - logger.atSevere().withCause(t).log("Error comparing signed mark revocation lists."); - } + suppressExceptionUnlessInTest( + () -> { + loadAndCompareCloudSqlList(datastoreList); + }, + "Error comparing signed mark revocation lists."); return datastoreList; }); @@ -229,11 +226,12 @@ public class SignedMarkRevocationList extends ImmutableObject implements NonRepl Maps.difference(datastoreList.revokes, cloudSqlList.revokes); if (!diff.areEqual()) { if (diff.entriesDiffering().size() > 10) { - logger.atWarning().log( + String message = String.format( "Unequal SM revocation lists detected, Cloud SQL list with revision id %d has %d" + " different records than the current Datastore list.", - cloudSqlList.revisionId, diff.entriesDiffering().size())); + cloudSqlList.revisionId, diff.entriesDiffering().size()); + throw new RuntimeException(message); } else { StringBuilder diffMessage = new StringBuilder("Unequal SM revocation lists detected:\n"); diff.entriesDiffering() @@ -243,11 +241,13 @@ public class SignedMarkRevocationList extends ImmutableObject implements NonRepl String.format( "SMD %s has key %s in Datastore and key %s in Cloud SQL.\n", label, valueDiff.leftValue(), valueDiff.rightValue()))); - logger.atWarning().log(diffMessage.toString()); + throw new RuntimeException(diffMessage.toString()); } } } else { - logger.atWarning().log("Signed mark revocation list in Cloud SQL is empty."); + if (datastoreList.size() != 0) { + throw new RuntimeException("Signed mark revocation list in Cloud SQL is empty."); + } } } diff --git a/core/src/main/java/google/registry/model/smd/SignedMarkRevocationListDao.java b/core/src/main/java/google/registry/model/smd/SignedMarkRevocationListDao.java index 2bc7e2e30..45f1284cc 100644 --- a/core/src/main/java/google/registry/model/smd/SignedMarkRevocationListDao.java +++ b/core/src/main/java/google/registry/model/smd/SignedMarkRevocationListDao.java @@ -15,6 +15,7 @@ package google.registry.model.smd; import static google.registry.model.CacheUtils.memoizeWithShortExpiration; +import static google.registry.model.DatabaseMigrationUtils.suppressExceptionUnlessInTest; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import com.google.common.base.Supplier; @@ -60,14 +61,14 @@ public class SignedMarkRevocationListDao { * the authoritative database. */ static void trySave(SignedMarkRevocationList signedMarkRevocationList) { - try { - SignedMarkRevocationListDao.save(signedMarkRevocationList); - logger.atInfo().log( - "Inserted %,d signed mark revocations into Cloud SQL", - signedMarkRevocationList.revokes.size()); - } catch (Throwable e) { - logger.atSevere().withCause(e).log("Error inserting signed mark revocations into Cloud SQL"); - } + suppressExceptionUnlessInTest( + () -> { + SignedMarkRevocationListDao.save(signedMarkRevocationList); + logger.atInfo().log( + "Inserted %,d signed mark revocations into Cloud SQL.", + signedMarkRevocationList.revokes.size()); + }, + "Error inserting signed mark revocations into Cloud SQL."); } private static void save(SignedMarkRevocationList signedMarkRevocationList) { diff --git a/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListDaoTest.java b/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListDaoTest.java index 3870137d1..e84ab1f7e 100644 --- a/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListDaoTest.java +++ b/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListDaoTest.java @@ -50,21 +50,6 @@ public class SignedMarkRevocationListDaoTest { assertAboutImmutableObjects().that(fromDb).isEqualExceptFields(list); } - @Test - void trySave_failureIsSwallowed() { - SignedMarkRevocationList list = - SignedMarkRevocationList.create( - fakeClock.nowUtc(), ImmutableMap.of("mark", fakeClock.nowUtc().minusHours(1))); - SignedMarkRevocationListDao.trySave(list); - SignedMarkRevocationList fromDb = SignedMarkRevocationListDao.getLatestRevision().get(); - assertAboutImmutableObjects().that(fromDb).isEqualExceptFields(list); - - // This should throw an exception, which is swallowed and nothing changed - SignedMarkRevocationListDao.trySave(list); - SignedMarkRevocationList secondFromDb = SignedMarkRevocationListDao.getLatestRevision().get(); - assertAboutImmutableObjects().that(secondFromDb).isEqualExceptFields(fromDb); - } - @Test void testRetrieval_notPresent() { assertThat(SignedMarkRevocationListDao.getLatestRevision().isPresent()).isFalse(); diff --git a/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListTest.java b/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListTest.java index e4fdde32f..3b55bf5bc 100644 --- a/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListTest.java +++ b/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.smd.SignedMarkRevocationList.SHARD_SIZE; +import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.util.DateTimeUtils.START_OF_TIME; import static org.joda.time.Duration.standardDays; @@ -138,6 +139,33 @@ public class SignedMarkRevocationListTest { .isEqualTo(DateTime.parse("2000-01-01T00:00:00Z")); } + @Test + void test_getCreationTime_missingInCloudSQL() { + clock.setTo(DateTime.parse("2000-01-01T00:00:00Z")); + createSaveGetHelper(1); + jpaTm().transact(() -> jpaTm().delete(SignedMarkRevocationListDao.getLatestRevision().get())); + RuntimeException thrown = + assertThrows(RuntimeException.class, () -> SignedMarkRevocationList.get()); + assertThat(thrown) + .hasMessageThat() + .isEqualTo("Signed mark revocation list in Cloud SQL is empty."); + } + + @Test + void test_getCreationTime_unequalListsInDatabases() { + clock.setTo(DateTime.parse("2000-01-01T00:00:00Z")); + createSaveGetHelper(1); + ImmutableMap.Builder revokes = new ImmutableMap.Builder<>(); + for (int i = 0; i < 3; i++) { + revokes.put(Integer.toString(i), clock.nowUtc()); + } + SignedMarkRevocationListDao.trySave( + SignedMarkRevocationList.create(clock.nowUtc(), revokes.build())); + RuntimeException thrown = + assertThrows(RuntimeException.class, () -> SignedMarkRevocationList.get()); + assertThat(thrown).hasMessageThat().contains("Unequal SM revocation lists detected:"); + } + @Test void test_isSmdRevoked_present() { final int rows = SHARD_SIZE + 1;