incubator-allura-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From john...@apache.org
Subject git commit: [#6530] Improved error handling on import_tool task
Date Fri, 23 Aug 2013 16:57:16 GMT
Updated Branches:
  refs/heads/cj/6530 [created] c75da0b48


[#6530] Improved error handling on import_tool task

Signed-off-by: Cory Johns <cjohns@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/c75da0b4
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/c75da0b4
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/c75da0b4

Branch: refs/heads/cj/6530
Commit: c75da0b489fceb09419682c68c96f70328648739
Parents: d1f150e
Author: Cory Johns <cjohns@slashdotmedia.com>
Authored: Thu Aug 22 20:59:59 2013 +0000
Committer: Cory Johns <cjohns@slashdotmedia.com>
Committed: Thu Aug 22 21:55:20 2013 +0000

----------------------------------------------------------------------
 Allura/allura/lib/helpers.py                    |  6 ++++
 Allura/allura/tests/test_helpers.py             | 34 ++++++++++++++++++++
 ForgeImporters/forgeimporters/base.py           | 18 +++++++++--
 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, 136 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/c75da0b4/Allura/allura/lib/helpers.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/helpers.py b/Allura/allura/lib/helpers.py
index e68eafb..f38ec8e 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/c75da0b4/Allura/allura/tests/test_helpers.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/test_helpers.py b/Allura/allura/tests/test_helpers.py
index 4a0b80d..fb81224 100644
--- a/Allura/allura/tests/test_helpers.py
+++ b/Allura/allura/tests/test_helpers.py
@@ -25,10 +25,13 @@ from pylons import tmpl_context as c
 from nose.tools import eq_, assert_equals
 from IPython.testing.decorators import skipif, module_not_available
 from datadiff import tools as dd
+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
 
@@ -103,6 +106,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/c75da0b4/ForgeImporters/forgeimporters/base.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/base.py b/ForgeImporters/forgeimporters/base.py
index 49397a5..3951c37 100644
--- a/ForgeImporters/forgeimporters/base.py
+++ b/ForgeImporters/forgeimporters/base.py
@@ -17,11 +17,13 @@
 
 import logging
 import urllib2
+import traceback
 
 from pkg_resources import iter_entry_points
 
 from tg import expose, validate, flash, redirect, config
 from tg.decorators import with_trailing_slash
+from pylons import app_globals as g
 from pylons import tmpl_context as c
 from formencode import validators as fev, schema
 
@@ -55,9 +57,19 @@ class ProjectImportForm(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/c75da0b4/ForgeImporters/forgeimporters/google/tracker.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/google/tracker.py b/ForgeImporters/forgeimporters/google/tracker.py
index 8b1747f..6fa55d5 100644
--- a/ForgeImporters/forgeimporters/google/tracker.py
+++ b/ForgeImporters/forgeimporters/google/tracker.py
@@ -77,6 +77,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/c75da0b4/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 4a7c28f..8a97913 100644
--- a/ForgeImporters/forgeimporters/tests/google/test_tracker.py
+++ b/ForgeImporters/forgeimporters/tests/google/test_tracker.py
@@ -79,6 +79,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/c75da0b4/ForgeImporters/forgeimporters/tests/test_base.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/tests/test_base.py b/ForgeImporters/forgeimporters/tests/test_base.py
index 57ed227..8c04947 100644
--- a/ForgeImporters/forgeimporters/tests/test_base.py
+++ b/ForgeImporters/forgeimporters/tests/test_base.py
@@ -38,7 +38,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',
@@ -47,6 +48,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/c75da0b4/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 2a539df..7373f3c 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/c75da0b4/ForgeImporters/forgeimporters/trac/tickets.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/trac/tickets.py b/ForgeImporters/forgeimporters/trac/tickets.py
index dfbb2a2..0c6e2b7 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
 from allura.lib.import_api import AlluraImportApiClient
 from allura.lib.validators import UserMapJsonFile
+from allura.lib import helpers as h
 from allura.model import ApiTicket
 from allura.scripts.trac_export import (
         TracExport,
@@ -100,16 +101,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


Mime
View raw message