Implement PhrictionDocumentQuery

Summary:
Companion for D5284. Move all the query logic to a policy-aware query class. In particular:

  - Currently, anyone can view and edit a project's wiki documents. For callsites using this query class, you must be able to view or edit the project to view or edit its documents.
  - There's some very sketchy logic with the content/document joins. This cleans that up.
  - This cleans up loading projects by moving it inside the query. We need to do this anyway to perform policy checks.

Test Plan: Viewed active/all/updated. Set page size to 2, verified pager works.

Reviewers: AnhNhan, chad

Reviewed By: AnhNhan

CC: aran

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

src/__phutil_library_map__.php

@@ -1497,6 +1497,7 @@ phutil_register_library_map(array(
     'PhrictionDocumentController' => 'applications/phriction/controller/PhrictionDocumentController.php',
     'PhrictionDocumentEditor' => 'applications/phriction/editor/PhrictionDocumentEditor.php',
     'PhrictionDocumentPreviewController' => 'applications/phriction/controller/PhrictionDocumentPreviewController.php',
+    'PhrictionDocumentQuery' => 'applications/phriction/query/PhrictionDocumentQuery.php',
     'PhrictionDocumentStatus' => 'applications/phriction/constants/PhrictionDocumentStatus.php',
     'PhrictionDocumentTestCase' => 'applications/phriction/storage/__tests__/PhrictionDocumentTestCase.php',
     'PhrictionEditController' => 'applications/phriction/controller/PhrictionEditController.php',
@@ -3010,6 +3011,7 @@ phutil_register_library_map(array(
     'PhrictionDocumentController' => 'PhrictionController',
     'PhrictionDocumentEditor' => 'PhabricatorEditor',
     'PhrictionDocumentPreviewController' => 'PhrictionController',
+    'PhrictionDocumentQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhrictionDocumentStatus' => 'PhrictionConstants',
     'PhrictionDocumentTestCase' => 'PhabricatorTestCase',
     'PhrictionEditController' => 'PhrictionController',

src/applications/phriction/controller/PhrictionListController.php

@@ -43,14 +43,36 @@ final class PhrictionListController
         $header,
       ));
 
-    $pager = new AphrontPagerView();
+    $pager = id(new AphrontCursorPagerView())
-    $pager->setURI($request->getRequestURI(), 'page');
+      ->readFromRequest($request);
-    $pager->setOffset($request->getInt('page'));
 
-    $documents = $this->loadDocuments($pager);
+    $query = id(new PhrictionDocumentQuery())
+      ->setViewer($user);
 
-    $content = mpull($documents, 'getContent');
+    switch ($this->view) {
-    $phids = mpull($content, 'getAuthorPHID');
+      case 'active':
+        $query->withStatus(PhrictionDocumentQuery::STATUS_OPEN);
+        break;
+      case 'all':
+        $query->withStatus(PhrictionDocumentQuery::STATUS_NONSTUB);
+        break;
+      case 'updates':
+        $query->withStatus(PhrictionDocumentQuery::STATUS_NONSTUB);
+        $query->setOrder(PhrictionDocumentQuery::ORDER_UPDATED);
+        break;
+      default:
+        throw new Exception("Unknown view '{$this->view}'!");
+    }
+
+    $documents = $query->executeWithCursorPager($pager);
+
+    $phids = array();
+    foreach ($documents as $document) {
+      $phids[] = $document->getContent()->getAuthorPHID();
+      if ($document->hasProject()) {
+        $phids[] = $document->getProject()->getPHID();
+      }
+    }
 
     $handles = $this->loadViewerHandles($phids);
 
@@ -103,69 +125,4 @@ final class PhrictionListController
       ));
   }
 
-  private function loadDocuments(AphrontPagerView $pager) {
-
-    // TODO: Do we want/need a query object for this?
-
-    $document_dao = new PhrictionDocument();
-    $content_dao = new PhrictionContent();
-    $conn = $document_dao->establishConnection('r');
-
-    switch ($this->view) {
-      case 'active':
-        $data = queryfx_all(
-          $conn,
-          'SELECT * FROM %T WHERE status NOT IN (%Ld) ORDER BY id DESC '.
-            'LIMIT %d, %d',
-          $document_dao->getTableName(),
-          array(
-            PhrictionDocumentStatus::STATUS_DELETED,
-            PhrictionDocumentStatus::STATUS_MOVED,
-          ),
-          $pager->getOffset(),
-          $pager->getPageSize() + 1);
-        break;
-      case 'all':
-        $data = queryfx_all(
-          $conn,
-          'SELECT * FROM %T ORDER BY id DESC LIMIT %d, %d',
-          $document_dao->getTableName(),
-          $pager->getOffset(),
-          $pager->getPageSize() + 1);
-        break;
-      case 'updates':
-
-        // TODO: This query is a little suspicious, verify we don't need to key
-        // or change it once we get more data.
-
-        $data = queryfx_all(
-          $conn,
-          'SELECT d.* FROM %T d JOIN %T c ON c.documentID = d.id
-            GROUP BY c.documentID
-            ORDER BY MAX(c.id) DESC LIMIT %d, %d',
-          $document_dao->getTableName(),
-          $content_dao->getTableName(),
-          $pager->getOffset(),
-          $pager->getPageSize() + 1);
-        break;
-      default:
-        throw new Exception("Unknown view '{$this->view}'!");
-    }
-
-    $data = $pager->sliceResults($data);
-
-    $documents = $document_dao->loadAllFromArray($data);
-    if ($documents) {
-      $content = $content_dao->loadAllWhere(
-        'documentID IN (%Ld)',
-        mpull($documents, 'getID'));
-      $content = mpull($content, null, 'getDocumentID');
-      foreach ($documents as $document) {
-        $document->attachContent($content[$document->getID()]);
-      }
-    }
-
-    return $documents;
-  }
-
 }

src/applications/phriction/query/PhrictionDocumentQuery.php

@@ -0,0 +1,174 @@
+<?php
+
+/**
+ * @group phriction
+ */
+final class PhrictionDocumentQuery
+  extends PhabricatorCursorPagedPolicyAwareQuery {
+
+  private $ids;
+  private $phids;
+
+  private $status       = 'status-any';
+  const STATUS_ANY      = 'status-any';
+  const STATUS_OPEN     = 'status-open';
+  const STATUS_NONSTUB  = 'status-nonstub';
+
+  private $order        = 'order-created';
+  const ORDER_CREATED   = 'order-created';
+  const ORDER_UPDATED   = 'order-updated';
+
+  public function withIDs(array $ids) {
+    $this->ids = $ids;
+    return $this;
+  }
+
+  public function withPHIDs(array $phids) {
+    $this->phids = $phids;
+    return $this;
+  }
+
+  public function withStatus($status) {
+    $this->status = $status;
+    return $this;
+  }
+
+  public function setOrder($order) {
+    $this->order = $order;
+    return $this;
+  }
+
+  protected function loadPage() {
+    $document = new PhrictionDocument();
+    $conn_r = $document->establishConnection('r');
+
+    $rows = queryfx_all(
+      $conn_r,
+      'SELECT * FROM %T %Q %Q %Q',
+      $document->getTableName(),
+      $this->buildWhereClause($conn_r),
+      $this->buildOrderClause($conn_r),
+      $this->buildLimitClause($conn_r));
+
+    return $document->loadAllFromArray($rows);
+  }
+
+  protected function willFilterPage(array $documents) {
+    if (!$documents) {
+      return array();
+    }
+
+    $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;
+      }
+      $document->attachContent($contents[$content_id]);
+    }
+
+    $project_slugs = array();
+    foreach ($documents as $key => $document) {
+      $slug = $document->getSlug();
+      if (!PhrictionDocument::isProjectSlug($slug)) {
+        continue;
+      }
+      $project_slugs[$key] = PhrictionDocument::getProjectSlugIdentifier($slug);
+    }
+
+    if ($project_slugs) {
+      $projects = id(new PhabricatorProjectQuery())
+        ->setViewer($this->getViewer())
+        ->withSlugs($project_slugs)
+        ->execute();
+      $projects = mpull($projects, null, 'getPhrictionSlug');
+      foreach ($documents as $key => $document) {
+        $slug = idx($project_slugs, $key);
+        if ($slug) {
+          $project = idx($projects, $slug);
+          if (!$project) {
+            unset($documents[$key]);
+            continue;
+          }
+          $document->attachProject($project);
+        }
+      }
+    }
+
+    return $documents;
+  }
+
+  protected function buildWhereClause(AphrontDatabaseConnection $conn) {
+    $where = array();
+
+    if ($this->ids) {
+      $where[] = qsprintf(
+        $conn,
+        'id IN (%Ld)',
+        $this->ids);
+    }
+
+    if ($this->phids) {
+      $where[] = qsprintf(
+        $conn,
+        'id IN (%Ld)',
+        $this->phids);
+    }
+
+    switch ($this->status) {
+      case self::STATUS_OPEN:
+        $where[] = qsprintf(
+          $conn,
+          'status NOT IN (%Ld)',
+          array(
+            PhrictionDocumentStatus::STATUS_DELETED,
+            PhrictionDocumentStatus::STATUS_MOVED,
+            PhrictionDocumentStatus::STATUS_STUB,
+          ));
+        break;
+      case self::STATUS_NONSTUB:
+        $where[] = qsprintf(
+          $conn,
+          'status NOT IN (%Ld)',
+          array(
+            PhrictionDocumentStatus::STATUS_STUB,
+          ));
+        break;
+      case self::STATUS_ANY:
+        break;
+      default:
+        throw new Exception("Unknown status '{$this->status}'!");
+    }
+
+    $where[] = $this->buildPagingClause($conn);
+
+    return $this->formatWhereClause($where);
+  }
+
+  protected function getPagingColumn() {
+    switch ($this->order) {
+      case self::ORDER_CREATED:
+        return 'id';
+      case self::ORDER_UPDATED:
+        return 'contentID';
+      default:
+        throw new Exception("Unknown order '{$this->order}'!");
+    }
+  }
+
+  protected function getPagingValue($result) {
+    switch ($this->order) {
+      case self::ORDER_CREATED:
+        return $result->getID();
+      case self::ORDER_UPDATED:
+        return $result->getContentID();
+      default:
+        throw new Exception("Unknown order '{$this->order}'!");
+    }
+  }
+
+}

src/applications/phriction/storage/PhrictionDocument.php

@@ -14,6 +14,7 @@ final class PhrictionDocument extends PhrictionDAO
   protected $status;
 
   private $contentObject;
+  private $project;
 
   public function getConfiguration() {
     return array(
@@ -68,6 +69,22 @@ final class PhrictionDocument extends PhrictionDAO
     return $this->contentObject;
   }
 
+  public function getProject() {
+    if ($this->project === null) {
+      throw new Exception("Call attachProject() before getProject().");
+    }
+    return $this->project;
+  }
+
+  public function attachProject(PhabricatorProject $project) {
+    $this->project = $project;
+    return $this;
+  }
+
+  public function hasProject() {
+    return (bool)$this->project;
+  }
+
   public static function isProjectSlug($slug) {
     $slug = PhabricatorSlug::normalize($slug);
     $prefix = 'projects/';
@@ -88,7 +105,6 @@ final class PhrictionDocument extends PhrictionDAO
     return $parts[1].'/';
   }
 
-  // TODO: Customize this? Copypasta from PhabricatorPaste.
   public function getCapabilities() {
     return array(
       PhabricatorPolicyCapability::CAN_VIEW,
@@ -97,10 +113,16 @@ final class PhrictionDocument extends PhrictionDAO
   }
 
   public function getPolicy($capability) {
+    if ($this->hasProject()) {
+      return $this->getProject()->getPolicy($capability);
+    }
     return PhabricatorPolicies::POLICY_USER;
   }
 
   public function hasAutomaticCapability($capability, PhabricatorUser $user) {
+    if ($this->hasProject()) {
+      return $this->getProject()->hasAutomaticCapability($capability, $user);
+    }
     return false;
   }
 }

src/applications/project/query/PhabricatorProjectQuery.php

@@ -6,6 +6,7 @@ final class PhabricatorProjectQuery
   private $ids;
   private $phids;
   private $memberPHIDs;
+  private $slugs;
 
   private $status       = 'status-any';
   const STATUS_ANY      = 'status-any';
@@ -36,6 +37,11 @@ final class PhabricatorProjectQuery
     return $this;
   }
 
+  public function withPhrictionSlugs(array $slugs) {
+    $this->slugs = $slugs;
+    return $this;
+  }
+
   public function needMembers($need_members) {
     $this->needMembers = $need_members;
     return $this;
@@ -155,6 +161,13 @@ final class PhabricatorProjectQuery
         $this->memberPHIDs);
     }
 
+    if ($this->slugs) {
+      $where[] = qsprintf(
+        $conn_r,
+        'phrictionSlug IN (%Ls)',
+        $this->slugs);
+    }
+
     $where[] = $this->buildPagingClause($conn_r);
 
     return $this->formatWhereClause($where);