james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Jencks <david_jen...@yahoo.com>
Subject Re: [mailets] POJOs (and in particular SieveToMultiMailbox)
Date Tue, 02 Sep 2008 21:32:55 GMT

On Sep 2, 2008, at 12:46 PM, Robert Burrell Donkin wrote:

> On Tue, Sep 2, 2008 at 9:53 AM, Stefano Bagnara <apache@bago.org>  
> wrote:
>> Robert Burrell Donkin ha scritto:
>>>
>>> On Mon, Sep 1, 2008 at 2:56 PM, Stefano Bagnara <apache@bago.org>  
>>> 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.
>
> i did remark that setting injection is more popular but i don't intend
> to set a president, just take one more step forward. it may give some
> pointers towards more general solutions
>
>> 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.
>
> setter injection would work best with annotations or interfaces
>
> paired interfaces could be retrofitted easily. for example
>
> interface Mailbox
>
> interface MailboxAware
> {
>   public void setMailbox(Mailbox mailbox)
> }
>
>>> 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 :-)
>
> IMHO MailetContext suffers from doing both too much and too little
>
>>>> 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.
>
> i'm not sure how you know that's true
>
FWIW although you have to supply some metadata (currently via code or  
annotations) Geronimo can construct components through constructor  
injection: there shouldn't be any problem constructing a maillet.

I thought spring could too if you jumped through enough configuration  
hoops but I'm definitely not a spring expert.

I am really really strongly in favor of constructor injection of final  
fields.

thanks
david jencks

>
>
>>> 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.
>
> i'm not sure why factoring out the processor would break configuration
> compatibility
>
>>>> 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?
>
> yep
>
>>> 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.
>
> i'm not sure i agree with this being the easiest approach. there are
> known issues when trying to implement good setter injection frameworks
> and conventions would have to be established for setter names unless
> annotations are used.
>
> the simplest approach would be to use constructor injection and
> exclude optional dependencies
>
> IMO the best approach for setter injection would be to create an
> alternative spring processor loader
>
> i'd like to be able to deploy a self contained jar'd mail application
> eg (sketching somewhat):
>
> META-INF/org/apache/james/list-processor.xml
>
> <procesor loader='org.apache.james.mailet.container.Spring'>
>   <mailet name='ListManager'>
> ....
>   </mailet>
> </processor>
>
> META-INF/org/apache/james/spring-mailets.xml
>
> <beans>
> <bean id='ListManager' class=...>
> ....
> </bean>
> </beans>
>
>>>> 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://joe@example.org" 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://joe@localhost".
>>>
>>> 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://joe@localhost/INBOX" could be distringuished from
>>> "james://joe@localhost/INBOX" and "mailbox://joe@localhost/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.
>
> the sieve mailet is still experiement: it's a good opportunity just to
> give it a go and take the chance to live with it for a while
>
> - robert
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>


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


Mime
View raw message