diff --git a/resources/sql/autopatches/20151223.proj.01.paths.sql b/resources/sql/autopatches/20151223.proj.01.paths.sql new file mode 100644 index 0000000000..724a2ff324 --- /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 index 0000000000..a2d3d238b7 --- /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 index 0000000000..875bf2675c --- /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 index 4f583f4b4a..d5c72b501d 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -7131,6 +7131,7 @@ phutil_register_library_map(array( 'PhabricatorApplicationTransactionInterface', 'PhabricatorFlaggableInterface', 'PhabricatorPolicyInterface', + 'PhabricatorExtendedPolicyInterface', 'PhabricatorSubscribableInterface', 'PhabricatorCustomFieldInterface', 'PhabricatorDestructibleInterface', diff --git a/src/applications/config/schema/PhabricatorConfigSchemaSpec.php b/src/applications/config/schema/PhabricatorConfigSchemaSpec.php index 9ed2c87dce..5b48fcbd03 100644 --- a/src/applications/config/schema/PhabricatorConfigSchemaSpec.php +++ b/src/applications/config/schema/PhabricatorConfigSchemaSpec.php @@ -321,6 +321,7 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject { 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 index fd42902090..d15878f74d 100644 --- a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php +++ b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php @@ -113,6 +113,59 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase { $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 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase { $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 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase { } } - 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 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase { ->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 index 7813195b02..7fc7ba0efb 100644 --- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php +++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php @@ -26,6 +26,7 @@ final class PhabricatorProjectTransactionEditor $types[] = PhabricatorProjectTransaction::TYPE_ICON; $types[] = PhabricatorProjectTransaction::TYPE_COLOR; $types[] = PhabricatorProjectTransaction::TYPE_LOCKED; + $types[] = PhabricatorProjectTransaction::TYPE_PARENT; return $types; } @@ -52,6 +53,8 @@ final class PhabricatorProjectTransactionEditor 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 @@ final class PhabricatorProjectTransactionEditor case PhabricatorProjectTransaction::TYPE_ICON: case PhabricatorProjectTransaction::TYPE_COLOR: case PhabricatorProjectTransaction::TYPE_LOCKED: + case PhabricatorProjectTransaction::TYPE_PARENT: return $xaction->getNewValue(); } @@ -102,6 +106,9 @@ final class PhabricatorProjectTransactionEditor 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 @@ final class PhabricatorProjectTransactionEditor case PhabricatorProjectTransaction::TYPE_ICON: case PhabricatorProjectTransaction::TYPE_COLOR: case PhabricatorProjectTransaction::TYPE_LOCKED: + case PhabricatorProjectTransaction::TYPE_PARENT: return; } @@ -324,7 +332,77 @@ final class PhabricatorProjectTransactionEditor } 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 index f1ccad105c..f540f25e7e 100644 --- a/src/applications/project/query/PhabricatorProjectQuery.php +++ b/src/applications/project/query/PhabricatorProjectQuery.php @@ -130,6 +130,27 @@ final class PhabricatorProjectQuery } 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 @@ final class PhabricatorProjectQuery $edge_query->execute(); + $membership_projects = array(); foreach ($projects as $project) { $project_phid = $project->getPHID(); @@ -161,9 +183,9 @@ final class PhabricatorProjectQuery 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 @@ final class PhabricatorProjectQuery } } + $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 @@ final class PhabricatorProjectQuery 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 index ba9d98c8d3..cd5eaff067 100644 --- a/src/applications/project/storage/PhabricatorProject.php +++ b/src/applications/project/storage/PhabricatorProject.php @@ -5,6 +5,7 @@ final class PhabricatorProject extends PhabricatorProjectDAO PhabricatorApplicationTransactionInterface, PhabricatorFlaggableInterface, PhabricatorPolicyInterface, + PhabricatorExtendedPolicyInterface, PhabricatorSubscribableInterface, PhabricatorCustomFieldInterface, PhabricatorDestructibleInterface, @@ -30,6 +31,9 @@ final class PhabricatorProject extends PhabricatorProjectDAO protected $hasSubprojects; protected $milestoneNumber; + protected $projectPath; + protected $projectDepth; + private $memberPHIDs = self::ATTACHABLE; private $watcherPHIDs = self::ATTACHABLE; private $sparseWatchers = self::ATTACHABLE; @@ -69,7 +73,8 @@ final class PhabricatorProject extends PhabricatorProjectDAO ->attachSlugs(array()) ->setHasWorkboard(0) ->setHasMilestones(0) - ->setHasSubprojects(0); + ->setHasSubprojects(0) + ->attachParentProject(null); } public function getCapabilities() { @@ -92,6 +97,7 @@ final class PhabricatorProject extends PhabricatorProjectDAO } public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + $can_edit = PhabricatorPolicyCapability::CAN_EDIT; switch ($capability) { case PhabricatorPolicyCapability::CAN_VIEW: @@ -101,9 +107,18 @@ final class PhabricatorProject extends PhabricatorProjectDAO } 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 @@ final class PhabricatorProject extends PhabricatorProjectDAO } 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 @@ final class PhabricatorProject extends PhabricatorProjectDAO 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 @@ final class PhabricatorProject extends PhabricatorProjectDAO 'hasMilestones' => 'bool', 'hasSubprojects' => 'bool', 'milestoneNumber' => 'uint32?', + 'projectPath' => 'hashpath64', + 'projectDepth' => 'uint32', ), self::CONFIG_KEY_SCHEMA => array( 'key_phid' => null, @@ -182,6 +221,9 @@ final class PhabricatorProject extends PhabricatorProjectDAO 'columns' => array('primarySlug'), 'unique' => true, ), + 'key_path' => array( + 'columns' => array('projectPath', 'projectDepth'), + ), ), ) + parent::getConfiguration(); } @@ -264,6 +306,31 @@ final class PhabricatorProject extends PhabricatorProjectDAO $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 @@ final class PhabricatorProject extends PhabricatorProjectDAO 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 @@ final class PhabricatorProject extends PhabricatorProjectDAO 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 index 6529c99d59..f22c763da1 100644 --- a/src/applications/project/storage/PhabricatorProjectTransaction.php +++ b/src/applications/project/storage/PhabricatorProjectTransaction.php @@ -10,6 +10,7 @@ final class PhabricatorProjectTransaction 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';