james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "A. Rothman" <amich...@amichais.net>
Subject Re: Mailet API documentation
Date Sun, 02 Aug 2009 10:09:10 GMT


Robert Burrell Donkin wrote:

> On Sun, Jul 26, 2009 at 2:50 PM, A. Rothman<amichai2@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
>
>
>   

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