Discussion moved over from James Dev as per Stefano's suggestion.
-------- Original Message --------
Subject: Re: [mailets] Proposal: MailetContext enhancement
Date: Sun, 18 Dec 2011 19:56:49 +0100
From: Norman Maurer <[email protected]>
Reply-To: James Developers List <[email protected]>
To: James Developers List <[email protected]>
I would prefer more to just add a dispose() method to the Mail interface.
Bye,
Norman
--
Norman Maurer
Am Sonntag, 18. Dezember 2011 um 17:58 schrieb Steve Brewin:
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<[email protected]
(mailto:[email protected])>:
> > > > 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<[email protected]
(mailto:[email protected])>
> > > > >
> > > > > > 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<[email protected]
(mailto:[email protected])>
> > > > > > >
> > > > > > > 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
(http://apache.org)<
> > > > > > > > server-dev-**[email protected]
(mailto:[email protected])<[email protected]
(mailto:[email protected])>
> > > > > > > >
> > > > > > > > For additional commands, e-mail:
> > > > > > > > [email protected]
(mailto:[email protected]).****org<
> > > > > > > >
server-dev-help@james.**apache.org<[email protected]
(mailto:[email protected])>>
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > >
> > > > > >
------------------------------**------------------------------**---------
> > > > > >
> > > > > > To unsubscribe, e-mail:
> > > > > >
server-dev-unsubscribe@james.**apache.org<[email protected]
(mailto:[email protected])>
> > > > > >
> > > > > > For additional commands, e-mail:
> > > > > > [email protected]
(mailto:[email protected]).**org<[email protected]
(mailto:[email protected])>
> > > > > >
> > > > >
> > > > >
> > > >
> > > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: [email protected]
(mailto:[email protected])
> > > For additional commands, e-mail: [email protected]
(mailto:[email protected])
> > >
> >
> >
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
(mailto:[email protected])
> For additional commands, e-mail: [email protected]
(mailto:[email protected])
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
(mailto:[email protected])
For additional commands, e-mail: [email protected]
(mailto:[email protected])
