From f4c8a34abe8a0e9b3b37cc1fa143502eb8d93e24 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 14 Apr 2014 12:07:32 -0700 Subject: [PATCH] Remove several "loadArcanistProject()" methods Summary: Ref T3551. Releeph has old-style `loadX()` methods; get rid of one of them. Differential has a couple of copies of this too, clean them up. Test Plan: - Viewed various differential revisions (with and without projects). - Viewed and edited Releeph products. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T3551 Differential Revision: https://secure.phabricator.com/D8768 --- .../DifferentialRevisionViewController.php | 16 ++++++++-- .../differential/storage/DifferentialDiff.php | 21 -------------- .../project/ReleephProductEditController.php | 3 +- .../project/ReleephProductListController.php | 2 +- .../query/ReleephProductSearchEngine.php | 3 +- .../releeph/query/ReleephProjectQuery.php | 29 +++++++++++++++++++ .../releeph/storage/ReleephProject.php | 14 +++++---- 7 files changed, 57 insertions(+), 31 deletions(-) diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index 98403e2a81..f62fb62ea7 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -33,6 +33,7 @@ final class DifferentialRevisionViewController extends DifferentialController { $diffs = id(new DifferentialDiffQuery()) ->setViewer($request->getUser()) ->withRevisionIDs(array($this->revisionID)) + ->needArcanistProjects(true) ->execute(); $diffs = array_reverse($diffs, $preserve_keys = true); @@ -61,8 +62,18 @@ final class DifferentialRevisionViewController extends DifferentialController { $diff_vs = null; } - $arc_project = $target->loadArcanistProject(); - $repository = ($arc_project ? $arc_project->loadRepository() : null); + $repository = null; + $repository_phid = $target->getRepositoryPHID(); + if ($repository_phid) { + if ($repository_phid == $revision->getRepositoryPHID()) { + $repository = $revision->getRepository(); + } else { + $repository = id(new PhabricatorRepositoryQuery()) + ->setViewer($user) + ->withPHIDs(array($repository_phid)) + ->executeOne(); + } + } list($changesets, $vs_map, $vs_changesets, $rendering_references) = $this->loadChangesetsAndVsMap( @@ -219,6 +230,7 @@ final class DifferentialRevisionViewController extends DifferentialController { 'whitespace', DifferentialChangesetParser::WHITESPACE_IGNORE_ALL); + $arc_project = $target->getArcanistProject(); if ($arc_project) { list($symbol_indexes, $project_phids) = $this->buildSymbolIndexes( $arc_project, diff --git a/src/applications/differential/storage/DifferentialDiff.php b/src/applications/differential/storage/DifferentialDiff.php index db6029233f..6af2bb9f6f 100644 --- a/src/applications/differential/storage/DifferentialDiff.php +++ b/src/applications/differential/storage/DifferentialDiff.php @@ -95,27 +95,6 @@ final class DifferentialDiff return $name; } - public function loadArcanistProject() { - if (!$this->getArcanistProjectPHID()) { - return null; - } - return id(new PhabricatorRepositoryArcanistProject())->loadOneWhere( - 'phid = %s', - $this->getArcanistProjectPHID()); - } - - public function getBackingVersionControlSystem() { - $arcanist_project = $this->loadArcanistProject(); - if (!$arcanist_project) { - return null; - } - $repository = $arcanist_project->loadRepository(); - if (!$repository) { - return null; - } - return $repository->getVersionControlSystem(); - } - public function save() { $this->openTransaction(); $ret = parent::save(); diff --git a/src/applications/releeph/controller/project/ReleephProductEditController.php b/src/applications/releeph/controller/project/ReleephProductEditController.php index b35d42ff5e..2f921db7e4 100644 --- a/src/applications/releeph/controller/project/ReleephProductEditController.php +++ b/src/applications/releeph/controller/project/ReleephProductEditController.php @@ -15,6 +15,7 @@ final class ReleephProductEditController extends ReleephProductController { $product = id(new ReleephProjectQuery()) ->setViewer($viewer) ->withIDs(array($this->productID)) + ->needArcanistProjects(true) ->requireCapabilities( array( PhabricatorPolicyCapability::CAN_VIEW, @@ -145,7 +146,7 @@ final class ReleephProductEditController extends ReleephProductController { id(new AphrontFormStaticControl()) ->setLabel(pht('Arc Project')) ->setValue( - $product->loadArcanistProject()->getName())) + $product->getArcanistProject()->getName())) ->appendChild( id(new AphrontFormStaticControl()) ->setLabel(pht('Releeph Project PHID')) diff --git a/src/applications/releeph/controller/project/ReleephProductListController.php b/src/applications/releeph/controller/project/ReleephProductListController.php index 3651358dbe..fd8e60289e 100644 --- a/src/applications/releeph/controller/project/ReleephProductListController.php +++ b/src/applications/releeph/controller/project/ReleephProductListController.php @@ -53,7 +53,7 @@ final class ReleephProductListController extends ReleephController ), 'r'.$repo->getCallsign())); - $arc = $product->loadArcanistProject(); + $arc = $product->getArcanistProject(); if ($arc) { $item->addAttribute($arc->getName()); } diff --git a/src/applications/releeph/query/ReleephProductSearchEngine.php b/src/applications/releeph/query/ReleephProductSearchEngine.php index 8269230c02..0d4b757069 100644 --- a/src/applications/releeph/query/ReleephProductSearchEngine.php +++ b/src/applications/releeph/query/ReleephProductSearchEngine.php @@ -13,7 +13,8 @@ final class ReleephProductSearchEngine public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) { $query = id(new ReleephProjectQuery()) - ->setOrder(ReleephProjectQuery::ORDER_NAME); + ->setOrder(ReleephProjectQuery::ORDER_NAME) + ->needArcanistProjects(true); $active = $saved->getParameter('active'); $value = idx($this->getActiveValues(), $active); diff --git a/src/applications/releeph/query/ReleephProjectQuery.php b/src/applications/releeph/query/ReleephProjectQuery.php index 2823633499..a4fe82a45f 100644 --- a/src/applications/releeph/query/ReleephProjectQuery.php +++ b/src/applications/releeph/query/ReleephProjectQuery.php @@ -8,6 +8,7 @@ final class ReleephProjectQuery private $phids; private $needRepositories; + private $needArcanistProjects; private $order = 'order-id'; const ORDER_ID = 'order-id'; @@ -33,6 +34,11 @@ final class ReleephProjectQuery return $this; } + public function needArcanistProjects($need) { + $this->needArcanistProjects = $need; + return $this; + } + public function loadPage() { $table = new ReleephProject(); $conn_r = $table->establishConnection('r'); @@ -71,6 +77,29 @@ final class ReleephProjectQuery return $projects; } + public function didFilterPage(array $products) { + + if ($this->needArcanistProjects) { + $project_ids = array_filter(mpull($products, 'getArcanistProjectID')); + if ($project_ids) { + $projects = id(new PhabricatorRepositoryArcanistProject()) + ->loadAllWhere('id IN (%Ld)', $project_ids); + $projects = mpull($projects, null, 'getID'); + } else { + $projects = array(); + } + + foreach ($products as $product) { + $project_id = $product->getArcanistProjectID(); + $project = idx($projects, $project_id); + $product->attachArcanistProject($project); + } + } + + return $products; + } + + private function buildWhereClause(AphrontDatabaseConnection $conn_r) { $where = array(); diff --git a/src/applications/releeph/storage/ReleephProject.php b/src/applications/releeph/storage/ReleephProject.php index 84623d24ed..989a556343 100644 --- a/src/applications/releeph/storage/ReleephProject.php +++ b/src/applications/releeph/storage/ReleephProject.php @@ -21,6 +21,7 @@ final class ReleephProject extends ReleephDAO protected $details = array(); private $repository = self::ATTACHABLE; + private $arcanistProject = self::ATTACHABLE; public function getConfiguration() { return array( @@ -53,11 +54,14 @@ final class ReleephProject extends ReleephDAO return $this; } - public function loadArcanistProject() { - return $this->loadOneRelative( - new PhabricatorRepositoryArcanistProject(), - 'id', - 'getArcanistProjectID'); + public function getArcanistProject() { + return $this->assertAttached($this->arcanistProject); + } + + public function attachArcanistProject( + PhabricatorRepositoryArcanistProject $arcanist_project = null) { + $this->arcanistProject = $arcanist_project; + return $this; } public function getPushers() {