Page MenuHomePhabricator

Allow ascending order for Conduit's maniphest.query's "order" parameter (order-created, order-modified)
Closed, ResolvedPublic

Description

Pulling data using Conduit's maniphest.query, the "order-created", "order-modified" values of the "order" parameter of the "maniphest.query" method only allow descending order.

There is no way to sort results in ascending order. Pulling a list of tasks from oldest to newest would make external processing easier and faster.

This depends on T6864: Sort Queries in Ascending/Descending Order.

(Shortly discussed with Evan on IRC today.)

Event Timeline

aklapper created this task.Apr 24 2015, 7:20 PM
aklapper raised the priority of this task from to Needs Triage.
aklapper updated the task description. (Show Details)
aklapper added a project: Conduit.
aklapper added subscribers: aklapper, qgil.

For context, it sounds like https://phabricator.wikimedia.org/T96238 is the root issue here, specifically https://phabricator.wikimedia.org/T96238#1234459. In general:

Pagination: Newer Conduit query methods use cursor-based paging and emit cursors. For example, almanac.querydevices returns a result like this:

{
  "data": [
    {
      "id": 7,
      "phid": "PHID-ADEV-pxe3ujkufyymul5k7cw4",
      "name": "storage.bak.local",
      "properties": {}
    }
  ],
  "cursor": {
    "limit": 1,
    "after": "7",
    "before": null
  }
}

The "cursor" part of the response allows you to page forward and backward on subsequent queries. Querying by non-unique date ranges is insufficient to achieve this on its own in the general case: if the system has 100 tasks which were updated in the same second, date-based pagination will not be able to retrieve the next page of results. The best solution to the paging problem is to modernize maniphest.query to emit responses which support cursor-based pagination, then use cursors in the client. This is trivial to implement (~10 lines of code) but will break backward compatibility in the API.

Ordering: The ordering backend was rewritten significantly recently in connection with T4100 / T5750 / T7803 (T7803 has most of the specific work). In the general case, it is now easy to add new orders to objects, and expose orders consistently in ApplicationSearch and Conduit. D12381 added the Conduit integration, and the ~6 lines of changes to RepositoryQueryConduitAPIMethod.php in that diff to add order support to repository.query are typical of the complexity of adding support in the general case.

Unfortunately, Maniphest is not the general case. Maniphest has "Group By", and supports Custom Field ordering. It is the only application which supports these features, and they aren't part of the core infrastructure yet. Maniphest currently implements these features in a somewhat fragile way where there's some messy glue between the old API and the new backend. This needs to be cleaned up before Maniphest can take full advantage of the newer, more general ordering infrastructure, including the Conduit integration.

Because maniphest.query already has an "order" field, adding new semantics to the field would be a breaking change, although we could hack in support for the old values as aliases of the new values. Ideally, it would be nice to not have to maintain these aliases indefinitely.

Edges: We want to implement general support for edges, custom fields, and ApplicationTransactions in Conduit, not piecemeal support. See discussion in T5873. This will almost certainly entail many API changes that will deprecate methods and create backward compatibility issues. It is likely to make sense to complete this in conjunction with T5955.

For this and T6864 specifically, the path forward is:

  1. Clean up how Maniphest expresses "Group By" and Custom Field ordering. Specifically, the hacky mess in ManiphestTaskQuery->willExecute() needs to be removed, almost certainly by extracting those capabilities into PhabricatorCursorPagedPolicyAwareQuery.
  2. Add ApplicationSearch and Conduit hooks (~20 lines of code).
  3. Add new orders (~5 lines of code each).

Steps (2) and (3) are easy and straightforward, anyone can implement them, and there are plenty of examples. Step (1) isn't a huge amount of work, but is tricky and complex and touches the internals, and may be difficult for contributors to complete.


One other possible approach is that Conduit is relatively extensible, so you could create a copy of maniphest.query and call it something like grimoire.querytasks, then implement whatever adjustments you want. You can use ManiphestTaskQuery->setOrderVector() to express the desired orderings already, emit cursors without worrying about API breaks, and augment the results with specific edge information. Once T5873, T5955, etc., complete, you could switch to the real maniphest.query, which would then support all the things you need, and throw away the custom method.

Although this is inherently somewhat fragile/perilous, it would require only a small amount of custom code and probably be a lot more stable than, e.g., the sprint stuff. (Adding API methods is explicitly an extension point, in a way that doing a lot of the sprint stuff isn't.)

epriestley closed this task as Resolved.Dec 16 2015, 8:06 PM
epriestley claimed this task.

The maniphest.search endpoint at HEAD now has full-power ordering controls.

https://secure.phabricator.com/conduit/method/maniphest.search/