Page MenuHomePhabricator
Files F13299595

D12355.diff
No OneTemporary
Actions

D12355.diff
View Options

diff --git a/src/infrastructure/query/order/PhabricatorQueryOrderVector.php b/src/infrastructure/query/order/PhabricatorQueryOrderVector.php
--- a/src/infrastructure/query/order/PhabricatorQueryOrderVector.php
+++ b/src/infrastructure/query/order/PhabricatorQueryOrderVector.php
@@ -72,7 +72,7 @@
'Order vector "%s" specifies order "%s" twice. Each component '.
'of an ordering must be unique.',
implode(', ', $vector),
- $item));
+ $item->getOrderKey()));
}
$items[$item->getOrderKey()] = $item;
@@ -84,6 +84,14 @@
return $obj;
}
+ public function getAsString() {
+ $scalars = array();
+ foreach ($this->items as $item) {
+ $scalars[] = $item->getAsScalar();
+ }
+ return implode(', ', $scalars);
+ }
+
/* -( Iterator Interface )------------------------------------------------- */
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
@@ -5,6 +5,7 @@
* performant than offset-based paging in the presence of policy filtering.
*
* @task appsearch Integration with ApplicationSearch
+ * @task paging Paging
* @task order Result Ordering
*/
abstract class PhabricatorCursorPagedPolicyAwareQuery
@@ -111,28 +112,6 @@
}
}
- protected function buildPagingClause(
- AphrontDatabaseConnection $conn_r) {
-
- if ($this->beforeID) {
- return qsprintf(
- $conn_r,
- '%Q %Q %s',
- $this->getPagingColumn(),
- $this->getReversePaging() ? '<' : '>',
- $this->beforeID);
- } else if ($this->afterID) {
- return qsprintf(
- $conn_r,
- '%Q %Q %s',
- $this->getPagingColumn(),
- $this->getReversePaging() ? '>' : '<',
- $this->afterID);
- }
-
- return null;
- }
-
final protected function didLoadResults(array $results) {
if ($this->beforeID) {
$results = array_reverse($results, $preserve_keys = true);
@@ -168,6 +147,89 @@
}
+/* -( Paging )------------------------------------------------------------- */
+
+
+ protected function buildPagingClause(AphrontDatabaseConnection $conn) {
+ $orderable = $this->getOrderableColumns();
+
+ // TODO: Remove this once subqueries modernize.
+ if (!$orderable) {
+ if ($this->beforeID) {
+ return qsprintf(
+ $conn,
+ '%Q %Q %s',
+ $this->getPagingColumn(),
+ $this->getReversePaging() ? '<' : '>',
+ $this->beforeID);
+ } else if ($this->afterID) {
+ return qsprintf(
+ $conn,
+ '%Q %Q %s',
+ $this->getPagingColumn(),
+ $this->getReversePaging() ? '>' : '<',
+ $this->afterID);
+ } else {
+ return null;
+ }
+ }
+
+ $vector = $this->getOrderVector();
+
+ if ($this->beforeID !== null) {
+ $cursor = $this->beforeID;
+ $reversed = true;
+ } else if ($this->afterID !== null) {
+ $cursor = $this->afterID;
+ $reversed = false;
+ } else {
+ // No paging is being applied to this query so we do not need to
+ // construct a paging clause.
+ return '';
+ }
+
+ $keys = array();
+ foreach ($vector as $order) {
+ $keys[] = $order->getOrderKey();
+ }
+
+ $value_map = $this->getPagingValueMap($cursor, $keys);
+
+ $columns = array();
+ foreach ($vector as $order) {
+ $key = $order->getOrderKey();
+
+ if (!array_key_exists($key, $value_map)) {
+ throw new Exception(
+ pht(
+ 'Query "%s" failed to return a value from getPagingValueMap() '.
+ 'for column "%s".',
+ get_class($this),
+ $key));
+ }
+
+ $column = $orderable[$key];
+ $column['value'] = $value_map[$key];
+
+ $columns[] = $column;
+ }
+
+ return $this->buildPagingClauseFromMultipleColumns(
+ $conn,
+ $columns,
+ array(
+ 'reversed' => $reversed,
+ ));
+ }
+
+ protected function getPagingValueMap($cursor, array $keys) {
+ // TODO: This is a hack to make this work with existing classes for now.
+ return array(
+ 'id' => $cursor,
+ );
+ }
+
+
/**
* Simplifies the task of constructing a paging clause across multiple
* columns. In the general case, this looks like:
@@ -214,11 +276,12 @@
PhutilTypeSpec::checkMap(
$column,
array(
- 'table' => 'optional string',
+ 'table' => 'optional string|null',
'column' => 'string',
'value' => 'wild',
'type' => 'string',
'reverse' => 'optional bool',
+ 'unique' => 'optional bool',
));
}
@@ -298,8 +361,11 @@
$vector = PhabricatorQueryOrderVector::newFromVector($vector);
$orderable = $this->getOrderableColumns();
+
+ // Make sure that all the components identify valid columns.
+ $unique = array();
foreach ($vector as $order) {
- $key = $vector->getOrderKey();
+ $key = $order->getOrderKey();
if (empty($orderable[$key])) {
$valid = implode(', ', array_keys($orderable));
throw new Exception(
@@ -307,8 +373,38 @@
'This query ("%s") does not support sorting by order key "%s". '.
'Supported orders are: %s.',
get_class($this),
+ $key,
$valid));
}
+
+ $unique[$key] = idx($orderable[$key], 'unique', false);
+ }
+
+ // Make sure that the last column is unique so that this is a strong
+ // ordering which can be used for paging.
+ $last = last($unique);
+ if ($last !== true) {
+ throw new Exception(
+ pht(
+ 'Order vector "%s" is invalid: the last column in an order must '.
+ 'be a column with unique values, but "%s" is not unique.',
+ $vector->getAsString(),
+ last_key($unique)));
+ }
+
+ // Make sure that other columns are not unique; an ordering like "id, name"
+ // does not make sense because only "id" can ever have an effect.
+ array_pop($unique);
+ foreach ($unique as $key => $is_unique) {
+ if ($is_unique) {
+ throw new Exception(
+ pht(
+ 'Order vector "%s" is invalid: only the last column in an order '.
+ 'may be unique, but "%s" is a unique column and not the last '.
+ 'column in the order.',
+ $vector->getAsString(),
+ $key));
+ }
}
$this->orderVector = $vector;
@@ -323,7 +419,10 @@
if (!$this->orderVector) {
$vector = $this->getDefaultOrderVector();
$vector = PhabricatorQueryOrderVector::newFromVector($vector);
- $this->orderVector = $vector;
+
+ // We call setOrderVector() here to apply checks to the default vector.
+ // This catches any errors in the implementation.
+ $this->setOrderVector($vector);
}
return $this->orderVector;
@@ -354,6 +453,8 @@
'table' => null,
'column' => 'id',
'reverse' => false,
+ 'type' => 'int',
+ 'unique' => true,
),
);
}

File Metadata

Mime Type
text/plain
Expires
Sat, Jun 8, 8:12 AM (2 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6302506
Default Alt Text
D12355.diff (7 KB)

Event Timeline

Phabricator · Built by Phacility