Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F14761342
D20293.id.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
22 KB
Referenced Files
None
Subscribers
None
D20293.id.diff
View Options
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
Details
Attached
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)
Attached To
Mode
D20293: Convert complex query subclasses to use internal cursors
Attached
Detach File
Event Timeline
Log In to Comment