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,20 +121,8 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
-
     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,6 +200,9 b' class GitRemote(object):'
 
     @reraise_safe_exceptions
     def assert_correct_path(self, wire):
+        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:
@@ -195,6 +214,7 b' class GitRemote(object):'
             return False
 
         return True
+        return _assert_correct_path(context_uid, repo_id)
 
     @reraise_safe_exceptions
     def bare(self, wire):
@@ -212,11 +232,17 b' class GitRemote(object):'
 
     @reraise_safe_exceptions
     def blob_raw_length(self, wire, sha):
+        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):
 
         spec_string = 'version https://git-lfs.github.com/spec'
@@ -236,6 +262,10 b' class GitRemote(object):'
 
     @reraise_safe_exceptions
     def is_large_file(self, wire, sha):
+
+        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]
@@ -244,6 +274,8 b' class GitRemote(object):'
 
             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):
         conf = self._wire_to_config(wire)
@@ -276,6 +308,9 b' class GitRemote(object):'
 
     @reraise_safe_exceptions
     def bulk_request(self, wire, rev, pre_load):
+        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:
@@ -283,9 +318,12 b' class GitRemote(object):'
                 args = [wire, rev]
                 result[attr] = method(*args)
             except KeyError as e:
-                raise exceptions.VcsException(e)("Unknown bulk attribute: %s" % attr)
+                    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 = []
         url_obj = url_parser(url)
@@ -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,6 +678,10 b' class GitRemote(object):'
 
     @reraise_safe_exceptions
     def get_object(self, wire, sha):
+
+        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:
 
@@ -639,19 +709,41 b' class GitRemote(object):'
                 'idx': 0
             }
 
+        return _get_object(context_uid, repo_id, sha)
+
     @reraise_safe_exceptions
     def get_refs(self, wire):
+        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:
-            result = {}
-            for ref in repo.references:
-                peeled_sha = repo.lookup_reference(ref).peel()
-                result[ref] = peeled_sha.hex
+                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):
+        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:
@@ -659,6 +751,7 b' class GitRemote(object):'
             except Exception:
                 if show_exc:
                     raise
+        return _head(context_uid, repo_id, show_exc)
 
     @reraise_safe_exceptions
     def init(self, wire):
@@ -672,6 +765,10 b' class GitRemote(object):'
 
     @reraise_safe_exceptions
     def revision(self, wire, rev):
+
+        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]
@@ -683,6 +780,7 b' class GitRemote(object):'
                 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):
+        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,6 +860,10 b' class GitRemote(object):'
 
     @reraise_safe_exceptions
     def tree_and_type_for_path(self, wire, commit_id, path):
+
+        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:
@@ -768,11 +874,16 b' class GitRemote(object):'
                 return None, None, None
 
             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):
+
+        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):
+
         repo_init = self._factory.repo_libgit2(wire)
-
         with repo_init as repo:
             try:
                 tree = repo[tree_id]
@@ -791,6 +902,7 b' class GitRemote(object):'
 
                 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,16 +910,11 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()
+    def get_all_commit_ids(self, wire):
 
-    @reraise_safe_exceptions
-    def get_all_commit_ids(self, wire):
+        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:
@@ -816,6 +923,7 b' class GitRemote(object):'
         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)
-
 
 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,11 +214,19 b' class HgRemote(object):'
 
     @reraise_safe_exceptions
     def bookmarks(self, wire):
+        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):
+        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 = {}
@@ -233,8 +238,13 b' class HgRemote(object):'
 
         return bt
 
+        return _branches(context_uid, repo_id, normal, closed)
+
     @reraise_safe_exceptions
     def bulk_request(self, wire, rev, pre_load):
+        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:
@@ -245,14 +255,15 b' class HgRemote(object):'
                     '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):
         baseui = self._factory._create_config(wire["config"], hooks=hooks)
         clone(baseui, source, dest, noupdate=not update_after_clone)
 
     @reraise_safe_exceptions
-    def commitctx(
-            self, wire, message, parents, commit_time, commit_timezone,
+    def commitctx(self, wire, message, parents, commit_time, commit_timezone,
             user, files, extra, removed, updated):
 
         repo = self._factory.repo(wire)
@@ -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):
+
+        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()
-
-    @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()]
+        return _ctx_branch(context_uid, repo_id, revision)
 
     @reraise_safe_exceptions
     def ctx_date(self, wire, revision):
@@ -333,10 +343,16 b' class HgRemote(object):'
 
     @reraise_safe_exceptions
     def ctx_files(self, wire, revision):
+
+        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):
         repo = self._factory.repo(path)
@@ -345,9 +361,27 b' class HgRemote(object):'
 
     @reraise_safe_exceptions
     def ctx_parents(self, wire, revision):
+        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()]
+            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):
+        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,8 +607,13 b' class HgRemote(object):'
 
     @reraise_safe_exceptions
     def is_large_file(self, wire, 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):
         repo = self._factory.repo(wire)
@@ -600,22 +641,25 b' class HgRemote(object):'
 
     @reraise_safe_exceptions
     def lookup(self, wire, revision, both):
+        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):
 
         repo = self._factory.repo(wire)
-
-        if isinstance(revision, int):
+            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 revision <= 0:
-                revision = revision + -1
+                if rev <= 0:
+                    rev = rev + -1
         try:
-            ctx = self._get_ctx(repo, revision)
+                ctx = self._get_ctx(repo, rev)
         except (TypeError, RepoLookupError) as e:
             e._org_exc_tb = traceback.format_exc()
-            raise exceptions.LookupException(e)(revision)
+                raise exceptions.LookupException(e)(rev)
         except LookupError as e:
             e._org_exc_tb = traceback.format_exc()
             raise exceptions.LookupException(e)(e.name)
@@ -626,6 +670,8 b' class HgRemote(object):'
         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):
         repo = self._factory.repo(wire)
@@ -667,11 +713,16 b' class HgRemote(object):'
         return ctx.rev()
 
     @reraise_safe_exceptions
-    def rev_range(self, wire, filter):
+    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, filter)]
+            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):
         repo = self._factory.repo(wire)
@@ -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,9 +790,14 b' class HgRemote(object):'
 
     @reraise_safe_exceptions
     def tags(self, wire):
+        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):
         repo = self._factory.repo(wire)
@@ -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,20 +98,8 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)
-
 
 NODE_TYPE_MAPPING = {
     svn.core.svn_node_file: 'file',
@@ -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):
+
+        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,7 +265,11 b' class SvnRemote(object):'
         }
         return changes
 
+    @reraise_safe_exceptions
     def node_history(self, wire, path, revision, limit):
+        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)
@@ -280,6 +285,7 b' class SvnRemote(object):'
                 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):
+    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 rev is None:
-            rev = svn.fs.youngest_rev(fs_ptr)
-        root = svn.fs.revision_root(fs_ptr, rev)
+            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):
+
+        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)
+            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):
+
+        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)
+            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