From f9fcaa1f8447073cb24c0804255ff7b3dc4aa718 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 7 Aug 2012 18:02:05 -0700 Subject: [PATCH] Migrate project membership to edges Summary: - Store project members in edges. - Migrate existing members to edge storage. - Delete PhabricatorProjectAffiliation. - I left the actual underlying data around just in case something goes wrong; we can delete it evenutally. Test Plan: - Ran migration. - Created a new project. - Joined and left a project. - Added and removed project members. - Manually called PhabricatorOwnersOwner::loadAffiliatedUserPHIDs() to verify its behavior. Reviewers: vrana, btrahan Reviewed By: vrana CC: aran Maniphest Tasks: T603 Differential Revision: https://secure.phabricator.com/D3186 --- .../sql/patches/migrate-project-edges.php | 49 +++++++++++ src/__phutil_library_map__.php | 2 - .../owners/storage/PhabricatorOwnersOwner.php | 13 +-- .../PhabricatorProjectCreateController.php | 14 ++-- .../PhabricatorProjectListController.php | 13 +-- .../editor/PhabricatorProjectEditor.php | 66 ++++++--------- .../project/query/PhabricatorProjectQuery.php | 82 +++++++++---------- .../project/storage/PhabricatorProject.php | 41 ++++------ .../storage/PhabricatorProjectAffiliation.php | 39 --------- .../edges/constants/PhabricatorEdgeConfig.php | 6 ++ .../patch/PhabricatorBuiltinPatchList.php | 4 + 11 files changed, 164 insertions(+), 165 deletions(-) create mode 100644 resources/sql/patches/migrate-project-edges.php delete mode 100644 src/applications/project/storage/PhabricatorProjectAffiliation.php diff --git a/resources/sql/patches/migrate-project-edges.php b/resources/sql/patches/migrate-project-edges.php new file mode 100644 index 0000000000..50a360a86a --- /dev/null +++ b/resources/sql/patches/migrate-project-edges.php @@ -0,0 +1,49 @@ +getID(); + echo "Project {$id}: "; + + $members = queryfx_all( + $proj->establishConnection('r'), + 'SELECT userPHID FROM %T WHERE projectPHID = %s', + 'project_affiliation', + $proj->getPHID()); + + if (!$members) { + echo "-\n"; + continue; + } + + $members = ipull($members, 'userPHID'); + + $editor = new PhabricatorEdgeEditor(); + $editor->setSuppressEvents(true); + foreach ($members as $user_phid) { + $editor->addEdge( + $proj->getPHID(), + PhabricatorEdgeConfig::TYPE_PROJ_MEMBER, + $user_phid); + } + $editor->save(); + echo "OKAY\n"; +} + +echo "Done.\n"; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4ee957cb1f..0f89c0148f 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -894,7 +894,6 @@ phutil_register_library_map(array( 'PhabricatorPolicyTestQuery' => 'applications/policy/__tests__/PhabricatorPolicyTestQuery.php', 'PhabricatorProfileHeaderView' => 'view/layout/PhabricatorProfileHeaderView.php', 'PhabricatorProject' => 'applications/project/storage/PhabricatorProject.php', - 'PhabricatorProjectAffiliation' => 'applications/project/storage/PhabricatorProjectAffiliation.php', 'PhabricatorProjectConstants' => 'applications/project/constants/PhabricatorProjectConstants.php', 'PhabricatorProjectController' => 'applications/project/controller/PhabricatorProjectController.php', 'PhabricatorProjectCreateController' => 'applications/project/controller/PhabricatorProjectCreateController.php', @@ -1933,7 +1932,6 @@ phutil_register_library_map(array( 'PhabricatorPolicyTestQuery' => 'PhabricatorPolicyQuery', 'PhabricatorProfileHeaderView' => 'AphrontView', 'PhabricatorProject' => 'PhabricatorProjectDAO', - 'PhabricatorProjectAffiliation' => 'PhabricatorProjectDAO', 'PhabricatorProjectController' => 'PhabricatorController', 'PhabricatorProjectCreateController' => 'PhabricatorProjectController', 'PhabricatorProjectDAO' => 'PhabricatorLiskDAO', diff --git a/src/applications/owners/storage/PhabricatorOwnersOwner.php b/src/applications/owners/storage/PhabricatorOwnersOwner.php index 72f35a2b06..39e6b50c49 100644 --- a/src/applications/owners/storage/PhabricatorOwnersOwner.php +++ b/src/applications/owners/storage/PhabricatorOwnersOwner.php @@ -58,12 +58,13 @@ final class PhabricatorOwnersOwner extends PhabricatorOwnersDAO { array()); $users_in_project_phids = array(); - if (idx($all_phids, PhabricatorPHIDConstants::PHID_TYPE_PROJ)) { - $users_in_project_phids = mpull( - id(new PhabricatorProjectAffiliation())->loadAllWhere( - 'projectPHID IN (%Ls)', - idx($all_phids, PhabricatorPHIDConstants::PHID_TYPE_PROJ, array())), - 'getUserPHID'); + $project_phids = idx($all_phids, PhabricatorPHIDConstants::PHID_TYPE_PROJ); + if ($project_phids) { + $query = id(new PhabricatorEdgeQuery()) + ->withSourcePHIDs($project_phids) + ->withEdgeTypes(array(PhabricatorEdgeConfig::TYPE_PROJ_MEMBER)); + $query->execute(); + $users_in_project_phids = $query->getDestinationPHIDs(); } return array_unique(array_merge($users_in_project_phids, $user_phids)); diff --git a/src/applications/project/controller/PhabricatorProjectCreateController.php b/src/applications/project/controller/PhabricatorProjectCreateController.php index e2a2ebe21e..e3e73efd34 100644 --- a/src/applications/project/controller/PhabricatorProjectCreateController.php +++ b/src/applications/project/controller/PhabricatorProjectCreateController.php @@ -35,12 +35,19 @@ final class PhabricatorProjectCreateController try { $xactions = array(); + $xaction = new PhabricatorProjectTransaction(); $xaction->setTransactionType( PhabricatorProjectTransactionType::TYPE_NAME); $xaction->setNewValue($request->getStr('name')); $xactions[] = $xaction; + $xaction = new PhabricatorProjectTransaction(); + $xaction->setTransactionType( + PhabricatorProjectTransactionType::TYPE_MEMBERS); + $xaction->setNewValue(array($user->getPHID())); + $xactions[] = $xaction; + $editor = new PhabricatorProjectEditor($project); $editor->setUser($user); $editor->applyTransactions($xactions); @@ -56,13 +63,6 @@ final class PhabricatorProjectCreateController $profile->setProjectPHID($project->getPHID()); $profile->save(); - id(new PhabricatorProjectAffiliation()) - ->setUserPHID($user->getPHID()) - ->setProjectPHID($project->getPHID()) - ->setRole('Owner') - ->setIsOwner(true) - ->save(); - if ($request->isAjax()) { return id(new AphrontAjaxResponse()) ->setContent(array( diff --git a/src/applications/project/controller/PhabricatorProjectListController.php b/src/applications/project/controller/PhabricatorProjectListController.php index 78536e9c33..fc3a0d7784 100644 --- a/src/applications/project/controller/PhabricatorProjectListController.php +++ b/src/applications/project/controller/PhabricatorProjectListController.php @@ -81,10 +81,12 @@ final class PhabricatorProjectListController $profiles = mpull($profiles, null, 'getProjectPHID'); } - $affil_groups = array(); + $edge_query = new PhabricatorEdgeQuery(); if ($projects) { - $affil_groups = PhabricatorProjectAffiliation::loadAllForProjectPHIDs( - $project_phids); + $edge_query + ->withSourcePHIDs($project_phids) + ->withEdgeTypes(array(PhabricatorEdgeConfig::TYPE_PROJ_MEMBER)) + ->execute(); } $tasks = array(); @@ -104,18 +106,17 @@ final class PhabricatorProjectListController } } - $rows = array(); foreach ($projects as $project) { $phid = $project->getPHID(); $profile = idx($profiles, $phid); - $affiliations = $affil_groups[$phid]; + $members = $edge_query->getDestinationPHIDs(array($phid)); $group = idx($groups, $phid, array()); $task_count = count($group); - $population = count($affiliations); + $population = count($members); if ($profile) { $blurb = $profile->getBlurb(); diff --git a/src/applications/project/editor/PhabricatorProjectEditor.php b/src/applications/project/editor/PhabricatorProjectEditor.php index 63190d6cee..86c39d0a98 100644 --- a/src/applications/project/editor/PhabricatorProjectEditor.php +++ b/src/applications/project/editor/PhabricatorProjectEditor.php @@ -22,8 +22,8 @@ final class PhabricatorProjectEditor { private $user; private $projectName; - private $addAffiliations; - private $remAffiliations; + private $addEdges = array(); + private $remEdges = array(); public function __construct(PhabricatorProject $project) { $this->project = $project; @@ -67,22 +67,26 @@ final class PhabricatorProjectEditor { } try { - $project->save(); + $project->openTransaction(); + $project->save(); - foreach ($transactions as $xaction) { - $xaction->setAuthorPHID($user->getPHID()); - $xaction->setProjectID($project->getID()); - $xaction->save(); - } + $edge_type = PhabricatorEdgeConfig::TYPE_PROJ_MEMBER; + $editor = new PhabricatorEdgeEditor(); + $editor->setUser($this->user); + foreach ($this->remEdges as $phid) { + $editor->removeEdge($project->getPHID(), $edge_type, $phid); + } + foreach ($this->addEdges as $phid) { + $editor->addEdge($project->getPHID(), $edge_type, $phid); + } + $editor->save(); - foreach ($this->remAffiliations as $affil) { - $affil->delete(); - } - - foreach ($this->addAffiliations as $affil) { - $affil->setProjectPHID($project->getPHID()); - $affil->save(); - } + foreach ($transactions as $xaction) { + $xaction->setAuthorPHID($user->getPHID()); + $xaction->setProjectID($project->getID()); + $xaction->save(); + } + $project->saveTransaction(); foreach ($transactions as $xaction) { $this->publishTransactionStory($project, $xaction); @@ -142,11 +146,10 @@ final class PhabricatorProjectEditor { $xaction->setOldValue($project->getStatus()); break; case PhabricatorProjectTransactionType::TYPE_MEMBERS: - $affils = $project->loadAffiliations(); - $project->attachAffiliations($affils); + $member_phids = $project->loadMemberPHIDs(); + $project->attachMemberPHIDs($member_phids); - $old_value = mpull($affils, 'getUserPHID'); - $old_value = array_values($old_value); + $old_value = array_values($member_phids); $xaction->setOldValue($old_value); $new_value = $xaction->getNewValue(); @@ -178,27 +181,8 @@ final class PhabricatorProjectEditor { case PhabricatorProjectTransactionType::TYPE_MEMBERS: $old = array_fill_keys($xaction->getOldValue(), true); $new = array_fill_keys($xaction->getNewValue(), true); - - $add = array(); - $rem = array(); - - foreach ($project->getAffiliations() as $affil) { - if (empty($new[$affil->getUserPHID()])) { - $rem[] = $affil; - } - } - - foreach ($new as $phid => $ignored) { - if (empty($old[$phid])) { - $affil = new PhabricatorProjectAffiliation(); - $affil->setRole(''); - $affil->setUserPHID($phid); - $add[] = $affil; - } - } - - $this->addAffiliations = $add; - $this->remAffiliations = $rem; + $this->addEdges = array_keys(array_diff_key($new, $old)); + $this->remEdges = array_keys(array_diff_key($old, $new)); break; default: throw new Exception("Unknown transaction type '{$type}'!"); diff --git a/src/applications/project/query/PhabricatorProjectQuery.php b/src/applications/project/query/PhabricatorProjectQuery.php index 4a87a723ca..4a5c3dd7bd 100644 --- a/src/applications/project/query/PhabricatorProjectQuery.php +++ b/src/applications/project/query/PhabricatorProjectQuery.php @@ -60,27 +60,26 @@ final class PhabricatorProjectQuery extends PhabricatorOffsetPagedQuery { $table = new PhabricatorProject(); $conn_r = $table->establishConnection('r'); - $where = $this->buildWhereClause($conn_r); - $joins = $this->buildJoinsClause($conn_r); - $order = 'ORDER BY name'; - $data = queryfx_all( $conn_r, - 'SELECT p.* FROM %T p %Q %Q %Q %Q', + 'SELECT p.* FROM %T p %Q %Q %Q %Q %Q', $table->getTableName(), - $joins, - $where, - $order, + $this->buildJoinClause($conn_r), + $this->buildWhereClause($conn_r), + $this->buildGroupClause($conn_r), + 'ORDER BY name', $this->buildLimitClause($conn_r)); $projects = $table->loadAllFromArray($data); if ($projects && $this->needMembers) { - $members = PhabricatorProjectAffiliation::loadAllForProjectPHIDs( - mpull($projects, 'getPHID')); + $members = id(new PhabricatorEdgeQuery()) + ->withSourcePHIDs(mpull($projects, 'getPHID')) + ->withTypes(array(PhabricatorEdgeConfig::TYPE_PROJ_MEMBER)) + ->execute(); foreach ($projects as $project) { - $project->attachAffiliations( - array_values(idx($members, $project->getPHID(), array()))); + $phid = $project->getPHID(); + $project->attachMemberPHIDs(array_keys($members[$phid])); } } @@ -93,37 +92,25 @@ final class PhabricatorProjectQuery extends PhabricatorOffsetPagedQuery { if ($this->status != self::STATUS_ANY) { switch ($this->status) { case self::STATUS_OPEN: - $where[] = qsprintf( - $conn_r, - 'status IN (%Ld)', - array( - PhabricatorProjectStatus::STATUS_ACTIVE, - )); + case self::STATUS_ACTIVE: + $filter = array( + PhabricatorProjectStatus::STATUS_ACTIVE, + ); break; case self::STATUS_CLOSED: - $where[] = qsprintf( - $conn_r, - 'status IN (%Ld)', - array( - PhabricatorProjectStatus::STATUS_ARCHIVED, - )); - break; - case self::STATUS_ACTIVE: - $where[] = qsprintf( - $conn_r, - 'status = %d', - PhabricatorProjectStatus::STATUS_ACTIVE); - break; case self::STATUS_ARCHIVED: - $where[] = qsprintf( - $conn_r, - 'status = %d', - PhabricatorProjectStatus::STATUS_ARCHIVED); + $filter = array( + PhabricatorProjectStatus::STATUS_ARCHIVED, + ); break; default: throw new Exception( "Unknown project status '{$this->status}'!"); } + $where[] = qsprintf( + $conn_r, + 'status IN (%Ld)', + $filter); } if ($this->ids) { @@ -140,20 +127,33 @@ final class PhabricatorProjectQuery extends PhabricatorOffsetPagedQuery { $this->phids); } + if ($this->memberPHIDs) { + $where[] = qsprintf( + $conn_r, + 'e.type = %s AND e.dst IN (%Ls)', + PhabricatorEdgeConfig::TYPE_PROJ_MEMBER, + $this->memberPHIDs); + } + return $this->formatWhereClause($where); } - private function buildJoinsClause($conn_r) { - $affil_table = new PhabricatorProjectAffiliation(); + private function buildGroupClause($conn_r) { + if ($this->memberPHIDs) { + return 'GROUP BY p.id'; + } else { + return ''; + } + } + private function buildJoinClause($conn_r) { $joins = array(); + if ($this->memberPHIDs) { $joins[] = qsprintf( $conn_r, - 'JOIN %T member ON member.projectPHID = p.phid - AND member.userPHID in (%Ls)', - $affil_table->getTableName(), - $this->memberPHIDs); + 'JOIN %T e ON e.src = p.phid', + PhabricatorEdgeConfig::TABLE_NAME_EDGE); } return implode(' ', $joins); diff --git a/src/applications/project/storage/PhabricatorProject.php b/src/applications/project/storage/PhabricatorProject.php index 92269a86a5..75ea187a9d 100644 --- a/src/applications/project/storage/PhabricatorProject.php +++ b/src/applications/project/storage/PhabricatorProject.php @@ -25,7 +25,8 @@ final class PhabricatorProject extends PhabricatorProjectDAO { protected $subprojectPHIDs = array(); protected $phrictionSlug; - private $affiliations; + private $subprojectsNeedUpdate; + private $memberPHIDs; public function getConfiguration() { return array( @@ -48,31 +49,25 @@ final class PhabricatorProject extends PhabricatorProjectDAO { return $profile; } - public function getMemberPHIDs() { - return mpull($this->getAffiliations(), 'getUserPHID'); - } - - public function loadMemberPHIDs() { - return mpull($this->loadAffiliations(), 'getUserPHID'); - } - - public function getAffiliations() { - if ($this->affiliations === null) { - throw new Exception('Attach affiliations first!'); - } - return $this->affiliations; - } - - public function attachAffiliations(array $affiliations) { - assert_instances_of($affiliations, 'PhabricatorProjectAffiliation'); - $this->affiliations = $affiliations; + public function attachMemberPHIDs(array $phids) { + $this->memberPHIDs = $phids; return $this; } - public function loadAffiliations() { - $affils = PhabricatorProjectAffiliation::loadAllForProjectPHIDs( - array($this->getPHID())); - return $affils[$this->getPHID()]; + public function getMemberPHIDs() { + if ($this->memberPHIDs === null) { + throw new Exception("Call attachMemberPHIDs() first!"); + } + return $this->memberPHIDs; + } + + public function loadMemberPHIDs() { + if (!$this->getPHID()) { + return array(); + } + return PhabricatorEdgeQuery::loadDestinationPHIDs( + $this->getPHID(), + PhabricatorEdgeConfig::TYPE_PROJ_MEMBER); } public function setPhrictionSlug($slug) { diff --git a/src/applications/project/storage/PhabricatorProjectAffiliation.php b/src/applications/project/storage/PhabricatorProjectAffiliation.php deleted file mode 100644 index b8d67a47d5..0000000000 --- a/src/applications/project/storage/PhabricatorProjectAffiliation.php +++ /dev/null @@ -1,39 +0,0 @@ -loadAllWhere( - 'projectPHID IN (%Ls) ORDER BY dateCreated', - $phids); - - return mgroup($affiliations, 'getProjectPHID') + $default; - } - -} diff --git a/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php b/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php index aaf02a8e7f..33497149b4 100644 --- a/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php +++ b/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php @@ -38,6 +38,9 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { const TYPE_TASK_HAS_RELATED_DREV = 11; const TYPE_DREV_HAS_RELATED_TASK = 12; + const TYPE_PROJ_MEMBER = 13; + const TYPE_MEMBER_OF_PROJ = 14; + const TYPE_TEST_NO_CYCLE = 9000; public static function getInverse($edge_type) { @@ -58,6 +61,9 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { self::TYPE_TASK_HAS_RELATED_DREV => self::TYPE_DREV_HAS_RELATED_TASK, self::TYPE_DREV_HAS_RELATED_TASK => self::TYPE_TASK_HAS_RELATED_DREV, + + self::TYPE_PROJ_MEMBER => self::TYPE_MEMBER_OF_PROJ, + self::TYPE_MEMBER_OF_PROJ => self::TYPE_PROJ_MEMBER, ); return idx($map, $edge_type); diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index 90cc6fdf26..a7473a34a0 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -940,6 +940,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('symbolcontexts.sql'), ), + 'migrate-project-edges.php' => array( + 'type' => 'php', + 'name' => $this->getPatchPath('migrate-project-edges.php'), + ), ); }