On Wed, Dec 26, 2012 at 4:18 PM, Vincent Massol <[email protected]> wrote:
> > 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? > > Yes, I add a BodyPart to my global mail multipart every time I call addContent. > 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") > Yes. Anyway I was already thinking about doing something like that for my "Meetings" application :) Thanks > > >> 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? > Yes you're right, I can just do Mail mail = new Mail(from, to, subject). I was just getting confused with the components :). > > (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 ;) > Indeed ! I will make an effort on the naming of my methods. > > 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 > _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
