From 576b73dc5375e80c3b687712686d10337386e06b Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 10 May 2016 06:20:48 -0700 Subject: [PATCH] Index all repository URIs, not just the "primary" repository URI Summary: Ref T10923. When regenerating the URI index for a repository, index every URI. - Also, make the index slightly stricter (domain + path instead of just path). Excluding the domain made more sense when we were generating only first-party URIs. - Make the index smarter about `/diffusion/123/` URIs. - Show normalized URIs in `diffusion.repository.search` results. Test Plan: - Ran migration. - Verified sensible-looking results in database. - Searched for a repository URI by first-party clone URI. - Searched for a repository URI by mirror URI. - Used `diffusion.repository.search` to get information about repository URIs. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10923 Differential Revision: https://secure.phabricator.com/D15876 --- .../20160112.repo.02.uri.index.php | 7 ++-- .../autopatches/20160510.repo.01.uriindex.php | 10 ++++++ .../PhabricatorRepositoryURINormalizer.php | 35 ++++++++++++++++++- .../query/PhabricatorRepositoryQuery.php | 20 +++++------ .../PhabricatorRepositorySearchEngine.php | 9 +++++ .../storage/PhabricatorRepository.php | 35 +++++-------------- .../storage/PhabricatorRepositoryURI.php | 21 +++++++++++ 7 files changed, 93 insertions(+), 44 deletions(-) create mode 100644 resources/sql/autopatches/20160510.repo.01.uriindex.php diff --git a/resources/sql/autopatches/20160112.repo.02.uri.index.php b/resources/sql/autopatches/20160112.repo.02.uri.index.php index 7a11be3179..e9cd061361 100644 --- a/resources/sql/autopatches/20160112.repo.02.uri.index.php +++ b/resources/sql/autopatches/20160112.repo.02.uri.index.php @@ -1,7 +1,4 @@ updateURIIndex(); -} +// A later patch ("20160510.repo.01.uriindex.php") performs an identical +// regeneration of the index, so we no longer need to do it here. diff --git a/resources/sql/autopatches/20160510.repo.01.uriindex.php b/resources/sql/autopatches/20160510.repo.01.uriindex.php new file mode 100644 index 0000000000..191985af47 --- /dev/null +++ b/resources/sql/autopatches/20160510.repo.01.uriindex.php @@ -0,0 +1,10 @@ +setViewer(PhabricatorUser::getOmnipotentUser()) + ->needURIs(true) + ->execute(); + +foreach ($repos as $repo) { + $repo->updateURIIndex(); +} diff --git a/src/applications/repository/data/PhabricatorRepositoryURINormalizer.php b/src/applications/repository/data/PhabricatorRepositoryURINormalizer.php index 3a9508e197..42224c9553 100644 --- a/src/applications/repository/data/PhabricatorRepositoryURINormalizer.php +++ b/src/applications/repository/data/PhabricatorRepositoryURINormalizer.php @@ -59,6 +59,14 @@ final class PhabricatorRepositoryURINormalizer extends Phobject { $this->uri = $uri; } + public static function getAllURITypes() { + return array( + self::TYPE_GIT, + self::TYPE_SVN, + self::TYPE_MERCURIAL, + ); + } + /* -( Normalizing URIs )--------------------------------------------------- */ @@ -91,6 +99,10 @@ final class PhabricatorRepositoryURINormalizer extends Phobject { } } + public function getNormalizedURI() { + return $this->getNormalizedDomain().'/'.$this->getNormalizedPath(); + } + /** * @task normal @@ -113,11 +125,32 @@ final class PhabricatorRepositoryURINormalizer extends Phobject { // example. $matches = null; - if (preg_match('@^(diffusion/[A-Z]+)@', $path, $matches)) { + if (preg_match('@^(diffusion/(?:[A-Z]+|\d+))@', $path, $matches)) { $path = $matches[1]; } return $path; } + public function getNormalizedDomain() { + $domain = null; + + $uri = new PhutilURI($this->uri); + if ($uri->getProtocol()) { + $domain = $uri->getDomain(); + } + + if (!strlen($domain)) { + $uri = new PhutilGitURI($this->uri); + $domain = $uri->getDomain(); + } + + if (!strlen($domain)) { + $domain = ''; + } + + return phutil_utf8_strtolower($domain); + } + + } diff --git a/src/applications/repository/query/PhabricatorRepositoryQuery.php b/src/applications/repository/query/PhabricatorRepositoryQuery.php index 78693d54d6..b67b3c5610 100644 --- a/src/applications/repository/query/PhabricatorRepositoryQuery.php +++ b/src/applications/repository/query/PhabricatorRepositoryQuery.php @@ -650,7 +650,7 @@ final class PhabricatorRepositoryQuery } if ($this->uris !== null) { - $try_uris = $this->getNormalizedPaths(); + $try_uris = $this->getNormalizedURIs(); $try_uris = array_fuse($try_uris); $where[] = qsprintf( @@ -666,7 +666,7 @@ final class PhabricatorRepositoryQuery return 'PhabricatorDiffusionApplication'; } - private function getNormalizedPaths() { + private function getNormalizedURIs() { $normalized_uris = array(); // Since we don't know which type of repository this URI is in the general @@ -675,19 +675,15 @@ 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. + $types = PhabricatorRepositoryURINormalizer::getAllURITypes(); foreach ($this->uris as $uri) { - $normalized_uris[] = new PhabricatorRepositoryURINormalizer( - PhabricatorRepositoryURINormalizer::TYPE_GIT, - $uri); - $normalized_uris[] = new PhabricatorRepositoryURINormalizer( - PhabricatorRepositoryURINormalizer::TYPE_SVN, - $uri); - $normalized_uris[] = new PhabricatorRepositoryURINormalizer( - PhabricatorRepositoryURINormalizer::TYPE_MERCURIAL, - $uri); + foreach ($types as $type) { + $normalized_uri = new PhabricatorRepositoryURINormalizer($type, $uri); + $normalized_uris[] = $normalized_uri->getNormalizedURI(); + } } - return array_unique(mpull($normalized_uris, 'getNormalizedPath')); + return array_unique($normalized_uris); } } diff --git a/src/applications/repository/query/PhabricatorRepositorySearchEngine.php b/src/applications/repository/query/PhabricatorRepositorySearchEngine.php index 18519f56be..78f29fad1a 100644 --- a/src/applications/repository/query/PhabricatorRepositorySearchEngine.php +++ b/src/applications/repository/query/PhabricatorRepositorySearchEngine.php @@ -38,6 +38,11 @@ final class PhabricatorRepositorySearchEngine ->setLabel(pht('Types')) ->setKey('types') ->setOptions(PhabricatorRepositoryType::getAllRepositoryTypes()), + id(new PhabricatorSearchStringListField()) + ->setLabel(pht('URIs')) + ->setKey('uris') + ->setDescription( + pht('Search for repositories by clone/checkout URI.')), ); } @@ -70,6 +75,10 @@ final class PhabricatorRepositorySearchEngine $query->withNameContains($map['name']); } + if ($map['uris']) { + $query->withURIs($map['uris']); + } + return $query; } diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index 36379eda6c..229059efcb 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -803,41 +803,24 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO } public function updateURIIndex() { - $uris = array( - (string)$this->getCloneURIObject(), - ); + $indexes = array(); - foreach ($uris as $key => $uri) { - $uris[$key] = $this->getNormalizedURI($uri) - ->getNormalizedPath(); + $uris = $this->getURIs(); + foreach ($uris as $uri) { + if ($uri->getIsDisabled()) { + continue; + } + + $indexes[] = $uri->getNormalizedURI(); } PhabricatorRepositoryURIIndex::updateRepositoryURIs( $this->getPHID(), - $uris); + $indexes); return $this; } - private function getNormalizedURI($uri) { - switch ($this->getVersionControlSystem()) { - case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: - return new PhabricatorRepositoryURINormalizer( - PhabricatorRepositoryURINormalizer::TYPE_GIT, - $uri); - case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: - return new PhabricatorRepositoryURINormalizer( - PhabricatorRepositoryURINormalizer::TYPE_SVN, - $uri); - case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: - return new PhabricatorRepositoryURINormalizer( - PhabricatorRepositoryURINormalizer::TYPE_MERCURIAL, - $uri); - default: - throw new Exception(pht('Unrecognized version control system.')); - } - } - public function isTracked() { $status = $this->getDetail('tracking-enabled'); $map = self::getStatusMap(); diff --git a/src/applications/repository/storage/PhabricatorRepositoryURI.php b/src/applications/repository/storage/PhabricatorRepositoryURI.php index 21fc791de5..53b9040453 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryURI.php +++ b/src/applications/repository/storage/PhabricatorRepositoryURI.php @@ -196,6 +196,26 @@ final class PhabricatorRepositoryURI return $this->getURIObject(false); } + public function getNormalizedURI() { + $vcs = $this->getRepository()->getVersionControlSystem(); + + $map = array( + PhabricatorRepositoryType::REPOSITORY_TYPE_GIT => + PhabricatorRepositoryURINormalizer::TYPE_GIT, + PhabricatorRepositoryType::REPOSITORY_TYPE_SVN => + PhabricatorRepositoryURINormalizer::TYPE_SVN, + PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL => + PhabricatorRepositoryURINormalizer::TYPE_MERCURIAL, + ); + + $type = $map[$vcs]; + $display = (string)$this->getDisplayURI(); + + $normal_uri = new PhabricatorRepositoryURINormalizer($type, $display); + + return $normal_uri->getNormalizedURI(); + } + public function getEffectiveURI() { return $this->getURIObject(true); } @@ -693,6 +713,7 @@ final class PhabricatorRepositoryURI 'raw' => $this->getURI(), 'display' => (string)$this->getDisplayURI(), 'effective' => (string)$this->getEffectiveURI(), + 'normalized' => (string)$this->getNormalizedURI(), ), 'io' => array( 'raw' => $this->getIOType(),