diff --git a/core/src/main/java/google/registry/model/UpdateAutoTimestamp.java b/core/src/main/java/google/registry/model/UpdateAutoTimestamp.java index 6e3114ba9..fcd254d7e 100644 --- a/core/src/main/java/google/registry/model/UpdateAutoTimestamp.java +++ b/core/src/main/java/google/registry/model/UpdateAutoTimestamp.java @@ -15,8 +15,12 @@ package google.registry.model; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; +import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm; import static google.registry.util.DateTimeUtils.START_OF_TIME; +import static org.joda.time.DateTimeZone.UTC; +import com.google.common.flogger.FluentLogger; +import com.google.common.flogger.StackSize; import com.googlecode.objectify.annotation.Ignore; import com.googlecode.objectify.annotation.OnLoad; import google.registry.model.translators.UpdateAutoTimestampTranslatorFactory; @@ -40,6 +44,8 @@ import org.joda.time.DateTime; @Embeddable public class UpdateAutoTimestamp extends ImmutableObject { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + // When set to true, database converters/translators should do the auto update. When set to // false, auto update should be suspended (this exists to allow us to preserve the original value // during a replay). @@ -57,7 +63,16 @@ public class UpdateAutoTimestamp extends ImmutableObject { @PrePersist @PreUpdate void setTimestamp() { - if (autoUpdateEnabled() || lastUpdateTime == null) { + // On the off chance that this is called outside of a transaction, log it instead of failing + // with an exception from attempting to call jpaTm().getTransactionTime(), and then fall back + // to DateTime.now(UTC). + if (!jpaTm().inTransaction()) { + logger.atSevere().withStackTrace(StackSize.MEDIUM).log( + "Failed to update automatic timestamp because this wasn't called in a JPA transaction%s.", + ofyTm().inTransaction() ? " (but there is an open Ofy transaction)" : ""); + timestamp = DateTime.now(UTC); + lastUpdateTime = DateTimeUtils.toZonedDateTime(timestamp); + } else if (autoUpdateEnabled() || lastUpdateTime == null) { timestamp = jpaTm().getTransactionTime(); lastUpdateTime = DateTimeUtils.toZonedDateTime(timestamp); } diff --git a/core/src/test/java/google/registry/model/registrar/RegistrarTest.java b/core/src/test/java/google/registry/model/registrar/RegistrarTest.java index 94ad9b3de..0fb78f682 100644 --- a/core/src/test/java/google/registry/model/registrar/RegistrarTest.java +++ b/core/src/test/java/google/registry/model/registrar/RegistrarTest.java @@ -270,8 +270,8 @@ class RegistrarTest extends EntityTestCase { persistSimpleResource( new RegistrarContact.Builder() .setParent(registrar) - .setName("John Abused") - .setEmailAddress("johnabuse@example.com") + .setName("John Abussy") + .setEmailAddress("johnabussy@example.com") .setPhoneNumber("+1.2125551213") .setFaxNumber("+1.2125551213") // No setTypes(...) diff --git a/core/src/test/java/google/registry/schema/replay/SqlEntityTest.java b/core/src/test/java/google/registry/schema/replay/SqlEntityTest.java index 03e155055..0bc61bb05 100644 --- a/core/src/test/java/google/registry/schema/replay/SqlEntityTest.java +++ b/core/src/test/java/google/registry/schema/replay/SqlEntityTest.java @@ -38,7 +38,8 @@ public class SqlEntityTest { final DatastoreEntityExtension datastoreEntityExtension = new DatastoreEntityExtension(); @RegisterExtension - final AppEngineExtension database = new AppEngineExtension.Builder().withCloudSql().build(); + final AppEngineExtension database = + new AppEngineExtension.Builder().withCloudSql().withoutCannedData().build(); @BeforeEach void setup() throws Exception { @@ -54,9 +55,10 @@ public class SqlEntityTest { @Test void getPrimaryKeyString_oneIdColumn() { // AppEngineExtension canned data: Registrar1 - VKey key = Registrar.createVKey("NewRegistrar"); - String expected = "NewRegistrar"; - assertThat(tm().transact(() -> tm().loadByKey(key)).getPrimaryKeyString()).contains(expected); + assertThat( + tm().transact(() -> tm().loadByKey(Registrar.createVKey("NewRegistrar"))) + .getPrimaryKeyString()) + .contains("NewRegistrar"); } @Test diff --git a/core/src/test/java/google/registry/testing/AppEngineExtension.java b/core/src/test/java/google/registry/testing/AppEngineExtension.java index ea71c2c63..ce5954702 100644 --- a/core/src/test/java/google/registry/testing/AppEngineExtension.java +++ b/core/src/test/java/google/registry/testing/AppEngineExtension.java @@ -18,7 +18,7 @@ import static com.google.common.base.Preconditions.checkState; import static com.google.common.io.Files.asCharSink; import static com.google.common.truth.Truth.assertWithMessage; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; -import static google.registry.testing.DatabaseHelper.persistSimpleResources; +import static google.registry.testing.DatabaseHelper.insertSimpleResources; import static google.registry.testing.DualDatabaseTestInvocationContextProvider.injectTmForDualDatabaseTest; import static google.registry.testing.DualDatabaseTestInvocationContextProvider.restoreTmAfterDualDatabaseTest; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; @@ -629,7 +629,7 @@ public final class AppEngineExtension implements BeforeEachCallback, AfterEachCa /** Create some fake registrars. */ public static void loadInitialData() { - persistSimpleResources( + insertSimpleResources( ImmutableList.of( makeRegistrar1(), makeRegistrarContact1(), diff --git a/core/src/test/java/google/registry/testing/DatabaseHelper.java b/core/src/test/java/google/registry/testing/DatabaseHelper.java index 75272ff86..3b613a1cb 100644 --- a/core/src/test/java/google/registry/testing/DatabaseHelper.java +++ b/core/src/test/java/google/registry/testing/DatabaseHelper.java @@ -1205,19 +1205,27 @@ public class DatabaseHelper { * ForeignKeyedEppResources. */ public static ImmutableList persistSimpleResources(final Iterable resources) { + insertSimpleResources(resources); + return transactIfJpaTm(() -> tm().loadByEntities(resources)); + } + + /** + * Like {@link #persistSimpleResources(Iterable)} but without reloading/returning the saved + * entities. + */ + public static void insertSimpleResources(final Iterable resources) { tm().transact( () -> { if (alwaysSaveWithBackup) { - tm().putAll(ImmutableList.copyOf(resources)); + tm().insertAll(ImmutableList.copyOf(resources)); } else { - tm().putAllWithoutBackup(ImmutableList.copyOf(resources)); + tm().insertAllWithoutBackup(ImmutableList.copyOf(resources)); } }); maybeAdvanceClock(); // Force the session to be cleared so that when we read it back, we read from Datastore // and not from the transaction's session cache. tm().clearSessionCache(); - return transactIfJpaTm(() -> tm().loadByEntities(resources)); } public static void deleteResource(final Object resource) {