Page MenuHomePhabricator
Files F13179601

D12352.diff
No OneTemporary
Actions

D12352.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -2320,6 +2320,9 @@
'PhabricatorProjectsPolicyRule' => 'applications/policy/rule/PhabricatorProjectsPolicyRule.php',
'PhabricatorPygmentSetupCheck' => 'applications/config/check/PhabricatorPygmentSetupCheck.php',
'PhabricatorQuery' => 'infrastructure/query/PhabricatorQuery.php',
+ 'PhabricatorQueryOrderItem' => 'infrastructure/query/order/PhabricatorQueryOrderItem.php',
+ 'PhabricatorQueryOrderTestCase' => 'infrastructure/query/order/__tests__/PhabricatorQueryOrderTestCase.php',
+ 'PhabricatorQueryOrderVector' => 'infrastructure/query/order/PhabricatorQueryOrderVector.php',
'PhabricatorRecaptchaConfigOptions' => 'applications/config/option/PhabricatorRecaptchaConfigOptions.php',
'PhabricatorRedirectController' => 'applications/base/controller/PhabricatorRedirectController.php',
'PhabricatorRefreshCSRFController' => 'applications/auth/controller/PhabricatorRefreshCSRFController.php',
@@ -5685,6 +5688,12 @@
'PhabricatorProjectWatchController' => 'PhabricatorProjectController',
'PhabricatorProjectsPolicyRule' => 'PhabricatorPolicyRule',
'PhabricatorPygmentSetupCheck' => 'PhabricatorSetupCheck',
+ 'PhabricatorQueryOrderItem' => 'Phobject',
+ 'PhabricatorQueryOrderTestCase' => 'PhabricatorTestCase',
+ 'PhabricatorQueryOrderVector' => array(
+ 'Phobject',
+ 'Iterator',
+ ),
'PhabricatorRecaptchaConfigOptions' => 'PhabricatorApplicationConfigOptions',
'PhabricatorRedirectController' => 'PhabricatorController',
'PhabricatorRefreshCSRFController' => 'PhabricatorAuthController',
diff --git a/src/infrastructure/query/order/PhabricatorQueryOrderItem.php b/src/infrastructure/query/order/PhabricatorQueryOrderItem.php
new file mode 100644
--- /dev/null
+++ b/src/infrastructure/query/order/PhabricatorQueryOrderItem.php
@@ -0,0 +1,62 @@
+<?php
+
+/**
+ * Structural class representing one item in an order vector.
+ *
+ * See @{class:PhabricatorQueryOrderVector} for discussion of order vectors.
+ * This represents one item in an order vector, like "id". When combined with
+ * the other items in the vector, a complete ordering (like "name, id") is
+ * described.
+ *
+ * Construct an item using @{method:newFromScalar}:
+ *
+ * $item = PhabricatorQueryOrderItem::newFromScalar('id');
+ *
+ * This class is primarily internal to the query infrastructure, and most
+ * application code should not need to interact with it directly.
+ */
+final class PhabricatorQueryOrderItem
+ extends Phobject {
+
+ private $orderKey;
+ private $isReversed;
+
+ private function __construct() {
+ // <private>
+ }
+
+ public static function newFromScalar($scalar) {
+ // If the string is something like "-id", strip the "-" off and mark it
+ // as reversed.
+ $is_reversed = false;
+ if (!strncmp($scalar, '-', 1)) {
+ $is_reversed = true;
+ $scalar = substr($scalar, 1);
+ }
+
+ $item = new PhabricatorQueryOrderItem();
+ $item->orderKey = $scalar;
+ $item->isReversed = $is_reversed;
+
+ return $item;
+ }
+
+ public function getIsReversed() {
+ return $this->isReversed;
+ }
+
+ public function getOrderKey() {
+ return $this->orderKey;
+ }
+
+ public function getAsScalar() {
+ if ($this->getIsReversed()) {
+ $prefix = '-';
+ } else {
+ $prefix = '';
+ }
+
+ return $prefix.$this->getOrderKey();
+ }
+
+}
diff --git a/src/infrastructure/query/order/PhabricatorQueryOrderVector.php b/src/infrastructure/query/order/PhabricatorQueryOrderVector.php
new file mode 100644
--- /dev/null
+++ b/src/infrastructure/query/order/PhabricatorQueryOrderVector.php
@@ -0,0 +1,115 @@
+<?php
+
+/**
+ * Structural class representing a column ordering for a query.
+ *
+ * Queries often order results on multiple columns. For example, projects might
+ * be ordered by "name, id". This class wraps a list of column orderings and
+ * makes them easier to manage.
+ *
+ * To construct an order vector, use @{method:newFromVector}:
+ *
+ * $vector = PhabricatorQueryOrderVector::newFromVector(array('name', 'id'));
+ *
+ * You can iterate over an order vector normally:
+ *
+ * foreach ($vector as $item) {
+ * // ...
+ * }
+ *
+ * The items are objects of class @{class:PhabricatorQueryOrderItem}.
+ *
+ * This class is primarily internal to the query infrastructure, and most
+ * application code should not need to interact with it directly.
+ */
+final class PhabricatorQueryOrderVector
+ extends Phobject
+ implements Iterator {
+
+ private $items;
+ private $keys;
+ private $cursor;
+
+ private function __construct() {
+ // <private>
+ }
+
+ public static function newFromVector($vector) {
+ if ($vector instanceof PhabricatorQueryOrderVector) {
+ return (clone $vector);
+ }
+
+ if (!is_array($vector)) {
+ throw new Exception(
+ pht(
+ 'An order vector can only be constructed from a list of strings or '.
+ 'another order vector.'));
+ }
+
+ if (!$vector) {
+ throw new Exception(
+ pht(
+ 'An order vector must not be empty.'));
+ }
+
+ $items = array();
+ foreach ($vector as $key => $scalar) {
+ if (!is_string($scalar)) {
+ throw new Exception(
+ pht(
+ 'Value with key "%s" in order vector is not a string (it has '.
+ 'type "%s"). An order vector must contain only strings.',
+ $key,
+ gettype($scalar)));
+ }
+
+ $item = PhabricatorQueryOrderItem::newFromScalar($scalar);
+
+ // Orderings like "id, id, id" or "id, -id" are meaningless and invalid.
+ if (isset($items[$item->getOrderKey()])) {
+ throw new Exception(
+ pht(
+ 'Order vector "%s" specifies order "%s" twice. Each component '.
+ 'of an ordering must be unique.',
+ implode(', ', $vector),
+ $item));
+ }
+
+ $items[$item->getOrderKey()] = $item;
+ }
+
+ $obj = new PhabricatorQueryOrderVector();
+ $obj->items = $items;
+ $obj->keys = array_keys($items);
+ return $obj;
+ }
+
+
+/* -( Iterator Interface )------------------------------------------------- */
+
+
+ public function rewind() {
+ $this->cursor = 0;
+ }
+
+
+ public function current() {
+ return $this->items[$this->key()];
+ }
+
+
+ public function key() {
+ return $this->keys[$this->cursor];
+ }
+
+
+ public function next() {
+ ++$this->cursor;
+ }
+
+
+ public function valid() {
+ return isset($this->keys[$this->cursor]);
+ }
+
+}
diff --git a/src/infrastructure/query/order/__tests__/PhabricatorQueryOrderTestCase.php b/src/infrastructure/query/order/__tests__/PhabricatorQueryOrderTestCase.php
new file mode 100644
--- /dev/null
+++ b/src/infrastructure/query/order/__tests__/PhabricatorQueryOrderTestCase.php
@@ -0,0 +1,65 @@
+<?php
+
+final class PhabricatorQueryOrderTestCase extends PhabricatorTestCase {
+
+ public function testQueryOrderItem() {
+ $item = PhabricatorQueryOrderItem::newFromScalar('id');
+ $this->assertEqual('id', $item->getOrderKey());
+ $this->assertEqual(false, $item->getIsReversed());
+
+ $item = PhabricatorQueryOrderItem::newFromScalar('-id');
+ $this->assertEqual('id', $item->getOrderKey());
+ $this->assertEqual(true, $item->getIsReversed());
+ }
+
+ public function testQueryOrderBadVectors() {
+ $bad = array(
+ array(),
+ null,
+ 1,
+ array(2),
+ array('id', 'id'),
+ array('id', '-id'),
+ );
+
+ foreach ($bad as $input) {
+ $caught = null;
+ try {
+ PhabricatorQueryOrderVector::newFromVector($input);
+ } catch (Exception $ex) {
+ $caught = $ex;
+ }
+
+ $this->assertTrue(($caught instanceof Exception));
+ }
+ }
+
+ public function testQueryOrderVector() {
+ $vector = PhabricatorQueryOrderVector::newFromVector(
+ array(
+ 'a',
+ 'b',
+ '-c',
+ 'd',
+ ));
+
+ $this->assertEqual(
+ array(
+ 'a' => 'a',
+ 'b' => 'b',
+ 'c' => 'c',
+ 'd' => 'd',
+ ),
+ mpull(iterator_to_array($vector), 'getOrderKey'));
+
+ $this->assertEqual(
+ array(
+ 'a' => false,
+ 'b' => false,
+ 'c' => true,
+ 'd' => false,
+ ),
+ mpull(iterator_to_array($vector), 'getIsReversed'));
+ }
+
+}
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 order Result Ordering
*/
abstract class PhabricatorCursorPagedPolicyAwareQuery
extends PhabricatorPolicyAwareQuery {
@@ -14,6 +15,7 @@
private $applicationSearchConstraints = array();
private $applicationSearchOrders = array();
private $internalPaging;
+ private $orderVector;
protected function getPagingColumn() {
return 'id';
@@ -131,22 +133,6 @@
return null;
}
- final protected function buildOrderClause(AphrontDatabaseConnection $conn_r) {
- if ($this->beforeID) {
- return qsprintf(
- $conn_r,
- 'ORDER BY %Q %Q',
- $this->getPagingColumn(),
- $this->getReversePaging() ? 'DESC' : 'ASC');
- } else {
- return qsprintf(
- $conn_r,
- 'ORDER BY %Q %Q',
- $this->getPagingColumn(),
- $this->getReversePaging() ? 'ASC' : 'DESC');
- }
- }
-
final protected function didLoadResults(array $results) {
if ($this->beforeID) {
$results = array_reverse($results, $preserve_keys = true);
@@ -284,6 +270,121 @@
return '('.implode(') OR (', $clauses).')';
}
+
+/* -( Result Ordering )---------------------------------------------------- */
+
+
+ /**
+ * @task order
+ */
+ public function setOrderVector($vector) {
+ $vector = PhabricatorQueryOrderVector::newFromVector($vector);
+
+ $orderable = $this->getOrderableColumns();
+ foreach ($vector as $order) {
+ $key = $vector->getOrderKey();
+ if (empty($orderable[$key])) {
+ $valid = implode(', ', array_keys($orderable));
+ throw new Exception(
+ pht(
+ 'This query ("%s") does not support sorting by order key "%s". '.
+ 'Supported orders are: %s.',
+ get_class($this),
+ $valid));
+ }
+ }
+
+ $this->orderVector = $vector;
+ return $this;
+ }
+
+
+ /**
+ * @task order
+ */
+ private function getOrderVector() {
+ if (!$this->orderVector) {
+ $vector = $this->getDefaultOrderVector();
+ $vector = PhabricatorQueryOrderVector::newFromVector($vector);
+ $this->orderVector = $vector;
+ }
+
+ return $this->orderVector;
+ }
+
+
+ /**
+ * @task order
+ */
+ protected function getDefaultOrderVector() {
+ return array('id');
+ }
+
+
+ /**
+ * @task order
+ */
+ public function getOrderableColumns() {
+ // TODO: Remove this once all subclasses move off the old stuff.
+ if ($this->getPagingColumn() !== 'id') {
+ // This class has bad old custom logic around paging, so return nothing
+ // here. This deactivates the new order code.
+ return array();
+ }
+
+ return array(
+ 'id' => array(
+ 'table' => null,
+ 'column' => 'id',
+ 'reverse' => false,
+ ),
+ );
+ }
+
+
+ /**
+ * @task order
+ */
+ final protected function buildOrderClause(AphrontDatabaseConnection $conn) {
+ $orderable = $this->getOrderableColumns();
+
+ // TODO: Remove this once all subclasses move off the old stuff. We'll
+ // only enter this block for code using older ordering mechanisms. New
+ // code should expose an orderable column list.
+ if (!$orderable) {
+ if ($this->beforeID) {
+ return qsprintf(
+ $conn,
+ 'ORDER BY %Q %Q',
+ $this->getPagingColumn(),
+ $this->getReversePaging() ? 'DESC' : 'ASC');
+ } else {
+ return qsprintf(
+ $conn,
+ 'ORDER BY %Q %Q',
+ $this->getPagingColumn(),
+ $this->getReversePaging() ? 'ASC' : 'DESC');
+ }
+ }
+
+ $vector = $this->getOrderVector();
+
+ $parts = array();
+ foreach ($vector as $order) {
+ $part = $orderable[$order->getOrderKey()];
+ if ($order->getIsReversed()) {
+ $part['reverse'] = !idx($part, 'reverse', false);
+ }
+ $parts[] = $part;
+ }
+
+ return $this->formatOrderClause($conn, $parts);
+ }
+
+
+ /**
+ * @task order
+ */
protected function formatOrderClause(
AphrontDatabaseConnection $conn,
array $parts) {

File Metadata

Mime Type
text/plain
Expires
Thu, May 9, 9:18 PM (1 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6277786
Default Alt Text
D12352.diff (12 KB)

Event Timeline

Phabricator · Built by Phacility