diff --git a/reviewboard/hostingsvcs/github.py b/reviewboard/hostingsvcs/github.py index 329f48e0b6cf54d984c6a557c301c450ec964068..a6915c4b32885e48c58cb61254cb6d2272678821 100644 --- a/reviewboard/hostingsvcs/github.py +++ b/reviewboard/hostingsvcs/github.py @@ -274,18 +274,27 @@ class GitHubClient(HostingServiceClient): url, e, exc_info=1) raise SCMError(six.text_type(e)) - def api_get_remote_repositories(self, api_url, owner, plan, - start=None, per_page=None): + def api_get_remote_repositories(self, api_url, owner, owner_type, + filter_type=None, start=None, + per_page=None): url = api_url - if plan.endswith('org'): + if owner_type == 'organization': url += 'orgs/%s/repos' % owner - elif owner == self.account.username: - # All repositories belonging to an authenticated user. - url += 'user/repos' + elif owner_type == 'user': + if owner == self.account.username: + # All repositories belonging to an authenticated user. + url += 'user/repos' + else: + # Only public repositories for the user. + url += 'users/%s/repos' % owner else: - # Only public repositories for the user. - url += 'users/%s/repos?type=all' % owner + raise ValueError( + "owner_type must be 'organization' or 'user', not %r'" + % owner_type) + + if filter_type: + url += '?type=%s' % (filter_type or 'all') return self.api_get_list(self._build_api_url(url), start=start, per_page=per_page) @@ -771,8 +780,8 @@ class GitHub(HostingService): return Commit(author_name, revision, date, message, parent_revision, diff=diff) - def get_remote_repositories(self, owner=None, plan=None, start=None, - per_page=None): + def get_remote_repositories(self, owner=None, owner_type='user', + filter_type=None, start=None, per_page=None): """Return a list of remote repositories matching the given criteria. This will look up each remote repository on GitHub that the given @@ -794,18 +803,14 @@ class GitHub(HostingService): `owner` defaults to the linked account's username, and `plan` defaults to 'public'. """ - if owner is None: + if owner is None and owner_type == 'user': owner = self.account.username - if plan is None: - plan = 'public' - - if plan not in ('public', 'private', 'public-org', 'private-org'): - raise InvalidPlanError(plan) + assert owner url = self.get_api_url(self.account.hosting_url) - paginator = self.client.api_get_remote_repositories(url, owner, plan, - start, per_page) + paginator = self.client.api_get_remote_repositories( + url, owner, owner_type, filter_type, start, per_page) return ProxyPaginator( paginator, diff --git a/reviewboard/hostingsvcs/service.py b/reviewboard/hostingsvcs/service.py index d12297bf3e79678f5482306b09e30fd785e88181..9f8c12f3b2fda168fc90955488aca25cb3bc9912 100644 --- a/reviewboard/hostingsvcs/service.py +++ b/reviewboard/hostingsvcs/service.py @@ -292,8 +292,8 @@ class HostingService(object): """ raise NotImplementedError - def get_remote_repositories(self, owner=None, plan=None, start=None, - per_page=None): + def get_remote_repositories(self, owner=None, owner_type=None, + filter_type=None, start=None, per_page=None): """Get a list of remote repositories for the owner and plan. This should be implemented by subclasses, and is expected to return an @@ -304,9 +304,12 @@ class HostingService(object): if the subclass supports it. ``owner`` is expected to default to a reasonable value (typically - the linked account's username). Likewise, ``plan`` is expected to - default to something appropriate, if the hosting service supports - plans. + the linked account's username). The hosting service may also require + an ``owner_type`` value that identifies what the ``owner`` means. + This value is specific to the hosting service backend. + + Likewise, ``filter_type`` is specific to the hosting service backend. + If supported, it may be used to filter the types of hosting services. """ raise NotImplementedError diff --git a/reviewboard/hostingsvcs/tests.py b/reviewboard/hostingsvcs/tests.py index 7423eb6d6dd0dee10d4ca804fb95ee5fed07b37d..a5e203751653f5af0e609b9f4d77a2b5b5242ee7 100644 --- a/reviewboard/hostingsvcs/tests.py +++ b/reviewboard/hostingsvcs/tests.py @@ -1342,7 +1342,7 @@ class GitHubTests(ServiceTests): service = account.service self.spy_on(service.client.http_get, call_fake=_http_get) - paginator = service.get_remote_repositories('myuser', 'public') + paginator = service.get_remote_repositories('myuser') # Check the first result. self.assertEqual(len(paginator.page_data), 1) @@ -1371,7 +1371,7 @@ class GitHubTests(ServiceTests): self.assertEqual(repo.path, 'myrepo_path2') self.assertEqual(repo.mirror_path, 'myrepo_mirror2') - def test_get_remote_repositories_with_other(self, **kwargs): + def test_get_remote_repositories_with_other_user(self, **kwargs): """Testing GitHub.get_remote_repositories with requesting user's repositories """ @@ -1391,7 +1391,7 @@ class GitHubTests(ServiceTests): def _http_get(service, url, *args, **kwargs): base_url = ('https://api.github.com/users/other/repos' - '?access_token=123&type=all') + '?access_token=123') self.assertIn(url, [base_url, '%s&page=2' % base_url]) @@ -1409,7 +1409,7 @@ class GitHubTests(ServiceTests): service = account.service self.spy_on(service.client.http_get, call_fake=_http_get) - paginator = service.get_remote_repositories('other', 'public') + paginator = service.get_remote_repositories('other') self.assertEqual(len(paginator.page_data), 1) public_repo = paginator.page_data[0] @@ -1461,7 +1461,7 @@ class GitHubTests(ServiceTests): service = account.service self.spy_on(service.client.http_get, call_fake=_http_get) - paginator = service.get_remote_repositories('myorg', 'public-org') + paginator = service.get_remote_repositories('myorg', 'organization') self.assertEqual(len(paginator.page_data), 2) public_repo, private_repo = paginator.page_data @@ -1479,6 +1479,44 @@ class GitHubTests(ServiceTests): self.assertEqual(private_repo.path, 'myrepo_path2') self.assertEqual(private_repo.mirror_path, 'myrepo_mirror2') + def test_get_remote_repositories_with_defaults(self, **kwargs): + """Testing GitHub.get_remote_repositories with default values""" + def _http_get(service, url, *args, **kwargs): + self.assertEqual( + url, + 'https://api.github.com/user/repos?access_token=123') + + return b'{}', {} + + account = self._get_hosting_account() + account.data['authorization'] = { + 'token': '123', + } + + service = account.service + self.spy_on(service.client.http_get, call_fake=_http_get) + + service.get_remote_repositories() + + def test_get_remote_repositories_with_filter(self, **kwargs): + """Testing GitHub.get_remote_repositories with ?filter-type=""" + def _http_get(service, url, *args, **kwargs): + self.assertEqual(url, + 'https://api.github.com/user/repos' + '?access_token=123&type=private') + + return json.dumps([]), {} + + account = self._get_hosting_account() + account.data['authorization'] = { + 'token': '123', + } + + service = account.service + self.spy_on(service.client.http_get, call_fake=_http_get) + + service.get_remote_repositories('myuser', filter_type='private') + def _test_check_repository(self, expected_user='myuser', **kwargs): def _http_get(service, url, *args, **kwargs): self.assertEqual(