kallithea Changeset - 865c1f65244c

Changeset - 865c1f65244c

Parent rev.

Child rev.

[Not reviewed]

default

0 5 0

domruf - 9 years ago 2017-03-23 23:49:19
dominikruf@gmail.com

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.

5 files changed with 66 insertions and 50 deletions:

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

0 comments (0 inline, 0 general)

kallithea/model/db.py

➞

Show inline comments

@@ @@ -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))

kallithea/model/validators.py

➞

Show inline comments

@@ @@ -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)
+                        )

kallithea/tests/functional/test_admin_repo_groups.py

➞

Show inline comments

 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()

kallithea/tests/functional/test_admin_repos.py

➞

Show inline comments

@@ @@ -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()

kallithea/tests/functional/test_search_indexing.py

➞

Show inline comments

@@ @@ -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

0 comments (0 inline, 0 general)