Return-Path: X-Original-To: apmail-incubator-allura-commits-archive@minotaur.apache.org Delivered-To: apmail-incubator-allura-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 36D7E1046C for ; Mon, 8 Jul 2013 18:58:36 +0000 (UTC) Received: (qmail 39442 invoked by uid 500); 8 Jul 2013 18:58:36 -0000 Delivered-To: apmail-incubator-allura-commits-archive@incubator.apache.org Received: (qmail 39424 invoked by uid 500); 8 Jul 2013 18:58:36 -0000 Mailing-List: contact allura-commits-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: allura-dev@incubator.apache.org Delivered-To: mailing list allura-commits@incubator.apache.org Received: (qmail 39416 invoked by uid 99); 8 Jul 2013 18:58:35 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 08 Jul 2013 18:58:35 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 728E6888004; Mon, 8 Jul 2013 18:58:35 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: johnsca@apache.org To: allura-commits@incubator.apache.org Message-Id: <69bab2966780435295e2ac007993f65f@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: git commit: [#4656] More refactor to project shortname validation Date: Mon, 8 Jul 2013 18:58:35 +0000 (UTC) Updated Branches: refs/heads/cj/4656 dd798ee2a -> 6fc50ebd2 [#4656] More refactor to project shortname validation Signed-off-by: Cory Johns Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/6fc50ebd Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/6fc50ebd Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/6fc50ebd Branch: refs/heads/cj/4656 Commit: 6fc50ebd2a121eb0a7814d8b74f3a6756a2a3e97 Parents: dd798ee Author: Cory Johns Authored: Mon Jul 8 18:57:19 2013 +0000 Committer: Cory Johns Committed: Mon Jul 8 18:57:19 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 +++---- 4 files changed, 44 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/6fc50ebd/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/6fc50ebd/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/6fc50ebd/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/6fc50ebd/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):