Use the correct text VKey for HostResource's superordinateDomain (#608)

* Store the superordinateDomain reference as a VKey rather than Key

This is a reference to a Domain object, so we should store it as a VKey
in reference to the Domain table. This should not affect any business
logic, but rather will allow us to set up the SQL tables for
HostResource et al. properly.

core/src/main/java/google/registry/batch/DeleteContactsAndHostsAction.java

@@ -347,7 +347,7 @@ public class DeleteContactsAndHostsAction implements Runnable {
       String resourceClientId = resource.getPersistedCurrentSponsorClientId();
       if (resource instanceof HostResource && ((HostResource) resource).isSubordinate()) {
         resourceClientId =
-            ofy().load().key(((HostResource) resource).getSuperordinateDomain()).now()
+            tm().load(((HostResource) resource).getSuperordinateDomain())
                 .cloneProjectedAtTime(now)
                 .getCurrentSponsorClientId();
       }
@@ -466,10 +466,11 @@ public class DeleteContactsAndHostsAction implements Runnable {
         HostResource host = (HostResource) existingResource;
         if (host.isSubordinate()) {
           dnsQueue.addHostRefreshTask(host.getFullyQualifiedHostName());
-          ofy().save().entity(
-              ofy().load().key(host.getSuperordinateDomain()).now().asBuilder()
-                  .removeSubordinateHost(host.getFullyQualifiedHostName())
-                  .build());
+          tm().saveNewOrUpdate(
+                  tm().load(host.getSuperordinateDomain())
+                      .asBuilder()
+                      .removeSubordinateHost(host.getFullyQualifiedHostName())
+                      .build());
         }
       } else {
         throw new IllegalStateException(

core/src/main/java/google/registry/flows/host/HostCreateFlow.java

@@ -128,7 +128,7 @@ public final class HostCreateFlow implements TransactionalFlow {
             .setFullyQualifiedHostName(targetId)
             .setInetAddresses(command.getInetAddresses())
             .setRepoId(createRepoId(ObjectifyService.allocateId(), roidSuffix))
-            .setSuperordinateDomain(superordinateDomain.map(Key::create).orElse(null))
+            .setSuperordinateDomain(superordinateDomain.map(DomainBase::createVKey).orElse(null))
             .build();
     historyBuilder
         .setType(HistoryEntry.Type.HOST_CREATE)

core/src/main/java/google/registry/flows/host/HostDeleteFlow.java

@@ -96,8 +96,7 @@ public final class HostDeleteFlow implements TransactionalFlow {
       // the client id, needs to be read off of it.
       EppResource owningResource =
           existingHost.isSubordinate()
-              ? ofy().load().key(existingHost.getSuperordinateDomain()).now()
-                  .cloneProjectedAtTime(now)
+              ? tm().load(existingHost.getSuperordinateDomain()).cloneProjectedAtTime(now)
               : existingHost;
       verifyResourceOwnership(clientId, owningResource);
     }

core/src/main/java/google/registry/flows/host/HostInfoFlow.java

@@ -18,7 +18,7 @@ import static google.registry.flows.FlowUtils.validateClientIsLoggedIn;
 import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence;
 import static google.registry.flows.host.HostFlowUtils.validateHostName;
 import static google.registry.model.EppResourceUtils.isLinked;
-import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 
 import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.Key;
@@ -77,7 +77,7 @@ public final class HostInfoFlow implements Flow {
     // there is no superordinate domain, the host's own values for these fields will be correct.
     if (host.isSubordinate()) {
       DomainBase superordinateDomain =
-          ofy().load().key(host.getSuperordinateDomain()).now().cloneProjectedAtTime(now);
+          tm().load(host.getSuperordinateDomain()).cloneProjectedAtTime(now);
       hostInfoDataBuilder
           .setCurrentSponsorClientId(superordinateDomain.getCurrentSponsorClientId())
           .setLastTransferTime(host.computeLastTransferTime(superordinateDomain));

core/src/main/java/google/registry/flows/host/HostUpdateFlow.java

@@ -60,6 +60,7 @@ import google.registry.model.host.HostResource;
 import google.registry.model.index.ForeignKeyIndex;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.model.reporting.IcannReportingTypes.ActivityReportField;
+import google.registry.persistence.VKey;
 import java.util.Objects;
 import java.util.Optional;
 import javax.inject.Inject;
@@ -136,9 +137,10 @@ public final class HostUpdateFlow implements TransactionalFlow {
     boolean isHostRename = suppliedNewHostName != null;
     String oldHostName = targetId;
     String newHostName = firstNonNull(suppliedNewHostName, oldHostName);
-    DomainBase oldSuperordinateDomain = existingHost.isSubordinate()
-        ? ofy().load().key(existingHost.getSuperordinateDomain()).now().cloneProjectedAtTime(now)
-        : null;
+    DomainBase oldSuperordinateDomain =
+        existingHost.isSubordinate()
+            ? tm().load(existingHost.getSuperordinateDomain()).cloneProjectedAtTime(now)
+            : null;
     // Note that lookupSuperordinateDomain calls cloneProjectedAtTime on the domain for us.
     Optional<DomainBase> newSuperordinateDomain =
         lookupSuperordinateDomain(validateHostName(newHostName), now);
@@ -153,8 +155,8 @@ public final class HostUpdateFlow implements TransactionalFlow {
     AddRemove remove = command.getInnerRemove();
     checkSameValuesNotAddedAndRemoved(add.getStatusValues(), remove.getStatusValues());
     checkSameValuesNotAddedAndRemoved(add.getInetAddresses(), remove.getInetAddresses());
-    Key<DomainBase> newSuperordinateDomainKey =
-        newSuperordinateDomain.map(Key::create).orElse(null);
+    VKey<DomainBase> newSuperordinateDomainKey =
+        newSuperordinateDomain.map(DomainBase::createVKey).orElse(null);
     // If the superordinateDomain field is changing, set the lastSuperordinateChange to now.
     DateTime lastSuperordinateChange =
         Objects.equals(newSuperordinateDomainKey, existingHost.getSuperordinateDomain())
@@ -280,28 +282,28 @@ public final class HostUpdateFlow implements TransactionalFlow {
     if (existingHost.isSubordinate()
         && newHost.isSubordinate()
         && Objects.equals(
-            existingHost.getSuperordinateDomain(),
-            newHost.getSuperordinateDomain())) {
-      ofy().save().entity(
-          ofy().load().key(existingHost.getSuperordinateDomain()).now().asBuilder()
-              .removeSubordinateHost(existingHost.getFullyQualifiedHostName())
-              .addSubordinateHost(newHost.getFullyQualifiedHostName())
-              .build());
+            existingHost.getSuperordinateDomain(), newHost.getSuperordinateDomain())) {
+      tm().saveNewOrUpdate(
+              tm().load(existingHost.getSuperordinateDomain())
+                  .asBuilder()
+                  .removeSubordinateHost(existingHost.getFullyQualifiedHostName())
+                  .addSubordinateHost(newHost.getFullyQualifiedHostName())
+                  .build());
       return;
     }
     if (existingHost.isSubordinate()) {
-      ofy().save().entity(
-          ofy().load().key(existingHost.getSuperordinateDomain()).now()
-              .asBuilder()
-              .removeSubordinateHost(existingHost.getFullyQualifiedHostName())
-              .build());
+      tm().saveNewOrUpdate(
+              tm().load(existingHost.getSuperordinateDomain())
+                  .asBuilder()
+                  .removeSubordinateHost(existingHost.getFullyQualifiedHostName())
+                  .build());
     }
     if (newHost.isSubordinate()) {
-      ofy().save().entity(
-          ofy().load().key(newHost.getSuperordinateDomain()).now()
-              .asBuilder()
-              .addSubordinateHost(newHost.getFullyQualifiedHostName())
-              .build());
+      tm().saveNewOrUpdate(
+              tm().load(newHost.getSuperordinateDomain())
+                  .asBuilder()
+                  .addSubordinateHost(newHost.getFullyQualifiedHostName())
+                  .build());
     }
   }
 

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

@@ -66,6 +66,7 @@ import google.registry.model.registry.Registry;
 import google.registry.model.transfer.TransferData;
 import google.registry.model.transfer.TransferStatus;
 import google.registry.persistence.VKey;
+import google.registry.persistence.WithStringVKey;
 import google.registry.schema.replay.DatastoreAndSqlEntity;
 import google.registry.util.CollectionUtils;
 import java.util.HashSet;
@@ -106,6 +107,7 @@ import org.joda.time.Interval;
       @javax.persistence.Index(columnList = "fullyQualifiedDomainName"),
       @javax.persistence.Index(columnList = "tld")
     })
+@WithStringVKey
 @ExternalMessagingName("domain")
 public class DomainBase extends EppResource
     implements DatastoreAndSqlEntity, ForeignKeyedEppResource, ResourceWithTransferData {

core/src/main/java/google/registry/model/host/HostResource.java

@@ -76,7 +76,7 @@ public class HostResource extends EppResource
   @Index
   @IgnoreSave(IfNull.class)
   @DoNotHydrate
-  Key<DomainBase> superordinateDomain;
+  VKey<DomainBase> superordinateDomain;
 
   /**
    * The time that this resource was last transferred.
@@ -98,7 +98,7 @@ public class HostResource extends EppResource
     return fullyQualifiedHostName;
   }
 
-  public Key<DomainBase> getSuperordinateDomain() {
+  public VKey<DomainBase> getSuperordinateDomain() {
     return superordinateDomain;
   }
 
@@ -145,18 +145,18 @@ public class HostResource extends EppResource
    *
    * <p>If the host is not subordinate the domain can be null and we just return last transfer time.
    *
-   * @param superordinateDomain the loaded superordinate domain, which must match the key in
-   *     the {@link #superordinateDomain} field. Passing it as a parameter allows the caller to
-   *     control the degree of consistency used to load it.
+   * @param superordinateDomain the loaded superordinate domain, which must match the key in the
+   *     {@link #superordinateDomain} field. Passing it as a parameter allows the caller to control
+   *     the degree of consistency used to load it.
    */
-   public DateTime computeLastTransferTime(@Nullable DomainBase superordinateDomain) {
+  public DateTime computeLastTransferTime(@Nullable DomainBase superordinateDomain) {
     if (!isSubordinate()) {
       checkArgument(superordinateDomain == null);
       return getLastTransferTime();
     }
     checkArgument(
         superordinateDomain != null
-            && Key.create(superordinateDomain).equals(getSuperordinateDomain()));
+            && superordinateDomain.createVKey().equals(getSuperordinateDomain()));
     DateTime lastSuperordinateChange =
         Optional.ofNullable(getLastSuperordinateChange()).orElse(getCreationTime());
     DateTime lastTransferOfCurrentSuperordinate =
@@ -207,7 +207,7 @@ public class HostResource extends EppResource
           difference(getInstance().getInetAddresses(), inetAddresses)));
     }
 
-    public Builder setSuperordinateDomain(Key<DomainBase> superordinateDomain) {
+    public Builder setSuperordinateDomain(VKey<DomainBase> superordinateDomain) {
       getInstance().superordinateDomain = superordinateDomain;
       return this;
     }

core/src/main/java/google/registry/rdap/RdapJsonFormatter.java

@@ -433,10 +433,7 @@ public class RdapJsonFormatter {
         statuses.add(StatusValue.LINKED);
       }
       if (hostResource.isSubordinate()
-          && ofy()
-              .load()
-              .key(hostResource.getSuperordinateDomain())
-              .now()
+          && tm().load(hostResource.getSuperordinateDomain())
               .cloneProjectedAtTime(getRequestTime())
               .getStatusValues()
               .contains(StatusValue.PENDING_TRANSFER)) {

core/src/main/java/google/registry/rde/HostResourceToXjcConverter.java

@@ -17,7 +17,6 @@ package google.registry.rde;
 import static com.google.common.base.Preconditions.checkArgument;
 
 import com.google.common.net.InetAddresses;
-import com.googlecode.objectify.Key;
 import google.registry.model.domain.DomainBase;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.host.HostResource;
@@ -35,9 +34,8 @@ import org.joda.time.DateTime;
 final class HostResourceToXjcConverter {
 
   /** Converts a subordinate {@link HostResource} to {@link XjcRdeHostElement}. */
-  static XjcRdeHostElement convertSubordinate(
-      HostResource host, DomainBase superordinateDomain) {
-    checkArgument(Key.create(superordinateDomain).equals(host.getSuperordinateDomain()));
+  static XjcRdeHostElement convertSubordinate(HostResource host, DomainBase superordinateDomain) {
+    checkArgument(superordinateDomain.createVKey().equals(host.getSuperordinateDomain()));
     return new XjcRdeHostElement(convertSubordinateHost(host, superordinateDomain));
   }
 

core/src/main/java/google/registry/rde/RdeStagingMapper.java

@@ -18,6 +18,7 @@ import static com.google.common.base.Strings.nullToEmpty;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static google.registry.model.EppResourceUtils.loadAtPointInTime;
 import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 
 import com.google.appengine.tools.mapreduce.Mapper;
 import com.google.auto.value.AutoValue;
@@ -186,13 +187,16 @@ public final class RdeStagingMapper extends Mapper<EppResource, PendingDeposit,
         return result;
       } else if (resource instanceof HostResource) {
         HostResource host = (HostResource) resource;
-        result = Optional.of(host.isSubordinate()
-            ? marshaller.marshalSubordinateHost(
-                host,
-                // Note that loadAtPointInTime() does cloneProjectedAtTime(watermark) for us.
-                loadAtPointInTime(
-                    ofy().load().key(host.getSuperordinateDomain()).now(), watermark).now())
-            : marshaller.marshalExternalHost(host));
+        result =
+            Optional.of(
+                host.isSubordinate()
+                    ? marshaller.marshalSubordinateHost(
+                        host,
+                        // Note that loadAtPointInTime() does cloneProjectedAtTime(watermark) for
+                        // us.
+                        loadAtPointInTime(tm().load(host.getSuperordinateDomain()), watermark)
+                            .now())
+                    : marshaller.marshalExternalHost(host));
         cache.put(WatermarkModePair.create(watermark, RdeMode.FULL), result);
         cache.put(WatermarkModePair.create(watermark, RdeMode.THIN), result);
         return result;

core/src/main/java/google/registry/whois/NameserverWhoisResponse.java

@@ -16,7 +16,7 @@ package google.registry.whois;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
-import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.net.InetAddresses;
@@ -49,7 +49,7 @@ final class NameserverWhoisResponse extends WhoisResponseImpl {
       HostResource host = hosts.get(i);
       String clientId =
           host.isSubordinate()
-              ? ofy().load().key(host.getSuperordinateDomain()).now()
+              ? tm().load(host.getSuperordinateDomain())
                   .cloneProjectedAtTime(getTimestamp())
                   .getCurrentSponsorClientId()
               : host.getPersistedCurrentSponsorClientId();

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

@@ -61,6 +61,7 @@
 
     <!-- Generated converters for VKey -->
     <class>google.registry.model.billing.VKeyConverter_BillingEvent</class>
+    <class>google.registry.model.domain.VKeyConverter_DomainBase</class>
     <class>google.registry.model.domain.token.VKeyConverter_AllocationToken</class>
     <class>google.registry.model.host.VKeyConverter_HostResource</class>
     <class>google.registry.model.contact.VKeyConverter_ContactResource</class>