JavaMail Code Suggestions

[Ankur Luthra]

Same topic has been posted on StackOverFlow. Need inputs from fellow Code Ranchers too.
http://stackoverflow.com/questions/25545226/javamail-code-suggestions

I am writing a Bean which can send same message to multiple user or can send different message to same user. My application is REST based and an email needs to be triggered when a service is hit.Application can send any number of emails in a day. I have written below class for the same.Any improvements that I can do in below code. I fetch values for properties like from ,host,etc from my Db.I am querying the Db only once and populating javax.mail. Session object with the same.

@Named
@Scope(value = "prototype")

public class JavaMail {

    private static String emailPort;
    private static Session mailSession;
    private MimeMessage emailMessage;
    private static String emailHost;
    private static String emailFrom;
    private static String emailUserPassword;
    private List emailTo;
    private String emailSubject;
    private String emailBody;
    private static boolean isInitialized=false;
    @Autowired
    public ConstantsDAO cons;

    private static Properties emailProperties=new Properties();

    public JavaMail()
    {
        emailSubject="";
        emailBody="";
        emailTo=new ArrayList();

    }

    private void createBulkEmailMessage(String emailSubject,String emailBody,List emailTo) throws AddressException,
            MessagingException {
        emailMessage = new MimeMessage(mailSession);

        for (int i = 0; i < emailTo.size(); i++) {
            String tempEmailTo=emailTo.get(i).toString();
            emailMessage.addRecipient(Message.RecipientType.TO, new InternetAddress(tempEmailTo));
        }

        emailMessage.setSubject(emailSubject);
        emailMessage.setText(emailBody);

    }

    private void createEmailMessage(String emailSubject,String emailBody,String emailTo) throws AddressException,
 MessagingException {
        emailMessage = new MimeMessage(mailSession);
        emailMessage.addRecipient(Message.RecipientType.TO, new InternetAddress(emailTo));
        emailMessage.setSubject(emailSubject);
        emailMessage.setText(emailBody);

    }

    public void sendEmailToUserList(List emailTo,String emailSubject,String emailBody) throws AddressException, MessagingException{

        if(!isInitialized)
        {
            initializeConstants();
            isInitialized=true;
        }
        createBulkEmailMessage(emailSubject,emailBody,emailTo);

        sendEmail();

    }

    public void sendEmailToUser(String emailTo,String emailSubject,String emailBody) throws AddressException, MessagingException{

        if(!isInitialized)
        {
            initializeConstants();
            isInitialized=true;
        }
        createEmailMessage(emailSubject,emailBody,emailTo);

        sendEmail();

    }

    private void sendEmail() throws AddressException, MessagingException
    {
        Transport transport = mailSession.getTransport("smtp");
        transport.connect(emailHost, emailFrom, emailUserPassword);
        transport.sendMessage(emailMessage, emailMessage.getAllRecipients());
        transport.close();
    }

    private void initializeConstants() {
        List constant = null;
        try {
            constant = cons.getTableConstants();

            for(int i=0;i<constant.size();i++){

                Constants cons=(Constants)constant.get(i);
                if(cons.getName().equalsIgnoreCase(MailConstants.EMAIL_PORT)){
                    emailProperties.put("mail.smtp.port",cons.getValue());
                    continue;
                }
                if(cons.getName().equalsIgnoreCase(MailConstants.EMAIL_TLSENABLE)){
                    emailProperties.put("mail.smtp.starttls.enable", cons.getValue());
                    continue;
                }
                if(cons.getName().equalsIgnoreCase(MailConstants.EMAIL_AUTH)){
                    emailProperties.put("mail.smtp.auth", cons.getValue());
                    continue;
                }
                if(cons.getName().equalsIgnoreCase(MailConstants.EMAIL_FROM)){
                    emailFrom=cons.getValue();
                    continue;
                }
                if(cons.getName().equalsIgnoreCase(MailConstants.EMAIL_HOST)){
                    emailHost=cons.getValue();
                    continue;
                }
                if(cons.getName().equalsIgnoreCase(MailConstants.EMAIL_PASSWORD)){
                    emailUserPassword=cons.getValue();
                    continue;
                }
            }
            //Initialize the mail Session with properties
            mailSession = Session.getDefaultInstance(emailProperties, null);

        } catch (Exception e) {
            //TODO Read the properties file named mail.properties and populate emailProperties
            e.printStackTrace();
        }

    }

}

[Ulf Dittmer]

What kinds of suggestions are you looking for? What is not working as you would like it to?

[Ankur Luthra]

Hi,
I need to know if I can implement the above code in a more elegant way.
Any re-factoring that can be done.

Should I be making the class a singleton?
Most of the variables in this class will remain constant except TO-Address ,Subject and Body.
Any other way by which I can make sure that all variables except (TO-Address ,Subject and Body) gets instantiated only once.

Regards,
Ankur

[Ulf Dittmer]

How you intend to use this class determines its design to a good deal - we can't really comment on that.

I guess my question would be why you're not using a library like http://commons.apache.org/proper/commons-email/ for this?

[Praful Thakare]

agree with Ulf, why to reinvent the wheel.

for code re factoring, one quick thing is to refactor sendEmailToUserList and sendEmailToUser should be refactored since they have 90% similar code. same goes with createEmailMessage and createBulkEmailMessage

[Ankur Luthra]

Hi,
I am facing some issue while using commons-mail API.

Getting following Exception .
Caused by: javax.mail.NoSuchProviderException: Unable to locate provider for protocol: smtp
at javax.mail.Session.getProvider(Session.java:237)
at javax.mail.Session.getTransport(Session.java:346)
at javax.mail.Session.getTransport(Session.java:376)
at javax.mail.Transport.send(Transport.java:67)
at javax.mail.Transport.send(Transport.java:48)
at org.apache.commons.mail.Email.sendMimeMessage(Email.java:1400)
... 3 more


Below is snippet of Code that I am using

MultiPartEmail  email = new MultiPartEmail ();
		email.setHostName("<hostName>");

		email.setAuthentication("<User Name>", "<Pwd>");

		email.setFrom("<From Email>");
		email.setSubject("TestMail");
		email.setMsg("This is a test mail ... :-)");
		email.addTo("To email");
		email.send();
	

[Ulf Dittmer]

Commons Email is built on top of JavaMail, so it needs all its libraries in the classpath - it seems that's not the case.

[Ankur Luthra]

Hi,
I have added mail-1.4.jar in my class path and it works.
Is there any issue if i keep geronimo-javamail_1.4_spec-1.7.1.jar and geronimo-activation_1.1_spec-1.1.jar also in my classpath?

Regards,
Ankur

[Ulf Dittmer]

I don't know what those are. Check what classes they contain, and make sure not to have overlaps with other jar files.

[Ankur Luthra]

The problem is geronimo*activation*.jar contains the package javax.activation and I see javax.activation as a dependency for commons-email.
hence the question.

[Ulf Dittmer]

JAF (a.k.a Java Activation Framework) can be downloaded from Oracle if you want/need the original jar file.