Use DomainHistoryVKey to restore symmetric VKey (#874)

* Use DomainHistoryVKey to restore symmetric VKey

* Rebase on HEAD

core/src/main/java/google/registry/flows/domain/DomainCreateFlow.java

@@ -110,6 +110,7 @@ import google.registry.model.reporting.DomainTransactionRecord;
 import google.registry.model.reporting.DomainTransactionRecord.TransactionReportField;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.model.reporting.IcannReportingTypes.ActivityReportField;
+import google.registry.persistence.DomainHistoryVKey;
 import google.registry.persistence.VKey;
 import google.registry.tmch.LordnTaskUtils;
 import java.util.Optional;
@@ -371,7 +372,7 @@ public class DomainCreateFlow implements TransactionalFlow {
         && TokenType.SINGLE_USE.equals(allocationToken.get().getTokenType())) {
       entitiesToSave.add(
           allocationTokenFlowUtils.redeemToken(
-              allocationToken.get(), HistoryEntry.createVKey(Key.create(historyEntry))));
+              allocationToken.get(), DomainHistoryVKey.create(Key.create(historyEntry))));
     }
     enqueueTasks(newDomain, hasSignedMarks, hasClaimsNotice);
 

core/src/main/java/google/registry/flows/domain/token/AllocationTokenFlowUtils.java

@@ -32,8 +32,7 @@ import google.registry.model.domain.token.AllocationToken;
 import google.registry.model.domain.token.AllocationToken.TokenStatus;
 import google.registry.model.domain.token.AllocationToken.TokenType;
 import google.registry.model.registry.Registry;
-import google.registry.model.reporting.HistoryEntry;
-import google.registry.persistence.VKey;
+import google.registry.persistence.DomainHistoryVKey;
 import java.util.List;
 import java.util.Optional;
 import javax.inject.Inject;
@@ -108,7 +107,7 @@ public class AllocationTokenFlowUtils {
 
   /** Redeems a SINGLE_USE {@link AllocationToken}, returning the redeemed copy. */
   public AllocationToken redeemToken(
-      AllocationToken token, VKey<HistoryEntry> redemptionHistoryEntry) {
+      AllocationToken token, DomainHistoryVKey redemptionHistoryEntry) {
     checkArgument(
         TokenType.SINGLE_USE.equals(token.getTokenType()),
         "Only SINGLE_USE tokens can be marked as redeemed");

core/src/main/java/google/registry/model/domain/DomainHistory.java

@@ -257,7 +257,7 @@ public class DomainHistory extends HistoryEntry implements SqlEntity {
   }
 
   /** Class to represent the composite primary key of {@link DomainHistory} entity. */
-  static class DomainHistoryId extends ImmutableObject implements Serializable {
+  public static class DomainHistoryId extends ImmutableObject implements Serializable {
 
     private String domainRepoId;
 
@@ -266,7 +266,7 @@ public class DomainHistory extends HistoryEntry implements SqlEntity {
     /** Hibernate requires this default constructor. */
     private DomainHistoryId() {}
 
-    DomainHistoryId(String domainRepoId, long id) {
+    public DomainHistoryId(String domainRepoId, long id) {
       this.domainRepoId = domainRepoId;
       this.id = id;
     }

core/src/main/java/google/registry/model/domain/token/AllocationToken.java

@@ -47,17 +47,19 @@ import google.registry.model.common.TimedTransitionProperty;
 import google.registry.model.common.TimedTransitionProperty.TimeMapper;
 import google.registry.model.common.TimedTransitionProperty.TimedTransition;
 import google.registry.model.reporting.HistoryEntry;
+import google.registry.persistence.DomainHistoryVKey;
 import google.registry.persistence.VKey;
 import google.registry.persistence.WithStringVKey;
 import google.registry.schema.replay.DatastoreAndSqlEntity;
 import java.util.Optional;
 import java.util.Set;
 import javax.annotation.Nullable;
+import javax.persistence.AttributeOverride;
+import javax.persistence.AttributeOverrides;
 import javax.persistence.Column;
 import javax.persistence.EnumType;
 import javax.persistence.Enumerated;
 import javax.persistence.Table;
-import javax.persistence.Transient;
 import org.joda.time.DateTime;
 
 /** An entity representing an allocation token. */
@@ -106,8 +108,15 @@ public class AllocationToken extends BackupGroupRoot implements Buildable, Datas
   @javax.persistence.Id @Id String token;
 
   /** The key of the history entry for which the token was used. Null if not yet used. */
-  // TODO(b/172848495): Remove the "Transient" when we can finally persist and restore this.
-  @Transient @Nullable @Index VKey<HistoryEntry> redemptionHistoryEntry;
+  @Nullable
+  @Index
+  @AttributeOverrides({
+    @AttributeOverride(name = "domainRepoId", column = @Column(name = "redemption_domain_repo_id")),
+    @AttributeOverride(
+        name = "domainHistoryId",
+        column = @Column(name = "redemption_domain_history_id"))
+  })
+  DomainHistoryVKey redemptionHistoryEntry;
 
   /** The fully-qualified domain name that this token is limited to, if any. */
   @Nullable @Index String domainName;
@@ -282,7 +291,7 @@ public class AllocationToken extends BackupGroupRoot implements Buildable, Datas
       return this;
     }
 
-    public Builder setRedemptionHistoryEntry(VKey<HistoryEntry> redemptionHistoryEntry) {
+    public Builder setRedemptionHistoryEntry(DomainHistoryVKey redemptionHistoryEntry) {
       getInstance().redemptionHistoryEntry =
           checkArgumentNotNull(redemptionHistoryEntry, "Redemption history entry must not be null");
       return this;

core/src/main/java/google/registry/model/ofy/ObjectifyService.java

@@ -42,6 +42,7 @@ import google.registry.model.translators.CidrAddressBlockTranslatorFactory;
 import google.registry.model.translators.CommitLogRevisionsTranslatorFactory;
 import google.registry.model.translators.CreateAutoTimestampTranslatorFactory;
 import google.registry.model.translators.CurrencyUnitTranslatorFactory;
+import google.registry.model.translators.DomainHistoryVKeyTranslatorFactory;
 import google.registry.model.translators.DurationTranslatorFactory;
 import google.registry.model.translators.InetAddressTranslatorFactory;
 import google.registry.model.translators.ReadableInstantUtcTranslatorFactory;
@@ -127,6 +128,7 @@ public class ObjectifyService {
             new CreateAutoTimestampTranslatorFactory(),
             new CurrencyUnitTranslatorFactory(),
             new DurationTranslatorFactory(),
+            new DomainHistoryVKeyTranslatorFactory(),
             new InetAddressTranslatorFactory(),
             new MoneyStringTranslatorFactory(),
             new ReadableInstantUtcTranslatorFactory(),

core/src/main/java/google/registry/model/reporting/HistoryEntry.java

@@ -39,8 +39,6 @@ import google.registry.model.domain.Period;
 import google.registry.model.eppcommon.Trid;
 import google.registry.model.host.HostHistory;
 import google.registry.model.host.HostResource;
-import google.registry.persistence.VKey;
-import google.registry.persistence.WithStringVKey;
 import google.registry.schema.replay.DatastoreEntity;
 import google.registry.schema.replay.SqlEntity;
 import java.util.Set;
@@ -60,7 +58,6 @@ import org.joda.time.DateTime;
 @ReportedOn
 @Entity
 @MappedSuperclass
-@WithStringVKey // TODO(b/162229294): This should be resolved during the course of that bug
 @Access(AccessType.FIELD)
 public class HistoryEntry extends ImmutableObject implements Buildable, DatastoreEntity {
 
@@ -281,19 +278,6 @@ public class HistoryEntry extends ImmutableObject implements Buildable, Datastor
         domainTransactionRecords == null ? null : ImmutableSet.copyOf(domainTransactionRecords);
   }
 
-  public static VKey<HistoryEntry> createVKey(Key<HistoryEntry> key) {
-    // TODO(b/159207551): This will likely need some revision.  As it stands, this method was
-    // introduced purely to facilitate testing of VKey specialization in VKeyTranslatorFactory.
-    // This class will likely require that functionality, though perhaps not this implementation of
-    // it.
-    // For now, just assume that the primary key of a history entry is comprised of the parent
-    // type, key and the object identifer.
-    return VKey.create(
-        HistoryEntry.class,
-        key.getParent().getKind() + "/" + key.getParent().getName() + "/" + key.getId(),
-        key);
-  }
-
   @Override
   public Builder asBuilder() {
     return new Builder(clone(this));

core/src/main/java/google/registry/model/translators/DomainHistoryVKeyTranslatorFactory.java

@@ -0,0 +1,48 @@
+// 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.model.translators;
+
+import com.google.appengine.api.datastore.Key;
+import google.registry.persistence.DomainHistoryVKey;
+import javax.annotation.Nullable;
+
+/** Translator factory for {@link DomainHistoryVKey}. */
+public class DomainHistoryVKeyTranslatorFactory
+    extends AbstractSimpleTranslatorFactory<DomainHistoryVKey, Key> {
+
+  public DomainHistoryVKeyTranslatorFactory() {
+    super(DomainHistoryVKey.class);
+  }
+
+  @Override
+  SimpleTranslator<DomainHistoryVKey, Key> createTranslator() {
+    return new SimpleTranslator<DomainHistoryVKey, Key>() {
+
+      @Nullable
+      @Override
+      public DomainHistoryVKey loadValue(@Nullable Key datastoreValue) {
+        return datastoreValue == null
+            ? null
+            : DomainHistoryVKey.create(com.googlecode.objectify.Key.create(datastoreValue));
+      }
+
+      @Nullable
+      @Override
+      public Key saveValue(@Nullable DomainHistoryVKey pojoValue) {
+        return pojoValue == null ? null : pojoValue.getOfyKey().getRaw();
+      }
+    };
+  }
+}

core/src/main/java/google/registry/model/translators/VKeyTranslatorFactory.java

@@ -43,7 +43,6 @@ public class VKeyTranslatorFactory extends AbstractSimpleTranslatorFactory<VKey,
       ALL_CLASSES.stream()
           .filter(clazz -> !clazz.isAnnotationPresent(EntitySubclass.class))
           .collect(toImmutableMap(com.googlecode.objectify.Key::getKind, identity()));
-  ;
 
   public VKeyTranslatorFactory() {
     super(VKey.class);

core/src/main/java/google/registry/persistence/DomainHistoryVKey.java

@@ -0,0 +1,62 @@
+// 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;
+
+import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
+
+import com.googlecode.objectify.Key;
+import google.registry.model.domain.DomainBase;
+import google.registry.model.domain.DomainHistory.DomainHistoryId;
+import google.registry.model.reporting.HistoryEntry;
+import javax.persistence.Embeddable;
+import javax.persistence.PostLoad;
+
+/** {@link VKey} for {@link HistoryEntry} which parent is {@link DomainBase}. */
+@Embeddable
+public class DomainHistoryVKey extends VKey<HistoryEntry> {
+
+  private String domainRepoId;
+
+  private Long domainHistoryId;
+
+  // Hibernate requires a default constructor
+  private DomainHistoryVKey() {}
+
+  private DomainHistoryVKey(String domainRepoId, long domainHistoryId) {
+    initWith(domainRepoId, domainHistoryId);
+  }
+
+  @PostLoad
+  void postLoad() {
+    initWith(domainRepoId, domainHistoryId);
+  }
+
+  private void initWith(String domainRepoId, long domainHistoryId) {
+    this.kind = HistoryEntry.class;
+    this.ofyKey =
+        Key.create(Key.create(DomainBase.class, domainRepoId), HistoryEntry.class, domainHistoryId);
+    this.sqlKey = new DomainHistoryId(domainRepoId, domainHistoryId);
+    this.domainRepoId = domainRepoId;
+    this.domainHistoryId = domainHistoryId;
+  }
+
+  /** Creates {@link DomainHistoryVKey} from the given {@link Key} instance. */
+  public static DomainHistoryVKey create(Key<HistoryEntry> ofyKey) {
+    checkArgumentNotNull(ofyKey, "ofyKey must be specified");
+    String domainRepoId = ofyKey.getParent().getName();
+    long domainHistoryId = ofyKey.getId();
+    return new DomainHistoryVKey(domainRepoId, domainHistoryId);
+  }
+}

core/src/main/java/google/registry/persistence/VKey.java

@@ -36,18 +36,20 @@ public class VKey<T> extends ImmutableObject implements Serializable {
 
   private static final long serialVersionUID = -5291472863840231240L;
 
-  // The primary key for the referenced entity.
-  private final Object primaryKey;
+  // The SQL key for the referenced entity.
+  Object sqlKey;
 
   // The objectify key for the referenced entity.
-  private final com.googlecode.objectify.Key<T> ofyKey;
+  Key<T> ofyKey;
 
-  private final Class<? extends T> kind;
+  Class<? extends T> kind;
 
-  private VKey(Class<? extends T> kind, com.googlecode.objectify.Key<T> ofyKey, Object primaryKey) {
+  VKey() {}
+
+  VKey(Class<? extends T> kind, Key<T> ofyKey, Object sqlKey) {
     this.kind = kind;
     this.ofyKey = ofyKey;
-    this.primaryKey = primaryKey;
+    this.sqlKey = sqlKey;
   }
 
   /**
@@ -62,15 +64,14 @@ public class VKey<T> extends ImmutableObject implements Serializable {
   }
 
   /** Creates a {@link VKey} which only contains the ofy primary key. */
-  public static <T> VKey<T> createOfy(Class<T> kind, com.googlecode.objectify.Key<T> ofyKey) {
+  public static <T> VKey<T> createOfy(Class<T> kind, Key<T> ofyKey) {
     checkArgumentNotNull(kind, "kind must not be null");
     checkArgumentNotNull(ofyKey, "ofyKey must not be null");
     return new VKey<T>(kind, ofyKey, null);
   }
 
   /** Creates a {@link VKey} which only contains both sql and ofy primary key. */
-  public static <T> VKey<T> create(
-      Class<T> kind, Object sqlKey, com.googlecode.objectify.Key<T> ofyKey) {
+  public static <T> VKey<T> create(Class<T> kind, Object sqlKey, Key<T> ofyKey) {
     checkArgumentNotNull(kind, "kind must not be null");
     checkArgumentNotNull(sqlKey, "sqlKey must not be null");
     checkArgumentNotNull(ofyKey, "ofyKey must not be null");
@@ -197,23 +198,23 @@ public class VKey<T> extends ImmutableObject implements Serializable {
 
   /** Returns the SQL primary key. */
   public Object getSqlKey() {
-    checkState(primaryKey != null, "Attempting obtain a null SQL key.");
-    return this.primaryKey;
+    checkState(sqlKey != null, "Attempting obtain a null SQL key.");
+    return this.sqlKey;
   }
 
   /** Returns the SQL primary key if it exists. */
   public Optional<Object> maybeGetSqlKey() {
-    return Optional.ofNullable(this.primaryKey);
+    return Optional.ofNullable(this.sqlKey);
   }
 
   /** Returns the objectify key. */
-  public com.googlecode.objectify.Key<T> getOfyKey() {
+  public Key<T> getOfyKey() {
     checkState(ofyKey != null, "Attempting obtain a null Objectify key.");
     return this.ofyKey;
   }
 
   /** Returns the objectify key if it exists. */
-  public Optional<com.googlecode.objectify.Key<T>> maybeGetOfyKey() {
+  public Optional<Key<T>> maybeGetOfyKey() {
     return Optional.ofNullable(this.ofyKey);
   }
 

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

@@ -112,7 +112,6 @@
     <class>google.registry.model.host.VKeyConverter_HostResource</class>
     <class>google.registry.model.poll.VKeyConverter_Autorenew</class>
     <class>google.registry.model.poll.VKeyConverter_OneTime</class>
-    <class>google.registry.model.reporting.VKeyConverter_HistoryEntry</class>
 
     <!-- TODO(weiminyu): check out application-layer validation. -->
     <validation-mode>NONE</validation-mode>