D14861.id35938.diff
No OneTemporary
Actions

Size

20 KB

Referenced Files

None

Subscribers

None

D14861.id35938.diff
View Options

	diff --git a/resources/sql/autopatches/20151223.proj.01.paths.sql b/resources/sql/autopatches/20151223.proj.01.paths.sql
	new file mode 100644
	--- /dev/null
	+++ b/resources/sql/autopatches/20151223.proj.01.paths.sql
	@@ -0,0 +1,2 @@
	+ALTER TABLE {$NAMESPACE}_project.project
	+ ADD projectPath VARBINARY(64) NOT NULL;
	diff --git a/resources/sql/autopatches/20151223.proj.02.depths.sql b/resources/sql/autopatches/20151223.proj.02.depths.sql
	new file mode 100644
	--- /dev/null
	+++ b/resources/sql/autopatches/20151223.proj.02.depths.sql
	@@ -0,0 +1,2 @@
	+ALTER TABLE {$NAMESPACE}_project.project
	+ ADD projectDepth INT UNSIGNED NOT NULL;
	diff --git a/resources/sql/autopatches/20151223.proj.03.pathkey.sql b/resources/sql/autopatches/20151223.proj.03.pathkey.sql
	new file mode 100644
	--- /dev/null
	+++ b/resources/sql/autopatches/20151223.proj.03.pathkey.sql
	@@ -0,0 +1,2 @@
	+ALTER TABLE {$NAMESPACE}_project.project
	+ ADD KEY `key_path` (projectPath, projectDepth);
	diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
	--- a/src/__phutil_library_map__.php
	+++ b/src/__phutil_library_map__.php
	@@ -7131,6 +7131,7 @@
	'PhabricatorApplicationTransactionInterface',
	'PhabricatorFlaggableInterface',
	'PhabricatorPolicyInterface',
	+ 'PhabricatorExtendedPolicyInterface',
	'PhabricatorSubscribableInterface',
	'PhabricatorCustomFieldInterface',
	'PhabricatorDestructibleInterface',
	diff --git a/src/applications/config/schema/PhabricatorConfigSchemaSpec.php b/src/applications/config/schema/PhabricatorConfigSchemaSpec.php
	--- a/src/applications/config/schema/PhabricatorConfigSchemaSpec.php
	+++ b/src/applications/config/schema/PhabricatorConfigSchemaSpec.php
	@@ -321,6 +321,7 @@
	break;
	case 'phid':
	case 'policy';
	+ case 'hashpath64':
	$column_type = 'varbinary(64)';
	break;
	case 'bytes64':
	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,59 @@
	$this->assertTrue($caught instanceof Exception);
	}

	+ public function testParentProject() {
	+ $user = $this->createUser();
	+ $user->save();
	+
	+ $parent = $this->createProject($user);
	+ $child = $this->createProject($user, $parent);
	+
	+ $this->assertTrue(true);
	+
	+ $child = $this->refreshProject($child, $user);
	+
	+ $this->assertEqual(
	+ $parent->getPHID(),
	+ $child->getParentProject()->getPHID());
	+
	+ $this->assertEqual(1, (int)$child->getProjectDepth());
	+
	+ $this->assertFalse(
	+ $child->isUserMember($user->getPHID()));
	+
	+ $this->assertFalse(
	+ $child->getParentProject()->isUserMember($user->getPHID()));
	+
	+ $this->joinProject($child, $user);
	+
	+ $child = $this->refreshProject($child, $user);
	+
	+ $this->assertTrue(
	+ $child->isUserMember($user->getPHID()));
	+
	+ $this->assertTrue(
	+ $child->getParentProject()->isUserMember($user->getPHID()));
	+
	+
	+ // Test that hiding a parent hides the child.
	+
	+ $user2 = $this->createUser();
	+ $user2->save();
	+
	+ // Second user can see the project for now.
	+ $this->assertTrue((bool)$this->refreshProject($child, $user2));
	+
	+ // Hide the parent.
	+ $this->setViewPolicy($parent, $user, $user->getPHID());
	+
	+ // First user (who can see the parent because they are a member of
	+ // the child) can see the project.
	+ $this->assertTrue((bool)$this->refreshProject($child, $user));
	+
	+ // Second user can not, because they can't see the parent.
	+ $this->assertFalse((bool)$this->refreshProject($child, $user2));
	+ }
	+
	private function attemptProjectEdit(
	PhabricatorProject $proj,
	PhabricatorUser $user,
	@@ -126,10 +179,7 @@
	$xaction->setTransactionType(PhabricatorProjectTransaction::TYPE_NAME);
	$xaction->setNewValue($new_name);

	- $editor = new PhabricatorProjectTransactionEditor();
	- $editor->setActor($user);
	- $editor->setContentSource(PhabricatorContentSource::newConsoleSource());
	- $editor->applyTransactions($proj, array($xaction));
	+ $this->applyTransactions($proj, $user, array($xaction));

	return true;
	}
	@@ -270,10 +320,43 @@
	}
	}

	- private function createProject(PhabricatorUser $user) {
	+ private function createProject(
	+ PhabricatorUser $user,
	+ PhabricatorProject $parent = null) {
	+
	$project = PhabricatorProject::initializeNewProject($user);
	- $project->setName(pht('Test Project %d', mt_rand()));
	- $project->save();
	+
	+ $name = pht('Test Project %d', mt_rand());
	+
	+ $xactions = array();
	+
	+ $xactions[] = id(new PhabricatorProjectTransaction())
	+ ->setTransactionType(PhabricatorProjectTransaction::TYPE_NAME)
	+ ->setNewValue($name);
	+
	+ if ($parent) {
	+ $xactions[] = id(new PhabricatorProjectTransaction())
	+ ->setTransactionType(PhabricatorProjectTransaction::TYPE_PARENT)
	+ ->setNewValue($parent->getPHID());
	+ }
	+
	+ $this->applyTransactions($project, $user, $xactions);
	+
	+ return $project;
	+ }
	+
	+ private function setViewPolicy(
	+ PhabricatorProject $project,
	+ PhabricatorUser $user,
	+ $policy) {
	+
	+ $xactions = array();
	+
	+ $xactions[] = id(new PhabricatorProjectTransaction())
	+ ->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY)
	+ ->setNewValue($policy);
	+
	+ $this->applyTransactions($project, $user, $xactions);

	return $project;
	}
	@@ -359,13 +442,21 @@
	->setMetadataValue('edge:type', $edge_type)
	->setNewValue($spec);

	+ $this->applyTransactions($project, $user, $xactions);
	+
	+ return $project;
	+ }
	+
	+ private function applyTransactions(
	+ PhabricatorProject $project,
	+ PhabricatorUser $user,
	+ array $xactions) {
	+
	$editor = id(new PhabricatorProjectTransactionEditor())
	->setActor($user)
	->setContentSource(PhabricatorContentSource::newConsoleSource())
	->setContinueOnNoEffect(true)
	->applyTransactions($project, $xactions);
	-
	- return $project;
	}


	diff --git a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
	--- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
	+++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
	@@ -26,6 +26,7 @@
	$types[] = PhabricatorProjectTransaction::TYPE_ICON;
	$types[] = PhabricatorProjectTransaction::TYPE_COLOR;
	$types[] = PhabricatorProjectTransaction::TYPE_LOCKED;
	+ $types[] = PhabricatorProjectTransaction::TYPE_PARENT;

	return $types;
	}
	@@ -52,6 +53,8 @@
	return $object->getColor();
	case PhabricatorProjectTransaction::TYPE_LOCKED:
	return (int)$object->getIsMembershipLocked();
	+ case PhabricatorProjectTransaction::TYPE_PARENT:
	+ return null;
	}

	return parent::getCustomTransactionOldValue($object, $xaction);
	@@ -69,6 +72,7 @@
	case PhabricatorProjectTransaction::TYPE_ICON:
	case PhabricatorProjectTransaction::TYPE_COLOR:
	case PhabricatorProjectTransaction::TYPE_LOCKED:
	+ case PhabricatorProjectTransaction::TYPE_PARENT:
	return $xaction->getNewValue();
	}

	@@ -102,6 +106,9 @@
	case PhabricatorProjectTransaction::TYPE_LOCKED:
	$object->setIsMembershipLocked($xaction->getNewValue());
	return;
	+ case PhabricatorProjectTransaction::TYPE_PARENT:
	+ $object->setParentProjectPHID($xaction->getNewValue());
	+ return;
	}

	return parent::applyCustomInternalTransaction($object, $xaction);
	@@ -153,6 +160,7 @@
	case PhabricatorProjectTransaction::TYPE_ICON:
	case PhabricatorProjectTransaction::TYPE_COLOR:
	case PhabricatorProjectTransaction::TYPE_LOCKED:
	+ case PhabricatorProjectTransaction::TYPE_PARENT:
	return;
	}

	@@ -324,7 +332,77 @@
	}

	break;
	+ case PhabricatorProjectTransaction::TYPE_PARENT:
	+ if (!$xactions) {
	+ break;
	+ }
	+
	+ $xaction = last($xactions);
	+
	+ if (!$this->getIsNewObject()) {
	+ $errors[] = new PhabricatorApplicationTransactionValidationError(
	+ $type,
	+ pht('Invalid'),
	+ pht(
	+ 'You can only set a parent project when creating a project '.
	+ 'for the first time.'),
	+ $xaction);
	+ break;
	+ }

	+ $parent_phid = $xaction->getNewValue();
	+
	+ $projects = id(new PhabricatorProjectQuery())
	+ ->setViewer($this->requireActor())
	+ ->withPHIDs(array($parent_phid))
	+ ->requireCapabilities(
	+ array(
	+ PhabricatorPolicyCapability::CAN_VIEW,
	+ PhabricatorPolicyCapability::CAN_EDIT,
	+ ))
	+ ->execute();
	+ if (!$projects) {
	+ $errors[] = new PhabricatorApplicationTransactionValidationError(
	+ $type,
	+ pht('Invalid'),
	+ pht(
	+ 'Parent project PHID ("%s") must be the PHID of a valid, '.
	+ 'visible project which you have permission to edit.',
	+ $parent_phid),
	+ $xaction);
	+ break;
	+ }
	+
	+ $project = head($projects);
	+
	+ if ($project->isMilestone()) {
	+ $errors[] = new PhabricatorApplicationTransactionValidationError(
	+ $type,
	+ pht('Invalid'),
	+ pht(
	+ 'Parent project PHID ("%s") must not be a milestone. '.
	+ 'Milestones may not have subprojects.',
	+ $parent_phid),
	+ $xaction);
	+ break;
	+ }
	+
	+ $limit = PhabricatorProject::getProjectDepthLimit();
	+ if ($project->getProjectDepth() >= ($limit - 1)) {
	+ $errors[] = new PhabricatorApplicationTransactionValidationError(
	+ $type,
	+ pht('Invalid'),
	+ pht(
	+ 'You can not create a subproject under this parent because '.
	+ 'it would nest projects too deeply. The maximum nesting '.
	+ 'depth of projects is %s.',
	+ new PhutilNumber($limit)),
	+ $xaction);
	+ break;
	+ }
	+
	+ $object->attachParentProject($project);
	+ break;
	}

	return $errors;
	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
	@@ -130,6 +130,27 @@
	}

	protected function willFilterPage(array $projects) {
	+ $project_phids = array();
	+ $ancestor_paths = array();
	+
	+ foreach ($projects as $project) {
	+ $project_phids[] = $project->getPHID();
	+
	+ foreach ($project->getAncestorProjectPaths() as $path) {
	+ $ancestor_paths[$path] = $path;
	+ }
	+ }
	+
	+ if ($ancestor_paths) {
	+ $ancestors = id(new PhabricatorProject())->loadAllWhere(
	+ 'projectPath IN (%Ls)',
	+ $ancestor_paths);
	+ } else {
	+ $ancestors = array();
	+ }
	+
	+ $projects = $this->linkProjectGraph($projects, $ancestors);
	+
	$viewer_phid = $this->getViewer()->getPHID();
	$project_phids = mpull($projects, 'getPHID');

	@@ -154,6 +175,7 @@

	$edge_query->execute();

	+ $membership_projects = array();
	foreach ($projects as $project) {
	$project_phid = $project->getPHID();

	@@ -161,9 +183,9 @@
	array($project_phid),
	array($member_type));

	- $project->setIsUserMember(
	- $viewer_phid,
	- in_array($viewer_phid, $member_phids));
	+ if (in_array($viewer_phid, $member_phids)) {
	+ $membership_projects[$project_phid] = $project;
	+ }

	if ($this->needMembers) {
	$project->attachMemberPHIDs($member_phids);
	@@ -180,6 +202,14 @@
	}
	}

	+ $all_graph = $this->getAllReachableAncestors($projects);
	+ $member_graph = $this->getAllReachableAncestors($membership_projects);
	+
	+ foreach ($all_graph as $phid => $project) {
	+ $is_member = isset($member_graph[$phid]);
	+ $project->setIsUserMember($viewer_phid, $is_member);
	+ }
	+
	return $projects;
	}

	@@ -360,4 +390,88 @@
	return 'p';
	}

	+ private function linkProjectGraph(array $projects, array $ancestors) {
	+ $ancestor_map = mpull($ancestors, null, 'getPHID');
	+ $projects_map = mpull($projects, null, 'getPHID');
	+
	+ $all_map = $projects_map + $ancestor_map;
	+
	+ $done = array();
	+ foreach ($projects as $key => $project) {
	+ $seen = array($project->getPHID() => true);
	+
	+ if (!$this->linkProject($project, $all_map, $done, $seen)) {
	+ $this->didRejectResult($project);
	+ unset($projects[$key]);
	+ continue;
	+ }
	+
	+ foreach ($project->getAncestorProjects() as $ancestor) {
	+ $seen[$ancestor->getPHID()] = true;
	+ }
	+ }
	+
	+ return $projects;
	+ }
	+
	+ private function linkProject($project, array $all, array $done, array $seen) {
	+ $parent_phid = $project->getParentProjectPHID();
	+
	+ // This project has no parent, so just attach `null` and return.
	+ if (!$parent_phid) {
	+ $project->attachParentProject(null);
	+ return true;
	+ }
	+
	+ // This project has a parent, but it failed to load.
	+ if (empty($all[$parent_phid])) {
	+ return false;
	+ }
	+
	+ // Test for graph cycles. If we encounter one, we're going to hide the
	+ // entire cycle since we can't meaningfully resolve it.
	+ if (isset($seen[$parent_phid])) {
	+ return false;
	+ }
	+
	+ $seen[$parent_phid] = true;
	+
	+ $parent = $all[$parent_phid];
	+ $project->attachParentProject($parent);
	+
	+ if (!empty($done[$parent_phid])) {
	+ return true;
	+ }
	+
	+ return $this->linkProject($parent, $all, $done, $seen);
	+ }
	+
	+ private function getAllReachableAncestors(array $projects) {
	+ $ancestors = array();
	+
	+ $seen = mpull($projects, null, 'getPHID');
	+
	+ $stack = $projects;
	+ while ($stack) {
	+ $project = array_pop($stack);
	+
	+ $phid = $project->getPHID();
	+ $ancestors[$phid] = $project;
	+
	+ $parent_phid = $project->getParentProjectPHID();
	+ if (!$parent_phid) {
	+ continue;
	+ }
	+
	+ if (isset($seen[$parent_phid])) {
	+ continue;
	+ }
	+
	+ $seen[$parent_phid] = true;
	+ $stack[] = $project->getParentProject();
	+ }
	+
	+ return $ancestors;
	+ }
	+
	}
	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
	@@ -5,6 +5,7 @@
	PhabricatorApplicationTransactionInterface,
	PhabricatorFlaggableInterface,
	PhabricatorPolicyInterface,
	+ PhabricatorExtendedPolicyInterface,
	PhabricatorSubscribableInterface,
	PhabricatorCustomFieldInterface,
	PhabricatorDestructibleInterface,
	@@ -30,6 +31,9 @@
	protected $hasSubprojects;
	protected $milestoneNumber;

	+ protected $projectPath;
	+ protected $projectDepth;
	+
	private $memberPHIDs = self::ATTACHABLE;
	private $watcherPHIDs = self::ATTACHABLE;
	private $sparseWatchers = self::ATTACHABLE;
	@@ -69,7 +73,8 @@
	->attachSlugs(array())
	->setHasWorkboard(0)
	->setHasMilestones(0)
	- ->setHasSubprojects(0);
	+ ->setHasSubprojects(0)
	+ ->attachParentProject(null);
	}

	public function getCapabilities() {
	@@ -92,6 +97,7 @@
	}

	public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
	+ $can_edit = PhabricatorPolicyCapability::CAN_EDIT;

	switch ($capability) {
	case PhabricatorPolicyCapability::CAN_VIEW:
	@@ -101,9 +107,18 @@
	}
	break;
	case PhabricatorPolicyCapability::CAN_EDIT:
	+ $parent = $this->getParentProject();
	+ if ($parent) {
	+ $can_edit_parent = PhabricatorPolicyFilter::hasCapability(
	+ $viewer,
	+ $parent,
	+ $can_edit);
	+ if ($can_edit_parent) {
	+ return true;
	+ }
	+ }
	break;
	case PhabricatorPolicyCapability::CAN_JOIN:
	- $can_edit = PhabricatorPolicyCapability::CAN_EDIT;
	if (PhabricatorPolicyFilter::hasCapability($viewer, $this, $can_edit)) {
	// Project editors can always join a project.
	return true;
	@@ -115,6 +130,9 @@
	}

	public function describeAutomaticCapability($capability) {
	+
	+ // TODO: Clarify the additional rules that parent and subprojects imply.
	+
	switch ($capability) {
	case PhabricatorPolicyCapability::CAN_VIEW:
	return pht('Members of a project can always view it.');
	@@ -124,6 +142,25 @@
	return null;
	}

	+ public function getExtendedPolicy($capability, PhabricatorUser $viewer) {
	+ $extended = array();
	+
	+ switch ($capability) {
	+ case PhabricatorPolicyCapability::CAN_VIEW:
	+ $parent = $this->getParentProject();
	+ if ($parent) {
	+ $extended[] = array(
	+ $parent,
	+ PhabricatorPolicyCapability::CAN_VIEW,
	+ );
	+ }
	+ break;
	+ }
	+
	+ return $extended;
	+ }
	+
	+
	public function isUserMember($user_phid) {
	if ($this->memberPHIDs !== self::ATTACHABLE) {
	return in_array($user_phid, $this->memberPHIDs);
	@@ -157,6 +194,8 @@
	'hasMilestones' => 'bool',
	'hasSubprojects' => 'bool',
	'milestoneNumber' => 'uint32?',
	+ 'projectPath' => 'hashpath64',
	+ 'projectDepth' => 'uint32',
	),
	self::CONFIG_KEY_SCHEMA => array(
	'key_phid' => null,
	@@ -182,6 +221,9 @@
	'columns' => array('primarySlug'),
	'unique' => true,
	),
	+ 'key_path' => array(
	+ 'columns' => array('projectPath', 'projectDepth'),
	+ ),
	),
	) + parent::getConfiguration();
	}
	@@ -264,6 +306,31 @@
	$this->setMailKey(Filesystem::readRandomCharacters(20));
	}

	+ if (!strlen($this->getPHID())) {
	+ $this->setPHID($this->generatePHID());
	+ }
	+
	+ $path = array();
	+ $depth = 0;
	+ if ($this->parentProjectPHID) {
	+ $parent = $this->getParentProject();
	+ $path[] = $parent->getProjectPath();
	+ $depth = $parent->getProjectDepth() + 1;
	+ }
	+ $hash = PhabricatorHash::digestForIndex($this->getPHID());
	+ $path[] = substr($hash, 0, 4);
	+
	+ $path = implode('', $path);
	+
	+ $limit = self::getProjectDepthLimit();
	+ if (strlen($path) > ($limit * 4)) {
	+ throw new Exception(
	+ pht('Unable to save project: path length is too long.'));
	+ }
	+
	+ $this->setProjectPath($path);
	+ $this->setProjectDepth($depth);
	+
	$this->openTransaction();
	$result = parent::save();
	$this->updateDatasourceTokens();
	@@ -272,6 +339,12 @@
	return $result;
	}

	+ public static function getProjectDepthLimit() {
	+ // This is limited by how many path hashes we can fit in the path
	+ // column.
	+ return 16;
	+ }
	+
	public function updateDatasourceTokens() {
	$table = self::TABLE_DATASOURCE_TOKEN;
	$conn_w = $this->establishConnection('w');
	@@ -319,11 +392,36 @@
	return $this->assertAttached($this->parentProject);
	}

	- public function attachParentProject(PhabricatorProject $project) {
	+ public function attachParentProject(PhabricatorProject $project = null) {
	$this->parentProject = $project;
	return $this;
	}

	+ public function getAncestorProjectPaths() {
	+ $parts = array();
	+
	+ $path = $this->getProjectPath();
	+ $parent_length = (strlen($path) - 4);
	+
	+ for ($ii = $parent_length; $ii >= 0; $ii -= 4) {
	+ $parts[] = substr($path, 0, $ii);
	+ }
	+
	+ return $parts;
	+ }
	+
	+ public function getAncestorProjects() {
	+ $ancestors = array();
	+
	+ $cursor = $this->getParentProject();
	+ while ($cursor) {
	+ $ancestors[] = $cursor;
	+ $cursor = $cursor->getParentProject();
	+ }
	+
	+ return $ancestors;
	+ }
	+

	/* -( PhabricatorSubscribableInterface )----------------------------------- */

	diff --git a/src/applications/project/storage/PhabricatorProjectTransaction.php b/src/applications/project/storage/PhabricatorProjectTransaction.php
	--- a/src/applications/project/storage/PhabricatorProjectTransaction.php
	+++ b/src/applications/project/storage/PhabricatorProjectTransaction.php
	@@ -10,6 +10,7 @@
	const TYPE_ICON = 'project:icon';
	const TYPE_COLOR = 'project:color';
	const TYPE_LOCKED = 'project:locked';
	+ const TYPE_PARENT = 'project:parent';

	// NOTE: This is deprecated, members are just a normal edge now.
	const TYPE_MEMBERS = 'project:members';

File Metadata

Mime Type: text/plain
Expires: Sun, Mar 30, 9:16 AM (3 w, 4 d ago)
Storage Engine: blob
Storage Format: Encrypted (AES-256-CBC)
Storage Handle: 7234966
Default Alt Text: D14861.id35938.diff (20 KB)

D14861.id35938.diffNo OneTemporaryActions

D14861.id35938.diffView Options

File Metadata

Event Timeline

D14861.id35938.diff
No OneTemporary
Actions

D14861.id35938.diff
View Options