diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php index e5c5fbbc83..a99ad41268 100644 --- a/src/applications/differential/controller/DifferentialChangesetViewController.php +++ b/src/applications/differential/controller/DifferentialChangesetViewController.php @@ -296,6 +296,8 @@ final class DifferentialChangesetViewController extends DifferentialController { DifferentialChangeset $changeset, $is_new) { + $viewer = $this->getRequest()->getUser(); + if ($is_new) { $key = 'raw:new:phid'; } else { @@ -307,9 +309,13 @@ final class DifferentialChangesetViewController extends DifferentialController { $file = null; $phid = idx($metadata, $key); if ($phid) { - $file = id(new PhabricatorFile())->loadOneWhere( - 'phid = %s', - $phid); + $file = id(new PhabricatorFileQuery()) + ->setViewer($viewer) + ->withPHIDs(array($phid)) + ->execute(); + if ($file) { + $file = head($file); + } } if (!$file) { diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index 3b1eac014a..8e59c2b380 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -887,6 +887,7 @@ final class DifferentialRevisionViewController extends DifferentialController { * @return mixed (@{class:PhabricatorFile} if found, null if not) */ public function loadFileByPHID($phid) { + // TODO: (T603) Factor this and the other one out. $file = id(new PhabricatorFile())->loadOneWhere( 'phid = %s', $phid); diff --git a/src/applications/differential/mail/DifferentialReviewRequestMail.php b/src/applications/differential/mail/DifferentialReviewRequestMail.php index a1c3e01c17..7a3f4523f5 100644 --- a/src/applications/differential/mail/DifferentialReviewRequestMail.php +++ b/src/applications/differential/mail/DifferentialReviewRequestMail.php @@ -104,6 +104,7 @@ abstract class DifferentialReviewRequestMail extends DifferentialMail { } public function loadFileByPHID($phid) { + // TODO: (T603) Factor this and the other one out. $file = id(new PhabricatorFile())->loadOneWhere( 'phid = %s', $phid); diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php index e68ca33ab5..637b3ac363 100644 --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/DifferentialChangesetParser.php @@ -851,6 +851,7 @@ final class DifferentialChangesetParser { $file_phids[] = $new_phid; } + // TODO: (T603) Probably fine to use omnipotent viewer here? $files = id(new PhabricatorFile())->loadAllWhere( 'phid IN (%Ls)', $file_phids); diff --git a/src/applications/files/conduit/ConduitAPI_file_download_Method.php b/src/applications/files/conduit/ConduitAPI_file_download_Method.php index 6d6215040f..66c7c700fd 100644 --- a/src/applications/files/conduit/ConduitAPI_file_download_Method.php +++ b/src/applications/files/conduit/ConduitAPI_file_download_Method.php @@ -29,9 +29,10 @@ final class ConduitAPI_file_download_Method protected function execute(ConduitAPIRequest $request) { $phid = $request->getValue('phid'); - $file = id(new PhabricatorFile())->loadOneWhere( - 'phid = %s', - $phid); + $file = id(new PhabricatorFileQuery()) + ->setViewer($request->getUser()) + ->withPHIDs(array($phid)) + ->executeOne(); if (!$file) { throw new ConduitException('ERR-BAD-PHID'); } diff --git a/src/applications/files/conduit/ConduitAPI_file_info_Method.php b/src/applications/files/conduit/ConduitAPI_file_info_Method.php index ca0d6ccdc9..555ec2585a 100644 --- a/src/applications/files/conduit/ConduitAPI_file_info_Method.php +++ b/src/applications/files/conduit/ConduitAPI_file_info_Method.php @@ -30,14 +30,16 @@ final class ConduitAPI_file_info_Method extends ConduitAPI_file_Method { $phid = $request->getValue('phid'); $id = $request->getValue('id'); + $query = id(new PhabricatorFileQuery()) + ->setViewer($request->getUser()); if ($id) { - $file = id(new PhabricatorFile())->load($id); + $query->withIDs(array($id)); } else { - $file = id(new PhabricatorFile())->loadOneWhere( - 'phid = %s', - $phid); + $query->withPHIDs(array($phid)); } + $file = $query->executeOne(); + if (!$file) { throw new ConduitException('ERR-NOT-FOUND'); } diff --git a/src/applications/files/controller/PhabricatorFileDataController.php b/src/applications/files/controller/PhabricatorFileDataController.php index 8423277955..bd42f01c9d 100644 --- a/src/applications/files/controller/PhabricatorFileDataController.php +++ b/src/applications/files/controller/PhabricatorFileDataController.php @@ -25,9 +25,10 @@ final class PhabricatorFileDataController extends PhabricatorFileController { ->setURI($uri->setPath($request->getPath())); } - $file = id(new PhabricatorFile())->loadOneWhere( - 'phid = %s', - $this->phid); + $file = id(new PhabricatorFileQuery()) + ->setViewer($request->getUser()) + ->withPHIDs(array($this->phid)) + ->executeOne(); if (!$file) { return new Aphront404Response(); } diff --git a/src/applications/files/controller/PhabricatorFileDeleteController.php b/src/applications/files/controller/PhabricatorFileDeleteController.php index 5f54debdc9..17be9f2ff2 100644 --- a/src/applications/files/controller/PhabricatorFileDeleteController.php +++ b/src/applications/files/controller/PhabricatorFileDeleteController.php @@ -9,13 +9,18 @@ final class PhabricatorFileDeleteController extends PhabricatorFileController { } public function processRequest() { - $request = $this->getRequest(); $user = $request->getUser(); - $file = id(new PhabricatorFile())->loadOneWhere( - 'id = %d', - $this->id); + $file = id(new PhabricatorFileQuery()) + ->setViewer($user) + ->withIDs(array($this->id)) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->executeOne(); if (!$file) { return new Aphront404Response(); } diff --git a/src/applications/files/controller/PhabricatorFileShortcutController.php b/src/applications/files/controller/PhabricatorFileShortcutController.php index 3d699e10d4..30eb9fa995 100644 --- a/src/applications/files/controller/PhabricatorFileShortcutController.php +++ b/src/applications/files/controller/PhabricatorFileShortcutController.php @@ -10,10 +10,14 @@ final class PhabricatorFileShortcutController } public function processRequest() { - $file = id(new PhabricatorFile())->load($this->id); + $file = id(new PhabricatorFileQuery()) + ->setViewer($this->getRequest()->getUser()) + ->withIDs(array($this->id)) + ->executeOne(); if (!$file) { return new Aphront404Response(); } + return id(new AphrontRedirectResponse())->setURI($file->getBestURI()); } diff --git a/src/applications/files/controller/PhabricatorFileTransformController.php b/src/applications/files/controller/PhabricatorFileTransformController.php index ffcf9935ea..7a8eb7772d 100644 --- a/src/applications/files/controller/PhabricatorFileTransformController.php +++ b/src/applications/files/controller/PhabricatorFileTransformController.php @@ -18,8 +18,12 @@ final class PhabricatorFileTransformController } public function processRequest() { + $viewer = $this->getRequest()->getUser(); - $file = id(new PhabricatorFile())->loadOneWhere('phid = %s', $this->phid); + $file = id(new PhabricatorFileQuery()) + ->setViewer($viewer) + ->withPHIDs(array($this->phid)) + ->executeOne(); if (!$file) { return new Aphront404Response(); } @@ -125,20 +129,17 @@ final class PhabricatorFileTransformController private function buildTransformedFileResponse( PhabricatorTransformedFile $xform) { - $file = id(new PhabricatorFile())->loadOneWhere( - 'phid = %s', - $xform->getTransformedPHID()); - if ($file) { - $uri = $file->getBestURI(); - } else { - $bad_phid = $xform->getTransformedPHID(); - throw new Exception( - "Unable to load file with phid {$bad_phid}." - ); + $file = id(new PhabricatorFileQuery()) + ->setViewer($this->getRequest()->getUser()) + ->withPHIDs(array($xform->getTransformedPHID())) + ->executeOne(); + if (!$file) { + return new Aphront404Response(); } // TODO: We could just delegate to the file view controller instead, // which would save the client a roundtrip, but is slightly more complex. + $uri = $file->getBestURI(); return id(new AphrontRedirectResponse())->setURI($uri); } diff --git a/src/applications/files/management/PhabricatorFilesManagementWorkflow.php b/src/applications/files/management/PhabricatorFilesManagementWorkflow.php index 4272122802..85de4aefcb 100644 --- a/src/applications/files/management/PhabricatorFilesManagementWorkflow.php +++ b/src/applications/files/management/PhabricatorFilesManagementWorkflow.php @@ -19,6 +19,8 @@ abstract class PhabricatorFilesManagementWorkflow if ($args->getArg('names')) { $iterator = array(); + // TODO: (T603) Convert this to ObjectNameQuery. + foreach ($args->getArg('names') as $name) { $name = trim($name); diff --git a/src/applications/files/remarkup/PhabricatorRemarkupRuleEmbedFile.php b/src/applications/files/remarkup/PhabricatorRemarkupRuleEmbedFile.php index 6758ccbea8..81a242c098 100644 --- a/src/applications/files/remarkup/PhabricatorRemarkupRuleEmbedFile.php +++ b/src/applications/files/remarkup/PhabricatorRemarkupRuleEmbedFile.php @@ -21,6 +21,8 @@ final class PhabricatorRemarkupRuleEmbedFile $file = null; if ($matches[1]) { // TODO: This is pretty inefficient if there are a bunch of files. + // TODO: (T603) This isn't policy-aware and should be extending + // PhabricatorRemarkupRuleObject. $file = id(new PhabricatorFile())->load($matches[1]); } diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php index 7f30a12e5d..7ec943d58d 100644 --- a/src/applications/files/storage/PhabricatorFile.php +++ b/src/applications/files/storage/PhabricatorFile.php @@ -828,6 +828,7 @@ final class PhabricatorFile extends PhabricatorFileDAO public function getCapabilities() { return array( PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, ); } diff --git a/src/applications/macro/controller/PhabricatorMacroEditController.php b/src/applications/macro/controller/PhabricatorMacroEditController.php index 1cdf858a8e..733c36f9a7 100644 --- a/src/applications/macro/controller/PhabricatorMacroEditController.php +++ b/src/applications/macro/controller/PhabricatorMacroEditController.php @@ -82,9 +82,10 @@ final class PhabricatorMacroEditController $errors[] = pht('Could not fetch URL: %s', $ex->getMessage()); } } else if ($request->getStr('phid')) { - $file = id(new PhabricatorFile())->loadOneWhere( - 'phid = %s', - $request->getStr('phid')); + $file = id(new PhabricatorFileQuery()) + ->setViewer($user) + ->withPHIDs(array($request->getStr('phid'))) + ->executeOne(); } if ($file) { diff --git a/src/applications/macro/controller/PhabricatorMacroMemeController.php b/src/applications/macro/controller/PhabricatorMacroMemeController.php index e876db8068..b0a79e3a4c 100644 --- a/src/applications/macro/controller/PhabricatorMacroMemeController.php +++ b/src/applications/macro/controller/PhabricatorMacroMemeController.php @@ -38,10 +38,15 @@ final class PhabricatorMacroMemeController $file->getPHID(), $hash); if ($xform) { - $memefile = id(new PhabricatorFile())->loadOneWhere( - 'phid = %s', $xform->getTransformedPHID()); - return $memefile->getBestURI(); + $memefile = id(new PhabricatorFileQuery()) + ->setViewer($user) + ->withPHIDs(array($xform->getTransformedPHID())) + ->executeOne(); + if ($memefile) { + return $memefile->getBestURI(); + } } + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); $transformers = (new PhabricatorImageTransformer()); $newfile = $transformers diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php index ee00597a3f..12290f74bf 100644 --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -611,9 +611,12 @@ final class ManiphestTaskDetailController extends ManiphestController { if ($file_infos) { $file_phids = array_keys($file_infos); - $files = id(new PhabricatorFile())->loadAllWhere( - 'phid IN (%Ls)', - $file_phids); + // TODO: These should probably be handles or something; clean this up + // as we sort out file attachments. + $files = id(new PhabricatorFileQuery()) + ->setViewer($viewer) + ->withPHIDs($file_phids) + ->execute(); $file_view = new PhabricatorFileLinkListView(); $file_view->setFiles($files); diff --git a/src/applications/maniphest/controller/ManiphestTaskEditController.php b/src/applications/maniphest/controller/ManiphestTaskEditController.php index 5735c50417..95d904ff77 100644 --- a/src/applications/maniphest/controller/ManiphestTaskEditController.php +++ b/src/applications/maniphest/controller/ManiphestTaskEditController.php @@ -72,9 +72,10 @@ final class ManiphestTaskEditController extends ManiphestController { } if ($file_phids) { - $files = id(new PhabricatorFile())->loadAllWhere( - 'phid IN (%Ls)', - $file_phids); + $files = id(new PhabricatorFileQuery()) + ->setViewer($user) + ->withPHIDs($file_phids) + ->execute(); } $template_id = $request->getInt('template'); diff --git a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php b/src/applications/maniphest/controller/ManiphestTransactionSaveController.php index 494bf9ac9f..cc98a9eccf 100644 --- a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php +++ b/src/applications/maniphest/controller/ManiphestTransactionSaveController.php @@ -33,9 +33,10 @@ final class ManiphestTransactionSaveController extends ManiphestController { // Look for drag-and-drop uploads first. $file_phids = $request->getArr('files'); if ($file_phids) { - $files = id(new PhabricatorFile())->loadAllWhere( - 'phid in (%Ls)', - $file_phids); + $files = id(new PhabricatorFileQuery()) + ->setViewer($user) + ->withPHIDs(array($file_phids)) + ->execute(); } // This means "attach a file" even though we store other types of data diff --git a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php index b37ce4db33..07245a50a5 100644 --- a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php +++ b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php @@ -315,6 +315,7 @@ EOBODY; return $body; } + // TODO: (T603) What's the policy here? $files = id(new PhabricatorFile()) ->loadAllWhere('phid in (%Ls)', $attachments); diff --git a/src/applications/paste/controller/PhabricatorPasteViewController.php b/src/applications/paste/controller/PhabricatorPasteViewController.php index d270fb4e06..1ffb3d08ef 100644 --- a/src/applications/paste/controller/PhabricatorPasteViewController.php +++ b/src/applications/paste/controller/PhabricatorPasteViewController.php @@ -44,9 +44,10 @@ final class PhabricatorPasteViewController extends PhabricatorPasteController { return new Aphront404Response(); } - $file = id(new PhabricatorFile())->loadOneWhere( - 'phid = %s', - $paste->getFilePHID()); + $file = id(new PhabricatorFileQuery()) + ->setViewer($user) + ->withPHIDs(array($paste->getFilePHID())) + ->executeOne(); if (!$file) { return new Aphront400Response(); } diff --git a/src/applications/paste/query/PhabricatorPasteQuery.php b/src/applications/paste/query/PhabricatorPasteQuery.php index 5b3c08be73..d05f78b457 100644 --- a/src/applications/paste/query/PhabricatorPasteQuery.php +++ b/src/applications/paste/query/PhabricatorPasteQuery.php @@ -162,9 +162,10 @@ final class PhabricatorPasteQuery private function loadRawContent(array $pastes) { $file_phids = mpull($pastes, 'getFilePHID'); - $files = id(new PhabricatorFile())->loadAllWhere( - 'phid IN (%Ls)', - $file_phids); + $files = id(new PhabricatorFileQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($file_phids) + ->execute(); $files = mpull($files, null, 'getPHID'); foreach ($pastes as $key => $paste) { diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index 0602ac2175..cf6942fb62 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -746,6 +746,8 @@ EOBODY; $src_phid = $this->getProfileImagePHID(); if ($src_phid) { + // TODO: (T603) Can we get rid of this entirely and move it to + // PeopleQuery with attach/attachable? $file = id(new PhabricatorFile())->loadOneWhere('phid = %s', $src_phid); if ($file) { $this->profileImage = $file->getBestURI(); diff --git a/src/applications/pholio/controller/PholioInlineThumbController.php b/src/applications/pholio/controller/PholioInlineThumbController.php index 219996fe26..9fb7c9a79a 100644 --- a/src/applications/pholio/controller/PholioInlineThumbController.php +++ b/src/applications/pholio/controller/PholioInlineThumbController.php @@ -34,9 +34,14 @@ final class PholioInlineThumbController extends PholioController { return new Aphront404Response(); } - $file = id(new PhabricatorFile())->loadOneWhere( - 'phid = %s', - $image->getFilePHID()); + $file = id(new PhabricatorFileQuery()) + ->setViewer($user) + ->witHPHIDs(array($image->getFilePHID())) + ->executeOne(); + + if (!$file) { + return new Aphront404Response(); + } return id(new AphrontRedirectResponse())->setURI($file->getThumb60x45URI()); } diff --git a/src/applications/pholio/query/PholioImageQuery.php b/src/applications/pholio/query/PholioImageQuery.php index cbffb17fc3..b619e25739 100644 --- a/src/applications/pholio/query/PholioImageQuery.php +++ b/src/applications/pholio/query/PholioImageQuery.php @@ -104,9 +104,12 @@ final class PholioImageQuery assert_instances_of($images, 'PholioImage'); $file_phids = mpull($images, 'getFilePHID'); - $all_files = mpull(id(new PhabricatorFile())->loadAllWhere( - 'phid IN (%Ls)', - $file_phids), null, 'getPHID'); + + $all_files = id(new PhabricatorFileQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($file_phids) + ->execute(); + $all_files = mpull($all_files, null, 'getPHID'); if ($this->needInlineComments) { $all_inline_comments = id(new PholioTransactionComment()) diff --git a/src/applications/pholio/query/PholioMockQuery.php b/src/applications/pholio/query/PholioMockQuery.php index 1f37935d28..f08c9e4633 100644 --- a/src/applications/pholio/query/PholioMockQuery.php +++ b/src/applications/pholio/query/PholioMockQuery.php @@ -132,9 +132,12 @@ final class PholioMockQuery private function loadCoverFiles(array $mocks) { assert_instances_of($mocks, 'PholioMock'); $cover_file_phids = mpull($mocks, 'getCoverPHID'); - $cover_files = mpull(id(new PhabricatorFile())->loadAllWhere( - 'phid IN (%Ls)', - $cover_file_phids), null, 'getPHID'); + $cover_files = id(new PhabricatorFileQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($cover_file_phids) + ->execute(); + + $cover_files = mpull($cover_files, null, 'getPHID'); foreach ($mocks as $mock) { $file = idx($cover_files, $mock->getCoverPHID()); diff --git a/src/applications/project/storage/PhabricatorProjectProfile.php b/src/applications/project/storage/PhabricatorProjectProfile.php index fb2e959320..ee8f42abea 100644 --- a/src/applications/project/storage/PhabricatorProjectProfile.php +++ b/src/applications/project/storage/PhabricatorProjectProfile.php @@ -9,6 +9,7 @@ final class PhabricatorProjectProfile extends PhabricatorProjectDAO { public function loadProfileImageURI() { $src_phid = $this->getProfileImagePHID(); + // TODO: (T603) Can we get rid of this and move it to a Query? $file = id(new PhabricatorFile())->loadOneWhere('phid = %s', $src_phid); if ($file) { return $file->getBestURI(); diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php index 1b893ac206..d7f66b0457 100644 --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php @@ -329,9 +329,10 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker $files = array(); if ($file_phids) { - $files = id(new PhabricatorFile())->loadAllWhere( - 'phid IN (%Ls)', - $file_phids); + $files = id(new PhabricatorFileQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withPHIDs($file_phids) + ->execute(); $files = mpull($files, null, 'getPHID'); } diff --git a/src/applications/xhprof/controller/PhabricatorXHProfProfileController.php b/src/applications/xhprof/controller/PhabricatorXHProfProfileController.php index dac92925a8..089fa8bd3f 100644 --- a/src/applications/xhprof/controller/PhabricatorXHProfProfileController.php +++ b/src/applications/xhprof/controller/PhabricatorXHProfProfileController.php @@ -10,11 +10,12 @@ final class PhabricatorXHProfProfileController } public function processRequest() { + $request = $this->getRequest(); - $file = id(new PhabricatorFile())->loadOneWhere( - 'phid = %s', - $this->phid); - + $file = id(new PhabricatorFileQuery()) + ->setViewer($request->getUser()) + ->withPHIDs(array($this->phid)) + ->executeOne(); if (!$file) { return new Aphront404Response(); } @@ -25,7 +26,6 @@ final class PhabricatorXHProfProfileController throw new Exception("Failed to unserialize XHProf profile!"); } - $request = $this->getRequest(); $symbol = $request->getStr('symbol'); $is_framed = $request->getBool('frame');