On Dec 26, 2012, at 4:01 PM, Thomas Delafosse <[email protected]> wrote:
> On Wed, Dec 26, 2012 at 3:23 PM, Vincent Massol <[email protected]> wrote: > >> >> On Dec 26, 2012, at 3:15 PM, Thomas Delafosse <[email protected]> >> 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 <[email protected]> >> wrote: >>> >>>> Hi Thomas, >>>> >>>> On Dec 26, 2012, at 2:58 PM, Thomas Delafosse < >> [email protected]> >>>> 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("[email protected]", "[email protected]", "", "", >>>>> "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 <[email protected]> >>>> 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 _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
