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.

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

@@ -106,7 +106,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Host> {
   }
 
   /**
-   * Setup a domain with a transfer that should have been server approved a day ago.
+   * Set up a domain with a transfer that should have been server approved a day ago.
    *
    * <p>The transfer is from "TheRegistrar" to "NewRegistrar".
    */
@@ -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());
-    }
+    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());
-    }
+    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());
-    }
+    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;