Clean up various pieces of dead/obsolete Differential code

Summary:
Ref T2222.

  - Removes `DifferentialTasksAttacher`, which has had no callsites for a very long time.
  - Moves `differential.getrevisioncomments` off `DifferentialCommentQuery`.
  - Moves Releeph churn field off `DifferentialCommentQuery`.
  - Removes dead code in `DifferentialRevisionViewController`.
  - Removes `DifferentialException` (no references).
  - Removes `DifferentialRevision->loadComments()` (no callsites).
  - Removes `DifferentialRevision->loadReviewedBy()` (all callsites updated).
  - Removes `DifferentialCommentQuery` (all callsites updated).

Test Plan: Mostly a lot of `grep`.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2222

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

src/__phutil_library_map__.php

@@ -345,7 +345,6 @@ phutil_register_library_map(array(
     'DifferentialChangesetViewController' => 'applications/differential/controller/DifferentialChangesetViewController.php',
     'DifferentialComment' => 'applications/differential/storage/DifferentialComment.php',
     'DifferentialCommentPreviewController' => 'applications/differential/controller/DifferentialCommentPreviewController.php',
-    'DifferentialCommentQuery' => 'applications/differential/query/DifferentialCommentQuery.php',
     'DifferentialCommentSaveController' => 'applications/differential/controller/DifferentialCommentSaveController.php',
     'DifferentialCommitMessageParser' => 'applications/differential/parser/DifferentialCommitMessageParser.php',
     'DifferentialCommitMessageParserTestCase' => 'applications/differential/parser/__tests__/DifferentialCommitMessageParserTestCase.php',
@@ -354,11 +353,11 @@ phutil_register_library_map(array(
     'DifferentialController' => 'applications/differential/controller/DifferentialController.php',
     'DifferentialCoreCustomField' => 'applications/differential/customfield/DifferentialCoreCustomField.php',
     'DifferentialCustomField' => 'applications/differential/customfield/DifferentialCustomField.php',
-    'DifferentialCustomFieldDependsOnParser' => 'applications/differential/field/parser/DifferentialCustomFieldDependsOnParser.php',
+    'DifferentialCustomFieldDependsOnParser' => 'applications/differential/parser/DifferentialCustomFieldDependsOnParser.php',
-    'DifferentialCustomFieldDependsOnParserTestCase' => 'applications/differential/field/parser/__tests__/DifferentialCustomFieldDependsOnParserTestCase.php',
+    'DifferentialCustomFieldDependsOnParserTestCase' => 'applications/differential/parser/__tests__/DifferentialCustomFieldDependsOnParserTestCase.php',
     'DifferentialCustomFieldNumericIndex' => 'applications/differential/storage/DifferentialCustomFieldNumericIndex.php',
-    'DifferentialCustomFieldRevertsParser' => 'applications/differential/field/parser/DifferentialCustomFieldRevertsParser.php',
+    'DifferentialCustomFieldRevertsParser' => 'applications/differential/parser/DifferentialCustomFieldRevertsParser.php',
-    'DifferentialCustomFieldRevertsParserTestCase' => 'applications/differential/field/parser/__tests__/DifferentialCustomFieldRevertsParserTestCase.php',
+    'DifferentialCustomFieldRevertsParserTestCase' => 'applications/differential/parser/__tests__/DifferentialCustomFieldRevertsParserTestCase.php',
     'DifferentialCustomFieldStorage' => 'applications/differential/storage/DifferentialCustomFieldStorage.php',
     'DifferentialCustomFieldStringIndex' => 'applications/differential/storage/DifferentialCustomFieldStringIndex.php',
     'DifferentialDAO' => 'applications/differential/storage/DifferentialDAO.php',
@@ -374,10 +373,9 @@ phutil_register_library_map(array(
     'DifferentialDoorkeeperRevisionFeedStoryPublisher' => 'applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php',
     'DifferentialDraft' => 'applications/differential/storage/DifferentialDraft.php',
     'DifferentialEditPolicyField' => 'applications/differential/customfield/DifferentialEditPolicyField.php',
-    'DifferentialException' => 'applications/differential/exception/DifferentialException.php',
     'DifferentialExceptionMail' => 'applications/differential/mail/DifferentialExceptionMail.php',
-    'DifferentialFieldParseException' => 'applications/differential/field/exception/DifferentialFieldParseException.php',
+    'DifferentialFieldParseException' => 'applications/differential/exception/DifferentialFieldParseException.php',
-    'DifferentialFieldValidationException' => 'applications/differential/field/exception/DifferentialFieldValidationException.php',
+    'DifferentialFieldValidationException' => 'applications/differential/exception/DifferentialFieldValidationException.php',
     'DifferentialGetWorkingCopy' => 'applications/differential/DifferentialGetWorkingCopy.php',
     'DifferentialGitSVNIDField' => 'applications/differential/customfield/DifferentialGitSVNIDField.php',
     'DifferentialHostField' => 'applications/differential/customfield/DifferentialHostField.php',
@@ -442,7 +440,6 @@ phutil_register_library_map(array(
     'DifferentialStoredCustomField' => 'applications/differential/customfield/DifferentialStoredCustomField.php',
     'DifferentialSubscribersField' => 'applications/differential/customfield/DifferentialSubscribersField.php',
     'DifferentialSummaryField' => 'applications/differential/customfield/DifferentialSummaryField.php',
-    'DifferentialTasksAttacher' => 'applications/differential/DifferentialTasksAttacher.php',
     'DifferentialTestPlanField' => 'applications/differential/customfield/DifferentialTestPlanField.php',
     'DifferentialTitleField' => 'applications/differential/customfield/DifferentialTitleField.php',
     'DifferentialTransaction' => 'applications/differential/storage/DifferentialTransaction.php',
@@ -2869,7 +2866,6 @@ phutil_register_library_map(array(
     'DifferentialChangesetViewController' => 'DifferentialController',
     'DifferentialComment' => 'PhabricatorMarkupInterface',
     'DifferentialCommentPreviewController' => 'DifferentialController',
-    'DifferentialCommentQuery' => 'PhabricatorOffsetPagedQuery',
     'DifferentialCommentSaveController' => 'DifferentialController',
     'DifferentialCommitMessageParserTestCase' => 'PhabricatorTestCase',
     'DifferentialCommitsField' => 'DifferentialCustomField',
@@ -2902,7 +2898,6 @@ phutil_register_library_map(array(
     'DifferentialDoorkeeperRevisionFeedStoryPublisher' => 'DoorkeeperFeedStoryPublisher',
     'DifferentialDraft' => 'DifferentialDAO',
     'DifferentialEditPolicyField' => 'DifferentialCoreCustomField',
-    'DifferentialException' => 'Exception',
     'DifferentialExceptionMail' => 'DifferentialMail',
     'DifferentialFieldParseException' => 'Exception',
     'DifferentialFieldValidationException' => 'Exception',

src/applications/differential/DifferentialTasksAttacher.php

@@ -1,24 +0,0 @@
-<?php
-
-abstract class DifferentialTasksAttacher {
-  /**
-   * Implementation of this function should attach given tasks to
-   * the given revision. The function is called when 'arc' has task
-   * ids defined in the commit message.
-   */
-  abstract public function attachTasksToRevision(
-    $user_phid,
-    DifferentialRevision $revision,
-    array $task_ids);
-
-  /**
-   * This method will be called with a task and its original and new
-   * associated revisions. Implementation of this method should update
-   * the affected revisions to maintain the new associations.
-   */
-  abstract public function updateTaskRevisionAssoc(
-    $task_phid,
-    array $orig_rev_phids,
-    array $new_rev_phids);
-
-}

src/applications/differential/conduit/ConduitAPI_differential_getrevisioncomments_Method.php

@@ -32,6 +32,7 @@ final class ConduitAPI_differential_getrevisioncomments_Method
   }
 
   protected function execute(ConduitAPIRequest $request) {
+    $viewer = $request->getUser();
     $results = array();
     $revision_ids = $request->getValue('ids');
 
@@ -40,7 +41,7 @@ final class ConduitAPI_differential_getrevisioncomments_Method
     }
 
     $revisions = id(new DifferentialRevisionQuery())
-      ->setViewer($request->getUser())
+      ->setViewer($viewer)
       ->withIDs($revision_ids)
       ->execute();
 
@@ -48,24 +49,36 @@ final class ConduitAPI_differential_getrevisioncomments_Method
       return $results;
     }
 
-    $comments = id(new DifferentialCommentQuery())
+    $xactions = id(new DifferentialTransactionQuery())
-      ->withRevisionPHIDs(mpull($revisions, 'getPHID'))
+      ->setViewer($viewer)
+      ->withObjectPHIDs(mpull($revisions, 'getPHID'))
       ->execute();
 
     $revisions = mpull($revisions, null, 'getPHID');
 
-    foreach ($comments as $comment) {
+    foreach ($xactions as $xaction) {
-      $revision = idx($revisions, $comment->getRevisionPHID());
+      $revision = idx($revisions, $xaction->getObjectPHID());
       if (!$revision) {
         continue;
       }
 
+      $type = $xaction->getTransactionType();
+      if ($type == DifferentialTransaction::TYPE_ACTION) {
+        $action = $xaction->getNewValue();
+      } else if ($type == PhabricatorTransactions::TYPE_COMMENT) {
+        $action = 'comment';
+      } else {
+        $action = 'none';
+      }
+
       $result = array(
         'revisionID'  => $revision->getID(),
-        'action'      => $comment->getAction(),
+        'action'      => $action,
-        'authorPHID'  => $comment->getAuthorPHID(),
+        'authorPHID'  => $xaction->getAuthorPHID(),
-        'dateCreated' => $comment->getDateCreated(),
+        'dateCreated' => $xaction->getDateCreated(),
-        'content'     => $comment->getContent(),
+        'content'     => ($xaction->hasComment()
+          ? $xaction->getComment()->getContent()
+          : null),
       );
 
       $results[$revision->getID()][] = $result;

src/applications/differential/controller/DifferentialRevisionViewController.php

@@ -84,8 +84,6 @@ final class DifferentialRevisionViewController extends DifferentialController {
       $target_manual->getID());
     $props = mpull($props, 'getData', 'getName');
 
-    $comments = $revision->loadComments();
-
     $all_changesets = $changesets;
     $inlines = $this->loadInlineComments(
       $revision,
@@ -98,14 +96,7 @@ final class DifferentialRevisionViewController extends DifferentialController {
       array(
         $revision->getAuthorPHID(),
         $user->getPHID(),
-      ),
+      ));
-      mpull($comments, 'getAuthorPHID'));
-
-    foreach ($comments as $comment) {
-      foreach ($comment->getRequiredHandlePHIDs() as $phid) {
-        $object_phids[] = $phid;
-      }
-    }
 
     foreach ($revision->getAttached() as $type => $phids) {
       foreach ($phids as $phid => $info) {

src/applications/differential/exception/DifferentialException.php

@@ -1,5 +0,0 @@
-<?php
-
-abstract class DifferentialException extends Exception {
-
-}

src/applications/differential/query/DifferentialCommentQuery.php

@@ -1,34 +0,0 @@
-<?php
-
-/**
- * Temporary wrapper for transitioning Differential to ApplicationTransactions.
- */
-final class DifferentialCommentQuery
-  extends PhabricatorOffsetPagedQuery {
-
-  private $revisionPHIDs;
-
-  public function withRevisionPHIDs(array $phids) {
-    $this->revisionPHIDs = $phids;
-    return $this;
-  }
-
-  public function execute() {
-    // TODO: We're getting rid of this, it is the bads.
-    $viewer = PhabricatorUser::getOmnipotentUser();
-
-    $xactions = id(new DifferentialTransactionQuery())
-      ->setViewer($viewer)
-      ->withObjectPHIDs($this->revisionPHIDs)
-      ->needComments(true)
-      ->execute();
-
-    $results = array();
-    foreach ($xactions as $xaction) {
-      $results[] = DifferentialComment::newFromModernTransaction($xaction);
-    }
-
-    return $results;
-  }
-
-}

src/applications/differential/storage/DifferentialRevision.php

@@ -160,15 +160,6 @@ final class DifferentialRevision extends DifferentialDAO
       DifferentialPHIDTypeRevision::TYPECONST);
   }
 
-  public function loadComments() {
-    if (!$this->getID()) {
-      return array();
-    }
-    return id(new DifferentialCommentQuery())
-      ->withRevisionPHIDs(array($this->getPHID()))
-      ->execute();
-  }
-
   public function loadActiveDiff() {
     return id(new DifferentialDiff())->loadOneWhere(
       'revisionID = %d ORDER BY id DESC LIMIT 1',
@@ -200,13 +191,6 @@ final class DifferentialRevision extends DifferentialDAO
         self::TABLE_COMMIT,
         $this->getID());
 
-      $comments = id(new DifferentialCommentQuery())
-        ->withRevisionPHIDs(array($this->getPHID()))
-        ->execute();
-      foreach ($comments as $comment) {
-        $comment->delete();
-      }
-
       $inlines = id(new DifferentialInlineCommentQuery())
         ->withRevisionIDs(array($this->getID()))
         ->execute();
@@ -295,27 +279,6 @@ final class DifferentialRevision extends DifferentialDAO
     return $last;
   }
 
-  public function loadReviewedBy() {
-    $reviewer = null;
-
-    if ($this->status == ArcanistDifferentialRevisionStatus::ACCEPTED ||
-        $this->status == ArcanistDifferentialRevisionStatus::CLOSED) {
-      $comments = $this->loadComments();
-      foreach ($comments as $comment) {
-        $action = $comment->getAction();
-        if ($action == DifferentialAction::ACTION_ACCEPT) {
-          $reviewer = $comment->getAuthorPHID();
-        } else if ($action == DifferentialAction::ACTION_REJECT ||
-                   $action == DifferentialAction::ACTION_ABANDON ||
-                   $action == DifferentialAction::ACTION_RETHINK) {
-          $reviewer = null;
-        }
-      }
-    }
-
-    return $reviewer;
-  }
-
   public function getHashes() {
     return $this->assertAttached($this->hashes);
   }

src/applications/herald/adapter/HeraldCommitAdapter.php

@@ -442,14 +442,14 @@ final class HeraldCommitAdapter extends HeraldAdapter {
         if (!$revision) {
           return null;
         }
-        // after a revision is accepted, it can be closed (say via arc land)
+
-        // so use this function to figure out if it was accepted at one point
+        switch ($revision->getStatus()) {
-        // *and* not later rejected...  what a function!
+          case ArcanistDifferentialRevisionStatus::ACCEPTED:
-        $reviewed_by = $revision->loadReviewedBy();
+          case ArcanistDifferentialRevisionStatus::CLOSED:
-        if (!$reviewed_by) {
-          return null;
-        }
             return $revision->getPHID();
+        }
+
+        return null;
       case self::FIELD_DIFFERENTIAL_REVIEWERS:
         $revision = $this->loadDifferentialRevision();
         if (!$revision) {

src/applications/releeph/field/specification/ReleephDiffChurnFieldSpecification.php

@@ -23,23 +23,32 @@ final class ReleephDiffChurnFieldSpecification
     }
 
     $diff_rev = $this->getReleephRequest()->loadDifferentialRevision();
-    $comments = id(new DifferentialCommentQuery())
+
-      ->withRevisionPHIDs(array($diff_rev->getPHID()))
+    $xactions = id(new DifferentialTransactionQuery())
+      ->setViewer(PhabricatorUser::getOmnipotentUser())
+      ->withObjectPHIDs(array($diff_rev->getPHID()))
       ->execute();
 
-    $counts = array();
+    $rejections = 0;
-    foreach ($comments as $comment) {
+    $comments = 0;
-      $action = $comment->getAction();
+    $updates = 0;
-      if (!isset($counts[$action])) {
+    foreach ($xactions as $xaction) {
-        $counts[$action] = 0;
+      switch ($xaction->getTransactionType()) {
+        case PhabricatorTransactions::TYPE_COMMENT:
+          $comments++;
+          break;
+        case DifferentialTransaction::TYPE_UPDATE:
+          $updates++;
+          break;
+        case DifferentialTransaction::TYPE_ACTION:
+          switch ($xaction->getNewValue()) {
+            case DifferentialAction::ACTION_REJECT:
+              $rejections++;
+              break;
+          }
+          break;
       }
-      $counts[$action] += 1;
     }
-
-    // 'none' action just means a plain comment
-    $comments   = idx($counts, 'none',     0);
-    $rejections = idx($counts, 'reject',   0);
-    $updates    = idx($counts, 'update',   0);
 
     $points =
       self::REJECTIONS_WEIGHT * $rejections +

src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php

@@ -100,15 +100,10 @@ final class PhabricatorRepositoryCommitOwnersWorker
       $revision = id(new DifferentialRevision())->load($revision_id);
       if ($revision) {
         $revision_author_phid = $revision->getAuthorPHID();
-        $revision_reviewedby_phid = $revision->loadReviewedBy();
         $commit_reviewedby_phid = $data->getCommitDetail('reviewerPHID');
         if ($revision_author_phid !== $commit_author_phid) {
           $reasons[] = "Author Not Matching with Revision";
         }
-        if ($revision_reviewedby_phid !== $commit_reviewedby_phid) {
-          $reasons[] = "ReviewedBy Not Matching with Revision";
-        }
-
       } else {
         $reasons[] = "Revision Not Found";
       }

src/infrastructure/storage/lisk/PhabricatorLiskDAO.php

@@ -186,4 +186,14 @@ abstract class PhabricatorLiskDAO extends LiskDAO {
     return phutil_utf8ize($string);
   }
 
+  public function delete() {
+
+    // TODO: We should make some reasonable effort to destroy related
+    // infrastructure objects here, like edges, transactions, custom field
+    // storage, flags, Phrequent tracking, tokens, etc. This doesn't need to
+    // be exhaustive, but we can get a lot of it pretty easily.
+
+    return parent::delete();
+  }
+
 }