Page MenuHomePhabricator

D20293.id.diff
No OneTemporary

D20293.id.diff

diff --git a/src/applications/almanac/query/AlmanacInterfaceQuery.php b/src/applications/almanac/query/AlmanacInterfaceQuery.php
--- a/src/applications/almanac/query/AlmanacInterfaceQuery.php
+++ b/src/applications/almanac/query/AlmanacInterfaceQuery.php
@@ -78,6 +78,16 @@
return $interfaces;
}
+ protected function buildSelectClauseParts(AphrontDatabaseConnection $conn) {
+ $select = parent::buildSelectClauseParts($conn);
+
+ if ($this->shouldJoinDeviceTable()) {
+ $select[] = qsprintf($conn, 'device.name');
+ }
+
+ return $select;
+ }
+
protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
$where = parent::buildWhereClauseParts($conn);
@@ -186,15 +196,16 @@
);
}
- protected function getPagingValueMap($cursor, array $keys) {
- $interface = $this->loadCursorObject($cursor);
+ protected function newPagingMapFromCursorObject(
+ PhabricatorQueryCursor $cursor,
+ array $keys) {
- $map = array(
- 'id' => $interface->getID(),
- 'name' => $interface->getDevice()->getName(),
- );
+ $interface = $cursor->getObject();
- return $map;
+ return array(
+ 'id' => (int)$interface->getID(),
+ 'name' => $cursor->getRawRowProperty('device.name'),
+ );
}
}
diff --git a/src/applications/feed/query/PhabricatorFeedQuery.php b/src/applications/feed/query/PhabricatorFeedQuery.php
--- a/src/applications/feed/query/PhabricatorFeedQuery.php
+++ b/src/applications/feed/query/PhabricatorFeedQuery.php
@@ -147,17 +147,21 @@
);
}
- protected function getPagingValueMap($cursor, array $keys) {
- return array(
- 'key' => $cursor,
- );
+ protected function applyExternalCursorConstraintsToQuery(
+ PhabricatorCursorPagedPolicyAwareQuery $subquery,
+ $cursor) {
+ $subquery->withChronologicalKeys(array($cursor));
}
- protected function getResultCursor($item) {
- if ($item instanceof PhabricatorFeedStory) {
- return $item->getChronologicalKey();
- }
- return $item['chronologicalKey'];
+ protected function newExternalCursorStringForResult($object) {
+ return $object->getChronologicalKey();
+ }
+
+ protected function newPagingMapFromPartialObject($object) {
+ // This query is unusual, and the "object" is a raw result row.
+ return array(
+ 'key' => $object['chronologicalKey'],
+ );
}
protected function getPrimaryTableAlias() {
diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php
--- a/src/applications/maniphest/query/ManiphestTaskQuery.php
+++ b/src/applications/maniphest/query/ManiphestTaskQuery.php
@@ -27,6 +27,7 @@
private $closedEpochMax;
private $closerPHIDs;
private $columnPHIDs;
+ private $specificGroupByProjectPHID;
private $status = 'status-any';
const STATUS_ANY = 'status-any';
@@ -227,6 +228,11 @@
return $this;
}
+ public function withSpecificGroupByProjectPHID($project_phid) {
+ $this->specificGroupByProjectPHID = $project_phid;
+ return $this;
+ }
+
public function newResultObject() {
return new ManiphestTask();
}
@@ -534,6 +540,13 @@
$select_phids);
}
+ if ($this->specificGroupByProjectPHID !== null) {
+ $where[] = qsprintf(
+ $conn,
+ 'projectGroupName.indexedObjectPHID = %s',
+ $this->specificGroupByProjectPHID);
+ }
+
return $where;
}
@@ -824,16 +837,6 @@
return array_mergev($phids);
}
- protected function getResultCursor($result) {
- $id = $result->getID();
-
- if ($this->groupBy == self::GROUP_PROJECT) {
- return rtrim($id.'.'.$result->getGroupByProjectPHID(), '.');
- }
-
- return $id;
- }
-
public function getBuiltinOrders() {
$orders = array(
'priority' => array(
@@ -926,39 +929,37 @@
);
}
- protected function getPagingValueMap($cursor, array $keys) {
- $cursor_parts = explode('.', $cursor, 2);
- $task_id = $cursor_parts[0];
- $group_id = idx($cursor_parts, 1);
+ protected function newPagingMapFromCursorObject(
+ PhabricatorQueryCursor $cursor,
+ array $keys) {
- $task = $this->loadCursorObject($task_id);
+ $task = $cursor->getObject();
$map = array(
- 'id' => $task->getID(),
- 'priority' => $task->getPriority(),
+ 'id' => (int)$task->getID(),
+ 'priority' => (int)$task->getPriority(),
'owner' => $task->getOwnerOrdering(),
'status' => $task->getStatus(),
'title' => $task->getTitle(),
- 'updated' => $task->getDateModified(),
+ 'updated' => (int)$task->getDateModified(),
'closed' => $task->getClosedEpoch(),
);
- foreach ($keys as $key) {
- switch ($key) {
- case 'project':
- $value = null;
- if ($group_id) {
- $paging_projects = id(new PhabricatorProjectQuery())
- ->setViewer($this->getViewer())
- ->withPHIDs(array($group_id))
- ->execute();
- if ($paging_projects) {
- $value = head($paging_projects)->getName();
- }
- }
- $map[$key] = $value;
- break;
+ if (isset($keys['project'])) {
+ $value = null;
+
+ $group_phid = $task->getGroupByProjectPHID();
+ if ($group_phid) {
+ $paging_projects = id(new PhabricatorProjectQuery())
+ ->setViewer($this->getViewer())
+ ->withPHIDs(array($group_phid))
+ ->execute();
+ if ($paging_projects) {
+ $value = head($paging_projects)->getName();
+ }
}
+
+ $map['project'] = $value;
}
foreach ($keys as $key) {
@@ -971,6 +972,77 @@
return $map;
}
+ protected function newExternalCursorStringForResult($object) {
+ $id = $object->getID();
+
+ if ($this->groupBy == self::GROUP_PROJECT) {
+ return rtrim($id.'.'.$object->getGroupByProjectPHID(), '.');
+ }
+
+ return $id;
+ }
+
+ protected function newInternalCursorFromExternalCursor($cursor) {
+ list($task_id, $group_phid) = $this->parseCursor($cursor);
+
+ $cursor_object = parent::newInternalCursorFromExternalCursor($cursor);
+
+ if ($group_phid !== null) {
+ $project = id(new PhabricatorProjectQuery())
+ ->setViewer($this->getViewer())
+ ->withPHIDs(array($group_phid))
+ ->execute();
+
+ if (!$project) {
+ $this->throwCursorException(
+ pht(
+ 'Group PHID ("%s") component of cursor ("%s") is not valid.',
+ $group_phid,
+ $cursor));
+ }
+
+ $cursor_object->getObject()->attachGroupByProjectPHID($group_phid);
+ }
+
+ return $cursor_object;
+ }
+
+ protected function applyExternalCursorConstraintsToQuery(
+ PhabricatorCursorPagedPolicyAwareQuery $subquery,
+ $cursor) {
+ list($task_id, $group_phid) = $this->parseCursor($cursor);
+
+ $subquery->withIDs(array($task_id));
+
+ if ($group_phid) {
+ $subquery->setGroupBy(self::GROUP_PROJECT);
+
+ // The subquery needs to return exactly one result. If a task is in
+ // several projects, the query may naturally return several results.
+ // Specify that we want only the particular instance of the task in
+ // the specified project.
+ $subquery->withSpecificGroupByProjectPHID($group_phid);
+ }
+ }
+
+
+ private function parseCursor($cursor) {
+ // Split a "123.PHID-PROJ-abcd" cursor into a "Task ID" part and a
+ // "Project PHID" part.
+
+ $parts = explode('.', $cursor, 2);
+
+ if (count($parts) < 2) {
+ $parts[] = null;
+ }
+
+ if (!strlen($parts[1])) {
+ $parts[1] = null;
+ }
+
+ return $parts;
+ }
+
protected function getPrimaryTableAlias() {
return 'task';
}
diff --git a/src/applications/phriction/query/PhrictionDocumentQuery.php b/src/applications/phriction/query/PhrictionDocumentQuery.php
--- a/src/applications/phriction/query/PhrictionDocumentQuery.php
+++ b/src/applications/phriction/query/PhrictionDocumentQuery.php
@@ -168,10 +168,20 @@
return $documents;
}
+ protected function buildSelectClauseParts(AphrontDatabaseConnection $conn) {
+ $select = parent::buildSelectClauseParts($conn);
+
+ if ($this->shouldJoinContentTable()) {
+ $select[] = qsprintf($conn, 'c.title');
+ }
+
+ return $select;
+ }
+
protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
$joins = parent::buildJoinClauseParts($conn);
- if ($this->getOrderVector()->containsKey('updated')) {
+ if ($this->shouldJoinContentTable()) {
$content_dao = new PhrictionContent();
$joins[] = qsprintf(
$conn,
@@ -182,6 +192,10 @@
return $joins;
}
+ private function shouldJoinContentTable() {
+ return $this->getOrderVector()->containsKey('updated');
+ }
+
protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
$where = parent::buildWhereClauseParts($conn);
@@ -354,35 +368,25 @@
);
}
- protected function getPagingValueMap($cursor, array $keys) {
- $document = $this->loadCursorObject($cursor);
+ protected function newPagingMapFromCursorObject(
+ PhabricatorQueryCursor $cursor,
+ array $keys) {
+
+ $document = $cursor->getObject();
$map = array(
- 'id' => $document->getID(),
+ 'id' => (int)$document->getID(),
'depth' => $document->getDepth(),
- 'updated' => $document->getEditedEpoch(),
+ 'updated' => (int)$document->getEditedEpoch(),
);
- foreach ($keys as $key) {
- switch ($key) {
- case 'title':
- $map[$key] = $document->getContent()->getTitle();
- break;
- }
+ if (isset($keys['title'])) {
+ $map['title'] = $cursor->getRawRowProperty('c.title');
}
return $map;
}
- protected function willExecuteCursorQuery(
- PhabricatorCursorPagedPolicyAwareQuery $query) {
- $vector = $this->getOrderVector();
-
- if ($vector->containsKey('title')) {
- $query->needContent(true);
- }
- }
-
protected function getPrimaryTableAlias() {
return 'd';
}
diff --git a/src/applications/repository/query/PhabricatorRepositoryQuery.php b/src/applications/repository/query/PhabricatorRepositoryQuery.php
--- a/src/applications/repository/query/PhabricatorRepositoryQuery.php
+++ b/src/applications/repository/query/PhabricatorRepositoryQuery.php
@@ -442,47 +442,24 @@
);
}
- protected function willExecuteCursorQuery(
- PhabricatorCursorPagedPolicyAwareQuery $query) {
- $vector = $this->getOrderVector();
+ protected function newPagingMapFromCursorObject(
+ PhabricatorQueryCursor $cursor,
+ array $keys) {
- if ($vector->containsKey('committed')) {
- $query->needMostRecentCommits(true);
- }
-
- if ($vector->containsKey('size')) {
- $query->needCommitCounts(true);
- }
- }
-
- protected function getPagingValueMap($cursor, array $keys) {
- $repository = $this->loadCursorObject($cursor);
+ $repository = $cursor->getObject();
$map = array(
- 'id' => $repository->getID(),
+ 'id' => (int)$repository->getID(),
'callsign' => $repository->getCallsign(),
'name' => $repository->getName(),
);
- foreach ($keys as $key) {
- switch ($key) {
- case 'committed':
- $commit = $repository->getMostRecentCommit();
- if ($commit) {
- $map[$key] = $commit->getEpoch();
- } else {
- $map[$key] = null;
- }
- break;
- case 'size':
- $count = $repository->getCommitCount();
- if ($count) {
- $map[$key] = $count;
- } else {
- $map[$key] = null;
- }
- break;
- }
+ if (isset($keys['committed'])) {
+ $map['committed'] = $cursor->getRawRowProperty('epoch');
+ }
+
+ if (isset($keys['size'])) {
+ $map['size'] = $cursor->getRawRowProperty('size');
}
return $map;
@@ -491,8 +468,6 @@
protected function buildSelectClauseParts(AphrontDatabaseConnection $conn) {
$parts = parent::buildSelectClauseParts($conn);
- $parts[] = qsprintf($conn, 'r.*');
-
if ($this->shouldJoinSummaryTable()) {
$parts[] = qsprintf($conn, 's.*');
}
diff --git a/src/infrastructure/edges/conduit/PhabricatorEdgeObject.php b/src/infrastructure/edges/conduit/PhabricatorEdgeObject.php
--- a/src/infrastructure/edges/conduit/PhabricatorEdgeObject.php
+++ b/src/infrastructure/edges/conduit/PhabricatorEdgeObject.php
@@ -8,14 +8,18 @@
private $src;
private $dst;
private $type;
+ private $dateCreated;
+ private $sequence;
public static function newFromRow(array $row) {
$edge = new self();
- $edge->id = $row['id'];
- $edge->src = $row['src'];
- $edge->dst = $row['dst'];
- $edge->type = $row['type'];
+ $edge->id = idx($row, 'id');
+ $edge->src = idx($row, 'src');
+ $edge->dst = idx($row, 'dst');
+ $edge->type = idx($row, 'type');
+ $edge->dateCreated = idx($row, 'dateCreated');
+ $edge->sequence = idx($row, 'seq');
return $edge;
}
@@ -40,6 +44,15 @@
return null;
}
+ public function getDateCreated() {
+ return $this->dateCreated;
+ }
+
+ public function getSequence() {
+ return $this->sequence;
+ }
+
+
/* -( PhabricatorPolicyInterface )----------------------------------------- */
diff --git a/src/infrastructure/edges/query/PhabricatorEdgeObjectQuery.php b/src/infrastructure/edges/query/PhabricatorEdgeObjectQuery.php
--- a/src/infrastructure/edges/query/PhabricatorEdgeObjectQuery.php
+++ b/src/infrastructure/edges/query/PhabricatorEdgeObjectQuery.php
@@ -12,7 +12,6 @@
private $edgeTypes;
private $destinationPHIDs;
-
public function withSourcePHIDs(array $source_phids) {
$this->sourcePHIDs = $source_phids;
return $this;
@@ -85,18 +84,6 @@
return $result;
}
- protected function buildSelectClauseParts(AphrontDatabaseConnection $conn) {
- $parts = parent::buildSelectClauseParts($conn);
-
- // TODO: This is hacky, because we don't have real IDs on this table.
- $parts[] = qsprintf(
- $conn,
- 'CONCAT(dateCreated, %s, seq) AS id',
- '_');
-
- return $parts;
- }
-
protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
$parts = parent::buildWhereClauseParts($conn);
@@ -151,13 +138,45 @@
return array('dateCreated', 'sequence');
}
- protected function getPagingValueMap($cursor, array $keys) {
- $parts = explode('_', $cursor);
+ protected function newInternalCursorFromExternalCursor($cursor) {
+ list($epoch, $sequence) = $this->parseCursor($cursor);
+
+ // Instead of actually loading an edge, we're just making a fake edge
+ // with the properties the cursor describes.
+
+ $edge_object = PhabricatorEdgeObject::newFromRow(
+ array(
+ 'dateCreated' => $epoch,
+ 'seq' => $sequence,
+ ));
+ return id(new PhabricatorQueryCursor())
+ ->setObject($edge_object);
+ }
+
+ protected function newPagingMapFromPartialObject($object) {
return array(
- 'dateCreated' => $parts[0],
- 'sequence' => $parts[1],
+ 'dateCreated' => $object->getDateCreated(),
+ 'sequence' => $object->getSequence(),
);
}
+ protected function newExternalCursorStringForResult($object) {
+ return sprintf(
+ '%d_%d',
+ $object->getDateCreated(),
+ $object->getSequence());
+ }
+
+ private function parseCursor($cursor) {
+ if (!preg_match('/^\d+_\d+\z/', $cursor)) {
+ $this->throwCursorException(
+ pht(
+ 'Expected edge cursor in the form "0123_6789", got "%s".',
+ $cursor));
+ }
+
+ return explode('_', $cursor);
+ }
+
}
diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
--- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
+++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
@@ -19,6 +19,7 @@
private $externalCursorString;
private $internalCursorObject;
private $isQueryOrderReversed = false;
+ private $rawCursorRow;
private $applicationSearchConstraints = array();
private $internalPaging;
@@ -53,33 +54,13 @@
}
protected function newInternalCursorFromExternalCursor($cursor) {
- return $this->newInternalCursorObjectFromID($cursor);
- }
-
- protected function newPagingMapFromCursorObject(
- PhabricatorQueryCursor $cursor,
- array $keys) {
-
- $object = $cursor->getObject();
-
- return $this->newPagingMapFromPartialObject($object);
- }
-
- protected function newPagingMapFromPartialObject($object) {
- return array(
- 'id' => (int)$object->getID(),
- );
- }
-
- final protected function newInternalCursorObjectFromID($id) {
$viewer = $this->getViewer();
$query = newv(get_class($this), array());
$query
->setParentQuery($this)
- ->setViewer($viewer)
- ->withIDs(array((int)$id));
+ ->setViewer($viewer);
// We're copying our order vector to the subquery so that the subquery
// knows it should generate any supplemental information required by the
@@ -96,18 +77,18 @@
// like a cursor this parent query would generate.
$query->setOrderVector($this->getOrderVector());
+ $this->applyExternalCursorConstraintsToQuery($query, $cursor);
+
// We're executing the subquery normally to make sure the viewer can
// actually see the object, and that it's a completely valid object which
// passes all filtering and policy checks. You aren't allowed to use an
// object you can't see as a cursor, since this can leak information.
$result = $query->executeOne();
if (!$result) {
- // TODO: Raise a more tailored exception here and make the UI a little
- // prettier?
- throw new Exception(
+ $this->throwCursorException(
pht(
'Cursor "%s" does not identify a valid object in query "%s".',
- $id,
+ $cursor,
get_class($this)));
}
@@ -117,6 +98,34 @@
return $query->getInternalCursorObject();
}
+ final protected function throwCursorException($message) {
+ // TODO: Raise a more tailored exception here and make the UI a little
+ // prettier?
+ throw new Exception($message);
+ }
+
+ protected function applyExternalCursorConstraintsToQuery(
+ PhabricatorCursorPagedPolicyAwareQuery $subquery,
+ $cursor) {
+ $subquery->withIDs(array($cursor));
+ }
+
+ protected function newPagingMapFromCursorObject(
+ PhabricatorQueryCursor $cursor,
+ array $keys) {
+
+ $object = $cursor->getObject();
+
+ return $this->newPagingMapFromPartialObject($object);
+ }
+
+ protected function newPagingMapFromPartialObject($object) {
+ return array(
+ 'id' => (int)$object->getID(),
+ );
+ }
+
+
final private function getExternalCursorStringForResult($object) {
$cursor = $this->newExternalCursorStringForResult($object);
@@ -215,6 +224,10 @@
$cursor = id(new PhabricatorQueryCursor())
->setObject(last($page));
+ if ($this->rawCursorRow) {
+ $cursor->setRawRow($this->rawCursorRow);
+ }
+
$this->setInternalCursorObject($cursor);
}
@@ -295,46 +308,9 @@
}
}
- return $rows;
- }
+ $this->rawCursorRow = last($rows);
- /**
- * Get the viewer for making cursor paging queries.
- *
- * NOTE: You should ONLY use this viewer to load cursor objects while
- * building paging queries.
- *
- * Cursor paging can happen in two ways. First, the user can request a page
- * like `/stuff/?after=33`, which explicitly causes paging. Otherwise, we
- * can fall back to implicit paging if we filter some results out of a
- * result list because the user can't see them and need to go fetch some more
- * results to generate a large enough result list.
- *
- * In the first case, want to use the viewer's policies to load the object.
- * This prevents an attacker from figuring out information about an object
- * they can't see by executing queries like `/stuff/?after=33&order=name`,
- * which would otherwise give them a hint about the name of the object.
- * Generally, if a user can't see an object, they can't use it to page.
- *
- * In the second case, we need to load the object whether the user can see
- * it or not, because we need to examine new results. For example, if a user
- * loads `/stuff/` and we run a query for the first 100 items that they can
- * see, but the first 100 rows in the database aren't visible, we need to
- * be able to issue a query for the next 100 results. If we can't load the
- * cursor object, we'll fail or issue the same query over and over again.
- * So, generally, internal paging must bypass policy controls.
- *
- * This method returns the appropriate viewer, based on the context in which
- * the paging is occurring.
- *
- * @return PhabricatorUser Viewer for executing paging queries.
- */
- final protected function getPagingViewer() {
- if ($this->internalPaging) {
- return PhabricatorUser::getOmnipotentUser();
- } else {
- return $this->getViewer();
- }
+ return $rows;
}
final protected function buildLimitClause(AphrontDatabaseConnection $conn) {
@@ -590,6 +566,7 @@
foreach ($vector as $order) {
$keys[] = $order->getOrderKey();
}
+ $keys = array_fuse($keys);
$value_map = $this->getPagingMapFromCursorObject(
$cursor_object,
diff --git a/src/infrastructure/query/policy/PhabricatorQueryCursor.php b/src/infrastructure/query/policy/PhabricatorQueryCursor.php
--- a/src/infrastructure/query/policy/PhabricatorQueryCursor.php
+++ b/src/infrastructure/query/policy/PhabricatorQueryCursor.php
@@ -4,6 +4,7 @@
extends Phobject {
private $object;
+ private $rawRow;
public function setObject($object) {
$this->object = $object;
@@ -14,4 +15,33 @@
return $this->object;
}
+ public function setRawRow(array $raw_row) {
+ $this->rawRow = $raw_row;
+ return $this;
+ }
+
+ public function getRawRow() {
+ return $this->rawRow;
+ }
+
+ public function getRawRowProperty($key) {
+ if (!is_array($this->rawRow)) {
+ throw new Exception(
+ pht(
+ 'Caller is trying to "getRawRowProperty()" with key "%s", but this '.
+ 'cursor has no raw row.',
+ $key));
+ }
+
+ if (!array_key_exists($key, $this->rawRow)) {
+ throw new Exception(
+ pht(
+ 'Caller is trying to access raw row property "%s", but the row '.
+ 'does not have this property.',
+ $key));
+ }
+
+ return $this->rawRow[$key];
+ }
+
}

File Metadata

Mime Type
text/plain
Expires
Fri, Jan 24, 12:40 AM (20 h, 24 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7038392
Default Alt Text
D20293.id.diff (22 KB)

Event Timeline