When creating or updating a revision, infer the repository from the diff

Summary:
Ref T603. When a diff is attached to a revision, try to guess the repository if possible. In cases where we succeed, this automatically gives us intuitive policy behavior (i.e., you can see a revision if you can see the repository the change is against).

I pulled this into a funky little "Lookup" class for two reasons:

  - It's used in two places;
  - I anticipate that we might need to add some sort of `explainWhy()` method if users find the heuristics confusing.

Test Plan: Created and updated revisions, saw them pick up the correct repository association. Ran Herald dry run against associable and nonassociable revisions, saw correct values populate.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

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

src/__phutil_library_map__.php

@@ -395,6 +395,7 @@ phutil_register_library_map(array(
     'DifferentialRemarkupRule' => 'applications/differential/remarkup/DifferentialRemarkupRule.php',
     'DifferentialReplyHandler' => 'applications/differential/mail/DifferentialReplyHandler.php',
     'DifferentialRepositoryFieldSpecification' => 'applications/differential/field/specification/DifferentialRepositoryFieldSpecification.php',
+    'DifferentialRepositoryLookup' => 'applications/differential/query/DifferentialRepositoryLookup.php',
     'DifferentialResultsTableView' => 'applications/differential/view/DifferentialResultsTableView.php',
     'DifferentialRevertPlanFieldSpecification' => 'applications/differential/field/specification/DifferentialRevertPlanFieldSpecification.php',
     'DifferentialReviewRequestMail' => 'applications/differential/mail/DifferentialReviewRequestMail.php',
@@ -2470,6 +2471,7 @@ phutil_register_library_map(array(
     'DifferentialRemarkupRule' => 'PhabricatorRemarkupRuleObject',
     'DifferentialReplyHandler' => 'PhabricatorMailReplyHandler',
     'DifferentialRepositoryFieldSpecification' => 'DifferentialFieldSpecification',
+    'DifferentialRepositoryLookup' => 'Phobject',
     'DifferentialResultsTableView' => 'AphrontView',
     'DifferentialRevertPlanFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialReviewRequestMail' => 'DifferentialMail',

src/applications/differential/editor/DifferentialRevisionEditor.php

@@ -126,6 +126,16 @@ final class DifferentialRevisionEditor extends PhabricatorEditor {
     }
     $this->diff = $diff;
     $this->comments = $comments;
+
+    $repository = id(new DifferentialRepositoryLookup())
+      ->setViewer($this->getActor())
+      ->setDiff($diff)
+      ->lookupRepository();
+
+    if ($repository) {
+      $this->getRevision()->setRepositoryPHID($repository->getPHID());
+    }
+
     return $this;
   }
 

src/applications/differential/query/DifferentialRepositoryLookup.php

@@ -0,0 +1,83 @@
+<?php
+
+/**
+ * Guess which tracked repository a diff comes from.
+ */
+final class DifferentialRepositoryLookup extends Phobject {
+
+  private $viewer;
+  private $diff;
+
+  public function setDiff(DifferentialDiff $diff) {
+    $this->diff = $diff;
+    return $this;
+  }
+
+  public function setViewer(PhabricatorUser $viewer) {
+    $this->viewer = $viewer;
+    return $this;
+  }
+
+  public function lookupRepository() {
+    $viewer = $this->viewer;
+    $diff = $this->diff;
+
+    // Look for an explicit Arcanist project.
+    if ($diff->getArcanistProjectPHID()) {
+      $project = id(new PhabricatorRepositoryArcanistProject())->loadOneWhere(
+        'phid = %s',
+        $diff->getArcanistProjectPHID());
+      if ($project && $project->getRepositoryID()) {
+        $repositories = id(new PhabricatorRepositoryQuery())
+          ->setViewer($viewer)
+          ->withIDs(array($project->getRepositoryID()))
+          ->execute();
+        if ($repositories) {
+          return head($repositories);
+        }
+      }
+    }
+
+    // Look for a repository UUID.
+    if ($diff->getRepositoryUUID()) {
+      $repositories = id(new PhabricatorRepositoryQuery())
+        ->setViewer($viewer)
+        ->withUUIDs($diff->getRepositoryUUID())
+        ->execute();
+      if ($repositories) {
+        return head($repositories);
+      }
+    }
+
+    // Look for the base commit in Git and Mercurial.
+    $vcs = $diff->getSourceControlSystem();
+    $vcs_git = PhabricatorRepositoryType::REPOSITORY_TYPE_GIT;
+    $vcs_hg = PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL;
+    if ($vcs == $vcs_git || $vcs == $vcs_hg) {
+      $base = $diff->getSourceControlBaseRevision();
+      if ($base) {
+        $commits = id(new DiffusionCommitQuery())
+          ->setViewer($viewer)
+          ->withIdentifiers(array($base))
+          ->execute();
+        $commits = mgroup($commits, 'getRepositoryID');
+        if (count($commits) == 1) {
+          $repository_id = key($commits);
+          $repositories = id(new PhabricatorRepositoryQuery())
+            ->setViewer($viewer)
+            ->withIDs(array($repository_id))
+            ->execute();
+          if ($repositories) {
+            return head($repositories);
+          }
+        }
+      }
+    }
+
+    // TODO: Compare SVN remote URIs? Compare Git/Hg remote URIs? Add
+    // an explicit option to `.arcconfig`?
+
+    return null;
+  }
+
+}

src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php

@@ -106,29 +106,33 @@ final class HeraldDifferentialRevisionAdapter extends HeraldAdapter {
 
   public function loadRepository() {
     if ($this->repository === null) {
-      $diff = $this->diff;
-
-      $repository = false;
+      $this->repository = false;
 
       // TODO: (T603) Implement policy stuff in Herald.
+      $viewer = PhabricatorUser::getOmnipotentUser();
 
-      if ($diff->getRepositoryUUID()) {
-        $repository = id(new PhabricatorRepository())->loadOneWhere(
-          'uuid = %s',
-          $diff->getRepositoryUUID());
-      }
-
-      if (!$repository && $diff->getArcanistProjectPHID()) {
-        $project = id(new PhabricatorRepositoryArcanistProject())->loadOneWhere(
-          'phid = %s',
-          $diff->getArcanistProjectPHID());
-        if ($project && $project->getRepositoryID()) {
-          $repository = id(new PhabricatorRepository())->load(
-            $project->getRepositoryID());
+      $revision = $this->revision;
+      if ($revision->getRepositoryPHID()) {
+        $repositories = id(new PhabricatorRepositoryQuery())
+          ->setViewer($viewer)
+          ->withPHIDs(array($revision->getRepositoryPHID()))
+          ->execute();
+        if ($repositories) {
+          $this->repository = head($repositories);
+          return $this->repository;
         }
       }
 
+      $repository = id(new DifferentialRepositoryLookup())
+        ->setViewer($viewer)
+        ->setDiff($this->diff)
+        ->lookupRepository();
+      if ($repository) {
         $this->repository = $repository;
+        return $this->repository;
+      }
+
+      $repository = false;
     }
     return $this->repository;
   }

src/applications/repository/query/PhabricatorRepositoryQuery.php

@@ -7,6 +7,7 @@ final class PhabricatorRepositoryQuery
   private $phids;
   private $callsigns;
   private $types;
+  private $uuids;
 
   const STATUS_OPEN = 'status-open';
   const STATUS_CLOSED = 'status-closed';
@@ -47,6 +48,11 @@ final class PhabricatorRepositoryQuery
     return $this;
   }
 
+  public function withUUIDs(array $uuids) {
+    $this->uuids = $uuids;
+    return $this;
+  }
+
   public function needCommitCounts($need_counts) {
     $this->needCommitCounts = $need_counts;
     return $this;
@@ -299,6 +305,13 @@ final class PhabricatorRepositoryQuery
         $this->types);
     }
 
+    if ($this->uuids) {
+      $where[] = qsprintf(
+        $conn_r,
+        'r.uuid IN (%Ls)',
+        $this->uuids);
+    }
+
     $where[] = $this->buildPagingClause($conn_r);
 
     return $this->formatWhereClause($where);