From a75640ec9cafd18b6a6f45b048f491fe63ca2b11 Mon Sep 17 00:00:00 2001 From: gbrodman Date: Fri, 22 Jan 2021 14:43:34 -0500 Subject: [PATCH] Convert (most) HistoryEntry ofy calls to tm (#933) * Convert (most) HistoryEntry ofy calls to tm As part of this change, it was necessary to do changes in the JPATM that are similar (but the opposite) of the changes that we did in DatastoreTM with regards to converting HistoryEntries to and from the *History classes. We leave the ofy() calls in the MapReduce ResaveAllHistoryEntriesAction for now; that can be converted during the Beam pipeline transition. Some other tests required registrar-name fixes as well -- because *History objects have a foreign key on the Registrar table, we have to use a "real" registrar name in tests. * Add simple HistoryEntryDaoTest --- .../ofy/DatastoreTransactionManager.java | 51 ++++--- .../model/reporting/HistoryEntryDao.java | 144 ++++++++++++++++++ .../JpaTransactionManagerImpl.java | 30 +++- .../registry/rdap/RdapJsonFormatter.java | 14 +- .../tools/GetHistoryEntriesCommand.java | 30 ++-- .../BackfillRegistryLocksCommand.java | 32 ++-- .../flows/domain/DomainCreateFlowTest.java | 3 +- .../model/reporting/HistoryEntryDaoTest.java | 127 +++++++++++++++ .../model/reporting/HistoryEntryTest.java | 40 +++-- .../registry/rdap/RdapDomainActionTest.java | 8 +- .../rdap/RdapDomainSearchActionTest.java | 12 +- .../registry/rdap/RdapJsonFormatterTest.java | 2 - .../google/registry/rdap/RdapTestHelper.java | 2 +- .../testing/FullFieldsTestEntityHelper.java | 2 +- .../tools/GetHistoryEntriesCommandTest.java | 62 +++++++- .../BackfillRegistryLocksCommandTest.java | 16 +- .../google/registry/rdap/rdap_domain.json | 2 +- .../registry/rdap/rdap_domain_cat2.json | 2 +- .../registry/rdap/rdap_domain_deleted.json | 4 +- .../rdap_domain_no_contacts_with_remark.json | 2 +- .../registry/rdap/rdap_domain_unicode.json | 2 +- ...omain_unicode_no_contacts_with_remark.json | 2 +- .../registry/rdap/rdapjson_domain_full.json | 4 +- .../rdap/rdapjson_domain_logged_out.json | 4 +- .../rdap/rdapjson_domain_no_nameservers.json | 2 +- .../rdap/rdapjson_registrant_nobase.json | 2 +- 26 files changed, 477 insertions(+), 124 deletions(-) create mode 100644 core/src/main/java/google/registry/model/reporting/HistoryEntryDao.java create mode 100644 core/src/test/java/google/registry/model/reporting/HistoryEntryDaoTest.java 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