From 71ed6fb43ccde1eba90ccfc30d546dca350d9055 Mon Sep 17 00:00:00 2001 From: Daniel Pan Date: Thu, 31 Dec 2015 10:13:17 +0800 Subject: [PATCH 1/2] Improve error message --- seahub/api2/endpoints/account.py | 33 +++++++------- seahub/api2/endpoints/dir_shared_items.py | 54 ++++++++++++----------- 2 files changed, 45 insertions(+), 42 deletions(-) diff --git a/seahub/api2/endpoints/account.py b/seahub/api2/endpoints/account.py index f4a26b3528..0f7ce50257 100644 --- a/seahub/api2/endpoints/account.py +++ b/seahub/api2/endpoints/account.py @@ -22,6 +22,7 @@ from seahub.profile.models import Profile from seahub.profile.utils import refresh_cache as refresh_profile_cache from seahub.utils import is_valid_username + logger = logging.getLogger(__name__) json_content_type = 'application/json; charset=utf-8' @@ -36,13 +37,13 @@ class Account(APIView): def get(self, request, email, format=None): if not is_valid_username(email): - return api_error(status.HTTP_404_NOT_FOUND, 'User not found.') + return api_error(status.HTTP_400_BAD_REQUEST, 'Email %s invalid.' % email) # query account info try: user = User.objects.get(email=email) except User.DoesNotExist: - return api_error(status.HTTP_404_NOT_FOUND, 'User not found.') + return api_error(status.HTTP_404_NOT_FOUND, 'User %s not found.' % email) info = {} info['email'] = user.email @@ -102,8 +103,8 @@ class Account(APIView): serializer.data['is_active']) except User.DoesNotExist as e: logger.error(e) - return api_error(status.HTTP_403_FORBIDDEN, - 'Fail to add user.') + return api_error(status.HTTP_520_OPERATION_FAILED, + 'Failed to add user.') self._update_account_profile(request, user.username) @@ -121,7 +122,7 @@ class Account(APIView): is_staff = to_python_boolean(is_staff) except ValueError: return api_error(status.HTTP_400_BAD_REQUEST, - '%s is not a valid value' % is_staff) + 'is_staff invalid.') is_active = request.data.get("is_active", None) if is_active is not None: @@ -129,7 +130,7 @@ class Account(APIView): is_active = to_python_boolean(is_active) except ValueError: return api_error(status.HTTP_400_BAD_REQUEST, - '%s is not a valid value' % is_active) + 'is_active invalid.') if password is not None: user.set_password(password) @@ -142,8 +143,8 @@ class Account(APIView): result_code = user.save() if result_code == -1: - return api_error(status.HTTP_403_FORBIDDEN, - 'Fail to update user.') + return api_error(status.HTTP_520_OPERATION_FAILED, + 'Failed to update user.') self._update_account_profile(request, user.username) @@ -151,7 +152,7 @@ class Account(APIView): self._update_account_quota(request, user.username) except SearpcError as e: logger.error(e) - return api_error(HTTP_520_OPERATION_FAILED, 'Failed to set account quota') + return api_error(HTTP_520_OPERATION_FAILED, 'Failed to set user quota.') is_trial = request.data.get("is_trial", None) if is_trial is not None: @@ -164,7 +165,7 @@ class Account(APIView): is_trial = to_python_boolean(is_trial) except ValueError: return api_error(status.HTTP_400_BAD_REQUEST, - '%s is not a valid value' % is_trial) + 'is_trial invalid') if is_trial is True: expire_date = timezone.now() + relativedelta(days=7) @@ -178,19 +179,19 @@ class Account(APIView): def post(self, request, email, format=None): # migrate an account's repos and groups to an exist account if not is_valid_username(email): - return api_error(status.HTTP_404_NOT_FOUND, 'User not found.') + return api_error(status.HTTP_400_BAD_REQUEST, 'Email %s invalid.' % email) op = request.data.get('op', '').lower() if op == 'migrate': from_user = email to_user = request.data.get('to_user', '') if not is_valid_username(to_user): - return api_error(status.HTTP_400_BAD_REQUEST, '%s is not valid email.' % to_user) + return api_error(status.HTTP_400_BAD_REQUEST, 'Email %s invalid.' % to_user) try: user2 = User.objects.get(email=to_user) except User.DoesNotExist: - return api_error(status.HTTP_400_BAD_REQUEST, '%s does not exist.' % to_user) + return api_error(status.HTTP_404_NOT_FOUND, 'User %s not found.' % to_user) # transfer owned repos to new user for r in seafile_api.get_owned_repo_list(from_user): @@ -208,11 +209,11 @@ class Account(APIView): return Response("success") else: - return api_error(status.HTTP_400_BAD_REQUEST, 'Op can only be migrate') + return api_error(status.HTTP_400_BAD_REQUEST, 'op can only be migrate.') def put(self, request, email, format=None): if not is_valid_username(email): - return api_error(status.HTTP_404_NOT_FOUND, 'User not found.') + return api_error(status.HTTP_400_BAD_REQUEST, 'Email %s invalid.' % email) try: user = User.objects.get(email=email) @@ -222,7 +223,7 @@ class Account(APIView): def delete(self, request, email, format=None): if not is_valid_username(email): - return api_error(status.HTTP_404_NOT_FOUND, 'User not found.') + return api_error(status.HTTP_400_BAD_REQUEST, 'Email %s invalid.' % email) # delete account try: diff --git a/seahub/api2/endpoints/dir_shared_items.py b/seahub/api2/endpoints/dir_shared_items.py index 8b3343766d..8cad6ec689 100644 --- a/seahub/api2/endpoints/dir_shared_items.py +++ b/seahub/api2/endpoints/dir_shared_items.py @@ -15,6 +15,7 @@ from seahub.api2.authentication import TokenAuthentication from seahub.api2.permissions import IsRepoAccessible from seahub.api2.throttling import UserRateThrottle from seahub.api2.utils import api_error +from seahub.api2.status import HTTP_520_OPERATION_FAILED from seahub.base.templatetags.seahub_tags import email2nickname from seahub.share.signals import share_repo_to_user_successful from seahub.share.views import check_user_share_quota @@ -128,13 +129,13 @@ class DirSharedItemsEndpoint(APIView): """ repo = seafile_api.get_repo(repo_id) if not repo: - return api_error(status.HTTP_400_BAD_REQUEST, 'Repo not found.') + return api_error(status.HTTP_404_NOT_FOUND, 'Library %s not found.' % repo_id) shared_to_user, shared_to_group = self.handle_shared_to_args(request) path = request.GET.get('p', '/') if seafile_api.get_dir_id_by_path(repo.id, path) is None: - return api_error(status.HTTP_400_BAD_REQUEST, 'Directory not found.') + return api_error(status.HTTP_404_NOT_FOUND, 'Folder %s not found.' % path) ret = [] if shared_to_user: @@ -152,17 +153,17 @@ class DirSharedItemsEndpoint(APIView): username = request.user.username repo = seafile_api.get_repo(repo_id) if not repo: - return api_error(status.HTTP_400_BAD_REQUEST, 'Repo not found.') + return api_error(status.HTTP_404_NOT_FOUND, 'Library %s not found.' % repo_id) shared_to_user, shared_to_group = self.handle_shared_to_args(request) permission = request.data.get('permission', 'r') if permission not in ['r', 'rw']: - return api_error(status.HTTP_400_BAD_REQUEST, 'Bad permission') + return api_error(status.HTTP_400_BAD_REQUEST, 'Permission invalid.') path = request.GET.get('p', '/') if seafile_api.get_dir_id_by_path(repo.id, path) is None: - return api_error(status.HTTP_400_BAD_REQUEST, 'Directory not found.') + return api_error(status.HTTP_404_NOT_FOUND, 'Folder %s not found.' % path) if path == '/': shared_repo = repo @@ -172,15 +173,16 @@ class DirSharedItemsEndpoint(APIView): if sub_repo: shared_repo = sub_repo else: - return api_error(status.HTTP_400_BAD_REQUEST, 'No sub repo found') + # unlikely to happen + return api_error(status.HTTP_404_NOT_FOUND, 'Failed to get sub repo') except SearpcError as e: logger.error(e) - return api_error(status.HTTP_500_INTERNAL_SERVER_ERROR, 'Failed to get sub repo') + return api_error(status.HTTP_500_INTERNAL_SERVER_ERROR, 'Internal Server Error') if shared_to_user: shared_to = request.GET.get('username') if shared_to is None or not is_valid_username(shared_to): - return api_error(status.HTTP_400_BAD_REQUEST, 'Bad username.') + return api_error(status.HTTP_400_BAD_REQUEST, 'Email %s invalid.' % shared_to) if is_org_context(request): org_id = request.user.org.org_id @@ -198,10 +200,10 @@ class DirSharedItemsEndpoint(APIView): try: gid = int(gid) except ValueError: - return api_error(status.HTTP_400_BAD_REQUEST, 'Bad group id: %s' % gid) + return api_error(status.HTTP_400_BAD_REQUEST, 'group_id %s invalid.' % gid) group = seaserv.get_group(gid) if not group: - return api_error(status.HTTP_400_BAD_REQUEST, 'Group not found: %s' % gid) + return api_error(status.HTTP_404_NOT_FOUND, 'Group %s not found.' % gid) if is_org_context(request): org_id = request.user.org.org_id @@ -221,28 +223,28 @@ class DirSharedItemsEndpoint(APIView): username = request.user.username repo = seafile_api.get_repo(repo_id) if not repo: - return api_error(status.HTTP_400_BAD_REQUEST, 'Repo not found.') + return api_error(status.HTTP_404_NOT_FOUND, 'Library %s not found.' % repo_id) path = request.GET.get('p', '/') if seafile_api.get_dir_id_by_path(repo.id, path) is None: - return api_error(status.HTTP_400_BAD_REQUEST, 'Directory not found.') + return api_error(status.HTTP_404_NOT_FOUND, 'Folder %s not found.' % path) if path != '/': try: sub_repo = self.get_or_create_sub_repo_by_path(request, repo, path) except SearpcError as e: logger.error(e) - return api_error(status.HTTP_500_INTERNAL_SERVER_ERROR, 'Failed to get sub repo') + return api_error(status.HTTP_500_INTERNAL_SERVER_ERROR, 'Failed to get sub repo.') else: sub_repo = None share_type = request.data.get('share_type') if share_type != 'user' and share_type != 'group': - return api_error(status.HTTP_400_BAD_REQUEST, 'Bad share type') + return api_error(status.HTTP_400_BAD_REQUEST, 'share_type invalid.') permission = request.data.get('permission', 'r') if permission not in ['r', 'rw']: - return api_error(status.HTTP_400_BAD_REQUEST, 'Bad permission') + return api_error(status.HTTP_400_BAD_REQUEST, 'permission invalid.') shared_repo = repo if path == '/' else sub_repo success, failed = [], [] @@ -251,11 +253,11 @@ class DirSharedItemsEndpoint(APIView): for to_user in share_to_users: if not is_valid_username(to_user): return api_error(status.HTTP_400_BAD_REQUEST, - 'Username must be a valid email address.') + 'Email %s not valid.' % to_user) if not check_user_share_quota(username, shared_repo, users=[to_user]): return api_error(status.HTTP_403_FORBIDDEN, - 'Failed to share: No enough quota.') + 'Not enough quota.') try: if is_org_context(request): @@ -294,14 +296,14 @@ class DirSharedItemsEndpoint(APIView): try: gid = int(gid) except ValueError: - return api_error(status.HTTP_400_BAD_REQUEST, 'Bad group id: %s' % gid) + return api_error(status.HTTP_400_BAD_REQUEST, 'group_id %s invalid.' % gid) group = seaserv.get_group(gid) if not group: - return api_error(status.HTTP_400_BAD_REQUEST, 'Group not found: %s' % gid) + return api_error(status.HTTP_400_BAD_REQUEST, 'Group %s not found' % gid) if not check_user_share_quota(username, shared_repo, groups=[group]): return api_error(status.HTTP_403_FORBIDDEN, - 'Failed to share: No enough quota.') + 'Not enough quota.') try: if is_org_context(request): @@ -338,13 +340,13 @@ class DirSharedItemsEndpoint(APIView): username = request.user.username repo = seafile_api.get_repo(repo_id) if not repo: - return api_error(status.HTTP_400_BAD_REQUEST, 'Repo not found.') + return api_error(status.HTTP_400_BAD_REQUEST, 'Library %s not found.' % repo_id) shared_to_user, shared_to_group = self.handle_shared_to_args(request) path = request.GET.get('p', '/') if seafile_api.get_dir_id_by_path(repo.id, path) is None: - return api_error(status.HTTP_400_BAD_REQUEST, 'Directory not found.') + return api_error(status.HTTP_400_BAD_REQUEST, 'Folder %s not found.' % path) if path == '/': shared_repo = repo @@ -354,15 +356,15 @@ class DirSharedItemsEndpoint(APIView): if sub_repo: shared_repo = sub_repo else: - return api_error(status.HTTP_400_BAD_REQUEST, 'No sub repo found') + return api_error(status.HTTP_400_BAD_REQUEST, 'Sub-library not found.') except SearpcError as e: logger.error(e) - return api_error(status.HTTP_500_INTERNAL_SERVER_ERROR, 'Failed to get sub repo') + return api_error(status.HTTP_500_INTERNAL_SERVER_ERROR, 'Failed to get sub-library.') if shared_to_user: shared_to = request.GET.get('username') if shared_to is None or not is_valid_username(shared_to): - return api_error(status.HTTP_400_BAD_REQUEST, 'Bad argument.') + return api_error(status.HTTP_400_BAD_REQUEST, 'Email %s invalid.' % share_to) if is_org_context(request): org_id = request.user.org.org_id @@ -381,7 +383,7 @@ class DirSharedItemsEndpoint(APIView): try: group_id = int(group_id) except ValueError: - return api_error(status.HTTP_400_BAD_REQUEST, 'Bad group id') + return api_error(status.HTTP_400_BAD_REQUEST, 'group_id %s invalid' % group_id) # hacky way to get group repo permission permission = '' From ed1b3204e0420149bfb90085a4f0f2e0a98bd963 Mon Sep 17 00:00:00 2001 From: lian Date: Wed, 6 Jan 2016 15:25:00 +0800 Subject: [PATCH 2/2] Improve error message Use 404 when repo/group not found --- seahub/api2/endpoints/dir_shared_items.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/seahub/api2/endpoints/dir_shared_items.py b/seahub/api2/endpoints/dir_shared_items.py index 8cad6ec689..abc9e6e67d 100644 --- a/seahub/api2/endpoints/dir_shared_items.py +++ b/seahub/api2/endpoints/dir_shared_items.py @@ -159,7 +159,7 @@ class DirSharedItemsEndpoint(APIView): permission = request.data.get('permission', 'r') if permission not in ['r', 'rw']: - return api_error(status.HTTP_400_BAD_REQUEST, 'Permission invalid.') + return api_error(status.HTTP_400_BAD_REQUEST, 'permission invalid.') path = request.GET.get('p', '/') if seafile_api.get_dir_id_by_path(repo.id, path) is None: @@ -299,7 +299,7 @@ class DirSharedItemsEndpoint(APIView): return api_error(status.HTTP_400_BAD_REQUEST, 'group_id %s invalid.' % gid) group = seaserv.get_group(gid) if not group: - return api_error(status.HTTP_400_BAD_REQUEST, 'Group %s not found' % gid) + return api_error(status.HTTP_404_NOT_FOUND, 'Group %s not found' % gid) if not check_user_share_quota(username, shared_repo, groups=[group]): return api_error(status.HTTP_403_FORBIDDEN, @@ -340,13 +340,13 @@ class DirSharedItemsEndpoint(APIView): username = request.user.username repo = seafile_api.get_repo(repo_id) if not repo: - return api_error(status.HTTP_400_BAD_REQUEST, 'Library %s not found.' % repo_id) + return api_error(status.HTTP_404_NOT_FOUND, 'Library %s not found.' % repo_id) shared_to_user, shared_to_group = self.handle_shared_to_args(request) path = request.GET.get('p', '/') if seafile_api.get_dir_id_by_path(repo.id, path) is None: - return api_error(status.HTTP_400_BAD_REQUEST, 'Folder %s not found.' % path) + return api_error(status.HTTP_404_NOT_FOUND, 'Folder %s not found.' % path) if path == '/': shared_repo = repo @@ -356,7 +356,7 @@ class DirSharedItemsEndpoint(APIView): if sub_repo: shared_repo = sub_repo else: - return api_error(status.HTTP_400_BAD_REQUEST, 'Sub-library not found.') + return api_error(status.HTTP_404_NOT_FOUND, 'Sub-library not found.') except SearpcError as e: logger.error(e) return api_error(status.HTTP_500_INTERNAL_SERVER_ERROR, 'Failed to get sub-library.')