diff --git a/pagure/api/fork.py b/pagure/api/fork.py index eedd6f3..fa4c74b 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -362,14 +362,14 @@ def api_pull_request_merge(repo, requestid, username=None, namespace=None): raise pagure.exceptions.APIError(403, error_code=APIERROR.EPRSCORE) try: - taskid = pagure.lib.tasks.merge_pull_request.delay( + task = pagure.lib.tasks.merge_pull_request.delay( repo.name, namespace, username, requestid, - flask.g.fas_user.username).id + flask.g.fas_user.username) output = {'message': 'Merging queued', - 'taskid': taskid} + 'taskid': task.id} if flask.request.form.get('wait', True): - pagure.lib.tasks.get_result(taskid).get() + task.get() output = {'message': 'Changes merged!'} except pagure.exceptions.PagureException as err: raise pagure.exceptions.APIError( diff --git a/pagure/api/project.py b/pagure/api/project.py index b5bba89..70d2dcf 100644 --- a/pagure/api/project.py +++ b/pagure/api/project.py @@ -841,7 +841,7 @@ def api_new_project(): private = form.private.data try: - taskid = pagure.lib.new_project( + task = pagure.lib.new_project( flask.g.session, name=name, namespace=namespace, @@ -864,10 +864,10 @@ def api_new_project(): ) flask.g.session.commit() output = {'message': 'Project creation queued', - 'taskid': taskid} + 'taskid': task.id} if flask.request.form.get('wait', True): - result = pagure.lib.tasks.get_result(taskid).get() + result = task.get() project = pagure.lib._get_project( flask.g.session, name=result['repo'], namespace=result['namespace'], @@ -1103,7 +1103,7 @@ def api_fork_project(): 404, error_code=APIERROR.ENOPROJECT) try: - taskid = pagure.lib.fork_project( + task = pagure.lib.fork_project( flask.g.session, user=flask.g.fas_user.username, repo=repo, @@ -1114,10 +1114,10 @@ def api_fork_project(): ) flask.g.session.commit() output = {'message': 'Project forking queued', - 'taskid': taskid} + 'taskid': task.id} if flask.request.form.get('wait', True): - pagure.lib.tasks.get_result(taskid).get() + task.get() output = {'message': 'Repo "%s" cloned to "%s/%s"' % (repo.fullname, flask.g.fas_user.username, repo.fullname)} @@ -1203,16 +1203,16 @@ def api_generate_acls(repo, username=None, namespace=None): wait = str(flask.request.form.get('wait')).lower() in ['true', '1'] try: - taskid = pagure.lib.git.generate_gitolite_acls( + task = pagure.lib.git.generate_gitolite_acls( project=project, - ).id + ) if wait: - pagure.lib.tasks.get_result(taskid).get() + task.get() output = {'message': 'Project ACLs generated'} else: output = {'message': 'Project ACL generation queued', - 'taskid': taskid} + 'taskid': task.id} except pagure.exceptions.PagureException as err: raise pagure.exceptions.APIError( 400, error_code=APIERROR.ENOCODE, error=str(err)) diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 4ccc5b5..f4e0877 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1578,7 +1578,7 @@ def new_project(session, user, name, blacklist, allowed_prefix, ) return tasks.create_project.delay(user_obj.username, namespace, name, - add_readme, ignore_existing_repo).id + add_readme, ignore_existing_repo) def new_issue(session, repo, title, content, user, ticketfolder, issue_id=None, @@ -2063,7 +2063,7 @@ def fork_project(session, user, repo, gitfolder, user, editbranch, editfile) - return task.id + return task def search_projects( diff --git a/pagure/ui/app.py b/pagure/ui/app.py index f8d3476..786b6e7 100644 --- a/pagure/ui/app.py +++ b/pagure/ui/app.py @@ -28,6 +28,7 @@ from pagure.utils import ( authenticated, is_safe_url, login_required, + get_task_redirect_url, ) @@ -513,7 +514,7 @@ def new_project(): namespace = namespace.strip() try: - taskid = pagure.lib.new_project( + task = pagure.lib.new_project( flask.g.session, name=name, private=private, @@ -535,7 +536,7 @@ def new_project(): user_ns=pagure_config.get('USER_NAMESPACE', False), ) flask.g.session.commit() - return pagure.utils.wait_for_task(taskid) + return pagure.utils.wait_for_task(task) except pagure.exceptions.PagureException as err: flask.flash(str(err), 'error') except SQLAlchemyError as err: # pragma: no cover @@ -574,16 +575,7 @@ def wait_task(taskid): if task.ready(): if is_js: flask.abort(417) - - result = task.get(timeout=0, propagate=False) - if task.failed(): - flask.flash('Your task failed: %s' % str(result)) - task.forget() - return flask.redirect(prev) - endpoint = result.pop('endpoint') - task.forget() - return flask.redirect( - flask.url_for(endpoint, **result)) + return flask.redirect(get_task_redirect_url(task, prev)) else: if is_js: return flask.jsonify({ diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index 63a112f..1424130 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -778,11 +778,11 @@ def merge_request_pull(repo, requestid, username=None, namespace=None): _log.info('All checks in the controller passed') try: - taskid = pagure.lib.tasks.merge_pull_request.delay( + task = pagure.lib.tasks.merge_pull_request.delay( repo.name, namespace, username, requestid, flask.g.fas_user.username) return pagure.utils.wait_for_task( - taskid, + task, prev=flask.url_for('ui_ns.request_pull', repo=repo.name, namespace=namespace, @@ -896,10 +896,10 @@ def refresh_request_pull(repo, requestid, username=None, namespace=None): 403, 'You are not allowed to refresh this pull request') - taskid = pagure.lib.tasks.refresh_remote_pr.delay( + task = pagure.lib.tasks.refresh_remote_pr.delay( flask.g.repo.name, namespace, username, requestid) return pagure.utils.wait_for_task( - taskid, + task, prev=flask.url_for('ui_ns.request_pull', repo=flask.g.repo.name, namespace=namespace, @@ -1035,7 +1035,7 @@ def fork_project(repo, username=None, namespace=None): namespace=namespace)) try: - taskid = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=flask.g.session, repo=repo, gitfolder=pagure_config['GIT_FOLDER'], @@ -1046,7 +1046,7 @@ def fork_project(repo, username=None, namespace=None): flask.g.session.commit() return pagure.utils.wait_for_task( - taskid, + task, prev=flask.url_for( 'ui_ns.view_repo', repo=repo.name, username=username, namespace=namespace, @@ -1477,7 +1477,7 @@ def fork_edit_file( )) try: - taskid = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=flask.g.session, repo=repo, gitfolder=pagure_config['GIT_FOLDER'], @@ -1489,7 +1489,7 @@ def fork_edit_file( editfile=filename) flask.g.session.commit() - return pagure.utils.wait_for_task(taskid) + return pagure.utils.wait_for_task(task) except pagure.exceptions.PagureException as err: flask.flash(str(err), 'error') except SQLAlchemyError as err: # pragma: no cover diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index fa8e83f..ff89933 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -1429,7 +1429,7 @@ def delete_repo(repo, username=None, namespace=None): name=repo.name, user=repo.user.user if repo.is_fork else None, action_user=flask.g.fas_user.username) - return pagure.utils.wait_for_task(task.id) + return pagure.utils.wait_for_task(task) @UI_NS.route('//hook_token', methods=['POST']) @@ -2048,7 +2048,7 @@ def edit_file(repo, branchname, filename, username=None, namespace=None): if form.validate_on_submit(): try: - taskid = pagure.lib.tasks.update_file_in_git.delay( + task = pagure.lib.tasks.update_file_in_git.delay( repo.name, repo.namespace, repo.user.username if repo.is_fork else None, @@ -2063,9 +2063,8 @@ def edit_file(repo, branchname, filename, username=None, namespace=None): username=user.username, email=form.email.data, runhook=True, - ).id - return flask.redirect(flask.url_for( - 'ui_ns.wait_task', taskid=taskid)) + ) + return pagure.utils.wait_for_task(task) except pagure.exceptions.PagureException as err: # pragma: no cover _log.exception(err) flask.flash('Commit could not be done', 'error') @@ -2132,9 +2131,9 @@ def delete_branch(repo, branchname, username=None, namespace=None): if branchname not in repo_obj.listall_branches(): flask.abort(404, 'Branch not found') - taskid = pagure.lib.tasks.delete_branch.delay(repo, namespace, username, - branchname).id - return pagure.utils.wait_for_task(taskid) + task = pagure.lib.tasks.delete_branch.delay(repo, namespace, username, + branchname) + return pagure.utils.wait_for_task(task) @UI_NS.route('/docs//') @@ -2568,10 +2567,10 @@ def project_dowait(repo, username=None, namespace=None): if not pagure_config.get('ALLOW_PROJECT_DOWAIT', False): flask.abort(401, 'No') - taskid = pagure.lib.tasks.project_dowait.delay( - name=repo, namespace=namespace, user=username).id + task = pagure.lib.tasks.project_dowait.delay( + name=repo, namespace=namespace, user=username) - return pagure.utils.wait_for_task(taskid) + return pagure.utils.wait_for_task(task) @UI_NS.route('//stats/') diff --git a/pagure/utils.py b/pagure/utils.py index 0a82058..48360ab 100644 --- a/pagure/utils.py +++ b/pagure/utils.py @@ -320,13 +320,28 @@ def get_remote_repo_path(remote_git, branch_from, ignore_non_exist=False): return repopath -def wait_for_task(taskid, prev=None): +def get_task_redirect_url(task, prev): + if not task.ready(): + return flask.url_for( + 'ui_ns.wait_task', + taskid=task.id, + prev=prev) + result = task.get(timeout=0, propagate=False) + if task.failed(): + flask.flash('Your task failed: %s' % str(result)) + task.forget() + return prev + endpoint = result.pop('endpoint') + task.forget() + return flask.url_for(endpoint, **result) + + +def wait_for_task(task, prev=None): if prev is None: prev = flask.request.full_path - return flask.redirect(flask.url_for( - 'ui_ns.wait_task', - taskid=taskid, - prev=prev)) + elif not is_safe_url(prev): + prev = flask.url_for('index') + return flask.redirect(get_task_redirect_url(task, prev)) def wait_for_task_post(taskid, form, endpoint, initial=False, **kwargs): diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index a950152..ca357a1 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -2616,9 +2616,8 @@ class PagureFlaskApiProjecttests(tests.Modeltests): mock_gen_acls.assert_called_once_with( name='test', namespace=None, user=None, group=None) - @patch('pagure.lib.tasks.get_result') @patch('pagure.lib.tasks.generate_gitolite_acls.delay') - def test_api_generate_acls_wait_true(self, mock_gen_acls, mock_get_result): + def test_api_generate_acls_wait_true(self, mock_gen_acls): """ Test the api_generate_acls method of the flask api when wait is set to True """ tests.create_projects(self.session) @@ -2631,9 +2630,6 @@ class PagureFlaskApiProjecttests(tests.Modeltests): mock_gen_acls_rv.id = 'abc-1234' mock_gen_acls.return_value = mock_gen_acls_rv - mock_get_result_rv = Mock() - mock_get_result.return_value = mock_get_result_rv - user = pagure.lib.get_user(self.session, 'pingou') with tests.user_set(self.app.application, user): output = self.app.post( @@ -2647,7 +2643,6 @@ class PagureFlaskApiProjecttests(tests.Modeltests): self.assertEqual(data, expected_output) mock_gen_acls.assert_called_once_with( name='test', namespace=None, user=None, group=None) - mock_get_result.assert_called_once_with('abc-1234') def test_api_generate_acls_no_project(self): """ Test the api_generate_acls method of the flask api when the project diff --git a/tests/test_pagure_flask_ui_plugins_default_hook.py b/tests/test_pagure_flask_ui_plugins_default_hook.py index 15863f9..462afed 100644 --- a/tests/test_pagure_flask_ui_plugins_default_hook.py +++ b/tests/test_pagure_flask_ui_plugins_default_hook.py @@ -46,7 +46,7 @@ class PagureFlaskPluginDefaultHooktests(tests.Modeltests): project is created. """ - taskid = pagure.lib.new_project( + task = pagure.lib.new_project( self.session, user='pingou', name='test', @@ -64,8 +64,7 @@ class PagureFlaskPluginDefaultHooktests(tests.Modeltests): prevent_40_chars=False, namespace=None ) - result = pagure.lib.tasks.get_result(taskid).get() - self.assertEqual(result, + self.assertEqual(task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'test', 'namespace': None}) diff --git a/tests/test_pagure_flask_ui_repo_delete_project.py b/tests/test_pagure_flask_ui_repo_delete_project.py index 7eaecf4..7641293 100644 --- a/tests/test_pagure_flask_ui_repo_delete_project.py +++ b/tests/test_pagure_flask_ui_repo_delete_project.py @@ -53,7 +53,7 @@ class PagureFlaskDeleteRepotests(tests.Modeltests): self.session.commit() # Create a fork - task_id = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=self.session, user='pingou', repo=project, @@ -62,7 +62,7 @@ class PagureFlaskDeleteRepotests(tests.Modeltests): ticketfolder=os.path.join(self.path, 'repos', 'tickets'), requestfolder=os.path.join(self.path, 'repos', 'requests'), ) - pagure.lib.tasks.get_result(task_id).get() + task.get() # Ensure everything was correctly created projects = pagure.lib.search_projects(self.session) diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index e4675fa..6f5e53b 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -1600,7 +1600,7 @@ class PagureLibtests(tests.Modeltests): # Create a new project pagure.config.config['GIT_FOLDER'] = gitfolder - tid = pagure.lib.new_project( + task = pagure.lib.new_project( session=self.session, user='pingou', name='testproject', @@ -1614,9 +1614,8 @@ class PagureLibtests(tests.Modeltests): parent_id=None, ) self.session.commit() - result = pagure.lib.tasks.get_result(tid).get() self.assertEqual( - result, + task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'testproject', 'namespace': None}) @@ -1700,7 +1699,7 @@ class PagureLibtests(tests.Modeltests): self.session.commit() self.session = pagure.lib.create_session(self.dbpath) - tid = pagure.lib.new_project( + task = pagure.lib.new_project( session=self.session, user='pingou', name='testproject', @@ -1715,9 +1714,8 @@ class PagureLibtests(tests.Modeltests): ignore_existing_repo=True ) self.session.commit() - result = pagure.lib.tasks.get_result(tid).get() self.assertEqual( - result, + task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'testproject', 'namespace': None}) @@ -1734,7 +1732,7 @@ class PagureLibtests(tests.Modeltests): # Drop the main git repo and try again shutil.rmtree(gitrepo) - tid = pagure.lib.new_project( + task = pagure.lib.new_project( session=self.session, user='pingou', name='testproject', @@ -1748,7 +1746,7 @@ class PagureLibtests(tests.Modeltests): parent_id=None) self.assertIn( 'already exists', - str(pagure.lib.tasks.get_result(tid).get(propagate=False))) + str(task.get(propagate=False))) self.session.rollback() self.assertFalse(os.path.exists(gitrepo)) @@ -1803,7 +1801,7 @@ class PagureLibtests(tests.Modeltests): self.assertTrue(os.path.exists(requestrepo)) # Re-Try creating a 40 chars project this time allowing it - tid = pagure.lib.new_project( + task = pagure.lib.new_project( session=self.session, user='pingou', name='pingou/' + 's' * 40, @@ -1817,9 +1815,8 @@ class PagureLibtests(tests.Modeltests): parent_id=None, ) self.session.commit() - result = pagure.lib.tasks.get_result(tid).get() self.assertEqual( - result, + task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'pingou/ssssssssssssssssssssssssssssssssssssssss', 'namespace': None}) @@ -1833,7 +1830,7 @@ class PagureLibtests(tests.Modeltests): # Create a new project with user_ns as True pagure.config.config['GIT_FOLDER'] = gitfolder - tid = pagure.lib.new_project( + task = pagure.lib.new_project( session=self.session, user='pingou', name='testproject', @@ -1848,9 +1845,8 @@ class PagureLibtests(tests.Modeltests): user_ns=True, ) self.session.commit() - result = pagure.lib.tasks.get_result(tid).get() self.assertEqual( - result, + task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'testproject', 'namespace': 'pingou'}) @@ -1870,7 +1866,7 @@ class PagureLibtests(tests.Modeltests): # Create a new project with a namespace and user_ns as True pagure.config.config['GIT_FOLDER'] = gitfolder - tid = pagure.lib.new_project( + task = pagure.lib.new_project( session=self.session, user='pingou', name='testproject2', @@ -1886,9 +1882,8 @@ class PagureLibtests(tests.Modeltests): user_ns=True, ) self.session.commit() - result = pagure.lib.tasks.get_result(tid).get() self.assertEqual( - result, + task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'testproject2', 'namespace': 'testns'}) @@ -2227,7 +2222,7 @@ class PagureLibtests(tests.Modeltests): self.assertEqual(len(projects), 0) # Create a new project - tid = pagure.lib.new_project( + task = pagure.lib.new_project( session=self.session, user='pingou', name='testproject', @@ -2241,9 +2236,8 @@ class PagureLibtests(tests.Modeltests): parent_id=None, ) self.session.commit() - result = pagure.lib.tasks.get_result(tid).get() self.assertEqual( - result, + task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'testproject', 'namespace': None}) @@ -2270,7 +2264,7 @@ class PagureLibtests(tests.Modeltests): # Fork - tid = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=self.session, user='foo', repo=project, @@ -2280,9 +2274,8 @@ class PagureLibtests(tests.Modeltests): requestfolder=requestfolder, ) self.session.commit() - result = pagure.lib.tasks.get_result(tid).get() self.assertEqual( - result, + task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'testproject', 'namespace': None, @@ -2310,7 +2303,7 @@ class PagureLibtests(tests.Modeltests): self.assertEqual(len(projects), 0) # Create a new project - taskid = pagure.lib.new_project( + task = pagure.lib.new_project( session=self.session, user='pingou', name='testproject', @@ -2325,8 +2318,7 @@ class PagureLibtests(tests.Modeltests): parent_id=None, ) self.session.commit() - result = pagure.lib.tasks.get_result(taskid).get() - self.assertEqual(result, + self.assertEqual(task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'testproject', 'namespace': 'foonamespace'}) @@ -2367,7 +2359,7 @@ class PagureLibtests(tests.Modeltests): grepo = '%s.git' % os.path.join( docfolder, 'forks', 'foo', 'foonamespace', 'testproject') os.makedirs(grepo) - tid = pagure.lib.fork_project(session=self.session, + task = pagure.lib.fork_project(session=self.session, user='foo', repo=repo, gitfolder=gitfolder, @@ -2376,7 +2368,7 @@ class PagureLibtests(tests.Modeltests): requestfolder=requestfolder) self.assertIn( 'already exists', - str(pagure.lib.tasks.get_result(tid).get(propagate=False))) + str(task.get(propagate=False))) self.session.rollback() shutil.rmtree(grepo) @@ -2384,7 +2376,7 @@ class PagureLibtests(tests.Modeltests): grepo = '%s.git' % os.path.join( ticketfolder, 'forks', 'foo', 'foonamespace', 'testproject') os.makedirs(grepo) - tid = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=self.session, user='foo', repo=repo, @@ -2394,7 +2386,7 @@ class PagureLibtests(tests.Modeltests): requestfolder=requestfolder) self.assertIn( 'already exists', - str(pagure.lib.tasks.get_result(tid).get(propagate=False))) + str(task.get(propagate=False))) self.session.rollback() shutil.rmtree(grepo) @@ -2402,7 +2394,7 @@ class PagureLibtests(tests.Modeltests): grepo = '%s.git' % os.path.join( requestfolder, 'forks', 'foo', 'foonamespace', 'testproject') os.makedirs(grepo) - tid = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=self.session, user='foo', repo=repo, @@ -2412,13 +2404,13 @@ class PagureLibtests(tests.Modeltests): requestfolder=requestfolder) self.assertIn( 'already exists', - str(pagure.lib.tasks.get_result(tid).get(propagate=False))) + str(task.get(propagate=False))) self.session.rollback() shutil.rmtree(grepo) # Fork worked - tid = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=self.session, user='foo', repo=repo, @@ -2428,9 +2420,8 @@ class PagureLibtests(tests.Modeltests): requestfolder=requestfolder, ) self.session.commit() - result = pagure.lib.tasks.get_result(tid).get() self.assertEqual( - result, + task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'testproject', 'namespace': 'foonamespace', @@ -2440,7 +2431,7 @@ class PagureLibtests(tests.Modeltests): repo = pagure.lib._get_project(self.session, 'testproject', user='foo', namespace='foonamespace') - tid = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=self.session, user='pingou', repo=repo, @@ -2450,9 +2441,8 @@ class PagureLibtests(tests.Modeltests): requestfolder=requestfolder, ) self.session.commit() - result = pagure.lib.tasks.get_result(tid).get() self.assertEqual( - result, + task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'testproject', 'namespace': 'foonamespace', diff --git a/tests/test_pagure_lib_git.py b/tests/test_pagure_lib_git.py index a041b4d..721aebf 100644 --- a/tests/test_pagure_lib_git.py +++ b/tests/test_pagure_lib_git.py @@ -3025,7 +3025,7 @@ index 0000000..60f7480 repo_obj = pygit2.init_repository(self.gitrepo, bare=True) # Fork the project - taskid = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=self.session, user='foo', repo=repo, @@ -3035,8 +3035,7 @@ index 0000000..60f7480 requestfolder=requestfolder, ) self.session.commit() - result = pagure.lib.tasks.get_result(taskid).get() - self.assertEqual(result, + self.assertEqual(task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'test', 'username': 'foo', @@ -3113,7 +3112,7 @@ index 0000000..60f7480 tests.add_content_git_repo(self.gitrepo, branch='master') # Fork the project - taskid = pagure.lib.fork_project( + task = pagure.lib.fork_project( session=self.session, user='foo', repo=repo, @@ -3123,8 +3122,7 @@ index 0000000..60f7480 requestfolder=requestfolder, ) self.session.commit() - result = pagure.lib.tasks.get_result(taskid).get() - self.assertEqual(result, + self.assertEqual(task.get(), {'endpoint': 'ui_ns.view_repo', 'repo': 'test', 'username': 'foo',