kallithea Changeset - ce2a4ef8cd5f

Changeset - ce2a4ef8cd5f

Parent rev.

Child rev.

[Not reviewed]

default

0 7 1

Mads Kiilerich - 7 years ago 2019-01-07 01:50:56
mads@kiilerich.com

Grafted from: fbbda81038e8

middleware: move handling of permanent repo URLs to separate middleware

This is about the handling of repo URLs like '_123' for the repo with repo_id
123. The de-mangling of such URLs was spread out across multiple layers. It
fits much more nicely as a middleware layer. The code in routing and simplehg /
simplegit can thus be removed.

The base _get_by_id function was confusing - fix it by removing it. To do that,
refactor utils introducing fix_repo_id_name to replace get_repo_by_id.

We now assume in the application that we never have any extra leading '/' in
URL paths.

And while trailing extra '/' might be fine in actual URLs, they must be handled
at the routing level, not propagated through all layers. This changeset is not
really changing that.

8 files changed with 96 insertions and 76 deletions:

kallithea/config/app_cfg.py

kallithea/config/routing.py

kallithea/lib/base.py

kallithea/lib/middleware/permanent_repo_url.py

kallithea/lib/middleware/simplegit.py

kallithea/lib/middleware/simplehg.py

kallithea/lib/utils.py

kallithea/tests/other/test_libs.py

0 comments (0 inline, 0 general)

kallithea/config/app_cfg.py

➞

Show inline comments

@@ @@ -30,6 +30,7 @@ from alembic.migration import MigrationC @@
 from sqlalchemy import create_engine
 import mercurial
 from kallithea.lib.middleware.permanent_repo_url import PermanentRepoUrl
 from kallithea.lib.middleware.https_fixup import HttpsFixup
 from kallithea.lib.middleware.simplegit import SimpleGit
 from kallithea.lib.middleware.simplehg import SimpleHg
@@ @@ -208,6 +209,8 @@ def setup_application(app): @@
     # Enable https redirects based on HTTP_X_URL_SCHEME set by proxy
     if any(asbool(config.get(x)) for x in ['https_fixup', 'force_https', 'use_htsts']):
         app = HttpsFixup(app, config)
     app = PermanentRepoUrl(app, config)
     return app

kallithea/config/routing.py

➞

Show inline comments

@@ @@ -33,28 +33,18 @@ def make_map(config): @@
     rmap.minimization = False
     rmap.explicit = False
     from kallithea.lib.utils import (is_valid_repo, is_valid_repo_group,
                                      get_repo_by_id)
     from kallithea.lib.utils import is_valid_repo, is_valid_repo_group
     def check_repo(environ, match_dict):
         """
         check for valid repository for proper 404 handling
         :param environ:
         :param match_dict:
         Check for valid repository for proper 404 handling.
         Also, a bit of side effect modifying match_dict ...
         """
         repo_name = match_dict.get('repo_name')
         if match_dict.get('f_path'):
             # fix for multiple initial slashes that causes errors
             match_dict['f_path'] = match_dict['f_path'].lstrip('/')
         by_id_match = get_repo_by_id(repo_name)
         if by_id_match:
             repo_name = by_id_match
             match_dict['repo_name'] = repo_name
         return is_valid_repo(repo_name, config['base_path'])
         return is_valid_repo(match_dict['repo_name'], config['base_path'])
     def check_group(environ, match_dict):
         """

kallithea/lib/base.py

➞

Show inline comments

@@ @@ -266,23 +266,6 @@ class BaseVCSController(object): @@
     def _handle_request(self, environ, start_response):
         raise NotImplementedError()
     def _get_by_id(self, repo_name):
         """
         Gets a special pattern _<ID> from clone url and tries to replace it
         with a repository_name for support of _<ID> permanent URLs
         :param repo_name:
         """
         data = repo_name.split('/')
         if len(data) >= 2:
             from kallithea.lib.utils import get_repo_by_id
             by_id_match = get_repo_by_id(repo_name)
             if by_id_match:
                 data[1] = safe_str(by_id_match)
         return '/'.join(data)
     def _check_permission(self, action, authuser, repo_name):
         """
         Checks permissions using action (push/pull) user and repository

kallithea/lib/middleware/permanent_repo_url.py

➞

Show inline comments

@@ new file 100644 @@
 # -*- coding: utf-8 -*-
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
 # the Free Software Foundation, either version 3 of the License, or
 # (at your option) any later version.
+#
 # This program is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
+#
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 """
 kallithea.lib.middleware.permanent_repo_url
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 middleware to handle permanent repo URLs, replacing PATH_INFO '/_123/yada' with
 '/name/of/repo/yada' after looking 123 up in the database.
 """
 from kallithea.lib.utils import safe_str, fix_repo_id_name
 class PermanentRepoUrl(object):
     def __init__(self, app, config):
         self.application = app
         self.config = config
     def __call__(self, environ, start_response):
         path_info = environ['PATH_INFO']
         if path_info.startswith('/'): # it must
             path_info = '/' + safe_str(fix_repo_id_name(path_info[1:]))
             environ['PATH_INFO'] = path_info
         return self.application(environ, start_response)

kallithea/lib/middleware/simplegit.py

➞

Show inline comments

@@ @@ -141,14 +141,11 @@ class SimpleGit(BaseVCSController): @@
         :param environ: environ where PATH_INFO is stored
         """
         try:
             environ['PATH_INFO'] = self._get_by_id(environ['PATH_INFO'])
             repo_name = GIT_PROTO_PAT.match(environ['PATH_INFO']).group(1)
             return GIT_PROTO_PAT.match(environ['PATH_INFO']).group(1)
         except Exception:
             log.error(traceback.format_exc())
             raise
         return repo_name
     def __get_action(self, environ):
         """
         Maps Git request commands into a pull or push command.

kallithea/lib/middleware/simplehg.py

➞

Show inline comments

@@ @@ -167,16 +167,13 @@ class SimpleHg(BaseVCSController): @@
         :param environ: environ where PATH_INFO is stored
         """
         try:
             environ['PATH_INFO'] = self._get_by_id(environ['PATH_INFO'])
             repo_name = '/'.join(environ['PATH_INFO'].split('/')[1:])
             if repo_name.endswith('/'):
                 repo_name = repo_name.rstrip('/')
             path_info = environ['PATH_INFO']
             if path_info.startswith('/'):
                 return path_info[1:].rstrip('/')
         except Exception:
             log.error(traceback.format_exc())
             raise
         return repo_name
     def __get_action(self, environ):
         """
         Maps Mercurial request commands into 'pull' or 'push'.

kallithea/lib/utils.py

➞

Show inline comments

@@ @@ -78,29 +78,35 @@ def get_user_group_slug(request): @@
     return None
 def _extract_id_from_repo_name(repo_name):
     if repo_name.startswith('/'):
         repo_name = repo_name.lstrip('/')
     by_id_match = re.match(r'^_(\d{1,})', repo_name)
     if by_id_match:
         return by_id_match.groups()[0]
 def _get_permanent_id(s):
     """Helper for decoding stable URLs with repo ID. For a string like '_123'
     return 123.
     """
     by_id_match = re.match(r'^_(\d+)$', s)
     if by_id_match is None:
         return None
     return int(by_id_match.group(1))
-def get_repo_by_id(repo_name):
+def fix_repo_id_name(path):
     """
-    Extracts repo_name by id from special urls. Example url is _11/repo_name
+    Rewrite repo_name for _<ID> permanent URLs.
     :param repo_name:
     :return: repo_name if matched else None
     Given a path, if the first path element is like _<ID>, return the path with
     this part expanded to the corresponding full repo name, else return the
     provided path.
     """
     _repo_id = _extract_id_from_repo_name(repo_name)
     if _repo_id:
     first, rest = path, ''
     if '/' in path:
         first, rest_ = path.split('/', 1)
         rest = '/' + rest_
     repo_id = _get_permanent_id(first)
     if repo_id is not None:
         from kallithea.model.db import Repository
         repo = Repository.get(_repo_id)
         if repo:
             # TODO: return repo instead of reponame? or would that be a layering violation?
             return repo.repo_name
     return None
         repo = Repository.get(repo_id)
         if repo is not None:
             return repo.repo_name + rest
     return path
 def action_logger(user, action, repo, ipaddr='', commit=False):

kallithea/tests/other/test_libs.py

➞

Show inline comments

@@ @@ -532,26 +532,32 @@ class TestLibs(TestController): @@
     @parametrize('test,expected', [
       ("", None),
       ("/_2", '2'),
       ("_2", '2'),
       ("/_2/", '2'),
       ("_2/", '2'),
       ("/_21", '21'),
       ("_21", '21'),
       ("/_21/", '21'),
       ("_21/", '21'),
       ("/_2", None),
       ("_2", 2),
       ("_2/", None),
     ])
     def test_get_permanent_id(self, test, expected):
         from kallithea.lib.utils import _get_permanent_id
         extracted = _get_permanent_id(test)
         assert extracted == expected, 'url:%s, got:`%s` expected: `%s`' % (test, _test, expected)
       ("/_21/foobar", '21'),
       ("_21/121", '21'),
       ("/_21/_12", '21'),
       ("_21/prefix/foo", '21'),
     @parametrize('test,expected', [
       ("", ""),
       ("/", "/"),
       ("/_ID", '/_ID'),
       ("ID", "ID"),
       ("_ID", 'NAME'),
       ("_ID/", 'NAME/'),
       ("_ID/1/2", 'NAME/1/2'),
       ("_IDa", '_IDa'),
     ])
     def test_get_repo_by_id(self, test, expected):
         from kallithea.lib.utils import _extract_id_from_repo_name
         _test = _extract_id_from_repo_name(test)
         assert _test == expected, 'url:%s, got:`%s` expected: `%s`' % (test, _test, expected)
     def test_fix_repo_id_name(self, test, expected):
         repo = Repository.get_by_repo_name(HG_REPO)
         test = test.replace('ID', str(repo.repo_id))
         expected = expected.replace('NAME', repo.repo_name).replace('ID', str(repo.repo_id))
         from kallithea.lib.utils import fix_repo_id_name
         replaced = fix_repo_id_name(test)
         assert replaced == expected, 'url:%s, got:`%s` expected: `%s`' % (test, replaced, expected)
     @parametrize('canonical,test,expected', [
         ('http://www.example.org/', '/abc/xyz', 'http://www.example.org/abc/xyz'),

0 comments (0 inline, 0 general)