Skip to content

Commit

Permalink
bug: fix private repo listing for a user
Browse files Browse the repository at this point in the history 
Private repositories are not included in the /users/$name/repos API
results. Use /user/repos?affiliation=owner instead to list only owned
repositories for the current authenticated user.
  • Loading branch information
@dhellmann
dhellmann committed Jul 3, 2022
1 parent 6f07b4d commit ecdee50
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 22 deletions.
33 changes: 19 additions & 14 deletions ghcloneall.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
__author__ = 'Marius Gedminas <marius@gedmin.as>'
__licence__ = 'MIT'
__url__ = 'https://github.com/mgedmin/ghcloneall'
__version__ = '1.10.2.dev0'
__version__ = '1.10.1'


CONFIG_FILE = '.ghcloneallrc'
Expand Down Expand Up @@ -425,27 +425,33 @@ def list_gists(self, user, pattern=None):
def list_repos(self, user=None, organization=None, pattern=None,
include_archived=False, include_forks=False,
include_private=True, include_disabled=True):

# User repositories default to sort=full_name, org repositories default
# to sort=created. In theory we don't care because we will sort the
# list ourselves, but in the future I may want to start cloning in
# parallel with the paginated fetching. This requires the sorting to
# happen before pagination, i.e. on the server side, as I want to
# process the repositories alphabetically (both for aesthetic reasons,
# and in order for --start-from to be useful).

if organization and not user:
owner = organization
list_url = 'https://api.github.com/orgs/{}/repos'.format(owner)
list_url = 'https://api.github.com/orgs/{}/repos?sort=full_name'.format(owner)
elif user and not organization:
owner = user
list_url = 'https://api.github.com/users/{}/repos'.format(owner)
if include_private:
# users/$name/repos does not include private repos, so we have
# to query for the repos owned by the current user instead.
# This only works if the current token is associated with that user.
list_url = 'https://api.github.com/user/repos?affiliation=owner&sort=full_name'
else:
list_url = 'https://api.github.com/users/{}/repos?sort=full_name'.format(owner)
else:
raise ValueError('specify either user or organization, not both')

message = "Fetching list of {}'s repositories from GitHub...".format(
owner)

# User repositories default to sort=full_name, org repositories default
# to sort=created. In theory we don't care because we will sort the
# list ourselves, but in the future I may want to start cloning in
# parallel with the paginated fetching. This requires the sorting to
# happen before pagination, i.e. on the server side, as I want to
# process the repositories alphabetically (both for aesthetic reasons,
# and in order for --start-from to be useful).
list_url += '?sort=full_name'

repos = self.get_github_list(list_url, message)
if not include_archived:
repos = (r for r in repos if not r['archived'])
Expand Down Expand Up @@ -832,8 +838,7 @@ def _main():
help='create a {} from command-line arguments'.format(CONFIG_FILE))
parser.add_argument(
'--http-cache', default='.httpcache', metavar='DBNAME',
# .sqlite will be appended automatically by requests-cache < 0.6
# requests-cache >= 0.6 adds .sqlite only when the name has no .
# .sqlite will be appended automatically
help='cache HTTP requests on disk in an sqlite database for 5 minutes'
' (default: .httpcache)')
parser.add_argument(
Expand Down
75 changes: 67 additions & 8 deletions tests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,15 @@ def mock_config_filename(monkeypatch):


def make_page_url(url, page, extra):
# Some of the incoming test URLs already have query args. If so,
# append the page arguments using the appropriate separator.
sep = '?'
if '?' in url:
sep = '&'
if page == 1:
return '%s?%sper_page=100' % (url, extra)
return '%s%s%sper_page=100' % (url, sep, extra)
else:
return '%s?%spage=%d&per_page=100' % (url, extra, page)
return '%s%s%spage=%d&per_page=100' % (url, sep, extra, page)


def mock_multi_page_api_responses(url, pages, extra='sort=full_name&'):
Expand Down Expand Up @@ -759,7 +764,7 @@ def Repo(name, **kwargs):

def test_RepoWrangler_list_repos_for_user(mock_requests_get):
mock_requests_get.update(mock_multi_page_api_responses(
url='https://api.github.com/users/test_user/repos',
url='https://api.github.com/user/repos?affiliation=owner',
pages=[
[
repo('xyzzy'),
Expand Down Expand Up @@ -793,7 +798,7 @@ def test_RepoWrangler_list_repos_for_org(mock_requests_get):

def test_RepoWrangler_list_repos_filter_by_name(mock_requests_get):
mock_requests_get.update(mock_multi_page_api_responses(
url='https://api.github.com/users/test_user/repos',
url='https://api.github.com/user/repos?affiliation=owner',
pages=[
[
repo('xyzzy'),
Expand All @@ -810,7 +815,7 @@ def test_RepoWrangler_list_repos_filter_by_name(mock_requests_get):

def test_RepoWrangler_list_repos_filter_by_status(mock_requests_get):
mock_requests_get.update(mock_multi_page_api_responses(
url='https://api.github.com/users/test_user/repos',
url='https://api.github.com/user/repos?affiliation=owner',
pages=[
[
repo('a', archived=True),
Expand Down Expand Up @@ -847,15 +852,56 @@ def test_RepoWrangler_list_repos_filter_by_status(mock_requests_get):
Repo('c'),
Repo('p', private=True),
]


def test_RepoWrangler_list_repos_no_private(mock_requests_get):
mock_requests_get.update(mock_multi_page_api_responses(
url='https://api.github.com/users/test_user/repos',
pages=[
[
repo('a', archived=True),
repo('f', fork=True),
repo('p', private=True),
repo('d', disabled=True),
repo('c'),
],
],
))
wrangler = ghcloneall.RepoWrangler()
result = wrangler.list_repos(user='test_user', include_private=False)
assert result == [
Repo('c'),
Repo('d', disabled=True),
]
result = wrangler.list_repos(user='test_user', include_private=False,
include_archived=True)
assert result == [
Repo('a', archived=True),
Repo('c'),
Repo('d', disabled=True),
]
result = wrangler.list_repos(user='test_user', include_private=False,
include_forks=True)
assert result == [
Repo('c'),
Repo('d', disabled=True),
Repo('f', fork=True),
]
result = wrangler.list_repos(user='test_user', include_private=False,
include_disabled=False)
assert result == [
Repo('c'),
]
result = wrangler.list_repos(user='test_user', include_private=False)
assert result == [
Repo('c'),
Repo('d', disabled=True),
]


def test_RepoWrangler_list_repos_progress_bar(mock_requests_get):
mock_requests_get.update(mock_multi_page_api_responses(
url='https://api.github.com/users/test_user/repos',
url='https://api.github.com/user/repos?affiliation=owner',
pages=[
[
repo('xyzzy'),
Expand Down Expand Up @@ -1332,6 +1378,19 @@ def test_main_run_error_handling(monkeypatch, capsys):
monkeypatch.setattr(sys, 'argv', [
'ghcloneall', '--user', 'mgedmin',
])
with pytest.raises(SystemExit) as ctx:
ghcloneall.main()
assert str(ctx.value) == (
'Failed to fetch https://api.github.com/user/repos?affiliation=owner'
'&sort=full_name&per_page=100:\n'
'not found'
)


def test_main_run_error_handling_no_private(monkeypatch, capsys):
monkeypatch.setattr(sys, 'argv', [
'ghcloneall', '--user', 'mgedmin', '--exclude-private',
])
with pytest.raises(SystemExit) as ctx:
ghcloneall.main()
assert str(ctx.value) == (
Expand All @@ -1346,7 +1405,7 @@ def test_main_run(monkeypatch, mock_requests_get, capsys):
'ghcloneall', '--user', 'mgedmin', '--concurrency=1',
])
mock_requests_get.update(mock_multi_page_api_responses(
url='https://api.github.com/users/mgedmin/repos',
url='https://api.github.com/user/repos?affiliation=owner',
pages=[
[
repo('ghcloneall'),
Expand All @@ -1369,7 +1428,7 @@ def test_main_run_start_from(monkeypatch, mock_requests_get, capsys):
'ghcloneall', '--user', 'mgedmin', '--start-from', 'x',
])
mock_requests_get.update(mock_multi_page_api_responses(
url='https://api.github.com/users/mgedmin/repos',
url='https://api.github.com/user/repos?affiliation=owner',
pages=[
[
repo('ghcloneall'),
Expand Down

0 comments on commit ecdee50

Please sign in to comment.