On 12/26/2012 10:18 AM, Vincent Massol 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? > > 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.
-1 for enums. That limits the possible content types we can add. I prefer: addMimePart(String content, string mimeType) There's also a MimePart in the standard javax.mail library, and we could actually use this one, since it's more standard, and more flexible: http://javamail.kenai.com/nonav/javadocs/javax/mail/internet/MimePart.html http://javamail.kenai.com/nonav/javadocs/javax/mail/internet/MimeBodyPart.html But this might be a bit too verbose and complex to use. I hope that the implementation will be smart enough to send a plain message when only one body part (of type text or html) is present. >>> 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. -1 for a specific addCalendar method. Why not addVCard, addImage, addPDF, addDoc and so on? This makes a stuffed API, where anything that doesn't have a dedicated API method will feel like a second-class citizen. > 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? Constructors are bad, in a component-based world. I'd rather have the Mail object an interface, with an internal implementation used by the MailSender implementation. > (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 ;) +1 for send. > 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 -- Sergiu Dumitriu http://purl.org/net/sergiu/ _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
