Remove ReservedList from Datastore schema (#1285)

* Remove ReservedList from Datastore schema

* Remove some Datastore references

* Add a different non-replicated entity to ReplayCommitLogsToSqlActionTest

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

@@ -43,7 +43,6 @@ import google.registry.model.reporting.HistoryEntry;
 import google.registry.model.server.Lock;
 import google.registry.model.server.ServerSecret;
 import google.registry.model.tld.Registry;
-import google.registry.model.tld.label.ReservedList;
 import google.registry.model.tmch.ClaimsList;
 import google.registry.model.tmch.ClaimsList.ClaimsListRevision;
 import google.registry.model.tmch.ClaimsList.ClaimsListSingleton;
@@ -92,7 +91,6 @@ public final class EntityClasses {
           Registrar.class,
           RegistrarContact.class,
           Registry.class,
-          ReservedList.class,
           ServerSecret.class,
           TmchCrl.class);
 

core/src/main/java/google/registry/model/rde/RdeNamingUtils.java

@@ -39,7 +39,7 @@ public final class RdeNamingUtils {
   }
 
   /** Returns same thing as {@link #makeRydeFilename} except without the series and revision. */
-  static String makePartialName(String tld, DateTime date, RdeMode mode) {
+  public static String makePartialName(String tld, DateTime date, RdeMode mode) {
     return String.format("%s_%s_%s",
         checkNotNull(tld), formatDate(date), mode.getFilenameComponent());
   }

core/src/main/java/google/registry/model/tld/label/ReservedList.java

@@ -33,13 +33,8 @@ import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.util.concurrent.UncheckedExecutionException;
-import com.googlecode.objectify.annotation.Embed;
-import com.googlecode.objectify.annotation.Entity;
-import com.googlecode.objectify.annotation.Mapify;
-import com.googlecode.objectify.mapper.Mapper;
 import google.registry.model.Buildable;
-import google.registry.model.annotations.ReportedOn;
-import google.registry.model.replay.NonReplicatedEntity;
+import google.registry.model.replay.SqlOnlyEntity;
 import google.registry.model.tld.Registry;
 import google.registry.model.tld.label.DomainLabelMetrics.MetricsReservedListMatch;
 import java.io.Serializable;
@@ -65,13 +60,11 @@ import org.joda.time.DateTime;
  * succeeds, we will end up with having two exact same reserved lists that differ only by
  * revisionId. This is fine though, because we only use the list with the highest revisionId.
  */
-@Entity
-@ReportedOn
 @javax.persistence.Entity
 @Table(indexes = {@Index(columnList = "name", name = "reservedlist_name_idx")})
 public final class ReservedList
     extends BaseDomainLabelList<ReservationType, ReservedList.ReservedListEntry>
-    implements NonReplicatedEntity {
+    implements SqlOnlyEntity {
 
   /**
    * Mapping from domain name to its reserved list info.
@@ -80,7 +73,6 @@ public final class ReservedList
    * from the immutability contract so we can modify it after construction and we have to handle the
    * database processing on our own so we can detach it after load.
    */
-  @Mapify(ReservedListEntry.LabelMapper.class)
   @Insignificant
   @Transient
   Map<String, ReservedListEntry> reservedListMap;
@@ -121,10 +113,9 @@ public final class ReservedList
    * A reserved list entry entity, persisted to Datastore, that represents a single label and its
    * reservation type.
    */
-  @Embed
   @javax.persistence.Entity(name = "ReservedEntry")
   public static class ReservedListEntry extends DomainLabelEntry<ReservationType, ReservedListEntry>
-      implements Buildable, NonReplicatedEntity, Serializable {
+      implements Buildable, SqlOnlyEntity, Serializable {
 
     @Insignificant @Id Long revisionId;
 
@@ -133,15 +124,6 @@ public final class ReservedList
 
     String comment;
 
-    /** Mapper for use with @Mapify */
-    static class LabelMapper implements Mapper<String, ReservedListEntry> {
-
-      @Override
-      public String getKey(ReservedListEntry entry) {
-        return entry.getDomainLabel();
-      }
-    }
-
     public String getComment(String comment) {
       return comment;
     }
@@ -239,7 +221,7 @@ public final class ReservedList
    * @return An Optional&lt;ReservedList&gt; that has a value if a reserved list exists by the given
    *     name, or absent if not.
    * @throws UncheckedExecutionException if some other error occurs while trying to load the
-   *     ReservedList from the cache or Datastore.
+   *     ReservedList from the cache or database.
    */
   public static Optional<ReservedList> get(String listName) {
     return getFromCache(listName, cache);

core/src/main/java/google/registry/persistence/converter/ReservedListKeySetConverter.java

@@ -1,36 +0,0 @@
-// Copyright 2020 The Nomulus Authors. All Rights Reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package google.registry.persistence.converter;
-
-import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
-
-import com.googlecode.objectify.Key;
-import google.registry.model.tld.label.ReservedList;
-import javax.persistence.Converter;
-
-/** JPA converter for a set of {@link Key} containing a {@link ReservedList} */
-@Converter(autoApply = true)
-public class ReservedListKeySetConverter extends StringSetConverterBase<Key<ReservedList>> {
-
-  @Override
-  String toString(Key<ReservedList> key) {
-    return key.getName();
-  }
-
-  @Override
-  Key<ReservedList> fromString(String value) {
-    return Key.create(getCrossTldKey(), ReservedList.class, value);
-  }
-}

core/src/main/java/google/registry/tools/CreateOrUpdateReservedListCommand.java

@@ -26,7 +26,8 @@ import javax.annotation.Nullable;
  * Base class for specification of command line parameters common to creating and updating reserved
  * lists.
  */
-public abstract class CreateOrUpdateReservedListCommand extends MutatingCommand {
+public abstract class CreateOrUpdateReservedListCommand extends ConfirmingCommand
+    implements CommandWithRemoteApi {
 
   static final FluentLogger logger = FluentLogger.forEnclosingClass();
 

core/src/main/java/google/registry/tools/CreateReservedListCommand.java

@@ -25,9 +25,7 @@ import com.beust.jcommander.Parameters;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
-import com.googlecode.objectify.Key;
 import google.registry.model.tld.label.ReservedList;
-import google.registry.persistence.VKey;
 import java.nio.file.Files;
 import java.util.List;
 import java.util.stream.Collectors;
@@ -48,7 +46,7 @@ final class CreateReservedListCommand extends CreateOrUpdateReservedListCommand
   boolean override;
 
   @Override
-  protected void init() throws Exception {
+  protected String prompt() throws Exception {
     name = Strings.isNullOrEmpty(name) ? convertFilePathToName(input) : name;
     checkArgument(
         !ReservedList.get(name).isPresent(), "A reserved list already exists by this name");
@@ -66,35 +64,11 @@ final class CreateReservedListCommand extends CreateOrUpdateReservedListCommand
             .setCreationTimestamp(now)
             .build();
 
-    // calls the stageEntityChange method that takes old entity, new entity and a new vkey;
-    // Because ReservedList is a sqlEntity and its primary key field (revisionId) is only set when
-    // it's being persisted; a vkey has to be created here explicitly for ReservedList instances.
-    stageEntityChange(
-        null, reservedList, VKey.createOfy(ReservedList.class, Key.create(reservedList)));
-  }
-
-  @Override
-  protected String prompt() {
-    return getChangedEntities().isEmpty()
-        ? "No entity changes to apply."
-        : getChangedEntities().stream()
-            .map(
-                entity -> {
-                  if (entity instanceof ReservedList) {
-                    // Format the entries of the reserved list as well.
-                    String entries =
-                        ((ReservedList) entity)
-                            .getReservedListEntries().entrySet().stream()
-                                .map(
-                                    entry ->
-                                        String.format("%s=%s", entry.getKey(), entry.getValue()))
-                                .collect(Collectors.joining(", "));
-                    return String.format("%s\nreservedListMap={%s}\n", entity, entries);
-                  } else {
-                    return entity.toString();
-                  }
-                })
-            .collect(Collectors.joining("\n"));
+    String entries =
+        reservedList.getReservedListEntries().entrySet().stream()
+            .map(entry -> String.format("%s=%s", entry.getKey(), entry.getValue()))
+            .collect(Collectors.joining(", "));
+    return String.format("%s\nreservedListMap={%s}\n", reservedList, entries);
   }
 
   private static void validateListName(String name) {

core/src/main/java/google/registry/tools/UpdateReservedListCommand.java

@@ -19,18 +19,17 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.beust.jcommander.Parameters;
 import com.google.common.base.Strings;
-import com.googlecode.objectify.Key;
 import google.registry.model.tld.label.ReservedList;
-import google.registry.persistence.VKey;
 import java.nio.file.Files;
 import java.util.List;
+import java.util.stream.Collectors;
 
 /** Command to safely update {@link ReservedList}. */
 @Parameters(separators = " =", commandDescription = "Update a ReservedList.")
 final class UpdateReservedListCommand extends CreateOrUpdateReservedListCommand {
 
   @Override
-  protected void init() throws Exception {
+  protected String prompt() throws Exception {
     name = Strings.isNullOrEmpty(name) ? convertFilePathToName(input) : name;
     ReservedList existingReservedList =
         ReservedList.get(name)
@@ -53,15 +52,18 @@ final class UpdateReservedListCommand extends CreateOrUpdateReservedListCommand
     if (!existingReservedList
         .getReservedListEntries()
         .equals(reservedList.getReservedListEntries())) {
-      // calls the stageEntityChange method that takes old entity, new entity and a new vkey;
-      // a vkey has to be created here explicitly for ReservedList instances.
-      // ReservedList is a sqlEntity; it triggers the static method Vkey.create(Key<?> ofyCall),
-      // which invokes a static ReservedList.createVkey(Key ofyKey) method that does not exist.
-      // the sql primary key field (revisionId) is only set when it's being persisted;
-      stageEntityChange(
-          existingReservedList,
-          reservedList,
-          VKey.createOfy(ReservedList.class, Key.create(existingReservedList)));
+      String oldEntries =
+          existingReservedList.getReservedListEntries().entrySet().stream()
+              .map(entry -> String.format("%s=%s", entry.getKey(), entry.getValue()))
+              .collect(Collectors.joining(", "));
+      String newEntries =
+          reservedList.getReservedListEntries().entrySet().stream()
+              .map(entry -> String.format("%s=%s", entry.getKey(), entry.getValue()))
+              .collect(Collectors.joining(", "));
+      return String.format(
+          "Update reserved list for %s?\nOld List: %s\n New List: %s",
+          name, oldEntries, newEntries);
     }
+    return "No entity changes to apply.";
   }
 }

core/src/main/resources/META-INF/persistence.xml

@@ -93,7 +93,6 @@
     <class>google.registry.persistence.converter.LocalDateConverter</class>
     <class>google.registry.persistence.converter.PostalInfoChoiceListConverter</class>
     <class>google.registry.persistence.converter.RegistrarPocSetConverter</class>
-    <class>google.registry.persistence.converter.ReservedListKeySetConverter</class>
     <class>google.registry.persistence.converter.Spec11ThreatMatchThreatTypeSetConverter</class>
     <class>google.registry.persistence.converter.StatusValueSetConverter</class>
     <class>google.registry.persistence.converter.StringListConverter</class>

core/src/test/java/google/registry/backup/ReplayCommitLogsToSqlActionTest.java

@@ -38,7 +38,6 @@ import static org.mockito.Mockito.verify;
 
 import com.google.cloud.storage.contrib.nio.testing.LocalStorageHelper;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedMap;
 import com.google.common.truth.Truth8;
@@ -55,12 +54,14 @@ import google.registry.model.index.ForeignKeyIndex;
 import google.registry.model.ofy.CommitLogBucket;
 import google.registry.model.ofy.CommitLogManifest;
 import google.registry.model.ofy.CommitLogMutation;
+import google.registry.model.rde.RdeMode;
+import google.registry.model.rde.RdeNamingUtils;
+import google.registry.model.rde.RdeRevision;
 import google.registry.model.registrar.RegistrarContact;
 import google.registry.model.replay.SqlReplayCheckpoint;
 import google.registry.model.server.Lock;
 import google.registry.model.tld.label.PremiumList;
 import google.registry.model.tld.label.PremiumList.PremiumEntry;
-import google.registry.model.tld.label.ReservedList;
 import google.registry.model.tmch.ClaimsList;
 import google.registry.model.translators.VKeyTranslatorFactory;
 import google.registry.persistence.VKey;
@@ -419,8 +420,9 @@ public class ReplayCommitLogsToSqlActionTest {
 
     createTld("tld");
     // Have a commit log with a couple objects that shouldn't be replayed
-    ReservedList reservedList =
-        new ReservedList.Builder().setReservedListMap(ImmutableMap.of()).setName("name").build();
+    String triplet = RdeNamingUtils.makePartialName("tld", fakeClock.nowUtc(), RdeMode.FULL);
+    RdeRevision rdeRevision =
+        RdeRevision.create(triplet, "tld", fakeClock.nowUtc().toLocalDate(), RdeMode.FULL, 1);
     ForeignKeyIndex<DomainBase> fki = ForeignKeyIndex.create(newDomainBase("foo.tld"), now);
     tm().transact(
             () -> {
@@ -430,8 +432,8 @@ public class ReplayCommitLogsToSqlActionTest {
                     createCheckpoint(now.minusMinutes(1)),
                     CommitLogManifest.create(
                         getBucketKey(1), now.minusMinutes(1), ImmutableSet.of()),
-                    // Reserved list is dually-written non-replicated
-                    CommitLogMutation.create(manifestKey, reservedList),
+                    // RDE Revisions are not replicated
+                    CommitLogMutation.create(manifestKey, rdeRevision),
                     // FKIs aren't replayed to SQL at all
                     CommitLogMutation.create(manifestKey, fki));
               } catch (IOException e) {

core/src/test/java/google/registry/model/tld/label/ReservedListTest.java

@@ -33,15 +33,11 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
-import google.registry.model.ofy.Ofy;
 import google.registry.model.tld.Registry;
-import google.registry.model.tld.label.PremiumList.PremiumEntry;
 import google.registry.testing.AppEngineExtension;
 import google.registry.testing.FakeClock;
-import google.registry.testing.InjectExtension;
 import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Order;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
@@ -50,19 +46,9 @@ class ReservedListTest {
 
   private FakeClock clock = new FakeClock(DateTime.parse("2010-01-01T10:00:00Z"));
 
-  @Order(value = Order.DEFAULT - 1)
   @RegisterExtension
-  final InjectExtension inject =
-      new InjectExtension().withStaticFieldOverride(Ofy.class, "clock", clock);
-
-  @RegisterExtension
-  final AppEngineExtension appEngine =
-      AppEngineExtension.builder()
-          .withClock(clock)
-          .withJpaUnitTestEntities(
-              PremiumList.class, PremiumEntry.class, ReservedList.class, ReservedListEntry.class)
-          .withDatastoreAndCloudSql()
-          .build();
+  public final AppEngineExtension appEngine =
+      AppEngineExtension.builder().withClock(clock).withCloudSql().build();
 
   @BeforeEach
   void beforeEach() {

core/src/test/java/google/registry/persistence/converter/ReservedListKeySetConverterTest.java

@@ -1,87 +0,0 @@
-// Copyright 2020 The Nomulus Authors. All Rights Reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package google.registry.persistence.converter;
-
-import static com.google.common.truth.Truth.assertThat;
-import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
-import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
-
-import com.googlecode.objectify.Key;
-import google.registry.model.ImmutableObject;
-import google.registry.model.tld.label.ReservedList;
-import google.registry.testing.AppEngineExtension;
-import java.util.Set;
-import javax.persistence.Entity;
-import javax.persistence.Id;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.RegisterExtension;
-import org.testcontainers.shaded.com.google.common.collect.ImmutableSet;
-
-/** Unit tests for {@link ReservedListKeySetConverter}. */
-class ReservedListKeySetConverterTest {
-
-  @RegisterExtension
-  final AppEngineExtension appEngine =
-      AppEngineExtension.builder()
-          .withDatastoreAndCloudSql()
-          .withJpaUnitTestEntities(ReservedListSetEntity.class)
-          .build();
-
-  @Test
-  void roundTripConversion_returnsSameSet() {
-    Key<ReservedList> key1 = Key.create(getCrossTldKey(), ReservedList.class, "test1");
-    Key<ReservedList> key2 = Key.create(getCrossTldKey(), ReservedList.class, "test2");
-    Key<ReservedList> key3 = Key.create(getCrossTldKey(), ReservedList.class, "test3");
-
-    Set<Key<ReservedList>> reservedLists = ImmutableSet.of(key1, key2, key3);
-    ReservedListSetEntity testEntity = new ReservedListSetEntity(reservedLists);
-    jpaTm().transact(() -> jpaTm().insert(testEntity));
-    ReservedListSetEntity persisted =
-        jpaTm().transact(() -> jpaTm().getEntityManager().find(ReservedListSetEntity.class, "id"));
-    assertThat(persisted.reservedList).containsExactly(key1, key2, key3);
-  }
-
-  @Test
-  void testNullValue_writesAndReadsNullSuccessfully() {
-    ReservedListSetEntity testEntity = new ReservedListSetEntity(null);
-    jpaTm().transact(() -> jpaTm().insert(testEntity));
-    ReservedListSetEntity persisted =
-        jpaTm().transact(() -> jpaTm().getEntityManager().find(ReservedListSetEntity.class, "id"));
-    assertThat(persisted.reservedList).isNull();
-  }
-
-  @Test
-  void testEmptyCollection_writesAndReadsEmptyCollectionSuccessfully() {
-    ReservedListSetEntity testEntity = new ReservedListSetEntity(ImmutableSet.of());
-    jpaTm().transact(() -> jpaTm().insert(testEntity));
-    ReservedListSetEntity persisted =
-        jpaTm().transact(() -> jpaTm().getEntityManager().find(ReservedListSetEntity.class, "id"));
-    assertThat(persisted.reservedList).isEmpty();
-  }
-
-  @Entity(name = "ReservedListSetEntity")
-  private static class ReservedListSetEntity extends ImmutableObject {
-
-    @Id String name = "id";
-
-    Set<Key<ReservedList>> reservedList;
-
-    public ReservedListSetEntity() {}
-
-    ReservedListSetEntity(Set<Key<ReservedList>> reservedList) {
-      this.reservedList = reservedList;
-    }
-  }
-}

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

@@ -161,6 +161,6 @@ class UpdateReservedListCommandTest
     command.input = Paths.get(reservedTermsPath);
     command.init();
 
-    assertThat(command.prompt()).contains("Update ReservedList@xn--q9jyb4c_common-reserved");
+    assertThat(command.prompt()).contains("Update reserved list for xn--q9jyb4c_common-reserved?");
   }
 }

core/src/test/resources/google/registry/export/backup_kinds.txt

@@ -19,4 +19,3 @@ Recurring
 Registrar
 RegistrarContact
 Registry
-ReservedList

core/src/test/resources/google/registry/export/crosstld_kinds.txt

@@ -4,6 +4,5 @@ Cursor
 Registrar
 RegistrarContact
 Registry
-ReservedList
 ServerSecret
 TmchCrl

core/src/test/resources/google/registry/export/reporting_kinds.txt

@@ -15,4 +15,3 @@ Recurring
 Registrar
 RegistrarContact
 Registry
-ReservedList

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

@@ -789,26 +789,6 @@ enum google.registry.model.tld.Registry$TldType {
   REAL;
   TEST;
 }
-enum google.registry.model.tld.label.ReservationType {
-  ALLOWED_IN_SUNRISE;
-  FULLY_BLOCKED;
-  NAME_COLLISION;
-  RESERVED_FOR_ANCHOR_TENANT;
-  RESERVED_FOR_SPECIFIC_USE;
-}
-class google.registry.model.tld.label.ReservedList {
-  @Id java.lang.String name;
-  @Parent com.googlecode.objectify.Key<google.registry.model.common.EntityGroupRoot> parent;
-  boolean shouldPublish;
-  java.util.Map<java.lang.String, google.registry.model.tld.label.ReservedList$ReservedListEntry> reservedListMap;
-  org.joda.time.DateTime creationTimestamp;
-}
-class google.registry.model.tld.label.ReservedList$ReservedListEntry {
-  @Id java.lang.String domainLabel;
-  google.registry.model.tld.label.ReservationType reservationType;
-  java.lang.Long revisionId;
-  java.lang.String comment;
-}
 class google.registry.model.tmch.ClaimsList {
   @Id long id;
   @Parent com.googlecode.objectify.Key<google.registry.model.tmch.ClaimsList$ClaimsListRevision> parent;