1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-25 06:50:55 +01:00

Provide a more modern way to load packages owning a set of files

Summary:
Ref T8320. Ref T8004. This just tries to generally modernize

It also replaces the nonfunctional "Find Owners" link with a new property that just shows owning packages.

Test Plan:
  - Created and edited packages.

{F720478}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8004, T8320

Differential Revision: https://secure.phabricator.com/D13911
This commit is contained in:
epriestley 2015-08-15 13:06:10 -07:00
parent 8149399312
commit 36a50ccbae
7 changed files with 242 additions and 29 deletions

View file

@ -111,25 +111,6 @@ abstract class DiffusionBrowseController extends DiffusionController {
->setIcon('fa-home') ->setIcon('fa-home')
->setDisabled(!$behind_head)); ->setDisabled(!$behind_head));
// TODO: Ideally, this should live in Owners and be event-triggered, but
// there's no reasonable object for it to react to right now.
$owners = 'PhabricatorOwnersApplication';
if (PhabricatorApplication::isClassInstalled($owners)) {
$owners_uri = id(new PhutilURI('/owners/view/search/'))
->setQueryParams(
array(
'repository' => $drequest->getCallsign(),
'path' => '/'.$drequest->getPath(),
));
$view->addAction(
id(new PhabricatorActionView())
->setName(pht('Find Owners'))
->setHref((string)$owners_uri)
->setIcon('fa-users'));
}
return $view; return $view;
} }
@ -137,7 +118,7 @@ abstract class DiffusionBrowseController extends DiffusionController {
DiffusionRequest $drequest, DiffusionRequest $drequest,
PhabricatorActionListView $actions) { PhabricatorActionListView $actions) {
$viewer = $this->getRequest()->getUser(); $viewer = $this->getViewer();
$view = id(new PHUIPropertyListView()) $view = id(new PHUIPropertyListView())
->setUser($viewer) ->setUser($viewer)
@ -180,6 +161,47 @@ abstract class DiffusionBrowseController extends DiffusionController {
} }
} }
$repository = $drequest->getRepository();
$owners = 'PhabricatorOwnersApplication';
if (PhabricatorApplication::isClassInstalled($owners)) {
$package_query = id(new PhabricatorOwnersPackageQuery())
->setViewer($viewer)
->withControl(
$repository->getPHID(),
array(
$drequest->getPath(),
));
$package_query->execute();
$packages = $package_query->getControllingPackagesForPath(
$repository->getPHID(),
$drequest->getPath());
if ($packages) {
$ownership = id(new PHUIStatusListView())
->setUser($viewer);
foreach ($packages as $package) {
$icon = 'fa-list-alt';
$color = 'grey';
$item = id(new PHUIStatusItemView())
->setIcon($icon, $color)
->setTarget($viewer->renderHandle($package->getPHID()));
$ownership->addItem($item);
}
} else {
$ownership = phutil_tag('em', array(), pht('None'));
}
$view->addProperty(pht('Packages'), $ownership);
}
return $view; return $view;
} }

View file

@ -13,12 +13,13 @@ final class PhabricatorOwnersDetailController
$package = id(new PhabricatorOwnersPackageQuery()) $package = id(new PhabricatorOwnersPackageQuery())
->setViewer($viewer) ->setViewer($viewer)
->withIDs(array($request->getURIData('id'))) ->withIDs(array($request->getURIData('id')))
->needPaths(true)
->executeOne(); ->executeOne();
if (!$package) { if (!$package) {
return new Aphront404Response(); return new Aphront404Response();
} }
$paths = $package->loadPaths(); $paths = $package->getPaths();
$repository_phids = array(); $repository_phids = array();
foreach ($paths as $path) { foreach ($paths as $path) {

View file

@ -15,6 +15,7 @@ final class PhabricatorOwnersPathsController
// TODO: Support this capability. // TODO: Support this capability.
// PhabricatorPolicyCapability::CAN_EDIT, // PhabricatorPolicyCapability::CAN_EDIT,
)) ))
->needPaths(true)
->executeOne(); ->executeOne();
if (!$package) { if (!$package) {
return new Aphront404Response(); return new Aphront404Response();
@ -66,7 +67,7 @@ final class PhabricatorOwnersPathsController
return id(new AphrontRedirectResponse()) return id(new AphrontRedirectResponse())
->setURI('/owners/package/'.$package->getID().'/'); ->setURI('/owners/package/'.$package->getID().'/');
} else { } else {
$paths = $package->loadPaths(); $paths = $package->getPaths();
$path_refs = mpull($paths, 'getRef'); $path_refs = mpull($paths, 'getRef');
} }

View file

@ -43,8 +43,7 @@ final class PhabricatorOwnersPackageTransactionEditor
case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION: case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION:
return $object->getDescription(); return $object->getDescription();
case PhabricatorOwnersPackageTransaction::TYPE_PATHS: case PhabricatorOwnersPackageTransaction::TYPE_PATHS:
// TODO: needPaths() this on the query $paths = $object->getPaths();
$paths = $object->loadPaths();
return mpull($paths, 'getRef'); return mpull($paths, 'getRef');
} }
} }
@ -152,8 +151,7 @@ final class PhabricatorOwnersPackageTransactionEditor
$old = $xaction->getOldValue(); $old = $xaction->getOldValue();
$new = $xaction->getNewValue(); $new = $xaction->getNewValue();
// TODO: needPaths this $paths = $object->getPaths();
$paths = $object->loadPaths();
$diffs = PhabricatorOwnersPath::getTransactionValueChanges($old, $new); $diffs = PhabricatorOwnersPath::getTransactionValueChanges($old, $new);
list($rem, $add) = $diffs; list($rem, $add) = $diffs;

View file

@ -8,6 +8,10 @@ final class PhabricatorOwnersPackageQuery
private $ownerPHIDs; private $ownerPHIDs;
private $repositoryPHIDs; private $repositoryPHIDs;
private $namePrefix; private $namePrefix;
private $needPaths;
private $controlMap = array();
private $controlResults;
/** /**
* Owners are direct owners, and members of owning projects. * Owners are direct owners, and members of owning projects.
@ -32,19 +36,64 @@ final class PhabricatorOwnersPackageQuery
return $this; return $this;
} }
public function withControl($repository_phid, array $paths) {
if (empty($this->controlMap[$repository_phid])) {
$this->controlMap[$repository_phid] = array();
}
foreach ($paths as $path) {
$this->controlMap[$repository_phid][$path] = $path;
}
// We need to load paths to execute control queries.
$this->needPaths = true;
return $this;
}
public function withNamePrefix($prefix) { public function withNamePrefix($prefix) {
$this->namePrefix = $prefix; $this->namePrefix = $prefix;
return $this; return $this;
} }
public function needPaths($need_paths) {
$this->needPaths = $need_paths;
return $this;
}
public function newResultObject() { public function newResultObject() {
return new PhabricatorOwnersPackage(); return new PhabricatorOwnersPackage();
} }
protected function willExecute() {
$this->controlResults = array();
}
protected function loadPage() { protected function loadPage() {
return $this->loadStandardPage(new PhabricatorOwnersPackage()); return $this->loadStandardPage(new PhabricatorOwnersPackage());
} }
protected function didFilterPage(array $packages) {
if ($this->needPaths) {
$package_ids = mpull($packages, 'getID');
$paths = id(new PhabricatorOwnersPath())->loadAllWhere(
'packageID IN (%Ld)',
$package_ids);
$paths = mgroup($paths, 'getPackageID');
foreach ($packages as $package) {
$package->attachPaths(idx($paths, $package->getID(), array()));
}
}
if ($this->controlMap) {
$this->controlResults += mpull($packages, null, 'getID');
}
return $packages;
}
protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) { protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
$joins = parent::buildJoinClauseParts($conn); $joins = parent::buildJoinClauseParts($conn);
@ -55,7 +104,7 @@ final class PhabricatorOwnersPackageQuery
id(new PhabricatorOwnersOwner())->getTableName()); id(new PhabricatorOwnersOwner())->getTableName());
} }
if ($this->repositoryPHIDs !== null) { if ($this->shouldJoinOwnersPathTable()) {
$joins[] = qsprintf( $joins[] = qsprintf(
$conn, $conn,
'JOIN %T rpath ON rpath.packageID = p.id', 'JOIN %T rpath ON rpath.packageID = p.id',
@ -115,11 +164,30 @@ final class PhabricatorOwnersPackageQuery
phutil_utf8_strtolower($this->namePrefix)); phutil_utf8_strtolower($this->namePrefix));
} }
if ($this->controlMap) {
$clauses = array();
foreach ($this->controlMap as $repository_phid => $paths) {
$fragments = array();
foreach ($paths as $path) {
foreach (PhabricatorOwnersPackage::splitPath($path) as $fragment) {
$fragments[$fragment] = $fragment;
}
}
$clauses[] = qsprintf(
$conn,
'(rpath.repositoryPHID = %s AND rpath.path IN (%Ls))',
$repository_phid,
$fragments);
}
$where[] = implode(' OR ', $clauses);
}
return $where; return $where;
} }
protected function shouldGroupQueryResultRows() { protected function shouldGroupQueryResultRows() {
if ($this->repositoryPHIDs) { if ($this->shouldJoinOwnersPathTable()) {
return true; return true;
} }
@ -167,4 +235,83 @@ final class PhabricatorOwnersPackageQuery
return 'p'; return 'p';
} }
private function shouldJoinOwnersPathTable() {
if ($this->repositoryPHIDs !== null) {
return true;
}
if ($this->controlMap) {
return true;
}
return false;
}
/* -( Path Control )------------------------------------------------------- */
/**
* Get the package which controls a path, if one exists.
*
* @return PhabricatorOwnersPackage|null Package, if one exists.
*/
public function getControllingPackageForPath($repository_phid, $path) {
$packages = $this->getControllingPackagesForPath($repository_phid, $path);
if (!$packages) {
return null;
}
return head($packages);
}
/**
* Get a list of all packages which control a path or its parent directories,
* ordered from weakest to strongest.
*
* The first package has the most specific claim on the path; the last
* package has the most general claim.
*
* @return list<PhabricatorOwnersPackage> List of controlling packages.
*/
public function getControllingPackagesForPath($repository_phid, $path) {
if (!isset($this->controlMap[$repository_phid][$path])) {
throw new PhutilInvalidStateException('withControl');
}
if ($this->controlResults === null) {
throw new PhutilInvalidStateException('execute');
}
$packages = $this->controlResults;
$matches = array();
foreach ($packages as $package_id => $package) {
$best_match = null;
$include = false;
foreach ($package->getPaths() as $package_path) {
$strength = $package_path->getPathMatchStrength($path);
if ($strength > $best_match) {
$best_match = $strength;
$include = !$package_path->getExcluded();
}
}
if ($best_match && $include) {
$matches[$package_id] = array(
'strength' => $best_match,
'package' => $package,
);
}
}
$matches = isort($matches, 'strength');
$matches = array_reverse($matches);
return array_values(ipull($matches, 'package'));
}
} }

View file

@ -13,6 +13,8 @@ final class PhabricatorOwnersPackage
protected $primaryOwnerPHID; protected $primaryOwnerPHID;
protected $mailKey; protected $mailKey;
private $paths = self::ATTACHABLE;
public static function initializeNewPackage(PhabricatorUser $actor) { public static function initializeNewPackage(PhabricatorUser $actor) {
return id(new PhabricatorOwnersPackage()) return id(new PhabricatorOwnersPackage())
->setAuditingEnabled(0) ->setAuditingEnabled(0)
@ -225,7 +227,7 @@ final class PhabricatorOwnersPackage
return $ids; return $ids;
} }
private static function splitPath($path) { public static function splitPath($path) {
$result = array('/'); $result = array('/');
$trailing_slash = preg_match('@/$@', $path) ? '/' : ''; $trailing_slash = preg_match('@/$@', $path) ? '/' : '';
$path = trim($path, '/'); $path = trim($path, '/');
@ -238,6 +240,16 @@ final class PhabricatorOwnersPackage
return $result; return $result;
} }
public function attachPaths(array $paths) {
assert_instances_of($paths, 'PhabricatorOwnersPath');
$this->paths = $paths;
return $this;
}
public function getPaths() {
return $this->assertAttached($this->paths);
}
/* -( PhabricatorApplicationTransactionInterface )------------------------- */ /* -( PhabricatorApplicationTransactionInterface )------------------------- */

View file

@ -70,4 +70,36 @@ final class PhabricatorOwnersPath extends PhabricatorOwnersDAO {
return isset($set[$ref['repositoryPHID']][$ref['path']][$ref['excluded']]); return isset($set[$ref['repositoryPHID']][$ref['path']][$ref['excluded']]);
} }
/**
* Get the number of directory matches between this path specification and
* some real path.
*/
public function getPathMatchStrength($path) {
$this_path = $this->getPath();
if ($this_path === '/') {
// The root path "/" just matches everything with strength 1.
return 1;
}
$self_fragments = PhabricatorOwnersPackage::splitPath($this_path);
$path_fragments = PhabricatorOwnersPackage::splitPath($path);
$self_count = count($self_fragments);
$path_count = count($path_fragments);
if ($self_count > $path_count) {
// If this path is longer (and therefor more specific) than the target
// path, we don't match it at all.
return 0;
}
for ($ii = 0; $ii < $self_count; $ii++) {
if ($self_fragments[$ii] != $path_fragments[$ii]) {
return 0;
}
}
return $self_count;
}
} }