Mercurial > p > roundup > code

changeset 1384:a87f59436895 maint-0.5

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
ported CGI fixes from HEAD
author Richard Jones <richard@users.sourceforge.net>
date Wed, 15 Jan 2003 22:38:14 +0000
parents 56c5b4509378
children ad9647a6d9fc
files CHANGES.txt roundup/cgi/client.py test/test_cgi.py
diffstat 3 files changed, 110 insertions(+), 16 deletions(-) [+]
line wrap: on
line diff
--- a/CHANGES.txt	Mon Jan 13 04:24:54 2003 +0000
+++ b/CHANGES.txt	Wed Jan 15 22:38:14 2003 +0000
@@ -7,6 +7,7 @@
 - detect corrupted index and raise semi-useful exception (sf bug 666767)
 - open server logfile unbuffered
 - revert StringHTMLProperty to not hyperlink text by default
+- fixes to CGI form handling
 
 
 2003-01-10 0.5.4
--- a/roundup/cgi/client.py	Mon Jan 13 04:24:54 2003 +0000
+++ b/roundup/cgi/client.py	Wed Jan 15 22:38:14 2003 +0000
@@ -1,4 +1,4 @@
-# $Id: client.py,v 1.65 2003-01-08 04:39:36 richard Exp $
+# $Id: client.py,v 1.65.2.1 2003-01-15 22:38:14 richard Exp $
 
 __doc__ = """
 WWW request handler (also used in the stand-alone server).
@@ -1150,6 +1150,7 @@
     text = text.replace('\r\n', '\n')
     return text.replace('\r', '\n')
 
+
 def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')):
     ''' Pull properties for the given class out of the form.
 
@@ -1173,7 +1174,6 @@
     props = {}
     keys = form.keys()
     properties = cl.getprops()
-    existing_cache = {}
     for key in keys:
         # see if we're performing a special multilink action
         mlaction = 'set'
@@ -1188,9 +1188,10 @@
 
         # does the property exist?
         if not properties.has_key(propname):
-            if mlaction == 'remove':
-                raise ValueError, 'You have submitted a remove action for'\
-                    ' the property "%s" which doesn\'t exist'%propname
+            if mlaction != 'set':
+                raise ValueError, 'You have submitted a %s action for'\
+                    ' the property "%s" which doesn\'t exist'%(mlaction,
+                    propname)
             continue
         proptype = properties[propname]
 
@@ -1208,6 +1209,9 @@
                 # 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([])):
@@ -1218,8 +1222,6 @@
             value = value.value.strip()
 
         if isinstance(proptype, hyperdb.String):
-            if not value:
-                continue
             # fix the CRLF/CR -> LF stuff
             value = fixNewlines(value)
         elif isinstance(proptype, hyperdb.Password):
@@ -1247,7 +1249,7 @@
                 value = None
         elif isinstance(proptype, hyperdb.Link):
             # see if it's the "no selection" choice
-            if value == '-1':
+            if value == '-1' or not value:
                 # if we're creating, just don't include this property
                 if not nodeid:
                     continue
@@ -1270,12 +1272,13 @@
         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')%{
@@ -1295,8 +1298,10 @@
                 # we're modifying the list - get the current list of ids
                 if props.has_key(propname):
                     existing = props[propname]
+                elif nodeid:
+                    existing = cl.get(nodeid, propname, [])
                 else:
-                    existing = cl.get(nodeid, propname, [])
+                    existing = []
 
                 # now either remove or add
                 if mlaction == 'remove':
@@ -1335,10 +1340,21 @@
                 if not properties.has_key(propname):
                     raise
 
+            # existing may be None, which won't equate to empty strings
+            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
         else:
+            # don't bother setting empty/unset values
+            if not value:
+                continue
             props[propname] = value
 
     # see if all the required properties have been supplied
@@ -1350,5 +1366,3 @@
         raise ValueError, 'Required %s %s not supplied'%(p, ', '.join(required))
 
     return props
-
-
--- a/test/test_cgi.py	Mon Jan 13 04:24:54 2003 +0000
+++ b/test/test_cgi.py	Wed Jan 15 22:38:14 2003 +0000
@@ -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.4 2003-01-15 11:14:01 richard Exp $
+# $Id: test_cgi.py,v 1.4.2.1 2003-01-15 22:38:14 richard Exp $
 
 import unittest, os, shutil, errno, sys, difflib, cgi
 
@@ -58,9 +58,14 @@
             makeForm({})), {})
 
     def testNothingWithRequired(self):
-        form = makeForm({':required': 'title'})
+        self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
+            self.db.issue, makeForm({':required': 'title'}))
         self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
-            self.db.issue, form)
+            self.db.issue, makeForm({':required': 'title,status',
+            'status':'1'}))
+        self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
+            self.db.issue, makeForm({':required': ['title','status'],
+            'status':'1'}))
 
     #
     # String
@@ -88,6 +93,38 @@
             makeForm({'title': ' '}), nodeid), {'title': ''})
 
     #
+    # Link
+    #
+    def testEmptyLink(self):
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({'status': ''})), {})
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({'status': ' '})), {})
+        self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
+            self.db.issue, makeForm({'status': ['', '']}))
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({'status': '-1'})), {})
+
+    def testSetLink(self):
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({'status': 'unread'})), {'status': '1'})
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({'status': '1'})), {'status': '1'})
+
+    def testUnsetLink(self):
+        nodeid = self.db.issue.create(status='unread')
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({'status': '-1'}), nodeid), {'status': None})
+
+    def testInvalidLinkValue(self):
+# XXX This is not the current behaviour - should we enforce this?
+#        self.assertRaises(IndexError, client.parsePropsFromForm, self.db,
+#            self.db.issue, makeForm({'status': '4'}))
+        self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
+            self.db.issue, makeForm({'status': 'frozzle'}))
+# XXX need a test for the TypeError where the link class doesn't define a key?
+
+    #
     # Multilink
     #
     def testEmptyMultilink(self):
@@ -124,7 +161,7 @@
             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
+# 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'])
@@ -162,6 +199,22 @@
         self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
             self.db.issue, makeForm({':remove:nosy': '4'}), nodeid)
 
+    def testMultilinkRetired(self):
+        self.db.user.retire('2')
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({'nosy': ['2','3']})), {'nosy': ['2','3']})
+        nodeid = self.db.issue.create(nosy=['1','2'])
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({':remove:nosy': '2'}), nodeid), {'nosy': ['1']})
+        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+            makeForm({':add:nosy': '3'}), nodeid), {'nosy': ['1','2','3']})
+
+    def testAddRemoveNonexistant(self):
+        self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
+            self.db.issue, makeForm({':remove:foo': '2'}))
+        self.assertRaises(ValueError, client.parsePropsFromForm, self.db,
+            self.db.issue, makeForm({':add:foo': '2'}))
+
     #
     # Password
     #
@@ -196,6 +249,32 @@
         self.assertEqual(client.parsePropsFromForm(self.db, self.db.user,
             makeForm({'password': '', 'password:confirm': ''}), nodeid), {})
 
+    #
+    # Boolean
+    #
+# XXX this needs a property to work on.
+#    def testEmptyBoolean(self):
+#        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+#            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 testSetBoolean(self):
+#        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+#            makeForm({'title': 'foo'})), {'title': 'foo'})
+#        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+#            makeForm({'title': 'a\r\nb\r\n'})), {'title': 'a\nb'})
+
+#    def testEmptyBooleanSet(self):
+#        nodeid = self.db.issue.create(title='foo')
+#        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+#            makeForm({'title': ''}), nodeid), {'title': ''})
+#        nodeid = self.db.issue.create(title='foo')
+#        self.assertEqual(client.parsePropsFromForm(self.db, self.db.issue,
+#            makeForm({'title': ' '}), nodeid), {'title': ''})
+
 
 def suite():
     l = [unittest.makeSuite(FormTestCase),
Roundup Issue Tracker: http://roundup-tracker.org/