Move more discovery responsibilities into DiscoveryEngine

Summary:
Ref T4327. This moves the last pieces of discovery responsibility out of the PullLocal daemon and into the DiscoveryEngine.

(This makes it easier to discover repositories in unit tests in the future, since we don't need to build a PullLocal daemon and can just build a DiscoveryEngine.)

Test Plan: Ran `phd debug pulllocal`. Ran `repostory discover`.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4327

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

src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php

@@ -29,14 +29,8 @@
 final class PhabricatorRepositoryPullLocalDaemon
   extends PhabricatorDaemon {
 
-  private $repair;
   private $discoveryEngines = array();
 
-  public function setRepair($repair) {
-    $this->repair = $repair;
-    return $this;
-  }
-
 
 /* -(  Pulling Repositories  )----------------------------------------------- */
 
@@ -225,14 +219,6 @@ final class PhabricatorRepositoryPullLocalDaemon
     $refs = $this->getDiscoveryEngine($repository)
       ->discoverCommits();
 
-    foreach ($refs as $ref) {
-      $this->recordCommit(
-        $repository,
-        $ref->getIdentifier(),
-        $ref->getEpoch(),
-        $ref->getCanCloseImmediately());
-    }
-
     $this->checkIfRepositoryIsFullyImported($repository);
 
     try {
@@ -262,102 +248,14 @@ final class PhabricatorRepositoryPullLocalDaemon
     $id = $repository->getID();
     if (empty($this->discoveryEngines[$id])) {
       $engine = id(new PhabricatorRepositoryDiscoveryEngine())
-          ->setRepository($repository)
+        ->setRepository($repository)
-          ->setVerbose($this->getVerbose())
+        ->setVerbose($this->getVerbose());
-          ->setRepairMode($this->repair);
 
       $this->discoveryEngines[$id] = $engine;
     }
     return $this->discoveryEngines[$id];
   }
 
-  private function recordCommit(
-    PhabricatorRepository $repository,
-    $commit_identifier,
-    $epoch,
-    $close_immediately) {
-
-    $commit = new PhabricatorRepositoryCommit();
-    $commit->setRepositoryID($repository->getID());
-    $commit->setCommitIdentifier($commit_identifier);
-    $commit->setEpoch($epoch);
-    if ($close_immediately) {
-      $commit->setImportStatus(PhabricatorRepositoryCommit::IMPORTED_CLOSEABLE);
-    }
-
-    $data = new PhabricatorRepositoryCommitData();
-
-    try {
-      $commit->openTransaction();
-        $commit->save();
-        $data->setCommitID($commit->getID());
-        $data->save();
-      $commit->saveTransaction();
-
-      $this->insertTask($repository, $commit);
-
-      queryfx(
-        $repository->establishConnection('w'),
-        'INSERT INTO %T (repositoryID, size, lastCommitID, epoch)
-          VALUES (%d, 1, %d, %d)
-          ON DUPLICATE KEY UPDATE
-            size = size + 1,
-            lastCommitID =
-              IF(VALUES(epoch) > epoch, VALUES(lastCommitID), lastCommitID),
-            epoch = IF(VALUES(epoch) > epoch, VALUES(epoch), epoch)',
-        PhabricatorRepository::TABLE_SUMMARY,
-        $repository->getID(),
-        $commit->getID(),
-        $epoch);
-
-      if ($this->repair) {
-        // Normally, the query should throw a duplicate key exception. If we
-        // reach this in repair mode, we've actually performed a repair.
-        $this->log("Repaired commit '{$commit_identifier}'.");
-      }
-
-      PhutilEventEngine::dispatchEvent(
-        new PhabricatorEvent(
-          PhabricatorEventType::TYPE_DIFFUSION_DIDDISCOVERCOMMIT,
-          array(
-            'repository'  => $repository,
-            'commit'      => $commit,
-          )));
-
-    } catch (AphrontQueryDuplicateKeyException $ex) {
-      $commit->killTransaction();
-      // Ignore. This can happen because we discover the same new commit
-      // more than once when looking at history, or because of races or
-      // data inconsistency or cosmic radiation; in any case, we're still
-      // in a good state if we ignore the failure.
-    }
-  }
-
-  private function insertTask(
-    PhabricatorRepository $repository,
-    PhabricatorRepositoryCommit $commit,
-    $data = array()) {
-
-    $vcs = $repository->getVersionControlSystem();
-    switch ($vcs) {
-      case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
-        $class = 'PhabricatorRepositoryGitCommitMessageParserWorker';
-        break;
-      case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
-        $class = 'PhabricatorRepositorySvnCommitMessageParserWorker';
-        break;
-      case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
-        $class = 'PhabricatorRepositoryMercurialCommitMessageParserWorker';
-        break;
-      default:
-        throw new Exception("Unknown repository type '{$vcs}'!");
-    }
-
-    $data['commitID'] = $commit->getID();
-
-    PhabricatorWorker::scheduleTask($class, $data);
-  }
-
   private function checkIfRepositoryIsFullyImported(
     PhabricatorRepository $repository) {
 

src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php

@@ -50,8 +50,14 @@ final class PhabricatorRepositoryDiscoveryEngine
         throw new Exception("Unknown VCS '{$vcs}'!");
     }
 
-    // Mark discovered commits in the cache.
+    // Record discovered commits and mark them in the cache.
     foreach ($refs as $ref) {
+      $this->recordCommit(
+        $repository,
+        $ref->getIdentifier(),
+        $ref->getEpoch(),
+        $ref->getCanCloseImmediately());
+
       $this->commitCache[$ref->getIdentifier()] = true;
     }
 
@@ -453,4 +459,92 @@ final class PhabricatorRepositoryDiscoveryEngine
     return array_merge($head_branches, $tail_branches);
   }
 
+
+  private function recordCommit(
+    PhabricatorRepository $repository,
+    $commit_identifier,
+    $epoch,
+    $close_immediately) {
+
+    $commit = new PhabricatorRepositoryCommit();
+    $commit->setRepositoryID($repository->getID());
+    $commit->setCommitIdentifier($commit_identifier);
+    $commit->setEpoch($epoch);
+    if ($close_immediately) {
+      $commit->setImportStatus(PhabricatorRepositoryCommit::IMPORTED_CLOSEABLE);
+    }
+
+    $data = new PhabricatorRepositoryCommitData();
+
+    try {
+      $commit->openTransaction();
+        $commit->save();
+        $data->setCommitID($commit->getID());
+        $data->save();
+      $commit->saveTransaction();
+
+      $this->insertTask($repository, $commit);
+
+      queryfx(
+        $repository->establishConnection('w'),
+        'INSERT INTO %T (repositoryID, size, lastCommitID, epoch)
+          VALUES (%d, 1, %d, %d)
+          ON DUPLICATE KEY UPDATE
+            size = size + 1,
+            lastCommitID =
+              IF(VALUES(epoch) > epoch, VALUES(lastCommitID), lastCommitID),
+            epoch = IF(VALUES(epoch) > epoch, VALUES(epoch), epoch)',
+        PhabricatorRepository::TABLE_SUMMARY,
+        $repository->getID(),
+        $commit->getID(),
+        $epoch);
+
+      if ($this->repairMode) {
+        // Normally, the query should throw a duplicate key exception. If we
+        // reach this in repair mode, we've actually performed a repair.
+        $this->log(pht('Repaired commit "%s".', $commit_identifier));
+      }
+
+      PhutilEventEngine::dispatchEvent(
+        new PhabricatorEvent(
+          PhabricatorEventType::TYPE_DIFFUSION_DIDDISCOVERCOMMIT,
+          array(
+            'repository'  => $repository,
+            'commit'      => $commit,
+          )));
+
+    } catch (AphrontQueryDuplicateKeyException $ex) {
+      $commit->killTransaction();
+      // Ignore. This can happen because we discover the same new commit
+      // more than once when looking at history, or because of races or
+      // data inconsistency or cosmic radiation; in any case, we're still
+      // in a good state if we ignore the failure.
+    }
+  }
+
+  private function insertTask(
+    PhabricatorRepository $repository,
+    PhabricatorRepositoryCommit $commit,
+    $data = array()) {
+
+    $vcs = $repository->getVersionControlSystem();
+    switch ($vcs) {
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
+        $class = 'PhabricatorRepositoryGitCommitMessageParserWorker';
+        break;
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
+        $class = 'PhabricatorRepositorySvnCommitMessageParserWorker';
+        break;
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
+        $class = 'PhabricatorRepositoryMercurialCommitMessageParserWorker';
+        break;
+      default:
+        throw new Exception("Unknown repository type '{$vcs}'!");
+    }
+
+    $data['commitID'] = $commit->getID();
+
+    PhabricatorWorker::scheduleTask($class, $data);
+  }
+
 }

src/applications/repository/management/PhabricatorRepositoryManagementDiscoverWorkflow.php

@@ -38,10 +38,11 @@ final class PhabricatorRepositoryManagementDiscoverWorkflow
     foreach ($repos as $repo) {
       $console->writeOut("Discovering '%s'...\n", $repo->getCallsign());
 
-      $daemon = new PhabricatorRepositoryPullLocalDaemon(array());
+      id(new PhabricatorRepositoryDiscoveryEngine())
-      $daemon->setVerbose($args->getArg('verbose'));
+        ->setRepository($repo)
-      $daemon->setRepair($args->getArg('repair'));
+        ->setVerbose($args->getArg('verbose'))
-      $daemon->discoverRepository($repo);
+        ->setRepairMode($args->getArg('repair'))
+        ->discoverCommits();
     }
 
     $console->writeOut("Done.\n");