From: richard Date: Wed, 15 Jan 2003 11:07:45 +0000 (+0000) Subject: more cgi form parsing tests, and a fix for an outstanding couple of bugs X-Git-Url: https://git.tokkee.org/?a=commitdiff_plain;h=7a208fb2843c5ce6bf385685a19456a1989bc06d;p=roundup.git more cgi form parsing tests, and a fix for an outstanding couple of bugs git-svn-id: http://svn.roundup-tracker.org/svnroot/roundup/trunk@1457 57a73879-2fb5-44c3-a270-3262357dd7e2 --- diff --git a/doc/upgrading.txt b/doc/upgrading.txt index 3a4c350..d52c073 100644 --- a/doc/upgrading.txt +++ b/doc/upgrading.txt @@ -13,7 +13,6 @@ Migrating from 0.5 to 0.6 - Introduced EMAIL_FROM_TAG config variable. - Migrating from 0.4.x to 0.5.0 ============================= diff --git a/roundup/cgi/client.py b/roundup/cgi/client.py index b4a310f..4affd22 100644 --- a/roundup/cgi/client.py +++ b/roundup/cgi/client.py @@ -1,4 +1,4 @@ -# $Id: client.py,v 1.68 2003-01-14 22:21:35 richard Exp $ +# $Id: client.py,v 1.69 2003-01-15 11:07:45 richard Exp $ __doc__ = """ WWW request handler (also used in the stand-alone server). @@ -1242,6 +1242,9 @@ def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')): # it's a MiniFieldStorage, but may be a comma-separated list # of values value = [i.strip() for i in value.value.split(',')] + + # filter out the empty bits + value = filter(None, value) else: # multiple values are not OK if isinstance(value, type([])): @@ -1302,12 +1305,13 @@ def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')): elif isinstance(proptype, hyperdb.Multilink): # perform link class key value lookup if necessary link = proptype.classname + link_cl = db.classes[link] l = [] for entry in value: if not entry: continue if not num_re.match(entry): try: - entry = db.classes[link].lookup(entry) + entry = link_cl.lookup(entry) except KeyError: raise ValueError, _('property "%(propname)s": ' '"%(value)s" not an entry of %(classname)s')%{ @@ -1371,6 +1375,10 @@ def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')): if not existing and not value: continue + # existing will come out unsorted in some cases + if isinstance(proptype, hyperdb.Multilink): + existing.sort() + # if changed, set it if value != existing: props[propname] = value diff --git a/test/test_cgi.py b/test/test_cgi.py index d9887a2..781dbc6 100644 --- a/test/test_cgi.py +++ b/test/test_cgi.py @@ -8,7 +8,7 @@ # but WITHOUT ANY WARRANTY; without even the implied warranty of # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. # -# $Id: test_cgi.py,v 1.2 2003-01-14 22:21:35 richard Exp $ +# $Id: test_cgi.py,v 1.3 2003-01-15 11:07:45 richard Exp $ import unittest, os, shutil, errno, sys, difflib, cgi @@ -70,6 +70,8 @@ class FormTestCase(unittest.TestCase): makeForm({'title': ''})), {}) self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue, makeForm({'title': ' '})), {}) + self.assertRaises(ValueError, client.parsePropsFromForm, self.db, + self.db.issue, makeForm({'title': ['', '']})) def testSetString(self): self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue, @@ -97,10 +99,14 @@ class FormTestCase(unittest.TestCase): def testSetMultilink(self): self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue, makeForm({'nosy': '1'})), {'nosy': ['1']}) + self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue, + makeForm({'nosy': 'admin'})), {'nosy': ['1']}) self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue, makeForm({'nosy': ['1','2']})), {'nosy': ['1','2']}) self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue, makeForm({'nosy': '1,2'})), {'nosy': ['1','2']}) + self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue, + makeForm({'nosy': 'admin,2'})), {'nosy': ['1','2']}) def testEmptyMultilinkSet(self): nodeid = self.db.issue.create(nosy=['1','2']) @@ -110,6 +116,48 @@ class FormTestCase(unittest.TestCase): self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue, makeForm({'nosy': ' '}), nodeid), {'nosy': []}) + def testInvalidMultilinkValue(self): +# XXX This is not the current behaviour - should we enforce this? +# self.assertRaises(IndexError, client.parsePropsFromForm, self.db, +# self.db.issue, makeForm({'nosy': '4'})) + self.assertRaises(ValueError, client.parsePropsFromForm, self.db, + self.db.issue, makeForm({'nosy': 'frozzle'})) + self.assertRaises(ValueError, client.parsePropsFromForm, self.db, + self.db.issue, makeForm({'nosy': '1,frozzle'})) +# XXX need a test for the TypeError (where the ML class doesn't define a key + + def testMultilinkAdd(self): + nodeid = self.db.issue.create(nosy=['1']) + # do nothing + self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue, + makeForm({':add:nosy': ''}), nodeid), {}) + + # do something ;) + self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue, + makeForm({':add:nosy': '2'}), nodeid), {'nosy': ['1','2']}) + self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue, + makeForm({':add:nosy': '2,mary'}), nodeid), {'nosy': ['1','2','4']}) + self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue, + makeForm({':add:nosy': ['2','3']}), nodeid), {'nosy': ['1','2','3']}) + + def testMultilinkRemove(self): + nodeid = self.db.issue.create(nosy=['1','2']) + # do nothing + self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue, + makeForm({':remove:nosy': ''}), nodeid), {}) + + # do something ;) + self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue, + makeForm({':remove:nosy': '1'}), nodeid), {'nosy': ['2']}) + self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue, + makeForm({':remove:nosy': 'admin,2'}), nodeid), {'nosy': []}) + self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue, + makeForm({':remove:nosy': ['1','2']}), nodeid), {'nosy': []}) + + # remove one that doesn't exist? + self.assertRaises(ValueError, client.parsePropsFromForm, self.db, + self.db.issue, makeForm({':remove:nosy': '4'}), nodeid) + # # Password # @@ -118,6 +166,11 @@ class FormTestCase(unittest.TestCase): makeForm({'password': ''})), {}) self.assertEqual(client.parsePropsFromForm(self.db, self.db.user, makeForm({'password': ''})), {}) + self.assertRaises(ValueError, client.parsePropsFromForm, self.db, + self.db.user, makeForm({'password': ['', '']})) + self.assertRaises(ValueError, client.parsePropsFromForm, self.db, + self.db.user, makeForm({'password': 'foo', + 'password:confirm': ['', '']})) def testSetPassword(self): self.assertEqual(client.parsePropsFromForm(self.db, self.db.user,