# HG changeset patch # User Mads Kiilerich # Date 2019-01-16 02:32:35 # Node ID 99edd97366e38500aef2e91875eeef5f3a9df589 # Parent b8f77a47d48541cf67dd91b710de7b988fae5f31 locking: drop the pull-to-lock / push-to-unlock functionality The feature is not worth the maintenance cost. The locking is too coarse and unflexible with insufficient UI and UX. The implementation is also quite invasive in tricky areas of the code, and thus high maintenance. Dropping this will enable other cleanup ... or at least make it easier. diff --git a/development.ini b/development.ini --- a/development.ini +++ b/development.ini @@ -219,10 +219,6 @@ issue_sub = ## handling that. Set this variable to 403 to return HTTPForbidden auth_ret_code = -## locking return code. When repository is locked return this HTTP code. 2XX -## codes don't break the transactions while 4XX codes do -lock_ret_code = 423 - ## allows to change the repository location in settings page allow_repo_location_change = True diff --git a/docs/api/api.rst b/docs/api/api.rst --- a/docs/api/api.rst +++ b/docs/api/api.rst @@ -162,37 +162,6 @@ OUTPUT:: result : "Caches of repository ``" error : null -lock -^^^^ - -Set the locking state on the given repository by the given user. -If the param ``userid`` is skipped, it is set to the ID of the user who is calling this method. -If param ``locked`` is skipped, the current lock state of the repository is returned. -This command can only be executed using the api_key of a user with admin rights, or that of a regular user with admin or write access to the repository. - -INPUT:: - - id : - api_key : "" - method : "lock" - args : { - "repoid" : "" - "userid" : "", - "locked" : "" - } - -OUTPUT:: - - id : - result : { - "repo": "", - "locked": "", - "locked_since": "", - "locked_by": "", - "msg": "User `` set lock state for repo `` to ``" - } - error : null - get_ip ^^^^^^ @@ -601,7 +570,6 @@ OUTPUT:: "repo_type" : "", "clone_uri" : "", "enable_downloads": "", - "enable_locking": "", "enable_statistics": "", "private": "", "created_on" : "", @@ -755,7 +723,6 @@ OUTPUT:: "owner": "", "fork_of": "", "enable_downloads": "", - "enable_locking": "", "enable_statistics": "", }, … @@ -820,7 +787,6 @@ INPUT:: "clone_uri" : " = Optional(None)", "landing_rev" : " = Optional('tip')", "enable_downloads": " = Optional(False)", - "enable_locking": " = Optional(False)", "enable_statistics": " = Optional(False)", } @@ -841,7 +807,6 @@ OUTPUT:: "owner": "", "fork_of": "", "enable_downloads": "", - "enable_locking": "", "enable_statistics": "", }, } @@ -870,7 +835,6 @@ INPUT:: "clone_uri" : " = Optional(None)", "landing_rev" : " = Optional('tip')", "enable_downloads": " = Optional(False)", - "enable_locking": " = Optional(False)", "enable_statistics": " = Optional(False)", } @@ -891,7 +855,6 @@ OUTPUT:: "owner": "", "fork_of": "", "enable_downloads": "", - "enable_locking": "", "enable_statistics": "", "last_changeset": { "author": "", @@ -901,8 +864,6 @@ OUTPUT:: "revision": "", "short_id": "" } - "locked_by": "", - "locked_date": "", }, } error: null diff --git a/docs/index.rst b/docs/index.rst --- a/docs/index.rst +++ b/docs/index.rst @@ -63,7 +63,6 @@ User guide usage/general usage/vcs_notes - usage/locking usage/statistics api/api diff --git a/docs/usage/locking.rst b/docs/usage/locking.rst deleted file mode 100644 --- a/docs/usage/locking.rst +++ /dev/null @@ -1,28 +0,0 @@ -.. _locking: - -================== -Repository locking -================== - -Kallithea has a *repository locking* feature, disabled by default. When -enabled, every initial clone and every pull gives users (with write permission) -the exclusive right to do a push. - -When repository locking is enabled, repositories get a ``locked`` flag. -The hg/git commands ``hg/git clone``, ``hg/git pull``, -and ``hg/git push`` influence this state: - -- A ``clone`` or ``pull`` action locks the target repository - if the user has write/admin permissions on this repository. - -- Kallithea will remember the user who locked the repository so only this - specific user can unlock the repo by performing a ``push`` - command. - -- Every other command on a locked repository from this user and every command - from any other user will result in an HTTP return code 423 (Locked). - Additionally, the HTTP error will mention the user that locked the repository - (e.g., “repository locked by user ”). - -Each repository can be manually unlocked by an administrator from the -repository settings menu. diff --git a/kallithea/alembic/versions/ad357ccd9521_drop_locking.py b/kallithea/alembic/versions/ad357ccd9521_drop_locking.py new file mode 100644 --- /dev/null +++ b/kallithea/alembic/versions/ad357ccd9521_drop_locking.py @@ -0,0 +1,59 @@ +# 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 . + +"""Drop locking + +Revision ID: ad357ccd9521 +Revises: a020f7044fd6 +Create Date: 2019-01-08 + +""" + +# The following opaque hexadecimal identifiers ("revisions") are used +# by Alembic to track this migration script and its relations to others. +revision = 'ad357ccd9521' +down_revision = 'a020f7044fd6' +branch_labels = None +depends_on = None + +from alembic import op +import sqlalchemy as sa +from kallithea.model.db import Ui +from sqlalchemy import Table, MetaData + +meta = MetaData() + + +def upgrade(): + with op.batch_alter_table('groups', schema=None) as batch_op: + batch_op.drop_column('enable_locking') + + with op.batch_alter_table('repositories', schema=None) as batch_op: + batch_op.drop_column('locked') + batch_op.drop_column('enable_locking') + + meta.bind = op.get_bind() + ui = Table(Ui.__tablename__, meta, autoload=True) + ui.delete().where(ui.c.ui_key == 'prechangegroup.push_lock_handling').execute() + ui.delete().where(ui.c.ui_key == 'preoutgoing.pull_lock_handling').execute() + + +def downgrade(): + with op.batch_alter_table('repositories', schema=None) as batch_op: + batch_op.add_column(sa.Column('enable_locking', sa.BOOLEAN(), nullable=False, default=False)) + batch_op.add_column(sa.Column('locked', sa.VARCHAR(length=255), nullable=True, default=False)) + + with op.batch_alter_table('groups', schema=None) as batch_op: + batch_op.add_column(sa.Column('enable_locking', sa.BOOLEAN(), nullable=False, default=False)) + + # Note: not restoring hooks diff --git a/kallithea/config/routing.py b/kallithea/config/routing.py --- a/kallithea/config/routing.py +++ b/kallithea/config/routing.py @@ -533,13 +533,6 @@ def make_map(config): controller='admin/repos', action="edit_advanced", conditions=dict(method=["GET"], function=check_repo)) - rmap.connect("edit_repo_advanced_locking", "/{repo_name:.*?}/settings/advanced/locking", - controller='admin/repos', action="edit_advanced_locking", - conditions=dict(method=["POST"], function=check_repo)) - rmap.connect('toggle_locking', "/{repo_name:.*?}/settings/advanced/locking_toggle", - controller='admin/repos', action="toggle_locking", - conditions=dict(method=["GET"], function=check_repo)) - rmap.connect("edit_repo_advanced_journal", "/{repo_name:.*?}/settings/advanced/journal", controller='admin/repos', action="edit_advanced_journal", conditions=dict(method=["POST"], function=check_repo)) diff --git a/kallithea/controllers/admin/repos.py b/kallithea/controllers/admin/repos.py --- a/kallithea/controllers/admin/repos.py +++ b/kallithea/controllers/admin/repos.py @@ -478,46 +478,6 @@ class ReposController(BaseRepoController raise HTTPFound(location=url('edit_repo_advanced', repo_name=repo_name)) @HasRepoPermissionLevelDecorator('admin') - def edit_advanced_locking(self, repo_name): - """ - Unlock repository when it is locked ! - - :param repo_name: - """ - try: - repo = Repository.get_by_repo_name(repo_name) - if request.POST.get('set_lock'): - Repository.lock(repo, request.authuser.user_id) - h.flash(_('Repository has been locked'), category='success') - elif request.POST.get('set_unlock'): - Repository.unlock(repo) - h.flash(_('Repository has been unlocked'), category='success') - except Exception as e: - log.error(traceback.format_exc()) - h.flash(_('An error occurred during unlocking'), - category='error') - raise HTTPFound(location=url('edit_repo_advanced', repo_name=repo_name)) - - @HasRepoPermissionLevelDecorator('write') - def toggle_locking(self, repo_name): - try: - repo = Repository.get_by_repo_name(repo_name) - - if repo.enable_locking: - if repo.locked[0]: - Repository.unlock(repo) - h.flash(_('Repository has been unlocked'), category='success') - else: - Repository.lock(repo, request.authuser.user_id) - h.flash(_('Repository has been locked'), category='success') - - except Exception as e: - log.error(traceback.format_exc()) - h.flash(_('An error occurred during unlocking'), - category='error') - raise HTTPFound(location=url('summary_home', repo_name=repo_name)) - - @HasRepoPermissionLevelDecorator('admin') def edit_caches(self, repo_name): c.repo_info = self._load_repo() c.active = 'caches' diff --git a/kallithea/controllers/api/api.py b/kallithea/controllers/api/api.py --- a/kallithea/controllers/api/api.py +++ b/kallithea/controllers/api/api.py @@ -303,168 +303,6 @@ class ApiController(JSONRPCController): 'Error occurred during cache invalidation action' ) - # permission check inside - def lock(self, repoid, locked=Optional(None), - userid=Optional(OAttr('apiuser'))): - """ - Set locking state on given repository by given user. If userid param - is skipped, then it is set to id of user who is calling this method. - If locked param is skipped then function shows current lock state of - given repo. This command can be executed only using api_key belonging - to user with admin rights or regular user that have admin or write - access to repository. - - :param repoid: repository name or repository id - :type repoid: str or int - :param locked: lock state to be set - :type locked: Optional(bool) - :param userid: set lock as user - :type userid: Optional(str or int) - - OUTPUT:: - - id : - result : { - 'repo': '', - 'locked': , - 'locked_since': , - 'locked_by': , - 'lock_state_changed': , - 'msg': 'Repo `` locked by `` on .' - or - 'msg': 'Repo `` not locked.' - or - 'msg': 'User `` set lock state for repo `` to ``' - } - error : null - - ERROR OUTPUT:: - - id : - result : null - error : { - 'Error occurred locking repository `` - } - - """ - repo = get_repo_or_error(repoid) - if HasPermissionAny('hg.admin')(): - pass - elif HasRepoPermissionLevel('write')(repo.repo_name): - # make sure normal user does not pass someone else userid, - # he is not allowed to do that - if not isinstance(userid, Optional) and userid != request.authuser.user_id: - raise JSONRPCError( - 'userid is not the same as your user' - ) - else: - raise JSONRPCError('repository `%s` does not exist' % (repoid,)) - - if isinstance(userid, Optional): - userid = request.authuser.user_id - - user = get_user_or_error(userid) - - if isinstance(locked, Optional): - lockobj = Repository.getlock(repo) - - if lockobj[0] is None: - _d = { - 'repo': repo.repo_name, - 'locked': False, - 'locked_since': None, - 'locked_by': None, - 'lock_state_changed': False, - 'msg': 'Repo `%s` not locked.' % repo.repo_name - } - return _d - else: - userid, time_ = lockobj - lock_user = get_user_or_error(userid) - _d = { - 'repo': repo.repo_name, - 'locked': True, - 'locked_since': time_, - 'locked_by': lock_user.username, - 'lock_state_changed': False, - 'msg': ('Repo `%s` locked by `%s` on `%s`.' - % (repo.repo_name, lock_user.username, - json.dumps(time_to_datetime(time_)))) - } - return _d - - # force locked state through a flag - else: - locked = str2bool(locked) - try: - if locked: - lock_time = time.time() - Repository.lock(repo, user.user_id, lock_time) - else: - lock_time = None - Repository.unlock(repo) - _d = { - 'repo': repo.repo_name, - 'locked': locked, - 'locked_since': lock_time, - 'locked_by': user.username, - 'lock_state_changed': True, - 'msg': ('User `%s` set lock state for repo `%s` to `%s`' - % (user.username, repo.repo_name, locked)) - } - return _d - except Exception: - log.error(traceback.format_exc()) - raise JSONRPCError( - 'Error occurred locking repository `%s`' % repo.repo_name - ) - - def get_locks(self, userid=Optional(OAttr('apiuser'))): - """ - Get all repositories with locks for given userid, if - this command is run by non-admin account userid is set to user - who is calling this method, thus returning locks for himself. - - :param userid: User to get locks for - :type userid: Optional(str or int) - - OUTPUT:: - - id : - result : { - [repo_object, repo_object,...] - } - error : null - """ - - if not HasPermissionAny('hg.admin')(): - # make sure normal user does not pass someone else userid, - # he is not allowed to do that - if not isinstance(userid, Optional) and userid != request.authuser.user_id: - raise JSONRPCError( - 'userid is not the same as your user' - ) - - ret = [] - if isinstance(userid, Optional): - user = None - else: - user = get_user_or_error(userid) - - # show all locks - for r in Repository.query(): - userid, time_ = r.locked - if time_: - _api_data = r.get_api_data() - # if we use userfilter just show the locks for this user - if user is not None: - if safe_int(userid) == user.user_id: - ret.append(_api_data) - else: - ret.append(_api_data) - - return ret - @HasPermissionAnyDecorator('hg.admin') def get_ip(self, userid=Optional(OAttr('apiuser'))): """ @@ -1155,7 +993,6 @@ class ApiController(JSONRPCController): "repo_type" : "", "clone_uri" : "", "enable_downloads": "", - "enable_locking": "", "enable_statistics": "", "private": "", "created_on" : "", @@ -1266,7 +1103,6 @@ class ApiController(JSONRPCController): "owner": "", "fork_of": "", "enable_downloads": "", - "enable_locking": "", "enable_statistics": "", }, … @@ -1346,7 +1182,6 @@ class ApiController(JSONRPCController): private=Optional(False), clone_uri=Optional(None), landing_rev=Optional('rev:tip'), enable_statistics=Optional(False), - enable_locking=Optional(False), enable_downloads=Optional(False), copy_permissions=Optional(False)): """ @@ -1371,8 +1206,6 @@ class ApiController(JSONRPCController): :type clone_uri: str :param landing_rev: : :type landing_rev: str - :param enable_locking: - :type enable_locking: bool :param enable_downloads: :type enable_downloads: bool :param enable_statistics: @@ -1421,8 +1254,6 @@ class ApiController(JSONRPCController): repo_type = defs.get('repo_type') if isinstance(enable_statistics, Optional): enable_statistics = defs.get('repo_enable_statistics') - if isinstance(enable_locking, Optional): - enable_locking = defs.get('repo_enable_locking') if isinstance(enable_downloads, Optional): enable_downloads = defs.get('repo_enable_downloads') @@ -1450,7 +1281,6 @@ class ApiController(JSONRPCController): repo_group=repo_group, repo_landing_rev=landing_rev, enable_statistics=enable_statistics, - enable_locking=enable_locking, enable_downloads=enable_downloads, repo_copy_permissions=copy_permissions, ) @@ -1476,7 +1306,6 @@ class ApiController(JSONRPCController): description=Optional(''), private=Optional(False), clone_uri=Optional(None), landing_rev=Optional('rev:tip'), enable_statistics=Optional(False), - enable_locking=Optional(False), enable_downloads=Optional(False)): """ @@ -1492,7 +1321,6 @@ class ApiController(JSONRPCController): :param clone_uri: :param landing_rev: :param enable_statistics: - :param enable_locking: :param enable_downloads: """ repo = get_repo_or_error(repoid) @@ -1525,7 +1353,6 @@ class ApiController(JSONRPCController): store_update(updates, clone_uri, 'clone_uri') store_update(updates, landing_rev, 'repo_landing_rev') store_update(updates, enable_statistics, 'repo_enable_statistics') - store_update(updates, enable_locking, 'repo_enable_locking') store_update(updates, enable_downloads, 'repo_enable_downloads') RepoModel().update(repo, **updates) @@ -2020,7 +1847,7 @@ class ApiController(JSONRPCController): def update_repo_group(self, repogroupid, group_name=Optional(''), description=Optional(''), owner=Optional(OAttr('apiuser')), - parent=Optional(None), enable_locking=Optional(False)): + parent=Optional(None)): repo_group = get_repo_group_or_error(repogroupid) updates = {} @@ -2029,7 +1856,6 @@ class ApiController(JSONRPCController): store_update(updates, description, 'group_description') store_update(updates, owner, 'owner') store_update(updates, parent, 'parent_group') - store_update(updates, enable_locking, 'enable_locking') repo_group = RepoGroupModel().update(repo_group, updates) Session().commit() return dict( diff --git a/kallithea/controllers/files.py b/kallithea/controllers/files.py --- a/kallithea/controllers/files.py +++ b/kallithea/controllers/files.py @@ -295,14 +295,6 @@ class FilesController(BaseRepoController @HasRepoPermissionLevelDecorator('write') def delete(self, repo_name, revision, f_path): repo = c.db_repo - if repo.enable_locking and repo.locked[0]: - h.flash(_('This repository has been locked by %s on %s') - % (h.person_by_id(repo.locked[0]), - h.fmt_date(h.time_to_datetime(repo.locked[1]))), - 'warning') - raise HTTPFound(location=h.url('files_home', - repo_name=repo_name, revision='tip')) - # check if revision is a branch identifier- basically we cannot # create multiple heads via file editing _branches = repo.scm_instance.branches @@ -355,14 +347,6 @@ class FilesController(BaseRepoController @HasRepoPermissionLevelDecorator('write') def edit(self, repo_name, revision, f_path): repo = c.db_repo - if repo.enable_locking and repo.locked[0]: - h.flash(_('This repository has been locked by %s on %s') - % (h.person_by_id(repo.locked[0]), - h.fmt_date(h.time_to_datetime(repo.locked[1]))), - 'warning') - raise HTTPFound(location=h.url('files_home', - repo_name=repo_name, revision='tip')) - # check if revision is a branch identifier- basically we cannot # create multiple heads via file editing _branches = repo.scm_instance.branches @@ -422,14 +406,6 @@ class FilesController(BaseRepoController def add(self, repo_name, revision, f_path): repo = c.db_repo - if repo.enable_locking and repo.locked[0]: - h.flash(_('This repository has been locked by %s on %s') - % (h.person_by_id(repo.locked[0]), - h.fmt_date(h.time_to_datetime(repo.locked[1]))), - 'warning') - raise HTTPFound(location=h.url('files_home', - repo_name=repo_name, revision='tip')) - r_post = request.POST c.cs = self.__get_cs(revision, silent_empty=True) if c.cs is None: diff --git a/kallithea/lib/base.py b/kallithea/lib/base.py --- a/kallithea/lib/base.py +++ b/kallithea/lib/base.py @@ -147,42 +147,6 @@ def log_in_user(user, remember, is_exter return auth_user -def check_locking_state(action, repo_name, user): - """ - Checks locking on this repository, if locking is enabled, and if lock - is present. Returns a tuple of make_lock, locked, locked_by. make_lock - can have 3 states: None (do nothing), True (make lock), and False - (release lock). This value is later propagated to hooks, telling them - what to do. - """ - locked = False # defines that locked error should be thrown to user - make_lock = None - repo = Repository.get_by_repo_name(repo_name) - locked_by = repo.locked - if repo and repo.enable_locking: - if action == 'push': - # Check if repo already is locked !, if it is compare users - user_id, _date = locked_by - if user.user_id == user_id: - log.debug('Got push from user %s, now unlocking', user) - # Unlock if we have push from the user who locked - make_lock = False - else: - # Another used tried to push - deny access with something like 423 Locked! - locked = True - if action == 'pull': - if repo.locked[0] and repo.locked[1]: - locked = True - else: - log.debug('Setting lock on repo %s by %s', repo, user) - make_lock = True - else: - log.debug('Repository %s does not have locking enabled', repo) - log.debug('FINAL locking values make_lock:%s,locked:%s,locked_by:%s', - make_lock, locked, locked_by) - return make_lock, locked, locked_by - - class BasicAuth(paste.auth.basic.AuthBasicAuthenticator): def __init__(self, realm, authfunc, auth_http_code=None): diff --git a/kallithea/lib/celerylib/tasks.py b/kallithea/lib/celerylib/tasks.py --- a/kallithea/lib/celerylib/tasks.py +++ b/kallithea/lib/celerylib/tasks.py @@ -349,7 +349,6 @@ def create_repo(form_data, cur_user): # repo creation defaults, private and repo_type are filled in form defs = Setting.get_default_repo_settings(strip_prefix=True) enable_statistics = defs.get('repo_enable_statistics') - enable_locking = defs.get('repo_enable_locking') enable_downloads = defs.get('repo_enable_downloads') try: @@ -366,7 +365,6 @@ def create_repo(form_data, cur_user): copy_fork_permissions=copy_fork_permissions, copy_group_permissions=copy_group_permissions, enable_statistics=enable_statistics, - enable_locking=enable_locking, enable_downloads=enable_downloads, state=state ) diff --git a/kallithea/lib/db_manage.py b/kallithea/lib/db_manage.py --- a/kallithea/lib/db_manage.py +++ b/kallithea/lib/db_manage.py @@ -240,7 +240,6 @@ class DbManage(object): """Creates default settings""" for k, v, t in [ - ('default_repo_enable_locking', False, 'bool'), ('default_repo_enable_downloads', False, 'bool'), ('default_repo_enable_statistics', False, 'bool'), ('default_repo_private', False, 'bool'), @@ -354,9 +353,7 @@ class DbManage(object): ('hooks', Ui.HOOK_UPDATE, 'hg update >&2', False), ('hooks', Ui.HOOK_REPO_SIZE, 'python:kallithea.lib.hooks.repo_size', True), ('hooks', Ui.HOOK_PUSH_LOG, 'python:kallithea.lib.hooks.log_push_action', True), - ('hooks', Ui.HOOK_PUSH_LOCK, 'python:kallithea.lib.hooks.push_lock_handling', True), ('hooks', Ui.HOOK_PULL_LOG, 'python:kallithea.lib.hooks.log_pull_action', True), - ('hooks', Ui.HOOK_PULL_LOCK, 'python:kallithea.lib.hooks.pull_lock_handling', True), ('extensions', 'largefiles', '', True), ('largefiles', 'usercache', os.path.join(path, '.cache', 'largefiles'), True), ('extensions', 'hgsubversion', '', False), diff --git a/kallithea/lib/exceptions.py b/kallithea/lib/exceptions.py --- a/kallithea/lib/exceptions.py +++ b/kallithea/lib/exceptions.py @@ -71,24 +71,6 @@ class NonRelativePathError(Exception): pass -class HTTPLockedRC(HTTPClientError): - """ - Special Exception For locked Repos in Kallithea, the return code can - be overwritten by _code keyword argument passed into constructors - """ - code = 423 - title = explanation = 'Repository Locked' - - def __init__(self, reponame, username, *args, **kwargs): - from kallithea import CONFIG - from kallithea.lib.utils2 import safe_int - _code = CONFIG.get('lock_ret_code') - self.code = safe_int(_code, self.code) - self.title = self.explanation = safe_str( - 'Repository `%s` locked by user `%s`' % (reponame, username)) - super(HTTPLockedRC, self).__init__(*args, **kwargs) - - class IMCCommitError(Exception): pass diff --git a/kallithea/lib/hooks.py b/kallithea/lib/hooks.py --- a/kallithea/lib/hooks.py +++ b/kallithea/lib/hooks.py @@ -33,7 +33,7 @@ from kallithea.lib.vcs.utils.hgcompat im from kallithea.lib import helpers as h from kallithea.lib.utils import action_logger from kallithea.lib.vcs.backends.base import EmptyChangeset -from kallithea.lib.exceptions import HTTPLockedRC, UserCreationError +from kallithea.lib.exceptions import UserCreationError from kallithea.lib.utils import make_ui, setup_cache_regions from kallithea.lib.utils2 import safe_str, safe_unicode, _extract_extras from kallithea.model.db import Repository, User, Ui @@ -78,44 +78,8 @@ def repo_size(ui, repo, hooktype=None, * ui.status(msg) -def push_lock_handling(ui, repo, **kwargs): - """Pre push function, currently used to ban pushing when repository is locked. - - Called as Mercurial hook prechangegroup.push_lock_handling or from the Git pre-receive hook calling handle_git_pre_receive. - """ - ex = _extract_extras() - - usr = User.get_by_username(ex.username) - if ex.locked_by[0] and usr.user_id != int(ex.locked_by[0]): - locked_by = User.get(ex.locked_by[0]).username - # this exception is interpreted in git/hg middlewares and based - # on that proper return code is server to client - _http_ret = HTTPLockedRC(ex.repository, locked_by) - if str(_http_ret.code).startswith('2'): - # 2xx Codes don't raise exceptions - ui.status(safe_str(_http_ret.title)) - else: - raise _http_ret - - -def pull_lock_handling(ui, repo, **kwargs): - """Called as Mercurial hook preoutgoing.pull_lock_handling or from Kallithea before invoking Git""" - # pre pull function ... - ex = _extract_extras() - if ex.locked_by[0]: - locked_by = User.get(ex.locked_by[0]).username - # this exception is interpreted in git/hg middlewares and based - # on that proper return code is server to client - _http_ret = HTTPLockedRC(ex.repository, locked_by) - if str(_http_ret.code).startswith('2'): - # 2xx Codes don't raise exceptions - ui.status(safe_str(_http_ret.title)) - else: - raise _http_ret - - def log_pull_action(ui, repo, **kwargs): - """Logs user last pull action, and also handle locking + """Logs user last pull action Called as Mercurial hook outgoing.pull_logger or from Kallithea before invoking Git. """ @@ -132,17 +96,6 @@ def log_pull_action(ui, repo, **kwargs): kw.update(ex) callback(**kw) - if ex.make_lock is not None and ex.make_lock: - Repository.lock(Repository.get_by_repo_name(ex.repository), user.user_id) - #msg = 'Made lock on repo `%s`' % repository - #ui.status(msg) - - if ex.locked_by[0]: - locked_by = User.get(ex.locked_by[0]).username - _http_ret = HTTPLockedRC(ex.repository, locked_by) - if str(_http_ret.code).startswith('2'): - # 2xx Codes don't raise exceptions - ui.status(safe_str(_http_ret.title)) return 0 @@ -196,17 +149,6 @@ def log_push_action(ui, repo, **kwargs): kw.update(ex) callback(**kw) - if ex.make_lock is not None and not ex.make_lock: - Repository.unlock(Repository.get_by_repo_name(ex.repository)) - ui.status(safe_str('Released lock on repo `%s`\n' % ex.repository)) - - if ex.locked_by[0]: - locked_by = User.get(ex.locked_by[0]).username - _http_ret = HTTPLockedRC(ex.repository, locked_by) - if str(_http_ret.code).startswith('2'): - # 2xx Codes don't raise exceptions - ui.status(safe_str(_http_ret.title)) - return 0 @@ -406,9 +348,6 @@ def _hook_environment(repo_path): def handle_git_pre_receive(repo_path, git_stdin_lines): """Called from Git pre-receive hook""" - baseui, repo = _hook_environment(repo_path) - scm_repo = repo.scm_instance - push_lock_handling(baseui, scm_repo) return 0 diff --git a/kallithea/lib/middleware/simplegit.py b/kallithea/lib/middleware/simplegit.py --- a/kallithea/lib/middleware/simplegit.py +++ b/kallithea/lib/middleware/simplegit.py @@ -39,10 +39,8 @@ from kallithea.model.db import Ui from kallithea.lib.utils2 import safe_str, safe_unicode, fix_PATH, get_server_url, \ _set_extras -from kallithea.lib.base import BaseVCSController, check_locking_state +from kallithea.lib.base import BaseVCSController from kallithea.lib.utils import make_ui, is_valid_repo -from kallithea.lib.exceptions import HTTPLockedRC -from kallithea.lib.hooks import pull_lock_handling log = logging.getLogger(__name__) @@ -110,8 +108,6 @@ class SimpleGit(BaseVCSController): 'scm': 'git', 'config': CONFIG['__file__'], 'server_url': server_url, - 'make_lock': None, - 'locked_by': [None, None] } #=================================================================== @@ -120,13 +116,6 @@ class SimpleGit(BaseVCSController): repo_path = os.path.join(safe_str(self.basepath), str_repo_name) log.debug('Repository path is %s', repo_path) - if not user.is_default_user: - log.debug('Checking locking on repository') - make_lock, locked, locked_by = check_locking_state(action, repo_name, user) - # store the make_lock for later evaluation in hooks - extras.update({'make_lock': make_lock, - 'locked_by': locked_by}) - fix_PATH() log.debug('HOOKS extras is %s', extras) baseui = make_ui('db') @@ -138,9 +127,6 @@ class SimpleGit(BaseVCSController): action, str_repo_name, safe_str(user.username), ip_addr) app = self.__make_app(repo_name, repo_path, extras) return app(environ, start_response) - except HTTPLockedRC as e: - log.debug('Locked, response %s: %s', e.code, e.title) - return e(environ, start_response) except Exception: log.error(traceback.format_exc()) return HTTPInternalServerError()(environ, start_response) @@ -211,8 +197,5 @@ class SimpleGit(BaseVCSController): _repo = _repo.scm_instance _hooks = dict(baseui.configitems('hooks')) or {} - if action == 'pull': - # stupid git, emulate pre-pull hook ! - pull_lock_handling(ui=baseui, repo=_repo._repo) if action == 'pull' and _hooks.get(Ui.HOOK_PULL_LOG): log_pull_action(ui=baseui, repo=_repo._repo) diff --git a/kallithea/lib/middleware/simplehg.py b/kallithea/lib/middleware/simplehg.py --- a/kallithea/lib/middleware/simplehg.py +++ b/kallithea/lib/middleware/simplehg.py @@ -38,10 +38,9 @@ from webob.exc import HTTPNotFound, HTTP from kallithea.lib.utils2 import safe_str, safe_unicode, fix_PATH, get_server_url, \ _set_extras -from kallithea.lib.base import BaseVCSController, check_locking_state +from kallithea.lib.base import BaseVCSController from kallithea.lib.utils import make_ui, is_valid_repo, ui_sections from kallithea.lib.vcs.utils.hgcompat import RepoError, hgweb_mod -from kallithea.lib.exceptions import HTTPLockedRC log = logging.getLogger(__name__) @@ -134,8 +133,6 @@ class SimpleHg(BaseVCSController): 'scm': 'hg', 'config': CONFIG['__file__'], 'server_url': server_url, - 'make_lock': None, - 'locked_by': [None, None] } #====================================================================== # MERCURIAL REQUEST HANDLING @@ -143,19 +140,6 @@ class SimpleHg(BaseVCSController): repo_path = os.path.join(safe_str(self.basepath), str_repo_name) log.debug('Repository path is %s', repo_path) - # A Mercurial HTTP server will see listkeys operations (bookmarks, - # phases and obsolescence marker) in a different request - we don't - # want to check locking on those - if environ['QUERY_STRING'] == 'cmd=listkeys': - pass - # CHECK LOCKING only if it's not ANONYMOUS USER - elif not user.is_default_user: - log.debug('Checking locking on repository') - make_lock, locked, locked_by = check_locking_state(action, repo_name, user) - # store the make_lock for later evaluation in hooks - extras.update({'make_lock': make_lock, - 'locked_by': locked_by}) - fix_PATH() log.debug('HOOKS extras is %s', extras) baseui = make_ui('db') @@ -171,10 +155,6 @@ class SimpleHg(BaseVCSController): except RepoError as e: if str(e).find('not found') != -1: return HTTPNotFound()(environ, start_response) - except HTTPLockedRC as e: - # Before Mercurial 3.6, lock exceptions were caught here - log.debug('Locked, response %s: %s', e.code, e.title) - return e(environ, start_response) except Exception: log.error(traceback.format_exc()) return HTTPInternalServerError()(environ, start_response) @@ -184,26 +164,7 @@ class SimpleHg(BaseVCSController): Make an wsgi application using hgweb, and inject generated baseui instance, additionally inject some extras into ui object """ - class HgWebWrapper(hgweb_mod.hgweb): - # Work-around for Mercurial 3.6+ causing lock exceptions to be - # thrown late - def _runwsgi(self, *args): - try: - return super(HgWebWrapper, self)._runwsgi(*args) - except HTTPLockedRC as e: - log.debug('Locked, response %s: %s', e.code, e.title) - try: - req, res, repo = args - res.status = e.status - res.headers['Content-Type'] = 'text/plain' - res.setbodybytes('') - return res.sendresponse() - except ValueError: # wsgiresponse was introduced in Mercurial 4.6 (a88d68dc3ee8) - req, repo = args - req.respond(e.status, 'text/plain') - return '' - - return HgWebWrapper(repo_name, name=repo_name, baseui=baseui) + return hgweb_mod.hgweb(repo_name, name=repo_name, baseui=baseui) def __get_repository(self, environ): """ diff --git a/kallithea/lib/paster_commands/template.ini.mako b/kallithea/lib/paster_commands/template.ini.mako --- a/kallithea/lib/paster_commands/template.ini.mako +++ b/kallithea/lib/paster_commands/template.ini.mako @@ -316,10 +316,6 @@ issue_sub = <%text>## handling that. Set this variable to 403 to return HTTPForbidden auth_ret_code = -<%text>## locking return code. When repository is locked return this HTTP code. 2XX -<%text>## codes don't break the transactions while 4XX codes do -lock_ret_code = 423 - <%text>## allows to change the repository location in settings page allow_repo_location_change = True diff --git a/kallithea/lib/utils.py b/kallithea/lib/utils.py --- a/kallithea/lib/utils.py +++ b/kallithea/lib/utils.py @@ -481,7 +481,6 @@ def repo2db_mapper(initial_repo_list, re # creation defaults defs = Setting.get_default_repo_settings(strip_prefix=True) enable_statistics = defs.get('repo_enable_statistics') - enable_locking = defs.get('repo_enable_locking') enable_downloads = defs.get('repo_enable_downloads') private = defs.get('repo_private') @@ -503,7 +502,6 @@ def repo2db_mapper(initial_repo_list, re description=desc, repo_group=getattr(group, 'group_id', None), owner=user, - enable_locking=enable_locking, enable_downloads=enable_downloads, enable_statistics=enable_statistics, private=private, diff --git a/kallithea/lib/utils2.py b/kallithea/lib/utils2.py --- a/kallithea/lib/utils2.py +++ b/kallithea/lib/utils2.py @@ -549,8 +549,7 @@ def _extract_extras(): raise Exception("Environment variable KALLITHEA_EXTRAS not found") try: - for k in ['username', 'repository', 'locked_by', 'scm', 'make_lock', - 'action', 'ip']: + for k in ['username', 'repository', 'scm', 'action', 'ip']: extras[k] except KeyError: raise Exception('Missing key %s in KALLITHEA_EXTRAS %s' % (k, extras)) diff --git a/kallithea/model/db.py b/kallithea/model/db.py --- a/kallithea/model/db.py +++ b/kallithea/model/db.py @@ -352,9 +352,7 @@ class Ui(Base, BaseDbModel): HOOK_UPDATE = 'changegroup.update' HOOK_REPO_SIZE = 'changegroup.repo_size' HOOK_PUSH_LOG = 'changegroup.push_logger' - HOOK_PUSH_LOCK = 'prechangegroup.push_lock_handling' HOOK_PULL_LOG = 'outgoing.pull_logger' - HOOK_PULL_LOCK = 'preoutgoing.pull_lock_handling' ui_id = Column(Integer(), primary_key=True) ui_section = Column(String(255), nullable=False) @@ -380,8 +378,7 @@ class Ui(Base, BaseDbModel): def get_builtin_hooks(cls): q = cls.query() q = q.filter(cls.ui_key.in_([cls.HOOK_UPDATE, cls.HOOK_REPO_SIZE, - cls.HOOK_PUSH_LOG, cls.HOOK_PUSH_LOCK, - cls.HOOK_PULL_LOG, cls.HOOK_PULL_LOCK])) + cls.HOOK_PUSH_LOG, cls.HOOK_PULL_LOG])) q = q.filter(cls.ui_section == 'hooks') return q.all() @@ -389,8 +386,7 @@ class Ui(Base, BaseDbModel): def get_custom_hooks(cls): q = cls.query() q = q.filter(~cls.ui_key.in_([cls.HOOK_UPDATE, cls.HOOK_REPO_SIZE, - cls.HOOK_PUSH_LOG, cls.HOOK_PUSH_LOCK, - cls.HOOK_PULL_LOG, cls.HOOK_PULL_LOCK])) + cls.HOOK_PUSH_LOG, cls.HOOK_PULL_LOG])) q = q.filter(cls.ui_section == 'hooks') return q.all() @@ -994,8 +990,6 @@ class Repository(Base, BaseDbModel): created_on = Column(DateTime(timezone=False), nullable=False, default=datetime.datetime.now) updated_on = Column(DateTime(timezone=False), nullable=False, default=datetime.datetime.now) _landing_revision = Column("landing_revision", String(255), nullable=False) - enable_locking = Column(Boolean(), nullable=False, default=False) - _locked = Column("locked", String(255), nullable=True) # FIXME: not nullable? _changeset_cache = Column("changeset_cache", LargeBinary(), nullable=True) # JSON data # FIXME: not nullable? fork_id = Column(Integer(), ForeignKey('repositories.repo_id'), nullable=True) @@ -1047,21 +1041,6 @@ class Repository(Base, BaseDbModel): self._landing_revision = val @hybrid_property - def locked(self): - # always should return [user_id, timelocked] - if self._locked: - _lock_info = self._locked.split(':') - return int(_lock_info[0]), _lock_info[1] - return [None, None] - - @locked.setter - def locked(self, val): - if val and isinstance(val, (list, tuple)): - self._locked = ':'.join(map(str, val)) - else: - self._locked = None - - @hybrid_property def changeset_cache(self): try: cs_cache = json.loads(self._changeset_cache) # might raise on bad data @@ -1254,13 +1233,8 @@ class Repository(Base, BaseDbModel): owner=repo.owner.username, fork_of=repo.fork.repo_name if repo.fork else None, enable_statistics=repo.enable_statistics, - enable_locking=repo.enable_locking, enable_downloads=repo.enable_downloads, last_changeset=repo.changeset_cache, - locked_by=User.get(self.locked[0]).get_api_data() \ - if self.locked[0] else None, - locked_date=time_to_datetime(self.locked[1]) \ - if self.locked[1] else None ) if with_revision_names: scm_repo = repo.scm_instance_no_cache() @@ -1279,22 +1253,6 @@ class Repository(Base, BaseDbModel): return data - @classmethod - def lock(cls, repo, user_id, lock_time=None): - if lock_time is not None: - lock_time = time.time() - repo.locked = [user_id, lock_time] - Session().commit() - - @classmethod - def unlock(cls, repo): - repo.locked = None - Session().commit() - - @classmethod - def getlock(cls, repo): - return repo.locked - @property def last_db_change(self): return self.updated_on @@ -1524,7 +1482,6 @@ class RepoGroup(Base, BaseDbModel): group_name = Column(Unicode(255), nullable=False, unique=True) # full path parent_group_id = Column('group_parent_id', Integer(), ForeignKey('groups.group_id'), nullable=True) group_description = Column(Unicode(10000), nullable=False) - enable_locking = Column(Boolean(), nullable=False, default=False) owner_id = Column('user_id', Integer(), ForeignKey('users.user_id'), nullable=False) created_on = Column(DateTime(timezone=False), nullable=False, default=datetime.datetime.now) diff --git a/kallithea/model/forms.py b/kallithea/model/forms.py --- a/kallithea/model/forms.py +++ b/kallithea/model/forms.py @@ -182,7 +182,6 @@ def RepoGroupForm(edit=False, old_data=N testValueList=True, if_missing=None, not_empty=True), v.Int(min=-1, not_empty=True)) - enable_locking = v.StringBoolean(if_missing=False) chained_validators = [v.ValidRepoGroup(edit, old_data)] return _RepoGroupForm @@ -264,7 +263,6 @@ def RepoForm(edit=False, old_data=None, repo_enable_statistics = v.StringBoolean(if_missing=False) repo_enable_downloads = v.StringBoolean(if_missing=False) - repo_enable_locking = v.StringBoolean(if_missing=False) if edit: owner = All(v.UnicodeString(not_empty=True), v.ValidRepoUser()) @@ -447,7 +445,6 @@ def DefaultsForm(edit=False, old_data=No default_repo_private = v.StringBoolean(if_missing=False) default_repo_enable_statistics = v.StringBoolean(if_missing=False) default_repo_enable_downloads = v.StringBoolean(if_missing=False) - default_repo_enable_locking = v.StringBoolean(if_missing=False) return _DefaultsForm diff --git a/kallithea/model/repo.py b/kallithea/model/repo.py --- a/kallithea/model/repo.py +++ b/kallithea/model/repo.py @@ -236,7 +236,7 @@ class RepoModel(object): defaults['repo_group'] = repo_info.group_id for strip, k in [(0, 'repo_type'), (1, 'repo_enable_downloads'), - (1, 'repo_description'), (1, 'repo_enable_locking'), + (1, 'repo_description'), (1, 'repo_landing_rev'), (0, 'clone_uri'), (1, 'repo_private'), (1, 'repo_enable_statistics')]: attr = k @@ -284,7 +284,6 @@ class RepoModel(object): log.debug('Updating repo %s with params:%s', cur_repo, kwargs) for k in ['repo_enable_downloads', 'repo_description', - 'repo_enable_locking', 'repo_landing_rev', 'repo_private', 'repo_enable_statistics', @@ -332,7 +331,7 @@ class RepoModel(object): private=False, clone_uri=None, repo_group=None, landing_rev='rev:tip', fork_of=None, copy_fork_permissions=False, enable_statistics=False, - enable_locking=False, enable_downloads=False, + enable_downloads=False, copy_group_permissions=False, state=Repository.STATE_PENDING): """ Create repository inside database with PENDING state. This should only be @@ -371,12 +370,8 @@ class RepoModel(object): new_repo.landing_rev = landing_rev new_repo.enable_statistics = enable_statistics - new_repo.enable_locking = enable_locking new_repo.enable_downloads = enable_downloads - if repo_group: - new_repo.enable_locking = repo_group.enable_locking - if fork_of: parent_repo = fork_of new_repo.fork = parent_repo diff --git a/kallithea/model/repo_group.py b/kallithea/model/repo_group.py --- a/kallithea/model/repo_group.py +++ b/kallithea/model/repo_group.py @@ -287,8 +287,6 @@ class RepoGroupModel(object): repo_group.group_description = repo_group_args['group_description'] if 'parent_group_id' in repo_group_args: repo_group.parent_group_id = repo_group_args['parent_group_id'] - if 'enable_locking' in repo_group_args: - repo_group.enable_locking = repo_group_args['enable_locking'] if 'parent_group_id' in repo_group_args: assert repo_group_args['parent_group_id'] != u'-1', repo_group_args # RepoGroupForm should have converted to None @@ -302,14 +300,12 @@ class RepoGroupModel(object): Session().add(repo_group) # iterate over all members of this groups and do fixes - # set locking if given # if obj is a repoGroup also fix the name of the group according # to the parent # if obj is a Repo fix it's name # this can be potentially heavy operation for obj in repo_group.recursive_groups_and_repos(): # set the value from it's parent - obj.enable_locking = repo_group.enable_locking if isinstance(obj, RepoGroup): new_name = obj.get_new_name(obj.name) log.debug('Fixing group %s to new name %s' \ diff --git a/kallithea/model/scm.py b/kallithea/model/scm.py --- a/kallithea/model/scm.py +++ b/kallithea/model/scm.py @@ -348,8 +348,6 @@ class ScmModel(object): 'scm': repo_alias, 'config': CONFIG['__file__'], 'server_url': get_server_url(environ), - 'make_lock': None, - 'locked_by': [None, None] } _set_extras(extras) diff --git a/kallithea/model/validators.py b/kallithea/model/validators.py --- a/kallithea/model/validators.py +++ b/kallithea/model/validators.py @@ -625,7 +625,7 @@ def ValidSettings(): # with forms forbidden_params = [ - 'user', 'repo_type', 'repo_enable_locking', + 'user', 'repo_type', 'repo_enable_downloads', 'repo_enable_statistics' ] diff --git a/kallithea/templates/admin/defaults/defaults.html b/kallithea/templates/admin/defaults/defaults.html --- a/kallithea/templates/admin/defaults/defaults.html +++ b/kallithea/templates/admin/defaults/defaults.html @@ -55,14 +55,6 @@
- -
- ${h.checkbox('default_repo_enable_locking',value="True")} - ${_('Enable lock-by-pulling on repository.')} -
-
- -
${h.submit('save',_('Save'),class_="btn btn-default")}
diff --git a/kallithea/templates/admin/repo_groups/repo_group_edit_settings.html b/kallithea/templates/admin/repo_groups/repo_group_edit_settings.html --- a/kallithea/templates/admin/repo_groups/repo_group_edit_settings.html +++ b/kallithea/templates/admin/repo_groups/repo_group_edit_settings.html @@ -23,14 +23,6 @@ ${h.form(url('update_repos_group',group_
- -
- ${h.checkbox('enable_locking',value="True")} - ${_('Enable lock-by-pulling on group. This option will be applied to all other groups and repositories inside')} -
-
- -
${h.submit('save',_('Save'),class_="btn btn-default")} ${h.reset('reset',_('Reset'),class_="btn btn-default")} diff --git a/kallithea/templates/admin/repos/repo_edit_advanced.html b/kallithea/templates/admin/repos/repo_edit_advanced.html --- a/kallithea/templates/admin/repos/repo_edit_advanced.html +++ b/kallithea/templates/admin/repos/repo_edit_advanced.html @@ -39,32 +39,6 @@ ${h.form(url('edit_repo_advanced_journal
${h.end_form()} -

${_('Change Locking')}

-${h.form(url('edit_repo_advanced_locking', repo_name=c.repo_info.repo_name))} -
- %if c.repo_info.locked[0]: - ${h.hidden('set_unlock', '1')} - - ${_('Locked by %s on %s') % (h.person_by_id(c.repo_info.locked[0]),h.fmt_date(h.time_to_datetime(c.repo_info.locked[1])))} - %else: - ${h.hidden('set_lock', '1')} - - ${_('Repository is not locked')} - %endif -
- ${_('Force locking on the repository. Works only when anonymous access is disabled. Triggering a pull locks the repository. The user who is pulling locks the repository; only the user who pulled and locked it can unlock it by doing a push.')} -
-
-${h.end_form()} -

${_('Delete')}

${h.form(url('delete_repo', repo_name=c.repo_name))}
diff --git a/kallithea/templates/admin/repos/repo_edit_settings.html b/kallithea/templates/admin/repos/repo_edit_settings.html --- a/kallithea/templates/admin/repos/repo_edit_settings.html +++ b/kallithea/templates/admin/repos/repo_edit_settings.html @@ -74,13 +74,6 @@ ${h.form(url('update_repo', repo_name=c. ${_('Enable download menu on summary page.')}
-
- -
- ${h.checkbox('repo_enable_locking',value="True")} - ${_('Enable lock-by-pulling on repository.')} -
-
%if c.visual.repository_fields: ## EXTRA FIELDS diff --git a/kallithea/templates/base/base.html b/kallithea/templates/base/base.html --- a/kallithea/templates/base/base.html +++ b/kallithea/templates/base/base.html @@ -159,13 +159,6 @@
  • ${_('Search')}
    • - %if h.HasRepoPermissionLevel('write')(c.repo_name) and c.db_repo.enable_locking: - %if c.db_repo.locked[0]: -
  • ${_('Unlock')}
    • - %else: -
  • ${_('Lock')}
    • - %endif - %endif ## TODO: this check feels wrong, it would be better to have a check for permissions ## also it feels like a job for the controller %if request.authuser.username != 'default': diff --git a/kallithea/templates/summary/summary.html b/kallithea/templates/summary/summary.html --- a/kallithea/templates/summary/summary.html +++ b/kallithea/templates/summary/summary.html @@ -8,15 +8,6 @@ <%def name="breadcrumbs_links()"> ${_('Summary')} - ## locking icon - %if c.db_repo.enable_locking: - %if c.db_repo.locked[0]: - - %else: - - %endif - %endif - ##FORK %if c.db_repo.fork: - ${_('Fork of')} "${c.db_repo.fork.repo_name}" diff --git a/kallithea/tests/api/api_base.py b/kallithea/tests/api/api_base.py --- a/kallithea/tests/api/api_base.py +++ b/kallithea/tests/api/api_base.py @@ -386,205 +386,6 @@ class _BaseTestApi(object): expected = "repository `%s` does not exist" % (self.REPO,) self._compare_error(id_, expected, given=response.body) - def test_api_lock_repo_lock_acquire(self): - try: - id_, params = _build_data(self.apikey, 'lock', - userid=TEST_USER_ADMIN_LOGIN, - repoid=self.REPO, - locked=True) - response = api_call(self, params) - expected = { - 'repo': self.REPO, 'locked': True, - 'locked_since': response.json['result']['locked_since'], - 'locked_by': TEST_USER_ADMIN_LOGIN, - 'lock_state_changed': True, - 'msg': ('User `%s` set lock state for repo `%s` to `%s`' - % (TEST_USER_ADMIN_LOGIN, self.REPO, True)) - } - self._compare_ok(id_, expected, given=response.body) - finally: - # cleanup - Repository.unlock(RepoModel().get_by_repo_name(self.REPO)) - - def test_api_lock_repo_lock_acquire_by_non_admin(self): - repo_name = u'api_delete_me' - fixture.create_repo(repo_name, repo_type=self.REPO_TYPE, - cur_user=self.TEST_USER_LOGIN) - try: - id_, params = _build_data(self.apikey_regular, 'lock', - repoid=repo_name, - locked=True) - response = api_call(self, params) - expected = { - 'repo': repo_name, - 'locked': True, - 'locked_since': response.json['result']['locked_since'], - 'locked_by': self.TEST_USER_LOGIN, - 'lock_state_changed': True, - 'msg': ('User `%s` set lock state for repo `%s` to `%s`' - % (self.TEST_USER_LOGIN, repo_name, True)) - } - self._compare_ok(id_, expected, given=response.body) - finally: - fixture.destroy_repo(repo_name) - - def test_api_lock_repo_lock_acquire_non_admin_with_userid(self): - repo_name = u'api_delete_me' - fixture.create_repo(repo_name, repo_type=self.REPO_TYPE, - cur_user=self.TEST_USER_LOGIN) - try: - id_, params = _build_data(self.apikey_regular, 'lock', - userid=TEST_USER_ADMIN_LOGIN, - repoid=repo_name, - locked=True) - response = api_call(self, params) - expected = 'userid is not the same as your user' - self._compare_error(id_, expected, given=response.body) - finally: - fixture.destroy_repo(repo_name) - - def test_api_lock_repo_lock_acquire_non_admin_not_his_repo(self): - id_, params = _build_data(self.apikey_regular, 'lock', - repoid=self.REPO, - locked=True) - response = api_call(self, params) - expected = 'repository `%s` does not exist' % (self.REPO) - self._compare_error(id_, expected, given=response.body) - - def test_api_lock_repo_lock_release(self): - id_, params = _build_data(self.apikey, 'lock', - userid=TEST_USER_ADMIN_LOGIN, - repoid=self.REPO, - locked=False) - response = api_call(self, params) - expected = { - 'repo': self.REPO, - 'locked': False, - 'locked_since': None, - 'locked_by': TEST_USER_ADMIN_LOGIN, - 'lock_state_changed': True, - 'msg': ('User `%s` set lock state for repo `%s` to `%s`' - % (TEST_USER_ADMIN_LOGIN, self.REPO, False)) - } - self._compare_ok(id_, expected, given=response.body) - - # test_api_lock_repo_lock_optional_not_locked(self): - id_, params = _build_data(self.apikey, 'lock', - repoid=self.REPO) - response = api_call(self, params) - expected = { - 'repo': self.REPO, - 'locked': False, - 'locked_since': None, - 'locked_by': None, - 'lock_state_changed': False, - 'msg': ('Repo `%s` not locked.' % (self.REPO,)) - } - self._compare_ok(id_, expected, given=response.body) - - def test_api_lock_repo_lock_acquire_optional_userid(self): - try: - id_, params = _build_data(self.apikey, 'lock', - repoid=self.REPO, - locked=True) - response = api_call(self, params) - time_ = response.json['result']['locked_since'] - expected = { - 'repo': self.REPO, - 'locked': True, - 'locked_since': time_, - 'locked_by': TEST_USER_ADMIN_LOGIN, - 'lock_state_changed': True, - 'msg': ('User `%s` set lock state for repo `%s` to `%s`' - % (TEST_USER_ADMIN_LOGIN, self.REPO, True)) - } - - self._compare_ok(id_, expected, given=response.body) - - # test_api_lock_repo_lock_optional_locked - id_, params = _build_data(self.apikey, 'lock', - repoid=self.REPO) - response = api_call(self, params) - time_ = response.json['result']['locked_since'] - expected = { - 'repo': self.REPO, - 'locked': True, - 'locked_since': time_, - 'locked_by': TEST_USER_ADMIN_LOGIN, - 'lock_state_changed': False, - 'msg': ('Repo `%s` locked by `%s` on `%s`.' - % (self.REPO, TEST_USER_ADMIN_LOGIN, - json.dumps(time_to_datetime(time_)))) - } - self._compare_ok(id_, expected, given=response.body) - finally: - # cleanup - Repository.unlock(RepoModel().get_by_repo_name(self.REPO)) - - @mock.patch.object(Repository, 'lock', crash) - def test_api_lock_error(self): - id_, params = _build_data(self.apikey, 'lock', - userid=TEST_USER_ADMIN_LOGIN, - repoid=self.REPO, - locked=True) - response = api_call(self, params) - - expected = 'Error occurred locking repository `%s`' % self.REPO - self._compare_error(id_, expected, given=response.body) - - def test_api_get_locks_regular_user(self): - id_, params = _build_data(self.apikey_regular, 'get_locks') - response = api_call(self, params) - expected = [] - self._compare_ok(id_, expected, given=response.body) - - def test_api_get_locks_with_userid_regular_user(self): - id_, params = _build_data(self.apikey_regular, 'get_locks', - userid=TEST_USER_ADMIN_LOGIN) - response = api_call(self, params) - expected = 'userid is not the same as your user' - self._compare_error(id_, expected, given=response.body) - - def test_api_get_locks(self): - id_, params = _build_data(self.apikey, 'get_locks') - response = api_call(self, params) - expected = [] - self._compare_ok(id_, expected, given=response.body) - - def test_api_get_locks_with_one_locked_repo(self): - repo_name = u'api_delete_me' - repo = fixture.create_repo(repo_name, repo_type=self.REPO_TYPE, - cur_user=self.TEST_USER_LOGIN) - Repository.lock(repo, User.get_by_username(self.TEST_USER_LOGIN).user_id) - try: - id_, params = _build_data(self.apikey, 'get_locks') - response = api_call(self, params) - expected = [repo.get_api_data()] - self._compare_ok(id_, expected, given=response.body) - finally: - fixture.destroy_repo(repo_name) - - def test_api_get_locks_with_one_locked_repo_for_specific_user(self): - repo_name = u'api_delete_me' - repo = fixture.create_repo(repo_name, repo_type=self.REPO_TYPE, - cur_user=self.TEST_USER_LOGIN) - Repository.lock(repo, User.get_by_username(self.TEST_USER_LOGIN).user_id) - try: - id_, params = _build_data(self.apikey, 'get_locks', - userid=self.TEST_USER_LOGIN) - response = api_call(self, params) - expected = [repo.get_api_data()] - self._compare_ok(id_, expected, given=response.body) - finally: - fixture.destroy_repo(repo_name) - - def test_api_get_locks_with_userid(self): - id_, params = _build_data(self.apikey, 'get_locks', - userid=TEST_USER_REGULAR_LOGIN) - response = api_call(self, params) - expected = [] - self._compare_ok(id_, expected, given=response.body) - def test_api_create_existing_user(self): id_, params = _build_data(self.apikey, 'create_user', username=TEST_USER_ADMIN_LOGIN, @@ -1259,7 +1060,6 @@ class _BaseTestApi(object): ('clone_uri', {'clone_uri': None}), ('landing_rev', {'landing_rev': 'branch:master'}), ('enable_statistics', {'enable_statistics': True}), - ('enable_locking', {'enable_locking': True}), ('enable_downloads', {'enable_downloads': True}), ('name', {'name': u'new_repo_name'}), ('repo_group', {'group': u'test_group_for_update'}), @@ -1300,7 +1100,6 @@ class _BaseTestApi(object): ('clone_uri', {'clone_uri': None}), ('landing_rev', {'landing_rev': 'branch:master'}), ('enable_statistics', {'enable_statistics': True}), - ('enable_locking', {'enable_locking': True}), ('enable_downloads', {'enable_downloads': True}), ('name', {'name': u'new_repo_name'}), ('repo_group', {'group': u'test_group_for_update'}), diff --git a/kallithea/tests/fixture.py b/kallithea/tests/fixture.py --- a/kallithea/tests/fixture.py +++ b/kallithea/tests/fixture.py @@ -117,7 +117,6 @@ class Fixture(object): parent_group_id=u'-1', perms_updates=[], perms_new=[], - enable_locking=False, recursive=False ) defs.update(custom) diff --git a/kallithea/tests/functional/test_admin_defaults.py b/kallithea/tests/functional/test_admin_defaults.py --- a/kallithea/tests/functional/test_admin_defaults.py +++ b/kallithea/tests/functional/test_admin_defaults.py @@ -10,12 +10,10 @@ class TestDefaultsController(TestControl response.mustcontain('default_repo_private') response.mustcontain('default_repo_enable_statistics') response.mustcontain('default_repo_enable_downloads') - response.mustcontain('default_repo_enable_locking') def test_update_params_true_hg(self): self.log_user() params = { - 'default_repo_enable_locking': True, 'default_repo_enable_downloads': True, 'default_repo_enable_statistics': True, 'default_repo_private': True, @@ -32,7 +30,6 @@ class TestDefaultsController(TestControl def test_update_params_false_git(self): self.log_user() params = { - 'default_repo_enable_locking': False, 'default_repo_enable_downloads': False, 'default_repo_enable_statistics': False, 'default_repo_private': False, diff --git a/kallithea/tests/other/test_vcs_operations.py b/kallithea/tests/other/test_vcs_operations.py --- a/kallithea/tests/other/test_vcs_operations.py +++ b/kallithea/tests/other/test_vcs_operations.py @@ -176,17 +176,6 @@ class TestVCSOperations(TestController): # DISABLE ANONYMOUS ACCESS set_anonymous_access(False) - def setup_method(self, method): - r = Repository.get_by_repo_name(GIT_REPO) - Repository.unlock(r) - r.enable_locking = False - Session().commit() - - r = Repository.get_by_repo_name(HG_REPO) - Repository.unlock(r) - r.enable_locking = False - Session().commit() - @pytest.fixture() def testhook_cleanup(self): yield @@ -434,157 +423,6 @@ class TestVCSOperations(TestController): assert 'not found' in stderr - def test_clone_and_create_lock_hg(self, webserver): - # enable locking - r = Repository.get_by_repo_name(HG_REPO) - r.enable_locking = True - Session().commit() - # clone - clone_url = webserver.repo_url(HG_REPO) - stdout, stderr = Command(TESTS_TMP_PATH).execute('hg clone', clone_url, _get_tmp_dir()) - - # check if lock was made - r = Repository.get_by_repo_name(HG_REPO) - assert r.locked[0] == User.get_by_username(TEST_USER_ADMIN_LOGIN).user_id - - def test_clone_and_create_lock_git(self, webserver): - # enable locking - r = Repository.get_by_repo_name(GIT_REPO) - r.enable_locking = True - Session().commit() - # clone - clone_url = webserver.repo_url(GIT_REPO) - stdout, stderr = Command(TESTS_TMP_PATH).execute('git clone', clone_url, _get_tmp_dir()) - - # check if lock was made - r = Repository.get_by_repo_name(GIT_REPO) - assert r.locked[0] == User.get_by_username(TEST_USER_ADMIN_LOGIN).user_id - - def test_clone_after_repo_was_locked_hg(self, webserver): - # lock repo - r = Repository.get_by_repo_name(HG_REPO) - Repository.lock(r, User.get_by_username(TEST_USER_ADMIN_LOGIN).user_id) - # pull fails since repo is locked - clone_url = webserver.repo_url(HG_REPO) - stdout, stderr = Command(TESTS_TMP_PATH).execute('hg clone', clone_url, _get_tmp_dir(), ignoreReturnCode=True) - msg = ("""abort: HTTP Error 423: Repository `%s` locked by user `%s`""" - % (HG_REPO, TEST_USER_ADMIN_LOGIN)) - assert msg in stderr - - def test_clone_after_repo_was_locked_git(self, webserver): - # lock repo - r = Repository.get_by_repo_name(GIT_REPO) - Repository.lock(r, User.get_by_username(TEST_USER_ADMIN_LOGIN).user_id) - # pull fails since repo is locked - clone_url = webserver.repo_url(GIT_REPO) - stdout, stderr = Command(TESTS_TMP_PATH).execute('git clone', clone_url, _get_tmp_dir(), ignoreReturnCode=True) - msg = ("""The requested URL returned error: 423""") - assert msg in stderr - - def test_push_on_locked_repo_by_other_user_hg(self, webserver): - # clone some temp - dest_dir = _get_tmp_dir() - clone_url = webserver.repo_url(HG_REPO) - stdout, stderr = Command(TESTS_TMP_PATH).execute('hg clone', clone_url, dest_dir) - - # lock repo - r = Repository.get_by_repo_name(HG_REPO) - # let this user actually push ! - RepoModel().grant_user_permission(repo=r, user=TEST_USER_REGULAR_LOGIN, - perm='repository.write') - Session().commit() - Repository.lock(r, User.get_by_username(TEST_USER_ADMIN_LOGIN).user_id) - - # push fails repo is locked by other user ! - stdout, stderr = _add_files_and_push(webserver, 'hg', dest_dir, - username=TEST_USER_REGULAR_LOGIN, - password=TEST_USER_REGULAR_PASS, - ignoreReturnCode=True) - msg = ("""abort: HTTP Error 423: Repository `%s` locked by user `%s`""" - % (HG_REPO, TEST_USER_ADMIN_LOGIN)) - assert msg in stderr - - def test_push_on_locked_repo_by_other_user_git(self, webserver): - # Note: Git hooks must be executable on unix. This test will thus fail - # for example on Linux if /tmp is mounted noexec. - - # clone some temp - dest_dir = _get_tmp_dir() - clone_url = webserver.repo_url(GIT_REPO) - stdout, stderr = Command(TESTS_TMP_PATH).execute('git clone', clone_url, dest_dir) - - # lock repo - r = Repository.get_by_repo_name(GIT_REPO) - # let this user actually push ! - RepoModel().grant_user_permission(repo=r, user=TEST_USER_REGULAR_LOGIN, - perm='repository.write') - Session().commit() - Repository.lock(r, User.get_by_username(TEST_USER_ADMIN_LOGIN).user_id) - - # push fails repo is locked by other user ! - stdout, stderr = _add_files_and_push(webserver, 'git', dest_dir, - username=TEST_USER_REGULAR_LOGIN, - password=TEST_USER_REGULAR_PASS, - ignoreReturnCode=True) - err = 'Repository `%s` locked by user `%s`' % (GIT_REPO, TEST_USER_ADMIN_LOGIN) - assert err in stderr - - # TODO: fix this somehow later on Git, Git is stupid and even if we throw - # back 423 to it, it makes ANOTHER request and we fail there with 405 :/ - - msg = ("""abort: HTTP Error 423: Repository `%s` locked by user `%s`""" - % (GIT_REPO, TEST_USER_ADMIN_LOGIN)) - #msg = "405 Method Not Allowed" - #assert msg in stderr - - def test_push_unlocks_repository_hg(self, webserver, testfork): - # enable locking - r = Repository.get_by_repo_name(testfork['hg']) - r.enable_locking = True - Session().commit() - # clone some temp - dest_dir = _get_tmp_dir() - clone_url = webserver.repo_url(testfork['hg']) - stdout, stderr = Command(TESTS_TMP_PATH).execute('hg clone', clone_url, dest_dir) - - # check for lock repo after clone - r = Repository.get_by_repo_name(testfork['hg']) - uid = User.get_by_username(TEST_USER_ADMIN_LOGIN).user_id - assert r.locked[0] == uid - - # push is ok and repo is now unlocked - stdout, stderr = _add_files_and_push(webserver, 'hg', dest_dir, clone_url=clone_url) - assert str('remote: Released lock on repo `%s`' % testfork['hg']) in stdout - # we need to cleanup the Session Here ! - Session.remove() - r = Repository.get_by_repo_name(testfork['hg']) - assert r.locked == [None, None] - - # TODO: fix me ! somehow during tests hooks don't get called on Git - def test_push_unlocks_repository_git(self, webserver, testfork): - # enable locking - r = Repository.get_by_repo_name(testfork['git']) - r.enable_locking = True - Session().commit() - # clone some temp - dest_dir = _get_tmp_dir() - clone_url = webserver.repo_url(testfork['git']) - stdout, stderr = Command(TESTS_TMP_PATH).execute('git clone', clone_url, dest_dir) - - # check for lock repo after clone - r = Repository.get_by_repo_name(testfork['git']) - assert r.locked[0] == User.get_by_username(TEST_USER_ADMIN_LOGIN).user_id - - # push is ok and repo is now unlocked - stdout, stderr = _add_files_and_push(webserver, 'git', dest_dir, clone_url=clone_url) - _check_proper_git_push(stdout, stderr) - - assert ('remote: Released lock on repo `%s`' % testfork['git']) in stderr - # we need to cleanup the Session Here ! - Session.remove() - r = Repository.get_by_repo_name(testfork['git']) - assert r.locked == [None, None] - def test_ip_restriction_hg(self, webserver): user_model = UserModel() try: