diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php index b459aafa83..a64e2efea3 100644 --- a/src/applications/differential/query/DifferentialRevisionQuery.php +++ b/src/applications/differential/query/DifferentialRevisionQuery.php @@ -731,7 +731,7 @@ final class DifferentialRevisionQuery private function loadCursorObject($id) { $results = id(new DifferentialRevisionQuery()) ->setViewer($this->getViewer()) - ->withIDs(array($id)) + ->withIDs(array((int)$id)) ->execute(); return head($results); } @@ -756,50 +756,39 @@ final class DifferentialRevisionQuery return null; } + $columns = array(); + switch ($this->order) { case self::ORDER_CREATED: return $default; case self::ORDER_MODIFIED: - if ($before_id) { - return qsprintf( - $conn_r, - '(r.dateModified %Q %d OR (r.dateModified = %d AND r.id %Q %d))', - $this->getReversePaging() ? '<' : '>', - $cursor->getDateModified(), - $cursor->getDateModified(), - $this->getReversePaging() ? '<' : '>', - $cursor->getID()); - } else { - return qsprintf( - $conn_r, - '(r.dateModified %Q %d OR (r.dateModified = %d AND r.id %Q %d))', - $this->getReversePaging() ? '>' : '<', - $cursor->getDateModified(), - $cursor->getDateModified(), - $this->getReversePaging() ? '>' : '<', - $cursor->getID()); - } + $columns[] = array( + 'name' => 'r.dateModified', + 'value' => $cursor->getDateModified(), + 'type' => 'int', + ); + break; case self::ORDER_PATH_MODIFIED: - if ($before_id) { - return qsprintf( - $conn_r, - '(p.epoch %Q %d OR (p.epoch = %d AND r.id %Q %d))', - $this->getReversePaging() ? '<' : '>', - $cursor->getDateCreated(), - $cursor->getDateCreated(), - $this->getReversePaging() ? '<' : '>', - $cursor->getID()); - } else { - return qsprintf( - $conn_r, - '(p.epoch %Q %d OR (p.epoch = %d AND r.id %Q %d))', - $this->getReversePaging() ? '>' : '<', - $cursor->getDateCreated(), - $cursor->getDateCreated(), - $this->getReversePaging() ? '>' : '<', - $cursor->getID()); - } + $columns[] = array( + 'name' => 'p.epoch', + 'value' => $cursor->getDateCreated(), + 'type' => 'int', + ); + break; } + + $columns[] = array( + 'name' => 'r.id', + 'value' => $cursor->getID(), + 'type' => 'int', + ); + + return $this->buildPagingClauseFromMultipleColumns( + $conn_r, + $columns, + array( + 'reversed' => (bool)($before_id xor $this->getReversePaging()), + )); } protected function getPagingColumn() { diff --git a/src/applications/repository/query/PhabricatorRepositoryQuery.php b/src/applications/repository/query/PhabricatorRepositoryQuery.php index 359cfb88b7..ba6f2032af 100644 --- a/src/applications/repository/query/PhabricatorRepositoryQuery.php +++ b/src/applications/repository/query/PhabricatorRepositoryQuery.php @@ -162,7 +162,7 @@ final class PhabricatorRepositoryQuery private function loadCursorObject($id) { $results = id(new PhabricatorRepositoryQuery()) ->setViewer($this->getViewer()) - ->withIDs(array($id)) + ->withIDs(array((int)$id)) ->execute(); return head($results); } @@ -192,69 +192,55 @@ final class PhabricatorRepositoryQuery return null; } + $id_column = array( + 'name' => 'r.id', + 'type' => 'int', + 'value' => $cursor->getID(), + ); + + $columns = array(); switch ($order) { case self::ORDER_COMMITTED: $commit = $cursor->getMostRecentCommit(); if (!$commit) { return null; } - $epoch = $commit->getEpoch(); - if ($before_id) { - return qsprintf( - $conn_r, - '(s.epoch %Q %d OR (s.epoch = %d AND r.id %Q %d))', - $this->getReversePaging() ? '<' : '>', - $epoch, - $epoch, - $this->getReversePaging() ? '<' : '>', - $cursor->getID()); - } else { - return qsprintf( - $conn_r, - '(s.epoch %Q %d OR (s.epoch = %d AND r.id %Q %d))', - $this->getReversePaging() ? '>' : '<', - $epoch, - $epoch, - $this->getReversePaging() ? '>' : '<', - $cursor->getID()); - } + $columns[] = array( + 'name' => 's.epoch', + 'type' => 'int', + 'value' => $commit->getEpoch(), + ); + $columns[] = $id_column; + break; case self::ORDER_CALLSIGN: - if ($before_id) { - return qsprintf( - $conn_r, - '(r.callsign %Q %s)', - $this->getReversePaging() ? '<' : '>', - $cursor->getCallsign()); - } else { - return qsprintf( - $conn_r, - '(r.callsign %Q %s)', - $this->getReversePaging() ? '>' : '<', - $cursor->getCallsign()); - } + $columns[] = array( + 'name' => 'r.callsign', + 'type' => 'string', + 'value' => $cursor->getCallsign(), + 'reverse' => true, + ); + break; case self::ORDER_NAME: - if ($before_id) { - return qsprintf( - $conn_r, - '(r.name %Q %s OR (r.name = %s AND r.id %Q %d))', - $this->getReversePaging() ? '<' : '>', - $cursor->getName(), - $cursor->getName(), - $this->getReversePaging() ? '<' : '>', - $cursor->getID()); - } else { - return qsprintf( - $conn_r, - '(r.name %Q %s OR (r.name = %s AND r.id %Q %d))', - $this->getReversePaging() ? '>' : '<', - $cursor->getName(), - $cursor->getName(), - $this->getReversePaging() ? '>' : '<', - $cursor->getID()); - } + $columns[] = array( + 'name' => 'r.name', + 'type' => 'string', + 'value' => $cursor->getName(), + 'reverse' => true, + ); + $columns[] = $id_column; + break; default: throw new Exception("Unknown order '{$order}'!"); } + + return $this->buildPagingClauseFromMultipleColumns( + $conn_r, + $columns, + array( + // TODO: Clean up the column ordering stuff and then make this + // depend on getReversePaging(). + 'reversed' => (bool)($before_id), + )); } private function buildJoinsClause(AphrontDatabaseConnection $conn_r) { diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php index 27ba920f83..a952d1ae58 100644 --- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php @@ -126,4 +126,104 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery return $sliced_results; } + + /** + * Simplifies the task of constructing a paging clause across multiple + * columns. In the general case, this looks like: + * + * A > a OR (A = a AND B > b) OR (A = a AND B = b AND C > c) + * + * To build a clause, specify the name, type, and value of each column + * to include: + * + * $this->buildPagingClauseFromMultipleColumns( + * $conn_r, + * array( + * array( + * 'name' => 'title', + * 'type' => 'string', + * 'value' => $cursor->getTitle(), + * 'reverse' => true, + * ), + * array( + * 'name' => 'id', + * 'type' => 'int', + * 'value' => $cursor->getID(), + * ), + * ), + * array( + * 'reversed' => $is_reversed, + * )); + * + * This method will then return a composable clause for inclusion in WHERE. + * + * @param AphrontDatabaseConnection Connection query will execute on. + * @param list Column description dictionaries. + * @param map Additional constuction options. + * @return string Query clause. + */ + final protected function buildPagingClauseFromMultipleColumns( + AphrontDatabaseConnection $conn, + array $columns, + array $options) { + + foreach ($columns as $column) { + PhutilTypeSpec::checkMap( + $column, + array( + 'name' => 'string', + 'value' => 'wild', + 'type' => 'string', + 'reverse' => 'optional bool', + )); + } + + PhutilTypeSpec::checkMap( + $options, + array( + 'reversed' => 'optional bool', + )); + + $is_query_reversed = idx($options, 'reversed', false); + + $clauses = array(); + $accumulated = array(); + $last_key = last_key($columns); + foreach ($columns as $key => $column) { + $name = $column['name']; + + $type = $column['type']; + switch ($type) { + case 'int': + $value = qsprintf($conn, '%d', $column['value']); + break; + case 'string': + $value = qsprintf($conn, '%s', $column['value']); + break; + default: + throw new Exception("Unknown column type '{$type}'!"); + } + + $is_column_reversed = idx($column, 'reverse', false); + $reverse = ($is_query_reversed xor $is_column_reversed); + + $clause = $accumulated; + $clause[] = qsprintf( + $conn, + '%Q %Q %Q', + $name, + $reverse ? '>' : '<', + $value); + $clauses[] = '('.implode(') AND (', $clause).')'; + + $accumulated[] = qsprintf( + $conn, + '%Q = %Q', + $name, + $value); + } + + return '('.implode(') OR (', $clauses).')'; + } + }