From db64ea8c8185258872e7c933bdae38ed03ae9e6a Mon Sep 17 00:00:00 2001 From: Marcin Kuzminski Date: Mon, 13 Mar 2017 12:56:22 +0100 Subject: [PATCH] api: use consistent way to extract users, repos, repo groups and user groups by id or name. - makes usage of Number vs String to differenciate if we pick objec ID or it's name this will allow easy fetching of objects by either id or it's name, including numeric string name - fixes #5230 --- rhodecode/api/tests/test_utils.py | 52 +++++++++++++++++++++++-------- rhodecode/api/utils.py | 50 +++++++++++++++++++++++------ 2 files changed, 79 insertions(+), 23 deletions(-) diff --git a/rhodecode/api/tests/test_utils.py b/rhodecode/api/tests/test_utils.py index 4cef8934..d0acd1aa 100644 --- a/rhodecode/api/tests/test_utils.py +++ b/rhodecode/api/tests/test_utils.py @@ -181,29 +181,56 @@ class TestGetRefHash(object): class TestUserByNameOrError(object): def test_user_found_by_id(self): fake_user = Mock(id=123) + patcher = patch('rhodecode.model.user.UserModel.get_user') with patcher as get_user: get_user.return_value = fake_user - result = utils.get_user_or_error('123') - assert result == fake_user + + patcher = patch('rhodecode.model.user.UserModel.get_by_username') + with patcher as get_by_username: + result = utils.get_user_or_error(123) + assert result == fake_user + + def test_user_not_found_by_id_as_str(self): + fake_user = Mock(id=123) + + patcher = patch('rhodecode.model.user.UserModel.get_user') + with patcher as get_user: + get_user.return_value = fake_user + patcher = patch('rhodecode.model.user.UserModel.get_by_username') + with patcher as get_by_username: + get_by_username.return_value = None + + with pytest.raises(JSONRPCError): + utils.get_user_or_error('123') def test_user_found_by_name(self): fake_user = Mock(id=123) - patcher = patch('rhodecode.model.user.UserModel.get_by_username') - with patcher as get_by_username: - get_by_username.return_value = fake_user - result = utils.get_user_or_error('test') - assert result == fake_user + + patcher = patch('rhodecode.model.user.UserModel.get_user') + with patcher as get_user: + get_user.return_value = None + + patcher = patch('rhodecode.model.user.UserModel.get_by_username') + with patcher as get_by_username: + get_by_username.return_value = fake_user + + result = utils.get_user_or_error('test') + assert result == fake_user def test_user_not_found_by_id(self): patcher = patch('rhodecode.model.user.UserModel.get_user') with patcher as get_user: get_user.return_value = None - with pytest.raises(JSONRPCError) as excinfo: - utils.get_user_or_error('123') + patcher = patch('rhodecode.model.user.UserModel.get_by_username') + with patcher as get_by_username: + get_by_username.return_value = None - expected_message = 'user `123` does not exist' - assert excinfo.value.message == expected_message + with pytest.raises(JSONRPCError) as excinfo: + utils.get_user_or_error(123) + + expected_message = 'user `123` does not exist' + assert excinfo.value.message == expected_message def test_user_not_found_by_name(self): patcher = patch('rhodecode.model.user.UserModel.get_by_username') @@ -216,8 +243,7 @@ class TestUserByNameOrError(object): assert excinfo.value.message == expected_message -class TestGetCommitDict: - +class TestGetCommitDict(object): @pytest.mark.parametrize('filename, expected', [ (b'sp\xc3\xa4cial', u'sp\xe4cial'), (b'sp\xa4cial', u'sp\ufffdcial'), diff --git a/rhodecode/api/utils.py b/rhodecode/api/utils.py index c2e43377..be8e4769 100644 --- a/rhodecode/api/utils.py +++ b/rhodecode/api/utils.py @@ -213,15 +213,19 @@ def get_user_or_error(userid): :param userid: """ from rhodecode.model.user import UserModel - user_model = UserModel() - try: - user = user_model.get_user(int(userid)) - except ValueError: + + if isinstance(userid, (int, long)): + try: + user = user_model.get_user(userid) + except ValueError: + user = None + else: user = user_model.get_by_username(userid) if user is None: - raise JSONRPCError("user `%s` does not exist" % (userid,)) + raise JSONRPCError( + 'user `%s` does not exist' % (userid,)) return user @@ -232,10 +236,19 @@ def get_repo_or_error(repoid): :param repoid: """ from rhodecode.model.repo import RepoModel + repo_model = RepoModel() + + if isinstance(repoid, (int, long)): + try: + repo = repo_model.get_repo(repoid) + except ValueError: + repo = None + else: + repo = repo_model.get_by_repo_name(repoid) - repo = RepoModel().get_repo(repoid) if repo is None: - raise JSONRPCError('repository `%s` does not exist' % (repoid,)) + raise JSONRPCError( + 'repository `%s` does not exist' % (repoid,)) return repo @@ -246,8 +259,16 @@ def get_repo_group_or_error(repogroupid): :param repogroupid: """ from rhodecode.model.repo_group import RepoGroupModel + repo_group_model = RepoGroupModel() + + if isinstance(repogroupid, (int, long)): + try: + repo_group = repo_group_model._get_repo_group(repogroupid) + except ValueError: + repo_group = None + else: + repo_group = repo_group_model.get_by_group_name(repogroupid) - repo_group = RepoGroupModel()._get_repo_group(repogroupid) if repo_group is None: raise JSONRPCError( 'repository group `%s` does not exist' % (repogroupid,)) @@ -261,10 +282,19 @@ def get_user_group_or_error(usergroupid): :param usergroupid: """ from rhodecode.model.user_group import UserGroupModel + user_group_model = UserGroupModel() + + if isinstance(usergroupid, (int, long)): + try: + user_group = user_group_model.get_group(usergroupid) + except ValueError: + user_group = None + else: + user_group = user_group_model.get_by_name(usergroupid) - user_group = UserGroupModel().get_group(usergroupid) if user_group is None: - raise JSONRPCError('user group `%s` does not exist' % (usergroupid,)) + raise JSONRPCError( + 'user group `%s` does not exist' % (usergroupid,)) return user_group