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