I dug into upload tonight to help some confusion on the users list and noticed some new things which were introduced since 2.1.3 (in nov. by Sylvain to support woody). I'd like to propose some tweaks which seem wise enough to me to get in quickly or it will be too late.

1) A new concept of disposeWithRequest (private boolean) was added to org.apache.cocoon.servlet.multipart.Part to signal cleanup behavior at the end of the request. MultipartHttpServletRequest method cleanup is called at the end of processing in CocoonServlet and is now:
/**
* Cleanup eventually uploaded parts that were saved on disk
*/
public void cleanup() throws IOException {
Enumeration e = getParameterNames();
while (e.hasMoreElements()) {
Object o = get( (String)e.nextElement() );
if (o instanceof Part) {
Part part = (Part)o;
if (part.disposeWithRequest()) {
part.dispose();
}
}
}
}


I like this (except the accessor name as noted below).

2) The method dispose() was added by having Part implement Disposable. I like the method, but didn't like implementing a lifecycle method on what is otherwise not a component. I think dispose (or another name if the overlapping name is deemed too confusing) should just be added to the abstract Part without Disposable.

3) dispose() is public - but it could be protected, or even package private. I can't think of a reason why code outside this limited scope (the cleanup method above) should be calling dispose() directly, unless Woody has a use for it that I haven't found.

3) A public accessor is created called boolean disposeWithRequest(). This seems better called isDisposeWithRequest() and I don't see why it needs to be public. It's intended use is in the cleanup method above, unless woody has a use for it that I haven't found.

Sorry I just noticed these things - I glanced at the cvs mail at the time it went by but didn't notice the issues above until I dug through tonight.

Geoff

Reply via email to