You are viewing a plain text version of this content. The canonical link for it is here.
Posted to mod_python-dev@quetz.apache.org by Mike Looijmans <nl...@natlab.research.philips.com> on 2005/11/24 17:13:53 UTC

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

Re: More efficient StringField and Field classes

Posted by Nicolas Lehuen <ni...@gmail.com>.
I've ran the unit test on Windows with your patch and everything seems fine.

Regards,
Nicolas

2005/11/25, Mike Looijmans <nl...@natlab.research.philips.com>:
> 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)
> +           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
> +           field.disposition_options = disp_options
> +           field.headers = headers
>             self.list.append(field)
>
> -   def make_file(self):
> -       return tempfile.TemporaryFile("w+b")
> +   def add_field(self, key, value):
> +       """Insert a field as key/value pair"""
> +       item = StringField(value)
> +       item.name = key
> +       self.list.append(item)
>
> -   def make_field(self):
> -       return cStringIO.StringIO()
> -
>     def read_to_boundary(self, req, boundary, file):
>          previous_delimiter = None
>          while True:
> @@ -288,11 +306,7 @@
>         found = []
>         for item in self.list:
>             if item.name == key:
> -               if isinstance(item.file, FileType) or \
> -                      isinstance(getattr(item.file, 'file', None), FileType):
> -                   found.append(item)
> -               else:
> -                   found.append(StringField(item.value))
> +               found.append(item)
>         if not found:
>             raise KeyError, key
>         if len(found) == 1:
> @@ -333,11 +347,7 @@
>         """ return the first value received """
>         for item in self.list:
>             if item.name == key:
> -               if isinstance(item.file, FileType) or \
> -                      isinstance(getattr(item.file, 'file', None), FileType):
> -                   return item
> -               else:
> -                   return StringField(item.value)
> +               return item
>         return default
>
>     def getlist(self, key):
> @@ -347,11 +357,7 @@
>         found = []
>         for item in self.list:
>             if item.name == key:
> -               if isinstance(item.file, FileType) or \
> -                      isinstance(getattr(item.file, 'file', None), FileType):
> -                   found.append(item)
> -               else:
> -                   found.append(StringField(item.value))
> +               found.append(item)
>         return found
>
>  def parse_header(line):
>
>
>

Re: More efficient StringField and Field classes

Posted by Mike Looijmans <nl...@natlab.research.philips.com>.
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

Re: More efficient StringField and Field classes

Posted by Jim Gallacher <jp...@jgassociates.ca>.
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