Re: More efficient StringField and Field classes
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
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
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