From f858caf6cde653a875121bc4ea6256bea72331c5 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 8 May 2015 16:43:07 -0400 Subject: [PATCH] Only return the team and repo permissions when listing robots when we absolutely need them. --- data/model/legacy.py | 23 +++++---- endpoints/api/robot.py | 61 ++++++++++++++--------- static/js/directives/ui/robots-manager.js | 6 ++- test/test_api_usage.py | 25 ++++++++++ 4 files changed, 81 insertions(+), 34 deletions(-) diff --git a/data/model/legacy.py b/data/model/legacy.py index 33918e9dc..49717c556 100644 --- a/data/model/legacy.py +++ b/data/model/legacy.py @@ -336,16 +336,21 @@ class TupleSelector(object): -def list_entity_robot_permission_teams(entity_name): - query = (_list_entity_robots(entity_name) - .join(RepositoryPermission, JOIN_LEFT_OUTER, - on=(RepositoryPermission.user == FederatedLogin.user)) - .join(Repository, JOIN_LEFT_OUTER) - .switch(User) - .join(TeamMember, JOIN_LEFT_OUTER) - .join(Team, JOIN_LEFT_OUTER)) +def list_entity_robot_permission_teams(entity_name, include_permissions=False): + query = (_list_entity_robots(entity_name)) + + fields = [User.username, FederatedLogin.service_ident] + if include_permissions: + query = (query.join(RepositoryPermission, JOIN_LEFT_OUTER, + on=(RepositoryPermission.user == FederatedLogin.user)) + .join(Repository, JOIN_LEFT_OUTER) + .switch(User) + .join(TeamMember, JOIN_LEFT_OUTER) + .join(Team, JOIN_LEFT_OUTER)) + + fields.append(Repository.name) + fields.append(Team.name) - fields = [User.username, FederatedLogin.service_ident, Repository.name, Team.name] return TupleSelector(query, fields) diff --git a/endpoints/api/robot.py b/endpoints/api/robot.py index ecb26e573..153ea2c1a 100644 --- a/endpoints/api/robot.py +++ b/endpoints/api/robot.py @@ -1,6 +1,6 @@ from endpoints.api import (resource, nickname, ApiResource, log_action, related_user_resource, Unauthorized, require_user_admin, internal_only, require_scope, - path_param) + path_param, parse_args, truthy_bool, query_param) from auth.permissions import AdministerOrganizationPermission, OrganizationMemberPermission from auth.auth_context import get_authenticated_user from auth import scopes @@ -27,8 +27,8 @@ def permission_view(permission): } -def robots_list(prefix): - tuples = model.list_entity_robot_permission_teams(prefix) +def robots_list(prefix, include_permissions=False): + tuples = model.list_entity_robot_permission_teams(prefix, include_permissions=include_permissions) robots = {} robot_teams = set() @@ -38,27 +38,32 @@ def robots_list(prefix): if not robot_name in robots: robots[robot_name] = { 'name': robot_name, - 'token': robot_tuple.get(FederatedLogin.service_ident), - 'teams': [], - 'repositories': [] + 'token': robot_tuple.get(FederatedLogin.service_ident) } - team_name = robot_tuple.get(Team.name) - repository_name = robot_tuple.get(Repository.name) - - if team_name is not None: - check_key = robot_name + ':' + team_name - if not check_key in robot_teams: - robot_teams.add(check_key) - - robots[robot_name]['teams'].append({ - 'name': team_name, - 'avatar': avatar.get_data(team_name, team_name, 'team') + if include_permissions: + robots[robot_name].update({ + 'teams': [], + 'repositories': [] }) - if repository_name is not None: - if not repository_name in robots[robot_name]['repositories']: - robots[robot_name]['repositories'].append(repository_name) + if include_permissions: + team_name = robot_tuple.get(Team.name) + repository_name = robot_tuple.get(Repository.name) + + if team_name is not None: + check_key = robot_name + ':' + team_name + if not check_key in robot_teams: + robot_teams.add(check_key) + + robots[robot_name]['teams'].append({ + 'name': team_name, + 'avatar': avatar.get_data(team_name, team_name, 'team') + }) + + if repository_name is not None: + if not repository_name in robots[robot_name]['repositories']: + robots[robot_name]['repositories'].append(repository_name) return {'robots': robots.values()} @@ -68,10 +73,14 @@ class UserRobotList(ApiResource): """ Resource for listing user robots. """ @require_user_admin @nickname('getUserRobots') - def get(self): + @parse_args + @query_param('permissions', + 'Whether to include repostories and teams in which the robots have permission.', + type=truthy_bool, default=False) + def get(self, args): """ List the available robots for the user. """ user = get_authenticated_user() - return robots_list(user.username) + return robots_list(user.username, include_permissions=args.get('permissions', False)) @resource('/v1/user/robots/') @@ -113,11 +122,15 @@ class OrgRobotList(ApiResource): """ Resource for listing an organization's robots. """ @require_scope(scopes.ORG_ADMIN) @nickname('getOrgRobots') - def get(self, orgname): + @parse_args + @query_param('permissions', + 'Whether to include repostories and teams in which the robots have permission.', + type=truthy_bool, default=False) + def get(self, args, orgname): """ List the organization's robots. """ permission = OrganizationMemberPermission(orgname) if permission.can(): - return robots_list(orgname) + return robots_list(orgname, include_permissions=args.get('permissions', False)) raise Unauthorized() diff --git a/static/js/directives/ui/robots-manager.js b/static/js/directives/ui/robots-manager.js index 6c706adf8..f3ba51b41 100644 --- a/static/js/directives/ui/robots-manager.js +++ b/static/js/directives/ui/robots-manager.js @@ -143,8 +143,12 @@ angular.module('quay').directive('robotsManager', function () { if (!$scope.user && !$scope.organization) { return; } if ($scope.loading || !$scope.isEnabled) { return; } + var params = { + 'permissions': true + }; + $scope.loading = true; - ApiService.getRobots($scope.organization).then(function(resp) { + ApiService.getRobots($scope.organization, null, params).then(function(resp) { $scope.robots = resp.robots; $scope.loading = false; diff --git a/test/test_api_usage.py b/test/test_api_usage.py index 44be2684d..c4f3b1349 100644 --- a/test/test_api_usage.py +++ b/test/test_api_usage.py @@ -2173,6 +2173,31 @@ class TestUserRobots(ApiTestCase): def getRobotNames(self): return [r['name'] for r in self.getJsonResponse(UserRobotList)['robots']] + def test_robot_list(self): + self.login(NO_ACCESS_USER) + + # Create some robots. + self.putJsonResponse(UserRobot, + params=dict(robot_shortname='bender'), + expected_code=201) + + self.putJsonResponse(UserRobot, + params=dict(robot_shortname='goldy'), + expected_code=201) + + self.putJsonResponse(UserRobot, + params=dict(robot_shortname='coolbot'), + expected_code=201) + + # Queries: Base + the list query + with assert_query_count(BASE_ACCESS_QUERY_COUNT + 1): + self.getJsonResponse(UserRobotList) + + # Queries: Base + the list query + with assert_query_count(BASE_ACCESS_QUERY_COUNT + 1): + self.getJsonResponse(UserRobotList, params=dict(permissions=True)) + + def test_robots(self): self.login(NO_ACCESS_USER)