diff --git a/core/src/main/java/google/registry/model/ofy/DatastoreTransactionManager.java b/core/src/main/java/google/registry/model/ofy/DatastoreTransactionManager.java index 46d5ddc4f..1be914365 100644 --- a/core/src/main/java/google/registry/model/ofy/DatastoreTransactionManager.java +++ b/core/src/main/java/google/registry/model/ofy/DatastoreTransactionManager.java @@ -30,6 +30,7 @@ import com.google.common.collect.Streams; import com.googlecode.objectify.Key; import com.googlecode.objectify.Result; import google.registry.model.contact.ContactHistory; +import google.registry.model.domain.DomainHistory; import google.registry.model.host.HostHistory; import google.registry.model.reporting.HistoryEntry; import google.registry.persistence.VKey; @@ -292,6 +293,21 @@ public class DatastoreTransactionManager implements TransactionManager { getOfy().clearSessionCache(); } + /** + * Executes the given {@link Result} instance synchronously if not in a transaction. + * + *

The {@link Result} instance contains a task that will be executed by Objectify + * asynchronously. If it is in a transaction, we don't need to execute the task immediately + * because it is guaranteed to be done by the end of the transaction. However, if it is not in a + * transaction, we need to execute it in case the following code expects that happens before + * themselves. + */ + private void syncIfTransactionless(Result result) { + if (!inTransaction()) { + result.now(); + } + } + /** * The following three methods exist due to the migration to Cloud SQL. * @@ -309,34 +325,23 @@ public class DatastoreTransactionManager implements TransactionManager { syncIfTransactionless(getOfy().save().entity(entity)); } - @SuppressWarnings("unchecked") - private T toChildHistoryEntryIfPossible(@Nullable T obj) { - // NB: The Key of the object in question may not necessarily be the resulting class that we - // wish to have. Because all *History classes are @EntitySubclasses, their Keys will have type - // HistoryEntry -- even if you create them based off the *History class. - if (obj != null && HistoryEntry.class.isAssignableFrom(obj.getClass())) { - return (T) ((HistoryEntry) obj).toChildHistoryEntity(); - } - return obj; - } - @Nullable private T loadNullable(VKey key) { return toChildHistoryEntryIfPossible(getOfy().load().key(key.getOfyKey()).now()); } - /** - * Executes the given {@link Result} instance synchronously if not in a transaction. - * - *

The {@link Result} instance contains a task that will be executed by Objectify - * asynchronously. If it is in a transaction, we don't need to execute the task immediately - * because it is guaranteed to be done by the end of the transaction. However, if it is not in a - * transaction, we need to execute it in case the following code expects that happens before - * themselves. - */ - private void syncIfTransactionless(Result result) { - if (!inTransaction()) { - result.now(); + /** Converts a nonnull {@link HistoryEntry} to the child format, e.g. {@link DomainHistory} */ + @SuppressWarnings("unchecked") + public static T toChildHistoryEntryIfPossible(@Nullable T obj) { + // NB: The Key of the object in question may not necessarily be the resulting class that we + // wish to have. Because all *History classes are @EntitySubclasses, their Keys will have type + // HistoryEntry -- even if you create them based off the *History class. + if (obj instanceof HistoryEntry + && !(obj instanceof ContactHistory) + && !(obj instanceof DomainHistory) + && !(obj instanceof HostHistory)) { + return (T) ((HistoryEntry) obj).toChildHistoryEntity(); } + return obj; } } diff --git a/core/src/main/java/google/registry/model/reporting/HistoryEntryDao.java b/core/src/main/java/google/registry/model/reporting/HistoryEntryDao.java new file mode 100644 index 000000000..b3f4e341a --- /dev/null +++ b/core/src/main/java/google/registry/model/reporting/HistoryEntryDao.java @@ -0,0 +1,144 @@ +// 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.reporting; + +import static com.google.common.collect.ImmutableList.toImmutableList; +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.util.DateTimeUtils.END_OF_TIME; +import static google.registry.util.DateTimeUtils.START_OF_TIME; + +import com.google.common.collect.Iterables; +import google.registry.model.EppResource; +import google.registry.model.contact.ContactHistory; +import google.registry.model.contact.ContactResource; +import google.registry.model.domain.DomainBase; +import google.registry.model.domain.DomainHistory; +import google.registry.model.host.HostHistory; +import google.registry.model.host.HostResource; +import google.registry.persistence.VKey; +import java.util.Comparator; +import javax.persistence.EntityManager; +import org.joda.time.DateTime; + +/** + * Retrieves {@link HistoryEntry} descendants (e.g. {@link DomainHistory}). + * + *

This class is configured to retrieve either from Datastore or SQL, depending on which database + * is currently considered the primary database. + */ +public class HistoryEntryDao { + + /** Loads all history objects in the times specified, including all types. */ + public static Iterable loadAllHistoryObjects( + DateTime afterTime, DateTime beforeTime) { + if (tm().isOfy()) { + return ofy() + .load() + .type(HistoryEntry.class) + .order("modificationTime") + .filter("modificationTime >=", afterTime) + .filter("modificationTime <=", beforeTime); + } else { + return jpaTm() + .transact( + () -> + Iterables.concat( + loadAllHistoryObjectsFromSql(ContactHistory.class, afterTime, beforeTime), + loadAllHistoryObjectsFromSql(DomainHistory.class, afterTime, beforeTime), + loadAllHistoryObjectsFromSql(HostHistory.class, afterTime, beforeTime))); + } + } + + /** Loads all history objects corresponding to the given {@link EppResource}. */ + public static Iterable loadHistoryObjectsForResource( + VKey parentKey) { + return loadHistoryObjectsForResource(parentKey, START_OF_TIME, END_OF_TIME); + } + + /** Loads all history objects in the time period specified for the given {@link EppResource}. */ + public static Iterable loadHistoryObjectsForResource( + VKey parentKey, DateTime afterTime, DateTime beforeTime) { + if (tm().isOfy()) { + return ofy() + .load() + .type(HistoryEntry.class) + .ancestor(parentKey.getOfyKey()) + .order("modificationTime") + .filter("modificationTime >=", afterTime) + .filter("modificationTime <=", beforeTime); + } else { + return jpaTm() + .transact(() -> loadHistoryObjectsForResourceFromSql(parentKey, afterTime, beforeTime)); + } + } + + private static Iterable loadHistoryObjectsForResourceFromSql( + VKey parentKey, DateTime afterTime, DateTime beforeTime) { + Class historyClass = getHistoryClassFromParent(parentKey.getKind()); + String repoIdFieldName = getRepoIdFieldNameFromHistoryClass(historyClass); + EntityManager entityManager = jpaTm().getEntityManager(); + String tableName = entityManager.getMetamodel().entity(historyClass).getName(); + String queryString = + String.format( + "SELECT entry FROM %s entry WHERE entry.modificationTime >= :afterTime AND " + + "entry.modificationTime <= :beforeTime AND entry.%s = :parentKey", + tableName, repoIdFieldName); + return entityManager + .createQuery(queryString, historyClass) + .setParameter("afterTime", afterTime) + .setParameter("beforeTime", beforeTime) + .setParameter("parentKey", parentKey.getSqlKey().toString()) + .getResultStream() + .sorted(Comparator.comparing(HistoryEntry::getModificationTime)) + .collect(toImmutableList()); + } + + private static Class getHistoryClassFromParent( + Class parent) { + if (parent.equals(ContactResource.class)) { + return ContactHistory.class; + } else if (parent.equals(DomainBase.class)) { + return DomainHistory.class; + } else if (parent.equals(HostResource.class)) { + return HostHistory.class; + } + throw new IllegalArgumentException( + String.format("Unknown history type for parent %s", parent.getName())); + } + + private static String getRepoIdFieldNameFromHistoryClass( + Class historyClass) { + return historyClass.equals(ContactHistory.class) + ? "contactRepoId" + : historyClass.equals(DomainHistory.class) ? "domainRepoId" : "hostRepoId"; + } + + private static Iterable loadAllHistoryObjectsFromSql( + Class historyClass, DateTime afterTime, DateTime beforeTime) { + EntityManager entityManager = jpaTm().getEntityManager(); + return entityManager + .createQuery( + String.format( + "SELECT entry FROM %s entry WHERE entry.modificationTime >= :afterTime AND " + + "entry.modificationTime <= :beforeTime", + entityManager.getMetamodel().entity(historyClass).getName()), + historyClass) + .setParameter("afterTime", afterTime) + .setParameter("beforeTime", beforeTime) + .getResultList(); + } +} diff --git a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java index 504d77908..0da9c11d0 100644 --- a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java +++ b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static google.registry.model.ofy.DatastoreTransactionManager.toChildHistoryEntryIfPossible; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import static java.util.AbstractMap.SimpleEntry; import static java.util.stream.Collectors.joining; @@ -34,6 +35,7 @@ import google.registry.model.index.EppResourceIndex; import google.registry.model.index.ForeignKeyIndex.ForeignKeyContactIndex; import google.registry.model.index.ForeignKeyIndex.ForeignKeyDomainIndex; import google.registry.model.index.ForeignKeyIndex.ForeignKeyHostIndex; +import google.registry.model.ofy.DatastoreTransactionManager; import google.registry.persistence.JpaRetries; import google.registry.persistence.VKey; import google.registry.util.Clock; @@ -250,8 +252,10 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { return; } assertInTransaction(); - getEntityManager().persist(entity); - transactionInfo.get().addUpdate(entity); + // Necessary due to the changes in HistoryEntry representation during the migration to SQL + Object toPersist = toChildHistoryEntryIfPossible(entity); + getEntityManager().persist(toPersist); + transactionInfo.get().addUpdate(toPersist); } @Override @@ -278,8 +282,10 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { return; } assertInTransaction(); - getEntityManager().merge(entity); - transactionInfo.get().addUpdate(entity); + // Necessary due to the changes in HistoryEntry representation during the migration to SQL + Object toPersist = toChildHistoryEntryIfPossible(entity); + getEntityManager().merge(toPersist); + transactionInfo.get().addUpdate(toPersist); } @Override @@ -307,8 +313,10 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { } assertInTransaction(); checkArgument(exists(entity), "Given entity does not exist"); - getEntityManager().merge(entity); - transactionInfo.get().addUpdate(entity); + // Necessary due to the changes in HistoryEntry representation during the migration to SQL + Object toPersist = toChildHistoryEntryIfPossible(entity); + getEntityManager().merge(toPersist); + transactionInfo.get().addUpdate(toPersist); } @Override @@ -339,6 +347,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { @Override public boolean exists(Object entity) { checkArgumentNotNull(entity, "entity must be specified"); + entity = toChildHistoryEntryIfPossible(entity); EntityType entityType = getEntityType(entity.getClass()); ImmutableSet entityIds = getEntityIdsFromEntity(entityType, entity); return exists(entityType.getName(), entityIds); @@ -382,6 +391,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { @Override public ImmutableList loadByEntitiesIfPresent(Iterable entities) { return Streams.stream(entities) + .map(DatastoreTransactionManager::toChildHistoryEntryIfPossible) .filter(this::exists) .map(this::loadByEntity) .collect(toImmutableList()); @@ -416,9 +426,14 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { public T loadByEntity(T entity) { checkArgumentNotNull(entity, "entity must be specified"); assertInTransaction(); + entity = toChildHistoryEntryIfPossible(entity); + // If the caller requested a HistoryEntry, load the corresponding *History class + T possibleChild = toChildHistoryEntryIfPossible(entity); return (T) loadByKey( - VKey.createSql(entity.getClass(), emf.getPersistenceUnitUtil().getIdentifier(entity))); + VKey.createSql( + possibleChild.getClass(), + emf.getPersistenceUnitUtil().getIdentifier(possibleChild))); } @Override @@ -469,6 +484,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { return; } assertInTransaction(); + entity = toChildHistoryEntryIfPossible(entity); Object managedEntity = entity; if (!getEntityManager().contains(entity)) { managedEntity = getEntityManager().merge(entity); diff --git a/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java b/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java index 8bdeb1589..018a90800 100644 --- a/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java +++ b/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java @@ -52,6 +52,7 @@ import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarAddress; import google.registry.model.registrar.RegistrarContact; import google.registry.model.reporting.HistoryEntry; +import google.registry.model.reporting.HistoryEntryDao; import google.registry.persistence.VKey; import google.registry.rdap.RdapDataStructures.Event; import google.registry.rdap.RdapDataStructures.EventAction; @@ -880,8 +881,9 @@ public class RdapJsonFormatter { // 2.3.2.3 An event of *eventAction* type *transfer*, with the last date and time that the // domain was transferred. The event of *eventAction* type *transfer* MUST be omitted if the // domain name has not been transferred since it was created. - for (HistoryEntry historyEntry : - ofy().load().type(HistoryEntry.class).ancestor(resource).order("modificationTime")) { + Iterable historyEntries = + HistoryEntryDao.loadHistoryObjectsForResource(resource.createVKey()); + for (HistoryEntry historyEntry : historyEntries) { EventAction rdapEventAction = HISTORY_ENTRY_TYPE_TO_RDAP_EVENT_ACTION_MAP.get(historyEntry.getType()); // Only save the historyEntries if this is a type we care about. @@ -930,13 +932,9 @@ public class RdapJsonFormatter { return eventsBuilder.build(); } - /** - * Creates an RDAP event object as defined by RFC 7483. - */ + /** Creates an RDAP event object as defined by RFC 7483. */ private static Event makeEvent( - EventAction eventAction, - @Nullable String eventActor, - DateTime eventDate) { + EventAction eventAction, @Nullable String eventActor, DateTime eventDate) { Event.Builder builder = Event.builder() .setEventAction(eventAction) .setEventDate(eventDate); diff --git a/core/src/main/java/google/registry/tools/GetHistoryEntriesCommand.java b/core/src/main/java/google/registry/tools/GetHistoryEntriesCommand.java index 4c527c3cf..fe5a1969f 100644 --- a/core/src/main/java/google/registry/tools/GetHistoryEntriesCommand.java +++ b/core/src/main/java/google/registry/tools/GetHistoryEntriesCommand.java @@ -15,7 +15,6 @@ package google.registry.tools; import static com.google.common.base.Preconditions.checkArgument; -import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.util.DateTimeUtils.END_OF_TIME; import static google.registry.util.DateTimeUtils.START_OF_TIME; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; @@ -25,14 +24,16 @@ import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; import google.registry.model.EppResource; import google.registry.model.reporting.HistoryEntry; +import google.registry.model.reporting.HistoryEntryDao; import google.registry.persistence.VKey; import google.registry.tools.CommandUtilities.ResourceType; import google.registry.xml.XmlTransformer; import org.joda.time.DateTime; /** Command to show history entries. */ -@Parameters(separators = " =", - commandDescription = "Show history entries that occurred in a given time range") +@Parameters( + separators = " =", + commandDescription = "Show history entries that occurred in a given time range") final class GetHistoryEntriesCommand implements CommandWithRemoteApi { @Parameter( @@ -45,33 +46,26 @@ final class GetHistoryEntriesCommand implements CommandWithRemoteApi { description = "Only show history entries that occurred at or before this time") private DateTime before = END_OF_TIME; - @Parameter( - names = "--type", - description = "Resource type.") + @Parameter(names = "--type", description = "Resource type.") private ResourceType type; - @Parameter( - names = "--id", - description = "Foreign key of the resource.") + @Parameter(names = "--id", description = "Foreign key of the resource.") private String uniqueId; @Override public void run() { - VKey parentKey = null; + Iterable historyEntries; if (type != null || uniqueId != null) { checkArgument( type != null && uniqueId != null, "If either of 'type' or 'id' are set then both must be"); - parentKey = type.getKey(uniqueId, DateTime.now(UTC)); + VKey parentKey = type.getKey(uniqueId, DateTime.now(UTC)); checkArgumentNotNull(parentKey, "Invalid resource ID"); + historyEntries = HistoryEntryDao.loadHistoryObjectsForResource(parentKey, after, before); + } else { + historyEntries = HistoryEntryDao.loadAllHistoryObjects(after, before); } - for (HistoryEntry entry : - (parentKey == null - ? ofy().load().type(HistoryEntry.class) - : ofy().load().type(HistoryEntry.class).ancestor(parentKey.getOfyKey())) - .order("modificationTime") - .filter("modificationTime >=", after) - .filter("modificationTime <=", before)) { + for (HistoryEntry entry : historyEntries) { System.out.printf( "Client: %s\nTime: %s\nClient TRID: %s\nServer TRID: %s\n%s\n", entry.getClientId(), diff --git a/core/src/main/java/google/registry/tools/javascrap/BackfillRegistryLocksCommand.java b/core/src/main/java/google/registry/tools/javascrap/BackfillRegistryLocksCommand.java index b26356f99..d8424e51a 100644 --- a/core/src/main/java/google/registry/tools/javascrap/BackfillRegistryLocksCommand.java +++ b/core/src/main/java/google/registry/tools/javascrap/BackfillRegistryLocksCommand.java @@ -16,20 +16,24 @@ package google.registry.tools.javascrap; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.ImmutableList.toImmutableList; -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.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STATUSES; import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; +import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Streams; import com.google.common.flogger.FluentLogger; -import com.googlecode.objectify.Key; import google.registry.config.RegistryConfig.Config; import google.registry.model.domain.DomainBase; import google.registry.model.registry.RegistryLockDao; import google.registry.model.reporting.HistoryEntry; +import google.registry.model.reporting.HistoryEntryDao; +import google.registry.persistence.VKey; import google.registry.schema.domain.RegistryLock; import google.registry.tools.CommandWithRemoteApi; import google.registry.tools.ConfirmingCommand; @@ -130,7 +134,7 @@ public class BackfillRegistryLocksCommand extends ConfirmingCommand private DateTime getLockCompletionTimestamp(DomainBase domainBase, DateTime now) { // Best-effort, if a domain was URS-locked we should use that time // If we can't find that, return now. - return ofy().load().type(HistoryEntry.class).ancestor(domainBase).list().stream() + return Streams.stream(HistoryEntryDao.loadHistoryObjectsForResource(domainBase.createVKey())) // sort by modification time descending so we get the most recent one if it was locked twice .sorted(Comparator.comparing(HistoryEntry::getModificationTime).reversed()) .filter(entry -> entry.getReason().equals("Uniform Rapid Suspension")) @@ -140,18 +144,14 @@ public class BackfillRegistryLocksCommand extends ConfirmingCommand } private ImmutableList getLockedDomainsWithoutLocks(DateTime now) { - return ImmutableList.copyOf( - ofy() - .load() - .keys( - roids.stream() - .map(roid -> Key.create(DomainBase.class, roid)) - .collect(toImmutableList())) - .values() - .stream() - .filter(d -> d.getDeletionTime().isAfter(now)) - .filter(d -> d.getStatusValues().containsAll(REGISTRY_LOCK_STATUSES)) - .filter(d -> !RegistryLockDao.getMostRecentByRepoId(d.getRepoId()).isPresent()) - .collect(toImmutableList())); + ImmutableList> domainKeys = + roids.stream().map(roid -> VKey.create(DomainBase.class, roid)).collect(toImmutableList()); + ImmutableCollection domains = + transactIfJpaTm(() -> tm().loadByKeys(domainKeys)).values(); + return domains.stream() + .filter(d -> d.getDeletionTime().isAfter(now)) + .filter(d -> d.getStatusValues().containsAll(REGISTRY_LOCK_STATUSES)) + .filter(d -> RegistryLockDao.getMostRecentByRepoId(d.getRepoId()).isEmpty()) + .collect(toImmutableList()); } } diff --git a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java index bb50dd786..fc4c911b1 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java @@ -525,8 +525,7 @@ class DomainCreateFlowTest extends ResourceFlowTestCase".getBytes(UTF_8)) + .setModificationTime(fakeClock.nowUtc()) + .setClientId("TheRegistrar") + .setOtherClientId("otherClient") + .setTrid(Trid.create("ABC-123", "server-trid")) + .setBySuperuser(false) + .setReason("reason") + .setRequestedByRegistrar(false) + .setDomainTransactionRecords(ImmutableSet.of(transactionRecord)) + .build(); + persistResource(historyEntry); + } + + @TestOfyAndSql + void testSimpleLoadAll() { + assertThat(HistoryEntryDao.loadAllHistoryObjects(START_OF_TIME, END_OF_TIME)) + .comparingElementsUsing(immutableObjectCorrespondence("nsHosts")) + .containsExactly(historyEntry); + } + + @TestOfyAndSql + void testSkips_tooEarly() { + assertThat(HistoryEntryDao.loadAllHistoryObjects(fakeClock.nowUtc().plusMillis(1), END_OF_TIME)) + .isEmpty(); + } + + @TestOfyAndSql + void testSkips_tooLate() { + assertThat( + HistoryEntryDao.loadAllHistoryObjects(START_OF_TIME, fakeClock.nowUtc().minusMillis(1))) + .isEmpty(); + } + + @TestOfyAndSql + void testLoadByResource() { + transactIfJpaTm( + () -> + assertThat(HistoryEntryDao.loadHistoryObjectsForResource(domain.createVKey())) + .comparingElementsUsing(immutableObjectCorrespondence("nsHosts")) + .containsExactly(historyEntry)); + } + + @TestOfyAndSql + void testLoadByResource_skips_tooEarly() { + assertThat( + HistoryEntryDao.loadHistoryObjectsForResource( + domain.createVKey(), fakeClock.nowUtc().plusMillis(1), END_OF_TIME)) + .isEmpty(); + } + + @TestOfyAndSql + void testLoadByResource_skips_tooLate() { + assertThat( + HistoryEntryDao.loadHistoryObjectsForResource( + domain.createVKey(), START_OF_TIME, fakeClock.nowUtc().minusMillis(1))) + .isEmpty(); + } + + @TestOfyAndSql + void testLoadByResource_noEntriesForResource() { + DomainBase newDomain = persistResource(newDomainBase("new.foobar")); + assertThat(HistoryEntryDao.loadHistoryObjectsForResource(newDomain.createVKey())).isEmpty(); + } +} diff --git a/core/src/test/java/google/registry/model/reporting/HistoryEntryTest.java b/core/src/test/java/google/registry/model/reporting/HistoryEntryTest.java index ad88dce6e..cb72526f7 100644 --- a/core/src/test/java/google/registry/model/reporting/HistoryEntryTest.java +++ b/core/src/test/java/google/registry/model/reporting/HistoryEntryTest.java @@ -14,22 +14,29 @@ package google.registry.model.reporting; -import static com.google.common.truth.Truth.assertThat; -import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; +import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm; import static google.registry.testing.DatabaseHelper.createTld; -import static google.registry.testing.DatabaseHelper.newDomainBase; +import static google.registry.testing.DatabaseHelper.persistActiveDomain; import static google.registry.testing.DatabaseHelper.persistResource; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import google.registry.model.EntityTestCase; +import google.registry.model.domain.DomainBase; +import google.registry.model.domain.DomainHistory; import google.registry.model.domain.Period; import google.registry.model.eppcommon.Trid; import google.registry.model.reporting.DomainTransactionRecord.TransactionReportField; +import google.registry.testing.DualDatabaseTest; +import google.registry.testing.TestOfyAndSql; +import google.registry.testing.TestOfyOnly; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; /** Unit tests for {@link HistoryEntry}. */ +@DualDatabaseTest class HistoryEntryTest extends EntityTestCase { private HistoryEntry historyEntry; @@ -37,6 +44,7 @@ class HistoryEntryTest extends EntityTestCase { @BeforeEach void setUp() { createTld("foobar"); + DomainBase domain = persistActiveDomain("foo.foobar"); DomainTransactionRecord transactionRecord = new DomainTransactionRecord.Builder() .setTld("foobar") @@ -46,13 +54,13 @@ class HistoryEntryTest extends EntityTestCase { .build(); // Set up a new persisted HistoryEntry entity. historyEntry = - new HistoryEntry.Builder() - .setParent(newDomainBase("foo.foobar")) + new DomainHistory.Builder() + .setParent(domain) .setType(HistoryEntry.Type.DOMAIN_CREATE) .setPeriod(Period.create(1, Period.Unit.YEARS)) .setXmlBytes("".getBytes(UTF_8)) .setModificationTime(fakeClock.nowUtc()) - .setClientId("foo") + .setClientId("TheRegistrar") .setOtherClientId("otherClient") .setTrid(Trid.create("ABC-123", "server-trid")) .setBySuperuser(false) @@ -63,13 +71,23 @@ class HistoryEntryTest extends EntityTestCase { persistResource(historyEntry); } - @Test + @TestOfyAndSql void testPersistence() { - assertThat(ofy().load().entity(historyEntry).now()).isEqualTo(historyEntry); + transactIfJpaTm( + () -> { + HistoryEntry fromDatabase = tm().loadByEntity(historyEntry); + assertAboutImmutableObjects() + .that(fromDatabase) + .isEqualExceptFields(historyEntry, "nsHosts", "domainTransactionRecords"); + assertAboutImmutableObjects() + .that(Iterables.getOnlyElement(fromDatabase.getDomainTransactionRecords())) + .isEqualExceptFields( + Iterables.getOnlyElement(historyEntry.getDomainTransactionRecords()), "id"); + }); } - @Test + @TestOfyOnly void testIndexing() throws Exception { - verifyIndexing(historyEntry, "modificationTime", "clientId"); + verifyIndexing(historyEntry.asHistoryEntry(), "modificationTime", "clientId"); } } diff --git a/core/src/test/java/google/registry/rdap/RdapDomainActionTest.java b/core/src/test/java/google/registry/rdap/RdapDomainActionTest.java index 6ec3b8935..b438f2bad 100644 --- a/core/src/test/java/google/registry/rdap/RdapDomainActionTest.java +++ b/core/src/test/java/google/registry/rdap/RdapDomainActionTest.java @@ -95,7 +95,7 @@ class RdapDomainActionTest extends RdapActionBaseTestCase { registrarLol) .asBuilder() .setCreationTimeForTest(clock.nowUtc().minusYears(3)) - .setCreationClientId("foo") + .setCreationClientId("TheRegistrar") .build()); // deleted domain in lol @@ -128,7 +128,7 @@ class RdapDomainActionTest extends RdapActionBaseTestCase { registrarLol) .asBuilder() .setCreationTimeForTest(clock.nowUtc().minusYears(3)) - .setCreationClientId("foo") + .setCreationClientId("TheRegistrar") .setDeletionTime(clock.nowUtc().minusDays(1)) .build()); // cat.みんな @@ -168,7 +168,7 @@ class RdapDomainActionTest extends RdapActionBaseTestCase { registrarIdn) .asBuilder() .setCreationTimeForTest(clock.nowUtc().minusYears(3)) - .setCreationClientId("foo") + .setCreationClientId("TheRegistrar") .build()); // 1.tld @@ -208,7 +208,7 @@ class RdapDomainActionTest extends RdapActionBaseTestCase { registrar1Tld) .asBuilder() .setCreationTimeForTest(clock.nowUtc().minusYears(3)) - .setCreationClientId("foo") + .setCreationClientId("TheRegistrar") .build()); // history entries diff --git a/core/src/test/java/google/registry/rdap/RdapDomainSearchActionTest.java b/core/src/test/java/google/registry/rdap/RdapDomainSearchActionTest.java index 02b8002b2..98b954b6d 100644 --- a/core/src/test/java/google/registry/rdap/RdapDomainSearchActionTest.java +++ b/core/src/test/java/google/registry/rdap/RdapDomainSearchActionTest.java @@ -176,7 +176,7 @@ class RdapDomainSearchActionTest extends RdapSearchActionTestCase %s\ninstead of %s\n\n", + "Actual: %s -> %s\nExpected: %s\n\n", name, jsonifyAndIndent(actual), jsonifyAndIndent(expected))); } } diff --git a/core/src/test/java/google/registry/testing/FullFieldsTestEntityHelper.java b/core/src/test/java/google/registry/testing/FullFieldsTestEntityHelper.java index 76311e4cc..cdac768a7 100644 --- a/core/src/test/java/google/registry/testing/FullFieldsTestEntityHelper.java +++ b/core/src/test/java/google/registry/testing/FullFieldsTestEntityHelper.java @@ -398,7 +398,7 @@ public final class FullFieldsTestEntityHelper { .setPeriod(period) .setXmlBytes("".getBytes(UTF_8)) .setModificationTime(modificationTime) - .setClientId("foo") + .setClientId(resource.getPersistedCurrentSponsorClientId()) .setTrid(Trid.create("ABC-123", "server-trid")) .setBySuperuser(false) .setReason(reason) diff --git a/core/src/test/java/google/registry/tools/GetHistoryEntriesCommandTest.java b/core/src/test/java/google/registry/tools/GetHistoryEntriesCommandTest.java index 100ac8e21..d56d96e4b 100644 --- a/core/src/test/java/google/registry/tools/GetHistoryEntriesCommandTest.java +++ b/core/src/test/java/google/registry/tools/GetHistoryEntriesCommandTest.java @@ -22,12 +22,14 @@ import static google.registry.testing.FullFieldsTestEntityHelper.makeHistoryEntr import google.registry.model.domain.DomainBase; import google.registry.model.domain.Period; import google.registry.model.reporting.HistoryEntry; +import google.registry.testing.DualDatabaseTest; import google.registry.testing.FakeClock; +import google.registry.testing.TestOfyAndSql; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; /** Unit tests for {@link GetClaimsListCommand}. */ +@DualDatabaseTest class GetHistoryEntriesCommandTest extends CommandTestCase { private final FakeClock clock = new FakeClock(DateTime.parse("2000-01-01T00:00:00Z")); @@ -40,7 +42,7 @@ class GetHistoryEntriesCommandTest extends CommandTestCase\n" + + "\n" + + "\n"); + } + + @TestOfyAndSql void testSuccess_noTrid() throws Exception { persistResource( makeHistoryEntry( @@ -74,7 +126,7 @@ class GetHistoryEntriesCommandTest extends CommandTestCase { @BeforeEach @@ -57,7 +59,7 @@ class BackfillRegistryLocksCommandTest extends CommandTestCase