Use PhrictionDocumentQuery to load documents

Summary: Ref T4029. We use a lot of very outdated content loading in Phriction, which blocks T4029.

Test Plan:
- Called phriction.info
- Called phriction.history
- Called phriction.edit
- Viewed document list.
- Deleted a document.
- Viewed history.
- Viewed a diff.
- Created a document.
- Edited a document.
- Moved a document.
- Tried to overwrite a document with "new".
- Tried to overwrite a document with "move".
- Viewed a moved document note.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: shadowhand, epriestley

Maniphest Tasks: T4029

Differential Revision: https://secure.phabricator.com/D9194

src/applications/phriction/conduit/ConduitAPI_phriction_edit_Method.php

@@ -1,8 +1,5 @@
 <?php
 
-/**
- * @group conduit
- */
 final class ConduitAPI_phriction_edit_Method
   extends ConduitAPI_phriction_Method {
 
@@ -31,6 +28,19 @@ final class ConduitAPI_phriction_edit_Method
   protected function execute(ConduitAPIRequest $request) {
     $slug = $request->getValue('slug');
 
+    $doc = id(new PhrictionDocumentQuery())
+      ->setViewer($request->getUser())
+      ->withSlugs(array(PhabricatorSlug::normalize($slug)))
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->executeOne();
+    if (!$doc) {
+      throw new Exception(pht('No such document.'));
+    }
+
     $editor = id(PhrictionDocumentEditor::newForSlug($slug))
       ->setActor($request->getUser())
       ->setTitle($request->getValue('title'))

src/applications/phriction/conduit/ConduitAPI_phriction_history_Method.php

@@ -1,13 +1,10 @@
 <?php
 
-/**
- * @group conduit
- */
 final class ConduitAPI_phriction_history_Method
   extends ConduitAPI_phriction_Method {
 
   public function getMethodDescription() {
-    return "Retrieve history about a Phriction docuemnt.";
+    return pht('Retrieve history about a Phriction document.');
   }
 
   public function defineParamTypes() {
@@ -28,9 +25,10 @@ final class ConduitAPI_phriction_history_Method
 
   protected function execute(ConduitAPIRequest $request) {
     $slug = $request->getValue('slug');
-    $doc = id(new PhrictionDocument())->loadOneWhere(
-      'slug = %s',
-      PhabricatorSlug::normalize($slug));
+    $doc = id(new PhrictionDocumentQuery())
+      ->setViewer($request->getUser())
+      ->withSlugs(array(PhabricatorSlug::normalize($slug)))
+      ->executeOne();
     if (!$doc) {
       throw new ConduitException('ERR-BAD-DOCUMENT');
     }

src/applications/phriction/conduit/ConduitAPI_phriction_info_Method.php

@@ -1,13 +1,10 @@
 <?php
 
-/**
- * @group conduit
- */
 final class ConduitAPI_phriction_info_Method
   extends ConduitAPI_phriction_Method {
 
   public function getMethodDescription() {
-    return "Retrieve information about a Phriction document.";
+    return pht('Retrieve information about a Phriction document.');
   }
 
   public function defineParamTypes() {
@@ -29,18 +26,18 @@ final class ConduitAPI_phriction_info_Method
   protected function execute(ConduitAPIRequest $request) {
     $slug = $request->getValue('slug');
 
-    $doc = id(new PhrictionDocument())->loadOneWhere(
-      'slug = %s',
-      PhabricatorSlug::normalize($slug));
-
-    if (!$doc) {
+    $document = id(new PhrictionDocumentQuery())
+      ->setViewer($request->getUser())
+      ->withSlugs(array(PhabricatorSlug::normalize($slug)))
+      ->needContent(true)
+      ->executeOne();
+    if (!$document) {
       throw new ConduitException('ERR-BAD-DOCUMENT');
     }
 
-    $content = id(new PhrictionContent())->load($doc->getContentID());
-    $doc->attachContent($content);
-
-    return $this->buildDocumentInfoDictionary($doc);
+    return $this->buildDocumentInfoDictionary(
+      $document,
+      $document->getContent());
   }
 
 }

src/applications/phriction/controller/PhrictionController.php

@@ -56,9 +56,10 @@ abstract class PhrictionController extends PhabricatorController {
     $ancestral_slugs[] = $slug;
     if ($ancestral_slugs) {
       $empty_slugs = array_fill_keys($ancestral_slugs, null);
-      $ancestors = id(new PhrictionDocument())->loadAllWhere(
-        'slug IN (%Ls)',
-        $ancestral_slugs);
+      $ancestors = id(new PhrictionDocumentQuery())
+        ->setViewer($this->getRequest()->getUser())
+        ->withSlugs($ancestral_slugs)
+        ->execute();
       $ancestors = mpull($ancestors, null, 'getSlug');
 
       $ancestor_phids = mpull($ancestors, 'getPHID');

src/applications/phriction/controller/PhrictionDeleteController.php

@@ -1,8 +1,5 @@
 <?php
 
-/**
- * @group phriction
- */
 final class PhrictionDeleteController extends PhrictionController {
 
   private $id;
@@ -12,11 +9,18 @@ final class PhrictionDeleteController extends PhrictionController {
   }
 
   public function processRequest() {
-
     $request = $this->getRequest();
     $user = $request->getUser();
 
-    $document = id(new PhrictionDocument())->load($this->id);
+    $document = id(new PhrictionDocumentQuery())
+      ->setViewer($user)
+      ->withIDs(array($this->id))
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_EDIT,
+          PhabricatorPolicyCapability::CAN_VIEW,
+        ))
+      ->executeOne();
     if (!$document) {
       return new Aphront404Response();
     }

src/applications/phriction/controller/PhrictionDiffController.php

@@ -13,16 +13,19 @@ final class PhrictionDiffController
   }
 
   public function processRequest() {
-
     $request = $this->getRequest();
     $user = $request->getUser();
 
-    $document = id(new PhrictionDocument())->load($this->id);
+    $document = id(new PhrictionDocumentQuery())
+      ->setViewer($user)
+      ->withIDs(array($this->id))
+      ->needContent(true)
+      ->executeOne();
     if (!$document) {
       return new Aphront404Response();
     }
 
-    $current = id(new PhrictionContent())->load($document->getContentID());
+    $current = $document->getContent();
 
     $l = $request->getInt('l');
     $r = $request->getInt('r');

src/applications/phriction/controller/PhrictionDocumentController.php

@@ -115,8 +115,10 @@ final class PhrictionDocumentController
         $core_content = $notice->render();
       } else if ($current_status == PhrictionChangeType::CHANGE_MOVE_AWAY) {
         $new_doc_id = $content->getChangeRef();
-        $new_doc = new PhrictionDocument();
-        $new_doc->load($new_doc_id);
+        $new_doc = id(new PhrictionDocumentQuery())
+          ->setViewer($user)
+          ->withIDs(array($new_doc_id))
+          ->exectueOne();
 
         $slug_uri = PhrictionDocument::getSlugURI($new_doc->getSlug());
 
@@ -135,7 +137,10 @@ final class PhrictionDocumentController
       $move_notice = null;
       if ($current_status == PhrictionChangeType::CHANGE_MOVE_HERE) {
         $from_doc_id = $content->getChangeRef();
-        $from_doc = id(new PhrictionDocument())->load($from_doc_id);
+        $from_doc = id(new PhrictionDocumentQuery())
+          ->setViewer($user)
+          ->withIDs(array($from_doc_id))
+          ->executeOne();
         $slug_uri = PhrictionDocument::getSlugURI($from_doc->getSlug());
 
         $move_notice = id(new AphrontErrorView())

src/applications/phriction/controller/PhrictionEditController.php

@@ -1,8 +1,5 @@
 <?php
 
-/**
- * @group phriction
- */
 final class PhrictionEditController
   extends PhrictionController {
 
@@ -18,7 +15,15 @@ final class PhrictionEditController
     $user = $request->getUser();
 
     if ($this->id) {
-      $document = id(new PhrictionDocument())->load($this->id);
+      $document = id(new PhrictionDocumentQuery())
+        ->setViewer($user)
+        ->withIDs(array($this->id))
+        ->requireCapabilities(
+          array(
+            PhabricatorPolicyCapability::CAN_VIEW,
+            PhabricatorPolicyCapability::CAN_EDIT,
+          ))
+        ->executeOne();
       if (!$document) {
         return new Aphront404Response();
       }
@@ -43,12 +48,14 @@ final class PhrictionEditController
         return new Aphront404Response();
       }
 
-      $document = id(new PhrictionDocument())->loadOneWhere(
-        'slug = %s',
-        $slug);
+      $document = id(new PhrictionDocumentQuery())
+        ->setViewer($user)
+        ->withSlugs(array($slug))
+        ->needContent(true)
+        ->executeOne();
 
       if ($document) {
-        $content = id(new PhrictionContent())->load($document->getContentID());
+        $content = $document->getContent();
       } else {
         if (PhrictionDocument::isProjectSlug($slug)) {
           $project = id(new PhabricatorProjectQuery())

src/applications/phriction/controller/PhrictionHistoryController.php

@@ -1,8 +1,5 @@
 <?php
 
-/**
- * @group phriction
- */
 final class PhrictionHistoryController
   extends PhrictionController {
 
@@ -17,15 +14,16 @@ final class PhrictionHistoryController
     $request = $this->getRequest();
     $user = $request->getUser();
 
-    $document = id(new PhrictionDocument())->loadOneWhere(
-      'slug = %s',
-      PhabricatorSlug::normalize($this->slug));
-
+    $document = id(new PhrictionDocumentQuery())
+      ->setViewer($user)
+      ->withSlugs(array(PhabricatorSlug::normalize($this->slug)))
+      ->needContent(true)
+      ->executeOne();
     if (!$document) {
       return new Aphront404Response();
     }
 
-    $current = id(new PhrictionContent())->load($document->getContentID());
+    $current = $document->getContent();
 
     $pager = new AphrontPagerView();
     $pager->setOffset($request->getInt('page'));

src/applications/phriction/controller/PhrictionMoveController.php

@@ -17,7 +17,15 @@ final class PhrictionMoveController
     $user = $request->getUser();
 
     if ($this->id) {
-      $document = id(new PhrictionDocument())->load($this->id);
+      $document = id(new PhrictionDocumentQuery())
+        ->setViewer($user)
+        ->withIDs(array($this->id))
+        ->requireCapabilities(
+          array(
+            PhabricatorPolicyCapability::CAN_VIEW,
+            PhabricatorPolicyCapability::CAN_EDIT,
+          ))
+        ->executeOne();
     } else {
       $slug = PhabricatorSlug::normalize(
         $request->getStr('slug'));
@@ -25,9 +33,15 @@ final class PhrictionMoveController
         return new Aphront404Response();
       }
 
-      $document = id(new PhrictionDocument())->loadOneWhere(
-        'slug = %s',
-        $slug);
+      $document = id(new PhrictionDocumentQuery())
+        ->setViewer($user)
+        ->withSlugs(array($slug))
+        ->requireCapabilities(
+          array(
+            PhabricatorPolicyCapability::CAN_VIEW,
+            PhabricatorPolicyCapability::CAN_EDIT,
+          ))
+        ->executeOne();
     }
 
     if (!$document) {
@@ -68,9 +82,13 @@ final class PhrictionMoveController
 
     if ($request->isFormPost() && !count($errors)) {
       if (!count($errors)) { // First check if the target document exists
-        $target_document = id(new PhrictionDocument())->loadOneWhere(
-          'slug = %s',
-          $target_slug);
+
+        // NOTE: We use the ominpotent user because we can't let users overwrite
+        // documents even if they can't see them.
+        $target_document = id(new PhrictionDocumentQuery())
+          ->setViewer(PhabricatorUser::getOmnipotentUser())
+          ->withSlugs(array($target_slug))
+          ->executeOne();
 
         // Considering to overwrite existing docs? Nuke this!
         if ($target_document && $target_document->getStatus() ==

src/applications/phriction/controller/PhrictionNewController.php

@@ -1,20 +1,17 @@
 <?php
 
-/**
- * @group phriction
- */
 final class PhrictionNewController extends PhrictionController {
 
   public function processRequest() {
-
     $request = $this->getRequest();
     $user    = $request->getUser();
     $slug    = PhabricatorSlug::normalize($request->getStr('slug'));
 
     if ($request->isFormPost()) {
-      $document = id(new PhrictionDocument())->loadOneWhere(
-        'slug = %s',
-        $slug);
+      $document = id(new PhrictionDocumentQuery())
+        ->setViewer($user)
+        ->withSlugs(array($slug))
+        ->executeOne();
       $prompt = $request->getStr('prompt', 'no');
       $document_exists = $document && $document->getStatus() ==
         PhrictionDocumentStatus::STATUS_EXISTS;

src/applications/phriction/editor/PhrictionDocumentEditor.php

@@ -23,6 +23,8 @@ final class PhrictionDocumentEditor extends PhabricatorEditor {
 
   public static function newForSlug($slug) {
     $slug = PhabricatorSlug::normalize($slug);
+
+    // TODO: Get rid of this.
     $document = id(new PhrictionDocument())->loadOneWhere(
       'slug = %s',
       $slug);

src/applications/phriction/phid/PhrictionPHIDTypeDocument.php

@@ -25,6 +25,7 @@ final class PhrictionPHIDTypeDocument extends PhabricatorPHIDType {
     array $phids) {
 
     return id(new PhrictionDocumentQuery())
+      ->needContent(true)
       ->withPHIDs($phids);
   }
 

src/applications/phriction/query/PhrictionDocumentQuery.php

@@ -1,8 +1,5 @@
 <?php
 
-/**
- * @group phriction
- */
 final class PhrictionDocumentQuery
   extends PhabricatorCursorPagedPolicyAwareQuery {
 
@@ -10,6 +7,8 @@ final class PhrictionDocumentQuery
   private $phids;
   private $slugs;
 
+  private $needContent;
+
   private $status       = 'status-any';
   const STATUS_ANY      = 'status-any';
   const STATUS_OPEN     = 'status-open';
@@ -39,6 +38,11 @@ final class PhrictionDocumentQuery
     return $this;
   }
 
+  public function needContent($need_content) {
+    $this->needContent = $need_content;
+    return $this;
+  }
+
   public function setOrder($order) {
     $this->order = $order;
     return $this;
@@ -60,17 +64,19 @@ final class PhrictionDocumentQuery
   }
 
   protected function willFilterPage(array $documents) {
-    $contents = id(new PhrictionContent())->loadAllWhere(
-      'id IN (%Ld)',
-      mpull($documents, 'getContentID'));
+    if ($this->needContent) {
+      $contents = id(new PhrictionContent())->loadAllWhere(
+        'id IN (%Ld)',
+        mpull($documents, 'getContentID'));
 
-    foreach ($documents as $key => $document) {
-      $content_id = $document->getContentID();
-      if (empty($contents[$content_id])) {
-        unset($documents[$key]);
-        continue;
+      foreach ($documents as $key => $document) {
+        $content_id = $document->getContentID();
+        if (empty($contents[$content_id])) {
+          unset($documents[$key]);
+          continue;
+        }
+        $document->attachContent($contents[$content_id]);
       }
-      $document->attachContent($contents[$content_id]);
     }
 
     foreach ($documents as $document) {

src/applications/phriction/query/PhrictionSearchEngine.php

@@ -14,6 +14,7 @@ final class PhrictionSearchEngine
 
   public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
     $query = id(new PhrictionDocumentQuery())
+      ->needContent(true)
       ->withStatus(PhrictionDocumentQuery::STATUS_NONSTUB);
 
     $status = $saved->getParameter('status');

src/applications/phriction/search/PhrictionSearchIndexer.php

@@ -1,8 +1,5 @@
 <?php
 
-/**
- * @group phriction
- */
 final class PhrictionSearchIndexer
   extends PhabricatorSearchDocumentIndexer {