I have a user registration controller:
@Autowired
private EmailTools emailTools;
@Controller
public class RegistrationController {
@RequestMapping(value="/registerUser", method=RequestMethod.POST )
public String registerUser(HttpSession session,Model model,User user){
//RegisterUserDetails of scope prototype
RegisterUserDetails regUserDetails =
appContext.getBean(RegisterUserDetails.class);
//rgistering user in db
regUserDetails.setUser(user);
//sending confirmation email
emailTools.SendEmail(user.getEmail(), subject, content);
return "home";
}
The emailTools class:
public class EmailTools {
private Properties props;
private Key internalKey;
public EmailTools(Properties props, Key internalKey){
this.props=props;
this.internalKey=internalKey;
}
public synchronized void SendEmail(String toAddress, String subject, String content) {
final String fromAddress = props.getProperty("from.address");
final String password = Encryptor.decrypt(props.getProperty("email.password"),internalKey);
Session session = Session.getInstance(props,
new javax.mail.Authenticator() {
protected PasswordAuthentication getPasswordAuthentication() {
return new PasswordAuthentication(fromAddress, password);
}
});
try {
Message message = new MimeMessage(session);
message.setFrom(new InternetAddress(fromAddress));
message.setRecipients(Message.RecipientType.TO,
InternetAddress.parse(toAddress));
message.setSubject(subject);
message.setText(content);
Transport.send(message);
System.out.println("Done");
} catch (MessagingException e) {
throw new RuntimeException(e);
}
}
}
At present the emailTools class is a singleton. I have put a synchronised block around the sendEmail method to make it thread safe?
1) Is the current method thread safe? 2) If so, is it better to make the scope of emailTools a prototpe/request/session or a singleton that uses synchronization? Which is better?
According to Threadsafety in Javamail , all Session- and Transport-related operations are thread-safe.
So if your Encryptor.decrypt() is thread-safe, then SendEmail() is thread-safe too and no synchronization is needed.
But it is not clear whether registerUser() is thread-safe. If RegisterUserDetails is a singleton AND setUser() is actually a setter that modifies some shared state (and not a method that just saves something to database), AND that shared state (user?) is used (indirectly) by SendEmail() (for instance, in Encryptor.decrypt()), then the whole construct may be not thread-safe.