Mercurial > kallithea
changeset 6619:865c1f65244c
repositories: make sure repositories not only differ in casing
Repositories only differing in case cause problems:
* it can't be stored on case insensitive filesystems (Windows and MacOS)
* some databases can't easily handle case sensitive queries
* users will most certainly be confused by names that only differ in case
We will keep trying to be case sensitive on systems that can ... but on some
systems wrong casings might work. We don't care.
The validators are changed to prevent mixed case repo and repo group names.
Repository sensitivity tests are removed, and insensitivity tests are added
instead.
| author | domruf <dominikruf@gmail.com> |
|---|---|
| date | Thu, 23 Mar 2017 23:49:19 +0100 |
| parents | cc1b5e0e01e8 |
| children | 6cc40e545e9a |
| files | kallithea/model/db.py kallithea/model/validators.py kallithea/tests/functional/test_admin_repo_groups.py kallithea/tests/functional/test_admin_repos.py kallithea/tests/functional/test_search_indexing.py |
| diffstat | 5 files changed, 66 insertions(+), 50 deletions(-) [+] |
line wrap: on
line diff
--- a/kallithea/model/db.py Mon May 08 05:25:41 2017 +0200 +++ b/kallithea/model/db.py Thu Mar 23 23:49:19 2017 +0100 @@ -1121,8 +1121,13 @@ return super(Repository, cls).guess_instance(value, Repository.get_by_repo_name) @classmethod - def get_by_repo_name(cls, repo_name): - q = Session().query(cls).filter(cls.repo_name == repo_name) + def get_by_repo_name(cls, repo_name, case_insensitive=False): + """Get the repo, defaulting to database case sensitivity. + case_insensitive will be slower and should only be specified if necessary.""" + if case_insensitive: + q = Session().query(cls).filter(func.lower(cls.repo_name) == func.lower(repo_name)) + else: + q = Session().query(cls).filter(cls.repo_name == repo_name) q = q.options(joinedload(Repository.fork)) \ .options(joinedload(Repository.owner)) \ .options(joinedload(Repository.group))
--- a/kallithea/model/validators.py Mon May 08 05:25:41 2017 +0200 +++ b/kallithea/model/validators.py Thu Mar 23 23:49:19 2017 +0100 @@ -21,6 +21,7 @@ import logging from collections import defaultdict from tg.i18n import ugettext as _ +from sqlalchemy import func from webhelpers.pylonslib.secure_form import authentication_token import sqlalchemy @@ -203,10 +204,9 @@ # check group gr = RepoGroup.query() \ - .filter(RepoGroup.group_name == slug) \ + .filter(func.lower(RepoGroup.group_name) == func.lower(slug)) \ .filter(RepoGroup.parent_group_id == parent_group_id) \ .scalar() - if gr is not None: msg = self.message('group_exists', state, group_name=slug) raise formencode.Invalid(msg, value, state, @@ -215,9 +215,8 @@ # check for same repo repo = Repository.query() \ - .filter(Repository.repo_name == slug) \ + .filter(func.lower(Repository.repo_name) == func.lower(slug)) \ .scalar() - if repo is not None: msg = self.message('repo_exists', state, group_name=slug) raise formencode.Invalid(msg, value, state, @@ -369,24 +368,24 @@ rename = old_data.get('repo_name') != repo_name_full create = not edit if rename or create: - + repo = Repository.get_by_repo_name(repo_name_full, case_insensitive=True) + repo_group = RepoGroup.get_by_group_name(repo_name_full, case_insensitive=True) if group_path != '': - if Repository.get_by_repo_name(repo_name_full): + if repo is not None: msg = self.message('repository_in_group_exists', state, - repo=repo_name, group=group_name) + repo=repo.repo_name, group=group_name) raise formencode.Invalid(msg, value, state, error_dict=dict(repo_name=msg) ) - elif RepoGroup.get_by_group_name(repo_name_full): + elif repo_group is not None: msg = self.message('same_group_exists', state, repo=repo_name) raise formencode.Invalid(msg, value, state, error_dict=dict(repo_name=msg) ) - - elif Repository.get_by_repo_name(repo_name_full): + elif repo is not None: msg = self.message('repository_exists', state, - repo=repo_name) + repo=repo.repo_name) raise formencode.Invalid(msg, value, state, error_dict=dict(repo_name=msg) )
--- a/kallithea/tests/functional/test_admin_repo_groups.py Mon May 08 05:25:41 2017 +0200 +++ b/kallithea/tests/functional/test_admin_repo_groups.py Thu Mar 23 23:49:19 2017 +0100 @@ -1,4 +1,26 @@ -from kallithea.tests.base import * +from kallithea.model.db import Repository +from kallithea.model.meta import Session +from kallithea.model.repo_group import RepoGroupModel +from kallithea.tests.base import TestController, url +from kallithea.tests.fixture import Fixture + + +fixture = Fixture() class TestRepoGroupsController(TestController): - pass + + def test_case_insensitivity(self): + self.log_user() + group_name = 'newgroup' + response = self.app.post(url('repos_groups'), + fixture._get_repo_group_create_params(group_name=group_name, + _authentication_token=self.authentication_token())) + # try to create repo group with swapped case + swapped_group_name = group_name.swapcase() + response = self.app.post(url('repos_groups'), + fixture._get_repo_group_create_params(group_name=swapped_group_name, + _authentication_token=self.authentication_token())) + response.mustcontain('already exists') + + RepoGroupModel().delete(group_name) + Session().commit()
--- a/kallithea/tests/functional/test_admin_repos.py Mon May 08 05:25:41 2017 +0200 +++ b/kallithea/tests/functional/test_admin_repos.py Thu Mar 23 23:49:19 2017 +0100 @@ -5,6 +5,7 @@ import urllib import pytest +from sqlalchemy import func from kallithea.lib import vcs from kallithea.lib.utils2 import safe_str, safe_unicode @@ -14,7 +15,7 @@ from kallithea.tests.base import * from kallithea.model.repo_group import RepoGroupModel from kallithea.model.repo import RepoModel -from kallithea.model.meta import Session +from kallithea.model.meta import Session, Base from kallithea.tests.fixture import Fixture, error_function fixture = Fixture() @@ -81,6 +82,29 @@ RepoModel().delete(repo_name) Session().commit() + def test_case_insensitivity(self): + self.log_user() + repo_name = self.NEW_REPO + description = u'description for newly created repo' + response = self.app.post(url('repos'), + fixture._get_repo_create_params(repo_private=False, + repo_name=repo_name, + repo_type=self.REPO_TYPE, + repo_description=description, + _authentication_token=self.authentication_token())) + # try to create repo with swapped case + swapped_repo_name = repo_name.swapcase() + response = self.app.post(url('repos'), + fixture._get_repo_create_params(repo_private=False, + repo_name=swapped_repo_name, + repo_type=self.REPO_TYPE, + repo_description=description, + _authentication_token=self.authentication_token())) + response.mustcontain('already exists') + + RepoModel().delete(repo_name) + Session().commit() + def test_create_in_group(self): self.log_user()
--- a/kallithea/tests/functional/test_search_indexing.py Mon May 08 05:25:41 2017 +0200 +++ b/kallithea/tests/functional/test_search_indexing.py Thu Mar 23 23:49:19 2017 +0100 @@ -41,7 +41,6 @@ (u'group/indexing_test', u'indexing_test', u'group'), (u'this-is-it', u'indexing_test', None), (u'indexing_test-foo', u'indexing_test', None), - (u'indexing_test-FOO', u'indexing_test', None), (u'stopword_test', init_stopword_test, None), ] @@ -145,39 +144,6 @@ response.mustcontain('>%d results' % hit) @parametrize('searchtype,query,hit', [ - ('content', 'this_should_be_unique_content', 1), - ('commit', 'this_should_be_unique_commit_log', 1), - ('path', 'this_should_be_unique_filename.txt', 1), - ]) - def test_repository_case_sensitivity(self, searchtype, query, hit): - self.log_user() - - lname = u'indexing_test-foo' - uname = u'indexing_test-FOO' - - # (1) "repository:REPONAME" condition should match against - # repositories case-insensitively - q = 'repository:%s %s' % (lname, query) - response = self.app.get(url(controller='search', action='index'), - {'q': q, 'type': searchtype}) - - response.mustcontain('>%d results' % (hit * 2)) - - # (2) on the other hand, searching under the specific - # repository should return results only for that repository, - # even if specified name matches against another repository - # case-insensitively. - response = self.app.get(url(controller='search', action='index', - repo_name=uname), - {'q': query, 'type': searchtype}) - - response.mustcontain('>%d results' % hit) - - # confirm that there is no matching against lower name repository - assert uname in response - assert lname not in response - - @parametrize('searchtype,query,hit', [ ('content', 'path:this/is/it def test', 1), ('commit', 'added:this/is/it bother to ask where', 1), # this condition matches against files below, because