diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 1924d407e2..7c21701ba5 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -467,7 +467,6 @@ phutil_register_library_map(array( 'DifferentialUnitStatus' => 'applications/differential/constants/DifferentialUnitStatus.php', 'DifferentialUnitTestResult' => 'applications/differential/constants/DifferentialUnitTestResult.php', 'DifferentialViewPolicyFieldSpecification' => 'applications/differential/field/specification/DifferentialViewPolicyFieldSpecification.php', - 'DiffusionBranchInformation' => 'applications/diffusion/data/DiffusionBranchInformation.php', 'DiffusionBranchTableController' => 'applications/diffusion/controller/DiffusionBranchTableController.php', 'DiffusionBranchTableView' => 'applications/diffusion/view/DiffusionBranchTableView.php', 'DiffusionBrowseController' => 'applications/diffusion/controller/DiffusionBrowseController.php', @@ -3014,6 +3013,7 @@ phutil_register_library_map(array( 1 => 'PhabricatorApplicationSearchResultsControllerInterface', ), 'DiffusionRepositoryNewController' => 'DiffusionController', + 'DiffusionRepositoryRef' => 'Phobject', 'DiffusionRepositoryRemarkupRule' => 'PhabricatorRemarkupRuleObject', 'DiffusionResolveUserQuery' => 'Phobject', 'DiffusionSSHGitReceivePackWorkflow' => 'DiffusionSSHGitWorkflow', diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php index 92f3b14165..4d4bd37390 100644 --- a/src/applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php +++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php @@ -21,62 +21,53 @@ final class ConduitAPI_diffusion_branchquery_Method ); } + protected function getGitResult(ConduitAPIRequest $request) { $drequest = $this->getDiffusionRequest(); $repository = $drequest->getRepository(); - $limit = $request->getValue('limit'); - $offset = $request->getValue('offset'); $refs = id(new DiffusionLowLevelGitRefQuery()) ->setRepository($repository) ->withIsOriginBranch(true) ->execute(); - $branches = array(); - foreach ($refs as $ref) { - $branch = id(new DiffusionBranchInformation()) - ->setName($ref->getShortName()) - ->setHeadCommitIdentifier($ref->getCommitIdentifier()); + return $this->processBranchRefs($request, $refs); + } - if (!$repository->shouldTrackBranch($branch->getName())) { - continue; + protected function getMercurialResult(ConduitAPIRequest $request) { + $drequest = $this->getDiffusionRequest(); + $repository = $drequest->getRepository(); + + $refs = id(new DiffusionLowLevelMercurialBranchesQuery()) + ->setRepository($repository) + ->execute(); + + return $this->processBranchRefs($request, $refs); + } + + private function processBranchRefs(ConduitAPIRequest $request, array $refs) { + $drequest = $this->getDiffusionRequest(); + $repository = $drequest->getRepository(); + $offset = $request->getValue('offset'); + $limit = $request->getValue('limit'); + + foreach ($refs as $key => $ref) { + if (!$repository->shouldTrackBranch($ref->getShortName())) { + unset($refs[$key]); } - - $branches[] = $branch->toDictionary(); } // NOTE: We can't apply the offset or limit until here, because we may have // filtered untrackable branches out of the result set. if ($offset) { - $branches = array_slice($branches, $offset); + $refs = array_slice($refs, $offset); } if ($limit) { - $branches = array_slice($branches, 0, $limit); + $refs = array_slice($refs, 0, $limit); } - return $branches; - } - - protected function getMercurialResult(ConduitAPIRequest $request) { - $drequest = $this->getDiffusionRequest(); - $repository = $drequest->getRepository(); - $offset = $request->getValue('offset'); - $limit = $request->getValue('limit'); - - $branches = id(new DiffusionLowLevelMercurialBranchesQuery()) - ->setRepository($repository) - ->execute(); - - if ($offset) { - $branches = array_slice($branches, $offset); - } - - if ($limit) { - $branches = array_slice($branches, 0, $limit); - } - - return mpull($branches, 'toDictionary'); + return mpull($refs, 'toDictionary'); } } diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_commitbranchesquery_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_commitbranchesquery_Method.php index 65d503d114..4ff454f0f5 100644 --- a/src/applications/diffusion/conduit/ConduitAPI_diffusion_commitbranchesquery_Method.php +++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_commitbranchesquery_Method.php @@ -39,7 +39,7 @@ final class ConduitAPI_diffusion_commitbranchesquery_Method $commit); return DiffusionGitBranch::parseRemoteBranchOutput( $contains, - DiffusionBranchInformation::DEFAULT_GIT_REMOTE); + DiffusionGitBranch::DEFAULT_GIT_REMOTE); } } @@ -48,7 +48,10 @@ final class ConduitAPI_diffusion_commitbranchesquery_Method $repository = $drequest->getRepository(); $commit = $request->getValue('commit'); - // TODO: This should use `branches`. + // TODO: This should use `branches`. Also, this entire method's API needs + // to be fixed to support multiple branch heads in Mercurial. We should + // probably return `DiffusionRepositoryRefs` and probably merge this into + // `diffusion.branchesquery`. list($contains) = $repository->execxLocalCommand( 'log --template %s --limit 1 --rev %s --', diff --git a/src/applications/diffusion/controller/DiffusionBranchTableController.php b/src/applications/diffusion/controller/DiffusionBranchTableController.php index dc3084aafc..d6507e6746 100644 --- a/src/applications/diffusion/controller/DiffusionBranchTableController.php +++ b/src/applications/diffusion/controller/DiffusionBranchTableController.php @@ -18,15 +18,16 @@ final class DiffusionBranchTableController extends DiffusionController { $pager->setOffset($request->getInt('offset')); // TODO: Add support for branches that contain commit - $branches = DiffusionBranchInformation::newFromConduit( - $this->callConduitWithDiffusionRequest( - 'diffusion.branchquery', - array( - 'offset' => $pager->getOffset(), - 'limit' => $pager->getPageSize() + 1 - ))); + $branches = $this->callConduitWithDiffusionRequest( + 'diffusion.branchquery', + array( + 'offset' => $pager->getOffset(), + 'limit' => $pager->getPageSize() + 1 + )); $branches = $pager->sliceResults($branches); + $branches = DiffusionRepositoryRef::loadAllFromDictionaries($branches); + $content = null; if (!$branches) { $content = $this->renderStatusMessage( @@ -35,7 +36,7 @@ final class DiffusionBranchTableController extends DiffusionController { } else { $commits = id(new DiffusionCommitQuery()) ->setViewer($viewer) - ->withIdentifiers(mpull($branches, 'getHeadCommitIdentifier')) + ->withIdentifiers(mpull($branches, 'getCommitIdentifier')) ->withRepository($repository) ->execute(); diff --git a/src/applications/diffusion/controller/DiffusionRepositoryController.php b/src/applications/diffusion/controller/DiffusionRepositoryController.php index b1d36d485c..f426d634fd 100644 --- a/src/applications/diffusion/controller/DiffusionRepositoryController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryController.php @@ -261,12 +261,11 @@ final class DiffusionRepositoryController extends DiffusionController { $limit = 15; - $branches = DiffusionBranchInformation::newFromConduit( - $this->callConduitWithDiffusionRequest( - 'diffusion.branchquery', - array( - 'limit' => $limit + 1, - ))); + $branches = $this->callConduitWithDiffusionRequest( + 'diffusion.branchquery', + array( + 'limit' => $limit + 1, + )); if (!$branches) { return null; } @@ -274,9 +273,11 @@ final class DiffusionRepositoryController extends DiffusionController { $more_branches = (count($branches) > $limit); $branches = array_slice($branches, 0, $limit); + $branches = DiffusionRepositoryRef::loadAllFromDictionaries($branches); + $commits = id(new DiffusionCommitQuery()) ->setViewer($viewer) - ->withIdentifiers(mpull($branches, 'getHeadCommitIdentifier')) + ->withIdentifiers(mpull($branches, 'getCommitIdentifier')) ->withRepository($drequest->getRepository()) ->execute(); diff --git a/src/applications/diffusion/data/DiffusionBranchInformation.php b/src/applications/diffusion/data/DiffusionBranchInformation.php deleted file mode 100644 index e84455b8da..0000000000 --- a/src/applications/diffusion/data/DiffusionBranchInformation.php +++ /dev/null @@ -1,56 +0,0 @@ -name = $name; - return $this; - } - - public function getName() { - return $this->name; - } - - public function setHeadCommitIdentifier($head_commit_identifier) { - $this->headCommitIdentifier = $head_commit_identifier; - return $this; - } - - public function getHeadCommitIdentifier() { - return $this->headCommitIdentifier; - } - - public static function newFromConduit(array $dicts) { - $branches = array(); - foreach ($dicts as $dict) { - $branches[] = id(new DiffusionBranchInformation()) - ->setName($dict['name']) - ->setHeadCommitIdentifier($dict['headCommitIdentifier']); - } - return $branches; - } - - public function toDictionary() { - return array( - 'name' => $this->getName(), - 'headCommitIdentifier' => $this->getHeadCommitIdentifier() - ); - } - - // TODO: These are hacks to make this compatible with DiffusionRepositoryRef - // for PhabricatorRepositoryRefEngine. The two classes should be merged. - - public function getShortName() { - return $this->getName(); - } - - public function getCommitIdentifier() { - return $this->getHeadCommitIdentifier(); - } - -} diff --git a/src/applications/diffusion/data/DiffusionGitBranch.php b/src/applications/diffusion/data/DiffusionGitBranch.php index b1456c6102..925ede568c 100644 --- a/src/applications/diffusion/data/DiffusionGitBranch.php +++ b/src/applications/diffusion/data/DiffusionGitBranch.php @@ -2,6 +2,8 @@ final class DiffusionGitBranch { + const DEFAULT_GIT_REMOTE = 'origin'; + /** * Parse the output of 'git branch -r --verbose --no-abbrev' or similar into * a map. For instance: diff --git a/src/applications/diffusion/data/DiffusionRepositoryRef.php b/src/applications/diffusion/data/DiffusionRepositoryRef.php index b19c6e5cdc..45c92c8c0a 100644 --- a/src/applications/diffusion/data/DiffusionRepositoryRef.php +++ b/src/applications/diffusion/data/DiffusionRepositoryRef.php @@ -1,10 +1,13 @@ rawFields = $raw_fields; @@ -33,4 +36,31 @@ final class DiffusionRepositoryRef { return $this->shortName; } + +/* -( Serialization )------------------------------------------------------ */ + + + public function toDictionary() { + return array( + 'shortName' => $this->shortName, + 'commitIdentifier' => $this->commitIdentifier, + 'rawFields' => $this->rawFields, + ); + } + + public static function newFromDictionary(array $dict) { + return id(new DiffusionRepositoryRef()) + ->setShortName($dict['shortName']) + ->setCommitIdentifier($dict['commitIdentifier']) + ->setRawFields($dict['rawFields']); + } + + public static function loadAllFromDictionaries(array $dictionaries) { + $refs = array(); + foreach ($dictionaries as $dictionary) { + $refs[] = self::newFromDictionary($dictionary); + } + return $refs; + } + } diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelGitRefQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelGitRefQuery.php index 9305042868..82de8e404a 100644 --- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelGitRefQuery.php +++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelGitRefQuery.php @@ -32,7 +32,7 @@ final class DiffusionLowLevelGitRefQuery extends DiffusionLowLevelQuery { if ($repository->isWorkingCopyBare()) { $prefix = 'refs/heads/'; } else { - $remote = DiffusionBranchInformation::DEFAULT_GIT_REMOTE; + $remote = DiffusionGitBranch::DEFAULT_GIT_REMOTE; $prefix = 'refs/remotes/'.$remote.'/'; } } else { diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialBranchesQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialBranchesQuery.php index 602b2b6123..f779794b51 100644 --- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialBranchesQuery.php +++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialBranchesQuery.php @@ -18,9 +18,9 @@ final class DiffusionLowLevelMercurialBranchesQuery $lines = ArcanistMercurialParser::parseMercurialBranches($stdout); foreach ($lines as $name => $spec) { - $branches[] = id(new DiffusionBranchInformation()) - ->setName($name) - ->setHeadCommitIdentifier($spec['rev']); + $branches[] = id(new DiffusionRepositoryRef()) + ->setShortName($name) + ->setCommitIdentifier($spec['rev']); } return $branches; diff --git a/src/applications/diffusion/request/DiffusionGitRequest.php b/src/applications/diffusion/request/DiffusionGitRequest.php index 2f411c6584..584ab3acf3 100644 --- a/src/applications/diffusion/request/DiffusionGitRequest.php +++ b/src/applications/diffusion/request/DiffusionGitRequest.php @@ -39,7 +39,7 @@ final class DiffusionGitRequest extends DiffusionRequest { if ($this->repository->isWorkingCopyBare()) { return $branch; } else { - $remote = DiffusionBranchInformation::DEFAULT_GIT_REMOTE; + $remote = DiffusionGitBranch::DEFAULT_GIT_REMOTE; return $remote.'/'.$branch; } } diff --git a/src/applications/diffusion/view/DiffusionBranchTableView.php b/src/applications/diffusion/view/DiffusionBranchTableView.php index 001af93685..7cbdb0d089 100644 --- a/src/applications/diffusion/view/DiffusionBranchTableView.php +++ b/src/applications/diffusion/view/DiffusionBranchTableView.php @@ -6,12 +6,13 @@ final class DiffusionBranchTableView extends DiffusionView { private $commits = array(); public function setBranches(array $branches) { - assert_instances_of($branches, 'DiffusionBranchInformation'); + assert_instances_of($branches, 'DiffusionRepositoryRef'); $this->branches = $branches; return $this; } public function setCommits(array $commits) { + assert_instances_of($commits, 'PhabricatorRepositoryCommit'); $this->commits = mpull($commits, null, 'getCommitIdentifier'); return $this; } @@ -23,7 +24,7 @@ final class DiffusionBranchTableView extends DiffusionView { $rows = array(); $rowc = array(); foreach ($this->branches as $branch) { - $commit = idx($this->commits, $branch->getHeadCommitIdentifier()); + $commit = idx($this->commits, $branch->getCommitIdentifier()); if ($commit) { $details = $commit->getSummary(); $datetime = phabricator_datetime($commit->getEpoch(), $this->user); @@ -39,7 +40,7 @@ final class DiffusionBranchTableView extends DiffusionView { 'href' => $drequest->generateURI( array( 'action' => 'history', - 'branch' => $branch->getName(), + 'branch' => $branch->getShortName(), )) ), pht('History')), @@ -49,17 +50,17 @@ final class DiffusionBranchTableView extends DiffusionView { 'href' => $drequest->generateURI( array( 'action' => 'browse', - 'branch' => $branch->getName(), + 'branch' => $branch->getShortName(), )), ), - $branch->getName()), + $branch->getShortName()), self::linkCommit( $drequest->getRepository(), - $branch->getHeadCommitIdentifier()), + $branch->getCommitIdentifier()), $datetime, AphrontTableView::renderSingleDisplayLine($details), ); - if ($branch->getName() == $current_branch) { + if ($branch->getShortName() == $current_branch) { $rowc[] = 'highlighted'; } else { $rowc[] = null;