kallithea Changeset - 7ad20e8f16bd

Changeset - 7ad20e8f16bd

Parent rev.

Child rev.

[Not reviewed]

default

0 4 0

Mads Kiilerich - 11 years ago 2014-07-18 19:22:01
madski@unity3d.com

redirect: don't redirect on error - it is better to have clear error messages and stable and meaningful URLs

4 files changed with 41 insertions and 61 deletions:

kallithea/controllers/changelog.py

kallithea/controllers/compare.py

kallithea/controllers/files.py

kallithea/lib/base.py

0 comments (0 inline, 0 general)

kallithea/controllers/changelog.py

➞

Show inline comments

@@ @@ -68,38 +68,30 @@ def _load_changelog_summary(): @@
 class ChangelogController(BaseRepoController):
     def __before__(self):
         super(ChangelogController, self).__before__()
         c.affected_files_cut_off = 60
     def __get_cs_or_redirect(self, rev, repo, redirect_after=True,
                              partial=False):
     @staticmethod
     def __get_cs(rev, repo):
         """
         Safe way to get changeset if error occur it redirects to changeset with
         proper message. If partial is set then don't do redirect raise Exception
         instead
         Safe way to get changeset. If error occur fail with error message.
         :param rev: revision to fetch
         :param repo: repo instance
         """
         try:
             return c.db_repo_scm_instance.get_changeset(rev)
         except EmptyRepositoryError, e:
             if not redirect_after:
                 return None
             h.flash(h.literal(_('There are no changesets yet')),
                     category='warning')
             redirect(url('changelog_home', repo_name=repo.repo_name))
                     category='error')
         except RepositoryError, e:
             log.error(traceback.format_exc())
             h.flash(safe_str(e), category='warning')
             if not partial:
                 redirect(h.url('changelog_home', repo_name=repo.repo_name))
             raise HTTPBadRequest()
             h.flash(safe_str(e), category='error')
         raise HTTPBadRequest()
     @LoginRequired()
     @HasRepoPermissionAnyDecorator('repository.read', 'repository.write',
                                    'repository.admin')
     def index(self, repo_name, revision=None, f_path=None):
         limit = 100
@@ @@ -133,13 +125,13 @@ class ChangelogController(BaseRepoContro @@
                 tip_cs = c.db_repo_scm_instance.get_changeset()
                 try:
                     collection = tip_cs.get_file_history(f_path)
                 except (NodeDoesNotExistError, ChangesetError):
                     #this node is not present at tip !
                     try:
-                        cs = self.__get_cs_or_redirect(revision, repo_name)
+                        cs = self.__get_cs(revision, repo_name)
                         collection = cs.get_file_history(f_path)
                     except RepositoryError, e:
                         h.flash(safe_str(e), category='warning')
                         redirect(h.url('changelog_home', repo_name=repo_name))
                 collection = list(reversed(collection))
             else:

kallithea/controllers/compare.py

➞

Show inline comments

@@ @@ -55,23 +55,16 @@ log = logging.getLogger(__name__) @@
 class CompareController(BaseRepoController):
     def __before__(self):
         super(CompareController, self).__before__()
     def __get_rev_or_redirect(self, ref, repo, redirect_after=True,
                               partial=False):
     @staticmethod
     def __get_rev(ref, repo):
         """
         Safe way to get changeset if error occur it redirects to changeset with
         proper message. If partial is set then don't do redirect raise Exception
         instead
         :param ref:
         :param repo:
         :param redirect_after:
         :param partial:
         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',
@@ @@ -85,23 +78,18 @@ class CompareController(BaseRepoControll @@
                 rev = revs[-1]
             # else: TODO: just report 'not found'
         try:
             return repo.scm_instance.get_changeset(rev).raw_id
         except EmptyRepositoryError, e:
             if not redirect_after:
                 return None
             h.flash(h.literal(_('There are no changesets yet')),
                     category='warning')
             redirect(url('summary_home', repo_name=repo.repo_name))
                     category='error')
             raise HTTPNotFound()
         except RepositoryError, e:
             log.error(traceback.format_exc())
             h.flash(safe_str(e), category='warning')
             if not partial:
                 redirect(h.url('summary_home', repo_name=repo.repo_name))
             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
@@ @@ -253,14 +241,14 @@ class CompareController(BaseRepoControll @@
         if org_repo.scm_instance.alias != other_repo.scm_instance.alias:
             msg = 'compare of two different kind of remote repos not available'
             log.error(msg)
             h.flash(msg, category='error')
             return redirect(url('compare_home', repo_name=c.repo_name))
         c.org_rev = self.__get_rev_or_redirect(ref=org_ref, repo=org_repo, partial=partial)
         c.other_rev = self.__get_rev_or_redirect(ref=other_ref, repo=other_repo, partial=partial)
         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.compare_home = False
         c.org_repo = org_repo
         c.other_repo = other_repo
         c.org_ref_name = org_ref_name
         c.other_ref_name = other_ref_name

kallithea/controllers/files.py

➞

Show inline comments

@@ @@ -69,48 +69,46 @@ log = logging.getLogger(__name__) @@
 class FilesController(BaseRepoController):
     def __before__(self):
         super(FilesController, self).__before__()
         c.cut_off_limit = self.cut_off_limit
-    def __get_cs_or_redirect(self, rev, repo_name, redirect_after=True):
+    def __get_cs(self, rev, silent_empty=False):
         """
         Safe way to get changeset if error occur it redirects to tip with
         proper message
         :param rev: revision to fetch
-        :param repo_name: repo name to redirect after
+        :silent_empty: return None if repository is empty
         """
         try:
             return c.db_repo_scm_instance.get_changeset(rev)
         except EmptyRepositoryError, e:
-            if not redirect_after:
+            if silent_empty:
                 return None
             url_ = url('files_add_home',
                        repo_name=c.repo_name,
                        revision=0, f_path='', anchor='edit')
             add_new = h.link_to(_('Click here to add new file'), url_, class_="alert-link")
             h.flash(h.literal(_('There are no files yet. %s') % add_new),
                     category='warning')
-            redirect(h.url('summary_home', repo_name=repo_name))
+            raise HTTPNotFound()
         except(ChangesetDoesNotExistError, LookupError), e:
             log.error(traceback.format_exc())
             msg = _('Such revision does not exist for this repository')
             h.flash(msg, category='error')
             raise HTTPNotFound()
         except RepositoryError, e:
             h.flash(safe_str(e), category='error')
             raise HTTPNotFound()
-    def __get_filenode_or_redirect(self, repo_name, cs, path):
+    def __get_filenode(self, cs, path):
         """
         Returns file_node, if error occurs or given path is directory,
         it'll redirect to top level path
         Returns file_node or raise HTTP error.
         :param repo_name: repo_name
         :param cs: given changeset
         :param path: path to lookup
         """
         try:
             file_node = cs.get_node(path)
@@ @@ -131,16 +129,16 @@ class FilesController(BaseRepoController @@
     @HasRepoPermissionAnyDecorator('repository.read', 'repository.write',
                                    'repository.admin')
     def index(self, repo_name, revision, f_path, annotate=False):
         # redirect to given revision from form if given
         post_revision = request.POST.get('at_rev', None)
         if post_revision:
-            cs = self.__get_cs_or_redirect(post_revision, repo_name)
+            cs = self.__get_cs(post_revision)
         c.revision = revision
-        c.changeset = self.__get_cs_or_redirect(revision, repo_name)
+        c.changeset = self.__get_cs(revision)
         c.branch = request.GET.get('branch', None)
         c.f_path = f_path
         c.annotate = annotate
         cur_rev = c.changeset.revision
         # prev link
@@ @@ -197,13 +195,13 @@ class FilesController(BaseRepoController @@
     @LoginRequired()
     @HasRepoPermissionAnyDecorator('repository.read', 'repository.write',
                                    'repository.admin')
     @jsonify
     def history(self, repo_name, revision, f_path):
-        changeset = self.__get_cs_or_redirect(revision, repo_name)
+        changeset = self.__get_cs(revision)
         f_path = f_path
         _file = changeset.get_node(f_path)
         if _file.is_file():
             file_history, _hist = self._get_node_history(changeset, f_path)
             res = []
@@ @@ -220,13 +218,13 @@ class FilesController(BaseRepoController @@
             return data
     @LoginRequired()
     @HasRepoPermissionAnyDecorator('repository.read', 'repository.write',
                                    'repository.admin')
     def authors(self, repo_name, revision, f_path):
-        changeset = self.__get_cs_or_redirect(revision, repo_name)
+        changeset = self.__get_cs(revision)
         f_path = f_path
         _file = changeset.get_node(f_path)
         if _file.is_file():
             file_history, _hist = self._get_node_history(changeset, f_path)
             c.authors = []
             for a in set([x.author for x in _hist]):
@@ @@ -234,27 +232,27 @@ class FilesController(BaseRepoController @@
             return render('files/files_history_box.html')
     @LoginRequired()
     @HasRepoPermissionAnyDecorator('repository.read', 'repository.write',
                                    'repository.admin')
     def rawfile(self, repo_name, revision, f_path):
         cs = self.__get_cs_or_redirect(revision, repo_name)
         file_node = self.__get_filenode_or_redirect(repo_name, cs, f_path)
         cs = self.__get_cs(revision)
         file_node = self.__get_filenode(cs, f_path)
         response.content_disposition = 'attachment; filename=%s' % \
             safe_str(f_path.split(Repository.url_sep())[-1])
         response.content_type = file_node.mimetype
         return file_node.content
     @LoginRequired()
     @HasRepoPermissionAnyDecorator('repository.read', 'repository.write',
                                    'repository.admin')
     def raw(self, repo_name, revision, f_path):
         cs = self.__get_cs_or_redirect(revision, repo_name)
         file_node = self.__get_filenode_or_redirect(repo_name, cs, f_path)
         cs = self.__get_cs(revision)
         file_node = self.__get_filenode(cs, f_path)
         raw_mimetype_mapping = {
             # map original mimetype to a mimetype used for "show as raw"
             # you can also provide a content-disposition to override the
             # default "attachment" disposition.
             # orig_type: (new_type, new_dispo)
@@ @@ -315,14 +313,14 @@ class FilesController(BaseRepoController @@
             return redirect(h.url('files_home',
                                   repo_name=repo_name, revision='tip',
                                   f_path=f_path))
         r_post = request.POST
         c.cs = self.__get_cs_or_redirect(revision, repo_name)
         c.file = self.__get_filenode_or_redirect(repo_name, c.cs, f_path)
         c.cs = self.__get_cs(revision)
         c.file = self.__get_filenode(c.cs, f_path)
         c.default_message = _('Deleted file %s via Kallithea') % (f_path)
         c.f_path = f_path
         node_path = f_path
         author = self.authuser.full_contact
@@ @@ -375,14 +373,14 @@ class FilesController(BaseRepoController @@
             return redirect(h.url('files_home',
                                   repo_name=repo_name, revision='tip',
                                   f_path=f_path))
         r_post = request.POST
         c.cs = self.__get_cs_or_redirect(revision, repo_name)
         c.file = self.__get_filenode_or_redirect(repo_name, c.cs, f_path)
         c.cs = self.__get_cs(revision)
         c.file = self.__get_filenode(c.cs, f_path)
         if c.file.is_binary:
             return redirect(url('files_home', repo_name=c.repo_name,
                             revision=c.cs.raw_id, f_path=f_path))
         c.default_message = _('Edited file %s via Kallithea') % (f_path)
         c.f_path = f_path
@@ @@ -430,14 +428,13 @@ class FilesController(BaseRepoController @@
                    h.fmt_date(h.time_to_datetime(repo.locked[1]))),
                   'warning')
             return redirect(h.url('files_home',
                                   repo_name=repo_name, revision='tip'))
         r_post = request.POST
         c.cs = self.__get_cs_or_redirect(revision, repo_name,
                                          redirect_after=False)
         c.cs = self.__get_cs(revision, silent_empty=True)
         if c.cs is None:
             c.cs = EmptyChangeset(alias=c.db_repo_scm_instance.alias)
         c.default_message = (_('Added file via Kallithea'))
         c.f_path = f_path
         if r_post:
@@ @@ -796,10 +793,10 @@ class FilesController(BaseRepoController @@
     @LoginRequired()
     @HasRepoPermissionAnyDecorator('repository.read', 'repository.write',
                                    'repository.admin')
     @jsonify
     def nodelist(self, repo_name, revision, f_path):
         if request.environ.get('HTTP_X_PARTIAL_XHR'):
-            cs = self.__get_cs_or_redirect(revision, repo_name)
+            cs = self.__get_cs(revision)
             _d, _f = ScmModel().get_nodes(repo_name, cs.raw_id, f_path,
                                           flat=False)
             return {'nodes': _d + _f}

kallithea/lib/base.py

➞

Show inline comments

@@ @@ -30,19 +30,20 @@ Original author and date, and relevant c @@
 import logging
 import time
 import traceback
 from paste.auth.basic import AuthBasicAuthenticator
 from paste.httpexceptions import HTTPUnauthorized, HTTPForbidden
+from paste.httpexceptions import HTTPUnauthorized, HTTPForbidden, HTTPNotFound
 from paste.httpheaders import WWW_AUTHENTICATE, AUTHORIZATION
 from pylons import config, tmpl_context as c, request, session, url
 from pylons.controllers import WSGIController
 from pylons.controllers.util import redirect
 from pylons.templating import render_mako as render  # don't remove this import
 from pylons.i18n.translation import _
 from kallithea import __version__, BACKENDS
 from kallithea.lib.utils2 import str2bool, safe_unicode, AttributeDict,\
     safe_str, safe_int
 from kallithea.lib import auth_modules
@@ @@ -405,14 +406,16 @@ class BaseRepoController(BaseController) @@
             dbr = c.db_repo = _dbr
             c.db_repo_scm_instance = c.db_repo.scm_instance
             if c.db_repo_scm_instance is None:
                 log.error('%s this repository is present in database but it '
                           'cannot be created as an scm instance', c.repo_name)
                 redirect(url('home'))
                 from kallithea.lib import helpers as h
                 h.flash(h.literal(_('Repository not found in the filesystem')),
                         category='error')
                 raise HTTPNotFound()
             # some globals counter for menu
             c.repository_followers = self.scm_model.get_followers(dbr)
             c.repository_forks = self.scm_model.get_forks(dbr)
             c.repository_pull_requests = self.scm_model.get_pull_requests(dbr)
             c.repository_following = self.scm_model.is_following_repo(

0 comments (0 inline, 0 general)