# HG changeset patch # User domruf # Date 2017-03-23 23:49:19 # Node ID 865c1f65244c2044c13e1172974177f420a9ecb4 # Parent cc1b5e0e01e80e71e44b916ac512f41f92b0ff1b 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. diff --git a/kallithea/model/db.py b/kallithea/model/db.py --- a/kallithea/model/db.py +++ b/kallithea/model/db.py @@ -1121,8 +1121,13 @@ class Repository(Base, BaseDbModel): 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)) diff --git a/kallithea/model/validators.py b/kallithea/model/validators.py --- a/kallithea/model/validators.py +++ b/kallithea/model/validators.py @@ -21,6 +21,7 @@ import formencode 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 @@ def ValidRepoGroup(edit=False, old_data= # 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 @@ def ValidRepoGroup(edit=False, old_data= # 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 @@ def ValidRepoName(edit=False, old_data=N 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) ) diff --git a/kallithea/tests/functional/test_admin_repo_groups.py b/kallithea/tests/functional/test_admin_repo_groups.py --- a/kallithea/tests/functional/test_admin_repo_groups.py +++ b/kallithea/tests/functional/test_admin_repo_groups.py @@ -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() diff --git a/kallithea/tests/functional/test_admin_repos.py b/kallithea/tests/functional/test_admin_repos.py --- a/kallithea/tests/functional/test_admin_repos.py +++ b/kallithea/tests/functional/test_admin_repos.py @@ -5,6 +5,7 @@ import mock 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.model.user import UserMod 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 @@ class _BaseTestCase(TestController): 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() diff --git a/kallithea/tests/functional/test_search_indexing.py b/kallithea/tests/functional/test_search_indexing.py --- a/kallithea/tests/functional/test_search_indexing.py +++ b/kallithea/tests/functional/test_search_indexing.py @@ -41,7 +41,6 @@ repos = [ (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 @@ class TestSearchControllerIndexing(TestC 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