Validate paths before saving them when editing an owners package
Summary: Paths in owners packages when referring to a directory should always end with a trailing slash. (Otherwise, some things break, like loading the owning packages for a path.) With this change, PhabricatorOwnersPackage now requires that the path provided for a package is valid, and if the path is for a directory, it adds a trailing slash if one was not provided. Test Plan: Edited a path in a package and left off the trailing slash. Saw that the slash was added. Tried again with the trailing slash, and checked that another slash was not added. Did this with a path in both a git and svn repository. Reviewers: epriestley, jungejason Reviewed By: epriestley CC: aran, epriestley Differential Revision: 1251
This commit is contained in:
parent
43430e154d
commit
7075222b4b
|
@ -178,8 +178,41 @@ class PhabricatorOwnersPackage extends PhabricatorOwnersDAO {
|
||||||
}
|
}
|
||||||
$cur_paths = mgroup($cur_paths, 'getRepositoryPHID', 'getPath');
|
$cur_paths = mgroup($cur_paths, 'getRepositoryPHID', 'getPath');
|
||||||
foreach ($new_paths as $repository_phid => $paths) {
|
foreach ($new_paths as $repository_phid => $paths) {
|
||||||
|
// get repository object for path validation
|
||||||
|
$repository = id(new PhabricatorRepository())->loadOneWhere(
|
||||||
|
'phid = %s',
|
||||||
|
$repository_phid);
|
||||||
|
if (!$repository) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
foreach ($paths as $path => $ignored) {
|
foreach ($paths as $path => $ignored) {
|
||||||
if (empty($cur_paths[$repository_phid][$path])) {
|
$path = ltrim($path, '/');
|
||||||
|
// build query to validate path
|
||||||
|
$drequest = DiffusionRequest::newFromAphrontRequestDictionary(
|
||||||
|
array(
|
||||||
|
'callsign' => $repository->getCallsign(),
|
||||||
|
'path' => ':/'.$path,
|
||||||
|
));
|
||||||
|
$query = DiffusionBrowseQuery::newFromDiffusionRequest($drequest);
|
||||||
|
$query->needValidityOnly(true);
|
||||||
|
$valid = $query->loadPaths();
|
||||||
|
$is_directory = true;
|
||||||
|
if (!$valid) {
|
||||||
|
switch ($query->getReasonForEmptyResultSet()) {
|
||||||
|
case DiffusionBrowseQuery::REASON_IS_FILE:
|
||||||
|
$valid = true;
|
||||||
|
$is_directory = false;
|
||||||
|
break;
|
||||||
|
case DiffusionBrowseQuery::REASON_IS_EMPTY:
|
||||||
|
$valid = true;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if ($is_directory && substr($path, -1) != '/') {
|
||||||
|
$path .= '/';
|
||||||
|
}
|
||||||
|
$path = '/'.$path;
|
||||||
|
if (empty($cur_paths[$repository_phid][$path]) && $valid) {
|
||||||
$obj = new PhabricatorOwnersPath();
|
$obj = new PhabricatorOwnersPath();
|
||||||
$obj->setPackageID($this->getID());
|
$obj->setPackageID($this->getID());
|
||||||
$obj->setRepositoryPHID($repository_phid);
|
$obj->setRepositoryPHID($repository_phid);
|
||||||
|
|
|
@ -6,10 +6,13 @@
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
phutil_require_module('phabricator', 'applications/diffusion/query/browse/base');
|
||||||
|
phutil_require_module('phabricator', 'applications/diffusion/request/base');
|
||||||
phutil_require_module('phabricator', 'applications/owners/storage/base');
|
phutil_require_module('phabricator', 'applications/owners/storage/base');
|
||||||
phutil_require_module('phabricator', 'applications/owners/storage/owner');
|
phutil_require_module('phabricator', 'applications/owners/storage/owner');
|
||||||
phutil_require_module('phabricator', 'applications/owners/storage/path');
|
phutil_require_module('phabricator', 'applications/owners/storage/path');
|
||||||
phutil_require_module('phabricator', 'applications/phid/storage/phid');
|
phutil_require_module('phabricator', 'applications/phid/storage/phid');
|
||||||
|
phutil_require_module('phabricator', 'applications/repository/storage/repository');
|
||||||
phutil_require_module('phabricator', 'storage/qsprintf');
|
phutil_require_module('phabricator', 'storage/qsprintf');
|
||||||
phutil_require_module('phabricator', 'storage/queryfx');
|
phutil_require_module('phabricator', 'storage/queryfx');
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue