diff --git a/core/src/main/java/google/registry/model/EppResourceUtils.java b/core/src/main/java/google/registry/model/EppResourceUtils.java index 25ca49871..4e5a7c7e8 100644 --- a/core/src/main/java/google/registry/model/EppResourceUtils.java +++ b/core/src/main/java/google/registry/model/EppResourceUtils.java @@ -17,10 +17,10 @@ package google.registry.model; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.model.ofy.ObjectifyService.auditedOfy; -import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm; +import static google.registry.util.DateTimeUtils.START_OF_TIME; import static google.registry.util.DateTimeUtils.isAtOrAfter; import static google.registry.util.DateTimeUtils.isBeforeOrAt; import static google.registry.util.DateTimeUtils.latestOf; @@ -43,10 +43,13 @@ import google.registry.model.index.ForeignKeyIndex; import google.registry.model.ofy.CommitLogManifest; import google.registry.model.ofy.CommitLogMutation; import google.registry.model.registry.Registry; +import google.registry.model.reporting.HistoryEntry; +import google.registry.model.reporting.HistoryEntryDao; import google.registry.model.transfer.DomainTransferData; import google.registry.model.transfer.TransferData; import google.registry.model.transfer.TransferStatus; import google.registry.persistence.VKey; +import java.util.Comparator; import java.util.List; import java.util.Map.Entry; import java.util.Optional; @@ -266,26 +269,43 @@ public final class EppResourceUtils { * Rewinds an {@link EppResource} object to a given point in time. * *

This method costs nothing if {@code resource} is already current. Otherwise it needs to - * perform a single asynchronous key fetch operation. + * perform a single fetch operation. * *

Warning: A resource can only be rolled backwards in time, not forwards; therefore * {@code resource} should be whatever's currently in Datastore. * - *

Warning: Revisions are granular to 24-hour periods. It's recommended that - * {@code timestamp} be set to midnight. Otherwise you must take into consideration that under - * certain circumstances, a resource might be restored to a revision on the previous day, even if - * there were revisions made earlier on the same date as {@code timestamp}; however, a resource - * will never be restored to a revision occurring after {@code timestamp}. This behavior is due to - * the way {@link google.registry.model.translators.CommitLogRevisionsTranslatorFactory + *

Warning: In Datastore, revisions are granular to 24-hour periods. It's recommended + * that {@code timestamp} be set to midnight. If you don't use midnight, you must take into + * consideration that under certain circumstances, a resource might be restored to a revision on + * the previous day, even if there were revisions made earlier on the same date as {@code + * timestamp}; however, a resource will never be restored to a revision occurring after {@code + * timestamp}. This behavior is due to the way {@link + * google.registry.model.translators.CommitLogRevisionsTranslatorFactory * CommitLogRevisionsTranslatorFactory} manages the {@link EppResource#revisions} field. Please * note however that the creation and deletion times of a resource are granular to the * millisecond. * + *

Example: a resource in Datastore has three revisions A, B, and C + * + *

+ * + *

If one requests the resource as of day 1 at 2pm, we will return revision A because as far as + * the commit logs are concerned, revision C completely overwrites the existence of revision B. + * + *

When using the SQL backend (post-Registry-3.0-migration) this restriction goes away and + * objects can be restored to any revision. + * + *

TODO(b/177567432): Once Datastore is completely removed, remove the Result wrapping. + * * @return an asynchronous operation returning resource at {@code timestamp} or {@code null} if * resource is deleted or not yet created */ - public static - Result loadAtPointInTime(final T resource, final DateTime timestamp) { + public static Result loadAtPointInTime( + final T resource, final DateTime timestamp) { // If we're before the resource creation time, don't try to find a "most recent revision". if (timestamp.isBefore(resource.getCreationTime())) { return new ResultNow<>(null); @@ -300,7 +320,8 @@ public final class EppResourceUtils { : loadMostRecentRevisionAtTime(resource, timestamp); return () -> { T loadedResource = loadResult.now(); - return (loadedResource == null) ? null + return (loadedResource == null) + ? null : (isActive(loadedResource, timestamp) ? cloneProjectedAtTime(loadedResource, timestamp) : null); @@ -308,26 +329,43 @@ public final class EppResourceUtils { } /** - * Returns an asynchronous result holding the most recent Datastore revision of a given - * EppResource before or at the provided timestamp using the EppResource revisions map, falling - * back to using the earliest revision or the resource as-is if there are no revisions. + * Returns an asynchronous result holding the most recent revision of a given EppResource before + * or at the provided timestamp, falling back to using the resource as-is if there are no + * revisions. * * @see #loadAtPointInTime(EppResource, DateTime) */ private static Result loadMostRecentRevisionAtTime( final T resource, final DateTime timestamp) { + if (tm().isOfy()) { + return loadMostRecentRevisionAtTimeDatastore(resource, timestamp); + } else { + return loadMostRecentRevisionAtTimeSql(resource, timestamp); + } + } + + /** + * Returns an asynchronous result holding the most recent Datastore revision of a given + * EppResource before or at the provided timestamp using the EppResource revisions map, falling + * back to using the resource as-is if there are no revisions. + * + * @see #loadAtPointInTime(EppResource, DateTime) + */ + private static Result loadMostRecentRevisionAtTimeDatastore( + final T resource, final DateTime timestamp) { final Key resourceKey = Key.create(resource); - final Key revision = findMostRecentRevisionAtTime(resource, timestamp); + final Key revision = + findMostRecentDatastoreRevisionAtTime(resource, timestamp); if (revision == null) { logger.atSevere().log("No revision found for %s, falling back to resource.", resourceKey); return new ResultNow<>(resource); } final Result mutationResult = - ofy().load().key(CommitLogMutation.createKey(revision, resourceKey)); + auditedOfy().load().key(CommitLogMutation.createKey(revision, resourceKey)); return () -> { CommitLogMutation mutation = mutationResult.now(); if (mutation != null) { - return ofy().load().fromEntity(mutation.getEntity()); + return auditedOfy().load().fromEntity(mutation.getEntity()); } logger.atSevere().log( "Couldn't load mutation for revision at %s for %s, falling back to resource." @@ -337,9 +375,37 @@ public final class EppResourceUtils { }; } + /** + * Returns an asynchronous result holding the most recent SQL revision of a given EppResource + * before or at the provided timestamp using *History objects, falling back to using the resource + * as-is if there are no revisions. + * + * @see #loadAtPointInTime(EppResource, DateTime) + */ + @SuppressWarnings("unchecked") + private static Result loadMostRecentRevisionAtTimeSql( + T resource, DateTime timestamp) { + T resourceAtPointInTime = + (T) + HistoryEntryDao.loadHistoryObjectsForResource( + resource.createVKey(), START_OF_TIME, timestamp) + .stream() + .max(Comparator.comparing(HistoryEntry::getModificationTime)) + .flatMap(HistoryEntry::getResourceAtPointInTime) + .orElse(null); + if (resourceAtPointInTime == null) { + logger.atSevere().log( + "Couldn't load resource at % for key %s, falling back to resource %s.", + timestamp, resource.createVKey(), resource); + return new ResultNow<>(resource); + } + return new ResultNow<>(resourceAtPointInTime); + } + @Nullable - private static Key - findMostRecentRevisionAtTime(final T resource, final DateTime timestamp) { + private static + Key findMostRecentDatastoreRevisionAtTime( + final T resource, final DateTime timestamp) { final Key resourceKey = Key.create(resource); Entry> revision = resource.getRevisions().floorEntry(timestamp); if (revision != null) { diff --git a/core/src/main/java/google/registry/model/contact/ContactHistory.java b/core/src/main/java/google/registry/model/contact/ContactHistory.java index 61bc07119..4f5ac5cdb 100644 --- a/core/src/main/java/google/registry/model/contact/ContactHistory.java +++ b/core/src/main/java/google/registry/model/contact/ContactHistory.java @@ -18,6 +18,7 @@ import static google.registry.persistence.transaction.TransactionManagerFactory. import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.EntitySubclass; +import google.registry.model.EppResource; import google.registry.model.ImmutableObject; import google.registry.model.contact.ContactHistory.ContactHistoryId; import google.registry.model.reporting.HistoryEntry; @@ -107,6 +108,11 @@ public class ContactHistory extends HistoryEntry implements SqlEntity { return (VKey) createVKey(Key.create(this)); } + @Override + public Optional getResourceAtPointInTime() { + return getContactBase(); + } + @PostLoad void postLoad() { // Normally Hibernate would see that the contact fields are all null and would fill contactBase diff --git a/core/src/main/java/google/registry/model/domain/DomainHistory.java b/core/src/main/java/google/registry/model/domain/DomainHistory.java index d96388f10..9fb4721e8 100644 --- a/core/src/main/java/google/registry/model/domain/DomainHistory.java +++ b/core/src/main/java/google/registry/model/domain/DomainHistory.java @@ -21,9 +21,11 @@ import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy; import com.google.common.collect.ImmutableSet; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.EntitySubclass; +import google.registry.model.EppResource; import google.registry.model.ImmutableObject; import google.registry.model.domain.DomainHistory.DomainHistoryId; import google.registry.model.domain.GracePeriod.GracePeriodHistory; +import google.registry.model.domain.secdns.DelegationSignerData; import google.registry.model.domain.secdns.DomainDsDataHistory; import google.registry.model.host.HostResource; import google.registry.model.reporting.DomainTransactionRecord; @@ -248,10 +250,21 @@ public class DomainHistory extends HistoryEntry implements SqlEntity { return (VKey) createVKey(Key.create(this)); } + @Override + public Optional getResourceAtPointInTime() { + return getDomainContent(); + } + @PostLoad void postLoad() { if (domainContent != null) { domainContent.nsHosts = nullToEmptyImmutableCopy(nsHosts); + domainContent.gracePeriods = + gracePeriodHistories.stream() + .map(GracePeriod::createFromHistory) + .collect(toImmutableSet()); + domainContent.dsData = + dsDataHistories.stream().map(DelegationSignerData::create).collect(toImmutableSet()); // Normally Hibernate would see that the domain fields are all null and would fill // domainContent with a null object. Unfortunately, the updateTimestamp is never null in SQL. if (domainContent.getDomainName() == null) { diff --git a/core/src/main/java/google/registry/model/domain/GracePeriod.java b/core/src/main/java/google/registry/model/domain/GracePeriod.java index 555978880..bd7b5fc44 100644 --- a/core/src/main/java/google/registry/model/domain/GracePeriod.java +++ b/core/src/main/java/google/registry/model/domain/GracePeriod.java @@ -115,6 +115,17 @@ public class GracePeriod extends GracePeriodBase implements DatastoreAndSqlEntit type, domainRepoId, expirationTime, clientId, billingEventOneTime, null, gracePeriodId); } + public static GracePeriod createFromHistory(GracePeriodHistory history) { + return createInternal( + history.type, + history.domainRepoId, + history.expirationTime, + history.clientId, + history.billingEventOneTime == null ? null : history.billingEventOneTime.createVKey(), + history.billingEventRecurring == null ? null : history.billingEventRecurring.createVKey(), + history.gracePeriodId); + } + /** Creates a GracePeriod for a Recurring billing event. */ public static GracePeriod createForRecurring( GracePeriodStatus type, diff --git a/core/src/main/java/google/registry/model/domain/secdns/DelegationSignerData.java b/core/src/main/java/google/registry/model/domain/secdns/DelegationSignerData.java index 2ec76a2bd..06e360ea3 100644 --- a/core/src/main/java/google/registry/model/domain/secdns/DelegationSignerData.java +++ b/core/src/main/java/google/registry/model/domain/secdns/DelegationSignerData.java @@ -114,6 +114,15 @@ public class DelegationSignerData extends DomainDsDataBase { return create(keyTag, algorithm, digestType, DatatypeConverter.parseHexBinary(digestAsHex)); } + public static DelegationSignerData create(DomainDsDataHistory history) { + return create( + history.keyTag, + history.algorithm, + history.digestType, + history.digest, + history.domainRepoId); + } + /** Class to represent the composite primary key of {@link DelegationSignerData} entity. */ static class DomainDsDataId extends ImmutableObject implements Serializable { diff --git a/core/src/main/java/google/registry/model/host/HostHistory.java b/core/src/main/java/google/registry/model/host/HostHistory.java index 0930f03c2..1313d4a50 100644 --- a/core/src/main/java/google/registry/model/host/HostHistory.java +++ b/core/src/main/java/google/registry/model/host/HostHistory.java @@ -18,6 +18,7 @@ import static google.registry.persistence.transaction.TransactionManagerFactory. import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.EntitySubclass; +import google.registry.model.EppResource; import google.registry.model.ImmutableObject; import google.registry.model.host.HostHistory.HostHistoryId; import google.registry.model.reporting.HistoryEntry; @@ -108,6 +109,11 @@ public class HostHistory extends HistoryEntry implements SqlEntity { return (VKey) createVKey(Key.create(this)); } + @Override + public Optional getResourceAtPointInTime() { + return getHostBase(); + } + @PostLoad void postLoad() { // Normally Hibernate would see that the host fields are all null and would fill hostBase diff --git a/core/src/main/java/google/registry/model/reporting/HistoryEntry.java b/core/src/main/java/google/registry/model/reporting/HistoryEntry.java index 0bbeabaf1..2d02c2982 100644 --- a/core/src/main/java/google/registry/model/reporting/HistoryEntry.java +++ b/core/src/main/java/google/registry/model/reporting/HistoryEntry.java @@ -290,6 +290,17 @@ public class HistoryEntry extends ImmutableObject implements Buildable, Datastor return nullToEmptyImmutableCopy(domainTransactionRecords); } + /** + * Throws an error when attempting to retrieve the EppResource at this point in time. + * + *

Subclasses must override this to return the resource; it is non-abstract for legacy reasons + * and objects created prior to the Registry 3.0 migration. + */ + public Optional getResourceAtPointInTime() { + throw new UnsupportedOperationException( + "Raw HistoryEntry objects do not store the resource at that point in time."); + } + /** This method exists solely to satisfy Hibernate. Use the {@link Builder} instead. */ @SuppressWarnings("UnusedMethod") private void setPeriod(Period period) { diff --git a/core/src/test/java/google/registry/flows/EppCommitLogsTest.java b/core/src/test/java/google/registry/flows/EppPointInTimeTest.java similarity index 66% rename from core/src/test/java/google/registry/flows/EppCommitLogsTest.java rename to core/src/test/java/google/registry/flows/EppPointInTimeTest.java index 05d369d05..7b1008bde 100644 --- a/core/src/test/java/google/registry/flows/EppCommitLogsTest.java +++ b/core/src/test/java/google/registry/flows/EppPointInTimeTest.java @@ -16,8 +16,11 @@ package google.registry.flows; import static com.google.common.truth.Truth.assertThat; import static google.registry.model.EppResourceUtils.loadAtPointInTime; -import static google.registry.model.ofy.ObjectifyService.auditedOfy; +import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.createTld; +import static google.registry.testing.DatabaseHelper.loadAllOf; +import static google.registry.testing.DatabaseHelper.loadByEntity; import static google.registry.testing.DatabaseHelper.persistActiveContact; import static google.registry.testing.DatabaseHelper.persistActiveHost; import static java.nio.charset.StandardCharsets.UTF_8; @@ -25,31 +28,38 @@ import static org.joda.time.DateTimeZone.UTC; import static org.joda.time.Duration.standardDays; import com.google.common.collect.ImmutableMap; -import com.googlecode.objectify.Key; +import com.google.common.collect.Iterables; import google.registry.flows.EppTestComponent.FakesAndMocksModule; import google.registry.model.domain.DomainBase; import google.registry.model.ofy.Ofy; import google.registry.monitoring.whitebox.EppMetric; import google.registry.testing.AppEngineExtension; +import google.registry.testing.DualDatabaseTest; import google.registry.testing.EppLoader; import google.registry.testing.FakeClock; import google.registry.testing.FakeHttpSession; import google.registry.testing.InjectExtension; +import google.registry.testing.TestOfyAndSql; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; -/** Test that domain flows create the commit logs needed to reload at points in the past. */ -class EppCommitLogsTest { +/** Test that we can reload EPP resources as they were in the past. */ +@DualDatabaseTest +class EppPointInTimeTest { + + private final FakeClock clock = new FakeClock(DateTime.now(UTC)); @RegisterExtension final AppEngineExtension appEngine = - AppEngineExtension.builder().withDatastoreAndCloudSql().withTaskQueue().build(); + AppEngineExtension.builder() + .withDatastoreAndCloudSql() + .withClock(clock) + .withTaskQueue() + .build(); @RegisterExtension final InjectExtension inject = new InjectExtension(); - private final FakeClock clock = new FakeClock(DateTime.now(UTC)); private EppLoader eppLoader; @BeforeEach @@ -81,7 +91,7 @@ class EppCommitLogsTest { .run(EppMetric.builder()); } - @Test + @TestOfyAndSql void testLoadAtPointInTime() throws Exception { clock.setTo(DateTime.parse("1984-12-18T12:30Z")); // not midnight @@ -95,64 +105,75 @@ class EppCommitLogsTest { clock.setTo(timeAtCreate); eppLoader = new EppLoader(this, "domain_create.xml", ImmutableMap.of("DOMAIN", "example.tld")); runFlow(); - auditedOfy().clearSessionCache(); - Key key = Key.create(auditedOfy().load().type(DomainBase.class).first().now()); - DomainBase domainAfterCreate = auditedOfy().load().key(key).now(); + tm().clearSessionCache(); + DomainBase domainAfterCreate = Iterables.getOnlyElement(loadAllOf(DomainBase.class)); assertThat(domainAfterCreate.getDomainName()).isEqualTo("example.tld"); clock.advanceBy(standardDays(2)); DateTime timeAtFirstUpdate = clock.nowUtc(); eppLoader = new EppLoader(this, "domain_update_dsdata_add.xml"); runFlow(); - auditedOfy().clearSessionCache(); + tm().clearSessionCache(); - DomainBase domainAfterFirstUpdate = auditedOfy().load().key(key).now(); + DomainBase domainAfterFirstUpdate = loadByEntity(domainAfterCreate); assertThat(domainAfterCreate).isNotEqualTo(domainAfterFirstUpdate); clock.advanceOneMilli(); // same day as first update DateTime timeAtSecondUpdate = clock.nowUtc(); eppLoader = new EppLoader(this, "domain_update_dsdata_rem.xml"); runFlow(); - auditedOfy().clearSessionCache(); - DomainBase domainAfterSecondUpdate = auditedOfy().load().key(key).now(); + tm().clearSessionCache(); + DomainBase domainAfterSecondUpdate = loadByEntity(domainAfterCreate); clock.advanceBy(standardDays(2)); DateTime timeAtDelete = clock.nowUtc(); // before 'add' grace period ends eppLoader = new EppLoader(this, "domain_delete.xml", ImmutableMap.of("DOMAIN", "example.tld")); runFlow(); - auditedOfy().clearSessionCache(); + tm().clearSessionCache(); assertThat(domainAfterFirstUpdate).isNotEqualTo(domainAfterSecondUpdate); // Point-in-time can only rewind an object from the current version, not roll forward. - DomainBase latest = auditedOfy().load().key(key).now(); + DomainBase latest = loadByEntity(domainAfterCreate); // Creation time has millisecond granularity due to isActive() check. - auditedOfy().clearSessionCache(); + tm().clearSessionCache(); assertThat(loadAtPointInTime(latest, timeAtCreate.minusMillis(1)).now()).isNull(); assertThat(loadAtPointInTime(latest, timeAtCreate).now()).isNotNull(); assertThat(loadAtPointInTime(latest, timeAtCreate.plusMillis(1)).now()).isNotNull(); - auditedOfy().clearSessionCache(); - assertThat(loadAtPointInTime(latest, timeAtCreate.plusDays(1)).now()) - .isEqualTo(domainAfterCreate); + tm().clearSessionCache(); + assertAboutImmutableObjects() + .that(loadAtPointInTime(latest, timeAtCreate.plusDays(1)).now()) + .hasFieldsEqualTo(domainAfterCreate); - // Both updates happened on the same day. Since the revisions field has day granularity, the - // key to the first update should have been overwritten by the second, and its timestamp rolled - // forward. So we have to fall back to the last revision before midnight. - auditedOfy().clearSessionCache(); - assertThat(loadAtPointInTime(latest, timeAtFirstUpdate).now()).isEqualTo(domainAfterCreate); + tm().clearSessionCache(); + if (tm().isOfy()) { + // Both updates happened on the same day. Since the revisions field has day granularity in + // Datastore, the key to the first update should have been overwritten by the second, and its + // timestamp rolled forward. So we have to fall back to the last revision before midnight. + assertThat(loadAtPointInTime(latest, timeAtFirstUpdate).now()).isEqualTo(domainAfterCreate); + } else { + // In SQL, however, we are not limited by the day granularity, so when we request the object + // at timeAtFirstUpdate we should receive the object at that first update, even though the + // second update occurred one millisecond later. + assertAboutImmutableObjects() + .that(loadAtPointInTime(latest, timeAtFirstUpdate).now()) + .hasFieldsEqualTo(domainAfterFirstUpdate); + } - auditedOfy().clearSessionCache(); - assertThat(loadAtPointInTime(latest, timeAtSecondUpdate).now()) - .isEqualTo(domainAfterSecondUpdate); + tm().clearSessionCache(); + assertAboutImmutableObjects() + .that(loadAtPointInTime(latest, timeAtSecondUpdate).now()) + .hasFieldsEqualTo(domainAfterSecondUpdate); - auditedOfy().clearSessionCache(); - assertThat(loadAtPointInTime(latest, timeAtSecondUpdate.plusDays(1)).now()) - .isEqualTo(domainAfterSecondUpdate); + tm().clearSessionCache(); + assertAboutImmutableObjects() + .that(loadAtPointInTime(latest, timeAtSecondUpdate.plusDays(1)).now()) + .hasFieldsEqualTo(domainAfterSecondUpdate); // Deletion time has millisecond granularity due to isActive() check. - auditedOfy().clearSessionCache(); + tm().clearSessionCache(); assertThat(loadAtPointInTime(latest, timeAtDelete.minusMillis(1)).now()).isNotNull(); assertThat(loadAtPointInTime(latest, timeAtDelete).now()).isNull(); assertThat(loadAtPointInTime(latest, timeAtDelete.plusMillis(1)).now()).isNull(); diff --git a/core/src/test/java/google/registry/model/EppResourceUtilsTest.java b/core/src/test/java/google/registry/model/EppResourceUtilsTest.java index ac2ba632b..f28b506d4 100644 --- a/core/src/test/java/google/registry/model/EppResourceUtilsTest.java +++ b/core/src/test/java/google/registry/model/EppResourceUtilsTest.java @@ -41,14 +41,18 @@ import org.junit.jupiter.api.extension.RegisterExtension; @DualDatabaseTest class EppResourceUtilsTest { + private final FakeClock clock = new FakeClock(DateTime.now(UTC)); + @RegisterExtension public final AppEngineExtension appEngine = - AppEngineExtension.builder().withDatastoreAndCloudSql().withTaskQueue().build(); + AppEngineExtension.builder() + .withDatastoreAndCloudSql() + .withClock(clock) + .withTaskQueue() + .build(); @RegisterExtension public final InjectExtension inject = new InjectExtension(); - private final FakeClock clock = new FakeClock(DateTime.now(UTC)); - @BeforeEach void beforeEach() { createTld("tld");