caches: new cache implementation for remote functions

vcsserver/git.py

@@ -14,6 +14,7 b''
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, write to the Free Software Foundation,
 # Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301  USA
+
 import collections
 import logging
 import os
@@ -39,7 +40,7 b' from dulwich.server import update_server'
 
 from vcsserver import exceptions, settings, subprocessio
 from vcsserver.utils import safe_str
-from vcsserver.base import RepoFactory, obfuscate_qs, raise_from_original
+from vcsserver.base import RepoFactory, obfuscate_qs
 from vcsserver.hgcompat import (
     hg_url as url_parser, httpbasicauthhandler, httpdigestauthhandler)
 from vcsserver.git_lfs.lib import LFSOidStore
@@ -47,10 +48,19 b' from vcsserver.git_lfs.lib import LFSOid'
 DIR_STAT = stat.S_IFDIR
 FILE_MODE = stat.S_IFMT
 GIT_LINK = objects.S_IFGITLINK
+PEELED_REF_MARKER = '^{}'
+
 
 log = logging.getLogger(__name__)
 
 
+def str_to_dulwich(value):
+    """
+    Dulwich 0.10.1a requires `unicode` objects to be passed in.
+    """
+    return value.decode(settings.WIRE_ENCODING)
+
+
 def reraise_safe_exceptions(func):
     """Converts Dulwich exceptions to something neutral."""
 
@@ -111,19 +121,7 b' class GitFactory(RepoFactory):'
         """
         Get a repository instance for the given path.
         """
-        region = self._cache_region
-        context = wire.get('context', None)
-        repo_path = wire.get('path', '')
-        context_uid = '{}'.format(context)
-        cache = wire.get('cache', True)
-        cache_on = context and cache
-
-        @region.conditional_cache_on_arguments(condition=cache_on)
-        def create_new_repo(_repo_type, _repo_path, _context_uid, _use_libgit2):
-            return self._create_repo(wire, create, use_libgit2)
-
-        repo = create_new_repo(self.repo_type, repo_path, context_uid, use_libgit2)
-        return repo
+        return self._create_repo(wire, create, use_libgit2)
 
     def repo_libgit2(self, wire):
         return self.repo(wire, use_libgit2=True)
@@ -133,14 +131,15 b' class GitRemote(object):'
 
     def __init__(self, factory):
         self._factory = factory
-        self.peeled_ref_marker = '^{}'
         self._bulk_methods = {
             "date": self.date,
             "author": self.author,
+            "branch": self.branch,
             "message": self.message,
             "parents": self.parents,
             "_commit": self.revision,
         }
+        self.region = self._factory._cache_region
 
     def _wire_to_config(self, wire):
         if 'config' in wire:
@@ -156,6 +155,23 b' class GitRemote(object):'
             params.extend(['-c', 'http.sslCAinfo={}'.format(ssl_cert_dir)])
         return params
 
+    def _cache_on(self, wire):
+        context = wire.get('context', '')
+        context_uid = '{}'.format(context)
+        repo_id = wire.get('repo_id', '')
+        cache = wire.get('cache', True)
+        cache_on = context and cache
+        return cache_on, context_uid, repo_id
+
+    @reraise_safe_exceptions
+    def discover_git_version(self):
+        stdout, _ = self.run_git_command(
+            {}, ['--version'], _bare=True, _safe=True)
+        prefix = 'git version'
+        if stdout.startswith(prefix):
+            stdout = stdout[len(prefix):]
+        return stdout.strip()
+
     @reraise_safe_exceptions
     def is_empty(self, wire):
         repo_init = self._factory.repo_libgit2(wire)
@@ -184,17 +200,21 b' class GitRemote(object):'
 
     @reraise_safe_exceptions
     def assert_correct_path(self, wire):
-        try:
-            repo_init = self._factory.repo_libgit2(wire)
-            with repo_init as repo:
-                pass
-        except pygit2.GitError:
-            path = wire.get('path')
-            tb = traceback.format_exc()
-            log.debug("Invalid Git path `%s`, tb: %s", path, tb)
-            return False
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _assert_correct_path(_context_uid, _repo_id):
+            try:
+                repo_init = self._factory.repo_libgit2(wire)
+                with repo_init as repo:
+                    pass
+            except pygit2.GitError:
+                path = wire.get('path')
+                tb = traceback.format_exc()
+                log.debug("Invalid Git path `%s`, tb: %s", path, tb)
+                return False
 
-        return True
+            return True
+        return _assert_correct_path(context_uid, repo_id)
 
     @reraise_safe_exceptions
     def bare(self, wire):
@@ -212,10 +232,16 b' class GitRemote(object):'
 
     @reraise_safe_exceptions
     def blob_raw_length(self, wire, sha):
-        repo_init = self._factory.repo_libgit2(wire)
-        with repo_init as repo:
-            blob = repo[sha]
-            return blob.size
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _blob_raw_length(_context_uid, _repo_id, _sha):
+
+            repo_init = self._factory.repo_libgit2(wire)
+            with repo_init as repo:
+                blob = repo[sha]
+                return blob.size
+
+        return _blob_raw_length(context_uid, repo_id, sha)
 
     def _parse_lfs_pointer(self, raw_content):
 
@@ -236,13 +262,19 b' class GitRemote(object):'
 
     @reraise_safe_exceptions
     def is_large_file(self, wire, sha):
-        repo_init = self._factory.repo_libgit2(wire)
-        with repo_init as repo:
-            blob = repo[sha]
-            if blob.is_binary:
-                return {}
 
-            return self._parse_lfs_pointer(blob.data)
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _is_large_file(_context_uid, _repo_id, _sha):
+            repo_init = self._factory.repo_libgit2(wire)
+            with repo_init as repo:
+                blob = repo[sha]
+                if blob.is_binary:
+                    return {}
+
+                return self._parse_lfs_pointer(blob.data)
+
+        return _is_large_file(context_uid, repo_id, sha)
 
     @reraise_safe_exceptions
     def in_largefiles_store(self, wire, oid):
@@ -276,15 +308,21 b' class GitRemote(object):'
 
     @reraise_safe_exceptions
     def bulk_request(self, wire, rev, pre_load):
-        result = {}
-        for attr in pre_load:
-            try:
-                method = self._bulk_methods[attr]
-                args = [wire, rev]
-                result[attr] = method(*args)
-            except KeyError as e:
-                raise exceptions.VcsException(e)("Unknown bulk attribute: %s" % attr)
-        return result
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _bulk_request(_context_uid, _repo_id, _rev, _pre_load):
+            result = {}
+            for attr in pre_load:
+                try:
+                    method = self._bulk_methods[attr]
+                    args = [wire, rev]
+                    result[attr] = method(*args)
+                except KeyError as e:
+                    raise exceptions.VcsException(e)(
+                        "Unknown bulk attribute: %s" % attr)
+            return result
+
+        return _bulk_request(context_uid, repo_id, rev, sorted(pre_load))
 
     def _build_opener(self, url):
         handlers = []
@@ -371,6 +409,34 b' class GitRemote(object):'
             index.build_index_from_tree(repo.path, repo.index_path(),
                                         repo.object_store, repo["HEAD"].tree)
 
+    @reraise_safe_exceptions
+    def branch(self, wire, commit_id):
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        cache_on = False
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _branch(_context_uid, _repo_id, _commit_id):
+            regex = re.compile('^refs/heads')
+
+            def filter_with(ref):
+                return regex.match(ref[0]) and ref[1] == _commit_id
+
+            branches = filter(filter_with, self.get_refs(wire).items())
+            return [x[0].split('refs/heads/')[-1] for x in branches]
+
+        return _branch(context_uid, repo_id, commit_id)
+
+    @reraise_safe_exceptions
+    def commit_branches(self, wire, commit_id):
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _commit_branches(_context_uid, _repo_id, _commit_id):
+            repo_init = self._factory.repo_libgit2(wire)
+            with repo_init as repo:
+                branches = [x for x in repo.branches.with_commit(_commit_id)]
+                return branches
+
+        return _commit_branches(context_uid, repo_id, commit_id)
+
     # TODO: this is quite complex, check if that can be simplified
     @reraise_safe_exceptions
     def commit(self, wire, commit_data, branch, commit_tree, updated, removed):
@@ -510,7 +576,7 b' class GitRemote(object):'
             # that contains a tag object, so that we would end up with
             # a peeled ref at this point.
             for k in remote_refs:
-                if k.endswith(self.peeled_ref_marker):
+                if k.endswith(PEELED_REF_MARKER):
                     log.debug("Skipping peeled reference %s", k)
                     continue
                 repo[k] = remote_refs[k]
@@ -547,7 +613,7 b' class GitRemote(object):'
             if ref in remote_refs:
                 # duplicate, skip
                 continue
-            if ref.endswith(self.peeled_ref_marker):
+            if ref.endswith(PEELED_REF_MARKER):
                 log.debug("Skipping peeled reference %s", ref)
                 continue
             # don't sync HEAD
@@ -579,7 +645,7 b' class GitRemote(object):'
         if not self.check_url(url, wire):
             return
         config = self._wire_to_config(wire)
-        repo = self._factory.repo(wire)
+        self._factory.repo(wire)
         self.run_git_command(
             wire, ['push', url, '--mirror'], fail_on_stderr=False,
             _copts=self._remote_conf(config),
@@ -612,53 +678,80 b' class GitRemote(object):'
 
     @reraise_safe_exceptions
     def get_object(self, wire, sha):
-        repo_init = self._factory.repo_libgit2(wire)
-        with repo_init as repo:
+
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _get_object(_context_uid, _repo_id, _sha):
+            repo_init = self._factory.repo_libgit2(wire)
+            with repo_init as repo:
 
-            missing_commit_err = 'Commit {} does not exist for `{}`'.format(sha, wire['path'])
-            try:
-                commit = repo.revparse_single(sha)
-            except (KeyError, ValueError) as e:
-                raise exceptions.LookupException(e)(missing_commit_err)
+                missing_commit_err = 'Commit {} does not exist for `{}`'.format(sha, wire['path'])
+                try:
+                    commit = repo.revparse_single(sha)
+                except (KeyError, ValueError) as e:
+                    raise exceptions.LookupException(e)(missing_commit_err)
 
-            if isinstance(commit, pygit2.Tag):
-                commit = repo.get(commit.target)
+                if isinstance(commit, pygit2.Tag):
+                    commit = repo.get(commit.target)
 
-            # check for dangling commit
-            branches = [x for x in repo.branches.with_commit(commit.hex)]
-            if not branches:
-                raise exceptions.LookupException(None)(missing_commit_err)
+                # check for dangling commit
+                branches = [x for x in repo.branches.with_commit(commit.hex)]
+                if not branches:
+                    raise exceptions.LookupException(None)(missing_commit_err)
+
+                commit_id = commit.hex
+                type_id = commit.type
 
-            commit_id = commit.hex
-            type_id = commit.type
+                return {
+                    'id': commit_id,
+                    'type': self._type_id_to_name(type_id),
+                    'commit_id': commit_id,
+                    'idx': 0
+                }
 
-            return {
-                'id': commit_id,
-                'type': self._type_id_to_name(type_id),
-                'commit_id': commit_id,
-                'idx': 0
-            }
+        return _get_object(context_uid, repo_id, sha)
 
     @reraise_safe_exceptions
     def get_refs(self, wire):
-        repo_init = self._factory.repo_libgit2(wire)
-        with repo_init as repo:
-            result = {}
-            for ref in repo.references:
-                peeled_sha = repo.lookup_reference(ref).peel()
-                result[ref] = peeled_sha.hex
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _get_refs(_context_uid, _repo_id):
+
+            repo_init = self._factory.repo_libgit2(wire)
+            with repo_init as repo:
+                regex = re.compile('^refs/(heads|tags)/')
+                return {x.name: x.target.hex for x in
+                        filter(lambda ref: regex.match(ref.name) ,repo.listall_reference_objects())}
+
+        return _get_refs(context_uid, repo_id)
 
-            return result
+    @reraise_safe_exceptions
+    def get_branch_pointers(self, wire):
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _get_branch_pointers(_context_uid, _repo_id):
+
+            repo_init = self._factory.repo_libgit2(wire)
+            regex = re.compile('^refs/heads')
+            with repo_init as repo:
+                branches = filter(lambda ref: regex.match(ref.name), repo.listall_reference_objects())
+                return {x.target.hex: x.shorthand for x in branches}
+
+        return _get_branch_pointers(context_uid, repo_id)
 
     @reraise_safe_exceptions
     def head(self, wire, show_exc=True):
-        repo_init = self._factory.repo_libgit2(wire)
-        with repo_init as repo:
-            try:
-                return repo.head.peel().hex
-            except Exception:
-                if show_exc:
-                    raise
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _head(_context_uid, _repo_id, _show_exc):
+            repo_init = self._factory.repo_libgit2(wire)
+            with repo_init as repo:
+                try:
+                    return repo.head.peel().hex
+                except Exception:
+                    if show_exc:
+                        raise
+        return _head(context_uid, repo_id, show_exc)
 
     @reraise_safe_exceptions
     def init(self, wire):
@@ -672,17 +765,22 b' class GitRemote(object):'
 
     @reraise_safe_exceptions
     def revision(self, wire, rev):
-        repo_init = self._factory.repo_libgit2(wire)
-        with repo_init as repo:
-            commit = repo[rev]
-            obj_data = {
-                'id': commit.id.hex,
-            }
-            # tree objects itself don't have tree_id attribute
-            if hasattr(commit, 'tree_id'):
-                obj_data['tree'] = commit.tree_id.hex
 
-            return obj_data
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _revision(_context_uid, _repo_id, _rev):
+            repo_init = self._factory.repo_libgit2(wire)
+            with repo_init as repo:
+                commit = repo[rev]
+                obj_data = {
+                    'id': commit.id.hex,
+                }
+                # tree objects itself don't have tree_id attribute
+                if hasattr(commit, 'tree_id'):
+                    obj_data['tree'] = commit.tree_id.hex
+
+                return obj_data
+        return _revision(context_uid, repo_id, rev)
 
     @reraise_safe_exceptions
     def date(self, wire, rev):
@@ -711,10 +809,14 b' class GitRemote(object):'
 
     @reraise_safe_exceptions
     def parents(self, wire, rev):
-        repo_init = self._factory.repo_libgit2(wire)
-        with repo_init as repo:
-            commit = repo[rev]
-            return [x.hex for x in commit.parent_ids]
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _parents(_context_uid, _repo_id, _rev):
+            repo_init = self._factory.repo_libgit2(wire)
+            with repo_init as repo:
+                commit = repo[rev]
+                return [x.hex for x in commit.parent_ids]
+        return _parents(context_uid, repo_id, rev)
 
     @reraise_safe_exceptions
     def set_refs(self, wire, key, value):
@@ -758,39 +860,49 b' class GitRemote(object):'
 
     @reraise_safe_exceptions
     def tree_and_type_for_path(self, wire, commit_id, path):
-        repo_init = self._factory.repo_libgit2(wire)
+
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _tree_and_type_for_path(_context_uid, _repo_id, _commit_id, _path):
+            repo_init = self._factory.repo_libgit2(wire)
 
-        with repo_init as repo:
-            commit = repo[commit_id]
-            try:
-                tree = commit.tree[path]
-            except KeyError:
-                return None, None, None
+            with repo_init as repo:
+                commit = repo[commit_id]
+                try:
+                    tree = commit.tree[path]
+                except KeyError:
+                    return None, None, None
 
-            return tree.id.hex, tree.type, tree.filemode
+                return tree.id.hex, tree.type, tree.filemode
+        return _tree_and_type_for_path(context_uid, repo_id, commit_id, path)
 
     @reraise_safe_exceptions
     def tree_items(self, wire, tree_id):
-        repo_init = self._factory.repo_libgit2(wire)
+
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _tree_items(_context_uid, _repo_id, _tree_id):
 
-        with repo_init as repo:
-            try:
-                tree = repo[tree_id]
-            except KeyError:
-                raise ObjectMissing('No tree with id: {}'.format(tree_id))
+            repo_init = self._factory.repo_libgit2(wire)
+            with repo_init as repo:
+                try:
+                    tree = repo[tree_id]
+                except KeyError:
+                    raise ObjectMissing('No tree with id: {}'.format(tree_id))
 
-            result = []
-            for item in tree:
-                item_sha = item.hex
-                item_mode = item.filemode
-                item_type = item.type
+                result = []
+                for item in tree:
+                    item_sha = item.hex
+                    item_mode = item.filemode
+                    item_type = item.type
 
-                if item_type == 'commit':
-                    # NOTE(marcink): submodules we translate to 'link' for backward compat
-                    item_type = 'link'
+                    if item_type == 'commit':
+                        # NOTE(marcink): submodules we translate to 'link' for backward compat
+                        item_type = 'link'
 
-                result.append((item.name, item_mode, item_sha, item_type))
-            return result
+                    result.append((item.name, item_mode, item_sha, item_type))
+                return result
+        return _tree_items(context_uid, repo_id, tree_id)
 
     @reraise_safe_exceptions
     def update_server_info(self, wire):
@@ -798,24 +910,20 b' class GitRemote(object):'
         update_server_info(repo)
 
     @reraise_safe_exceptions
-    def discover_git_version(self):
-        stdout, _ = self.run_git_command(
-            {}, ['--version'], _bare=True, _safe=True)
-        prefix = 'git version'
-        if stdout.startswith(prefix):
-            stdout = stdout[len(prefix):]
-        return stdout.strip()
-
-    @reraise_safe_exceptions
     def get_all_commit_ids(self, wire):
 
-        cmd = ['rev-list', '--reverse', '--date-order', '--branches', '--tags']
-        try:
-            output, __ = self.run_git_command(wire, cmd)
-            return output.splitlines()
-        except Exception:
-            # Can be raised for empty repositories
-            return []
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _get_all_commit_ids(_context_uid, _repo_id):
+
+            cmd = ['rev-list', '--reverse', '--date-order', '--branches', '--tags']
+            try:
+                output, __ = self.run_git_command(wire, cmd)
+                return output.splitlines()
+            except Exception:
+                # Can be raised for empty repositories
+                return []
+        return _get_all_commit_ids(context_uid, repo_id)
 
     @reraise_safe_exceptions
     def run_git_command(self, wire, cmd, **opts):
@@ -870,22 +978,17 b' class GitRemote(object):'
     @reraise_safe_exceptions
     def install_hooks(self, wire, force=False):
         from vcsserver.hook_utils import install_git_hooks
-        repo = self._factory.repo(wire)
-        return install_git_hooks(repo.path, repo.bare, force_create=force)
+        bare = self.bare(wire)
+        path = wire['path']
+        return install_git_hooks(path, bare, force_create=force)
 
     @reraise_safe_exceptions
     def get_hooks_info(self, wire):
         from vcsserver.hook_utils import (
             get_git_pre_hook_version, get_git_post_hook_version)
-        repo = self._factory.repo(wire)
+        bare = self.bare(wire)
+        path = wire['path']
         return {
-            'pre_version': get_git_pre_hook_version(repo.path, repo.bare),
-            'post_version': get_git_post_hook_version(repo.path, repo.bare),
+            'pre_version': get_git_pre_hook_version(path, bare),
+            'post_version': get_git_post_hook_version(path, bare),
         }
-
-
-def str_to_dulwich(value):
-    """
-    Dulwich 0.10.1a requires `unicode` objects to be passed in.
-    """
-    return value.decode(settings.WIRE_ENCODING)

vcsserver/hg.py

@@ -147,25 +147,13 b' class MercurialFactory(RepoFactory):'
         """
         Get a repository instance for the given path.
         """
-        region = self._cache_region
-        context = wire.get('context', None)
-        repo_path = wire.get('path', '')
-        context_uid = '{}'.format(context)
-        cache = wire.get('cache', True)
-        cache_on = context and cache
-
-        @region.conditional_cache_on_arguments(condition=cache_on)
-        def create_new_repo(_repo_type, _repo_path, _context_uid):
-            return self._create_repo(wire, create)
-
-        return create_new_repo(self.repo_type, repo_path, context_uid)
+        return self._create_repo(wire, create)
 
 
 class HgRemote(object):
 
     def __init__(self, factory):
         self._factory = factory
-
         self._bulk_methods = {
             "affected_files": self.ctx_files,
             "author": self.ctx_user,
@@ -180,10 +168,19 b' class HgRemote(object):'
             "hidden": self.ctx_hidden,
             "_file_paths": self.ctx_list,
         }
+        self.region = self._factory._cache_region
 
     def _get_ctx(self, repo, ref):
         return get_ctx(repo, ref)
 
+    def _cache_on(self, wire):
+        context = wire.get('context', '')
+        context_uid = '{}'.format(context)
+        repo_id = wire.get('repo_id', '')
+        cache = wire.get('cache', True)
+        cache_on = context and cache
+        return cache_on, context_uid, repo_id
+
     @reraise_safe_exceptions
     def discover_hg_version(self):
         from mercurial import util
@@ -217,33 +214,48 b' class HgRemote(object):'
 
     @reraise_safe_exceptions
     def bookmarks(self, wire):
-        repo = self._factory.repo(wire)
-        return dict(repo._bookmarks)
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _bookmarks(_context_uid, _repo_id):
+            repo = self._factory.repo(wire)
+            return dict(repo._bookmarks)
+
+        return _bookmarks(context_uid, repo_id)
 
     @reraise_safe_exceptions
     def branches(self, wire, normal, closed):
-        repo = self._factory.repo(wire)
-        iter_branches = repo.branchmap().iterbranches()
-        bt = {}
-        for branch_name, _heads, tip, is_closed in iter_branches:
-            if normal and not is_closed:
-                bt[branch_name] = tip
-            if closed and is_closed:
-                bt[branch_name] = tip
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _branches(_context_uid, _repo_id, _normal, _closed):
+            repo = self._factory.repo(wire)
+            iter_branches = repo.branchmap().iterbranches()
+            bt = {}
+            for branch_name, _heads, tip, is_closed in iter_branches:
+                if normal and not is_closed:
+                    bt[branch_name] = tip
+                if closed and is_closed:
+                    bt[branch_name] = tip
 
-        return bt
+            return bt
+
+        return _branches(context_uid, repo_id, normal, closed)
 
     @reraise_safe_exceptions
     def bulk_request(self, wire, rev, pre_load):
-        result = {}
-        for attr in pre_load:
-            try:
-                method = self._bulk_methods[attr]
-                result[attr] = method(wire, rev)
-            except KeyError as e:
-                raise exceptions.VcsException(e)(
-                    'Unknown bulk attribute: "%s"' % attr)
-        return result
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _bulk_request(_context_uid, _repo_id, _rev, _pre_load):
+            result = {}
+            for attr in pre_load:
+                try:
+                    method = self._bulk_methods[attr]
+                    result[attr] = method(wire, rev)
+                except KeyError as e:
+                    raise exceptions.VcsException(e)(
+                        'Unknown bulk attribute: "%s"' % attr)
+            return result
+
+        return _bulk_request(context_uid, repo_id, rev, sorted(pre_load))
 
     @reraise_safe_exceptions
     def clone(self, wire, source, dest, update_after_clone=False, hooks=True):
@@ -251,9 +263,8 b' class HgRemote(object):'
         clone(baseui, source, dest, noupdate=not update_after_clone)
 
     @reraise_safe_exceptions
-    def commitctx(
-            self, wire, message, parents, commit_time, commit_timezone,
-            user, files, extra, removed, updated):
+    def commitctx(self, wire, message, parents, commit_time, commit_timezone,
+                  user, files, extra, removed, updated):
 
         repo = self._factory.repo(wire)
         baseui = self._factory._create_config(wire['config'])
@@ -284,7 +295,7 b' class HgRemote(object):'
                         data=node['content'],
                         islink=False,
                         isexec=bool(node['mode'] & stat.S_IXUSR),
-                        copied=False)
+                        copysource=False)
 
             raise exceptions.AbortException()(
                 "Given path haven't been marked as added, "
@@ -309,15 +320,14 b' class HgRemote(object):'
 
     @reraise_safe_exceptions
     def ctx_branch(self, wire, revision):
-        repo = self._factory.repo(wire)
-        ctx = self._get_ctx(repo, revision)
-        return ctx.branch()
 
-    @reraise_safe_exceptions
-    def ctx_children(self, wire, revision):
-        repo = self._factory.repo(wire)
-        ctx = self._get_ctx(repo, revision)
-        return [child.rev() for child in ctx.children()]
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _ctx_branch(_context_uid, _repo_id, _revision):
+            repo = self._factory.repo(wire)
+            ctx = self._get_ctx(repo, revision)
+            return ctx.branch()
+        return _ctx_branch(context_uid, repo_id, revision)
 
     @reraise_safe_exceptions
     def ctx_date(self, wire, revision):
@@ -333,9 +343,15 b' class HgRemote(object):'
 
     @reraise_safe_exceptions
     def ctx_files(self, wire, revision):
-        repo = self._factory.repo(wire)
-        ctx = self._get_ctx(repo, revision)
-        return ctx.files()
+
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _ctx_files(_context_uid, _repo_id, _revision):
+            repo = self._factory.repo(wire)
+            ctx = self._get_ctx(repo, revision)
+            return ctx.files()
+
+        return _ctx_files(context_uid, repo_id, revision)
 
     @reraise_safe_exceptions
     def ctx_list(self, path, revision):
@@ -345,9 +361,27 b' class HgRemote(object):'
 
     @reraise_safe_exceptions
     def ctx_parents(self, wire, revision):
-        repo = self._factory.repo(wire)
-        ctx = self._get_ctx(repo, revision)
-        return [parent.rev() for parent in ctx.parents()]
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _ctx_parents(_context_uid, _repo_id, _revision):
+            repo = self._factory.repo(wire)
+            ctx = self._get_ctx(repo, revision)
+            return [parent.rev() for parent in ctx.parents()
+                    if not (parent.hidden() or parent.obsolete())]
+
+        return _ctx_parents(context_uid, repo_id, revision)
+
+    @reraise_safe_exceptions
+    def ctx_children(self, wire, revision):
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _ctx_children(_context_uid, _repo_id, _revision):
+            repo = self._factory.repo(wire)
+            ctx = self._get_ctx(repo, revision)
+            return [child.rev() for child in ctx.children()
+                    if not (child.hidden() or child.obsolete())]
+
+        return _ctx_children(context_uid, repo_id, revision)
 
     @reraise_safe_exceptions
     def ctx_phase(self, wire, revision):
@@ -456,9 +490,7 b' class HgRemote(object):'
         return True
 
     @reraise_safe_exceptions
-    def diff(
-            self, wire, rev1, rev2, file_filter, opt_git, opt_ignorews,
-            context):
+    def diff(self, wire, rev1, rev2, file_filter, opt_git, opt_ignorews, context):
         repo = self._factory.repo(wire)
 
         if file_filter:
@@ -527,7 +559,7 b' class HgRemote(object):'
         return result
 
     @reraise_safe_exceptions
-    def fctx_data(self, wire, revision, path):
+    def fctx_node_data(self, wire, revision, path):
         repo = self._factory.repo(wire)
         ctx = self._get_ctx(repo, revision)
         fctx = ctx.filectx(path)
@@ -549,10 +581,14 b' class HgRemote(object):'
 
     @reraise_safe_exceptions
     def get_all_commit_ids(self, wire, name):
-        repo = self._factory.repo(wire)
-        repo = repo.filtered(name)
-        revs = map(lambda x: hex(x[7]), repo.changelog.index)
-        return revs
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _get_all_commit_ids(_context_uid, _repo_id, _name):
+            repo = self._factory.repo(wire)
+            repo = repo.filtered(name)
+            revs = map(lambda x: hex(x[7]), repo.changelog.index)
+            return revs
+        return _get_all_commit_ids(context_uid, repo_id, name)
 
     @reraise_safe_exceptions
     def get_config_value(self, wire, section, name, untrusted=False):
@@ -571,7 +607,12 b' class HgRemote(object):'
 
     @reraise_safe_exceptions
     def is_large_file(self, wire, path):
-        return largefiles.lfutil.isstandin(path)
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _is_large_file(_context_uid, _repo_id, _path):
+            return largefiles.lfutil.isstandin(path)
+
+        return _is_large_file(context_uid, repo_id, path)
 
     @reraise_safe_exceptions
     def in_largefiles_store(self, wire, sha):
@@ -600,31 +641,36 b' class HgRemote(object):'
 
     @reraise_safe_exceptions
     def lookup(self, wire, revision, both):
-
-        repo = self._factory.repo(wire)
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _lookup(_context_uid, _repo_id, _revision, _both):
 
-        if isinstance(revision, int):
-            # NOTE(marcink):
-            # since Mercurial doesn't support negative indexes properly
-            # we need to shift accordingly by one to get proper index, e.g
-            # repo[-1] => repo[-2]
-            # repo[0]  => repo[-1]
-            if revision <= 0:
-                revision = revision + -1
-        try:
-            ctx = self._get_ctx(repo, revision)
-        except (TypeError, RepoLookupError) as e:
-            e._org_exc_tb = traceback.format_exc()
-            raise exceptions.LookupException(e)(revision)
-        except LookupError as e:
-            e._org_exc_tb = traceback.format_exc()
-            raise exceptions.LookupException(e)(e.name)
+            repo = self._factory.repo(wire)
+            rev = _revision
+            if isinstance(rev, int):
+                # NOTE(marcink):
+                # since Mercurial doesn't support negative indexes properly
+                # we need to shift accordingly by one to get proper index, e.g
+                # repo[-1] => repo[-2]
+                # repo[0]  => repo[-1]
+                if rev <= 0:
+                    rev = rev + -1
+            try:
+                ctx = self._get_ctx(repo, rev)
+            except (TypeError, RepoLookupError) as e:
+                e._org_exc_tb = traceback.format_exc()
+                raise exceptions.LookupException(e)(rev)
+            except LookupError as e:
+                e._org_exc_tb = traceback.format_exc()
+                raise exceptions.LookupException(e)(e.name)
 
-        if not both:
-            return ctx.hex()
+            if not both:
+                return ctx.hex()
 
-        ctx = repo[ctx.hex()]
-        return ctx.hex(), ctx.rev()
+            ctx = repo[ctx.hex()]
+            return ctx.hex(), ctx.rev()
+
+        return _lookup(context_uid, repo_id, revision, both)
 
     @reraise_safe_exceptions
     def pull(self, wire, url, commit_ids=None):
@@ -667,10 +713,15 b' class HgRemote(object):'
         return ctx.rev()
 
     @reraise_safe_exceptions
-    def rev_range(self, wire, filter):
-        repo = self._factory.repo(wire)
-        revisions = [rev for rev in revrange(repo, filter)]
-        return revisions
+    def rev_range(self, wire, commit_filter):
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _rev_range(_context_uid, _repo_id, _filter):
+            repo = self._factory.repo(wire)
+            revisions = [rev for rev in revrange(repo, commit_filter)]
+            return revisions
+
+        return _rev_range(context_uid, repo_id, sorted(commit_filter))
 
     @reraise_safe_exceptions
     def rev_range_hash(self, wire, node):
@@ -724,8 +775,7 b' class HgRemote(object):'
         return output.getvalue()
 
     @reraise_safe_exceptions
-    def tag(self, wire, name, revision, message, local, user,
-            tag_time, tag_timezone):
+    def tag(self, wire, name, revision, message, local, user, tag_time, tag_timezone):
         repo = self._factory.repo(wire)
         ctx = self._get_ctx(repo, revision)
         node = ctx.node()
@@ -740,8 +790,13 b' class HgRemote(object):'
 
     @reraise_safe_exceptions
     def tags(self, wire):
-        repo = self._factory.repo(wire)
-        return repo.tags()
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _tags(_context_uid, _repo_id):
+            repo = self._factory.repo(wire)
+            return repo.tags()
+
+        return _tags(context_uid, repo_id)
 
     @reraise_safe_exceptions
     def update(self, wire, node=None, clean=False):
@@ -762,8 +817,7 b' class HgRemote(object):'
         return output.getvalue()
 
     @reraise_safe_exceptions
-    def pull_cmd(self, wire, source, bookmark=None, branch=None, revision=None,
-                 hooks=True):
+    def pull_cmd(self, wire, source, bookmark=None, branch=None, revision=None, hooks=True):
         repo = self._factory.repo(wire)
         baseui = self._factory._create_config(wire['config'], hooks=hooks)
 
@@ -806,8 +860,7 b' class HgRemote(object):'
         return hex(a)
 
     @reraise_safe_exceptions
-    def push(self, wire, revisions, dest_path, hooks=True,
-             push_branches=False):
+    def push(self, wire, revisions, dest_path, hooks=True, push_branches=False):
         repo = self._factory.repo(wire)
         baseui = self._factory._create_config(wire['config'], hooks=hooks)
         commands.push(baseui, repo, dest=dest_path, rev=revisions,
@@ -846,7 +899,6 b' class HgRemote(object):'
         repo.ui.setconfig('ui', 'username', username)
         commands.commit(baseui, repo, message=message, close_branch=close_branch)
 
-
     @reraise_safe_exceptions
     def rebase(self, wire, source=None, dest=None, abort=False):
         repo = self._factory.repo(wire)

vcsserver/svn.py

@@ -98,19 +98,7 b' class SubversionFactory(RepoFactory):'
         """
         Get a repository instance for the given path.
         """
-        region = self._cache_region
-        context = wire.get('context', None)
-        repo_path = wire.get('path', '')
-        context_uid = '{}'.format(context)
-        cache = wire.get('cache', True)
-        cache_on = context and cache
-
-        @region.conditional_cache_on_arguments(condition=cache_on)
-        def create_new_repo(_repo_type, _repo_path, _context_uid, compatible_version_id):
-            return self._create_repo(wire, create, compatible_version)
-
-        return create_new_repo(self.repo_type, repo_path, context_uid,
-                               compatible_version)
+        return self._create_repo(wire, create, compatible_version)
 
 
 NODE_TYPE_MAPPING = {
@@ -126,6 +114,15 b' class SvnRemote(object):'
         # TODO: Remove once we do not use internal Mercurial objects anymore
         # for subversion
         self._hg_factory = hg_factory
+        self.region = self._factory._cache_region
+
+    def _cache_on(self, wire):
+        context = wire.get('context', '')
+        context_uid = '{}'.format(context)
+        repo_id = wire.get('repo_id', '')
+        cache = wire.get('cache', True)
+        cache_on = context and cache
+        return cache_on, context_uid, repo_id
 
     @reraise_safe_exceptions
     def discover_svn_version(self):
@@ -138,7 +135,6 b' class SvnRemote(object):'
 
     @reraise_safe_exceptions
     def is_empty(self, wire):
-        repo = self._factory.repo(wire)
 
         try:
             return self.lookup(wire, -1) == 0
@@ -216,9 +212,14 b' class SvnRemote(object):'
         return start_rev, end_rev
 
     def revision_properties(self, wire, revision):
-        repo = self._factory.repo(wire)
-        fs_ptr = svn.repos.fs(repo)
-        return svn.fs.revision_proplist(fs_ptr, revision)
+
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _revision_properties(_context_uid, _repo_id, _revision):
+            repo = self._factory.repo(wire)
+            fs_ptr = svn.repos.fs(repo)
+            return svn.fs.revision_proplist(fs_ptr, revision)
+        return _revision_properties(context_uid, repo_id, revision)
 
     def revision_changes(self, wire, revision):
 
@@ -264,22 +265,27 b' class SvnRemote(object):'
         }
         return changes
 
+    @reraise_safe_exceptions
     def node_history(self, wire, path, revision, limit):
-        cross_copies = False
-        repo = self._factory.repo(wire)
-        fsobj = svn.repos.fs(repo)
-        rev_root = svn.fs.revision_root(fsobj, revision)
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _assert_correct_path(_context_uid, _repo_id, _path, _revision, _limit):
+            cross_copies = False
+            repo = self._factory.repo(wire)
+            fsobj = svn.repos.fs(repo)
+            rev_root = svn.fs.revision_root(fsobj, revision)
 
-        history_revisions = []
-        history = svn.fs.node_history(rev_root, path)
-        history = svn.fs.history_prev(history, cross_copies)
-        while history:
-            __, node_revision = svn.fs.history_location(history)
-            history_revisions.append(node_revision)
-            if limit and len(history_revisions) >= limit:
-                break
+            history_revisions = []
+            history = svn.fs.node_history(rev_root, path)
             history = svn.fs.history_prev(history, cross_copies)
-        return history_revisions
+            while history:
+                __, node_revision = svn.fs.history_location(history)
+                history_revisions.append(node_revision)
+                if limit and len(history_revisions) >= limit:
+                    break
+                history = svn.fs.history_prev(history, cross_copies)
+            return history_revisions
+        return _assert_correct_path(context_uid, repo_id, path, revision, limit)
 
     def node_properties(self, wire, path, revision):
         repo = self._factory.repo(wire)
@@ -314,27 +320,37 b' class SvnRemote(object):'
 
         return annotations
 
-    def get_node_type(self, wire, path, rev=None):
-        repo = self._factory.repo(wire)
-        fs_ptr = svn.repos.fs(repo)
-        if rev is None:
-            rev = svn.fs.youngest_rev(fs_ptr)
-        root = svn.fs.revision_root(fs_ptr, rev)
-        node = svn.fs.check_path(root, path)
-        return NODE_TYPE_MAPPING.get(node, None)
+    def get_node_type(self, wire, path, revision=None):
+
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _get_node_type(_context_uid, _repo_id, _path, _revision):
+            repo = self._factory.repo(wire)
+            fs_ptr = svn.repos.fs(repo)
+            if _revision is None:
+                _revision = svn.fs.youngest_rev(fs_ptr)
+            root = svn.fs.revision_root(fs_ptr, _revision)
+            node = svn.fs.check_path(root, path)
+            return NODE_TYPE_MAPPING.get(node, None)
+        return _get_node_type(context_uid, repo_id, path, revision)
 
     def get_nodes(self, wire, path, revision=None):
-        repo = self._factory.repo(wire)
-        fsobj = svn.repos.fs(repo)
-        if revision is None:
-            revision = svn.fs.youngest_rev(fsobj)
-        root = svn.fs.revision_root(fsobj, revision)
-        entries = svn.fs.dir_entries(root, path)
-        result = []
-        for entry_path, entry_info in entries.iteritems():
-            result.append(
-                (entry_path, NODE_TYPE_MAPPING.get(entry_info.kind, None)))
-        return result
+
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _get_nodes(_context_uid, _repo_id, _path, _revision):
+            repo = self._factory.repo(wire)
+            fsobj = svn.repos.fs(repo)
+            if _revision is None:
+                _revision = svn.fs.youngest_rev(fsobj)
+            root = svn.fs.revision_root(fsobj, _revision)
+            entries = svn.fs.dir_entries(root, path)
+            result = []
+            for entry_path, entry_info in entries.iteritems():
+                result.append(
+                    (entry_path, NODE_TYPE_MAPPING.get(entry_info.kind, None)))
+            return result
+        return _get_nodes(context_uid, repo_id, path, revision)
 
     def get_file_content(self, wire, path, rev=None):
         repo = self._factory.repo(wire)
@@ -346,13 +362,18 b' class SvnRemote(object):'
         return content.read()
 
     def get_file_size(self, wire, path, revision=None):
-        repo = self._factory.repo(wire)
-        fsobj = svn.repos.fs(repo)
-        if revision is None:
-            revision = svn.fs.youngest_revision(fsobj)
-        root = svn.fs.revision_root(fsobj, revision)
-        size = svn.fs.file_length(root, path)
-        return size
+
+        cache_on, context_uid, repo_id = self._cache_on(wire)
+        @self.region.conditional_cache_on_arguments(condition=cache_on)
+        def _get_file_size(_context_uid, _repo_id, _path, _revision):
+            repo = self._factory.repo(wire)
+            fsobj = svn.repos.fs(repo)
+            if _revision is None:
+                _revision = svn.fs.youngest_revision(fsobj)
+            root = svn.fs.revision_root(fsobj, _revision)
+            size = svn.fs.file_length(root, path)
+            return size
+        return _get_file_size(context_uid, repo_id, path, revision)
 
     def create_repository(self, wire, compatible_version=None):
         log.info('Creating Subversion repository in path "%s"', wire['path'])

vcsserver/tests/test_git.py

@@ -61,7 +61,7 b' class TestGitFetch(object):'
 
         with patch('dulwich.client.LocalGitClient.fetch') as mock_fetch:
             mock_fetch.side_effect = side_effect
-            self.remote_git.pull(wire=None, url='/tmp/', apply_refs=False)
+            self.remote_git.pull(wire={}, url='/tmp/', apply_refs=False)
             determine_wants = self.mock_repo.object_store.determine_wants_all
             determine_wants.assert_called_once_with(SAMPLE_REFS)
 
@@ -79,7 +79,7 b' class TestGitFetch(object):'
         with patch('dulwich.client.LocalGitClient.fetch') as mock_fetch:
             mock_fetch.side_effect = side_effect
             self.remote_git.pull(
-                wire=None, url='/tmp/', apply_refs=False,
+                wire={}, url='/tmp/', apply_refs=False,
                 refs=selected_refs.keys())
             determine_wants = self.mock_repo.object_store.determine_wants_all
             assert determine_wants.call_count == 0
@@ -95,7 +95,7 b' class TestGitFetch(object):'
 
         with patch('vcsserver.git.Repo', create=False) as mock_repo:
             mock_repo().get_refs.return_value = sample_refs
-            remote_refs = remote_git.get_remote_refs(wire=None, url=url)
+            remote_refs = remote_git.get_remote_refs(wire={}, url=url)
             mock_repo().get_refs.assert_called_once_with()
             assert remote_refs == sample_refs
 

vcsserver/tests/test_hg.py

@@ -26,36 +26,17 b' from mock import Mock, MagicMock, patch'
 from vcsserver import exceptions, hg, hgcompat
 
 
-class TestHGLookup(object):
-    def setup(self):
-        self.mock_repo = MagicMock()
-        self.mock_repo.__getitem__.side_effect = LookupError(
-            'revision_or_commit_id', 'index', 'message')
-        factory = Mock()
-        factory.repo = Mock(return_value=self.mock_repo)
-        self.remote_hg = hg.HgRemote(factory)
-
-    def test_fail_lookup_hg(self):
-        with pytest.raises(Exception) as exc_info:
-            self.remote_hg.lookup(
-                wire=None, revision='revision_or_commit_id', both=True)
-
-        assert exc_info.value._vcs_kind == 'lookup'
-        assert 'revision_or_commit_id' in exc_info.value.args
-
-
 class TestDiff(object):
     def test_raising_safe_exception_when_lookup_failed(self):
-        repo = Mock()
+
         factory = Mock()
-        factory.repo = Mock(return_value=repo)
         hg_remote = hg.HgRemote(factory)
         with patch('mercurial.patch.diff') as diff_mock:
             diff_mock.side_effect = LookupError(
                 'deadbeef', 'index', 'message')
             with pytest.raises(Exception) as exc_info:
                 hg_remote.diff(
-                    wire=None, rev1='deadbeef', rev2='deadbee1',
+                    wire={}, rev1='deadbeef', rev2='deadbee1',
                     file_filter=None, opt_git=True, opt_ignorews=True,
                     context=3)
             assert type(exc_info.value) == Exception

vcsserver/tests/test_svn.py

@@ -45,8 +45,10 b" INVALID_CERTIFICATE_STDERR = '\\n'.join(["
                    reason="SVN not packaged for Cygwin")
 def test_import_remote_repository_certificate_error(stderr, expected_reason):
     from vcsserver import svn
+    factory = mock.Mock()
+    factory.repo = mock.Mock(return_value=mock.Mock())
 
-    remote = svn.SvnRemote(None)
+    remote = svn.SvnRemote(factory)
     remote.is_path_valid_repository = lambda wire, path: True
 
     with mock.patch('subprocess.Popen',
@@ -76,7 +78,10 b' def test_svn_libraries_can_be_imported()'
 def test_username_password_extraction_from_url(example_url, parts):
     from vcsserver import svn
 
-    remote = svn.SvnRemote(None)
+    factory = mock.Mock()
+    factory.repo = mock.Mock(return_value=mock.Mock())
+
+    remote = svn.SvnRemote(factory)
     remote.is_path_valid_repository = lambda wire, path: True
 
     assert remote.get_url_and_credentials(example_url) == parts