diff --git a/resources/sql/autopatches/20150504.symbolsproject.1.php b/resources/sql/autopatches/20150504.symbolsproject.1.php index 8ab77bbef4..037397998b 100644 --- a/resources/sql/autopatches/20150504.symbolsproject.1.php +++ b/resources/sql/autopatches/20150504.symbolsproject.1.php @@ -11,6 +11,10 @@ $raw_projects_data = queryfx_all($conn_r, 'SELECT * FROM %T', $projects_table); $raw_projects_data = ipull($raw_projects_data, null, 'id'); $repository_ids = ipull($raw_projects_data, 'repositoryID'); +if (!$repository_ids) { + return; +} + $repositories = id(new PhabricatorRepositoryQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withIDs($repository_ids) diff --git a/src/applications/maniphest/__tests__/ManiphestTaskTestCase.php b/src/applications/maniphest/__tests__/ManiphestTaskTestCase.php index b7fafa83ff..18f433fe52 100644 --- a/src/applications/maniphest/__tests__/ManiphestTaskTestCase.php +++ b/src/applications/maniphest/__tests__/ManiphestTaskTestCase.php @@ -145,7 +145,7 @@ final class ManiphestTaskTestCase extends PhabricatorTestCase { private function loadTasks(PhabricatorUser $viewer, $auto_base) { $tasks = id(new ManiphestTaskQuery()) ->setViewer($viewer) - ->setOrderBy(ManiphestTaskQuery::ORDER_PRIORITY) + ->setOrder(ManiphestTaskQuery::ORDER_PRIORITY) ->execute(); // NOTE: AUTO_INCREMENT changes survive ROLLBACK, and we can't throw them diff --git a/src/applications/maniphest/conduit/ManiphestQueryConduitAPIMethod.php b/src/applications/maniphest/conduit/ManiphestQueryConduitAPIMethod.php index 8a2321b2d3..ea8aeb95ed 100644 --- a/src/applications/maniphest/conduit/ManiphestQueryConduitAPIMethod.php +++ b/src/applications/maniphest/conduit/ManiphestQueryConduitAPIMethod.php @@ -102,7 +102,7 @@ final class ManiphestQueryConduitAPIMethod extends ManiphestConduitAPIMethod { $order = $request->getValue('order'); if ($order) { - $query->setOrderBy($order); + $query->setOrder($order); } $limit = $request->getValue('limit'); diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php index 76a8c7f6f1..39111644cb 100644 --- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php @@ -632,7 +632,7 @@ final class ManiphestTransactionEditor $query = id(new ManiphestTaskQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) - ->setOrderBy(ManiphestTaskQuery::ORDER_PRIORITY) + ->setOrder(ManiphestTaskQuery::ORDER_PRIORITY) ->withPriorities(array($dst->getPriority())) ->setLimit(1); diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php index 636fea218d..956810524d 100644 --- a/src/applications/maniphest/query/ManiphestTaskQuery.php +++ b/src/applications/maniphest/query/ManiphestTaskQuery.php @@ -43,7 +43,6 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery { const GROUP_STATUS = 'group-status'; const GROUP_PROJECT = 'group-project'; - private $orderBy = 'order-modified'; const ORDER_PRIORITY = 'order-priority'; const ORDER_CREATED = 'order-created'; const ORDER_MODIFIED = 'order-modified'; @@ -127,11 +126,27 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery { public function setGroupBy($group) { $this->groupBy = $group; - return $this; - } - public function setOrderBy($order) { - $this->orderBy = $order; + switch ($this->groupBy) { + case self::GROUP_NONE: + $vector = array(); + break; + case self::GROUP_PRIORITY: + $vector = array('priority'); + break; + case self::GROUP_OWNER: + $vector = array('owner'); + break; + case self::GROUP_STATUS: + $vector = array('status'); + break; + case self::GROUP_PROJECT: + $vector = array('project'); + break; + } + + $this->setGroupVector($vector); + return $this; } @@ -197,69 +212,6 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery { return new ManiphestTask(); } - protected function willExecute() { - // If we already have an order vector, use it as provided. - // TODO: This is a messy hack to make setOrderVector() stronger than - // setPriority(). - $vector = $this->getOrderVector(); - $keys = mpull(iterator_to_array($vector), 'getOrderKey'); - if (array_values($keys) !== array('id')) { - return; - } - - $parts = array(); - switch ($this->groupBy) { - case self::GROUP_NONE: - break; - case self::GROUP_PRIORITY: - $parts[] = array('priority'); - break; - case self::GROUP_OWNER: - $parts[] = array('owner'); - break; - case self::GROUP_STATUS: - $parts[] = array('status'); - break; - case self::GROUP_PROJECT: - $parts[] = array('project'); - break; - } - - if ($this->applicationSearchOrders) { - $columns = array(); - foreach ($this->applicationSearchOrders as $order) { - $part = 'custom:'.$order['key']; - if ($order['ascending']) { - $part = '-'.$part; - } - $columns[] = $part; - } - $columns[] = 'id'; - $parts[] = $columns; - } else { - switch ($this->orderBy) { - case self::ORDER_PRIORITY: - $parts[] = array('priority', 'subpriority', 'id'); - break; - case self::ORDER_CREATED: - $parts[] = array('id'); - break; - case self::ORDER_MODIFIED: - $parts[] = array('updated', 'id'); - break; - case self::ORDER_TITLE: - $parts[] = array('title', 'id'); - break; - } - } - - $parts = array_mergev($parts); - // We may have a duplicate column if we are both ordering and grouping - // by priority. - $parts = array_unique($parts); - $this->setOrderVector($parts); - } - protected function loadPage() { $task_dao = new ManiphestTask(); $conn = $task_dao->establishConnection('r'); @@ -760,6 +712,41 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery { return $id; } + public function getBuiltinOrders() { + $orders = array( + 'priority' => array( + 'vector' => array('priority', 'subpriority', 'id'), + 'name' => pht('Priority'), + 'aliases' => array(self::ORDER_PRIORITY), + ), + 'updated' => array( + 'vector' => array('updated', 'id'), + 'name' => pht('Date Updated'), + 'aliases' => array(self::ORDER_MODIFIED), + ), + 'title' => array( + 'vector' => array('title', 'id'), + 'name' => pht('Title'), + 'aliases' => array(self::ORDER_TITLE), + ), + ) + parent::getBuiltinOrders(); + + // Alias the "newest" builtin to the historical key for it. + $orders['newest']['aliases'][] = self::ORDER_CREATED; + + $orders = array_select_keys( + $orders, + array( + 'priority', + 'updated', + 'newest', + 'oldest', + 'title', + )) + $orders; + + return $orders; + } + public function getOrderableColumns() { return parent::getOrderableColumns() + array( 'priority' => array( diff --git a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php index c587de5c10..1cdb71994d 100644 --- a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php +++ b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php @@ -38,8 +38,9 @@ final class ManiphestTaskSearchEngine return 'PhabricatorManiphestApplication'; } - public function getCustomFieldObject() { - return new ManiphestTask(); + public function newQuery() { + return id(new ManiphestTaskQuery()) + ->needProjectPHIDs(true); } public function buildSavedQueryFromRequest(AphrontRequest $request) { @@ -108,8 +109,7 @@ final class ManiphestTaskSearchEngine } public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) { - $query = id(new ManiphestTaskQuery()) - ->needProjectPHIDs(true); + $query = $this->newQuery(); $viewer = $this->requireViewer(); @@ -156,10 +156,14 @@ final class ManiphestTaskSearchEngine $query->withBlockingTasks($saved->getParameter('blocking')); $query->withBlockedTasks($saved->getParameter('blocked')); - $this->applyOrderByToQuery( - $query, - $this->getOrderValues(), - $saved->getParameter('order')); + // TODO: This is glue that will be obsolete soon. + $order = $saved->getParameter('order'); + $builtin = $query->getBuiltinOrderAliasMap(); + if (strlen($order) && isset($builtin[$order])) { + $query->setOrder($order); + } else { + $query->setOrder(head_key($builtin)); + } $group = $saved->getParameter('group'); $group = idx($this->getGroupValues(), $group); @@ -246,9 +250,7 @@ final class ManiphestTaskSearchEngine $ids = $saved->getParameter('ids', array()); - $builtin_orders = $this->getOrderOptions(); - $custom_orders = $this->getCustomFieldOrderOptions(); - $all_orders = $builtin_orders + $custom_orders; + $all_orders = ipull($this->newQuery()->getBuiltinOrders(), 'name'); $form ->appendControl( @@ -405,24 +407,6 @@ final class ManiphestTaskSearchEngine return parent::buildSavedQueryFromBuiltin($query_key); } - private function getOrderOptions() { - return array( - 'priority' => pht('Priority'), - 'updated' => pht('Date Updated'), - 'created' => pht('Date Created'), - 'title' => pht('Title'), - ); - } - - private function getOrderValues() { - return array( - 'priority' => ManiphestTaskQuery::ORDER_PRIORITY, - 'updated' => ManiphestTaskQuery::ORDER_MODIFIED, - 'created' => ManiphestTaskQuery::ORDER_CREATED, - 'title' => ManiphestTaskQuery::ORDER_TITLE, - ); - } - private function getGroupOptions() { return array( 'priority' => pht('Priority'), diff --git a/src/applications/project/controller/PhabricatorProjectBoardViewController.php b/src/applications/project/controller/PhabricatorProjectBoardViewController.php index b9c366fecc..bcb2b281ad 100644 --- a/src/applications/project/controller/PhabricatorProjectBoardViewController.php +++ b/src/applications/project/controller/PhabricatorProjectBoardViewController.php @@ -163,7 +163,7 @@ final class PhabricatorProjectBoardViewController PhabricatorProjectObjectHasProjectEdgeType::EDGECONST, PhabricatorQueryConstraint::OPERATOR_AND, array($project->getPHID())) - ->setOrderBy(ManiphestTaskQuery::ORDER_PRIORITY) + ->setOrder(ManiphestTaskQuery::ORDER_PRIORITY) ->setViewer($viewer) ->execute(); $tasks = mpull($tasks, null, 'getPHID'); diff --git a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php index 419447497c..6204a37c35 100644 --- a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php +++ b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php @@ -157,7 +157,7 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { } $order = $saved->getParameter('order'); - $builtin = $query->getBuiltinOrders(); + $builtin = $query->getBuiltinOrderAliasMap(); if (strlen($order) && isset($builtin[$order])) { $query->setOrder($order); } else { @@ -1113,66 +1113,6 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject { } } - protected function applyOrderByToQuery( - PhabricatorCursorPagedPolicyAwareQuery $query, - array $standard_values, - $order) { - - if (substr($order, 0, 7) === 'custom:') { - $list = $this->getCustomFieldList(); - if (!$list) { - $query->setOrderBy(head($standard_values)); - return; - } - - foreach ($list->getFields() as $field) { - $key = $this->getKeyForCustomField($field); - - if ($key === $order) { - $index = $field->buildOrderIndex(); - - if ($index === null) { - $query->setOrderBy(head($standard_values)); - return; - } - - $query->withApplicationSearchOrder( - $field, - $index, - false); - break; - } - } - } else { - $order = idx($standard_values, $order); - if ($order) { - $query->setOrderBy($order); - } else { - $query->setOrderBy(head($standard_values)); - } - } - } - - - protected function getCustomFieldOrderOptions() { - $list = $this->getCustomFieldList(); - if (!$list) { - return; - } - - $custom_order = array(); - foreach ($list->getFields() as $field) { - if ($field->shouldAppearInApplicationSearch()) { - if ($field->buildOrderIndex() !== null) { - $key = $this->getKeyForCustomField($field); - $custom_order[$key] = $field->getFieldName(); - } - } - } - - return $custom_order; - } - /** * Get a unique key identifying a field. * diff --git a/src/infrastructure/query/order/PhabricatorQueryOrderVector.php b/src/infrastructure/query/order/PhabricatorQueryOrderVector.php index 37c0fda440..d4b100437b 100644 --- a/src/infrastructure/query/order/PhabricatorQueryOrderVector.php +++ b/src/infrastructure/query/order/PhabricatorQueryOrderVector.php @@ -84,6 +84,22 @@ final class PhabricatorQueryOrderVector return $obj; } + public function appendVector($vector) { + $vector = self::newFromVector($vector); + + // When combining vectors (like "group by" and "order by" vectors), there + // may be redundant columns. We only want to append unique columns which + // aren't already present in the vector. + foreach ($vector->items as $key => $item) { + if (empty($this->items[$key])) { + $this->items[$key] = $item; + $this->keys[] = $key; + } + } + + return $this; + } + public function getAsString() { $scalars = array(); foreach ($this->items as $item) { diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php index aace1c728c..b48e1e5c46 100644 --- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php @@ -18,9 +18,9 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery private $afterID; private $beforeID; private $applicationSearchConstraints = array(); - protected $applicationSearchOrders = array(); private $internalPaging; private $orderVector; + private $groupVector; private $builtinOrder; private $edgeLogicConstraints = array(); private $edgeLogicConstraintsAreValid = false; @@ -628,19 +628,40 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery * @task order */ public function setOrder($order) { - $orders = $this->getBuiltinOrders(); + $aliases = $this->getBuiltinOrderAliasMap(); - if (empty($orders[$order])) { + if (empty($aliases[$order])) { throw new Exception( pht( 'Query "%s" does not support a builtin order "%s". Supported orders '. 'are: %s.', get_class($this), $order, - implode(', ', array_keys($orders)))); + implode(', ', array_keys($aliases)))); } - $this->builtinOrder = $order; + $this->builtinOrder = $aliases[$order]; + $this->orderVector = null; + + return $this; + } + + + /** + * Set a grouping order to apply before primary result ordering. + * + * This allows you to preface the query order vector with additional orders, + * so you can effect "group by" queries while still respecting "order by". + * + * This is a high-level method which works alongside @{method:setOrder}. For + * lower-level control over order vectors, use @{method:setOrderVector}. + * + * @param PhabricatorQueryOrderVector|list List of order keys. + * @return this + * @task order + */ + public function setGroupVector($vector) { + $this->groupVector = $vector; $this->orderVector = null; return $this; @@ -707,6 +728,35 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery return $orders; } + public function getBuiltinOrderAliasMap() { + $orders = $this->getBuiltinOrders(); + + $map = array(); + foreach ($orders as $key => $order) { + $keys = array(); + $keys[] = $key; + foreach (idx($order, 'aliases', array()) as $alias) { + $keys[] = $alias; + } + + foreach ($keys as $alias) { + if (isset($map[$alias])) { + throw new Exception( + pht( + 'Two builtin orders ("%s" and "%s") define the same key or '. + 'alias ("%s"). Each order alias and key must be unique and '. + 'identify a single order.', + $key, + $map[$alias], + $alias)); + } + $map[$alias] = $key; + } + } + + return $map; + } + /** * Set a low-level column ordering. @@ -791,6 +841,13 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery } else { $vector = $this->getDefaultOrderVector(); } + + if ($this->groupVector) { + $group = PhabricatorQueryOrderVector::newFromVector($this->groupVector); + $group->appendVector($vector); + $vector = $group; + } + $vector = PhabricatorQueryOrderVector::newFromVector($vector); // We call setOrderVector() here to apply checks to the default vector. @@ -1022,32 +1079,6 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery } - /** - * Order the results by an ApplicationSearch index. - * - * @param PhabricatorCustomField Field to which the index belongs. - * @param PhabricatorCustomFieldIndexStorage Table where the index is stored. - * @param bool True to sort ascending. - * @return this - * @task appsearch - */ - public function withApplicationSearchOrder( - PhabricatorCustomField $field, - PhabricatorCustomFieldIndexStorage $index, - $ascending) { - - $this->applicationSearchOrders[] = array( - 'key' => $field->getFieldKey(), - 'type' => $index->getIndexValueType(), - 'table' => $index->getTableName(), - 'index' => $index->getIndexKey(), - 'ascending' => $ascending, - ); - - return $this; - } - - /** * Get the name of the query's primary object PHID column, for constructing * JOIN clauses. Normally (and by default) this is just `"phid"`, but it may @@ -1232,25 +1263,6 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery } } - // TODO: Get rid of this. - foreach ($this->applicationSearchOrders as $key => $order) { - $table = $order['table']; - $index = $order['index']; - $alias = 'appsearch_order_'.$index; - $phid_column = $this->getApplicationSearchObjectPHIDColumn(); - - $joins[] = qsprintf( - $conn_r, - 'LEFT JOIN %T %T ON %T.objectPHID = %Q - AND %T.indexKey = %s', - $table, - $alias, - $alias, - $phid_column, - $alias, - $index); - } - $phid_column = $this->getApplicationSearchObjectPHIDColumn(); $orderable = $this->getOrderableColumns();