You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by jo...@apache.org on 2013/09/05 20:21:08 UTC
[43/50] git commit: [#6530] Improved error handling on import_tool
task
[#6530] Improved error handling on import_tool task
Signed-off-by: Cory Johns <cj...@slashdotmedia.com>
Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/43f56c26
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/43f56c26
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/43f56c26
Branch: refs/heads/cj/6596
Commit: 43f56c26f225c0a5835a0c80c1ed374f240be829
Parents: 19070bd
Author: Cory Johns <cj...@slashdotmedia.com>
Authored: Thu Aug 22 20:59:59 2013 +0000
Committer: Dave Brondsema <db...@slashdotmedia.com>
Committed: Wed Sep 4 14:30:55 2013 +0000
----------------------------------------------------------------------
Allura/allura/lib/helpers.py | 6 ++++
Allura/allura/tests/test_helpers.py | 34 ++++++++++++++++++++
ForgeImporters/forgeimporters/base.py | 20 +++++++++---
ForgeImporters/forgeimporters/google/tracker.py | 3 ++
.../forgeimporters/tests/google/test_tracker.py | 14 ++++++++
.../forgeimporters/tests/test_base.py | 20 +++++++++++-
.../forgeimporters/trac/tests/test_tickets.py | 33 +++++++++++++++----
ForgeImporters/forgeimporters/trac/tickets.py | 31 ++++++++++--------
8 files changed, 137 insertions(+), 24 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/43f56c26/Allura/allura/lib/helpers.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/helpers.py b/Allura/allura/lib/helpers.py
index adfb339..0e13fad 100644
--- a/Allura/allura/lib/helpers.py
+++ b/Allura/allura/lib/helpers.py
@@ -214,6 +214,12 @@ def _make_xs(X, ids):
result = (results.get(i) for i in ids)
return (r for r in result if r is not None)
+def make_app_admin_only(app):
+ from allura.model.auth import ProjectRole
+ admin_role = ProjectRole.by_name('Admin', app.project)
+ for ace in [ace for ace in app.acl if ace.role_id != admin_role._id]:
+ app.acl.remove(ace)
+
@contextmanager
def push_config(obj, **kw):
saved_attrs = {}
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/43f56c26/Allura/allura/tests/test_helpers.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/test_helpers.py b/Allura/allura/tests/test_helpers.py
index 3d4ad96..e9bc8f9 100644
--- a/Allura/allura/tests/test_helpers.py
+++ b/Allura/allura/tests/test_helpers.py
@@ -26,10 +26,13 @@ from nose.tools import eq_, assert_equals
from IPython.testing.decorators import skipif, module_not_available
from datadiff import tools as dd
from webob import Request
+from ming.orm import ThreadLocalORMSession
from allura import model as M
from allura.lib import helpers as h
from allura.lib.search import inject_user
+from allura.lib.security import has_access
+from allura.lib.security import Credentials
from allura.tests import decorators as td
from alluratest.controller import setup_basic_test
@@ -104,6 +107,37 @@ def test_make_roles():
assert h.make_roles([pr._id]).next() == pr
@td.with_wiki
+def test_make_app_admin_only():
+ h.set_context('test', 'wiki', neighborhood='Projects')
+ anon = M.User.anonymous()
+ dev = M.User.query.get(username='test-user')
+ admin = M.User.query.get(username='test-admin')
+ c.project.add_user(dev, ['Developer'])
+ ThreadLocalORMSession.flush_all()
+ Credentials.get().clear()
+ assert has_access(c.app, 'read', user=anon)()
+ assert has_access(c.app, 'read', user=dev)()
+ assert has_access(c.app, 'read', user=admin)()
+ assert not has_access(c.app, 'create', user=anon)()
+ assert has_access(c.app, 'create', user=dev)()
+ assert has_access(c.app, 'create', user=admin)()
+ assert c.app.is_visible_to(anon)
+ assert c.app.is_visible_to(dev)
+ assert c.app.is_visible_to(admin)
+ h.make_app_admin_only(c.app)
+ ThreadLocalORMSession.flush_all()
+ Credentials.get().clear()
+ assert not has_access(c.app, 'read', user=anon)()
+ assert not has_access(c.app, 'read', user=dev)()
+ assert has_access(c.app, 'read', user=admin)()
+ assert not has_access(c.app, 'create', user=anon)()
+ assert not has_access(c.app, 'create', user=dev)()
+ assert has_access(c.app, 'create', user=admin)()
+ assert not c.app.is_visible_to(anon)
+ assert not c.app.is_visible_to(dev)
+ assert c.app.is_visible_to(admin)
+
+@td.with_wiki
def test_context_setters():
h.set_context('test', 'wiki', neighborhood='Projects')
assert c.project is not None
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/43f56c26/ForgeImporters/forgeimporters/base.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/base.py b/ForgeImporters/forgeimporters/base.py
index 7f25e69..69db8d6 100644
--- a/ForgeImporters/forgeimporters/base.py
+++ b/ForgeImporters/forgeimporters/base.py
@@ -19,11 +19,13 @@ import logging
import urllib
import urllib2
from collections import defaultdict
+import traceback
from BeautifulSoup import BeautifulSoup
from tg import expose, validate, flash, redirect, config
from tg.decorators import with_trailing_slash
-from pylons import tmpl_context as c, app_globals as g
+from pylons import app_globals as g
+from pylons import tmpl_context as c
from formencode import validators as fev, schema
from webob import exc
@@ -66,9 +68,19 @@ class ToolImportForm(schema.Schema):
@task(notifications_disabled=True)
def import_tool(importer_name, project_name=None, mount_point=None, mount_label=None, **kw):
- importer = ToolImporter.by_name(importer_name)
- importer.import_tool(c.project, c.user, project_name=project_name,
- mount_point=mount_point, mount_label=mount_label, **kw)
+ try:
+ importer = ToolImporter.by_name(importer_name)
+ importer.import_tool(c.project, c.user, project_name=project_name,
+ mount_point=mount_point, mount_label=mount_label, **kw)
+ except Exception as e:
+ g.post_event('import_tool_task_failed',
+ error=str(e),
+ traceback=traceback.format_exc(),
+ importer_name=importer_name,
+ project_name=project_name,
+ mount_point=mount_point,
+ mount_label=mount_label,
+ **kw)
class ProjectExtractor(object):
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/43f56c26/ForgeImporters/forgeimporters/google/tracker.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/google/tracker.py b/ForgeImporters/forgeimporters/google/tracker.py
index b7fccbe..eb97c07 100644
--- a/ForgeImporters/forgeimporters/google/tracker.py
+++ b/ForgeImporters/forgeimporters/google/tracker.py
@@ -135,6 +135,9 @@ class GoogleCodeTrackerImporter(ToolImporter):
g.post_event('project_updated')
app.globals.invalidate_bin_counts()
return app
+ except Exception as e:
+ h.make_app_admin_only(app)
+ raise
finally:
M.session.artifact_orm_session._get().skip_mod_date = False
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/43f56c26/ForgeImporters/forgeimporters/tests/google/test_tracker.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/tests/google/test_tracker.py b/ForgeImporters/forgeimporters/tests/google/test_tracker.py
index 3c7f0aa..5bbf836 100644
--- a/ForgeImporters/forgeimporters/tests/google/test_tracker.py
+++ b/ForgeImporters/forgeimporters/tests/google/test_tracker.py
@@ -82,6 +82,20 @@ class TestTrackerImporter(TestCase):
g.post_event.assert_called_once_with('project_updated')
app.globals.invalidate_bin_counts.assert_called_once_with()
+ @mock.patch.object(tracker, 'ThreadLocalORMSession')
+ @mock.patch.object(tracker, 'M')
+ @mock.patch.object(tracker, 'h')
+ def test_import_tool_failure(self, h, M, ThreadLocalORMSession):
+ h.push_config.side_effect = ValueError
+ project = mock.Mock()
+ user = mock.Mock()
+
+ importer = tracker.GoogleCodeTrackerImporter()
+ self.assertRaises(ValueError, importer.import_tool, project, user, project_name='project_name',
+ mount_point='mount_point', mount_label='mount_label')
+
+ h.make_app_admin_only.assert_called_once_with(project.install_app.return_value)
+
def test_custom_fields(self):
importer = tracker.GoogleCodeTrackerImporter()
importer.custom_fields = {}
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/43f56c26/ForgeImporters/forgeimporters/tests/test_base.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/tests/test_base.py b/ForgeImporters/forgeimporters/tests/test_base.py
index 92fb0cc..a61ecda 100644
--- a/ForgeImporters/forgeimporters/tests/test_base.py
+++ b/ForgeImporters/forgeimporters/tests/test_base.py
@@ -42,7 +42,8 @@ class TestProjectExtractor(TestCase):
@mock.patch.object(base.ToolImporter, 'by_name')
@mock.patch.object(base, 'c')
-def test_import_tool(c, by_name):
+@mock.patch.object(base, 'g')
+def test_import_tool(g, c, by_name):
c.project = mock.Mock(name='project')
c.user = mock.Mock(name='user')
base.import_tool('importer_name', project_name='project_name',
@@ -51,6 +52,23 @@ def test_import_tool(c, by_name):
by_name.return_value.import_tool.assert_called_once_with(c.project,
c.user, project_name='project_name', mount_point='mount_point',
mount_label='mount_label')
+ assert not g.post_event.called
+
+@mock.patch.object(base.ToolImporter, 'by_name')
+@mock.patch.object(base, 'g')
+def test_import_tool_failed(g, by_name):
+ by_name.side_effect = RuntimeError('my error')
+ base.import_tool('importer_name', project_name='project_name',
+ mount_point='mount_point', mount_label='mount_label', other='other')
+ g.post_event.assert_called_once_with(
+ 'import_tool_task_failed',
+ error=by_name.side_effect,
+ importer_name='importer_name',
+ project_name='project_name',
+ mount_point='mount_point',
+ mount_label='mount_label',
+ other='other',
+ )
def ep(name, source=None, importer=None, **kw):
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/43f56c26/ForgeImporters/forgeimporters/trac/tests/test_tickets.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/trac/tests/test_tickets.py b/ForgeImporters/forgeimporters/trac/tests/test_tickets.py
index 953c6d7..74e1049 100644
--- a/ForgeImporters/forgeimporters/trac/tests/test_tickets.py
+++ b/ForgeImporters/forgeimporters/trac/tests/test_tickets.py
@@ -46,12 +46,13 @@ class TestTracTicketImporter(TestCase):
project = Mock(name='Project', shortname='myproject')
project.install_app.return_value = app
user = Mock(name='User', _id='id')
- res = importer.import_tool(project, user,
- mount_point='bugs',
- mount_label='Bugs',
- trac_url='http://example.com/trac/url',
- user_map=json.dumps(user_map),
- )
+ with patch.dict('forgeimporters.trac.tickets.config', {'base_url': 'foo'}):
+ res = importer.import_tool(project, user,
+ mount_point='bugs',
+ mount_label='Bugs',
+ trac_url='http://example.com/trac/url',
+ user_map=json.dumps(user_map),
+ )
self.assertEqual(res, app)
project.install_app.assert_called_once_with(
'Tickets', mount_point='bugs', mount_label='Bugs')
@@ -68,6 +69,26 @@ class TestTracTicketImporter(TestCase):
validate=False)
g.post_event.assert_called_once_with('project_updated')
+ @patch('forgeimporters.trac.tickets.session')
+ @patch('forgeimporters.trac.tickets.h')
+ @patch('forgeimporters.trac.tickets.TracExport')
+ def test_import_tool_failure(self, TracExport, h, session):
+ importer = TracTicketImporter()
+ app = Mock(name='ForgeTrackerApp')
+ project = Mock(name='Project', shortname='myproject')
+ project.install_app.return_value = app
+ user = Mock(name='User', _id='id')
+ TracExport.side_effect = ValueError
+
+ self.assertRaises(ValueError, importer.import_tool, project, user,
+ mount_point='bugs',
+ mount_label='Bugs',
+ trac_url='http://example.com/trac/url',
+ user_map=None,
+ )
+
+ h.make_app_admin_only.assert_called_once_with(app)
+
class TestTracTicketImportController(TestController, TestCase):
def setUp(self):
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/43f56c26/ForgeImporters/forgeimporters/trac/tickets.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/trac/tickets.py b/ForgeImporters/forgeimporters/trac/tickets.py
index 3e0e486..941ff32 100644
--- a/ForgeImporters/forgeimporters/trac/tickets.py
+++ b/ForgeImporters/forgeimporters/trac/tickets.py
@@ -42,6 +42,7 @@ from allura.controllers import BaseController
from allura.lib.decorators import require_post, task
from allura.lib.import_api import AlluraImportApiClient
from allura.lib import validators as v
+from allura.lib import helpers as h
from allura.model import ApiTicket
from allura.scripts.trac_export import (
TracExport,
@@ -116,16 +117,20 @@ class TracTicketImporter(ToolImporter):
)
session(app.config).flush(app.config)
session(app.globals).flush(app.globals)
- export = [ticket for ticket in TracExport(trac_url)]
- export_string = json.dumps(export, cls=DateJSONEncoder)
- api_ticket = ApiTicket(user_id=user._id,
- capabilities={"import": ["Projects", project.shortname]},
- expires=datetime.utcnow() + timedelta(minutes=60))
- session(api_ticket).flush(api_ticket)
- cli = AlluraImportApiClient(config['base_url'], api_ticket.api_key,
- api_ticket.secret_key, verbose=True)
- import_tracker(cli, project.shortname, mount_point,
- {'user_map': json.loads(user_map) if user_map else {}},
- export_string, validate=False)
- g.post_event('project_updated')
- return app
+ try:
+ export = [ticket for ticket in TracExport(trac_url)]
+ export_string = json.dumps(export, cls=DateJSONEncoder)
+ api_ticket = ApiTicket(user_id=user._id,
+ capabilities={"import": ["Projects", project.shortname]},
+ expires=datetime.utcnow() + timedelta(minutes=60))
+ session(api_ticket).flush(api_ticket)
+ cli = AlluraImportApiClient(config['base_url'], api_ticket.api_key,
+ api_ticket.secret_key, verbose=True)
+ import_tracker(cli, project.shortname, mount_point,
+ {'user_map': json.loads(user_map) if user_map else {}},
+ export_string, validate=False)
+ g.post_event('project_updated')
+ return app
+ except Exception as e:
+ h.make_app_admin_only(app)
+ raise