[SPAM] [Fwd: Re: [mod_python] util.FieldStorage]

I for one like the changes - it's much cleaner this way. There's only
one list to deal with now, and that list handles all access..

Just one minor optimization, in __len__ i think you can replace:
         return len(self.list.table().keys())
with
         return len(self.list.table())
since that prevents the keylist from being generated.

We might solve all the "dictionary" questions if we provide a
"getdict()" method (which would return a copy of list.table()). Another,
idea is, could we support the following:

form = FieldStorage(...)
myfrm = {}
myfrm.update(form)

This would give those who really want that dictionary a real dictionary
to work with, if they don't need the fancy FieldStorage features.


Mike Looijmans
Philips Natlab / Topic Automation


Graham Dumpleton wrote:
> Mike Looijmans wrote ..
> 
>>Being the one who moved all that code around, I'm probably the one to 
>>explain why.
>>
>>In the original version, there was a 'bad' way of detecting whether 
>>objects were files or not. If one used the callback mechanism, and 
>>provided a file-like object, this object would not be recognised as a 
>>file by that code, and that either resulted in the whole file being read
>>into memory as a string or in some obscure exception being thrown.
> 
> 
> Okay, so more than just performance issues with field access.
> 
> Anyway, I have committed some changes into repository now which
> fixes the issue with add_field() but also does things in such a way that
> code such as Trac which creates Field instances direct and adds them
> to the the list of form fields will still work. It does this by using a
> derived list class instance which invalidates the key lookup table on
> changes to the list so that the lookup table is rebuilt the next time
> it is required. Doing this wasn't as bad as I first thought it would be
> as hadn't gone to the extent of properly understanding the code before.
> Also include a semi hack in Field class constructor to detect old style
> use where code expects more than just the name of the field as
> argument and do appropriate initialisation as appropriate.
> 
> I'd add a comment to JIRA, but it seems to be playing up at the moment
> or at least firewall where I work is screwing up as it tends to do a bit
> when JIRA is accessed using https from here.
> 
> In all, what the changes allow is for the following to work:
> 
> from mod_python import util
> from StringIO import StringIO
> 
> def index(req):
>   req.content_type = 'text/plain'
> 
>   req.write('%s\n\n' % req.form.keys())
> 
>   for key in req.form.keys():
>     req.write('%s %s\n' % (key, req.form.getlist(key)))
> 
>   req.write('\n')
> 
>   req.form.add_field('e', 'f')
> 
>   req.form.list.append(util.Field('g', StringIO('h'),
>       "text/plain", {}, None, {}))
>   req.form.list.extend([util.Field('i', StringIO('j'),
>       "text/plain", {}, None, {})])
>   req.form.list += [util.Field('k', StringIO('l'),
>       "text/plain", {}, None, {})]
> 
>   req.write('%s\n\n' % req.form.keys())
> 
>   for key in req.form.keys():
>     req.write('%s %s\n' % (key, req.form.getlist(key)))
> 
>   req.write('\n')
> 
>   return None
> 
> If this is accessed with:
> 
>   http://localhost:8002/~grahamd/form/?a=b&c=d&e=f
> 
> I get:
> 
> ['a', 'c', 'e']
> 
> a [Field('a', 'b')]
> c [Field('c', 'd')]
> e [Field('e', 'f')]
> 
> ['a', 'c', 'e', 'g', 'i', 'k']
> 
> a [Field('a', 'b')]
> c [Field('c', 'd')]
> e [Field('e', 'f'), Field('e', 'f')]
> g [Field('g', 'h')]
> i [Field('i', 'j')]
> k [Field('k', 'l')]
> 
> As the req.form.keys() method works off the lookup table, you can see
> how updates direct to list work okay, meaning stuff like Trac should
> work.
> 
> You will also note that the 'dictionary' attribute has gone and is hidden
> elsewhere. Hopefully people will not try and directly access the
> equivalent, but some stern notes in the documentation saying only
> to use documented interface might help. :-)
> 
> If you can test the updates and especially if someone can test it with
> older versions of Trac that would be great.
> 
> Graham
> 
> 
>>Graham Dumpleton wrote:
>>
>>>I'd probably hold off trying to work out any patches yourself. The more
>>>I look at FieldStorage the less I understand why some of the changes
>>>were made. I'm not talking about how a dictionary based index was
>>>added, but more how some of the code was shifted around when it
>>>probably didn't need to, with the result being that the code then became
>>>incompatible with versions of Trac which were out at the time. It would
>>>not have been that hard to maintain compatibility with Trac and still
>>
>>get
>>
>>>the desired gains they were after. I can see myself therefore doing a
>>
>>bit
>>
>>>or reorganisation of the code to fix the identified issue, but also 
>>>reinstate
>>>Trac compatibility.
> 
> 


-- 
Mike Looijmans
Philips Natlab / Topic Automation