[api] Add repo last modifier and add cache for memcache ops

seahub/api2/endpoints/shared_repos.py

@@ -15,7 +15,7 @@ from seahub.api2.authentication import TokenAuthentication
 from seahub.api2.throttling import UserRateThrottle
 from seahub.profile.models import Profile
 from seahub.utils import is_org_context, is_valid_username, send_perm_audit_msg
-from seahub.base.templatetags.seahub_tags import email2nickname
+from seahub.base.templatetags.seahub_tags import email2nickname, email2contact_email
 
 logger = logging.getLogger(__name__)
 
@@ -61,6 +61,9 @@ class SharedRepos(APIView):
             result['repo_name'] = repo.repo_name
             result['share_type'] = repo.share_type
             result['share_permission'] = repo.permission
+            result['modifier_email'] = repo.last_modifier
+            result['modifier_name'] = email2nickname(repo.last_modifier)
+            result['modifier_contact_email'] = email2contact_email(repo.last_modifier)
 
             if repo.share_type == 'personal':
                 result['user_name'] = email2nickname(repo.user)

seahub/api2/views.py

@@ -417,6 +417,10 @@ class Repos(APIView):
 
         email = request.user.username
 
+        # Use dict to reduce memcache fetch cost in large for-loop.
+        contact_email_dict = {}
+        nickname_dict = {}
+
         repos_json = []
         if filter_by['mine']:
             if is_org_context(request):
@@ -427,6 +431,14 @@ class Repos(APIView):
                 owned_repos = seafile_api.get_owned_repo_list(email,
                         ret_corrupted=True)
 
+            # Reduce memcache fetch ops.
+            modifiers_set = set([x.last_modifier for x in owned_repos])
+            for e in modifiers_set:
+                if e not in contact_email_dict:
+                    contact_email_dict[e] = email2contact_email(e)
+                if e not in nickname_dict:
+                    nickname_dict[e] = email2nickname(e)
+
             owned_repos.sort(lambda x, y: cmp(y.last_modify, x.last_modify))
             for r in owned_repos:
                 # do not return virtual repos
@@ -439,6 +451,9 @@ class Repos(APIView):
                     "owner": email,
                     "name": r.name,
                     "mtime": r.last_modify,
+                    "modifier_email": r.last_modifier,
+                    "modifier_contact_email": contact_email_dict.get(r.last_modifier, ''),
+                    "modifier_name": nickname_dict.get(r.last_modifier, ''),
                     "mtime_relative": translate_seahub_time(r.last_modify),
                     "size": r.size,
                     "size_formatted": filesizeformat(r.size),
@@ -461,6 +476,15 @@ class Repos(APIView):
                 shared_repos = seafile_api.get_share_in_repo_list(
                         email, -1, -1)
 
+            # Reduce memcache fetch ops.
+            owners_set = set([x.user for x in shared_repos])
+            modifiers_set = set([x.last_modifier for x in shared_repos])
+            for e in owners_set | modifiers_set:
+                if e not in contact_email_dict:
+                    contact_email_dict[e] = email2contact_email(e)
+                if e not in nickname_dict:
+                    nickname_dict[e] = email2nickname(e)
+
             shared_repos.sort(lambda x, y: cmp(y.last_modify, x.last_modify))
             for r in shared_repos:
                 r.password_need = is_passwd_set(r.repo_id, email)
@@ -469,9 +493,12 @@ class Repos(APIView):
                     "id": r.repo_id,
                     "owner": r.user,
                     "name": r.repo_name,
-                    "owner_nickname": email2nickname(r.user),
+                    "owner_nickname": nickname_dict.get(r.user, ''),
                     "mtime": r.last_modify,
                     "mtime_relative": translate_seahub_time(r.last_modify),
+                    "modifier_email": r.last_modifier,
+                    "modifier_contact_email": contact_email_dict.get(r.last_modifier, ''),
+                    "modifier_name": nickname_dict.get(r.last_modifier, ''),
                     "size": r.size,
                     "size_formatted": filesizeformat(r.size),
                     "encrypted": r.encrypted,
@@ -487,6 +514,15 @@ class Repos(APIView):
             groups = get_groups_by_user(request)
             group_repos = get_group_repos(request, groups)
             group_repos.sort(lambda x, y: cmp(y.last_modify, x.last_modify))
+
+            # Reduce memcache fetch ops.
+            modifiers_set = set([x.last_modifier for x in group_repos])
+            for e in modifiers_set:
+                if e not in contact_email_dict:
+                    contact_email_dict[e] = email2contact_email(e)
+                if e not in nickname_dict:
+                    nickname_dict[e] = email2nickname(e)
+
             for r in group_repos:
                 repo = {
                     "type": "grepo",
@@ -495,6 +531,9 @@ class Repos(APIView):
                     "groupid": r.group.id,
                     "name": r.name,
                     "mtime": r.last_modify,
+                    "modifier_email": r.last_modifier,
+                    "modifier_contact_email": contact_email_dict.get(r.last_modifier, ''),
+                    "modifier_name": nickname_dict.get(r.last_modifier, ''),
                     "size": r.size,
                     "encrypted": r.encrypted,
                     "permission": check_permission(r.id, email),
@@ -506,6 +545,15 @@ class Repos(APIView):
 
         if filter_by['org'] and request.user.permissions.can_view_org():
             public_repos = list_inner_pub_repos(request)
+
+            # Reduce memcache fetch ops.
+            modifiers_set = set([x.last_modifier for x in public_repos])
+            for e in modifiers_set:
+                if e not in contact_email_dict:
+                    contact_email_dict[e] = email2contact_email(e)
+                if e not in nickname_dict:
+                    nickname_dict[e] = email2nickname(e)
+
             for r in public_repos:
                 repo = {
                     "type": "grepo",
@@ -514,6 +562,9 @@ class Repos(APIView):
                     "owner": "Organization",
                     "mtime": r.last_modified,
                     "mtime_relative": translate_seahub_time(r.last_modified),
+                    "modifier_email": r.last_modifier,
+                    "modifier_contact_email": contact_email_dict.get(r.last_modifier, ''),
+                    "modifier_name": nickname_dict.get(r.last_modifier, ''),
                     "size": r.size,
                     "size_formatted": filesizeformat(r.size),
                     "encrypted": r.encrypted,
@@ -806,6 +857,9 @@ class Repo(APIView):
             "encrypted":repo.encrypted,
             "root":root_id,
             "permission": check_permission(repo.id, username),
+            "modifier_email": repo.last_modifier,
+            "modifier_contact_email": email2contact_email(repo.last_modifier),
+            "modifier_name": email2nickname(repo.last_modifier),
             }
         if repo.encrypted:
             repo_json["enc_version"] = repo.enc_version
@@ -3798,6 +3852,9 @@ class GroupRepos(APIView):
             "owner": username,
             "owner_nickname": email2nickname(username),
             "share_from_me": True,
+            "modifier_email": repo.last_modifier,
+            "modifier_contact_email": email2contact_email(repo.last_modifier),
+            "modifier_name": email2nickname(repo.last_modifier),
         }
 
         return Response(group_repo, status=200)
@@ -3819,6 +3876,17 @@ class GroupRepos(APIView):
         repos.sort(lambda x, y: cmp(y.last_modified, x.last_modified))
         group.is_staff = is_group_staff(group, request.user)
 
+        # Use dict to reduce memcache fetch cost in large for-loop.
+        contact_email_dict = {}
+        nickname_dict = {}
+        owner_set = set([x.user for x in repos])
+        modifiers_set = set([x.modifier for x in repos])
+        for e in owner_set | modifiers_set:
+            if e not in contact_email_dict:
+                contact_email_dict[e] = email2contact_email(e)
+            if e not in nickname_dict:
+                nickname_dict[e] = email2nickname(e)
+
         repos_json = []
         for r in repos:
             repo = {
@@ -3832,8 +3900,11 @@ class GroupRepos(APIView):
                 "encrypted": r.encrypted,
                 "permission": r.permission,
                 "owner": r.user,
-                "owner_nickname": email2nickname(r.user),
+                "owner_nickname": nickname_dict.get(r.user, ''),
                 "share_from_me": True if username == r.user else False,
+                "modifier_email": r.last_modifier,
+                "modifier_contact_email": contact_email_dict.get(r.last_modifier, ''),
+                "modifier_name": nickname_dict.get(r.last_modifier, ''),
             }
             repos_json.append(repo)
 

tests/api/endpoints/test_shared_repos.py

@@ -58,6 +58,9 @@ class SharedReposTest(BaseTestCase):
         assert json_resp[0]['user_email'] == self.admin_name
         assert json_resp[0]['user_name'] == nickname
         assert json_resp[0]['contact_email'] == contact_email
+        assert len(json_resp[0]['modifier_email']) > 0
+        assert len(json_resp[0]['modifier_name']) > 0
+        assert len(json_resp[0]['modifier_contact_email']) > 0
 
     def test_can_get_when_share_to_group(self):
         self.share_repo_to_group()

tests/api/test_group_repos.py

@@ -58,6 +58,10 @@ class GroupRepoTest(ApiTestBase):
             assert resp_repo['mtime'] > 0
             assert resp_repo['permission'] in ('r', 'rw')
             assert '</time>' in resp_repo['mtime_relative']
+            assert len(resp_repo['owner_nickname']) > 0
+            assert len(resp_repo['modifier_email']) > 0
+            assert len(resp_repo['modifier_contact_email']) > 0
+            assert len(resp_repo['modifier_name']) > 0
 
     def test_order_by_mtime(self):
         with self.get_tmp_group() as group:

tests/api/test_repo_group_folder_perm.py

@@ -147,7 +147,10 @@ class RepoGroupFolderPermTest(BaseTestCase):
         }
 
         resp = self.client.post(url, data)
-        self.assertEqual(409, resp.status_code)
+        json_resp = json.loads(resp.content)
+        assert len(json_resp['failed']) == 1
+        assert len(json_resp['success']) == 0
+        assert json_resp['failed'][0]['group_id'] == self.group_id
 
     def test_can_delete_folder_perm(self):
 

tests/api/test_repo_owner.py

@@ -65,13 +65,21 @@ class RepoOwnerTest(BaseTestCase):
                 '/', tmp_user) == 'rw'
 
     def test_not_reshare_to_user_after_transfer_repo(self):
-        # Remove share if repo already shared to new owner
+
+        # remove all share
+        shared_repos = seafile_api.get_share_in_repo_list(self.admin.username, -1, -1)
+        for repo in  shared_repos:
+            seafile_api.remove_share(repo.repo_id, self.admin.username,
+                    self.user.username)
+
+            seafile_api.remove_share(repo.repo_id, self.user.username,
+                    self.admin.username)
 
         # share user's repo to admin with 'rw' permission
         seafile_api.share_repo(self.user_repo_id, self.user.username,
                 self.admin.username, 'rw')
 
-        # repo in admin's be shared repo list
+        # assert repo in admin's be shared repo list
         shared_repos = seafile_api.get_share_in_repo_list(self.admin.username, -1, -1)
         assert shared_repos[0].repo_name == self.repo.repo_name
 
@@ -83,7 +91,7 @@ class RepoOwnerTest(BaseTestCase):
         resp = self.client.put(url, data, 'application/x-www-form-urlencoded')
         self.assertEqual(200, resp.status_code)
 
-        # repo NOT in admin's be shared repo list
+        # assert repo NOT in admin's be shared repo list
         shared_repos = seafile_api.get_share_in_repo_list(self.admin.username, -1, -1)
         assert len(shared_repos) == 0
 

tests/api/test_repo_user_folder_perm.py

@@ -146,7 +146,10 @@ class RepoUserFolderPermTest(BaseTestCase):
         }
 
         resp = self.client.post(url, data)
-        self.assertEqual(409, resp.status_code)
+        json_resp = json.loads(resp.content)
+        assert len(json_resp['failed']) == 1
+        assert len(json_resp['success']) == 0
+        assert json_resp['failed'][0]['user_email'] == self.admin_email
 
     def test_can_delete_folder_perm(self):
 

tests/api/test_repos.py

@@ -2,10 +2,14 @@
 """
 Test repos api.
 """
+import json
+import time
 import pytest
 import uuid
 
+from django.core.urlresolvers import reverse
 from seaserv import seafile_api
+
 from tests.api.apitestbase import ApiTestBase
 from tests.api.urls import (
     REPOS_URL, DEFAULT_REPO_URL, GET_REPO_TOKENS_URL
@@ -13,6 +17,8 @@ from tests.api.urls import (
 from tests.common.utils import apiurl, urljoin, randstring
 from tests.common.common import SEAFILE_BASE_URL
 
+from seahub.test_utils import BaseTestCase
+
 # TODO: all tests should be run on an encrypted repo
 class ReposApiTest(ApiTestBase):
     def test_get_default_repo(self):
@@ -39,6 +45,9 @@ class ReposApiTest(ApiTestBase):
             self.assertIsNotNone(repo['name'])
             self.assertIsNotNone(repo['type'])
             self.assertIsNotNone(repo['head_commit_id'])
+            assert len(repo['modifier_email']) > 0
+            assert len(repo['modifier_name']) > 0
+            assert len(repo['modifier_contact_email']) > 0
 
     def test_get_repo_info(self):
         with self.get_tmp_repo() as repo:
@@ -52,7 +61,9 @@ class ReposApiTest(ApiTestBase):
             self.assertIsNotNone(rinfo['root'])
             self.assertIsNotNone(rinfo['desc'])
             self.assertIsNotNone(rinfo['type'])
-            # elf.assertIsNotNone(rinfo['password_need']) # allow null here
+            assert len(rinfo['modifier_email']) > 0
+            assert len(rinfo['modifier_name']) > 0
+            assert len(rinfo['modifier_contact_email']) > 0
 
     def test_get_repo_history(self):
         with self.get_tmp_repo() as repo:
@@ -176,3 +187,35 @@ class ReposApiTest(ApiTestBase):
 
         # do some file operation
         self.create_file(repo['repo_id'])
+
+
+# Uncomment following to test api performance.
+# class ReposApiTest2(BaseTestCase):
+#     def setUp(self):
+#         self.num_repos = 500
+#         self.tmp_user = self.create_user()
+#         self.login_as(self.tmp_user)
+
+#         self.repo_ids = []
+#         for i in range(self.num_repos):
+#             r = self.create_repo(name='test-repo%d' % i, desc='',
+#                                  username=self.tmp_user.username, passwd=None)
+#             self.repo_ids.append(r)
+#             time.sleep(0.01)
+
+#         assert len(self.repo_ids) == self.num_repos
+
+#     def tearDown(self):
+#         self.remove_user(self.tmp_user.username)
+#         for e in self.repo_ids:
+#             self.remove_repo(e)
+
+#     def test_list_repos(self):
+#         time.sleep(1)
+#         print '--------> start list repos...'
+
+#         resp = self.client.get(reverse('api2-repos') + '?type=mine')
+#         json_resp = json.loads(resp.content)
+#         assert len(json_resp) == self.num_repos
+
+#         print '--------> end list repos.'