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/03 22:08:50 UTC

[1/3] git commit: [#4656] Add tests for new provider methods

Updated Branches:
  refs/heads/cj/4656 [created] dd798ee2a


[#4656] Add tests for new provider methods

Signed-off-by: Tim Van Steenburgh <tv...@gmail.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/eccc08f8
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/eccc08f8
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/eccc08f8

Branch: refs/heads/cj/4656
Commit: eccc08f86b4d00b12fea7fd3c3f68fcdb4e658f4
Parents: 7c2bb7e
Author: Tim Van Steenburgh <tv...@gmail.com>
Authored: Tue Jul 2 21:36:57 2013 +0000
Committer: Cory Johns <cj...@slashdotmedia.com>
Committed: Wed Jul 3 17:14:34 2013 +0000

----------------------------------------------------------------------
 Allura/allura/tests/test_plugin.py | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/eccc08f8/Allura/allura/tests/test_plugin.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/test_plugin.py b/Allura/allura/tests/test_plugin.py
index 6e1a07a..aa2aaeb 100644
--- a/Allura/allura/tests/test_plugin.py
+++ b/Allura/allura/tests/test_plugin.py
@@ -16,7 +16,7 @@
 #       under the License.
 
 from nose.tools import assert_equals
-from mock import MagicMock, patch
+from mock import Mock, MagicMock, patch
 
 from allura import model as M
 from allura.lib.utils import TruthyCallable
@@ -25,16 +25,31 @@ from allura.lib.plugin import ProjectRegistrationProvider
 
 class TestProjectRegistrationProvider(object):
 
+    def setUp(self):
+        self.provider = ProjectRegistrationProvider()
+
     @patch('allura.lib.security.has_access')
     def test_validate_project_15char_user(self, has_access):
         has_access.return_value = TruthyCallable(lambda: True)
-        provider = ProjectRegistrationProvider()
         nbhd = M.Neighborhood()
-        provider.validate_project(
+        self.provider.validate_project(
             neighborhood=nbhd,
             shortname='u/' + ('a' * 15),
             project_name='15 char username',
             user=MagicMock(),
             user_project=True,
             private_project=False,
-        )
\ No newline at end of file
+        )
+
+    def test_suggest_name(self):
+        f = self.provider.suggest_name
+        assert_equals(f('A More Than Fifteen Character Name', Mock()),
+                'amorethanfifteencharactername')
+
+    def test_validate_project_shortname(self):
+        f = self.provider.validate_project_shortname
+        p = Mock()
+        assert_equals(f('thisislegit', p), None)
+        assert_equals(f('this is invalid and too long', p),
+                'Please use only letters, numbers, and dashes '
+                '3-15 characters long.')


[3/3] git commit: [#4656] Refactored project name validation

Posted by jo...@apache.org.
[#4656] Refactored project name validation

AJAX validation and validation on submit were not using the same
validation checks.  Refactored the validation code so that both use
the same checks and consolidated redundant API methods related to those
checks.

The "extra_name_checks" method is no longer used, and those checks
should be folded into the new, more general "validate_project_shortname"
method.

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

Branch: refs/heads/cj/4656
Commit: dd798ee2ad53350b2d6c9b6a151edfbafe1e927f
Parents: eccc08f
Author: Cory Johns <cj...@slashdotmedia.com>
Authored: Wed Jul 3 20:05:19 2013 +0000
Committer: Cory Johns <cj...@slashdotmedia.com>
Committed: Wed Jul 3 20:05:22 2013 +0000

----------------------------------------------------------------------
 Allura/allura/controllers/project.py | 19 +++++--------------
 Allura/allura/lib/plugin.py          | 13 -------------
 Allura/allura/lib/widgets/forms.py   | 30 +++++++++++++++---------------
 3 files changed, 20 insertions(+), 42 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/dd798ee2/Allura/allura/controllers/project.py
----------------------------------------------------------------------
diff --git a/Allura/allura/controllers/project.py b/Allura/allura/controllers/project.py
index e5edd80..36c4667 100644
--- a/Allura/allura/controllers/project.py
+++ b/Allura/allura/controllers/project.py
@@ -80,7 +80,8 @@ class NeighborhoodController(object):
     @expose()
     def _lookup(self, pname, *remainder):
         pname = unquote(pname)
-        if not h.re_project_name.match(pname):
+        provider = plugin.ProjectRegistrationProvider.get()
+        if provider.validate_project_shortname(pname, self.neighborhood):
             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/':
@@ -180,19 +181,9 @@ class NeighborhoodController(object):
             self.neighborhood))
 
     @expose('json:')
-    def check_names(self, project_name='', unix_name=''):
-        provider = plugin.ProjectRegistrationProvider.get()
-        result = dict()
-        try:
-            W.add_project.fields['project_name'].validate(project_name, '')
-        except Invalid as e:
-            result['name_message'] = str(e)
-
-        unixname_invalid_err = provider.validate_project_shortname(unix_name,
-                self.neighborhood)
-        result['unixname_message'] = (unixname_invalid_err or
-                provider.name_taken(unix_name, self.neighborhood))
-        return result
+    @validate(W.add_project)
+    def check_names(self, **raw_data):
+        return c.form_errors
 
     @h.vardec
     @expose()

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/dd798ee2/Allura/allura/lib/plugin.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/plugin.py b/Allura/allura/lib/plugin.py
index c0319f7..111aa77 100644
--- a/Allura/allura/lib/plugin.py
+++ b/Allura/allura/lib/plugin.py
@@ -373,21 +373,8 @@ class ProjectRegistrationProvider(object):
         p = M.Project.query.get(shortname=project_name, neighborhood_id=neighborhood._id)
         if p:
             return 'This project name is taken.'
-        for check in self.extra_name_checks():
-            if re.match(str(check[1]),project_name) is not None:
-                return check[0]
         return False
 
-    def extra_name_checks(self):
-        """Return an iterable of ``(error_message, regex)`` tuples.
-
-        If user attempts to register a project with a name that matches
-        ``regex``, the field will be marked invalid, and ``error_message``
-        displayed to the user.
-
-        """
-        return []
-
     def suggest_name(self, project_name, neighborhood):
         """Return a suggested project shortname for the full ``project_name``.
 

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/dd798ee2/Allura/allura/lib/widgets/forms.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/widgets/forms.py b/Allura/allura/lib/widgets/forms.py
index 9501b70..2312c71 100644
--- a/Allura/allura/lib/widgets/forms.py
+++ b/Allura/allura/lib/widgets/forms.py
@@ -45,12 +45,16 @@ class _HTMLExplanation(ew.InputField):
         ''',
         'jinja2')
 
-class NeighborhoodProjectTakenValidator(fev.FancyValidator):
+class NeighborhoodProjectShortNameValidator(fev.FancyValidator):
 
     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'])
-        message = plugin.ProjectRegistrationProvider.get().name_taken(value, 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:
             raise formencode.Invalid(message, value, state)
         return value
@@ -779,14 +783,7 @@ class NeighborhoodAddProjectForm(ForgeForm):
                 V.MaxBytesValidator(max=40)))
         project_unixname = ew.InputField(
             label='Short Name', field_type='text',
-            validator=formencode.All(
-                fev.String(not_empty=True),
-                fev.MinLength(3),
-                fev.MaxLength(15),
-                fev.Regex(
-                    r'^[A-z][-A-z0-9]{2,}$',
-                    messages={'invalid':'Please use only letters, numbers, and dashes 3-15 characters long.'}),
-                NeighborhoodProjectTakenValidator()))
+            validator=NeighborhoodProjectShortNameValidator())
         tools = ew.CheckboxSet(name='tools', options=[
             ## Required for Neighborhood functional tests to pass
             ew.Option(label='Wiki', html_value='wiki', selected=True)
@@ -805,12 +802,14 @@ class NeighborhoodAddProjectForm(ForgeForm):
     def resources(self):
         for r in super(NeighborhoodAddProjectForm, self).resources(): yield r
         yield ew.CSSLink('css/add_project.css')
+        neighborhood = g.antispam.enc('neighborhood')
         project_name = g.antispam.enc('project_name')
         project_unixname = g.antispam.enc('project_unixname')
 
         yield ew.JSScript('''
             $(function(){
                 var $scms = $('input[type=checkbox].scm');
+                var $nbhd_input = $('input[name="%(neighborhood)s"]');
                 var $name_input = $('input[name="%(project_name)s"]');
                 var $unixname_input = $('input[name="%(project_unixname)s"]');
                 var $url_fragment = $('#url_fragment');
@@ -864,12 +863,13 @@ class NeighborhoodAddProjectForm(ForgeForm):
                 });
                 var check_names = function() {
                     var data = {
-                        'project_name':$name_input.val(),
-                        'unix_name': $unixname_input.val()
+                        'neighborhood': $nbhd_input.val(),
+                        'project_name': $name_input.val(),
+                        'project_unixname': $unixname_input.val()
                     };
                     $.getJSON('check_names', data, function(result){
-                        handle_error($name_input, result.name_message);
-                        handle_error($unixname_input, result.unixname_message);
+                        handle_error($name_input, result.project_name);
+                        handle_error($unixname_input, result.project_unixname);
                     });
                 };
                 var manual = false;
@@ -897,7 +897,7 @@ class NeighborhoodAddProjectForm(ForgeForm):
                     delay(check_names, 500);
                 });
             });
-        ''' % dict(project_name=project_name, project_unixname=project_unixname))
+        ''' % dict(neighborhood=neighborhood, project_name=project_name, project_unixname=project_unixname))
 
 
 class MoveTicketForm(ForgeForm):


[2/3] git commit: [#4656] Refactor project name validation

Posted by jo...@apache.org.
[#4656] Refactor project name validation

Signed-off-by: Tim Van Steenburgh <tv...@gmail.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/7c2bb7e6
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/7c2bb7e6
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/7c2bb7e6

Branch: refs/heads/cj/4656
Commit: 7c2bb7e64f272ee92544d23907af8123d19bdbdc
Parents: ff44014
Author: Tim Van Steenburgh <tv...@gmail.com>
Authored: Tue Jul 2 19:56:40 2013 +0000
Committer: Cory Johns <cj...@slashdotmedia.com>
Committed: Wed Jul 3 17:14:34 2013 +0000

----------------------------------------------------------------------
 Allura/allura/controllers/project.py | 16 ++++++------
 Allura/allura/lib/plugin.py          | 41 ++++++++++++++++++++++++-------
 2 files changed, 41 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7c2bb7e6/Allura/allura/controllers/project.py
----------------------------------------------------------------------
diff --git a/Allura/allura/controllers/project.py b/Allura/allura/controllers/project.py
index bff0399..e5edd80 100644
--- a/Allura/allura/controllers/project.py
+++ b/Allura/allura/controllers/project.py
@@ -175,21 +175,23 @@ class NeighborhoodController(object):
 
     @expose('json:')
     def suggest_name(self, project_name=''):
-        result = dict()
-        result['suggested_name'] = re.sub("[^A-Za-z0-9]", "", project_name).lower()[:15]
-        return result
+        provider = plugin.ProjectRegistrationProvider.get()
+        return dict(suggested_name=provider.suggest_name(project_name,
+            self.neighborhood))
 
     @expose('json:')
     def check_names(self, project_name='', unix_name=''):
+        provider = plugin.ProjectRegistrationProvider.get()
         result = dict()
         try:
             W.add_project.fields['project_name'].validate(project_name, '')
         except Invalid as e:
             result['name_message'] = str(e)
-        if not h.re_project_name.match(unix_name) or not (3 <= len(unix_name) <= 15):
-            result['unixname_message'] = 'Please use only letters, numbers, and dashes 3-15 characters long.'
-        else:
-            result['unixname_message'] = plugin.ProjectRegistrationProvider.get().name_taken(unix_name, self.neighborhood)
+
+        unixname_invalid_err = provider.validate_project_shortname(unix_name,
+                self.neighborhood)
+        result['unixname_message'] = (unixname_invalid_err or
+                provider.name_taken(unix_name, self.neighborhood))
         return result
 
     @h.vardec

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7c2bb7e6/Allura/allura/lib/plugin.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/plugin.py b/Allura/allura/lib/plugin.py
index 855e71c..c0319f7 100644
--- a/Allura/allura/lib/plugin.py
+++ b/Allura/allura/lib/plugin.py
@@ -365,6 +365,10 @@ class ProjectRegistrationProvider(object):
         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)
         if p:
@@ -375,18 +379,28 @@ class ProjectRegistrationProvider(object):
         return False
 
     def extra_name_checks(self):
-        '''This should be a list or iterator containing tuples.
-        The first tiem in the tuple should be an error message and the
-        second should be a regex. If the user attempts to register a
-        project with a name that matches the regex, the field will
-        be marked invalid with the message displayed to the user.
-        '''
+        """Return an iterable of ``(error_message, regex)`` tuples.
+
+        If user attempts to register a project with a name that matches
+        ``regex``, the field will be marked invalid, and ``error_message``
+        displayed to the user.
+
+        """
         return []
 
+    def suggest_name(self, project_name, neighborhood):
+        """Return a suggested project shortname for the full ``project_name``.
+
+        Example: "My Great Project" -> "mygreatproject"
+
+        """
+        return re.sub("[^A-Za-z0-9]", "", project_name).lower()
+
     def rate_limit(self, user, neighborhood):
-        '''Check the various config-defined project registration rate
+        """Check the various config-defined project registration rate
         limits, and if any are exceeded, raise ProjectRatelimitError.
-        '''
+
+        """
         if security.has_access(neighborhood, 'admin', user=user)():
             return
         # have to have the replace because, despite being UTC,
@@ -468,13 +482,22 @@ class ProjectRegistrationProvider(object):
             check_shortname = shortname.replace('u/', '', 1)
         else:
             check_shortname = shortname
-        if not h.re_project_name.match(check_shortname):
+        err = self.validate_project_shortname(check_shortname, neighborhood)
+        if err:
             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.
+
+        """
+        if not h.re_project_name.match(shortname):
+            return 'Please use only letters, numbers, and dashes 3-15 characters long.'
+
     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