Page MenuHomePhabricator
Files F14010662

D14862.diff
No OneTemporary
Actions

D14862.diff
View Options

diff --git a/resources/sql/autopatches/20151223.proj.04.keycol.sql b/resources/sql/autopatches/20151223.proj.04.keycol.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20151223.proj.04.keycol.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_project.project
+ ADD projectPathKey BINARY(4) NOT NULL;
diff --git a/resources/sql/autopatches/20151223.proj.05.updatekeys.php b/resources/sql/autopatches/20151223.proj.05.updatekeys.php
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20151223.proj.05.updatekeys.php
@@ -0,0 +1,24 @@
+<?php
+
+$table = new PhabricatorProject();
+$conn_w = $table->establishConnection('w');
+
+foreach (new LiskMigrationIterator($table) as $project) {
+ $path = $project->getProjectPath();
+ $key = $project->getProjectPathKey();
+
+ if (strlen($path) && ($key !== "\0\0\0\0")) {
+ continue;
+ }
+
+ $path_key = PhabricatorHash::digestForIndex($project->getPHID());
+ $path_key = substr($path_key, 0, 4);
+
+ queryfx(
+ $conn_w,
+ 'UPDATE %T SET projectPath = %s, projectPathKey = %s WHERE id = %d',
+ $project->getTableName(),
+ $path_key,
+ $path_key,
+ $project->getID());
+}
diff --git a/resources/sql/autopatches/20151223.proj.06.uniq.sql b/resources/sql/autopatches/20151223.proj.06.uniq.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20151223.proj.06.uniq.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_project.project
+ ADD UNIQUE KEY `key_pathkey` (projectPathKey);
diff --git a/src/applications/policy/filter/PhabricatorPolicyFilter.php b/src/applications/policy/filter/PhabricatorPolicyFilter.php
--- a/src/applications/policy/filter/PhabricatorPolicyFilter.php
+++ b/src/applications/policy/filter/PhabricatorPolicyFilter.php
@@ -243,50 +243,6 @@
}
private function applyExtendedPolicyChecks(array $extended_objects) {
- // First, we're going to detect cycles and reject any objects which are
- // part of a cycle. We don't want to loop forever if an object has a
- // self-referential or nonsense policy.
-
- static $in_flight = array();
-
- $all_phids = array();
- foreach ($extended_objects as $key => $object) {
- $phid = $object->getPHID();
- if (isset($in_flight[$phid])) {
- // TODO: This could be more user-friendly.
- $this->rejectObject($extended_objects[$key], false, '<cycle>');
- unset($extended_objects[$key]);
- continue;
- }
-
- // We might throw from rejectObject(), so we don't want to actually mark
- // anything as in-flight until we survive this entire step.
- $all_phids[$phid] = $phid;
- }
-
- foreach ($all_phids as $phid) {
- $in_flight[$phid] = true;
- }
-
- $caught = null;
- try {
- $extended_objects = $this->executeExtendedPolicyChecks($extended_objects);
- } catch (Exception $ex) {
- $caught = $ex;
- }
-
- foreach ($all_phids as $phid) {
- unset($in_flight[$phid]);
- }
-
- if ($caught) {
- throw $caught;
- }
-
- return $extended_objects;
- }
-
- private function executeExtendedPolicyChecks(array $extended_objects) {
$viewer = $this->viewer;
$filter_capabilities = $this->capabilities;
@@ -396,10 +352,11 @@
}
if ($objects_in) {
- $objects_out = id(new PhabricatorPolicyFilter())
- ->setViewer($viewer)
- ->requireCapabilities($capabilities)
- ->apply($objects_in);
+ $objects_out = $this->executeExtendedPolicyChecks(
+ $viewer,
+ $capabilities,
+ $objects_in,
+ $key_map);
$objects_out = mpull($objects_out, null, 'getPHID');
} else {
$objects_out = array();
@@ -435,6 +392,53 @@
return $extended_objects;
}
+ private function executeExtendedPolicyChecks(
+ PhabricatorUser $viewer,
+ array $capabilities,
+ array $objects,
+ array $key_map) {
+
+ // Do crude cycle detection by seeing if we have a huge stack depth.
+ // Although more sophisticated cycle detection is possible in theory,
+ // it is difficult with hierarchical objects like subprojects. Many other
+ // checks make it difficult to create cycles normally, so just do a
+ // simple check here to limit damage.
+
+ static $depth;
+
+ $depth++;
+
+ if ($depth > 32) {
+ foreach ($objects as $key => $object) {
+ $this->rejectObject($objects[$key], false, '<cycle>');
+ unset($objects[$key]);
+ continue;
+ }
+ }
+
+ if (!$objects) {
+ return array();
+ }
+
+ $caught = null;
+ try {
+ $result = id(new PhabricatorPolicyFilter())
+ ->setViewer($viewer)
+ ->requireCapabilities($capabilities)
+ ->apply($objects);
+ } catch (Exception $ex) {
+ $caught = $ex;
+ }
+
+ $depth--;
+
+ if ($caught) {
+ throw $caught;
+ }
+
+ return $result;
+ }
+
private function checkCapability(
PhabricatorPolicyInterface $object,
$capability) {
diff --git a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
--- a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
+++ b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
@@ -113,6 +113,80 @@
$this->assertTrue($caught instanceof Exception);
}
+ public function testAncestryQueries() {
+ $user = $this->createUser();
+ $user->save();
+
+ $ancestor = $this->createProject($user);
+ $parent = $this->createProject($user, $ancestor);
+ $child = $this->createProject($user, $parent);
+
+ $projects = id(new PhabricatorProjectQuery())
+ ->setViewer($user)
+ ->withAncestorProjectPHIDs(array($ancestor->getPHID()))
+ ->execute();
+ $this->assertEqual(2, count($projects));
+
+ $projects = id(new PhabricatorProjectQuery())
+ ->setViewer($user)
+ ->withParentProjectPHIDs(array($ancestor->getPHID()))
+ ->execute();
+ $this->assertEqual(1, count($projects));
+ $this->assertEqual(
+ $parent->getPHID(),
+ head($projects)->getPHID());
+
+ $projects = id(new PhabricatorProjectQuery())
+ ->setViewer($user)
+ ->withAncestorProjectPHIDs(array($ancestor->getPHID()))
+ ->withDepthBetween(2, null)
+ ->execute();
+ $this->assertEqual(1, count($projects));
+ $this->assertEqual(
+ $child->getPHID(),
+ head($projects)->getPHID());
+
+ $parent2 = $this->createProject($user, $ancestor);
+ $child2 = $this->createProject($user, $parent2);
+ $grandchild2 = $this->createProject($user, $child2);
+
+ $projects = id(new PhabricatorProjectQuery())
+ ->setViewer($user)
+ ->withAncestorProjectPHIDs(array($ancestor->getPHID()))
+ ->execute();
+ $this->assertEqual(5, count($projects));
+
+ $projects = id(new PhabricatorProjectQuery())
+ ->setViewer($user)
+ ->withParentProjectPHIDs(array($ancestor->getPHID()))
+ ->execute();
+ $this->assertEqual(2, count($projects));
+
+ $projects = id(new PhabricatorProjectQuery())
+ ->setViewer($user)
+ ->withAncestorProjectPHIDs(array($ancestor->getPHID()))
+ ->withDepthBetween(2, null)
+ ->execute();
+ $this->assertEqual(3, count($projects));
+
+ $projects = id(new PhabricatorProjectQuery())
+ ->setViewer($user)
+ ->withAncestorProjectPHIDs(array($ancestor->getPHID()))
+ ->withDepthBetween(3, null)
+ ->execute();
+ $this->assertEqual(1, count($projects));
+
+ $projects = id(new PhabricatorProjectQuery())
+ ->setViewer($user)
+ ->withPHIDs(
+ array(
+ $child->getPHID(),
+ $grandchild2->getPHID(),
+ ))
+ ->execute();
+ $this->assertEqual(2, count($projects));
+ }
+
public function testParentProject() {
$user = $this->createUser();
$user->save();
diff --git a/src/applications/project/query/PhabricatorProjectQuery.php b/src/applications/project/query/PhabricatorProjectQuery.php
--- a/src/applications/project/query/PhabricatorProjectQuery.php
+++ b/src/applications/project/query/PhabricatorProjectQuery.php
@@ -11,6 +11,11 @@
private $nameTokens;
private $icons;
private $colors;
+ private $ancestorPHIDs;
+ private $parentPHIDs;
+ private $isMilestone;
+ private $minDepth;
+ private $maxDepth;
private $status = 'status-any';
const STATUS_ANY = 'status-any';
@@ -69,6 +74,27 @@
return $this;
}
+ public function withParentProjectPHIDs($parent_phids) {
+ $this->parentPHIDs = $parent_phids;
+ return $this;
+ }
+
+ public function withAncestorProjectPHIDs($ancestor_phids) {
+ $this->ancestorPHIDs = $ancestor_phids;
+ return $this;
+ }
+
+ public function withIsMilestone($is_milestone) {
+ $this->isMilestone = $is_milestone;
+ return $this;
+ }
+
+ public function withDepthBetween($min, $max) {
+ $this->minDepth = $min;
+ $this->maxDepth = $max;
+ return $this;
+ }
+
public function needMembers($need_members) {
$this->needMembers = $need_members;
return $this;
@@ -337,6 +363,65 @@
$this->colors);
}
+ if ($this->parentPHIDs !== null) {
+ $where[] = qsprintf(
+ $conn,
+ 'parentProjectPHID IN (%Ls)',
+ $this->parentPHIDs);
+ }
+
+ if ($this->ancestorPHIDs !== null) {
+ $ancestor_paths = queryfx_all(
+ $conn,
+ 'SELECT projectPath, projectDepth FROM %T WHERE phid IN (%Ls)',
+ id(new PhabricatorProject())->getTableName(),
+ $this->ancestorPHIDs);
+ if (!$ancestor_paths) {
+ throw new PhabricatorEmptyQueryException();
+ }
+
+ $sql = array();
+ foreach ($ancestor_paths as $ancestor_path) {
+ $sql[] = qsprintf(
+ $conn,
+ '(projectPath LIKE %> AND projectDepth > %d)',
+ $ancestor_path['projectPath'],
+ $ancestor_path['projectDepth']);
+ }
+
+ $where[] = '('.implode(' OR ', $sql).')';
+
+ $where[] = qsprintf(
+ $conn,
+ 'parentProjectPHID IS NOT NULL');
+ }
+
+ if ($this->isMilestone !== null) {
+ if ($this->isMilestone) {
+ $where[] = qsprintf(
+ $conn,
+ 'milestoneNumber IS NOT NULL');
+ } else {
+ $where[] = qsprintf(
+ $conn,
+ 'milestoneNumber IS NULL');
+ }
+ }
+
+ if ($this->minDepth !== null) {
+ $where[] = qsprintf(
+ $conn,
+ 'projectDepth >= %d',
+ $this->minDepth);
+ }
+
+ if ($this->maxDepth !== null) {
+ $where[] = qsprintf(
+ $conn,
+ 'projectDepth <= %d',
+ $this->maxDepth);
+ }
+
return $where;
}
diff --git a/src/applications/project/storage/PhabricatorProject.php b/src/applications/project/storage/PhabricatorProject.php
--- a/src/applications/project/storage/PhabricatorProject.php
+++ b/src/applications/project/storage/PhabricatorProject.php
@@ -33,6 +33,7 @@
protected $projectPath;
protected $projectDepth;
+ protected $projectPathKey;
private $memberPHIDs = self::ATTACHABLE;
private $watcherPHIDs = self::ATTACHABLE;
@@ -196,6 +197,7 @@
'milestoneNumber' => 'uint32?',
'projectPath' => 'hashpath64',
'projectDepth' => 'uint32',
+ 'projectPathKey' => 'bytes4',
),
self::CONFIG_KEY_SCHEMA => array(
'key_phid' => null,
@@ -224,6 +226,10 @@
'key_path' => array(
'columns' => array('projectPath', 'projectDepth'),
),
+ 'key_pathkey' => array(
+ 'columns' => array('projectPathKey'),
+ 'unique' => true,
+ ),
),
) + parent::getConfiguration();
}
@@ -310,6 +316,12 @@
$this->setPHID($this->generatePHID());
}
+ if (!strlen($this->getProjectPathKey())) {
+ $hash = PhabricatorHash::digestForIndex($this->getPHID());
+ $hash = substr($hash, 0, 4);
+ $this->setProjectPathKey($hash);
+ }
+
$path = array();
$depth = 0;
if ($this->parentProjectPHID) {
@@ -317,15 +329,12 @@
$path[] = $parent->getProjectPath();
$depth = $parent->getProjectDepth() + 1;
}
- $hash = PhabricatorHash::digestForIndex($this->getPHID());
- $path[] = substr($hash, 0, 4);
-
+ $path[] = $this->getProjectPathKey();
$path = implode('', $path);
$limit = self::getProjectDepthLimit();
- if (strlen($path) > ($limit * 4)) {
- throw new Exception(
- pht('Unable to save project: path length is too long.'));
+ if ($depth >= $limit) {
+ throw new Exception(pht('Project depth is too great.'));
}
$this->setProjectPath($path);
@@ -403,7 +412,7 @@
$path = $this->getProjectPath();
$parent_length = (strlen($path) - 4);
- for ($ii = $parent_length; $ii >= 0; $ii -= 4) {
+ for ($ii = $parent_length; $ii > 0; $ii -= 4) {
$parts[] = substr($path, 0, $ii);
}

File Metadata

Mime Type
text/plain
Expires
Fri, Nov 1, 11:34 AM (2 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6743250
Default Alt Text
D14862.diff (12 KB)

Event Timeline

Phabricator · Built by Phacility