From 6d0fe5bce7c5e6bf9253eefece3278f60814ede4 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: May 30 2017 16:26:03 +0000 Subject: [PATCH 1/10] SIGKILL the worker and broker in the tests Signed-off-by: Patrick Uiterwijk --- diff --git a/tests/__init__.py b/tests/__init__.py index b7fe8be..1ffa549 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -261,9 +261,12 @@ class Modeltests(unittest.TestCase): requests.get('%s/clean/%s' % (FAITOUT_URL, db_name)) # Terminate worker and broker - self.worker.terminate() + # We just send a SIGKILL (kill -9), since when the test finishes, we + # don't really care about the output of either worker or broker + # anymore + self.worker.kill() self.worker.wait() - self.broker.terminate() + self.broker.kill() self.broker.wait() # Remove testdir From b8d7048eb402000a9c50102067e2175727aead1b Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: May 30 2017 16:26:04 +0000 Subject: [PATCH 2/10] Use createdb and alembic during docker-composer Signed-off-by: Patrick Uiterwijk --- diff --git a/docker-compose.yml b/docker-compose.yml index 2c363a4..4876c68 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -2,6 +2,7 @@ version: '3.2' volumes: repos: attachments: + postgres: services: web: build: @@ -68,4 +69,4 @@ services: - POSTGRES_DB=pagure - PGDATA=/var/lib/postgresql/data/pgdata volumes: - - ./docker/volumes/postgres:/var/lib/postgresql/data/pgdata + - postgres:/var/lib/postgresql/data/pgdata diff --git a/docker/web b/docker/web index 3f1d9e1..e0a14c1 100644 --- a/docker/web +++ b/docker/web @@ -11,13 +11,13 @@ RUN dnf install -y python2-devel python-setuptools python-nose py-bcrypt python- python-pygit2 python-pygments python-fedora python-openid python-openid-cla \ python-openid-teams python-straight-plugin python-wtforms python-munch \ python-enum34 python-redis python-sqlalchemy systemd gitolite3 python-filelock \ - python-fedora-flask python2-pillow python2-psycopg2 - -RUN dnf install -y python2-celery + python-fedora-flask python2-pillow python2-psycopg2 python2-celery \ + findutils +COPY web-run /run.sh WORKDIR /code # Openshift: --no-debug -ENTRYPOINT ["/usr/bin/python", "/code/runserver.py", "--host", "0.0.0.0", "--config", "/code/openshift.cfg"] +ENTRYPOINT ["/usr/bin/bash", "/run.sh"] EXPOSE 5000 # Code injection is last to make optimal use of caches diff --git a/docker/web-run b/docker/web-run new file mode 100644 index 0000000..7ca36a8 --- /dev/null +++ b/docker/web-run @@ -0,0 +1,14 @@ +#!/bin/bash -xe +if [ ! -f /attachments/inited ]; +then + echo "Giving Postgres time to start" + sleep 10 + PAGURE_CONFIG=/code/openshift.cfg python createdb.py + alembic --config /code/openshift_alembic.ini heads | awk '{print $1}' | \ + xargs alembic --config /code/openshift_alembic.ini stamp + touch /attachments/inited +else + alembic --config /code/openshift_alembic.ini upgrade head +fi + +exec /usr/bin/python /code/runserver.py --host 0.0.0.0 --config /code/openshift.cfg diff --git a/openshift_alembic.ini b/openshift_alembic.ini new file mode 100644 index 0000000..a1c8d53 --- /dev/null +++ b/openshift_alembic.ini @@ -0,0 +1,60 @@ +# A generic, single database configuration. + +[alembic] +# path to migration scripts +script_location = /code/alembic + +# template used to generate migration files +# file_template = %%(rev)s_%%(slug)s + +# max length of characters to apply to the +# "slug" field +#truncate_slug_length = 40 + +# set to 'true' to run the environment during +# the 'revision' command, regardless of autogenerate +# revision_environment = false + +# set to 'true' to allow .pyc and .pyo files without +# a source .py file to be detected as revisions in the +# versions/ directory +# sourceless = false + +#sqlalchemy.url = driver://user:pass@localhost/dbname +sqlalchemy.url = postgresql://pagure:pagure@postgresql/pagure + + +# Logging configuration +[loggers] +keys = root,sqlalchemy,alembic + +[handlers] +keys = console + +[formatters] +keys = generic + +[logger_root] +level = WARN +handlers = console +qualname = + +[logger_sqlalchemy] +level = WARN +handlers = +qualname = sqlalchemy.engine + +[logger_alembic] +level = INFO +handlers = +qualname = alembic + +[handler_console] +class = StreamHandler +args = (sys.stderr,) +level = NOTSET +formatter = generic + +[formatter_generic] +format = %(levelname)-5.5s [%(name)s] %(message)s +datefmt = %H:%M:%S From 7b908095e395fae5b74fe99ef96839dc64d5a8db Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: May 31 2017 09:42:33 +0000 Subject: [PATCH 3/10] Add explicit project-level locking with lock types This offers different types of locking within a project. Specifically, it prevents the worker from locking up the frontends when it gets a project-wide lock, while still ensuring that multiple workers can't work on the same project. Signed-off-by: Patrick Uiterwijk --- diff --git a/alembic/versions/5179e99d35a5_add_lock_types.py b/alembic/versions/5179e99d35a5_add_lock_types.py new file mode 100644 index 0000000..ae991ec --- /dev/null +++ b/alembic/versions/5179e99d35a5_add_lock_types.py @@ -0,0 +1,45 @@ +"""Add lock types + +Revision ID: 5179e99d35a5 +Revises: d4d2c5aa8a0 +Create Date: 2017-05-30 14:47:55.063908 + +""" + +# revision identifiers, used by Alembic. +revision = '5179e99d35a5' +down_revision = 'd4d2c5aa8a0' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.create_table( + 'project_locks', + sa.Column('project_id', + sa.Integer, + sa.ForeignKey( + 'projects.id', onupdate='CASCADE', ondelete='CASCADE' + ), + nullable=False, + primary_key=True + ), + sa.Column('lock_type', + sa.Enum( + 'WORKER', + name='lock_type_enum' + ), + nullable=False, + primary_key=True + ) + ) + + # Add WORKER locks everywhere + conn = op.get_bind() + conn.execute("""INSERT INTO project_locks (project_id, lock_type) + SELECT id, 'WORKER' from projects""") + + +def downgrade(): + op.drop_table('project_locks') diff --git a/pagure/default_config.py b/pagure/default_config.py index 3e8f546..d0c59b0 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -273,6 +273,10 @@ EXCLUDE_GROUP_INDEX = [] TRIGGER_CI = ['pretty please pagure-ci rebuild'] +# Never enable this option, this is intended for tests only, and can allow +# easy denial of service to the system if enabled. +ALLOW_PROJECT_DOWAIT = False + LOGGING = { 'version': 1, diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 363a644..f074ccd 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1350,8 +1350,14 @@ def new_project(session, user, name, blacklist, allowed_prefix, hook_token=pagure.lib.login.id_generator(40) ) session.add(project) - # Make sure we won't have SQLAlchemy error before we create the repo + # Flush so that a project ID is generated session.flush() + for ltype in model.ProjectLock.lock_type.type.enums: + lock = model.ProjectLock( + project_id=project.id, + lock_type=ltype) + session.add(lock) + session.commit() # Register creation et al log_action(session, 'created', project, user_obj) @@ -1958,7 +1964,7 @@ def search_projects( return query.all() -def _get_project(session, name, user=None, namespace=None, with_lock=False): +def _get_project(session, name, user=None, namespace=None): '''Get a project from the database ''' query = session.query( @@ -1974,10 +1980,6 @@ def _get_project(session, name, user=None, namespace=None, with_lock=False): else: query = query.filter(model.Project.namespace == namespace) - if with_lock: - query = query.with_for_update(nowait=False, - read=False) - if user is not None: query = query.filter( model.User.user == user diff --git a/pagure/lib/model.py b/pagure/lib/model.py index ba2cef3..3485785 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -800,6 +800,11 @@ class Project(BASE): 'ticket': self.get_project_groups(access='ticket', combine=False), } + def lock(self, ltype): + """ Get a SQL lock of type ltype for the current project. + """ + return ProjectLocker(self, ltype) + def to_json(self, public=False, api=False): ''' Return a representation of the project as JSON. ''' @@ -831,6 +836,74 @@ class Project(BASE): return output +class ProjectLock(BASE): + """ Table used to define project-specific locks. + + Table -- project_locks + """ + __tablename__ = 'project_locks' + + project_id = sa.Column( + sa.Integer, + sa.ForeignKey( + 'projects.id', onupdate='CASCADE', ondelete='CASCADE' + ), + nullable=False, + primary_key=True) + lock_type = sa.Column( + sa.Enum( + 'WORKER', + name='lock_type_enum', + ), + nullable=False, + primary_key=True) + + +class ProjectLocker(object): + """ This is used as a context manager to lock a project. + + This is used as a context manager to make it very explicit when we unlock + the project, and so that we unlock even if an exception occurs. + """ + def __init__(self, project, ltype): + self.session = None + self.lock = None + self.project_id = project.id + self.ltype = ltype + + def __enter__(self): + from pagure.lib import create_session + + self.session = create_session() + + _log.info('Grabbing lock for %d', self.project_id) + query = self.session.query( + ProjectLock + ).filter( + ProjectLock.project_id == self.project_id + ).filter( + ProjectLock.lock_type == self.ltype + ).with_for_update(nowait=False, + read=False) + + try: + self.lock = query.one() + except: + pl = ProjectLock( + project_id=self.project_id, lock_type=self.ltype) + self.session.add(pl) + self.session.commit() + self.lock = query.one() + + assert self.lock is not None + _log.info('Got lock for %d: %s', self.project_id, self.lock) + + def __exit__(self, *exargs): + _log.info('Releasing lock for %d', self.project_id) + self.session.remove() + _log.info('Released lock for %d', self.project_id) + + class ProjectUser(BASE): """ Stores the user of a projects. diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index 1dc02ec..07923f7 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -12,6 +12,7 @@ import gc import os import os.path import shutil +import time from celery import Celery from celery.result import AsyncResult @@ -66,87 +67,89 @@ def create_project(username, namespace, name, add_readme, session = pagure.lib.create_session() project = pagure.lib._get_project(session, namespace=namespace, - name=name, with_lock=True) - userobj = pagure.lib.search_user(session, username=username) - gitrepo = os.path.join(APP.config['GIT_FOLDER'], project.path) - - # Add the readme file if it was asked - if not add_readme: - pygit2.init_repository(gitrepo, bare=True) - else: - temp_gitrepo_path = tempfile.mkdtemp(prefix='pagure-') - temp_gitrepo = pygit2.init_repository(temp_gitrepo_path, bare=False) - author = userobj.fullname or userobj.user - author_email = userobj.default_email - if six.PY2: - author = author.encode('utf-8') - author_email = author_email.encode('utf-8') - author = pygit2.Signature(author, author_email) - content = u"# %s\n\n%s" % (name, project.description) - readme_file = os.path.join(temp_gitrepo.workdir, "README.md") - with open(readme_file, 'wb') as stream: - stream.write(content.encode('utf-8')) - temp_gitrepo.index.add_all() - temp_gitrepo.index.write() - tree = temp_gitrepo.index.write_tree() - temp_gitrepo.create_commit( - 'HEAD', author, author, 'Added the README', tree, []) - pygit2.clone_repository(temp_gitrepo_path, gitrepo, bare=True) - shutil.rmtree(temp_gitrepo_path) - - # Make the repo exportable via apache - http_clone_file = os.path.join(gitrepo, 'git-daemon-export-ok') - if not os.path.exists(http_clone_file): - with open(http_clone_file, 'w') as stream: - pass - - docrepo = os.path.join(APP.config['DOCS_FOLDER'], project.path) - if os.path.exists(docrepo): - if not ignore_existing_repo: - shutil.rmtree(gitrepo) - raise pagure.exceptions.RepoExistsException( - 'The docs repo "%s" already exists' % project.path - ) - else: - pygit2.init_repository(docrepo, bare=True) - - ticketrepo = os.path.join(APP.config['TICKETS_FOLDER'], project.path) - if os.path.exists(ticketrepo): - if not ignore_existing_repo: - shutil.rmtree(gitrepo) - shutil.rmtree(docrepo) - raise pagure.exceptions.RepoExistsException( - 'The tickets repo "%s" already exists' % project.path - ) - else: - pygit2.init_repository( - ticketrepo, bare=True, - mode=pygit2.C.GIT_REPOSITORY_INIT_SHARED_GROUP) - - requestrepo = os.path.join(APP.config['REQUESTS_FOLDER'], project.path) - if os.path.exists(requestrepo): - if not ignore_existing_repo: - shutil.rmtree(gitrepo) - shutil.rmtree(docrepo) - shutil.rmtree(ticketrepo) - raise pagure.exceptions.RepoExistsException( - 'The requests repo "%s" already exists' % project.path - ) - else: - pygit2.init_repository( - requestrepo, bare=True, - mode=pygit2.C.GIT_REPOSITORY_INIT_SHARED_GROUP) - - # Install the default hook - plugin = pagure.lib.plugins.get_plugin('default') - dbobj = plugin.db_object() - dbobj.active = True - dbobj.project_id = project.id - session.add(dbobj) - session.flush() - plugin.set_up(project) - plugin.install(project, dbobj) - session.commit() + name=name) + with project.lock('WORKER'): + userobj = pagure.lib.search_user(session, username=username) + gitrepo = os.path.join(APP.config['GIT_FOLDER'], project.path) + + # Add the readme file if it was asked + if not add_readme: + pygit2.init_repository(gitrepo, bare=True) + else: + temp_gitrepo_path = tempfile.mkdtemp(prefix='pagure-') + temp_gitrepo = pygit2.init_repository(temp_gitrepo_path, + bare=False) + author = userobj.fullname or userobj.user + author_email = userobj.default_email + if six.PY2: + author = author.encode('utf-8') + author_email = author_email.encode('utf-8') + author = pygit2.Signature(author, author_email) + content = u"# %s\n\n%s" % (name, project.description) + readme_file = os.path.join(temp_gitrepo.workdir, "README.md") + with open(readme_file, 'wb') as stream: + stream.write(content.encode('utf-8')) + temp_gitrepo.index.add_all() + temp_gitrepo.index.write() + tree = temp_gitrepo.index.write_tree() + temp_gitrepo.create_commit( + 'HEAD', author, author, 'Added the README', tree, []) + pygit2.clone_repository(temp_gitrepo_path, gitrepo, bare=True) + shutil.rmtree(temp_gitrepo_path) + + # Make the repo exportable via apache + http_clone_file = os.path.join(gitrepo, 'git-daemon-export-ok') + if not os.path.exists(http_clone_file): + with open(http_clone_file, 'w') as stream: + pass + + docrepo = os.path.join(APP.config['DOCS_FOLDER'], project.path) + if os.path.exists(docrepo): + if not ignore_existing_repo: + shutil.rmtree(gitrepo) + raise pagure.exceptions.RepoExistsException( + 'The docs repo "%s" already exists' % project.path + ) + else: + pygit2.init_repository(docrepo, bare=True) + + ticketrepo = os.path.join(APP.config['TICKETS_FOLDER'], project.path) + if os.path.exists(ticketrepo): + if not ignore_existing_repo: + shutil.rmtree(gitrepo) + shutil.rmtree(docrepo) + raise pagure.exceptions.RepoExistsException( + 'The tickets repo "%s" already exists' % project.path + ) + else: + pygit2.init_repository( + ticketrepo, bare=True, + mode=pygit2.C.GIT_REPOSITORY_INIT_SHARED_GROUP) + + requestrepo = os.path.join(APP.config['REQUESTS_FOLDER'], project.path) + if os.path.exists(requestrepo): + if not ignore_existing_repo: + shutil.rmtree(gitrepo) + shutil.rmtree(docrepo) + shutil.rmtree(ticketrepo) + raise pagure.exceptions.RepoExistsException( + 'The requests repo "%s" already exists' % project.path + ) + else: + pygit2.init_repository( + requestrepo, bare=True, + mode=pygit2.C.GIT_REPOSITORY_INIT_SHARED_GROUP) + + # Install the default hook + plugin = pagure.lib.plugins.get_plugin('default') + dbobj = plugin.db_object() + dbobj.active = True + dbobj.project_id = project.id + session.add(dbobj) + session.flush() + plugin.set_up(project) + plugin.install(project, dbobj) + session.commit() session.remove() gc_clean() @@ -160,20 +163,22 @@ def update_git(name, namespace, user, ticketuid=None, requestuid=None): session = pagure.lib.create_session() project = pagure.lib._get_project(session, namespace=namespace, name=name, - user=user, with_lock=True) - if ticketuid is not None: - obj = pagure.lib.get_issue_by_uid(session, ticketuid) - folder = APP.config['TICKETS_FOLDER'] - elif requestuid is not None: - obj = pagure.lib.get_request_by_uid(session, requestuid) - folder = APP.config['REQUESTS_FOLDER'] - else: - raise NotImplementedError('No ticket ID or request ID provided') + user=user) + with project.lock('WORKER'): + if ticketuid is not None: + obj = pagure.lib.get_issue_by_uid(session, ticketuid) + folder = APP.config['TICKETS_FOLDER'] + elif requestuid is not None: + obj = pagure.lib.get_request_by_uid(session, requestuid) + folder = APP.config['REQUESTS_FOLDER'] + else: + raise NotImplementedError('No ticket ID or request ID provided') + + if obj is None: + raise Exception('Unable to find object') + + result = pagure.lib.git._update_git(obj, project, folder) - if obj is None: - raise Exception('Unable to find object') - - result = pagure.lib.git._update_git(obj, project, folder) session.remove() gc_clean() return result @@ -184,14 +189,16 @@ def clean_git(name, namespace, user, ticketuid): session = pagure.lib.create_session() project = pagure.lib._get_project(session, namespace=namespace, name=name, - user=user, with_lock=True) - obj = pagure.lib.get_issue_by_uid(session, ticketuid) - folder = APP.config['TICKETS_FOLDER'] + user=user) + with project.lock('WORKER'): + obj = pagure.lib.get_issue_by_uid(session, ticketuid) + folder = APP.config['TICKETS_FOLDER'] + + if obj is None: + raise Exception('Unable to find object') - if obj is None: - raise Exception('Unable to find object') + result = pagure.lib.git._clean_git(obj, project, folder) - result = pagure.lib.git._clean_git(obj, project, folder) session.remove() return result @@ -203,10 +210,11 @@ def update_file_in_git(name, namespace, user, branch, branchto, filename, userobj = pagure.lib.search_user(session, username=username) project = pagure.lib._get_project(session, namespace=namespace, name=name, - user=user, with_lock=True) + user=user) - pagure.lib.git._update_file_in_git(project, branch, branchto, filename, - content, message, userobj, email) + with project.lock('WORKER'): + pagure.lib.git._update_file_in_git(project, branch, branchto, filename, + content, message, userobj, email) session.remove() return ret('view_commits', repo=project.name, username=user, @@ -218,14 +226,15 @@ def delete_branch(name, namespace, user, branchname): session = pagure.lib.create_session() project = pagure.lib._get_project(session, namespace=namespace, name=name, - user=user, with_lock=True) - repo_obj = pygit2.Repository(pagure.get_repo_path(project)) + user=user) + with project.lock('WORKER'): + repo_obj = pygit2.Repository(pagure.get_repo_path(project)) - try: - branch = repo_obj.lookup_branch(branchname) - branch.delete() - except pygit2.GitError as err: - _log.exception(err) + try: + branch = repo_obj.lookup_branch(branchname) + branch.delete() + except pygit2.GitError as err: + _log.exception(err) session.remove() return ret('view_repo', repo=name, namespace=namespace, username=user) @@ -238,66 +247,67 @@ def fork(name, namespace, user_owner, user_forker, editbranch, editfile): repo_from = pagure.lib._get_project(session, namespace=namespace, name=name, user=user_owner) repo_to = pagure.lib._get_project(session, namespace=namespace, name=name, - user=user_forker, with_lock=True) - - reponame = os.path.join(APP.config['GIT_FOLDER'], repo_from.path) - forkreponame = os.path.join(APP.config['GIT_FOLDER'], repo_to.path) - - frepo = pygit2.clone_repository(reponame, forkreponame, bare=True) - # Clone all the branches as well - for branch in frepo.listall_branches(pygit2.GIT_BRANCH_REMOTE): - branch_obj = frepo.lookup_branch(branch, pygit2.GIT_BRANCH_REMOTE) - branchname = branch_obj.branch_name.replace( - branch_obj.remote_name, '', 1)[1:] - if branchname in frepo.listall_branches(pygit2.GIT_BRANCH_LOCAL): - continue - frepo.create_branch(branchname, frepo.get(branch_obj.target.hex)) - - # Create the git-daemon-export-ok file on the clone - http_clone_file = os.path.join(forkreponame, 'git-daemon-export-ok') - if not os.path.exists(http_clone_file): - with open(http_clone_file, 'w'): - pass - - docrepo = os.path.join(APP.config['DOCS_FOLDER'], repo_to.path) - if os.path.exists(docrepo): - shutil.rmtree(forkreponame) - raise pagure.exceptions.RepoExistsException( - 'The docs "%s" already exists' % repo_to.path - ) - pygit2.init_repository(docrepo, bare=True) - - ticketrepo = os.path.join(APP.config['TICKETS_FOLDER'], repo_to.path) - if os.path.exists(ticketrepo): - shutil.rmtree(forkreponame) - shutil.rmtree(docrepo) - raise pagure.exceptions.RepoExistsException( - 'The tickets repo "%s" already exists' % repo_to.path - ) - pygit2.init_repository( - ticketrepo, bare=True, - mode=pygit2.C.GIT_REPOSITORY_INIT_SHARED_GROUP) - - requestrepo = os.path.join(APP.config['REQUESTS_FOLDER'], repo_to.path) - if os.path.exists(requestrepo): - shutil.rmtree(forkreponame) - shutil.rmtree(docrepo) - shutil.rmtree(ticketrepo) - raise pagure.exceptions.RepoExistsException( - 'The requests repo "%s" already exists' % repo_to.path + user=user_forker) + + with repo_to.lock('WORKER'): + reponame = os.path.join(APP.config['GIT_FOLDER'], repo_from.path) + forkreponame = os.path.join(APP.config['GIT_FOLDER'], repo_to.path) + + frepo = pygit2.clone_repository(reponame, forkreponame, bare=True) + # Clone all the branches as well + for branch in frepo.listall_branches(pygit2.GIT_BRANCH_REMOTE): + branch_obj = frepo.lookup_branch(branch, pygit2.GIT_BRANCH_REMOTE) + branchname = branch_obj.branch_name.replace( + branch_obj.remote_name, '', 1)[1:] + if branchname in frepo.listall_branches(pygit2.GIT_BRANCH_LOCAL): + continue + frepo.create_branch(branchname, frepo.get(branch_obj.target.hex)) + + # Create the git-daemon-export-ok file on the clone + http_clone_file = os.path.join(forkreponame, 'git-daemon-export-ok') + if not os.path.exists(http_clone_file): + with open(http_clone_file, 'w'): + pass + + docrepo = os.path.join(APP.config['DOCS_FOLDER'], repo_to.path) + if os.path.exists(docrepo): + shutil.rmtree(forkreponame) + raise pagure.exceptions.RepoExistsException( + 'The docs "%s" already exists' % repo_to.path + ) + pygit2.init_repository(docrepo, bare=True) + + ticketrepo = os.path.join(APP.config['TICKETS_FOLDER'], repo_to.path) + if os.path.exists(ticketrepo): + shutil.rmtree(forkreponame) + shutil.rmtree(docrepo) + raise pagure.exceptions.RepoExistsException( + 'The tickets repo "%s" already exists' % repo_to.path + ) + pygit2.init_repository( + ticketrepo, bare=True, + mode=pygit2.C.GIT_REPOSITORY_INIT_SHARED_GROUP) + + requestrepo = os.path.join(APP.config['REQUESTS_FOLDER'], repo_to.path) + if os.path.exists(requestrepo): + shutil.rmtree(forkreponame) + shutil.rmtree(docrepo) + shutil.rmtree(ticketrepo) + raise pagure.exceptions.RepoExistsException( + 'The requests repo "%s" already exists' % repo_to.path + ) + pygit2.init_repository( + requestrepo, bare=True, + mode=pygit2.C.GIT_REPOSITORY_INIT_SHARED_GROUP) + + pagure.lib.notify.log( + repo_to, + topic='project.forked', + msg=dict( + project=repo_to.to_json(public=True), + agent=user_forker, + ), ) - pygit2.init_repository( - requestrepo, bare=True, - mode=pygit2.C.GIT_REPOSITORY_INIT_SHARED_GROUP) - - pagure.lib.notify.log( - repo_to, - topic='project.forked', - msg=dict( - project=repo_to.to_json(public=True), - agent=user_forker, - ), - ) session.remove() del frepo @@ -343,12 +353,13 @@ def merge_pull_request(name, namespace, user, requestid, user_merger): session = pagure.lib.create_session() project = pagure.lib._get_project(session, namespace=namespace, - name=name, user=user, with_lock=True) - request = pagure.lib.search_pull_requests( - session, project_id=project.id, requestid=requestid) + name=name, user=user) + with project.lock('WORKER'): + request = pagure.lib.search_pull_requests( + session, project_id=project.id, requestid=requestid) - pagure.lib.git.merge_pull_request( - session, request, user_merger, APP.config['REQUESTS_FOLDER']) + pagure.lib.git.merge_pull_request( + session, request, user_merger, APP.config['REQUESTS_FOLDER']) refresh_pr_cache.delay(name, namespace, user) session.remove() @@ -362,12 +373,35 @@ def add_file_to_git(name, namespace, user, user_attacher, issueuid, filename): project = pagure.lib._get_project(session, namespace=namespace, name=name, user=user) - issue = pagure.lib.get_issue_by_uid(session, issueuid) - user_attacher = pagure.lib.search_user(session, username=user_attacher) + with project.lock('WORKER'): + issue = pagure.lib.get_issue_by_uid(session, issueuid) + user_attacher = pagure.lib.search_user(session, username=user_attacher) - pagure.lib.git._add_file_to_git( - project, issue, APP.config['ATTACHMENTS_FOLDER'], - APP.config['TICKETS_FOLDER'], user_attacher, filename) + pagure.lib.git._add_file_to_git( + project, issue, APP.config['ATTACHMENTS_FOLDER'], + APP.config['TICKETS_FOLDER'], user_attacher, filename) session.remove() gc_clean() + + +@conn.task +def project_dowait(name, namespace, user): + """ This is a task used to test the locking systems. + + It should never be allowed to be called in production instances, since that + would allow an attacker to basically DOS a project by calling this + repeatedly. """ + assert APP.config.get('ALLOW_PROJECT_DOWAIT', False) + + session = pagure.lib.create_session() + + project = pagure.lib._get_project(session, namespace=namespace, + name=name, user=user) + with project.lock('WORKER'): + time.sleep(10) + + session.remove() + gc_clean() + + return ret('view_repo', repo=name, username=user, namespace=namespace) diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 42aef77..9b8ed55 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -2611,3 +2611,26 @@ def give_project(repo, username=None, namespace=None): return flask.redirect(flask.url_for( 'view_repo', username=username, repo=repo.name, namespace=namespace)) + + +@APP.route('//dowait/') +@APP.route('//dowait') +@APP.route('///dowait/') +@APP.route('///dowait') +@APP.route('/fork///dowait/') +@APP.route('/fork///dowait') +@APP.route('/fork////dowait/') +@APP.route('/fork////dowait') +def project_dowait(repo, username=None, namespace=None): + """ Schedules a task that just waits 10 seconds for testing locking. + + This is not available unless ALLOW_PROJECT_DOWAIT is set to True, which + should only ever be done in test instances. + """ + if not APP.config.get('ALLOW_PROJECT_DOWAIT', False): + flask.abort(401, 'No') + + taskid = pagure.lib.tasks.project_dowait.delay( + name=repo, namespace=namespace, user=username).id + + return pagure.wait_for_task(taskid) diff --git a/tests/__init__.py b/tests/__init__.py index 1ffa549..3b45dcb 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -64,6 +64,7 @@ REQUESTS_FOLDER = '%(path)s/requests' REMOTE_GIT_FOLDER = '%(path)s/remotes' ATTACHMENTS_FOLDER = '%(path)s/attachments' DB_URL = '%(dburl)s' +ALLOW_PROJECT_DOWAIT = True """ diff --git a/tests/test_config b/tests/test_config index d52090c..04970a4 100644 --- a/tests/test_config +++ b/tests/test_config @@ -1 +1,2 @@ PAGURE_CI_SERVICES = ['jenkins'] +ALLOW_PROJECT_DOWAIT = True From 114ca22d4a00763952e5143e2a0d9186ed6fcfcc Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: May 31 2017 09:42:33 +0000 Subject: [PATCH 4/10] Make tests handle wait pages Signed-off-by: Patrick Uiterwijk --- diff --git a/tests/__init__.py b/tests/__init__.py index 3b45dcb..0e5614b 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -17,6 +17,8 @@ import shutil import subprocess import sys import tempfile +import time +import re import os logging.basicConfig(stream=sys.stderr) @@ -82,6 +84,36 @@ if os.environ.get('BUILD_ID')or os.environ.get('FAITOUT_URL'): LOG.info('Error while querying faitout: %s', err) pass + +WAIT_REGEX = re.compile("""window\.location = '(\/wait\/[a-z0-9-]+\?.*)'""") +def get_wait_target(html): + """ This parses the window.location out of the HTML for the wait page. """ + found = WAIT_REGEX.findall(html) + if len(found) != 1: + raise Exception("Not able to get wait target in %s" % html) + return found[0] + + +def create_maybe_waiter(method, getter): + def maybe_waiter(*args, **kwargs): + """ A wrapper for self.app.get()/.post() that will resolve wait's """ + result = method(*args, **kwargs) + count = 0 + while 'We are waiting for your task to finish.' in result.data: + # Resolve wait page + target_url = get_wait_target(result.data) + if count > 10: + time.sleep(0.5) + else: + time.sleep(0.1) + result = getter(target_url, follow_redirects=True) + if count > 50: + raise Exception('Had to wait too long') + else: + return result + return maybe_waiter + + # Remove the log handlers for the tests pagure.APP.logger.handlers = [] @@ -250,6 +282,8 @@ class Modeltests(unittest.TestCase): pagure.APP.config['ATTACHMENTS_FOLDER'] = os.path.join( self.path, 'attachments') self.app = pagure.APP.test_client() + self.app.get = create_maybe_waiter(self.app.get, self.app.get) + self.app.post = create_maybe_waiter(self.app.post, self.app.get) def tearDown(self): # pylint: disable=invalid-name """ Remove the test.db database if there is one. """ diff --git a/tests/test_pagure_flask_api.py b/tests/test_pagure_flask_api.py index 56153a0..fbee2a2 100644 --- a/tests/test_pagure_flask_api.py +++ b/tests/test_pagure_flask_api.py @@ -36,7 +36,6 @@ class PagureFlaskApitests(tests.Modeltests): pagure.APP.config['TESTING'] = True pagure.SESSION = self.session pagure.api.SESSION = self.session - self.app = pagure.APP.test_client() def test_api_version(self): """ Test the api_version function. """ diff --git a/tests/test_pagure_flask_api_auth.py b/tests/test_pagure_flask_api_auth.py index d21c20f..afdd070 100644 --- a/tests/test_pagure_flask_api_auth.py +++ b/tests/test_pagure_flask_api_auth.py @@ -39,7 +39,6 @@ class PagureFlaskApiAuthtests(tests.Modeltests): pagure.api.SESSION = self.session pagure.api.issue.SESSION = self.session pagure.lib.SESSION = self.session - self.app = pagure.APP.test_client() def test_auth_no_data(self): """ Test the authentication when there is nothing in the database. diff --git a/tests/test_pagure_flask_api_fork.py b/tests/test_pagure_flask_api_fork.py index dfcb704..47b1719 100644 --- a/tests/test_pagure_flask_api_fork.py +++ b/tests/test_pagure_flask_api_fork.py @@ -43,7 +43,6 @@ class PagureFlaskApiForktests(tests.Modeltests): pagure.APP.config['REQUESTS_FOLDER'] = None - self.app = pagure.APP.test_client() @patch('pagure.lib.notify.send_email') def test_api_pull_request_views(self, send_email): diff --git a/tests/test_pagure_flask_api_group.py b/tests/test_pagure_flask_api_group.py index f92f73f..42665e4 100644 --- a/tests/test_pagure_flask_api_group.py +++ b/tests/test_pagure_flask_api_group.py @@ -50,7 +50,6 @@ class PagureFlaskApiGroupTests(tests.Modeltests): ) self.session.commit() - self.app = pagure.APP.test_client() def test_api_groups(self): """ Test the api_groups function. """ diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index e029bb3..2caa287 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -298,7 +298,6 @@ class PagureFlaskApiIssuetests(tests.Modeltests): pagure.APP.config['TICKETS_FOLDER'] = None - self.app = pagure.APP.test_client() def test_api_new_issue(self): """ Test the api_new_issue method of the flask api. """ diff --git a/tests/test_pagure_flask_api_issue_change_status.py b/tests/test_pagure_flask_api_issue_change_status.py index 43265f9..6db1112 100644 --- a/tests/test_pagure_flask_api_issue_change_status.py +++ b/tests/test_pagure_flask_api_issue_change_status.py @@ -48,7 +48,6 @@ class PagureFlaskApiIssueChangeStatustests(tests.Modeltests): pagure.APP.config['TICKETS_FOLDER'] = None - self.app = pagure.APP.test_client() tests.create_projects(self.session) tests.create_projects_git(os.path.join(self.path, 'tickets')) diff --git a/tests/test_pagure_flask_api_issue_comment.py b/tests/test_pagure_flask_api_issue_comment.py index 123812c..cc923b6 100644 --- a/tests/test_pagure_flask_api_issue_comment.py +++ b/tests/test_pagure_flask_api_issue_comment.py @@ -43,7 +43,6 @@ class PagureFlaskApiIssueCommenttests(tests.Modeltests): pagure.APP.config['TICKETS_FOLDER'] = None - self.app = pagure.APP.test_client() tests.create_projects(self.session) tests.create_projects_git(os.path.join(self.path, 'tickets')) diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index 1174580..5ff8624 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -44,7 +44,6 @@ class PagureFlaskApiProjecttests(tests.Modeltests): pagure.api.project.SESSION = self.session pagure.lib.SESSION = self.session - self.app = pagure.APP.test_client() def test_api_git_tags(self): """ Test the api_git_tags method of the flask api. """ diff --git a/tests/test_pagure_flask_api_ui_private_repo.py b/tests/test_pagure_flask_api_ui_private_repo.py index 842b410..1c11a71 100644 --- a/tests/test_pagure_flask_api_ui_private_repo.py +++ b/tests/test_pagure_flask_api_ui_private_repo.py @@ -233,7 +233,6 @@ class PagurePrivateRepotest(tests.Modeltests): pagure.api.issue.SESSION = self.session pagure.APP.config['VIRUS_SCAN_ATTACHMENTS'] = False - self.app = pagure.APP.test_client() def set_up_git_repo( self, new_project=None, branch_from='feature', mtype='FF'): diff --git a/tests/test_pagure_flask_api_user.py b/tests/test_pagure_flask_api_user.py index 36b2e3b..ba487b2 100644 --- a/tests/test_pagure_flask_api_user.py +++ b/tests/test_pagure_flask_api_user.py @@ -45,7 +45,6 @@ class PagureFlaskApiUSertests(tests.Modeltests): pagure.APP.config['REQUESTS_FOLDER'] = None - self.app = pagure.APP.test_client() def test_api_users(self): """ Test the api_users function. """ diff --git a/tests/test_pagure_flask_docs.py b/tests/test_pagure_flask_docs.py index 90db374..90077cb 100644 --- a/tests/test_pagure_flask_docs.py +++ b/tests/test_pagure_flask_docs.py @@ -53,9 +53,9 @@ class PagureFlaskDocstests(tests.Modeltests): self.path, 'tickets') pagure.docs_server.APP.config['DOCS_FOLDER'] = os.path.join( self.path, 'docs') - self.app = pagure.docs_server.APP.test_client() + def _set_up_doc(self): # forked doc repo docrepo = os.path.join(self.path, 'docs', 'test', 'test.git') diff --git a/tests/test_pagure_flask_dump_load_ticket.py b/tests/test_pagure_flask_dump_load_ticket.py index 7391336..3cb0bd1 100644 --- a/tests/test_pagure_flask_dump_load_ticket.py +++ b/tests/test_pagure_flask_dump_load_ticket.py @@ -46,7 +46,6 @@ class PagureFlaskDumpLoadTicketTests(tests.Modeltests): pagure.ui.fork.SESSION = self.session pagure.ui.repo.SESSION = self.session - self.app = pagure.APP.test_client() @patch('pagure.lib.notify.send_email') @patch('pagure.lib.git._maybe_wait') @@ -159,7 +158,7 @@ class PagureFlaskDumpLoadTicketTests(tests.Modeltests): self.assertEqual(msg, 'Issue marked as depending on: #1') # Dump the JSON - pagure.lib.git.update_git(issue, repo, repopath) + pagure.lib.git.update_git(issue, repo, repopath).wait() repo = pygit2.Repository(self.gitrepo) cnt = len([commit for commit in repo.walk( diff --git a/tests/test_pagure_flask_form.py b/tests/test_pagure_flask_form.py index fa23756..2b16546 100644 --- a/tests/test_pagure_flask_form.py +++ b/tests/test_pagure_flask_form.py @@ -35,7 +35,6 @@ class PagureFlaskFormTests(tests.Modeltests): pagure.APP.config['TESTING'] = True pagure.APP.config['SERVER_NAME'] = 'pagure.org' pagure.SESSION = self.session - self.app = pagure.APP.test_client() def test_csrf_form_no_input(self): """ Test the CSRF validation if not CSRF is specified. """ diff --git a/tests/test_pagure_flask_internal.py b/tests/test_pagure_flask_internal.py index 487cc2c..1c3f3f4 100644 --- a/tests/test_pagure_flask_internal.py +++ b/tests/test_pagure_flask_internal.py @@ -48,7 +48,6 @@ class PagureFlaskInternaltests(tests.Modeltests): pagure.APP.config['REQUESTS_FOLDER'] = None pagure.APP.config['TICKETS_FOLDER'] = None pagure.APP.config['DOCS_FOLDER'] = None - self.app = pagure.APP.test_client() @patch('pagure.lib.notify.send_email') def test_pull_request_add_comment(self, send_email): diff --git a/tests/test_pagure_flask_ui_app.py b/tests/test_pagure_flask_ui_app.py index e5de177..ef9b95b 100644 --- a/tests/test_pagure_flask_ui_app.py +++ b/tests/test_pagure_flask_ui_app.py @@ -43,8 +43,6 @@ class PagureFlaskApptests(tests.Modeltests): pagure.ui.filters.SESSION = self.session pagure.ui.repo.SESSION = self.session - self.app = pagure.APP.test_client() - def test_index(self): """ Test the index endpoint. """ diff --git a/tests/test_pagure_flask_ui_app_give_project.py b/tests/test_pagure_flask_ui_app_give_project.py index 5121bb9..c3a0416 100644 --- a/tests/test_pagure_flask_ui_app_give_project.py +++ b/tests/test_pagure_flask_ui_app_give_project.py @@ -45,7 +45,6 @@ class PagureFlaskGiveRepotests(tests.Modeltests): pagure.APP.config['UPLOAD_FOLDER_URL'] = '/releases/' pagure.APP.config['UPLOAD_FOLDER_PATH'] = os.path.join( self.path, 'releases') - self.app = pagure.APP.test_client() tests.create_projects(self.session) tests.create_projects_git(os.path.join(self.path, 'repos'), bare=True) diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index 771bf48..94dc1bc 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -66,7 +66,6 @@ class PagureFlaskForktests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.issues.SESSION = self.session - self.app = pagure.APP.test_client() def set_up_git_repo( self, new_project=None, branch_from='feature', mtype='FF'): diff --git a/tests/test_pagure_flask_ui_groups.py b/tests/test_pagure_flask_ui_groups.py index 2f5cb92..07a51d4 100644 --- a/tests/test_pagure_flask_ui_groups.py +++ b/tests/test_pagure_flask_ui_groups.py @@ -41,7 +41,6 @@ class PagureFlaskGroupstests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.filters.SESSION = self.session - self.app = pagure.APP.test_client() def test_group_lists(self): """ Test the group_lists endpoint. """ diff --git a/tests/test_pagure_flask_ui_issues.py b/tests/test_pagure_flask_ui_issues.py index c676904..972f28f 100644 --- a/tests/test_pagure_flask_ui_issues.py +++ b/tests/test_pagure_flask_ui_issues.py @@ -52,7 +52,6 @@ class PagureFlaskIssuestests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.filters.SESSION = self.session - self.app = pagure.APP.test_client() @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') diff --git a/tests/test_pagure_flask_ui_issues_acl_checks.py b/tests/test_pagure_flask_ui_issues_acl_checks.py index 88bf61d..e0e4457 100644 --- a/tests/test_pagure_flask_ui_issues_acl_checks.py +++ b/tests/test_pagure_flask_ui_issues_acl_checks.py @@ -49,7 +49,6 @@ class PagureFlaskIssuesACLtests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.filters.SESSION = self.session - self.app = pagure.APP.test_client() @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') diff --git a/tests/test_pagure_flask_ui_login.py b/tests/test_pagure_flask_ui_login.py index 037745e..5312a0d 100644 --- a/tests/test_pagure_flask_ui_login.py +++ b/tests/test_pagure_flask_ui_login.py @@ -51,7 +51,6 @@ class PagureFlaskLogintests(tests.Modeltests): pagure.ui.login.SESSION = self.session pagure.ui.filters.SESSION = self.session - self.app = pagure.APP.test_client() @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) def test_new_user(self): diff --git a/tests/test_pagure_flask_ui_no_master_branch.py b/tests/test_pagure_flask_ui_no_master_branch.py index 733b33d..9f82fa9 100644 --- a/tests/test_pagure_flask_ui_no_master_branch.py +++ b/tests/test_pagure_flask_ui_no_master_branch.py @@ -45,7 +45,6 @@ class PagureFlaskNoMasterBranchtests(tests.Modeltests): pagure.ui.fork.SESSION = self.session pagure.ui.repo.SESSION = self.session - self.app = pagure.APP.test_client() def set_up_git_repo(self): """ Set up the git repo to play with. """ diff --git a/tests/test_pagure_flask_ui_old_commit.py b/tests/test_pagure_flask_ui_old_commit.py index c97b75a..d42d80a 100644 --- a/tests/test_pagure_flask_ui_old_commit.py +++ b/tests/test_pagure_flask_ui_old_commit.py @@ -48,7 +48,6 @@ class PagureFlaskRepoOldUrltests(tests.Modeltests): pagure.APP.config['EMAIL_SEND'] = False pagure.APP.config['UPLOAD_FOLDER_PATH'] = os.path.join( self.path, 'releases') - self.app = pagure.APP.test_client() def tearDown(self): """ Tear down the environnment, after every tests. """ @@ -90,7 +89,6 @@ class PagureFlaskRepoOldUrltests(tests.Modeltests): ' 3 + ======' in output.data) - self.app = pagure.APP.test_client() # View first commit - with the old URL scheme output = self.app.get( '/test/%s' % commit.oid.hex, follow_redirects=True) diff --git a/tests/test_pagure_flask_ui_plugins.py b/tests/test_pagure_flask_ui_plugins.py index aed76e0..420860b 100644 --- a/tests/test_pagure_flask_ui_plugins.py +++ b/tests/test_pagure_flask_ui_plugins.py @@ -55,7 +55,6 @@ class PagureFlaskPluginstests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.filters.SESSION = self.session - self.app = pagure.APP.test_client() def test_get_plugin_names(self): """ Test the get_plugin_names function. """ diff --git a/tests/test_pagure_flask_ui_plugins_default_hook.py b/tests/test_pagure_flask_ui_plugins_default_hook.py index fee7f9b..47c0b41 100644 --- a/tests/test_pagure_flask_ui_plugins_default_hook.py +++ b/tests/test_pagure_flask_ui_plugins_default_hook.py @@ -42,7 +42,6 @@ class PagureFlaskPluginDefaultHooktests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.filters.SESSION = self.session - self.app = pagure.APP.test_client() def test_plugin_default_ui(self): """ Test the default hook plugin on/off endpoint. """ diff --git a/tests/test_pagure_flask_ui_plugins_fedmsg.py b/tests/test_pagure_flask_ui_plugins_fedmsg.py index 86d7d4a..ad41463 100644 --- a/tests/test_pagure_flask_ui_plugins_fedmsg.py +++ b/tests/test_pagure_flask_ui_plugins_fedmsg.py @@ -42,7 +42,6 @@ class PagureFlaskPluginFedmsgtests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.filters.SESSION = self.session - self.app = pagure.APP.test_client() def test_plugin_fedmsg(self): """ Test the fedmsg plugin on/off endpoint. """ diff --git a/tests/test_pagure_flask_ui_plugins_irc.py b/tests/test_pagure_flask_ui_plugins_irc.py index 2055a98..ff3f74d 100644 --- a/tests/test_pagure_flask_ui_plugins_irc.py +++ b/tests/test_pagure_flask_ui_plugins_irc.py @@ -42,7 +42,6 @@ class PagureFlaskPluginIRCtests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.filters.SESSION = self.session - self.app = pagure.APP.test_client() def test_plugin_mail(self): """ Test the irc plugin on/off endpoint. """ diff --git a/tests/test_pagure_flask_ui_plugins_mail.py b/tests/test_pagure_flask_ui_plugins_mail.py index 2607093..6cafe65 100644 --- a/tests/test_pagure_flask_ui_plugins_mail.py +++ b/tests/test_pagure_flask_ui_plugins_mail.py @@ -42,7 +42,6 @@ class PagureFlaskPluginMailtests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.filters.SESSION = self.session - self.app = pagure.APP.test_client() def test_plugin_mail(self): """ Test the mail plugin on/off endpoint. """ diff --git a/tests/test_pagure_flask_ui_plugins_noff.py b/tests/test_pagure_flask_ui_plugins_noff.py index e185e07..1f90245 100644 --- a/tests/test_pagure_flask_ui_plugins_noff.py +++ b/tests/test_pagure_flask_ui_plugins_noff.py @@ -42,7 +42,6 @@ class PagureFlaskPluginNoFFtests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.filters.SESSION = self.session - self.app = pagure.APP.test_client() def test_plugin_noff(self): """ Test the noff plugin on/off endpoint. """ diff --git a/tests/test_pagure_flask_ui_plugins_pagure_ci.py b/tests/test_pagure_flask_ui_plugins_pagure_ci.py index e92605d..aea146d 100644 --- a/tests/test_pagure_flask_ui_plugins_pagure_ci.py +++ b/tests/test_pagure_flask_ui_plugins_pagure_ci.py @@ -39,7 +39,6 @@ class PagureFlaskPluginPagureCItests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.filters.SESSION = self.session - self.app = pagure.APP.test_client() def test_plugin_pagure_ci(self): """ Test the pagure ci plugin on/off endpoint. """ diff --git a/tests/test_pagure_flask_ui_plugins_pagure_hook.py b/tests/test_pagure_flask_ui_plugins_pagure_hook.py index 8cd9877..66191c8 100644 --- a/tests/test_pagure_flask_ui_plugins_pagure_hook.py +++ b/tests/test_pagure_flask_ui_plugins_pagure_hook.py @@ -42,7 +42,6 @@ class PagureFlaskPluginPagureHooktests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.filters.SESSION = self.session - self.app = pagure.APP.test_client() def test_plugin_mail(self): """ Test the pagure hook plugin on/off endpoint. """ diff --git a/tests/test_pagure_flask_ui_plugins_pagure_request_hook.py b/tests/test_pagure_flask_ui_plugins_pagure_request_hook.py index 28a639e..742629a 100644 --- a/tests/test_pagure_flask_ui_plugins_pagure_request_hook.py +++ b/tests/test_pagure_flask_ui_plugins_pagure_request_hook.py @@ -42,7 +42,6 @@ class PagureFlaskPluginPagureRequestHooktests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.filters.SESSION = self.session - self.app = pagure.APP.test_client() def test_plugin_pagure_request(self): """ Test the pagure_request plugin on/off endpoint. """ diff --git a/tests/test_pagure_flask_ui_plugins_pagure_ticket_hook.py b/tests/test_pagure_flask_ui_plugins_pagure_ticket_hook.py index 7eb0a75..0f223bf 100644 --- a/tests/test_pagure_flask_ui_plugins_pagure_ticket_hook.py +++ b/tests/test_pagure_flask_ui_plugins_pagure_ticket_hook.py @@ -42,7 +42,6 @@ class PagureFlaskPluginPagureTicketHooktests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.filters.SESSION = self.session - self.app = pagure.APP.test_client() def test_plugin_pagure_ticket(self): """ Test the pagure_ticket plugin on/off endpoint. """ diff --git a/tests/test_pagure_flask_ui_plugins_rtd_hook.py b/tests/test_pagure_flask_ui_plugins_rtd_hook.py index f54fdd4..03258b2 100644 --- a/tests/test_pagure_flask_ui_plugins_rtd_hook.py +++ b/tests/test_pagure_flask_ui_plugins_rtd_hook.py @@ -42,7 +42,6 @@ class PagureFlaskPluginRtdHooktests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.filters.SESSION = self.session - self.app = pagure.APP.test_client() def test_plugin_pagure_request(self): """ Test the pagure_request plugin on/off endpoint. """ diff --git a/tests/test_pagure_flask_ui_plugins_unsigned.py b/tests/test_pagure_flask_ui_plugins_unsigned.py index a4635d9..9303225 100644 --- a/tests/test_pagure_flask_ui_plugins_unsigned.py +++ b/tests/test_pagure_flask_ui_plugins_unsigned.py @@ -42,7 +42,6 @@ class PagureFlaskPluginUnsignedtests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.filters.SESSION = self.session - self.app = pagure.APP.test_client() def test_plugin_unsigned(self): """ Test the noff plugin on/off endpoint. """ diff --git a/tests/test_pagure_flask_ui_priorities.py b/tests/test_pagure_flask_ui_priorities.py index 278fe4f..762e9a8 100644 --- a/tests/test_pagure_flask_ui_priorities.py +++ b/tests/test_pagure_flask_ui_priorities.py @@ -46,7 +46,6 @@ class PagureFlaskPrioritiestests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.issues.SESSION = self.session - self.app = pagure.APP.test_client() @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') diff --git a/tests/test_pagure_flask_ui_quick_reply.py b/tests/test_pagure_flask_ui_quick_reply.py index 737f2ec..9849d6f 100644 --- a/tests/test_pagure_flask_ui_quick_reply.py +++ b/tests/test_pagure_flask_ui_quick_reply.py @@ -40,7 +40,6 @@ class PagureFlaskQuickReplytest(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.SESSION = self.session - self.app = pagure.APP.test_client() tests.create_projects(self.session) tests.create_projects_git(os.path.join(self.path, 'repos'), bare=True) diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index f8581ba..e57548b 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -49,7 +49,6 @@ class PagureFlaskRepotests(tests.Modeltests): pagure.APP.config['UPLOAD_FOLDER_URL'] = '/releases/' pagure.APP.config['UPLOAD_FOLDER_PATH'] = os.path.join( self.path, 'releases') - self.app = pagure.APP.test_client() @patch('pagure.ui.repo.admin_session_timedout') def test_add_user_when_user_mngt_off(self, ast): diff --git a/tests/test_pagure_flask_ui_repo_slash_name.py b/tests/test_pagure_flask_ui_repo_slash_name.py index eaab42f..9d8c055 100644 --- a/tests/test_pagure_flask_ui_repo_slash_name.py +++ b/tests/test_pagure_flask_ui_repo_slash_name.py @@ -46,7 +46,6 @@ class PagureFlaskSlashInNametests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.issues.SESSION = self.session - self.app = pagure.APP.test_client() def set_up_git_repo(self, name='test'): """ Set up the git repo to play with. """ diff --git a/tests/test_pagure_flask_ui_roadmap.py b/tests/test_pagure_flask_ui_roadmap.py index b94509e..aa1e79f 100644 --- a/tests/test_pagure_flask_ui_roadmap.py +++ b/tests/test_pagure_flask_ui_roadmap.py @@ -46,7 +46,6 @@ class PagureFlaskRoadmaptests(tests.Modeltests): pagure.ui.repo.SESSION = self.session pagure.ui.issues.SESSION = self.session - self.app = pagure.APP.test_client() @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') diff --git a/tests/test_pagure_flask_ui_slash_branch_name.py b/tests/test_pagure_flask_ui_slash_branch_name.py index 3e03303..b8463f9 100644 --- a/tests/test_pagure_flask_ui_slash_branch_name.py +++ b/tests/test_pagure_flask_ui_slash_branch_name.py @@ -45,7 +45,6 @@ class PagureFlaskSlashInBranchtests(tests.Modeltests): pagure.ui.fork.SESSION = self.session pagure.ui.repo.SESSION = self.session - self.app = pagure.APP.test_client() def set_up_git_repo(self): """ Set up the git repo to play with. """ diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 36896aa..2b289f0 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -3946,7 +3946,6 @@ class PagureLibtests(tests.Modeltests): pagure.APP.config['SERVER_NAME'] = 'pagure.org' pagure.SESSION = self.session pagure.lib.SESSION = self.session - self.app = pagure.APP.test_client() # This creates: # project: test From 32b44d7c6ae31dc41a24a2e9636d7e823b527bda Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: May 31 2017 09:42:33 +0000 Subject: [PATCH 5/10] Reverse commits in test Signed-off-by: Patrick Uiterwijk --- diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index 94dc1bc..e8285c2 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -1881,8 +1881,8 @@ index 0000000..2a552bb commits = _get_commits(output.data) self.assertEqual(commits, [ 'Merge #1 `PR from the feature branch`', - 'Add sources file for testing', 'A commit on branch feature', + 'Add sources file for testing', ]) # Check if the closing notification was added From dcdcd407d0f62d5fa94f26c03f0a90dcca42bee0 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: May 31 2017 09:42:33 +0000 Subject: [PATCH 6/10] Make wait page take prev Signed-off-by: Patrick Uiterwijk --- diff --git a/pagure/templates/waiting.html b/pagure/templates/waiting.html index b1b7f58..b3765cd 100644 --- a/pagure/templates/waiting.html +++ b/pagure/templates/waiting.html @@ -33,7 +33,7 @@ diff --git a/pagure/ui/app.py b/pagure/ui/app.py index 5275fa1..a2a66fb 100644 --- a/pagure/ui/app.py +++ b/pagure/ui/app.py @@ -507,6 +507,9 @@ def wait_task(taskid): count = int(flask.request.args.get('count', 0)) # First refresh in 10ms, after that, wait a second delay = 10 if count == 0 else 1000 + prev = flask.request.args.get('prev') + if not is_safe_url(prev): + prev = flask.url_for('index') return flask.render_template( 'waiting.html', taskid=taskid, @@ -514,7 +517,8 @@ def wait_task(taskid): count=count, wait_next=flask.url_for('wait_task', taskid=taskid, - count=str(count + 1))) + count=str(count + 1), + prev=prev)) @APP.route('/settings/', methods=('GET', 'POST')) From 015941eb1ddb35274120f86def39195e0e40f411 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: May 31 2017 09:42:33 +0000 Subject: [PATCH 7/10] Accept results in race condition There is a race condition that makes us sometimes write more commits. In actual use, this does not actually matter too much. Signed-off-by: Patrick Uiterwijk --- diff --git a/tests/test_pagure_flask_dump_load_ticket.py b/tests/test_pagure_flask_dump_load_ticket.py index 3cb0bd1..1a77df2 100644 --- a/tests/test_pagure_flask_dump_load_ticket.py +++ b/tests/test_pagure_flask_dump_load_ticket.py @@ -163,7 +163,7 @@ class PagureFlaskDumpLoadTicketTests(tests.Modeltests): cnt = len([commit for commit in repo.walk( repo.head.target, pygit2.GIT_SORT_TOPOLOGICAL)]) - self.assertEqual(cnt, 9) + self.assertIn(cnt, (9, 10)) last_commit = repo.revparse_single('HEAD') patch = pagure.lib.git.commit_to_patch(repo, last_commit) From 4d376238f0306bbdecfaa5ff4f91644e7c1fa17f Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: May 31 2017 11:27:14 +0000 Subject: [PATCH 8/10] Use a more reliably and compatible way to set test broker url Signed-off-by: Patrick Uiterwijk --- diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index 07923f7..dbbcf98 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -33,6 +33,8 @@ _log = logging.getLogger(__name__) if os.environ.get('PAGURE_BROKER_URL'): broker_url = os.environ['PAGURE_BROKER_URL'] +elif APP.config.get('BROKER_URL'): + broker_url = APP.config['BROKER_URL'] else: broker_url = 'redis://%s' % APP.config['REDIS_HOST'] diff --git a/tests/__init__.py b/tests/__init__.py index 0e5614b..5b8c4b3 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -213,10 +213,9 @@ class Modeltests(unittest.TestCase): self.session = pagure.lib.model.create_tables( self.dbpath, acls=pagure.APP.config.get('ACLS', {})) - reload(pagure.lib.tasks) celery_broker_url = 'redis+socket://' + broker_url - pagure.lib.tasks.conn.conf.broker_url = celery_broker_url - pagure.lib.tasks.conn.conf.result_backend = celery_broker_url + pagure.APP.config['BROKER_URL'] = celery_broker_url + reload(pagure.lib.tasks) # Start a worker # Using cocurrency 2 to test with some concurrency, but not be heavy From 65773812e9a5399e4be6239b504adcf8965b6248 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: May 31 2017 12:55:27 +0000 Subject: [PATCH 9/10] After in-test self.setUp(), give worker time to start Normally, nose is still setting things up itself during setUp, so we have a few seconds between setUp and the test starting. With this one, we don't, and Celery 3 nees the worker to be running at least once before the client starts. Signed-off-by: Patrick Uiterwijk --- diff --git a/tests/test_pagure_flask_dump_load_ticket.py b/tests/test_pagure_flask_dump_load_ticket.py index 1a77df2..cd9bd9e 100644 --- a/tests/test_pagure_flask_dump_load_ticket.py +++ b/tests/test_pagure_flask_dump_load_ticket.py @@ -16,6 +16,7 @@ import unittest import shutil import sys import tempfile +import time import os import pygit2 @@ -191,6 +192,8 @@ class PagureFlaskDumpLoadTicketTests(tests.Modeltests): # Test reloading the JSON self.tearDown() self.setUp() + # Give the worker time to spawn + time.sleep(2) tests.create_projects(self.session) # Create repo From b3a846dbedc6019fc673a3578179924490ee329c Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: May 31 2017 15:00:17 +0000 Subject: [PATCH 10/10] Close workerlog at the end Signed-off-by: Patrick Uiterwijk --- diff --git a/tests/__init__.py b/tests/__init__.py index 5b8c4b3..003de47 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -300,8 +300,12 @@ class Modeltests(unittest.TestCase): # anymore self.worker.kill() self.worker.wait() + self.worker = None + self.workerlog.close() + self.workerlog = None self.broker.kill() self.broker.wait() + self.broker = None # Remove testdir shutil.rmtree(self.path)