1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-29 10:12:41 +01:00

Correct Diffusion browse behavior when visiting a path URI with no trailing slash

Summary:
See PHI1983. Ref T13599. Ref T13589. Currently, if you browse to a path browse URI in Diffusion without a trailing slash (`/browse/master/src`), you get a nonsensical view (the directory as a single item).

Be more precise in how "git ls-tree" arguments are constructed.

Test Plan: Visited files and directories in the browse view, with and without trailing slashes. Saw improved behavior for directories with no trailing slash and reasonable behavior in all other cases.

Maniphest Tasks: T13599, T13589

Differential Revision: https://secure.phabricator.com/D21528
This commit is contained in:
epriestley 2021-01-27 16:31:08 -08:00
parent b4f2cef76c
commit d6fd365704

View file

@ -37,7 +37,7 @@ final class DiffusionBrowseQueryConduitAPIMethod
$repository = $drequest->getRepository(); $repository = $drequest->getRepository();
$path = $request->getValue('path'); $path = $request->getValue('path');
if (!strlen($path)) { if (!strlen($path) || $path === '/') {
$path = null; $path = null;
} }
@ -49,12 +49,13 @@ final class DiffusionBrowseQueryConduitAPIMethod
if ($path === null) { if ($path === null) {
// Fast path to improve the performance of the repository view; we know // Fast path to improve the performance of the repository view; we know
// the root is always a tree at any commit and always exists. // the root is always a tree at any commit and always exists.
$stdout = 'tree'; $path_type = 'tree';
} else { } else {
try { try {
list($stdout) = $repository->execxLocalCommand( list($stdout) = $repository->execxLocalCommand(
'cat-file -t -- %s', 'cat-file -t -- %s',
sprintf('%s:%s', $commit, $path)); sprintf('%s:%s', $commit, $path));
$path_type = trim($stdout);
} catch (CommandException $e) { } catch (CommandException $e) {
// The "cat-file" command may fail if the path legitimately does not // The "cat-file" command may fail if the path legitimately does not
// exist, but it may also fail if the path is a submodule. This can // exist, but it may also fail if the path is a submodule. This can
@ -114,7 +115,7 @@ final class DiffusionBrowseQueryConduitAPIMethod
} }
} }
if (trim($stdout) == 'blob') { if ($path_type === 'blob') {
$result->setReasonForEmptyResultSet( $result->setReasonForEmptyResultSet(
DiffusionBrowseResultSet::REASON_IS_FILE); DiffusionBrowseResultSet::REASON_IS_FILE);
return $result; return $result;
@ -130,6 +131,12 @@ final class DiffusionBrowseQueryConduitAPIMethod
'ls-tree -z -l %s --', 'ls-tree -z -l %s --',
gitsprintf('%s', $commit)); gitsprintf('%s', $commit));
} else { } else {
if ($path_type === 'tree') {
$path = rtrim($path, '/').'/';
} else {
$path = rtrim($path, '/');
}
list($stdout) = $repository->execxLocalCommand( list($stdout) = $repository->execxLocalCommand(
'ls-tree -z -l %s -- %s', 'ls-tree -z -l %s -- %s',
gitsprintf('%s', $commit), gitsprintf('%s', $commit),