Merge branch 'mail-server-timeout-impl' of github.com:desoliture1/thingsboard into develop/3.4

application/src/main/java/org/thingsboard/server/service/mail/DefaultMailService.java

@@ -20,7 +20,9 @@ import freemarker.template.Configuration;
 import freemarker.template.Template;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.exception.ExceptionUtils;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.context.MessageSource;
 import org.springframework.context.annotation.Lazy;
 import org.springframework.core.NestedRuntimeException;
@@ -47,13 +49,17 @@ import org.thingsboard.server.queue.usagestats.TbApiUsageClient;
 import org.thingsboard.server.service.apiusage.TbApiUsageStateService;
 
 import javax.annotation.PostConstruct;
-import javax.mail.MessagingException;
+import javax.annotation.PreDestroy;
 import javax.mail.internet.MimeMessage;
 import java.io.ByteArrayInputStream;
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Properties;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 @Service
 @Slf4j
@@ -70,6 +76,8 @@ public class DefaultMailService implements MailService {
     private final AdminSettingsService adminSettingsService;
     private final TbApiUsageClient apiUsageClient;
 
+    private static final long DEFAULT_TIMEOUT = 10_000;
+
     @Lazy
     @Autowired
     private TbApiUsageStateService apiUsageStateService;
@@ -77,10 +85,17 @@ public class DefaultMailService implements MailService {
     @Autowired
     private MailExecutorService mailExecutorService;
 
+    @Value("${actors.rule.mail_timeout_thread_pool_size}")
+    private int timeoutExecutorPoolSize;
+
+    private ExecutorService timeoutExecutorService;
+
     private JavaMailSenderImpl mailSender;
 
     private String mailFrom;
 
+    private long timeout;
+
     public DefaultMailService(MessageSource messages, Configuration freemarkerConfig, AdminSettingsService adminSettingsService, TbApiUsageClient apiUsageClient) {
         this.messages = messages;
         this.freemarkerConfig = freemarkerConfig;
@@ -91,6 +106,14 @@ public class DefaultMailService implements MailService {
     @PostConstruct
     private void init() {
         updateMailConfiguration();
+        timeoutExecutorService = Executors.newFixedThreadPool(timeoutExecutorPoolSize);
+    }
+
+    @PreDestroy
+    private void destroy() {
+        if (timeoutExecutorService != null) {
+            timeoutExecutorService.shutdown();
+        }
     }
 
     @Override
@@ -100,6 +123,7 @@ public class DefaultMailService implements MailService {
             JsonNode jsonConfig = settings.getJsonValue();
             mailSender = createMailSender(jsonConfig);
             mailFrom = jsonConfig.get("mailFrom").asText();
+            timeout = jsonConfig.get("timeout").asLong(DEFAULT_TIMEOUT);
         } else {
             throw new IncorrectParameterException("Failed to update mail configuration. Settings not found!");
         }
@@ -166,7 +190,7 @@ public class DefaultMailService implements MailService {
 
     @Override
     public void sendEmail(TenantId tenantId, String email, String subject, String message) throws ThingsboardException {
-        sendMail(mailSender, mailFrom, email, subject, message);
+        sendMail(mailSender, mailFrom, email, subject, message, timeout);
     }
 
     @Override
@@ -174,13 +198,14 @@ public class DefaultMailService implements MailService {
         JavaMailSenderImpl testMailSender = createMailSender(jsonConfig);
         String mailFrom = jsonConfig.get("mailFrom").asText();
         String subject = messages.getMessage("test.message.subject", null, Locale.US);
+        long timeout = jsonConfig.get("timeout").asLong(DEFAULT_TIMEOUT);
 
         Map<String, Object> model = new HashMap<>();
         model.put(TARGET_EMAIL, email);
 
         String message = mergeTemplateIntoString("test.ftl", model);
 
-        sendMail(testMailSender, mailFrom, email, subject, message);
+        sendMail(testMailSender, mailFrom, email, subject, message, timeout);
     }
 
     @Override
@@ -194,7 +219,7 @@ public class DefaultMailService implements MailService {
 
         String message = mergeTemplateIntoString("activation.ftl", model);
 
-        sendMail(mailSender, mailFrom, email, subject, message);
+        sendMail(mailSender, mailFrom, email, subject, message, timeout);
     }
 
     @Override
@@ -208,7 +233,7 @@ public class DefaultMailService implements MailService {
 
         String message = mergeTemplateIntoString("account.activated.ftl", model);
 
-        sendMail(mailSender, mailFrom, email, subject, message);
+        sendMail(mailSender, mailFrom, email, subject, message, timeout);
     }
 
     @Override
@@ -222,7 +247,7 @@ public class DefaultMailService implements MailService {
 
         String message = mergeTemplateIntoString("reset.password.ftl", model);
 
-        sendMail(mailSender, mailFrom, email, subject, message);
+        sendMail(mailSender, mailFrom, email, subject, message, timeout);
     }
 
     @Override
@@ -247,20 +272,20 @@ public class DefaultMailService implements MailService {
 
         String message = mergeTemplateIntoString("password.was.reset.ftl", model);
 
-        sendMail(mailSender, mailFrom, email, subject, message);
+        sendMail(mailSender, mailFrom, email, subject, message, timeout);
     }
 
     @Override
     public void send(TenantId tenantId, CustomerId customerId, TbEmail tbEmail) throws ThingsboardException {
-        sendMail(tenantId, customerId, tbEmail, this.mailSender);
+        sendMail(tenantId, customerId, tbEmail, this.mailSender, timeout);
     }
 
     @Override
-    public void send(TenantId tenantId, CustomerId customerId, TbEmail tbEmail, JavaMailSender javaMailSender) throws ThingsboardException {
+    public void send(TenantId tenantId, CustomerId customerId, TbEmail tbEmail, JavaMailSender javaMailSender, long timeout) throws ThingsboardException {
-        sendMail(tenantId, customerId, tbEmail, javaMailSender);
+        sendMail(tenantId, customerId, tbEmail, javaMailSender, timeout);
     }
 
-    private void sendMail(TenantId tenantId, CustomerId customerId, TbEmail tbEmail, JavaMailSender javaMailSender) throws ThingsboardException {
+    private void sendMail(TenantId tenantId, CustomerId customerId, TbEmail tbEmail, JavaMailSender javaMailSender, long timeout) throws ThingsboardException {
         if (apiUsageStateService.getApiUsageState(tenantId).isEmailSendEnabled()) {
             try {
                 MimeMessage mailMsg = javaMailSender.createMimeMessage();
@@ -287,7 +312,7 @@ public class DefaultMailService implements MailService {
                         helper.addInline(imgId, iss, contentType);
                     }
                 }
-                javaMailSender.send(helper.getMimeMessage());
+                sendMailWithTimeout(javaMailSender, helper.getMimeMessage(), timeout);
                 apiUsageClient.report(tenantId, customerId, ApiUsageRecordKey.EMAIL_EXEC_COUNT, 1);
             } catch (Exception e) {
                 throw handleException(e);
@@ -308,7 +333,7 @@ public class DefaultMailService implements MailService {
 
         String message = mergeTemplateIntoString("account.lockout.ftl", model);
 
-        sendMail(mailSender, mailFrom, email, subject, message);
+        sendMail(mailSender, mailFrom, email, subject, message, timeout);
     }
 
     @Override
@@ -320,7 +345,7 @@ public class DefaultMailService implements MailService {
                 "expirationTimeSeconds", expirationTimeSeconds
         ));
 
-        sendMail(mailSender, mailFrom, email, subject, message);
+        sendMail(mailSender, mailFrom, email, subject, message, timeout);
     }
 
     @Override
@@ -347,7 +372,7 @@ public class DefaultMailService implements MailService {
                 message = mergeTemplateIntoString("state.disabled.ftl", model);
                 break;
         }
-        sendMail(mailSender, mailFrom, email, subject, message);
+        sendMail(mailSender, mailFrom, email, subject, message, timeout);
     }
 
     @Override
@@ -447,9 +472,8 @@ public class DefaultMailService implements MailService {
         }
     }
 
-    private void sendMail(JavaMailSenderImpl mailSender,
+    private void sendMail(JavaMailSenderImpl mailSender, String mailFrom, String email,
-                          String mailFrom, String email,
+                          String subject, String message, long timeout) throws ThingsboardException {
-                          String subject, String message) throws ThingsboardException {
         try {
             MimeMessage mimeMsg = mailSender.createMimeMessage();
             MimeMessageHelper helper = new MimeMessageHelper(mimeMsg, UTF_8);
@@ -457,12 +481,25 @@ public class DefaultMailService implements MailService {
             helper.setTo(email);
             helper.setSubject(subject);
             helper.setText(message, true);
-            mailSender.send(helper.getMimeMessage());
+
+            sendMailWithTimeout(mailSender, helper.getMimeMessage(), timeout);
         } catch (Exception e) {
             throw handleException(e);
         }
     }
 
+    private void sendMailWithTimeout(JavaMailSender mailSender, MimeMessage msg, long timeout) {
+        var submittedMail = timeoutExecutorService.submit(() -> mailSender.send(msg));
+        try {
+            submittedMail.get(timeout, TimeUnit.MILLISECONDS);
+        } catch (TimeoutException e) {
+            log.debug("Error during mail submission", e);
+            throw new RuntimeException("Timeout!");
+        } catch (Exception e) {
+            throw new RuntimeException(ExceptionUtils.getRootCause(e));
+        }
+    }
+
     private String mergeTemplateIntoString(String templateLocation,
                                            Map<String, Object> model) throws ThingsboardException {
         try {

application/src/main/resources/thingsboard.yml

@@ -326,7 +326,9 @@ actors:
     # Specify thread pool size for javascript executor service
     js_thread_pool_size: "${ACTORS_RULE_JS_THREAD_POOL_SIZE:50}"
     # Specify thread pool size for mail sender executor service
-    mail_thread_pool_size: "${ACTORS_RULE_MAIL_THREAD_POOL_SIZE:50}"
+    mail_thread_pool_size: "${ACTORS_RULE_MAIL_THREAD_POOL_SIZE:40}"
+    # Specify thread pool size for mail sender executor service
+    mail_timeout_thread_pool_size: "${ACTORS_RULE_MAIL_TIMEOUT_THREAD_POOL_SIZE:10}"
     # Specify thread pool size for sms sender executor service
     sms_thread_pool_size: "${ACTORS_RULE_SMS_THREAD_POOL_SIZE:50}"
     # Whether to allow usage of system mail service for rules

application/src/test/java/org/thingsboard/server/controller/BaseAdminControllerTest.java

@@ -18,14 +18,29 @@ package org.thingsboard.server.controller;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import org.junit.Test;
+import org.mockito.Mockito;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.thingsboard.common.util.JacksonUtil;
+import org.thingsboard.rule.engine.api.MailService;
 import org.thingsboard.server.common.data.AdminSettings;
+import org.thingsboard.server.service.mail.DefaultMailService;
 
-import static org.hamcrest.Matchers.*;
+import static org.hamcrest.Matchers.containsString;
-import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
 
 public abstract class BaseAdminControllerTest extends AbstractControllerTest {
 
+    @Autowired
+    MailService mailService;
+
+    @Autowired
+    DefaultMailService defaultMailService;
+
     @Test
     public void testFindAdminSettingsByKey() throws Exception {
         loginSysAdmin();
@@ -101,4 +116,27 @@ public abstract class BaseAdminControllerTest extends AbstractControllerTest {
         .andExpect(status().isOk());
     }
 
+    @Test
+    public void testSendTestMailTimeout() throws Exception {
+        loginSysAdmin();
+        AdminSettings adminSettings = doGet("/api/admin/settings/mail", AdminSettings.class);
+        ObjectNode objectNode = JacksonUtil.fromString(adminSettings.getJsonValue().toString(), ObjectNode.class);
+
+        objectNode.put("smtpHost", "mail.gandi.net");
+        objectNode.put("timeout", 1_000);
+        objectNode.put("username", "username");
+        objectNode.put("password", "password");
+
+        adminSettings.setJsonValue(objectNode);
+
+        Mockito.doAnswer((invocations) -> {
+            var jsonConfig = (JsonNode) invocations.getArgument(0);
+            var email = (String) invocations.getArgument(1);
+
+            defaultMailService.sendTestMail(jsonConfig, email);
+            return null;
+        }).when(mailService).sendTestMail(Mockito.any(), Mockito.anyString());
+        doPost("/api/admin/settings/testMail", adminSettings).andExpect(status().is5xxServerError());
+        Mockito.doNothing().when(mailService).sendTestMail(Mockito.any(), Mockito.any());
+    }
 }

rule-engine/rule-engine-api/src/main/java/org/thingsboard/rule/engine/api/MailService.java

@@ -47,8 +47,7 @@ public interface MailService {
     void sendTwoFaVerificationEmail(String email, String verificationCode, int expirationTimeSeconds) throws ThingsboardException;
 
     void send(TenantId tenantId, CustomerId customerId, TbEmail tbEmail) throws ThingsboardException;
-
+    void send(TenantId tenantId, CustomerId customerId, TbEmail tbEmail, JavaMailSender javaMailSender, long timeout) throws ThingsboardException;
-    void send(TenantId tenantId, CustomerId customerId, TbEmail tbEmail, JavaMailSender javaMailSender) throws ThingsboardException;
 
     void sendApiFeatureStateEmail(ApiFeature apiFeature, ApiUsageStateValue stateValue, String email, ApiUsageStateMailMessage msg) throws ThingsboardException;
 

rule-engine/rule-engine-components/src/main/java/org/thingsboard/rule/engine/mail/TbSendEmailNode.java

@@ -88,7 +88,7 @@ public class TbSendEmailNode implements TbNode {
         if (this.config.isUseSystemSmtpSettings()) {
             ctx.getMailService(true).send(ctx.getTenantId(), msg.getCustomerId(), email);
         } else {
-            ctx.getMailService(false).send(ctx.getTenantId(), msg.getCustomerId(), email, this.mailSender);
+            ctx.getMailService(false).send(ctx.getTenantId(), msg.getCustomerId(), email, this.mailSender, config.getTimeout());
         }
     }