From ed7e6e6d19e087a01e486dbfeae1766e876296d9 Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Sep 21 2017 10:26:38 +0000 Subject: [PATCH 1/7] Read only repo: Add migration script, lib methods Signed-off-by: Vivek Anand --- diff --git a/alembic/versions/3237fc64b306_read_only_mode_in_projects.py b/alembic/versions/3237fc64b306_read_only_mode_in_projects.py new file mode 100644 index 0000000..f59f8ee --- /dev/null +++ b/alembic/versions/3237fc64b306_read_only_mode_in_projects.py @@ -0,0 +1,39 @@ +"""Read Only mode in projects + +Revision ID: 3237fc64b306 +Revises: c34f4b09ef18 +Create Date: 2017-09-01 22:51:18.232541 + +""" + +# revision identifiers, used by Alembic. +revision = '3237fc64b306' +down_revision = 'c34f4b09ef18' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ''' Add a column to mark a project read only ''' + op.add_column( + 'projects', + sa.Column( + 'read_only', + sa.Boolean, + default=True, + nullable=True, + ) + ) + op.execute(''' UPDATE projects SET read_only=False ''') + op.alter_column( + 'projects', + 'read_only', + nullable=False, + existing_nullable=True + ) + + +def downgrade(): + ''' Remove the read_only column from Projects ''' + op.drop_column('projects', 'read_only') diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index f7b8203..d450eee 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -977,6 +977,7 @@ def add_user_to_project( access_obj = get_obj_access(session, project, new_user_obj) access_obj.access = access project.date_modified = datetime.datetime.utcnow() + update_read_only_mode(session, project, read_only=True) session.add(access_obj) session.add(project) session.flush() @@ -1072,6 +1073,7 @@ def add_group_to_project( access_obj.access = access session.add(access_obj) project.date_modified = datetime.datetime.utcnow() + update_read_only_mode(session, project, read_only=True) session.add(project) session.flush() @@ -4488,3 +4490,21 @@ def has_starred(session, repo, user): if isinstance(stargazer_obj, model.Star): return True return False + + +def update_read_only_mode(session, repo, read_only=True): + ''' Remove the read only mode from the project + + :arg session: The session object to query the db with + :arg repo: model.Project object to mark/unmark read only + :arg read_only: True if project is to be made read only, + False otherwise + ''' + + if ( + not repo + or not isinstance(repo, model.Project) + or read_only not in [True, False]): + return + repo.read_only = read_only + session.add(repo) diff --git a/pagure/lib/model.py b/pagure/lib/model.py index fe1955e..fb3df1e 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -356,6 +356,7 @@ class Project(BASE): hook_token = sa.Column(sa.String(40), nullable=False, unique=True) avatar_email = sa.Column(sa.Text, nullable=True) is_fork = sa.Column(sa.Boolean, default=False, nullable=False) + read_only = sa.Column(sa.Boolean, default=True, nullable=False) parent_id = sa.Column( sa.Integer, sa.ForeignKey( diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index ef5f7b3..b95608a 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -23,6 +23,8 @@ import six import logging +from sqlalchemy.exc import SQLAlchemyError + import pagure from pagure import APP import pagure.lib @@ -102,6 +104,15 @@ def generate_gitolite_acls(namespace=None, name=None, user=None, group=None): 'Calling helper: %s with arg: project=%s, group=%s', helper, project, group_obj) helper.generate_acls(project=project, group=group_obj) + pagure.lib.update_read_only_mode( + session, project, read_only=False) + try: + session.commit() + _log.debug('Project %s is in Read Only Mode', project) + except SQLAlchemyError: + session.rollback() + _log.error( + 'Failed to unmark read_only for: %s project', project) session.remove() gc_clean() From 9888e8bbe0043561bc64e20cd6d36a1677e673c1 Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Sep 21 2017 10:26:39 +0000 Subject: [PATCH 2/7] read only repo: mark read only while adding/removing user/group Signed-off-by: Vivek Anand --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index d450eee..36a3d28 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1003,6 +1003,8 @@ def add_user_to_project( ) project.date_modified = datetime.datetime.utcnow() session.add(project_user) + # Mark the project as read only, celery will then unmark it + update_read_only_mode(session, project, read_only=True) session.add(project) # Make sure we won't have SQLAlchemy error before we continue session.flush() @@ -1099,6 +1101,8 @@ def add_group_to_project( session.add(project_group) # Make sure we won't have SQLAlchemy error before we continue project.date_modified = datetime.datetime.utcnow() + # Mark the project read_only, celery will then unmark it + update_read_only_mode(session, project, read_only=True) session.add(project) session.flush() diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 8dbd4ec..52b93c2 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -1606,6 +1606,8 @@ def remove_user(repo, userid, username=None, namespace=None): repo.users.remove(user) break try: + # Mark the project as read_only, celery will unmark it + pagure.lib.update_read_only_mode(session, repo, read_only=True) SESSION.commit() pagure.lib.git.generate_gitolite_acls(project=repo) flask.flash('User removed') @@ -1818,6 +1820,8 @@ def remove_group_project(repo, groupid, username=None, namespace=None): repo.groups.remove(grp) break try: + # Mark the project as read_only, celery will unmark it + pagure.lib.update_read_only_mode(session, repo, read_only=True) SESSION.commit() pagure.lib.git.generate_gitolite_acls(project=repo) flask.flash('Group removed') From f49e024d0c8cfb464b0fda7e7877efbbf309fe6c Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Sep 21 2017 10:26:39 +0000 Subject: [PATCH 3/7] read only repo: front end banner Signed-off-by: Vivek Anand --- diff --git a/pagure/static/pagure.css b/pagure/static/pagure.css index 6542064..3033695 100644 --- a/pagure/static/pagure.css +++ b/pagure/static/pagure.css @@ -23,6 +23,12 @@ html border-bottom:1px solid #DDD; } +.repo-header-read-only +{ + background:#ffc0cb; + border-bottom:1px solid #DDD; +} + .issue_comment table, .readme table, .card .m-a-2 table diff --git a/pagure/templates/repo_master.html b/pagure/templates/repo_master.html index 7377f81..37331bb 100644 --- a/pagure/templates/repo_master.html +++ b/pagure/templates/repo_master.html @@ -18,7 +18,11 @@ {% endif %} {% block content %} +{% if repo.read_only == true %} +
+{% else %}
+{% endif %}

@@ -26,6 +30,10 @@ {% endif %} + {% if repo.read_only == true %} + + {% endif %} {% if repo.is_fork -%} Date: Sep 21 2017 12:15:51 +0000 Subject: [PATCH 6/7] give project: generate gitolite acls after giving a project Signed-off-by: Vivek Anand --- diff --git a/pagure/static/pagure.css b/pagure/static/pagure.css index 3033695..6542064 100644 --- a/pagure/static/pagure.css +++ b/pagure/static/pagure.css @@ -23,12 +23,6 @@ html border-bottom:1px solid #DDD; } -.repo-header-read-only -{ - background:#ffc0cb; - border-bottom:1px solid #DDD; -} - .issue_comment table, .readme table, .card .m-a-2 table diff --git a/pagure/templates/repo_master.html b/pagure/templates/repo_master.html index 37331bb..1f372e2 100644 --- a/pagure/templates/repo_master.html +++ b/pagure/templates/repo_master.html @@ -18,11 +18,7 @@ {% endif %} {% block content %} -{% if repo.read_only == true %} -
-{% else %}
-{% endif %}

@@ -30,10 +26,6 @@ {% endif %} - {% if repo.read_only == true %} - - {% endif %} {% if repo.is_fork -%} + {% if repo.read_only %} +
+ +
+ {% endif %} {% block repo %} {% endblock %}

diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 52b93c2..a217ce0 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -1607,7 +1607,7 @@ def remove_user(repo, userid, username=None, namespace=None): break try: # Mark the project as read_only, celery will unmark it - pagure.lib.update_read_only_mode(session, repo, read_only=True) + pagure.lib.update_read_only_mode(SESSION, repo, read_only=True) SESSION.commit() pagure.lib.git.generate_gitolite_acls(project=repo) flask.flash('User removed') @@ -1821,7 +1821,7 @@ def remove_group_project(repo, groupid, username=None, namespace=None): break try: # Mark the project as read_only, celery will unmark it - pagure.lib.update_read_only_mode(session, repo, read_only=True) + pagure.lib.update_read_only_mode(SESSION, repo, read_only=True) SESSION.commit() pagure.lib.git.generate_gitolite_acls(project=repo) flask.flash('Group removed') @@ -2689,6 +2689,7 @@ def give_project(repo, username=None, namespace=None): SESSION, repo, new_user=flask.g.fas_user.username, user=flask.g.fas_user.username) SESSION.commit() + pagure.lib.git.generate_gitolite_acls(project=repo) flask.flash( 'The project has been transferred to %s' % new_username) except SQLAlchemyError: # pragma: no cover From a324b4120738bb9765b59625f3e767f9b50d599f Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Sep 25 2017 12:50:46 +0000 Subject: [PATCH 7/7] Fix the unit-tests Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/__init__.py b/pagure/__init__.py index 6c07a5e..6c00461 100644 --- a/pagure/__init__.py +++ b/pagure/__init__.py @@ -200,7 +200,7 @@ if APP.config.get('PAGURE_CI_SERVICES'): pagure.lib.set_pagure_ci(APP.config['PAGURE_CI_SERVICES']) -if not APP.debug: +if not APP.debug and not APP.config.get('DEBUG', False): APP.logger.addHandler(pagure.mail_logging.get_mail_handler( smtp_server=APP.config.get('SMTP_SERVER', '127.0.0.1'), mail_admin=APP.config.get('MAIL_ADMIN', APP.config['EMAIL_ERROR']), diff --git a/pagure/lib/notify.py b/pagure/lib/notify.py index c14bd67..3ff5018 100644 --- a/pagure/lib/notify.py +++ b/pagure/lib/notify.py @@ -22,7 +22,6 @@ import urlparse import re import smtplib import time -import warnings import flask import pagure @@ -50,7 +49,7 @@ def fedmsg_publish(*args, **kwargs): # pragma: no cover try: import fedmsg fedmsg.publish(*args, **kwargs) - except Exception as err: + except Exception: _log.exception('Error sending fedmsg') diff --git a/tests/__init__.py b/tests/__init__.py index 9c962ed..9142cd5 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -25,14 +25,15 @@ logging.basicConfig(stream=sys.stderr) # Always enable performance counting for tests os.environ['PAGURE_PERFREPO'] = 'true' +from contextlib import contextmanager from datetime import date from datetime import datetime from datetime import timedelta from functools import wraps +import mock import pygit2 -from contextlib import contextmanager from sqlalchemy import create_engine from sqlalchemy.orm import sessionmaker from sqlalchemy.orm import scoped_session @@ -67,6 +68,7 @@ REMOTE_GIT_FOLDER = '%(path)s/remotes' ATTACHMENTS_FOLDER = '%(path)s/attachments' DB_URL = '%(dburl)s' ALLOW_PROJECT_DOWAIT = True +DEBUG = True """ @@ -145,6 +147,7 @@ class SimplePagureTest(unittest.TestCase): Simple Test class that does not set a broker/worker """ + @mock.patch('pagure.lib.notify.fedmsg_publish', mock.MagicMock()) def __init__(self, method_name='runTest'): """ Constructor. """ unittest.TestCase.__init__(self, method_name) diff --git a/tests/test_config b/tests/test_config index 04970a4..a810db7 100644 --- a/tests/test_config +++ b/tests/test_config @@ -1,2 +1,3 @@ PAGURE_CI_SERVICES = ['jenkins'] ALLOW_PROJECT_DOWAIT = True +DEBUG=True diff --git a/tests/test_pagure_flask_ui_app_give_project.py b/tests/test_pagure_flask_ui_app_give_project.py index 1b07529..2335694 100644 --- a/tests/test_pagure_flask_ui_app_give_project.py +++ b/tests/test_pagure_flask_ui_app_give_project.py @@ -196,6 +196,7 @@ class PagureFlaskGiveRepotests(tests.SimplePagureTest): self._check_user() @patch.dict('pagure.APP.config', {'PAGURE_ADMIN_USERS': 'foo'}) + @patch('pagure.lib.git.generate_gitolite_acls', MagicMock()) def test_give_project_not_owner_but_admin(self): """ Test the give_project endpoint. @@ -229,6 +230,7 @@ class PagureFlaskGiveRepotests(tests.SimplePagureTest): self._check_user('foo') @patch.dict('pagure.APP.config', {'PAGURE_ADMIN_USERS': 'foo'}) + @patch('pagure.lib.git.generate_gitolite_acls', MagicMock()) def test_give_project(self): """ Test the give_project endpoint. """ @@ -261,6 +263,7 @@ class PagureFlaskGiveRepotests(tests.SimplePagureTest): self.assertEqual(project.users[0].user, 'pingou') @patch.dict('pagure.APP.config', {'PAGURE_ADMIN_USERS': 'foo'}) + @patch('pagure.lib.git.generate_gitolite_acls', MagicMock()) def test_give_project_already_user(self): """ Test the give_project endpoint when the new main_admin is already a committer on the project. """ diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index ffd9cf4..f13a3b8 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -2207,6 +2207,7 @@ index 0000000..2a552bb # UI test for deleted main output = self.app.get('/fork/foo/test') self.assertEqual(output.status_code, 200) + print output.data self.assertIn('Fork from a deleted repository\n', output.data) # Testing commit endpoint diff --git a/tests/test_pagure_flask_ui_no_master_branch.py b/tests/test_pagure_flask_ui_no_master_branch.py index a410eb8..9a899e6 100644 --- a/tests/test_pagure_flask_ui_no_master_branch.py +++ b/tests/test_pagure_flask_ui_no_master_branch.py @@ -128,8 +128,9 @@ class PagureFlaskNoMasterBranchtests(tests.SimplePagureTest): # With git repo output = self.app.get('/test') self.assertEqual(output.status_code, 200) + self.assertIn('
', output.data) self.assertIn( - '
\n
', + "The test project's README file is empty or unavailable.", output.data) self.assertEqual( output.data.count('', output.data) self.assertIn( - '
\n
', + "The test project's README file is empty or unavailable.", output.data) @patch('pagure.lib.notify.send_email') @@ -181,7 +183,7 @@ class PagureFlaskNoMasterBranchtests(tests.SimplePagureTest): output = self.app.get('/test/commits/feature') self.assertEqual(output.status_code, 200) self.assertIn( - '
\n
', + 'Commits 2', output.data) self.assertIn('Add sources file for testing', output.data) self.assertIn('Add .gitignore file for testing', output.data) diff --git a/tests/test_pagure_lib_gitolite_config.py b/tests/test_pagure_lib_gitolite_config.py index 184256d..d74881f 100644 --- a/tests/test_pagure_lib_gitolite_config.py +++ b/tests/test_pagure_lib_gitolite_config.py @@ -734,7 +734,7 @@ repo requests/test get_helper.assert_called_with('gitolite3') args = helper.generate_acls.call_args self.assertIsNone(args[1].get('group')) - self.assertEqual(args[1].get('project').fullname, 'test') + self.assertIsNotNone(args[1].get('project')) def test_write_gitolite_project_test_private(self): """ Test the write_gitolite_acls function of pagure.lib.git with