diff --git a/reviewboard/hostingsvcs/github.py b/reviewboard/hostingsvcs/github.py index 2c85db88650f8c380b758b390d222ecaf52af6ae..bcf6e0dcfc0d8af53d7c9e9575adf89eabcbb5c9 100644 --- a/reviewboard/hostingsvcs/github.py +++ b/reviewboard/hostingsvcs/github.py @@ -437,13 +437,13 @@ class GitHub(HostingService): if refname.startswith(self.REFNAME_PREFIX): name = refname[self.REFNAME_PREFIX_LEN:] - results.append(Branch(name=name, + results.append(Branch(id=name, commit=ref['object']['sha'], default=(name == 'master'))) return results - def get_commits(self, repository, start=None): + def get_commits(self, repository, branch=None, start=None): results = [] resource = 'commits' diff --git a/reviewboard/hostingsvcs/service.py b/reviewboard/hostingsvcs/service.py index 9ffd5cbffd355a6ece8600b9f99c40bbd433520b..ba4c3db783db73867fa4252f6dc7d9dbb590ebda 100644 --- a/reviewboard/hostingsvcs/service.py +++ b/reviewboard/hostingsvcs/service.py @@ -145,7 +145,7 @@ class HostingService(object): """ raise NotImplementedError - def get_commits(self, repository, start=None): + def get_commits(self, repository, branch=None, start=None): """Get a list of commits backward in history from a given point. This should be implemented by subclasses, and is expected to return a diff --git a/reviewboard/hostingsvcs/tests.py b/reviewboard/hostingsvcs/tests.py index 811f31340f4747d21f17521154da42ea7e1e7c7c..f364cabb53cc7daeef8195dfe8216e3c21e16211 100644 --- a/reviewboard/hostingsvcs/tests.py +++ b/reviewboard/hostingsvcs/tests.py @@ -1046,15 +1046,15 @@ class GitHubTests(ServiceTests): self.assertEqual( branches, [ - Branch('master', - '859d4e148ce3ce60bbda6622cdbe5c2c2f8d9817', - True), - Branch('release-1.7.x', - '92463764015ef463b4b6d1a1825fee7aeec8cb15', - False), - Branch('some-component/fix', - '764015ef492c8cb1546363b45fee7ab6d1a182ee', - False), + Branch(id='master', + commit='859d4e148ce3ce60bbda6622cdbe5c2c2f8d9817', + default=True), + Branch(id='release-1.7.x', + commit='92463764015ef463b4b6d1a1825fee7aeec8cb15', + default=False), + Branch(id='some-component/fix', + commit='764015ef492c8cb1546363b45fee7ab6d1a182ee', + default=False), ]) def test_get_commits(self): @@ -1112,7 +1112,7 @@ class GitHubTests(ServiceTests): service = account.service commits = service.get_commits( - repository, '859d4e148ce3ce60bbda6622cdbe5c2c2f8d9817') + repository, start='859d4e148ce3ce60bbda6622cdbe5c2c2f8d9817') self.assertEqual(len(commits), 3) self.assertEqual(commits[0].parent, commits[1].id) diff --git a/reviewboard/scmtools/core.py b/reviewboard/scmtools/core.py index bc69d0a24ccf47211a168c64de14fa2008e85e9e..cc8c2f2f75af7c5344bdbd215474ff43600899dc 100644 --- a/reviewboard/scmtools/core.py +++ b/reviewboard/scmtools/core.py @@ -54,19 +54,23 @@ class Revision(object): class Branch(object): - def __init__(self, name='', commit='', default=False): - self.name = name + def __init__(self, id, name=None, commit='', default=False): + assert id + + self.id = id + self.name = name or self.id self.commit = commit self.default = default def __eq__(self, other): - return (self.name == other.name and + return (self.id == other.id and + self.name == other.name and self.commit == other.commit and self.default == other.default) def __repr__(self): - return ('' - % (self.name, self.commit, self.default)) + return ('' + % (self.id, self.name, self.commit, self.default)) class Commit(object): @@ -182,7 +186,7 @@ class SCMTool(object): """ raise NotImplementedError - def get_commits(self, start): + def get_commits(self, branch=None, start=None): """Get a list of commits backward in history from a given point. This should be implemented by subclasses, and is expected to return a diff --git a/reviewboard/scmtools/models.py b/reviewboard/scmtools/models.py index f82f4c5fc09fcd6666c36cfd59f669c1d0566ee3..0d989b2cdc7ab2d2e60bfc9fa8a401db625cadc9 100644 --- a/reviewboard/scmtools/models.py +++ b/reviewboard/scmtools/models.py @@ -259,7 +259,7 @@ class Repository(models.Model): def get_commit_cache_key(self, commit): return 'repository-commit:%s:%s' % (self.pk, commit) - def get_commits(self, start=None): + def get_commits(self, branch=None, start=None): """Returns a list of commits. This is paginated via the 'start' parameter. Any exceptions are @@ -267,12 +267,19 @@ class Repository(models.Model): """ hosting_service = self.hosting_service - cache_key = make_cache_key('repository-commits:%s:%s' - % (self.pk, start)) + cache_key = make_cache_key('repository-commits:%s:%s:%s' + % (self.pk, branch, start)) + commits_kwargs = { + 'branch': branch, + 'start': start, + } + if hosting_service: - commits_callable = lambda: hosting_service.get_commits(self, start) + commits_callable = \ + lambda: hosting_service.get_commits(self, **commits_kwargs) else: - commits_callable = lambda: self.get_scmtool().get_commits(start) + commits_callable = \ + lambda: self.get_scmtool().get_commits(**commits_kwargs) # We cache both the entire list for 'start', as well as each individual # commit. This allows us to reduce API load when people are looking at diff --git a/reviewboard/scmtools/svn/__init__.py b/reviewboard/scmtools/svn/__init__.py index 7f844f6e364fb95d8efe12a13bb10b89779887f5..2a2708f649f69072d44e69b0f03e03475193436e 100644 --- a/reviewboard/scmtools/svn/__init__.py +++ b/reviewboard/scmtools/svn/__init__.py @@ -133,16 +133,16 @@ class SVNTool(SCMTool): if 'trunk' in root_dirents: # Looks like the standard layout. Adds trunk and any branches. - results.append(Branch(name='trunk', - commit=root_dirents['trunk']['created_rev'], - default=True)) + trunk = root_dirents['trunk'] + results.append(self._create_branch_from_dirent( + 'trunk', trunk, default=True)) if 'branches' in root_dirents: try: dirents = self.client.list_dir('branches') results += [ - Branch(name=name, commit=dirents[name]['created_rev']) + self._create_branch_from_dirent(name, dirents[name]) for name in sorted(six.iterkeys(dirents)) ] except Exception as e: @@ -155,16 +155,15 @@ class SVNTool(SCMTool): default = True for name in sorted(six.iterkeys(root_dirents)): - results.append(Branch(name=name, - commit=dirents[name]['created_rev'], - default=default)) + results.append(self._create_branch_from_dirent( + name, root_dirents[name], default)) default = False return results - def get_commits(self, start): + def get_commits(self, branch=None, start=None): """Return a list of commits.""" - commits = self.client.get_log('/', + commits = self.client.get_log(branch or '/', start=start, limit=self.COMMITS_PAGE_LIMIT, limit_to_path=False) @@ -296,6 +295,13 @@ class SVNTool(SCMTool): def get_parser(self, data): return SVNDiffParser(data) + def _create_branch_from_dirent(self, name, dirent, default=False): + return Branch( + id=dirent['path'].strip('/'), + name=name, + commit=dirent['created_rev'], + default=default) + @classmethod def _ssl_server_trust_prompt(cls, trust_dict, repository): """Callback for SSL cert verification. diff --git a/reviewboard/scmtools/svn/pysvn.py b/reviewboard/scmtools/svn/pysvn.py index 55b337d03a438b10fdbf444a69691f21fc0be7ef..9ca4226fe4e35f1e1a455ab8ec066e0b7b7dc342 100644 --- a/reviewboard/scmtools/svn/pysvn.py +++ b/reviewboard/scmtools/svn/pysvn.py @@ -235,14 +235,16 @@ class Client(base.Client): created. """ result = SortedDict() - dirents = self.client.list(self.normalize_path(path), - recurse=False)[1:] + norm_path = self.normalize_path(path) + dirents = self.client.list(norm_path, recurse=False)[1:] + + repo_path_len = len(self.repopath) for dirent, unused in dirents: name = dirent['path'].split('/')[-1] result[name] = { - 'path': dirent['path'], + 'path': dirent['path'][repo_path_len:], 'created_rev': six.text_type(dirent['created_rev'].number), } diff --git a/reviewboard/scmtools/tests.py b/reviewboard/scmtools/tests.py index 546d61fe3d52bc6213f3a409163904a7e1ed3d93..d976eb3163ce545536df56172b70eb016b218aaf 100644 --- a/reviewboard/scmtools/tests.py +++ b/reviewboard/scmtools/tests.py @@ -942,12 +942,15 @@ class CommonSVNTestsBase(SCMTestCase): branches = self.tool.get_branches() self.assertEqual(len(branches), 2) - self.assertEqual(branches[0], Branch('trunk', '5', True)) - self.assertEqual(branches[1], Branch('branch1', '7', False)) + self.assertEqual(branches[0], Branch(id='trunk', name='trunk', + commit='5', default=True)) + self.assertEqual(branches[1], Branch(id='branches/branch1', + name='branch1', + commit='7', default=False)) def test_get_commits(self): """Testing SVN () get_commits""" - commits = self.tool.get_commits('5') + commits = self.tool.get_commits(start='5') self.assertEqual(len(commits), 5) self.assertEqual( @@ -958,7 +961,7 @@ class CommonSVNTestsBase(SCMTestCase): 'Add an unterminated keyword for testing bug #1523\n', '4')) - commits = self.tool.get_commits('7') + commits = self.tool.get_commits(start='7') self.assertEqual(len(commits), 7) self.assertEqual( commits[1], @@ -968,6 +971,36 @@ class CommonSVNTestsBase(SCMTestCase): 'Add a branches directory', '5')) + def test_get_commits_with_branch(self): + """Testing SVN () get_commits with branch""" + commits = self.tool.get_commits(branch='/branches/branch1', start='5') + + self.assertEqual(len(commits), 5) + self.assertEqual( + commits[0], + Commit('chipx86', + '5', + '2010-05-21T09:33:40.893946', + 'Add an unterminated keyword for testing bug #1523\n', + '4')) + + commits = self.tool.get_commits(branch='/branches/branch1', start='7') + self.assertEqual(len(commits), 6) + self.assertEqual( + commits[0], + Commit('david', + '7', + '2013-06-13T07:43:27.259554', + 'Add a branch', + '5')) + self.assertEqual( + commits[1], + Commit('chipx86', + '5', + '2010-05-21T09:33:40.893946', + 'Add an unterminated keyword for testing bug #1523\n', + '4')) + def test_get_change(self): """Testing SVN () get_change""" commit = self.tool.get_change('5') diff --git a/reviewboard/webapi/resources/repository_commits.py b/reviewboard/webapi/resources/repository_commits.py index 9499cc5e4fc3a00a6180ffc2e7ed5bd78271f606..e7cfa6f31cf3b2f5e2c6eb8c0205cdff755f39af 100644 --- a/reviewboard/webapi/resources/repository_commits.py +++ b/reviewboard/webapi/resources/repository_commits.py @@ -74,7 +74,7 @@ class RepositoryCommitsResource(WebAPIResource): return DOES_NOT_EXIST try: - items = repository.get_commits(start) + items = repository.get_commits(start=start) except NotImplementedError: return REPO_NOT_IMPLEMENTED