From 71a97d8af56b0926e91b35e45bd1b5d11e1e0376 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 10 May 2016 06:40:30 -0700
Subject: [PATCH] When observing a repository, switch to "importing" mode on a
 large discovery in an empty repository

Summary:
Ref T10923. Fixes T9554.

When hosting a repository, we currently have a heuristic that tries to detect when you're doing an initial import: if you push more than 7 commits to an empty repository, it counts as an import and we disable mail/feed/etc.

Do something similar for observed repositories: if the repository is empty and we discover more than 7 commits, switch to import mode until we catch up.

This should align behavior with user expectation more often when juggling hosted vs imported repositories.

Test Plan:
  - Created a new hosted repository.
  - Activated it and allowed it to fully import.
  - Added an "Observe URI".
  - Saw it automatically drop into "Importing" mode until the import completed.
  - Swapped it back to hosted mode.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9554, T10923

Differential Revision: https://secure.phabricator.com/D15877
---
 .../engine/DiffusionCommitHookEngine.php      | 11 ++----
 .../PhabricatorRepositoryDiscoveryEngine.php  | 36 +++++++++++++++++++
 .../PhabricatorWorkingCopyTestCase.php        |  1 -
 .../storage/PhabricatorRepository.php         | 19 +++++++++-
 4 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
index 54fa32a6a3..50fc828364 100644
--- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
+++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
@@ -172,14 +172,7 @@ final class DiffusionCommitHookEngine extends Phobject {
 
     if ($this->isInitialImport($all_updates)) {
       $repository = $this->getRepository();
-
-      $repository->openTransaction();
-        $repository->beginReadLocking();
-          $repository = $repository->reload();
-          $repository->setDetail('importing', true);
-          $repository->save();
-        $repository->endReadLocking();
-      $repository->saveTransaction();
+      $repository->markImporting();
     }
 
     if ($this->emailPHIDs) {
@@ -1244,7 +1237,7 @@ final class DiffusionCommitHookEngine extends Phobject {
       $commit_count++;
     }
 
-    if ($commit_count <= 7) {
+    if ($commit_count <= PhabricatorRepository::IMPORT_THRESHOLD) {
       // If this pushes a very small number of commits, assume it's an
       // initial commit or stack of a few initial commits.
       return false;
diff --git a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
index 3c3a0526be..82bd7706aa 100644
--- a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
@@ -52,6 +52,16 @@ final class PhabricatorRepositoryDiscoveryEngine
         throw new Exception(pht("Unknown VCS '%s'!", $vcs));
     }
 
+    if ($this->isInitialImport($refs)) {
+      $this->log(
+        pht(
+          'Discovered more than %s commit(s) in an empty repository, '.
+          'marking repository as importing.',
+          new PhutilNumber(PhabricatorRepository::IMPORT_THRESHOLD)));
+
+      $repository->markImporting();
+    }
+
     // Clear the working set cache.
     $this->workingSet = array();
 
@@ -579,4 +589,30 @@ final class PhabricatorRepositoryDiscoveryEngine
     PhabricatorWorker::scheduleTask($class, $data);
   }
 
+  private function isInitialImport(array $refs) {
+    $commit_count = count($refs);
+
+    if ($commit_count <= PhabricatorRepository::IMPORT_THRESHOLD) {
+      // If we fetched a small number of commits, assume it's an initial
+      // commit or a stack of a few initial commits.
+      return false;
+    }
+
+    $viewer = $this->getViewer();
+    $repository = $this->getRepository();
+
+    $any_commits = id(new DiffusionCommitQuery())
+      ->setViewer($viewer)
+      ->withRepository($repository)
+      ->setLimit(1)
+      ->execute();
+
+    if ($any_commits) {
+      // If the repository already has commits, this isn't an import.
+      return false;
+    }
+
+    return true;
+  }
+
 }
diff --git a/src/applications/repository/engine/__tests__/PhabricatorWorkingCopyTestCase.php b/src/applications/repository/engine/__tests__/PhabricatorWorkingCopyTestCase.php
index 5b37a9e655..c489d69424 100644
--- a/src/applications/repository/engine/__tests__/PhabricatorWorkingCopyTestCase.php
+++ b/src/applications/repository/engine/__tests__/PhabricatorWorkingCopyTestCase.php
@@ -71,7 +71,6 @@ abstract class PhabricatorWorkingCopyTestCase extends PhabricatorTestCase {
     $this->didConstructRepository($repo);
 
     $repo->save();
-    $repo->makeEphemeral();
 
     // Keep the disk resources around until we exit.
     $this->dirs[] = $dir;
diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php
index 229059efcb..5705bb719c 100644
--- a/src/applications/repository/storage/PhabricatorRepository.php
+++ b/src/applications/repository/storage/PhabricatorRepository.php
@@ -26,6 +26,11 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
    */
   const MINIMUM_QUALIFIED_HASH = 5;
 
+  /**
+   * Minimum number of commits to an empty repository to trigger "import" mode.
+   */
+  const IMPORT_THRESHOLD = 7;
+
   const TABLE_PATH = 'repository_path';
   const TABLE_PATHCHANGE = 'repository_pathchange';
   const TABLE_FILESYSTEM = 'repository_filesystem';
@@ -354,7 +359,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
     if (!strlen($name)) {
       $name = $this->getName();
       $name = phutil_utf8_strtolower($name);
-      $name = preg_replace('@[/ -:]+@', '-', $name);
+      $name = preg_replace('@[/ -:<>]+@', '-', $name);
       $name = trim($name, '-');
       if (!strlen($name)) {
         $name = $this->getCallsign();
@@ -2155,6 +2160,18 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
     return $service;
   }
 
+  public function markImporting() {
+    $this->openTransaction();
+      $this->beginReadLocking();
+        $repository = $this->reload();
+        $repository->setDetail('importing', true);
+        $repository->save();
+      $this->endReadLocking();
+    $this->saveTransaction();
+
+    return $repository;
+  }
+
 
 /* -(  Symbols  )-------------------------------------------------------------*/