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/12/20 18:29:51 UTC

git commit: [#7007] Fixed validation on install_tool API end-point

Updated Branches:
  refs/heads/master 4ab4c42e1 -> 65ca68f15


[#7007] Fixed validation on install_tool API end-point

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/65ca68f1
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/65ca68f1
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/65ca68f1

Branch: refs/heads/master
Commit: 65ca68f15fe84ef774d1c960d139d56664377e4d
Parents: 4ab4c42
Author: Cory Johns <cj...@slashdotmedia.com>
Authored: Fri Dec 20 17:29:10 2013 +0000
Committer: Cory Johns <cj...@slashdotmedia.com>
Committed: Fri Dec 20 17:29:10 2013 +0000

----------------------------------------------------------------------
 Allura/allura/ext/admin/admin_main.py        | 69 ++++++++++++-----------
 Allura/allura/tests/functional/test_admin.py |  4 +-
 2 files changed, 39 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/65ca68f1/Allura/allura/ext/admin/admin_main.py
----------------------------------------------------------------------
diff --git a/Allura/allura/ext/admin/admin_main.py b/Allura/allura/ext/admin/admin_main.py
index 647b454..f60bd39 100644
--- a/Allura/allura/ext/admin/admin_main.py
+++ b/Allura/allura/ext/admin/admin_main.py
@@ -612,41 +612,41 @@ class ProjectAdminController(BaseController):
                 options = c.project.app_config(p['mount_point']).options
                 options.mount_label = p['mount_label']
                 options.ordinal = int(p['ordinal'])
-        try:
-            if new and new.get('install'):
-                ep_name = new.get('ep_name', None)
-                if not ep_name:
-                    require_access(c.project, 'create')
-                    mount_point = new['mount_point'].lower() or h.nonce()
-                    M.AuditLog.log('create subproject %s', mount_point)
-                    h.log_action(log, 'create subproject').info(
-                        'create subproject %s', mount_point,
-                        meta=dict(mount_point=mount_point,name=new['mount_label']))
-                    sp = c.project.new_subproject(mount_point)
-                    sp.name = new['mount_label']
-                    sp.ordinal = int(new['ordinal'])
-                else:
-                    require_access(c.project, 'admin')
-                    installable_tools = AdminApp.installable_tools_for(c.project)
-                    if not ep_name.lower() in [t['name'].lower() for t in installable_tools]:
-                        flash('Installation limit exceeded.', 'error')
-                        return
-                    mount_point = new['mount_point'] or ep_name
-                    M.AuditLog.log('install tool %s', mount_point)
-                    h.log_action(log, 'install tool').info(
-                        'install tool %s', mount_point,
-                        meta=dict(tool_type=ep_name, mount_point=mount_point, mount_label=new['mount_label']))
-                    c.project.install_app(ep_name, mount_point, mount_label=new['mount_label'], ordinal=new['ordinal'])
-        except forge_exc.ForgeError, exc:
-            flash('%s: %s' % (exc.__class__.__name__, exc.args[0]),
-                  'error')
+        if new and new.get('install'):
+            ep_name = new.get('ep_name', None)
+            if not ep_name:
+                require_access(c.project, 'create')
+                mount_point = new['mount_point'].lower() or h.nonce()
+                M.AuditLog.log('create subproject %s', mount_point)
+                h.log_action(log, 'create subproject').info(
+                    'create subproject %s', mount_point,
+                    meta=dict(mount_point=mount_point,name=new['mount_label']))
+                sp = c.project.new_subproject(mount_point)
+                sp.name = new['mount_label']
+                sp.ordinal = int(new['ordinal'])
+            else:
+                require_access(c.project, 'admin')
+                installable_tools = AdminApp.installable_tools_for(c.project)
+                if not ep_name.lower() in [t['name'].lower() for t in installable_tools]:
+                    flash('Installation limit exceeded.', 'error')
+                    return
+                mount_point = new['mount_point'] or ep_name
+                M.AuditLog.log('install tool %s', mount_point)
+                h.log_action(log, 'install tool').info(
+                    'install tool %s', mount_point,
+                    meta=dict(tool_type=ep_name, mount_point=mount_point, mount_label=new['mount_label']))
+                c.project.install_app(ep_name, mount_point, mount_label=new['mount_label'], ordinal=new['ordinal'])
         g.post_event('project_updated')
 
     @h.vardec
     @expose()
     @require_post()
     def update_mounts(self, subproject=None, tool=None, new=None, **kw):
-        self._update_mounts(subproject, tool, new, **kw)
+        try:
+            self._update_mounts(subproject, tool, new, **kw)
+        except forge_exc.ForgeError, exc:
+            flash('%s: %s' % (exc.__class__.__name__, exc.args[0]),
+                  'error')
         redirect('tools')
 
     @expose('jinja:allura.ext.admin:templates/export.html')
@@ -773,9 +773,9 @@ class ProjectAdminRestController(BaseController):
             return {'success': False,
                     'info': 'Incorrect tool name, or limit is reached.'
                     }
-        if not h.re_tool_mount_point.match(mount_point) or c.project.app_instance(mount_point) is not None:
+        if c.project.app_instance(mount_point) is not None:
             return {'success': False,
-                    'info': 'Incorrect mount point name, or mount point already exists.'
+                    'info': 'Mount point already exists.',
                     }
 
         if order is None:
@@ -822,7 +822,12 @@ class ProjectAdminRestController(BaseController):
             'mount_point': mount_point,
             'mount_label': mount_label
         }
-        controller._update_mounts(new=data)
+        try:
+            controller._update_mounts(new=data)
+        except forge_exc.ForgeError as e:
+            return {'success': False,
+                    'info': str(e),
+                    }
         return {'success': True,
                 'info': 'Tool %s with mount_point %s and mount_label %s was created.'
                         % (tool, mount_point, mount_label)

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/65ca68f1/Allura/allura/tests/functional/test_admin.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/functional/test_admin.py b/Allura/allura/tests/functional/test_admin.py
index 54bdf85..bf09009 100644
--- a/Allura/allura/tests/functional/test_admin.py
+++ b/Allura/allura/tests/functional/test_admin.py
@@ -1105,7 +1105,7 @@ class TestRestInstallTool(TestRestApiBase):
         }
         r = self.api_post('/rest/p/test/admin/install_tool/', **data)
         assert_equals(r.json['success'], False)
-        assert_equals(r.json['info'], 'Incorrect mount point name, or mount point already exists.')
+        assert_equals(r.json['info'], 'Mount point "tickets_mount1" is invalid')
 
     def test_install_tool_ok(self):
         r = self.api_get('/rest/p/test/')
@@ -1143,7 +1143,7 @@ class TestRestInstallTool(TestRestApiBase):
             c.project.install_app('tickets', mount_point=data['mount_point'])
             r = self.api_post('/rest/p/test/admin/install_tool/', **data)
             assert_equals(r.json['success'], False)
-            assert_equals(r.json['info'], 'Incorrect mount point name, or mount point already exists.')
+            assert_equals(r.json['info'], 'Mount point already exists.')
 
     def test_tool_installation_limit(self):
         with mock.patch.object(ForgeWikiApp, 'max_instances') as mi: