commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jochen Wiedmann <jochen.wiedm...@gmail.com>
Subject [FILEUPLOAD] RfC: Proposed API changes for streaming
Date Fri, 09 Jun 2006 20:04:56 GMT

Hi,

first of all, let me say thanks for giving me the possibility to contribute
to the project as a committer. Let's hope, I'll be doing fine. ;-)

As already written, the changes required for streaming are nontrivial. I'd
like to discuss them even if I were an experienced project member. Which I
am not, so please be alert even more.

The main reason for API changes is, that the existing interface FileItem is
to powerful. Therefore I propose to introduce a new interface, let's call it
SlimFileItem, or StreamingFileItem, with the following signature:

    public interface SlimFileItem extends Serializable {
        InputStream getInputStream() throws IOException;
        String getContentType();
        String getName();
        String getFieldName();
        boolean isFormField();
    }

As you can see, the FileItem can be changed to extend SlimFileItem.
Additionally, FileItem has a semantic difference: It allows to invoke
getInputStream() more than once, while SlimFileItem should throw an
IllegalStateException on the second invocation. So far, these changes should
not introduce a problem.

The SlimFileItem is not to be created by a factory: The need for a factory
is mainly caused by the fact, that the FileItem's data needs to be written.
SlimFileItem is best implemented as an inner class of the MultipartStream.

So the API changes can be reduced to an additional method

    Iterator /* SlimFileItem */ getItemIterator(RequestContext ctx)
      throws FileUploadException;

in FileUploadBase. The existing method

    List parseRequest(RequestContext ctx) throws FileUploadException;

would be changed to use getItemIterator internally and convert the
SlimFileItem instances into FileItem.

Finally, a suggestion for simplification: Let's change the
FileUploadException to extend IOException, and not Exception. It will avoid
a lot of casting, type checking, and conversions. Besides, it also makes
sense in the semantic level. At least, I do thing so.

The only argument I can see against the exception change is the following:
People will see compiler errors when using constructs like

    try {
       ...
    } catch (IOException e) {
       ...
    } catch (FileUploadException e) {
       ...
    }

The obvious solution would be to change the order. IMO, that's acceptable.


Regards for any comments,

Jochen

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message