Merge ClaimsList into ClaimsListShard (#694)

* Merge ClaimsList into ClaimsListShard

* Add a TODO to rename the class

* Rebase on HEAD

* Improve javadoc

core/src/main/java/google/registry/model/tmch/ClaimsListDao.java

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package google.registry.schema.tmch;
+package google.registry.model.tmch;
 
 import static google.registry.config.RegistryConfig.getDomainLabelListCacheDuration;
 import static google.registry.model.CacheUtils.tryMemoizeWithExpiration;
@@ -24,28 +24,28 @@ import google.registry.util.NonFinalForTesting;
 import java.util.Optional;
 import javax.persistence.EntityManager;
 
-/** Data access object for {@link ClaimsList}. */
+/** Data access object for {@link ClaimsListShard}. */
 public class ClaimsListDao {
 
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
   /** In-memory cache for claims list. */
   @NonFinalForTesting
-  private static Supplier<Optional<ClaimsList>> cacheClaimsList =
+  private static Supplier<Optional<ClaimsListShard>> cacheClaimsList =
       tryMemoizeWithExpiration(getDomainLabelListCacheDuration(), ClaimsListDao::getLatestRevision);
 
-  private static void save(ClaimsList claimsList) {
+  private static void save(ClaimsListShard claimsList) {
     jpaTm().transact(() -> jpaTm().getEntityManager().persist(claimsList));
   }
 
   /**
-   * Try to save the given {@link ClaimsList} into Cloud SQL. If the save fails, the error will be
-   * logged but no exception will be thrown.
+   * Try to save the given {@link ClaimsListShard} into Cloud SQL. If the save fails, the error will
+   * be logged but no exception will be thrown.
    *
    * <p>This method is used during the dual-write phase of database migration as Datastore is still
    * the authoritative database.
    */
-  public static void trySave(ClaimsList claimsList) {
+  static void trySave(ClaimsListShard claimsList) {
     try {
       ClaimsListDao.save(claimsList);
       logger.atInfo().log(
@@ -57,12 +57,12 @@ public class ClaimsListDao {
   }
 
   /**
-   * Returns the most recent revision of the {@link ClaimsList} in Cloud SQL, if it exists.
+   * Returns the most recent revision of the {@link ClaimsListShard} in Cloud SQL, if it exists.
    * TODO(shicong): Change this method to package level access after dual-read phase.
    * ClaimsListShard uses this method to retrieve claims list in Cloud SQL for the comparison, and
    * ClaimsListShard is not in this package.
    */
-  public static Optional<ClaimsList> getLatestRevision() {
+  public static Optional<ClaimsListShard> getLatestRevision() {
     return jpaTm()
         .transact(
             () -> {
@@ -73,15 +73,15 @@ public class ClaimsListDao {
               return em.createQuery(
                       "FROM ClaimsList cl LEFT JOIN FETCH cl.labelsToKeys WHERE cl.revisionId ="
                           + " :revisionId",
-                      ClaimsList.class)
+                      ClaimsListShard.class)
                   .setParameter("revisionId", revisionId)
                   .getResultStream()
                   .findFirst();
             });
   }
 
-  /** Returns the most recent revision of the {@link ClaimsList}, from cache. */
-  public static Optional<ClaimsList> getLatestRevisionCached() {
+  /** Returns the most recent revision of the {@link ClaimsListShard}, from cache. */
+  public static Optional<ClaimsListShard> getLatestRevisionCached() {
     return cacheClaimsList.get();
   }
 

core/src/main/java/google/registry/model/tmch/ClaimsListShard.java

@@ -40,6 +40,7 @@ import com.googlecode.objectify.annotation.Id;
 import com.googlecode.objectify.annotation.Ignore;
 import com.googlecode.objectify.annotation.OnSave;
 import com.googlecode.objectify.annotation.Parent;
+import google.registry.model.CreateAutoTimestamp;
 import google.registry.model.ImmutableObject;
 import google.registry.model.annotations.NotBackedUp;
 import google.registry.model.annotations.NotBackedUp.Reason;
@@ -47,8 +48,6 @@ import google.registry.model.annotations.VirtualEntity;
 import google.registry.model.common.CrossTldSingleton;
 import google.registry.schema.replay.DatastoreEntity;
 import google.registry.schema.replay.SqlEntity;
-import google.registry.schema.tmch.ClaimsList;
-import google.registry.schema.tmch.ClaimsListDao;
 import google.registry.util.CollectionUtils;
 import google.registry.util.Concurrent;
 import google.registry.util.Retrier;
@@ -59,6 +58,15 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.Callable;
 import javax.annotation.Nullable;
+import javax.persistence.CollectionTable;
+import javax.persistence.Column;
+import javax.persistence.ElementCollection;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.JoinColumn;
+import javax.persistence.MapKeyColumn;
+import javax.persistence.Table;
+import javax.persistence.Transient;
 import org.joda.time.DateTime;
 
 /**
@@ -74,10 +82,21 @@ import org.joda.time.DateTime;
  * 10MB per transaction limit.
  *
  * <p>Therefore, it is never OK to save an instance of this class directly to Datastore. Instead you
- * must use the {@link #save} method to do it for you.
+ * must use the {@link #saveToDatastore} method to do it for you.
+ *
+ * <p>Note that the primary key of this entity is {@link #revisionId}, which is auto-generated by
+ * the database. So, if a retry of insertion happens after the previous attempt unexpectedly
+ * succeeds, we will end up with having two exact same claims list with only different {@link
+ * #revisionId}. However, this is not an actual problem because we only use the claims list with
+ * highest {@link #revisionId}.
+ *
+ * <p>TODO(b/162007765): Rename the class to ClaimsList and remove Datastore related fields and
+ * methods.
  */
 @Entity
 @NotBackedUp(reason = Reason.EXTERNALLY_SOURCED)
+@javax.persistence.Entity(name = "ClaimsList")
+@Table
 public class ClaimsListShard extends ImmutableObject implements DatastoreEntity {
 
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -85,22 +104,44 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreEntity
   /** The number of claims list entries to store per shard. */
   private static final int SHARD_SIZE = 10000;
 
-  @Id
-  long id;
+  @Transient @Id long id;
 
-  @Parent
-  Key<ClaimsListRevision> parent;
+  @Transient @Parent Key<ClaimsListRevision> parent;
 
-  /** When the claims list was last updated. */
+  @Ignore
+  @javax.persistence.Id
+  @GeneratedValue(strategy = GenerationType.IDENTITY)
+  Long revisionId;
+
+  @Ignore
+  @Column(nullable = false)
+  CreateAutoTimestamp creationTimestamp = CreateAutoTimestamp.create(null);
+
+  /**
+   * When the claims list was last updated.
+   *
+   * <p>Note that the value of this field is parsed from the claims list file(See this <a
+   * href="https://tools.ietf.org/html/draft-lozano-tmch-func-spec-08#section-6.1">RFC</>), it is
+   * the DNL List creation datetime from the rfc. Since this field has been used by Datastore, we
+   * cannot change its name until we finish the migration.
+   *
+   * <p>TODO(b/166784536): Rename this field to tmdbGenerationTime.
+   */
+  @Column(name = "tmdb_generation_time", nullable = false)
   DateTime creationTime;
 
   /** A map from labels to claims keys. */
   @EmbedMap
+  @ElementCollection
+  @CollectionTable(
+      name = "ClaimsEntry",
+      joinColumns = @JoinColumn(name = "revisionId", referencedColumnName = "revisionId"))
+  @MapKeyColumn(name = "domainLabel", nullable = false)
+  @Column(name = "claimKey", nullable = false)
   Map<String, String> labelsToKeys;
 
   /** Indicates that this is a shard rather than a "full" list. */
-  @Ignore
-  boolean isShard = false;
+  @Ignore @Transient boolean isShard = false;
 
   private static final Retrier LOADER_RETRIER = new Retrier(new SystemSleeper(), 2);
 
@@ -164,10 +205,10 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreEntity
         return datastoreList;
       };
 
-  private static final void loadAndCompareCloudSqlList(ClaimsListShard datastoreList) {
-    Optional<ClaimsList> maybeCloudSqlList = ClaimsListDao.getLatestRevision();
+  private static void loadAndCompareCloudSqlList(ClaimsListShard datastoreList) {
+    Optional<ClaimsListShard> maybeCloudSqlList = ClaimsListDao.getLatestRevision();
     if (maybeCloudSqlList.isPresent()) {
-      ClaimsList cloudSqlList = maybeCloudSqlList.get();
+      ClaimsListShard cloudSqlList = maybeCloudSqlList.get();
       MapDifference<String, String> diff =
           Maps.difference(datastoreList.labelsToKeys, cloudSqlList.getLabelsToKeys());
       if (!diff.areEqual()) {
@@ -206,15 +247,34 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreEntity
       memoizeWithShortExpiration(
           () -> LOADER_RETRIER.callWithRetry(LOADER_CALLABLE, IllegalStateException.class));
 
-  public DateTime getCreationTime() {
+  /** Returns the revision id of this claims list, or throws exception if it is null. */
+  public Long getRevisionId() {
+    checkState(
+        revisionId != null, "revisionId is null because it is not persisted in the database");
+    return revisionId;
+  }
+
+  /**
+   * Returns the time when the external TMDB service generated this revision of the claims list.
+   *
+   * @see <a href="https://tools.ietf.org/html/draft-lozano-tmch-func-spec-08#section-6.1">DNL List
+   *     creation datetime</a>
+   */
+  public DateTime getTmdbGenerationTime() {
     return creationTime;
   }
 
+  /** Returns the creation time of this claims list. */
+  public DateTime getCreationTimestamp() {
+    return creationTimestamp.getTimestamp();
+  }
+
   /** Returns the claim key for a given domain if there is one, empty otherwise. */
   public Optional<String> getClaimKey(String label) {
     return Optional.ofNullable(labelsToKeys.get(label));
   }
 
+  /** Returns an {@link Map} mapping domain label to its lookup key. */
   public ImmutableMap<String, String> getLabelsToKeys() {
     return ImmutableMap.copyOf(labelsToKeys);
   }
@@ -229,11 +289,12 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreEntity
    * switching over to using them atomically, then deleting the old ones.
    */
   public void save() {
-    save(SHARD_SIZE);
+    saveToDatastore(SHARD_SIZE);
+    ClaimsListDao.trySave(this);
   }
 
   @VisibleForTesting
-  void save(int shardSize) {
+  void saveToDatastore(int shardSize) {
     // Figure out what the next versionId should be based on which ones already exist.
     final Key<ClaimsListRevision> oldRevision = getCurrentRevision();
     final Key<ClaimsListRevision> parentKey = ClaimsListRevision.createKey();
@@ -270,10 +331,11 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreEntity
             });
   }
 
-  public static ClaimsListShard create(DateTime creationTime, Map<String, String> labelsToKeys) {
+  public static ClaimsListShard create(
+      DateTime tmdbGenerationTime, Map<String, String> labelsToKeys) {
     ClaimsListShard instance = new ClaimsListShard();
     instance.id = allocateId();
-    instance.creationTime = checkNotNull(creationTime);
+    instance.creationTime = checkNotNull(tmdbGenerationTime);
     instance.labelsToKeys = checkNotNull(labelsToKeys);
     return instance;
   }

core/src/main/java/google/registry/schema/tmch/ClaimsList.java

@@ -1,116 +0,0 @@
-// Copyright 2019 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.schema.tmch;
-
-import static com.google.common.base.Preconditions.checkState;
-import static google.registry.util.DateTimeUtils.toJodaDateTime;
-import static google.registry.util.DateTimeUtils.toZonedDateTime;
-
-import com.google.common.collect.ImmutableList;
-import google.registry.model.CreateAutoTimestamp;
-import google.registry.model.ImmutableObject;
-import google.registry.schema.replay.DatastoreEntity;
-import google.registry.schema.replay.SqlEntity;
-import java.time.ZonedDateTime;
-import java.util.Map;
-import java.util.Optional;
-import javax.persistence.CollectionTable;
-import javax.persistence.Column;
-import javax.persistence.ElementCollection;
-import javax.persistence.Entity;
-import javax.persistence.GeneratedValue;
-import javax.persistence.GenerationType;
-import javax.persistence.Id;
-import javax.persistence.JoinColumn;
-import javax.persistence.MapKeyColumn;
-import javax.persistence.Table;
-import org.joda.time.DateTime;
-
-/**
- * A list of TMCH claims labels and their associated claims keys.
- *
- * <p>Note that the primary key of this entity is {@link #revisionId}, which is auto-generated by
- * the database. So, if a retry of insertion happens after the previous attempt unexpectedly
- * succeeds, we will end up with having two exact same claims list with only different {@link
- * #revisionId}. However, this is not an actual problem because we only use the claims list with
- * highest {@link #revisionId}.
- */
-@Entity
-@Table
-public class ClaimsList extends ImmutableObject implements SqlEntity {
-  @Id
-  @GeneratedValue(strategy = GenerationType.IDENTITY)
-  @Column
-  private Long revisionId;
-
-  @Column(nullable = false)
-  private CreateAutoTimestamp creationTimestamp = CreateAutoTimestamp.create(null);
-
-  @Column(nullable = false)
-  private ZonedDateTime tmdbGenerationTime;
-
-  @ElementCollection
-  @CollectionTable(
-      name = "ClaimsEntry",
-      joinColumns = @JoinColumn(name = "revisionId", referencedColumnName = "revisionId"))
-  @MapKeyColumn(name = "domainLabel", nullable = false)
-  @Column(name = "claimKey", nullable = false)
-  private Map<String, String> labelsToKeys;
-
-  private ClaimsList(ZonedDateTime tmdbGenerationTime, Map<String, String> labelsToKeys) {
-    this.tmdbGenerationTime = tmdbGenerationTime;
-    this.labelsToKeys = labelsToKeys;
-  }
-
-  // Hibernate requires this default constructor.
-  private ClaimsList() {}
-
-  /** Constructs a {@link ClaimsList} object. */
-  public static ClaimsList create(DateTime creationTimestamp, Map<String, String> labelsToKeys) {
-    return new ClaimsList(toZonedDateTime(creationTimestamp), labelsToKeys);
-  }
-
-  /** Returns the revision id of this claims list, or throws exception if it is null. */
-  public Long getRevisionId() {
-    checkState(
-        revisionId != null, "revisionId is null because it is not persisted in the database");
-    return revisionId;
-  }
-
-  /** Returns the TMDB generation time of this claims list. */
-  public DateTime getTmdbGenerationTime() {
-    return toJodaDateTime(tmdbGenerationTime);
-  }
-
-  /** Returns the creation time of this claims list. */
-  public DateTime getCreationTimestamp() {
-    return creationTimestamp.getTimestamp();
-  }
-
-  /** Returns an {@link Map} mapping domain label to its lookup key. */
-  public Map<String, String> getLabelsToKeys() {
-    return labelsToKeys;
-  }
-
-  /** Returns the claim key for a given domain if there is one, empty otherwise. */
-  public Optional<String> getClaimKey(String label) {
-    return Optional.ofNullable(labelsToKeys.get(label));
-  }
-
-  @Override
-  public ImmutableList<DatastoreEntity> toDatastoreEntities() {
-    return ImmutableList.of(); // ClaimsList is dual-written
-  }
-}

core/src/main/java/google/registry/tmch/ClaimsListParser.java

@@ -18,7 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument;
 
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableMap;
-import google.registry.schema.tmch.ClaimsList;
+import google.registry.model.tmch.ClaimsListShard;
 import java.util.List;
 import org.joda.time.DateTime;
 
@@ -34,11 +34,11 @@ import org.joda.time.DateTime;
 public class ClaimsListParser {
 
   /**
-   * Converts the lines from the DNL CSV file into a {@link ClaimsList} object.
+   * Converts the lines from the DNL CSV file into a {@link ClaimsListShard} object.
    *
    * <p>Please note that this does <b>not</b> insert the object into Datastore.
    */
-  public static ClaimsList parse(List<String> lines) {
+  public static ClaimsListShard parse(List<String> lines) {
     ImmutableMap.Builder<String, String> builder = new ImmutableMap.Builder<>();
 
     // First line: <version>,<DNL List creation datetime>
@@ -74,6 +74,6 @@ public class ClaimsListParser {
       builder.put(label, lookupKey);
     }
 
-    return ClaimsList.create(creationTime, builder.build());
+    return ClaimsListShard.create(creationTime, builder.build());
   }
 }

core/src/main/java/google/registry/tmch/TmchDnlAction.java

@@ -21,8 +21,6 @@ import google.registry.keyring.api.KeyModule.Key;
 import google.registry.model.tmch.ClaimsListShard;
 import google.registry.request.Action;
 import google.registry.request.auth.Auth;
-import google.registry.schema.tmch.ClaimsList;
-import google.registry.schema.tmch.ClaimsListDao;
 import java.io.IOException;
 import java.security.SignatureException;
 import java.util.List;
@@ -56,14 +54,10 @@ public final class TmchDnlAction implements Runnable {
     } catch (SignatureException | IOException | PGPException e) {
       throw new RuntimeException(e);
     }
-    ClaimsList claims = ClaimsListParser.parse(lines);
-    ClaimsListShard claimsListShard =
-        ClaimsListShard.create(claims.getTmdbGenerationTime(), claims.getLabelsToKeys());
-    claimsListShard.save();
+    ClaimsListShard claims = ClaimsListParser.parse(lines);
+    claims.save();
     logger.atInfo().log(
         "Inserted %,d claims into Datastore, created at %s",
-        claimsListShard.size(), claimsListShard.getCreationTime());
-
-    ClaimsListDao.trySave(claims);
+        claims.size(), claims.getTmdbGenerationTime());
   }
 }

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

@@ -22,8 +22,6 @@ import com.beust.jcommander.Parameters;
 import com.google.common.base.Joiner;
 import com.google.common.io.Files;
 import google.registry.model.tmch.ClaimsListShard;
-import google.registry.schema.tmch.ClaimsList;
-import google.registry.schema.tmch.ClaimsListDao;
 import google.registry.tmch.ClaimsListParser;
 import java.io.File;
 import java.io.IOException;
@@ -39,7 +37,7 @@ final class UploadClaimsListCommand extends ConfirmingCommand implements Command
 
   private String claimsListFilename;
 
-  private ClaimsList claimsList;
+  private ClaimsListShard claimsList;
 
   @Override
   protected void init() throws IOException {
@@ -58,8 +56,7 @@ final class UploadClaimsListCommand extends ConfirmingCommand implements Command
 
   @Override
   public String execute() {
-    ClaimsListShard.create(claimsList.getTmdbGenerationTime(), claimsList.getLabelsToKeys()).save();
-    ClaimsListDao.trySave(claimsList);
+    claimsList.save();
     return String.format("Successfully uploaded claims list %s", claimsListFilename);
   }
 }

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

@@ -53,8 +53,8 @@
     <class>google.registry.model.registry.label.PremiumList</class>
     <class>google.registry.model.reporting.Spec11ThreatMatch</class>
     <class>google.registry.persistence.transaction.TransactionEntity</class>
+    <class>google.registry.model.tmch.ClaimsListShard</class>
     <class>google.registry.schema.domain.RegistryLock</class>
-    <class>google.registry.schema.tmch.ClaimsList</class>
     <class>google.registry.schema.cursor.Cursor</class>
     <class>google.registry.schema.server.Lock</class>
     <class>google.registry.schema.tld.PremiumEntry</class>