mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-23 05:50:55 +01:00
Move repository URI normalization out of PullLocalDaemon
Summary: Ref T4327. Moves us one small step forward toward testable change parsers by separating out this unrelated logic from the PullLocal daemon. We will also probably want to run this logic so we can do remote path lookups to limit the role of Arcanist Projects in the future, which is why I made the URI type (here, only "git") a parameter rather than calling this a `GitURINormalizer` or something. Test Plan: - Ran unit tests. - Ran `repository discover` on a correctly-configured remote repository. - Ran `repository discover` on an incorrectly-configured remote, got an error message. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T4327 Differential Revision: https://secure.phabricator.com/D7981
This commit is contained in:
epriestley
parent
b26918aae1
commit
0ac58d7db6
5 changed files with 144 additions and 127 deletions
|
|
@ -1881,6 +1881,8 @@ phutil_register_library_map(array(
|
|||
'PhabricatorRepositoryTransaction' => 'applications/repository/storage/PhabricatorRepositoryTransaction.php',
|
||||
'PhabricatorRepositoryTransactionQuery' => 'applications/repository/query/PhabricatorRepositoryTransactionQuery.php',
|
||||
'PhabricatorRepositoryType' => 'applications/repository/constants/PhabricatorRepositoryType.php',
|
||||
'PhabricatorRepositoryURINormalizer' => 'applications/repository/data/PhabricatorRepositoryURINormalizer.php',
|
||||
'PhabricatorRepositoryURINormalizerTestCase' => 'applications/repository/data/__tests__/PhabricatorRepositoryURINormalizerTestCase.php',
|
||||
'PhabricatorRepositoryURITestCase' => 'applications/repository/storage/__tests__/PhabricatorRepositoryURITestCase.php',
|
||||
'PhabricatorRepositoryVCSPassword' => 'applications/repository/storage/PhabricatorRepositoryVCSPassword.php',
|
||||
'PhabricatorS3FileStorageEngine' => 'applications/files/engine/PhabricatorS3FileStorageEngine.php',
|
||||
|
|
@ -4551,6 +4553,8 @@ phutil_register_library_map(array(
|
|||
'PhabricatorRepositoryTestCase' => 'PhabricatorTestCase',
|
||||
'PhabricatorRepositoryTransaction' => 'PhabricatorApplicationTransaction',
|
||||
'PhabricatorRepositoryTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
|
||||
'PhabricatorRepositoryURINormalizer' => 'Phobject',
|
||||
'PhabricatorRepositoryURINormalizerTestCase' => 'PhabricatorTestCase',
|
||||
'PhabricatorRepositoryURITestCase' => 'PhabricatorTestCase',
|
||||
'PhabricatorRepositoryVCSPassword' => 'PhabricatorRepositoryDAO',
|
||||
'PhabricatorS3FileStorageEngine' => 'PhabricatorFileStorageEngine',
|
||||
|
|
|
|||
|
|
@ -719,7 +719,17 @@ final class PhabricatorRepositoryPullLocalDaemon
|
|||
$valid = false;
|
||||
$exists = false;
|
||||
} else {
|
||||
$valid = self::isSameGitOrigin($remote_uri, $expect_remote);
|
||||
$normal_type_git = PhabricatorRepositoryURINormalizer::TYPE_GIT;
|
||||
|
||||
$remote_normal = id(new PhabricatorRepositoryURINormalizer(
|
||||
$normal_type_git,
|
||||
$remote_uri))->getNormalizedPath();
|
||||
|
||||
$expect_normal = id(new PhabricatorRepositoryURINormalizer(
|
||||
$normal_type_git,
|
||||
$expect_remote))->getNormalizedPath();
|
||||
|
||||
$valid = ($remote_normal == $expect_normal);
|
||||
$exists = true;
|
||||
}
|
||||
|
||||
|
|
@ -768,48 +778,6 @@ final class PhabricatorRepositoryPullLocalDaemon
|
|||
}
|
||||
}
|
||||
|
||||
|
||||
|
||||
/**
|
||||
* @task git
|
||||
*/
|
||||
public static function isSameGitOrigin($remote, $expect) {
|
||||
$remote_path = self::getPathFromGitURI($remote);
|
||||
$expect_path = self::getPathFromGitURI($expect);
|
||||
|
||||
$remote_match = self::executeGitNormalizePath($remote_path);
|
||||
$expect_match = self::executeGitNormalizePath($expect_path);
|
||||
|
||||
return ($remote_match == $expect_match);
|
||||
}
|
||||
|
||||
private static function getPathFromGitURI($raw_uri) {
|
||||
$uri = new PhutilURI($raw_uri);
|
||||
if ($uri->getProtocol()) {
|
||||
return $uri->getPath();
|
||||
}
|
||||
|
||||
$uri = new PhutilGitURI($raw_uri);
|
||||
if ($uri->getDomain()) {
|
||||
return $uri->getPath();
|
||||
}
|
||||
|
||||
return $raw_uri;
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* @task git
|
||||
*/
|
||||
private static function executeGitNormalizePath($path) {
|
||||
// Strip away "/" and ".git", so similar paths correctly match.
|
||||
|
||||
$path = trim($path, '/');
|
||||
$path = preg_replace('/\.git$/', '', $path);
|
||||
return $path;
|
||||
}
|
||||
|
||||
|
||||
private function pushToMirrors(PhabricatorRepository $repository) {
|
||||
if (!$repository->canMirror()) {
|
||||
return;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,98 @@
|
|||
<?php
|
||||
|
||||
/**
|
||||
* Normalize repository URIs. For example, these URIs are generally equivalent
|
||||
* and all point at the same repository:
|
||||
*
|
||||
* ssh://user@host/repo
|
||||
* ssh://user@host/repo/
|
||||
* ssh://user@host:22/repo
|
||||
* user@host:/repo
|
||||
* ssh://user@host/repo.git
|
||||
*
|
||||
* This class can be used to normalize URIs like this, in order to detect
|
||||
* alternate spellings of the same repository URI. In particular, the
|
||||
* @{method:getNormalizedPath} method will return:
|
||||
*
|
||||
* repo
|
||||
*
|
||||
* ...for all of these URIs. Generally, usage looks like this:
|
||||
*
|
||||
* $norm_a = new PhabricatorRepositoryURINormalizer($type, $uri_a);
|
||||
* $norm_b = new PhabricatorRepositoryURINormalizer($type, $uri_b);
|
||||
*
|
||||
* if ($norm_a->getNormalizedPath() == $norm_b->getNormalizedPath()) {
|
||||
* // URIs appear to point at the same repository.
|
||||
* } else {
|
||||
* // URIs are very unlikely to be the same repository.
|
||||
* }
|
||||
*
|
||||
* Because a repository can be hosted at arbitrarly many arbitrary URIs, there
|
||||
* is no way to completely prevent false negatives by only examining URIs
|
||||
* (that is, repositories with totally different URIs could really be the same).
|
||||
* However, normalization is relatively agressive and false negatives should
|
||||
* be rare: if normalization says two URIs are different repositories, they
|
||||
* probably are.
|
||||
*
|
||||
* @task normal Normalizing URIs
|
||||
*/
|
||||
final class PhabricatorRepositoryURINormalizer extends Phobject {
|
||||
|
||||
const TYPE_GIT = 'git';
|
||||
private $type;
|
||||
private $uri;
|
||||
|
||||
public function __construct($type, $uri) {
|
||||
switch ($type) {
|
||||
case self::TYPE_GIT:
|
||||
break;
|
||||
default:
|
||||
throw new Exception(pht('Unknown URI type "%s"!'));
|
||||
}
|
||||
|
||||
$this->type = $type;
|
||||
$this->uri = $uri;
|
||||
}
|
||||
|
||||
|
||||
/* -( Normalizing URIs )--------------------------------------------------- */
|
||||
|
||||
|
||||
/**
|
||||
* @task normal
|
||||
*/
|
||||
public function getPath() {
|
||||
switch ($this->type) {
|
||||
case self::TYPE_GIT:
|
||||
$uri = new PhutilURI($this->uri);
|
||||
if ($uri->getProtocol()) {
|
||||
return $uri->getPath();
|
||||
}
|
||||
|
||||
$uri = new PhutilGitURI($this->uri);
|
||||
if ($uri->getDomain()) {
|
||||
return $uri->getPath();
|
||||
}
|
||||
|
||||
return $this->uri;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* @task normal
|
||||
*/
|
||||
public function getNormalizedPath() {
|
||||
$path = $this->getPath();
|
||||
$path = trim($path, '/');
|
||||
|
||||
switch ($this->type) {
|
||||
case self::TYPE_GIT:
|
||||
$path = preg_replace('/\.git$/', '', $path);
|
||||
break;
|
||||
}
|
||||
|
||||
return $path;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
@ -0,0 +1,31 @@
|
|||
<?php
|
||||
|
||||
final class PhabricatorRepositoryURINormalizerTestCase
|
||||
extends PhabricatorTestCase {
|
||||
|
||||
public function testGitURINormalizer() {
|
||||
$cases = array(
|
||||
'ssh://user@domain.com/path.git' => 'path',
|
||||
'https://user@domain.com/path.git' => 'path',
|
||||
'git@domain.com:path.git' => 'path',
|
||||
'ssh://user@gitserv002.com/path.git' => 'path',
|
||||
'ssh://htaft@domain.com/path.git' => 'path',
|
||||
'ssh://user@domain.com/bananas.git' => 'bananas',
|
||||
'git@domain.com:bananas.git' => 'bananas',
|
||||
'user@domain.com:path/repo' => 'path/repo',
|
||||
'user@domain.com:path/repo/' => 'path/repo',
|
||||
'file:///path/to/local/repo.git' => 'path/to/local/repo',
|
||||
'/path/to/local/repo.git' => 'path/to/local/repo',
|
||||
);
|
||||
|
||||
$type_git = PhabricatorRepositoryURINormalizer::TYPE_GIT;
|
||||
|
||||
foreach ($cases as $input => $expect) {
|
||||
$normal = new PhabricatorRepositoryURINormalizer($type_git, $input);
|
||||
$this->assertEqual(
|
||||
$expect,
|
||||
$normal->getNormalizedPath(),
|
||||
pht('Normalized path for "%s".', $input));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -24,88 +24,4 @@ final class PhabricatorWorkingCopyDiscoveryTestCase
|
|||
$this->assertEqual(array(), $refs);
|
||||
}
|
||||
|
||||
public function testExecuteGitVerifySameOrigin() {
|
||||
$cases = array(
|
||||
array(
|
||||
'ssh://user@domain.com/path.git',
|
||||
'ssh://user@domain.com/path.git',
|
||||
true,
|
||||
'Identical paths should pass.',
|
||||
),
|
||||
array(
|
||||
'ssh://user@domain.com/path.git',
|
||||
'https://user@domain.com/path.git',
|
||||
true,
|
||||
'Protocol changes should pass.',
|
||||
),
|
||||
array(
|
||||
'ssh://user@domain.com/path.git',
|
||||
'git@domain.com:path.git',
|
||||
true,
|
||||
'Git implicit SSH should pass.',
|
||||
),
|
||||
array(
|
||||
'ssh://user@gitserv001.com/path.git',
|
||||
'ssh://user@gitserv002.com/path.git',
|
||||
true,
|
||||
'Domain changes should pass.',
|
||||
),
|
||||
array(
|
||||
'ssh://alincoln@domain.com/path.git',
|
||||
'ssh://htaft@domain.com/path.git',
|
||||
true,
|
||||
'User/auth changes should pass.',
|
||||
),
|
||||
array(
|
||||
'ssh://user@domain.com/apples.git',
|
||||
'ssh://user@domain.com/bananas.git',
|
||||
false,
|
||||
'Path changes should fail.',
|
||||
),
|
||||
array(
|
||||
'ssh://user@domain.com/apples.git',
|
||||
'git@domain.com:bananas.git',
|
||||
false,
|
||||
'Git implicit SSH path changes should fail.',
|
||||
),
|
||||
array(
|
||||
'user@domain.com:path/repo.git',
|
||||
'user@domain.com:path/repo',
|
||||
true,
|
||||
'Optional .git extension should not prevent matches.',
|
||||
),
|
||||
array(
|
||||
'user@domain.com:path/repo/',
|
||||
'user@domain.com:path/repo',
|
||||
true,
|
||||
'Optional trailing slash should not prevent matches.',
|
||||
),
|
||||
array(
|
||||
'file:///path/to/local/repo.git',
|
||||
'file:///path/to/local/repo.git',
|
||||
true,
|
||||
'file:// protocol should be supported.',
|
||||
),
|
||||
array(
|
||||
'/path/to/local/repo.git',
|
||||
'file:///path/to/local/repo.git',
|
||||
true,
|
||||
'Implicit file:// protocol should be recognized.',
|
||||
),
|
||||
);
|
||||
|
||||
foreach ($cases as $case) {
|
||||
list($remote, $config, $expect, $message) = $case;
|
||||
|
||||
$this->assertEqual(
|
||||
$expect,
|
||||
PhabricatorRepositoryPullLocalDaemon::isSameGitOrigin(
|
||||
$remote,
|
||||
$config,
|
||||
'(a test case)'),
|
||||
"Verification that '{$remote}' and '{$config}' are the same origin ".
|
||||
"had a different outcome than expected: {$message}");
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue