diff --git a/resources/sql/patches/130.denormalrevisionquery.sql b/resources/sql/patches/130.denormalrevisionquery.sql new file mode 100644 index 0000000000..125d7131a3 --- /dev/null +++ b/resources/sql/patches/130.denormalrevisionquery.sql @@ -0,0 +1,5 @@ +ALTER TABLE phabricator_differential.differential_revision + ADD branchName VARCHAR(255) COLLATE utf8_general_ci; + +ALTER TABLE phabricator_differential.differential_revision + ADD arcanistProjectPHID VARCHAR(64) COLLATE utf8_bin; diff --git a/resources/sql/patches/131.migraterevisionquery.php b/resources/sql/patches/131.migraterevisionquery.php new file mode 100644 index 0000000000..aacb1ea9df --- /dev/null +++ b/resources/sql/patches/131.migraterevisionquery.php @@ -0,0 +1,44 @@ +establishConnection('w'); + +$revisions = id(new DifferentialRevision())->loadAll(); + +echo "Migrating ".count($revisions)." revisions"; +foreach ($revisions as $revision) { + echo "."; + + $diff = $revision->loadActiveDiff(); + if (!$diff) { + continue; + } + + $branch_name = $diff->getBranch(); + $arc_project_phid = $diff->getArcanistProjectPHID(); + + queryfx( + $conn_w, + 'UPDATE %T SET branchName = %s, arcanistProjectPHID = %s WHERE id = %d', + $table->getTableName(), + $branch_name, + $arc_project_phid, + $revision->getID()); +} +echo "\nDone.\n"; 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 ac75247814..983a4c0e2d 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 @@ -62,6 +62,7 @@ final class ConduitAPI_differential_query_Method 'subscribers' => 'optional list', 'responsibleUsers' => 'optional list', 'branches' => 'optional list', + 'arcanistProjects' => 'optional list', ); } @@ -89,6 +90,7 @@ final class ConduitAPI_differential_query_Method $subscribers = $request->getValue('subscribers'); $responsible_users = $request->getValue('responsibleUsers'); $branches = $request->getValue('branches'); + $arc_projects = $request->getValue('arcanistProjects'); $query = new DifferentialRevisionQuery(); if ($authors) { @@ -151,6 +153,19 @@ final class ConduitAPI_differential_query_Method if ($branches) { $query->withBranches($branches); } + if ($arc_projects) { + // This is sort of special-cased, but don't make arc do an extra round + // trip. + $projects = id(new PhabricatorRepositoryArcanistProject()) + ->loadAllWhere( + 'name in (%Ls)', + $arc_projects); + if (!$projects) { + return array(); + } + + $query->withArcanistProjectPHIDs(mpull($projects, 'getPHID')); + } $query->needRelationships(true); $query->needCommitPHIDs(true); diff --git a/src/applications/conduit/method/differential/query/__init__.php b/src/applications/conduit/method/differential/query/__init__.php index 131a224e23..2e58c93283 100644 --- a/src/applications/conduit/method/differential/query/__init__.php +++ b/src/applications/conduit/method/differential/query/__init__.php @@ -12,7 +12,10 @@ phutil_require_module('arcanist', 'differential/constants/revisionstatus'); phutil_require_module('phabricator', 'applications/conduit/method/base'); phutil_require_module('phabricator', 'applications/conduit/protocol/exception'); phutil_require_module('phabricator', 'applications/differential/query/revision'); +phutil_require_module('phabricator', 'applications/repository/storage/arcanistproject'); phutil_require_module('phabricator', 'infrastructure/env'); +phutil_require_module('phutil', 'utils'); + phutil_require_source('ConduitAPI_differential_query_Method.php'); diff --git a/src/applications/differential/editor/revision/DifferentialRevisionEditor.php b/src/applications/differential/editor/revision/DifferentialRevisionEditor.php index 496b4766fb..e31ad1f260 100644 --- a/src/applications/differential/editor/revision/DifferentialRevisionEditor.php +++ b/src/applications/differential/editor/revision/DifferentialRevisionEditor.php @@ -206,7 +206,13 @@ final class DifferentialRevisionEditor { $revision->openTransaction(); + if ($diff) { + $revision->setBranchName($diff->getBranch()); + $revision->setArcanistProjectPHID($diff->getArcanistProjectPHID()); + } + $revision->save(); + if ($diff) { $diff->setRevisionID($revision->getID()); $diff->save(); diff --git a/src/applications/differential/query/revision/DifferentialRevisionQuery.php b/src/applications/differential/query/revision/DifferentialRevisionQuery.php index 2e512a613f..e14aa25470 100644 --- a/src/applications/differential/query/revision/DifferentialRevisionQuery.php +++ b/src/applications/differential/query/revision/DifferentialRevisionQuery.php @@ -51,6 +51,7 @@ final class DifferentialRevisionQuery { private $subscribers = array(); private $responsibles = array(); private $branches = array(); + private $arcanistProjectPHIDs = array(); private $order = 'order-modified'; const ORDER_MODIFIED = 'order-modified'; @@ -244,6 +245,20 @@ final class DifferentialRevisionQuery { } + /** + * Filter results to only return revisions with a given set of arcanist + * projects. + * + * @param array List of project PHIDs. + * @return this + * @task config + */ + public function withArcanistProjectPHIDs(array $arc_project_phids) { + $this->arcanistProjectPHIDs = $arc_project_phids; + return $this; + } + + /** * Set result ordering. Provide a class constant, such as * ##DifferentialRevisionQuery::ORDER_CREATED##. @@ -368,13 +383,10 @@ final class DifferentialRevisionQuery { $this->loadCommitPHIDs($conn_r, $revisions); } - $need_active = $this->needActiveDiffs || - $this->branches; - + $need_active = $this->needActiveDiffs; $need_ids = $need_active || $this->needDiffIDs; - if ($need_ids) { $this->loadDiffIDs($conn_r, $revisions); } @@ -382,31 +394,6 @@ final class DifferentialRevisionQuery { if ($need_active) { $this->loadActiveDiffs($conn_r, $revisions); } - - if ($this->branches) { - - // TODO: We could filter this in SQL instead and might get better - // performance in some cases. - - $branch_map = array_fill_keys($this->branches, true); - foreach ($revisions as $key => $revision) { - $diff = $revision->getActiveDiff(); - if (!$diff) { - unset($revisions[$key]); - continue; - } - - // TODO: Old arc uploaded the wrong branch name for Mercurial (i.e., - // with a trailing "\n"). Once the arc version gets bumped, do a - // migration and remove this. - $branch = trim($diff->getBranch()); - - if (!$diff || empty($branch_map[$branch])) { - unset($revisions[$key]); - continue; - } - } - } } return $revisions; @@ -431,7 +418,9 @@ final class DifferentialRevisionQuery { !$this->authors && !$this->revIDs && !$this->commitHashes && - !$this->phids) { + !$this->phids && + !$this->branches && + !$this->arcanistProjectPHIDs) { return true; } return false; @@ -656,6 +645,20 @@ final class DifferentialRevisionQuery { $this->responsibles); } + if ($this->branches) { + $where[] = qsprintf( + $conn_r, + 'r.branchName in (%Ls)', + $this->branches); + } + + if ($this->arcanistProjectPHIDs) { + $where[] = qsprintf( + $conn_r, + 'r.arcanistProjectPHID in (%Ls)', + $this->arcanistProjectPHIDs); + } + switch ($this->status) { case self::STATUS_ANY: break; diff --git a/src/applications/differential/storage/revision/DifferentialRevision.php b/src/applications/differential/storage/revision/DifferentialRevision.php index bdfb745c96..e25431f6ee 100644 --- a/src/applications/differential/storage/revision/DifferentialRevision.php +++ b/src/applications/differential/storage/revision/DifferentialRevision.php @@ -35,12 +35,15 @@ final class DifferentialRevision extends DifferentialDAO { protected $unsubscribed = array(); protected $mailKey; + protected $branchName; + protected $arcanistProjectPHID; private $relationships; private $commits; private $activeDiff = false; private $diffIDs; + const RELATIONSHIP_TABLE = 'differential_relationship'; const TABLE_COMMIT = 'differential_commit';