james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Norman Maurer <nor...@apache.org>
Subject Re: [mailets] Proposal: MailetContext enhancement
Date Sun, 18 Dec 2011 18:56:49 GMT
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<sbrewin@synsys.com (mailto: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 (mailto: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 (mailto: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 (http://apache.org)<
> > > > > > > > > server-dev-**unsubscribe@james.apache.org (mailto:unsubscribe@james.apache.org)<server-dev-unsubscribe@james.apache.org
(mailto:server-dev-unsubscribe@james.apache.org)> 
> > > > > > > > > 
> > > > > > > > > For additional commands, e-mail: 
> > > > > > > > > server-dev-help@james.apache (mailto:server-dev-help@james.apache).****org<
> > > > > > > > > server-dev-help@james.**apache.org<server-dev-help@james.apache.org
(mailto:server-dev-help@james.apache.org)>> 
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > 
> > > > > > > ------------------------------**------------------------------**---------

> > > > > > > 
> > > > > > > To unsubscribe, e-mail:
> > > > > > > server-dev-unsubscribe@james.**apache.org<server-dev-unsubscribe@james.apache.org
(mailto:server-dev-unsubscribe@james.apache.org)> 
> > > > > > > 
> > > > > > > For additional commands, e-mail:
> > > > > > > server-dev-help@james.apache (mailto:server-dev-help@james.apache).**org<server-dev-help@james.apache.org
(mailto:server-dev-help@james.apache.org)>
> > > > > > > 
> > > > > > 
> > > > > > 
> > > > > 
> > > > > 
> > > > 
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org (mailto:server-dev-unsubscribe@james.apache.org)
> > > > For additional commands, e-mail: server-dev-help@james.apache.org (mailto:server-dev-help@james.apache.org)
> > > > 
> > > 
> > > 
> > 
> > 
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org (mailto:server-dev-unsubscribe@james.apache.org)
> > For additional commands, e-mail: server-dev-help@james.apache.org (mailto:server-dev-help@james.apache.org)
> > 
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org (mailto:server-dev-unsubscribe@james.apache.org)
> For additional commands, e-mail: server-dev-help@james.apache.org (mailto:server-dev-help@james.apache.org)
> 
> 



Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message