mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-28 00:10:57 +01:00
Move project slug normalization inside project Query
Summary: Ref T10010. We currently require `withSlugs()` to have properly formatted slugs, but this leads to similar code in several places. Instead: accept any slug, normalize slugs in the query, return a map so callers can figure out what happened if they want. This tends to do the right thing by default, while keeping enough information around to do more complex things if necessary. A similar approach for querying commits has worked well in Diffusion. Test Plan: Added and executed unit tests. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10010 Differential Revision: https://secure.phabricator.com/D14888
This commit is contained in:
epriestley
parent
aa2089ba68
commit
211a6c0d55
2 changed files with 178 additions and 12 deletions
|
|
@ -493,6 +493,90 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase {
|
|||
$this->assertFalse((bool)$this->refreshProject($child, $user2));
|
||||
}
|
||||
|
||||
public function testSlugMaps() {
|
||||
// When querying by slugs, slugs should be normalized and the mapping
|
||||
// should be reported correctly.
|
||||
$user = $this->createUser();
|
||||
$user->save();
|
||||
|
||||
$name = 'queryslugproject';
|
||||
$name2 = 'QUERYslugPROJECT';
|
||||
$slug = 'queryslugextra';
|
||||
$slug2 = 'QuErYSlUgExTrA';
|
||||
|
||||
$project = PhabricatorProject::initializeNewProject($user);
|
||||
|
||||
$xactions = array();
|
||||
|
||||
$xactions[] = id(new PhabricatorProjectTransaction())
|
||||
->setTransactionType(PhabricatorProjectTransaction::TYPE_NAME)
|
||||
->setNewValue($name);
|
||||
|
||||
$xactions[] = id(new PhabricatorProjectTransaction())
|
||||
->setTransactionType(PhabricatorProjectTransaction::TYPE_SLUGS)
|
||||
->setNewValue(array($slug));
|
||||
|
||||
$this->applyTransactions($project, $user, $xactions);
|
||||
|
||||
$project_query = id(new PhabricatorProjectQuery())
|
||||
->setViewer($user)
|
||||
->withSlugs(array($name));
|
||||
$project_query->execute();
|
||||
$map = $project_query->getSlugMap();
|
||||
|
||||
$this->assertEqual(
|
||||
array(
|
||||
$name => $project->getPHID(),
|
||||
),
|
||||
ipull($map, 'projectPHID'));
|
||||
|
||||
$project_query = id(new PhabricatorProjectQuery())
|
||||
->setViewer($user)
|
||||
->withSlugs(array($slug));
|
||||
$project_query->execute();
|
||||
$map = $project_query->getSlugMap();
|
||||
|
||||
$this->assertEqual(
|
||||
array(
|
||||
$slug => $project->getPHID(),
|
||||
),
|
||||
ipull($map, 'projectPHID'));
|
||||
|
||||
$project_query = id(new PhabricatorProjectQuery())
|
||||
->setViewer($user)
|
||||
->withSlugs(array($name, $slug, $name2, $slug2));
|
||||
$project_query->execute();
|
||||
$map = $project_query->getSlugMap();
|
||||
|
||||
$expect = array(
|
||||
$name => $project->getPHID(),
|
||||
$slug => $project->getPHID(),
|
||||
$name2 => $project->getPHID(),
|
||||
$slug2 => $project->getPHID(),
|
||||
);
|
||||
|
||||
$actual = ipull($map, 'projectPHID');
|
||||
|
||||
ksort($expect);
|
||||
ksort($actual);
|
||||
|
||||
$this->assertEqual($expect, $actual);
|
||||
|
||||
$expect = array(
|
||||
$name => $name,
|
||||
$slug => $slug,
|
||||
$name2 => $name,
|
||||
$slug2 => $slug,
|
||||
);
|
||||
|
||||
$actual = ipull($map, 'slug');
|
||||
|
||||
ksort($expect);
|
||||
ksort($actual);
|
||||
|
||||
$this->assertEqual($expect, $actual);
|
||||
}
|
||||
|
||||
private function attemptProjectEdit(
|
||||
PhabricatorProject $proj,
|
||||
PhabricatorUser $user,
|
||||
|
|
|
|||
|
|
@ -7,6 +7,8 @@ final class PhabricatorProjectQuery
|
|||
private $phids;
|
||||
private $memberPHIDs;
|
||||
private $slugs;
|
||||
private $slugNormals;
|
||||
private $slugMap;
|
||||
private $names;
|
||||
private $nameTokens;
|
||||
private $icons;
|
||||
|
|
@ -157,6 +159,24 @@ final class PhabricatorProjectQuery
|
|||
);
|
||||
}
|
||||
|
||||
public function getSlugMap() {
|
||||
if ($this->slugMap === null) {
|
||||
throw new PhutilInvalidStateException('execute');
|
||||
}
|
||||
return $this->slugMap;
|
||||
}
|
||||
|
||||
protected function willExecute() {
|
||||
$this->slugMap = array();
|
||||
$this->slugNormals = array();
|
||||
if ($this->slugs) {
|
||||
foreach ($this->slugs as $slug) {
|
||||
$normal = PhabricatorSlug::normalizeProjectSlug($slug);
|
||||
$this->slugNormals[$slug] = $normal;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
protected function loadPage() {
|
||||
return $this->loadStandardPage($this->newResultObject());
|
||||
}
|
||||
|
|
@ -287,17 +307,7 @@ final class PhabricatorProjectQuery
|
|||
}
|
||||
}
|
||||
|
||||
if ($this->needSlugs) {
|
||||
$slugs = id(new PhabricatorProjectSlug())
|
||||
->loadAllWhere(
|
||||
'projectPHID IN (%Ls)',
|
||||
mpull($projects, 'getPHID'));
|
||||
$slugs = mgroup($slugs, 'getProjectPHID');
|
||||
foreach ($projects as $project) {
|
||||
$project_slugs = idx($slugs, $project->getPHID(), array());
|
||||
$project->attachSlugs($project_slugs);
|
||||
}
|
||||
}
|
||||
$this->loadSlugs($projects);
|
||||
|
||||
return $projects;
|
||||
}
|
||||
|
|
@ -356,7 +366,7 @@ final class PhabricatorProjectQuery
|
|||
$where[] = qsprintf(
|
||||
$conn,
|
||||
'slug.slug IN (%Ls)',
|
||||
$this->slugs);
|
||||
$this->slugNormals);
|
||||
}
|
||||
|
||||
if ($this->names !== null) {
|
||||
|
|
@ -583,4 +593,76 @@ final class PhabricatorProjectQuery
|
|||
return $ancestors;
|
||||
}
|
||||
|
||||
private function loadSlugs(array $projects) {
|
||||
// Build a map from primary slugs to projects.
|
||||
$primary_map = array();
|
||||
foreach ($projects as $project) {
|
||||
$primary_slug = $project->getPrimarySlug();
|
||||
if ($primary_slug === null) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$primary_map[$primary_slug] = $project;
|
||||
}
|
||||
|
||||
// Link up all of the queried slugs which correspond to primary
|
||||
// slugs. If we can link up everything from this (no slugs were queried,
|
||||
// or only primary slugs were queried) we don't need to load anything
|
||||
// else.
|
||||
$unknown = $this->slugNormals;
|
||||
foreach ($unknown as $input => $normal) {
|
||||
if (!isset($primary_map[$normal])) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$this->slugMap[$input] = array(
|
||||
'slug' => $normal,
|
||||
'projectPHID' => $primary_map[$normal]->getPHID(),
|
||||
);
|
||||
|
||||
unset($unknown[$input]);
|
||||
}
|
||||
|
||||
// If we need slugs, we have to load everything.
|
||||
// If we still have some queried slugs which we haven't mapped, we only
|
||||
// need to look for them.
|
||||
// If we've mapped everything, we don't have to do any work.
|
||||
$project_phids = mpull($projects, 'getPHID');
|
||||
if ($this->needSlugs) {
|
||||
$slugs = id(new PhabricatorProjectSlug())->loadAllWhere(
|
||||
'projectPHID IN (%Ls)',
|
||||
$project_phids);
|
||||
} else if ($unknown) {
|
||||
$slugs = id(new PhabricatorProjectSlug())->loadAllWhere(
|
||||
'projectPHID IN (%Ls) AND slug IN (%Ls)',
|
||||
$project_phids,
|
||||
$unknown);
|
||||
} else {
|
||||
$slugs = array();
|
||||
}
|
||||
|
||||
// Link up any slugs we were not able to link up earlier.
|
||||
$extra_map = mpull($slugs, 'getProjectPHID', 'getSlug');
|
||||
foreach ($unknown as $input => $normal) {
|
||||
if (!isset($extra_map[$normal])) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$this->slugMap[$input] = array(
|
||||
'slug' => $normal,
|
||||
'projectPHID' => $extra_map[$normal],
|
||||
);
|
||||
|
||||
unset($unknown[$input]);
|
||||
}
|
||||
|
||||
if ($this->needSlugs) {
|
||||
$slug_groups = mgroup($slugs, 'getProjectPHID');
|
||||
foreach ($projects as $project) {
|
||||
$project_slugs = idx($slug_groups, $project->getPHID(), array());
|
||||
$project->attachSlugs($project_slugs);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue