Move repository URIs to a dedicated index

Summary:
Ref T4705 (there are also some other adjacent related tasks dealing with URIs).

Currently, we issue a "get repositories matching URIs: ..." query by loading every possible repository and then checking their URIs in PHP.

Instead, put URIs in a separate table. I plan for each repository to potentially have multiple URIs soon, so this prepares for that.

Test Plan:
  - Ran migrations.
  - Looked at index table, made sure it appeared sensible.
  - Ran some queries by `uri` to find repositories, found the repositories I expected.
  - Updated the remote URI of a repository, saw queries / index update appropriately.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4705

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

resources/sql/autopatches/20160112.repo.01.uri.sql

@@ -0,0 +1,7 @@
+CREATE TABLE {$NAMESPACE}_repository.repository_uriindex (
+  id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
+  repositoryPHID VARBINARY(64) NOT NULL,
+  repositoryURI LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT},
+  KEY `key_repository` (repositoryPHID),
+  KEY `key_uri` (repositoryURI(128))
+) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT};

resources/sql/autopatches/20160112.repo.02.uri.index.php

@@ -0,0 +1,7 @@
+<?php
+
+$table = new PhabricatorRepository();
+
+foreach (new LiskMigrationIterator($table) as $repo) {
+  $repo->updateURIIndex();
+}

src/__phutil_library_map__.php

@@ -734,6 +734,7 @@ phutil_register_library_map(array(
     'DiffusionRepositorySymbolsController' => 'applications/diffusion/controller/DiffusionRepositorySymbolsController.php',
     'DiffusionRepositoryTag' => 'applications/diffusion/data/DiffusionRepositoryTag.php',
     'DiffusionRepositoryTestAutomationController' => 'applications/diffusion/controller/DiffusionRepositoryTestAutomationController.php',
+    'DiffusionRepositoryURIsIndexEngineExtension' => 'applications/diffusion/engineextension/DiffusionRepositoryURIsIndexEngineExtension.php',
     'DiffusionRequest' => 'applications/diffusion/request/DiffusionRequest.php',
     'DiffusionResolveRefsConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionResolveRefsConduitAPIMethod.php',
     'DiffusionResolveUserQuery' => 'applications/diffusion/query/DiffusionResolveUserQuery.php',
@@ -3009,6 +3010,7 @@ phutil_register_library_map(array(
     'PhabricatorRepositoryTransaction' => 'applications/repository/storage/PhabricatorRepositoryTransaction.php',
     'PhabricatorRepositoryTransactionQuery' => 'applications/repository/query/PhabricatorRepositoryTransactionQuery.php',
     'PhabricatorRepositoryType' => 'applications/repository/constants/PhabricatorRepositoryType.php',
+    'PhabricatorRepositoryURIIndex' => 'applications/repository/storage/PhabricatorRepositoryURIIndex.php',
     'PhabricatorRepositoryURINormalizer' => 'applications/repository/data/PhabricatorRepositoryURINormalizer.php',
     'PhabricatorRepositoryURINormalizerTestCase' => 'applications/repository/data/__tests__/PhabricatorRepositoryURINormalizerTestCase.php',
     'PhabricatorRepositoryURITestCase' => 'applications/repository/storage/__tests__/PhabricatorRepositoryURITestCase.php',
@@ -4707,6 +4709,7 @@ phutil_register_library_map(array(
     'DiffusionRepositorySymbolsController' => 'DiffusionRepositoryEditController',
     'DiffusionRepositoryTag' => 'Phobject',
     'DiffusionRepositoryTestAutomationController' => 'DiffusionRepositoryEditController',
+    'DiffusionRepositoryURIsIndexEngineExtension' => 'PhabricatorIndexEngineExtension',
     'DiffusionRequest' => 'Phobject',
     'DiffusionResolveRefsConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
     'DiffusionResolveUserQuery' => 'Phobject',
@@ -7415,6 +7418,7 @@ phutil_register_library_map(array(
     'PhabricatorRepositoryTransaction' => 'PhabricatorApplicationTransaction',
     'PhabricatorRepositoryTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
     'PhabricatorRepositoryType' => 'Phobject',
+    'PhabricatorRepositoryURIIndex' => 'PhabricatorRepositoryDAO',
     'PhabricatorRepositoryURINormalizer' => 'Phobject',
     'PhabricatorRepositoryURINormalizerTestCase' => 'PhabricatorTestCase',
     'PhabricatorRepositoryURITestCase' => 'PhabricatorTestCase',

src/applications/diffusion/DiffusionLintSaveRunner.php

@@ -71,7 +71,7 @@ final class DiffusionLintSaveRunner extends Phobject {
     } else if ($uuid) {
       $repository_query->withUUIDs(array($uuid));
     } else if ($remote_uri) {
-      $repository_query->withRemoteURIs(array($remote_uri));
+      $repository_query->withURIs(array($remote_uri));
     }
 
     $repository = $repository_query->executeOne();

src/applications/diffusion/engineextension/DiffusionRepositoryURIsIndexEngineExtension.php

@@ -0,0 +1,22 @@
+<?php
+
+final class DiffusionRepositoryURIsIndexEngineExtension
+  extends PhabricatorIndexEngineExtension {
+
+  const EXTENSIONKEY = 'diffusion.repositories.uri';
+
+  public function getExtensionName() {
+    return pht('Repository URIs');
+  }
+
+  public function shouldIndexObject($object) {
+    return ($object instanceof PhabricatorRepository);
+  }
+
+  public function indexObject(
+    PhabricatorIndexEngine $engine,
+    $object) {
+    $object->updateURIIndex();
+  }
+
+}

src/applications/repository/conduit/RepositoryQueryConduitAPIMethod.php

@@ -63,7 +63,7 @@ final class RepositoryQueryConduitAPIMethod
 
     $remote_uris = $request->getValue('remoteURIs', array());
     if ($remote_uris) {
-      $query->withRemoteURIs($remote_uris);
+      $query->withURIs($remote_uris);
     }
 
     $uuids = $request->getValue('uuids', array());

src/applications/repository/editor/PhabricatorRepositoryEditor.php

@@ -518,4 +518,8 @@ final class PhabricatorRepositoryEditor
     throw new PhabricatorApplicationTransactionValidationException($errors);
   }
 
+  protected function supportsSearch() {
+    return true;
+  }
+
 }

src/applications/repository/query/PhabricatorRepositoryQuery.php

@@ -9,7 +9,7 @@ final class PhabricatorRepositoryQuery
   private $types;
   private $uuids;
   private $nameContains;
-  private $remoteURIs;
+  private $uris;
   private $datasourceQuery;
   private $slugs;
 
@@ -118,8 +118,8 @@ final class PhabricatorRepositoryQuery
     return $this;
   }
 
-  public function withRemoteURIs(array $uris) {
-    $this->remoteURIs = $uris;
+  public function withURIs(array $uris) {
+    $this->uris = $uris;
     return $this;
   }
 
@@ -263,17 +263,6 @@ final class PhabricatorRepositoryQuery
       }
     }
 
-    // TODO: Denormalize this, too.
-    if ($this->remoteURIs) {
-      $try_uris = $this->getNormalizedPaths();
-      $try_uris = array_fuse($try_uris);
-      foreach ($repositories as $key => $repository) {
-        if (!isset($try_uris[$repository->getNormalizedPath()])) {
-          unset($repositories[$key]);
-        }
-      }
-    }
-
     // Build the identifierMap
     if ($this->numericIdentifiers) {
       foreach ($this->numericIdentifiers as $id) {
@@ -445,6 +434,8 @@ final class PhabricatorRepositoryQuery
   protected function buildSelectClauseParts(AphrontDatabaseConnection $conn) {
     $parts = parent::buildSelectClauseParts($conn);
 
+    $parts[] = 'r.*';
+
     if ($this->shouldJoinSummaryTable()) {
       $parts[] = 's.*';
     }
@@ -462,9 +453,28 @@ final class PhabricatorRepositoryQuery
         PhabricatorRepository::TABLE_SUMMARY);
     }
 
+    if ($this->shouldJoinURITable()) {
+      $joins[] = qsprintf(
+        $conn,
+        'LEFT JOIN %T uri ON r.phid = uri.repositoryPHID',
+        id(new PhabricatorRepositoryURIIndex())->getTableName());
+    }
+
     return $joins;
   }
 
+  protected function shouldGroupQueryResultRows() {
+    if ($this->shouldJoinURITable()) {
+      return true;
+    }
+
+    return parent::shouldGroupQueryResultRows();
+  }
+
+  private function shouldJoinURITable() {
+    return ($this->uris !== null);
+  }
+
   private function shouldJoinSummaryTable() {
     if ($this->needCommitCounts) {
       return true;
@@ -592,7 +602,7 @@ final class PhabricatorRepositoryQuery
     if (strlen($this->nameContains)) {
       $where[] = qsprintf(
         $conn,
-        'name LIKE %~',
+        'r.name LIKE %~',
         $this->nameContains);
     }
 
@@ -619,6 +629,16 @@ final class PhabricatorRepositoryQuery
         $this->slugs);
     }
 
+    if ($this->uris !== null) {
+      $try_uris = $this->getNormalizedPaths();
+      $try_uris = array_fuse($try_uris);
+
+      $where[] = qsprintf(
+        $conn,
+        'uri.repositoryURI IN (%Ls)',
+        $try_uris);
+    }
+
     return $where;
   }
 
@@ -635,7 +655,7 @@ final class PhabricatorRepositoryQuery
     // or an `svn+ssh` URI, we could deduce how to normalize it. However, this
     // would be more complicated and it's not clear if it matters in practice.
 
-    foreach ($this->remoteURIs as $uri) {
+    foreach ($this->uris as $uri) {
       $normalized_uris[] = new PhabricatorRepositoryURINormalizer(
         PhabricatorRepositoryURINormalizer::TYPE_GIT,
         $uri);

src/applications/repository/storage/PhabricatorRepository.php

@@ -823,30 +823,40 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
     return $uri;
   }
 
-  public function getNormalizedPath() {
-    $uri = (string)$this->getCloneURIObject();
+  public function updateURIIndex() {
+    $uris = array(
+      (string)$this->getCloneURIObject(),
+    );
 
+    foreach ($uris as $key => $uri) {
+      $uris[$key] = $this->getNormalizedURI($uri)
+        ->getNormalizedPath();
+    }
+
+    PhabricatorRepositoryURIIndex::updateRepositoryURIs(
+      $this->getPHID(),
+      $uris);
+
+    return $this;
+  }
+
+  private function getNormalizedURI($uri) {
     switch ($this->getVersionControlSystem()) {
       case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
-        $normalized_uri = new PhabricatorRepositoryURINormalizer(
+        return new PhabricatorRepositoryURINormalizer(
           PhabricatorRepositoryURINormalizer::TYPE_GIT,
           $uri);
-        break;
       case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
-        $normalized_uri = new PhabricatorRepositoryURINormalizer(
+        return new PhabricatorRepositoryURINormalizer(
           PhabricatorRepositoryURINormalizer::TYPE_SVN,
           $uri);
-        break;
       case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
-        $normalized_uri = new PhabricatorRepositoryURINormalizer(
+        return new PhabricatorRepositoryURINormalizer(
           PhabricatorRepositoryURINormalizer::TYPE_MERCURIAL,
           $uri);
-        break;
       default:
         throw new Exception(pht('Unrecognized version control system.'));
     }
-
-    return $normalized_uri->getNormalizedPath();
   }
 
   public function isTracked() {
@@ -2231,13 +2241,17 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
   public function destroyObjectPermanently(
     PhabricatorDestructionEngine $engine) {
 
+    $phid = $this->getPHID();
+
     $this->openTransaction();
 
       $this->delete();
 
+      PhabricatorRepositoryURIIndex::updateRepositoryURIs($phid, array());
+
       $books = id(new DivinerBookQuery())
         ->setViewer($engine->getViewer())
-        ->withRepositoryPHIDs(array($this->getPHID()))
+        ->withRepositoryPHIDs(array($phid))
         ->execute();
       foreach ($books as $book) {
         $engine->destroyObject($book);
@@ -2245,7 +2259,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
 
       $atoms = id(new DivinerAtomQuery())
         ->setViewer($engine->getViewer())
-        ->withRepositoryPHIDs(array($this->getPHID()))
+        ->withRepositoryPHIDs(array($phid))
         ->execute();
       foreach ($atoms as $atom) {
         $engine->destroyObject($atom);

src/applications/repository/storage/PhabricatorRepositoryURIIndex.php

@@ -0,0 +1,67 @@
+<?php
+
+final class PhabricatorRepositoryURIIndex
+  extends PhabricatorRepositoryDAO {
+
+  protected $repositoryPHID;
+  protected $repositoryURI;
+
+  protected function getConfiguration() {
+    return array(
+      self::CONFIG_TIMESTAMPS => false,
+      self::CONFIG_COLUMN_SCHEMA => array(
+        'repositoryURI' => 'text',
+      ),
+      self::CONFIG_KEY_SCHEMA => array(
+        'key_repository' => array(
+          'columns' => array('repositoryPHID'),
+        ),
+        'key_uri' => array(
+          'columns' => array('repositoryURI(128)'),
+        ),
+      ),
+    ) + parent::getConfiguration();
+  }
+
+  public static function updateRepositoryURIs(
+    $repository_phid,
+    array $uris) {
+
+    $table = new self();
+    $conn_w = $table->establishConnection('w');
+
+    $sql = array();
+    foreach ($uris as $key => $uri) {
+      if (!strlen($uri)) {
+        unset($uris[$key]);
+        continue;
+      }
+
+      $sql[] = qsprintf(
+        $conn_w,
+        '(%s, %s)',
+        $repository_phid,
+        $uri);
+    }
+
+    $table->openTransaction();
+
+      queryfx(
+        $conn_w,
+        'DELETE FROM %T WHERE repositoryPHID = %s',
+        $table->getTableName(),
+        $repository_phid);
+
+      if ($sql) {
+        queryfx(
+          $conn_w,
+          'INSERT INTO %T (repositoryPHID, repositoryURI) VALUES %Q',
+          $table->getTableName(),
+          implode(', ', $sql));
+      }
+
+    $table->saveTransaction();
+
+  }
+
+}