Partially convert RDAP ofy calls to tm calls (#964)

* Partially convert RDAP ofy calls to tm calls

This converts the simple retrieval actions but does not touch the more
complicated search actions -- those use some ofy() query searching logic
and will likely end up being significantly more complicated than this
change. Here, though, we are just changing the calls that can be
converted easily to tm() lookups.

To change in future PRs:
- RdapDomainSearchAction
- RdapEntitySearchAction
- RdapNameserverSearchAction
- RdapSearchActionBase

core/src/main/java/google/registry/model/registrar/Registrar.java

@@ -996,6 +996,13 @@ public class Registrar extends ImmutableObject
     return CACHE_BY_CLIENT_ID.get().values();
   }
 
+  /** Loads all registrar keys using an in-memory cache. */
+  public static ImmutableSet<VKey<Registrar>> loadAllKeysCached() {
+    return CACHE_BY_CLIENT_ID.get().keySet().stream()
+        .map(Registrar::createVKey)
+        .collect(toImmutableSet());
+  }
+
   /** Loads and returns a registrar entity by its client id directly from Datastore. */
   public static Optional<Registrar> loadByClientId(String clientId) {
     checkArgument(!Strings.isNullOrEmpty(clientId), "clientId must be specified");

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

@@ -14,7 +14,8 @@
 
 package google.registry.rdap;
 
-import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
+import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm;
 import static google.registry.rdap.RdapUtils.getRegistrarByIanaIdentifier;
 import static google.registry.rdap.RdapUtils.getRegistrarByName;
 import static google.registry.request.Action.Method.GET;
@@ -23,9 +24,9 @@ import static google.registry.request.Action.Method.HEAD;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.primitives.Longs;
 import com.google.re2j.Pattern;
-import com.googlecode.objectify.Key;
 import google.registry.model.contact.ContactResource;
 import google.registry.model.registrar.Registrar;
+import google.registry.persistence.VKey;
 import google.registry.rdap.RdapJsonFormatter.OutputDataType;
 import google.registry.rdap.RdapMetrics.EndpointType;
 import google.registry.rdap.RdapObjectClasses.RdapEntity;
@@ -69,13 +70,14 @@ public class RdapEntityAction extends RdapActionBase {
     // RDAP Technical Implementation Guide 2.3.1 - MUST support contact entity lookup using the
     // handle
     if (ROID_PATTERN.matcher(pathSearchString).matches()) {
-      Key<ContactResource> contactKey = Key.create(ContactResource.class, pathSearchString);
-      ContactResource contactResource = ofy().load().key(contactKey).now();
+      VKey<ContactResource> contactVKey = VKey.create(ContactResource.class, pathSearchString);
+      Optional<ContactResource> contactResource =
+          transactIfJpaTm(() -> tm().loadByKeyIfPresent(contactVKey));
       // As per Andy Newton on the regext mailing list, contacts by themselves have no role, since
       // they are global, and might have different roles for different domains.
-      if (contactResource != null && isAuthorized(contactResource)) {
+      if (contactResource.isPresent() && isAuthorized(contactResource.get())) {
         return rdapJsonFormatter.createRdapContactEntity(
-            contactResource, ImmutableSet.of(), OutputDataType.FULL);
+            contactResource.get(), ImmutableSet.of(), OutputDataType.FULL);
       }
     }
 

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

@@ -20,8 +20,8 @@ import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap;
 import static google.registry.model.EppResourceUtils.isLinked;
-import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
+import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm;
 import static google.registry.rdap.RdapIcannStandardInformation.CONTACT_REDACTED_VALUE;
 import static google.registry.util.CollectionUtils.union;
 
@@ -35,7 +35,6 @@ import com.google.common.collect.Streams;
 import com.google.common.flogger.FluentLogger;
 import com.google.common.net.InetAddresses;
 import com.google.gson.JsonArray;
-import com.googlecode.objectify.Key;
 import google.registry.config.RegistryConfig.Config;
 import google.registry.model.EppResource;
 import google.registry.model.contact.ContactAddress;
@@ -77,7 +76,6 @@ import java.net.URI;
 import java.nio.file.Paths;
 import java.util.HashMap;
 import java.util.Locale;
-import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Stream;
@@ -343,15 +341,11 @@ public class RdapJsonFormatter {
     // Kick off the database loads of the nameservers that we will need, so it can load
     // asynchronously while we load and process the contacts.
     ImmutableSet<HostResource> loadedHosts =
-        ImmutableSet.copyOf(tm().loadByKeys(domainBase.getNameservers()).values());
+        transactIfJpaTm(
+            () -> ImmutableSet.copyOf(tm().loadByKeys(domainBase.getNameservers()).values()));
     // Load the registrant and other contacts and add them to the data.
-    Map<Key<ContactResource>, ContactResource> loadedContacts =
-        ofy()
-            .load()
-            .keys(
-                domainBase.getReferencedContacts().stream()
-                    .map(VKey::getOfyKey)
-                    .collect(toImmutableSet()));
+    ImmutableMap<VKey<? extends ContactResource>, ContactResource> loadedContacts =
+        transactIfJpaTm(() -> tm().loadByKeysIfPresent(domainBase.getReferencedContacts()));
     // RDAP Response Profile 2.7.3, A domain MUST have the REGISTRANT, ADMIN, TECH roles and MAY
     // have others. We also add the BILLING.
     //
@@ -359,16 +353,16 @@ public class RdapJsonFormatter {
     // fields we don't want to show (as opposed to not having contacts at all) because of GDPR etc.
     //
     // the GDPR redaction is handled in createRdapContactEntity
-    ImmutableSetMultimap<Key<ContactResource>, Type> contactsToRoles =
+    ImmutableSetMultimap<VKey<ContactResource>, Type> contactsToRoles =
         Streams.concat(
                 domainBase.getContacts().stream(),
                 Stream.of(DesignatedContact.create(Type.REGISTRANT, domainBase.getRegistrant())))
             .sorted(DESIGNATED_CONTACT_ORDERING)
             .collect(
                 toImmutableSetMultimap(
-                    contact -> contact.getContactKey().getOfyKey(), DesignatedContact::getType));
+                    DesignatedContact::getContactKey, DesignatedContact::getType));
 
-    for (Key<ContactResource> contactKey : contactsToRoles.keySet()) {
+    for (VKey<ContactResource> contactKey : contactsToRoles.keySet()) {
       Set<RdapEntity.Role> roles =
           contactsToRoles.get(contactKey).stream()
               .map(RdapJsonFormatter::convertContactTypeToRdapRole)
@@ -430,10 +424,12 @@ public class RdapJsonFormatter {
         statuses.add(StatusValue.LINKED);
       }
       if (hostResource.isSubordinate()
-          && tm().loadByKey(hostResource.getSuperordinateDomain())
-              .cloneProjectedAtTime(getRequestTime())
-              .getStatusValues()
-              .contains(StatusValue.PENDING_TRANSFER)) {
+          && transactIfJpaTm(
+              () ->
+                  tm().loadByKey(hostResource.getSuperordinateDomain())
+                      .cloneProjectedAtTime(getRequestTime())
+                      .getStatusValues()
+                      .contains(StatusValue.PENDING_TRANSFER))) {
         statuses.add(StatusValue.PENDING_TRANSFER);
       }
       builder

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

@@ -392,22 +392,6 @@ public abstract class RdapSearchActionBase extends RdapActionBase {
     return setOtherQueryAttributes(query, deletedItemHandling, resultSetMaxSize);
   }
 
-  /** Handles searches by key using a simple string. */
-  static <T extends EppResource> Query<T> queryItemsByKey(
-      Class<T> clazz,
-      String queryString,
-      DeletedItemHandling deletedItemHandling,
-      int resultSetMaxSize) {
-    if (queryString.length() < RdapSearchPattern.MIN_INITIAL_STRING_LENGTH) {
-      throw new UnprocessableEntityException(
-          String.format(
-              "Initial search string must be at least %d characters",
-              RdapSearchPattern.MIN_INITIAL_STRING_LENGTH));
-    }
-    Query<T> query = ofy().load().type(clazz).filterKey("=", Key.create(clazz, queryString));
-    return setOtherQueryAttributes(query, deletedItemHandling, resultSetMaxSize);
-  }
-
   static <T extends EppResource> Query<T> setOtherQueryAttributes(
       Query<T> query, DeletedItemHandling deletedItemHandling, int resultSetMaxSize) {
     if (deletedItemHandling != DeletedItemHandling.INCLUDE) {

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

@@ -16,7 +16,6 @@ package google.registry.rdap;
 
 import static com.google.common.base.Preconditions.checkArgument;
 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 static java.nio.charset.StandardCharsets.UTF_8;
 
@@ -34,7 +33,6 @@ import com.google.gson.Gson;
 import com.google.gson.JsonArray;
 import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
-import com.googlecode.objectify.Key;
 import google.registry.keyring.api.KeyModule;
 import google.registry.model.registrar.Registrar;
 import google.registry.model.registry.Registries;
@@ -197,37 +195,37 @@ public final class UpdateRegistrarRdapBaseUrlsAction implements Runnable {
   @Override
   public void run() {
     ImmutableSetMultimap<String, String> ianaToBaseUrls = getRdapBaseUrlsPerIanaId();
-
-    for (Key<Registrar> registrarKey : ofy().load().type(Registrar.class).keys()) {
-      tm()
-          .transact(
-              () -> {
-                Registrar registrar = ofy().load().key(registrarKey).now();
-                // Has the registrar been deleted since we loaded the key? (unlikly, especially
-                // given we don't delete registrars...)
-                if (registrar == null) {
-                  return;
-                }
-                // Only update REAL registrars
-                if (registrar.getType() != Registrar.Type.REAL) {
-                  return;
-                }
-                String ianaId = String.valueOf(registrar.getIanaIdentifier());
-                ImmutableSet<String> baseUrls = ianaToBaseUrls.get(ianaId);
-                // If this registrar already has these values, skip it
-                if (registrar.getRdapBaseUrls().equals(baseUrls)) {
-                  logger.atInfo().log(
-                      "No change in RdapBaseUrls for registrar %s (ianaId %s)",
-                      registrar.getClientId(), ianaId);
-                  return;
-                }
-                logger.atInfo().log(
-                    "Updating RdapBaseUrls for registrar %s (ianaId %s) from %s to %s",
-                    registrar.getClientId(), ianaId, registrar.getRdapBaseUrls(), baseUrls);
-                ofy()
-                    .save()
-                    .entity(registrar.asBuilder().setRdapBaseUrls(baseUrls).build());
-              });
-    }
+    Registrar.loadAllKeysCached()
+        .forEach(
+            (key) ->
+                tm().transact(
+                        () -> {
+                          Registrar registrar = tm().loadByKey(key);
+                          // Has the registrar been deleted since we loaded the key? (unlikely,
+                          // especially given we don't delete registrars...)
+                          if (registrar == null) {
+                            return;
+                          }
+                          // Only update REAL registrars
+                          if (registrar.getType() != Registrar.Type.REAL) {
+                            return;
+                          }
+                          String ianaId = String.valueOf(registrar.getIanaIdentifier());
+                          ImmutableSet<String> baseUrls = ianaToBaseUrls.get(ianaId);
+                          // If this registrar already has these values, skip it
+                          if (registrar.getRdapBaseUrls().equals(baseUrls)) {
+                            logger.atInfo().log(
+                                "No change in RdapBaseUrls for registrar %s (ianaId %s)",
+                                registrar.getClientId(), ianaId);
+                            return;
+                          }
+                          logger.atInfo().log(
+                              "Updating RdapBaseUrls for registrar %s (ianaId %s) from %s to %s",
+                              registrar.getClientId(),
+                              ianaId,
+                              registrar.getRdapBaseUrls(),
+                              baseUrls);
+                          tm().put(registrar.asBuilder().setRdapBaseUrls(baseUrls).build());
+                        }));
   }
 }