Remove commit from DiffusionRequest

Summary: Ref T2683. This field is //almost// entirely redundant with `symbolicCommit`. Improve how some of the diff query stuff works a bit, then remove it.

Test Plan: Browsed around in all interfaces, looked at a bunch of diffs, etc.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2683

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

src/applications/diffusion/conduit/ConduitAPI_diffusion_diffquery_Method.php

@@ -168,7 +168,7 @@ final class ConduitAPI_diffusion_diffquery_Method
         $drequest,
         'diffusion.lastmodifiedquery',
         array(
-          'paths' => array($path => $drequest->getCommit()),
+          'paths' => array($path => $drequest->getStableCommit()),
         ));
 
       $this->effectiveCommit = idx($result, $path);
@@ -199,10 +199,10 @@ final class ConduitAPI_diffusion_diffquery_Method
     if (!$effective_commit) {
       return $this->getEmptyResult(1);
     }
-    // TODO: This side effect is kind of sketchy.
-    $drequest->setCommit($effective_commit);
 
-    $raw_query = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest);
+    $raw_query = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest)
+      ->setAnchorCommit($effective_commit);
+
     $raw_diff = $raw_query->loadRawDiff();
     if (!$raw_diff) {
       return $this->getEmptyResult(2);

src/applications/diffusion/controller/DiffusionChangeController.php

@@ -18,7 +18,9 @@ final class DiffusionChangeController extends DiffusionController {
         'commit' => $drequest->getCommit(),
         'path' => $drequest->getPath(),
       ));
-    $drequest->setCommit($data['effectiveCommit']);
+
+    $drequest->updateSymbolicCommit($data['effectiveCommit']);
+
     $raw_changes = ArcanistDiffChange::newFromConduit($data['changes']);
     $diff = DifferentialDiff::newFromRawChanges($raw_changes);
     $changesets = $diff->getChangesets();
@@ -31,7 +33,6 @@ final class DiffusionChangeController extends DiffusionController {
 
     $repository = $drequest->getRepository();
     $callsign = $repository->getCallsign();
-    $commit = $drequest->getSymbolicCommit();
     $changesets = array(
       0 => $changeset,
     );

src/applications/diffusion/controller/DiffusionDiffController.php

@@ -51,7 +51,7 @@ final class DiffusionDiffController extends DiffusionController {
       array(
         'commit' => $drequest->getCommit(),
         'path' => $drequest->getPath()));
-    $drequest->setCommit($data['effectiveCommit']);
+    $drequest->updateSymbolicCommit($data['effectiveCommit']);
     $raw_changes = ArcanistDiffChange::newFromConduit($data['changes']);
     $diff = DifferentialDiff::newFromRawChanges($raw_changes);
     $changesets = $diff->getChangesets();

src/applications/diffusion/controller/DiffusionHistoryController.php

@@ -152,7 +152,7 @@ final class DiffusionHistoryController extends DiffusionController {
       ->setUser($viewer)
       ->setActionList($actions);
 
-    $stable_commit = $drequest->getStableCommitName();
+    $stable_commit = $drequest->getStableCommit();
     $callsign = $drequest->getRepository()->getCallsign();
 
     $view->addProperty(

src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php

@@ -6,7 +6,7 @@ final class DiffusionGitRawDiffQuery extends DiffusionRawDiffQuery {
     $drequest = $this->getRequest();
     $repository = $drequest->getRepository();
 
-    $commit = $drequest->getCommit();
+    $commit = $this->getAnchorCommit();
 
     $options = array(
       '-M',

src/applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php

@@ -10,7 +10,7 @@ final class DiffusionMercurialRawDiffQuery extends DiffusionRawDiffQuery {
     $drequest = $this->getRequest();
     $repository = $drequest->getRepository();
 
-    $commit = $drequest->getCommit();
+    $commit = $this->getAnchorCommit();
 
     // If there's no path, get the entire raw diff.
     $path = nonempty($drequest->getPath(), '.');

src/applications/diffusion/query/rawdiff/DiffusionRawDiffQuery.php

@@ -2,9 +2,9 @@
 
 abstract class DiffusionRawDiffQuery extends DiffusionQuery {
 
-  private $request;
   private $timeout;
   private $linesOfContext = 65535;
+  private $anchorCommit;
   private $againstCommit;
   private $byteLimit;
 
@@ -53,6 +53,19 @@ abstract class DiffusionRawDiffQuery extends DiffusionQuery {
     return $this->againstCommit;
   }
 
+  public function setAnchorCommit($anchor_commit) {
+    $this->anchorCommit = $anchor_commit;
+    return $this;
+  }
+
+  public function getAnchorCommit() {
+    if ($this->anchorCommit) {
+      return $this->anchorCommit;
+    }
+
+    return $this->getRequest()->getStableCommit();
+  }
+
   protected function configureFuture(ExecFuture $future) {
     if ($this->getTimeout()) {
       $future->setTimeout($this->getTimeout());

src/applications/diffusion/query/rawdiff/DiffusionSvnRawDiffQuery.php

@@ -6,7 +6,7 @@ final class DiffusionSvnRawDiffQuery extends DiffusionRawDiffQuery {
     $drequest = $this->getRequest();
     $repository = $drequest->getRepository();
 
-    $commit = $drequest->getCommit();
+    $commit = $this->getAnchorCommit();
     $arc_root = phutil_get_library_root('arcanist');
 
     $against = $this->getAgainstCommit();

src/applications/diffusion/request/DiffusionRequest.php

@@ -12,7 +12,6 @@ abstract class DiffusionRequest {
   protected $callsign;
   protected $path;
   protected $line;
-  protected $commit;
   protected $branch;
   protected $lint;
 
@@ -241,11 +240,6 @@ abstract class DiffusionRequest {
 
     // TODO: Probably remove all of this.
 
-    // Required for sketchy sins that `diffusion.diffquery` commits.
-    if ($this->commit) {
-      return $this->commit;
-    }
-
     if ($this->getSymbolicCommit() !== null) {
       return $this->getSymbolicCommit();
     }
@@ -272,6 +266,20 @@ abstract class DiffusionRequest {
   }
 
 
+  /**
+   * Modify the request to move the symbolic commit elsewhere.
+   *
+   * @param string New symbolic commit.
+   * @return this
+   */
+  public function updateSymbolicCommit($symbol) {
+    $this->symbolicCommit = $symbol;
+    $this->symbolicType = null;
+    $this->stableCommit = null;
+    return $this;
+  }
+
+
   /**
    * Get the ref type (`commit` or `tag`) of the location associated with this
    * request.
@@ -380,11 +388,6 @@ abstract class DiffusionRequest {
     return $this->repositoryCommitData;
   }
 
-  public function setCommit($commit) {
-    $this->commit = $commit;
-    return $this;
-  }
-
 /* -(  Managing Diffusion URIs  )-------------------------------------------- */