diff --git a/application/src/main/java/org/thingsboard/server/service/mail/DefaultMailService.java b/application/src/main/java/org/thingsboard/server/service/mail/DefaultMailService.java index 7e1a3d7fa7..aa8e019317 100644 --- a/application/src/main/java/org/thingsboard/server/service/mail/DefaultMailService.java +++ b/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,15 @@ public class DefaultMailService implements MailService { @Autowired private MailExecutorService mailExecutorService; + @Autowired + private PasswordResetExecutorService passwordResetExecutorService; + 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; @@ -100,6 +113,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 +180,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 +188,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 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 +209,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 +223,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,12 +237,12 @@ 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 public void sendResetPasswordEmailAsync(String passwordResetLink, String email) { - mailExecutorService.execute(() -> { + passwordResetExecutorService.execute(() -> { try { this.sendResetPasswordEmail(passwordResetLink, email); } catch (ThingsboardException e) { @@ -247,20 +262,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 { - sendMail(tenantId, customerId, tbEmail, javaMailSender); + public void send(TenantId tenantId, CustomerId customerId, TbEmail tbEmail, JavaMailSender javaMailSender, long timeout) throws ThingsboardException { + 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 +302,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 +323,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 +335,7 @@ public class DefaultMailService implements MailService { "expirationTimeSeconds", expirationTimeSeconds )); - sendMail(mailSender, mailFrom, email, subject, message); + sendMail(mailSender, mailFrom, email, subject, message, timeout); } @Override @@ -347,7 +362,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 +462,8 @@ public class DefaultMailService implements MailService { } } - private void sendMail(JavaMailSenderImpl mailSender, - String mailFrom, String email, - String subject, String message) throws ThingsboardException { + private void sendMail(JavaMailSenderImpl mailSender, String mailFrom, String email, + String subject, String message, long timeout) throws ThingsboardException { try { MimeMessage mimeMsg = mailSender.createMimeMessage(); MimeMessageHelper helper = new MimeMessageHelper(mimeMsg, UTF_8); @@ -457,12 +471,24 @@ 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) { + try { + mailExecutorService.submit(() -> mailSender.send(msg)).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 model) throws ThingsboardException { try { diff --git a/application/src/main/java/org/thingsboard/server/service/mail/PasswordResetExecutorService.java b/application/src/main/java/org/thingsboard/server/service/mail/PasswordResetExecutorService.java new file mode 100644 index 0000000000..ea761c144d --- /dev/null +++ b/application/src/main/java/org/thingsboard/server/service/mail/PasswordResetExecutorService.java @@ -0,0 +1,33 @@ +/** + * Copyright © 2016-2022 The Thingsboard Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.thingsboard.server.service.mail; + +import org.springframework.beans.factory.annotation.Value; +import org.springframework.stereotype.Component; +import org.thingsboard.common.util.AbstractListeningExecutor; + +@Component +public class PasswordResetExecutorService extends AbstractListeningExecutor { + + @Value("${actors.rule.mail_password_reset_thread_pool_size:10}") + private int mailExecutorThreadPoolSize; + + @Override + protected int getThreadPollSize() { + return mailExecutorThreadPoolSize; + } + +} diff --git a/application/src/main/resources/thingsboard.yml b/application/src/main/resources/thingsboard.yml index 7721fd64d1..c3f3517486 100644 --- a/application/src/main/resources/thingsboard.yml +++ b/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 password reset emails + mail_password_reset_thread_pool_size: "${ACTORS_RULE_MAIL_PASSWORD_RESET_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 diff --git a/application/src/test/java/org/thingsboard/server/controller/BaseAdminControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/BaseAdminControllerTest.java index b89133f29a..81ecce8300 100644 --- a/application/src/test/java/org/thingsboard/server/controller/BaseAdminControllerTest.java +++ b/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.springframework.test.web.servlet.result.MockMvcResultMatchers.*; +import static org.hamcrest.Matchers.containsString; +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(); @@ -100,5 +115,28 @@ public abstract class BaseAdminControllerTest extends AbstractControllerTest { doPost("/api/admin/settings/testMail", adminSettings) .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()); + } } diff --git a/common/util/src/main/java/org/thingsboard/common/util/AbstractListeningExecutor.java b/common/util/src/main/java/org/thingsboard/common/util/AbstractListeningExecutor.java index edace3cee5..f7da0b07a7 100644 --- a/common/util/src/main/java/org/thingsboard/common/util/AbstractListeningExecutor.java +++ b/common/util/src/main/java/org/thingsboard/common/util/AbstractListeningExecutor.java @@ -49,6 +49,10 @@ public abstract class AbstractListeningExecutor implements ListeningExecutor { return service.submit(task); } + public ListenableFuture executeAsync(Runnable task) { + return service.submit(task); + } + @Override public void execute(Runnable command) { service.execute(command); diff --git a/common/util/src/main/java/org/thingsboard/common/util/ListeningExecutor.java b/common/util/src/main/java/org/thingsboard/common/util/ListeningExecutor.java index 2726868f19..e63613d935 100644 --- a/common/util/src/main/java/org/thingsboard/common/util/ListeningExecutor.java +++ b/common/util/src/main/java/org/thingsboard/common/util/ListeningExecutor.java @@ -24,8 +24,19 @@ public interface ListeningExecutor extends Executor { ListenableFuture executeAsync(Callable task); + default ListenableFuture executeAsync(Runnable task) { + return executeAsync(() -> { + task.run(); + return null; + }); + } + default ListenableFuture submit(Callable task) { return executeAsync(task); } + default ListenableFuture submit(Runnable task) { + return executeAsync(task); + } + } diff --git a/rule-engine/rule-engine-api/src/main/java/org/thingsboard/rule/engine/api/MailService.java b/rule-engine/rule-engine-api/src/main/java/org/thingsboard/rule/engine/api/MailService.java index c4b47d2e46..408ee8eaaf 100644 --- a/rule-engine/rule-engine-api/src/main/java/org/thingsboard/rule/engine/api/MailService.java +++ b/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) throws ThingsboardException; + void send(TenantId tenantId, CustomerId customerId, TbEmail tbEmail, JavaMailSender javaMailSender, long timeout) throws ThingsboardException; void sendApiFeatureStateEmail(ApiFeature apiFeature, ApiUsageStateValue stateValue, String email, ApiUsageStateMailMessage msg) throws ThingsboardException; diff --git a/rule-engine/rule-engine-components/src/main/java/org/thingsboard/rule/engine/mail/TbSendEmailNode.java b/rule-engine/rule-engine-components/src/main/java/org/thingsboard/rule/engine/mail/TbSendEmailNode.java index c7d1d6c900..bf65dc1fc3 100644 --- a/rule-engine/rule-engine-components/src/main/java/org/thingsboard/rule/engine/mail/TbSendEmailNode.java +++ b/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()); } }