mime4j storage changes

[Stefano Bagnara <io...@bago.org>]

I saw in order to extract storage to its own module you introduced a
dependency on the "message" package (BodyFactory) while previously it
depended only on the core packages and "dom" API.

I'm not saying this is a bad thing, but I'm recording this is another
"code" that is no more using the dom api and instead relay on the
specific implementation.

IIRC mime4j 0.6 by default used temporary files to store bodies
whenever the body was large (switchable through the use of a system
property). Instead with your latest changes mime4j uses in memory byte
arrays unless the user provides a custom "BodyFactory" implementation
to the MessageServiceFactory.

This change in the default way mime4j deals with parsing and
memory/storage is something that deserved its own thread as it's not a
API refactoring like the others.

Some TODO in case we keep this change:
1) Javadocs still refer to StorageProvider wherever you replaced it
with the BodyFactory.
2) Improve release notes to explain what people have to do to keep 0.6
storage behaviour (thresholdstorage) or keep using their system
property.

Stefano

PS: Please remember to use issue number in svn commit (so that it is
easier to review what are the svn changes related to MIME4J-131)

Re: mime4j storage changes

[Oleg Kalnichevski <ol...@apache.org>]

On Mon, 2011-01-31 at 11:25 +0100, Markus Wiederkehr wrote:
> On Sun, Jan 30, 2011 at 7:40 PM, Stefano Bagnara <io...@bago.org> wrote:
> > IIRC mime4j 0.6 by default used temporary files to store bodies
> > whenever the body was large (switchable through the use of a system
> > property). Instead with your latest changes mime4j uses in memory byte
> > arrays unless the user provides a custom "BodyFactory" implementation
> > to the MessageServiceFactory.
> >
> > This change in the default way mime4j deals with parsing and
> > memory/storage is something that deserved its own thread as it's not a
> > API refactoring like the others.
> 
> I'm not very enthusiastic but I would prefer to keep the previous
> design. I think it is good out-of-the-box behavior if mime4j keeps
> smaller messages in memory and automatically stores larger ones on
> disk.

(1) The advantage of using an all-in-memory backend per default is
relieving the users from having to worry about having to explicitly
dispose of Message objects. Advanced storage backends may come with
additional responsibilities such as resource deallocation, so it would
be nice to make users realize that by making them explicitly declare
dependency on the storage module.  

(2) It was a big and a very unpleasant surprise to find out that mime4j
could quietly create temp files per default when parsing messages. This
makes mime4j prone to security issues when used for parsing confidential
messages. I understand this problem can be addressed by documenting the
default behavior more prominently, but the sad fact is that people do
not RTFM.    

> The motivation probably is to make DOM independent of storage but I
> don't think that's really necessary. 

Yes, that was the main motivating factor. I felt it was regrettable that
storage classes dragged in dependency on java crypto into 'dom'
packages.

> One could think of storage as
> part of DOM (or at least part of a certain DOM implementation) that
> can also be used standalone. If you really wanted to remove that
> dependency maybe dom and dom-impl modules would do it?
> 
> Cheers,
> Markus
> 

Stefano, Markus 

I'll revert the change or make 'dom' depend on 'storage' if you tell
which option sounds more preferable to you.

Oleg

Re: mime4j storage changes

[Markus Wiederkehr <ma...@gmail.com>]

On Sun, Jan 30, 2011 at 7:40 PM, Stefano Bagnara <io...@bago.org> wrote:
> IIRC mime4j 0.6 by default used temporary files to store bodies
> whenever the body was large (switchable through the use of a system
> property). Instead with your latest changes mime4j uses in memory byte
> arrays unless the user provides a custom "BodyFactory" implementation
> to the MessageServiceFactory.
>
> This change in the default way mime4j deals with parsing and
> memory/storage is something that deserved its own thread as it's not a
> API refactoring like the others.

I'm not very enthusiastic but I would prefer to keep the previous
design. I think it is good out-of-the-box behavior if mime4j keeps
smaller messages in memory and automatically stores larger ones on
disk.

The motivation probably is to make DOM independent of storage but I
don't think that's really necessary. One could think of storage as
part of DOM (or at least part of a certain DOM implementation) that
can also be used standalone. If you really wanted to remove that
dependency maybe dom and dom-impl modules would do it?

Cheers,
Markus