Assign PHIDs to all diffs

Summary:
Ref T1049. Ref T2222. `DifferentialDiff` does not currently have a PHID, but we need it for Harbormaster and ApplicationTransactions. See some discussion in D7501.

(I split the SQL into two sections so we can't fail in the middle. At some point, I'd like to do a pass on the migration stuff and get this happening automatically, and also simplify the PatchList.)

Test Plan:
  - Ran `bin/storage upgrade`.
  - Checked for valid PHIDs in the database.
  - Used `phid.query` to look up a diff by PHID.
  - Created a new diff and verified it got a PHID.

Reviewers: btrahan, hach-que

Reviewed By: btrahan

CC: aran, vrana

Maniphest Tasks: T2222, T1049

Differential Revision: https://secure.phabricator.com/D7513

resources/sql/patches/20131106.diffphid.1.col.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_differential.differential_diff
+  ADD phid VARCHAR(64) NOT NULL COLLATE utf8_bin AFTER id;

resources/sql/patches/20131106.diffphid.2.mig.php

@@ -0,0 +1,47 @@
+<?php
+
+$diff_table = new DifferentialDiff();
+$conn_w = $diff_table->establishConnection('w');
+
+$size = 1000;
+
+$row_iter = id(new LiskMigrationIterator($diff_table))->setPageSize($size);
+$chunk_iter = new PhutilChunkedIterator($row_iter, $size);
+
+foreach ($chunk_iter as $chunk) {
+  $sql = array();
+
+  foreach ($chunk as $diff) {
+    $id = $diff->getID();
+    echo "Migrating diff ID {$id}...\n";
+
+    $phid = $diff->getPHID();
+    if (strlen($phid)) {
+      continue;
+    }
+
+    $type_diff = DifferentialPHIDTypeDiff::TYPECONST;
+    $new_phid = PhabricatorPHID::generateNewPHID($type_diff);
+
+    $sql[] = qsprintf(
+      $conn_w,
+      '(%d, %s)',
+      $id,
+      $new_phid);
+  }
+
+  if (!$sql) {
+    continue;
+  }
+
+  foreach (PhabricatorLiskDAO::chunkSQL($sql, ', ') as $sql_chunk) {
+    queryfx(
+      $conn_w,
+      'INSERT IGNORE INTO %T (id, phid) VALUES %Q
+        ON DUPLICATE KEY UPDATE phid = VALUES(phid)',
+      $diff_table->getTableName(),
+      $sql_chunk);
+  }
+}
+
+echo "Done.\n";

resources/sql/patches/20131106.diffphid.3.key.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_differential.differential_diff
+  ADD UNIQUE KEY `key_phid` (phid);

src/__phutil_library_map__.php

@@ -396,6 +396,7 @@ phutil_register_library_map(array(
     'DifferentialMailPhase' => 'applications/differential/constants/DifferentialMailPhase.php',
     'DifferentialManiphestTasksFieldSpecification' => 'applications/differential/field/specification/DifferentialManiphestTasksFieldSpecification.php',
     'DifferentialNewDiffMail' => 'applications/differential/mail/DifferentialNewDiffMail.php',
+    'DifferentialPHIDTypeDiff' => 'applications/differential/phid/DifferentialPHIDTypeDiff.php',
     'DifferentialPHIDTypeRevision' => 'applications/differential/phid/DifferentialPHIDTypeRevision.php',
     'DifferentialParseRenderTestCase' => 'applications/differential/__tests__/DifferentialParseRenderTestCase.php',
     'DifferentialPathFieldSpecification' => 'applications/differential/field/specification/DifferentialPathFieldSpecification.php',
@@ -2606,6 +2607,7 @@ phutil_register_library_map(array(
     'DifferentialMail' => 'PhabricatorMail',
     'DifferentialManiphestTasksFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialNewDiffMail' => 'DifferentialReviewRequestMail',
+    'DifferentialPHIDTypeDiff' => 'PhabricatorPHIDType',
     'DifferentialPHIDTypeRevision' => 'PhabricatorPHIDType',
     'DifferentialParseRenderTestCase' => 'PhabricatorTestCase',
     'DifferentialPathFieldSpecification' => 'DifferentialFieldSpecification',

src/applications/differential/phid/DifferentialPHIDTypeDiff.php

@@ -0,0 +1,45 @@
+<?php
+
+final class DifferentialPHIDTypeDiff extends PhabricatorPHIDType {
+
+  const TYPECONST = 'DIFF';
+
+  public function getTypeConstant() {
+    return self::TYPECONST;
+  }
+
+  public function getTypeName() {
+    return pht('Differential Diff');
+  }
+
+  public function newObject() {
+    return new DifferentialDiff();
+  }
+
+  protected function buildQueryForObjects(
+    PhabricatorObjectQuery $query,
+    array $phids) {
+
+    return id(new DifferentialDiffQuery())
+      ->withPHIDs($phids);
+  }
+
+  public function loadHandles(
+    PhabricatorHandleQuery $query,
+    array $handles,
+    array $objects) {
+
+    foreach ($handles as $phid => $handle) {
+      $diff = $objects[$phid];
+
+      $id = $diff->getID();
+
+      $handle->setName(pht('Diff %d', $id));
+    }
+  }
+
+  public function canLoadNamedObject($name) {
+    return false;
+  }
+
+}

src/applications/differential/query/DifferentialDiffQuery.php

@@ -4,6 +4,7 @@ final class DifferentialDiffQuery
   extends PhabricatorCursorPagedPolicyAwareQuery {
 
   private $ids;
+  private $phids;
   private $revisionIDs;
   private $needChangesets = false;
   private $needArcanistProjects = false;
@@ -13,6 +14,11 @@ final class DifferentialDiffQuery
     return $this;
   }
 
+  public function withPHIDs(array $phids) {
+    $this->phids = $phids;
+    return $this;
+  }
+
   public function withRevisionIDs(array $revision_ids) {
     $this->revisionIDs = $revision_ids;
     return $this;
@@ -126,6 +132,13 @@ final class DifferentialDiffQuery
         $this->ids);
     }
 
+    if ($this->phids) {
+      $where[] = qsprintf(
+        $conn_r,
+        'phid IN (%Ls)',
+        $this->phids);
+    }
+
     if ($this->revisionIDs) {
       $where[] = qsprintf(
         $conn_r,

src/applications/differential/storage/DifferentialDiff.php

@@ -34,6 +34,17 @@ final class DifferentialDiff
   private $arcanistProject = self::ATTACHABLE;
   private $revision = self::ATTACHABLE;
 
+  public function getConfiguration() {
+    return array(
+      self::CONFIG_AUX_PHID => true,
+    ) + parent::getConfiguration();
+  }
+
+  public function generatePHID() {
+    return PhabricatorPHID::generateNewPHID(
+      DifferentialPHIDTypeDiff::TYPECONST);
+  }
+
   public function addUnsavedChangeset(DifferentialChangeset $changeset) {
     if ($this->changesets === null) {
       $this->changesets = array();

src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php

@@ -1724,6 +1724,18 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList {
         'type' => 'sql',
         'name' => $this->getPatchPath('20131105.buildstep.sql'),
       ),
+      '20131106.diffphid.1.col.sql' => array(
+        'type' => 'sql',
+        'name' => $this->getPatchPath('20131106.diffphid.1.col.sql'),
+      ),
+      '20131106.diffphid.2.mig.php' => array(
+        'type' => 'php',
+        'name' => $this->getPatchPath('20131106.diffphid.2.mig.php'),
+      ),
+      '20131106.diffphid.3.key.sql' => array(
+        'type' => 'sql',
+        'name' => $this->getPatchPath('20131106.diffphid.3.key.sql'),
+      ),
     );
   }
 }