From 69ff58f7bd1ea8b02641788170a3ea24d5146845 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 21 Apr 2017 17:37:00 -0400 Subject: [PATCH] Gitlab can return `None` for avatar blocks Because reasons! --- buildtrigger/gitlabhandler.py | 13 +++-- buildtrigger/test/gitlabmock.py | 70 +++++++++++++++---------- buildtrigger/test/test_gitlabhandler.py | 19 ++++++- 3 files changed, 70 insertions(+), 32 deletions(-) diff --git a/buildtrigger/gitlabhandler.py b/buildtrigger/gitlabhandler.py index 0ddbfc3a3..f23b4a80a 100644 --- a/buildtrigger/gitlabhandler.py +++ b/buildtrigger/gitlabhandler.py @@ -275,19 +275,24 @@ class GitLabBuildTrigger(BuildTriggerHandler): namespaces = {} repositories = _paginated_iterator(gl_client.getprojects, RepositoryReadException) for repo in repositories: - namespace = repo['namespace'] + namespace = repo.get('namespace') or {} + if not namespace: + continue + namespace_id = namespace['id'] avatar_url = '' if 'avatar' in namespace: - avatar_url = namespace.get('avatar', {}).get('url') + avatar_data = namespace.get('avatar') or {} + avatar_url = avatar_data.get('url') elif 'owner' in repo: - avatar_url = repo.get('owner', {}).get('avatar_url') + owner_data = repo.get('owner') or {} + avatar_url = owner_data.get('avatar_url') if namespace_id in namespaces: namespaces[namespace_id]['score'] = namespaces[namespace_id]['score'] + 1 else: - owner = repo['namespace']['name'] + owner = namespace['name'] namespaces[namespace_id] = { 'personal': owner == current_user['username'], 'id': namespace['path'], diff --git a/buildtrigger/test/gitlabmock.py b/buildtrigger/test/gitlabmock.py index e792c30d1..2c9489ae6 100644 --- a/buildtrigger/test/gitlabmock.py +++ b/buildtrigger/test/gitlabmock.py @@ -12,7 +12,7 @@ def get_gitlab_trigger(dockerfile_path=''): 'username': 'knownuser' }) - trigger._get_authorized_client = get_mock_gitlab + trigger._get_authorized_client = get_mock_gitlab(with_nullavatar=False) return trigger def adddeploykey_mock(project_id, name, public_key): @@ -52,18 +52,31 @@ def project(namespace, name, is_org=False): } } - if is_org: + if name == 'nullavatar': + del data['owner']['avatar_url'] + data['namespace']['avatar'] = None + elif is_org: del data['owner']['avatar_url'] data['namespace']['avatar'] = {'url': 'avatarurl'} return data -def getprojects_mock(page=1, per_page=100): - return [ - project('knownuser', 'somerepo'), - project('someorg', 'somerepo', is_org=True), - project('someorg', 'anotherrepo', is_org=True), - ] +def getprojects_mock(with_nullavatar=False): + if with_nullavatar: + def _getprojs(page=1, per_page=100): + return [ + project('someorg', 'nullavatar', is_org=True), + ] + return _getprojs + + else: + def _getprojs(page=1, per_page=100): + return [ + project('knownuser', 'somerepo'), + project('someorg', 'somerepo', is_org=True), + project('someorg', 'anotherrepo', is_org=True), + ] + return _getprojs def getproject_mock(project_name): if project_name == 'knownuser/somerepo': @@ -165,28 +178,31 @@ def getrawfile_mock(repo_id, branch_name, path): return False -def get_mock_gitlab(): - mock_gitlab = Mock() - mock_gitlab.host = 'https://bitbucket.org' +def get_mock_gitlab(with_nullavatar=False): + def _get_mock(): + mock_gitlab = Mock() + mock_gitlab.host = 'https://bitbucket.org' - mock_gitlab.currentuser = Mock(side_effect=get_currentuser_mock) - mock_gitlab.getusers = Mock(side_effect=getusers_mock) + mock_gitlab.currentuser = Mock(side_effect=get_currentuser_mock) + mock_gitlab.getusers = Mock(side_effect=getusers_mock) - mock_gitlab.getprojects = Mock(side_effect=getprojects_mock) - mock_gitlab.getproject = Mock(side_effect=getproject_mock) - mock_gitlab.getbranches = Mock(side_effect=getbranches_mock) + mock_gitlab.getprojects = Mock(side_effect=getprojects_mock(with_nullavatar)) + mock_gitlab.getproject = Mock(side_effect=getproject_mock) + mock_gitlab.getbranches = Mock(side_effect=getbranches_mock) - mock_gitlab.getbranch = Mock(side_effect=getbranch_mock) - mock_gitlab.gettag = Mock(side_effect=gettag_mock) + mock_gitlab.getbranch = Mock(side_effect=getbranch_mock) + mock_gitlab.gettag = Mock(side_effect=gettag_mock) - mock_gitlab.getrepositorytags = Mock(side_effect=getrepositorytags_mock) - mock_gitlab.getrepositorytree = Mock(side_effect=getrepositorytree_mock) - mock_gitlab.getrepositorycommit = Mock(side_effect=getrepositorycommit_mock) + mock_gitlab.getrepositorytags = Mock(side_effect=getrepositorytags_mock) + mock_gitlab.getrepositorytree = Mock(side_effect=getrepositorytree_mock) + mock_gitlab.getrepositorycommit = Mock(side_effect=getrepositorycommit_mock) - mock_gitlab.getrawfile = Mock(side_effect=getrawfile_mock) + mock_gitlab.getrawfile = Mock(side_effect=getrawfile_mock) - mock_gitlab.adddeploykey = Mock(side_effect=adddeploykey_mock) - mock_gitlab.addprojecthook = Mock(side_effect=addprojecthook_mock) - mock_gitlab.deletedeploykey = Mock(return_value=True) - mock_gitlab.deleteprojecthook = Mock(return_value=True) - return mock_gitlab + mock_gitlab.adddeploykey = Mock(side_effect=adddeploykey_mock) + mock_gitlab.addprojecthook = Mock(side_effect=addprojecthook_mock) + mock_gitlab.deletedeploykey = Mock(return_value=True) + mock_gitlab.deleteprojecthook = Mock(return_value=True) + return mock_gitlab + + return _get_mock diff --git a/buildtrigger/test/test_gitlabhandler.py b/buildtrigger/test/test_gitlabhandler.py index 6f6555769..f8a8931c6 100644 --- a/buildtrigger/test/test_gitlabhandler.py +++ b/buildtrigger/test/test_gitlabhandler.py @@ -1,7 +1,9 @@ import json import pytest -from buildtrigger.test.gitlabmock import get_gitlab_trigger +from mock import Mock + +from buildtrigger.test.gitlabmock import get_gitlab_trigger, get_mock_gitlab from buildtrigger.triggerutil import (SkipRequestException, ValidationRequestException, InvalidPayloadException) from endpoints.building import PreparedBuild @@ -35,6 +37,21 @@ def test_lookup_user(email, expected_response, gitlab_trigger): assert gitlab_trigger.lookup_user(email) == expected_response +def test_null_avatar(gitlab_trigger): + gitlab_trigger._get_authorized_client = get_mock_gitlab(with_nullavatar=True) + namespace_data = gitlab_trigger.list_build_source_namespaces() + expected = { + 'avatar_url': None, + 'personal': False, + 'title': 'someorg', + 'url': 'https://bitbucket.org/someorg', + 'score': 1, + 'id': 'someorg', + } + + assert namespace_data == [expected] + + @pytest.mark.parametrize('payload, expected_error, expected_message', [ ('{}', InvalidPayloadException, ''),