From 9f41032693556ee517a60c4a42d0d41bed2820e4 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 14 Aug 2013 09:01:38 -0700 Subject: [PATCH] Remove "cut point commit identifier" from Releeph Summary: Ref T3656. Releeph denormalizes branch cut point identifiers into Branch objects, but this information isn't useful or used for sorting, filtering, or enforcing unique constraints. Instead, derive it via noramlized pathways from the `cutPointCommitPHID`. Test Plan: Ran storage upgrade. Ran `releephwork.getbranch` and `releeph.getbranches`. Grepped for `cutPointCommitIdentifier`. Reviewers: btrahan Reviewed By: btrahan CC: LegNeato, aran Maniphest Tasks: T3656 Differential Revision: https://secure.phabricator.com/D6636 --- .../20130731.releephcutpointidentifier.sql | 2 ++ .../ConduitAPI_releeph_getbranches_Method.php | 2 +- ...onduitAPI_releephwork_getbranch_Method.php | 14 ++++++++----- .../releeph/editor/ReleephBranchEditor.php | 1 - .../releeph/query/ReleephBranchQuery.php | 21 +++++++++++++++++++ .../releeph/storage/ReleephBranch.php | 12 ++++++++++- .../patch/PhabricatorBuiltinPatchList.php | 4 ++++ 7 files changed, 48 insertions(+), 8 deletions(-) create mode 100644 resources/sql/patches/20130731.releephcutpointidentifier.sql diff --git a/resources/sql/patches/20130731.releephcutpointidentifier.sql b/resources/sql/patches/20130731.releephcutpointidentifier.sql new file mode 100644 index 0000000000..a7eb4ff38d --- /dev/null +++ b/resources/sql/patches/20130731.releephcutpointidentifier.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_releeph.releeph_branch + DROP cutPointCommitIdentifier; diff --git a/src/applications/releeph/conduit/ConduitAPI_releeph_getbranches_Method.php b/src/applications/releeph/conduit/ConduitAPI_releeph_getbranches_Method.php index 861e69eb81..982e0e4cc5 100644 --- a/src/applications/releeph/conduit/ConduitAPI_releeph_getbranches_Method.php +++ b/src/applications/releeph/conduit/ConduitAPI_releeph_getbranches_Method.php @@ -52,7 +52,7 @@ final class ConduitAPI_releeph_getbranches_Method 'branch' => $branch->getBasename(), 'fullBranchName' => $full_branch_name, 'symbolicName' => $branch->getSymbolicName(), - 'cutPoint' => $branch->getCutPointCommitIdentifier(), + 'cutPoint' => $cut_point_commit->getCommitIdentifier(), ); } } diff --git a/src/applications/releeph/conduit/work/ConduitAPI_releephwork_getbranch_Method.php b/src/applications/releeph/conduit/work/ConduitAPI_releephwork_getbranch_Method.php index 9b9f57e40d..7cbbf9053c 100644 --- a/src/applications/releeph/conduit/work/ConduitAPI_releephwork_getbranch_Method.php +++ b/src/applications/releeph/conduit/work/ConduitAPI_releephwork_getbranch_Method.php @@ -26,8 +26,11 @@ final class ConduitAPI_releephwork_getbranch_Method } protected function execute(ConduitAPIRequest $request) { - $branch = id(new ReleephBranch()) - ->loadOneWhere('phid = %s', $request->getValue('branchPHID')); + $branch = id(new ReleephBranchQuery()) + ->setViewer($request->getUser()) + ->withPHIDs(array($request->getValue('branchPHID'))) + ->needCutPointCommits(true) + ->executeOne(); $cut_phid = $branch->getCutPointCommitPHID(); $phids = array($cut_phid); @@ -35,14 +38,15 @@ final class ConduitAPI_releephwork_getbranch_Method ->setViewer($request->getUser()) ->loadHandles(); - $project = $branch->loadReleephProject(); - $repo = $project->loadPhabricatorRepository(); + $project = $branch->getProject(); + $repo = $project->getRepository(); + $commit = $branch->getCutPointCommit(); return array( 'branchName' => $branch->getName(), 'branchPHID' => $branch->getPHID(), 'vcsType' => $repo->getVersionControlSystem(), - 'cutCommitID' => $branch->getCutPointCommitIdentifier(), + 'cutCommitID' => $commit->getCommitIdentifier(), 'cutCommitName' => $handles[$cut_phid]->getName(), 'creatorPHID' => $branch->getCreatedByUserPHID(), 'trunk' => $project->getTrunkBranch(), diff --git a/src/applications/releeph/editor/ReleephBranchEditor.php b/src/applications/releeph/editor/ReleephBranchEditor.php index d3f679502e..61fbce06f8 100644 --- a/src/applications/releeph/editor/ReleephBranchEditor.php +++ b/src/applications/releeph/editor/ReleephBranchEditor.php @@ -46,7 +46,6 @@ final class ReleephBranchEditor extends PhabricatorEditor { ->setBasename($basename) ->setReleephProjectID($this->releephProject->getID()) ->setCreatedByUserPHID($this->requireActor()->getPHID()) - ->setCutPointCommitIdentifier($cut_point->getCommitIdentifier()) ->setCutPointCommitPHID($cut_point->getPHID()) ->setIsActive(1) ->setDetail('branchDate', $branch_date) diff --git a/src/applications/releeph/query/ReleephBranchQuery.php b/src/applications/releeph/query/ReleephBranchQuery.php index 82fe8d4e14..727d5a3617 100644 --- a/src/applications/releeph/query/ReleephBranchQuery.php +++ b/src/applications/releeph/query/ReleephBranchQuery.php @@ -6,6 +6,8 @@ final class ReleephBranchQuery private $ids; private $phids; + private $needCutPointCommits; + public function withIDs(array $ids) { $this->ids = $ids; return $this; @@ -16,6 +18,11 @@ final class ReleephBranchQuery return $this; } + public function needCutPointCommits($need_commits) { + $this->needCutPointCommits = $need_commits; + return $this; + } + public function loadPage() { $table = new ReleephBranch(); $conn_r = $table->establishConnection('r'); @@ -48,6 +55,20 @@ final class ReleephBranchQuery } } + if ($this->needCutPointCommits) { + $commit_phids = mpull($branches, 'getCutPointCommitPHID'); + $commits = id(new DiffusionCommitQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($commit_phids) + ->execute(); + $commits = mpull($commits, null, 'getPHID'); + + foreach ($branches as $branch) { + $commit = idx($commits, $branch->getCutPointCommitPHID()); + $branch->attachCutPointCommit($commit); + } + } + return $branches; } diff --git a/src/applications/releeph/storage/ReleephBranch.php b/src/applications/releeph/storage/ReleephBranch.php index 9053b53d91..b5b7a3dc17 100644 --- a/src/applications/releeph/storage/ReleephBranch.php +++ b/src/applications/releeph/storage/ReleephBranch.php @@ -17,12 +17,12 @@ final class ReleephBranch extends ReleephDAO protected $symbolicName; // Where to cut the branch - protected $cutPointCommitIdentifier; protected $cutPointCommitPHID; protected $details = array(); private $project = self::ATTACHABLE; + private $cutPointCommit = self::ATTACHABLE; public function getConfiguration() { return array( @@ -162,6 +162,16 @@ final class ReleephBranch extends ReleephDAO return $this->assertAttached($this->project); } + public function attachCutPointCommit( + PhabricatorRepositoryCommit $commit = null) { + $this->cutPointCommit = $commit; + return $this; + } + + public function getCutPointCommit() { + return $this->assertAttached($this->cutPointCommit); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index f3ca066322..dd8b695eaf 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -1543,6 +1543,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('20130731.releephproject.sql'), ), + '20130731.releephcutpointidentifier.sql' => array( + 'type' => 'sql', + 'name' => $this->getPatchPath('20130731.releephcutpointidentifier.sql'), + ), ); } }