Add @VirtualEntity checking to Ofy's deleteWithoutBackup()

This was an oversight I noticed ages ago, so resurrecting some old local changes I had to correct it.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=146812322

java/google/registry/backup/RestoreCommitLogsAction.java

@@ -186,7 +186,7 @@ public class RestoreCommitLogsAction implements Runnable {
     }
     return dryRun || keysToDelete.isEmpty()
         ? new ResultNow<Void>(null)
-        : ofy().deleteWithoutBackup().entities(keysToDelete);
+        : ofy().deleteWithoutBackup().keys(keysToDelete);
    }
 
   /** Retrier for saves and deletes, since we can't proceed with any failures. */

java/google/registry/model/ofy/Ofy.java

@@ -147,12 +147,17 @@ public class Ofy {
   }
 
   /**
-   * Delete, without any augmentations.
+   * Delete, without any augmentations except to check that we're not saving any virtual entities.
    *
    * <p>No backups get written.
    */
   public Deleter deleteWithoutBackup() {
-    return ofy().delete();
+    return new AugmentedDeleter() {
+      @Override
+      protected void handleDeletion(Iterable<Key<?>> keys) {
+        checkProhibitedAnnotations(keys, VirtualEntity.class);
+      }
+    };
   }
 
   /**

javatests/google/registry/backup/ExportCommitLogDiffActionTest.java

@@ -29,7 +29,6 @@ import com.google.appengine.tools.cloudstorage.GcsService;
 import com.google.appengine.tools.cloudstorage.GcsServiceFactory;
 import com.google.common.collect.ImmutableMap;
 import com.googlecode.objectify.Key;
-import com.googlecode.objectify.ObjectifyService;
 import google.registry.model.ImmutableObject;
 import google.registry.model.ofy.CommitLogBucket;
 import google.registry.model.ofy.CommitLogCheckpoint;
@@ -65,7 +64,6 @@ public class ExportCommitLogDiffActionTest {
 
   @Before
   public void before() {
-    ObjectifyService.register(TestObject.class);
     task.gcsService = gcsService;
     task.gcsBucket = "gcs bucket";
     task.batchSize = 5;

javatests/google/registry/backup/GcsDiffFileListerTest.java

@@ -31,9 +31,7 @@ import com.google.appengine.tools.cloudstorage.ListItem;
 import com.google.appengine.tools.cloudstorage.ListResult;
 import com.google.common.base.Function;
 import com.google.common.collect.Iterators;
-import com.googlecode.objectify.ObjectifyService;
 import google.registry.testing.AppEngineRule;
-import google.registry.testing.TestObject;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
 import java.nio.ByteBuffer;
@@ -64,7 +62,6 @@ public class GcsDiffFileListerTest {
 
   @Before
   public void before() throws Exception {
-    ObjectifyService.register(TestObject.class);
     diffLister.gcsService = gcsService;
     diffLister.gcsBucket = GCS_BUCKET;
     diffLister.executor = newDirectExecutorService();

javatests/google/registry/backup/RestoreCommitLogsActionTest.java

@@ -39,7 +39,6 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 import com.google.common.primitives.Longs;
 import com.googlecode.objectify.Key;
-import com.googlecode.objectify.ObjectifyService;
 import google.registry.model.ImmutableObject;
 import google.registry.model.ofy.CommitLogBucket;
 import google.registry.model.ofy.CommitLogCheckpoint;
@@ -81,7 +80,6 @@ public class RestoreCommitLogsActionTest {
 
   @Before
   public void init() {
-    ObjectifyService.register(TestObject.class);
     action.gcsService = gcsService;
     action.dryRun = false;
     action.datastoreService = DatastoreServiceFactory.getDatastoreService();

javatests/google/registry/model/ofy/OfyCommitLogTest.java

@@ -20,6 +20,7 @@ import static com.googlecode.objectify.ObjectifyService.register;
 import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
 import static google.registry.model.ofy.CommitLogBucket.getBucketKey;
 import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.testing.TestObject.TestVirtualObject;
 
 import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.Key;
@@ -110,7 +111,7 @@ public class OfyCommitLogTest {
     ofy().transact(new VoidWork() {
       @Override
       public void vrun() {
-        ofy().deleteWithoutBackup().entity(Key.create(Root.class, 1));
+        ofy().deleteWithoutBackup().key(Key.create(Root.class, 1));
       }});
     assertThat(ofy().load().key(Key.create(Root.class, 1)).now()).isNull();
     assertThat(ofy().load().type(CommitLogManifest.class)).isEmpty();
@@ -212,7 +213,6 @@ public class OfyCommitLogTest {
   public void testTransactNew_saveNotBackedUpKind_throws() throws Exception {
     final CommitLogManifest backupsArentAllowedOnMe =
         CommitLogManifest.create(getBucketKey(1), clock.nowUtc(), ImmutableSet.<Key<?>>of());
-    ofy().saveWithoutBackup().entity(backupsArentAllowedOnMe).now();
     thrown.expect(IllegalArgumentException.class, "Can't save/delete a @NotBackedUp");
     ofy().transactNew(new VoidWork() {
       @Override
@@ -221,6 +221,42 @@ public class OfyCommitLogTest {
       }});
   }
 
+  @Test
+  public void testTransactNew_deleteVirtualEntityKey_throws() throws Exception {
+    final Key<TestVirtualObject> virtualEntityKey = TestVirtualObject.createKey("virtual");
+    thrown.expect(IllegalArgumentException.class, "Can't save/delete a @VirtualEntity");
+    ofy().transactNew(new VoidWork() {
+      @Override
+      public void vrun() {
+        ofy().delete().key(virtualEntityKey);
+      }});
+  }
+
+  @Test
+  public void testTransactNew_saveVirtualEntity_throws() throws Exception {
+    final TestVirtualObject virtualEntity = TestVirtualObject.create("virtual");
+    thrown.expect(IllegalArgumentException.class, "Can't save/delete a @VirtualEntity");
+    ofy().transactNew(new VoidWork() {
+      @Override
+      public void vrun() {
+        ofy().save().entity(virtualEntity);
+      }});
+  }
+
+  @Test
+  public void test_deleteWithoutBackup_withVirtualEntityKey_throws() throws Exception {
+    final Key<TestVirtualObject> virtualEntityKey = TestVirtualObject.createKey("virtual");
+    thrown.expect(IllegalArgumentException.class, "Can't save/delete a @VirtualEntity");
+    ofy().deleteWithoutBackup().key(virtualEntityKey);
+  }
+
+  @Test
+  public void test_saveWithoutBackup_withVirtualEntity_throws() throws Exception {
+    final TestVirtualObject virtualEntity = TestVirtualObject.create("virtual");
+    thrown.expect(IllegalArgumentException.class, "Can't save/delete a @VirtualEntity");
+    ofy().saveWithoutBackup().entity(virtualEntity);
+  }
+
   @Test
   public void testTransact_twoSavesOnSameKey_throws() throws Exception {
     thrown.expect(IllegalArgumentException.class, "Multiple entries with same key");

javatests/google/registry/testing/TestObject.java

@@ -17,10 +17,12 @@ package google.registry.testing;
 import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
 
 import com.googlecode.objectify.Key;
+import com.googlecode.objectify.ObjectifyService;
 import com.googlecode.objectify.annotation.Entity;
 import com.googlecode.objectify.annotation.Id;
 import com.googlecode.objectify.annotation.Parent;
 import google.registry.model.ImmutableObject;
+import google.registry.model.annotations.VirtualEntity;
 import google.registry.model.common.EntityGroupRoot;
 
 /**
@@ -28,6 +30,10 @@ import google.registry.model.common.EntityGroupRoot;
  */
 @Entity
 public class TestObject extends ImmutableObject {
+  static {
+    ObjectifyService.register(TestObject.class);  // Register this kind on first reference.
+  }
+
   @Parent
   Key<EntityGroupRoot> parent;
 
@@ -59,4 +65,30 @@ public class TestObject extends ImmutableObject {
     instance.parent = parent;
     return instance;
   }
+
+  /** A test @VirtualEntity model object, which should not be persisted. */
+  @Entity
+  @VirtualEntity
+  public static class TestVirtualObject extends ImmutableObject {
+    static {
+      ObjectifyService.register(TestVirtualObject.class);  // Register this kind on first reference.
+    }
+
+    @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);
+    }
+  }
 }