Remove ofy support from ForeignKeyIndex (#1777)

FKI used to be persisted in datastore to help speed up loading by foreign key.
Now it is just a helper class to do the same thing in SQL because
indexing is natively supported in SQL.

config/checkstyle/checkstyle.xml

@@ -61,12 +61,6 @@ by Joshua Bloch in his book Effective Java -->
     <property name="message" value="Use assertThrows and expectThrows from JUnitBackports instead of the deprecated methods on ExpectedException."/>
   </module>
 
-  <!-- Checks that the deprecated MockitoJUnitRunner is not used. -->
-  <module name="RegexpSingleline">
-    <property name="format" value="MockitoJUnitRunner"/>
-    <property name="message" value="MockitoJUnitRunner is deprecated. Use @RunWith(JUnit4.class) and MockitoRule instead."/>
-  </module>
-
   <module name="LineLength">
     <!-- Checks if a line is too long. -->
     <property name="max" value="${com.puppycrawl.tools.checkstyle.checks.sizes.LineLength.max}" default="100"/>

core/src/main/java/google/registry/batch/DeleteProberDataAction.java

@@ -19,7 +19,6 @@ import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static google.registry.batch.BatchModule.PARAM_DRY_RUN;
 import static google.registry.config.RegistryEnvironment.PRODUCTION;
-import static google.registry.model.ResourceTransferUtils.updateForeignKeyIndexDeletionTime;
 import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_DELETE;
 import static google.registry.model.tld.Registries.getTldsOfType;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
@@ -267,8 +266,6 @@ public class DeleteProberDataAction implements Runnable {
     // messages, or auto-renews because those will all be hard-deleted the next time the job runs
     // anyway.
     tm().putAll(ImmutableList.of(deletedDomain, historyEntry));
-    // updating foreign keys is a no-op in SQL
-    updateForeignKeyIndexDeletionTime(deletedDomain);
     dnsQueue.addDomainRefreshTask(deletedDomain.getDomainName());
   }
 }

core/src/main/java/google/registry/flows/contact/ContactCreateFlow.java

@@ -41,7 +41,6 @@ import google.registry.model.eppinput.ResourceCommand;
 import google.registry.model.eppoutput.CreateData.ContactCreateData;
 import google.registry.model.eppoutput.EppResponse;
 import google.registry.model.index.EppResourceIndex;
-import google.registry.model.index.ForeignKeyIndex;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.model.reporting.IcannReportingTypes.ActivityReportField;
 import javax.inject.Inject;
@@ -100,7 +99,6 @@ public final class ContactCreateFlow implements TransactionalFlow {
             ImmutableSet.of(
                 newContact,
                 historyBuilder.build(),
-                ForeignKeyIndex.create(newContact, newContact.getDeletionTime()),
                 EppResourceIndex.create(Key.create(newContact))));
     return responseBuilder
         .setResData(ContactCreateData.create(newContact.getContactId(), now))

core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java

@@ -106,7 +106,6 @@ import google.registry.model.eppinput.ResourceCommand;
 import google.registry.model.eppoutput.CreateData.DomainCreateData;
 import google.registry.model.eppoutput.EppResponse;
 import google.registry.model.index.EppResourceIndex;
-import google.registry.model.index.ForeignKeyIndex;
 import google.registry.model.poll.PendingActionNotificationResponse.DomainPendingActionNotificationResponse;
 import google.registry.model.poll.PollMessage;
 import google.registry.model.poll.PollMessage.Autorenew;
@@ -404,7 +403,6 @@ public final class DomainCreateFlow implements TransactionalFlow {
     entitiesToSave.add(
         domain,
         domainHistory,
-        ForeignKeyIndex.create(domain, domain.getDeletionTime()),
         EppResourceIndex.create(Key.create(domain)));
     if (allocationToken.isPresent()
         && TokenType.SINGLE_USE.equals(allocationToken.get().getTokenType())) {

core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java

@@ -29,7 +29,6 @@ import static google.registry.flows.domain.DomainFlowUtils.updateAutorenewRecurr
 import static google.registry.flows.domain.DomainFlowUtils.verifyNotInPredelegation;
 import static google.registry.model.ResourceTransferUtils.denyPendingTransfer;
 import static google.registry.model.ResourceTransferUtils.handlePendingTransferOnDelete;
-import static google.registry.model.ResourceTransferUtils.updateForeignKeyIndexDeletionTime;
 import static google.registry.model.eppoutput.Result.Code.SUCCESS;
 import static google.registry.model.eppoutput.Result.Code.SUCCESS_WITH_ACTION_PENDING;
 import static google.registry.model.reporting.DomainTransactionRecord.TransactionReportField.ADD_FIELDS;
@@ -257,7 +256,6 @@ public final class DomainDeleteFlow implements TransactionalFlow {
     Domain newDomain = builder.build();
     DomainHistory domainHistory =
         buildDomainHistory(newDomain, registry, now, durationUntilDelete, inAddGracePeriod);
-    updateForeignKeyIndexDeletionTime(newDomain);
     handlePendingTransferOnDelete(existingDomain, newDomain, now, domainHistory);
     // Close the autorenew billing event and poll message. This may delete the poll message.  Store
     // the updated recurring billing event, we'll need it later and can't reload it.

core/src/main/java/google/registry/flows/domain/DomainRestoreRequestFlow.java

@@ -27,7 +27,6 @@ import static google.registry.flows.domain.DomainFlowUtils.validateFeeChallenge;
 import static google.registry.flows.domain.DomainFlowUtils.verifyNotReserved;
 import static google.registry.flows.domain.DomainFlowUtils.verifyPremiumNameIsNotBlocked;
 import static google.registry.flows.domain.DomainFlowUtils.verifyRegistrarIsActive;
-import static google.registry.model.ResourceTransferUtils.updateForeignKeyIndexDeletionTime;
 import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_RESTORE;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.util.DateTimeUtils.END_OF_TIME;
@@ -188,7 +187,6 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow {
             autorenewPollMessage,
             now,
             registrarId);
-    updateForeignKeyIndexDeletionTime(newDomain);
     DomainHistory domainHistory = buildDomainHistory(newDomain, now);
     entitiesToSave.add(newDomain, domainHistory, autorenewEvent, autorenewPollMessage);
     tm().putAll(entitiesToSave.build());

core/src/main/java/google/registry/flows/host/HostCreateFlow.java

@@ -50,7 +50,6 @@ import google.registry.model.host.Host;
 import google.registry.model.host.HostCommand.Create;
 import google.registry.model.host.HostHistory;
 import google.registry.model.index.EppResourceIndex;
-import google.registry.model.index.ForeignKeyIndex;
 import google.registry.model.reporting.IcannReportingTypes.ActivityReportField;
 import java.util.Optional;
 import javax.inject.Inject;
@@ -135,7 +134,6 @@ public final class HostCreateFlow implements TransactionalFlow {
         ImmutableSet.of(
             newHost,
             historyBuilder.build(),
-            ForeignKeyIndex.create(newHost, newHost.getDeletionTime()),
             EppResourceIndex.create(Key.create(newHost)));
     if (superordinateDomain.isPresent()) {
       tm().update(

core/src/main/java/google/registry/flows/host/HostUpdateFlow.java

@@ -57,7 +57,6 @@ import google.registry.model.host.HostCommand.Update;
 import google.registry.model.host.HostCommand.Update.AddRemove;
 import google.registry.model.host.HostCommand.Update.Change;
 import google.registry.model.host.HostHistory;
-import google.registry.model.index.ForeignKeyIndex;
 import google.registry.model.reporting.IcannReportingTypes.ActivityReportField;
 import google.registry.persistence.VKey;
 import java.util.Objects;
@@ -194,11 +193,7 @@ public final class HostUpdateFlow implements TransactionalFlow {
     ImmutableSet.Builder<ImmutableObject> entitiesToInsert = new ImmutableSet.Builder<>();
     ImmutableSet.Builder<ImmutableObject> entitiesToUpdate = new ImmutableSet.Builder<>();
     entitiesToUpdate.add(newHost);
-    // Keep the {@link ForeignKeyIndex} for this host up to date.
     if (isHostRename) {
-      // Update the foreign key for the old host name and save one for the new host name.
-      entitiesToUpdate.add(ForeignKeyIndex.create(existingHost, now));
-      entitiesToUpdate.add(ForeignKeyIndex.create(newHost, newHost.getDeletionTime()));
       updateSuperordinateDomains(existingHost, newHost);
     }
     enqueueTasks(existingHost, newHost);

core/src/main/java/google/registry/model/EntityClasses.java

@@ -25,7 +25,6 @@ import google.registry.model.host.Host;
 import google.registry.model.host.HostHistory;
 import google.registry.model.index.EppResourceIndex;
 import google.registry.model.index.EppResourceIndexBucket;
-import google.registry.model.index.ForeignKeyIndex;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.model.server.ServerSecret;
 
@@ -42,9 +41,6 @@ public final class EntityClasses {
           DomainHistory.class,
           EppResourceIndex.class,
           EppResourceIndexBucket.class,
-          ForeignKeyIndex.ForeignKeyContactIndex.class,
-          ForeignKeyIndex.ForeignKeyDomainIndex.class,
-          ForeignKeyIndex.ForeignKeyHostIndex.class,
           GaeUserIdConverter.class,
           HistoryEntry.class,
           Host.class,

core/src/main/java/google/registry/model/ResourceTransferUtils.java

@@ -23,13 +23,11 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import google.registry.model.EppResource.BuilderWithTransferData;
-import google.registry.model.EppResource.ForeignKeyedEppResource;
 import google.registry.model.EppResource.ResourceWithTransferData;
 import google.registry.model.contact.Contact;
 import google.registry.model.domain.Domain;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.eppcommon.Trid;
-import google.registry.model.index.ForeignKeyIndex;
 import google.registry.model.poll.PendingActionNotificationResponse;
 import google.registry.model.poll.PendingActionNotificationResponse.ContactPendingActionNotificationResponse;
 import google.registry.model.poll.PendingActionNotificationResponse.DomainPendingActionNotificationResponse;
@@ -104,13 +102,6 @@ public final class ResourceTransferUtils {
     checkState(eppResource instanceof Contact || eppResource instanceof Domain);
   }
 
-  /** Update the relevant {@link ForeignKeyIndex} to cache the new deletion time. */
-  public static <R extends EppResource> void updateForeignKeyIndexDeletionTime(R resource) {
-    if (resource instanceof ForeignKeyedEppResource) {
-      tm().insert(ForeignKeyIndex.create(resource, resource.getDeletionTime()));
-    }
-  }
-
   /** If there is a transfer out, delete the server-approve entities and enqueue a poll message. */
   public static <R extends EppResource & ResourceWithTransferData>
       void handlePendingTransferOnDelete(

core/src/main/java/google/registry/model/index/ForeignKeyIndex.java

@@ -19,10 +19,8 @@ import static com.google.common.collect.ImmutableMap.toImmutableMap;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static google.registry.config.RegistryConfig.getEppResourceCachingDuration;
 import static google.registry.config.RegistryConfig.getEppResourceMaxCachedEntries;
-import static google.registry.model.ofy.ObjectifyService.auditedOfy;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.persistence.transaction.TransactionManagerFactory.replicaJpaTm;
-import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.util.CollectionUtils.entriesToImmutableMap;
 import static google.registry.util.TypeUtils.instantiate;
 
@@ -36,17 +34,10 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Multimaps;
 import com.google.common.collect.Streams;
-import com.googlecode.objectify.Key;
-import com.googlecode.objectify.annotation.Entity;
-import com.googlecode.objectify.annotation.Id;
-import com.googlecode.objectify.annotation.Index;
 import google.registry.config.RegistryConfig;
 import google.registry.model.BackupGroupRoot;
 import google.registry.model.CacheUtils;
-import google.registry.model.CacheUtils.AppEngineEnvironmentCacheLoader;
 import google.registry.model.EppResource;
-import google.registry.model.annotations.DeleteAfterMigration;
-import google.registry.model.annotations.ReportedOn;
 import google.registry.model.contact.Contact;
 import google.registry.model.domain.Domain;
 import google.registry.model.host.Host;
@@ -67,22 +58,15 @@ import org.joda.time.DateTime;
  * the foreign key string. The instance is never deleted, but it is updated if a newer entity
  * becomes the active entity.
  */
-@DeleteAfterMigration
 public abstract class ForeignKeyIndex<E extends EppResource> extends BackupGroupRoot {
 
   /** The {@link ForeignKeyIndex} type for {@link Contact} entities. */
-  @ReportedOn
-  @Entity
   public static class ForeignKeyContactIndex extends ForeignKeyIndex<Contact> {}
 
   /** The {@link ForeignKeyIndex} type for {@link Domain} entities. */
-  @ReportedOn
-  @Entity
   public static class ForeignKeyDomainIndex extends ForeignKeyIndex<Domain> {}
 
   /** The {@link ForeignKeyIndex} type for {@link Host} entities. */
-  @ReportedOn
-  @Entity
   public static class ForeignKeyHostIndex extends ForeignKeyIndex<Host> {}
 
   private static final ImmutableBiMap<
@@ -100,23 +84,18 @@ public abstract class ForeignKeyIndex<E extends EppResource> extends BackupGroup
               Domain.class, "fullyQualifiedDomainName",
               Host.class, "fullyQualifiedHostName");
 
-  @Id String foreignKey;
+  String foreignKey;
 
   /**
    * The deletion time of this {@link ForeignKeyIndex}.
    *
-   * <p>This will generally be equal to the deletion time of {@link #topReference}. However, in the
+   * <p>This will generally be equal to the deletion time of {@link #reference}. However, in the
    * case of a {@link Host} that was renamed, this field will hold the time of the rename.
    */
-  @Index DateTime deletionTime;
+  DateTime deletionTime;
 
-  /**
+  /** The referenced resource. */
-   * The referenced resource.
+  VKey<E> reference;
-   *
-   * <p>This field holds a key to the only referenced resource. It is named "topReference" for
-   * historical reasons.
-   */
-  VKey<E> topReference;
 
   public String getForeignKey() {
     return foreignKey;
@@ -127,7 +106,7 @@ public abstract class ForeignKeyIndex<E extends EppResource> extends BackupGroup
   }
 
   public VKey<E> getResourceKey() {
-    return topReference;
+    return reference;
   }
 
   @SuppressWarnings("unchecked")
@@ -137,26 +116,19 @@ public abstract class ForeignKeyIndex<E extends EppResource> extends BackupGroup
   }
 
   /** Create a {@link ForeignKeyIndex} instance for a resource, expiring at a specified time. */
-  public static <E extends EppResource> ForeignKeyIndex<E> create(
-      E resource, DateTime deletionTime) {
   @SuppressWarnings("unchecked")
+  private static <E extends EppResource> ForeignKeyIndex<E> create(
+      E resource, DateTime deletionTime) {
     Class<E> resourceClass = (Class<E>) resource.getClass();
     ForeignKeyIndex<E> instance = instantiate(mapToFkiClass(resourceClass));
-    instance.topReference = (VKey<E>) resource.createVKey();
+    instance.reference = (VKey<E>) resource.createVKey();
     instance.foreignKey = resource.getForeignKey();
     instance.deletionTime = deletionTime;
     return instance;
   }
 
-  /** Create a {@link ForeignKeyIndex} key for a resource. */
-  public static <E extends EppResource> Key<ForeignKeyIndex<E>> createKey(E resource) {
-    @SuppressWarnings("unchecked")
-    Class<E> resourceClass = (Class<E>) resource.getClass();
-    return Key.create(mapToFkiClass(resourceClass), resource.getForeignKey());
-  }
-
   /**
-   * Loads a {@link Key} to an {@link EppResource} from Datastore by foreign key.
+   * Loads a {@link VKey} to an {@link EppResource} from the database by foreign key.
    *
    * <p>Returns null if no foreign key index with this foreign key was ever created, or if the most
    * recently created foreign key index was deleted before time "now". This method does not actually
@@ -172,7 +144,7 @@ public abstract class ForeignKeyIndex<E extends EppResource> extends BackupGroup
   public static <E extends EppResource> VKey<E> loadAndGetKey(
       Class<E> clazz, String foreignKey, DateTime now) {
     ForeignKeyIndex<E> index = load(clazz, foreignKey, now);
-    return (index == null) ? null : index.getResourceKey();
+    return index == null ? null : index.getResourceKey();
   }
 
   /**
@@ -196,7 +168,7 @@ public abstract class ForeignKeyIndex<E extends EppResource> extends BackupGroup
    */
   public static <E extends EppResource> ImmutableMap<String, ForeignKeyIndex<E>> load(
       Class<E> clazz, Collection<String> foreignKeys, final DateTime now) {
-    return loadIndexesFromStore(clazz, foreignKeys, true, false).entrySet().stream()
+    return loadIndexesFromStore(clazz, foreignKeys, false).entrySet().stream()
         .filter(e -> now.isBefore(e.getValue().getDeletionTime()))
         .collect(entriesToImmutableMap());
   }
@@ -206,26 +178,10 @@ public abstract class ForeignKeyIndex<E extends EppResource> extends BackupGroup
    * keys, regardless of whether or not they have been soft-deleted.
    *
    * <p>Used by both the cached (w/o deletion check) and the non-cached (with deletion check) calls.
-   *
-   * <p>Note that in the cached case, we wish to run this outside of any transaction because we may
-   * be loading many entities, going over the Datastore limit on the number of enrolled entity
-   * groups per transaction (25). If we require consistency, however, we must use a transaction.
-   *
-   * @param inTransaction whether or not to use an Objectify transaction
    */
   private static <E extends EppResource>
       ImmutableMap<String, ForeignKeyIndex<E>> loadIndexesFromStore(
-          Class<E> clazz,
+          Class<E> clazz, Collection<String> foreignKeys, boolean useReplicaJpaTm) {
-          Collection<String> foreignKeys,
-          boolean inTransaction,
-          boolean useReplicaJpaTm) {
-    if (tm().isOfy()) {
-      Class<ForeignKeyIndex<E>> fkiClass = mapToFkiClass(clazz);
-      return ImmutableMap.copyOf(
-          inTransaction
-              ? auditedOfy().load().type(fkiClass).ids(foreignKeys)
-              : tm().doTransactionless(() -> auditedOfy().load().type(fkiClass).ids(foreignKeys)));
-    } else {
     String property = RESOURCE_CLASS_TO_FKI_PROPERTY.get(clazz);
     JpaTransactionManager jpaTmToUse = useReplicaJpaTm ? replicaJpaTm() : jpaTm();
     ImmutableList<ForeignKeyIndex<E>> indexes =
@@ -237,7 +193,7 @@ public abstract class ForeignKeyIndex<E extends EppResource> extends BackupGroup
                             .whereFieldIsIn(property, foreignKeys)
                             .build())
                     .getResultStream()
-                      .map(e -> ForeignKeyIndex.create(e, e.getDeletionTime()))
+                    .map(e -> create(e, e.getDeletionTime()))
                     .collect(toImmutableList()));
     // We need to find and return the entities with the maximum deletionTime for each foreign key.
     return Multimaps.index(indexes, ForeignKeyIndex::getForeignKey).asMap().entrySet().stream()
@@ -250,11 +206,9 @@ public abstract class ForeignKeyIndex<E extends EppResource> extends BackupGroup
                         .get()))
         .collect(entriesToImmutableMap());
   }
-  }
 
   static final CacheLoader<VKey<ForeignKeyIndex<?>>, Optional<ForeignKeyIndex<?>>> CACHE_LOADER =
-      new AppEngineEnvironmentCacheLoader<
+      new CacheLoader<VKey<ForeignKeyIndex<?>>, Optional<ForeignKeyIndex<?>>>() {
-          VKey<ForeignKeyIndex<?>>, Optional<ForeignKeyIndex<?>>>() {
 
         @Override
         public Optional<ForeignKeyIndex<?>> load(VKey<ForeignKeyIndex<?>> key) {
@@ -263,7 +217,6 @@ public abstract class ForeignKeyIndex<E extends EppResource> extends BackupGroup
               loadIndexesFromStore(
                       RESOURCE_CLASS_TO_FKI_CLASS.inverse().get(key.getKind()),
                       ImmutableSet.of(foreignKey),
-                      false,
                       true)
                   .get(foreignKey));
         }
@@ -280,7 +233,7 @@ public abstract class ForeignKeyIndex<E extends EppResource> extends BackupGroup
               Streams.stream(keys).map(v -> v.getSqlKey().toString()).collect(toImmutableSet());
           ImmutableSet<VKey<ForeignKeyIndex<?>>> typedKeys = ImmutableSet.copyOf(keys);
           ImmutableMap<String, ? extends ForeignKeyIndex<? extends EppResource>> existingFkis =
-              loadIndexesFromStore(resourceClass, foreignKeys, false, true);
+              loadIndexesFromStore(resourceClass, foreignKeys, true);
           // ofy omits keys that don't have values in Datastore, so re-add them in
           // here with Optional.empty() values.
           return Maps.asMap(
@@ -333,14 +286,14 @@ public abstract class ForeignKeyIndex<E extends EppResource> extends BackupGroup
   public static <E extends EppResource> ImmutableMap<String, ForeignKeyIndex<E>> loadCached(
       Class<E> clazz, Collection<String> foreignKeys, final DateTime now) {
     if (!RegistryConfig.isEppResourceCachingEnabled()) {
-      return tm().doTransactionless(() -> load(clazz, foreignKeys, now));
+      return load(clazz, foreignKeys, now);
     }
     Class<? extends ForeignKeyIndex<?>> fkiClass = mapToFkiClass(clazz);
     // Safe to cast VKey<FKI<E>> to VKey<FKI<?>>
     @SuppressWarnings("unchecked")
     ImmutableList<VKey<ForeignKeyIndex<?>>> fkiVKeys =
         foreignKeys.stream()
-            .map(fk -> (VKey<ForeignKeyIndex<?>>) VKey.create(fkiClass, fk))
+            .map(fk -> (VKey<ForeignKeyIndex<?>>) VKey.createSql(fkiClass, fk))
             .collect(toImmutableList());
     // This cast is safe because when we loaded ForeignKeyIndexes above we used type clazz, which
     // is scoped to E.

core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java

@@ -31,9 +31,6 @@ import com.google.common.collect.Streams;
 import com.google.common.flogger.FluentLogger;
 import google.registry.model.ImmutableObject;
 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.persistence.JpaRetries;
 import google.registry.persistence.VKey;
 import google.registry.util.Clock;
@@ -83,11 +80,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
   // to exclude the Datastore specific entities when the underlying tm() is jpaTm().
   // TODO(b/176108270): Remove this property after database migration.
   private static final ImmutableSet<Class<? extends ImmutableObject>> IGNORED_ENTITY_CLASSES =
-      ImmutableSet.of(
+      ImmutableSet.of(EppResourceIndex.class);
-          EppResourceIndex.class,
-          ForeignKeyContactIndex.class,
-          ForeignKeyDomainIndex.class,
-          ForeignKeyHostIndex.class);
 
   // EntityManagerFactory is thread safe.
   private final EntityManagerFactory emf;

core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java

@@ -508,7 +508,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Host> {
                 .asBuilder()
                 .setLastTransferTime(lastTransferTime)
                 .build());
-    // Set the new domain to have a last transfer time that is different than the last transfer
+    // Set the new domain to have a last transfer time that is different from the last transfer
     // time on the host in question.
     persistResource(
         DatabaseHelper.newDomain("example.tld")
@@ -546,7 +546,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Host> {
                 .asBuilder()
                 .setLastTransferTime(clock.nowUtc().minusDays(5))
                 .build());
-    // Set the new domain to have a last transfer time that is different than the last transfer
+    // Set the new domain to have a last transfer time that is different from the last transfer
     // time on the host in question.
     persistResource(
         DatabaseHelper.newDomain("example.tld")
@@ -814,7 +814,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Host> {
         clock.nowUtc().minusDays(2), clock.nowUtc().minusDays(3));
   }
 
-  /** Test when the new superdordinate domain has never been transferred before. */
+  /** Test when the new superordinate domain has never been transferred before. */
   @Test
   void testSuccess_externalToSubord_lastTransferTimeNotOverridden_whenNull() throws Exception {
     doExternalToInternalLastTransferTimeTest(clock.nowUtc().minusDays(2), null);

core/src/test/java/google/registry/model/common/ClassPathManagerTest.java

@@ -23,9 +23,6 @@ import google.registry.model.domain.DomainHistory;
 import google.registry.model.host.Host;
 import google.registry.model.index.EppResourceIndex;
 import google.registry.model.index.EppResourceIndexBucket;
-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.reporting.HistoryEntry;
 import google.registry.model.server.ServerSecret;
 import google.registry.testing.TestObject;
@@ -44,8 +41,6 @@ public class ClassPathManagerTest {
      * below are all classes supported in CLASS_REGISTRY. This test breaks if someone changes a
      * classname without preserving the original name.
      */
-    assertThat(ClassPathManager.getClass("ForeignKeyContactIndex"))
-        .isEqualTo(ForeignKeyContactIndex.class);
     assertThat(ClassPathManager.getClass("Host")).isEqualTo(Host.class);
     assertThat(ClassPathManager.getClass("Contact")).isEqualTo(Contact.class);
     assertThat(ClassPathManager.getClass("GaeUserIdConverter")).isEqualTo(GaeUserIdConverter.class);
@@ -53,12 +48,8 @@ public class ClassPathManagerTest {
         .isEqualTo(EppResourceIndexBucket.class);
     assertThat(ClassPathManager.getClass("Domain")).isEqualTo(Domain.class);
     assertThat(ClassPathManager.getClass("HistoryEntry")).isEqualTo(HistoryEntry.class);
-    assertThat(ClassPathManager.getClass("ForeignKeyHostIndex"))
-        .isEqualTo(ForeignKeyHostIndex.class);
     assertThat(ClassPathManager.getClass("ServerSecret")).isEqualTo(ServerSecret.class);
     assertThat(ClassPathManager.getClass("EppResourceIndex")).isEqualTo(EppResourceIndex.class);
-    assertThat(ClassPathManager.getClass("ForeignKeyDomainIndex"))
-        .isEqualTo(ForeignKeyDomainIndex.class);
   }
 
   @Test
@@ -91,8 +82,6 @@ public class ClassPathManagerTest {
      * The classes below are all classes supported in CLASS_NAME_REGISTRY. This test breaks if
      * someone changes a classname without preserving the original name.
      */
-    assertThat(ClassPathManager.getClassName(ForeignKeyContactIndex.class))
-        .isEqualTo("ForeignKeyContactIndex");
     assertThat(ClassPathManager.getClassName(Host.class)).isEqualTo("Host");
     assertThat(ClassPathManager.getClassName(Contact.class)).isEqualTo("Contact");
     assertThat(ClassPathManager.getClassName(GaeUserIdConverter.class))
@@ -101,12 +90,8 @@ public class ClassPathManagerTest {
         .isEqualTo("EppResourceIndexBucket");
     assertThat(ClassPathManager.getClassName(Domain.class)).isEqualTo("Domain");
     assertThat(ClassPathManager.getClassName(HistoryEntry.class)).isEqualTo("HistoryEntry");
-    assertThat(ClassPathManager.getClassName(ForeignKeyHostIndex.class))
-        .isEqualTo("ForeignKeyHostIndex");
     assertThat(ClassPathManager.getClassName(ServerSecret.class)).isEqualTo("ServerSecret");
     assertThat(ClassPathManager.getClassName(EppResourceIndex.class)).isEqualTo("EppResourceIndex");
-    assertThat(ClassPathManager.getClassName(ForeignKeyDomainIndex.class))
-        .isEqualTo("ForeignKeyDomainIndex");
   }
 
   @Test

core/src/test/java/google/registry/model/index/ForeignKeyIndexTest.java

@@ -15,17 +15,13 @@
 package google.registry.model.index;
 
 import static com.google.common.truth.Truth.assertThat;
-import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.testing.DatabaseHelper.createTld;
 import static google.registry.testing.DatabaseHelper.persistActiveHost;
 import static google.registry.testing.DatabaseHelper.persistResource;
 
 import com.google.common.collect.ImmutableList;
 import google.registry.model.EntityTestCase;
-import google.registry.model.domain.Domain;
 import google.registry.model.host.Host;
-import google.registry.model.index.ForeignKeyIndex.ForeignKeyHostIndex;
-import google.registry.testing.DatabaseHelper;
 import google.registry.testing.TestCacheExtension;
 import java.time.Duration;
 import org.junit.jupiter.api.BeforeEach;
@@ -44,16 +40,6 @@ class ForeignKeyIndexTest extends EntityTestCase {
     createTld("com");
   }
 
-  @Test
-  void testModifyForeignKeyIndex_notThrowExceptionInSql() {
-    Domain domain = DatabaseHelper.newDomain("test.com");
-    ForeignKeyIndex<Domain> fki = ForeignKeyIndex.create(domain, fakeClock.nowUtc());
-    tm().transact(() -> tm().insert(fki));
-    tm().transact(() -> tm().put(fki));
-    tm().transact(() -> tm().delete(fki));
-    tm().transact(() -> tm().update(fki));
-  }
-
   @Test
   void testLoadForNonexistentForeignKey_returnsNull() {
     assertThat(ForeignKeyIndex.load(Host.class, "ns1.example.com", fakeClock.nowUtc())).isNull();
@@ -62,11 +48,7 @@ class ForeignKeyIndexTest extends EntityTestCase {
   @Test
   void testLoadForDeletedForeignKey_returnsNull() {
     Host host = persistActiveHost("ns1.example.com");
-    if (tm().isOfy()) {
-      persistResource(ForeignKeyIndex.create(host, fakeClock.nowUtc().minusDays(1)));
-    } else {
     persistResource(host.asBuilder().setDeletionTime(fakeClock.nowUtc().minusDays(1)).build());
-    }
     assertThat(ForeignKeyIndex.load(Host.class, "ns1.example.com", fakeClock.nowUtc())).isNull();
   }
 
@@ -74,15 +56,7 @@ class ForeignKeyIndexTest extends EntityTestCase {
   void testLoad_newerKeyHasBeenSoftDeleted() {
     Host host1 = persistActiveHost("ns1.example.com");
     fakeClock.advanceOneMilli();
-    if (tm().isOfy()) {
-      ForeignKeyHostIndex fki = new ForeignKeyHostIndex();
-      fki.foreignKey = "ns1.example.com";
-      fki.topReference = host1.createVKey();
-      fki.deletionTime = fakeClock.nowUtc();
-      persistResource(fki);
-    } else {
     persistResource(host1.asBuilder().setDeletionTime(fakeClock.nowUtc()).build());
-    }
     assertThat(ForeignKeyIndex.load(Host.class, "ns1.example.com", fakeClock.nowUtc())).isNull();
   }
 
@@ -90,11 +64,7 @@ class ForeignKeyIndexTest extends EntityTestCase {
   void testBatchLoad_skipsDeletedAndNonexistent() {
     persistActiveHost("ns1.example.com");
     Host host = persistActiveHost("ns2.example.com");
-    if (tm().isOfy()) {
-      persistResource(ForeignKeyIndex.create(host, fakeClock.nowUtc().minusDays(1)));
-    } else {
     persistResource(host.asBuilder().setDeletionTime(fakeClock.nowUtc().minusDays(1)).build());
-    }
     assertThat(
             ForeignKeyIndex.load(
                     Host.class,

core/src/test/java/google/registry/testing/DatabaseHelper.java

@@ -63,7 +63,6 @@ import com.googlecode.objectify.Key;
 import google.registry.dns.writer.VoidDnsWriter;
 import google.registry.model.Buildable;
 import google.registry.model.EppResource;
-import google.registry.model.EppResource.ForeignKeyedEppResource;
 import google.registry.model.EppResourceUtils;
 import google.registry.model.ImmutableObject;
 import google.registry.model.billing.BillingEvent;
@@ -91,7 +90,6 @@ import google.registry.model.eppcommon.Trid;
 import google.registry.model.host.Host;
 import google.registry.model.index.EppResourceIndex;
 import google.registry.model.index.EppResourceIndexBucket;
-import google.registry.model.index.ForeignKeyIndex;
 import google.registry.model.poll.PollMessage;
 import google.registry.model.pricing.StaticPremiumListPricingEngine;
 import google.registry.model.registrar.Registrar;
@@ -1011,9 +1009,6 @@ public class DatabaseHelper {
         .that(resource.getRepoId())
         .isNotEmpty();
     saver.accept(index);
-    if (resource instanceof ForeignKeyedEppResource) {
-      saver.accept(ForeignKeyIndex.create(resource, resource.getDeletionTime()));
-    }
   }
 
   /** Persists an object in the DB for tests. */

core/src/test/resources/google/registry/model/schema.txt

@@ -294,21 +294,6 @@ class google.registry.model.index.EppResourceIndex {
 class google.registry.model.index.EppResourceIndexBucket {
   @Id long bucketId;
 }
-class google.registry.model.index.ForeignKeyIndex$ForeignKeyContactIndex {
-  @Id java.lang.String foreignKey;
-  google.registry.persistence.VKey<E> topReference;
-  org.joda.time.DateTime deletionTime;
-}
-class google.registry.model.index.ForeignKeyIndex$ForeignKeyDomainIndex {
-  @Id java.lang.String foreignKey;
-  google.registry.persistence.VKey<E> topReference;
-  org.joda.time.DateTime deletionTime;
-}
-class google.registry.model.index.ForeignKeyIndex$ForeignKeyHostIndex {
-  @Id java.lang.String foreignKey;
-  google.registry.persistence.VKey<E> topReference;
-  org.joda.time.DateTime deletionTime;
-}
 class google.registry.model.reporting.DomainTransactionRecord {
   google.registry.model.reporting.DomainTransactionRecord$TransactionReportField reportField;
   java.lang.Integer reportAmount;

util/src/main/java/google/registry/util/TypeUtils.java

@@ -28,7 +28,9 @@ import java.lang.reflect.Modifier;
 import java.util.function.Predicate;
 
 /** Utilities methods related to reflection. */
-public class TypeUtils {
+public final class TypeUtils {
+
+  private TypeUtils() {}
 
   /** A {@code TypeToken} that removes an ugly cast in the common cases of getting a known type. */
   public static class TypeInstantiator<T> extends TypeToken<T> {
@@ -47,7 +49,8 @@ public class TypeUtils {
   }
 
   public static <T> T instantiate(Class<? extends T> clazz) {
-    checkArgument(Modifier.isPublic(clazz.getModifiers()),
+    checkArgument(
+        Modifier.isPublic(clazz.getModifiers()),
         "AppEngine's custom security manager won't let us reflectively access non-public types");
     try {
       return clazz.getConstructor().newInstance();
@@ -59,14 +62,15 @@ public class TypeUtils {
   /**
    * Instantiate a class with the specified constructor argument.
    *
-   * <p>Because we use arg1's type to lookup the constructor, this only works if arg1's class is
+   * <p>Because we use {@code arg}'s type to look up the constructor, this only works if arg1's
-   * exactly the same type as the constructor argument. Subtypes are not allowed.
+   * class is exactly the same type as the constructor argument. Subtypes are not allowed.
    */
-  public static <T, U> T instantiate(Class<? extends T> clazz, U arg1) {
+  public static <T, U> T instantiate(Class<? extends T> clazz, U arg) {
-    checkArgument(Modifier.isPublic(clazz.getModifiers()),
+    checkArgument(
+        Modifier.isPublic(clazz.getModifiers()),
         "AppEngine's custom security manager won't let us reflectively access non-public types");
     try {
-      return clazz.getConstructor(arg1.getClass()).newInstance(arg1);
+      return clazz.getConstructor(arg.getClass()).newInstance(arg);
     } catch (ReflectiveOperationException e) {
       throw new RuntimeException(e);
     }
@@ -106,8 +110,10 @@ public class TypeUtils {
           T enumField = (T) field.get(null);
           builder.put(field.getName(), enumField);
         } catch (IllegalArgumentException | IllegalAccessException e) {
-          throw new RuntimeException(String.format(
+          throw new RuntimeException(
-              "Could not retrieve static final field mapping for %s", clazz.getName()), e);
+              String.format(
+                  "Could not retrieve static final field mapping for %s", clazz.getName()),
+              e);
         }
       }
     }
@@ -115,8 +121,7 @@ public class TypeUtils {
   }
 
   /** Returns a predicate that tests whether classes are annotated with the given annotation. */
-  public static Predicate<Class<?>> hasAnnotation(
+  public static Predicate<Class<?>> hasAnnotation(final Class<? extends Annotation> annotation) {
-      final Class<? extends Annotation> annotation) {
     return clazz -> clazz.isAnnotationPresent(annotation);
   }