incubator-allura-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From john...@apache.org
Subject [03/50] git commit: [#6504] Refactored all project name validation into validator class
Date Mon, 12 Aug 2013 16:40:48 GMT
[#6504] Refactored all project name validation into validator class

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/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/6422
Commit: fff526fe7cbb03cacc941872731f61f0dd84225e
Parents: be9d822
Author: Cory Johns <cjohns@slashdotmedia.com>
Authored: Tue Jul 30 18:21:34 2013 +0000
Committer: Tim Van Steenburgh <tvansteenburgh@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)


Mime
View raw message