incubator-allura-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From john...@apache.org
Subject [4/9] git commit: [#4656] More refactor to project shortname validation
Date Wed, 17 Jul 2013 21:17:19 GMT
[#4656] More refactor to project shortname validation

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

Branch: refs/heads/cj/6456
Commit: c78912c4692def26a65263e762f33a61b0173c2a
Parents: 7efec56
Author: Cory Johns <cjohns@slashdotmedia.com>
Authored: Mon Jul 8 18:57:19 2013 +0000
Committer: Cory Johns <cjohns@slashdotmedia.com>
Committed: Tue Jul 16 20:15:14 2013 +0000

----------------------------------------------------------------------
 Allura/allura/controllers/project.py            |  3 +-
 Allura/allura/lib/plugin.py                     | 42 +++++++++++++++-----
 Allura/allura/lib/widgets/forms.py              |  9 ++---
 .../tests/functional/test_neighborhood.py       | 14 +++----
 Allura/allura/tests/test_plugin.py              | 30 ++++++++++++--
 5 files changed, 70 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/c78912c4/Allura/allura/controllers/project.py
----------------------------------------------------------------------
diff --git a/Allura/allura/controllers/project.py b/Allura/allura/controllers/project.py
index 36c4667..3cf4112 100644
--- a/Allura/allura/controllers/project.py
+++ b/Allura/allura/controllers/project.py
@@ -81,7 +81,8 @@ class NeighborhoodController(object):
     def _lookup(self, pname, *remainder):
         pname = unquote(pname)
         provider = plugin.ProjectRegistrationProvider.get()
-        if provider.validate_project_shortname(pname, self.neighborhood):
+        valid, reason = provider.valid_project_shortname(pname, self.neighborhood)
+        if not valid:
             raise exc.HTTPNotFound, pname
         project = M.Project.query.get(shortname=self.prefix + pname, neighborhood_id=self.neighborhood._id)
         if project is None and self.prefix == 'u/':

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/c78912c4/Allura/allura/lib/plugin.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/plugin.py b/Allura/allura/lib/plugin.py
index 111aa77..76a18ae 100644
--- a/Allura/allura/lib/plugin.py
+++ b/Allura/allura/lib/plugin.py
@@ -364,16 +364,14 @@ class ProjectRegistrationProvider(object):
         method = config.get('registration.method', 'local')
         return app_globals.Globals().entry_points['registration'][method]()
 
-    def name_taken(self, project_name, neighborhood):
+    def _name_taken(self, project_name, neighborhood):
         """Return False if ``project_name`` is available in ``neighborhood``.
         If unavailable, return an error message (str) explaining why.
 
         """
         from allura import model as M
         p = M.Project.query.get(shortname=project_name, neighborhood_id=neighborhood._id)
-        if p:
-            return 'This project name is taken.'
-        return False
+        return bool(p)
 
     def suggest_name(self, project_name, neighborhood):
         """Return a suggested project shortname for the full ``project_name``.
@@ -469,21 +467,45 @@ class ProjectRegistrationProvider(object):
             check_shortname = shortname.replace('u/', '', 1)
         else:
             check_shortname = shortname
-        err = self.validate_project_shortname(check_shortname, neighborhood)
-        if err:
+        allowed, err = self.allowed_project_shortname(check_shortname, neighborhood)
+        if not allowed:
             raise ValueError('Invalid project shortname: %s' % shortname)
 
         p = M.Project.query.get(shortname=shortname, neighborhood_id=neighborhood._id)
         if p:
             raise forge_exc.ProjectConflict('%s already exists in nbhd %s' % (shortname,
neighborhood._id))
 
-    def validate_project_shortname(self, shortname, neighborhood):
-        """Return an error message if ``shortname`` is invalid for
-        ``neighborhood``, else return None.
+    def valid_project_shortname(self, shortname, neighborhood):
+        """Determine if the project shortname appears to be valid.
+
+        Returns a pair of values, the first being a bool indicating whether
+        the name appears to be valid, and the second being a message indicating
+        the reason, if any, why the name is invalid.
 
+        NB: Even if a project shortname is valid, it might still not be
+        allowed (it could already be taken, for example).  Use the method
+        ``allowed_project_shortname`` instead to check if the shortname can
+        actually be used.
         """
         if not h.re_project_name.match(shortname):
-            return 'Please use only letters, numbers, and dashes 3-15 characters long.'
+            return (False, 'Please use only letters, numbers, and dashes 3-15 characters
long.')
+        return (True, None)
+
+    def allowed_project_shortname(self, shortname, neighborhood):
+        """Determine if a project shortname can be used.
+
+        A shortname can be used if it is valid and is not already taken.
+
+        Returns a pair of values, the first being a bool indicating whether
+        the name can be used, and the second being a message indicating the
+        reason, if any, why the name cannot be used.
+        """
+        valid, reason = self.valid_project_shortname(shortname, neighborhood)
+        if not valid:
+            return (False, reason)
+        if self._name_taken(shortname, neighborhood):
+            return (False, 'This project name is taken.')
+        return (True, None)
 
     def _create_project(self, neighborhood, shortname, project_name, user, user_project,
private_project, apps):
         '''

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/c78912c4/Allura/allura/lib/widgets/forms.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/widgets/forms.py b/Allura/allura/lib/widgets/forms.py
index 2312c71..a5ebe15 100644
--- a/Allura/allura/lib/widgets/forms.py
+++ b/Allura/allura/lib/widgets/forms.py
@@ -47,15 +47,12 @@ class _HTMLExplanation(ew.InputField):
 
 class NeighborhoodProjectShortNameValidator(fev.FancyValidator):
 
-    def _to_python(self, value, state):
+    def to_python(self, value, state):
         value = h.really_unicode(value or '').encode('utf-8').lower()
         neighborhood = M.Neighborhood.query.get(name=state.full_dict['neighborhood'])
         provider = plugin.ProjectRegistrationProvider.get()
-        message = provider.validate_project_shortname(value, neighborhood)
-        if message:
-            raise formencode.Invalid(message, value, state)
-        message = provider.name_taken(value, neighborhood)
-        if message:
+        allowed, message = provider.allowed_project_shortname(value, neighborhood)
+        if not allowed:
             raise formencode.Invalid(message, value, state)
         return value
 

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/c78912c4/Allura/allura/tests/functional/test_neighborhood.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/functional/test_neighborhood.py b/Allura/allura/tests/functional/test_neighborhood.py
index fd6c3e4..f63d597 100644
--- a/Allura/allura/tests/functional/test_neighborhood.py
+++ b/Allura/allura/tests/functional/test_neighborhood.py
@@ -449,7 +449,7 @@ class TestNeighborhood(TestController):
                           params=dict(project_unixname='', project_name='Nothing', project_description='',
neighborhood='Adobe'),
                           antispam=True,
                           extra_environ=dict(username='root'))
-        assert r.html.find('div', {'class':'error'}).string == 'Please enter a value'
+        assert r.html.find('div', {'class':'error'}).string == 'Please use only letters,
numbers, and dashes 3-15 characters long.'
         r = self.app.post('/adobe/register',
                           params=dict(project_unixname='mymoz', project_name='My Moz', project_description='',
neighborhood='Adobe'),
                           antispam=True,
@@ -742,12 +742,12 @@ class TestNeighborhood(TestController):
 
     def test_name_check(self):
         for name in ('My+Moz', 'Te%st!', 'ab', 'a' * 16):
-            r = self.app.get('/p/check_names?unix_name=%s' % name)
-            assert r.json['unixname_message'] == 'Please use only letters, numbers, and dashes
3-15 characters long.'
-        r = self.app.get('/p/check_names?unix_name=mymoz')
-        assert_equal(r.json['unixname_message'], False)
-        r = self.app.get('/p/check_names?unix_name=test')
-        assert r.json['unixname_message'] == 'This project name is taken.'
+            r = self.app.get('/p/check_names?neighborhood=Projects&project_unixname=%s'
% name)
+            assert_equal(r.json, {'project_unixname': 'Please use only letters, numbers,
and dashes 3-15 characters long.'})
+        r = self.app.get('/p/check_names?neighborhood=Projects&project_unixname=mymoz')
+        assert_equal(r.json, {})
+        r = self.app.get('/p/check_names?neighborhood=Projects&project_unixname=test')
+        assert_equal(r.json, {'project_unixname': 'This project name is taken.'})
 
     @td.with_tool('test/sub1', 'Wiki', 'wiki')
     def test_neighborhood_project(self):

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/c78912c4/Allura/allura/tests/test_plugin.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/test_plugin.py b/Allura/allura/tests/test_plugin.py
index aa2aaeb..712fe52 100644
--- a/Allura/allura/tests/test_plugin.py
+++ b/Allura/allura/tests/test_plugin.py
@@ -46,10 +46,32 @@ class TestProjectRegistrationProvider(object):
         assert_equals(f('A More Than Fifteen Character Name', Mock()),
                 'amorethanfifteencharactername')
 
-    def test_validate_project_shortname(self):
-        f = self.provider.validate_project_shortname
+    def test_valid_project_shortname(self):
+        f = self.provider.valid_project_shortname
         p = Mock()
-        assert_equals(f('thisislegit', p), None)
-        assert_equals(f('this is invalid and too long', p),
+        valid = (True, None)
+        invalid = (False,
                 'Please use only letters, numbers, and dashes '
                 '3-15 characters long.')
+        assert_equals(f('thisislegit', p), valid)
+        assert_equals(f('not valid', p), invalid)
+        assert_equals(f('this-is-valid-but-too-long', p), invalid)
+        assert_equals(f('this is invalid and too long', p), invalid)
+
+    def test_allowed_project_shortname(self):
+        allowed = valid = (True, None)
+        invalid = (False, 'invalid')
+        taken = (False, 'This project name is taken.')
+        cases = [
+                (valid, False, allowed),
+                (invalid, False, invalid),
+                (valid, True, taken),
+            ]
+        p = Mock()
+        vps = self.provider.valid_project_shortname = Mock()
+        nt = self.provider._name_taken = Mock()
+        f = self.provider.allowed_project_shortname
+        for vps_v, nt_v, result in cases:
+            vps.return_value = vps_v
+            nt.return_value = nt_v
+            assert_equals(f('project', p), result)


Mime
View raw message