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/07/31 21:14:24 UTC
[04/23] git commit: [#6504] Refactored all project name validation
into validator class
[#6504] Refactored all project name validation into validator class
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/fff526fe
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/fff526fe
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/fff526fe
Branch: refs/heads/cj/6461
Commit: fff526fe7cbb03cacc941872731f61f0dd84225e
Parents: be9d822
Author: Cory Johns <cj...@slashdotmedia.com>
Authored: Tue Jul 30 18:21:34 2013 +0000
Committer: Tim Van Steenburgh <tv...@gmail.com>
Committed: Tue Jul 30 20:12:00 2013 +0000
----------------------------------------------------------------------
Allura/allura/controllers/project.py | 5 +--
Allura/allura/lib/exceptions.py | 5 ++-
Allura/allura/lib/plugin.py | 52 +++++--------------------------
Allura/allura/lib/widgets/forms.py | 39 ++++++++++++++++++-----
Allura/allura/tests/test_plugin.py | 46 ++++++++++-----------------
5 files changed, 62 insertions(+), 85 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/fff526fe/Allura/allura/controllers/project.py
----------------------------------------------------------------------
diff --git a/Allura/allura/controllers/project.py b/Allura/allura/controllers/project.py
index 91d5a4f..8af4396 100644
--- a/Allura/allura/controllers/project.py
+++ b/Allura/allura/controllers/project.py
@@ -83,8 +83,9 @@ class NeighborhoodController(object):
def _lookup(self, pname, *remainder):
pname = unquote(pname)
provider = plugin.ProjectRegistrationProvider.get()
- valid, reason = provider.valid_project_shortname(pname, self.neighborhood)
- if not valid:
+ try:
+ provider.shortname_validator.to_python(pname, check_allowed=False, neighborhood=self.neighborhood)
+ except Invalid as e:
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/fff526fe/Allura/allura/lib/exceptions.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/exceptions.py b/Allura/allura/lib/exceptions.py
index e222cf5..4c33991 100644
--- a/Allura/allura/lib/exceptions.py
+++ b/Allura/allura/lib/exceptions.py
@@ -15,8 +15,11 @@
# specific language governing permissions and limitations
# under the License.
+from formencode import Invalid
+
class ForgeError(Exception): pass
-class ProjectConflict(ForgeError): pass
+class ProjectConflict(ForgeError, Invalid): pass
+class ProjectShortnameInvalid(ForgeError, Invalid): pass
class ProjectOverlimitError(ForgeError): pass
class ProjectRatelimitError(ForgeError): pass
class ToolError(ForgeError): pass
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/fff526fe/Allura/allura/lib/plugin.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/plugin.py b/Allura/allura/lib/plugin.py
index 7626bdb..4afd9fc 100644
--- a/Allura/allura/lib/plugin.py
+++ b/Allura/allura/lib/plugin.py
@@ -352,11 +352,18 @@ class ProjectRegistrationProvider(object):
myprovider = foo.bar:MyAuthProvider
Then in your .ini file, set registration.method=myprovider
+
+ The provider should expose an attribute, `shortname_validator` which is
+ an instance of a FormEncode validator that validates project shortnames.
+ The `to_python()` method of the validator should accept a `check_allowed`
+ argument to indicate whether additional checks beyond correctness of the
+ name should be done, such as whether the name is already in use.
'''
def __init__(self):
from allura.lib.widgets import forms
self.add_project_widget = forms.NeighborhoodAddProjectForm
+ self.shortname_validator = forms.NeighborhoodProjectShortNameValidator()
@classmethod
def get(cls):
@@ -364,15 +371,6 @@ class ProjectRegistrationProvider(object):
method = config.get('registration.method', 'local')
return app_globals.Globals().entry_points['registration'][method]()
- 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)
- return bool(p)
-
def suggest_name(self, project_name, neighborhood):
"""Return a suggested project shortname for the full ``project_name``.
@@ -467,46 +465,12 @@ class ProjectRegistrationProvider(object):
check_shortname = shortname.replace('u/', '', 1)
else:
check_shortname = shortname
- allowed, err = self.allowed_project_shortname(check_shortname, neighborhood)
- if not allowed:
- raise ValueError('Invalid project shortname: %s error: %s' % (shortname, err))
+ self.shortname_validator.to_python(check_shortname, neighborhood=neighborhood)
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 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 (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):
'''
Actually create the project, no validation. This should not be called directly
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/fff526fe/Allura/allura/lib/widgets/forms.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/widgets/forms.py b/Allura/allura/lib/widgets/forms.py
index a5ebe15..ae90b26 100644
--- a/Allura/allura/lib/widgets/forms.py
+++ b/Allura/allura/lib/widgets/forms.py
@@ -22,6 +22,7 @@ from allura.lib import validators as V
from allura.lib import helpers as h
from allura.lib import plugin
from allura.lib.widgets import form_fields as ffw
+from allura.lib import exceptions as forge_exc
from allura import model as M
from datetime import datetime
@@ -46,14 +47,33 @@ class _HTMLExplanation(ew.InputField):
'jinja2')
class NeighborhoodProjectShortNameValidator(fev.FancyValidator):
-
- def to_python(self, value, state):
+ def _validate_shortname(self, shortname, neighborhood, state):
+ if not h.re_project_name.match(shortname):
+ raise forge_exc.ProjectShortnameInvalid(
+ 'Please use only letters, numbers, and dashes 3-15 characters long.',
+ shortname, state)
+
+ def _validate_allowed(self, shortname, neighborhood, state):
+ p = M.Project.query.get(shortname=shortname, neighborhood_id=neighborhood._id)
+ if p:
+ raise forge_exc.ProjectConflict(
+ 'This project name is taken.',
+ shortname, state)
+
+ def to_python(self, value, state=None, check_allowed=True, neighborhood=None):
+ """
+ Validate a project shortname.
+
+ If check_allowed is False, the shortname will only be checked for
+ correctness. Otherwise, it will be rejected if already in use or
+ otherwise disallowed.
+ """
+ if neighborhood is None:
+ neighborhood = M.Neighborhood.query.get(name=state.full_dict['neighborhood'])
value = h.really_unicode(value or '').encode('utf-8').lower()
- neighborhood = M.Neighborhood.query.get(name=state.full_dict['neighborhood'])
- provider = plugin.ProjectRegistrationProvider.get()
- allowed, message = provider.allowed_project_shortname(value, neighborhood)
- if not allowed:
- raise formencode.Invalid(message, value, state)
+ self._validate_shortname(value, neighborhood, state)
+ if check_allowed:
+ self._validate_allowed(value, neighborhood, state)
return value
class ForgeForm(ew.SimpleForm):
@@ -780,7 +800,7 @@ class NeighborhoodAddProjectForm(ForgeForm):
V.MaxBytesValidator(max=40)))
project_unixname = ew.InputField(
label='Short Name', field_type='text',
- validator=NeighborhoodProjectShortNameValidator())
+ validator=None) # will be set in __init__
tools = ew.CheckboxSet(name='tools', options=[
## Required for Neighborhood functional tests to pass
ew.Option(label='Wiki', html_value='wiki', selected=True)
@@ -788,6 +808,9 @@ class NeighborhoodAddProjectForm(ForgeForm):
def __init__(self, *args, **kwargs):
super(NeighborhoodAddProjectForm, self).__init__(*args, **kwargs)
+ # get the shortname validator from the provider
+ provider = plugin.ProjectRegistrationProvider.get()
+ self.fields.project_unixname.validator = provider.shortname_validator
## Dynamically generating CheckboxSet of installable tools
from allura.lib.widgets import forms
self.fields.tools.options = [
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/fff526fe/Allura/allura/tests/test_plugin.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/test_plugin.py b/Allura/allura/tests/test_plugin.py
index 712fe52..57f8dbf 100644
--- a/Allura/allura/tests/test_plugin.py
+++ b/Allura/allura/tests/test_plugin.py
@@ -15,12 +15,15 @@
# specific language governing permissions and limitations
# under the License.
-from nose.tools import assert_equals
+from functools import partial
+from nose.tools import assert_equals, assert_raises
from mock import Mock, MagicMock, patch
+from formencode import Invalid
from allura import model as M
from allura.lib.utils import TruthyCallable
from allura.lib.plugin import ProjectRegistrationProvider
+from allura.lib.exceptions import ProjectConflict, ProjectShortnameInvalid
class TestProjectRegistrationProvider(object):
@@ -46,32 +49,15 @@ class TestProjectRegistrationProvider(object):
assert_equals(f('A More Than Fifteen Character Name', Mock()),
'amorethanfifteencharactername')
- def test_valid_project_shortname(self):
- f = self.provider.valid_project_shortname
- p = Mock()
- 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)
+ @patch('allura.model.Project')
+ def test_shortname_validator(self, Project):
+ Project.query.get.return_value = None
+ nbhd = Mock()
+ v = self.provider.shortname_validator.to_python
+
+ v('thisislegit', neighborhood=nbhd)
+ assert_raises(ProjectShortnameInvalid, v, 'not valid', neighborhood=nbhd)
+ assert_raises(ProjectShortnameInvalid, v, 'this-is-valid-but-too-long', neighborhood=nbhd)
+ assert_raises(ProjectShortnameInvalid, v, 'this is invalid and too long', neighborhood=nbhd)
+ Project.query.get.return_value = Mock()
+ assert_raises(ProjectConflict, v, 'thisislegit', neighborhood=nbhd)