Jorg Heymans wrote:
Hi,
I have provided a patch [1] that removes cocoon's multipart upload parser with one based on commons-fileupload.
My approach breaks existing client code however, so Antonio suggested to get a few more opinions before continueing.
1) Part, PartOnDisk, PartInMemory are not necessary anymore. They are replaced by the FileItem interface of commons fileupload. It should be doable to wrap the FileItems in a Part class as suggested by Reinhard (with a slight change to the contract, see [1] ) .
PartOnDisk and PartInMemory are particular implementations of the general Part contract. An application can rely on obtaining a Part, but cannot expect to have a particular implementation of it if it aims to be truly independent of the deployment environment.
However, testing for "instanceof PartOnDisk" may allow for performance improvements by allowing the application to move around a file rather than reading it and writing it in another location. Commons upload takes a different approach for this with the "write(File)" method that abstracts this. We could add this convenience method to Part to avoid the above-mentioned "instanceof" test.
So my opinion is that we should have a "CommonsUploadPart" that implements the original Part contract. Especially important is the "disposeWithRequest" stuff that is used by CForms: the default is to automatically trash uploaded content when a request ends, but the CForms upload widget can "detach" a Part from the requests so that uploaded content survives across continuation calls.
2) web.xml parameter "overwrite-uploads" is gone. Instead, we need a parameter like "where-to-store-uploads" with values "disk" or "memory". In addition to this (not included in the patch) commons-fileupload offers a "threshold" for memory based storage, files larger than the threshold will get written to disk anyway - we don't *have* to use this however.
3) There is no replacement for getHeaders() in Part.java.
Mmmh... let's keep it and map it the headers provided as methods in FileItem (content-type, length, etc).
The nice thing to do would be to deprecate the Part classes, and wrap the FileItem interface inside Part for 2.1.6. , remove Part in 2.1.7?
Rather than deprecating Part, we should maybe move it to o.a.c.environment, as multipart requests can exist in environments other than servlets (i.e. SMTP).
Sylvain
-- Sylvain Wallez Anyware Technologies http://www.apache.org/~sylvain http://www.anyware-tech.com { XML, Java, Cocoon, OpenSource }*{ Training, Consulting, Projects }
