Re: Mailet API documentation

["A. Rothman" <am...@amichais.net>]

Robert Burrell Donkin wrote:

> On Sun, Jul 26, 2009 at 2:50 PM, A. Rothman<am...@amichais.net> wrote:
>   
>> Hi,
>>     
>
> hi
>
> (apologies for the slow reply)
>
>   
>> I've been going over the Mailet API, considering implementing it in some
>> project I'm working on. I've found lots of issues with the documentation -
>> from trivial typos to entire methods whose contract is unclear. So I went
>> over the entire package's javadocs and fixed them up, occasionally
>> consulting the James implementation to try and decipher what the method
>> contracts were intended to be.
>>     
>
> cool
>
>   
>> I have two requests of anyone who is capable - the first is to review the
>> new docs and point out anything that needs to be corrected or further
>> clarified (I hope the attached diff makes it through to the mailing list).
>>     
>
> could you please attach the patch to a JIRA
> (http://issues.apache.org/jira/) since it has better legal clarity and
> makes tracking easier
>
>   
opened: https://issues.apache.org/jira/browse/MAILET-26
>> The second, is to help clarify and answer the issues below. At this point
>> this is only about understanding and documenting the existing API - not
>> fixing the API itself.
>>     
>
> i'll do my best but hopefully others will jump in...
>
>   
>> So here we go:
>>
>>
>> 1. Can someone clarify the distinction or relationship between 'state' and
>> 'processor name'? are they interchangeable? must there be a processor for
>> each of the constants in Mail? e.g., what is the "transport"
>> processor/state? what is the "default" state? Is "ghost" the only special
>> reserved state which is not a processor name?
>>     
>
> the mailet API supports multiple named mail processors each with their
> own particular mailets and matchers. for example, 'Tom, 'Dick' and
> 'Harry' might be processors, and a matcher in 'Tom' might direct a
> mail to 'Dick' for further processing.
>
> state describes the current processing status of a mail.
>
>   
The implementation of the standard ToProcessor Mailet is 
mail.setState(processor), and it is treated similarly in other parts of 
the imlementation of both the container and mailets, so there's got to 
be a more well-defined relationship between the two - that's what I'm 
trying to understand and document.

In addition, there are the Mail constants which require clarification - 
ERROR is intuitive, GHOST is clarified in the docs, but 
DEFAULT/TRANSPORT are not mentioned anywhere, so the difference between 
them is unclear.
>> 2. Mail.get/setErrorMessage docs say "Not sure why this is needed" - should
>> this be deprecated? I saw one place in James that stores an integer in this
>> field, parsing it from a string every time around... isn't that what the
>> general purpose attributes are for?
>>     
>
> i'm not sure: hopefully someone will jump in with an explanation
>
>   
>> 3. Mail.getMessageSize() - which size is this meant to be? only the mime
>> message size? note that MimeMessage.getSize() isn't guaranteed to be exact -
>> does this method also provide no guarantees? if so it should be stated.
>>     
>
> AIUI it's a best estimate of the octet size of the mail, and i agree
> that it should be in the javadocs
>   
ok, I'll use something similar to the MailImpl docs which I just looked 
at which do state this.
>   
>> 4. MailAddress.equals breaks the equals contract and the equals/hashcode
>> relationship - I added a doc explicitly stating this to avoid future bugs,
>> but it would be much better to fix it (without breaking james, of course)...
>>     
>
> do you have an example of the breakage?
>   
 From the javadoc I added:

     * Note that this implementation breaks the general contract of the
     * equals method as well as its relationship with the hashcode method,
     * since it can return true when compared to a String instance
     * (braking the symmetry property, and allowing equal objects to have
     * different hash codes).

in other words, it can return true when compared to a String, but a 
String will always return false when compared to a non-String, and this 
breaks the contract in two different ways.
>   
>> 5. From the Mailet docs: "Instead of creating new messages, the mailet can
>> put a message with new recipients at the top of the mail queue, or insert
>> them  immediately after it's execution through the API are provided by the
>> MailetContext interface" - anyone have an idea what was meant here?
>>     
>
> AIUI MailetContext#sendMail accepts a Message, and is preferred to
> creating a Mail
>   
There are 4 sendMail overloads, one with a Mail, one with a Message... 
except the API doesn't provide any way to create a new Mail instance in 
the first place (Mail implements Cloneable, but does not provide a 
public clone method - see Cloneable interface docs. There's no other way 
to create a Mail in the API). And sendMail(Mail) is not deprecated 
either. So I still can't quite make sense out of this sentence. Maybe we 
can just delete it and not tell anyone :-)
>   
>> 6. MailetContext.bounce "Bounces the message using a standard format with
>> the given message". What is this standard format?
>>     
>
> AIUI it's that described by the second paragraph
>
>   
I don't see anything about message formats anywhere - which paragraph do 
you mean?
>> 7. Several places like MailetContext.bounce/send talk about "adding message
>> to top of mail server queue" - is it really to the top of the queue? why? or
>> just added to the queue normally?
>>     
>
> not sure but may well be added to the top
>
>   
>> 8. The MailetContext.getAttribute docs says it reserves names matching
>> java.*, javax.*, and sun.*, whereas the Mail attributes reserve
>> org.apache.james.* and org.apache.mailet.*. - is the former a copy/paste
>> error which should be the same as the latter? (I've seen the word 'servlet'
>> in the docs too, which would explain the copy/paste bug, if that's what it
>> is)
>>     
>
> looks like a cut'N'paste bug to me
>
>   
>> 9. How should the MailContext set/remove attributes be explained? the
>> current docs imply that context attributes are provided by the container and
>> feel like they should be read only - what else are they used for?
>>     
>
> not sure
>
>   
>> 10.  MailContext.isLocalEmail - does it check whether the address is under
>> responsibility of the local server, or also that the account actually
>> exists?
>>     
>
> just whether it's under the responsibility of the local server
>   
I looked at the impl and it doesn't seem to do this, so I'll clarify my 
question in case I didn't explain it well:
if isLocalServer("localdomain.com") is true, and there's *no* local user 
named "user", then should isLocalEmail(new 
MailAddress("user@localdomain.com")) return true (i.e. under 
responsibility of local server) or false (i.e. account does not actually 
exist)?
>   
>> 11. MailetContext.getMailServers/getSMTPHostAddresses - what is the
>> difference? if the former returns only MX record values, why do the docs say
>> they can be IP addresses (the RFC disallows IP literals in MX records)? Did
>> I understand correctly that the latter is equivalent to resolving the host
>> names on the output of the former?
>>     
>
> the methods have different return types. i don't know why there are
> two similar methods
>
>   
>> 12. Does MailetContext.getMailServers go through the CNAME/A records as
>> backup like specified in the RFC? or only look for MX records as the doc
>> states?
>>     
>
> not sure
>
>   
>> 13. MailetContext.storeMail says "@deprecated - use sparingly.  Service will
>> be replaced with resource acquired via JNDI." what's that about? Is this an
>> implementatio detail? is this supposed to be a version of sendMail which
>> bypasses all mailet processing?
>>     
>
> AIUI this call used to commit mail to long term storage, but now
> mailets are preferred for this
>
>   
Wouldn't that necessarily make the Mailets 
container-implementation-dependent? I thought the point of the 
container/Mailet separation was (similar to servlets and other *lets) 
that the Mailets become portable and reusable between different 
containers, and that implementation-specific services (such as 
send,bounce,store,etc.) are provided via the MailetContext API, no?

I actually find it unfortunate to see various different Mailets each 
implementing their own bounce method internally instead of using the 
context's services... seems counterproductive. Perhaps the Mailet API 
needs some more adjustments to get there, but shouldn't this at least be 
considered a theoretical goal?
>> 14. Clarification - sendMail does not send any remote mail, but only local
>> mail (as if received by a remote SMTP session)? The MailetContext doesn't
>> provide the ability to send remote mail other than bounce?
>>     
>
> a mailet can be used to send mail remotely
>   
would it be correct if we remove "Send an outgoing message" from the 
original docs, and only state that these methods enqueue the mail for 
processing by the appropriate processor (either specified or the mail 
state, depending on overloaded method)?
>   
>> 15. Are there any other known implementations of the Mailet API outside of
>> James that need to be considered when assuming all of the above?
>>     
>
> yes, there are some other known implementations but AFAIK they are now
> all mature
>   
If they are mature, it seems even more important to make sure all of 
them are aligned to the same interpretation of the API, no?
> - robert
>   
Thanks Robert!
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>
>   

Re: Mailet API documentation

["A. Rothman" <am...@amichais.net>]

>
>>>> 4. MailAddress.equals breaks the equals contract and the equals/hashcode
>>>> relationship - I added a doc explicitly stating this to avoid future
>>>> bugs,
>>>> but it would be much better to fix it (without breaking james, of
>>>> course)...
>>>>
>>>>         
>>> do you have an example of the breakage?
>>>
>>>       
>> From the javadoc I added:
>>
>>    * Note that this implementation breaks the general contract of the
>>    * equals method as well as its relationship with the hashcode method,
>>    * since it can return true when compared to a String instance
>>    * (braking the symmetry property, and allowing equal objects to have
>>    * different hash codes).
>>
>> in other words, it can return true when compared to a String, but a String
>> will always return false when compared to a non-String, and this breaks the
>> contract in two different ways.
>>     
>
> i agree about the symmetry but IIRC when i analysed the hash code it
> was ok(ish). given that the whole idea breaks a basic contract in
> java, this is probably just splitting hairs. i strongly dislike this
> design and would prefer a separate method to allow strings to be
> explicitly checked.
>
> in general, MailAddress has issues, and should have been modelled as
> an interface
I still think hashCode is also broken -  a MailAddress("me@here") will 
be equal to the String "ME@HERE" but they will have different hash codes.

Two more doc issues:

1. Can Matcher.match return null for no recipients? (more efficient... 
should be documented)


2. Are multi-valued init parameters always returned as a comma-separated 
string?


Robert - thanks for reviewing all the javadoc fixes :-)

Amichai

Re: Mailet API documentation

[Robert Burrell Donkin <ro...@gmail.com>]

On Sun, Aug 2, 2009 at 11:09 AM, A. Rothman<am...@amichais.net> wrote:
> Robert Burrell Donkin wrote:
>> On Sun, Jul 26, 2009 at 2:50 PM, A. Rothman<am...@amichais.net> wrote:

<snip>

>>> So here we go:
>>>
>>>
>>> 1. Can someone clarify the distinction or relationship between 'state'
>>> and
>>> 'processor name'? are they interchangeable? must there be a processor for
>>> each of the constants in Mail? e.g., what is the "transport"
>>> processor/state? what is the "default" state? Is "ghost" the only special
>>> reserved state which is not a processor name?
>>>
>>
>> the mailet API supports multiple named mail processors each with their
>> own particular mailets and matchers. for example, 'Tom, 'Dick' and
>> 'Harry' might be processors, and a matcher in 'Tom' might direct a
>> mail to 'Dick' for further processing.
>>
>> state describes the current processing status of a mail.
>>
>>
>
> The implementation of the standard ToProcessor Mailet is
> mail.setState(processor), and it is treated similarly in other parts of the
> imlementation of both the container and mailets, so there's got to be a more
> well-defined relationship between the two - that's what I'm trying to
> understand and document.
>
> In addition, there are the Mail constants which require clarification -
> ERROR is intuitive, GHOST is clarified in the docs, but DEFAULT/TRANSPORT
> are not mentioned anywhere, so the difference between them is unclear.

someone else will need to jump in with an answer

>>> 3. Mail.getMessageSize() - which size is this meant to be? only the mime
>>> message size? note that MimeMessage.getSize() isn't guaranteed to be
>>> exact -
>>> does this method also provide no guarantees? if so it should be stated.
>>>
>>
>> AIUI it's a best estimate of the octet size of the mail, and i agree
>> that it should be in the javadocs
>>
>
> ok, I'll use something similar to the MailImpl docs which I just looked at
> which do state this.

i've added something which hopefully covers this

>>> 4. MailAddress.equals breaks the equals contract and the equals/hashcode
>>> relationship - I added a doc explicitly stating this to avoid future
>>> bugs,
>>> but it would be much better to fix it (without breaking james, of
>>> course)...
>>>
>>
>> do you have an example of the breakage?
>>
>
> From the javadoc I added:
>
>    * Note that this implementation breaks the general contract of the
>    * equals method as well as its relationship with the hashcode method,
>    * since it can return true when compared to a String instance
>    * (braking the symmetry property, and allowing equal objects to have
>    * different hash codes).
>
> in other words, it can return true when compared to a String, but a String
> will always return false when compared to a non-String, and this breaks the
> contract in two different ways.

i agree about the symmetry but IIRC when i analysed the hash code it
was ok(ish). given that the whole idea breaks a basic contract in
java, this is probably just splitting hairs. i strongly dislike this
design and would prefer a separate method to allow strings to be
explicitly checked.

in general, MailAddress has issues, and should have been modelled as
an interface

>>> 5. From the Mailet docs: "Instead of creating new messages, the mailet
>>> can
>>> put a message with new recipients at the top of the mail queue, or insert
>>> them  immediately after it's execution through the API are provided by
>>> the
>>> MailetContext interface" - anyone have an idea what was meant here?
>>>
>>
>> AIUI MailetContext#sendMail accepts a Message, and is preferred to
>> creating a Mail
>>
>
> There are 4 sendMail overloads, one with a Mail, one with a Message...
> except the API doesn't provide any way to create a new Mail instance in the
> first place (Mail implements Cloneable, but does not provide a public clone
> method - see Cloneable interface docs. There's no other way to create a Mail
> in the API). And sendMail(Mail) is not deprecated either. So I still can't
> quite make sense out of this sentence. Maybe we can just delete it and not
> tell anyone :-)

IMHO the problem's not the javadocs but lack of a factory method for
mail in the API. this needlessly couples a lot of Mailets to a
particular container. moving MailImpl to mailet base might ease this
issue but including a factory method could allow containers to
optimise.

>>> 6. MailetContext.bounce "Bounces the message using a standard format with
>>> the given message". What is this standard format?
>>>
>>
>> AIUI it's that described by the second paragraph
>>
>>
>
> I don't see anything about message formats anywhere - which paragraph do you
> mean?

"The message will be sent back to the sender from the postmaster as
specified for this mailet context"

AFAIK (hopefully someone who knows more will jump in) this doesn't
mean "standard" as in "standardised by this specification" but just
that the message will form part of the body for a bounce email created
by the mailet container with appropriate standard headers set and
postmaster as sender.

<snip>

>>> 10.  MailContext.isLocalEmail - does it check whether the address is
>>> under
>>> responsibility of the local server, or also that the account actually
>>> exists?
>>>
>>
>> just whether it's under the responsibility of the local server
>>
>
> I looked at the impl and it doesn't seem to do this, so I'll clarify my
> question in case I didn't explain it well:
> if isLocalServer("localdomain.com") is true, and there's *no* local user
> named "user", then should isLocalEmail(new
> MailAddress("user@localdomain.com")) return true (i.e. under responsibility
> of local server) or false (i.e. account does not actually exist)?

<snip>

>>> 13. MailetContext.storeMail says "@deprecated - use sparingly.  Service
>>> will
>>> be replaced with resource acquired via JNDI." what's that about? Is this
>>> an
>>> implementatio detail? is this supposed to be a version of sendMail which
>>> bypasses all mailet processing?
>>>
>>
>> AIUI this call used to commit mail to long term storage, but now
>> mailets are preferred for this
>>
>>
>
> Wouldn't that necessarily make the Mailets
> container-implementation-dependent? I thought the point of the
> container/Mailet separation was (similar to servlets and other *lets) that
> the Mailets become portable and reusable between different containers, and
> that implementation-specific services (such as send,bounce,store,etc.) are
> provided via the MailetContext API, no?

there are differing schools of thought on this ;-)

in some ways, using mailets for storage is neater and more portable
since the storage implementations can be more easily reused

there are lots of services which mailets may need: much more than
could be reasonably added to the MailetContext. service discovery and
binding is an issue, and restricts portability. there quite a few
mailet in james that are strong coupled to james via avalon service
lookup. i would prefer to see this coupling broken.

> I actually find it unfortunate to see various different Mailets each
> implementing their own bounce method internally instead of using the
> context's services... seems counterproductive. Perhaps the Mailet API needs
> some more adjustments to get there, but shouldn't this at least be
> considered a theoretical goal?

AIUI the current API has numerous issues and limitation but is mature

IMHO it should have been replaced by a new API requiring java 1.5

>>> 14. Clarification - sendMail does not send any remote mail, but only
>>> local
>>> mail (as if received by a remote SMTP session)? The MailetContext doesn't
>>> provide the ability to send remote mail other than bounce?
>>>
>>
>> a mailet can be used to send mail remotely
>>
>
> would it be correct if we remove "Send an outgoing message" from the
> original docs, and only state that these methods enqueue the mail for
> processing by the appropriate processor (either specified or the mail state,
> depending on overloaded method)?

AFAIK yes

>>> 15. Are there any other known implementations of the Mailet API outside
>>> of
>>> James that need to be considered when assuming all of the above?
>>>
>>
>> yes, there are some other known implementations but AFAIK they are now
>> all mature
>>
>
> If they are mature, it seems even more important to make sure all of them
> are aligned to the same interpretation of the API, no?

i'm not sure how many are actively interested in pushing mailets
forward any more (there hasn't seemed to be much discussion on the
mailet api list for a long while) but anyone who wanted to get in
contact would be welcomed...

- robert

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org