Page MenuHomePhabricator

D15301.id36920.diff
No OneTemporary

D15301.id36920.diff

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -689,7 +689,7 @@
'DiffusionPreCommitRefRepositoryHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitRefRepositoryHeraldField.php',
'DiffusionPreCommitRefRepositoryProjectsHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitRefRepositoryProjectsHeraldField.php',
'DiffusionPreCommitRefTypeHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitRefTypeHeraldField.php',
- 'DiffusionPullEventGarbageCollector' => 'applications/diffusion/DiffusionPullEventGarbageCollector.php',
+ 'DiffusionPullEventGarbageCollector' => 'applications/diffusion/garbagecollector/DiffusionPullEventGarbageCollector.php',
'DiffusionPushCapability' => 'applications/diffusion/capability/DiffusionPushCapability.php',
'DiffusionPushEventViewController' => 'applications/diffusion/controller/DiffusionPushEventViewController.php',
'DiffusionPushLogController' => 'applications/diffusion/controller/DiffusionPushLogController.php',
diff --git a/src/applications/diffusion/application/PhabricatorDiffusionApplication.php b/src/applications/diffusion/application/PhabricatorDiffusionApplication.php
--- a/src/applications/diffusion/application/PhabricatorDiffusionApplication.php
+++ b/src/applications/diffusion/application/PhabricatorDiffusionApplication.php
@@ -64,7 +64,11 @@
'(?:query/(?P<queryKey>[^/]+)/)?' => 'DiffusionPushLogListController',
'view/(?P<id>\d+)/' => 'DiffusionPushEventViewController',
),
- '(?P<repositoryCallsign>[A-Z]+)/' => array(
+ '(?:'.
+ '(?P<repositoryCallsign>[A-Z]+)'.
+ '|'.
+ '(?P<repositoryID>[1-9]\d*)'.
+ ')/' => array(
'' => 'DiffusionRepositoryController',
'repository/(?P<dblob>.*)' => 'DiffusionRepositoryController',
@@ -115,8 +119,9 @@
// catch-all for serving repositories over HTTP. We must accept
// requests without the trailing "/" because SVN commands don't
// necessarily include it.
- '(?P<repositoryCallsign>[A-Z]+)(?:/.*)?' =>
- 'DiffusionRepositoryDefaultController',
+ '(?:(?P<repositoryCallsign>[A-Z]+)|(?P<repositoryID>[1-9]\d*))'.
+ '(?:/.*)?'
+ => 'DiffusionRepositoryDefaultController',
'inline/' => array(
'edit/(?P<phid>[^/]+)/' => 'DiffusionInlineCommentController',
diff --git a/src/applications/diffusion/controller/DiffusionController.php b/src/applications/diffusion/controller/DiffusionController.php
--- a/src/applications/diffusion/controller/DiffusionController.php
+++ b/src/applications/diffusion/controller/DiffusionController.php
@@ -64,6 +64,20 @@
return new Aphront404Response();
}
+ // If the client is making a request like "/diffusion/1/...", but the
+ // repository has a different canonical path like "/diffusion/XYZ/...",
+ // redirect them to the canonical path.
+
+ $request_path = $request->getPath();
+ $repository = $drequest->getRepository();
+
+ $canonical_path = $repository->getCanonicalPath($request_path);
+ if ($canonical_path !== null) {
+ if ($canonical_path != $request_path) {
+ return id(new AphrontRedirectResponse())->setURI($canonical_path);
+ }
+ }
+
$this->diffusionRequest = $drequest;
return null;
diff --git a/src/applications/diffusion/DiffusionPullEventGarbageCollector.php b/src/applications/diffusion/garbagecollector/DiffusionPullEventGarbageCollector.php
rename from src/applications/diffusion/DiffusionPullEventGarbageCollector.php
rename to src/applications/diffusion/garbagecollector/DiffusionPullEventGarbageCollector.php
diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php
--- a/src/applications/repository/storage/PhabricatorRepository.php
+++ b/src/applications/repository/storage/PhabricatorRepository.php
@@ -687,6 +687,55 @@
return "/r{$callsign}{$identifier}";
}
+ public function getCanonicalPath($request_path) {
+ $standard_pattern =
+ '(^'.
+ '(?P<prefix>/diffusion/)'.
+ '(?P<identifier>[^/]+)'.
+ '(?P<suffix>(?:/.*)?)'.
+ '\z)';
+
+ $matches = null;
+ if (preg_match($standard_pattern, $request_path, $matches)) {
+ $prefix = $matches['prefix'];
+
+ $callsign = $this->getCallsign();
+ if ($callsign) {
+ $identifier = $callsign;
+ } else {
+ $identifier = $this->getID();
+ }
+
+ $suffix = $matches['suffix'];
+ if (!strlen($suffix)) {
+ $suffix = '/';
+ }
+
+ return $prefix.$identifier.$suffix;
+ }
+
+ $commit_pattern =
+ '(^'.
+ '(?P<prefix>/)'.
+ '(?P<monogram>'.
+ '(?:'.
+ 'r(?P<repositoryCallsign>[A-Z]+)'.
+ '|'.
+ 'R(?P<repositoryID>[1-9]\d*):'.
+ ')'.
+ '(?P<commit>[a-f0-9]+)'.
+ ')'.
+ '\z)';
+
+ $matches = null;
+ if (preg_match($commit_pattern, $request_path, $matches)) {
+ $commit = $matches['commit'];
+ return $this->getCommitURI($commit);
+ }
+
+ return null;
+ }
+
public function generateURI(array $params) {
$req_branch = false;
$req_commit = false;
diff --git a/src/applications/repository/storage/__tests__/PhabricatorRepositoryURITestCase.php b/src/applications/repository/storage/__tests__/PhabricatorRepositoryURITestCase.php
--- a/src/applications/repository/storage/__tests__/PhabricatorRepositoryURITestCase.php
+++ b/src/applications/repository/storage/__tests__/PhabricatorRepositoryURITestCase.php
@@ -9,6 +9,47 @@
);
}
+ public function testRepositoryURICanonicalization() {
+ $repo = id(new PhabricatorRepository())
+ ->makeEphemeral()
+ ->setID(123);
+
+ $tests = array(
+ '/diffusion/123' => '/diffusion/123/',
+ '/diffusion/123/' => '/diffusion/123/',
+ '/diffusion/123/browse/master/' => '/diffusion/123/browse/master/',
+ '/kangaroo/' => null,
+ );
+
+ foreach ($tests as $input => $expect) {
+ $this->assertEqual(
+ $expect,
+ $repo->getCanonicalPath($input),
+ pht('Canonical Path (ID, No Callsign): %s', $input));
+ }
+
+ $repo->setCallsign('XYZ');
+
+ $tests = array(
+ '/diffusion/123' => '/diffusion/XYZ/',
+ '/diffusion/123/' => '/diffusion/XYZ/',
+ '/diffusion/123/browse/master/' => '/diffusion/XYZ/browse/master/',
+ '/diffusion/XYZ' => '/diffusion/XYZ/',
+ '/diffusion/XYZ/' => '/diffusion/XYZ/',
+ '/diffusion/XYZ/browse/master/' => '/diffusion/XYZ/browse/master/',
+ '/diffusion/ABC/' => '/diffusion/XYZ/',
+ '/kangaroo/' => null,
+ '/R1:abcdef' => '/rXYZabcdef',
+ );
+
+ foreach ($tests as $input => $expect) {
+ $this->assertEqual(
+ $expect,
+ $repo->getCanonicalPath($input),
+ pht('Canonical Path (ID, Callsign): %s', $input));
+ }
+ }
+
public function testURIGeneration() {
$svn = PhabricatorRepositoryType::REPOSITORY_TYPE_SVN;
$git = PhabricatorRepositoryType::REPOSITORY_TYPE_GIT;

File Metadata

Mime Type
text/plain
Expires
Wed, Mar 26, 3:06 AM (4 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7386744
Default Alt Text
D15301.id36920.diff (7 KB)

Event Timeline