diff --git a/src/applications/conduit/method/differential/query/ConduitAPI_differential_query_Method.php b/src/applications/conduit/method/differential/query/ConduitAPI_differential_query_Method.php index 73b30f007a..f1ce5a63ff 100644 --- a/src/applications/conduit/method/differential/query/ConduitAPI_differential_query_Method.php +++ b/src/applications/conduit/method/differential/query/ConduitAPI_differential_query_Method.php @@ -1,7 +1,7 @@ 'optional list>', + 'commitHashes' => 'optional list, string>>', 'status' => 'optional enum<'.$status_types.'>', 'order' => 'optional enum<'.$order_types.'>', 'limit' => 'optional uint', @@ -62,6 +67,7 @@ class ConduitAPI_differential_query_Method extends ConduitAPIMethod { public function defineErrorTypes() { return array( + 'ERR-INVALID-PARAMETER' => 'Missing or malformed parameter.', ); } @@ -71,6 +77,7 @@ class ConduitAPI_differential_query_Method extends ConduitAPIMethod { $reviewers = $request->getValue('reviewers'); $status = $request->getValue('status'); $order = $request->getValue('order'); + $commit_hashes = $request->getValue('commitHashes'); $limit = $request->getValue('limit'); $offset = $request->getValue('offset'); $ids = $request->getValue('ids'); @@ -98,7 +105,20 @@ class ConduitAPI_differential_query_Method extends ConduitAPIMethod { $query->withPath($repository_id, $path); } } -*/ + */ + if ($commit_hashes) { + $hash_types = DifferentialRevisionHash::getTypes(); + foreach ($commit_hashes as $info) { + list($type, $hash) = $info; + if (empty($type) || + !in_array($type, $hash_types) || + empty($hash)) { + throw new ConduitException('ERR-INVALID-PARAMETER'); + } + } + $query->withCommitHashes($commit_hashes); + } + if ($status) { $query->withStatus($status); } @@ -163,5 +183,4 @@ class ConduitAPI_differential_query_Method extends ConduitAPIMethod { return $results; } - } diff --git a/src/applications/conduit/method/differential/query/__init__.php b/src/applications/conduit/method/differential/query/__init__.php index edaf82fedc..5b3038f125 100644 --- a/src/applications/conduit/method/differential/query/__init__.php +++ b/src/applications/conduit/method/differential/query/__init__.php @@ -7,6 +7,8 @@ phutil_require_module('phabricator', 'applications/conduit/method/base'); +phutil_require_module('phabricator', 'applications/conduit/protocol/exception'); +phutil_require_module('phabricator', 'applications/differential/constants/revisionhash'); phutil_require_module('phabricator', 'applications/differential/constants/revisionstatus'); phutil_require_module('phabricator', 'applications/differential/query/revision'); phutil_require_module('phabricator', 'infrastructure/env'); diff --git a/src/applications/differential/constants/revisionhash/DifferentialRevisionHash.php b/src/applications/differential/constants/revisionhash/DifferentialRevisionHash.php index 5ce334d9d9..9de9dbe6cc 100644 --- a/src/applications/differential/constants/revisionhash/DifferentialRevisionHash.php +++ b/src/applications/differential/constants/revisionhash/DifferentialRevisionHash.php @@ -1,7 +1,7 @@ + * @return this + * @task config + */ + public function withCommitHashes(array $commit_hashes) { + $this->commitHashes = $commit_hashes; + return $this; + } /** * Filter results to revisions with a given status. Provide a class constant, @@ -351,6 +367,7 @@ final class DifferentialRevisionQuery { !$this->ccs && !$this->authors && !$this->revIDs && + !$this->commitHashes && !$this->phids) { return true; } @@ -440,6 +457,13 @@ final class DifferentialRevisionQuery { $path_table->getTableName()); } + if ($this->commitHashes) { + $joins[] = qsprintf( + $conn_r, + 'JOIN %T hash_rel ON hash_rel.revisionID = r.id', + DifferentialRevisionHash::TABLE_NAME); + } + if ($this->ccs) { $joins[] = qsprintf( $conn_r, @@ -527,6 +551,20 @@ final class DifferentialRevisionQuery { $this->revIDs); } + if ($this->commitHashes) { + $hash_clauses = array(); + foreach ($this->commitHashes as $info) { + list($type, $hash) = $info; + $hash_clauses[] = qsprintf( + $conn_r, + '(hash_rel.type = %s AND hash_rel.hash = %s)', + $type, + $hash); + } + $hash_clauses = '('.implode(' OR ', $hash_clauses).')'; + $where[] = $hash_clauses; + } + if ($this->phids) { $where[] = qsprintf( $conn_r, diff --git a/src/applications/differential/query/revision/__init__.php b/src/applications/differential/query/revision/__init__.php index b15dbd2138..2d35e400f0 100644 --- a/src/applications/differential/query/revision/__init__.php +++ b/src/applications/differential/query/revision/__init__.php @@ -6,6 +6,7 @@ +phutil_require_module('phabricator', 'applications/differential/constants/revisionhash'); phutil_require_module('phabricator', 'applications/differential/constants/revisionstatus'); phutil_require_module('phabricator', 'applications/differential/storage/affectedpath'); phutil_require_module('phabricator', 'applications/differential/storage/diff'); diff --git a/src/applications/repository/worker/commitmessageparser/base/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/base/PhabricatorRepositoryCommitMessageParserWorker.php index feb2632ef2..82d902082a 100644 --- a/src/applications/repository/worker/commitmessageparser/base/PhabricatorRepositoryCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/base/PhabricatorRepositoryCommitMessageParserWorker.php @@ -1,7 +1,7 @@ repository, $this->commit); if ($hashes) { - $sql = array(); - foreach ($hashes as $info) { - list($type, $hash) = $info; - $sql[] = qsprintf( - $conn_w, - '(type = %s AND hash = %s)', - $type, - $hash); - } - $revision = queryfx_one( - $conn_w, - 'SELECT revisionID FROM %T WHERE %Q LIMIT 1', - DifferentialRevisionHash::TABLE_NAME, - implode(' OR ', $sql)); - if ($revision) { - $revision_id = $revision['revisionID']; + + $query = new DifferentialRevisionQuery(); + $query->withCommitHashes($hashes); + $revisions = $query->execute(); + + if (!empty($revisions)) { + $revision = $this->identifyBestRevision($revisions); + $revision_id = $revision->getID(); } } } @@ -112,4 +104,60 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker } } + /** + * When querying for revisions by hash, more than one revision may be found. + * This function identifies the "best" revision from such a set. Typically, + * there is only one revision found. Otherwise, we try to pick an accepted + * revision first, followed by an open revision, and otherwise we go with a + * committed or abandoned revision as a last resort. + */ + private function identifyBestRevision(array $revisions) { + // get the simplest, common case out of the way + if (count($revisions) == 1) { + return reset($revisions); + } + + $first_choice = array(); + $second_choice = array(); + $third_choice = array(); + foreach ($revisions as $revision) { + switch ($revision->getStatus()) { + // "Accepted" revisions -- ostensibly what we're looking for! + case DifferentialRevisionStatus::ACCEPTED: + $first_choice[] = $revision; + break; + // "Open" revisions + case DifferentialRevisionStatus::NEEDS_REVIEW: + case DifferentialRevisionStatus::NEEDS_REVISION: + $second_choice[] = $revision; + break; + // default is a wtf? here + default: + case DifferentialRevisionStatus::ABANDONED: + case DifferentialRevisionStatus::COMMITTED: + $third_choice[] = $revision; + break; + } + } + + // go down the ladder like a bro at last call + if (!empty($first_choice)) { + return $this->identifyMostRecentRevision($first_choice); + } + if (!empty($second_choice)) { + return $this->identifyMostRecentRevision($second_choice); + } + if (!empty($third_choice)) { + return $this->identifyMostRecentRevision($third_choice); + } + } + + /** + * Given a set of revisions, returns the revision with the latest + * updated time. This is ostensibly the most recent revision. + */ + private function identifyMostRecentRevision(array $revisions) { + $revisions = msort($revisions, 'getDateModified'); + return end($revisions); + } } diff --git a/src/applications/repository/worker/commitmessageparser/base/__init__.php b/src/applications/repository/worker/commitmessageparser/base/__init__.php index 59dcb12de3..8a61255fbe 100644 --- a/src/applications/repository/worker/commitmessageparser/base/__init__.php +++ b/src/applications/repository/worker/commitmessageparser/base/__init__.php @@ -7,13 +7,12 @@ phutil_require_module('phabricator', 'applications/differential/constants/action'); -phutil_require_module('phabricator', 'applications/differential/constants/revisionhash'); phutil_require_module('phabricator', 'applications/differential/constants/revisionstatus'); phutil_require_module('phabricator', 'applications/differential/editor/comment'); +phutil_require_module('phabricator', 'applications/differential/query/revision'); phutil_require_module('phabricator', 'applications/differential/storage/revision'); phutil_require_module('phabricator', 'applications/repository/storage/commitdata'); phutil_require_module('phabricator', 'applications/repository/worker/base'); -phutil_require_module('phabricator', 'storage/qsprintf'); phutil_require_module('phabricator', 'storage/queryfx'); phutil_require_module('phutil', 'symbols');