@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Detect if a commit *really* doesn't exist and 4oh4 from Diffusion commit view
Summary: rather than showing an erroneous "we still parsing" message.
Test Plan: for each version control system, typed in a garbage URL and got a 4oh4. (note this actually fails for SVN -- see comment about how my code fails atm -- and DiffusionGitRequest seems to pick off this error in advance and returns an AphrontUsageException.)
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1624
Differential Revision: https://secure.phabricator.com/D3201
+150
-4
+8
src/__phutil_library_map__.php
+8
src/__phutil_library_map__.php
···
327
327
'DiffusionDiffController' => 'applications/diffusion/controller/DiffusionDiffController.php',
328
328
'DiffusionDiffQuery' => 'applications/diffusion/query/diff/DiffusionDiffQuery.php',
329
329
'DiffusionEmptyResultView' => 'applications/diffusion/view/DiffusionEmptyResultView.php',
330
+
'DiffusionExistsQuery' => 'applications/diffusion/query/exists/DiffusionExistsQuery.php',
330
331
'DiffusionExternalController' => 'applications/diffusion/controller/DiffusionExternalController.php',
331
332
'DiffusionFileContent' => 'applications/diffusion/data/DiffusionFileContent.php',
332
333
'DiffusionFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionFileContentQuery.php',
···
337
338
'DiffusionGitCommitTagsQuery' => 'applications/diffusion/query/committags/DiffusionGitCommitTagsQuery.php',
338
339
'DiffusionGitContainsQuery' => 'applications/diffusion/query/contains/DiffusionGitContainsQuery.php',
339
340
'DiffusionGitDiffQuery' => 'applications/diffusion/query/diff/DiffusionGitDiffQuery.php',
341
+
'DiffusionGitExistsQuery' => 'applications/diffusion/query/exists/DiffusionGitExistsQuery.php',
340
342
'DiffusionGitFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php',
341
343
'DiffusionGitHistoryQuery' => 'applications/diffusion/query/history/DiffusionGitHistoryQuery.php',
342
344
'DiffusionGitLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionGitLastModifiedQuery.php',
···
359
361
'DiffusionMercurialCommitTagsQuery' => 'applications/diffusion/query/committags/DiffusionMercurialCommitTagsQuery.php',
360
362
'DiffusionMercurialContainsQuery' => 'applications/diffusion/query/contains/DiffusionMercurialContainsQuery.php',
361
363
'DiffusionMercurialDiffQuery' => 'applications/diffusion/query/diff/DiffusionMercurialDiffQuery.php',
364
+
'DiffusionMercurialExistsQuery' => 'applications/diffusion/query/exists/DiffusionMercurialExistsQuery.php',
362
365
'DiffusionMercurialFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php',
363
366
'DiffusionMercurialHistoryQuery' => 'applications/diffusion/query/history/DiffusionMercurialHistoryQuery.php',
364
367
'DiffusionMercurialLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionMercurialLastModifiedQuery.php',
···
388
391
'DiffusionSvnCommitTagsQuery' => 'applications/diffusion/query/committags/DiffusionSvnCommitTagsQuery.php',
389
392
'DiffusionSvnContainsQuery' => 'applications/diffusion/query/contains/DiffusionSvnContainsQuery.php',
390
393
'DiffusionSvnDiffQuery' => 'applications/diffusion/query/diff/DiffusionSvnDiffQuery.php',
394
+
'DiffusionSvnExistsQuery' => 'applications/diffusion/query/exists/DiffusionSvnExistsQuery.php',
391
395
'DiffusionSvnFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php',
392
396
'DiffusionSvnHistoryQuery' => 'applications/diffusion/query/history/DiffusionSvnHistoryQuery.php',
393
397
'DiffusionSvnLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionSvnLastModifiedQuery.php',
···
1440
1444
'DiffusionDiffController' => 'DiffusionController',
1441
1445
'DiffusionDiffQuery' => 'DiffusionQuery',
1442
1446
'DiffusionEmptyResultView' => 'DiffusionView',
1447
+
'DiffusionExistsQuery' => 'DiffusionQuery',
1443
1448
'DiffusionExternalController' => 'DiffusionController',
1444
1449
'DiffusionFileContentQuery' => 'DiffusionQuery',
1445
1450
'DiffusionGitBranchQuery' => 'DiffusionBranchQuery',
···
1449
1454
'DiffusionGitCommitTagsQuery' => 'DiffusionCommitTagsQuery',
1450
1455
'DiffusionGitContainsQuery' => 'DiffusionContainsQuery',
1451
1456
'DiffusionGitDiffQuery' => 'DiffusionDiffQuery',
1457
+
'DiffusionGitExistsQuery' => 'DiffusionExistsQuery',
1452
1458
'DiffusionGitFileContentQuery' => 'DiffusionFileContentQuery',
1453
1459
'DiffusionGitHistoryQuery' => 'DiffusionHistoryQuery',
1454
1460
'DiffusionGitLastModifiedQuery' => 'DiffusionLastModifiedQuery',
···
1471
1477
'DiffusionMercurialCommitTagsQuery' => 'DiffusionCommitTagsQuery',
1472
1478
'DiffusionMercurialContainsQuery' => 'DiffusionContainsQuery',
1473
1479
'DiffusionMercurialDiffQuery' => 'DiffusionDiffQuery',
1480
+
'DiffusionMercurialExistsQuery' => 'DiffusionExistsQuery',
1474
1481
'DiffusionMercurialFileContentQuery' => 'DiffusionFileContentQuery',
1475
1482
'DiffusionMercurialHistoryQuery' => 'DiffusionHistoryQuery',
1476
1483
'DiffusionMercurialLastModifiedQuery' => 'DiffusionLastModifiedQuery',
···
1492
1499
'DiffusionSvnCommitTagsQuery' => 'DiffusionCommitTagsQuery',
1493
1500
'DiffusionSvnContainsQuery' => 'DiffusionContainsQuery',
1494
1501
'DiffusionSvnDiffQuery' => 'DiffusionDiffQuery',
1502
+
'DiffusionSvnExistsQuery' => 'DiffusionExistsQuery',
1495
1503
'DiffusionSvnFileContentQuery' => 'DiffusionFileContentQuery',
1496
1504
'DiffusionSvnHistoryQuery' => 'DiffusionHistoryQuery',
1497
1505
'DiffusionSvnLastModifiedQuery' => 'DiffusionLastModifiedQuery',
+7
-4
src/applications/diffusion/controller/DiffusionCommitController.php
+7
-4
src/applications/diffusion/controller/DiffusionCommitController.php
···
50
50
$commit = $drequest->loadCommit();
51
51
52
52
if (!$commit) {
53
-
// TODO -- T1624 -- detect if this has actually not been parsed yet
54
-
// and show this UI if so, else 404
53
+
$query = DiffusionExistsQuery::newFromDiffusionRequest($drequest);
54
+
$exists = $query->loadExistentialData();
55
+
if (!$exists) {
56
+
return new Aphront404Response();
57
+
}
55
58
return $this->buildStandardPageResponse(
56
59
id(new AphrontErrorView())
57
60
->setTitle('Error displaying commit.')
58
-
->appendChild('Failed to load the commit. The commit has not been '.
59
-
'parsed yet.'),
61
+
->appendChild('Failed to load the commit because the commit has not '.
62
+
'been parsed yet.'),
60
63
array('title' => 'Commit Still Parsing')
61
64
);
62
65
}
+29
src/applications/diffusion/query/exists/DiffusionExistsQuery.php
+29
src/applications/diffusion/query/exists/DiffusionExistsQuery.php
···
1
+
<?php
2
+
3
+
/*
4
+
* Copyright 2012 Facebook, Inc.
5
+
*
6
+
* Licensed under the Apache License, Version 2.0 (the "License");
7
+
* you may not use this file except in compliance with the License.
8
+
* You may obtain a copy of the License at
9
+
*
10
+
* http://www.apache.org/licenses/LICENSE-2.0
11
+
*
12
+
* Unless required by applicable law or agreed to in writing, software
13
+
* distributed under the License is distributed on an "AS IS" BASIS,
14
+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15
+
* See the License for the specific language governing permissions and
16
+
* limitations under the License.
17
+
*/
18
+
19
+
abstract class DiffusionExistsQuery extends DiffusionQuery {
20
+
21
+
final public static function newFromDiffusionRequest(
22
+
DiffusionRequest $request) {
23
+
return self::newQueryObject(__CLASS__, $request);
24
+
}
25
+
26
+
final public function loadExistentialData() {
27
+
return $this->executeQuery();
28
+
}
29
+
}
+33
src/applications/diffusion/query/exists/DiffusionGitExistsQuery.php
+33
src/applications/diffusion/query/exists/DiffusionGitExistsQuery.php
···
1
+
<?php
2
+
3
+
/*
4
+
* Copyright 2012 Facebook, Inc.
5
+
*
6
+
* Licensed under the Apache License, Version 2.0 (the "License");
7
+
* you may not use this file except in compliance with the License.
8
+
* You may obtain a copy of the License at
9
+
*
10
+
* http://www.apache.org/licenses/LICENSE-2.0
11
+
*
12
+
* Unless required by applicable law or agreed to in writing, software
13
+
* distributed under the License is distributed on an "AS IS" BASIS,
14
+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15
+
* See the License for the specific language governing permissions and
16
+
* limitations under the License.
17
+
*/
18
+
19
+
final class DiffusionGitExistsQuery extends DiffusionExistsQuery {
20
+
21
+
final protected function executeQuery() {
22
+
$request = $this->getRequest();
23
+
$repository = $request->getRepository();
24
+
25
+
list($err, $merge_base) = $repository->execLocalCommand(
26
+
'cat-file -t %s',
27
+
$request->getCommit()
28
+
);
29
+
30
+
return !$err;
31
+
}
32
+
33
+
}
+33
src/applications/diffusion/query/exists/DiffusionMercurialExistsQuery.php
+33
src/applications/diffusion/query/exists/DiffusionMercurialExistsQuery.php
···
1
+
<?php
2
+
3
+
/*
4
+
* Copyright 2012 Facebook, Inc.
5
+
*
6
+
* Licensed under the Apache License, Version 2.0 (the "License");
7
+
* you may not use this file except in compliance with the License.
8
+
* You may obtain a copy of the License at
9
+
*
10
+
* http://www.apache.org/licenses/LICENSE-2.0
11
+
*
12
+
* Unless required by applicable law or agreed to in writing, software
13
+
* distributed under the License is distributed on an "AS IS" BASIS,
14
+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15
+
* See the License for the specific language governing permissions and
16
+
* limitations under the License.
17
+
*/
18
+
19
+
final class DiffusionMercurialExistsQuery extends DiffusionExistsQuery {
20
+
21
+
protected function executeQuery() {
22
+
$request = $this->getRequest();
23
+
$repository = $request->getRepository();
24
+
25
+
list($err, $stdout) = $repository->execLocalCommand(
26
+
'id --rev %s',
27
+
$request->getCommit()
28
+
);
29
+
30
+
return !$err;
31
+
32
+
}
33
+
}
+40
src/applications/diffusion/query/exists/DiffusionSvnExistsQuery.php
+40
src/applications/diffusion/query/exists/DiffusionSvnExistsQuery.php
···
1
+
<?php
2
+
3
+
/*
4
+
* Copyright 2012 Facebook, Inc.
5
+
*
6
+
* Licensed under the Apache License, Version 2.0 (the "License");
7
+
* you may not use this file except in compliance with the License.
8
+
* You may obtain a copy of the License at
9
+
*
10
+
* http://www.apache.org/licenses/LICENSE-2.0
11
+
*
12
+
* Unless required by applicable law or agreed to in writing, software
13
+
* distributed under the License is distributed on an "AS IS" BASIS,
14
+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15
+
* See the License for the specific language governing permissions and
16
+
* limitations under the License.
17
+
*/
18
+
19
+
final class DiffusionSvnExistsQuery extends DiffusionExistsQuery {
20
+
21
+
protected function executeQuery() {
22
+
$request = $this->getRequest();
23
+
$repository = $request->getRepository();
24
+
25
+
list($info) = $repository->execxRemoteCommand(
26
+
'info %s',
27
+
$repository->getRemoteURI()
28
+
);
29
+
30
+
$matches = null;
31
+
$exists = false;
32
+
if (preg_match('/^Revision: (\d+)$/m', $info, $matches)) {
33
+
$base_revision = $matches[1];
34
+
$exists = $base_revision >= $request->getCommit();
35
+
}
36
+
37
+
return $exists;
38
+
}
39
+
40
+
}