From 2a37459a5f5a68638924188dff8d5b3b98873eb9 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 27 Apr 2015 03:51:53 -0700 Subject: [PATCH] Only resolve branch names to branches Summary: Fixes T7100. In the bizarre case that a Git repository has a branch and tag with the same name, don't resolve branch names into tag names. Test Plan: Test repo with branch and tag both named "git" no longer reports ambiguity. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T7100 Differential Revision: https://secure.phabricator.com/D12553 --- .../DiffusionResolveRefsConduitAPIMethod.php | 13 +++++++++--- .../query/DiffusionCachedResolveRefsQuery.php | 10 ++++++++++ .../query/lowlevel/DiffusionLowLevelQuery.php | 17 ++++++++++++++++ .../DiffusionLowLevelResolveRefsQuery.php | 10 ++++++++++ .../diffusion/request/DiffusionRequest.php | 20 ++++++++++++++----- 5 files changed, 62 insertions(+), 8 deletions(-) diff --git a/src/applications/diffusion/conduit/DiffusionResolveRefsConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionResolveRefsConduitAPIMethod.php index 0cfb2bf565..4971ef5552 100644 --- a/src/applications/diffusion/conduit/DiffusionResolveRefsConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionResolveRefsConduitAPIMethod.php @@ -18,16 +18,23 @@ final class DiffusionResolveRefsConduitAPIMethod protected function defineCustomParamTypes() { return array( 'refs' => 'required list', + 'types' => 'optional list', ); } protected function getResult(ConduitAPIRequest $request) { $refs = $request->getValue('refs'); + $types = $request->getValue('types'); - return id(new DiffusionLowLevelResolveRefsQuery()) + $query = id(new DiffusionLowLevelResolveRefsQuery()) ->setRepository($this->getDiffusionRequest()->getRepository()) - ->withRefs($refs) - ->execute(); + ->withRefs($refs); + + if ($types) { + $query->withTypes($types); + } + + return $query->execute(); } } diff --git a/src/applications/diffusion/query/DiffusionCachedResolveRefsQuery.php b/src/applications/diffusion/query/DiffusionCachedResolveRefsQuery.php index 5dbadb3791..8190a035f2 100644 --- a/src/applications/diffusion/query/DiffusionCachedResolveRefsQuery.php +++ b/src/applications/diffusion/query/DiffusionCachedResolveRefsQuery.php @@ -16,12 +16,18 @@ final class DiffusionCachedResolveRefsQuery extends DiffusionLowLevelQuery { private $refs; + private $types; public function withRefs(array $refs) { $this->refs = $refs; return $this; } + public function withTypes(array $types) { + $this->types = $types; + return $this; + } + protected function executeQuery() { if (!$this->refs) { return array(); @@ -39,6 +45,10 @@ final class DiffusionCachedResolveRefsQuery throw new Exception('Unsupported repository type!'); } + if ($this->types !== null) { + $result = $this->filterRefsByType($result, $this->types); + } + return $result; } diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelQuery.php index 6c19f9839b..0e8472c24d 100644 --- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelQuery.php +++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelQuery.php @@ -23,4 +23,21 @@ abstract class DiffusionLowLevelQuery extends Phobject { return $this->executeQuery(); } + protected function filterRefsByType(array $refs, array $types) { + $type_map = array_fuse($types); + + foreach ($refs as $name => $ref_list) { + foreach ($ref_list as $key => $ref) { + if (empty($type_map[$ref['type']])) { + unset($refs[$name][$key]); + } + } + if (!$refs[$name]) { + unset($refs[$name]); + } + } + + return $refs; + } + } diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php index 7d8c01d890..556ddf0904 100644 --- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php +++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php @@ -14,12 +14,18 @@ final class DiffusionLowLevelResolveRefsQuery extends DiffusionLowLevelQuery { private $refs; + private $types; public function withRefs(array $refs) { $this->refs = $refs; return $this; } + public function withTypes(array $types) { + $this->types = $types; + return $this; + } + protected function executeQuery() { if (!$this->refs) { return array(); @@ -39,6 +45,10 @@ final class DiffusionLowLevelResolveRefsQuery throw new Exception('Unsupported repository type!'); } + if ($this->types !== null) { + $result = $this->filterRefsByType($result, $this->types); + } + return $result; } diff --git a/src/applications/diffusion/request/DiffusionRequest.php b/src/applications/diffusion/request/DiffusionRequest.php index 0bd386d7ff..2af40ac930 100644 --- a/src/applications/diffusion/request/DiffusionRequest.php +++ b/src/applications/diffusion/request/DiffusionRequest.php @@ -718,17 +718,21 @@ abstract class DiffusionRequest { } private function queryStableCommit() { + $types = array(); if ($this->symbolicCommit) { $ref = $this->symbolicCommit; } else { if ($this->supportsBranches()) { $ref = $this->getResolvableBranchName($this->getBranch()); + $types = array( + PhabricatorRepositoryRefCursor::TYPE_BRANCH, + ); } else { $ref = 'HEAD'; } } - $results = $this->resolveRefs(array($ref)); + $results = $this->resolveRefs(array($ref), $types); $matches = idx($results, $ref, array()); if (!$matches) { @@ -790,12 +794,17 @@ abstract class DiffusionRequest { return $branch; } - private function resolveRefs(array $refs) { + private function resolveRefs(array $refs, array $types) { // First, try to resolve refs from fast cache sources. - $cached_results = id(new DiffusionCachedResolveRefsQuery()) + $cached_query = id(new DiffusionCachedResolveRefsQuery()) ->setRepository($this->getRepository()) - ->withRefs($refs) - ->execute(); + ->withRefs($refs); + + if ($types) { + $cached_query->withTypes($types); + } + + $cached_results = $cached_query->execute(); // Throw away all the refs we resolved. Hopefully, we'll throw away // everything here. @@ -813,6 +822,7 @@ abstract class DiffusionRequest { $this, 'diffusion.resolverefs', array( + 'types' => $types, 'refs' => $refs, )); } else { -- 2.51.2