From 6d36eb911309d4741ab26c7f7212b642826752b1 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 1 Nov 2017 10:42:27 -0700
Subject: [PATCH] Denormalize Diff PHIDs onto Revisions

Summary:
Ref T12539. See PHI190. Currently, each Diff has a `revisionID`, but Revisions do not point at the current active diff. To find the active diff for a given revision, we need to issue a separate query.

Furthermore, this query is inefficient for bulk loads: if we have a lot of revisions, we end up querying for all diff IDs for all those revisions first, then selecting the largest ones and querying again to get the actual diff objects. This strategy could likely be optimized but the query is a mess in any case.

In several cases, it's useful to have the active diff PHID without needing to do a second query -- sometimes for convenience, and sometimes for performance.

T12539 is an example of such a case: it would be nice to refine the bucketing logic (which only depends on active diff PHIDs), but it feels bad to make the page heavier to do it.

For now, this is unused. I'll start using it to fix the bucketing issue, and then we can expand it gradually to address other performance/convenience issues.

Test Plan:
  - Ran migrations, inspected database, saw sensible values.
  - Created a new revision, saw a sensible database value.
  - Updated an existing revision, saw database update properly.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12539

Differential Revision: https://secure.phabricator.com/D18756
---
 .../autopatches/20171101.diff.01.active.sql   |  2 ++
 .../autopatches/20171101.diff.02.populate.php | 24 +++++++++++++++++++
 .../editor/DifferentialTransactionEditor.php  |  3 +--
 .../storage/DifferentialRevision.php          |  2 ++
 4 files changed, 29 insertions(+), 2 deletions(-)
 create mode 100644 resources/sql/autopatches/20171101.diff.01.active.sql
 create mode 100644 resources/sql/autopatches/20171101.diff.02.populate.php

diff --git a/resources/sql/autopatches/20171101.diff.01.active.sql b/resources/sql/autopatches/20171101.diff.01.active.sql
new file mode 100644
index 0000000000..aee8c5aa13
--- /dev/null
+++ b/resources/sql/autopatches/20171101.diff.01.active.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_differential.differential_revision
+  ADD activeDiffPHID VARBINARY(64) NOT NULL;
diff --git a/resources/sql/autopatches/20171101.diff.02.populate.php b/resources/sql/autopatches/20171101.diff.02.populate.php
new file mode 100644
index 0000000000..b41b0aa51d
--- /dev/null
+++ b/resources/sql/autopatches/20171101.diff.02.populate.php
@@ -0,0 +1,24 @@
+<?php
+
+$table = new DifferentialRevision();
+$conn = $table->establishConnection('w');
+$diff_table = new DifferentialDiff();
+
+foreach (new LiskMigrationIterator($table) as $revision) {
+  $revision_id = $revision->getID();
+
+  $diff_row = queryfx_one(
+    $conn,
+    'SELECT phid FROM %T WHERE revisionID = %d ORDER BY id DESC LIMIT 1',
+    $diff_table->getTableName(),
+    $revision_id);
+
+  if ($diff_row) {
+    queryfx(
+      $conn,
+      'UPDATE %T SET activeDiffPHID = %s WHERE id = %d',
+      $table->getTableName(),
+      $diff_row['phid'],
+      $revision_id);
+  }
+}
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
index 79a582a43f..3e6cca1edb 100644
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -138,8 +138,7 @@ final class DifferentialTransactionEditor
           $object->setRepositoryPHID($diff->getRepositoryPHID());
         }
         $object->attachActiveDiff($diff);
-
-        // TODO: Update the `diffPHID` once we add that.
+        $object->setActiveDiffPHID($diff->getPHID());
         return;
     }
 
diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php
index 467e503f6f..61f934f13b 100644
--- a/src/applications/differential/storage/DifferentialRevision.php
+++ b/src/applications/differential/storage/DifferentialRevision.php
@@ -35,6 +35,8 @@ final class DifferentialRevision extends DifferentialDAO
   protected $mailKey;
   protected $branchName;
   protected $repositoryPHID;
+  protected $activeDiffPHID;
+
   protected $viewPolicy = PhabricatorPolicies::POLICY_USER;
   protected $editPolicy = PhabricatorPolicies::POLICY_USER;
   protected $properties = array();