changeset 4364:e50e6384c529

compare: handle revset revision errors more gracefully
author Mads Kiilerich <madski@unity3d.com>
date Fri, 18 Jul 2014 19:22:01 +0200
parents 7ad20e8f16bd
children 3abfe76f1ac7
files kallithea/controllers/changeset.py kallithea/controllers/compare.py kallithea/lib/base.py kallithea/lib/vcs/backends/git/repository.py kallithea/lib/vcs/backends/hg/changeset.py kallithea/lib/vcs/backends/hg/repository.py
diffstat 6 files changed, 70 insertions(+), 50 deletions(-) [+]
line wrap: on
line diff
--- a/kallithea/controllers/changeset.py	Fri Jul 18 19:22:01 2014 +0200
+++ b/kallithea/controllers/changeset.py	Fri Jul 18 19:22:01 2014 +0200
@@ -211,9 +211,6 @@
             msg = _('Such revision does not exist for this repository')
             h.flash(msg, category='error')
             raise HTTPNotFound()
-        except (Exception,), e:
-            log.error(traceback.format_exc())
-            raise HTTPNotFound()
 
         c.changes = OrderedDict()
 
--- a/kallithea/controllers/compare.py	Fri Jul 18 19:22:01 2014 +0200
+++ b/kallithea/controllers/compare.py	Fri Jul 18 19:22:01 2014 +0200
@@ -36,7 +36,6 @@
 from pylons.controllers.util import abort, redirect
 from pylons.i18n.translation import _
 
-from kallithea.lib.vcs.exceptions import EmptyRepositoryError, RepositoryError
 from kallithea.lib.vcs.utils import safe_str
 from kallithea.lib.vcs.utils.hgcompat import unionrepo
 from kallithea.lib import helpers as h
@@ -59,37 +58,6 @@
         super(CompareController, self).__before__()
 
     @staticmethod
-    def __get_rev(ref, repo):
-        """
-        Safe way to get changeset. If error occurs show error.
-        """
-        rev = ref[1] # default and used for git
-        if repo.scm_instance.alias == 'hg':
-            # lookup up the exact node id
-            _revset_predicates = {
-                    'branch': 'branch',
-                    'book': 'bookmark',
-                    'tag': 'tag',
-                    'rev': 'id',
-                }
-            rev_spec = "max(%s(%%s))" % _revset_predicates[ref[0]]
-            revs = repo.scm_instance._repo.revs(rev_spec, safe_str(ref[1]))
-            if revs:
-                rev = revs[-1]
-            # else: TODO: just report 'not found'
-
-        try:
-            return repo.scm_instance.get_changeset(rev).raw_id
-        except EmptyRepositoryError, e:
-            h.flash(h.literal(_('There are no changesets yet')),
-                    category='error')
-            raise HTTPNotFound()
-        except RepositoryError, e:
-            log.error(traceback.format_exc())
-            h.flash(safe_str(e), category='error')
-            raise HTTPBadRequest()
-
-    @staticmethod
     def _get_changesets(alias, org_repo, org_rev, other_repo, other_rev):
         """
         Returns lists of changesets that can be merged from org_repo@org_rev
@@ -188,11 +156,7 @@
     @HasRepoPermissionAnyDecorator('repository.read', 'repository.write',
                                    'repository.admin')
     def compare(self, repo_name, org_ref_type, org_ref_name, other_ref_type, other_ref_name):
-        # org_ref will be evaluated in org_repo
         org_repo = c.db_repo.repo_name
-        org_ref = (org_ref_type, org_ref_name)
-        # other_ref will be evaluated in other_repo
-        other_ref = (other_ref_type, other_ref_name)
         other_repo = request.GET.get('other_repo', org_repo)
         # If merge is True:
         #   Show what org would get if merged with other:
@@ -244,8 +208,8 @@
             h.flash(msg, category='error')
             return redirect(url('compare_home', repo_name=c.repo_name))
 
-        c.org_rev = self.__get_rev(ref=org_ref, repo=org_repo)
-        c.other_rev = self.__get_rev(ref=other_ref, repo=other_repo)
+        c.org_rev = self._get_ref_rev(org_repo, org_ref_type, org_ref_name)
+        c.other_rev = self._get_ref_rev(other_repo, other_ref_type, other_ref_name)
 
         c.compare_home = False
         c.org_repo = org_repo
--- a/kallithea/lib/base.py	Fri Jul 18 19:22:01 2014 +0200
+++ b/kallithea/lib/base.py	Fri Jul 18 19:22:01 2014 +0200
@@ -32,9 +32,10 @@
 import time
 import traceback
 
-from paste.auth.basic import AuthBasicAuthenticator
-from paste.httpexceptions import HTTPUnauthorized, HTTPForbidden, HTTPNotFound
-from paste.httpheaders import WWW_AUTHENTICATE, AUTHORIZATION
+import webob.exc
+import paste.httpexceptions
+import paste.auth.basic
+import paste.httpheaders
 
 from pylons import config, tmpl_context as c, request, session, url
 from pylons.controllers import WSGIController
@@ -50,6 +51,7 @@
 from kallithea.lib.auth import AuthUser, HasPermissionAnyMiddleware, CookieStoreWrapper
 from kallithea.lib.utils import get_repo_slug
 from kallithea.lib.exceptions import UserCreationError
+from kallithea.lib.vcs.exceptions import RepositoryError, EmptyRepositoryError, ChangesetDoesNotExistError
 from kallithea.model import meta
 
 from kallithea.model.db import Repository, Ui, User, Setting
@@ -102,7 +104,7 @@
     return path
 
 
-class BasicAuth(AuthBasicAuthenticator):
+class BasicAuth(paste.auth.basic.AuthBasicAuthenticator):
 
     def __init__(self, realm, authfunc, auth_http_code=None):
         self.realm = realm
@@ -110,15 +112,15 @@
         self._rc_auth_http_code = auth_http_code
 
     def build_authentication(self):
-        head = WWW_AUTHENTICATE.tuples('Basic realm="%s"' % self.realm)
+        head = paste.httpheaders.WWW_AUTHENTICATE.tuples('Basic realm="%s"' % self.realm)
         if self._rc_auth_http_code and self._rc_auth_http_code == '403':
             # return 403 if alternative http return code is specified in
             # Kallithea config
-            return HTTPForbidden(headers=head)
-        return HTTPUnauthorized(headers=head)
+            return paste.httpexceptions.HTTPForbidden(headers=head)
+        return paste.httpexceptions.HTTPUnauthorized(headers=head)
 
     def authenticate(self, environ):
-        authorization = AUTHORIZATION(environ)
+        authorization = paste.httpheaders.AUTHORIZATION(environ)
         if not authorization:
             return self.build_authentication()
         (authmeth, auth) = authorization.split(' ', 1)
@@ -412,7 +414,7 @@
                 from kallithea.lib import helpers as h
                 h.flash(h.literal(_('Repository not found in the filesystem')),
                         category='error')
-                raise HTTPNotFound()
+                raise paste.httpexceptions.HTTPNotFound()
 
             # some globals counter for menu
             c.repository_followers = self.scm_model.get_followers(dbr)
@@ -420,3 +422,24 @@
             c.repository_pull_requests = self.scm_model.get_pull_requests(dbr)
             c.repository_following = self.scm_model.is_following_repo(
                                     c.repo_name, self.authuser.user_id)
+
+    @staticmethod
+    def _get_ref_rev(repo, ref_type, ref_name):
+        """
+        Safe way to get changeset. If error occurs show error.
+        """
+        from kallithea.lib import helpers as h
+        try:
+            return repo.scm_instance.get_ref_revision(ref_type, ref_name)
+        except EmptyRepositoryError as e:
+            h.flash(h.literal(_('There are no changesets yet')),
+                    category='error')
+            raise webob.exc.HTTPNotFound()
+        except ChangesetDoesNotExistError as e:
+            h.flash(h.literal(_('Changeset not found')),
+                    category='error')
+            raise webob.exc.HTTPNotFound()
+        except RepositoryError as e:
+            log.error(traceback.format_exc())
+            h.flash(safe_str(e), category='error')
+            raise webob.exc.HTTPBadRequest()
--- a/kallithea/lib/vcs/backends/git/repository.py	Fri Jul 18 19:22:01 2014 +0200
+++ b/kallithea/lib/vcs/backends/git/repository.py	Fri Jul 18 19:22:01 2014 +0200
@@ -313,6 +313,13 @@
                 % revision)
         return revision
 
+    def get_ref_revision(self, ref_type, ref_name):
+        """
+        Returns ``MercurialChangeset`` object representing repository's
+        changeset at the given ``revision``.
+        """
+        return self._get_revision(ref_name)
+
     def _get_archives(self, archive_name='tip'):
 
         for i in [('zip', '.zip'), ('gz', '.tar.gz'), ('bz2', '.tar.bz2')]:
--- a/kallithea/lib/vcs/backends/hg/changeset.py	Fri Jul 18 19:22:01 2014 +0200
+++ b/kallithea/lib/vcs/backends/hg/changeset.py	Fri Jul 18 19:22:01 2014 +0200
@@ -24,6 +24,7 @@
 
     def __init__(self, repository, revision):
         self.repository = repository
+        assert isinstance(revision, basestring), repr(revision)
         self.raw_id = revision
         self._ctx = repository._repo[revision]
         self.revision = self._ctx._rev
--- a/kallithea/lib/vcs/backends/hg/repository.py	Fri Jul 18 19:22:01 2014 +0200
+++ b/kallithea/lib/vcs/backends/hg/repository.py	Fri Jul 18 19:22:01 2014 +0200
@@ -449,6 +449,34 @@
 
         return revision
 
+    def get_ref_revision(self, ref_type, ref_name):
+        """
+        Returns revision number for the given reference.
+        """
+        # lookup up the exact node id
+        _revset_predicates = {
+                'branch': 'branch',
+                'book': 'bookmark',
+                'tag': 'tag',
+                'rev': 'id',
+            }
+        rev_spec = "max(%s(%%s))" % _revset_predicates[ref_type]
+        try:
+            revs = self._repo.revs(rev_spec, safe_str(ref_name))
+        except (LookupError, ):
+            msg = ("Ambiguous identifier %s:%s for %s" % (ref_type, ref_name, self.name))
+            raise ChangesetDoesNotExistError(msg)
+        except (IndexError, ValueError, RepoLookupError, TypeError):
+            msg = ("Revision %s:%s does not exist for %s" % (ref_type, ref_name, self.name))
+            raise ChangesetDoesNotExistError(msg)
+        if revs:
+            revision = revs[-1]
+        else:
+            # TODO: just report 'not found'?
+            revision = ref_name
+
+        return self._get_revision(revision)
+
     def _get_archives(self, archive_name='tip'):
         allowed = self.baseui.configlist("web", "allow_archive",
                                          untrusted=True)