Robert Burrell Donkin ha scritto:
On Mon, Sep 1, 2008 at 2:56 PM, Stefano Bagnara <[EMAIL PROTECTED]> wrote:
Robert Burrell Donkin ha scritto:
SieveToMultiMailbox is the major remaining issue on the road to
separating out an IMAP protocol library. it's a good example of some
of the problems with the remaining mailets in JAMES. hopefully,
thinking about solving them in this case may be applied more widely.
----
SieveToMultiMailbox is tightly coupled to a large IMAP specific API
(mailboxmanager) but uses only a tiny portion of it. an alternative
implementation of the mailboxmanager API would be faced with
implementing at least 29 methods spread over 3 interfaces. the mailet
only delivers a mail to a mailbox which should really require a single
call to a single method. this pattern is repeated across JAMES. the
API is shared by protocols, data stores and mailets resulting in
unnecessary large and imprecise interfaces. mailets typically only use
a small subset of the function required by protocols and stores.
explicit SPIs specialised for mailets would be a better approach.
these could be independently versioned allowing the APIs used
internally by JAMES to be safely varied.
an SPI interface for mailboxes as simple as (for example):
public void put(String url, Mail mail);
would suffice
----
SieveToMultiMailbox is tightly coupled to avalon. the IMAP dependency
is loaded by a service lookup using avalon. JAMES server is the only
mailet container that is likely to support avalon. more modern IoC
containers (for example, spring) would unobtrusively inject the
required dependency. replacing magic lookup with construction
injection would mean adding something like:
public SieveToMultiMailbox(MailboxSPI spi)
this would mean refactoring the mailet/matcher loading code to allow
replacement by an implementation that uses a modern container. (i've
always been keen on being able to ship independent self-contained mail
applications built from mailets which could just be dropped into a
running server.) the complexity would come creating a mixed system.
using some sort of avalon-aware service adapters would probably be
necessary.
MailboxSPI might be provided by adapting a mailbox manager service.
----
opinions?
I prefer setter injection to constructor injection (expecially as we have an
init method in the mailet api).
i know a lot of people prefer setter injection but constructor
injection has some advantages in this case:
1. it prevents uninitialised use of the mailet. this should give a
clean failure when anyone tries to use it with an incompatible mailet
loader.
THe mailet api require an "init" method. In that method we can check if
the services are there and throw exceptions to prevent this.
This also does not introduce multiple constructors when dealing with
optional dependencies.
IN past we discussed about what IoC style we prefer and SDI was the
"winner":
http://issues.apache.org/jira/browse/JAMES-494
Here you can find the biggest poll we've ever had ;-)
http://markmail.org/message/rfagbuq6t3au7uia
"G" questions was about dependency injection.
Removing the default constructor will also make my new mailet report
maven module to fail obtaining the getMailetInfo/getMatcherInfo answer.
It have to get a newInstance in order to invoke that method.
Not a big deal, and if you go with CDI it will anyway be easy to
refactor to SDI in any moment.
2. no conventions need to be established or explained. as a temporary
measure, it's easy to check and inject a limited number of SPIs
through reflection.
i would prefer to avoid major changes before moving IMAP out
We have a "store" method in MailetContext and we deprecated it because
it was not clear how to proceed and we simply discouraged it's usage to
feel more free to change it later :-)
I don't understand if you are proposing something to be added to the mailet
specification
ATM AIUI the mailet specification is silent on assembly and service
acquisition. one advantage of this strategy is that it's possible to
alter these characteristics without altering the mailet specification.
True and False... there was not support anything using services will not
work on current containers. So let's say "there's no way to be backward
compatible, so we can do whatever we want" ;-)
Every existing container will fail to load a mailet without a default
constructor.
IMHO factoring out the mailet loading and the processor assembly from
the spool management would be a major step forward
Mmmm.. this is already this way.
MailetLoader and MatcherLoader are 2 components used by the SpoolManager
to get instances of the mailets.
<!-- The James Spool Manager block -->
<block name="spoolmanager"
class="org.apache.james.transport.JamesSpoolManager" >
<provide name="spoolrepository"
role="org.apache.james.services.SpoolRepository"/>
<provide name="matcherpackages"
role="org.apache.james.transport.MatcherLoader"/>
<provide name="mailetpackages"
role="org.apache.james.transport.MailetLoader"/>
</block>
<block name="matcherpackages"
class="org.apache.james.transport.JamesMatcherLoader" >
<provide name="James" role="org.apache.mailet.MailetContext"/>
<provide name="filesystem" role="org.apache.james.services.FileSystem" />
</block>
<block name="mailetpackages"
class="org.apache.james.transport.JamesMailetLoader" >
<provide name="James" role="org.apache.mailet.MailetContext"/>
<provide name="filesystem" role="org.apache.james.services.FileSystem" />
</block>
JamesSpoolManager is configurable to define what class to use as
processor defaulting to StateAwareProcessorList.
---
processorClass =
conf.getChild("processorClass").getValue("org.apache.james.transport.StateAwareProcessorList");
----
When I refactored this in my mind there was the idea to bring as many
components to a top level. The idea is that StateAwareProcessorList
could be a component itself and the SpoolManager could simply depend on
a MailProcessor component.
This should be easy to do now. I didn't do that before simply because
when I worked on this everyone was worried about backward compatibility,
so no change was allowed to break config.xml and db content
compatibility. There are many hacks in the code to deal with this.
or simply you are proposing to change the way james specific
mailets are written so to not require avalon knowledge to the james specific
mailets.
IMHO JAMES specific mailets are an anti-pattern. we need to work
towards decoupling minimal SPIs for mailets from the large APIs used
internally by JAMES. i prefer to think about mailet loaders and
processor assemblers indepedently. avalon is not a good match for this
problem. more modern IoC containers like pico or spring as *much*
better.
I agree that james specific mailets are antipattern.
To make them generic you have to define services at the mailet level.
And make the services generic.
To change the service lookup method without publicizing the services
does not make the mailets portable to other services.
If a Mailet depends on SpoolRepository interface we can remove avalon
and the service manager by using simple CDI/SDI, but in the end the
mailet will be only usable where SpoolRepository implementations are
available. SpoolRepository is james specific.
MailboxSPI is james specific until we make it part of a generic API
supported by compliant containers.
I don't understand what is your idea about this? Do you want to make an
mailet-spi package with optional services to be supported by advanced
containers?
i strongly dislike the use of the magic avalon service attribute. IMHO
if a mailet is coupled to avalon then it should implement the standard
servicable interface. the container could then recognize this and
configure the mailet appropriately.
I agree that the context and the service lookup are a bad idea for
mailets. IMHO this is a necessary change, but this alone won't change
the fact that mailets are james specific.
In http://issues.apache.org/jira/browse/JAMES-494 you can see the
easiest approach is to add setters for dependencies and make the
"service" method lookup the services and set them via setters. This way
the avalon container will keep working while non avalon containers can
simply use setters.
Another doubt is about the use of "String url": can you give more details on
the allowed values for url and the way it works (a couple of examples would
suffice, I guess)?
i've been thinking for a long while that there might be a lot to gain
in flexibility by moving towards APIs using URIs
but it's still hazy...
Last point, and the least important, I don't like the SPI postfix.
The same interface could be used by SMTPServer/Fetchmail to store messages
to the spool. In fact a spool repository could expose this interface
(ignoring the url) or we could expose it via the Store by looking up the
appropriate spool repository and storing the message to it.
i have been thinking about that direction
for example, POSTing to "mailto://[EMAIL PROTECTED]" is an elegant and
concise way to express the idea of forwarding a mail. this can be used
for delivery as well. for example "mailet://[EMAIL PROTECTED]".
the advantage of using URLs is that it's easier to present interfaces
which work ok for a wider variety of protocols. for example POSTing to
"imap://[EMAIL PROTECTED]/INBOX" could be distringuished from
"james://[EMAIL PROTECTED]/INBOX" and "mailbox://[EMAIL PROTECTED]/INBOX".
might be possible to do some interesting stuff this way.
I like this stuff very much. I discussed a similar thing in past. I
named it "destinations":
http://markmail.org/message/xkcttgyqmfwpieew
In my 2005 idea "destinations" was particular email addresses because
email address is more "mail" oriented than url, but mail is moving to
different transport and everyone probably better understand urls than
email addresses.
But I think it would be a very interesting paradigm to use urls for
anything from local spool management, to remote delivery to forward and
anything else.
The issue I see here is that we should understand how they works before
starting to add interfaces using this urls.
Stefano
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
