diff --git a/core/src/main/java/google/registry/model/CreateAutoTimestamp.java b/core/src/main/java/google/registry/model/CreateAutoTimestamp.java index 606e3a13e..9aece0d27 100644 --- a/core/src/main/java/google/registry/model/CreateAutoTimestamp.java +++ b/core/src/main/java/google/registry/model/CreateAutoTimestamp.java @@ -14,8 +14,20 @@ package google.registry.model; +import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; + +import com.googlecode.objectify.annotation.Ignore; +import com.googlecode.objectify.annotation.OnLoad; import google.registry.model.translators.CreateAutoTimestampTranslatorFactory; +import google.registry.util.DateTimeUtils; +import java.time.ZonedDateTime; import javax.annotation.Nullable; +import javax.persistence.Column; +import javax.persistence.Embeddable; +import javax.persistence.PostLoad; +import javax.persistence.PrePersist; +import javax.persistence.PreUpdate; +import javax.persistence.Transient; import org.joda.time.DateTime; /** @@ -23,9 +35,37 @@ import org.joda.time.DateTime; * * @see CreateAutoTimestampTranslatorFactory */ +@Embeddable public class CreateAutoTimestamp extends ImmutableObject implements UnsafeSerializable { - DateTime timestamp; + @Transient DateTime timestamp; + + @Column(nullable = false) + @Ignore + ZonedDateTime creationTime; + + @PrePersist + @PreUpdate + void setTimestamp() { + if (creationTime == null) { + timestamp = jpaTm().getTransactionTime(); + creationTime = DateTimeUtils.toZonedDateTime(timestamp); + } + } + + @OnLoad + void onLoad() { + if (timestamp != null) { + creationTime = DateTimeUtils.toZonedDateTime(timestamp); + } + } + + @PostLoad + void postLoad() { + if (creationTime != null) { + timestamp = DateTimeUtils.toJodaDateTime(creationTime); + } + } /** Returns the timestamp. */ @Nullable @@ -36,6 +76,7 @@ public class CreateAutoTimestamp extends ImmutableObject implements UnsafeSerial public static CreateAutoTimestamp create(@Nullable DateTime timestamp) { CreateAutoTimestamp instance = new CreateAutoTimestamp(); instance.timestamp = timestamp; + instance.creationTime = (timestamp == null) ? null : DateTimeUtils.toZonedDateTime(timestamp); return instance; } } diff --git a/core/src/main/java/google/registry/model/EppResource.java b/core/src/main/java/google/registry/model/EppResource.java index 79967a220..a084ad497 100644 --- a/core/src/main/java/google/registry/model/EppResource.java +++ b/core/src/main/java/google/registry/model/EppResource.java @@ -50,6 +50,8 @@ import java.util.Set; import java.util.concurrent.ExecutionException; import javax.persistence.Access; import javax.persistence.AccessType; +import javax.persistence.AttributeOverride; +import javax.persistence.AttributeOverrides; import javax.persistence.Column; import javax.persistence.MappedSuperclass; import javax.persistence.Transient; @@ -112,7 +114,9 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable { *

This can be null in the case of pre-Registry-3.0-migration history objects with null * resource fields. */ - @Index CreateAutoTimestamp creationTime = CreateAutoTimestamp.create(null); + @AttributeOverrides({@AttributeOverride(name = "creationTime", column = @Column())}) + @Index + CreateAutoTimestamp creationTime = CreateAutoTimestamp.create(null); /** * The time when this resource was or will be deleted. diff --git a/core/src/main/java/google/registry/model/domain/RegistryLock.java b/core/src/main/java/google/registry/model/domain/RegistryLock.java index 43856c304..cf1adb511 100644 --- a/core/src/main/java/google/registry/model/domain/RegistryLock.java +++ b/core/src/main/java/google/registry/model/domain/RegistryLock.java @@ -28,6 +28,8 @@ import google.registry.util.DateTimeUtils; import java.time.ZonedDateTime; import java.util.Optional; import javax.annotation.Nullable; +import javax.persistence.AttributeOverride; +import javax.persistence.AttributeOverrides; import javax.persistence.Column; import javax.persistence.Entity; import javax.persistence.FetchType; @@ -100,7 +102,11 @@ public final class RegistryLock extends ImmutableObject implements Buildable, Sq private String registrarPocId; /** When the lock is first requested. */ - @Column(nullable = false) + @AttributeOverrides({ + @AttributeOverride( + name = "creationTime", + column = @Column(name = "lockRequestTime", nullable = false)) + }) private CreateAutoTimestamp lockRequestTime = CreateAutoTimestamp.create(null); /** When the unlock is first requested. */ diff --git a/core/src/main/java/google/registry/model/domain/token/AllocationToken.java b/core/src/main/java/google/registry/model/domain/token/AllocationToken.java index e3f20b028..09fe23746 100644 --- a/core/src/main/java/google/registry/model/domain/token/AllocationToken.java +++ b/core/src/main/java/google/registry/model/domain/token/AllocationToken.java @@ -122,7 +122,6 @@ public class AllocationToken extends BackupGroupRoot implements Buildable, Datas @Nullable @Index String domainName; /** When this token was created. */ - @Column(nullable = false) CreateAutoTimestamp creationTime = CreateAutoTimestamp.create(null); /** Allowed registrar client IDs for this token, or null if all registrars are allowed. */ diff --git a/core/src/main/java/google/registry/model/tmch/ClaimsList.java b/core/src/main/java/google/registry/model/tmch/ClaimsList.java index 644c0298b..37210eb4e 100644 --- a/core/src/main/java/google/registry/model/tmch/ClaimsList.java +++ b/core/src/main/java/google/registry/model/tmch/ClaimsList.java @@ -27,6 +27,8 @@ import google.registry.model.replay.SqlOnlyEntity; import google.registry.model.tld.label.ReservedList.ReservedListEntry; import java.util.Map; import java.util.Optional; +import javax.persistence.AttributeOverride; +import javax.persistence.AttributeOverrides; import javax.persistence.Column; import javax.persistence.Entity; import javax.persistence.GeneratedValue; @@ -56,7 +58,11 @@ public class ClaimsList extends ImmutableObject implements SqlOnlyEntity { @GeneratedValue(strategy = GenerationType.IDENTITY) Long revisionId; - @Column(nullable = false) + @AttributeOverrides({ + @AttributeOverride( + name = "creationTime", + column = @Column(name = "creationTimestamp", nullable = false)) + }) CreateAutoTimestamp creationTimestamp = CreateAutoTimestamp.create(null); /** diff --git a/core/src/main/java/google/registry/persistence/converter/CreateAutoTimestampConverter.java b/core/src/main/java/google/registry/persistence/converter/CreateAutoTimestampConverter.java deleted file mode 100644 index b70605c27..000000000 --- a/core/src/main/java/google/registry/persistence/converter/CreateAutoTimestampConverter.java +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright 2019 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.persistence.converter; - -import static com.google.common.base.MoreObjects.firstNonNull; -import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; - -import google.registry.model.CreateAutoTimestamp; -import google.registry.util.DateTimeUtils; -import java.sql.Timestamp; -import java.time.ZoneOffset; -import java.time.ZonedDateTime; -import javax.annotation.Nullable; -import javax.persistence.AttributeConverter; -import javax.persistence.Converter; -import org.joda.time.DateTime; - -/** JPA converter to for storing/retrieving CreateAutoTimestamp objects. */ -@Converter(autoApply = true) -public class CreateAutoTimestampConverter - implements AttributeConverter { - - @Override - @Nullable - public Timestamp convertToDatabaseColumn(@Nullable CreateAutoTimestamp entity) { - if (entity == null) { - return null; - } - DateTime dateTime = firstNonNull(entity.getTimestamp(), jpaTm().getTransactionTime()); - return Timestamp.from(DateTimeUtils.toZonedDateTime(dateTime).toInstant()); - } - - @Override - @Nullable - public CreateAutoTimestamp convertToEntityAttribute(@Nullable Timestamp columnValue) { - if (columnValue == null) { - return null; - } - ZonedDateTime zdt = ZonedDateTime.ofInstant(columnValue.toInstant(), ZoneOffset.UTC); - return CreateAutoTimestamp.create(DateTimeUtils.toJodaDateTime(zdt)); - } -} diff --git a/core/src/main/resources/META-INF/persistence.xml b/core/src/main/resources/META-INF/persistence.xml index 449423cbf..8bdc87cec 100644 --- a/core/src/main/resources/META-INF/persistence.xml +++ b/core/src/main/resources/META-INF/persistence.xml @@ -83,7 +83,6 @@ google.registry.persistence.converter.BillingEventFlagSetConverter google.registry.persistence.converter.BloomFilterConverter google.registry.persistence.converter.CidrAddressBlockListConverter - google.registry.persistence.converter.CreateAutoTimestampConverter google.registry.persistence.converter.CurrencyToBillingConverter google.registry.persistence.converter.CurrencyUnitConverter google.registry.persistence.converter.DatabaseMigrationScheduleTransitionConverter diff --git a/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java b/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java index 578ceb508..169c3ec29 100644 --- a/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java +++ b/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java @@ -657,20 +657,18 @@ public class DomainBaseSqlTest { } private void assertEqualDomainExcept(DomainBase thatDomain, String... excepts) { - // Fix the original creation timestamp (this gets initialized on first write) - DomainBase org = domain.asBuilder().setCreationTime(thatDomain.getCreationTime()).build(); - ImmutableList moreExcepts = new ImmutableList.Builder() .addAll(Arrays.asList(excepts)) + .add("creationTime") .add("updateTimestamp") .add("transferData") .build(); // Note that the equality comparison forces a lazy load of all fields. - assertAboutImmutableObjects().that(thatDomain).isEqualExceptFields(org, moreExcepts); + assertAboutImmutableObjects().that(thatDomain).isEqualExceptFields(domain, moreExcepts); // Transfer data cannot be directly compared due to serverApproveEtities inequalities assertAboutImmutableObjects() .that(domain.getTransferData()) - .isEqualExceptFields(org.getTransferData(), "serverApproveEntities"); + .isEqualExceptFields(thatDomain.getTransferData(), "serverApproveEntities"); } } 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 3b6366a69..630820193 100644 --- a/core/src/test/java/google/registry/model/replay/ReplicateToDatastoreActionTest.java +++ b/core/src/test/java/google/registry/model/replay/ReplicateToDatastoreActionTest.java @@ -24,6 +24,7 @@ import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT; import static javax.servlet.http.HttpServletResponse.SC_OK; import static org.junit.Assert.assertThrows; +import static org.junit.jupiter.api.Assumptions.assumeTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -33,6 +34,8 @@ import com.google.common.testing.TestLogHandler; import com.google.common.truth.Truth8; import google.registry.model.common.DatabaseMigrationStateSchedule; import google.registry.model.common.DatabaseMigrationStateSchedule.MigrationState; +import google.registry.model.domain.token.AllocationToken; +import google.registry.model.domain.token.AllocationToken.TokenType; import google.registry.model.ofy.CommitLogBucket; import google.registry.model.ofy.Ofy; import google.registry.model.server.Lock; @@ -64,8 +67,8 @@ public class ReplicateToDatastoreActionTest { public final AppEngineExtension appEngine = AppEngineExtension.builder() .withDatastoreAndCloudSql() - .withOfyTestEntities(Lock.class, TestObject.class) - .withJpaUnitTestEntities(Lock.class, TestObject.class) + .withOfyTestEntities(AllocationToken.class, Lock.class, TestObject.class) + .withJpaUnitTestEntities(AllocationToken.class, Lock.class, TestObject.class) .withClock(fakeClock) .build(); @@ -97,9 +100,7 @@ public class ReplicateToDatastoreActionTest { @RetryingTest(4) void testReplication() { - if (!ReplayExtension.replayTestsEnabled()) { - return; - } + assumeTrue(ReplayExtension.replayTestsEnabled()); TestObject foo = TestObject.create("foo"); TestObject bar = TestObject.create("bar"); @@ -126,9 +127,7 @@ public class ReplicateToDatastoreActionTest { @RetryingTest(4) void testReplayFromLastTxn() { - if (!ReplayExtension.replayTestsEnabled()) { - return; - } + assumeTrue(ReplayExtension.replayTestsEnabled()); TestObject foo = TestObject.create("foo"); TestObject bar = TestObject.create("bar"); @@ -152,9 +151,7 @@ public class ReplicateToDatastoreActionTest { @RetryingTest(4) void testUnintentionalConcurrency() { - if (!ReplayExtension.replayTestsEnabled()) { - return; - } + assumeTrue(ReplayExtension.replayTestsEnabled()); TestObject foo = TestObject.create("foo"); TestObject bar = TestObject.create("bar"); @@ -189,11 +186,24 @@ public class ReplicateToDatastoreActionTest { Level.WARNING, "Ignoring transaction 1, which appears to have already been applied."); } + @RetryingTest(4) + void testCreateAutoTimestamp() { + // Verify that fields populated by the DB (e.g. CreateAutoTimestamp) correctly get populated in + // both databases. + assumeTrue(ReplayExtension.replayTestsEnabled()); + + AllocationToken allocationToken = + new AllocationToken.Builder().setToken("abc123").setTokenType(TokenType.SINGLE_USE).build(); + insertInDb(allocationToken); + runAndVerifySuccess(); + + assertThat(ofyTm().transact(() -> ofyTm().loadByEntity(allocationToken))) + .isEqualTo(jpaTm().transact(() -> jpaTm().loadByEntity(allocationToken))); + } + @RetryingTest(4) void testMissingTransactions() { - if (!ReplayExtension.replayTestsEnabled()) { - return; - } + assumeTrue(ReplayExtension.replayTestsEnabled()); // Write a transaction (should have a transaction id of 1). TestObject foo = TestObject.create("foo"); @@ -212,9 +222,7 @@ public class ReplicateToDatastoreActionTest { @Test void testMissingTransactions_fullTask() { - if (!ReplayExtension.replayTestsEnabled()) { - return; - } + assumeTrue(ReplayExtension.replayTestsEnabled()); // Write a transaction (should have a transaction id of 1). TestObject foo = TestObject.create("foo"); @@ -234,9 +242,7 @@ public class ReplicateToDatastoreActionTest { @Test void testBeforeDatastoreSaveCallback() { - if (!ReplayExtension.replayTestsEnabled()) { - return; - } + assumeTrue(ReplayExtension.replayTestsEnabled()); TestObject testObject = TestObject.create("foo"); insertInDb(testObject); @@ -247,9 +253,7 @@ public class ReplicateToDatastoreActionTest { @Test void testNotInMigrationState_doesNothing() { - if (!ReplayExtension.replayTestsEnabled()) { - return; - } + assumeTrue(ReplayExtension.replayTestsEnabled()); // set a schedule that backtracks the current status to DATASTORE_PRIMARY DateTime now = fakeClock.nowUtc(); @@ -287,9 +291,7 @@ public class ReplicateToDatastoreActionTest { @Test void testFailure_cannotAcquireLock() { - if (!ReplayExtension.replayTestsEnabled()) { - return; - } + assumeTrue(ReplayExtension.replayTestsEnabled()); RequestStatusChecker requestStatusChecker = mock(RequestStatusChecker.class); when(requestStatusChecker.getLogId()).thenReturn("logId"); diff --git a/core/src/test/java/google/registry/persistence/converter/CreateAutoTimestampConverterTest.java b/core/src/test/java/google/registry/persistence/converter/CreateAutoTimestampConverterTest.java deleted file mode 100644 index 0992d78a8..000000000 --- a/core/src/test/java/google/registry/persistence/converter/CreateAutoTimestampConverterTest.java +++ /dev/null @@ -1,82 +0,0 @@ -// Copyright 2019 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.persistence.converter; - -import static com.google.common.truth.Truth.assertThat; -import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; -import static google.registry.testing.DatabaseHelper.insertInDb; - -import google.registry.model.CreateAutoTimestamp; -import google.registry.model.ImmutableObject; -import google.registry.model.replay.EntityTest.EntityForTesting; -import google.registry.persistence.transaction.JpaTestExtensions; -import google.registry.persistence.transaction.JpaTestExtensions.JpaUnitTestExtension; -import google.registry.testing.FakeClock; -import javax.persistence.Entity; -import javax.persistence.Id; -import org.joda.time.DateTime; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; - -/** Unit tests for {@link CreateAutoTimestampConverter}. */ -public class CreateAutoTimestampConverterTest { - - private final FakeClock fakeClock = new FakeClock(); - - @RegisterExtension - public final JpaUnitTestExtension jpaExtension = - new JpaTestExtensions.Builder() - .withClock(fakeClock) - .withEntityClass(TestEntity.class) - .buildUnitTestExtension(); - - @Test - void testTypeConversion() { - CreateAutoTimestamp ts = CreateAutoTimestamp.create(DateTime.parse("2019-09-9T11:39:00Z")); - TestEntity ent = new TestEntity("myinst", ts); - - insertInDb(ent); - TestEntity result = - jpaTm().transact(() -> jpaTm().getEntityManager().find(TestEntity.class, "myinst")); - assertThat(result).isEqualTo(new TestEntity("myinst", ts)); - } - - @Test - void testAutoInitialization() { - CreateAutoTimestamp ts = CreateAutoTimestamp.create(null); - TestEntity ent = new TestEntity("autoinit", ts); - - insertInDb(ent); - - TestEntity result = - jpaTm().transact(() -> jpaTm().getEntityManager().find(TestEntity.class, "autoinit")); - assertThat(result.cat.getTimestamp()).isEqualTo(fakeClock.nowUtc()); - } - - @Entity(name = "TestEntity") // Override entity name to avoid the nested class reference. - @EntityForTesting - public static class TestEntity extends ImmutableObject { - - @Id String name; - - CreateAutoTimestamp cat; - - public TestEntity() {} - - TestEntity(String name, CreateAutoTimestamp cat) { - this.name = name; - this.cat = cat; - } - } -} diff --git a/db/src/main/resources/sql/er_diagram/brief_er_diagram.html b/db/src/main/resources/sql/er_diagram/brief_er_diagram.html index 1fd0e5614..1b8319974 100644 --- a/db/src/main/resources/sql/er_diagram/brief_er_diagram.html +++ b/db/src/main/resources/sql/er_diagram/brief_er_diagram.html @@ -261,11 +261,11 @@ td.section { generated on - 2021-09-14 16:11:21.688911 + 2021-11-19 21:30:39.304221 last flyway file - V102__add_indexes_to_domain_history_sub_tables.sql + V103__creation_time_not_null.sql @@ -284,7 +284,7 @@ td.section { generated on - 2021-09-14 16:11:21.688911 + 2021-11-19 21:30:39.304221 diff --git a/db/src/main/resources/sql/er_diagram/full_er_diagram.html b/db/src/main/resources/sql/er_diagram/full_er_diagram.html index e9a088edb..3bc438269 100644 --- a/db/src/main/resources/sql/er_diagram/full_er_diagram.html +++ b/db/src/main/resources/sql/er_diagram/full_er_diagram.html @@ -261,11 +261,11 @@ td.section { generated on - 2021-09-14 16:11:19.580097 + 2021-11-19 21:30:37.273092 last flyway file - V102__add_indexes_to_domain_history_sub_tables.sql + V103__creation_time_not_null.sql @@ -284,7 +284,7 @@ td.section { generated on - 2021-09-14 16:11:19.580097 + 2021-11-19 21:30:37.273092 @@ -1365,7 +1365,7 @@ td.section { - timestamptz + timestamptz not null drive_folder_id @@ -12184,7 +12184,7 @@ td.section { creation_time - timestamptz + timestamptz not null diff --git a/db/src/main/resources/sql/flyway.txt b/db/src/main/resources/sql/flyway.txt index 9ea9f5196..fa4c4669b 100644 --- a/db/src/main/resources/sql/flyway.txt +++ b/db/src/main/resources/sql/flyway.txt @@ -100,3 +100,4 @@ V99__drop_kms_secret_table.sql V100__database_migration_schedule.sql V101__domain_add_dns_refresh_request_time.sql V102__add_indexes_to_domain_history_sub_tables.sql +V103__creation_time_not_null.sql diff --git a/db/src/main/resources/sql/flyway/V103__creation_time_not_null.sql b/db/src/main/resources/sql/flyway/V103__creation_time_not_null.sql new file mode 100644 index 000000000..d520b687f --- /dev/null +++ b/db/src/main/resources/sql/flyway/V103__creation_time_not_null.sql @@ -0,0 +1,15 @@ +-- Copyright 2021 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. + +ALTER TABLE "Registrar" ALTER COLUMN creation_time SET NOT NULL; diff --git a/db/src/main/resources/sql/schema/db-schema.sql.generated b/db/src/main/resources/sql/schema/db-schema.sql.generated index 90a0b9db9..8d22e2874 100644 --- a/db/src/main/resources/sql/schema/db-schema.sql.generated +++ b/db/src/main/resources/sql/schema/db-schema.sql.generated @@ -572,7 +572,7 @@ client_certificate text, client_certificate_hash text, contacts_require_syncing boolean not null, - creation_time timestamptz, + creation_time timestamptz not null, drive_folder_id text, email_address text, failover_client_certificate text, diff --git a/db/src/main/resources/sql/schema/nomulus.golden.sql b/db/src/main/resources/sql/schema/nomulus.golden.sql index d11e46c77..b1a986aa0 100644 --- a/db/src/main/resources/sql/schema/nomulus.golden.sql +++ b/db/src/main/resources/sql/schema/nomulus.golden.sql @@ -763,7 +763,7 @@ CREATE TABLE public."Registrar" ( client_certificate text, client_certificate_hash text, contacts_require_syncing boolean NOT NULL, - creation_time timestamp with time zone, + creation_time timestamp with time zone NOT NULL, drive_folder_id text, email_address text, failover_client_certificate text,