diff --git a/src/applications/config/controller/PhabricatorConfigEditController.php b/src/applications/config/controller/PhabricatorConfigEditController.php index 06df0de889..37f7db6c7c 100644 --- a/src/applications/config/controller/PhabricatorConfigEditController.php +++ b/src/applications/config/controller/PhabricatorConfigEditController.php @@ -231,7 +231,6 @@ final class PhabricatorConfigEditController $box_header[] = $key; $crumbs = $this->buildApplicationCrumbs(); - $crumbs->addTextCrumb(pht('Config'), $this->getApplicationURI()); if ($group) { $crumbs->addTextCrumb($group->getName(), $group_uri); } diff --git a/src/applications/config/controller/PhabricatorConfigGroupController.php b/src/applications/config/controller/PhabricatorConfigGroupController.php index 920b2092ac..7068ddaecc 100644 --- a/src/applications/config/controller/PhabricatorConfigGroupController.php +++ b/src/applications/config/controller/PhabricatorConfigGroupController.php @@ -30,7 +30,7 @@ final class PhabricatorConfigGroupController $view = $this->buildConfigBoxView($box_header, $list); $crumbs = $this->buildApplicationCrumbs() - ->addTextCrumb($group_name, $this->getApplicationURI($group_uri)) + ->addTextCrumb($group_name, $group_uri) ->addTextCrumb($options->getName()) ->setBorder(true); diff --git a/src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php index d42c45a96b..62878c0291 100644 --- a/src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php @@ -30,18 +30,31 @@ final class DiffusionBranchQueryConduitAPIMethod $contains = $request->getValue('contains'); if (strlen($contains)) { + + // See PHI720. If the standard "branch" field is provided, use it + // as the "pattern" argument to "git branch ..." to let callers test + // for reachability from a particular branch head. + $pattern = $request->getValue('branch'); + if (strlen($pattern)) { + $pattern_argv = array($pattern); + } else { + $pattern_argv = array(); + } + // NOTE: We can't use DiffusionLowLevelGitRefQuery here because // `git for-each-ref` does not support `--contains`. if ($repository->isWorkingCopyBare()) { list($stdout) = $repository->execxLocalCommand( - 'branch --verbose --no-abbrev --contains %s --', - $contains); + 'branch --verbose --no-abbrev --contains %s -- %Ls', + $contains, + $pattern_argv); $ref_map = DiffusionGitBranch::parseLocalBranchOutput( $stdout); } else { list($stdout) = $repository->execxLocalCommand( - 'branch -r --verbose --no-abbrev --contains %s --', - $contains); + 'branch -r --verbose --no-abbrev --contains %s -- %Ls', + $contains, + $pattern_argv); $ref_map = DiffusionGitBranch::parseRemoteBranchOutput( $stdout, DiffusionGitBranch::DEFAULT_GIT_REMOTE); diff --git a/src/applications/diffusion/controller/DiffusionCommitBranchesController.php b/src/applications/diffusion/controller/DiffusionCommitBranchesController.php index e698ff6fc7..633c3b9174 100644 --- a/src/applications/diffusion/controller/DiffusionCommitBranchesController.php +++ b/src/applications/diffusion/controller/DiffusionCommitBranchesController.php @@ -22,6 +22,7 @@ final class DiffusionCommitBranchesController extends DiffusionController { array( 'contains' => $drequest->getCommit(), 'limit' => $branch_limit + 1, + 'branch' => null, ))); $has_more_branches = (count($branches) > $branch_limit); diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php index 857c939a23..59fb4b5e12 100644 --- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php +++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php @@ -1212,7 +1212,7 @@ final class DiffusionCommitHookEngine extends Phobject { if (!strlen($raw_diff)) { // If the commit is actually empty, just return no changesets. - return array(); + return array(array(), 0); } $parser = new ArcanistDiffParser(); diff --git a/src/applications/diffusion/query/DiffusionCommitQuery.php b/src/applications/diffusion/query/DiffusionCommitQuery.php index 53e8d51bf9..0ca9cd97d4 100644 --- a/src/applications/diffusion/query/DiffusionCommitQuery.php +++ b/src/applications/diffusion/query/DiffusionCommitQuery.php @@ -26,6 +26,7 @@ final class DiffusionCommitQuery private $needCommitData; private $needDrafts; + private $needIdentities; private $mustFilterRefs = false; private $refRepository; @@ -110,6 +111,11 @@ final class DiffusionCommitQuery return $this; } + public function needIdentities($need) { + $this->needIdentities = $need; + return $this; + } + public function needAuditRequests($need) { $this->needAuditRequests = $need; return $this; @@ -393,6 +399,24 @@ final class DiffusionCommitQuery } } + if ($this->needIdentities) { + $identity_phids = array_merge( + mpull($commits, 'getAuthorIdentityPHID'), + mpull($commits, 'getCommitterIdentityPHID')); + + $data = id(new PhabricatorRepositoryIdentityQuery()) + ->withPHIDs($identity_phids) + ->setViewer($this->getViewer()) + ->execute(); + $data = mpull($data, null, 'getPHID'); + + foreach ($commits as $commit) { + $author_identity = idx($data, $commit->getAuthorIdentityPHID()); + $committer_identity = idx($data, $commit->getCommitterIdentityPHID()); + $commit->attachIdentities($author_identity, $committer_identity); + } + } + if ($this->needDrafts) { PhabricatorDraftEngine::attachDrafts( $viewer, diff --git a/src/applications/repository/phid/PhabricatorRepositoryIdentityPHIDType.php b/src/applications/repository/phid/PhabricatorRepositoryIdentityPHIDType.php index 4572e7b005..5bb4d5b907 100644 --- a/src/applications/repository/phid/PhabricatorRepositoryIdentityPHIDType.php +++ b/src/applications/repository/phid/PhabricatorRepositoryIdentityPHIDType.php @@ -28,6 +28,18 @@ final class PhabricatorRepositoryIdentityPHIDType public function loadHandles( PhabricatorHandleQuery $query, array $handles, - array $objects) {} + array $objects) { + + foreach ($handles as $phid => $handle) { + $identity = $objects[$phid]; + + $id = $identity->getID(); + $name = $identity->getIdentityNameRaw(); + + $handle->setObjectName(pht('Identity %d', $id)); + $handle->setName($name); + $handle->setURI($identity->getURI()); + } + } } diff --git a/src/applications/repository/query/PhabricatorRepositoryIdentityQuery.php b/src/applications/repository/query/PhabricatorRepositoryIdentityQuery.php index c64b1a296b..ef038f045f 100644 --- a/src/applications/repository/query/PhabricatorRepositoryIdentityQuery.php +++ b/src/applications/repository/query/PhabricatorRepositoryIdentityQuery.php @@ -124,6 +124,29 @@ final class PhabricatorRepositoryIdentityQuery return $where; } + protected function didFilterPage(array $identities) { + $user_ids = array_filter( + mpull($identities, 'getCurrentEffectiveUserPHID', 'getID')); + if (!$user_ids) { + return $identities; + } + + $users = id(new PhabricatorPeopleQuery()) + ->withPHIDs($user_ids) + ->setViewer($this->getViewer()) + ->execute(); + $users = mpull($users, null, 'getPHID'); + + foreach ($identities as $identity) { + if ($identity->hasEffectiveUser()) { + $user = idx($users, $identity->getCurrentEffectiveUserPHID()); + $identity->attachEffectiveUser($user); + } + } + + return $identities; + } + public function getQueryApplicationClass() { return 'PhabricatorDiffusionApplication'; } diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php index 347f9afcde..abcf862b40 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php +++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php @@ -44,6 +44,9 @@ final class PhabricatorRepositoryCommit private $audits = self::ATTACHABLE; private $repository = self::ATTACHABLE; private $customFields = self::ATTACHABLE; + private $authorIdentity = self::ATTACHABLE; + private $committerIdentity = self::ATTACHABLE; + private $drafts = array(); private $auditAuthorityPHIDs = array(); @@ -191,6 +194,22 @@ final class PhabricatorRepositoryCommit return ($this->audits !== self::ATTACHABLE); } + public function attachIdentities( + PhabricatorRepositoryIdentity $author = null, + PhabricatorRepositoryIdentity $committer = null) { + + $this->authorIdentity = $author; + $this->committerIdentity = $committer; + } + + public function getAuthorIdentity() { + return $this->assertAttached($this->authorIdentity); + } + + public function getCommiterIdentity() { + return $this->assertAttached($this->committerIdentity); + } + public function loadAndAttachAuditAuthority( PhabricatorUser $viewer, $actor_phid = null) { diff --git a/src/applications/repository/storage/PhabricatorRepositoryIdentity.php b/src/applications/repository/storage/PhabricatorRepositoryIdentity.php index 60d4ccb148..bf421692a0 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryIdentity.php +++ b/src/applications/repository/storage/PhabricatorRepositoryIdentity.php @@ -14,6 +14,17 @@ final class PhabricatorRepositoryIdentity protected $manuallySetUserPHID; protected $currentEffectiveUserPHID; + private $effectiveUser = self::ATTACHABLE; + + public function attachEffectiveUser(PhabricatorUser $user) { + $this->effectiveUser = $user; + return $this; + } + + public function getEffectiveUser() { + return $this->assertAttached($this->effectiveUser); + } + protected function getConfiguration() { return array( self::CONFIG_AUX_PHID => true, @@ -63,6 +74,10 @@ final class PhabricatorRepositoryIdentity return '/diffusion/identity/view/'.$this->getID().'/'; } + public function hasEffectiveUser() { + return ($this->currentEffectiveUserPHID != null); + } + public function save() { if ($this->manuallySetUserPHID) { $this->currentEffectiveUserPHID = $this->manuallySetUserPHID; diff --git a/src/applications/search/worker/PhabricatorSearchWorker.php b/src/applications/search/worker/PhabricatorSearchWorker.php index 607baed69a..f93df63981 100644 --- a/src/applications/search/worker/PhabricatorSearchWorker.php +++ b/src/applications/search/worker/PhabricatorSearchWorker.php @@ -15,6 +15,7 @@ final class PhabricatorSearchWorker extends PhabricatorWorker { ), array( 'priority' => parent::PRIORITY_IMPORT, + 'objectPHID' => $phid, )); } @@ -37,7 +38,15 @@ final class PhabricatorSearchWorker extends PhabricatorWorker { $key = "index.{$object_phid}"; $lock = PhabricatorGlobalLock::newLock($key); - $lock->lock(1); + try { + $lock->lock(1); + } catch (PhutilLockException $ex) { + // If we fail to acquire the lock, just yield. It's expected that we may + // contend on this lock occasionally if a large object receives many + // updates in a short period of time, and it's appropriate to just retry + // rebuilding the index later. + throw new PhabricatorWorkerYieldException(15); + } try { // Reload the object now that we have a lock, to make sure we have the diff --git a/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php b/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php index 66285547b3..17f53cfeee 100644 --- a/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php +++ b/src/applications/transactions/conduit/TransactionSearchConduitAPIMethod.php @@ -205,6 +205,9 @@ final class TransactionSearchConduitAPIMethod case PhabricatorTransactions::TYPE_COMMENT: $type = 'comment'; break; + case PhabricatorTransactions::TYPE_CREATE: + $type = 'create'; + break; } } diff --git a/src/applications/transactions/engineextension/PhabricatorTransactionsFulltextEngineExtension.php b/src/applications/transactions/engineextension/PhabricatorTransactionsFulltextEngineExtension.php index c9f43f49b2..4eeda3251c 100644 --- a/src/applications/transactions/engineextension/PhabricatorTransactionsFulltextEngineExtension.php +++ b/src/applications/transactions/engineextension/PhabricatorTransactionsFulltextEngineExtension.php @@ -22,12 +22,28 @@ final class PhabricatorTransactionsFulltextEngineExtension return; } - $xactions = $query + $query ->setViewer($this->getViewer()) ->withObjectPHIDs(array($object->getPHID())) ->withComments(true) - ->needComments(true) - ->execute(); + ->needComments(true); + + // See PHI719. Users occasionally create objects with huge numbers of + // comments, which can be slow to index. We handle this with reasonable + // grace: at time of writing, we can index a task with 100K comments in + // about 30 seconds. However, we do need to hold all the comments in + // memory in the AbstractDocument, so there's some practical limit to what + // we can realistically index. + + // Since objects with more than 1,000 comments are not likely to be + // legitimate objects with actual discussion, index only the first + // thousand comments. + + $query + ->setOrderVector(array('-id')) + ->setLimit(1000); + + $xactions = $query->execute(); foreach ($xactions as $xaction) { if (!$xaction->hasComment()) {