james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Steve Brewin <sbre...@synsys.com>
Subject Re: [mailets] Proposal: MailetContext enhancement
Date Sun, 18 Dec 2011 16:58:46 GMT
There's a problem with all of this!

The sendMail(.) methods implemented by JamesMailetContext perform their 
processing within a try/catch/finally block where the finally stanza 
guarantees the invocation of LifecycleUtil.dispose(..) to release the 
resources held by the MailImpl instance.

If, as proposed, we allow a MailContext to create a Mail, we can no 
longer guarantee that dispose will be called and therefore expose the 
server to resource leaks. Overriding Object.finalize() to call 
LifecycleUtil.dispose(..) cannot be relied on as it cannot be guaranteed 
that Mailet processing, which is outside of our control, correctly 
releases its resources so that the MailImpl is eventually garbage collected.

Well >>could<< guarantee disposal by adding a further parameter to the 
createMail(..) methods enabling mailet behaviour to be performed within 
a try/catch/finally block where the finally stanza guarantees the 
invocation of LifecycleUtil.dispose(..) as before. For example, give...

interface Performable {
public void perform(Mail mail);
}

interface MailetContext {
...
void createMail(Mail mail, new Performable();
}

...we could safely write...

  void createMail(Mail mail, new Performable(){
    perform(Mail mail)
    {
        mail.setAttribute("key", "value");
        sendMail(mail, Mail.State.DEFAULT);
    }
};

I say >>could<< as this complicates things,  perhaps more than is 
desirable. This said, I like it! Should we go this route we certainly 
need to add the sendMail(..) methods to Mailet Base to wrap this stuff, 
hiding the complications for simple use-cases.

Opinions?

Cheers
--Steve

On 18/12/2011 16:29, Steve Brewin wrote:
> Hi
>
> To decouple org.apache.james.transport mailets from 
> org.apache.james.core.MailImpl the following methods need to be added 
> to org.apache.mailet.Mail:
>
> setRemoteAddr(String);
>
> setRemoteHost(String);
>
> setSender(MailAddress);
>
> Nothing wrong with this in my view. They probably should have been 
> there all along!
>
> The Mailets coupled to MailImpl are:
>
> AbstractRecipientRewriteTable
>
> AbstractRedirect
>
> DSNBounce
>
> Each uses new MailImpl(Mail) to create a new instance of Mail. This 
> would change to getMailetContext().createMail(Mail).
>
> If we are to update MailetContext, which will require a new version of 
> Mailet API,  we should make the changes to Mail in the same version.
>
> Opinions?
>
> Cheers
> --Steve
>
> On 18/12/2011 11:45, Steve Brewin wrote:
>> Missed...
>>
>> createMail(Mail mail, Mail.State state)
>>
>> ...to satisfy the a need to create a copy of a Mail. I'll review the 
>> needs of the org.apache.james.transport.mailets to make sure we 
>> haven't missed anything else!
>>
>> Cheers
>> --Steve
>>
>> On 18/12/2011 10:25, Norman Maurer wrote:
>>> Yeah I think the latter makes most sense.
>>>
>>> Bye
>>> Norman
>>>
>>> 2011/12/18, Steve Brewin<sbrewin@synsys.com>:
>>>> Hi Norman
>>>>
>>>> Well I was trying to be less radical, but a createMail() method or
>>>> methods as a replacement for the sendMail() ones is a better solution.
>>>>
>>>> If we were to have just a single create method, it would be:
>>>>
>>>> createMail(MimeMessage message, Mail.State state)
>>>>
>>>> Note that I have changed 'state' from a simple String to an enum.
>>>>
>>>> As the API deals in things like MailAddress, it is perhaps 
>>>> reasonable to
>>>> add:
>>>>
>>>> createMail(MailAddress sender, Collection<MailAddress>   recipients,
>>>> MimeMessage message, Mail.State state)
>>>>
>>>> The other variants are just helper methods that should not be 
>>>> forced on
>>>> an API. They could be implemented in Mailet Base if someone cared 
>>>> to do
>>>> so, as could the old sendMail() methods.
>>>>
>>>> Cheers
>>>> --Steve
>>>>
>>>> On 18/12/2011 08:52, Norman Maurer wrote:
>>>>> Hi Steve,
>>>>>
>>>>> I think it would make more sense to add a method to the 
>>>>> MailetContext to
>>>>> create a new Mail object, so we could also make some other mailets 
>>>>> that
>>>>> are
>>>>> currently using MailImpl directly independent of James Server.
>>>>>
>>>>> So I'm in favor to add such a method and @deprecate all the 
>>>>> sendMail(..)
>>>>> methods except the one the take a Mail object as parameter.
>>>>>
>>>>> WDYT ?
>>>>>
>>>>> Norman
>>>>>
>>>>>
>>>>> 2011/12/17 Steve Brewin<sbrewin@synsys.com>
>>>>>
>>>>>> For a new Mail, using MailetContext.sendMail(Mail mail) requires

>>>>>> that the
>>>>>> mailet knows how to create an implementation of Mail that is 
>>>>>> specific to
>>>>>> the server hosting the Mailets. This breaks Mailet portability, 
>>>>>> which is
>>>>>> why the other sendMail() methods exist.
>>>>>>
>>>>>> MailetContext.sendMail(Mail mail)  exists to resend an existing 
>>>>>> Mail, the
>>>>>> others are for creating and sending new Mails.
>>>>>>
>>>>>> --Steve
>>>>>>
>>>>>>
>>>>>> On 17/12/2011 19:53, Norman Maurer wrote:
>>>>>>
>>>>>>> I wonder why you can not use :
>>>>>>>
>>>>>>> MailetContext.sendMail(Mail mail)
>>>>>>>
>>>>>>> Can you give some more details ?
>>>>>>>
>>>>>>> Bye,
>>>>>>> Norman
>>>>>>>
>>>>>>> 2011/12/17 Steve Brewin<sbrewin@synsys.com>
>>>>>>>
>>>>>>>    Hi
>>>>>>>> Interface org.apache.mailet.****MailetContext defines four

>>>>>>>> sendMail()
>>>>>>>>
>>>>>>>> methods that construct and send an org.apache.mailet.Mail

>>>>>>>> instance.
>>>>>>>> None
>>>>>>>> of
>>>>>>>> these methods provide the ability to specify the mail 
>>>>>>>> attributes that
>>>>>>>> should be attached.
>>>>>>>>
>>>>>>>> I propose adding a further four methods mirroring the existing

>>>>>>>> ones
>>>>>>>> each
>>>>>>>> with an extra parameter:
>>>>>>>>
>>>>>>>> @param attributes
>>>>>>>>          The Mail attributes to attach
>>>>>>>>
>>>>>>>> This functionality is generally useful. The lack of it is
a 
>>>>>>>> blocker to
>>>>>>>> some SieveMailboxMailet enhancements.
>>>>>>>>
>>>>>>>> Q1: Are there any objections to this?
>>>>>>>>
>>>>>>>> Q2: The release version of the Mailet API is 2.4, so logically
we
>>>>>>>> should
>>>>>>>> step to 2.5. There is already a 2.5 version in trunk containing

>>>>>>>> a few
>>>>>>>> changes. We can:
>>>>>>>>
>>>>>>>> a) Combine the existing changes with these proposed changes
>>>>>>>> b) Park the existing changes and make 2.5 = 2.4 plus these

>>>>>>>> proposed
>>>>>>>> changes
>>>>>>>> c) Something else?
>>>>>>>>
>>>>>>>> Opinions please.
>>>>>>>>
>>>>>>>> Q3: Whatever we decide to do for Q2, for JSieve development
to 
>>>>>>>> proceed
>>>>>>>> this version of the Mailet API needs to be implemented by
>>>>>>>> JamesMailetContext in James Server trunk. Are there any 
>>>>>>>> objections to
>>>>>>>> this?
>>>>>>>>
>>>>>>>> Cheers
>>>>>>>> --Steve
>>>>>>>>
>>>>>>>>
>>>>>>>> ------------------------------****----------------------------**
>>>>>>>> --**---------
>>>>>>>> To unsubscribe, e-mail: 
>>>>>>>> server-dev-unsubscribe@james.****apache.org<
>>>>>>>> server-dev-**unsubscribe@james.apache.org<server-dev-unsubscribe@james.apache.org>

>>>>>>>>
>>>>>>>> For additional commands, e-mail: 
>>>>>>>> server-dev-help@james.apache.****org<
>>>>>>>> server-dev-help@james.**apache.org<server-dev-help@james.apache.org>>

>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>> ------------------------------**------------------------------**---------

>>>>>>
>>>>>> To unsubscribe, e-mail:
>>>>>> server-dev-unsubscribe@james.**apache.org<server-dev-unsubscribe@james.apache.org>

>>>>>>
>>>>>> For additional commands, e-mail:
>>>>>> server-dev-help@james.apache.**org<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
>>>
>>>
>>
>
>
> ---------------------------------------------------------------------
> 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