On Dec 26, 2012, at 4:01 PM, Thomas Delafosse <thomas.delafosse(a)xwiki.com> wrote:
On Wed, Dec 26, 2012 at 3:23 PM, Vincent Massol
<vincent(a)massol.net> wrote:
On Dec 26, 2012, at 3:15 PM, Thomas Delafosse <thomas.delafosse(a)xwiki.com>
wrote:
Ok, so I would rather have a component API like
- Mail prepareMail(from, to, cc, bcc, subject)
createMail is better than prepareMail IMO.
I'd make the cc and bcc not part of the constructor and instead move them
as setters since they're optional.
- int sendMail(Mail mail)
Either that or add a send() method in Mail.
while the methods addToContent, addHtml,
addAttachment, etc... would be
directly used from the Mail class.
I don't understand what addToContent is and what different it has to
addHtml.
addToContent (String mimeType, String partToAdd) is more generic : you
specify the Mime Type of the part you want to add. So addHtml(String s) is
just the same as addToContent("text/html", s). But as most of the time you
add only Html or text, I was thinking it was better to have a specific
method to add an Html part in the scripting API. I can do the same with a
addTextContent method.
I think I prefer addContent instead of addToContent.
So just to be sure, doing the following will work:
addContent("content1", "text")
addContent("content2", "text")
addContent("content3", "html")
right?
It's going to create a multipart email?
I think a single addContent method is good enough, passing an enum as the second parameter
(the mimetype). Enums are magically constructed from velocity with our custom
uberspector.
Can I call
addContent several times?
Yes, so for example if you want to have an email with an html part and a
calendar part, you call addToContent("text/html", html Text) and then
addToContent("text/calendar", calendar Code).
So a use-case would rather be :
{{velocity}}
$mail = $services.mailSender.prepareMail(from, to,...)
$mail.addHtml('<p>Blabla</p>')
addHTMLContent would be nicer. So you need also a addTextContent?
why not have an addContent(String, boolean isHTML)
or a more generic addContent(String, String mimeType)
or both
$mail.addCalendar()
What is a calendar?
It is either a vCalendar or an iCalendar (it is used by Gmail to send
invitations). It corresponds to the Mime Type "text/calendar". Here again
addCalendar(String calendar) is just the same as
addToContent("text/calendar", calendar). It's just to make it easier to
use.
ok. So I think in the future we could add some calendar helper that will create the
calendar string information.
For now this is good enough IMO:
addContent("calendar info content as per RFC 2445", "calendar")
And then later on something like:
addContent($mailsender.createCalendarMimeTypeData(param1, param2, ….),
"calendar")
You should
also show an example when using the Java API.
On Java it would give something like :
@Inject
private MailSender mailSender
Mail mail = this.mailSender.newMail(from,to,subject) ;
I don't like this too much. Why not use a constructor on the Mail object?
(The other option is a perlookup component is you really need to have some other
components injected in the Mail object; in that case you'll need setters to
from/to/subject since we currently don't support constructor injection).
String htmlCode =
"<p>Blabla</p>" ;
String calendar = "BEGIN VCALENDAR... END VCALENDAR" ;
mail.addToContent("text/html", htmlCode) ;
mail.addToContent("text/calendar", calendar) ;
this.mailSender.sendMail(mail) ;
why sendMail and not send? We're in a MailSender component so we know we're
sending mail already ;)
Thanks
-Vincent
> Thanks
> -Vincent
>
>> $services.mailSender.sendMail($mail)
>> {{/velocity}}
>>
>> Thanks,
>>
>> Thomas
>>
>> On Wed, Dec 26, 2012 at 3:04 PM, Vincent Massol <vincent(a)massol.net>
> wrote:
>>
>>> Hi Thomas,
>>>
>>> On Dec 26, 2012, at 2:58 PM, Thomas Delafosse <
> thomas.delafosse(a)xwiki.com>
>>> wrote:
>>>
>>>> I've been thinking a bit more on the mailSender component, and
here's
> the
>>>> APIs I have in mind :
>>>>
>>>> The component API would have the following methods :
>>>> - void prepareMail(String from, String to, String cc, String bcc,
> String
>>>> subject)
>>>> - void addToContent(String contentType, String content) //To add a
>>> part
>>>> to the mail, contentType being the Mime Type of this part
>>>> - void addAttachment(Attachment file)
>>>> - int sendMail() //Returns 1 on success and 0 otherwise
>>>>
>>>> And the scripting API would have the following :
>>>> - void prepareMail(String from, String to, String cc, String bcc,
> String
>>>> subject)
>>>> - void addToContent(String contentType, String content)
>>>> - void addHtml(String content)
>>>> - void addCalendar(String vCalendar)
>>>> - int sendMail()
>>>> - int sendHtmlMail(String from, String to, String subject, String html,
>>>> String alternativeText) //Simple method for non-experienced users
>>> sending
>>>> a simple html mail
>>>> {
>>>> this.mailSender.prepareMail(from, to, null, null, subject) ;
>>>> this.mailSender.addToContent("text/html", html) ;
>>>> this.mailSender.addToContent("text/plain",
alternativeText);
>>>> return this.mailSender.sendMail() ;
>>>> }
>>>>
>>>> So, a simple use-case would look something like :
>>>> {{velocity}}
>>>> $services.mailSender.prepareMail("foo(a)bar.fr"r.fr",
"foo2(a)bar.fr"r.fr", "", "",
>>>> "Subject")
>>>> $services.mailSender.addHtml("<strong>This is an email with a
>>>> calendar</strong>")
>>>> $services.mailSender.addCalendar($calendar)
>>>> $services.mailSender.sendMail()
>>>> {{/velocity}}
>>>
>>> This is not very good because you're making the service stateful and
>>> services mist absolutely be stateless. They need to be able to be used
> by
>>> several threads and not hold any state. Your API calls must return some
>>> object if you want to have several calls.
>>>
>>> Thanks
>>> -Vincent
>>>
>>>>
>>>> What do you think ? Is there anything you think is missing ? In
>>> peticular,
>>>> I'm wondering whether it would be useful to recreate methods similar
to
>>> the
>>>> parseRawMessage() and sendMailFromTemplate() methods that were
>>> implemented
>>>> in the former mailSender ?
>>>>
>>>> Cheers,
>>>>
>>>> Thomas
>>>>
>>>>
>>>> On Thu, Dec 20, 2012 at 7:00 PM, Sergiu Dumitriu
<sergiu(a)xwiki.org>
>>> wrote:
>>>>
>>>>> On 12/20/2012 06:55 AM, Thomas Delafosse wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> I would be happy to work on the mailSender plugin.
>>>>>> I propose to make it a component and add it a few
functionalities.
>>>>> Namely,
>>>>>> I was thinking about adding an API like:
>>>>>> public int sendMultiContentMessage (String from, String to,
String
> cc,
>>>>>> String bcc, String subject, String[] contents,
List<Attachment>
>>>>>> attachments) (1)
>>>>>
>>>>> Methods with too many arguments are not recommended. It even breaks
> our
>>>>> checkstyle, which allows at most 7 parameters (which I think is too
>>>>> much, anyway). Listing possible mail tokens is bad, since in most
> cases
>>>>> not all of them are needed, and in some cases others will be needed
> with
>>>>> no way of specifying them, other than writing the whole message
>>>>> including headers by hand.
>>>>>
>>>>> Either use a typed object, or a generic map.
>>>>>
>>>>>> where contents would be a string array containing all the
contents to
>>> be
>>>>>> embed in the mail (text, html but also a vCalendar for example)
along
>>>>> with
>>>>>> their MIME type.
>>>>>> So for example, if you want to send a mail containing some html
part
>>> and
>>>>> a
>>>>>> vCalendar, "contents" would look something like :
>>>>>> contents = ['text/html', Your Html code,
'text/calendar', Your
>>>>> vCalendar] .
>>>>>
>>>>> This is an untyped convention. You're hoping that all users will
read
>>>>> the documentation and know that they're supposed to provide pairs
of
>>>>> values, MIME + content. That's not a nice thing to do. A list of
typed
>>>>> objects would be better, since it doesn't allow mistakes.
>>>>>
>>>>>> Another way to achieve this would be to use a single String
"body"
>>>>> instead
>>>>>> of "contents", with a specific syntax indicating each
part MIME type,
>>>>> thus
>>>>>> allowing us to parse it. For example we could imagine having
> something
>>>>> like
>>>>>> :
>>>>>> public int sendMultiContentMessage (String from, String to,
String
> cc,
>>>>>> String bcc, String subject, String body, List<Attachment>
> attachments)
>>>>> with
>>>>>> body = "{{html}}HTML code{{/html}} {{calendar}}Calendar
>>>>> code{{/calendar}}"
>>>>>> (2) or even
>>>>>> body = "{{mailPart type='text/html'}}HTML
code{{/mailPart}}
> {{mailPart
>>>>>> type="text/calendar"}}Calendar code{{/mailPart}}"
(3).
>>>>>> This would be easier to use ((2) most of all), but probably
trickier,
>>>>>> slower and for (2), less flexible.
>>>>>
>>>>> I don't like this either, it's even more error prone.
>>>>>
>>>>> Java is an OOP language, use good OOP design as much as possible.
>>>>>
>>>>>> WDYT ? And of course, if there is anything else you would like
to
>>> change
>>>>> in
>>>>>> the mailSender, let me know !
>>>>>>
>>>>>> Thomas