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])
>
>
