Remove Ofy (#1863)

So long, farewell, adios, ciao, sayonara, 再见!

TESTED=deployed to alpha and used `nomulus list_tlds` to confirm that the web app can receive and serve requests.

<!-- Reviewable:start -->
- - -
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/google/nomulus/1863)
<!-- Reviewable:end -->

core/src/test/java/google/registry/beam/TestPipelineExtension.java

@@ -20,7 +20,6 @@
 package google.registry.beam;
 
 import static com.google.common.base.Preconditions.checkState;
-import static com.google.common.truth.Truth.assertWithMessage;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
 
@@ -28,7 +27,6 @@ import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.base.Strings;
 import com.google.common.collect.Maps;
 import java.io.IOException;
-import java.lang.reflect.InvocationTargetException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -298,15 +296,6 @@ public class TestPipelineExtension extends Pipeline
       enableAbandonedNodeEnforcement(true);
     }
 
-    // Clear this property so that when default Guava ThreadFactory is created
-    // it will not think that it is in App Engine and return an unusable
-    // ThreadFactory.
-    System.clearProperty("com.google.appengine.runtime.environment");
-    assertWithMessage(
-            "Beam pipelines don't run in an App Engine environment, and thus"
-                + " the tests shouldn't be mocking one either.")
-        .that(isAppEngine())
-        .isFalse();
   }
 
   @Override
@@ -512,25 +501,6 @@ public class TestPipelineExtension extends Pipeline
     }
   }
 
-  // Adapted from Guava's MoreExecutors (where it is a private method)
-  private static boolean isAppEngine() {
-    if (System.getProperty("com.google.appengine.runtime.environment") == null) {
-      return false;
-    } else {
-      try {
-        return Class.forName("com.google.apphosting.api.ApiProxy")
-                .getMethod("getCurrentEnvironment")
-                .invoke(null)
-            != null;
-      } catch (ClassNotFoundException
-          | InvocationTargetException
-          | IllegalAccessException
-          | NoSuchMethodException e) {
-        return false;
-      }
-    }
-  }
-
   private static class IsEmptyVisitor extends PipelineVisitor.Defaults {
     private boolean empty = true;
 

core/src/test/java/google/registry/model/ImmutableObjectTest.java

@@ -25,9 +25,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
-import com.googlecode.objectify.Key;
-import com.googlecode.objectify.annotation.Entity;
-import com.googlecode.objectify.annotation.Id;
+import google.registry.persistence.VKey;
 import google.registry.testing.AppEngineExtension;
 import google.registry.util.CidrAddressBlock;
 import java.lang.reflect.Field;
@@ -38,6 +36,8 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import javax.persistence.Entity;
+import javax.persistence.Id;
 import org.joda.time.DateTime;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -50,7 +50,6 @@ public class ImmutableObjectTest {
       AppEngineExtension.builder()
           .withCloudSql()
           .withJpaUnitTestEntities(ValueObject.class)
-          .withOfyTestEntities(ValueObject.class)
           .build();
 
   /** Simple subclass of ImmutableObject. */
@@ -266,21 +265,19 @@ public class ImmutableObjectTest {
   /** Subclass of ImmutableObject with keys to other objects. */
   public static class RootObject extends ImmutableObject {
 
-    Key<ValueObject> hydrateMe;
+    VKey<ValueObject> hydrateMe;
 
-    @DoNotHydrate
-    Key<ValueObject> skipMe;
+    @DoNotHydrate VKey<ValueObject> skipMe;
 
-    Map<String, Key<ValueObject>> map;
+    Map<String, VKey<ValueObject>> map;
 
-    Set<Key<ValueObject>> set;
+    Set<VKey<ValueObject>> set;
   }
 
   /** Simple subclass of ImmutableObject. */
   @Entity
-  @javax.persistence.Entity
   public static class ValueObject extends ImmutableObject {
-    @Id @javax.persistence.Id long id;
+    @Id long id;
 
     String value;
 

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

@@ -1,52 +0,0 @@
-// Copyright 2017 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.common;
-
-import static com.google.common.truth.Truth.assertThat;
-import static google.registry.model.ofy.ObjectifyService.auditedOfy;
-
-import google.registry.testing.AppEngineExtension;
-import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.RegisterExtension;
-
-/** Unit tests for {@link GaeUserIdConverter}. */
-public class GaeUserIdConverterTest {
-
-  @RegisterExtension
-  public final AppEngineExtension appEngine = AppEngineExtension.builder().withCloudSql().build();
-
-  @AfterEach
-  void verifyNoLingeringEntities() {
-    assertThat(auditedOfy().load().type(GaeUserIdConverter.class).count()).isEqualTo(0);
-  }
-
-  @Test
-  void testSuccess() {
-    assertThat(GaeUserIdConverter.convertEmailAddressToGaeUserId("example@example.com"))
-        .matches("[0-9]+");
-  }
-
-  @Test
-  void testSuccess_inTransaction() {
-    auditedOfy()
-        .transactNew(
-            () -> {
-              assertThat(GaeUserIdConverter.convertEmailAddressToGaeUserId("example@example.com"))
-                  .matches("[0-9]+");
-              return null;
-            });
-  }
-}

core/src/test/java/google/registry/model/ofy/ObjectifyServiceTest.java

@@ -1,32 +0,0 @@
-// Copyright 2017 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.ofy;
-
-import google.registry.testing.AppEngineExtension;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.RegisterExtension;
-
-/** Tests for our replacement for ObjectifyService. */
-public class ObjectifyServiceTest {
-
-  @RegisterExtension
-  public final AppEngineExtension appEngine = AppEngineExtension.builder().withCloudSql().build();
-
-  @Test
-  void test_initOfy_canBeCalledTwice() {
-    ObjectifyService.initOfy();
-    ObjectifyService.initOfy();
-  }
-}

core/src/test/java/google/registry/model/ofy/OfyFilterTest.java

@@ -1,104 +0,0 @@
-// Copyright 2017 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.ofy;
-
-import static com.google.common.truth.Truth.assertThat;
-import static google.registry.model.ofy.ObjectifyService.initOfy;
-import static org.junit.jupiter.api.Assertions.assertThrows;
-
-import com.google.appengine.tools.development.testing.LocalDatastoreServiceTestConfig;
-import com.google.appengine.tools.development.testing.LocalServiceTestHelper;
-import com.googlecode.objectify.Key;
-import com.googlecode.objectify.ObjectifyFactory;
-import com.googlecode.objectify.ObjectifyFilter;
-import com.googlecode.objectify.ObjectifyService;
-import com.googlecode.objectify.annotation.Entity;
-import com.googlecode.objectify.annotation.Id;
-import google.registry.model.common.GaeUserIdConverter;
-import google.registry.persistence.transaction.JpaTestExtensions;
-import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension;
-import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.RegisterExtension;
-
-/** Tests for our replacement Objectify filter. */
-class OfyFilterTest {
-
-  private LocalServiceTestHelper helper;
-  private ObjectifyFactory factory;
-
-  // We can't use AppEngineExtension, because it triggers the precise behavior that we are testing.
-
-  @BeforeEach
-  void beforeEach() {
-    helper = new LocalServiceTestHelper(new LocalDatastoreServiceTestConfig()).setUp();
-    // Clear out the factory so that it requires re-registration on each test method.
-    // Otherwise, static registration of types in one method would persist across methods.
-    initOfy();
-    factory = ObjectifyService.factory();
-    ObjectifyService.setFactory(new ObjectifyFactory(false));
-  }
-
-  @AfterEach
-  void afterEach() {
-    ObjectifyFilter.complete();
-    ObjectifyService.setFactory(factory);
-    ObjectifyFilter.complete();
-    helper.tearDown();
-  }
-
-  @RegisterExtension
-  final JpaIntegrationTestExtension database =
-      new JpaTestExtensions.Builder().buildIntegrationTestExtension();
-
-  /**
-   * Key.create looks up kind metadata for the class of the object it is given. If this happens
-   * before the first reference to ObjectifyService, which statically triggers type registrations,
-   * then the create will fail. Note that this is only a problem if the type in question doesn't
-   * call ObjectifyService.allocateId() inside its own builder or create method, since if it does
-   * that would trigger the statics as well. In this example, Registrar has a string id, so the bug
-   * occurs, were it not for OfyFilter.
-   */
-  @Test
-  void testFilterRegistersTypes() {
-    UnregisteredEntity entity = new UnregisteredEntity(5L);
-    IllegalStateException e = assertThrows(IllegalStateException.class, () -> Key.create(entity));
-    assertThat(e)
-        .hasMessageThat()
-        .isEqualTo(
-            "class google.registry.model.ofy.OfyFilterTest$UnregisteredEntity "
-                + "has not been registered");
-  }
-
-  /** The filter should register all types for us. */
-  @Test
-  void testKeyCreateAfterFilter() {
-    new OfyFilter().init(null);
-    GaeUserIdConverter userIdConverter = new GaeUserIdConverter();
-    userIdConverter.id = 1;
-    Key.create(userIdConverter);
-  }
-
-  @Entity
-  private static class UnregisteredEntity {
-
-    @Id long id;
-
-    UnregisteredEntity(long id) {
-      this.id = id;
-    }
-  }
-}

core/src/test/java/google/registry/server/RegistryTestServer.java

@@ -21,13 +21,10 @@ import static google.registry.util.BuildPathUtils.getResourcesDir;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.net.HostAndPort;
-import com.googlecode.objectify.ObjectifyFilter;
-import google.registry.model.ofy.OfyFilter;
 import google.registry.module.backend.BackendServlet;
 import google.registry.module.frontend.FrontendServlet;
 import java.net.URL;
 import java.nio.file.Path;
-import javax.servlet.Filter;
 
 /** Lightweight HTTP server for testing the Nomulus Admin and Registrar consoles. */
 public final class RegistryTestServer {
@@ -87,15 +84,11 @@ public final class RegistryTestServer {
           route("/registry-lock-post", FrontendServlet.class),
           route("/registry-lock-verify", FrontendServlet.class));
 
-  private static final ImmutableList<Class<? extends Filter>> FILTERS = ImmutableList.of(
-      ObjectifyFilter.class,
-      OfyFilter.class);
-
   private final TestServer server;
 
   /** @see TestServer#TestServer(HostAndPort, ImmutableMap, ImmutableList, ImmutableList) */
   public RegistryTestServer(HostAndPort address) {
-    server = new TestServer(address, RUNFILES, ROUTES, FILTERS);
+    server = new TestServer(address, RUNFILES, ROUTES);
   }
 
   /** @see TestServer#start() */

core/src/test/java/google/registry/server/ServletWrapperDelegatorServlet.java

@@ -18,20 +18,14 @@ import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Throwables.throwIfInstanceOf;
 import static google.registry.util.TypeUtils.instantiate;
 
-import com.google.common.collect.ImmutableList;
 import com.google.common.util.concurrent.Uninterruptibles;
 import java.io.IOException;
-import java.util.Iterator;
 import java.util.Queue;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.FutureTask;
 import javax.annotation.Nullable;
-import javax.servlet.Filter;
-import javax.servlet.FilterChain;
 import javax.servlet.ServletException;
-import javax.servlet.ServletRequest;
-import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -39,49 +33,33 @@ import javax.servlet.http.HttpServletResponse;
 /**
  * Servlet that wraps a servlet and delegates request execution to a queue.
  *
- * <p>The actual invocation of the delegate does not happen within this servlet's lifecycle.
- * Therefore, the task on the queue must manually invoke filters within the queue task.
- *
  * @see TestServer
  */
 public final class ServletWrapperDelegatorServlet extends HttpServlet {
 
   private final Queue<FutureTask<Void>> requestQueue;
   private final Class<? extends HttpServlet> servletClass;
-  private final ImmutableList<Class<? extends Filter>> filterClasses;
 
   ServletWrapperDelegatorServlet(
       Class<? extends HttpServlet> servletClass,
-      ImmutableList<Class<? extends Filter>> filterClasses,
       Queue<FutureTask<Void>> requestQueue) {
     this.servletClass = servletClass;
-    this.filterClasses = filterClasses;
     this.requestQueue = checkNotNull(requestQueue, "requestQueue");
   }
 
   @Override
   public void service(final HttpServletRequest req, final HttpServletResponse rsp)
       throws ServletException, IOException {
-    FutureTask<Void> task = new FutureTask<>(new Callable<Void>() {
-      @Nullable
-      @Override
-      public Void call() throws ServletException, IOException {
-        // Simulate the full filter chain with the servlet at the end.
-        final Iterator<Class<? extends Filter>> filtersIter = filterClasses.iterator();
-        FilterChain filterChain =
-            new FilterChain() {
+    FutureTask<Void> task =
+        new FutureTask<>(
+            new Callable<Void>() {
+              @Nullable
               @Override
-              public void doFilter(ServletRequest request, ServletResponse response)
-                  throws IOException, ServletException {
-                if (filtersIter.hasNext()) {
-                  instantiate(filtersIter.next()).doFilter(request, response, this);
-                } else {
-                  instantiate(servletClass).service(request, response);
-                }
-              }};
-        filterChain.doFilter(req, rsp);
-        return null;
-      }});
+              public Void call() throws ServletException, IOException {
+                instantiate(servletClass).service(req, rsp);
+                return null;
+              }
+            });
     requestQueue.add(task);
     try {
       Uninterruptibles.getUninterruptibly(task);

core/src/test/java/google/registry/server/TestServer.java

@@ -34,7 +34,6 @@ import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.FutureTask;
 import java.util.concurrent.LinkedBlockingDeque;
 import java.util.concurrent.TimeUnit;
-import javax.servlet.Filter;
 import javax.servlet.http.HttpServlet;
 import org.mortbay.jetty.Connector;
 import org.mortbay.jetty.Server;
@@ -85,13 +84,10 @@ public final class TestServer {
    * @param routes list of servlet endpoints
    */
   public TestServer(
-      HostAndPort address,
-      ImmutableMap<String, Path> runfiles,
-      ImmutableList<Route> routes,
-      ImmutableList<Class<? extends Filter>> filters) {
+      HostAndPort address, ImmutableMap<String, Path> runfiles, ImmutableList<Route> routes) {
     urlAddress = createUrlAddress(address);
     server.addConnector(createConnector(address));
-    server.addHandler(createHandler(runfiles, routes, filters));
+    server.addHandler(createHandler(runfiles, routes));
   }
 
   /** Starts the HTTP server in a new thread and returns once it's online. */
@@ -156,10 +152,7 @@ public final class TestServer {
     }
   }
 
-  private Context createHandler(
-      Map<String, Path> runfiles,
-      ImmutableList<Route> routes,
-      ImmutableList<Class<? extends Filter>> filters) {
+  private Context createHandler(Map<String, Path> runfiles, ImmutableList<Route> routes) {
     Context context = new Context(server, CONTEXT_PATH, Context.SESSIONS);
     context.addServlet(new ServletHolder(HealthzServlet.class), "/healthz");
     for (Map.Entry<String, Path> runfile : runfiles.entrySet()) {
@@ -168,8 +161,7 @@ public final class TestServer {
           runfile.getKey());
     }
     for (Route route : routes) {
-      context.addServlet(
-          new ServletHolder(wrapServlet(route.servletClass(), filters)), route.path());
+      context.addServlet(new ServletHolder(wrapServlet(route.servletClass())), route.path());
     }
     ServletHolder holder = new ServletHolder(DefaultServlet.class);
     holder.setInitParameter("aliases", "1");
@@ -177,9 +169,8 @@ public final class TestServer {
     return context;
   }
 
-  private HttpServlet wrapServlet(
-      Class<? extends HttpServlet> servletClass, ImmutableList<Class<? extends Filter>> filters) {
-    return new ServletWrapperDelegatorServlet(servletClass, filters, requestQueue);
+  private HttpServlet wrapServlet(Class<? extends HttpServlet> servletClass) {
+    return new ServletWrapperDelegatorServlet(servletClass, requestQueue);
   }
 
   private static Connector createConnector(HostAndPort address) {

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

@@ -39,9 +39,6 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import com.google.common.io.Files;
-import com.googlecode.objectify.Key;
-import com.googlecode.objectify.ObjectifyFilter;
-import google.registry.model.ofy.ObjectifyService;
 import google.registry.model.registrar.Registrar;
 import google.registry.model.registrar.Registrar.State;
 import google.registry.model.registrar.RegistrarAddress;
@@ -135,7 +132,6 @@ public final class AppEngineExtension implements BeforeEachCallback, AfterEachCa
   private UserInfo userInfo;
 
   // Test Objectify entity classes to be used with this AppEngineExtension instance.
-  private ImmutableList<Class<?>> ofyTestEntities;
   private ImmutableList<Class<?>> jpaTestEntities;
 
   public Optional<JpaIntegrationTestExtension> getJpaIntegrationTestExtension() {
@@ -146,7 +142,6 @@ public final class AppEngineExtension implements BeforeEachCallback, AfterEachCa
   public static class Builder {
 
     private AppEngineExtension extension = new AppEngineExtension();
-    private ImmutableList.Builder<Class<?>> ofyTestEntities = new ImmutableList.Builder<>();
     private ImmutableList.Builder<Class<?>> jpaTestEntities = new ImmutableList.Builder<>();
 
     /** Turns on Cloud SQL only, for use by test data generators. */
@@ -205,24 +200,6 @@ public final class AppEngineExtension implements BeforeEachCallback, AfterEachCa
       return this;
     }
 
-    /**
-     * Declares test-only entities to be registered with {@code ObjectifyService}.
-     *
-     * <p>Note that {@code ObjectifyService} silently replaces the current registration for a given
-     * kind when a different class is registered for this kind. Since {@code ObjectifyService} does
-     * not support de-registration, each test entity class must be of a unique kind across the
-     * entire code base. Although this requirement can be worked around by using different {@code
-     * ObjectifyService} instances for each test (class), the setup overhead would rise
-     * significantly.
-     *
-     * @see AppEngineExtension#register(Class)
-     */
-    @SafeVarargs
-    public final Builder withOfyTestEntities(Class<?>... entities) {
-      ofyTestEntities.add(entities);
-      return this;
-    }
-
     public Builder withJpaUnitTestEntities(Class<?>... entities) {
       jpaTestEntities.add(entities);
       extension.withJpaUnitTest = true;
@@ -239,7 +216,6 @@ public final class AppEngineExtension implements BeforeEachCallback, AfterEachCa
       checkState(
           !extension.withJpaUnitTest || !extension.enableJpaEntityCoverageCheck,
           "withJpaUnitTestEntities cannot be set when enableJpaEntityCoverageCheck");
-      extension.ofyTestEntities = this.ofyTestEntities.build();
       extension.jpaTestEntities = this.jpaTestEntities.build();
       return extension;
     }
@@ -440,9 +416,6 @@ public final class AppEngineExtension implements BeforeEachCallback, AfterEachCa
       helper.setEnvInstance("0");
     }
     helper.setUp();
-
-    ObjectifyService.initOfy();
-    this.ofyTestEntities.forEach(AppEngineExtension::register);
   }
 
   /** Called after each test method. */
@@ -472,7 +445,6 @@ public final class AppEngineExtension implements BeforeEachCallback, AfterEachCa
   public void tearDown() throws Exception {
     // Resets Objectify. Although it would seem more obvious to do this at the start of a request
     // instead of at the end, this is more consistent with what ObjectifyFilter does in real code.
-    ObjectifyFilter.complete();
     helper.tearDown();
     helper = null;
     // Test that Datastore didn't need any indexes we don't have listed in our index file.
@@ -502,24 +474,6 @@ public final class AppEngineExtension implements BeforeEachCallback, AfterEachCa
     }
   }
 
-  /**
-   * Registers test-only Objectify entities and checks for re-registrations for the same kind by
-   * different classes.
-   */
-  private static void register(Class<?> entityClass) {
-    String kind = Key.getKind(entityClass);
-    Optional.ofNullable(com.googlecode.objectify.ObjectifyService.factory().getMetadata(kind))
-        .ifPresent(
-            meta ->
-                checkState(
-                    meta.getEntityClass() == entityClass,
-                    "Cannot register %s. The Kind %s is already registered with %s.",
-                    entityClass.getName(),
-                    kind,
-                    meta.getEntityClass().getName()));
-    com.googlecode.objectify.ObjectifyService.register(entityClass);
-  }
-
   /** Install {@code testing/logging.properties} so logging is less noisy. */
   private static void setupLogging() throws IOException {
     LogManager.getLogManager()

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

@@ -15,27 +15,15 @@
 package google.registry.testing;
 
 import static com.google.common.io.Files.asCharSink;
-import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
-import static google.registry.util.CollectionUtils.entriesToImmutableMap;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import com.google.common.base.Joiner;
-import com.google.common.collect.Multimap;
-import com.google.common.collect.MultimapBuilder;
-import com.google.common.collect.Multimaps;
-import com.googlecode.objectify.Key;
-import com.googlecode.objectify.annotation.Entity;
-import com.googlecode.objectify.annotation.Id;
 import google.registry.persistence.transaction.JpaTransactionManager;
-import io.github.classgraph.ClassGraph;
-import io.github.classgraph.ScanResult;
 import java.io.File;
 import java.io.IOException;
-import java.util.Collection;
-import java.util.Map;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -120,65 +108,7 @@ class AppEngineExtensionTest {
     assertThrows(AssertionError.class, () -> appEngine.afterEach(context.getContext()));
   }
 
-  @Test
-  void testRegisterOfyEntities_duplicateEntitiesWithSameName_fails() throws Exception {
-    AppEngineExtension appEngineExtension =
-        AppEngineExtension.builder()
-            .withCloudSql()
-            .withOfyTestEntities(google.registry.testing.TestObject.class, TestObject.class)
-            .build();
-    // Thrown before JPA is set up, therefore no need to call afterEach.
-    IllegalStateException thrown =
-        assertThrows(
-            IllegalStateException.class, () -> appEngineExtension.beforeEach(context.getContext()));
-    assertThat(thrown)
-        .hasMessageThat()
-        .isEqualTo(
-            String.format(
-                "Cannot register %s. The Kind %s is already registered with %s.",
-                TestObject.class.getName(),
-                "TestObject",
-                google.registry.testing.TestObject.class.getName()));
-    // The class level extension.
-    appEngine.afterEach(context.getContext());
-  }
-
-  @Test
-  void testOfyEntities_uniqueKinds() throws Exception {
-    try (ScanResult scanResult =
-        new ClassGraph()
-            .enableAnnotationInfo()
-            .ignoreClassVisibility()
-            .whitelistPackages("google.registry")
-            .scan()) {
-      Multimap<String, Class<?>> kindToEntityMultiMap =
-          scanResult.getClassesWithAnnotation(Entity.class.getName()).stream()
-              .filter(clazz -> !clazz.getName().equals(TestObject.class.getName()))
-              .map(clazz -> clazz.loadClass())
-              .collect(
-                  Multimaps.toMultimap(
-                      Key::getKind,
-                      clazz -> clazz,
-                      MultimapBuilder.hashKeys().linkedListValues()::build));
-      Map<String, Collection<Class<?>>> conflictingKinds =
-          kindToEntityMultiMap.asMap().entrySet().stream()
-              .filter(e -> e.getValue().size() > 1)
-              .collect(entriesToImmutableMap());
-      assertWithMessage(
-              "Conflicting Ofy kinds found. Tests will break if they are registered with "
-                  + " AppEngineExtension in the same test executor.")
-          .that(conflictingKinds)
-          .isEmpty();
-    }
-    appEngine.afterEach(context.getContext());
-  }
-
   private void writeAutoIndexFile(String content) throws IOException {
     asCharSink(new File(appEngine.tmpDir, "datastore-indexes-auto.xml"), UTF_8).write(content);
   }
-
-  @Entity
-  private static final class TestObject {
-    @Id long id;
-  }
 }

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

@@ -1,78 +0,0 @@
-// Copyright 2017 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.testing;
-
-import com.googlecode.objectify.Key;
-import com.googlecode.objectify.annotation.Entity;
-import com.googlecode.objectify.annotation.Id;
-import google.registry.model.ImmutableObject;
-import google.registry.model.annotations.DeleteAfterMigration;
-import google.registry.model.annotations.VirtualEntity;
-import google.registry.persistence.VKey;
-
-/** A test model object that can be persisted in any entity group. */
-@DeleteAfterMigration
-@Entity
-public class TestObject extends ImmutableObject {
-
-  @Id @javax.persistence.Id String id;
-
-  String field;
-
-  public String getId() {
-    return id;
-  }
-
-  public String getField() {
-    return field;
-  }
-
-  public VKey<TestObject> key() {
-    return VKey.create(TestObject.class, id);
-  }
-
-  public static TestObject create(String id) {
-    return create(id, null);
-  }
-
-  public static TestObject create(String id, String field) {
-    TestObject instance = new TestObject();
-    instance.id = id;
-    instance.field = field;
-    return instance;
-  }
-
-  /** A test @VirtualEntity model object, which should not be persisted. */
-  @Entity
-  @VirtualEntity
-  public static class TestVirtualObject extends ImmutableObject {
-
-    @Id String id;
-
-    /**
-     * Expose a factory method for testing saves of virtual entities; in real life this would never
-     * be needed for an actual @VirtualEntity.
-     */
-    public static TestVirtualObject create(String id) {
-      TestVirtualObject instance = new TestVirtualObject();
-      instance.id = id;
-      return instance;
-    }
-
-    public static Key<TestVirtualObject> createKey(String id) {
-      return Key.create(TestVirtualObject.class, id);
-    }
-  }
-}

core/src/test/java/google/registry/tools/EntityWrapperTest.java

@@ -1,123 +0,0 @@
-// Copyright 2017 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.tools;
-
-import static com.google.common.truth.Truth.assertThat;
-
-import com.google.appengine.api.datastore.Entity;
-import com.google.appengine.api.datastore.EntityTranslator;
-import com.google.common.collect.ImmutableList;
-import com.google.storage.onestore.v3.OnestoreEntity.EntityProto;
-import com.google.storage.onestore.v3.OnestoreEntity.Property;
-import google.registry.testing.AppEngineExtension;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.RegisterExtension;
-
-/** Unit tests for {@link EntityWrapper}. */
-public final class EntityWrapperTest {
-
-  private static final String TEST_ENTITY_KIND = "TestEntity";
-  private static final int ARBITRARY_KEY_ID = 1001;
-
-  @RegisterExtension
-  public final AppEngineExtension appEngine = AppEngineExtension.builder().withCloudSql().build();
-
-  @Test
-  void testEquals() {
-    // Create an entity with a key and some properties.
-    Entity entity = new Entity(TEST_ENTITY_KIND, ARBITRARY_KEY_ID);
-    // Note that we need to specify these as long for property comparisons to work because that's
-    // how they are deserialized from protos.
-    entity.setProperty("eeny", 100L);
-    entity.setProperty("meeny", 200L);
-    entity.setProperty("miney", 300L);
-
-    EntityProto proto1 = EntityTranslator.convertToPb(entity);
-    EntityProto proto2 = EntityTranslator.convertToPb(entity);
-
-    // Reorder the property list of proto2 (the protobuf stores this as a repeated field, so
-    // we just have to clear and re-add them in a different order).
-    ImmutableList<Property> properties =
-        ImmutableList.of(proto2.getProperty(2), proto2.getProperty(0), proto2.getProperty(1));
-    proto2.clearProperty();
-    for (Property property : properties) {
-      proto2.addProperty(property);
-    }
-
-    // Construct entity objects from the two protos.
-    Entity e1 = EntityTranslator.createFromPb(proto1);
-    Entity e2 = EntityTranslator.createFromPb(proto2);
-
-    // Ensure that we have a normalized representation.
-    EntityWrapper ce1 = new EntityWrapper(e1);
-    EntityWrapper ce2 = new EntityWrapper(e2);
-    assertThat(ce1).isEqualTo(ce2);
-    assertThat(ce1.hashCode()).isEqualTo(ce2.hashCode());
-
-    // Ensure that the original entity is equal.
-    assertThat(new EntityWrapper(entity)).isEqualTo(ce1);
-  }
-
-  @Test
-  void testDifferentPropertiesNotEqual() {
-    Entity entity = new Entity(TEST_ENTITY_KIND, ARBITRARY_KEY_ID);
-    // Note that we need to specify these as long for property comparisons to work because that's
-    // how they are deserialized from protos.
-    entity.setProperty("eeny", 100L);
-    entity.setProperty("meeny", 200L);
-    entity.setProperty("miney", 300L);
-
-    EntityProto proto1 = EntityTranslator.convertToPb(entity);
-
-    entity.setProperty("tiger!", 400);
-    EntityProto proto2 = EntityTranslator.convertToPb(entity);
-
-    // Construct entity objects from the two protos.
-    Entity e1 = EntityTranslator.createFromPb(proto1);
-    Entity e2 = EntityTranslator.createFromPb(proto2);
-
-    EntityWrapper ce1 = new EntityWrapper(e1);
-    EntityWrapper ce2 = new EntityWrapper(e2);
-    assertThat(e1).isEqualTo(e2); // The keys should still be the same.
-    assertThat(ce1).isNotEqualTo(ce2);
-    assertThat(ce1.hashCode()).isNotEqualTo(ce2.hashCode());
-  }
-
-  @Test
-  void testDifferentKeysNotEqual() {
-    EntityProto proto1 =
-        EntityTranslator.convertToPb(new Entity(TEST_ENTITY_KIND, ARBITRARY_KEY_ID));
-    EntityProto proto2 =
-        EntityTranslator.convertToPb(new Entity(TEST_ENTITY_KIND, ARBITRARY_KEY_ID + 1));
-
-    // Construct entity objects from the two protos.
-    Entity e1 = EntityTranslator.createFromPb(proto1);
-    Entity e2 = EntityTranslator.createFromPb(proto2);
-
-    EntityWrapper ce1 = new EntityWrapper(e1);
-    EntityWrapper ce2 = new EntityWrapper(e2);
-    assertThat(ce1).isNotEqualTo(ce2);
-    assertThat(ce1.hashCode()).isNotEqualTo(ce2.hashCode());
-  }
-
-  @Test
-  void testComparisonAgainstNonComparableEntities() {
-    EntityWrapper ce = new EntityWrapper(new Entity(TEST_ENTITY_KIND, ARBITRARY_KEY_ID));
-    // Note: this has to be "isNotEqualTo()" and not isNotNull() because we want to test the
-    // equals() method and isNotNull() just checks for "ce != null".
-    assertThat(ce).isNotEqualTo(null);
-    assertThat(ce).isNotEqualTo(new Object());
-  }
-}

core/src/test/java/google/registry/webdriver/OteSetupConsoleScreenshotTest.java

@@ -17,8 +17,6 @@ package google.registry.webdriver;
 import static google.registry.server.Fixture.BASIC;
 import static google.registry.server.Route.route;
 
-import com.googlecode.objectify.ObjectifyFilter;
-import google.registry.model.ofy.OfyFilter;
 import google.registry.module.frontend.FrontendServlet;
 import google.registry.server.RegistryTestServer;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -33,7 +31,6 @@ public class OteSetupConsoleScreenshotTest extends WebDriverTestCase {
       new TestServerExtension.Builder()
           .setRunfiles(RegistryTestServer.RUNFILES)
           .setRoutes(route("/registrar-ote-setup", FrontendServlet.class))
-          .setFilters(ObjectifyFilter.class, OfyFilter.class)
           .setFixtures(BASIC)
           .setEmail("Marla.Singer@google.com")
           .build();

core/src/test/java/google/registry/webdriver/RegistrarConsoleScreenshotTest.java

@@ -28,10 +28,8 @@ import static google.registry.tools.LockOrUnlockDomainCommand.REGISTRY_LOCK_STAT
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
 
 import com.google.common.collect.ImmutableMap;
-import com.googlecode.objectify.ObjectifyFilter;
 import google.registry.model.domain.Domain;
 import google.registry.model.domain.RegistryLock;
-import google.registry.model.ofy.OfyFilter;
 import google.registry.model.registrar.Registrar.State;
 import google.registry.model.registrar.RegistrarPoc;
 import google.registry.module.frontend.FrontendServlet;
@@ -59,7 +57,6 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase {
               route("/registrar-settings", FrontendServlet.class),
               route("/registry-lock-get", FrontendServlet.class),
               route("/registry-lock-verify", FrontendServlet.class))
-          .setFilters(ObjectifyFilter.class, OfyFilter.class)
           .setFixtures(BASIC)
           .setEmail("Marla.Singer@crr.com") // from AppEngineExtension.makeRegistrarContact3
           .build();

core/src/test/java/google/registry/webdriver/RegistrarConsoleWebTest.java

@@ -21,8 +21,6 @@ import static google.registry.testing.DatabaseHelper.loadRegistrar;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.googlecode.objectify.ObjectifyFilter;
-import google.registry.model.ofy.OfyFilter;
 import google.registry.model.registrar.Registrar;
 import google.registry.model.registrar.RegistrarAddress;
 import google.registry.model.registrar.RegistrarPoc;
@@ -43,7 +41,6 @@ public class RegistrarConsoleWebTest extends WebDriverTestCase {
           .setRoutes(
               route("/registrar", FrontendServlet.class),
               route("/registrar-settings", FrontendServlet.class))
-          .setFilters(ObjectifyFilter.class, OfyFilter.class)
           .setFixtures(BASIC)
           .setEmail("Marla.Singer@crr.com")
           .build();

core/src/test/java/google/registry/webdriver/RegistrarCreateConsoleScreenshotTest.java

@@ -17,8 +17,6 @@ package google.registry.webdriver;
 import static google.registry.server.Fixture.BASIC;
 import static google.registry.server.Route.route;
 
-import com.googlecode.objectify.ObjectifyFilter;
-import google.registry.model.ofy.OfyFilter;
 import google.registry.module.frontend.FrontendServlet;
 import google.registry.server.RegistryTestServer;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -33,7 +31,6 @@ class RegistrarCreateConsoleScreenshotTest extends WebDriverTestCase {
       new TestServerExtension.Builder()
           .setRunfiles(RegistryTestServer.RUNFILES)
           .setRoutes(route("/registrar-create", FrontendServlet.class))
-          .setFilters(ObjectifyFilter.class, OfyFilter.class)
           .setFixtures(BASIC)
           .setEmail("Marla.Singer@google.com")
           .build();

core/src/test/java/google/registry/webdriver/TestServerExtension.java

@@ -37,7 +37,6 @@ import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.Callable;
 import java.util.concurrent.FutureTask;
 import java.util.concurrent.LinkedBlockingDeque;
-import javax.servlet.Filter;
 import org.junit.jupiter.api.extension.AfterEachCallback;
 import org.junit.jupiter.api.extension.BeforeEachCallback;
 import org.junit.jupiter.api.extension.ExtensionContext;
@@ -59,7 +58,6 @@ public final class TestServerExtension implements BeforeEachCallback, AfterEachC
   private final BlockingQueue<FutureTask<?>> jobs = new LinkedBlockingDeque<>();
   private final ImmutableMap<String, Path> runfiles;
   private final ImmutableList<Route> routes;
-  private final ImmutableList<Class<? extends Filter>> filters;
 
   private TestServer testServer;
   private Thread serverThread;
@@ -67,12 +65,10 @@ public final class TestServerExtension implements BeforeEachCallback, AfterEachC
   private TestServerExtension(
       ImmutableMap<String, Path> runfiles,
       ImmutableList<Route> routes,
-      ImmutableList<Class<? extends Filter>> filters,
       ImmutableList<Fixture> fixtures,
       String email) {
     this.runfiles = runfiles;
     this.routes = routes;
-    this.filters = filters;
     this.fixtures = fixtures;
     // We create an GAE-Admin user, and then use AuthenticatedRegistrarAccessor.bypassAdminCheck to
     // choose whether the user is an admin or not.
@@ -96,8 +92,7 @@ public final class TestServerExtension implements BeforeEachCallback, AfterEachC
                   // can access this server.
                   getExternalAddressOfLocalSystem().getHostAddress(), pickUnusedPort()),
               runfiles,
-              routes,
-              filters);
+              routes);
     } catch (UnknownHostException e) {
       throw new IllegalStateException(e);
     }
@@ -239,14 +234,12 @@ public final class TestServerExtension implements BeforeEachCallback, AfterEachC
   /**
    * Builder for {@link TestServerExtension}.
    *
-   * <p>This builder has three required fields: {@link #setRunfiles}, {@link #setRoutes}, and {@link
-   * #setFilters}.
+   * <p>This builder has two required fields: {@link #setRunfiles} and {@link #setRoutes}.
    */
   public static final class Builder {
 
     private ImmutableMap<String, Path> runfiles;
     private ImmutableList<Route> routes;
-    ImmutableList<Class<? extends Filter>> filters;
     private ImmutableList<Fixture> fixtures = ImmutableList.of();
     private String email;
 
@@ -263,13 +256,6 @@ public final class TestServerExtension implements BeforeEachCallback, AfterEachC
       return this;
     }
 
-    /** Sets the list of servlet {@link Filter} objects for {@link TestServer}. */
-    @SafeVarargs
-    public final Builder setFilters(Class<? extends Filter>... filters) {
-      this.filters = ImmutableList.copyOf(filters);
-      return this;
-    }
-
     /** Sets an ordered list of Datastore fixtures that should be loaded on startup. */
     public Builder setFixtures(Fixture... fixtures) {
       this.fixtures = ImmutableList.copyOf(fixtures);
@@ -291,7 +277,6 @@ public final class TestServerExtension implements BeforeEachCallback, AfterEachC
       return new TestServerExtension(
           checkNotNull(this.runfiles),
           checkNotNull(this.routes),
-          checkNotNull(this.filters),
           checkNotNull(this.fixtures),
           checkNotNull(this.email));
     }