From 1e6287372d9317dfd444093caccd1aba8c1bad22 Mon Sep 17 00:00:00 2001
From: gbrodman <gbrodman@google.com>
Date: Wed, 6 Oct 2021 15:20:31 -0400
Subject: [PATCH] Use a more efficient query to find resources in histories
 (#1354)

---
 config/presubmits.py                          |  1 +
 .../CreateSyntheticHistoryEntriesAction.java  | 45 ++++++++++---------
 2 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/config/presubmits.py b/config/presubmits.py
index f1f499a8f..0fff4499d 100644
--- a/config/presubmits.py
+++ b/config/presubmits.py
@@ -213,6 +213,7 @@ PRESUBMITS = {
         {"src/test", "ActivityReportingQueryBuilder.java",
          # This class contains helper method to make queries in Beam.
          "RegistryJpaIO.java",
+         "CreateSyntheticHistoryEntriesAction.java",
          # TODO(b/179158393): Remove everything below, which should be done
          # using Criteria
          "JpaTransactionManager.java",
diff --git a/core/src/main/java/google/registry/tools/javascrap/CreateSyntheticHistoryEntriesAction.java b/core/src/main/java/google/registry/tools/javascrap/CreateSyntheticHistoryEntriesAction.java
index 64e756d15..8beebf0f4 100644
--- a/core/src/main/java/google/registry/tools/javascrap/CreateSyntheticHistoryEntriesAction.java
+++ b/core/src/main/java/google/registry/tools/javascrap/CreateSyntheticHistoryEntriesAction.java
@@ -21,6 +21,7 @@ import static google.registry.persistence.transaction.TransactionManagerFactory.
 import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm;
 
 import com.google.appengine.tools.mapreduce.Mapper;
+import com.google.common.base.CaseFormat;
 import com.google.common.collect.ImmutableList;
 import com.googlecode.objectify.Key;
 import google.registry.config.RegistryConfig.Config;
@@ -29,16 +30,12 @@ import google.registry.mapreduce.inputs.EppResourceInputs;
 import google.registry.model.EppResource;
 import google.registry.model.domain.DomainHistory;
 import google.registry.model.reporting.HistoryEntry;
-import google.registry.persistence.transaction.CriteriaQueryBuilder;
 import google.registry.rde.RdeStagingAction;
 import google.registry.request.Action;
 import google.registry.request.Response;
 import google.registry.request.auth.Auth;
 import google.registry.tools.server.GenerateZoneFilesAction;
-import java.util.Optional;
 import javax.inject.Inject;
-import javax.persistence.criteria.CriteriaBuilder;
-import javax.persistence.criteria.CriteriaQuery;
 
 /**
  * A mapreduce that creates synthetic history objects in SQL for all {@link EppResource} objects.
@@ -105,8 +102,9 @@ public class CreateSyntheticHistoryEntriesAction implements Runnable {
         .sendLinkToMapreduceConsole(response);
   }
 
-  // Lifted from HistoryEntryDao
-  private static Optional<? extends HistoryEntry> mostRecentHistoryFromSql(EppResource resource) {
+  // Returns true iff any of the *History objects in SQL contain a representation of this resource
+  // at the point in time that the *History object was created.
+  private static boolean hasHistoryContainingResource(EppResource resource) {
     return jpaTm()
         .transact(
             () -> {
@@ -114,18 +112,24 @@ public class CreateSyntheticHistoryEntriesAction implements Runnable {
               Class<? extends HistoryEntry> historyClass =
                   getHistoryClassFromParent(resource.getClass());
               // The field representing repo ID unfortunately varies by history class
-              String repoIdFieldName = getRepoIdFieldNameFromHistoryClass(historyClass);
-              CriteriaBuilder criteriaBuilder = jpaTm().getEntityManager().getCriteriaBuilder();
-              CriteriaQuery<? extends HistoryEntry> criteriaQuery =
-                  CriteriaQueryBuilder.create(historyClass)
-                      .where(repoIdFieldName, criteriaBuilder::equal, resource.getRepoId())
-                      .orderByDesc("modificationTime")
-                      .build();
-              return jpaTm()
-                  .criteriaQuery(criteriaQuery)
-                  .setMaxResults(1)
-                  .getResultStream()
-                  .findFirst();
+              String repoIdFieldName =
+                  CaseFormat.LOWER_CAMEL.to(
+                      CaseFormat.LOWER_UNDERSCORE,
+                      getRepoIdFieldNameFromHistoryClass(historyClass));
+              // The "history" fields in the *History objects are all prefixed with "history_". If
+              // any of the non-"history_" fields are non-null, that means that that row contains
+              // a representation of that EppResource at that point in time. We use creation_time as
+              // a marker since it's the simplest field and all EppResources will have it.
+              return (boolean)
+                  jpaTm()
+                      .getEntityManager()
+                      .createNativeQuery(
+                          String.format(
+                              "SELECT EXISTS (SELECT 1 FROM \"%s\" WHERE %s = :repoId AND"
+                                  + " creation_time IS NOT NULL)",
+                              historyClass.getSimpleName(), repoIdFieldName))
+                      .setParameter("repoId", resource.getRepoId())
+                      .getSingleResult();
             });
   }
 
@@ -164,10 +168,7 @@ public class CreateSyntheticHistoryEntriesAction implements Runnable {
       EppResource eppResource = auditedOfy().load().key(resourceKey).now();
       // Only save new history entries if the most recent history for this object in SQL does not
       // have the resource at that point in time already
-      Optional<? extends HistoryEntry> maybeHistory = mostRecentHistoryFromSql(eppResource);
-      if (maybeHistory
-          .map(history -> !history.getResourceAtPointInTime().isPresent())
-          .orElse(true)) {
+      if (!hasHistoryContainingResource(eppResource)) {
         ofyTm()
             .transact(
                 () ->