Re: More efficient StringField and Field classes

2005-11-25 Thread Mike Looijmans
Is there a way that I can run the unit tests on a Windows systems? I 
have plenty of Linux boxes here, but I'd have to build and install 
apache in a private corner just to run the tests.


Anyway, I see where the problem is, in tests.py it does:

def util_fieldstorage(req):
from mod_python import util
req.write(`util.FieldStorage(req).list`)
return apache.OK

So I'll just add a __repr__ function to the StringField class and the 
test should pass then.


I have attached a patch (on the 3.2.5b version of util.py) to include 
this change. That should fix the unit test, and it did not break any of 
my own code.



Jim Gallacher wrote:

Hi Mike,

I don't have time to dig into this tonight, but your patch causes one of 
the unit tests to fail.


FAIL: test_util_fieldstorage (__main__.PerRequestTestCase)
--
Traceback (most recent call last):
  File test.py, line 1117, in test_util_fieldstorage
self.fail(`rsp`)
  File /usr/lib/python2.3/unittest.py, line 270, in fail
raise self.failureException, msg
AssertionError: ['1', '2', '3', '4']


Jim




--
Mike Looijmans
Philips Natlab / Topic Automation
Index: util.py
===
--- util.py (revision 348746)
+++ util.py (working copy)
@@ -48,19 +48,8 @@
 
 
 class Field:
-   def __init__(self, name, file, ctype, type_options,
-disp, disp_options, headers = {}):
+   def __init__(self, name):
self.name = name
-   self.file = file
-   self.type = ctype
-   self.type_options = type_options
-   self.disposition = disp
-   self.disposition_options = disp_options
-   if disp_options.has_key(filename):
-   self.filename = disp_options[filename]
-   else:
-   self.filename = None
-   self.headers = headers
 
def __repr__(self):
Return printable representation.
@@ -81,13 +70,34 @@
self.file.close()
 
 class StringField(str):
-This class is basically a string with
-   a value attribute for compatibility with std lib cgi.py
-   
+ This class is basically a string with
+added attributes for compatibility with std lib cgi.py. Basically, this
+works the opposite of Field, as it stores its data in a string, but creates
+a file on demand. Field creates a value on demand and stores data in a 
file.
+
+filename = None
+headers = {}
+ctype = text/plain
+type_options = {}
+disposition = None
+disp_options = None
+
+# I wanted __init__(name, value) but that does not work (apparently, you
+# cannot subclass str with a constructor that takes 1 argument)
+def __init__(self, value):
+'''Create StringField instance. You'll have to set name yourself.'''
+str.__init__(self, value)
+self.value = value
 
-   def __init__(self, str=):
-   str.__init__(self, str)
-   self.value = self.__str__()
+def __getattr__(self, name):
+if name != 'file':
+raise AttributeError, name
+self.file = cStringIO.StringIO(self.value)
+return self.file
+
+def __repr__(self):
+   Return printable representation (to pass unit tests).
+   return Field(%s, %s) % (`self.name`, `self.value`)
 
 class FieldStorage:
 
@@ -103,8 +113,7 @@
if req.args:
pairs = parse_qsl(req.args, keep_blank_values)
for pair in pairs:
-   file = cStringIO.StringIO(pair[1])
-   self.list.append(Field(pair[0], file, text/plain, {}, None, 
{}))
+   self.add_field(pair[0], pair[1])
 
if req.method != POST:
return
@@ -123,9 +132,7 @@
if ctype.startswith(application/x-www-form-urlencoded):
pairs = parse_qsl(req.read(clen), keep_blank_values)
for pair in pairs:
-   # TODO : isn't this a bit heavyweight just for form fields ?
-   file = cStringIO.StringIO(pair[1])
-   self.list.append(Field(pair[0], file, text/plain, {}, None, 
{}))
+   self.add_field(pair[0], pair[1])
return
 
if not ctype.startswith(multipart/):
@@ -205,33 +212,44 @@
else:
name = None
 
+   # create a file object
# is this a file?
if disp_options.has_key(filename):
if file_callback and callable(file_callback):
file = file_callback(disp_options[filename])
else:
-   file = self.make_file()
+   file = tempfile.TemporaryFile(w+b)
else:
if field_callback and callable(field_callback):
file = field_callback()
else:
-   file = self.make_field()
+   file = cStringIO.StringIO()
 
# read it in
-   end_of_stream = self.read_to_boundary(req, boundary, file)
+   

Re: More efficient StringField and Field classes

2005-11-24 Thread Mike Looijmans

It has been awhile, but I finally made the patch.
Advice and feedback is welcome.

What it does:
- Simplifies the creation of StringField objects. This was already
marked as a TODO in the 3.2.5b code.
- Does not create a file object (cStringIO) for each and every field.
- FieldStorage.get() will always return the same instance(s) for any
given name.
- FieldStorage.get() is very cheap now (does not create new objects,
which is expensive in Python)

It may break:
- Code that creates an instance of Field or StringField directly. This
can be fixed by extending the __init__() routine, but I don't think it's
needed.

It works:
- Perfect on all of my projects.

Attached:
- Patch created by SVN

--
Mike Looijmans
Philips Natlab / Topic Automation
Index: util.py
===
--- util.py (revision 348746)
+++ util.py (working copy)
@@ -48,19 +48,8 @@
 
 
 class Field:
-   def __init__(self, name, file, ctype, type_options,
-disp, disp_options, headers = {}):
+   def __init__(self, name):
self.name = name
-   self.file = file
-   self.type = ctype
-   self.type_options = type_options
-   self.disposition = disp
-   self.disposition_options = disp_options
-   if disp_options.has_key(filename):
-   self.filename = disp_options[filename]
-   else:
-   self.filename = None
-   self.headers = headers
 
def __repr__(self):
Return printable representation.
@@ -81,13 +70,30 @@
self.file.close()
 
 class StringField(str):
-This class is basically a string with
-   a value attribute for compatibility with std lib cgi.py
-   
+ This class is basically a string with
+added attributes for compatibility with std lib cgi.py. Basically, this
+works the opposite of Field, as it stores its data in a string, but creates
+a file on demand. Field creates a value on demand and stores data in a 
file.
+
+filename = None
+headers = {}
+ctype = text/plain
+type_options = {}
+disposition = None
+disp_options = None
+
+# I wanted __init__(name, value) but that does not work (apparently, you
+# cannot subclass str with a constructor that takes 1 argument)
+def __init__(self, value):
+'''Create StringField instance. You'll have to set name yourself.'''
+str.__init__(self, value)
+self.value = value
 
-   def __init__(self, str=):
-   str.__init__(self, str)
-   self.value = self.__str__()
+def __getattr__(self, name):
+if name != 'file':
+raise AttributeError, name
+self.file = cStringIO.StringIO(self.value)
+return self.file
 
 class FieldStorage:
 
@@ -103,8 +109,7 @@
if req.args:
pairs = parse_qsl(req.args, keep_blank_values)
for pair in pairs:
-   file = cStringIO.StringIO(pair[1])
-   self.list.append(Field(pair[0], file, text/plain, {}, None, 
{}))
+   self.add_field(pair[0], pair[1])
 
if req.method != POST:
return
@@ -123,9 +128,7 @@
if ctype.startswith(application/x-www-form-urlencoded):
pairs = parse_qsl(req.read(clen), keep_blank_values)
for pair in pairs:
-   # TODO : isn't this a bit heavyweight just for form fields ?
-   file = cStringIO.StringIO(pair[1])
-   self.list.append(Field(pair[0], file, text/plain, {}, None, 
{}))
+   self.add_field(pair[0], pair[1])
return
 
if not ctype.startswith(multipart/):
@@ -205,33 +208,44 @@
else:
name = None
 
+   # create a file object
# is this a file?
if disp_options.has_key(filename):
if file_callback and callable(file_callback):
file = file_callback(disp_options[filename])
else:
-   file = self.make_file()
+   file = tempfile.TemporaryFile(w+b)
else:
if field_callback and callable(field_callback):
file = field_callback()
else:
-   file = self.make_field()
+   file = cStringIO.StringIO()
 
# read it in
-   end_of_stream = self.read_to_boundary(req, boundary, file)
+   self.read_to_boundary(req, boundary, file)
file.seek(0)
-
+ 
# make a Field
-   field = Field(name, file, ctype, type_options, disp, disp_options, 
headers)
-
+   if disp_options.has_key(filename):
+   field = Field(name)
+   field.filename = disp_options[filename]
+   else:
+   field = StringField(file.read())
+   field.name = name
+   field.file = file
+   field.type = ctype
+   field.type_options = type_options
+   field.disposition = disp
+   

Re: More efficient StringField and Field classes

2005-11-24 Thread Jim Gallacher

Hi Mike,

I don't have time to dig into this tonight, but your patch causes one of 
the unit tests to fail.


FAIL: test_util_fieldstorage (__main__.PerRequestTestCase)
--
Traceback (most recent call last):
  File test.py, line 1117, in test_util_fieldstorage
self.fail(`rsp`)
  File /usr/lib/python2.3/unittest.py, line 270, in fail
raise self.failureException, msg
AssertionError: ['1', '2', '3', '4']


Jim