From c467cc464fb52eff08c06f29f7a4cee7913c9c89 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 25 Sep 2013 16:54:48 -0700 Subject: [PATCH] Make most repository reads policy-aware Summary: Ref T603. This swaps almost all queries against the repository table over to be policy aware. Test Plan: - Made an audit comment on a commit. - Ran `save_lint.php`. - Looked up a commit with `diffusion.getcommits`. - Looked up lint messages with `diffusion.getlintmessages`. - Clicked an external/submodule in Diffusion. - Viewed main lint and repository lint in Diffusion. - Completed and validated Owners paths in Owners. - Executed dry runs via Herald. - Queried for package owners with `owners.query`. - Viewed Owners package. - Edited Owners package. - Viewed Owners package list. - Executed `repository.query`. - Viewed "Repository" tool repository list. - Edited Arcanist project. - Hit "Delete" on repository (this just tells you to use the CLI). - Created a repository. - Edited a repository. - Ran `bin/repository list`. - Ran `bin/search index rGTESTff45d13dffcfb3ea85b03aac8cc36251cacdf01c` - Pushed and parsed a commit. - Skipped all the Drydock stuff, as it it's hard to test and isn't normally reachable. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T603 Differential Revision: https://secure.phabricator.com/D7132 --- .../editor/PhabricatorAuditCommentEditor.php | 6 ++- .../ConduitAPI_differential_query_Method.php | 7 ++-- .../diffusion/DiffusionLintSaveRunner.php | 6 ++- ...ConduitAPI_diffusion_getcommits_Method.php | 7 ++-- ...itAPI_diffusion_getlintmessages_Method.php | 6 ++- .../DiffusionExternalController.php | 4 +- .../controller/DiffusionLintController.php | 13 +++++-- .../DiffusionPathCompleteController.php | 7 ++-- .../DiffusionPathValidateController.php | 7 ++-- .../diffusion/query/DiffusionSymbolQuery.php | 1 + .../blueprint/DrydockWorkingCopyBlueprint.php | 2 + .../worker/HarbormasterRunnerWorker.php | 1 + .../HeraldDifferentialRevisionAdapter.php | 2 + .../HeraldTestConsoleController.php | 37 ++++--------------- .../ConduitAPI_owners_query_Method.php | 16 +++++--- .../PhabricatorOwnersDetailController.php | 7 ++-- .../PhabricatorOwnersEditController.php | 4 +- .../PhabricatorOwnersListController.php | 19 ++++++---- .../storage/PhabricatorOwnersPackage.php | 2 + .../releeph/storage/ReleephProject.php | 2 +- .../ConduitAPI_repository_query_Method.php | 4 +- ...epositoryArcanistProjectEditController.php | 4 +- .../PhabricatorRepositoryDeleteController.php | 6 ++- .../PhabricatorRepositoryEditController.php | 6 ++- .../PhabricatorRepositoryListController.php | 8 ++-- ...icatorRepositoryManagementListWorkflow.php | 4 +- ...abricatorRepositoryCommitSearchIndexer.php | 8 ++-- .../storage/PhabricatorRepository.php | 2 + .../PhabricatorRepositoryArcanistProject.php | 2 +- ...habricatorRepositoryCommitParserWorker.php | 9 +++-- .../PhabricatorOwnersPackagePathValidator.php | 1 + 31 files changed, 123 insertions(+), 87 deletions(-) diff --git a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php index 9007dd87cb..01b178c1e0 100644 --- a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php @@ -403,8 +403,10 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor { $prefix = PhabricatorEnv::getEnvConfig('metamta.diffusion.subject-prefix'); - $repository = id(new PhabricatorRepository()) - ->load($commit->getRepositoryID()); + $repository = id(new PhabricatorRepositoryQuery()) + ->setViewer($this->getActor()) + ->withIDs(array($commit->getRepositoryID())) + ->executeOne(); $threading = self::getMailThreading($repository, $commit); list($thread_id, $thread_topic) = $threading; diff --git a/src/applications/differential/conduit/ConduitAPI_differential_query_Method.php b/src/applications/differential/conduit/ConduitAPI_differential_query_Method.php index 4a3c819120..6096c8386d 100644 --- a/src/applications/differential/conduit/ConduitAPI_differential_query_Method.php +++ b/src/applications/differential/conduit/ConduitAPI_differential_query_Method.php @@ -112,9 +112,10 @@ final class ConduitAPI_differential_query_Method foreach ($path_pairs as $pair) { list($callsign, $path) = $pair; if (!idx($repos, $callsign)) { - $repos[$callsign] = id(new PhabricatorRepository())->loadOneWhere( - 'callsign = %s', - $callsign); + $repos[$callsign] = id(new PhabricatorRepositoryQuery()) + ->setViewer($request->getUser()) + ->withCallsigns(array($callsign)) + ->executeOne(); if (!$repos[$callsign]) { throw id(new ConduitException('ERR-INVALID-PARAMETER')) diff --git a/src/applications/diffusion/DiffusionLintSaveRunner.php b/src/applications/diffusion/DiffusionLintSaveRunner.php index 39612a88f1..687c08ea96 100644 --- a/src/applications/diffusion/DiffusionLintSaveRunner.php +++ b/src/applications/diffusion/DiffusionLintSaveRunner.php @@ -228,8 +228,10 @@ final class DiffusionLintSaveRunner { private function blameAuthors() { - $repository = id(new PhabricatorRepository())->load( - $this->branch->getRepositoryID()); + $repository = id(new PhabricatorRepositoryQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withIDs(array($this->branch->getRepositoryID())) + ->executeOne(); $queries = array(); $futures = array(); diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_getcommits_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_getcommits_Method.php index a6ac2bda2d..ac5e5d31c8 100644 --- a/src/applications/diffusion/conduit/ConduitAPI_diffusion_getcommits_Method.php +++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_getcommits_Method.php @@ -52,9 +52,10 @@ final class ConduitAPI_diffusion_getcommits_Method $callsigns = ipull($commits, 'callsign'); $callsigns = array_unique($callsigns); - $repos = id(new PhabricatorRepository())->loadAllWhere( - 'callsign IN (%Ls)', - $callsigns); + $repos = id(new PhabricatorRepositoryQuery()) + ->setViewer($request->getUser()) + ->withCallsigns($callsigns) + ->execute(); $repos = mpull($repos, null, 'getCallsign'); foreach ($commits as $name => $info) { diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_getlintmessages_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_getlintmessages_Method.php index 3e1c636020..8d80588c89 100644 --- a/src/applications/diffusion/conduit/ConduitAPI_diffusion_getlintmessages_Method.php +++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_getlintmessages_Method.php @@ -42,8 +42,10 @@ final class ConduitAPI_diffusion_getlintmessages_Method $branch_name = $request->getValue('branch'); if ($branch_name == '') { - $repository = id(new PhabricatorRepository()) - ->load($project->getRepositoryID()); + $repository = id(new PhabricatorRepositoryQuery()) + ->setViewer($request->getUser()) + ->withIDs(array($project->getRepositoryID())) + ->executeOne(); $branch_name = $repository->getDefaultArcanistBranch(); } diff --git a/src/applications/diffusion/controller/DiffusionExternalController.php b/src/applications/diffusion/controller/DiffusionExternalController.php index 63e30fc7a3..4ed7c30361 100644 --- a/src/applications/diffusion/controller/DiffusionExternalController.php +++ b/src/applications/diffusion/controller/DiffusionExternalController.php @@ -12,7 +12,9 @@ final class DiffusionExternalController extends DiffusionController { $uri = $request->getStr('uri'); $id = $request->getStr('id'); - $repositories = id(new PhabricatorRepository())->loadAll(); + $repositories = id(new PhabricatorRepositoryQuery()) + ->setViewer($request->getUser()) + ->execute(); if ($uri) { $uri_path = id(new PhutilURI($uri))->getPath(); diff --git a/src/applications/diffusion/controller/DiffusionLintController.php b/src/applications/diffusion/controller/DiffusionLintController.php index ff4ca65700..6cd9dc9759 100644 --- a/src/applications/diffusion/controller/DiffusionLintController.php +++ b/src/applications/diffusion/controller/DiffusionLintController.php @@ -221,9 +221,10 @@ final class DiffusionLintController extends DiffusionController { } if ($paths) { - $repositories = id(new PhabricatorRepository())->loadAllWhere( - 'phid IN (%Ls)', - array_unique(mpull($paths, 'getRepositoryPHID'))); + $repositories = id(new PhabricatorRepositoryQuery()) + ->setViewer($this->getRequest()->getUser()) + ->withPHIDs(mpull($paths, 'getRepositoryPHID')) + ->execute(); $repositories = mpull($repositories, 'getID', 'getPHID'); $branches = id(new PhabricatorRepositoryBranch())->loadAllWhere( @@ -233,7 +234,11 @@ final class DiffusionLintController extends DiffusionController { } foreach ($paths as $path) { - $branch = idx($branches, $repositories[$path->getRepositoryPHID()]); + $branch = idx( + $branches, + idx( + $repositories, + $path->getRepositoryPHID())); if ($branch) { $condition = qsprintf( $conn, diff --git a/src/applications/diffusion/controller/DiffusionPathCompleteController.php b/src/applications/diffusion/controller/DiffusionPathCompleteController.php index b9dd4f117c..be21cd0331 100644 --- a/src/applications/diffusion/controller/DiffusionPathCompleteController.php +++ b/src/applications/diffusion/controller/DiffusionPathCompleteController.php @@ -10,9 +10,10 @@ final class DiffusionPathCompleteController extends DiffusionController { $request = $this->getRequest(); $repository_phid = $request->getStr('repositoryPHID'); - $repository = id(new PhabricatorRepository())->loadOneWhere( - 'phid = %s', - $repository_phid); + $repository = id(new PhabricatorRepositoryQuery()) + ->setViewer($request->getUser()) + ->withPHIDs(array($repository_phid)) + ->executeOne(); if (!$repository) { return new Aphront400Response(); } diff --git a/src/applications/diffusion/controller/DiffusionPathValidateController.php b/src/applications/diffusion/controller/DiffusionPathValidateController.php index 980b1bcb2f..ea806a6829 100644 --- a/src/applications/diffusion/controller/DiffusionPathValidateController.php +++ b/src/applications/diffusion/controller/DiffusionPathValidateController.php @@ -10,9 +10,10 @@ final class DiffusionPathValidateController extends DiffusionController { $request = $this->getRequest(); $repository_phid = $request->getStr('repositoryPHID'); - $repository = id(new PhabricatorRepository())->loadOneWhere( - 'phid = %s', - $repository_phid); + $repository = id(new PhabricatorRepositoryQuery()) + ->setViewer($request->getUser()) + ->withPHIDs(array($repository_phid)) + ->executeOne(); if (!$repository) { return new Aphront400Response(); } diff --git a/src/applications/diffusion/query/DiffusionSymbolQuery.php b/src/applications/diffusion/query/DiffusionSymbolQuery.php index 462f52b02e..ec546ec1a5 100644 --- a/src/applications/diffusion/query/DiffusionSymbolQuery.php +++ b/src/applications/diffusion/query/DiffusionSymbolQuery.php @@ -265,6 +265,7 @@ final class DiffusionSymbolQuery extends PhabricatorOffsetPagedQuery { $repo_ids = array_filter($repo_ids); if ($repo_ids) { + // TODO: (T603) Provide a viewer here. $repos = id(new PhabricatorRepository())->loadAllWhere( 'id IN (%Ld)', $repo_ids); diff --git a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprint.php b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprint.php index 32f8b05118..8039f67037 100644 --- a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprint.php +++ b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprint.php @@ -31,6 +31,8 @@ final class DrydockWorkingCopyBlueprint extends DrydockBlueprint { "Lease is missing required 'repositoryID' attribute."); } + // TODO: (T603) Figure out the interaction between policies and + // Drydock. $repository = id(new PhabricatorRepository())->load($repository_id); if (!$repository) { diff --git a/src/applications/harbormaster/worker/HarbormasterRunnerWorker.php b/src/applications/harbormaster/worker/HarbormasterRunnerWorker.php index 5b0e578ead..088f072bf2 100644 --- a/src/applications/harbormaster/worker/HarbormasterRunnerWorker.php +++ b/src/applications/harbormaster/worker/HarbormasterRunnerWorker.php @@ -19,6 +19,7 @@ final class HarbormasterRunnerWorker extends PhabricatorWorker { "Commit '{$id}' does not exist!"); } + // TODO: (T603) Policy interaction? $repository = id(new PhabricatorRepository())->loadOneWhere( 'id = %d', $commit->getRepositoryID()); diff --git a/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php b/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php index aef9921afb..e10832fdad 100644 --- a/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php +++ b/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php @@ -110,6 +110,8 @@ final class HeraldDifferentialRevisionAdapter extends HeraldAdapter { $repository = false; + // TODO: (T603) Implement policy stuff in Herald. + if ($diff->getRepositoryUUID()) { $repository = id(new PhabricatorRepository())->loadOneWhere( 'uuid = %s', diff --git a/src/applications/herald/controller/HeraldTestConsoleController.php b/src/applications/herald/controller/HeraldTestConsoleController.php index 80ec24e47c..8630d52fe3 100644 --- a/src/applications/herald/controller/HeraldTestConsoleController.php +++ b/src/applications/herald/controller/HeraldTestConsoleController.php @@ -20,35 +20,14 @@ final class HeraldTestConsoleController extends HeraldController { } if (!$errors) { - $matches = null; - $object = null; - if (preg_match('/^D(\d+)$/', $object_name, $matches)) { - $object = id(new DifferentialRevision())->load($matches[1]); - if (!$object) { - $e_name = pht('Invalid'); - $errors[] = pht('No Differential Revision with that ID exists.'); - } - } else if (preg_match('/^r([A-Z]+)(\w+)$/', $object_name, $matches)) { - $repo = id(new PhabricatorRepository())->loadOneWhere( - 'callsign = %s', - $matches[1]); - if (!$repo) { - $e_name = pht('Invalid'); - $errors[] = pht('There is no repository with the callsign: %s.', - $matches[1]); - } - $commit = id(new PhabricatorRepositoryCommit())->loadOneWhere( - 'repositoryID = %d AND commitIdentifier = %s', - $repo->getID(), - $matches[2]); - if (!$commit) { - $e_name = pht('Invalid'); - $errors[] = pht('There is no commit with that identifier.'); - } - $object = $commit; - } else { + $object = id(new PhabricatorObjectQuery()) + ->setViewer($user) + ->withNames(array($object_name)) + ->executeOne(); + + if (!$object) { $e_name = pht('Invalid'); - $errors[] = pht('This object name is not recognized.'); + $errors[] = pht('No object exists with that name.'); } if (!$errors) { @@ -61,7 +40,7 @@ final class HeraldTestConsoleController extends HeraldController { 'commitID = %d', $object->getID()); $adapter = HeraldCommitAdapter::newLegacyAdapter( - $repo, + $object->getRepository(), $object, $data); } else { diff --git a/src/applications/owners/conduit/ConduitAPI_owners_query_Method.php b/src/applications/owners/conduit/ConduitAPI_owners_query_Method.php index b36b977aed..6d93d659ba 100644 --- a/src/applications/owners/conduit/ConduitAPI_owners_query_Method.php +++ b/src/applications/owners/conduit/ConduitAPI_owners_query_Method.php @@ -65,11 +65,17 @@ final class ConduitAPI_owners_query_Method return $packages; } - private static function queryByPath($repo_callsign, $path) { - $repository = id(new PhabricatorRepository())->loadOneWhere('callsign = %s', - $repo_callsign); + private static function queryByPath( + PhabricatorUser $viewer, + $repo_callsign, + $path) { - if (empty($repository)) { + $repository = id(new PhabricatorRepositoryQuery()) + ->setViewer($viewer) + ->withCallsigns(array($repo_callsign)) + ->executeOne(); + + if (!$repository) { throw id(new ConduitException('ERR_REP_NOT_FOUND')) ->setErrorDescription( 'Repository callsign '.$repo_callsign.' not recognized'); @@ -144,7 +150,7 @@ final class ConduitAPI_owners_query_Method $packages = self::queryByOwner($owner); } else if ($is_path_query) { - $packages = self::queryByPath($repo, $path); + $packages = self::queryByPath($request->getUser(), $repo, $path); } return self::buildPackageInformationDictionaries($packages); diff --git a/src/applications/owners/controller/PhabricatorOwnersDetailController.php b/src/applications/owners/controller/PhabricatorOwnersDetailController.php index 545c565e27..4f6fa77e8d 100644 --- a/src/applications/owners/controller/PhabricatorOwnersDetailController.php +++ b/src/applications/owners/controller/PhabricatorOwnersDetailController.php @@ -29,9 +29,10 @@ final class PhabricatorOwnersDetailController } if ($repository_phids) { - $repositories = id(new PhabricatorRepository())->loadAllWhere( - 'phid in (%Ls)', - array_keys($repository_phids)); + $repositories = id(new PhabricatorRepositoryQuery()) + ->setViewer($user) + ->withPHIDs(array_keys($repository_phids)) + ->execute(); $repositories = mpull($repositories, null, 'getPHID'); } else { $repositories = array(); diff --git a/src/applications/owners/controller/PhabricatorOwnersEditController.php b/src/applications/owners/controller/PhabricatorOwnersEditController.php index 8ba519bdb9..33a1aabeb7 100644 --- a/src/applications/owners/controller/PhabricatorOwnersEditController.php +++ b/src/applications/owners/controller/PhabricatorOwnersEditController.php @@ -142,7 +142,9 @@ final class PhabricatorOwnersEditController } $this->setSideNavFilter($side_nav_filter); - $repos = id(new PhabricatorRepository())->loadAll(); + $repos = id(new PhabricatorRepositoryQuery()) + ->setViewer($user) + ->execute(); $default_paths = array(); foreach ($repos as $repo) { diff --git a/src/applications/owners/controller/PhabricatorOwnersListController.php b/src/applications/owners/controller/PhabricatorOwnersListController.php index 1a3bc58f6d..0d6130a59c 100644 --- a/src/applications/owners/controller/PhabricatorOwnersListController.php +++ b/src/applications/owners/controller/PhabricatorOwnersListController.php @@ -21,8 +21,10 @@ final class PhabricatorOwnersListController $repository_phid = ''; if ($request->getStr('repository') != '') { - $repository_phid = id(new PhabricatorRepository()) - ->loadOneWhere('callsign = %s', $request->getStr('repository')) + $repository_phid = id(new PhabricatorRepositoryQuery()) + ->setViewer($user) + ->withCallsigns(array($request->getStr('repository'))) + ->executeOne() ->getPHID(); } @@ -157,8 +159,10 @@ final class PhabricatorOwnersListController } $callsigns = array('' => pht('(Any Repository)')); - $repositories = id(new PhabricatorRepository()) - ->loadAllWhere('1 = 1 ORDER BY callsign'); + $repositories = id(new PhabricatorRepositoryQuery()) + ->setViewer($user) + ->setOrder(PhabricatorRepositoryQuery::ORDER_CALLSIGN) + ->execute(); foreach ($repositories as $repository) { $callsigns[$repository->getCallsign()] = $repository->getCallsign().': '.$repository->getName(); @@ -238,9 +242,10 @@ final class PhabricatorOwnersListController } if ($repository_phids) { - $repositories = id(new PhabricatorRepository())->loadAllWhere( - 'phid in (%Ls)', - array_keys($repository_phids)); + $repositories = id(new PhabricatorRepositoryQuery()) + ->setViewer($this->getRequest()->getUser()) + ->withPHIDs(array_keys($repository_phids)) + ->execute(); } else { $repositories = array(); } diff --git a/src/applications/owners/storage/PhabricatorOwnersPackage.php b/src/applications/owners/storage/PhabricatorOwnersPackage.php index 4d90f1d0af..90af440eec 100644 --- a/src/applications/owners/storage/PhabricatorOwnersPackage.php +++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php @@ -297,6 +297,8 @@ final class PhabricatorOwnersPackage extends PhabricatorOwnersDAO $cur_paths = mgroup($cur_paths, 'getRepositoryPHID', 'getPath'); foreach ($new_paths as $repository_phid => $paths) { + // TODO: (T603) Thread policy stuff in here. + // get repository object for path validation $repository = id(new PhabricatorRepository())->loadOneWhere( 'phid = %s', diff --git a/src/applications/releeph/storage/ReleephProject.php b/src/applications/releeph/storage/ReleephProject.php index 8f31a038a4..ec5b0dd085 100644 --- a/src/applications/releeph/storage/ReleephProject.php +++ b/src/applications/releeph/storage/ReleephProject.php @@ -113,7 +113,7 @@ final class ReleephProject extends ReleephDAO return $this->assertAttached($this->repository); } - // TODO: Remove once everything uses ProjectQuery. + // TODO: Remove once everything uses ProjectQuery. Also, T603. public function loadPhabricatorRepository() { return $this->loadOneRelative( new PhabricatorRepository(), diff --git a/src/applications/repository/conduit/ConduitAPI_repository_query_Method.php b/src/applications/repository/conduit/ConduitAPI_repository_query_Method.php index 39803b3c30..8742d375ef 100644 --- a/src/applications/repository/conduit/ConduitAPI_repository_query_Method.php +++ b/src/applications/repository/conduit/ConduitAPI_repository_query_Method.php @@ -33,7 +33,9 @@ final class ConduitAPI_repository_query_Method } protected function execute(ConduitAPIRequest $request) { - $repositories = id(new PhabricatorRepository())->loadAll(); + $repositories = id(new PhabricatorRepositoryQuery()) + ->setViewer($request->getUser()) + ->execute(); $results = array(); foreach ($repositories as $repository) { diff --git a/src/applications/repository/controller/PhabricatorRepositoryArcanistProjectEditController.php b/src/applications/repository/controller/PhabricatorRepositoryArcanistProjectEditController.php index 46556dc879..15a0b6f970 100644 --- a/src/applications/repository/controller/PhabricatorRepositoryArcanistProjectEditController.php +++ b/src/applications/repository/controller/PhabricatorRepositoryArcanistProjectEditController.php @@ -19,7 +19,9 @@ final class PhabricatorRepositoryArcanistProjectEditController return new Aphront404Response(); } - $repositories = id(new PhabricatorRepository())->loadAll(); + $repositories = id(new PhabricatorRepositoryQuery()) + ->setViewer($user) + ->execute(); $repos = array( 0 => 'None', ); diff --git a/src/applications/repository/controller/PhabricatorRepositoryDeleteController.php b/src/applications/repository/controller/PhabricatorRepositoryDeleteController.php index 4e89e68839..284eb646c5 100644 --- a/src/applications/repository/controller/PhabricatorRepositoryDeleteController.php +++ b/src/applications/repository/controller/PhabricatorRepositoryDeleteController.php @@ -10,8 +10,12 @@ final class PhabricatorRepositoryDeleteController } public function processRequest() { + $viewer = $this->getRequest()->getUser(); - $repository = id(new PhabricatorRepository())->load($this->id); + $repository = id(new PhabricatorRepositoryQuery()) + ->setViewer($viewer) + ->withIDs(array($this->id)) + ->executeOne(); if (!$repository) { return new Aphront404Response(); } diff --git a/src/applications/repository/controller/PhabricatorRepositoryEditController.php b/src/applications/repository/controller/PhabricatorRepositoryEditController.php index 37ffaf8e87..468dac45e9 100644 --- a/src/applications/repository/controller/PhabricatorRepositoryEditController.php +++ b/src/applications/repository/controller/PhabricatorRepositoryEditController.php @@ -16,8 +16,12 @@ final class PhabricatorRepositoryEditController public function processRequest() { $request = $this->getRequest(); + $viewer = $request->getUser(); - $repository = id(new PhabricatorRepository())->load($this->id); + $repository = id(new PhabricatorRepositoryQuery()) + ->setViewer($viewer) + ->withIDs(array($this->id)) + ->executeOne(); if (!$repository) { return new Aphront404Response(); } diff --git a/src/applications/repository/controller/PhabricatorRepositoryListController.php b/src/applications/repository/controller/PhabricatorRepositoryListController.php index f09c5096ae..b8d289a7cb 100644 --- a/src/applications/repository/controller/PhabricatorRepositoryListController.php +++ b/src/applications/repository/controller/PhabricatorRepositoryListController.php @@ -3,17 +3,15 @@ final class PhabricatorRepositoryListController extends PhabricatorRepositoryController { - public function shouldRequireAdmin() { - return false; - } - public function processRequest() { $request = $this->getRequest(); $user = $request->getUser(); $is_admin = $user->getIsAdmin(); - $repos = id(new PhabricatorRepository())->loadAll(); + $repos = id(new PhabricatorRepositoryQuery()) + ->setViewer($user) + ->execute(); $repos = msort($repos, 'getName'); $rows = array(); diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementListWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementListWorkflow.php index d99cccfdbb..6ea6992f24 100644 --- a/src/applications/repository/management/PhabricatorRepositoryManagementListWorkflow.php +++ b/src/applications/repository/management/PhabricatorRepositoryManagementListWorkflow.php @@ -13,7 +13,9 @@ final class PhabricatorRepositoryManagementListWorkflow public function execute(PhutilArgumentParser $args) { $console = PhutilConsole::getConsole(); - $repos = id(new PhabricatorRepository())->loadAll(); + $repos = id(new PhabricatorRepositoryQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->execute(); if ($repos) { foreach ($repos as $repo) { $console->writeOut("%s\n", $repo->getCallsign()); diff --git a/src/applications/repository/search/PhabricatorRepositoryCommitSearchIndexer.php b/src/applications/repository/search/PhabricatorRepositoryCommitSearchIndexer.php index 219ea29a41..7147b37a66 100644 --- a/src/applications/repository/search/PhabricatorRepositoryCommitSearchIndexer.php +++ b/src/applications/repository/search/PhabricatorRepositoryCommitSearchIndexer.php @@ -17,10 +17,10 @@ final class PhabricatorRepositoryCommitSearchIndexer $commit_message = $commit_data->getCommitMessage(); $author_phid = $commit_data->getCommitDetail('authorPHID'); - $repository = id(new PhabricatorRepository())->loadOneWhere( - 'id = %d', - $commit->getRepositoryID()); - + $repository = id(new PhabricatorRepositoryQuery()) + ->setViewer($this->getViewer()) + ->withIDs(array($commit->getRepositoryID())) + ->executeOne(); if (!$repository) { throw new Exception("No such repository!"); } diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index 4c6877bbda..7a5df61611 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -452,6 +452,8 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO } public static function loadAllByPHIDOrCallsign(array $names) { + // TODO: (T603) Get rid of this. + $repositories = array(); foreach ($names as $name) { $repo = id(new PhabricatorRepository())->loadOneWhere( diff --git a/src/applications/repository/storage/PhabricatorRepositoryArcanistProject.php b/src/applications/repository/storage/PhabricatorRepositoryArcanistProject.php index 0ce6daafb2..fc02ffb9c5 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryArcanistProject.php +++ b/src/applications/repository/storage/PhabricatorRepositoryArcanistProject.php @@ -32,7 +32,7 @@ final class PhabricatorRepositoryArcanistProject PhabricatorRepositoryPHIDTypeArcanistProject::TYPECONST); } - // TODO: Remove. + // TODO: Remove. Also, T603. public function loadRepository() { if (!$this->getRepositoryID()) { return null; diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php index 5aa3327100..e75e6022c8 100644 --- a/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php +++ b/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php @@ -31,9 +31,10 @@ abstract class PhabricatorRepositoryCommitParserWorker return; } - $repository = id(new PhabricatorRepository())->load( - $this->commit->getRepositoryID()); - + $repository = id(new PhabricatorRepositoryQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withIDs(array($this->commit->getRepositoryID())) + ->executeOne(); if (!$repository) { return; } @@ -92,6 +93,8 @@ abstract class PhabricatorRepositoryCommitParserWorker return $suffix; } + // TODO: (T603) This method should probably take a viewer. + $repository = id(new PhabricatorRepository()) ->load($commit->getRepositoryID()); $link = DiffusionView::linkCommit($repository, diff --git a/src/applications/repository/worker/commitchangeparser/PhabricatorOwnersPackagePathValidator.php b/src/applications/repository/worker/commitchangeparser/PhabricatorOwnersPackagePathValidator.php index 3a62f07656..43f50217c2 100644 --- a/src/applications/repository/worker/commitchangeparser/PhabricatorOwnersPackagePathValidator.php +++ b/src/applications/repository/worker/commitchangeparser/PhabricatorOwnersPackagePathValidator.php @@ -15,6 +15,7 @@ final class PhabricatorOwnersPackagePathValidator { return; } + // TODO: (T603) This should be policy-aware. $repository = id(new PhabricatorRepository())->load($commit->getRepositoryID()); $move_map = array();