[#4656] More refactor to project shortname validation
Signed-off-by: Cory Johns <cjohns@slashdotmedia.com>
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 @@
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/':
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 @@
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 @@
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):
'''
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 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
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 @@
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 @@
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):
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 @@
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)
